From 73a9b2cd5ad2a166b1aeba66f0d3dd2cc6c0ff3e Mon Sep 17 00:00:00 2001 From: Daniel Gustafsson Date: Wed, 20 Jan 2021 17:55:21 +0100 Subject: [PATCH] Refactor library specific SSL test setup into separate module This moves TLS library specific setup and teardown to its own classes under SSL::Server. If another TLS library is supported then this will make it easier to plug into the existing testing framework. This changeset was extracted from the larger patchset for supporting libnss as a TLS backend in PostgreSQL. --- src/test/ssl/t/001_ssltests.pl | 55 ++-------- src/test/ssl/t/002_scram.pl | 2 +- src/test/ssl/t/SSL/Backend/OpenSSL.pm | 103 ++++++++++++++++++ .../ssl/t/{SSLServer.pm => SSL/Server.pm} | 70 +++++++++--- 4 files changed, 167 insertions(+), 63 deletions(-) create mode 100644 src/test/ssl/t/SSL/Backend/OpenSSL.pm rename src/test/ssl/t/{SSLServer.pm => SSL/Server.pm} (81%) diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl index fd2727b568..8ae1e4e505 100644 --- a/src/test/ssl/t/001_ssltests.pl +++ b/src/test/ssl/t/001_ssltests.pl @@ -4,12 +4,10 @@ use PostgresNode; use TestLib; use Test::More; -use File::Copy; - use FindBin; use lib $FindBin::RealBin; -use SSLServer; +use SSL::Server; if ($ENV{with_openssl} eq 'yes') { @@ -32,32 +30,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 ssl/client.key to ssl/client_tmp.key etc for the rest -# of the tests. -my @keys = ( - "client", "client-revoked", - "client-der", "client-encrypted-pem", - "client-encrypted-der"); -foreach my $key (@keys) -{ - copy("ssl/${key}.key", "ssl/${key}_tmp.key") - or die - "couldn't copy ssl/${key}.key to ssl/${key}_tmp.key for permissions change: $!"; - chmod 0600, "ssl/${key}_tmp.key" - or die "failed to change permissions on ssl/${key}_tmp.key: $!"; -} - -# Also make a copy of that explicitly world-readable. We can't -# necessarily rely on the file in the source tree having those -# permissions. Add it to @keys to include it in the final clean -# up phase. -copy("ssl/client.key", "ssl/client_wrongperms_tmp.key"); -chmod 0644, "ssl/client_wrongperms_tmp.key"; -push @keys, 'client_wrongperms'; - #### Set up the server. note "setting up data directory"; @@ -72,32 +44,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'); @@ -546,7 +508,4 @@ test_connect_fails($common_connstr, "sslmode=require sslcert=ssl/client.crt", qr/SSL error/, "intermediate client certificate is missing"); # clean up -foreach my $key (@keys) -{ - unlink("ssl/${key}_tmp.key"); -} +SSL::Server::cleanup(); diff --git a/src/test/ssl/t/002_scram.pl b/src/test/ssl/t/002_scram.pl index a088f71a1a..dfbf781027 100644 --- a/src/test/ssl/t/002_scram.pl +++ b/src/test/ssl/t/002_scram.pl @@ -11,7 +11,7 @@ use File::Copy; use FindBin; use lib $FindBin::RealBin; -use SSLServer; +use SSL::Server; if ($ENV{with_openssl} ne 'yes') { 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..62b11b7632 --- /dev/null +++ b/src/test/ssl/t/SSL/Backend/OpenSSL.pm @@ -0,0 +1,103 @@ +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); + +our (@keys); + +INIT +{ + @keys = ( + "client", "client-revoked", + "client-der", "client-encrypted-pem", + "client-encrypted-der"); +} + +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 ssl/client.key to ssl/client_tmp.key etc for the rest + # of the tests. + foreach my $key (@keys) + { + copy("ssl/${key}.key", "ssl/${key}_tmp.key") + or die + "couldn't copy ssl/${key}.key to ssl/${key}_tmp.key for permissions change: $!"; + chmod 0600, "ssl/${key}_tmp.key" + or die "failed to change permissions on ssl/${key}_tmp.key: $!"; + } + + # Also make a copy of that explicitly world-readable. We can't + # necessarily rely on the file in the source tree having those + # permissions. Add it to @keys to include it in the final clean + # up phase. + copy("ssl/client.key", "ssl/client_wrongperms_tmp.key") + or die + "couldn't copy ssl/client.key to ssl/client_wrongperms_tmp.key: $!"; + chmod 0644, "ssl/client_wrongperms_tmp.key" + or die + "failed to change permissions on ssl/client_wrongperms_tmp.key: $!"; + push @keys, 'client_wrongperms'; +} + +# 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 +{ + foreach my $key (@keys) + { + unlink("ssl/${key}_tmp.key"); + } +} + +1; diff --git a/src/test/ssl/t/SSLServer.pm b/src/test/ssl/t/SSL/Server.pm similarity index 81% rename from src/test/ssl/t/SSLServer.pm rename to src/test/ssl/t/SSL/Server.pm index f5987a003e..fc56e9f47a 100644 --- a/src/test/ssl/t/SSLServer.pm +++ b/src/test/ssl/t/SSL/Server.pm @@ -23,19 +23,33 @@ # explicitly because an invalid sslcert or sslrootcert, respectively, # causes those to be ignored.) -package SSLServer; +package SSL::Server; use strict; use warnings; use PostgresNode; +use RecursiveCopy; use TestLib; use File::Basename; use File::Copy; use Test::More; +use SSL::Backend::OpenSSL qw(get_new_openssl_backend); + +our ($openssl, $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_openssl} eq 'yes') +{ + $backend = get_new_openssl_backend(); + $openssl = 1; +} use Exporter 'import'; our @EXPORT = qw( configure_test_server_for_ssl + set_server_cert switch_server_cert test_connect_fails test_connect_ok @@ -144,12 +158,15 @@ 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); + 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); + } # Stop and restart server to load new listen_addresses. $node->restart; @@ -157,26 +174,51 @@ 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 $pgdata = $node->data_dir; + $keyfile = $certfile if $keyfile eq ''; + 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 "ssl_crl_file='root+client.crl'\n"; + print $sslconf $backend->set_server_cert($certfile, $cafile, $keyfile); + 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.21.1 (Apple Git-122.3)