On 6/26/24 11:33 AM, Yann Ylavic wrote:
> On Wed, Jun 26, 2024 at 10:28 AM Ruediger Pluem <rpl...@apache.org> wrote:
>>
>> On 3/19/23 10:30 PM, yla...@apache.org wrote:
>>>
>>> --- httpd/httpd/trunk/modules/ssl/ssl_engine_io.c (original)
>>> +++ httpd/httpd/trunk/modules/ssl/ssl_engine_io.c Sun Mar 19 21:30:47 2023
>>
>>> @@ -2402,7 +2403,7 @@ long ssl_io_data_cb(BIO *bio, int cmd,
>>>                      "%s: %s %ld/%d bytes %s BIO#%pp [mem: %pp] %s",
>>>                      MODSSL_LIBRARY_NAME,
>>>                      (cmd == (BIO_CB_WRITE|BIO_CB_RETURN) ? "write" : 
>>> "read"),
>>> -                    rc, argi, (cmd == (BIO_CB_WRITE|BIO_CB_RETURN) ? "to" 
>>> : "from"),
>>> +                    (long)rc, argi, (cmd == (BIO_CB_WRITE|BIO_CB_RETURN) ? 
>>> "to" : "from"),
>>
>> I think rc has a different meaning with OpenSSL 3. I think we need to use 
>> len in case of OpenSSL 3.
>> I noticed that with OpenSSL 3 only single bytes get dumped and no longer the 
>> whole buffer.
>>
>>>                      bio, argp, dump);
>>>              if (*dump != '\0' && argp != NULL)
>>>                  ssl_io_data_dump(c, s, argp, rc);
>>
>> Similar to above. I think we need to use len here in case of OpenSSL 3.
>> If my findings are seen as correct I could work on a patch :-).
> 
> I think you are right from the man page of BIO_set_callback{,_ex}(),
> please go ahead ;)

How about the below? I am yet undecided if I should follow the below approach 
to have
two completely separate callbacks depending on the OpenSSL version or one with 
a lot of
#If statements in it, but as much shared code as possible. Thoughts?

Index: modules/ssl/ssl_engine_io.c
===================================================================
--- modules/ssl/ssl_engine_io.c (revision 1918662)
+++ modules/ssl/ssl_engine_io.c (working copy)
@@ -2308,7 +2308,7 @@
 #define DUMP_WIDTH 16

 static void ssl_io_data_dump(conn_rec *c, server_rec *s,
-                             const char *b, long len)
+                             const char *b, size_t len)
 {
     char buf[256];
     int i, j, rows, trunc, pos;
@@ -2361,7 +2361,7 @@
     }
     if (trunc > 0)
         ap_log_cserror(APLOG_MARK, APLOG_TRACE7, 0, c, s,
-                "| %04ld - <SPACES/NULS>", len + trunc);
+                "| %04" APR_SIZE_T_FMT " - <SPACES/NULS>", len + trunc);
     ap_log_cserror(APLOG_MARK, APLOG_TRACE7, 0, c, s,
             
"+-------------------------------------------------------------------------+");
 }
@@ -2370,18 +2370,54 @@
 static long modssl_io_cb(BIO *bio, int cmd, const char *argp,
                          size_t len, int argi, long argl, int rc,
                          size_t *processed)
+{
+    SSL *ssl;
+    conn_rec *c;
+    server_rec *s;
+
+    if ((ssl = (SSL *)BIO_get_callback_arg(bio)) == NULL)
+        return rc;
+    if ((c = (conn_rec *)SSL_get_app_data(ssl)) == NULL)
+        return rc;
+    s = mySrvFromConn(c);
+
+    if (   cmd == (BIO_CB_WRITE|BIO_CB_RETURN)
+        || cmd == (BIO_CB_READ |BIO_CB_RETURN) ) {
+        if (rc > 0) {
+            const char *dump = "";
+            if (APLOG_CS_IS_LEVEL(c, s, APLOG_TRACE7)) {
+                if (argp != NULL)
+                    dump = "(BIO dump follows)";
+                else
+                    dump = "(Oops, no memory buffer?)";
+            }
+            ap_log_cserror(APLOG_MARK, APLOG_TRACE4, 0, c, 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"),
+                    *processed, len, (cmd == (BIO_CB_WRITE|BIO_CB_RETURN) ? 
"to" : "from"),
+                    bio, argp, dump);
+            if (*dump != '\0' && argp != NULL)
+                ssl_io_data_dump(c, s, argp, *processed);
+        }
+        else {
+            ap_log_cserror(APLOG_MARK, APLOG_TRACE4, 0, c, s,
+                    "%s: I/O error, " APR_SIZE_T_FMT " bytes expected to %s on 
BIO#%pp [mem: %pp]",
+                    MODSSL_LIBRARY_NAME, len,
+                    (cmd == (BIO_CB_WRITE|BIO_CB_RETURN) ? "write" : "read"),
+                    bio, argp);
+        }
+    }
+    return rc;
+}
+
 #else
 static long modssl_io_cb(BIO *bio, int cmd, const char *argp,
                          int argi, long argl, long rc)
-#endif
 {
     SSL *ssl;
     conn_rec *c;
     server_rec *s;
-#if OPENSSL_VERSION_NUMBER >= 0x30000000L
-    (void)len;
-    (void)processed;
-#endif

     if ((ssl = (SSL *)BIO_get_callback_arg(bio)) == NULL)
         return rc;
@@ -2406,7 +2442,7 @@
                     (long)rc, argi, (cmd == (BIO_CB_WRITE|BIO_CB_RETURN) ? 
"to" : "from"),
                     bio, argp, dump);
             if (*dump != '\0' && argp != NULL)
-                ssl_io_data_dump(c, s, argp, rc);
+                ssl_io_data_dump(c, s, argp, (size_t)rc);
         }
         else {
             ap_log_cserror(APLOG_MARK, APLOG_TRACE4, 0, c, s,
@@ -2418,6 +2454,7 @@
     }
     return rc;
 }
+#endif

 static APR_INLINE void set_bio_callback(BIO *bio, void *arg)
 {


Regards

Rüdiger

Reply via email to