From c2f18700afe83e0ac30d410e4d3b1b8cfed024f9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= <tniessen@tnie.de>
Date: Fri, 13 Apr 2018 18:02:46 +0200
Subject: [PATCH 1/2] crypto: allow to restrict valid GCM tag length

This change allows users to restrict accepted GCM authentication tag
lengths to a single value.

Fixes: https://github.com/nodejs/node/issues/17523
---
 doc/api/crypto.md                          |  8 ++++-
 src/node_crypto.cc                         | 26 +++++++++++++--
 src/node_crypto.h                          |  4 +--
 test/parallel/test-crypto-authenticated.js | 37 ++++++++++++++++++++++
 4 files changed, 69 insertions(+), 6 deletions(-)

diff --git a/doc/api/crypto.md b/doc/api/crypto.md
index b4ff52d89d6181..d42d396fb9b19d 100644
--- a/doc/api/crypto.md
+++ b/doc/api/crypto.md
@@ -1457,6 +1457,10 @@ to create the `Decipher` object.
 <!-- YAML
 added: v0.1.94
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/???
+    description: The `authTagLength` option can now be used to restrict accepted
+                 GCM authentication tag lengths.
   - version: v9.9.0
     pr-url: https://github.com/nodejs/node/pull/18644
     description: The `iv` parameter may now be `null` for ciphers which do not
@@ -1474,7 +1478,9 @@ and initialization vector (`iv`).
 The `options` argument controls stream behavior and is optional except when a
 cipher in CCM mode is used (e.g. `'aes-128-ccm'`). In that case, the
 `authTagLength` option is required and specifies the length of the
-authentication tag in bytes, see [CCM mode][].
+authentication tag in bytes, see [CCM mode][]. In GCM mode, the `authTagLength`
+option is not required but can be used to restrict accepted authentication tags
+to those with the specified length.
 
 The `algorithm` is dependent on OpenSSL, examples are `'aes192'`, etc. On
 recent OpenSSL releases, `openssl list-cipher-algorithms` will display the
diff --git a/src/node_crypto.cc b/src/node_crypto.cc
index 1a1eca297b7a07..6ec56bef46764a 100644
--- a/src/node_crypto.cc
+++ b/src/node_crypto.cc
@@ -2806,6 +2806,10 @@ void CipherBase::InitIv(const FunctionCallbackInfo<Value>& args) {
 }
 
 
