AES-GCM is not defined for empty nonces.

It shouldn't have been defined for variable-length nonces at all, but so
it goes. EVP_CIPHER rejected this by way of EVP_CTRL_GCM_SET_IVLEN
comparing <= 0, but the EVP_AEAD API did not.

I've done the test in a separate file on the assumption that aead_test
will become GTest shortly, at which point it will be easy to stick extra
tests into the same file as the FileTest ones.

Thanks to Daniel Bleichenbacher and Thanh Bui of Project Wycheproof for
the report.

Change-Id: Ic4616b39a1d7fe74a1f14fb58cccec2ce7c4f2f3
Reviewed-on: https://boringssl-review.googlesource.com/16544
Reviewed-by: Adam Langley <agl@google.com>

crypto/CMakeLists.txt

@@ -232,6 +232,7 @@ add_executable(
   bio/bio_test.cc
   bytestring/bytestring_test.cc
   chacha/chacha_test.cc
+  cipher_extra/aead_extra_test.cc
   cmac/cmac_test.cc
   compiler_test.cc
   constant_time_test.cc

crypto/cipher_extra/aead_extra_test.cc

@@ -0,0 +1,68 @@
+/* Copyright (c) 2017, Google Inc.
+ *
+ * Permission to use, copy, modify, and/or distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY
+ * SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION
+ * OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
+ * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */
+
+#include <gtest/gtest.h>
+
+#include <openssl/aead.h>
+#include <openssl/cipher.h>
+#include <openssl/err.h>
+
+
+// Test that EVP_aead_aes_128_gcm and EVP_aead_aes_256_gcm reject empty nonces.
+// AES-GCM is not defined for those.
+//
+// TODO(davidben): Fold this into aead_test.cc, once it is converted to GTest.
+TEST(AEADTest, AESGCMEmptyNonce) {
+  static const uint8_t kZeros[32] = {0};
+
+  // 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));
+
+  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));
+
+  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));
+
+  // 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));
+}

crypto/fipsmodule/cipher/e_aes.c

@@ -1193,6 +1193,11 @@ static int aead_aes_gcm_seal(const EVP_AEAD_CTX *ctx, uint8_t *out,
   const struct aead_aes_gcm_ctx *gcm_ctx = ctx->aead_state;
   GCM128_CONTEXT gcm;
 
+  if (nonce_len == 0) {
+    OPENSSL_PUT_ERROR(CIPHER, CIPHER_R_INVALID_NONCE_SIZE);
+    return 0;
+  }
+
   if (in_len + gcm_ctx->tag_len < in_len) {
     OPENSSL_PUT_ERROR(CIPHER, CIPHER_R_TOO_LARGE);
     return 0;
@@ -1238,6 +1243,11 @@ static int aead_aes_gcm_open(const EVP_AEAD_CTX *ctx, uint8_t *out,
   size_t plaintext_len;
   GCM128_CONTEXT gcm;
 
+  if (nonce_len == 0) {
+    OPENSSL_PUT_ERROR(CIPHER, CIPHER_R_INVALID_NONCE_SIZE);
+    return 0;
+  }
+
   if (in_len < gcm_ctx->tag_len) {
     OPENSSL_PUT_ERROR(CIPHER, CIPHER_R_BAD_DECRYPT);
     return 0;