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 <matt@openssl.org>
Reviewed-by: Saša Nedvědický <sashan@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/29251)

diff --git a/doc/man3/SSL_CTX_set_tlsext_status_cb.pod b/doc/man3/SSL_CTX_set_tlsext_status_cb.pod
index 94f74aaeafc0ee4530d9bb82fefa3f0f504392f3..3775cd61509522640cc66371a3f623ef3e65c394 100644 (file)
--- 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<resp> argument, and
-the length of that data should be in the B<len> argument.
+the length of that data should be in the B<len> argument. The ownership of
+the data is transferred to the B<ssl> 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 226493a1615abc8e238db76f7eee1d0b947435eb..1c6710e81712c898e270cde06b2401864b0cba69 100644 (file)
--- 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 9b0cbd9bd90314ad4794b554d73e06fc47053998..42b1ccc97c1312811955b6cc7b260b337a178c0c 100644 (file)
--- 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 df2f8ef289159c3c6fd01f3034fafd2129ceca8c..370b9b8a373f3bbb74ca5094bbea3e100189cb69 100644 (file)
--- 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;
 }