On Fri, 2021-04-02 at 13:45 +0900, Michael Paquier wrote: > Attached is what I have come up with as the first building piece, > which is basically a combination of 0001 and 0002, except that I > modified things so as the number of arguments remains minimal for all > the routines. This avoids the manipulation of the list of parameters > passed down to PostgresNode::psql. The arguments for the optional > query, the expected stdout and stderr are part of the parameter set > (0001 was not doing that).
I made a few changes, highlighted in the since-v18 diff: > + # The result is assumed to match "true", or "t", here. > + $node->connect_ok($connstr, $test_name, sql => $query, > + expected_stdout => qr/t/); I've anchored this as qr/^t$/ so we don't accidentally match a stray "t" in some larger string. > - is($res, 0, $test_name); > - like($stdoutres, $expected, $test_name); > - is($stderrres, "", $test_name); > + my ($stdoutres, $stderrres); > + > + $node->connect_ok($connstr, $test_name, $query, $expected); $query and $expected need to be given as named parameters. We also lost the stderr check from the previous version of the test, so I added expected_stderr to connect_ok(). > @@ -446,14 +446,14 @@ TODO: > # correct client cert in encrypted PEM with empty password > $node->connect_fails( > "$common_connstr user=ssltestuser sslcert=ssl/client.crt > sslkey=ssl/client-encrypted-pem_tmp.key sslpassword=''", > - qr!\Qprivate key file "ssl/client-encrypted-pem_tmp.key": > processing error\E!, > + expected_stderr => qr!\Qprivate key file > "ssl/client-encrypted-pem_tmp.key": processing error\E!, > "certificate authorization fails with correct client cert and > empty password in encrypted PEM format" > ); These tests don't run yet inside the TODO block, but I've put the expected_stderr parameter at the end of the list for them. > For the main patch, this will need to be > extended with two more parameters in each routine: log_like and > log_unlike to match for the log patterns, handled as arrays of > regexes. That's what 0003 is basically doing already. Rebased on top of your patch as v19, attached. (v17 disappeared into the ether somewhere, I think. :D) Now that it's easy to add log_like to existing tests, I fleshed out the LDAP tests with a few more cases. They don't add code coverage, but they pin the desired behavior for a few more types of LDAP auth. --Jacob
commit d80742fbe2124687591e76dc069d7cab6a2cecc8 Author: Jacob Champion <pchamp...@vmware.com> Date: Fri Apr 2 10:31:40 2021 -0700 squash! Plug more TAP test suites with new PostgresNode's new routines diff --git a/src/test/kerberos/t/001_auth.pl b/src/test/kerberos/t/001_auth.pl index 31fdc49b86..2395664145 100644 --- a/src/test/kerberos/t/001_auth.pl +++ b/src/test/kerberos/t/001_auth.pl @@ -20,7 +20,7 @@ use Time::HiRes qw(usleep); if ($ENV{with_gssapi} eq 'yes') { - plan tests => 30; + plan tests => 34; } else { @@ -192,7 +192,7 @@ sub test_access { # The result is assumed to match "true", or "t", here. $node->connect_ok($connstr, $test_name, sql => $query, - expected_stdout => qr/t/); + expected_stdout => qr/^t$/); } else { @@ -223,9 +223,9 @@ sub test_query my $connstr = $node->connstr('postgres') . " user=$role host=$host hostaddr=$hostaddr $gssencmode"; - my ($stdoutres, $stderrres); - - $node->connect_ok($connstr, $test_name, $query, $expected); + $node->connect_ok($connstr, $test_name, sql => $query, + expected_stdout => $expected, + expected_stderr => qr/^$/); return; } diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm index ba1407e761..f29c877c79 100644 --- a/src/test/perl/PostgresNode.pm +++ b/src/test/perl/PostgresNode.pm @@ -1876,6 +1876,10 @@ instead of the default. If this regular expression is set, matches it with the output generated. +=item expected_stderr => B<value> + +If this regular expression is set, matches it with the stderr generated. + =back =cut @@ -1906,7 +1910,11 @@ sub connect_ok if (defined($params{expected_stdout})) { - like($stdout, $params{expected_stdout}, "$test_name: matches"); + like($stdout, $params{expected_stdout}, "$test_name: stdout matches"); + } + if (defined($params{expected_stderr})) + { + like($stderr, $params{expected_stderr}, "$test_name: stderr matches"); } } diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl index 21beef01aa..6df558fca7 100644 --- a/src/test/ssl/t/001_ssltests.pl +++ b/src/test/ssl/t/001_ssltests.pl @@ -446,15 +446,15 @@ TODO: # correct client cert in encrypted PEM with empty password $node->connect_fails( "$common_connstr user=ssltestuser sslcert=ssl/client.crt sslkey=ssl/client-encrypted-pem_tmp.key sslpassword=''", - expected_stderr => qr!\Qprivate key file "ssl/client-encrypted-pem_tmp.key": processing error\E!, - "certificate authorization fails with correct client cert and empty password in encrypted PEM format" + "certificate authorization fails with correct client cert and empty password in encrypted PEM format", + expected_stderr => qr!\Qprivate key file "ssl/client-encrypted-pem_tmp.key": processing error\E! ); # correct client cert in encrypted PEM with no password $node->connect_fails( "$common_connstr user=ssltestuser sslcert=ssl/client.crt sslkey=ssl/client-encrypted-pem_tmp.key", - expected_stderr => qr!\Qprivate key file "ssl/client-encrypted-pem_tmp.key": processing error\E!, - "certificate authorization fails with correct client cert and no password in encrypted PEM format" + "certificate authorization fails with correct client cert and no password in encrypted PEM format", + expected_stderr => qr!\Qprivate key file "ssl/client-encrypted-pem_tmp.key": processing error\E! ); }
From 0bb565c41fdd9b1fcafc59775baa39cdd43eb4e7 Mon Sep 17 00:00:00 2001 From: Michael Paquier <mich...@paquier.xyz> Date: Fri, 2 Apr 2021 13:44:39 +0900 Subject: [PATCH v19 1/2] Plug more TAP test suites with new PostgresNode's new routines This switches the kerberos, ldap and authentication to use connect_ok() and connect_fails() recently introduced in PostgresNode.pm. The SSL tests need some extra juggling to accomodate with the changes. --- src/test/authentication/t/001_password.pl | 19 ++-- src/test/authentication/t/002_saslprep.pl | 18 +++- src/test/kerberos/t/001_auth.pl | 45 +++----- src/test/ldap/t/001_auth.pl | 17 +-- src/test/perl/PostgresNode.pm | 75 ++++++++++--- src/test/ssl/t/001_ssltests.pl | 122 +++++++++++----------- src/test/ssl/t/002_scram.pl | 16 +-- 7 files changed, 184 insertions(+), 128 deletions(-) diff --git a/src/test/authentication/t/001_password.pl b/src/test/authentication/t/001_password.pl index 36a616d7c7..4d5e304de1 100644 --- a/src/test/authentication/t/001_password.pl +++ b/src/test/authentication/t/001_password.pl @@ -46,12 +46,19 @@ sub test_role $status_string = 'success' if ($expected_res eq 0); - local $Test::Builder::Level = $Test::Builder::Level + 1; - - my $res = $node->psql('postgres', undef, extra_params => [ '-U', $role, '-w' ]); - is($res, $expected_res, - "authentication $status_string for method $method, role $role"); - return; + my $connstr = "user=$role"; + my $testname = "authentication $status_string for method $method, role $role"; + + if ($expected_res eq 0) + { + $node->connect_ok($connstr, $testname); + } + else + { + # No match pattern checks are done here on errors, only the + # status code. + $node->connect_fails($connstr, $testname); + } } # Initialize primary node diff --git a/src/test/authentication/t/002_saslprep.pl b/src/test/authentication/t/002_saslprep.pl index 0aaab090ec..530344a5d6 100644 --- a/src/test/authentication/t/002_saslprep.pl +++ b/src/test/authentication/t/002_saslprep.pl @@ -41,12 +41,20 @@ sub test_login $status_string = 'success' if ($expected_res eq 0); + my $connstr = "user=$role"; + my $testname = "authentication $status_string for role $role with password $password"; + $ENV{"PGPASSWORD"} = $password; - my $res = $node->psql('postgres', undef, extra_params => [ '-U', $role ]); - is($res, $expected_res, - "authentication $status_string for role $role with password $password" - ); - return; + if ($expected_res eq 0) + { + $node->connect_ok($connstr, $testname); + } + else + { + # No match pattern checks are done here on errors, only the + # status code + $node->connect_fails($connstr, $testname); + } } # Initialize primary node. Force UTF-8 encoding, so that we can use non-ASCII diff --git a/src/test/kerberos/t/001_auth.pl b/src/test/kerberos/t/001_auth.pl index 38e9ef7b1f..2395664145 100644 --- a/src/test/kerberos/t/001_auth.pl +++ b/src/test/kerberos/t/001_auth.pl @@ -20,7 +20,7 @@ use Time::HiRes qw(usleep); if ($ENV{with_gssapi} eq 'yes') { - plan tests => 26; + plan tests => 34; } else { @@ -185,25 +185,18 @@ sub test_access my ($node, $role, $query, $expected_res, $gssencmode, $test_name, $expect_log_msg) = @_; # need to connect over TCP/IP for Kerberos - my ($res, $stdoutres, $stderrres) = $node->psql( - 'postgres', - "$query", - extra_params => [ - '-XAtd', - $node->connstr('postgres') - . " host=$host hostaddr=$hostaddr $gssencmode", - '-U', - $role - ]); - - # If we get a query result back, it should be true. - if ($res == $expected_res and $res eq 0) + my $connstr = $node->connstr('postgres') . + " user=$role host=$host hostaddr=$hostaddr $gssencmode"; + + if ($expected_res eq 0) { - is($stdoutres, "t", $test_name); + # The result is assumed to match "true", or "t", here. + $node->connect_ok($connstr, $test_name, sql => $query, + expected_stdout => qr/^t$/); } else { - is($res, $expected_res, $test_name); + $node->connect_fails($connstr, $test_name); } # Verify specified log message is logged in the log file. @@ -227,20 +220,12 @@ sub test_query my ($node, $role, $query, $expected, $gssencmode, $test_name) = @_; # need to connect over TCP/IP for Kerberos - my ($res, $stdoutres, $stderrres) = $node->psql( - 'postgres', - "$query", - extra_params => [ - '-XAtd', - $node->connstr('postgres') - . " host=$host hostaddr=$hostaddr $gssencmode", - '-U', - $role - ]); - - is($res, 0, $test_name); - like($stdoutres, $expected, $test_name); - is($stderrres, "", $test_name); + my $connstr = $node->connstr('postgres') . + " user=$role host=$host hostaddr=$hostaddr $gssencmode"; + + $node->connect_ok($connstr, $test_name, sql => $query, + expected_stdout => $expected, + expected_stderr => qr/^$/); return; } diff --git a/src/test/ldap/t/001_auth.pl b/src/test/ldap/t/001_auth.pl index 3bc7672451..b08ba6b281 100644 --- a/src/test/ldap/t/001_auth.pl +++ b/src/test/ldap/t/001_auth.pl @@ -163,12 +163,17 @@ note "running tests"; sub test_access { my ($node, $role, $expected_res, $test_name) = @_; - - my $res = - $node->psql('postgres', undef, - extra_params => [ '-U', $role, '-c', 'SELECT 1' ]); - is($res, $expected_res, $test_name); - return; + my $connstr = "user=$role"; + + if ($expected_res eq 0) + { + $node->connect_ok($connstr, $test_name); + } + else + { + # Currently, we don't check the error message, just the code. + $node->connect_fails($connstr, $test_name); + } } note "simple bind"; diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm index d6e10544bb..f29c877c79 100644 --- a/src/test/perl/PostgresNode.pm +++ b/src/test/perl/PostgresNode.pm @@ -1860,52 +1860,103 @@ sub interactive_psql =pod -=item $node->connect_ok($connstr, $test_name) +=item $node->connect_ok($connstr, $test_name, %params) Attempt a connection with a custom connection string. This is expected to succeed. +=over + +=item sql => B<value> + +If this parameter is set, this query is used for the connection attempt +instead of the default. + +=item expected_stdout => B<value> + +If this regular expression is set, matches it with the output generated. + +=item expected_stderr => B<value> + +If this regular expression is set, matches it with the stderr generated. + +=back + =cut sub connect_ok { local $Test::Builder::Level = $Test::Builder::Level + 1; - my ($self, $connstr, $test_name) = @_; - my ($ret, $stdout, $stderr) = $self->psql( + my ($self, $connstr, $test_name, %params) = @_; + + my $sql; + if (defined($params{sql})) + { + $sql = $params{sql}; + } + else + { + $sql = "SELECT \$\$connected with $connstr\$\$"; + } + + my ($ret, $stdout, $stderr) = $self->psql( 'postgres', - "SELECT \$\$connected with $connstr\$\$", + $sql, + extra_params => [ '-w'], connstr => "$connstr", on_error_stop => 0); - ok($ret == 0, $test_name); + is($ret, 0, $test_name); + + if (defined($params{expected_stdout})) + { + like($stdout, $params{expected_stdout}, "$test_name: stdout matches"); + } + if (defined($params{expected_stderr})) + { + like($stderr, $params{expected_stderr}, "$test_name: stderr matches"); + } } =pod -=item $node->connect_fails($connstr, $expected_stderr, $test_name) +=item $node->connect_fails($connstr, $test_name, %params) Attempt a connection with a custom connection string. This is expected -to fail with a message that matches the regular expression -$expected_stderr. +to fail. The generated error message can be checked by specifying the +regular expression $expected_stderr for a match. + +=over + +=item expected_stderr => B<value> + +If this regular expression is set, matches it with the output generated. + +=back =cut sub connect_fails { local $Test::Builder::Level = $Test::Builder::Level + 1; - my ($self, $connstr, $expected_stderr, $test_name) = @_; + my ($self, $connstr, $test_name, %params) = @_; my ($ret, $stdout, $stderr) = $self->psql( 'postgres', "SELECT \$\$connected with $connstr\$\$", + extra_params => [ '-w'], connstr => "$connstr"); - ok($ret != 0, $test_name); - like($stderr, $expected_stderr, "$test_name: matches"); + isnt($ret, 0, $test_name); + + if (defined($params{expected_stderr})) + { + like($stderr, $params{expected_stderr}, "$test_name: matches"); + } } =pod -=item $node->poll_query_until($dbname, $query [, $expected ]) +=item $node->poll_query_until($dbname, $query [, $expected ) Run B<$query> repeatedly, until it returns the B<$expected> result ('t', or SQL boolean true, by default). diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl index b1a63f279c..6df558fca7 100644 --- a/src/test/ssl/t/001_ssltests.pl +++ b/src/test/ssl/t/001_ssltests.pl @@ -137,8 +137,8 @@ $common_connstr = # The server should not accept non-SSL connections. $node->connect_fails( "$common_connstr sslmode=disable", - qr/\Qno pg_hba.conf entry\E/, - "server doesn't accept non-SSL connections"); + "server doesn't accept non-SSL connections", + expected_stderr => qr/\Qno pg_hba.conf entry\E/); # Try without a root cert. In sslmode=require, this should work. In verify-ca # or verify-full mode it should fail. @@ -147,34 +147,34 @@ $node->connect_ok( "connect without server root cert sslmode=require"); $node->connect_fails( "$common_connstr sslrootcert=invalid sslmode=verify-ca", - qr/root certificate file "invalid" does not exist/, - "connect without server root cert sslmode=verify-ca"); + "connect without server root cert sslmode=verify-ca", + expected_stderr => qr/root certificate file "invalid" does not exist/); $node->connect_fails( "$common_connstr sslrootcert=invalid sslmode=verify-full", - qr/root certificate file "invalid" does not exist/, - "connect without server root cert sslmode=verify-full"); + "connect without server root cert sslmode=verify-full", + expected_stderr => qr/root certificate file "invalid" does not exist/); # Try with wrong root cert, should fail. (We're using the client CA as the # root, but the server's key is signed by the server CA.) $node->connect_fails( "$common_connstr sslrootcert=ssl/client_ca.crt sslmode=require", - qr/SSL error/, - "connect with wrong server root cert sslmode=require"); + "connect with wrong server root cert sslmode=require", + expected_stderr => qr/SSL error/); $node->connect_fails( "$common_connstr sslrootcert=ssl/client_ca.crt sslmode=verify-ca", - qr/SSL error/, - "connect with wrong server root cert sslmode=verify-ca"); + "connect with wrong server root cert sslmode=verify-ca", + expected_stderr => qr/SSL error/); $node->connect_fails( "$common_connstr sslrootcert=ssl/client_ca.crt sslmode=verify-full", - qr/SSL error/, - "connect with wrong server root cert sslmode=verify-full"); + "connect with wrong server root cert sslmode=verify-full", + expected_stderr => qr/SSL error/); # Try with just the server CA's cert. This fails because the root file # must contain the whole chain up to the root CA. $node->connect_fails( "$common_connstr sslrootcert=ssl/server_ca.crt sslmode=verify-ca", - qr/SSL error/, - "connect with server CA cert, without root CA"); + "connect with server CA cert, without root CA", + expected_stderr => qr/SSL error/); # And finally, with the correct root cert. $node->connect_ok( @@ -206,14 +206,14 @@ $node->connect_ok( # A CRL belonging to a different CA is not accepted, fails $node->connect_fails( "$common_connstr sslrootcert=ssl/root+server_ca.crt sslmode=verify-ca sslcrl=ssl/client.crl", - qr/SSL error/, - "CRL belonging to a different CA"); + "CRL belonging to a different CA", + expected_stderr => qr/SSL error/); # The same for CRL directory $node->connect_fails( "$common_connstr sslrootcert=ssl/root+server_ca.crt sslmode=verify-ca sslcrldir=ssl/client-crldir", - qr/SSL error/, - "directory CRL belonging to a different CA"); + "directory CRL belonging to a different CA", + expected_stderr => qr/SSL error/); # With the correct CRL, succeeds (this cert is not revoked) $node->connect_ok( @@ -237,8 +237,8 @@ $node->connect_ok( "mismatch between host name and server certificate sslmode=verify-ca"); $node->connect_fails( "$common_connstr sslmode=verify-full host=wronghost.test", - qr/\Qserver certificate for "common-name.pg-ssltest.test" does not match host name "wronghost.test"\E/, - "mismatch between host name and server certificate sslmode=verify-full"); + "mismatch between host name and server certificate sslmode=verify-full", + expected_stderr => qr/\Qserver certificate for "common-name.pg-ssltest.test" does not match host name "wronghost.test"\E/); # Test Subject Alternative Names. switch_server_cert($node, 'server-multiple-alt-names'); @@ -257,12 +257,12 @@ $node->connect_ok("$common_connstr host=foo.wildcard.pg-ssltest.test", $node->connect_fails( "$common_connstr host=wronghost.alt-name.pg-ssltest.test", - qr/\Qserver certificate for "dns1.alt-name.pg-ssltest.test" (and 2 other names) does not match host name "wronghost.alt-name.pg-ssltest.test"\E/, - "host name not matching with X.509 Subject Alternative Names"); + "host name not matching with X.509 Subject Alternative Names", + expected_stderr => qr/\Qserver certificate for "dns1.alt-name.pg-ssltest.test" (and 2 other names) does not match host name "wronghost.alt-name.pg-ssltest.test"\E/); $node->connect_fails( "$common_connstr host=deep.subdomain.wildcard.pg-ssltest.test", - qr/\Qserver certificate for "dns1.alt-name.pg-ssltest.test" (and 2 other names) does not match host name "deep.subdomain.wildcard.pg-ssltest.test"\E/, - "host name not matching with X.509 Subject Alternative Names wildcard"); + "host name not matching with X.509 Subject Alternative Names wildcard", + expected_stderr => qr/\Qserver certificate for "dns1.alt-name.pg-ssltest.test" (and 2 other names) does not match host name "deep.subdomain.wildcard.pg-ssltest.test"\E/); # Test certificate with a single Subject Alternative Name. (this gives a # slightly different error message, that's all) @@ -277,12 +277,12 @@ $node->connect_ok( $node->connect_fails( "$common_connstr host=wronghost.alt-name.pg-ssltest.test", - qr/\Qserver certificate for "single.alt-name.pg-ssltest.test" does not match host name "wronghost.alt-name.pg-ssltest.test"\E/, - "host name not matching with a single X.509 Subject Alternative Name"); + "host name not matching with a single X.509 Subject Alternative Name", + expected_stderr => qr/\Qserver certificate for "single.alt-name.pg-ssltest.test" does not match host name "wronghost.alt-name.pg-ssltest.test"\E/); $node->connect_fails( "$common_connstr host=deep.subdomain.wildcard.pg-ssltest.test", - qr/\Qserver certificate for "single.alt-name.pg-ssltest.test" does not match host name "deep.subdomain.wildcard.pg-ssltest.test"\E/, - "host name not matching with a single X.509 Subject Alternative Name wildcard" + "host name not matching with a single X.509 Subject Alternative Name wildcard", + expected_stderr => qr/\Qserver certificate for "single.alt-name.pg-ssltest.test" does not match host name "deep.subdomain.wildcard.pg-ssltest.test"\E/ ); # Test server certificate with a CN and SANs. Per RFCs 2818 and 6125, the CN @@ -298,8 +298,8 @@ $node->connect_ok("$common_connstr host=dns2.alt-name.pg-ssltest.test", "certificate with both a CN and SANs 2"); $node->connect_fails( "$common_connstr host=common-name.pg-ssltest.test", - qr/\Qserver certificate for "dns1.alt-name.pg-ssltest.test" (and 1 other name) does not match host name "common-name.pg-ssltest.test"\E/, - "certificate with both a CN and SANs ignores CN"); + "certificate with both a CN and SANs ignores CN", + expected_stderr => qr/\Qserver certificate for "dns1.alt-name.pg-ssltest.test" (and 1 other name) does not match host name "common-name.pg-ssltest.test"\E/); # Finally, test a server certificate that has no CN or SANs. Of course, that's # not a very sensible certificate, but libpq should handle it gracefully. @@ -313,8 +313,8 @@ $node->connect_ok( $node->connect_fails( $common_connstr . " " . "sslmode=verify-full host=common-name.pg-ssltest.test", - qr/could not get server's host name from server certificate/, - "server certificate without CN or SANs sslmode=verify-full"); + "server certificate without CN or SANs sslmode=verify-full", + expected_stderr => qr/could not get server's host name from server certificate/); # Test that the CRL works switch_server_cert($node, 'server-revoked'); @@ -328,12 +328,12 @@ $node->connect_ok( "connects without client-side CRL"); $node->connect_fails( "$common_connstr sslrootcert=ssl/root+server_ca.crt sslmode=verify-ca sslcrl=ssl/root+server.crl", - qr/SSL error/, - "does not connect with client-side CRL file"); + "does not connect with client-side CRL file", + expected_stderr => qr/SSL error/); $node->connect_fails( "$common_connstr sslrootcert=ssl/root+server_ca.crt sslmode=verify-ca sslcrldir=ssl/root+server-crldir", - qr/SSL error/, - "does not connect with client-side CRL directory"); + "does not connect with client-side CRL directory", + expected_stderr => qr/SSL error/); # pg_stat_ssl command_like( @@ -355,16 +355,16 @@ $node->connect_ok( "connection success with correct range of TLS protocol versions"); $node->connect_fails( "$common_connstr sslrootcert=ssl/root+server_ca.crt sslmode=require ssl_min_protocol_version=TLSv1.2 ssl_max_protocol_version=TLSv1.1", - qr/invalid SSL protocol version range/, - "connection failure with incorrect range of TLS protocol versions"); + "connection failure with incorrect range of TLS protocol versions", + expected_stderr => qr/invalid SSL protocol version range/); $node->connect_fails( "$common_connstr sslrootcert=ssl/root+server_ca.crt sslmode=require ssl_min_protocol_version=incorrect_tls", - qr/invalid ssl_min_protocol_version value/, - "connection failure with an incorrect SSL protocol minimum bound"); + "connection failure with an incorrect SSL protocol minimum bound", + expected_stderr => qr/invalid ssl_min_protocol_version value/); $node->connect_fails( "$common_connstr sslrootcert=ssl/root+server_ca.crt sslmode=require ssl_max_protocol_version=incorrect_tls", - qr/invalid ssl_max_protocol_version value/, - "connection failure with an incorrect SSL protocol maximum bound"); + "connection failure with an incorrect SSL protocol maximum bound", + expected_stderr => qr/invalid ssl_max_protocol_version value/); ### Server-side tests. ### @@ -378,8 +378,8 @@ $common_connstr = # no client cert $node->connect_fails( "$common_connstr user=ssltestuser sslcert=invalid", - qr/connection requires a valid client certificate/, - "certificate authorization fails without client cert"); + "certificate authorization fails without client cert", + expected_stderr => qr/connection requires a valid client certificate/); # correct client cert in unencrypted PEM $node->connect_ok( @@ -408,8 +408,8 @@ $node->connect_ok( # correct client cert in encrypted PEM with wrong password $node->connect_fails( "$common_connstr user=ssltestuser sslcert=ssl/client.crt sslkey=ssl/client-encrypted-pem_tmp.key sslpassword='wrong'", - qr!\Qprivate key file "ssl/client-encrypted-pem_tmp.key": bad decrypt\E!, - "certificate authorization fails with correct client cert and wrong password in encrypted PEM format" + "certificate authorization fails with correct client cert and wrong password in encrypted PEM format", + expected_stderr => qr!\Qprivate key file "ssl/client-encrypted-pem_tmp.key": bad decrypt\E! ); @@ -446,15 +446,15 @@ TODO: # correct client cert in encrypted PEM with empty password $node->connect_fails( "$common_connstr user=ssltestuser sslcert=ssl/client.crt sslkey=ssl/client-encrypted-pem_tmp.key sslpassword=''", - qr!\Qprivate key file "ssl/client-encrypted-pem_tmp.key": processing error\E!, - "certificate authorization fails with correct client cert and empty password in encrypted PEM format" + "certificate authorization fails with correct client cert and empty password in encrypted PEM format", + expected_stderr => qr!\Qprivate key file "ssl/client-encrypted-pem_tmp.key": processing error\E! ); # correct client cert in encrypted PEM with no password $node->connect_fails( "$common_connstr user=ssltestuser sslcert=ssl/client.crt sslkey=ssl/client-encrypted-pem_tmp.key", - qr!\Qprivate key file "ssl/client-encrypted-pem_tmp.key": processing error\E!, - "certificate authorization fails with correct client cert and no password in encrypted PEM format" + "certificate authorization fails with correct client cert and no password in encrypted PEM format", + expected_stderr => qr!\Qprivate key file "ssl/client-encrypted-pem_tmp.key": processing error\E! ); } @@ -485,22 +485,22 @@ SKIP: $node->connect_fails( "$common_connstr user=ssltestuser sslcert=ssl/client.crt sslkey=ssl/client_wrongperms_tmp.key", - qr!\Qprivate key file "ssl/client_wrongperms_tmp.key" has group or world access\E!, - "certificate authorization fails because of file permissions"); + "certificate authorization fails because of file permissions", + expected_stderr => qr!\Qprivate key file "ssl/client_wrongperms_tmp.key" has group or world access\E!); } # client cert belonging to another user $node->connect_fails( "$common_connstr user=anotheruser sslcert=ssl/client.crt sslkey=ssl/client_tmp.key", - qr/certificate authentication failed for user "anotheruser"/, - "certificate authorization fails with client cert belonging to another user" + "certificate authorization fails with client cert belonging to another user", + expected_stderr => qr/certificate authentication failed for user "anotheruser"/ ); # revoked client cert $node->connect_fails( "$common_connstr user=ssltestuser sslcert=ssl/client-revoked.crt sslkey=ssl/client-revoked_tmp.key", - qr/SSL error/, - "certificate authorization fails with revoked client cert"); + "certificate authorization fails with revoked client cert", + expected_stderr => qr/SSL error/); # Check that connecting with auth-option verify-full in pg_hba: # works, iff username matches Common Name @@ -515,8 +515,8 @@ $node->connect_ok( $node->connect_fails( "$common_connstr user=anotheruser sslcert=ssl/client.crt sslkey=ssl/client_tmp.key", - qr/FATAL/, - "auth_option clientcert=verify-full fails with mismatching username and Common Name" + "auth_option clientcert=verify-full fails with mismatching username and Common Name", + expected_stderr => qr/FATAL/, ); # Check that connecting with auth-optionverify-ca in pg_hba : @@ -536,7 +536,7 @@ $node->connect_ok( "intermediate client certificate is provided by client"); $node->connect_fails( $common_connstr . " " . "sslmode=require sslcert=ssl/client.crt", - qr/SSL error/, "intermediate client certificate is missing"); + "intermediate client certificate is missing", expected_stderr => qr/SSL error/); # test server-side CRL directory switch_server_cert($node, 'server-cn-only', undef, undef, 'root+client-crldir'); @@ -544,8 +544,8 @@ switch_server_cert($node, 'server-cn-only', undef, undef, 'root+client-crldir'); # revoked client cert $node->connect_fails( "$common_connstr user=ssltestuser sslcert=ssl/client-revoked.crt sslkey=ssl/client-revoked_tmp.key", - qr/SSL error/, - "certificate authorization fails with revoked client cert with server-side CRL directory"); + "certificate authorization fails with revoked client cert with server-side CRL directory", + expected_stderr => qr/SSL error/); # clean up foreach my $key (@keys) diff --git a/src/test/ssl/t/002_scram.pl b/src/test/ssl/t/002_scram.pl index e31650b931..640b95099a 100644 --- a/src/test/ssl/t/002_scram.pl +++ b/src/test/ssl/t/002_scram.pl @@ -60,8 +60,8 @@ $node->connect_ok( # Test channel_binding $node->connect_fails( "$common_connstr user=ssltestuser channel_binding=invalid_value", - qr/invalid channel_binding value: "invalid_value"/, - "SCRAM with SSL and channel_binding=invalid_value"); + "SCRAM with SSL and channel_binding=invalid_value", + expected_stderr => qr/invalid channel_binding value: "invalid_value"/); $node->connect_ok("$common_connstr user=ssltestuser channel_binding=disable", "SCRAM with SSL and channel_binding=disable"); if ($supports_tls_server_end_point) @@ -74,15 +74,15 @@ else { $node->connect_fails( "$common_connstr user=ssltestuser channel_binding=require", - qr/channel binding is required, but server did not offer an authentication method that supports channel binding/, - "SCRAM with SSL and channel_binding=require"); + "SCRAM with SSL and channel_binding=require", + expected_stderr => qr/channel binding is required, but server did not offer an authentication method that supports channel binding/); } # Now test when the user has an MD5-encrypted password; should fail $node->connect_fails( "$common_connstr user=md5testuser channel_binding=require", - qr/channel binding required but not supported by server's authentication request/, - "MD5 with SSL and channel_binding=require"); + "MD5 with SSL and channel_binding=require", + expected_stderr => qr/channel binding required but not supported by server's authentication request/); # Now test with auth method 'cert' by connecting to 'certdb'. Should fail, # because channel binding is not performed. Note that ssl/client.key may @@ -93,8 +93,8 @@ copy("ssl/client.key", $client_tmp_key); chmod 0600, $client_tmp_key; $node->connect_fails( "sslcert=ssl/client.crt sslkey=$client_tmp_key sslrootcert=invalid hostaddr=$SERVERHOSTADDR dbname=certdb user=ssltestuser channel_binding=require", - qr/channel binding required, but server authenticated client without channel binding/, - "Cert authentication and channel_binding=require"); + "Cert authentication and channel_binding=require", + expected_stderr => qr/channel binding required, but server authenticated client without channel binding/); # clean up unlink($client_tmp_key); -- 2.25.1
From dc1f3c590a27ff71c28ec47e7f303bffc917580e Mon Sep 17 00:00:00 2001 From: Jacob Champion <pchamp...@vmware.com> Date: Thu, 1 Apr 2021 16:01:27 -0700 Subject: [PATCH v19 2/2] Log authenticated identity from all auth backends The "authenticated identity" is the string used by an auth method to identify a particular user. In many common cases this is the same as the Postgres username, but for some third-party auth methods, the identifier in use may be shortened or otherwise translated (e.g. through pg_ident mappings) before the server stores it. To help DBAs see who has actually interacted with the system, store the original identity when authentication succeeds, and expose it via the log_connections setting. The log entries look something like this example (where a local user named "pchampion" is connecting to the database as the "admin" user): LOG: connection received: host=[local] LOG: connection authenticated: identity="pchampion" method=peer (/data/pg_hba.conf:88) LOG: connection authorized: user=admin database=postgres application_name=psql port->authn_id is set according to the auth method: bsd: the Postgres username (which is the local username) cert: the client's Subject DN gss: the user principal ident: the remote username ldap: the final bind DN pam: the Postgres username (which is the PAM username) password (and all pw-challenge methods): the Postgres username peer: the peer's pw_name radius: the Postgres username (which is the RADIUS username) sspi: either the down-level (SAM-compatible) logon name, if compat_realm=1, or the User Principal Name if compat_realm=0 The trust auth method does not set an authenticated identity. Neither does using clientcert=verify-full (use the cert auth method instead). PostgresNode::connect_ok/fails() have been modified to let tests check the logfiles for required or prohibited patterns, using the respective log_like and log_unlike parameters. --- doc/src/sgml/config.sgml | 3 +- src/backend/libpq/auth.c | 136 ++++++++++++++++++++-- src/backend/libpq/hba.c | 24 ++++ src/include/libpq/hba.h | 1 + src/include/libpq/libpq-be.h | 13 +++ src/test/authentication/t/001_password.pl | 49 +++++--- src/test/kerberos/t/001_auth.pl | 78 +++++++++---- src/test/ldap/t/001_auth.pl | 32 +++-- src/test/perl/PostgresNode.pm | 81 ++++++++++++- src/test/ssl/t/001_ssltests.pl | 30 +++-- src/test/ssl/t/002_scram.pl | 8 +- 11 files changed, 387 insertions(+), 68 deletions(-) diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 9d87b5097a..7379dd3650 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -6714,7 +6714,8 @@ local0.* /var/log/postgresql <listitem> <para> Causes each attempted connection to the server to be logged, - as well as successful completion of client authentication. + as well as successful completion of both client authentication (if + necessary) and authorization. Only superusers can change this parameter at session start, and it cannot be changed at all within a session. The default is <literal>off</literal>. diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c index 9dc28e19aa..dee056b0d6 100644 --- a/src/backend/libpq/auth.c +++ b/src/backend/libpq/auth.c @@ -34,8 +34,10 @@ #include "libpq/scram.h" #include "miscadmin.h" #include "port/pg_bswap.h" +#include "postmaster/postmaster.h" #include "replication/walsender.h" #include "storage/ipc.h" +#include "utils/guc.h" #include "utils/memutils.h" #include "utils/timestamp.h" @@ -47,6 +49,7 @@ static void sendAuthRequest(Port *port, AuthRequest areq, const char *extradata, int extralen); static void auth_failed(Port *port, int status, char *logdetail); static char *recv_password_packet(Port *port); +static void set_authn_id(Port *port, const char *id); /*---------------------------------------------------------------- @@ -337,6 +340,51 @@ auth_failed(Port *port, int status, char *logdetail) } +/* + * Sets the authenticated identity for the current user. The provided string + * will be copied into the TopMemoryContext. The ID will be logged if + * log_connections is enabled. + * + * Auth methods should call this routine exactly once, as soon as the user is + * successfully authenticated, even if they have reasons to know that + * authorization will fail later. + * + * The provided string will be copied into TopMemoryContext, to match the + * lifetime of the Port, so it is safe to pass a string that is managed by an + * external library. + */ +static void +set_authn_id(Port *port, const char *id) +{ + Assert(id); + + if (port->authn_id) + { + /* + * An existing authn_id should never be overwritten; that means two + * authentication providers are fighting (or one is fighting itself). + * Don't leak any authn details to the client, but don't let the + * connection continue, either. + */ + ereport(FATAL, + (errmsg("connection was re-authenticated"), + errdetail_log("previous ID: \"%s\"; new ID: \"%s\"", + port->authn_id, id))); + } + + port->authn_id = MemoryContextStrdup(TopMemoryContext, id); + + if (Log_connections) + { + ereport(LOG, + errmsg("connection authenticated: identity=\"%s\" method=%s " + "(%s:%d)", + port->authn_id, hba_authname(port), HbaFileName, + port->hba->linenumber)); + } +} + + /* * Client authentication starts here. If there is an error, this * function does not return and the backend process is terminated. @@ -757,6 +805,9 @@ CheckPasswordAuth(Port *port, char **logdetail) pfree(shadow_pass); pfree(passwd); + if (result == STATUS_OK) + set_authn_id(port, port->user_name); + return result; } @@ -816,6 +867,10 @@ CheckPWChallengeAuth(Port *port, char **logdetail) Assert(auth_result != STATUS_OK); return STATUS_ERROR; } + + if (auth_result == STATUS_OK) + set_authn_id(port, port->user_name); + return auth_result; } @@ -1174,8 +1229,13 @@ pg_GSS_checkauth(Port *port) /* * Copy the original name of the authenticated principal into our backend * memory for display later. + * + * This is also our authenticated identity. Set it now, rather than + * waiting for the usermap check below, because authentication has already + * succeeded and we want the log file to reflect that. */ port->gss->princ = MemoryContextStrdup(TopMemoryContext, gbuf.value); + set_authn_id(port, gbuf.value); /* * Split the username at the realm separator @@ -1285,6 +1345,7 @@ pg_SSPI_recvauth(Port *port) DWORD domainnamesize = sizeof(domainname); SID_NAME_USE accountnameuse; HMODULE secur32; + char *authn_id; QUERY_SECURITY_CONTEXT_TOKEN_FN _QuerySecurityContextToken; @@ -1514,6 +1575,26 @@ pg_SSPI_recvauth(Port *port) return status; } + /* + * We have all of the information necessary to construct the authenticated + * identity. Set it now, rather than waiting for check_usermap below, + * because authentication has already succeeded and we want the log file + * to reflect that. + */ + if (port->hba->compat_realm) + { + /* SAM-compatible format. */ + authn_id = psprintf("%s\\%s", domainname, accountname); + } + else + { + /* Kerberos principal format. */ + authn_id = psprintf("%s@%s", accountname, domainname); + } + + set_authn_id(port, authn_id); + pfree(authn_id); + /* * Compare realm/domain if requested. In SSPI, always compare case * insensitive. @@ -1901,8 +1982,15 @@ ident_inet_done: pg_freeaddrinfo_all(local_addr.addr.ss_family, la); if (ident_return) - /* Success! Check the usermap */ + { + /* + * Success! Store the identity, then check the usermap. Note that + * setting the authenticated identity is done before checking the + * usermap, because at this point authentication has succeeded. + */ + set_authn_id(port, ident_user); return check_usermap(port->hba->usermap, port->user_name, ident_user, false); + } return STATUS_ERROR; } @@ -1926,7 +2014,6 @@ auth_peer(hbaPort *port) gid_t gid; #ifndef WIN32 struct passwd *pw; - char *peer_user; int ret; #endif @@ -1958,12 +2045,14 @@ auth_peer(hbaPort *port) return STATUS_ERROR; } - /* Make a copy of static getpw*() result area. */ - peer_user = pstrdup(pw->pw_name); - - ret = check_usermap(port->hba->usermap, port->user_name, peer_user, false); + /* + * Make a copy of static getpw*() result area; this is our authenticated + * identity. Set it before calling check_usermap, because authentication + * has already succeeded and we want the log file to reflect that. + */ + set_authn_id(port, pw->pw_name); - pfree(peer_user); + ret = check_usermap(port->hba->usermap, port->user_name, port->authn_id, false); return ret; #else @@ -2220,6 +2309,9 @@ CheckPAMAuth(Port *port, const char *user, const char *password) pam_passwd = NULL; /* Unset pam_passwd */ + if (retval == PAM_SUCCESS) + set_authn_id(port, user); + return (retval == PAM_SUCCESS ? STATUS_OK : STATUS_ERROR); } #endif /* USE_PAM */ @@ -2255,6 +2347,7 @@ CheckBSDAuth(Port *port, char *user) if (!retval) return STATUS_ERROR; + set_authn_id(port, user); return STATUS_OK; } #endif /* USE_BSD_AUTH */ @@ -2761,6 +2854,9 @@ CheckLDAPAuth(Port *port) return STATUS_ERROR; } + /* Save the original bind DN as the authenticated identity. */ + set_authn_id(port, fulluser); + ldap_unbind(ldap); pfree(passwd); pfree(fulluser); @@ -2824,6 +2920,30 @@ CheckCertAuth(Port *port) return STATUS_ERROR; } + if (port->hba->auth_method == uaCert) + { + /* + * For cert auth, the client's Subject DN is always our authenticated + * identity, even if we're only using its CN for authorization. Set + * it now, rather than waiting for check_usermap() below, because + * authentication has already succeeded and we want the log file to + * reflect that. + */ + if (!port->peer_dn) + { + /* + * This should not happen as both peer_dn and peer_cn should be + * set in this context. + */ + ereport(LOG, + (errmsg("certificate authentication failed for user \"%s\": unable to retrieve subject DN", + port->user_name))); + return STATUS_ERROR; + } + + set_authn_id(port, port->peer_dn); + } + /* Just pass the certificate cn/dn to the usermap check */ status_check_usermap = check_usermap(port->hba->usermap, port->user_name, peer_username, false); if (status_check_usermap != STATUS_OK) @@ -2995,6 +3115,8 @@ CheckRADIUSAuth(Port *port) */ if (ret == STATUS_OK) { + set_authn_id(port, port->user_name); + pfree(passwd); return STATUS_OK; } diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c index feb711a6ef..b720b03e9a 100644 --- a/src/backend/libpq/hba.c +++ b/src/backend/libpq/hba.c @@ -3141,3 +3141,27 @@ hba_getauthmethod(hbaPort *port) { check_hba(port); } + + +/* + * Return the name of the auth method in use ("gss", "md5", "trust", etc.). + * + * The return value is statically allocated (see the UserAuthName array) and + * should not be freed. + */ +const char * +hba_authname(hbaPort *port) +{ + UserAuth auth_method; + + Assert(port->hba); + auth_method = port->hba->auth_method; + + if (auth_method < 0 || USER_AUTH_LAST < auth_method) + { + /* Should never happen. */ + elog(FATAL, "port has out-of-bounds UserAuth: %d", auth_method); + } + + return UserAuthName[auth_method]; +} diff --git a/src/include/libpq/hba.h b/src/include/libpq/hba.h index 1ec8603da7..63f2962139 100644 --- a/src/include/libpq/hba.h +++ b/src/include/libpq/hba.h @@ -137,6 +137,7 @@ typedef struct Port hbaPort; extern bool load_hba(void); extern bool load_ident(void); +extern const char *hba_authname(hbaPort *port); extern void hba_getauthmethod(hbaPort *port); extern int check_usermap(const char *usermap_name, const char *pg_role, const char *auth_user, diff --git a/src/include/libpq/libpq-be.h b/src/include/libpq/libpq-be.h index 713c34fedd..02015efe13 100644 --- a/src/include/libpq/libpq-be.h +++ b/src/include/libpq/libpq-be.h @@ -159,6 +159,19 @@ typedef struct Port */ HbaLine *hba; + /* + * Authenticated identity. The meaning of this identifier is dependent on + * hba->auth_method; it is the identity (if any) that the user presented + * during the authentication cycle, before they were assigned a database + * role. (It is effectively the "SYSTEM-USERNAME" of a pg_ident usermap + * -- though the exact string in use may be different, depending on pg_hba + * options.) + * + * authn_id is NULL if the user has not actually been authenticated, for + * example if the "trust" auth method is in use. + */ + const char *authn_id; + /* * TCP keepalive and user timeout settings. * diff --git a/src/test/authentication/t/001_password.pl b/src/test/authentication/t/001_password.pl index 4d5e304de1..f1975bf5d7 100644 --- a/src/test/authentication/t/001_password.pl +++ b/src/test/authentication/t/001_password.pl @@ -17,7 +17,7 @@ if (!$use_unix_sockets) } else { - plan tests => 13; + plan tests => 23; } @@ -35,15 +35,12 @@ sub reset_pg_hba return; } -# Test access for a single role, useful to wrap all tests into one. +# Test access for a single role, useful to wrap all tests into one. Extra +# named parameters are passed to connect_ok/fails as-is. sub test_role { - my $node = shift; - my $role = shift; - my $method = shift; - my $expected_res = shift; + my ($node, $role, $method, $expected_res, %params) = @_; my $status_string = 'failed'; - $status_string = 'success' if ($expected_res eq 0); my $connstr = "user=$role"; @@ -51,19 +48,20 @@ sub test_role if ($expected_res eq 0) { - $node->connect_ok($connstr, $testname); + $node->connect_ok($connstr, $testname, %params); } else { # No match pattern checks are done here on errors, only the # status code. - $node->connect_fails($connstr, $testname); + $node->connect_fails($connstr, $testname, %params); } } # Initialize primary node my $node = get_new_node('primary'); $node->init; +$node->append_conf('postgresql.conf', "log_connections = on\n"); $node->start; # Create 3 roles with different password methods for each one. The same @@ -76,26 +74,41 @@ $node->safe_psql('postgres', ); $ENV{"PGPASSWORD"} = 'pass'; -# For "trust" method, all users should be able to connect. +# For "trust" method, all users should be able to connect. These users are not +# considered to be authenticated. reset_pg_hba($node, 'trust'); -test_role($node, 'scram_role', 'trust', 0); -test_role($node, 'md5_role', 'trust', 0); +test_role($node, 'scram_role', 'trust', 0, + log_unlike => [ qr/connection authenticated:/ ]); +test_role($node, 'md5_role', 'trust', 0, + log_unlike => [ qr/connection authenticated:/ ]); # For plain "password" method, all users should also be able to connect. reset_pg_hba($node, 'password'); -test_role($node, 'scram_role', 'password', 0); -test_role($node, 'md5_role', 'password', 0); +test_role($node, 'scram_role', 'password', 0, + log_like => [ qr/connection authenticated: identity="scram_role" method=password/ ]); +test_role($node, 'md5_role', 'password', 0, + log_like => [ qr/connection authenticated: identity="md5_role" method=password/ ]); # For "scram-sha-256" method, user "scram_role" should be able to connect. reset_pg_hba($node, 'scram-sha-256'); -test_role($node, 'scram_role', 'scram-sha-256', 0); -test_role($node, 'md5_role', 'scram-sha-256', 2); +test_role($node, 'scram_role', 'scram-sha-256', 0, + log_like => [ qr/connection authenticated: identity="scram_role" method=scram-sha-256/ ]); +test_role($node, 'md5_role', 'scram-sha-256', 2, + log_unlike => [ qr/connection authenticated:/ ]); + +# Test that bad passwords are rejected. +$ENV{"PGPASSWORD"} = 'badpass'; +test_role($node, 'scram_role', 'scram-sha-256', 2, + log_unlike => [ qr/connection authenticated:/ ]); +$ENV{"PGPASSWORD"} = 'pass'; # For "md5" method, all users should be able to connect (SCRAM # authentication will be performed for the user with a SCRAM secret.) reset_pg_hba($node, 'md5'); -test_role($node, 'scram_role', 'md5', 0); -test_role($node, 'md5_role', 'md5', 0); +test_role($node, 'scram_role', 'md5', 0, + log_like => [ qr/connection authenticated: identity="scram_role" method=md5/ ]); +test_role($node, 'md5_role', 'md5', 0, + log_like => [ qr/connection authenticated: identity="md5_role" method=md5/ ]); # Tests for channel binding without SSL. # Using the password authentication method; channel binding can't work diff --git a/src/test/kerberos/t/001_auth.pl b/src/test/kerberos/t/001_auth.pl index 2395664145..ff4d7503a6 100644 --- a/src/test/kerberos/t/001_auth.pl +++ b/src/test/kerberos/t/001_auth.pl @@ -20,7 +20,7 @@ use Time::HiRes qw(usleep); if ($ENV{with_gssapi} eq 'yes') { - plan tests => 34; + plan tests => 46; } else { @@ -182,36 +182,36 @@ note "running tests"; # Test connection success or failure, and if success, that query returns true. sub test_access { - my ($node, $role, $query, $expected_res, $gssencmode, $test_name, $expect_log_msg) = @_; + my ($node, $role, $query, $expected_res, $gssencmode, $test_name, + @expect_log_msgs) = @_; # need to connect over TCP/IP for Kerberos my $connstr = $node->connstr('postgres') . " user=$role host=$host hostaddr=$hostaddr $gssencmode"; + my %params = ( + sql => $query, + ); + + if (@expect_log_msgs) + { + # Match every message literally. + my @regexes = map { qr/\Q$_\E/ } @expect_log_msgs; + + $params{log_like} = \@regexes; + } + if ($expected_res eq 0) { # The result is assumed to match "true", or "t", here. - $node->connect_ok($connstr, $test_name, sql => $query, - expected_stdout => qr/^t$/); + $params{expected_stdout} = qr/^t$/; + + $node->connect_ok($connstr, $test_name, %params); } else { - $node->connect_fails($connstr, $test_name); + $node->connect_fails($connstr, $test_name, %params); } - - # Verify specified log message is logged in the log file. - if ($expect_log_msg ne '') - { - my $first_logfile = slurp_file($node->logfile); - - like($first_logfile, qr/\Q$expect_log_msg\E/, - 'found expected log file content'); - } - - # Clean up any existing contents in the node's log file so as - # future tests don't step on each other's generated contents. - truncate $node->logfile, 0; - return; } # As above, but test for an arbitrary query result. @@ -234,11 +234,19 @@ $node->append_conf('pg_hba.conf', qq{host all all $hostaddr/32 gss map=mymap}); $node->restart; -test_access($node, 'test1', 'SELECT true', 2, '', 'fails without ticket', ''); +test_access($node, 'test1', 'SELECT true', 2, '', 'fails without ticket'); run_log [ $kinit, 'test1' ], \$test1_password or BAIL_OUT($?); -test_access($node, 'test1', 'SELECT true', 2, '', 'fails without mapping', ''); +test_access( + $node, + 'test1', + 'SELECT true', + 2, + '', + 'fails without mapping', + "connection authenticated: identity=\"test1\@$realm\" method=gss", + "no match in usermap \"mymap\" for user \"test1\""); $node->append_conf('pg_ident.conf', qq{mymap /^(.*)\@$realm\$ \\1}); $node->restart; @@ -250,6 +258,7 @@ test_access( 0, '', 'succeeds with mapping with default gssencmode and host hba', + "connection authenticated: identity=\"test1\@$realm\" method=gss", "connection authorized: user=$username database=$dbname application_name=$application GSS (authenticated=yes, encrypted=yes, principal=test1\@$realm)" ); @@ -260,6 +269,7 @@ test_access( 0, 'gssencmode=prefer', 'succeeds with GSS-encrypted access preferred with host hba', + "connection authenticated: identity=\"test1\@$realm\" method=gss", "connection authorized: user=$username database=$dbname application_name=$application GSS (authenticated=yes, encrypted=yes, principal=test1\@$realm)" ); test_access( @@ -269,6 +279,7 @@ test_access( 0, 'gssencmode=require', 'succeeds with GSS-encrypted access required with host hba', + "connection authenticated: identity=\"test1\@$realm\" method=gss", "connection authorized: user=$username database=$dbname application_name=$application GSS (authenticated=yes, encrypted=yes, principal=test1\@$realm)" ); @@ -305,6 +316,7 @@ test_access( 0, 'gssencmode=prefer', 'succeeds with GSS-encrypted access preferred and hostgssenc hba', + "connection authenticated: identity=\"test1\@$realm\" method=gss", "connection authorized: user=$username database=$dbname application_name=$application GSS (authenticated=yes, encrypted=yes, principal=test1\@$realm)" ); test_access( @@ -314,10 +326,11 @@ test_access( 0, 'gssencmode=require', 'succeeds with GSS-encrypted access required and hostgssenc hba', + "connection authenticated: identity=\"test1\@$realm\" method=gss", "connection authorized: user=$username database=$dbname application_name=$application GSS (authenticated=yes, encrypted=yes, principal=test1\@$realm)" ); test_access($node, 'test1', 'SELECT true', 2, 'gssencmode=disable', - 'fails with GSS encryption disabled and hostgssenc hba', ''); + 'fails with GSS encryption disabled and hostgssenc hba'); unlink($node->data_dir . '/pg_hba.conf'); $node->append_conf('pg_hba.conf', @@ -331,10 +344,11 @@ test_access( 0, 'gssencmode=prefer', 'succeeds with GSS-encrypted access preferred and hostnogssenc hba, but no encryption', + "connection authenticated: identity=\"test1\@$realm\" method=gss", "connection authorized: user=$username database=$dbname application_name=$application GSS (authenticated=yes, encrypted=no, principal=test1\@$realm)" ); test_access($node, 'test1', 'SELECT true', 2, 'gssencmode=require', - 'fails with GSS-encrypted access required and hostnogssenc hba', ''); + 'fails with GSS-encrypted access required and hostnogssenc hba'); test_access( $node, 'test1', @@ -342,6 +356,7 @@ test_access( 0, 'gssencmode=disable', 'succeeds with GSS encryption disabled and hostnogssenc hba', + "connection authenticated: identity=\"test1\@$realm\" method=gss", "connection authorized: user=$username database=$dbname application_name=$application GSS (authenticated=yes, encrypted=no, principal=test1\@$realm)" ); @@ -358,5 +373,22 @@ test_access( 0, '', 'succeeds with include_realm=0 and defaults', + "connection authenticated: identity=\"test1\@$realm\" method=gss", "connection authorized: user=$username database=$dbname application_name=$application GSS (authenticated=yes, encrypted=yes, principal=test1\@$realm)" ); + +# Reset pg_hba.conf, and cause a usermap failure with an authentication +# that has passed. +unlink($node->data_dir . '/pg_hba.conf'); +$node->append_conf('pg_hba.conf', + qq{host all all $hostaddr/32 gss include_realm=0 krb_realm=EXAMPLE.ORG}); +$node->restart; + +test_access( + $node, + 'test1', + 'SELECT true', + 2, + '', + 'fails with wrong krb_realm, but still authenticates', + "connection authenticated: identity=\"test1\@$realm\" method=gss"); diff --git a/src/test/ldap/t/001_auth.pl b/src/test/ldap/t/001_auth.pl index b08ba6b281..a03e4ef0df 100644 --- a/src/test/ldap/t/001_auth.pl +++ b/src/test/ldap/t/001_auth.pl @@ -6,7 +6,7 @@ use Test::More; if ($ENV{with_ldap} eq 'yes') { - plan tests => 22; + plan tests => 28; } else { @@ -152,6 +152,7 @@ note "setting up PostgreSQL instance"; my $node = get_new_node('node'); $node->init; +$node->append_conf('postgresql.conf', "log_connections = on\n"); $node->start; $node->safe_psql('postgres', 'CREATE USER test0;'); @@ -162,17 +163,17 @@ note "running tests"; sub test_access { - my ($node, $role, $expected_res, $test_name) = @_; + my ($node, $role, $expected_res, $test_name, %params) = @_; my $connstr = "user=$role"; if ($expected_res eq 0) { - $node->connect_ok($connstr, $test_name); + $node->connect_ok($connstr, $test_name, %params); } else { # Currently, we don't check the error message, just the code. - $node->connect_fails($connstr, $test_name); + $node->connect_fails($connstr, $test_name, %params); } } @@ -186,11 +187,16 @@ $node->restart; $ENV{"PGPASSWORD"} = 'wrong'; test_access($node, 'test0', 2, - 'simple bind authentication fails if user not found in LDAP'); + 'simple bind authentication fails if user not found in LDAP', + log_unlike => [ qr/connection authenticated:/ ]); test_access($node, 'test1', 2, - 'simple bind authentication fails with wrong password'); + 'simple bind authentication fails with wrong password', + log_unlike => [ qr/connection authenticated:/ ]); + $ENV{"PGPASSWORD"} = 'secret1'; -test_access($node, 'test1', 0, 'simple bind authentication succeeds'); +test_access($node, 'test1', 0, 'simple bind authentication succeeds', + log_like => [ qr/connection authenticated: identity="uid=test1,dc=example,dc=net" method=ldap/ ], +); note "search+bind"; @@ -206,7 +212,9 @@ test_access($node, 'test0', 2, test_access($node, 'test1', 2, 'search+bind authentication fails with wrong password'); $ENV{"PGPASSWORD"} = 'secret1'; -test_access($node, 'test1', 0, 'search+bind authentication succeeds'); +test_access($node, 'test1', 0, 'search+bind authentication succeeds', + log_like => [ qr/connection authenticated: identity="uid=test1,dc=example,dc=net" method=ldap/ ], +); note "multiple servers"; @@ -250,9 +258,13 @@ $node->append_conf('pg_hba.conf', $node->restart; $ENV{"PGPASSWORD"} = 'secret1'; -test_access($node, 'test1', 0, 'search filter finds by uid'); +test_access($node, 'test1', 0, 'search filter finds by uid', + log_like => [ qr/connection authenticated: identity="uid=test1,dc=example,dc=net" method=ldap/ ], +); $ENV{"PGPASSWORD"} = 'secret2'; -test_access($node, 'te...@example.net', 0, 'search filter finds by mail'); +test_access($node, 'te...@example.net', 0, 'search filter finds by mail', + log_like => [ qr/connection authenticated: identity="uid=test2,dc=example,dc=net" method=ldap/ ], +); note "search filters in LDAP URLs"; diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm index f29c877c79..aca920fe16 100644 --- a/src/test/perl/PostgresNode.pm +++ b/src/test/perl/PostgresNode.pm @@ -1880,6 +1880,19 @@ If this regular expression is set, matches it with the output generated. If this regular expression is set, matches it with the stderr generated. +=item log_like => [ qr/required message/ ] + +If given, it must be an array reference containing a list of regular expressions +that must match against the server log, using C<Test::More::like()>. + +=item log_unlike => [ qr/prohibited message/ ] + +If given, it must be an array reference containing a list of regular expressions +that must NOT match against the server log. They will be passed to +C<Test::More::unlike()>. + +=back + =back =cut @@ -1899,6 +1912,22 @@ sub connect_ok $sql = "SELECT \$\$connected with $connstr\$\$"; } + my (@log_like, @log_unlike); + if (defined($params{log_like})) + { + @log_like = @{ $params{log_like} }; + } + if (defined($params{log_unlike})) + { + @log_unlike = @{ $params{log_unlike} }; + } + + if (@log_like or @log_unlike) + { + # Don't let previous log entries match for this connection. + truncate $self->logfile, 0; + } + my ($ret, $stdout, $stderr) = $self->psql( 'postgres', $sql, @@ -1916,6 +1945,19 @@ sub connect_ok { like($stderr, $params{expected_stderr}, "$test_name: stderr matches"); } + if (@log_like or @log_unlike) + { + my $log_contents = TestLib::slurp_file($self->logfile); + + while (my $regex = shift @log_like) + { + like($log_contents, $regex, "$test_name: log matches"); + } + while (my $regex = shift @log_unlike) + { + unlike($log_contents, $regex, "$test_name: log does not match"); + } + } } =pod @@ -1932,6 +1974,12 @@ regular expression $expected_stderr for a match. If this regular expression is set, matches it with the output generated. +=item log_like => [ qr/required message/ ] + +=item log_unlike => [ qr/prohibited message/ ] + +See C<connect_ok()>, above. + =back =cut @@ -1940,6 +1988,23 @@ sub connect_fails { local $Test::Builder::Level = $Test::Builder::Level + 1; my ($self, $connstr, $test_name, %params) = @_; + + my (@log_like, @log_unlike); + if (defined($params{log_like})) + { + @log_like = @{ delete $params{log_like} }; + } + if (defined($params{log_unlike})) + { + @log_unlike = @{ delete $params{log_unlike} }; + } + + if (@log_like or @log_unlike) + { + # Don't let previous log entries match for this connection. + truncate $self->logfile, 0; + } + my ($ret, $stdout, $stderr) = $self->psql( 'postgres', "SELECT \$\$connected with $connstr\$\$", @@ -1950,7 +2015,21 @@ sub connect_fails if (defined($params{expected_stderr})) { - like($stderr, $params{expected_stderr}, "$test_name: matches"); + like($stderr, $params{expected_stderr}, "$test_name: stderr matches"); + } + + if (@log_like or @log_unlike) + { + my $log_contents = TestLib::slurp_file($self->logfile); + + while (my $regex = shift @log_like) + { + like($log_contents, $regex, "$test_name: log matches"); + } + while (my $regex = shift @log_unlike) + { + unlike($log_contents, $regex, "$test_name: log does not match"); + } } } diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl index 6df558fca7..dacb395b37 100644 --- a/src/test/ssl/t/001_ssltests.pl +++ b/src/test/ssl/t/001_ssltests.pl @@ -17,7 +17,7 @@ if ($ENV{with_ssl} ne 'openssl') } else { - plan tests => 103; + plan tests => 110; } #### Some configuration @@ -418,7 +418,9 @@ my $dn_connstr = "$common_connstr dbname=certdb_dn"; $node->connect_ok( "$dn_connstr user=ssltestuser sslcert=ssl/client-dn.crt sslkey=ssl/client-dn_tmp.key", - "certificate authorization succeeds with DN mapping"); + "certificate authorization succeeds with DN mapping", + log_like => [ qr/connection authenticated: identity="CN=ssltestuser-dn,OU=Testing,OU=Engineering,O=PGDG" method=cert/ ], +); # same thing but with a regex $dn_connstr = "$common_connstr dbname=certdb_dn_re"; @@ -432,7 +434,10 @@ $dn_connstr = "$common_connstr dbname=certdb_cn"; $node->connect_ok( "$dn_connstr user=ssltestuser sslcert=ssl/client-dn.crt sslkey=ssl/client-dn_tmp.key", - "certificate authorization succeeds with CN mapping"); + "certificate authorization succeeds with CN mapping", + # the full DN should still be used as the authenticated identity + log_like => [ qr/connection authenticated: identity="CN=ssltestuser-dn,OU=Testing,OU=Engineering,O=PGDG" method=cert/ ], +); @@ -493,14 +498,19 @@ SKIP: $node->connect_fails( "$common_connstr user=anotheruser sslcert=ssl/client.crt sslkey=ssl/client_tmp.key", "certificate authorization fails with client cert belonging to another user", - expected_stderr => qr/certificate authentication failed for user "anotheruser"/ + expected_stderr => qr/certificate authentication failed for user "anotheruser"/, + # certificate authentication should be logged even on failure + log_like => [ qr/connection authenticated: identity="CN=ssltestuser" method=cert/ ], ); # revoked client cert $node->connect_fails( "$common_connstr user=ssltestuser sslcert=ssl/client-revoked.crt sslkey=ssl/client-revoked_tmp.key", "certificate authorization fails with revoked client cert", - expected_stderr => qr/SSL error/); + expected_stderr => qr/SSL error/, + # revoked certificates should not authenticate the user + log_unlike => [ qr/connection authenticated:/ ], +); # Check that connecting with auth-option verify-full in pg_hba: # works, iff username matches Common Name @@ -510,20 +520,26 @@ $common_connstr = $node->connect_ok( "$common_connstr user=ssltestuser sslcert=ssl/client.crt sslkey=ssl/client_tmp.key", - "auth_option clientcert=verify-full succeeds with matching username and Common Name" + "auth_option clientcert=verify-full succeeds with matching username and Common Name", + # verify-full does not provide authentication + log_unlike => [ qr/connection authenticated:/ ], ); $node->connect_fails( "$common_connstr user=anotheruser sslcert=ssl/client.crt sslkey=ssl/client_tmp.key", "auth_option clientcert=verify-full fails with mismatching username and Common Name", expected_stderr => qr/FATAL/, + # verify-full does not provide authentication + log_unlike => [ qr/connection authenticated:/ ], ); # Check that connecting with auth-optionverify-ca in pg_hba : # works, when username doesn't match Common Name $node->connect_ok( "$common_connstr user=yetanotheruser sslcert=ssl/client.crt sslkey=ssl/client_tmp.key", - "auth_option clientcert=verify-ca succeeds with mismatching username and Common Name" + "auth_option clientcert=verify-ca succeeds with mismatching username and Common Name", + # verify-full does not provide authentication + log_unlike => [ qr/connection authenticated:/ ], ); # intermediate client_ca.crt is provided by client, and isn't in server's ssl_ca_file diff --git a/src/test/ssl/t/002_scram.pl b/src/test/ssl/t/002_scram.pl index 640b95099a..3f77fdcd2d 100644 --- a/src/test/ssl/t/002_scram.pl +++ b/src/test/ssl/t/002_scram.pl @@ -27,7 +27,7 @@ my $SERVERHOSTCIDR = '127.0.0.1/32'; my $supports_tls_server_end_point = check_pg_config("#define HAVE_X509_GET_SIGNATURE_NID 1"); -my $number_of_tests = $supports_tls_server_end_point ? 9 : 10; +my $number_of_tests = $supports_tls_server_end_point ? 11 : 12; # Allocation of base connection string shared among multiple tests. my $common_connstr; @@ -96,6 +96,12 @@ $node->connect_fails( "Cert authentication and channel_binding=require", expected_stderr => qr/channel binding required, but server authenticated client without channel binding/); +# Certificate verification at the connection level should still work fine. +$node->connect_ok( + "sslcert=ssl/client.crt sslkey=$client_tmp_key sslrootcert=invalid hostaddr=$SERVERHOSTADDR dbname=verifydb user=ssltestuser channel_binding=require", + "SCRAM with clientcert=verify-full and channel_binding=require", + log_like => [ qr/connection authenticated: identity="ssltestuser" method=scram-sha-256/ ]); + # clean up unlink($client_tmp_key); -- 2.25.1