From: Ruediger Pluem Date: Thu, 4 Jul 2024 15:58:17 +0000 (+0000) Subject: Merge r1918880, r1918881, r1918883 from trunk: X-Git-Tag: 2.4.62-rc1-candidate~30 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d2497ca5d06d87554189aebc61f4dd4d999ec820;p=thirdparty%2Fapache%2Fhttpd.git 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 e9c7a0383ce..8cf714b967e 100644 --- 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 index 00000000000..771f979743b --- /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 9c7d2163f8a..0be5318a989 100644 --- 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 - ", len + trunc); + "| %04d - ", 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 fa1b3a81567..9c510218441 100644 --- 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 25d79ce8dfc..fef2525e429 100644 --- 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. */