Skip to content

Commit d387c17

Browse files
tniessenMylesBorins
authored andcommitted
crypto: warn on invalid authentication tag length
PR-URL: #17566 Refs: #17523 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]>
1 parent f79d2ef commit d387c17

File tree

2 files changed

+29
-2
lines changed

2 files changed

+29
-2
lines changed

src/node_crypto.cc

+10-2
Original file line numberDiff line numberDiff line change
@@ -3806,9 +3806,17 @@ void CipherBase::SetAuthTag(const FunctionCallbackInfo<Value>& args) {
38063806
return env->ThrowError("Attempting to set auth tag in unsupported state");
38073807
}
38083808

3809-
// FIXME(bnoordhuis) Throw when buffer length is not a valid tag size.
3809+
// Restrict GCM tag lengths according to NIST 800-38d, page 9.
3810+
unsigned int tag_len = Buffer::Length(args[0]);
3811+
if (tag_len > 16 || (tag_len < 12 && tag_len != 8 && tag_len != 4)) {
3812+
ProcessEmitWarning(cipher->env(),
3813+
"Permitting authentication tag lengths of %u bytes is deprecated. "
3814+
"Valid GCM tag lengths are 4, 8, 12, 13, 14, 15, 16.",
3815+
tag_len);
3816+
}
3817+
38103818
// Note: we don't use std::max() here to work around a header conflict.
3811-
cipher->auth_tag_len_ = Buffer::Length(args[0]);
3819+
cipher->auth_tag_len_ = tag_len;
38123820
if (cipher->auth_tag_len_ > sizeof(cipher->auth_tag_))
38133821
cipher->auth_tag_len_ = sizeof(cipher->auth_tag_);
38143822

test/parallel/test-crypto-authenticated.js

+19
Original file line numberDiff line numberDiff line change
@@ -335,6 +335,14 @@ const errMessages = {
335335

336336
const ciphers = crypto.getCiphers();
337337

338+
common.expectWarning('Warning', (common.hasFipsCrypto ? [] : [
339+
'Use Cipheriv for counter mode of aes-192-gcm'
340+
]).concat(
341+
[0, 1, 2, 6, 9, 10, 11, 17]
342+
.map((i) => `Permitting authentication tag lengths of ${i} bytes is ` +
343+
'deprecated. Valid GCM tag lengths are 4, 8, 12, 13, 14, 15, 16.')
344+
));
345+
338346
for (const i in TEST_CASES) {
339347
const test = TEST_CASES[i];
340348

@@ -476,3 +484,14 @@ for (const i in TEST_CASES) {
476484
assert.throws(() => encrypt.setAAD(Buffer.from('123', 'ascii')),
477485
errMessages.state);
478486
}
487+
488+
// GCM only supports specific authentication tag lengths, invalid lengths should
489+
// produce warnings.
490+
{
491+
for (const length of [0, 1, 2, 4, 6, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17]) {
492+
const decrypt = crypto.createDecipheriv('aes-256-gcm',
493+
'FxLKsqdmv0E9xrQhp0b1ZgI0K7JFZJM8',
494+
'qkuZpJWCewa6Szih');
495+
decrypt.setAuthTag(Buffer.from('1'.repeat(length)));
496+
}
497+
}

0 commit comments

Comments
 (0)