Merge r1918880, r1918881, r1918883 from trunk:

* Restore SSL dumping for OpenSSL >= 3.0.

  Since r1908537 BIO_set_callback_ex is used with OpenSSL >= 3.0 instead of
  BIO_set_callback to set the BIO callback. The meaning of parameters and
  their range of values in the callback function set by BIO_set_callback_ex
  has changed compared to the callback function set by BIO_set_callback
  although parameters kept their names. Accommodate for this and adjust the
  code accordingly.
  Furthermore limit the size of dumps to APR_UINT16_MAX bytes. Given the length
  of SSL records of 16k this should not have practical implications.

* Changelog for r1918880

mod_ssl: Let modssl_set_io_callbacks() decide which callback is needed.

* modules/ssl/ssl_private.h:
  Add conn_rec and server_rec args to modssl_set_io_callbacks().

* modules/ssl/ssl_engine_io.c(modssl_set_io_callbacks):
  Don't set modssl_io_cb for log levels below TRACE4.

* modules/ssl/ssl_engine_io.c(ssl_io_filter_init),
  modules/ssl/ssl_engine_kernel.c(ssl_find_vhost):
  Call modssl_set_io_callbacks() unconditionally.

* modules/ssl/ssl_engine_io.c(modssl_io_cb):
  While at it, (cmd & BIO_CB_WRITE) is enough to differentiate a
  write from read.

Reviewed by: rpluem, ylavic, jorton

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1918912 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/STATUS b/STATUS
index e9c7a0383ce2fd0ca4a9f6a9b0a81200b8825029..8cf714b967e2db0a1860cc95a0404b8d443f8fd9 100644 (file)
--- a/STATUS
+++ b/STATUS
@@ -156,15 +156,6 @@ RELEASE SHOWSTOPPERS:
 PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
   [ start all new proposals below, under PATCHES PROPOSED. ]
 
-  *) mod_ssl: Restore SSL dumping on trace7 loglevel with OpenSSL >= 3.0.
-     Trunk version of patch:
-        https://svn.apache.org/r1918880
-        https://svn.apache.org/r1918881
-        https://svn.apache.org/r1918883
-     Backport version for 2.4.x of patch:
-      Trunk version of patch works
-      svn merge -c 1918880,1918881,1918883 ^/httpd/httpd/trunk .
-     +1: rpluem, ylavic, jorton
 
 PATCHES PROPOSED TO BACKPORT FROM TRUNK:
   [ New proposals should be added at the end of the list ]

diff --git a/changes-entries/restore_ssl_dump_with_3.txt b/changes-entries/restore_ssl_dump_with_3.txt
new file mode 100644 (file)
index 0000000..771f979
--- /dev/null
+++ b/changes-entries/restore_ssl_dump_with_3.txt
@@ -0,0 +1,2 @@
+  *) mod_ssl: Restore SSL dumping on trace7 loglevel with OpenSSL >= 3.0.
+     [Ruediger Pluem, Yann Ylavic]

diff --git a/modules/ssl/ssl_engine_io.c b/modules/ssl/ssl_engine_io.c
index 9c7d2163f8a742a984bf668ce1942eba55654681..0be5318a989a30e01f8d4535782a7122e9de34a8 100644 (file)
--- a/modules/ssl/ssl_engine_io.c
+++ b/modules/ssl/ssl_engine_io.c
@@ -2285,9 +2285,7 @@ void ssl_io_filter_init(conn_rec *c, request_rec *r, SSL *ssl)
     apr_pool_cleanup_register(c->pool, (void*)filter_ctx,
                               ssl_io_filter_cleanup, apr_pool_cleanup_null);
 
-    if (APLOG_CS_IS_LEVEL(c, mySrvFromConn(c), APLOG_TRACE4)) {
-        modssl_set_io_callbacks(ssl);
-    }
+    modssl_set_io_callbacks(ssl, c, mySrvFromConn(c));
 
     return;
 }
