v53-0002-Refactor-SSL-testharness-for-multiple-library.patch

application/octet-stream

Filename: v53-0002-Refactor-SSL-testharness-for-multiple-library.patch
Type: application/octet-stream
Part: 9
Message: Re: Support for NSS as a libpq TLS backend

Patch

Format: format-patch
Series: patch v53-0002
Subject: Refactor SSL testharness for multiple library
File+
src/test/ssl/t/001_ssltests.pl 6 51
src/test/ssl/t/002_scram.pl 2 2
src/test/ssl/t/003_sslinfo.pl 1 1
src/test/ssl/t/SSL/Backend/OpenSSL.pm 110 0
src/test/ssl/t/SSL/Server.pm 77 15
From 5735872edf3f6d3e12f09c4c61c3d8511c33ef82 Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <daniel@yesql.se>
Date: Mon, 8 Feb 2021 23:52:34 +0100
Subject: [PATCH v53 02/11] Refactor SSL testharness for multiple library

The SSL testharness was fully tied to OpenSSL in the way the server was
set up and reconfigured. This refactors the SSLServer module into a SSL
library agnostic SSL/Server module which in turn use SSL/Backend/<lib>
modules for the implementation details.

No changes are done to the actual tests, this only change how setup and
teardown is performed.
---
 src/test/ssl/t/001_ssltests.pl                |  57 +--------
 src/test/ssl/t/002_scram.pl                   |   4 +-
 src/test/ssl/t/003_sslinfo.pl                 |   2 +-
 src/test/ssl/t/SSL/Backend/OpenSSL.pm         | 110 ++++++++++++++++++
 .../ssl/t/{SSLServer.pm => SSL/Server.pm}     |  92 ++++++++++++---
 5 files changed, 196 insertions(+), 69 deletions(-)
 create mode 100644 src/test/ssl/t/SSL/Backend/OpenSSL.pm
 rename src/test/ssl/t/{SSLServer.pm => SSL/Server.pm} (77%)

diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl
index b1fb15ce80..8d6b9dfbdc 100644
--- a/src/test/ssl/t/001_ssltests.pl
+++ b/src/test/ssl/t/001_ssltests.pl
@@ -8,12 +8,10 @@ use PostgreSQL::Test::Cluster;
 use PostgreSQL::Test::Utils;
 use Test::More;
 
-use File::Copy;
-
 use FindBin;
 use lib $FindBin::RealBin;
 
