commit 559f0644a5feb25c44c9324bf193545f9fc01f65
author Alessandro Ghedini <alessandro@ghedini.me> Wed Dec 07 12:55:32 2016 +0000
committer CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org> Thu Dec 08 20:29:43 2016 +0000
tree ddf12aecb94e5d7c35fa3eed26d6c22feba306c6
parent 7c5728649affe20e2952b11a0aeaf0e7b114aad9

Support setting per-connection OCSP staple

Right now the only way to set an OCSP response is SSL_CTX_set_ocsp_response
however this assumes that all the SSLs generated from a SSL_CTX share the
same OCSP response, which is wrong.

This is similar to the OpenSSL "function" SSL_get_tlsext_status_ocsp_resp,
the main difference being that this doesn't take ownership of the OCSP buffer.

In order to avoid memory duplication in case SSL_CTX has its own response,
a CRYPTO_BUFFER is used for both SSL_CTX and SSL.

Change-Id: I3a0697f82b805ac42a22be9b6bb596aa0b530025
Reviewed-on: https://boringssl-review.googlesource.com/12660
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>

ssl/handshake_server.c

diff --git a/ssl/handshake_server.c b/ssl/handshake_server.c
index c114074..1c1c708 100644
--- a/ssl/handshake_server.c
+++ b/ssl/handshake_server.c
@@ -1143,8 +1143,8 @@
                                  SSL3_MT_CERTIFICATE_STATUS) ||
       !CBB_add_u8(&body, TLSEXT_STATUSTYPE_ocsp) ||
       !CBB_add_u24_length_prefixed(&body, &ocsp_response) ||
-      !CBB_add_bytes(&ocsp_response, ssl->ctx->ocsp_response,
-                     ssl->ctx->ocsp_response_length) ||
+      !CBB_add_bytes(&ocsp_response, CRYPTO_BUFFER_data(ssl->ocsp_response),
+                     CRYPTO_BUFFER_len(ssl->ocsp_response)) ||
       !ssl_complete_message(ssl, &cbb)) {
     OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
     CBB_cleanup(&cbb);

ssl/ssl_lib.c

diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index b0c0906..ac5a7dd 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -364,7 +364,7 @@
   OPENSSL_free(ctx->psk_identity_hint);
   OPENSSL_free(ctx->supported_group_list);
   OPENSSL_free(ctx->alpn_client_proto_list);
-  OPENSSL_free(ctx->ocsp_response);
+  CRYPTO_BUFFER_free(ctx->ocsp_response);
   OPENSSL_free(ctx->signed_cert_timestamp_list);
   EVP_PKEY_free(ctx->tlsext_channel_id_private);
 
@@ -484,6 +484,12 @@
     ssl->session_timeout = ctx->session_timeout;
   }
 
+  /* If the context has an OCSP response, use it. */
+  if (ctx->ocsp_response != NULL) {
+    CRYPTO_BUFFER_up_ref(ctx->ocsp_response);
+    ssl->ocsp_response = ctx->ocsp_response;
+  }
+
   return ssl;
 
 err:
@@ -525,6 +531,7 @@
   OPENSSL_free(ssl->psk_identity_hint);
   sk_X509_NAME_pop_free(ssl->client_CA, X509_NAME_free);
   sk_SRTP_PROTECTION_PROFILE_free(ssl->srtp_profiles);
+  CRYPTO_BUFFER_free(ssl->ocsp_response);
 
   if (ssl->method != NULL) {
     ssl->method->ssl_free(ssl);
@@ -1791,16 +1798,16 @@
 
 int SSL_CTX_set_ocsp_response(SSL_CTX *ctx, const uint8_t *response,
                               size_t response_len) {
-  OPENSSL_free(ctx->ocsp_response);
-  ctx->ocsp_response_length = 0;
+  CRYPTO_BUFFER_free(ctx->ocsp_response);
+  ctx->ocsp_response = CRYPTO_BUFFER_new(response, response_len, NULL);
+  return ctx->ocsp_response != NULL;
+}
 
-  ctx->ocsp_response = BUF_memdup(response, response_len);
-  if (ctx->ocsp_response == NULL) {
-    return 0;
-  }
-  ctx->ocsp_response_length = response_len;
-
-  return 1;
+int SSL_set_ocsp_response(SSL *ssl, const uint8_t *response,
+                          size_t response_len) {
+  CRYPTO_BUFFER_free(ssl->ocsp_response);
+  ssl->ocsp_response = CRYPTO_BUFFER_new(response, response_len, NULL);
+  return ssl->ocsp_response != NULL;
 }
 
 int SSL_set_tlsext_host_name(SSL *ssl, const char *name) {

ssl/t1_lib.c

diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index 3530ff5..086af3c 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -1212,7 +1212,7 @@
   SSL *const ssl = hs->ssl;
   if (ssl3_protocol_version(ssl) >= TLS1_3_VERSION ||
       !hs->ocsp_stapling_requested ||
-      ssl->ctx->ocsp_response_length == 0 ||
+      ssl->ocsp_response == NULL ||
       ssl->s3->session_reused ||
       !ssl_cipher_uses_certificate_auth(ssl->s3->tmp.new_cipher)) {
     return 1;

ssl/test/bssl_shim.cc

diff --git a/ssl/test/bssl_shim.cc b/ssl/test/bssl_shim.cc
index b7f9c9d..dbfaf10 100644
--- a/ssl/test/bssl_shim.cc
+++ b/ssl/test/bssl_shim.cc
@@ -398,9 +398,8 @@
     return false;
   }
   if (!config->ocsp_response.empty() &&
-      !SSL_CTX_set_ocsp_response(SSL_get_SSL_CTX(ssl),
-                                 (const uint8_t *)config->ocsp_response.data(),
-                                 config->ocsp_response.size())) {
+      !SSL_set_ocsp_response(ssl, (const uint8_t *)config->ocsp_response.data(),
+                             config->ocsp_response.size())) {
     return false;
   }
   return true;

ssl/tls13_both.c

diff --git a/ssl/tls13_both.c b/ssl/tls13_both.c
index ea3eb77..1a1d8b9 100644
--- a/ssl/tls13_both.c
+++ b/ssl/tls13_both.c
@@ -466,14 +466,14 @@
   }
 
   if (hs->ocsp_stapling_requested &&
-      ssl->ctx->ocsp_response_length != 0) {
+      ssl->ocsp_response != NULL) {
     CBB contents, ocsp_response;
     if (!CBB_add_u16(&extensions, TLSEXT_TYPE_status_request) ||
         !CBB_add_u16_length_prefixed(&extensions, &contents) ||
         !CBB_add_u8(&contents, TLSEXT_STATUSTYPE_ocsp) ||
         !CBB_add_u24_length_prefixed(&contents, &ocsp_response) ||
-        !CBB_add_bytes(&ocsp_response, ssl->ctx->ocsp_response,
-                       ssl->ctx->ocsp_response_length) ||
+        !CBB_add_bytes(&ocsp_response, CRYPTO_BUFFER_data(ssl->ocsp_response),
+                       CRYPTO_BUFFER_len(ssl->ocsp_response)) ||
         !CBB_flush(&extensions)) {
       OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
       goto err;