On Thu, Jun 27, 2024 at 12:38 PM Yann Ylavic <[email protected]> wrote:
>
> On Thu, Jun 27, 2024 at 12:07 PM Ruediger Pluem <[email protected]> wrote:
> >
> > 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?
>
> Hm, I wouldn't duplicate the whole thing depending on openssl version.
>
> Something like the attached maybe? modssl_io_cb() will just consider
> up to APR_INT32_MAX bytes, more shouldn't happen anyway and it's more
> than enough for debug logs..
We could even log the real length still if it matters, see attached v2.
>
> Regards;
> Yann.
Index: modules/ssl/ssl_engine_io.c
===================================================================
--- modules/ssl/ssl_engine_io.c (revision 1918653)
+++ modules/ssl/ssl_engine_io.c (working copy)
@@ -2308,7 +2308,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;
@@ -2361,17 +2361,18 @@ static void ssl_io_data_dump(conn_rec *c, server_r
}
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,
"+-------------------------------------------------------------------------+");
}
#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,
+static long modssl_io_cb(BIO *bio, int cmd,
+ const char *data, size_t data_len,
+ int argi, long argl, int rc,
size_t *processed)
#else
-static long modssl_io_cb(BIO *bio, int cmd, const char *argp,
+static long modssl_io_cb(BIO *bio, int cmd, const char *data,
int argi, long argl, long rc)
#endif
{
@@ -2378,8 +2379,10 @@ static void ssl_io_data_dump(conn_rec *c, server_r
SSL *ssl;
conn_rec *c;
server_rec *s;
+
+ /* unused */
+ (void)argl;
#if OPENSSL_VERSION_NUMBER >= 0x30000000L
- (void)len;
(void)processed;
#endif
@@ -2392,28 +2395,35 @@ static void ssl_io_data_dump(conn_rec *c, server_r
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 real_len = data_len;
+ int len = data_len & APR_INT32_MAX;
+#else
+ apr_size_t real_len = rc;
+ int len = rc & APR_INT32_MAX;
+#endif
const char *dump = "";
if (APLOG_CS_IS_LEVEL(c, s, APLOG_TRACE7)) {
- if (argp != NULL)
+ if (data != NULL)
dump = "(BIO dump follows)";
else
dump = "(Oops, no memory buffer?)";
}
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 "/%d 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"),
- bio, argp, dump);
- if (*dump != '\0' && argp != NULL)
- ssl_io_data_dump(c, s, argp, rc);
+ real_len, argi, (cmd == (BIO_CB_WRITE|BIO_CB_RETURN) ? "to" : "from"),
+ bio, data, dump);
+ if (*dump != '\0' && data != NULL)
+ ssl_io_data_dump(c, s, data, 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,
+ "%s: I/O error %ld, %d bytes expected to %s on BIO#%pp [mem: %pp]",
+ MODSSL_LIBRARY_NAME, (long)rc, argi,
(cmd == (BIO_CB_WRITE|BIO_CB_RETURN) ? "write" : "read"),
- bio, argp);
+ bio, data);
}
}
return rc;
