From d656092340dafedd847ee89cc5f48c22e47abc30 Mon Sep 17 00:00:00 2001 From: Appelmans Date: Tue, 25 Jun 2024 16:53:47 -0700 Subject: [PATCH 1/6] Adds new monotonicity check --- BUILDING.md | 2 +- crypto/cipher_extra/aead_test.cc | 42 ++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/BUILDING.md b/BUILDING.md index a3894a8285..3037beb1c8 100644 --- a/BUILDING.md +++ b/BUILDING.md @@ -20,7 +20,7 @@ If in doubt, use the most recent stable version of each build tool. `PERL_EXECUTABLE`. * To build without Perl (not recommended) see [this section.](#using-pre-generated-build-files) - * [Go](https://golang.org/dl/) 1.18 or later is required. If not found by + * [Go](https://golang.org/dl/) 1.20 or later is required. If not found by CMake, the go executable may be configured explicitly by setting `GO_EXECUTABLE`. * To build without Go (not recommended) see [this section.](#using-pre-generated-build-files) diff --git a/crypto/cipher_extra/aead_test.cc b/crypto/cipher_extra/aead_test.cc index 0648cc2637..7b8e9d0e25 100644 --- a/crypto/cipher_extra/aead_test.cc +++ b/crypto/cipher_extra/aead_test.cc @@ -1364,6 +1364,48 @@ TEST(AEADTest, TestGCMSIV256Change16Alignment) { free(encrypt_ctx_256); } +TEST(AEADTest, TestMonotonicityCheck) { + + static const uint8_t kEvpAeadCtxKey[32] = {0}; + + // Only the tls13() ciphers have monotonicity checks + struct { + const EVP_AEAD *cipher; + const size_t key_len; + } ctx[] = { { .cipher = EVP_aead_aes_128_gcm_tls13(), .key_len = 16}, + { .cipher = EVP_aead_aes_256_gcm_tls13(), .key_len = 32} }; + + for (int i = 0; i < 2; i++) { + const EVP_AEAD *cipher = ctx[i].cipher; + EVP_AEAD_CTX *encrypt_ctx = + (EVP_AEAD_CTX *)malloc(sizeof(EVP_AEAD_CTX) + 8); + ASSERT_TRUE(encrypt_ctx); + + EVP_AEAD_CTX_zero(encrypt_ctx); + ASSERT_TRUE(EVP_AEAD_CTX_init(encrypt_ctx, cipher, kEvpAeadCtxKey, ctx[i].key_len, 16, NULL)) + << ERR_error_string(ERR_get_error(), NULL); + + uint8_t nonce[12] = {0}; + uint8_t last_byte = sizeof(nonce) - 1; + uint8_t plaintext[16] = {0}; + uint8_t ciphertext[32] = {0}; + size_t out_len = 0; + + // Checks that sequence numbers are allowed to increment by more than one + // as long as monotonicity is preserved. Here the implicit IV is presumed + // to be a zero-filled array. That lets us update the nonce value directly + // with an increasing sequence number. + for (size_t sequence_num = 0; sequence_num <= 255; sequence_num+=10) { + nonce[last_byte] = sequence_num; + ASSERT_TRUE(EVP_AEAD_CTX_seal(encrypt_ctx, ciphertext, &out_len, + sizeof(ciphertext), nonce, sizeof(nonce), plaintext, + sizeof(plaintext), nullptr /* ad */, 0)); + } + + free(encrypt_ctx); + } +} + struct EvpAeadCtxSerdeTestParams { const char *name; const EVP_AEAD *cipher; From 9b49430035efb2bab92aa334d990884d9bfd1286 Mon Sep 17 00:00:00 2001 From: Appelmans Date: Tue, 25 Jun 2024 17:38:08 -0700 Subject: [PATCH 2/6] removing designated initializers --- crypto/cipher_extra/aead_test.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crypto/cipher_extra/aead_test.cc b/crypto/cipher_extra/aead_test.cc index 7b8e9d0e25..f478f881e8 100644 --- a/crypto/cipher_extra/aead_test.cc +++ b/crypto/cipher_extra/aead_test.cc @@ -1372,8 +1372,8 @@ TEST(AEADTest, TestMonotonicityCheck) { struct { const EVP_AEAD *cipher; const size_t key_len; - } ctx[] = { { .cipher = EVP_aead_aes_128_gcm_tls13(), .key_len = 16}, - { .cipher = EVP_aead_aes_256_gcm_tls13(), .key_len = 32} }; + } ctx[] = { { EVP_aead_aes_128_gcm_tls13(), 16}, + { EVP_aead_aes_256_gcm_tls13(), 32} }; for (int i = 0; i < 2; i++) { const EVP_AEAD *cipher = ctx[i].cipher; @@ -1396,8 +1396,8 @@ TEST(AEADTest, TestMonotonicityCheck) { // to be a zero-filled array. That lets us update the nonce value directly // with an increasing sequence number. for (size_t sequence_num = 0; sequence_num <= 255; sequence_num+=10) { - nonce[last_byte] = sequence_num; - ASSERT_TRUE(EVP_AEAD_CTX_seal(encrypt_ctx, ciphertext, &out_len, + nonce[last_byte] = sequence_num; + ASSERT_TRUE(EVP_AEAD_CTX_seal(encrypt_ctx, ciphertext, &out_len, sizeof(ciphertext), nonce, sizeof(nonce), plaintext, sizeof(plaintext), nullptr /* ad */, 0)); } From d9e13d524e25217b3ad78f6cbff4dcf419d7ac58 Mon Sep 17 00:00:00 2001 From: Appelmans Date: Tue, 2 Jul 2024 17:14:48 -0700 Subject: [PATCH 3/6] PR feedback --- BUILDING.md | 2 +- crypto/cipher_extra/aead_test.cc | 22 ++++++---------------- 2 files changed, 7 insertions(+), 17 deletions(-) diff --git a/BUILDING.md b/BUILDING.md index 3037beb1c8..a3894a8285 100644 --- a/BUILDING.md +++ b/BUILDING.md @@ -20,7 +20,7 @@ If in doubt, use the most recent stable version of each build tool. `PERL_EXECUTABLE`. * To build without Perl (not recommended) see [this section.](#using-pre-generated-build-files) - * [Go](https://golang.org/dl/) 1.20 or later is required. If not found by + * [Go](https://golang.org/dl/) 1.18 or later is required. If not found by CMake, the go executable may be configured explicitly by setting `GO_EXECUTABLE`. * To build without Go (not recommended) see [this section.](#using-pre-generated-build-files) diff --git a/crypto/cipher_extra/aead_test.cc b/crypto/cipher_extra/aead_test.cc index f478f881e8..6465e426f9 100644 --- a/crypto/cipher_extra/aead_test.cc +++ b/crypto/cipher_extra/aead_test.cc @@ -1369,20 +1369,12 @@ TEST(AEADTest, TestMonotonicityCheck) { static const uint8_t kEvpAeadCtxKey[32] = {0}; // Only the tls13() ciphers have monotonicity checks - struct { - const EVP_AEAD *cipher; - const size_t key_len; - } ctx[] = { { EVP_aead_aes_128_gcm_tls13(), 16}, - { EVP_aead_aes_256_gcm_tls13(), 32} }; - - for (int i = 0; i < 2; i++) { - const EVP_AEAD *cipher = ctx[i].cipher; - EVP_AEAD_CTX *encrypt_ctx = - (EVP_AEAD_CTX *)malloc(sizeof(EVP_AEAD_CTX) + 8); - ASSERT_TRUE(encrypt_ctx); + const EVP_AEAD *aeads_to_test[] = { EVP_aead_aes_128_gcm_tls13(), EVP_aead_aes_256_gcm_tls13() }; + + for (const EVP_AEAD *cipher : aeads_to_test) { + bssl::ScopedEVP_AEAD_CTX encrypt_ctx; - EVP_AEAD_CTX_zero(encrypt_ctx); - ASSERT_TRUE(EVP_AEAD_CTX_init(encrypt_ctx, cipher, kEvpAeadCtxKey, ctx[i].key_len, 16, NULL)) + ASSERT_TRUE(EVP_AEAD_CTX_init(encrypt_ctx.get(), cipher, kEvpAeadCtxKey, cipher->key_len, 16, NULL)) << ERR_error_string(ERR_get_error(), NULL); uint8_t nonce[12] = {0}; @@ -1397,12 +1389,10 @@ TEST(AEADTest, TestMonotonicityCheck) { // with an increasing sequence number. for (size_t sequence_num = 0; sequence_num <= 255; sequence_num+=10) { nonce[last_byte] = sequence_num; - ASSERT_TRUE(EVP_AEAD_CTX_seal(encrypt_ctx, ciphertext, &out_len, + ASSERT_TRUE(EVP_AEAD_CTX_seal(encrypt_ctx.get(), ciphertext, &out_len, sizeof(ciphertext), nonce, sizeof(nonce), plaintext, sizeof(plaintext), nullptr /* ad */, 0)); } - - free(encrypt_ctx); } } From c9fa68dd21c85ebe261461e852103a9dc268c6d6 Mon Sep 17 00:00:00 2001 From: Appelmans Date: Wed, 3 Jul 2024 10:07:15 -0700 Subject: [PATCH 4/6] Whoops --- crypto/cipher_extra/aead_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crypto/cipher_extra/aead_test.cc b/crypto/cipher_extra/aead_test.cc index 6465e426f9..2842ab7de5 100644 --- a/crypto/cipher_extra/aead_test.cc +++ b/crypto/cipher_extra/aead_test.cc @@ -1387,7 +1387,7 @@ TEST(AEADTest, TestMonotonicityCheck) { // as long as monotonicity is preserved. Here the implicit IV is presumed // to be a zero-filled array. That lets us update the nonce value directly // with an increasing sequence number. - for (size_t sequence_num = 0; sequence_num <= 255; sequence_num+=10) { + for (uint8_t sequence_num = 0; sequence_num <= 255; sequence_num+=10) { nonce[last_byte] = sequence_num; ASSERT_TRUE(EVP_AEAD_CTX_seal(encrypt_ctx.get(), ciphertext, &out_len, sizeof(ciphertext), nonce, sizeof(nonce), plaintext, From 246bd945162c7e155c5082d19428ab4644b7e3a6 Mon Sep 17 00:00:00 2001 From: Appelmans Date: Wed, 3 Jul 2024 12:12:42 -0700 Subject: [PATCH 5/6] Adding size_t back --- crypto/cipher_extra/aead_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crypto/cipher_extra/aead_test.cc b/crypto/cipher_extra/aead_test.cc index 2842ab7de5..6465e426f9 100644 --- a/crypto/cipher_extra/aead_test.cc +++ b/crypto/cipher_extra/aead_test.cc @@ -1387,7 +1387,7 @@ TEST(AEADTest, TestMonotonicityCheck) { // as long as monotonicity is preserved. Here the implicit IV is presumed // to be a zero-filled array. That lets us update the nonce value directly // with an increasing sequence number. - for (uint8_t sequence_num = 0; sequence_num <= 255; sequence_num+=10) { + for (size_t sequence_num = 0; sequence_num <= 255; sequence_num+=10) { nonce[last_byte] = sequence_num; ASSERT_TRUE(EVP_AEAD_CTX_seal(encrypt_ctx.get(), ciphertext, &out_len, sizeof(ciphertext), nonce, sizeof(nonce), plaintext, From 34dd91276f567d0ea1a7922e68419b110898b105 Mon Sep 17 00:00:00 2001 From: Appelmans Date: Tue, 9 Jul 2024 11:31:55 -0700 Subject: [PATCH 6/6] Adds smaller seq num assert --- crypto/cipher_extra/aead_test.cc | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/crypto/cipher_extra/aead_test.cc b/crypto/cipher_extra/aead_test.cc index 6465e426f9..8d8cceb0c3 100644 --- a/crypto/cipher_extra/aead_test.cc +++ b/crypto/cipher_extra/aead_test.cc @@ -1393,6 +1393,12 @@ TEST(AEADTest, TestMonotonicityCheck) { sizeof(ciphertext), nonce, sizeof(nonce), plaintext, sizeof(plaintext), nullptr /* ad */, 0)); } + + // Attempting to encrypt with a decreased sequence number causes the monotonicity check to fail. + nonce[last_byte] = 0; + ASSERT_FALSE(EVP_AEAD_CTX_seal(encrypt_ctx.get(), ciphertext, &out_len, + sizeof(ciphertext), nonce, sizeof(nonce), plaintext, + sizeof(plaintext), nullptr /* ad */, 0)); } }