+static bool IsValidGCMTagLength(int tag_len) {
+  return tag_len == 4 || tag_len == 8 || (tag_len >= 12 && tag_len <= 16);
+}
+
 bool CipherBase::InitAuthenticated(const char *cipher_type, int iv_len,
                                    int auth_tag_len) {
   CHECK(IsAuthenticatedMode());
@@ -2818,7 +2822,8 @@ bool CipherBase::InitAuthenticated(const char *cipher_type, int iv_len,
     return false;
   }
 
-  if (EVP_CIPHER_CTX_mode(ctx_) == EVP_CIPH_CCM_MODE) {
+  const int mode = EVP_CIPHER_CTX_mode(ctx_);
+  if (mode == EVP_CIPH_CCM_MODE) {
     if (auth_tag_len < 0) {
       char msg[128];
       snprintf(msg, sizeof(msg), "authTagLength required for %s", cipher_type);
@@ -2851,6 +2856,19 @@ bool CipherBase::InitAuthenticated(const char *cipher_type, int iv_len,
     } else {
       max_message_size_ = INT_MAX;
     }
+  } else {
+    CHECK_EQ(mode, EVP_CIPH_GCM_MODE);
+
+    if (auth_tag_len >= 0 && !IsValidGCMTagLength(auth_tag_len)) {
+      char msg[50];
+      snprintf(msg, sizeof(msg),
+          "Invalid GCM authentication tag length: %u", auth_tag_len);
+      env()->ThrowError(msg);
+      return false;
+    }
+
+    // Remember the given authentication tag length for later.
+    auth_tag_len_ = auth_tag_len;
   }
 
   return true;
@@ -2886,10 +2904,11 @@ void CipherBase::GetAuthTag(const FunctionCallbackInfo<Value>& args) {
   // Only callable after Final and if encrypting.
   if (cipher->ctx_ != nullptr ||
       cipher->kind_ != kCipher ||
-      cipher->auth_tag_len_ == 0) {
+      cipher->auth_tag_len_ <= 0) {
     return args.GetReturnValue().SetUndefined();
   }
 
+  CHECK_GE(cipher->auth_tag_len_, 0);
   Local<Object> buf =
       Buffer::Copy(env, cipher->auth_tag_, cipher->auth_tag_len_)
       .ToLocalChecked();
@@ -2911,7 +2930,8 @@ void CipherBase::SetAuthTag(const FunctionCallbackInfo<Value>& args) {
   unsigned int tag_len = Buffer::Length(args[0]);
   const int mode = EVP_CIPHER_CTX_mode(cipher->ctx_);
   if (mode == EVP_CIPH_GCM_MODE) {
-    if (tag_len > 16 || (tag_len < 12 && tag_len != 8 && tag_len != 4)) {
+    if ((cipher->auth_tag_len_ >= 0 && cipher->auth_tag_len_ != tag_len) ||
+        !IsValidGCMTagLength(tag_len)) {
       char msg[50];
       snprintf(msg, sizeof(msg),
           "Invalid GCM authentication tag length: %u", tag_len);
diff --git a/src/node_crypto.h b/src/node_crypto.h
index 2f7c904ee9212d..2ee3e2ea5955c7 100644
--- a/src/node_crypto.h
+++ b/src/node_crypto.h
@@ -398,7 +398,7 @@ class CipherBase : public BaseObject {
         ctx_(nullptr),
         kind_(kind),
         auth_tag_set_(false),
-        auth_tag_len_(0),
+        auth_tag_len_(-1),
         pending_auth_failed_(false) {
     MakeWeak<CipherBase>(this);
   }
@@ -407,7 +407,7 @@ class CipherBase : public BaseObject {
   EVP_CIPHER_CTX* ctx_;
   const CipherKind kind_;
   bool auth_tag_set_;
-  unsigned int auth_tag_len_;
+  int auth_tag_len_;
   char auth_tag_[EVP_GCM_TLS_TAG_LEN];
   bool pending_auth_failed_;
   int max_message_size_;
diff --git a/test/parallel/test-crypto-authenticated.js b/test/parallel/test-crypto-authenticated.js
index df5ba0392395af..6c4153875264fd 100644
--- a/test/parallel/test-crypto-authenticated.js
+++ b/test/parallel/test-crypto-authenticated.js
@@ -726,9 +726,46 @@ for (const test of TEST_CASES) {
       type: Error,
       message: `Invalid GCM authentication tag length: ${length}`
     });
+
+    common.expectsError(() => {
+      crypto.createDecipheriv('aes-256-gcm',
+                              'FxLKsqdmv0E9xrQhp0b1ZgI0K7JFZJM8',
+                              'qkuZpJWCewa6Szih',
+                              {
+                                authTagLength: length
+                              });
+    }, {
+      type: Error,
+      message: `Invalid GCM authentication tag length: ${length}`
+    });
   }
 }
 
+// Test that users can manually restrict the GCM tag length to a single value.
+{
+  const decipher = crypto.createDecipheriv('aes-256-gcm',
+                                           'FxLKsqdmv0E9xrQhp0b1ZgI0K7JFZJM8',
+                                           'qkuZpJWCewa6Szih', {
+                                             authTagLength: 8
+                                           });
+
+  common.expectsError(() => {
+    // This tag would normally be allowed.
+    decipher.setAuthTag(Buffer.from('1'.repeat(12)));
+  }, {
+    type: Error,
+    message: 'Invalid GCM authentication tag length: 12'
+  });
+
+  // The Decipher object should be left intact.
+  decipher.setAuthTag(Buffer.from('445352d3ff85cf94', 'hex'));
+  const text = Buffer.concat([
+    decipher.update('3a2a3647', 'hex'),
+    decipher.final()
+  ]);
+  assert.strictEqual(text.toString('utf8'), 'node');
+}
+
 // Test that create(De|C)ipher(iv)? throws if the mode is CCM and an invalid
 // authentication tag length has been specified.
 {

From d4520f88467b5f545491fe7a4d851303d518746a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= <tniessen@tnie.de>
Date: Sat, 21 Apr 2018 14:41:41 +0200
Subject: [PATCH 2/2] fixup! crypto: allow to restrict valid GCM tag length

---
 src/node_crypto.cc | 35 +++++++++++++++++++----------------
 src/node_crypto.h  |  5 +++--
 2 files changed, 22 insertions(+), 18 deletions(-)

diff --git a/src/node_crypto.cc b/src/node_crypto.cc
index 6ec56bef46764a..041ee80fcc71d7 100644
--- a/src/node_crypto.cc
+++ b/src/node_crypto.cc
@@ -2806,8 +2806,8 @@ void CipherBase::InitIv(const FunctionCallbackInfo<Value>& args) {
 }
 
 
-static bool IsValidGCMTagLength(int tag_len) {
-  return tag_len == 4 || tag_len == 8 || (tag_len >= 12 && tag_len <= 16);
+static bool IsValidGCMTagLength(unsigned int tag_len) {
+  return tag_len == 4 || tag_len == 8 || tag_len >= 12 && tag_len <= 16;
 }
 
 bool CipherBase::InitAuthenticated(const char *cipher_type, int iv_len,
@@ -2859,16 +2859,18 @@ bool CipherBase::InitAuthenticated(const char *cipher_type, int iv_len,
   } else {
     CHECK_EQ(mode, EVP_CIPH_GCM_MODE);
 
-    if (auth_tag_len >= 0 && !IsValidGCMTagLength(auth_tag_len)) {
-      char msg[50];
-      snprintf(msg, sizeof(msg),
-          "Invalid GCM authentication tag length: %u", auth_tag_len);
-      env()->ThrowError(msg);
-      return false;
-    }
+    if (auth_tag_len >= 0) {
+      if (!IsValidGCMTagLength(auth_tag_len)) {
+        char msg[50];
+        snprintf(msg, sizeof(msg),
+            "Invalid GCM authentication tag length: %u", auth_tag_len);
+        env()->ThrowError(msg);
+        return false;
+      }
 
-    // Remember the given authentication tag length for later.
-    auth_tag_len_ = auth_tag_len;
+      // Remember the given authentication tag length for later.
+      auth_tag_len_ = auth_tag_len;
+    }
   }
 
   return true;
@@ -2904,11 +2906,10 @@ void CipherBase::GetAuthTag(const FunctionCallbackInfo<Value>& args) {
   // Only callable after Final and if encrypting.
   if (cipher->ctx_ != nullptr ||
       cipher->kind_ != kCipher ||
-      cipher->auth_tag_len_ <= 0) {
+      cipher->auth_tag_len_ == kNoAuthTagLength) {
     return args.GetReturnValue().SetUndefined();
   }
 
-  CHECK_GE(cipher->auth_tag_len_, 0);
   Local<Object> buf =
       Buffer::Copy(env, cipher->auth_tag_, cipher->auth_tag_len_)
       .ToLocalChecked();
@@ -2930,7 +2931,8 @@ void CipherBase::SetAuthTag(const FunctionCallbackInfo<Value>& args) {
   unsigned int tag_len = Buffer::Length(args[0]);
   const int mode = EVP_CIPHER_CTX_mode(cipher->ctx_);
   if (mode == EVP_CIPH_GCM_MODE) {
-    if ((cipher->auth_tag_len_ >= 0 && cipher->auth_tag_len_ != tag_len) ||
+    if (cipher->auth_tag_len_ != kNoAuthTagLength &&
+        cipher->auth_tag_len_ != tag_len ||
         !IsValidGCMTagLength(tag_len)) {
       char msg[50];
       snprintf(msg, sizeof(msg),
@@ -2967,7 +2969,8 @@ bool CipherBase::SetAAD(const char* data, unsigned int len, int plaintext_len) {
     if (!CheckCCMMessageLength(plaintext_len))
       return false;
 
-    if (kind_ == kDecipher && !auth_tag_set_ && auth_tag_len_ > 0) {
+    if (kind_ == kDecipher && !auth_tag_set_ && auth_tag_len_ > 0 &&
+        auth_tag_len_ != kNoAuthTagLength) {
       if (!EVP_CIPHER_CTX_ctrl(ctx_,
                                EVP_CTRL_CCM_SET_TAG,
                                auth_tag_len_,
@@ -3020,7 +3023,7 @@ CipherBase::UpdateResult CipherBase::Update(const char* data,
 
   // on first update:
   if (kind_ == kDecipher && IsAuthenticatedMode() && auth_tag_len_ > 0 &&
-      !auth_tag_set_) {
+      auth_tag_len_ != kNoAuthTagLength && !auth_tag_set_) {
     EVP_CIPHER_CTX_ctrl(ctx_,
                         EVP_CTRL_GCM_SET_TAG,
                         auth_tag_len_,
diff --git a/src/node_crypto.h b/src/node_crypto.h
index 2ee3e2ea5955c7..3c166f5dcc89fc 100644
--- a/src/node_crypto.h
+++ b/src/node_crypto.h
@@ -359,6 +359,7 @@ class CipherBase : public BaseObject {
     kErrorMessageSize,
     kErrorState
   };
+  static const unsigned kNoAuthTagLength = static_cast<unsigned>(-1);
 
   void Init(const char* cipher_type,
             const char* key_buf,
@@ -398,7 +399,7 @@ class CipherBase : public BaseObject {
         ctx_(nullptr),
         kind_(kind),
         auth_tag_set_(false),
-        auth_tag_len_(-1),
+        auth_tag_len_(kNoAuthTagLength),
         pending_auth_failed_(false) {
     MakeWeak<CipherBase>(this);
   }
@@ -407,7 +408,7 @@ class CipherBase : public BaseObject {
   EVP_CIPHER_CTX* ctx_;
   const CipherKind kind_;
   bool auth_tag_set_;
-  int auth_tag_len_;
+  unsigned int auth_tag_len_;
   char auth_tag_[EVP_GCM_TLS_TAG_LEN];
   bool pending_auth_failed_;
   int max_message_size_;