From cd09b440de3369de0cea48fefdeb9afacc106d93 Mon Sep 17 00:00:00 2001 From: Daniel Gustafsson Date: Fri, 27 Jun 2025 11:03:21 +0200 Subject: [PATCH] Fix sslkeylogfile logging When sslkeylogfile has been set but the file fails to open in an otherwise successful connection, the log entry added to the conn object is never printed. Instead print the error on stderr for increased visibility. This is a debugging tool so using stderr for logging is appropriate. Also while there, remove the WARNING prefix on other key logging errors to better match libpq logging style and remove the useless call to umask in the callback. Issues noted by Peter Eisentraut in post-commit review. Author: Daniel Gustafsson Reported-by: Peter Eisentraut Discussion: https://postgr.es/m/70450bee-cfaa-48ce-8980-fc7efcfebb03@eisentraut.org --- src/interfaces/libpq/fe-secure-openssl.c | 21 +++++++++++---------- src/test/ssl/t/001_ssltests.pl | 7 +++++++ 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c index b08b3a6901b..cb2cfa55e1a 100644 --- a/src/interfaces/libpq/fe-secure-openssl.c +++ b/src/interfaces/libpq/fe-secure-openssl.c @@ -693,34 +693,36 @@ static unsigned char alpn_protos[] = PG_ALPN_PROTOCOL_VECTOR; * purposes. The file will be written using the NSS keylog format. LibreSSL * 3.5 introduced stub function to set the callback for OpenSSL compatibility * but the callback is never invoked. + * + * Error messages added to the connection object wont be printed anywhere if + * the connection is successful. Errors in processing keylogging are printed + * to stderr to overcome this. */ static void SSL_CTX_keylog_cb(const SSL *ssl, const char *line) { int fd; - mode_t old_umask; ssize_t rc; PGconn *conn = SSL_get_app_data(ssl); if (conn == NULL) return; - old_umask = umask(077); + errno = 0; fd = open(conn->sslkeylogfile, O_WRONLY | O_APPEND | O_CREAT, 0600); - umask(old_umask); if (fd == -1) { - libpq_append_conn_error(conn, "could not open SSL key logging file \"%s\": %s", - conn->sslkeylogfile, pg_strerror(errno)); + fprintf(stderr, libpq_gettext("could not open SSL key logging file \"%s\": %m\n"), + conn->sslkeylogfile); return; } /* line is guaranteed by OpenSSL to be NUL terminated */ rc = write(fd, line, strlen(line)); if (rc < 0) - libpq_append_conn_error(conn, "could not write to SSL key logging file \"%s\": %s", - conn->sslkeylogfile, pg_strerror(errno)); + fprintf(stderr, libpq_gettext("could not write to SSL key logging file \"%s\": %m\n"), + conn->sslkeylogfile); else rc = write(fd, "\n", 1); (void) rc; /* silence compiler warnings */ @@ -1050,14 +1052,13 @@ initialize_SSL(PGconn *conn) SSL_CTX_set_keylog_callback(SSL_context, SSL_CTX_keylog_cb); #else #ifdef LIBRESSL_VERSION_NUMBER - fprintf(stderr, libpq_gettext("WARNING: sslkeylogfile support requires OpenSSL\n")); + fprintf(stderr, libpq_gettext("sslkeylogfile support requires OpenSSL\n")); #else - fprintf(stderr, libpq_gettext("WARNING: libpq was not built with sslkeylogfile support\n")); + fprintf(stderr, libpq_gettext("libpq was not built with sslkeylogfile support\n")); #endif #endif } - /* * SSL contexts are reference counted by OpenSSL. We can free it as soon * as we have created the SSL object, and it will stick around for as long diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl index 2cb4d0ffd41..b2eb18d3e81 100644 --- a/src/test/ssl/t/001_ssltests.pl +++ b/src/test/ssl/t/001_ssltests.pl @@ -173,6 +173,13 @@ SKIP: ok( (@status = stat("$tempdir/key.txt")), "keylog file exists and returned status"); ok(@status && !($status[2] & 0006), "keylog file is not world readable"); + + # Connect should work with an incorrect sslkeylogfile, with the error to + # open the logfile printed to stderr + $node->connect_ok( + "$common_connstr sslrootcert=ssl/root+server_ca.crt sslkeylogfile=$tempdir/invalid/key.txt sslmode=require", + "connect with server root cert and incorrect sslkeylogfile path", + expected_stderr => qr/could not open/); } # The server should not accept non-SSL connections. -- 2.39.3 (Apple Git-146)