Fix switching between AEAD and non-AEAD in a renegotiation. https://code.google.com/p/chromium/issues/detail?id=423998 Change-Id: I29d67db92b47d6cd303125b44e5ba552d97d54ff Reviewed-on: https://boringssl-review.googlesource.com/1960 Reviewed-by: David Benjamin <davidben@chromium.org> Reviewed-by: Adam Langley <agl@google.com>
diff --git a/ssl/t1_enc.c b/ssl/t1_enc.c index 48fcd87..ea52899 100644 --- a/ssl/t1_enc.c +++ b/ssl/t1_enc.c
@@ -334,6 +334,20 @@ return 1; } +static void tls1_cleanup_enc_ctx(EVP_CIPHER_CTX **ctx) + { + if (*ctx != NULL) + EVP_CIPHER_CTX_free(*ctx); + *ctx = NULL; + } + +static void tls1_cleanup_hash_ctx(EVP_MD_CTX **ctx) + { + if (*ctx != NULL) + EVP_MD_CTX_destroy(*ctx); + *ctx = NULL; + } + static int tls1_change_cipher_state_aead(SSL *s, char is_read, const unsigned char *key, unsigned key_len, const unsigned char *iv, unsigned iv_len, @@ -346,6 +360,17 @@ * to cope with the largest pair of keys. */ uint8_t mac_key_and_key[32 /* HMAC(SHA256) */ + 32 /* AES-256 */]; + if (is_read) + { + tls1_cleanup_enc_ctx(&s->enc_read_ctx); + tls1_cleanup_hash_ctx(&s->read_hash); + } + else + { + tls1_cleanup_enc_ctx(&s->enc_write_ctx); + tls1_cleanup_hash_ctx(&s->write_hash); + } + if (mac_secret_len > 0) { /* This is a "stateful" AEAD (for compatibility with pre-AEAD @@ -376,7 +401,14 @@ if (!EVP_AEAD_CTX_init(&aead_ctx->ctx, aead, key, key_len, EVP_AEAD_DEFAULT_TAG_LENGTH, NULL /* engine */)) + { + OPENSSL_free(aead_ctx); + if (is_read) + s->aead_read_ctx = NULL; + else + s->aead_write_ctx = NULL; return 0; + } if (iv_len > sizeof(aead_ctx->fixed_nonce)) { OPENSSL_PUT_ERROR(SSL, tls1_change_cipher_state_aead, ERR_R_INTERNAL_ERROR); @@ -399,6 +431,16 @@ return 1; } +static void tls1_cleanup_aead_ctx(SSL_AEAD_CTX **ctx) + { + if (*ctx != NULL) + { + EVP_AEAD_CTX_cleanup(&(*ctx)->ctx); + OPENSSL_free(*ctx); + } + *ctx = NULL; + } + /* tls1_change_cipher_state_cipher performs the work needed to switch cipher * states when using EVP_CIPHER. The argument |is_read| is true iff this * function is being called due to reading, as opposed to writing, a @@ -416,6 +458,11 @@ EVP_MD_CTX *mac_ctx; if (is_read) + tls1_cleanup_aead_ctx(&s->aead_read_ctx); + else + tls1_cleanup_aead_ctx(&s->aead_write_ctx); + + if (is_read) { if (s->enc_read_ctx != NULL && !SSL_IS_DTLS(s)) EVP_CIPHER_CTX_cleanup(s->enc_read_ctx);