From 601b4f23eb8a3d304faee94124ca8f5153fb7fb6 Mon Sep 17 00:00:00 2001 From: Jacob Champion Date: Fri, 8 Jul 2022 11:12:43 -0700 Subject: [PATCH v4 2/2] squash! Log details for client certificate failures Add an ex_data pointer to the SSL handle, so that we can store error information from the verification callback. This lets us add our error details directly to the final "could not accept SSL connection" log message, as opposed to issuing intermediate LOGs. The new format looks like this: LOG: connection received: host=localhost port=43112 LOG: could not accept SSL connection: certificate verify failed DETAIL: client certificate verification failed at depth 1: unable to get local issuer certificate HINT: 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' --- src/backend/libpq/be-secure-openssl.c | 68 ++++++++++++++++++--------- 1 file changed, 46 insertions(+), 22 deletions(-) diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c index a2cbcdad5f..b3d5e681ed 100644 --- a/src/backend/libpq/be-secure-openssl.c +++ b/src/backend/libpq/be-secure-openssl.c @@ -81,6 +81,15 @@ 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); +/* Helpers for storing application data in the SSL handle */ +struct pg_ex_data +{ + /* to bubble errors out of callbacks */ + char *errdetail; + char *errhint; +}; +static int ssl_ex_index; + /* ------------------------------------------------------------ */ /* Public interface */ /* ------------------------------------------------------------ */ @@ -102,6 +111,7 @@ be_tls_init(bool isServerStart) SSL_library_init(); SSL_load_error_strings(); #endif + ssl_ex_index = SSL_get_ex_new_index(0, NULL, NULL, NULL, NULL); SSL_initialized = true; } @@ -413,6 +423,7 @@ be_tls_open_server(Port *port) int err; int waitfor; unsigned long ecode; + struct pg_ex_data exdata = {0}; bool give_proto_hint; Assert(!port->ssl); @@ -445,6 +456,15 @@ be_tls_open_server(Port *port) SSLerrmessage(ERR_get_error())))); return -1; } + if (!SSL_set_ex_data(port->ssl, ssl_ex_index, &exdata)) + { + ereport(COMMERROR, + (errcode(ERRCODE_PROTOCOL_VIOLATION), + errmsg("could not attach application data to SSL handle: %s", + SSLerrmessage(ERR_get_error())))); + return -1; + } + port->ssl_in_use = true; aloop: @@ -537,10 +557,15 @@ aloop: give_proto_hint = false; break; } + ereport(COMMERROR, (errcode(ERRCODE_PROTOCOL_VIOLATION), errmsg("could not accept SSL connection: %s", SSLerrmessage(ecode)), + exdata.errdetail ? + errdetail("%s", exdata.errdetail) : 0, + exdata.errhint ? + errhint("%s", exdata.errhint) : 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 ? @@ -1113,8 +1138,8 @@ truncate_cert_name(char *name) /* * Certificate verification callback * - * This callback allows us to log intermediate problems during - * verification. + * 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 @@ -1123,13 +1148,12 @@ truncate_cert_name(char *name) static int verify_cb(int ok, X509_STORE_CTX *ctx) { + SSL *ssl; int depth; int errcode; const char *errstring; + struct pg_ex_data *exdata; X509 *cert; - char *subject = NULL, *issuer = NULL; - char *sub_truncated = NULL, *iss_truncated = NULL; - char *serialno = NULL; if (ok) { @@ -1138,13 +1162,23 @@ verify_cb(int ok, X509_STORE_CTX *ctx) } /* Pull all the information we have on the verification failure. */ + ssl = X509_STORE_CTX_get_ex_data(ctx, SSL_get_ex_data_X509_STORE_CTX_idx()); depth = X509_STORE_CTX_get_error_depth(ctx); errcode = X509_STORE_CTX_get_error(ctx); errstring = X509_verify_cert_error_string(errcode); + /* Store our detail message in SSL application data, to be logged later. */ + exdata = SSL_get_ex_data(ssl, ssl_ex_index); + exdata->errdetail = + psprintf(_("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; @@ -1166,27 +1200,17 @@ verify_cb(int ok, X509_STORE_CTX *ctx) b = ASN1_INTEGER_to_BN(sn, NULL); serialno = BN_bn2dec(b); - BN_free(b); - } + exdata->errhint = + psprintf(_("Failed certificate data (unverified): subject '%s', serial number %s, issuer '%s'"), + sub_truncated ? sub_truncated : subject, + serialno ? serialno : _("unknown"), + iss_truncated ? iss_truncated : issuer); - ereport(COMMERROR, - (errmsg("client certificate verification failed at depth %d: %s", - depth, errstring), - /* only print detail if we have a certificate to print */ - subject && errdetail("Failed certificate data (unverified): " - "subject '%s', " - "serial number %s, " - "issuer '%s'", - sub_truncated ? sub_truncated : subject, - serialno ? serialno : _("unknown"), - iss_truncated ? iss_truncated : issuer))); - - if (serialno) + BN_free(b); OPENSSL_free(serialno); - if (issuer) pfree(issuer); - if (subject) pfree(subject); + } return ok; } -- 2.25.1