pg.ddx.io  pgsql-committers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Michael Paquier <michael@paquier.xyz>
Cc: pgsql-committers@lists.postgresql.org
Subject: Re: pgsql: Remove support for OpenSSL 0.9.8 and 1.0.0
Date: Mon, 06 Jan 2020 19:14:49 -0500
Message-ID: <1899.1578356089@sss.pgh.pa.us> (raw)
In-Reply-To: <31057.1578341305@sss.pgh.pa.us>
References: <E1ioJS0-0005f9-7G@gemulon.postgresql.org>
	<20200106045858.GJ3598@paquier.xyz>
	<18698.1578287190@sss.pgh.pa.us>
	<31057.1578341305@sss.pgh.pa.us>

I wrote:
> * gaur fell over in the ssl test [2].  I had not asked it to run that
> test before, so this may well be a pre-existing issue not something
> new with the version change.  It looks like something in that test
> is assuming that we have IPv6 support, which maybe it shouldn't be,
> even in 2020.

Yeah ... SSLServer.pm has code like this:

	print $hba
	  "hostssl trustdb         all             $serverhost/32            $authmethod\n";
	print $hba
	  "hostssl trustdb         all             ::1/128                 $authmethod\n";

This seems to me to be approximately the worst of all possible worlds.
Not only will this not work on a machine where IPv6 isn't working, but
it's not possible to actually use IPv6 if you want to, because the netmask
for $serverhost is hard-wired.  Furthermore, because the client side of
the tests always connects to $serverhost, the IPv6 entries are useless.
All they're doing is letting in connections we don't want, contrary to
the clear comment just above this.

I propose the attached, which removes the unnecessary entries
and puts full control of the IPv4/IPv6 decision in one place
(well, two places).  The test will still always connect over IPv4,
but at least there's now a clear route to changing that if
someone wants to.

			regards, tom lane

Attachments:

  [text/x-diff] rationalize-address-usage-in-ssl-tests.patch (4.7K, ../1899.1578356089@sss.pgh.pa.us/2-rationalize-address-usage-in-ssl-tests.patch)
  download | inline diff:
diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl
index 93e2b79..83fcd5e 100644
--- a/src/test/ssl/t/001_ssltests.pl
+++ b/src/test/ssl/t/001_ssltests.pl
@@ -26,6 +26,8 @@ else
 # hostname, because the server certificate is always for the domain
 # postgresql-ssl-regression.test.
 my $SERVERHOSTADDR = '127.0.0.1';
+# This is the pattern to use in pg_hba.conf to match incoming connections.
+my $SERVERHOSTCIDR = '127.0.0.1/32';
 
 # Allocation of base connection string shared among multiple tests.
 my $common_connstr;
@@ -66,7 +68,8 @@ $node->start;
 my $result = $node->safe_psql('postgres', "SHOW ssl_library");
 is($result, 'OpenSSL', 'ssl_library parameter');
 
-configure_test_server_for_ssl($node, $SERVERHOSTADDR, 'trust');
+configure_test_server_for_ssl($node, $SERVERHOSTADDR, $SERVERHOSTCIDR,
+	'trust');
 
 note "testing password-protected keys";
 
diff --git a/src/test/ssl/t/002_scram.pl b/src/test/ssl/t/002_scram.pl
index c08aa19..a6642f8 100644
--- a/src/test/ssl/t/002_scram.pl
+++ b/src/test/ssl/t/002_scram.pl
@@ -20,6 +20,8 @@ if ($ENV{with_openssl} ne 'yes')
 
 # This is the hostname used to connect to the server.
 my $SERVERHOSTADDR = '127.0.0.1';
+# This is the pattern to use in pg_hba.conf to match incoming connections.
+my $SERVERHOSTCIDR = '127.0.0.1/32';
 
 # Determine whether build supports tls-server-end-point.
 my $supports_tls_server_end_point =
@@ -43,8 +45,8 @@ $ENV{PGPORT} = $node->port;
 $node->start;
 
 # Configure server for SSL connections, with password handling.
-configure_test_server_for_ssl($node, $SERVERHOSTADDR, "scram-sha-256",
-	"pass", "scram-sha-256");
+configure_test_server_for_ssl($node, $SERVERHOSTADDR, $SERVERHOSTCIDR,
+	"scram-sha-256", "pass", "scram-sha-256");
 switch_server_cert($node, 'server-cn-only');
 $ENV{PGPASSWORD} = "pass";
 $common_connstr =
