commit dfe12fa0c3b93cf98169da2095e53edd79c936e6 Author: Jacob Champion Date: Fri Jul 1 13:39:34 2022 -0700 squash! Log details for client certificate failures Add the Issuer to the log. diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c index 3ccc23ba62..80b361b105 100644 --- a/src/backend/libpq/be-secure-openssl.c +++ b/src/backend/libpq/be-secure-openssl.c @@ -1076,6 +1076,40 @@ 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 * @@ -1093,8 +1127,8 @@ verify_cb(int ok, X509_STORE_CTX *ctx) int errcode; const char *errstring; X509 *cert; - char *certname = NULL; - char *truncated = NULL; + char *subject = NULL, *issuer = NULL; + char *sub_truncated = NULL, *iss_truncated = NULL; char *serialno = NULL; if (ok) @@ -1111,33 +1145,18 @@ verify_cb(int ok, X509_STORE_CTX *ctx) cert = X509_STORE_CTX_get_current_cert(ctx); if (cert) { - size_t namelen; ASN1_INTEGER *sn; BIGNUM *b; /* - * Get the Subject for logging, but don't let maliciously huge certs - * flood the logs. - * - * 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. + * Get the Subject and Issuer for logging, but don't let maliciously + * huge certs flood the logs. */ -#define MAXLEN 71 - certname = X509_NAME_to_cstring(X509_get_subject_name(cert)); - namelen = strlen(certname); + subject = X509_NAME_to_cstring(X509_get_subject_name(cert)); + sub_truncated = truncate_cert_name(subject); - if (namelen > MAXLEN) - { - /* - * Keep the end of the Subject, not the beginning, since the most - * specific field is likely to give users the most information. - */ - truncated = certname + namelen - MAXLEN; - truncated[0] = truncated[1] = truncated[2] = '.'; - } -#undef MAXLEN + 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. @@ -1154,14 +1173,19 @@ verify_cb(int ok, X509_STORE_CTX *ctx) (errmsg("client certificate verification failed at depth %d: %s", depth, errstring), /* only print detail if we have a certificate to print */ - certname && errdetail("failed certificate had subject '%s', serial number %s", - truncated ? truncated : certname, - serialno ? serialno : _("unknown")))); + subject && errdetail("failed certificate had subject '%s', " + "serial number %s, " + "purported issuer '%s'", + sub_truncated ? sub_truncated : subject, + serialno ? serialno : _("unknown"), + iss_truncated ? iss_truncated : issuer))); if (serialno) OPENSSL_free(serialno); - if (certname) - pfree(certname); + if (issuer) + pfree(issuer); + if (subject) + pfree(subject); return ok; } diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl index 7378cb5dc6..a9b737ed09 100644 --- a/src/test/ssl/t/001_ssltests.pl +++ b/src/test/ssl/t/001_ssltests.pl @@ -685,7 +685,7 @@ $node->connect_fails( expected_stderr => qr/SSL error: sslv3 alert certificate revoked/, log_like => [ qr/client certificate verification failed at depth 0: certificate revoked/, - qr/failed certificate had subject '\/CN=ssltestuser', serial number 2315134995201656577/, + qr/failed certificate had subject '\/CN=ssltestuser', serial number 2315134995201656577, purported issuer '\/CN=Test CA for PostgreSQL SSL regression test client certs'/, ], # revoked certificates should not authenticate the user log_unlike => [qr/connection authenticated:/],); @@ -737,7 +737,7 @@ $node->connect_fails( 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 had subject '\/CN=ssltestuser', serial number 2315134995201656576/, + qr/failed certificate had subject '\/CN=ssltestuser', serial number 2315134995201656576, purported issuer '\/CN=Test CA for PostgreSQL SSL regression test client certs'/, ]); $node->connect_fails( @@ -746,7 +746,7 @@ $node->connect_fails( 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 had subject '\.\.\.\/CN=ssl-123456789012345678901234567890123456789012345678901234567890', serial number 2315418733629425152/, + qr/failed certificate had subject '\.\.\.\/CN=ssl-123456789012345678901234567890123456789012345678901234567890', serial number 2315418733629425152, purported 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 @@ -761,7 +761,7 @@ $node->connect_fails( 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 had subject '\/CN=Test CA for PostgreSQL SSL regression test client certs', serial number 2315134995201656577/, + qr/failed certificate had subject '\/CN=Test CA for PostgreSQL SSL regression test client certs', serial number 2315134995201656577, purported issuer '\/CN=Test root CA for PostgreSQL SSL regression test suite'/, ]); # test server-side CRL directory @@ -778,7 +778,7 @@ $node->connect_fails( expected_stderr => qr/SSL error: sslv3 alert certificate revoked/, log_like => [ qr/client certificate verification failed at depth 0: certificate revoked/, - qr/failed certificate had subject '\/CN=ssltestuser', serial number 2315134995201656577/, + qr/failed certificate had subject '\/CN=ssltestuser', serial number 2315134995201656577, purported issuer '\/CN=Test CA for PostgreSQL SSL regression test client certs'/, ]); done_testing();