From 4b0d0e4c5ef19055b3ff17ed3d760f9b10d76641 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Fri, 28 Oct 2016 17:17:14 -0400 Subject: [PATCH] Validate input iv/mac sizes in SSL_AEAD_CTX_new. This should never happen, but the SSL_AEAD_CTX_new layer should enforce key sizes as it's not locally obvious at the call site the caller didn't get confused. There's still a mess of asserts below, but those should be fixed by cutting the SSL_CIPHER/SSL_AEAD_CTX boundary differently. (enc_key_len is validated by virtue of being passed into EVP_AEAD.) BUG=chromium:659593 Change-Id: I8c91609bcef14ca1509c87aab981bbad6556975f Reviewed-on: https://boringssl-review.googlesource.com/11940 Reviewed-by: Steven Valdez Reviewed-by: David Benjamin Commit-Queue: David Benjamin CQ-Verified: CQ bot account: commit-bot@chromium.org --- ssl/ssl_aead_ctx.c | 8 ++++++-- ssl/tls13_enc.c | 4 ++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/ssl/ssl_aead_ctx.c b/ssl/ssl_aead_ctx.c index 0f6f64fa..b05df0b1 100644 --- a/ssl/ssl_aead_ctx.c +++ b/ssl/ssl_aead_ctx.c @@ -34,8 +34,12 @@ SSL_AEAD_CTX *SSL_AEAD_CTX_new(enum evp_aead_direction_t direction, const uint8_t *mac_key, size_t mac_key_len, const uint8_t *fixed_iv, size_t fixed_iv_len) { const EVP_AEAD *aead; - size_t discard; - if (!ssl_cipher_get_evp_aead(&aead, &discard, &discard, cipher, version)) { + size_t expected_mac_key_len, expected_fixed_iv_len; + if (!ssl_cipher_get_evp_aead(&aead, &expected_mac_key_len, + &expected_fixed_iv_len, cipher, version) || + /* Ensure the caller returned correct key sizes. */ + expected_fixed_iv_len != fixed_iv_len || + expected_mac_key_len != mac_key_len) { OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); return 0; } diff --git a/ssl/tls13_enc.c b/ssl/tls13_enc.c index 1fcde514..95132cc9 100644 --- a/ssl/tls13_enc.c +++ b/ssl/tls13_enc.c @@ -159,8 +159,8 @@ int tls13_set_traffic_key(SSL *ssl, enum tls_record_type_t type, /* Look up cipher suite properties. */ const EVP_AEAD *aead; const EVP_MD *digest = ssl_get_handshake_digest(ssl_get_algorithm_prf(ssl)); - size_t mac_secret_len, fixed_iv_len; - if (!ssl_cipher_get_evp_aead(&aead, &mac_secret_len, &fixed_iv_len, + size_t discard; + if (!ssl_cipher_get_evp_aead(&aead, &discard, &discard, SSL_get_session(ssl)->cipher, ssl3_protocol_version(ssl))) { return 0;