The branch openssl-3.0 has been updated via e0314df5f21dd537602d4ea8d9272a21aac66356 (commit) from fbadef597c906711d82d8bfd9c4d5276ea981db7 (commit)
- Log ----------------------------------------------------------------- commit e0314df5f21dd537602d4ea8d9272a21aac66356 Author: Dr. David von Oheimb <david.von.ohe...@siemens.com> Date: Sun Nov 21 20:55:35 2021 +0100 HTTP client: Fix cleanup of TLS BIO via 'bio_update_fn' callback function Make app_http_tls_cb() tidy up on disconnect the SSL BIO it pushes on connect. Make OSSL_HTTP_close() respect this. Reviewed-by: Paul Dale <pa...@openssl.org> (Merged from https://github.com/openssl/openssl/pull/17318) (cherry picked from commit cdaf072f90399efb9e8e19ee4f387d1425f12274) ----------------------------------------------------------------------- Summary of changes: apps/lib/apps.c | 33 ++++++++++++++++++++------------- crypto/http/http_client.c | 12 +++++++++--- doc/man3/OSSL_HTTP_transfer.pod | 18 +++++++++++++----- 3 files changed, 42 insertions(+), 21 deletions(-) diff --git a/apps/lib/apps.c b/apps/lib/apps.c index e01633c5b5..6a762b7668 100644 --- a/apps/lib/apps.c +++ b/apps/lib/apps.c @@ -2442,7 +2442,7 @@ static const char *tls_error_hint(void) } /* HTTP callback function that supports TLS connection also via HTTPS proxy */ -BIO *app_http_tls_cb(BIO *hbio, void *arg, int connect, int detail) +BIO *app_http_tls_cb(BIO *bio, void *arg, int connect, int detail) { if (connect && detail) { /* connecting with TLS */ APP_HTTP_TLS_INFO *info = (APP_HTTP_TLS_INFO *)arg; @@ -2451,7 +2451,7 @@ BIO *app_http_tls_cb(BIO *hbio, void *arg, int connect, int detail) BIO *sbio = NULL; if ((info->use_proxy - && !OSSL_HTTP_proxy_connect(hbio, info->server, info->port, + && !OSSL_HTTP_proxy_connect(bio, info->server, info->port, NULL, NULL, /* no proxy credentials */ info->timeout, bio_err, opt_getprog())) || (sbio = BIO_new(BIO_f_ssl())) == NULL) { @@ -2467,18 +2467,25 @@ BIO *app_http_tls_cb(BIO *hbio, void *arg, int connect, int detail) SSL_set_connect_state(ssl); BIO_set_ssl(sbio, ssl, BIO_CLOSE); - hbio = BIO_push(sbio, hbio); - } else if (!connect && !detail) { /* disconnecting after error */ - const char *hint = tls_error_hint(); - - if (hint != NULL) - ERR_add_error_data(2, " : ", hint); - /* - * If we pop sbio and BIO_free() it this may lead to libssl double free. - * Rely on BIO_free_all() done by OSSL_HTTP_transfer() in http_client.c - */ + bio = BIO_push(sbio, bio); } - return hbio; + if (!connect) { + const char *hint; + BIO *cbio; + + if (!detail) { /* disconnecting after error */ + hint = tls_error_hint(); + if (hint != NULL) + ERR_add_error_data(2, " : ", hint); + } + (void)ERR_set_mark(); + BIO_ssl_shutdown(bio); + cbio = BIO_pop(bio); /* connect+HTTP BIO */ + BIO_free(bio); /* SSL BIO */ + (void)ERR_pop_to_mark(); /* hide SSL_R_READ_BIO_NOT_SET etc. */ + bio = cbio; + } + return bio; } void APP_HTTP_TLS_INFO_free(APP_HTTP_TLS_INFO *info) diff --git a/crypto/http/http_client.c b/crypto/http/http_client.c index 7f8d8fc8d7..c80d4fe519 100644 --- a/crypto/http/http_client.c +++ b/crypto/http/http_client.c @@ -1197,11 +1197,17 @@ BIO *OSSL_HTTP_transfer(OSSL_HTTP_REQ_CTX **prctx, int OSSL_HTTP_close(OSSL_HTTP_REQ_CTX *rctx, int ok) { + BIO *wbio; int ret = 1; - /* callback can be used to clean up TLS session on disconnect */ - if (rctx != NULL && rctx->upd_fn != NULL) - ret = (*rctx->upd_fn)(rctx->wbio, rctx->upd_arg, 0, ok) != NULL; + /* callback can be used to finish TLS session and free its BIO */ + if (rctx != NULL && rctx->upd_fn != NULL) { + wbio = (*rctx->upd_fn)(rctx->wbio, rctx->upd_arg, + 0 /* disconnect */, ok); + ret = wbio != NULL; + if (ret) + rctx->wbio = wbio; + } OSSL_HTTP_REQ_CTX_free(rctx); return ret; } diff --git a/doc/man3/OSSL_HTTP_transfer.pod b/doc/man3/OSSL_HTTP_transfer.pod index 7fcd71dbe0..7e823db3ea 100644 --- a/doc/man3/OSSL_HTTP_transfer.pod +++ b/doc/man3/OSSL_HTTP_transfer.pod @@ -113,17 +113,25 @@ or NULL to indicate failure, in which case it should not modify the BIO. Here is a simple example that supports TLS connections (but not via a proxy): - BIO *http_tls_cb(BIO *hbio, void *arg, int connect, int detail) + BIO *http_tls_cb(BIO *bio, void *arg, int connect, int detail) { if (connect && detail) { /* connecting with TLS */ SSL_CTX *ctx = (SSL_CTX *)arg; BIO *sbio = BIO_new_ssl(ctx, 1); - hbio = sbio != NULL ? BIO_push(sbio, hbio) : NULL; - } else if (!connect && !detail) { /* disconnecting after error */ - /* optionally add diagnostics here */ + bio = sbio != NULL ? BIO_push(sbio, bio) : NULL; + } else if (!connect) { /* disconnecting */ + BIO *hbio; + + if (!detail) { /* an error has occurred */ + /* optionally add diagnostics here */ + } + BIO_ssl_shutdown(bio); + hbio = BIO_pop(bio); + BIO_free(bio); /* SSL BIO */ + bio = hbio; } - return hbio; + return bio; } After disconnect the modified BIO will be deallocated using BIO_free_all().