diff options
author | Tobias Nießen <tniessen@tnie.de> | 2018-09-18 14:14:50 +0200 |
---|---|---|
committer | Tobias Nießen <tniessen@tnie.de> | 2018-09-21 11:40:35 +0200 |
commit | 058c5b81cdbabe8989a194ba5d388f4c230f4af6 (patch) | |
tree | 0ddb8aebf76c79696c5af68d19d9cd9fb6f68d79 | |
parent | 56493bf1ebfab3ec102fe017f30fa4f81ba6a256 (diff) | |
download | android-node-v8-058c5b81cdbabe8989a194ba5d388f4c230f4af6.tar.gz android-node-v8-058c5b81cdbabe8989a194ba5d388f4c230f4af6.tar.bz2 android-node-v8-058c5b81cdbabe8989a194ba5d388f4c230f4af6.zip |
crypto: do not allow multiple calls to setAuthTag
Calling setAuthTag multiple times can result in hard to detect bugs
since to the user, it is unclear which invocation actually affected
OpenSSL.
PR-URL: https://github.com/nodejs/node/pull/22931
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>
-rw-r--r-- | doc/api/crypto.md | 2 | ||||
-rw-r--r-- | src/node_crypto.cc | 7 | ||||
-rw-r--r-- | test/parallel/test-crypto-authenticated.js | 26 |
3 files changed, 29 insertions, 6 deletions
diff --git a/doc/api/crypto.md b/doc/api/crypto.md index b26e9cae04..ecb0c8186c 100644 --- a/doc/api/crypto.md +++ b/doc/api/crypto.md @@ -445,7 +445,7 @@ is invalid according to [NIST SP 800-38D][] or does not match the value of the `authTagLength` option, `decipher.setAuthTag()` will throw an error. The `decipher.setAuthTag()` method must be called before -[`decipher.final()`][]. +[`decipher.final()`][] and can only be called once. ### decipher.setAutoPadding([autoPadding]) <!-- YAML diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 47e0ba5349..4cf3ac5652 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -2894,14 +2894,11 @@ void CipherBase::SetAuthTag(const FunctionCallbackInfo<Value>& args) { if (!cipher->ctx_ || !cipher->IsAuthenticatedMode() || - cipher->kind_ != kDecipher) { + cipher->kind_ != kDecipher || + cipher->auth_tag_state_ != kAuthTagUnknown) { return args.GetReturnValue().Set(false); } - // TODO(tniessen): Throw if the authentication tag has already been set. - if (cipher->auth_tag_state_ == kAuthTagPassedToOpenSSL) - return args.GetReturnValue().Set(true); - unsigned int tag_len = Buffer::Length(args[0]); const int mode = EVP_CIPHER_CTX_mode(cipher->ctx_.get()); bool is_valid; diff --git a/test/parallel/test-crypto-authenticated.js b/test/parallel/test-crypto-authenticated.js index 77587fadf7..ec5c05cb12 100644 --- a/test/parallel/test-crypto-authenticated.js +++ b/test/parallel/test-crypto-authenticated.js @@ -589,3 +589,29 @@ for (const test of TEST_CASES) { } } } + +// Test that setAuthTag can only be called once. +{ + const plain = Buffer.from('Hello world', 'utf8'); + const key = Buffer.from('0123456789abcdef', 'utf8'); + const iv = Buffer.from('0123456789ab', 'utf8'); + const opts = { authTagLength: 8 }; + + for (const mode of ['gcm', 'ccm', 'ocb']) { + const cipher = crypto.createCipheriv(`aes-128-${mode}`, key, iv, opts); + const ciphertext = Buffer.concat([cipher.update(plain), cipher.final()]); + const tag = cipher.getAuthTag(); + + const decipher = crypto.createDecipheriv(`aes-128-${mode}`, key, iv, opts); + decipher.setAuthTag(tag); + assert.throws(() => { + decipher.setAuthTag(tag); + }, errMessages.state); + // Decryption should still work. + const plaintext = Buffer.concat([ + decipher.update(ciphertext), + decipher.final() + ]); + assert(plain.equals(plaintext)); + } +} |