I noticed that serf does not support BIO_CTRL_EOF in its bio implementation. The lack of such support can cause incorrect results if this ctrl is called by OpenSSL. For example, OpenSSL 1.1.1e [1] and OpenSSL 3.0+ [2] have such EOF checks.
The attached patch implements support for this ctrl. I have splitted the implementation for SSL buckets and APR file wrappers, to allow for separate hit eof logic. [1] https://github.com/openssl/openssl/commit/db943f43a60d1b5b1277e4b5317e8f288e7a0a3a [2] https://github.com/openssl/openssl/commit/d924dbf4ae127c68463bcbece04b6e06abc58928 Kind Regards, Denis Kovalchuk
Add support for BIO_CTRL_EOF and implement the hit eof logic for ssl buckets and APR files. Potentially OpenSSL can check BIOs for EOF. For example, OpenSSL 1.1.1e [1] and OpenSSL 3.0+ [2] have such checks. [1] https://github.com/openssl/openssl/commit/db943f43a60d1b5b1277e4b5317e8f288e7a0a3a [2] https://github.com/openssl/openssl/commit/d924dbf4ae127c68463bcbece04b6e06abc58928 * test/MockHTTPinC/MockHTTP_server.c: (sslCtx_t): New hit_eof variable. (bio_apr_socket_ctrl): Rework to support BIO_CTRL_EOF. (bio_apr_socket_read): Properly set the hit_eof variable. (initSSLCtx): Init the hit_eof variable. * buckets/ssl_buckets.c: (serf_ssl_context_t): New hit_eof variable. (bio_bucket_read): Properly set the hit_eof variable. (bio_bucket_ctrl): Rework to support BIO_CTRL_EOF. (bio_file_ctrl): New ctrl function for APR files to support BIO_CTRL_EOF. (bio_file_method, bio_meth_file_new): Use the new bio_file_ctrl(). (ssl_init_context): Init the hit_eof variable. Index: test/MockHTTPinC/MockHTTP_server.c =================================================================== --- test/MockHTTPinC/MockHTTP_server.c (revision 1902009) +++ test/MockHTTPinC/MockHTTP_server.c (working copy) @@ -2242,6 +2242,7 @@ mhSetServerEnableOCSP(mhServCtx_t *ctx) struct sslCtx_t { bool handshake_done; bool renegotiate; + bool hit_eof; apr_status_t bio_status; SSL_CTX* ctx; @@ -2324,21 +2325,25 @@ static int bio_apr_socket_destroy(BIO *bio) */ static long bio_apr_socket_ctrl(BIO *bio, int cmd, long num, void *ptr) { - long ret = 1; + _mhClientCtx_t *cctx = bio_get_data(bio); + sslCtx_t *ssl_ctx = cctx->ssl_ctx; switch (cmd) { - default: - /* abort(); */ - break; case BIO_CTRL_FLUSH: /* At this point we can't force a flush. */ - break; + return 1; case BIO_CTRL_PUSH: case BIO_CTRL_POP: - ret = 0; - break; + return 0; + case BIO_CTRL_EOF: + if (ssl_ctx->hit_eof == YES) + return 1; + else + return 0; + default: + /* abort(); */ + return 1; } - return ret; } /** @@ -2360,7 +2365,9 @@ static int bio_apr_socket_read(BIO *bio, char *in, _mhLog(MH_VERBOSE, cctx->skt, "Read %d bytes from ssl socket with " "status %d.\n", len, status); - if (APR_STATUS_IS_EAGAIN(status)) { + if (APR_STATUS_IS_EOF(status)) { + ssl_ctx->hit_eof = YES; + } else if (APR_STATUS_IS_EAGAIN(status)) { BIO_set_retry_read(bio); } @@ -2661,6 +2668,7 @@ static apr_status_t initSSLCtx(_mhClientCtx_t *cct { sslCtx_t *ssl_ctx = apr_pcalloc(cctx->pool, sizeof(*ssl_ctx)); cctx->ssl_ctx = ssl_ctx; + ssl_ctx->hit_eof = NO; ssl_ctx->bio_status = APR_SUCCESS; _mhLog(MH_VERBOSE, cctx->skt, "Initializing SSL context.\n"); Index: buckets/ssl_buckets.c =================================================================== --- buckets/ssl_buckets.c (revision 1902009) +++ buckets/ssl_buckets.c (working copy) @@ -161,6 +161,9 @@ struct serf_ssl_context_t { /* Should we read before we can write again? */ int want_read; int handshake_done; + /* OpenSSL 1.1.1e introduced BIO_FLAGS_IN_EOF, but we implement + our own hit eof to support versions < 1.1.1e. */ + int hit_eof; /* Client cert callbacks */ serf_ssl_need_client_cert_t cert_callback; @@ -383,7 +386,9 @@ static int bio_bucket_read(BIO *bio, char *in, int return -1; /* Raises: SSL_ERROR_SYSCALL; caller reads crypt_status */ } - if (status && !APR_STATUS_IS_EOF(status)) { + if (APR_STATUS_IS_EOF(status)) { + ctx->hit_eof = TRUE; + } else if (status) { BIO_set_retry_read(bio); /* Signal SSL: Retry later */ } @@ -506,21 +511,43 @@ static int bio_bucket_destroy(BIO *bio) static long bio_bucket_ctrl(BIO *bio, int cmd, long num, void *ptr) { - long ret = 1; + serf_ssl_context_t *ctx = bio_get_data(bio); switch (cmd) { + case BIO_CTRL_FLUSH: + /* At this point we can't force a flush. */ + return 1; + case BIO_CTRL_PUSH: + case BIO_CTRL_POP: + return 0; + case BIO_CTRL_EOF: + return ctx->hit_eof; default: /* abort(); */ - break; + return 1; + } +} + +static long bio_file_ctrl(BIO *bio, int cmd, long num, void *ptr) +{ + apr_file_t *file = bio_get_data(bio); + + switch (cmd) { case BIO_CTRL_FLUSH: /* At this point we can't force a flush. */ - break; + return 1; case BIO_CTRL_PUSH: case BIO_CTRL_POP: - ret = 0; - break; + return 0; + case BIO_CTRL_EOF: + if (apr_file_eof(file) == APR_EOF) + return 1; + else + return 0; + default: + /* abort(); */ + return 1; } - return ret; } #ifdef SERF_NO_SSL_BIO_WRAPPERS @@ -546,7 +573,7 @@ static BIO_METHOD bio_file_method = { bio_file_read, NULL, /* Is this called? */ bio_file_gets, /* Is this called? */ - bio_bucket_ctrl, + bio_file_ctrl, bio_bucket_create, bio_bucket_destroy, #ifdef OPENSSL_VERSION_NUMBER @@ -586,7 +613,7 @@ static BIO_METHOD *bio_meth_file_new(void) BIO_meth_set_write(biom, bio_file_write); BIO_meth_set_read(biom, bio_file_read); BIO_meth_set_gets(biom, bio_file_gets); - BIO_meth_set_ctrl(biom, bio_bucket_ctrl); + BIO_meth_set_ctrl(biom, bio_file_ctrl); BIO_meth_set_create(biom, bio_bucket_create); BIO_meth_set_destroy(biom, bio_bucket_destroy); } @@ -1820,6 +1847,7 @@ static serf_ssl_context_t *ssl_init_context(serf_b ssl_ctx->crypt_status = APR_SUCCESS; ssl_ctx->want_read = FALSE; ssl_ctx->handshake_done = FALSE; + ssl_ctx->hit_eof = FALSE; return ssl_ctx; }