Add missing nonce_len check to aead_aes_gcm_siv_asm_open.
Test invalid nonce lengths more thoroughly to cover this case on all our
AEADs. Thanks to Guido Vranken for catching this!
In doing so, this also reveals we have a ton of redundant error codes
(https://crbug.com/boringssl/269). I'll tidy that up in a separate
change as it may require some changes to code in Android. For now, this
change uses CIPHER_R_UNSUPPORTED_NONCE_SIZE just to be consistent with
the rest of that file.
Bug: 268
Change-Id: I0a479000ec3005ee55c828eaa92c8302b4625847
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/35545
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/crypto/cipher_extra/aead_test.cc b/crypto/cipher_extra/aead_test.cc
index 25d4a8e..25924bd 100644
--- a/crypto/cipher_extra/aead_test.cc
+++ b/crypto/cipher_extra/aead_test.cc
@@ -42,56 +42,58 @@
// truncated_tags is true if the AEAD supports truncating tags to arbitrary
// lengths.
bool truncated_tags;
+ // variable_nonce is true if the AEAD supports a variable nonce length.
+ bool variable_nonce;
// ad_len, if non-zero, is the required length of the AD.
size_t ad_len;
};
static const struct KnownAEAD kAEADs[] = {
{"AES_128_GCM", EVP_aead_aes_128_gcm, "aes_128_gcm_tests.txt", false, true,
- 0},
+ true, 0},
{"AES_128_GCM_NIST", EVP_aead_aes_128_gcm, "nist_cavp/aes_128_gcm.txt",
- false, true, 0},
+ false, true, true, 0},
{"AES_256_GCM", EVP_aead_aes_256_gcm, "aes_256_gcm_tests.txt", false, true,
- 0},
+ true, 0},
{"AES_256_GCM_NIST", EVP_aead_aes_256_gcm, "nist_cavp/aes_256_gcm.txt",
- false, true, 0},
+ false, true, true, 0},
{"AES_128_GCM_SIV", EVP_aead_aes_128_gcm_siv, "aes_128_gcm_siv_tests.txt",
- false, false, 0},
+ false, false, false, 0},
{"AES_256_GCM_SIV", EVP_aead_aes_256_gcm_siv, "aes_256_gcm_siv_tests.txt",
- false, false, 0},
+ false, false, false, 0},
{"ChaCha20Poly1305", EVP_aead_chacha20_poly1305,
- "chacha20_poly1305_tests.txt", false, true, 0},
+ "chacha20_poly1305_tests.txt", false, true, false, 0},
{"XChaCha20Poly1305", EVP_aead_xchacha20_poly1305,
- "xchacha20_poly1305_tests.txt", false, true, 0},
+ "xchacha20_poly1305_tests.txt", false, true, false, 0},
{"AES_128_CBC_SHA1_TLS", EVP_aead_aes_128_cbc_sha1_tls,
- "aes_128_cbc_sha1_tls_tests.txt", true, false, 11},
+ "aes_128_cbc_sha1_tls_tests.txt", true, false, false, 11},
{"AES_128_CBC_SHA1_TLSImplicitIV",
EVP_aead_aes_128_cbc_sha1_tls_implicit_iv,
- "aes_128_cbc_sha1_tls_implicit_iv_tests.txt", true, false, 11},
+ "aes_128_cbc_sha1_tls_implicit_iv_tests.txt", true, false, false, 11},
{"AES_128_CBC_SHA256_TLS", EVP_aead_aes_128_cbc_sha256_tls,
- "aes_128_cbc_sha256_tls_tests.txt", true, false, 11},
+ "aes_128_cbc_sha256_tls_tests.txt", true, false, false, 11},
{"AES_256_CBC_SHA1_TLS", EVP_aead_aes_256_cbc_sha1_tls,
- "aes_256_cbc_sha1_tls_tests.txt", true, false, 11},
+ "aes_256_cbc_sha1_tls_tests.txt", true, false, false, 11},
{"AES_256_CBC_SHA1_TLSImplicitIV",
EVP_aead_aes_256_cbc_sha1_tls_implicit_iv,
- "aes_256_cbc_sha1_tls_implicit_iv_tests.txt", true, false, 11},
+ "aes_256_cbc_sha1_tls_implicit_iv_tests.txt", true, false, false, 11},
{"AES_256_CBC_SHA256_TLS", EVP_aead_aes_256_cbc_sha256_tls,
- "aes_256_cbc_sha256_tls_tests.txt", true, false, 11},
+ "aes_256_cbc_sha256_tls_tests.txt", true, false, false, 11},
{"AES_256_CBC_SHA384_TLS", EVP_aead_aes_256_cbc_sha384_tls,
- "aes_256_cbc_sha384_tls_tests.txt", true, false, 11},
+ "aes_256_cbc_sha384_tls_tests.txt", true, false, false, 11},
{"DES_EDE3_CBC_SHA1_TLS", EVP_aead_des_ede3_cbc_sha1_tls,
- "des_ede3_cbc_sha1_tls_tests.txt", true, false, 11},
+ "des_ede3_cbc_sha1_tls_tests.txt", true, false, false, 11},
{"DES_EDE3_CBC_SHA1_TLSImplicitIV",
EVP_aead_des_ede3_cbc_sha1_tls_implicit_iv,
- "des_ede3_cbc_sha1_tls_implicit_iv_tests.txt", true, false, 11},
+ "des_ede3_cbc_sha1_tls_implicit_iv_tests.txt", true, false, false, 11},
{"AES_128_CTR_HMAC_SHA256", EVP_aead_aes_128_ctr_hmac_sha256,
- "aes_128_ctr_hmac_sha256.txt", false, true, 0},
+ "aes_128_ctr_hmac_sha256.txt", false, true, false, 0},
{"AES_256_CTR_HMAC_SHA256", EVP_aead_aes_256_ctr_hmac_sha256,
- "aes_256_ctr_hmac_sha256.txt", false, true, 0},
+ "aes_256_ctr_hmac_sha256.txt", false, true, false, 0},
{"AES_128_CCM_BLUETOOTH", EVP_aead_aes_128_ccm_bluetooth,
- "aes_128_ccm_bluetooth_tests.txt", false, false, 0},
+ "aes_128_ccm_bluetooth_tests.txt", false, false, false, 0},
{"AES_128_CCM_BLUETOOTH_8", EVP_aead_aes_128_ccm_bluetooth_8,
- "aes_128_ccm_bluetooth_8_tests.txt", false, false, 0},
+ "aes_128_ccm_bluetooth_8_tests.txt", false, false, false, 0},
};
class PerAEADTest : public testing::TestWithParam<KnownAEAD> {
@@ -605,50 +607,59 @@
// as the input.)
}
-// Test that EVP_aead_aes_128_gcm and EVP_aead_aes_256_gcm reject empty nonces.
-// AES-GCM is not defined for those.
-TEST(AEADTest, AESGCMEmptyNonce) {
- static const uint8_t kZeros[32] = {0};
+TEST_P(PerAEADTest, InvalidNonceLength) {
+ size_t valid_nonce_len = EVP_AEAD_nonce_length(aead());
+ std::vector<size_t> nonce_lens;
+ if (valid_nonce_len != 0) {
+ // Other than the implicit IV TLS "AEAD"s, none of our AEADs allow empty
+ // nonces. In particular, although AES-GCM was incorrectly specified with
+ // variable-length nonces, it does not allow the empty nonce.
+ nonce_lens.push_back(0);
+ }
+ if (!GetParam().variable_nonce) {
+ nonce_lens.push_back(valid_nonce_len + 1);
+ if (valid_nonce_len != 0) {
+ nonce_lens.push_back(valid_nonce_len - 1);
+ }
+ }
- // Test AES-128-GCM.
- uint8_t buf[16];
- size_t len;
- bssl::ScopedEVP_AEAD_CTX ctx;
- ASSERT_TRUE(EVP_AEAD_CTX_init(ctx.get(), EVP_aead_aes_128_gcm(), kZeros, 16,
- EVP_AEAD_DEFAULT_TAG_LENGTH, nullptr));
+ static const uint8_t kZeros[EVP_AEAD_MAX_KEY_LENGTH] = {0};
+ const size_t ad_len = GetParam().ad_len != 0 ? GetParam().ad_len : 16;
+ ASSERT_LE(ad_len, sizeof(kZeros));
- EXPECT_FALSE(EVP_AEAD_CTX_seal(ctx.get(), buf, &len, sizeof(buf),
- nullptr /* nonce */, 0, nullptr /* in */, 0,
- nullptr /* ad */, 0));
- uint32_t err = ERR_get_error();
- EXPECT_EQ(ERR_LIB_CIPHER, ERR_GET_LIB(err));
- EXPECT_EQ(CIPHER_R_INVALID_NONCE_SIZE, ERR_GET_REASON(err));
+ for (size_t nonce_len : nonce_lens) {
+ SCOPED_TRACE(nonce_len);
+ uint8_t buf[256];
+ size_t len;
+ std::vector<uint8_t> nonce(nonce_len);
+ bssl::ScopedEVP_AEAD_CTX ctx;
+ ASSERT_TRUE(EVP_AEAD_CTX_init_with_direction(
+ ctx.get(), aead(), kZeros, EVP_AEAD_key_length(aead()),
+ EVP_AEAD_DEFAULT_TAG_LENGTH, evp_aead_seal));
- EXPECT_FALSE(EVP_AEAD_CTX_open(ctx.get(), buf, &len, sizeof(buf),
- nullptr /* nonce */, 0, kZeros /* in */,
- sizeof(kZeros), nullptr /* ad */, 0));
- err = ERR_get_error();
- EXPECT_EQ(ERR_LIB_CIPHER, ERR_GET_LIB(err));
- EXPECT_EQ(CIPHER_R_INVALID_NONCE_SIZE, ERR_GET_REASON(err));
+ EXPECT_FALSE(EVP_AEAD_CTX_seal(ctx.get(), buf, &len, sizeof(buf),
+ nonce.data(), nonce.size(), nullptr /* in */,
+ 0, kZeros /* ad */, ad_len));
+ uint32_t err = ERR_get_error();
+ EXPECT_EQ(ERR_LIB_CIPHER, ERR_GET_LIB(err));
+ // TODO(davidben): Merge these errors. https://crbug.com/boringssl/129.
+ if (ERR_GET_REASON(err) != CIPHER_R_UNSUPPORTED_NONCE_SIZE) {
+ EXPECT_EQ(CIPHER_R_INVALID_NONCE_SIZE, ERR_GET_REASON(err));
+ }
- // Test AES-256-GCM.
- ctx.Reset();
- ASSERT_TRUE(EVP_AEAD_CTX_init(ctx.get(), EVP_aead_aes_256_gcm(), kZeros, 32,
- EVP_AEAD_DEFAULT_TAG_LENGTH, nullptr));
-
- EXPECT_FALSE(EVP_AEAD_CTX_seal(ctx.get(), buf, &len, sizeof(buf),
- nullptr /* nonce */, 0, nullptr /* in */, 0,
- nullptr /* ad */, 0));
- err = ERR_get_error();
- EXPECT_EQ(ERR_LIB_CIPHER, ERR_GET_LIB(err));
- EXPECT_EQ(CIPHER_R_INVALID_NONCE_SIZE, ERR_GET_REASON(err));
-
- EXPECT_FALSE(EVP_AEAD_CTX_open(ctx.get(), buf, &len, sizeof(buf),
- nullptr /* nonce */, 0, kZeros /* in */,
- sizeof(kZeros), nullptr /* ad */, 0));
- err = ERR_get_error();
- EXPECT_EQ(ERR_LIB_CIPHER, ERR_GET_LIB(err));
- EXPECT_EQ(CIPHER_R_INVALID_NONCE_SIZE, ERR_GET_REASON(err));
+ ctx.Reset();
+ ASSERT_TRUE(EVP_AEAD_CTX_init_with_direction(
+ ctx.get(), aead(), kZeros, EVP_AEAD_key_length(aead()),
+ EVP_AEAD_DEFAULT_TAG_LENGTH, evp_aead_open));
+ EXPECT_FALSE(EVP_AEAD_CTX_open(ctx.get(), buf, &len, sizeof(buf),
+ nonce.data(), nonce.size(), kZeros /* in */,
+ sizeof(kZeros), kZeros /* ad */, ad_len));
+ err = ERR_get_error();
+ EXPECT_EQ(ERR_LIB_CIPHER, ERR_GET_LIB(err));
+ if (ERR_GET_REASON(err) != CIPHER_R_UNSUPPORTED_NONCE_SIZE) {
+ EXPECT_EQ(CIPHER_R_INVALID_NONCE_SIZE, ERR_GET_REASON(err));
+ }
+ }
}
TEST(AEADTest, AESCCMLargeAD) {
diff --git a/crypto/cipher_extra/e_aesgcmsiv.c b/crypto/cipher_extra/e_aesgcmsiv.c
index 71a71fa..64febae 100644
--- a/crypto/cipher_extra/e_aesgcmsiv.c
+++ b/crypto/cipher_extra/e_aesgcmsiv.c
@@ -426,6 +426,11 @@
return 0;
}
+ if (nonce_len != EVP_AEAD_AES_GCM_SIV_NONCE_LEN) {
+ OPENSSL_PUT_ERROR(CIPHER, CIPHER_R_UNSUPPORTED_NONCE_SIZE);
+ return 0;
+ }
+
const struct aead_aes_gcm_siv_asm_ctx *gcm_siv_ctx = asm_ctx_from_ctx(ctx);
const size_t plaintext_len = in_len - EVP_AEAD_AES_GCM_SIV_TAG_LEN;
const uint8_t *const given_tag = in + plaintext_len;
