kris/boringssl
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Add missing nonce_len check to aead_aes_gcm_siv_asm_open.

Browse Source 
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>
This commit is contained in:
David Benjamin 2019-04-10 17:03:42 -05:00 committed by Adam Langley
parent 4a136ea005
commit 31ef16ac2d
2 changed files with 77 additions and 61 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

133
crypto/cipher_extra/aead_test.cc
View File

@ -42,56 +42,58 @@ struct KnownAEAD {
// 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 @@ TEST_P(PerAEADTest, Overflow) {
// 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) {

5
crypto/cipher_extra/e_aesgcmsiv.c
View File

@ -426,6 +426,11 @@ static int aead_aes_gcm_siv_asm_open(const EVP_AEAD_CTX *ctx, uint8_t *out,
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;