diff --git a/src/test/ssl/t/SSLServer.pm b/src/test/ssl/t/SSLServer.pm
index 005955a..1e392b8 100644
--- a/src/test/ssl/t/SSLServer.pm
+++ b/src/test/ssl/t/SSLServer.pm
@@ -94,9 +94,12 @@ sub copy_files
 	return;
 }
 
+# serverhost: what to put in listen_addresses, e.g. '127.0.0.1'
+# servercidr: what to put in pg_hba.conf, e.g. '127.0.0.1/32'
 sub configure_test_server_for_ssl
 {
-	my ($node, $serverhost, $authmethod, $password, $password_enc) = @_;
+	my ($node, $serverhost, $servercidr, $authmethod, $password,
+		$password_enc) = @_;
 
 	my $pgdata = $node->data_dir;
 
@@ -153,7 +156,7 @@ sub configure_test_server_for_ssl
 	$node->restart;
 
 	# Change pg_hba after restart because hostssl requires ssl=on
-	configure_hba_for_ssl($node, $serverhost, $authmethod);
+	configure_hba_for_ssl($node, $servercidr, $authmethod);
 
 	return;
 }
@@ -181,10 +184,10 @@ sub switch_server_cert
 
 sub configure_hba_for_ssl
 {
-	my ($node, $serverhost, $authmethod) = @_;
+	my ($node, $servercidr, $authmethod) = @_;
 	my $pgdata = $node->data_dir;
 
-	# Only accept SSL connections from localhost. Our tests don't depend on this
+	# Only accept SSL connections from $servercidr. Our tests don't depend on this
 	# but seems best to keep it as narrow as possible for security reasons.
 	#
 	# When connecting to certdb, also check the client certificate.
@@ -192,21 +195,17 @@ sub configure_hba_for_ssl
 	print $hba
 	  "# TYPE  DATABASE        USER            ADDRESS                 METHOD             OPTIONS\n";
 	print $hba
-	  "hostssl trustdb         md5testuser     $serverhost/32            md5\n";
+	  "hostssl trustdb         md5testuser     $servercidr            md5\n";
 	print $hba
-	  "hostssl trustdb         all             $serverhost/32            $authmethod\n";
+	  "hostssl trustdb         all             $servercidr            $authmethod\n";
 	print $hba
-	  "hostssl trustdb         all             ::1/128                 $authmethod\n";
+	  "hostssl verifydb        ssltestuser     $servercidr            $authmethod        clientcert=verify-full\n";
 	print $hba
-	  "hostssl verifydb        ssltestuser     $serverhost/32          $authmethod        clientcert=verify-full\n";
+	  "hostssl verifydb        anotheruser     $servercidr            $authmethod        clientcert=verify-full\n";
 	print $hba
-	  "hostssl verifydb        anotheruser     $serverhost/32          $authmethod        clientcert=verify-full\n";
+	  "hostssl verifydb        yetanotheruser  $servercidr            $authmethod        clientcert=verify-ca\n";
 	print $hba
-	  "hostssl verifydb        yetanotheruser  $serverhost/32          $authmethod        clientcert=verify-ca\n";
-	print $hba
-	  "hostssl certdb          all             $serverhost/32            cert\n";
-	print $hba
-	  "hostssl certdb          all             ::1/128                 cert\n";
+	  "hostssl certdb          all             $servercidr            cert\n";
 	close $hba;
 	return;
 }

view thread (12+ messages)  latest in thread

Message-ID: <1899.1578356089@sss.pgh.pa.us>
Permalink:  ../1899.1578356089@sss.pgh.pa.us/
Also on:    postgresql.org/message-id/1899.1578356089@sss.pgh.pa.us

 ·  · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-committers@postgresql.org
  Cc: tgl@sss.pgh.pa.us, michael@paquier.xyz, pgsql-committers@lists.postgresql.org
  Subject: Re: pgsql: Remove support for OpenSSL 0.9.8 and 1.0.0
  In-Reply-To: <1899.1578356089@sss.pgh.pa.us>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox