From 4427dc245328b946a207ed503b5d51721ad59929 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= Date: Wed, 5 Sep 2018 11:55:00 +0200 Subject: [PATCH 1/2] crypto: hide native handles from JS modules --- doc/api/deprecations.md | 10 +++++++++ lib/internal/crypto/cipher.js | 24 +++++++++++---------- lib/internal/crypto/diffiehellman.js | 32 +++++++++++++++------------- lib/internal/crypto/hash.js | 18 +++++++++------- lib/internal/crypto/sig.js | 16 ++++++++------ lib/internal/crypto/util.js | 18 ++++++++++++++++ 6 files changed, 77 insertions(+), 41 deletions(-) diff --git a/doc/api/deprecations.md b/doc/api/deprecations.md index 340f577c17c0aa..44da1d0ee6d313 100644 --- a/doc/api/deprecations.md +++ b/doc/api/deprecations.md @@ -1058,6 +1058,16 @@ The [Legacy URL API][] is deprecated. This includes [`url.format()`][], [`url.parse()`][], [`url.resolve()`][], and the [legacy `urlObject`][]. Please use the [WHATWG URL API][] instead. + +### DEP00XX: Native crypto handles + +Type: Runtime + +Previous versions of Node.js exposed handles to internal native objects through +the `_handle` property of the `Cipher`, `Decipher`, `DiffieHellman`, +`DiffieHellmanGroup`, `ECDH`, `Hash`, `Hmac`, `Sign`, and `Verify` classes. +Using the `_handle` property to access the native object is deprecated because +improper use of the native object can lead to crashing the application. [`--pending-deprecation`]: cli.html#cli_pending_deprecation [`Buffer.allocUnsafeSlow(size)`]: buffer.html#buffer_class_method_buffer_allocunsafeslow_size diff --git a/lib/internal/crypto/cipher.js b/lib/internal/crypto/cipher.js index 4ca4171c1ae2d0..51dd3cea86606d 100644 --- a/lib/internal/crypto/cipher.js +++ b/lib/internal/crypto/cipher.js @@ -14,6 +14,8 @@ const { validateString } = require('internal/validators'); const { getDefaultEncoding, + kHandle, + legacyNativeHandle, toBuf } = require('internal/crypto/util'); @@ -73,11 +75,11 @@ function getUIntOption(options, key) { function createCipherBase(cipher, credential, options, decipher, iv) { const authTagLength = getUIntOption(options, 'authTagLength'); - this._handle = new CipherBase(decipher); + legacyNativeHandle(this, new CipherBase(decipher)); if (iv === undefined) { - this._handle.init(cipher, credential, authTagLength); + this[kHandle].init(cipher, credential, authTagLength); } else { - this._handle.initiv(cipher, credential, iv, authTagLength); + this[kHandle].initiv(cipher, credential, iv, authTagLength); } this._decoder = null; @@ -130,13 +132,13 @@ function Cipher(cipher, password, options) { inherits(Cipher, LazyTransform); Cipher.prototype._transform = function _transform(chunk, encoding, callback) { - this.push(this._handle.update(chunk, encoding)); + this.push(this[kHandle].update(chunk, encoding)); callback(); }; Cipher.prototype._flush = function _flush(callback) { try { - this.push(this._handle.final()); + this.push(this[kHandle].final()); } catch (e) { callback(e); return; @@ -157,7 +159,7 @@ Cipher.prototype.update = function update(data, inputEncoding, outputEncoding) { ); } - const ret = this._handle.update(data, inputEncoding); + const ret = this[kHandle].update(data, inputEncoding); if (outputEncoding && outputEncoding !== 'buffer') { this._decoder = getDecoder(this._decoder, outputEncoding); @@ -170,7 +172,7 @@ Cipher.prototype.update = function update(data, inputEncoding, outputEncoding) { Cipher.prototype.final = function final(outputEncoding) { outputEncoding = outputEncoding || getDefaultEncoding(); - const ret = this._handle.final(); + const ret = this[kHandle].final(); if (outputEncoding && outputEncoding !== 'buffer') { this._decoder = getDecoder(this._decoder, outputEncoding); @@ -182,13 +184,13 @@ Cipher.prototype.final = function final(outputEncoding) { Cipher.prototype.setAutoPadding = function setAutoPadding(ap) { - if (!this._handle.setAutoPadding(!!ap)) + if (!this[kHandle].setAutoPadding(!!ap)) throw new ERR_CRYPTO_INVALID_STATE('setAutoPadding'); return this; }; Cipher.prototype.getAuthTag = function getAuthTag() { - const ret = this._handle.getAuthTag(); + const ret = this[kHandle].getAuthTag(); if (ret === undefined) throw new ERR_CRYPTO_INVALID_STATE('getAuthTag'); return ret; @@ -201,7 +203,7 @@ function setAuthTag(tagbuf) { ['Buffer', 'TypedArray', 'DataView'], tagbuf); } - if (!this._handle.setAuthTag(tagbuf)) + if (!this[kHandle].setAuthTag(tagbuf)) throw new ERR_CRYPTO_INVALID_STATE('setAuthTag'); return this; } @@ -221,7 +223,7 @@ Cipher.prototype.setAAD = function setAAD(aadbuf, options) { } const plaintextLength = getUIntOption(options, 'plaintextLength'); - if (!this._handle.setAAD(aadbuf, plaintextLength)) + if (!this[kHandle].setAAD(aadbuf, plaintextLength)) throw new ERR_CRYPTO_INVALID_STATE('setAAD'); return this; }; diff --git a/lib/internal/crypto/diffiehellman.js b/lib/internal/crypto/diffiehellman.js index 1daf5df905f8ac..02142d4b1f0b2d 100644 --- a/lib/internal/crypto/diffiehellman.js +++ b/lib/internal/crypto/diffiehellman.js @@ -10,6 +10,8 @@ const { validateString } = require('internal/validators'); const { isArrayBufferView } = require('internal/util/types'); const { getDefaultEncoding, + kHandle, + legacyNativeHandle, toBuf } = require('internal/crypto/util'); const { @@ -59,10 +61,10 @@ function DiffieHellman(sizeOrKey, keyEncoding, generator, genEncoding) { else if (typeof generator !== 'number') generator = toBuf(generator, genEncoding); - this._handle = new _DiffieHellman(sizeOrKey, generator); + legacyNativeHandle(this, new _DiffieHellman(sizeOrKey, generator)); Object.defineProperty(this, 'verifyError', { enumerable: true, - value: this._handle.verifyError, + value: this[kHandle].verifyError, writable: false }); } @@ -71,10 +73,10 @@ function DiffieHellman(sizeOrKey, keyEncoding, generator, genEncoding) { function DiffieHellmanGroup(name) { if (!(this instanceof DiffieHellmanGroup)) return new DiffieHellmanGroup(name); - this._handle = new _DiffieHellmanGroup(name); + legacyNativeHandle(this, new _DiffieHellmanGroup(name)); Object.defineProperty(this, 'verifyError', { enumerable: true, - value: this._handle.verifyError, + value: this[kHandle].verifyError, writable: false }); } @@ -85,7 +87,7 @@ DiffieHellmanGroup.prototype.generateKeys = dhGenerateKeys; function dhGenerateKeys(encoding) { - const keys = this._handle.generateKeys(); + const keys = this[kHandle].generateKeys(); encoding = encoding || getDefaultEncoding(); return encode(keys, encoding); } @@ -99,7 +101,7 @@ function dhComputeSecret(key, inEnc, outEnc) { const encoding = getDefaultEncoding(); inEnc = inEnc || encoding; outEnc = outEnc || encoding; - const ret = this._handle.computeSecret(toBuf(key, inEnc)); + const ret = this[kHandle].computeSecret(toBuf(key, inEnc)); if (typeof ret === 'string') throw new ERR_CRYPTO_ECDH_INVALID_PUBLIC_KEY(); return encode(ret, outEnc); @@ -111,7 +113,7 @@ DiffieHellmanGroup.prototype.getPrime = dhGetPrime; function dhGetPrime(encoding) { - const prime = this._handle.getPrime(); + const prime = this[kHandle].getPrime(); encoding = encoding || getDefaultEncoding(); return encode(prime, encoding); } @@ -122,7 +124,7 @@ DiffieHellmanGroup.prototype.getGenerator = dhGetGenerator; function dhGetGenerator(encoding) { - const generator = this._handle.getGenerator(); + const generator = this[kHandle].getGenerator(); encoding = encoding || getDefaultEncoding(); return encode(generator, encoding); } @@ -133,7 +135,7 @@ DiffieHellmanGroup.prototype.getPublicKey = dhGetPublicKey; function dhGetPublicKey(encoding) { - const key = this._handle.getPublicKey(); + const key = this[kHandle].getPublicKey(); encoding = encoding || getDefaultEncoding(); return encode(key, encoding); } @@ -144,7 +146,7 @@ DiffieHellmanGroup.prototype.getPrivateKey = dhGetPrivateKey; function dhGetPrivateKey(encoding) { - const key = this._handle.getPrivateKey(); + const key = this[kHandle].getPrivateKey(); encoding = encoding || getDefaultEncoding(); return encode(key, encoding); } @@ -152,14 +154,14 @@ function dhGetPrivateKey(encoding) { DiffieHellman.prototype.setPublicKey = function setPublicKey(key, encoding) { encoding = encoding || getDefaultEncoding(); - this._handle.setPublicKey(toBuf(key, encoding)); + this[kHandle].setPublicKey(toBuf(key, encoding)); return this; }; DiffieHellman.prototype.setPrivateKey = function setPrivateKey(key, encoding) { encoding = encoding || getDefaultEncoding(); - this._handle.setPrivateKey(toBuf(key, encoding)); + this[kHandle].setPrivateKey(toBuf(key, encoding)); return this; }; @@ -169,7 +171,7 @@ function ECDH(curve) { return new ECDH(curve); validateString(curve, 'curve'); - this._handle = new _ECDH(curve); + legacyNativeHandle(this, new _ECDH(curve)); } ECDH.prototype.computeSecret = DiffieHellman.prototype.computeSecret; @@ -178,14 +180,14 @@ ECDH.prototype.setPublicKey = DiffieHellman.prototype.setPublicKey; ECDH.prototype.getPrivateKey = DiffieHellman.prototype.getPrivateKey; ECDH.prototype.generateKeys = function generateKeys(encoding, format) { - this._handle.generateKeys(); + this[kHandle].generateKeys(); return this.getPublicKey(encoding, format); }; ECDH.prototype.getPublicKey = function getPublicKey(encoding, format) { const f = getFormat(format); - const key = this._handle.getPublicKey(f); + const key = this[kHandle].getPublicKey(f); encoding = encoding || getDefaultEncoding(); return encode(key, encoding); }; diff --git a/lib/internal/crypto/hash.js b/lib/internal/crypto/hash.js index 7127d4d7efb17a..6522a19509f353 100644 --- a/lib/internal/crypto/hash.js +++ b/lib/internal/crypto/hash.js @@ -7,6 +7,8 @@ const { const { getDefaultEncoding, + kHandle, + legacyNativeHandle, toBuf } = require('internal/crypto/util'); @@ -30,7 +32,7 @@ function Hash(algorithm, options) { if (!(this instanceof Hash)) return new Hash(algorithm, options); validateString(algorithm, 'algorithm'); - this._handle = new _Hash(algorithm); + legacyNativeHandle(this, new _Hash(algorithm)); this[kState] = { [kFinalized]: false }; @@ -40,12 +42,12 @@ function Hash(algorithm, options) { inherits(Hash, LazyTransform); Hash.prototype._transform = function _transform(chunk, encoding, callback) { - this._handle.update(chunk, encoding); + this[kHandle].update(chunk, encoding); callback(); }; Hash.prototype._flush = function _flush(callback) { - this.push(this._handle.digest()); + this.push(this[kHandle].digest()); callback(); }; @@ -59,7 +61,7 @@ Hash.prototype.update = function update(data, encoding) { ['string', 'TypedArray', 'DataView'], data); } - if (!this._handle.update(data, encoding || getDefaultEncoding())) + if (!this[kHandle].update(data, encoding || getDefaultEncoding())) throw new ERR_CRYPTO_HASH_UPDATE_FAILED(); return this; }; @@ -74,7 +76,7 @@ Hash.prototype.digest = function digest(outputEncoding) { throw new ERR_CRYPTO_HASH_DIGEST_NO_UTF16(); // Explicit conversion for backward compatibility. - const ret = this._handle.digest(`${outputEncoding}`); + const ret = this[kHandle].digest(`${outputEncoding}`); state[kFinalized] = true; return ret; }; @@ -88,8 +90,8 @@ function Hmac(hmac, key, options) { throw new ERR_INVALID_ARG_TYPE('key', ['string', 'TypedArray', 'DataView'], key); } - this._handle = new _Hmac(); - this._handle.init(hmac, toBuf(key)); + legacyNativeHandle(this, new _Hmac()); + this[kHandle].init(hmac, toBuf(key)); this[kState] = { [kFinalized]: false }; @@ -112,7 +114,7 @@ Hmac.prototype.digest = function digest(outputEncoding) { } // Explicit conversion for backward compatibility. - const ret = this._handle.digest(`${outputEncoding}`); + const ret = this[kHandle].digest(`${outputEncoding}`); state[kFinalized] = true; return ret; }; diff --git a/lib/internal/crypto/sig.js b/lib/internal/crypto/sig.js index fd2db9016f4b5d..5a1787d00f3639 100644 --- a/lib/internal/crypto/sig.js +++ b/lib/internal/crypto/sig.js @@ -13,6 +13,8 @@ const { } = process.binding('constants').crypto; const { getDefaultEncoding, + kHandle, + legacyNativeHandle, toBuf, validateArrayBufferView, } = require('internal/crypto/util'); @@ -23,8 +25,8 @@ function Sign(algorithm, options) { if (!(this instanceof Sign)) return new Sign(algorithm, options); validateString(algorithm, 'algorithm'); - this._handle = new _Sign(); - this._handle.init(algorithm); + legacyNativeHandle(this, new _Sign()); + this[kHandle].init(algorithm); Writable.call(this, options); } @@ -40,7 +42,7 @@ Sign.prototype.update = function update(data, encoding) { encoding = encoding || getDefaultEncoding(); data = validateArrayBufferView(toBuf(data, encoding), 'data'); - this._handle.update(data); + this[kHandle].update(data); return this; }; @@ -78,7 +80,7 @@ Sign.prototype.sign = function sign(options, encoding) { key = validateArrayBufferView(key, 'key'); - var ret = this._handle.sign(key, passphrase, rsaPadding, pssSaltLength); + var ret = this[kHandle].sign(key, passphrase, rsaPadding, pssSaltLength); encoding = encoding || getDefaultEncoding(); if (encoding && encoding !== 'buffer') @@ -92,8 +94,8 @@ function Verify(algorithm, options) { if (!(this instanceof Verify)) return new Verify(algorithm, options); validateString(algorithm, 'algorithm'); - this._handle = new _Verify(); - this._handle.init(algorithm); + legacyNativeHandle(this, new _Verify()); + this[kHandle].init(algorithm); Writable.call(this, options); } @@ -117,7 +119,7 @@ Verify.prototype.verify = function verify(options, signature, sigEncoding) { signature = validateArrayBufferView(toBuf(signature, sigEncoding), 'signature'); - return this._handle.verify(key, signature, rsaPadding, pssSaltLength); + return this[kHandle].verify(key, signature, rsaPadding, pssSaltLength); }; module.exports = { diff --git a/lib/internal/crypto/util.js b/lib/internal/crypto/util.js index 19d18be5b003c8..7ff25be186ba4a 100644 --- a/lib/internal/crypto/util.js +++ b/lib/internal/crypto/util.js @@ -22,12 +22,28 @@ const { validateString } = require('internal/validators'); const { Buffer } = require('buffer'); const { cachedResult, + deprecate, filterDuplicateStrings } = require('internal/util'); const { isArrayBufferView } = require('internal/util/types'); +const kHandle = Symbol('kHandle'); + +function legacyNativeHandle(obj, handle) { + obj[kHandle] = handle; + Object.defineProperty(obj, '_handle', { + get: deprecate(() => handle, + `${obj.constructor.name}._handle is deprecated. Use the ` + + 'public API instead.', 'DEP00XX'), + set: deprecate((h) => obj[kHandle] = handle = h, + `${obj.constructor.name}._handle is deprecated. Use the ` + + 'public API instead.', 'DEP00XX'), + enumerable: false + }); +} + var defaultEncoding = 'buffer'; function setDefaultEncoding(val) { @@ -101,6 +117,8 @@ module.exports = { getCurves, getDefaultEncoding, getHashes, + kHandle, + legacyNativeHandle, setDefaultEncoding, setEngine, timingSafeEqual, From 2e3353b0a091d52163fce96c75806245f52b896b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= Date: Wed, 12 Sep 2018 21:45:29 +0200 Subject: [PATCH 2/2] Add metadata for the added deprecation --- doc/api/deprecations.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/doc/api/deprecations.md b/doc/api/deprecations.md index 44da1d0ee6d313..ad07428ece4118 100644 --- a/doc/api/deprecations.md +++ b/doc/api/deprecations.md @@ -1060,6 +1060,12 @@ use the [WHATWG URL API][] instead. ### DEP00XX: Native crypto handles + Type: Runtime