-use SSLServer;
+use SSL::Server;
 
 if ($ENV{with_ssl} ne 'openssl')
 {
@@ -36,39 +34,6 @@ my $SERVERHOSTCIDR = '127.0.0.1/32';
 # Allocation of base connection string shared among multiple tests.
 my $common_connstr;
 
-# The client's private key must not be world-readable, so take a copy
-# of the key stored in the code tree and update its permissions.
-#
-# This changes to using keys stored in a temporary path for the rest of
-# the tests. To get the full path for inclusion in connection strings, the
-# %key hash can be interrogated.
-my $cert_tempdir = PostgreSQL::Test::Utils::tempdir();
-my %key;
-my @keys = (
-	"client.key",               "client-revoked.key",
-	"client-der.key",           "client-encrypted-pem.key",
-	"client-encrypted-der.key", "client-dn.key");
-foreach my $keyfile (@keys)
-{
-	copy("ssl/$keyfile", "$cert_tempdir/$keyfile")
-	  or die
-	  "couldn't copy ssl/$keyfile to $cert_tempdir/$keyfile for permissions change: $!";
-	chmod 0600, "$cert_tempdir/$keyfile"
-	  or die "failed to change permissions on $cert_tempdir/$keyfile: $!";
-	$key{$keyfile} = PostgreSQL::Test::Utils::perl2host("$cert_tempdir/$keyfile");
-	$key{$keyfile} =~ s!\\!/!g if $PostgreSQL::Test::Utils::windows_os;
-}
-
-# Also make a copy of that explicitly world-readable.  We can't
-# necessarily rely on the file in the source tree having those
-# permissions.
-copy("ssl/client.key", "$cert_tempdir/client_wrongperms.key")
-  or die
-  "couldn't copy ssl/client_key to $cert_tempdir/client_wrongperms.key for permission change: $!";
-chmod 0644, "$cert_tempdir/client_wrongperms.key"
-  or die "failed to change permissions on $cert_tempdir/client_wrongperms.key: $!";
-$key{'client_wrongperms.key'} = PostgreSQL::Test::Utils::perl2host("$cert_tempdir/client_wrongperms.key");
-$key{'client_wrongperms.key'} =~ s!\\!/!g if $PostgreSQL::Test::Utils::windows_os;
 #### Set up the server.
 
 note "setting up data directory";
@@ -83,32 +48,22 @@ $node->start;
 
 # Run this before we lock down access below.
 my $result = $node->safe_psql('postgres', "SHOW ssl_library");
-is($result, 'OpenSSL', 'ssl_library parameter');
+is($result, SSL::Server::ssl_library(), 'ssl_library parameter');
 
 configure_test_server_for_ssl($node, $SERVERHOSTADDR, $SERVERHOSTCIDR,
 	'trust');
 
 note "testing password-protected keys";
 
-open my $sslconf, '>', $node->data_dir . "/sslconfig.conf";
-print $sslconf "ssl=on\n";
-print $sslconf "ssl_cert_file='server-cn-only.crt'\n";
-print $sslconf "ssl_key_file='server-password.key'\n";
-print $sslconf "ssl_passphrase_command='echo wrongpassword'\n";
-close $sslconf;
-
+set_server_cert($node, 'server-cn-only', 'root+client_ca',
+				   'server-password', 'echo wrongpassword');
 command_fails(
 	[ 'pg_ctl', '-D', $node->data_dir, '-l', $node->logfile, 'restart' ],
 	'restart fails with password-protected key file with wrong password');
 $node->_update_pid(0);
 
-open $sslconf, '>', $node->data_dir . "/sslconfig.conf";
-print $sslconf "ssl=on\n";
-print $sslconf "ssl_cert_file='server-cn-only.crt'\n";
-print $sslconf "ssl_key_file='server-password.key'\n";
-print $sslconf "ssl_passphrase_command='echo secret1'\n";
-close $sslconf;
-
+set_server_cert($node, 'server-cn-only', 'root+client_ca',
+				'server-password', 'echo secret1');
 command_ok(
 	[ 'pg_ctl', '-D', $node->data_dir, '-l', $node->logfile, 'restart' ],
 	'restart succeeds with password-protected key file');
diff --git a/src/test/ssl/t/002_scram.pl b/src/test/ssl/t/002_scram.pl
index 86312be88c..56c5f88d5f 100644
--- a/src/test/ssl/t/002_scram.pl
+++ b/src/test/ssl/t/002_scram.pl
@@ -14,11 +14,11 @@ use File::Copy;
 use FindBin;
 use lib $FindBin::RealBin;
 
-use SSLServer;
+use SSL::Server;
 
 if ($ENV{with_ssl} ne 'openssl')
 {
-	plan skip_all => 'OpenSSL not supported by this build';
+	plan skip_all => 'SSL not supported by this build';
 }
 
 # This is the hostname used to connect to the server.
diff --git a/src/test/ssl/t/003_sslinfo.pl b/src/test/ssl/t/003_sslinfo.pl
index 8c760b39db..6c733c8a75 100644
--- a/src/test/ssl/t/003_sslinfo.pl
+++ b/src/test/ssl/t/003_sslinfo.pl
@@ -12,7 +12,7 @@ use File::Copy;
 use FindBin;
 use lib $FindBin::RealBin;
 
-use SSLServer;
+use SSL::Server;
 
 if ($ENV{with_ssl} ne 'openssl')
 {
diff --git a/src/test/ssl/t/SSL/Backend/OpenSSL.pm b/src/test/ssl/t/SSL/Backend/OpenSSL.pm
new file mode 100644
index 0000000000..991953bf2c
--- /dev/null
+++ b/src/test/ssl/t/SSL/Backend/OpenSSL.pm
@@ -0,0 +1,110 @@
+package SSL::Backend::OpenSSL;
+
+use strict;
+use warnings;
+use Exporter;
+use File::Copy;
+
+our @ISA       = qw(Exporter);
+our @EXPORT_OK = qw(get_new_openssl_backend get_openssl_key);
+
+our (%key);
+
+INIT
+{
+}
+
+sub new
+{
+	my ($class) = @_;
+
+	my $self = { _library => 'OpenSSL' };
+
+	bless $self, $class;
+
+	return $self;
+}
+
+sub get_new_openssl_backend
+{
+	my $class = 'SSL::Backend::OpenSSL';
+
+	my $backend = $class->new();
+
+	return $backend;
+}
+
+sub init
+{
+	# The client's private key must not be world-readable, so take a copy
+	# of the key stored in the code tree and update its permissions.
+	#
+	# This changes to using keys stored in a temporary path for the rest of
+	# the tests. To get the full path for inclusion in connection strings, the
+	# %key hash can be interrogated.
+	my $cert_tempdir = PostgreSQL::Test::Utils::tempdir();
+	my @keys = (
+		"client.key",               "client-revoked.key",
+		"client-der.key",           "client-encrypted-pem.key",
+		"client-encrypted-der.key", "client-dn.key");
+	foreach my $keyfile (@keys)
+	{
+		copy("ssl/$keyfile", "$cert_tempdir/$keyfile")
+		  or die
+		  "couldn't copy ssl/$keyfile to $cert_tempdir/$keyfile for permissions change: $!";
+		chmod 0600, "$cert_tempdir/$keyfile"
+		  or die "failed to change permissions on $cert_tempdir/$keyfile: $!";
+		$key{$keyfile} = PostgreSQL::Test::Utils::perl2host("$cert_tempdir/$keyfile");
+		$key{$keyfile} =~ s!\\!/!g if $PostgreSQL::Test::Utils::windows_os;
+	}
+
+	# Also make a copy of that explicitly world-readable.  We can't
+	# necessarily rely on the file in the source tree having those
+	# permissions.
+	copy("ssl/client.key", "$cert_tempdir/client_wrongperms.key")
+	  or die
+	  "couldn't copy ssl/client_key to $cert_tempdir/client_wrongperms.key for permission change: $!";
+	chmod 0644, "$cert_tempdir/client_wrongperms.key"
+	  or die "failed to change permissions on $cert_tempdir/client_wrongperms.key: $!";
+	$key{'client_wrongperms.key'} = PostgreSQL::Test::Utils::perl2host("$cert_tempdir/client_wrongperms.key");
+	$key{'client_wrongperms.key'} =~ s!\\!/!g if $PostgreSQL::Test::Utils::windows_os;
+}
+
+sub get_openssl_key
+{
+	my $self = $_[0];
+	my $keyfile = $_[1];
+
+	return $key{$keyfile};
+}
+
+# Change the configuration to use given server cert file, and reload
+# the server so that the configuration takes effect.
+sub set_server_cert
+{
+	my $self     = $_[0];
+	my $certfile = $_[1];
+	my $cafile   = $_[2] || "root+client_ca";
+	my $keyfile  = $_[3] || $certfile;
+
+	my $sslconf =
+	    "ssl_ca_file='$cafile.crt'\n"
+	  . "ssl_cert_file='$certfile.crt'\n"
+	  . "ssl_key_file='$keyfile.key'\n"
+	  . "ssl_crl_file='root+client.crl'\n";
+
+	return $sslconf;
+}
+
+sub get_library
+{
+	my ($self) = @_;
+
+	return $self->{_library};
+}
+
+sub cleanup
+{
+}
+
+1;
diff --git a/src/test/ssl/t/SSLServer.pm b/src/test/ssl/t/SSL/Server.pm
similarity index 77%
rename from src/test/ssl/t/SSLServer.pm
rename to src/test/ssl/t/SSL/Server.pm
index c85c6fd997..9275876250 100644
--- a/src/test/ssl/t/SSLServer.pm
+++ b/src/test/ssl/t/SSL/Server.pm
@@ -26,20 +26,41 @@
 # explicitly because an invalid sslcert or sslrootcert, respectively,
 # causes those to be ignored.)
 
-package SSLServer;
+package SSL::Server;
 
 use strict;
 use warnings;
 use PostgreSQL::Test::Cluster;
 use PostgreSQL::Test::Utils;
+use PostgreSQL::Test::RecursiveCopy;
 use File::Basename;
 use File::Copy;
 use Test::More;
+use SSL::Backend::OpenSSL qw(get_new_openssl_backend get_openssl_key);
+use SSL::Backend::NSS qw(get_new_nss_backend);
+
+our ($openssl, $nss, $backend);
+
+# The TLS backend which the server is using should be mostly transparent for
+# the user, apart from individual configuration settings, so keep the backend
+# specific things abstracted behind SSL::Server.
+if ($ENV{with_ssl} eq 'openssl')
+{
+	$backend = get_new_openssl_backend();
+	$openssl = 1;
+}
+elsif ($ENV{with_ssl} eq 'nss')
+{
+	$backend = get_new_nss_backend();
+	$nss     = 1;
+}
 
 use Exporter 'import';
 our @EXPORT = qw(
   configure_test_server_for_ssl
+  set_server_cert
   switch_server_cert
+  key
 );
 
 # Copy a set of files, taking into account wildcards
@@ -58,6 +79,14 @@ sub copy_files
 	return;
 }
 
+sub key
+{
+	my ($node, $keyfile) = @_;
+
+	return get_openssl_key($keyfile) if (defined($openssl);
+	return get_nss_key($keyfile) if (defined($nss));
+}
+
 # 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
@@ -126,14 +155,21 @@ sub configure_test_server_for_ssl
 	close $sslconf;
 
 	# Copy all server certificates and keys, and client root cert, to the data dir
-	copy_files("ssl/server-*.crt", $pgdata);
-	copy_files("ssl/server-*.key", $pgdata);
-	chmod(0600, glob "$pgdata/server-*.key") or die $!;
-	copy_files("ssl/root+client_ca.crt", $pgdata);
-	copy_files("ssl/root_ca.crt",        $pgdata);
-	copy_files("ssl/root+client.crl",    $pgdata);
-	mkdir("$pgdata/root+client-crldir");
-	copy_files("ssl/root+client-crldir/*", "$pgdata/root+client-crldir/");
+	if (defined($openssl))
+	{
+		copy_files("ssl/server-*.crt", $pgdata);
+		copy_files("ssl/server-*.key", $pgdata);
+		chmod(0600, glob "$pgdata/server-*.key") or die $!;
+		copy_files("ssl/root+client_ca.crt", $pgdata);
+		copy_files("ssl/root_ca.crt",        $pgdata);
+		copy_files("ssl/root+client.crl",    $pgdata);
+		mkdir("$pgdata/root+client-crldir");
+		copy_files("ssl/root+client-crldir/*", "$pgdata/root+client-crldir/");
+	}
+	elsif (defined($nss))
+	{
+		RecursiveCopy::copypath("ssl/nss", $pgdata . "/nss") if -e "ssl/nss";
+	}
 
 	# Stop and restart server to load new listen_addresses.
 	$node->restart;
@@ -141,20 +177,36 @@ sub configure_test_server_for_ssl
 	# Change pg_hba after restart because hostssl requires ssl=on
 	configure_hba_for_ssl($node, $servercidr, $authmethod);
 
+	# Finally, perform backend specific configuration
+	$backend->init();
+
 	return;
 }
 
-# Change the configuration to use given server cert file, and reload
-# the server so that the configuration takes effect.
-sub switch_server_cert
+sub ssl_library
+{
+	return $backend->get_library();
+}
+
+sub cleanup
+{
+	$backend->cleanup();
+}
+
+# Change the configuration to use given server cert file,
+sub set_server_cert
 {
 	my $node     = $_[0];
 	my $certfile = $_[1];
 	my $cafile   = $_[2] || "root+client_ca";
+	my $keyfile  = $_[3] || '';
+	my $pwcmd    = $_[4] || '';
 	my $crlfile  = "root+client.crl";
 	my $crldir;
 	my $pgdata = $node->data_dir;
 
+	$keyfile = $certfile if $keyfile eq '';
+
 	# defaults to use crl file
 	if (defined $_[3] || defined $_[4])
 	{
@@ -164,13 +216,23 @@ sub switch_server_cert
 
 	open my $sslconf, '>', "$pgdata/sslconfig.conf";
 	print $sslconf "ssl=on\n";
-	print $sslconf "ssl_ca_file='$cafile.crt'\n";
-	print $sslconf "ssl_cert_file='$certfile.crt'\n";
-	print $sslconf "ssl_key_file='$certfile.key'\n";
+	print $sslconf $backend->set_server_cert($certfile, $cafile, $keyfile);
 	print $sslconf "ssl_crl_file='$crlfile'\n" if defined $crlfile;
 	print $sslconf "ssl_crl_dir='$crldir'\n"   if defined $crldir;
+	print $sslconf "ssl_passphrase_command='$pwcmd'\n"
+	  unless $pwcmd eq '';
 	close $sslconf;
+	return;
+}
 
+# Change the configuration to use given server cert file, and reload
+# the server so that the configuration takes effect.
+# Takes the same arguments as set_server_cert, which it calls to do that
+# piece of the work.
+sub switch_server_cert
+{
+	my $node = $_[0];
+	set_server_cert(@_);
 	$node->restart;
 	return;
 }
-- 
2.24.3 (Apple Git-128)