Move SCT lists and OCSP responses to CERT.
Recent changes added SSL-level setters to these APIs. Unfortunately,
this has the side effect of breaking SSL_set_SSL_CTX, which is how SNI
is typically handled. SSL_set_SSL_CTX is kind of a weird function in
that it's very sensitive to which of the hodge-podge of config styles is
in use. I previously listed out all the config styles here, but it was
long and unhelpful. (I counted up to 7.)
Of the various SSL_set_SSL_CTX-visible config styles, the sanest seems
to be to move it to CERT. In this case, it's actually quite reasonable
since they're very certificate-related.
Later we may wish to think about whether we can cut down all 7 kinds of
config styles because this is kinda nuts. I'm wondering we should do
CERT => SSL_CONFIG, move everything there, and make that be the same
structure that is dropped post-handshake (supposing the caller has
disavowed SSL_clear and renego). Fruit for later thought. (Note though
that comes with a behavior change for all the existing config.)
Change-Id: I9aa47d8bd37bf2847869e0b577739d4d579ee4ae
Reviewed-on: https://boringssl-review.googlesource.com/13864
Reviewed-by: Martin Kreichgauer <martinkr@google.com>
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>
diff --git a/ssl/handshake_server.c b/ssl/handshake_server.c
index 01a1aad..d3a32ef 100644
--- a/ssl/handshake_server.c
+++ b/ssl/handshake_server.c
@@ -1118,8 +1118,9 @@
SSL3_MT_CERTIFICATE_STATUS) ||
!CBB_add_u8(&body, TLSEXT_STATUSTYPE_ocsp) ||
!CBB_add_u24_length_prefixed(&body, &ocsp_response) ||
- !CBB_add_bytes(&ocsp_response, CRYPTO_BUFFER_data(ssl->ocsp_response),
- CRYPTO_BUFFER_len(ssl->ocsp_response)) ||
+ !CBB_add_bytes(&ocsp_response,
+ CRYPTO_BUFFER_data(ssl->cert->ocsp_response),
+ CRYPTO_BUFFER_len(ssl->cert->ocsp_response)) ||
!ssl_add_message_cbb(ssl, &cbb)) {
OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
CBB_cleanup(&cbb);
diff --git a/ssl/internal.h b/ssl/internal.h
index 5b93f47..d671308 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -1323,6 +1323,12 @@
/* Optional X509_STORE for certificate validation. If NULL the parent SSL_CTX
* store is used instead. */
X509_STORE *verify_store;
+
+ /* Signed certificate timestamp list to be sent to the client, if requested */
+ CRYPTO_BUFFER *signed_cert_timestamp_list;
+
+ /* OCSP response to be sent to the client, if requested. */
+ CRYPTO_BUFFER *ocsp_response;
} CERT;
/* SSL_METHOD is a compatibility structure to support the legacy version-locked
@@ -1887,12 +1893,6 @@
* hash of the peer's certificate and then discard it to save memory and
* session space. Only effective on the server side. */
unsigned retain_only_sha256_of_client_certs:1;
-
- /* Signed certificate timestamp list to be sent to the client, if requested */
- CRYPTO_BUFFER *signed_cert_timestamp_list;
-
- /* OCSP response to be sent to the client, if requested. */
- CRYPTO_BUFFER *ocsp_response;
};
/* From draft-ietf-tls-tls13-18, used in determining PSK modes. */
diff --git a/ssl/ssl_cert.c b/ssl/ssl_cert.c
index 4177a48..c94ca49 100644
--- a/ssl/ssl_cert.c
+++ b/ssl/ssl_cert.c
@@ -203,6 +203,16 @@
ret->verify_store = cert->verify_store;
}
+ if (cert->signed_cert_timestamp_list != NULL) {
+ CRYPTO_BUFFER_up_ref(cert->signed_cert_timestamp_list);
+ ret->signed_cert_timestamp_list = cert->signed_cert_timestamp_list;
+ }
+
+ if (cert->ocsp_response != NULL) {
+ CRYPTO_BUFFER_up_ref(cert->ocsp_response);
+ ret->ocsp_response = cert->ocsp_response;
+ }
+
return ret;
err:
@@ -235,6 +245,8 @@
ssl_cert_clear_certs(c);
OPENSSL_free(c->sigalgs);
X509_STORE_free(c->verify_store);
+ CRYPTO_BUFFER_free(c->signed_cert_timestamp_list);
+ CRYPTO_BUFFER_free(c->ocsp_response);
OPENSSL_free(c);
}
@@ -956,3 +968,42 @@
SSL_CTX_set_cert_cb(ctx, do_client_cert_cb, NULL);
ctx->client_cert_cb = cb;
}
+
+static int set_signed_cert_timestamp_list(CERT *cert, const uint8_t *list,
+ size_t list_len) {
+ CBS sct_list;
+ CBS_init(&sct_list, list, list_len);
+ if (!ssl_is_sct_list_valid(&sct_list)) {
+ OPENSSL_PUT_ERROR(SSL, SSL_R_INVALID_SCT_LIST);
+ return 0;
+ }
+
+ CRYPTO_BUFFER_free(cert->signed_cert_timestamp_list);
+ cert->signed_cert_timestamp_list =
+ CRYPTO_BUFFER_new(CBS_data(&sct_list), CBS_len(&sct_list), NULL);
+ return cert->signed_cert_timestamp_list != NULL;
+}
+
+int SSL_CTX_set_signed_cert_timestamp_list(SSL_CTX *ctx, const uint8_t *list,
+ size_t list_len) {
+ return set_signed_cert_timestamp_list(ctx->cert, list, list_len);
+}
+
+int SSL_set_signed_cert_timestamp_list(SSL *ssl, const uint8_t *list,
+ size_t list_len) {
+ return set_signed_cert_timestamp_list(ssl->cert, list, list_len);
+}
+
+int SSL_CTX_set_ocsp_response(SSL_CTX *ctx, const uint8_t *response,
+ size_t response_len) {
+ CRYPTO_BUFFER_free(ctx->cert->ocsp_response);
+ ctx->cert->ocsp_response = CRYPTO_BUFFER_new(response, response_len, NULL);
+ return ctx->cert->ocsp_response != NULL;
+}
+
+int SSL_set_ocsp_response(SSL *ssl, const uint8_t *response,
+ size_t response_len) {
+ CRYPTO_BUFFER_free(ssl->cert->ocsp_response);
+ ssl->cert->ocsp_response = CRYPTO_BUFFER_new(response, response_len, NULL);
+ return ssl->cert->ocsp_response != NULL;
+}
diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index 5a26681..5ba8256 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -363,8 +363,6 @@
OPENSSL_free(ctx->psk_identity_hint);
OPENSSL_free(ctx->supported_group_list);
OPENSSL_free(ctx->alpn_client_proto_list);
- CRYPTO_BUFFER_free(ctx->signed_cert_timestamp_list);
- CRYPTO_BUFFER_free(ctx->ocsp_response);
EVP_PKEY_free(ctx->tlsext_channel_id_private);
OPENSSL_free(ctx);
@@ -472,18 +470,6 @@
ssl->signed_cert_timestamps_enabled = ctx->signed_cert_timestamps_enabled;
ssl->ocsp_stapling_enabled = ctx->ocsp_stapling_enabled;
- /* If the context has an SCT list, use it. */
- if (ctx->signed_cert_timestamp_list != NULL) {
- CRYPTO_BUFFER_up_ref(ctx->signed_cert_timestamp_list);
- ssl->signed_cert_timestamp_list = ctx->signed_cert_timestamp_list;
- }
-
- /* 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:
@@ -522,8 +508,6 @@
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->signed_cert_timestamp_list);
- CRYPTO_BUFFER_free(ssl->ocsp_response);
if (ssl->method != NULL) {
ssl->method->ssl_free(ssl);
@@ -1624,52 +1608,6 @@
*out_len = session->ocsp_response_length;
}
-int SSL_CTX_set_signed_cert_timestamp_list(SSL_CTX *ctx, const uint8_t *list,
- size_t list_len) {
- CBS sct_list;
- CBS_init(&sct_list, list, list_len);
- if (!ssl_is_sct_list_valid(&sct_list)) {
- OPENSSL_PUT_ERROR(SSL, SSL_R_INVALID_SCT_LIST);
- return 0;
- }
-
- CRYPTO_BUFFER_free(ctx->signed_cert_timestamp_list);
- ctx->signed_cert_timestamp_list = CRYPTO_BUFFER_new(CBS_data(&sct_list),
- CBS_len(&sct_list),
- NULL);
- return ctx->signed_cert_timestamp_list != NULL;
-}
-
-int SSL_set_signed_cert_timestamp_list(SSL *ssl, const uint8_t *list,
- size_t list_len) {
- CBS sct_list;
- CBS_init(&sct_list, list, list_len);
- if (!ssl_is_sct_list_valid(&sct_list)) {
- OPENSSL_PUT_ERROR(SSL, SSL_R_INVALID_SCT_LIST);
- return 0;
- }
-
- CRYPTO_BUFFER_free(ssl->signed_cert_timestamp_list);
- ssl->signed_cert_timestamp_list = CRYPTO_BUFFER_new(CBS_data(&sct_list),
- CBS_len(&sct_list),
- NULL);
- return ssl->signed_cert_timestamp_list != NULL;
-}
-
-int SSL_CTX_set_ocsp_response(SSL_CTX *ctx, const uint8_t *response,
- size_t response_len) {
- CRYPTO_BUFFER_free(ctx->ocsp_response);
- ctx->ocsp_response = CRYPTO_BUFFER_new(response, response_len, NULL);
- return ctx->ocsp_response != NULL;
-}
-
-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) {
OPENSSL_free(ssl->tlsext_hostname);
ssl->tlsext_hostname = NULL;
diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc
index dfab976..4e0c274 100644
--- a/ssl/ssl_test.cc
+++ b/ssl/ssl_test.cc
@@ -2415,6 +2415,9 @@
// Test that switching the |SSL_CTX| at the SNI callback behaves correctly.
static const uint16_t kECDSAWithSHA256 = SSL_SIGN_ECDSA_SECP256R1_SHA256;
+ static const uint8_t kSCTList[] = {0, 6, 0, 4, 5, 6, 7, 8};
+ static const uint8_t kOCSPResponse[] = {1, 2, 3, 4};
+
bssl::UniquePtr<SSL_CTX> server_ctx(SSL_CTX_new(method));
bssl::UniquePtr<SSL_CTX> server_ctx2(SSL_CTX_new(method));
bssl::UniquePtr<SSL_CTX> client_ctx(SSL_CTX_new(method));
@@ -2423,6 +2426,10 @@
!SSL_CTX_use_PrivateKey(server_ctx.get(), key.get()) ||
!SSL_CTX_use_certificate(server_ctx2.get(), cert2.get()) ||
!SSL_CTX_use_PrivateKey(server_ctx2.get(), key2.get()) ||
+ !SSL_CTX_set_signed_cert_timestamp_list(server_ctx2.get(), kSCTList,
+ sizeof(kSCTList)) ||
+ !SSL_CTX_set_ocsp_response(server_ctx2.get(), kOCSPResponse,
+ sizeof(kOCSPResponse)) ||
// Historically signing preferences would be lost in some cases with the
// SNI callback, which triggers the TLS 1.2 SHA-1 default. To ensure
// this doesn't happen when |version| is TLS 1.2, configure the private
@@ -2441,6 +2448,9 @@
SSL_CTX_set_tlsext_servername_callback(server_ctx.get(), SwitchContext);
SSL_CTX_set_tlsext_servername_arg(server_ctx.get(), server_ctx2.get());
+ SSL_CTX_enable_signed_cert_timestamps(client_ctx.get());
+ SSL_CTX_enable_ocsp_stapling(client_ctx.get());
+
bssl::UniquePtr<SSL> client, server;
if (!ConnectClientAndServer(&client, &server, client_ctx.get(),
server_ctx.get(), nullptr)) {
@@ -2455,6 +2465,22 @@
return false;
}
+ // The client should have received |server_ctx2|'s SCT list.
+ const uint8_t *data;
+ size_t len;
+ SSL_get0_signed_cert_timestamp_list(client.get(), &data, &len);
+ if (Bytes(kSCTList) != Bytes(data, len)) {
+ fprintf(stderr, "Incorrect SCT list received.\n");
+ return false;
+ }
+
+ // The client should have received |server_ctx2|'s OCSP response.
+ SSL_get0_ocsp_response(client.get(), &data, &len);
+ if (Bytes(kOCSPResponse) != Bytes(data, len)) {
+ fprintf(stderr, "Incorrect OCSP response received.\n");
+ return false;
+ }
+
return true;
}
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index 7723ccd..eadaff9 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -1152,7 +1152,7 @@
SSL *const ssl = hs->ssl;
if (ssl3_protocol_version(ssl) >= TLS1_3_VERSION ||
!hs->ocsp_stapling_requested ||
- ssl->ocsp_response == NULL ||
+ ssl->cert->ocsp_response == NULL ||
ssl->s3->session_reused ||
!ssl_cipher_uses_certificate_auth(ssl->s3->tmp.new_cipher)) {
return 1;
@@ -1371,16 +1371,17 @@
/* The extension shouldn't be sent when resuming sessions. */
if (ssl3_protocol_version(ssl) >= TLS1_3_VERSION ||
ssl->s3->session_reused ||
- ssl->signed_cert_timestamp_list == NULL) {
+ ssl->cert->signed_cert_timestamp_list == NULL) {
return 1;
}
CBB contents;
return CBB_add_u16(out, TLSEXT_TYPE_certificate_timestamp) &&
CBB_add_u16_length_prefixed(out, &contents) &&
- CBB_add_bytes(&contents,
- CRYPTO_BUFFER_data(ssl->signed_cert_timestamp_list),
- CRYPTO_BUFFER_len(ssl->signed_cert_timestamp_list)) &&
+ CBB_add_bytes(
+ &contents,
+ CRYPTO_BUFFER_data(ssl->cert->signed_cert_timestamp_list),
+ CRYPTO_BUFFER_len(ssl->cert->signed_cert_timestamp_list)) &&
CBB_flush(out);
}
diff --git a/ssl/tls13_both.c b/ssl/tls13_both.c
index 19dd555..063652e 100644
--- a/ssl/tls13_both.c
+++ b/ssl/tls13_both.c
@@ -452,13 +452,14 @@
goto err;
}
- if (hs->scts_requested && ssl->signed_cert_timestamp_list != NULL) {
+ if (hs->scts_requested && ssl->cert->signed_cert_timestamp_list != NULL) {
CBB contents;
if (!CBB_add_u16(&extensions, TLSEXT_TYPE_certificate_timestamp) ||
!CBB_add_u16_length_prefixed(&extensions, &contents) ||
- !CBB_add_bytes(&contents,
- CRYPTO_BUFFER_data(ssl->signed_cert_timestamp_list),
- CRYPTO_BUFFER_len(ssl->signed_cert_timestamp_list)) ||
+ !CBB_add_bytes(
+ &contents,
+ CRYPTO_BUFFER_data(ssl->cert->signed_cert_timestamp_list),
+ CRYPTO_BUFFER_len(ssl->cert->signed_cert_timestamp_list)) ||
!CBB_flush(&extensions)) {
OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
goto err;
@@ -466,14 +467,15 @@
}
if (hs->ocsp_stapling_requested &&
- ssl->ocsp_response != NULL) {
+ ssl->cert->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, CRYPTO_BUFFER_data(ssl->ocsp_response),
- CRYPTO_BUFFER_len(ssl->ocsp_response)) ||
+ !CBB_add_bytes(&ocsp_response,
+ CRYPTO_BUFFER_data(ssl->cert->ocsp_response),
+ CRYPTO_BUFFER_len(ssl->cert->ocsp_response)) ||
!CBB_flush(&extensions)) {
OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
goto err;
