Put SCTs and OCSP responses in CRYPTO_BUFFERs.
They both can be moderately large. This should hopefully relieve a little
memory pressure from both connections to hosts which serve SCTs and
TLS 1.3's single-use tickets.
Change-Id: I034bbf057fe5a064015a0f554b3ae9ea7797cd4e
Reviewed-on: https://boringssl-review.googlesource.com/19584
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
diff --git a/ssl/handshake_client.cc b/ssl/handshake_client.cc
index dd09797..e3c4641 100644
--- a/ssl/handshake_client.cc
+++ b/ssl/handshake_client.cc
@@ -1162,9 +1162,10 @@
return -1;
}
- if (!CBS_stow(&ocsp_response, &hs->new_session->ocsp_response,
- &hs->new_session->ocsp_response_length)) {
- OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE);
+ CRYPTO_BUFFER_free(hs->new_session->ocsp_response);
+ hs->new_session->ocsp_response =
+ CRYPTO_BUFFER_new_from_CBS(&ocsp_response, ssl->ctx->pool);
+ if (hs->new_session->ocsp_response == nullptr) {
ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR);
return -1;
}
diff --git a/ssl/ssl_asn1.cc b/ssl/ssl_asn1.cc
index 0cf90d1..b87c795 100644
--- a/ssl/ssl_asn1.cc
+++ b/ssl/ssl_asn1.cc
@@ -311,20 +311,22 @@
}
}
- if (in->tlsext_signed_cert_timestamp_list_length > 0) {
+ if (in->signed_cert_timestamp_list != nullptr) {
if (!CBB_add_asn1(&session, &child, kSignedCertTimestampListTag) ||
!CBB_add_asn1(&child, &child2, CBS_ASN1_OCTETSTRING) ||
- !CBB_add_bytes(&child2, in->tlsext_signed_cert_timestamp_list,
- in->tlsext_signed_cert_timestamp_list_length)) {
+ !CBB_add_bytes(&child2,
+ CRYPTO_BUFFER_data(in->signed_cert_timestamp_list),
+ CRYPTO_BUFFER_len(in->signed_cert_timestamp_list))) {
OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE);
return 0;
}
}
- if (in->ocsp_response_length > 0) {
+ if (in->ocsp_response != nullptr) {
if (!CBB_add_asn1(&session, &child, kOCSPResponseTag) ||
!CBB_add_asn1(&child, &child2, CBS_ASN1_OCTETSTRING) ||
- !CBB_add_bytes(&child2, in->ocsp_response, in->ocsp_response_length)) {
+ !CBB_add_bytes(&child2, CRYPTO_BUFFER_data(in->ocsp_response),
+ CRYPTO_BUFFER_len(in->ocsp_response))) {
OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE);
return 0;
}
@@ -470,6 +472,29 @@
return 1;
}
+static int SSL_SESSION_parse_crypto_buffer(CBS *cbs, CRYPTO_BUFFER **out,
+ unsigned tag,
+ CRYPTO_BUFFER_POOL *pool) {
+ if (!CBS_peek_asn1_tag(cbs, tag)) {
+ return 1;
+ }
+
+ CBS child, value;
+ if (!CBS_get_asn1(cbs, &child, tag) ||
+ !CBS_get_asn1(&child, &value, CBS_ASN1_OCTETSTRING) ||
+ CBS_len(&child) != 0) {
+ OPENSSL_PUT_ERROR(SSL, SSL_R_INVALID_SSL_SESSION);
+ return 0;
+ }
+ CRYPTO_BUFFER_free(*out);
+ *out = CRYPTO_BUFFER_new_from_CBS(&value, pool);
+ if (*out == nullptr) {
+ OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE);
+ return 0;
+ }
+ return 1;
+}
+
/* SSL_SESSION_parse_bounded_octet_string parses an optional ASN.1 OCTET STRING
* explicitly tagged with |tag| of size at most |max_out|. */
static int SSL_SESSION_parse_bounded_octet_string(
@@ -635,13 +660,11 @@
&session, ret->original_handshake_hash,
&ret->original_handshake_hash_len,
sizeof(ret->original_handshake_hash), kOriginalHandshakeHashTag) ||
- !SSL_SESSION_parse_octet_string(
- &session, &ret->tlsext_signed_cert_timestamp_list,
- &ret->tlsext_signed_cert_timestamp_list_length,
- kSignedCertTimestampListTag) ||
- !SSL_SESSION_parse_octet_string(
- &session, &ret->ocsp_response, &ret->ocsp_response_length,
- kOCSPResponseTag)) {
+ !SSL_SESSION_parse_crypto_buffer(&session,
+ &ret->signed_cert_timestamp_list,
+ kSignedCertTimestampListTag, pool) ||
+ !SSL_SESSION_parse_crypto_buffer(&session, &ret->ocsp_response,
+ kOCSPResponseTag, pool)) {
return nullptr;
}
diff --git a/ssl/ssl_lib.cc b/ssl/ssl_lib.cc
index 026e218..32ec272 100644
--- a/ssl/ssl_lib.cc
+++ b/ssl/ssl_lib.cc
@@ -1811,28 +1811,27 @@
void SSL_get0_signed_cert_timestamp_list(const SSL *ssl, const uint8_t **out,
size_t *out_len) {
SSL_SESSION *session = SSL_get_session(ssl);
-
- *out_len = 0;
- *out = NULL;
- if (ssl->server || !session || !session->tlsext_signed_cert_timestamp_list) {
+ if (ssl->server || !session || !session->signed_cert_timestamp_list) {
+ *out_len = 0;
+ *out = NULL;
return;
}
- *out = session->tlsext_signed_cert_timestamp_list;
- *out_len = session->tlsext_signed_cert_timestamp_list_length;
+ *out = CRYPTO_BUFFER_data(session->signed_cert_timestamp_list);
+ *out_len = CRYPTO_BUFFER_len(session->signed_cert_timestamp_list);
}
void SSL_get0_ocsp_response(const SSL *ssl, const uint8_t **out,
size_t *out_len) {
SSL_SESSION *session = SSL_get_session(ssl);
-
- *out_len = 0;
- *out = NULL;
if (ssl->server || !session || !session->ocsp_response) {
+ *out_len = 0;
+ *out = NULL;
return;
}
- *out = session->ocsp_response;
- *out_len = session->ocsp_response_length;
+
+ *out = CRYPTO_BUFFER_data(session->ocsp_response);
+ *out_len = CRYPTO_BUFFER_len(session->ocsp_response);
}
int SSL_set_tlsext_host_name(SSL *ssl, const char *name) {
diff --git a/ssl/ssl_session.cc b/ssl/ssl_session.cc
index dad0656..6bacc80 100644
--- a/ssl/ssl_session.cc
+++ b/ssl/ssl_session.cc
@@ -227,24 +227,15 @@
new_session->verify_result = session->verify_result;
- new_session->ocsp_response_length = session->ocsp_response_length;
if (session->ocsp_response != NULL) {
- new_session->ocsp_response = (uint8_t *)BUF_memdup(
- session->ocsp_response, session->ocsp_response_length);
- if (new_session->ocsp_response == NULL) {
- return nullptr;
- }
+ new_session->ocsp_response = session->ocsp_response;
+ CRYPTO_BUFFER_up_ref(new_session->ocsp_response);
}
- new_session->tlsext_signed_cert_timestamp_list_length =
- session->tlsext_signed_cert_timestamp_list_length;
- if (session->tlsext_signed_cert_timestamp_list != NULL) {
- new_session->tlsext_signed_cert_timestamp_list = (uint8_t *)BUF_memdup(
- session->tlsext_signed_cert_timestamp_list,
- session->tlsext_signed_cert_timestamp_list_length);
- if (new_session->tlsext_signed_cert_timestamp_list == NULL) {
- return nullptr;
- }
+ if (session->signed_cert_timestamp_list != NULL) {
+ new_session->signed_cert_timestamp_list =
+ session->signed_cert_timestamp_list;
+ CRYPTO_BUFFER_up_ref(new_session->signed_cert_timestamp_list);
}
OPENSSL_memcpy(new_session->peer_sha256, session->peer_sha256,
@@ -898,8 +889,8 @@
session->x509_method->session_clear(session);
OPENSSL_free(session->tlsext_hostname);
OPENSSL_free(session->tlsext_tick);
- OPENSSL_free(session->tlsext_signed_cert_timestamp_list);
- OPENSSL_free(session->ocsp_response);
+ CRYPTO_BUFFER_free(session->signed_cert_timestamp_list);
+ CRYPTO_BUFFER_free(session->ocsp_response);
OPENSSL_free(session->psk_identity);
OPENSSL_free(session->early_alpn);
OPENSSL_cleanse(session, sizeof(*session));
diff --git a/ssl/t1_lib.cc b/ssl/t1_lib.cc
index bbe6401..e50710a 100644
--- a/ssl/t1_lib.cc
+++ b/ssl/t1_lib.cc
@@ -1327,11 +1327,14 @@
* requirement, so tolerate this.
*
* TODO(davidben): Enforce this anyway. */
- if (!ssl->s3->session_reused &&
- !CBS_stow(contents, &hs->new_session->tlsext_signed_cert_timestamp_list,
- &hs->new_session->tlsext_signed_cert_timestamp_list_length)) {
- *out_alert = SSL_AD_INTERNAL_ERROR;
- return 0;
+ if (!ssl->s3->session_reused) {
+ CRYPTO_BUFFER_free(hs->new_session->signed_cert_timestamp_list);
+ hs->new_session->signed_cert_timestamp_list =
+ CRYPTO_BUFFER_new_from_CBS(contents, ssl->ctx->pool);
+ if (hs->new_session->signed_cert_timestamp_list == nullptr) {
+ *out_alert = SSL_AD_INTERNAL_ERROR;
+ return 0;
+ }
}
return 1;
diff --git a/ssl/tls13_both.cc b/ssl/tls13_both.cc
index 39e0cb3..6e7260e 100644
--- a/ssl/tls13_both.cc
+++ b/ssl/tls13_both.cc
@@ -284,11 +284,14 @@
return 0;
}
- if (sk_CRYPTO_BUFFER_num(certs.get()) == 1 &&
- !CBS_stow(&ocsp_response, &hs->new_session->ocsp_response,
- &hs->new_session->ocsp_response_length)) {
- ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR);
- return 0;
+ if (sk_CRYPTO_BUFFER_num(certs.get()) == 1) {
+ CRYPTO_BUFFER_free(hs->new_session->ocsp_response);
+ hs->new_session->ocsp_response =
+ CRYPTO_BUFFER_new_from_CBS(&ocsp_response, ssl->ctx->pool);
+ if (hs->new_session->ocsp_response == nullptr) {
+ ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR);
+ return 0;
+ }
}
}
@@ -305,12 +308,14 @@
return 0;
}
- if (sk_CRYPTO_BUFFER_num(certs.get()) == 1 &&
- !CBS_stow(
- &sct, &hs->new_session->tlsext_signed_cert_timestamp_list,
- &hs->new_session->tlsext_signed_cert_timestamp_list_length)) {
- ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR);
- return 0;
+ if (sk_CRYPTO_BUFFER_num(certs.get()) == 1) {
+ CRYPTO_BUFFER_free(hs->new_session->signed_cert_timestamp_list);
+ hs->new_session->signed_cert_timestamp_list =
+ CRYPTO_BUFFER_new_from_CBS(&sct, ssl->ctx->pool);
+ if (hs->new_session->signed_cert_timestamp_list == nullptr) {
+ ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR);
+ return 0;
+ }
}
}
}
