Skip to content

Commit

Permalink
crypto: warn on invalid authentication tag length
Browse files Browse the repository at this point in the history
Backport-PR-URL: #18347
PR-URL: #17566
Refs: #17523
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
tniessen authored and MylesBorins committed Feb 12, 2018
1 parent d28fae8 commit 5bdb18e
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 1 deletion.
10 changes: 9 additions & 1 deletion src/node_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3525,8 +3525,16 @@ void CipherBase::SetAuthTag(const FunctionCallbackInfo<Value>& args) {

CipherBase* cipher;
ASSIGN_OR_RETURN_UNWRAP(&cipher, args.Holder());
// Restrict GCM tag lengths according to NIST 800-38d, page 9.
unsigned int tag_len = Buffer::Length(buf);
if (tag_len > 16 || (tag_len < 12 && tag_len != 8 && tag_len != 4)) {
ProcessEmitWarning(cipher->env(),
"Permitting authentication tag lengths of %u bytes is deprecated. "
"Valid GCM tag lengths are 4, 8, 12, 13, 14, 15, 16.",
tag_len);
}

if (!cipher->SetAuthTag(Buffer::Data(buf), Buffer::Length(buf)))
if (!cipher->SetAuthTag(Buffer::Data(buf), tag_len))
env->ThrowError("Attempting to set auth tag in unsupported state");
}

Expand Down
19 changes: 19 additions & 0 deletions test/parallel/test-crypto-authenticated.js
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,14 @@ const errMessages = {

const ciphers = crypto.getCiphers();

common.expectWarning('Warning', (common.hasFipsCrypto ? [] : [
'Use Cipheriv for counter mode of aes-192-gcm'
]).concat(
[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.')
));

for (const i in TEST_CASES) {
const test = TEST_CASES[i];

Expand Down Expand Up @@ -455,3 +463,14 @@ for (const i in TEST_CASES) {
assert.throws(() => encrypt.setAAD(Buffer.from('123', 'ascii')),
errMessages.state);
}

// GCM only supports specific authentication tag lengths, invalid lengths should
// produce warnings.
{
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)));
}
}

0 comments on commit 5bdb18e

Please sign in to comment.