diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c index 3d0168a369..9cec6866a3 100644 --- a/src/backend/libpq/be-secure-openssl.c +++ b/src/backend/libpq/be-secure-openssl.c @@ -81,6 +81,9 @@ static bool ssl_is_server_start; static int ssl_protocol_version_to_openssl(int v); static const char *ssl_protocol_version_to_string(int v); +/* for passing data back from verify_cb() */ +static const char *cert_errdetail; + /* ------------------------------------------------------------ */ /* Public interface */ /* ------------------------------------------------------------ */ @@ -541,6 +544,7 @@ aloop: (errcode(ERRCODE_PROTOCOL_VIOLATION), errmsg("could not accept SSL connection: %s", SSLerrmessage(ecode)), + cert_errdetail ? errdetail_internal("%s", cert_errdetail) : 0, give_proto_hint ? errhint("This may indicate that the client does not support any SSL protocol version between %s and %s.", ssl_min_protocol_version ? @@ -549,6 +553,7 @@ aloop: ssl_max_protocol_version ? ssl_protocol_version_to_string(ssl_max_protocol_version) : MAX_OPENSSL_TLS_VERSION) : 0)); + cert_errdetail = NULL; break; case SSL_ERROR_ZERO_RETURN: ereport(COMMERROR, @@ -1076,12 +1081,46 @@ dummy_ssl_passwd_cb(char *buf, int size, int rwflag, void *userdata) return 0; } +/* + * Examines the provided certificate name, and if it's too long to log, modifies + * and truncates it. The return value is NULL if no truncation was needed; it + * otherwise points into the middle of the input string, and should not be + * freed. + */ +static char * +truncate_cert_name(char *name) +{ + size_t namelen = strlen(name); + char *truncated; + + /* + * Common Names are 64 chars max, so for a common case where the CN is the + * last field, we can still print the longest possible CN with a + * 7-character prefix (".../CN=[64 chars]"), for a reasonable limit of 71 + * characters. + */ +#define MAXLEN 71 + + if (namelen <= MAXLEN) + return NULL; + + /* + * Keep the end of the name, not the beginning, since the most specific + * field is likely to give users the most information. + */ + truncated = name + namelen - MAXLEN; + truncated[0] = truncated[1] = truncated[2] = '.'; + +#undef MAXLEN + + return truncated; +} + /* * Certificate verification callback * - * This callback allows us to log intermediate problems during - * verification, but for now we'll see if the final error message - * contains enough information. + * This callback allows us to examine intermediate problems during + * verification, for later logging. * * This callback also allows us to override the default acceptance * criteria (e.g., accepting self-signed or expired certs), but @@ -1090,6 +1129,73 @@ dummy_ssl_passwd_cb(char *buf, int size, int rwflag, void *userdata) static int verify_cb(int ok, X509_STORE_CTX *ctx) { + int depth; + int errcode; + const char *errstring; + StringInfoData str; + X509 *cert; + + if (ok) + { + /* Nothing to do for the successful case. */ + return ok; + } + + /* Pull all the information we have on the verification failure. */ + depth = X509_STORE_CTX_get_error_depth(ctx); + errcode = X509_STORE_CTX_get_error(ctx); + errstring = X509_verify_cert_error_string(errcode); + + initStringInfo(&str); + appendStringInfo(&str, + _("Client certificate verification failed at depth %d: %s."), + depth, errstring); + + cert = X509_STORE_CTX_get_current_cert(ctx); + if (cert) + { + char *subject, + *issuer; + char *sub_truncated, + *iss_truncated; + char *serialno; + ASN1_INTEGER *sn; + BIGNUM *b; + + /* + * Get the Subject and Issuer for logging, but don't let maliciously + * huge certs flood the logs. + */ + subject = X509_NAME_to_cstring(X509_get_subject_name(cert)); + sub_truncated = truncate_cert_name(subject); + + issuer = X509_NAME_to_cstring(X509_get_issuer_name(cert)); + iss_truncated = truncate_cert_name(issuer); + + /* + * Pull the serial number, too, in case a Subject is still ambiguous. + * This mirrors be_tls_get_peer_serial(). + */ + sn = X509_get_serialNumber(cert); + b = ASN1_INTEGER_to_BN(sn, NULL); + serialno = BN_bn2dec(b); + + appendStringInfoChar(&str, '\n'); + appendStringInfo(&str, + _("Failed certificate data (unverified): subject \"%s\", serial number %s, issuer \"%s\"."), + sub_truncated ? sub_truncated : subject, + serialno ? serialno : _("unknown"), + iss_truncated ? iss_truncated : issuer); + + BN_free(b); + OPENSSL_free(serialno); + pfree(issuer); + pfree(subject); + } + + /* Store our detail message to be logged later. */ + cert_errdetail = str.data; + return ok; } diff --git a/src/test/ssl/conf/client-long.config b/src/test/ssl/conf/client-long.config new file mode 100644 index 0000000000..0e92a8fbfe --- /dev/null +++ b/src/test/ssl/conf/client-long.config @@ -0,0 +1,14 @@ +# An OpenSSL format CSR config file for creating a client certificate with a +# long Subject. + +[ req ] +distinguished_name = req_distinguished_name +prompt = no + +[ req_distinguished_name ] +# Common Names are 64 characters max +CN = ssl-123456789012345678901234567890123456789012345678901234567890 +OU = Some Organizational Unit +O = PostgreSQL Global Development Group + +# no extensions in client certs diff --git a/src/test/ssl/ssl/client-long.crt b/src/test/ssl/ssl/client-long.crt new file mode 100644 index 0000000000..a1db55b5c3 --- /dev/null +++ b/src/test/ssl/ssl/client-long.crt @@ -0,0 +1,20 @@ +-----BEGIN CERTIFICATE----- +MIIDWjCCAkICCCAiBRIUREYAMA0GCSqGSIb3DQEBCwUAMEIxQDA+BgNVBAMMN1Rl +c3QgQ0EgZm9yIFBvc3RncmVTUUwgU1NMIHJlZ3Jlc3Npb24gdGVzdCBjbGllbnQg +Y2VydHMwHhcNMjIwNTEyMjE0NDQ3WhcNNDkwOTI3MjE0NDQ3WjCBnDEsMCoGA1UE +CgwjUG9zdGdyZVNRTCBHbG9iYWwgRGV2ZWxvcG1lbnQgR3JvdXAxITAfBgNVBAsM +GFNvbWUgT3JnYW5pemF0aW9uYWwgVW5pdDFJMEcGA1UEAwxAc3NsLTEyMzQ1Njc4 +OTAxMjM0NTY3ODkwMTIzNDU2Nzg5MDEyMzQ1Njc4OTAxMjM0NTY3ODkwMTIzNDU2 +Nzg5MDCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBANgxmeHiVRuBTwlG +Q1oM2M1ckQCI/o4hYcO9BYdxDYHiA7jy1WVenyj8BtUi5Aj9VDhpfiuewDarGQ5a +TggD1pMjkw0MorBKBr9+1u1xGH/8Q3lkgU+OQXrPglo4IrVcqaoZFQ0nuMaVbieX +0dDyTfsTaVQYYtqAtzhI/UGSIOhk2+lB9fP68jw9cLH0QYvR+qQ0IPG13I5zmSYP +Mj0VYwMn9TF9/2sBOSRVgTVAcrYgOQLk3s/fGe66tmVBIWYcq65ygqD1+weu+Pax +jPnwsefkdnf6JdYRG1F1Co7g52poPEYieAHfQOJ69sG0LYx0lBODC69qvSJ4WdCQ +0zKw288CAwEAATANBgkqhkiG9w0BAQsFAAOCAQEArr5r1UxgUzPykmu5ZdL6y8TA +ZbSQ1yBY0nhsRwRkDd66iPK9U6T6K2+pL8Vc6ioov9WOtHQ6ohP3gSavd40cHRmF +auwIsZ4Wk0mjftpOuPFp1hyo8d/QYrbEm3qNe5qln5S9h8ipoYvFtf5zlK2KHJFz +9ehZMZ1zGAERNCVM8UUQKyUuZB5GyrZlbslf6P/9Bsc54YUWxP2pr5r/RJ6DeXfI +zAFfXT8AFVlClARA949gpX0LVrXryDN60CUJ88QJmYCQ3AtIgzYYeqcdYHTd8eS2 +9P5whDdU5NvROP+LjETeReJF4Bfyc2gM7zxZD2BDSf5exvnNqiy42/lR1b4szw== +-----END CERTIFICATE----- diff --git a/src/test/ssl/ssl/client-long.key b/src/test/ssl/ssl/client-long.key new file mode 100644 index 0000000000..5b455a021f --- /dev/null +++ b/src/test/ssl/ssl/client-long.key @@ -0,0 +1,27 @@ +-----BEGIN RSA PRIVATE KEY----- +MIIEpAIBAAKCAQEA2DGZ4eJVG4FPCUZDWgzYzVyRAIj+jiFhw70Fh3ENgeIDuPLV +ZV6fKPwG1SLkCP1UOGl+K57ANqsZDlpOCAPWkyOTDQyisEoGv37W7XEYf/xDeWSB +T45Bes+CWjgitVypqhkVDSe4xpVuJ5fR0PJN+xNpVBhi2oC3OEj9QZIg6GTb6UH1 +8/ryPD1wsfRBi9H6pDQg8bXcjnOZJg8yPRVjAyf1MX3/awE5JFWBNUBytiA5AuTe +z98Z7rq2ZUEhZhyrrnKCoPX7B6749rGM+fCx5+R2d/ol1hEbUXUKjuDnamg8RiJ4 +Ad9A4nr2wbQtjHSUE4MLr2q9InhZ0JDTMrDbzwIDAQABAoIBADwJykpIqIny5xgU +QzAG0U52nm4fnVGrQ5MwMxDh/HZNZes+xLRaCqk/FEasYdd9Qp5H7Zn/hDGqYlLy +ESl4p2ZFQtkk4SlD5YvYladq+PrR+4sCtkZ5owWQCwsy+7CSAywRux7kIRRE+0pT +hxkXsUBAq8eG3i0AAeHHo01KX4kptlJ5d1pFKKAPThTUHCT4VPHg8r59IdsNy6wC ++0E5ZRWsVUePy+ERuarX/um896hgbaiDJLFk02Orlc87+OBmRwO8J+KoUOEcAiTO +OZqGGaDEn5Y2mEdp2cCmq7+Izcklaha6CPsoV8+O2HK8PKvBIQmlgbDmal4/RNqr +JFqYz0ECgYEA+5z74Tmj+tzH57lcdMqVpndG39N8spBe8JbiFL16qOb6gRDytXjc +hY6IQo4JStpJulnPBZ5JQSbSBgCOzYWJJVBnnwMJKjNCd1th4znjxxMOe4LiDTtw +D3hQtzBU9FlI2sjWEUKf1xCyi9N41ApQC5eDWWd/0GN9+xAsxRjLL00CgYEA2/aH +4kNVsBHQ7vmv+sNsWeIgKg7PC7hRjcCYQG9ylBbBnFtv5XJYicXwqorqngzJPoGw +gB7iaSWL1UNAOSWRSFYe+woPpkY7n6Pbq211nzqV1avAdVrLylJwyE+EOQgTS30D +8BHv0I714PMd/QLK5NSUEr1IRtCfLeMpcSg6YYsCgYEAv3O86KxeTMTvyy9s3WVE +p4y8vhUDHi/iPbjhQBzJF3nhhJGrzE+xpGJG5jWDdpRQY15wuvqtDMkIKA8GmfWQ +3Hao0gKSV6z3VzCOdEKZQeILNAnsDVt7shm/eRRqoB7L48XLtQh37UJESUbY+qb6 +L0fTZxTs2VjLBF1TY4mxGUUCgYEA1PLENKnJkA5/fowd8aA2CoKfbvgtPARyd9Bn +1aHPhEzPnabsGm7sBl2qFAEvCFoKjkgR7sd3nCHsUUetKmYTU7uEfLcN1YSS/oct +CLaMs92M53JCfZqsRrAvXc2VjX0i6Ocb49QJnph4tBHKC4MjmAuxWr8C9QPNxyfv +nAw9EOcCgYBYzejUzp6xiv5YzpwIncIF0A5E6VITcsW+LOR/FZOUPso0X2hQoIEs +wx8HjKCGfvX6628vnaWJC099hTmOzVwpEgik5zOmeAmZ//gt2I53Yg/loQUzH0CD +iXxrg/4Up7Yxx897w11ukOZv2xwmAFO1o52Q8k7d5FiMfEIzAkS3Pg== +-----END RSA PRIVATE KEY----- diff --git a/src/test/ssl/sslfiles.mk b/src/test/ssl/sslfiles.mk index cc023667af..5f67aba795 100644 --- a/src/test/ssl/sslfiles.mk +++ b/src/test/ssl/sslfiles.mk @@ -33,7 +33,7 @@ SERVERS := server-cn-and-alt-names \ server-multiple-alt-names \ server-no-names \ server-revoked -CLIENTS := client client-dn client-revoked client_ext +CLIENTS := client client-dn client-revoked client_ext client-long # # To add a new non-standard key, add it to SPECIAL_KEYS and then add a recipe diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl index 707f4005af..90ba5a7515 100644 --- a/src/test/ssl/t/001_ssltests.pl +++ b/src/test/ssl/t/001_ssltests.pl @@ -683,6 +683,10 @@ $node->connect_fails( . sslkey('client-revoked.key'), "certificate authorization fails with revoked client cert", expected_stderr => qr/SSL error: sslv3 alert certificate revoked/, + log_like => [ + qr{Client certificate verification failed at depth 0: certificate revoked}, + qr{Failed certificate data \(unverified\): subject "/CN=ssltestuser", serial number 2315134995201656577, issuer "/CN=Test CA for PostgreSQL SSL regression test client certs"}, + ], # revoked certificates should not authenticate the user log_unlike => [qr/connection authenticated:/],); @@ -730,7 +734,35 @@ $node->connect_ok( $node->connect_fails( $common_connstr . " " . "sslmode=require sslcert=ssl/client.crt", "intermediate client certificate is missing", - expected_stderr => qr/SSL error: tlsv1 alert unknown ca/); + expected_stderr => qr/SSL error: tlsv1 alert unknown ca/, + log_like => [ + qr{Client certificate verification failed at depth 0: unable to get local issuer certificate}, + qr{Failed certificate data \(unverified\): subject "/CN=ssltestuser", serial number 2315134995201656576, issuer "/CN=Test CA for PostgreSQL SSL regression test client certs"}, + ]); + +$node->connect_fails( + "$common_connstr sslmode=require sslcert=ssl/client-long.crt " . sslkey('client-long.key'), + "logged client certificate Subjects are truncated if they're too long", + expected_stderr => qr/SSL error: tlsv1 alert unknown ca/, + log_like => [ + qr{Client certificate verification failed at depth 0: unable to get local issuer certificate}, + qr{Failed certificate data \(unverified\): subject "\.\.\./CN=ssl-123456789012345678901234567890123456789012345678901234567890", serial number 2315418733629425152, issuer "/CN=Test CA for PostgreSQL SSL regression test client certs"}, + ]); + +# Use an invalid cafile here so that the next test won't be able to verify the +# client CA. +switch_server_cert($node, certfile => 'server-cn-only', cafile => 'server-cn-only'); + +# intermediate CA is provided but doesn't have a trusted root (checks error +# logging for cert chain depths > 0) +$node->connect_fails( + "$common_connstr sslmode=require sslcert=ssl/client+client_ca.crt", + "intermediate client certificate is untrusted", + expected_stderr => qr/SSL error: tlsv1 alert unknown ca/, + log_like => [ + qr{Client certificate verification failed at depth 1: unable to get local issuer certificate}, + qr{Failed certificate data \(unverified\): subject "/CN=Test CA for PostgreSQL SSL regression test client certs", serial number 2315134995201656577, issuer "/CN=Test root CA for PostgreSQL SSL regression test suite"}, + ]); # test server-side CRL directory switch_server_cert( @@ -743,6 +775,10 @@ $node->connect_fails( "$common_connstr user=ssltestuser sslcert=ssl/client-revoked.crt " . sslkey('client-revoked.key'), "certificate authorization fails with revoked client cert with server-side CRL directory", - expected_stderr => qr/SSL error: sslv3 alert certificate revoked/); + expected_stderr => qr/SSL error: sslv3 alert certificate revoked/, + log_like => [ + qr{Client certificate verification failed at depth 0: certificate revoked}, + qr{Failed certificate data \(unverified\): subject "/CN=ssltestuser", serial number 2315134995201656577, issuer "/CN=Test CA for PostgreSQL SSL regression test client certs"}, + ]); done_testing(); diff --git a/src/test/ssl/t/SSL/Backend/OpenSSL.pm b/src/test/ssl/t/SSL/Backend/OpenSSL.pm index aed6005b43..a43e64c04f 100644 --- a/src/test/ssl/t/SSL/Backend/OpenSSL.pm +++ b/src/test/ssl/t/SSL/Backend/OpenSSL.pm @@ -88,7 +88,7 @@ sub init "client.key", "client-revoked.key", "client-der.key", "client-encrypted-pem.key", "client-encrypted-der.key", "client-dn.key", - "client_ext.key"); + "client_ext.key", "client-long.key"); foreach my $keyfile (@keys) { copy("ssl/$keyfile", "$cert_tempdir/$keyfile")