From: Tomas Mraz Date: Fri, 28 Nov 2025 15:27:29 +0000 (+0100) Subject: Fix change of behavior of the single stapled OCSP response API X-Git-Tag: 3.6-PRE-CLANG-FORMAT-WEBKIT~26 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=61127b01d3873343a140a29e6a3cf69f6fc34259;p=thirdparty%2Fopenssl.git Fix change of behavior of the single stapled OCSP response API Fixes #28888 Fixes b1b4b154 Instead of transferring the ownership of the single OCSP response to the SSL object, the multi-stapling PR modified the semantics of SSL_set_tlsext_status_ocsp_resp() to copying semantics. This change reverts the behavior to the previous one. Partially based on fix by Remi Gacogne: https://github.com/openssl/openssl/pull/28894 Reviewed-by: Matt Caswell Reviewed-by: Saša Nedvědický (Merged from https://github.com/openssl/openssl/pull/29251) (cherry picked from commit 7e50e034b0ca20dc492cfcffb41f974451da8ea9) --- diff --git a/doc/man3/SSL_CTX_set_tlsext_status_cb.pod b/doc/man3/SSL_CTX_set_tlsext_status_cb.pod index 94f74aaeafc..3775cd61509 100644 --- a/doc/man3/SSL_CTX_set_tlsext_status_cb.pod +++ b/doc/man3/SSL_CTX_set_tlsext_status_cb.pod @@ -87,7 +87,8 @@ certificate that is being sent back to the client via a call to SSL_get_certificate(); retrieve the related OCSP response to be sent back; and then set that response data by calling SSL_set_tlsext_status_ocsp_resp(). A pointer to the response data should be provided in the B argument, and -the length of that data should be in the B argument. +the length of that data should be in the B argument. The ownership of +the data is transferred to the B object. In the case of multi-stapling the responses to be returned by the server can be obtained via a call to SSL_get0_tlsext_status_ocsp_resp_ex(). The value B<*resp> diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c index 226493a1615..1c6710e8171 100644 --- a/ssl/s3_lib.c +++ b/ssl/s3_lib.c @@ -3694,14 +3694,13 @@ long ssl3_ctrl(SSL *s, int cmd, long larg, void *parg) ret = 1; #ifndef OPENSSL_NO_OCSP /* - * cleanup single values, which might be set somewhere else - * we only use the extended values + * In case of success keep the single value so we do not need to + * free it immediately. + * However in the handshake code we only use the extended values. */ - if (sc->ext.ocsp.resp != NULL) { - OPENSSL_free(sc->ext.ocsp.resp); - sc->ext.ocsp.resp = NULL; - sc->ext.ocsp.resp_len = 0; - } + OPENSSL_free(sc->ext.ocsp.resp); + sc->ext.ocsp.resp = NULL; + sc->ext.ocsp.resp_len = 0; sk_OCSP_RESPONSE_pop_free(sc->ext.ocsp.resp_ex, OCSP_RESPONSE_free); sc->ext.ocsp.resp_ex = NULL; @@ -3715,6 +3714,9 @@ long ssl3_ctrl(SSL *s, int cmd, long larg, void *parg) resp = d2i_OCSP_RESPONSE(NULL, (const unsigned char **)&p, larg); if (resp != NULL) sk_OCSP_RESPONSE_push(sc->ext.ocsp.resp_ex, resp); + + sc->ext.ocsp.resp = parg; + sc->ext.ocsp.resp_len = larg; } #endif break; diff --git a/test/helpers/handshake.c b/test/helpers/handshake.c index 9b0cbd9bd90..42b1ccc97c1 100644 --- a/test/helpers/handshake.c +++ b/test/helpers/handshake.c @@ -278,15 +278,14 @@ static int server_ocsp_cb(SSL *s, void *arg) resplen = i2d_OCSP_RESPONSE(arg, &respder); /* - * For the purposes of testing we just send back a dummy OCSP response + * For the purposes of testing we just send back a dummy OCSP response. + * This is a set0 kind of function. The ownership is transferred. */ if (!SSL_set_tlsext_status_ocsp_resp(s, respder, resplen)) { OPENSSL_free(respder); return SSL_TLSEXT_ERR_ALERT_FATAL; } - OPENSSL_free(respder); - return SSL_TLSEXT_ERR_OK; } @@ -677,7 +676,7 @@ static int configure_handshake_ctx(SSL_CTX *server_ctx, SSL_CTX *server2_ctx, SSL_CTX_set_tlsext_status_cb(client_ctx, client_ocsp_cb); SSL_CTX_set_tlsext_status_cb(server_ctx, server_ocsp_cb); - SSL_CTX_set_tlsext_status_arg(server_ctx, &dummy_ocsp_resp); + SSL_CTX_set_tlsext_status_arg(server_ctx, dummy_ocsp_resp); break; } diff --git a/test/sslapitest.c b/test/sslapitest.c index df2f8ef2891..370b9b8a373 100644 --- a/test/sslapitest.c +++ b/test/sslapitest.c @@ -1946,12 +1946,11 @@ static int ocsp_server_cb_single(SSL *s, void *arg) resplen = i2d_OCSP_RESPONSE(ocsp_resp, &ocsp_resp_der); OCSP_RESPONSE_free(ocsp_resp); + /* This is a set0 kind of function. The ownership is transferred. */ if (!TEST_true(SSL_set_tlsext_status_ocsp_resp(s, ocsp_resp_der, resplen))) { OPENSSL_free(ocsp_resp_der); return SSL_TLSEXT_ERR_ALERT_FATAL; } - OPENSSL_free(ocsp_resp_der); - ocsp_server_called = 1; return SSL_TLSEXT_ERR_OK; }