From 4f9fb30ff37a13380d89597fc7b213f6dc98c258 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= Date: Fri, 22 Dec 2017 18:51:33 +0100 Subject: [PATCH 1/3] crypto: throw on invalid authentication tag length Refs: https://github.com/nodejs/node/issues/17523 --- src/node_crypto.cc | 7 +++--- test/parallel/test-crypto-authenticated.js | 26 +++++++++++----------- 2 files changed, 16 insertions(+), 17 deletions(-) diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 9733fddcf3e2ec..be7e385c41e07f 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -2911,11 +2911,10 @@ void CipherBase::SetAuthTag(const FunctionCallbackInfo& 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); } } diff --git a/test/parallel/test-crypto-authenticated.js b/test/parallel/test-crypto-authenticated.js index 73e3a23f72ac81..df5ba0392395af 100644 --- a/test/parallel/test-crypto-authenticated.js +++ b/test/parallel/test-crypto-authenticated.js @@ -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, @@ -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}` + }); } } From fe805b76a17dbf3d94b47bf72ec4f15af19052f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= Date: Thu, 28 Dec 2017 14:33:19 +0100 Subject: [PATCH 2/3] doc: note that setAuthTag throws on invalid length --- doc/api/crypto.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/doc/api/crypto.md b/doc/api/crypto.md index ea38da218a119f..9f93d8f5789956 100644 --- a/doc/api/crypto.md +++ b/doc/api/crypto.md @@ -421,6 +421,9 @@ The `decipher.setAAD()` method must be called before [`decipher.update()`][].