From 880b14e98cbdf2bc6bd686454f572bc8209c6ad1 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Mon, 25 Aug 2014 22:35:07 -0400 Subject: [PATCH] Compute the Channel ID hash after ssl_get_message. This avoids needing the save the hash on the SSL* (and use some field for two purposes). Instead, use the new SSL_GET_MESSAGE_DONT_HASH_MESSAGE flag (which actually was already used here, but at the time, pointlessly). Also fix a minor bug where the hash would be recomputed in non-blocking mode because init_num may stay zero for a few state machine iterations. Change-Id: I3d8331cf3134c5f9a3eda9e988bba5bcebe40933 Reviewed-on: https://boringssl-review.googlesource.com/1631 Reviewed-by: Adam Langley --- ssl/s3_srvr.c | 38 ++++++++++++++++++++------------------ 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/ssl/s3_srvr.c b/ssl/s3_srvr.c index a82c061d..06549068 100644 --- a/ssl/s3_srvr.c +++ b/ssl/s3_srvr.c @@ -148,6 +148,7 @@ #define NETSCAPE_HANG_BUG +#include #include #include @@ -2880,6 +2881,9 @@ int ssl3_get_channel_id(SSL *s) { int ret = -1, ok; long n; + EVP_MD_CTX md_ctx; + uint8_t channel_id_hash[SHA256_DIGEST_LENGTH]; + unsigned int channel_id_hash_len; const uint8_t *p; uint16_t extension_type, expected_extension_type; EC_GROUP* p256 = NULL; @@ -2889,22 +2893,6 @@ int ssl3_get_channel_id(SSL *s) BIGNUM x, y; CBS encrypted_extensions, extension; - if (s->state == SSL3_ST_SR_CHANNEL_ID_A && s->init_num == 0) - { - /* The first time that we're called we take the current - * handshake hash and store it. */ - EVP_MD_CTX md_ctx; - unsigned int len; - - EVP_MD_CTX_init(&md_ctx); - EVP_DigestInit_ex(&md_ctx, EVP_sha256(), NULL); - if (!tls1_channel_id_hash(&md_ctx, s)) - return -1; - len = sizeof(s->s3->tlsext_channel_id); - EVP_DigestFinal(&md_ctx, s->s3->tlsext_channel_id, &len); - EVP_MD_CTX_cleanup(&md_ctx); - } - n = s->method->ssl_get_message(s, SSL3_ST_SR_CHANNEL_ID_A, SSL3_ST_SR_CHANNEL_ID_B, @@ -2916,7 +2904,21 @@ int ssl3_get_channel_id(SSL *s) if (!ok) return((int)n); - ssl3_finish_mac(s, (unsigned char*)s->init_buf->data, s->init_num + 4); + /* Before incorporating the EncryptedExtensions message to the + * handshake hash, compute the hash that should have been signed. */ + channel_id_hash_len = sizeof(channel_id_hash); + EVP_MD_CTX_init(&md_ctx); + if (!EVP_DigestInit_ex(&md_ctx, EVP_sha256(), NULL) || + !tls1_channel_id_hash(&md_ctx, s) || + !EVP_DigestFinal(&md_ctx, channel_id_hash, &channel_id_hash_len)) + { + EVP_MD_CTX_cleanup(&md_ctx); + return -1; + } + EVP_MD_CTX_cleanup(&md_ctx); + assert(channel_id_hash_len == SHA256_DIGEST_LENGTH); + + ssl3_hash_current_message(s); /* s->state doesn't reflect whether ChangeCipherSpec has been received * in this handshake, but s->s3->change_cipher_spec does (will be reset @@ -2989,7 +2991,7 @@ int ssl3_get_channel_id(SSL *s) /* We stored the handshake hash in |tlsext_channel_id| the first time * that we were called. */ - if (!ECDSA_do_verify(s->s3->tlsext_channel_id, SHA256_DIGEST_LENGTH, &sig, key)) + if (!ECDSA_do_verify(channel_id_hash, channel_id_hash_len, &sig, key)) { OPENSSL_PUT_ERROR(SSL, ssl3_get_channel_id, SSL_R_CHANNEL_ID_SIGNATURE_INVALID); s->s3->tlsext_channel_id_valid = 0;