From 4ad11b5a61b557deb2dbd739b3402e7bb4955d2a Mon Sep 17 00:00:00 2001 From: James M Snell Date: Thu, 26 Oct 2017 13:06:09 -0700 Subject: [PATCH] crypto: migrate CipherBase to internal/errors Migrates most of CipherBase errors to use internal/errors. There are still a handful remaining that need to be handled separately PR-URL: https://github.com/nodejs/node/pull/16527 Reviewed-By: Matteo Collina Reviewed-By: Benjamin Gruenbaum Reviewed-By: Anatoli Papirovski Reviewed-By: Joyee Cheung --- doc/api/errors.md | 8 ++ lib/internal/crypto/cipher.js | 100 ++++++++++++++++--- lib/internal/errors.js | 1 + src/node_crypto.cc | 43 ++------ test/parallel/test-crypto-cipher-decipher.js | 46 ++++++--- 5 files changed, 135 insertions(+), 63 deletions(-) diff --git a/doc/api/errors.md b/doc/api/errors.md index 6d8eed9492..8805e5ec03 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -679,6 +679,13 @@ ever, happen. Used when an invalid [crypto digest algorithm][] is specified. + +### ERR_CRYPTO_INVALID_STATE + +Used generically when a crypto method is used on an object that is in an +invalid state. For instance, calling [`cipher.getAuthTag()`][] before calling +`cipher.final()`. + ### ERR_CRYPTO_SIGN_KEY_REQUIRED @@ -1498,6 +1505,7 @@ closed. Used when creation of a [`zlib`][] object fails due to incorrect configuration. [`--force-fips`]: cli.html#cli_force_fips +[`cipher.getAuthTag()`]: crypto.html#crypto_cipher_getauthtag [`crypto.timingSafeEqual()`]: crypto.html#crypto_crypto_timingsafeequal_a_b [`dgram.createSocket()`]: dgram.html#dgram_dgram_createsocket_options_callback [`ERR_INVALID_ARG_TYPE`]: #ERR_INVALID_ARG_TYPE diff --git a/lib/internal/crypto/cipher.js b/lib/internal/crypto/cipher.js index d9b31674c1..cd2f3960e5 100644 --- a/lib/internal/crypto/cipher.js +++ b/lib/internal/crypto/cipher.js @@ -5,11 +5,15 @@ const { RSA_PKCS1_PADDING } = process.binding('constants').crypto; +const errors = require('internal/errors'); + const { getDefaultEncoding, toBuf } = require('internal/crypto/util'); +const { isArrayBufferView } = require('internal/util/types'); + const { CipherBase, privateDecrypt: _privateDecrypt, @@ -58,9 +62,19 @@ function getDecoder(decoder, encoding) { function Cipher(cipher, password, options) { if (!(this instanceof Cipher)) return new Cipher(cipher, password, options); + + if (typeof cipher !== 'string') + throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'cipher', 'string'); + + password = toBuf(password); + if (!isArrayBufferView(password)) { + throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'password', + ['string', 'Buffer', 'TypedArray', 'DataView']); + } + this._handle = new CipherBase(true); - this._handle.init(cipher, toBuf(password)); + this._handle.init(cipher, password); this._decoder = null; LazyTransform.call(this, options); @@ -88,11 +102,16 @@ Cipher.prototype.update = function update(data, inputEncoding, outputEncoding) { inputEncoding = inputEncoding || encoding; outputEncoding = outputEncoding || encoding; - var ret = this._handle.update(data, inputEncoding); + if (typeof data !== 'string' && !isArrayBufferView(data)) { + throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'data', + ['string', 'Buffer', 'TypedArray', 'DataView']); + } + + const ret = this._handle.update(data, inputEncoding); if (outputEncoding && outputEncoding !== 'buffer') { this._decoder = getDecoder(this._decoder, outputEncoding); - ret = this._decoder.write(ret); + return this._decoder.write(ret); } return ret; @@ -101,11 +120,11 @@ Cipher.prototype.update = function update(data, inputEncoding, outputEncoding) { Cipher.prototype.final = function final(outputEncoding) { outputEncoding = outputEncoding || getDefaultEncoding(); - var ret = this._handle.final(); + const ret = this._handle.final(); if (outputEncoding && outputEncoding !== 'buffer') { this._decoder = getDecoder(this._decoder, outputEncoding); - ret = this._decoder.end(ret); + return this._decoder.end(ret); } return ret; @@ -113,30 +132,63 @@ Cipher.prototype.final = function final(outputEncoding) { Cipher.prototype.setAutoPadding = function setAutoPadding(ap) { - this._handle.setAutoPadding(ap); + if (this._handle.setAutoPadding(ap) === false) + throw new errors.Error('ERR_CRYPTO_INVALID_STATE', 'setAutoPadding'); return this; }; Cipher.prototype.getAuthTag = function getAuthTag() { - return this._handle.getAuthTag(); + const ret = this._handle.getAuthTag(); + if (ret === undefined) + throw new errors.Error('ERR_CRYPTO_INVALID_STATE', 'getAuthTag'); + return ret; }; Cipher.prototype.setAuthTag = function setAuthTag(tagbuf) { - this._handle.setAuthTag(tagbuf); + if (!isArrayBufferView(tagbuf)) { + throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'buffer', + ['Buffer', 'TypedArray', 'DataView']); + } + // Do not do a normal falsy check because the method returns + // undefined if it succeeds. Returns false specifically if it + // errored + if (this._handle.setAuthTag(tagbuf) === false) + throw new errors.Error('ERR_CRYPTO_INVALID_STATE', 'setAuthTag'); return this; }; Cipher.prototype.setAAD = function setAAD(aadbuf) { - this._handle.setAAD(aadbuf); + if (!isArrayBufferView(aadbuf)) { + throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'buffer', + ['Buffer', 'TypedArray', 'DataView']); + } + if (this._handle.setAAD(aadbuf) === false) + throw new errors.Error('ERR_CRYPTO_INVALID_STATE', 'setAAD'); return this; }; function Cipheriv(cipher, key, iv, options) { if (!(this instanceof Cipheriv)) return new Cipheriv(cipher, key, iv, options); + + if (typeof cipher !== 'string') + throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'cipher', 'string'); + + key = toBuf(key); + if (!isArrayBufferView(key)) { + throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'key', + ['string', 'Buffer', 'TypedArray', 'DataView']); + } + + iv = toBuf(iv); + if (!isArrayBufferView(iv)) { + throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'iv', + ['string', 'Buffer', 'TypedArray', 'DataView']); + } + this._handle = new CipherBase(true); - this._handle.initiv(cipher, toBuf(key), toBuf(iv)); + this._handle.initiv(cipher, key, iv); this._decoder = null; LazyTransform.call(this, options); @@ -158,8 +210,17 @@ function Decipher(cipher, password, options) { if (!(this instanceof Decipher)) return new Decipher(cipher, password, options); + if (typeof cipher !== 'string') + throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'cipher', 'string'); + + password = toBuf(password); + if (!isArrayBufferView(password)) { + throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'password', + ['string', 'Buffer', 'TypedArray', 'DataView']); + } + this._handle = new CipherBase(false); - this._handle.init(cipher, toBuf(password)); + this._handle.init(cipher, password); this._decoder = null; LazyTransform.call(this, options); @@ -182,8 +243,23 @@ function Decipheriv(cipher, key, iv, options) { if (!(this instanceof Decipheriv)) return new Decipheriv(cipher, key, iv, options); + if (typeof cipher !== 'string') + throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'cipher', 'string'); + + key = toBuf(key); + if (!isArrayBufferView(key)) { + throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'key', + ['string', 'Buffer', 'TypedArray', 'DataView']); + } + + iv = toBuf(iv); + if (!isArrayBufferView(iv)) { + throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'iv', + ['string', 'Buffer', 'TypedArray', 'DataView']); + } + this._handle = new CipherBase(false); - this._handle.initiv(cipher, toBuf(key), toBuf(iv)); + this._handle.initiv(cipher, key, iv); this._decoder = null; LazyTransform.call(this, options); diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 4e6a2c4a5c..8862ca390d 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -164,6 +164,7 @@ E('ERR_CRYPTO_HASH_DIGEST_NO_UTF16', 'hash.digest() does not support UTF-16'); E('ERR_CRYPTO_HASH_FINALIZED', 'Digest already called'); E('ERR_CRYPTO_HASH_UPDATE_FAILED', 'Hash update failed'); E('ERR_CRYPTO_INVALID_DIGEST', 'Invalid digest: %s'); +E('ERR_CRYPTO_INVALID_STATE', 'Invalid state for operation %s'); E('ERR_CRYPTO_SIGN_KEY_REQUIRED', 'No key provided to sign'); E('ERR_CRYPTO_TIMING_SAFE_EQUAL_LENGTH', 'Input buffers must have the same length'); diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 5b4ce7c059..b6e7d54ea0 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -48,13 +48,6 @@ #include #include -#define THROW_AND_RETURN_IF_NOT_STRING_OR_BUFFER(val, prefix) \ - do { \ - if (!Buffer::HasInstance(val) && !val->IsString()) { \ - return env->ThrowTypeError(prefix " must be a string or a buffer"); \ - } \ - } while (0) - #define THROW_AND_RETURN_IF_NOT_BUFFER(val, prefix) \ do { \ if (!Buffer::HasInstance(val)) { \ @@ -3410,14 +3403,8 @@ void CipherBase::Init(const char* cipher_type, void CipherBase::Init(const FunctionCallbackInfo& args) { CipherBase* cipher; ASSIGN_OR_RETURN_UNWRAP(&cipher, args.Holder()); - Environment* env = cipher->env(); - - if (args.Length() < 2) { - return env->ThrowError("Cipher type and key arguments are mandatory"); - } - THROW_AND_RETURN_IF_NOT_STRING(args[0], "Cipher type"); - THROW_AND_RETURN_IF_NOT_BUFFER(args[1], "Key"); + CHECK_GE(args.Length(), 2); const node::Utf8Value cipher_type(args.GetIsolate(), args[0]); const char* key_buf = Buffer::Data(args[1]); @@ -3480,13 +3467,7 @@ void CipherBase::InitIv(const FunctionCallbackInfo& args) { ASSIGN_OR_RETURN_UNWRAP(&cipher, args.Holder()); Environment* env = cipher->env(); - if (args.Length() < 3) { - return env->ThrowError("Cipher type, key, and IV arguments are mandatory"); - } - - THROW_AND_RETURN_IF_NOT_STRING(args[0], "Cipher type"); - THROW_AND_RETURN_IF_NOT_BUFFER(args[1], "Key"); - THROW_AND_RETURN_IF_NOT_BUFFER(args[2], "IV"); + CHECK_GE(args.Length(), 3); const node::Utf8Value cipher_type(env->isolate(), args[0]); ssize_t key_len = Buffer::Length(args[1]); @@ -3515,7 +3496,7 @@ void CipherBase::GetAuthTag(const FunctionCallbackInfo& args) { if (cipher->initialised_ || cipher->kind_ != kCipher || cipher->auth_tag_len_ == 0) { - return env->ThrowError("Attempting to get auth tag in unsupported state"); + return args.GetReturnValue().SetUndefined(); } Local buf = @@ -3526,17 +3507,13 @@ void CipherBase::GetAuthTag(const FunctionCallbackInfo& args) { void CipherBase::SetAuthTag(const FunctionCallbackInfo& args) { - Environment* env = Environment::GetCurrent(args); - - THROW_AND_RETURN_IF_NOT_BUFFER(args[0], "Auth tag"); - CipherBase* cipher; ASSIGN_OR_RETURN_UNWRAP(&cipher, args.Holder()); if (!cipher->initialised_ || !cipher->IsAuthenticatedMode() || cipher->kind_ != kDecipher) { - return env->ThrowError("Attempting to set auth tag in unsupported state"); + return args.GetReturnValue().Set(false); } // FIXME(bnoordhuis) Throw when buffer length is not a valid tag size. @@ -3566,15 +3543,11 @@ bool CipherBase::SetAAD(const char* data, unsigned int len) { void CipherBase::SetAAD(const FunctionCallbackInfo& args) { - Environment* env = Environment::GetCurrent(args); - - THROW_AND_RETURN_IF_NOT_BUFFER(args[0], "AAD"); - CipherBase* cipher; ASSIGN_OR_RETURN_UNWRAP(&cipher, args.Holder()); if (!cipher->SetAAD(Buffer::Data(args[0]), Buffer::Length(args[0]))) - env->ThrowError("Attempting to set AAD in unsupported state"); + args.GetReturnValue().Set(false); // Report invalid state failure } @@ -3610,8 +3583,6 @@ void CipherBase::Update(const FunctionCallbackInfo& args) { CipherBase* cipher; ASSIGN_OR_RETURN_UNWRAP(&cipher, args.Holder()); - THROW_AND_RETURN_IF_NOT_STRING_OR_BUFFER(args[0], "Cipher data"); - unsigned char* out = nullptr; bool r; int out_len = 0; @@ -3651,13 +3622,11 @@ bool CipherBase::SetAutoPadding(bool auto_padding) { void CipherBase::SetAutoPadding(const FunctionCallbackInfo& args) { - Environment* env = Environment::GetCurrent(args); - CipherBase* cipher; ASSIGN_OR_RETURN_UNWRAP(&cipher, args.Holder()); if (!cipher->SetAutoPadding(args.Length() < 1 || args[0]->BooleanValue())) - env->ThrowError("Attempting to set auto padding in unsupported state"); + args.GetReturnValue().Set(false); // Report invalid state failure } diff --git a/test/parallel/test-crypto-cipher-decipher.js b/test/parallel/test-crypto-cipher-decipher.js index c1f1d84b79..78172d0003 100644 --- a/test/parallel/test-crypto-cipher-decipher.js +++ b/test/parallel/test-crypto-cipher-decipher.js @@ -162,9 +162,14 @@ testCipher2(Buffer.from('0123456789abcdef')); cipher.setAAD(aadbuf); cipher.setAutoPadding(); - assert.throws(() => { - cipher.getAuthTag(); - }, /^Error: Attempting to get auth tag in unsupported state$/); + common.expectsError( + () => cipher.getAuthTag(), + { + code: 'ERR_CRYPTO_INVALID_STATE', + type: Error, + message: 'Invalid state for operation getAuthTag' + } + ); const encrypted = Buffer.concat([cipher.update(data), cipher.final()]); @@ -175,15 +180,28 @@ testCipher2(Buffer.from('0123456789abcdef')); decipher.update(encrypted); decipher.final(); - assert.throws(() => { - decipher.setAAD(aadbuf); - }, /^Error: Attempting to set AAD in unsupported state$/); - - assert.throws(() => { - decipher.setAuthTag(cipher.getAuthTag()); - }, /^Error: Attempting to set auth tag in unsupported state$/); - - assert.throws(() => { - decipher.setAutoPadding(); - }, /^Error: Attempting to set auto padding in unsupported state$/); + common.expectsError( + () => decipher.setAAD(aadbuf), + { + code: 'ERR_CRYPTO_INVALID_STATE', + type: Error, + message: 'Invalid state for operation setAAD' + }); + + common.expectsError( + () => decipher.setAuthTag(cipher.getAuthTag()), + { + code: 'ERR_CRYPTO_INVALID_STATE', + type: Error, + message: 'Invalid state for operation setAuthTag' + }); + + common.expectsError( + () => decipher.setAutoPadding(), + { + code: 'ERR_CRYPTO_INVALID_STATE', + type: Error, + message: 'Invalid state for operation setAutoPadding' + } + ); }