Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

crypto: throw on invalid authentication tag length #17825

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion doc/api/crypto.md
Original file line number Diff line number Diff line change
Expand Up @@ -421,6 +421,9 @@ The `decipher.setAAD()` method must be called before [`decipher.update()`][].
<!-- YAML
added: v1.0.0
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/17825
description: This method now throws if the GCM tag length is invalid.
- version: v7.2.0
pr-url: https://github.com/nodejs/node/pull/9398
description: This method now returns a reference to `decipher`.
Expand All @@ -432,7 +435,9 @@ When using an authenticated encryption mode (only `GCM` is currently
supported), the `decipher.setAuthTag()` method is used to pass in the
received _authentication tag_. If no tag is provided, or if the cipher text
has been tampered with, [`decipher.final()`][] will throw, indicating that the
cipher text should be discarded due to failed authentication.
cipher text should be discarded due to failed authentication. If the tag length
is invalid according to [NIST SP 800-38D][], `decipher.setAuthTag()` will throw
an error.

Note that this Node.js version does not verify the length of GCM authentication
tags. Such a check *must* be implemented by applications and is crucial to the
Expand Down
13 changes: 6 additions & 7 deletions doc/api/deprecations.md
Original file line number Diff line number Diff line change
Expand Up @@ -822,14 +822,13 @@ same as the legacy assert but it will always use strict equality checks.
<a id="DEP0090"></a>
### DEP0090: Invalid GCM authentication tag lengths

Type: Runtime
Type: End-of-Life

Node.js supports all GCM authentication tag lengths which are accepted by
OpenSSL when calling [`decipher.setAuthTag()`][]. This behavior will change in
a future version at which point only authentication tag lengths of 128, 120,
112, 104, 96, 64, and 32 bits will be allowed. Authentication tags whose length
is not included in this list will be considered invalid in compliance with
[NIST SP 800-38D][].
Node.js used to support all GCM authentication tag lengths which are accepted by
OpenSSL when calling [`decipher.setAuthTag()`][]. Beginning with node REPLACEME,
only authentication tag lengths of 128, 120, 112, 104, 96, 64, and 32 bits are
allowed. Authentication tags whose length is not included in this list are
considered invalid in compliance with [NIST SP 800-38D][].

<a id="DEP0091"></a>
### DEP0091: crypto.DEFAULT_ENCODING
Expand Down
7 changes: 3 additions & 4 deletions src/node_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2911,11 +2911,10 @@ void CipherBase::SetAuthTag(const FunctionCallbackInfo<Value>& args) {
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)) {
char msg[125];
char msg[50];
snprintf(msg, sizeof(msg),
"Permitting authentication tag lengths of %u bytes is deprecated. "
"Valid GCM tag lengths are 4, 8, 12, 13, 14, 15, 16.", tag_len);
ProcessEmitDeprecationWarning(cipher->env(), msg, "DEP0090");
"Invalid GCM authentication tag length: %u", tag_len);
return cipher->env()->ThrowError(msg);
}
}

Expand Down
26 changes: 13 additions & 13 deletions test/parallel/test-crypto-authenticated.js
Original file line number Diff line number Diff line change
Expand Up @@ -534,13 +534,8 @@ const expectedWarnings = common.hasFipsCrypto ?
['Use Cipheriv for counter mode of aes-256-ccm', common.noWarnCode]
];

const expectedDeprecationWarnings = [0, 1, 2, 6, 9, 10, 11, 17]
.map((i) => [`Permitting authentication tag lengths of ${i} bytes is ` +
'deprecated. Valid GCM tag lengths are 4, 8, 12, 13, 14, 15, 16.',
'DEP0090']);

expectedDeprecationWarnings.push(['crypto.DEFAULT_ENCODING is deprecated.',
'DEP0091']);
const expectedDeprecationWarnings = ['crypto.DEFAULT_ENCODING is deprecated.',
'DEP0091'];

common.expectWarning({
Warning: expectedWarnings,
Expand Down Expand Up @@ -719,13 +714,18 @@ for (const test of TEST_CASES) {
}

// GCM only supports specific authentication tag lengths, invalid lengths should
// produce warnings.
// throw.
{
for (const length of [0, 1, 2, 4, 6, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17]) {
const decrypt = crypto.createDecipheriv('aes-256-gcm',
'FxLKsqdmv0E9xrQhp0b1ZgI0K7JFZJM8',
'qkuZpJWCewa6Szih');
decrypt.setAuthTag(Buffer.from('1'.repeat(length)));
for (const length of [0, 1, 2, 6, 9, 10, 11, 17]) {
common.expectsError(() => {
const decrypt = crypto.createDecipheriv('aes-128-gcm',
'FxLKsqdmv0E9xrQh',
'qkuZpJWCewa6Szih');
decrypt.setAuthTag(Buffer.from('1'.repeat(length)));
}, {
type: Error,
message: `Invalid GCM authentication tag length: ${length}`
});
}
}

Expand Down