Reject invalid IV lengths in EVP_CTRL_GCM_SET_IV_INV

While I'm here, move the test added in
https://boringssl-review.googlesource.com/c/boringssl/+/95627 to a more
appropriate test. CipherTest.SetIVLengthResets is about something else.

Change-Id: Idfa75920464045a633ac6a5aef5946b35af910c1
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/96507
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Presubmit-BoringSSL-Verified: boringssl-scoped@luci-project-accounts.iam.gserviceaccount.com <boringssl-scoped@luci-project-accounts.iam.gserviceaccount.com>
Commit-Queue: Adam Langley <agl@google.com>

crypto/cipher/cipher_test.cc

diff --git a/crypto/cipher/cipher_test.cc b/crypto/cipher/cipher_test.cc
index a732427..665f344 100644
--- a/crypto/cipher/cipher_test.cc
+++ b/crypto/cipher/cipher_test.cc
@@ -1190,6 +1190,42 @@
     memcpy(iv + sizeof(kFixedIV), counter2, sizeof(counter2));
     ASSERT_NO_FATAL_FAILURE(expect_iv(ctx.get(), iv, /*enc=*/true));
   }
+
+  {
+    // If GCM IV length is less than 8, SET_IV_FIXED(-1) restores the whole IV,
+    // but a subsequent EVP_CTRL_GCM_IV_GEN call must fail rather than
+    // underflow.
+    const uint8_t kIV[7] = {1, 2, 3, 4, 5, 6, 7};
+    ScopedEVP_CIPHER_CTX ctx;
+    ASSERT_TRUE(EVP_EncryptInit_ex(ctx.get(), kCipher, /*impl=*/nullptr, kKey,
+                                   /*iv=*/nullptr));
+    ASSERT_TRUE(
+        EVP_CIPHER_CTX_ctrl(ctx.get(), EVP_CTRL_AEAD_SET_IVLEN, 7, nullptr));
+    ASSERT_TRUE(EVP_CIPHER_CTX_ctrl(ctx.get(), EVP_CTRL_AEAD_SET_IV_FIXED, -1,
+                                    const_cast<uint8_t *>(kIV)));
+    uint8_t counter[8];
+    EXPECT_FALSE(EVP_CIPHER_CTX_ctrl(ctx.get(), EVP_CTRL_GCM_IV_GEN,
+                                     sizeof(counter), counter));
+  }
+
+  {
+    // EVP_CTRL_GCM_SET_IV_INV should not overflow the IV.
+    const uint8_t kFixedIV[4] = {1, 2, 3, 4};
+    const uint8_t kIVInvTooLarge[13] = {1, 2, 3,  4,  5,  6, 7,
+                                        8, 9, 10, 11, 12, 13};
+
+    ScopedEVP_CIPHER_CTX ctx;
+    ASSERT_TRUE(EVP_DecryptInit_ex(ctx.get(), kCipher, /*impl=*/nullptr, kKey,
+                                   /*iv=*/nullptr));
+    ASSERT_TRUE(EVP_CIPHER_CTX_ctrl(ctx.get(), EVP_CTRL_AEAD_SET_IV_FIXED, 4,
+                                    const_cast<uint8_t *>(kFixedIV)));
+    // Negative lengths are invalid.
+    EXPECT_FALSE(EVP_CIPHER_CTX_ctrl(ctx.get(), EVP_CTRL_GCM_SET_IV_INV, -1,
+                                     const_cast<uint8_t *>(kIVInvTooLarge)));
+    // Overflows the IV.
+    EXPECT_FALSE(EVP_CIPHER_CTX_ctrl(ctx.get(), EVP_CTRL_GCM_SET_IV_INV, 13,
+                                     const_cast<uint8_t *>(kIVInvTooLarge)));
+  }
 }
 
 TEST(CipherTest, SetIVLengthResets) {
@@ -1247,22 +1283,6 @@
     int out_len;
     EXPECT_TRUE(EVP_EncryptUpdate(ctx.get(), out, &out_len, in, sizeof(in)));
   }
-
-  {
-    // If GCM IV length is less than 8, SET_IV_FIXED(-1) is allowed to succeed,
-    // but a subsequent EVP_CTRL_GCM_IV_GEN call must fail rather than
-    // underflow.
-    ScopedEVP_CIPHER_CTX ctx;
-    ASSERT_TRUE(EVP_EncryptInit_ex(ctx.get(), kCipher, /*impl=*/nullptr, kKey,
-                                   /*iv=*/nullptr));
-    ASSERT_TRUE(
-        EVP_CIPHER_CTX_ctrl(ctx.get(), EVP_CTRL_AEAD_SET_IVLEN, 7, nullptr));
-    ASSERT_TRUE(EVP_CIPHER_CTX_ctrl(ctx.get(), EVP_CTRL_AEAD_SET_IV_FIXED, -1,
-                                    const_cast<uint8_t *>(kIV)));
-    uint8_t counter[8];
-    EXPECT_FALSE(EVP_CIPHER_CTX_ctrl(ctx.get(), EVP_CTRL_GCM_IV_GEN,
-                                     sizeof(counter), counter));
-  }
 }
 
 // EVP_CIPHER's buffer management for AES-GCM's variable-length IV is messy.

crypto/fipsmodule/cipher/e_aes.cc.inc

diff --git a/crypto/fipsmodule/cipher/e_aes.cc.inc b/crypto/fipsmodule/cipher/e_aes.cc.inc
index bfdb647..83fa9ff 100644
--- a/crypto/fipsmodule/cipher/e_aes.cc.inc
+++ b/crypto/fipsmodule/cipher/e_aes.cc.inc
@@ -323,8 +323,8 @@
         gctx->iv_gen = 1;
         return 1;
       }
-      // Fixed field must be at least 4 bytes and invocation field
-      // at least 8.
+      // The fixed field must be at least 4 bytes and the invocation field at
+      // least 8.
       if (arg < 4 || (gctx->ivlen - arg) < 8) {
         return 0;
       }
@@ -356,7 +356,8 @@
     }
 
     case EVP_CTRL_GCM_SET_IV_INV:
-      if (gctx->iv_gen == 0 || gctx->key_set == 0 || c->encrypt) {
+      if (gctx->iv_gen == 0 || gctx->key_set == 0 || c->encrypt || arg < 0 ||
+          arg > gctx->ivlen) {
         return 0;
       }
       OPENSSL_memcpy(gctx->iv + gctx->ivlen - arg, ptr, arg);