Convert keylogging in response to comments

1) Convert failures in keylog setup to trace messages for a warning-like
   mechanism

2) Convert sslkeylogfile_cb to be a flag used to determine making a
   direct call to the internal logging function

Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Saša Nedvědický <sashan@openssl.org>
Reviewed-by: Hugo Landau <hlandau@devever.net>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/25297)

diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index 2087331f586b25d8d398c790c83320f763e28372..ba4d6904687528881af26a7d3b260bd969802ef9 100644 (file)
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -4197,15 +4197,15 @@ SSL_CTX *SSL_CTX_new_ex(OSSL_LIB_CTX *libctx, const char *propq,
     if (keylogfile != NULL && strlen(keylogfile) != 0) {
         /* Make sure we have a global lock allocated */
         if (!RUN_ONCE(&ssl_keylog_once, ssl_keylog_init)) {
-            /* log an error, but don't fail here */
-            ERR_raise(ERR_LIB_SSL, ERR_R_SSL_LIB);
-            goto err;
+            /* use a trace message as a warning */
+            OSSL_TRACE(TLS, "Unable to initalize keylog data\n");
+            goto out;
         }
 
         /* Grab out global lock */
         if (!CRYPTO_THREAD_write_lock(keylog_lock)) {
-            ERR_raise(ERR_LIB_SSL, ERR_R_SSL_LIB);
-            goto err;
+            OSSL_TRACE(TLS, "Unable to acquire keylog write lock\n");
+            goto out;
         } else {
             /*
              * If the bio for the requested keylog file hasn't been
@@ -4215,8 +4215,8 @@ SSL_CTX *SSL_CTX_new_ex(OSSL_LIB_CTX *libctx, const char *propq,
             if (keylog_bio == NULL) {
                 keylog_bio = BIO_new_file(keylogfile, "a");
                 if (keylog_bio == NULL) {
-                    ERR_raise(ERR_LIB_SSL, ERR_R_SSL_LIB);
-                    goto err;
+                    OSSL_TRACE(TLS, "Unable to create keylog bio\n");
+                    goto out;
                 }
                 BIO_set_callback_ex(keylog_bio, check_keylog_bio_free);
             } else {
@@ -4225,11 +4225,12 @@ SSL_CTX *SSL_CTX_new_ex(OSSL_LIB_CTX *libctx, const char *propq,
             }
             /* If we have a bio now, assign the callback handler */
             if (keylog_bio != NULL)
-                ret->sslkeylog_callback = sslkeylogfile_cb;
+                ret->do_sslkeylog = 1;
             /* unlock, and we're done */
             CRYPTO_THREAD_unlock(keylog_lock);
         }
     }
+out:
 #endif
     return ret;
  err:
@@ -4273,9 +4274,9 @@ void SSL_CTX_free(SSL_CTX *a)
 
 #ifndef OPENSSL_NO_SSLKEYLOG
     if (keylog_lock != NULL && CRYPTO_THREAD_write_lock(keylog_lock)) {
-        if (a->sslkeylog_callback != NULL)
+        if (a->do_sslkeylog == 1)
             BIO_free(keylog_bio);
-        a->sslkeylog_callback = NULL;
+        a->do_sslkeylog = 0;
         CRYPTO_THREAD_unlock(keylog_lock);
     }
 #endif
@@ -6874,7 +6875,7 @@ static int nss_keylog_int(const char *prefix,
     SSL_CTX *sctx = SSL_CONNECTION_GET_CTX(sc);
 
 #ifndef OPENSSL_NO_SSLKEYLOG
-    if (sctx->keylog_callback == NULL && sctx->sslkeylog_callback == NULL)
+    if (sctx->keylog_callback == NULL && sctx->do_sslkeylog == 0)
         return 1;
 #else
     if (sctx->keylog_callback == NULL)
@@ -6907,8 +6908,8 @@ static int nss_keylog_int(const char *prefix,
     *cursor = '\0';
 
 #ifndef OPENSSL_NO_SSLKEYLOG
-    if (sctx->sslkeylog_callback != NULL)
-        sctx->sslkeylog_callback(SSL_CONNECTION_GET_SSL(sc), (const char *)out);
+    if (sctx->do_sslkeylog == 1)
+        sslkeylogfile_cb(SSL_CONNECTION_GET_SSL(sc), (const char *)out);
 #endif
     if (sctx->keylog_callback != NULL)
         sctx->keylog_callback(SSL_CONNECTION_GET_SSL(sc), (const char *)out);

diff --git a/ssl/ssl_local.h b/ssl/ssl_local.h
index 2d2e4674b05f65f036ae82d0e338316c4d8938ea..06d78dd6359fa95bd0ea7c588def55fb5868caba 100644 (file)
--- a/ssl/ssl_local.h
+++ b/ssl/ssl_local.h
@@ -1106,7 +1106,7 @@ struct ssl_ctx_st {
      * the one set via SSLKEYLOGFILE, so we just keep them separate
      */
 # ifndef OPENSSL_NO_SSLKEYLOG
-    SSL_CTX_keylog_cb_func sslkeylog_callback;
+    uint32_t do_sslkeylog;
 # endif
 
     /*