crypto: fix edge case in authenticated encryption

Restricting the authentication tag length and calling update or
setAAD before setAuthTag caused an incorrect authentication tag to
be passed to OpenSSL: The auth_tag_len_ field was already set, so
the implementation assumed that the tag itself was known as well.

PR-URL: https://github.com/nodejs/node/pull/22828
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>

2 files changed, 14 insertions, 4 deletions

diff --git a/src/node_crypto.cc b/src/node_crypto.cc
index 4e97c39084..35b06e4ff0 100644
--- a/src/node_crypto.cc
+++ b/src/node_crypto.cc
@@ -2897,6 +2897,10 @@ void CipherBase::SetAuthTag(const FunctionCallbackInfo<Value>& args) {
     return args.GetReturnValue().Set(false);
   }
 
+  // TODO(tniessen): Throw if the authentication tag has already been set.
+  if (cipher->auth_tag_state_ == kAuthTagPassedToOpenSSL)
+    return args.GetReturnValue().Set(true);
+
   unsigned int tag_len = Buffer::Length(args[0]);
   const int mode = EVP_CIPHER_CTX_mode(cipher->ctx_.get());
   bool is_valid;
@@ -2921,6 +2925,7 @@ void CipherBase::SetAuthTag(const FunctionCallbackInfo<Value>& args) {
   }
 
   cipher->auth_tag_len_ = tag_len;
+  cipher->auth_tag_state_ = kAuthTagKnown;
   CHECK_LE(cipher->auth_tag_len_, sizeof(cipher->auth_tag_));
 
   memset(cipher->auth_tag_, 0, sizeof(cipher->auth_tag_));
@@ -2931,14 +2936,14 @@ void CipherBase::SetAuthTag(const FunctionCallbackInfo<Value>& args) {
 
 
 bool CipherBase::MaybePassAuthTagToOpenSSL() {
-  if (!auth_tag_set_ && auth_tag_len_ != kNoAuthTagLength) {
+  if (auth_tag_state_ == kAuthTagKnown) {
     if (!EVP_CIPHER_CTX_ctrl(ctx_.get(),
                              EVP_CTRL_AEAD_SET_TAG,
                              auth_tag_len_,
                              reinterpret_cast<unsigned char*>(auth_tag_))) {
       return false;
     }
-    auth_tag_set_ = true;
+    auth_tag_state_ = kAuthTagPassedToOpenSSL;
   }
   return true;
 }
diff --git a/src/node_crypto.h b/src/node_crypto.h
index 86aa3ba4ba..1a93ae7a47 100644
--- a/src/node_crypto.h
+++ b/src/node_crypto.h
@@ -363,6 +363,11 @@ class CipherBase : public BaseObject {
     kErrorMessageSize,
     kErrorState
   };
+  enum AuthTagState {
+    kAuthTagUnknown,
+    kAuthTagKnown,
+    kAuthTagPassedToOpenSSL
+  };
   static const unsigned kNoAuthTagLength = static_cast<unsigned>(-1);
 
   void Init(const char* cipher_type,
@@ -404,7 +409,7 @@ class CipherBase : public BaseObject {
       : BaseObject(env, wrap),
         ctx_(nullptr),
         kind_(kind),
-        auth_tag_set_(false),
+        auth_tag_state_(kAuthTagUnknown),
         auth_tag_len_(kNoAuthTagLength),
         pending_auth_failed_(false) {
     MakeWeak();
@@ -413,7 +418,7 @@ class CipherBase : public BaseObject {
  private:
   DeleteFnPtr<EVP_CIPHER_CTX, EVP_CIPHER_CTX_free> ctx_;
   const CipherKind kind_;
-  bool auth_tag_set_;
+  AuthTagState auth_tag_state_;
   unsigned int auth_tag_len_;
   char auth_tag_[EVP_GCM_TLS_TAG_LEN];
   bool pending_auth_failed_;