crypto: make auth tag size assumption explicit #57803
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
tniessen
added
crypto
|
Review requested:
|
nodejs-github-bot
added
lib / src
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #57803 +/- ##
==========================================
- Coverage 90.23% 90.22% -0.02%
==========================================
Files 630 630
Lines 185288 185516 +228
Branches 36344 36384 +40
==========================================
+ Hits 167203 167375 +172
- Misses 11006 11033 +27
- Partials 7079 7108 +29
🚀 New features to boost your workflow:
|
addaleax
approved these changes
Apr 9, 2025
tniessen
added
author ready
github-actions
bot
removed
the
request-ci
This comment has been minimized.
This comment has been minimized.
jasnell
approved these changes
Apr 10, 2025
jasnell
pushed a commit
that referenced
this pull request
Apr 12, 2025
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 <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com>
|
Landed in 195ed4a |
RafaelGSS
pushed a commit
that referenced
this pull request
May 1, 2025
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 <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com>
RafaelGSS
pushed a commit
that referenced
this pull request
May 2, 2025
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 <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com>
aduh95
added
the
backport-requested-v22.x
|
This needs a manual backport for v22.x |
npaun
pushed a commit
to npaun/ncrypto
that referenced
this pull request
Sep 18, 2025
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: nodejs/node#57803 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com>
npaun
pushed a commit
to npaun/ncrypto
that referenced
this pull request
Sep 18, 2025
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: nodejs/node#57803 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com>
npaun
pushed a commit
to npaun/ncrypto
that referenced
this pull request
Sep 25, 2025
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: nodejs/node#57803 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com>
npaun
pushed a commit
to npaun/ncrypto
that referenced
this pull request
Sep 25, 2025
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: nodejs/node#57803 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com>
npaun
pushed a commit
to npaun/ncrypto
that referenced
this pull request
Sep 25, 2025
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: nodejs/node#57803 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com>
npaun
pushed a commit
to npaun/ncrypto
that referenced
this pull request
Sep 26, 2025
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: nodejs/node#57803 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com>
anonrig
pushed a commit
to nodejs/ncrypto
that referenced
this pull request
Sep 26, 2025
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: nodejs/node#57803 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The
CipherBaseclass assumes that any authentication tag will fit intoEVP_GCM_TLS_TAG_LENbytes, which is true because Node.js only supports GCM with AES as the block 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_LENGTHwhich is the maximum length of authentication tags produced by algorithms that Node.js supports and replaces some constants inCipherBasewith 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.(Then again,
ncrypto::Cipher::MAX_KEY_LENGTHis set to 512 bits and I do not believe that we currently support any ciphers that use 512-bit keys, so in the same sense, we could increasencrypto::Cipher::MAX_AUTH_TAG_LENGTHin the future just to be on the safe side for user-provisioned ciphers at the cost of allocating a few more bytes for eachCipherBaseobject.)