Decouple the handshake buffer and digest.
The handshake hash is initialized from the buffer as soon as the cipher
is known. When adding a message to the transcript, independently update
the buffer and rolling hash, whichever is active. This avoids the
complications around dont_free_handshake_buffer and EMS.
BUG=492371
Change-Id: I3b1065796a50fd1be5d42ead7210c2f253ef0aca
Reviewed-on: https://boringssl-review.googlesource.com/5615
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/ssl/d1_clnt.c b/ssl/d1_clnt.c
index 2a692f6..6edf2c3 100644
--- a/ssl/d1_clnt.c
+++ b/ssl/d1_clnt.c
@@ -188,8 +188,7 @@
case SSL3_ST_CW_CLNT_HELLO_B:
s->shutdown = 0;
- /* every DTLS ClientHello resets Finished MAC */
- if (!ssl3_init_finished_mac(s)) {
+ if (!ssl3_init_handshake_buffer(s)) {
OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
ret = -1;
goto end;
diff --git a/ssl/d1_lib.c b/ssl/d1_lib.c
index 5b58d73..2dc55d6 100644
--- a/ssl/d1_lib.c
+++ b/ssl/d1_lib.c
@@ -328,8 +328,9 @@
s2n(msg_hdr->seq, p);
l2n3(0, p);
l2n3(msg_hdr->msg_len, p);
- return ssl3_finish_mac(s, serialised_header, sizeof(serialised_header)) &&
- ssl3_finish_mac(s, message + DTLS1_HM_HEADER_LENGTH, len);
+ return ssl3_update_handshake_hash(s, serialised_header,
+ sizeof(serialised_header)) &&
+ ssl3_update_handshake_hash(s, message + DTLS1_HM_HEADER_LENGTH, len);
}
int dtls1_handshake_write(SSL *s) {
diff --git a/ssl/d1_srvr.c b/ssl/d1_srvr.c
index 7a7d1ea..ea2e93a 100644
--- a/ssl/d1_srvr.c
+++ b/ssl/d1_srvr.c
@@ -181,7 +181,7 @@
goto end;
}
- if (!ssl3_init_finished_mac(s)) {
+ if (!ssl3_init_handshake_buffer(s)) {
OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
ret = -1;
goto end;
diff --git a/ssl/internal.h b/ssl/internal.h
index 7f2dd52..1d6426b 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -398,6 +398,35 @@
int custom_ext_add_serverhello(SSL *ssl, CBB *extensions);
+/* Handshake hash.
+ *
+ * The TLS handshake maintains a transcript of all handshake messages. At
+ * various points in the protocol, this is either a handshake buffer, a rolling
+ * hash (selected by cipher suite) or both. */
+
+/* ssl3_init_handshake_buffer initializes the handshake buffer and resets the
+ * handshake hash. It returns one success and zero on failure. */
+int ssl3_init_handshake_buffer(SSL *ssl);
+
+/* ssl3_init_handshake_hash initializes the handshake hash based on the pending
+ * cipher and the contents of the handshake buffer. Subsequent calls to
+ * |ssl3_update_handshake_hash| will update the rolling hash. It returns one on
+ * success and zero on failure. It is an error to call this function after the
+ * handshake buffer is released. */
+int ssl3_init_handshake_hash(SSL *ssl);
+
+/* ssl3_free_handshake_buffer releases the handshake buffer. Subsequent calls
+ * to |ssl3_update_handshake_hash| will not update the handshake buffer. */
+void ssl3_free_handshake_buffer(SSL *ssl);
+
+/* ssl3_free_handshake_hash releases the handshake hash. */
+void ssl3_free_handshake_hash(SSL *s);
+
+/* ssl3_update_handshake_hash adds |in| to the handshake buffer and handshake
+ * hash, whichever is enabled. It returns one on success and zero on failure. */
+int ssl3_update_handshake_hash(SSL *ssl, const uint8_t *in, size_t in_len);
+
+
/* Underdocumented functions.
*
* Functions below here haven't been touched up and may be underdocumented. */
@@ -880,7 +909,6 @@
* |len|. It returns one on success and zero on failure. */
int ssl_fill_hello_random(uint8_t *out, size_t len, int is_server);
-int ssl3_init_finished_mac(SSL *s);
int ssl3_send_server_certificate(SSL *s);
int ssl3_send_new_session_ticket(SSL *s);
int ssl3_send_cert_status(SSL *s);
@@ -921,8 +949,6 @@
int ssl3_write_bytes(SSL *s, int type, const void *buf, int len);
int ssl3_final_finish_mac(SSL *s, const char *sender, int slen, uint8_t *p);
int ssl3_cert_verify_mac(SSL *s, int md_nid, uint8_t *p);
-int ssl3_finish_mac(SSL *s, const uint8_t *buf, int len);
-void ssl3_free_digest_list(SSL *s);
int ssl3_output_cert_chain(SSL *s);
const SSL_CIPHER *ssl3_choose_cipher(
SSL *ssl, STACK_OF(SSL_CIPHER) *clnt,
@@ -932,12 +958,6 @@
int ssl3_release_read_buffer(SSL *s);
int ssl3_release_write_buffer(SSL *s);
-enum should_free_handshake_buffer_t {
- free_handshake_buffer,
- dont_free_handshake_buffer,
-};
-int ssl3_digest_cached_records(SSL *s, enum should_free_handshake_buffer_t);
-
int ssl3_new(SSL *s);
void ssl3_free(SSL *s);
int ssl3_accept(SSL *s);
diff --git a/ssl/s3_both.c b/ssl/s3_both.c
index 7a2236a..85dc9d6 100644
--- a/ssl/s3_both.c
+++ b/ssl/s3_both.c
@@ -441,8 +441,8 @@
/* The handshake header (different size between DTLS and TLS) is included in
* the hash. */
size_t header_len = s->init_msg - (uint8_t *)s->init_buf->data;
- return ssl3_finish_mac(s, (uint8_t *)s->init_buf->data,
- s->init_num + header_len);
+ return ssl3_update_handshake_hash(s, (uint8_t *)s->init_buf->data,
+ s->init_num + header_len);
}
/* ssl3_cert_verify_hash is documented as needing EVP_MAX_MD_SIZE because that
diff --git a/ssl/s3_clnt.c b/ssl/s3_clnt.c
index a1ed39a..6a3f157 100644
--- a/ssl/s3_clnt.c
+++ b/ssl/s3_clnt.c
@@ -220,7 +220,7 @@
/* don't push the buffering BIO quite yet */
- if (!ssl3_init_finished_mac(s)) {
+ if (!ssl3_init_handshake_buffer(s)) {
OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
ret = -1;
goto end;
@@ -855,12 +855,16 @@
}
s->s3->tmp.new_cipher = c;
+ /* Now that the cipher is known, initialize the handshake hash. */
+ if (!ssl3_init_handshake_hash(s)) {
+ goto f_err;
+ }
+
/* If doing a full handshake with TLS 1.2, the server may request a client
* certificate which requires hashing the handshake transcript under a
- * different hash. Otherwise, release the handshake buffer. */
- if ((!SSL_USE_SIGALGS(s) || s->hit) &&
- !ssl3_digest_cached_records(s, free_handshake_buffer)) {
- goto f_err;
+ * different hash. Otherwise, the handshake buffer may be released. */
+ if (!SSL_USE_SIGALGS(s) || s->hit) {
+ ssl3_free_handshake_buffer(s);
}
/* Only the NULL compression algorithm is supported. */
@@ -1349,12 +1353,9 @@
if (s->s3->tmp.message_type == SSL3_MT_SERVER_DONE) {
s->s3->tmp.reuse_message = 1;
- /* If we get here we don't need any cached handshake records as we wont be
- * doing client auth. */
- if (s->s3->handshake_buffer &&
- !ssl3_digest_cached_records(s, free_handshake_buffer)) {
- goto err;
- }
+ /* If we get here we don't need the handshake buffer as we won't be doing
+ * client auth. */
+ ssl3_free_handshake_buffer(s);
return 1;
}
@@ -1985,10 +1986,7 @@
}
/* The handshake buffer is no longer necessary. */
- if (s->s3->handshake_buffer &&
- !ssl3_digest_cached_records(s, free_handshake_buffer)) {
- return -1;
- }
+ ssl3_free_handshake_buffer(s);
/* Sign the digest. */
signature_length = ssl_private_key_max_signature_len(s);
@@ -2101,11 +2099,7 @@
s->s3->tmp.cert_req = 2;
/* There is no client certificate, so the handshake buffer may be
* released. */
- if (s->s3->handshake_buffer &&
- !ssl3_digest_cached_records(s, free_handshake_buffer)) {
- ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR);
- return -1;
- }
+ ssl3_free_handshake_buffer(s);
}
}
diff --git a/ssl/s3_enc.c b/ssl/s3_enc.c
index 97555cd..af72549 100644
--- a/ssl/s3_enc.c
+++ b/ssl/s3_enc.c
@@ -235,49 +235,19 @@
s->s3->tmp.key_block_length = 0;
}
-int ssl3_init_finished_mac(SSL *s) {
- BIO_free(s->s3->handshake_buffer);
- ssl3_free_digest_list(s);
- s->s3->handshake_buffer = BIO_new(BIO_s_mem());
- if (s->s3->handshake_buffer == NULL) {
+int ssl3_init_handshake_buffer(SSL *ssl) {
+ ssl3_free_handshake_buffer(ssl);
+ ssl3_free_handshake_hash(ssl);
+ ssl->s3->handshake_buffer = BIO_new(BIO_s_mem());
+ if (ssl->s3->handshake_buffer == NULL) {
return 0;
}
- BIO_set_close(s->s3->handshake_buffer, BIO_CLOSE);
+ BIO_set_close(ssl->s3->handshake_buffer, BIO_CLOSE);
return 1;
}
-void ssl3_free_digest_list(SSL *s) {
- int i;
- if (!s->s3->handshake_dgst) {
- return;
- }
- for (i = 0; i < SSL_MAX_DIGEST; i++) {
- if (s->s3->handshake_dgst[i]) {
- EVP_MD_CTX_destroy(s->s3->handshake_dgst[i]);
- }
- }
- OPENSSL_free(s->s3->handshake_dgst);
- s->s3->handshake_dgst = NULL;
-}
-
-int ssl3_finish_mac(SSL *s, const uint8_t *buf, int len) {
- int i;
-
- if (s->s3->handshake_buffer) {
- return BIO_write(s->s3->handshake_buffer, (void *)buf, len) >= 0;
- }
-
- for (i = 0; i < SSL_MAX_DIGEST; i++) {
- if (s->s3->handshake_dgst[i] != NULL) {
- EVP_DigestUpdate(s->s3->handshake_dgst[i], buf, len);
- }
- }
- return 1;
-}
-
-int ssl3_digest_cached_records(
- SSL *s, enum should_free_handshake_buffer_t should_free_handshake_buffer) {
+int ssl3_init_handshake_hash(SSL *ssl) {
int i;
uint32_t mask;
const EVP_MD *md;
@@ -285,45 +255,81 @@
size_t hdatalen;
/* Allocate handshake_dgst array */
- ssl3_free_digest_list(s);
- s->s3->handshake_dgst = OPENSSL_malloc(SSL_MAX_DIGEST * sizeof(EVP_MD_CTX *));
- if (s->s3->handshake_dgst == NULL) {
+ ssl3_free_handshake_hash(ssl);
+ ssl->s3->handshake_dgst = OPENSSL_malloc(SSL_MAX_DIGEST *
+ sizeof(EVP_MD_CTX *));
+ if (ssl->s3->handshake_dgst == NULL) {
OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE);
return 0;
}
- memset(s->s3->handshake_dgst, 0, SSL_MAX_DIGEST * sizeof(EVP_MD_CTX *));
- if (!BIO_mem_contents(s->s3->handshake_buffer, &hdata, &hdatalen)) {
+ memset(ssl->s3->handshake_dgst, 0, SSL_MAX_DIGEST * sizeof(EVP_MD_CTX *));
+ if (!BIO_mem_contents(ssl->s3->handshake_buffer, &hdata, &hdatalen)) {
OPENSSL_PUT_ERROR(SSL, SSL_R_BAD_HANDSHAKE_LENGTH);
return 0;
}
/* Loop through bits of algorithm_prf field and create MD_CTX-es */
for (i = 0; ssl_get_handshake_digest(&mask, &md, i); i++) {
- if ((mask & ssl_get_algorithm_prf(s)) && md) {
- s->s3->handshake_dgst[i] = EVP_MD_CTX_create();
- if (s->s3->handshake_dgst[i] == NULL) {
+ if ((mask & ssl_get_algorithm_prf(ssl)) && md) {
+ ssl->s3->handshake_dgst[i] = EVP_MD_CTX_create();
+ if (ssl->s3->handshake_dgst[i] == NULL) {
OPENSSL_PUT_ERROR(SSL, ERR_LIB_EVP);
return 0;
}
- if (!EVP_DigestInit_ex(s->s3->handshake_dgst[i], md, NULL)) {
- EVP_MD_CTX_destroy(s->s3->handshake_dgst[i]);
- s->s3->handshake_dgst[i] = NULL;
+ if (!EVP_DigestInit_ex(ssl->s3->handshake_dgst[i], md, NULL)) {
+ EVP_MD_CTX_destroy(ssl->s3->handshake_dgst[i]);
+ ssl->s3->handshake_dgst[i] = NULL;
OPENSSL_PUT_ERROR(SSL, ERR_LIB_EVP);
return 0;
}
- EVP_DigestUpdate(s->s3->handshake_dgst[i], hdata, hdatalen);
+ EVP_DigestUpdate(ssl->s3->handshake_dgst[i], hdata, hdatalen);
} else {
- s->s3->handshake_dgst[i] = NULL;
+ ssl->s3->handshake_dgst[i] = NULL;
}
}
- if (should_free_handshake_buffer == free_handshake_buffer) {
- /* Free handshake_buffer BIO */
- BIO_free(s->s3->handshake_buffer);
- s->s3->handshake_buffer = NULL;
+ return 1;
+}
+
+void ssl3_free_handshake_hash(SSL *ssl) {
+ int i;
+ if (!ssl->s3->handshake_dgst) {
+ return;
+ }
+ for (i = 0; i < SSL_MAX_DIGEST; i++) {
+ if (ssl->s3->handshake_dgst[i]) {
+ EVP_MD_CTX_destroy(ssl->s3->handshake_dgst[i]);
+ }
+ }
+ OPENSSL_free(ssl->s3->handshake_dgst);
+ ssl->s3->handshake_dgst = NULL;
+}
+
+void ssl3_free_handshake_buffer(SSL *ssl) {
+ BIO_free(ssl->s3->handshake_buffer);
+ ssl->s3->handshake_buffer = NULL;
+}
+
+int ssl3_update_handshake_hash(SSL *ssl, const uint8_t *in, size_t in_len) {
+ /* Depending on the state of the handshake, either the handshake buffer may be
+ * active, the rolling hash, or both. */
+
+ /* TODO(davidben): Replace |handshake_buffer| with a simpler type that doesn't
+ * require a cast. */
+ if (ssl->s3->handshake_buffer != NULL &&
+ BIO_write(ssl->s3->handshake_buffer, in, (int)in_len) < 0) {
+ return 0;
}
+ if (ssl->s3->handshake_dgst != NULL) {
+ int i;
+ for (i = 0; i < SSL_MAX_DIGEST; i++) {
+ if (ssl->s3->handshake_dgst[i] != NULL) {
+ EVP_DigestUpdate(ssl->s3->handshake_dgst[i], in, in_len);
+ }
+ }
+ }
return 1;
}
@@ -357,11 +363,6 @@
uint8_t md_buf[EVP_MAX_MD_SIZE];
EVP_MD_CTX ctx, *d = NULL;
- if (s->s3->handshake_buffer &&
- !ssl3_digest_cached_records(s, free_handshake_buffer)) {
- return 0;
- }
-
/* Search for digest of specified type in the handshake_dgst array. */
for (i = 0; i < SSL_MAX_DIGEST; i++) {
if (s->s3->handshake_dgst[i] &&
diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c
index 2566a4d..4ee68dd 100644
--- a/ssl/s3_lib.c
+++ b/ssl/s3_lib.c
@@ -186,7 +186,8 @@
s->init_off = 0;
/* Add the message to the handshake hash. */
- return ssl3_finish_mac(s, (uint8_t *)s->init_buf->data, s->init_num);
+ return ssl3_update_handshake_hash(s, (uint8_t *)s->init_buf->data,
+ s->init_num);
}
int ssl3_handshake_write(SSL *s) { return ssl3_do_write(s, SSL3_RT_HANDSHAKE); }
@@ -229,8 +230,8 @@
OPENSSL_free(s->s3->tmp.certificate_types);
OPENSSL_free(s->s3->tmp.peer_ellipticcurvelist);
OPENSSL_free(s->s3->tmp.peer_psk_identity_hint);
- BIO_free(s->s3->handshake_buffer);
- ssl3_free_digest_list(s);
+ ssl3_free_handshake_buffer(s);
+ ssl3_free_handshake_hash(s);
OPENSSL_free(s->s3->alpn_selected);
OPENSSL_cleanse(s->s3, sizeof *s->s3);
diff --git a/ssl/s3_srvr.c b/ssl/s3_srvr.c
index dca6207..fa9dc9a 100644
--- a/ssl/s3_srvr.c
+++ b/ssl/s3_srvr.c
@@ -230,7 +230,7 @@
goto end;
}
- if (!ssl3_init_finished_mac(s)) {
+ if (!ssl3_init_handshake_buffer(s)) {
OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
ret = -1;
goto end;
@@ -710,10 +710,10 @@
CBS_init(&v2_client_hello, (const uint8_t *)s->s3->sniff_buffer->data + 2,
msg_length);
- /* The V2ClientHello without the length is incorporated into the Finished
+ /* The V2ClientHello without the length is incorporated into the handshake
* hash. */
- if (!ssl3_finish_mac(s, CBS_data(&v2_client_hello),
- CBS_len(&v2_client_hello))) {
+ if (!ssl3_update_handshake_hash(s, CBS_data(&v2_client_hello),
+ CBS_len(&v2_client_hello))) {
return -1;
}
if (s->msg_callback) {
@@ -1110,11 +1110,15 @@
s->s3->tmp.cert_request = 0;
}
+ /* Now that the cipher is known, initialize the handshake hash. */
+ if (!ssl3_init_handshake_hash(s)) {
+ goto f_err;
+ }
+
/* In TLS 1.2, client authentication requires hashing the handshake transcript
* under a different hash. Otherwise, release the handshake buffer. */
- if ((!SSL_USE_SIGALGS(s) || !s->s3->tmp.cert_request) &&
- !ssl3_digest_cached_records(s, free_handshake_buffer)) {
- goto f_err;
+ if (!SSL_USE_SIGALGS(s) || !s->s3->tmp.cert_request) {
+ ssl3_free_handshake_buffer(s);
}
/* we now have the following setup;
@@ -2034,10 +2038,7 @@
* CertificateVerify is required if and only if there's a client certificate.
* */
if (peer == NULL) {
- if (s->s3->handshake_buffer &&
- !ssl3_digest_cached_records(s, free_handshake_buffer)) {
- return -1;
- }
+ ssl3_free_handshake_buffer(s);
return 1;
}
@@ -2077,10 +2078,7 @@
/* The handshake buffer is no longer necessary, and we may hash the current
* message.*/
- if (s->s3->handshake_buffer &&
- !ssl3_digest_cached_records(s, free_handshake_buffer)) {
- goto err;
- }
+ ssl3_free_handshake_buffer(s);
if (!ssl3_hash_current_message(s)) {
goto err;
}
@@ -2217,25 +2215,21 @@
}
if (sk_X509_num(sk) <= 0) {
+ /* No client certificate so the handshake buffer may be discarded. */
+ ssl3_free_handshake_buffer(s);
+
/* TLS does not mind 0 certs returned */
if (s->version == SSL3_VERSION) {
al = SSL_AD_HANDSHAKE_FAILURE;
OPENSSL_PUT_ERROR(SSL, SSL_R_NO_CERTIFICATES_RETURNED);
goto f_err;
- }
- /* Fail for TLS only if we required a certificate */
- else if ((s->verify_mode & SSL_VERIFY_PEER) &&
+ } else if ((s->verify_mode & SSL_VERIFY_PEER) &&
(s->verify_mode & SSL_VERIFY_FAIL_IF_NO_PEER_CERT)) {
+ /* Fail for TLS only if we required a certificate */
OPENSSL_PUT_ERROR(SSL, SSL_R_PEER_DID_NOT_RETURN_A_CERTIFICATE);
al = SSL_AD_HANDSHAKE_FAILURE;
goto f_err;
}
- /* No client certificate so digest cached records */
- if (s->s3->handshake_buffer &&
- !ssl3_digest_cached_records(s, free_handshake_buffer)) {
- al = SSL_AD_INTERNAL_ERROR;
- goto f_err;
- }
} else {
i = ssl_verify_cert_chain(s, sk);
if (i <= 0) {
diff --git a/ssl/t1_enc.c b/ssl/t1_enc.c
index 0aa4d1a..febd54d 100644
--- a/ssl/t1_enc.c
+++ b/ssl/t1_enc.c
@@ -473,11 +473,6 @@
EVP_MD_CTX ctx, *d = NULL;
int i;
- if (s->s3->handshake_buffer &&
- !ssl3_digest_cached_records(s, free_handshake_buffer)) {
- return 0;
- }
-
for (i = 0; i < SSL_MAX_DIGEST; i++) {
if (s->s3->handshake_dgst[i] &&
EVP_MD_CTX_type(s->s3->handshake_dgst[i]) == md_nid) {
@@ -554,14 +549,8 @@
int digests_len;
/* At this point, the handshake should have released the handshake buffer on
- * its own.
- * TODO(davidben): Apart from initialization, the handshake buffer should be
- * orthogonal to the handshake digest. https://crbug.com/492371 */
+ * its own. */
assert(s->s3->handshake_buffer == NULL);
- if (s->s3->handshake_buffer &&
- !ssl3_digest_cached_records(s, free_handshake_buffer)) {
- return 0;
- }
digests_len = tls1_handshake_digest(s, buf, sizeof(buf));
if (digests_len < 0) {
@@ -586,21 +575,7 @@
size_t premaster_len) {
if (s->s3->tmp.extended_master_secret) {
uint8_t digests[2 * EVP_MAX_MD_SIZE];
- int digests_len;
-
- /* The master secret is based on the handshake hash just after sending the
- * ClientKeyExchange. However, we might have a client certificate to send,
- * in which case we might need different hashes for the verification and
- * thus still need the handshake buffer around. Keeping both a handshake
- * buffer *and* running hashes isn't yet supported so, when it comes to
- * calculating the Finished hash, we'll have to hash the handshake buffer
- * again. */
- if (s->s3->handshake_buffer &&
- !ssl3_digest_cached_records(s, dont_free_handshake_buffer)) {
- return 0;
- }
-
- digests_len = tls1_handshake_digest(s, digests, sizeof(digests));
+ int digests_len = tls1_handshake_digest(s, digests, sizeof(digests));
if (digests_len == -1) {
return 0;
}
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index eb47e19..1dedb32 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -2932,11 +2932,6 @@
int i;
static const char kClientIDMagic[] = "TLS Channel ID signature";
- if (s->s3->handshake_buffer &&
- !ssl3_digest_cached_records(s, free_handshake_buffer)) {
- return 0;
- }
-
EVP_DigestUpdate(md, kClientIDMagic, sizeof(kClientIDMagic));
if (s->hit) {
