crypto: make auth tag size assumption explicit

The `CipherBase` class assumes that any authentication tag will fit into
`EVP_GCM_TLS_TAG_LEN` bytes, which is true because Node.js only supports
GCM with AES as the blocker cipher, and the block size of AES happens to
be 16 bytes, which coincidentally is also the output size of the
Poly1305 construction used by ChaCha20-Poly1305 as well as the maximum
size of authentication tags produced by AES in CCM or OCB mode.

This commit adds a new constant `ncrypto::Cipher::MAX_AUTH_TAG_LENGTH`
which is the maximum length of authentication tags produced by
algorithms that Node.js supports and replaces some constants in
`CipherBase` with semantically more meaningful named constants.

The OpenSSL team is debating whether a constant like
`MAX_AUTH_TAG_LENGTH` (`EVP_MAX_AEAD_TAG_LENGTH`) should exist at all
since its value necessarily depends on the set of AEAD algorithms
supported, but I do believe that, for Node.js, this is a step in the
right direction. It certainly makes more sense than to use the AES-GCM
tag size as defined by TLS.

PR-URL: #57803
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>

Author: tniessen

deps/ncrypto/ncrypto.h

@@ -285,6 +285,14 @@ class Cipher final {
  public:
   static constexpr size_t MAX_KEY_LENGTH = EVP_MAX_KEY_LENGTH;
   static constexpr size_t MAX_IV_LENGTH = EVP_MAX_IV_LENGTH;
+#ifdef EVP_MAX_AEAD_TAG_LENGTH
+  static constexpr size_t MAX_AUTH_TAG_LENGTH = EVP_MAX_AEAD_TAG_LENGTH;
+#else
+  static constexpr size_t MAX_AUTH_TAG_LENGTH = 16;
+#endif
+  static_assert(EVP_GCM_TLS_TAG_LEN <= MAX_AUTH_TAG_LENGTH &&
+                EVP_CCM_TLS_TAG_LEN <= MAX_AUTH_TAG_LENGTH &&
+                EVP_CHACHAPOLY_TLS_TAG_LEN <= MAX_AUTH_TAG_LENGTH);
 
   Cipher() = default;
   Cipher(const EVP_CIPHER* cipher) : cipher_(cipher) {}

src/crypto/crypto_cipher.cc

@@ -459,7 +459,7 @@ bool CipherBase::InitAuthenticated(std::string_view cipher_type,
       // authentication tag length also defaults to 16 bytes when decrypting,
       // whereas GCM would accept any valid authentication tag length.
       if (ctx_.isChaCha20Poly1305()) {
-        auth_tag_len = 16;
+        auth_tag_len = EVP_CHACHAPOLY_TLS_TAG_LEN;
       } else {
         THROW_ERR_CRYPTO_INVALID_AUTH_TAG(
           env(), "authTagLength required for %s", cipher_type);
@@ -572,7 +572,7 @@ void CipherBase::SetAuthTag(const FunctionCallbackInfo<Value>& args) {
   }
 
   if (cipher->ctx_.isGcmMode() && cipher->auth_tag_len_ == kNoAuthTagLength &&
-      tag_len != 16 && env->EmitProcessEnvWarning()) {
+      tag_len != EVP_GCM_TLS_TAG_LEN && env->EmitProcessEnvWarning()) {
     if (ProcessEmitDeprecationWarning(
             env,
             "Using AES-GCM authentication tags of less than 128 bits without "
@@ -821,7 +821,7 @@ bool CipherBase::Final(std::unique_ptr<BackingStore>* out) {
       // always be given by the user.
       if (auth_tag_len_ == kNoAuthTagLength) {
         CHECK(ctx_.isGcmMode());
-        auth_tag_len_ = sizeof(auth_tag_);
+        auth_tag_len_ = EVP_GCM_TLS_TAG_LEN;
       }
       ok = ctx_.getAeadTag(auth_tag_len_,
                            reinterpret_cast<unsigned char*>(auth_tag_));

src/crypto/crypto_cipher.h

@@ -86,7 +86,7 @@ class CipherBase : public BaseObject {
   const CipherKind kind_;
   AuthTagState auth_tag_state_;
   unsigned int auth_tag_len_;
-  char auth_tag_[EVP_GCM_TLS_TAG_LEN];
+  char auth_tag_[ncrypto::Cipher::MAX_AUTH_TAG_LENGTH];
   bool pending_auth_failed_;
   int max_message_size_;
 };