Skip to content

Commit

Permalink
crypto: deprecate implicitly shortened GCM tags
Browse files Browse the repository at this point in the history
This introduces a doc-only deprecation of using GCM authentication tags
that are shorter than the cipher's block size, unless the user specified
the authTagLength option.

Refs: #52327
PR-URL: #52345
Reviewed-By: Filip Skokan <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: James M Snell <[email protected]>
  • Loading branch information
tniessen authored and marco-ippolito committed May 2, 2024
1 parent 58c26ed commit ec3148a
Show file tree
Hide file tree
Showing 6 changed files with 107 additions and 2 deletions.
5 changes: 5 additions & 0 deletions doc/api/crypto.md
Original file line number Diff line number Diff line change
Expand Up @@ -891,6 +891,11 @@ When passing a string as the `buffer`, please consider
<!-- YAML
added: v1.0.0
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/52345
description: Using GCM tag lengths other than 128 bits without specifying
the `authTagLength` option when creating `decipher` is
deprecated.
- version: v15.0.0
pr-url: https://github.com/nodejs/node/pull/35093
description: The buffer argument can be a string or ArrayBuffer and is
Expand Down
19 changes: 19 additions & 0 deletions doc/api/deprecations.md
Original file line number Diff line number Diff line change
Expand Up @@ -3507,6 +3507,25 @@ Calling `Hmac` class directly with `Hmac()` or `new Hmac()` is
deprecated due to being internals, not intended for public use.
Please use the [`crypto.createHmac()`][] method to create Hmac instances.

### DEP0182: Short GCM authentication tags without explicit `authTagLength`

<!-- YAML
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/52345
description: Documentation-only deprecation.
-->

Type: Documentation-only (supports [`--pending-deprecation`][])

Applications that intend to use authentication tags that are shorter than the
default authentication tag length should set the `authTagLength` option of the
[`crypto.createDecipheriv()`][] function to the appropriate length.

For ciphers in GCM mode, the [`decipher.setAuthTag()`][] function accepts
authentication tags of any valid length (see [DEP0090](#DEP0090)). This behavior
is deprecated to better align with recommendations per [NIST SP 800-38D][].

[NIST SP 800-38D]: https://nvlpubs.nist.gov/nistpubs/Legacy/SP/nistspecialpublication800-38d.pdf
[RFC 6066]: https://tools.ietf.org/html/rfc6066#section-3
[RFC 8247 Section 2.4]: https://www.rfc-editor.org/rfc/rfc8247#section-2.4
Expand Down
13 changes: 13 additions & 0 deletions src/crypto/crypto_cipher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -707,6 +707,19 @@ void CipherBase::SetAuthTag(const FunctionCallbackInfo<Value>& args) {
env, "Invalid authentication tag length: %u", tag_len);
}

if (mode == EVP_CIPH_GCM_MODE && cipher->auth_tag_len_ == kNoAuthTagLength &&
tag_len != 16 && env->options()->pending_deprecation &&
env->EmitProcessEnvWarning()) {
if (ProcessEmitDeprecationWarning(
env,
"Using AES-GCM authentication tags of less than 128 bits without "
"specifying the authTagLength option when initializing decryption "
"is deprecated.",
"DEP0182")
.IsNothing())
return;
}

cipher->auth_tag_len_ = tag_len;
cipher->auth_tag_state_ = kAuthTagKnown;
CHECK_LE(cipher->auth_tag_len_, sizeof(cipher->auth_tag_));
Expand Down
4 changes: 2 additions & 2 deletions test/common/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -654,12 +654,12 @@ function _expectWarning(name, expected, code) {
expected = [[expected, code]];
} else if (!Array.isArray(expected)) {
expected = Object.entries(expected).map(([a, b]) => [b, a]);
} else if (!(Array.isArray(expected[0]))) {
} else if (expected.length !== 0 && !Array.isArray(expected[0])) {
expected = [[expected[0], expected[1]]];
}
// Deprecation codes are mandatory, everything else is not.
if (name === 'DeprecationWarning') {
expected.forEach(([_, code]) => assert(code, expected));
expected.forEach(([_, code]) => assert(code, `Missing deprecation code: ${expected}`));
}
return mustCall((warning) => {
const expectedProperties = expected.shift();
Expand Down
47 changes: 47 additions & 0 deletions test/parallel/test-crypto-gcm-explicit-short-tag.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
// Flags: --pending-deprecation
'use strict';

const common = require('../common');
if (!common.hasCrypto)
common.skip('missing crypto');

const assert = require('assert');
const { createDecipheriv, randomBytes } = require('crypto');

common.expectWarning({
DeprecationWarning: []
});

const key = randomBytes(32);
const iv = randomBytes(16);

{
// Full 128-bit tag.

const tag = randomBytes(16);
createDecipheriv('aes-256-gcm', key, iv).setAuthTag(tag);
}

{
// Shortened tag with explicit length option.

const tag = randomBytes(12);
createDecipheriv('aes-256-gcm', key, iv, {
authTagLength: tag.byteLength
}).setAuthTag(tag);
}

{
// Shortened tag with explicit but incorrect length option.

const tag = randomBytes(12);
assert.throws(() => {
createDecipheriv('aes-256-gcm', key, iv, {
authTagLength: 14
}).setAuthTag(tag);
}, {
name: 'TypeError',
message: 'Invalid authentication tag length: 12',
code: 'ERR_CRYPTO_INVALID_AUTH_TAG'
});
}
21 changes: 21 additions & 0 deletions test/parallel/test-crypto-gcm-implicit-short-tag.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Flags: --pending-deprecation
'use strict';
const common = require('../common');
if (!common.hasCrypto)
common.skip('missing crypto');

const { createDecipheriv, randomBytes } = require('crypto');

common.expectWarning({
DeprecationWarning: [
['Using AES-GCM authentication tags of less than 128 bits without ' +
'specifying the authTagLength option when initializing decryption is ' +
'deprecated.',
'DEP0182'],
]
});

const key = randomBytes(32);
const iv = randomBytes(16);
const tag = randomBytes(12);
createDecipheriv('aes-256-gcm', key, iv).setAuthTag(tag);

0 comments on commit ec3148a

Please sign in to comment.