@@ -2312,7 +2310,7 @@ void ssl_io_filter_register(apr_pool_t *p)
 #define DUMP_WIDTH 16
 
 static void ssl_io_data_dump(conn_rec *c, server_rec *s,
-                             const char *b, long len)
+                             const char *b, int len)
 {
     char buf[256];
     int i, j, rows, trunc, pos;
@@ -2365,11 +2363,13 @@ static void ssl_io_data_dump(conn_rec *c, server_rec *s,
     }
     if (trunc > 0)
         ap_log_cserror(APLOG_MARK, APLOG_TRACE7, 0, c, s,
-                "| %04ld - <SPACES/NULS>", len + trunc);
+                "| %04d - <SPACES/NULS>", len + trunc);
     ap_log_cserror(APLOG_MARK, APLOG_TRACE7, 0, c, s,
             "+-------------------------------------------------------------------------+");
 }
 
+#define MODSSL_IO_DUMP_MAX APR_UINT16_MAX
+
 #if OPENSSL_VERSION_NUMBER >= 0x30000000L
 static long modssl_io_cb(BIO *bio, int cmd, const char *argp,
                          size_t len, int argi, long argl, int rc,
@@ -2382,10 +2382,12 @@ static long modssl_io_cb(BIO *bio, int cmd, const char *argp,
     SSL *ssl;
     conn_rec *c;
     server_rec *s;
+
+    /* unused */
 #if OPENSSL_VERSION_NUMBER >= 0x30000000L
-    (void)len;
-    (void)processed;
+    (void)argi;
 #endif
+    (void)argl;
 
     if ((ssl = (SSL *)BIO_get_callback_arg(bio)) == NULL)
         return rc;
@@ -2395,28 +2397,59 @@ static long modssl_io_cb(BIO *bio, int cmd, const char *argp,
 
     if (   cmd == (BIO_CB_WRITE|BIO_CB_RETURN)
         || cmd == (BIO_CB_READ |BIO_CB_RETURN) ) {
-        if (rc >= 0) {
+#if OPENSSL_VERSION_NUMBER >= 0x30000000L
+        apr_size_t requested_len = len;
+        /*
+         * On OpenSSL >= 3 rc uses the meaning of the BIO_read_ex and
+         * BIO_write_ex functions return value and not the one of
+         * BIO_read and BIO_write. Hence 0 indicates an error.
+         */
+        int ok = (rc > 0);
+#else
+        apr_size_t requested_len = (apr_size_t)argi;
+        int ok = (rc >= 0);
+#endif
+        if (ok) {
+#if OPENSSL_VERSION_NUMBER >= 0x30000000L
+            apr_size_t actual_len = *processed;
+#else
+            apr_size_t actual_len = (apr_size_t)rc;
+#endif
             const char *dump = "";
             if (APLOG_CS_IS_LEVEL(c, s, APLOG_TRACE7)) {
-                if (argp != NULL)
-                    dump = "(BIO dump follows)";
-                else
+                if (argp == NULL)
                     dump = "(Oops, no memory buffer?)";
+                else if (actual_len > MODSSL_IO_DUMP_MAX)
+                    dump = "(BIO dump follows, truncated to "
+                             APR_STRINGIFY(MODSSL_IO_DUMP_MAX) ")";
+                else
+                    dump = "(BIO dump follows)";
             }
             ap_log_cserror(APLOG_MARK, APLOG_TRACE4, 0, c, s,
-                    "%s: %s %ld/%d bytes %s BIO#%pp [mem: %pp] %s",
+                    "%s: %s %" APR_SIZE_T_FMT "/%" APR_SIZE_T_FMT 
+                    " bytes %s BIO#%pp [mem: %pp] %s",
                     MODSSL_LIBRARY_NAME,
-                    (cmd == (BIO_CB_WRITE|BIO_CB_RETURN) ? "write" : "read"),
-                    (long)rc, argi, (cmd == (BIO_CB_WRITE|BIO_CB_RETURN) ? "to" : "from"),
+                    (cmd & BIO_CB_WRITE) ? "write" : "read",
+                    actual_len, requested_len,
+                    (cmd & BIO_CB_WRITE) ? "to" : "from",
                     bio, argp, dump);
-            if (*dump != '\0' && argp != NULL)
-                ssl_io_data_dump(c, s, argp, rc);
+            /*
+             * *dump will only be != '\0' if
+             * APLOG_CS_IS_LEVEL(c, s, APLOG_TRACE7)
+             */
+            if (*dump != '\0' && argp != NULL) {
+                int dump_len = (actual_len >= MODSSL_IO_DUMP_MAX
+                                       ? MODSSL_IO_DUMP_MAX
+                                       : actual_len);
+                ssl_io_data_dump(c, s, argp, dump_len);
+            }
         }
         else {
             ap_log_cserror(APLOG_MARK, APLOG_TRACE4, 0, c, s,
-                    "%s: I/O error, %d bytes expected to %s on BIO#%pp [mem: %pp]",
-                    MODSSL_LIBRARY_NAME, argi,
-                    (cmd == (BIO_CB_WRITE|BIO_CB_RETURN) ? "write" : "read"),
+                    "%s: I/O error, %" APR_SIZE_T_FMT 
+                    " bytes expected to %s on BIO#%pp [mem: %pp]",
+                    MODSSL_LIBRARY_NAME, requested_len,
+                    (cmd & BIO_CB_WRITE) ? "write" : "read",
                     bio, argp);
         }
     }
@@ -2433,10 +2466,15 @@ static APR_INLINE void set_bio_callback(BIO *bio, void *arg)
     BIO_set_callback_arg(bio, arg);
 }
 
-void modssl_set_io_callbacks(SSL *ssl)
+void modssl_set_io_callbacks(SSL *ssl, conn_rec *c, server_rec *s)
 {
-    BIO *rbio = SSL_get_rbio(ssl),
-        *wbio = SSL_get_wbio(ssl);
+    BIO *rbio, *wbio;
+
+    if (!APLOG_CS_IS_LEVEL(c, s, APLOG_TRACE4))
+        return;
+
+    rbio = SSL_get_rbio(ssl);
+    wbio = SSL_get_wbio(ssl);
     if (rbio) {
         set_bio_callback(rbio, ssl);
     }

diff --git a/modules/ssl/ssl_engine_kernel.c b/modules/ssl/ssl_engine_kernel.c
index fa1b3a81567fe676bb5928112c9f70c387c67155..9c51021844175635719e4ce059b19b17167bf62c 100644 (file)
--- a/modules/ssl/ssl_engine_kernel.c
+++ b/modules/ssl/ssl_engine_kernel.c
@@ -2585,9 +2585,7 @@ static int ssl_find_vhost(void *servername, conn_rec *c, server_rec *s)
          * (and the first vhost doesn't use APLOG_TRACE4), then
          * we need to set that callback here.
          */
-        if (APLOGtrace4(s)) {
-            modssl_set_io_callbacks(ssl);
-        }
+        modssl_set_io_callbacks(ssl, c, s);
 
         return 1;
     }

diff --git a/modules/ssl/ssl_private.h b/modules/ssl/ssl_private.h
index 25d79ce8dfca33bab9e23dc794815d3507340379..fef2525e429f81e7e381453f02c45b6e9ee3d16f 100644 (file)
--- a/modules/ssl/ssl_private.h
+++ b/modules/ssl/ssl_private.h
@@ -1049,7 +1049,7 @@ void         modssl_callback_keylog(const SSL *ssl, const char *line);
 /**  I/O  */
 void         ssl_io_filter_init(conn_rec *, request_rec *r, SSL *);
 void         ssl_io_filter_register(apr_pool_t *);
-void         modssl_set_io_callbacks(SSL *ssl);
+void         modssl_set_io_callbacks(SSL *ssl, conn_rec *c, server_rec *s);
 
 /* ssl_io_buffer_fill fills the setaside buffering of the HTTP request
  * to allow an SSL renegotiation to take place. */