Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

crypto: hide native handles from JS modules #22747

Closed
wants to merge 2 commits into from
Closed

crypto: hide native handles from JS modules #22747

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4427dc2
crypto: hide native handles from JS modules
tniessen Sep 5, 2018
2e3353b
Add metadata for the added deprecation
tniessen Sep 12, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 10 additions & 0 deletions doc/api/deprecations.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
<a id="DEP00XX"></a>
### 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
Expand Down
24 changes: 13 additions & 11 deletions lib/internal/crypto/cipher.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ const { validateString } = require('internal/validators');

const {
getDefaultEncoding,
kHandle,
legacyNativeHandle,
toBuf
} = require('internal/crypto/util');

Expand Down Expand Up @@ -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) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the purpose of this is to hide the actual implementation details, I would prefer to use a non-enumerable symbol. Otherwise it will still show up in the repl and when inspecting the value (including console calls).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had the idea to do this after reviewing #22684. If we want to use non-enumerable symbols, maybe we should add that to the guidelines?

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;

Expand Down Expand Up @@ -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;
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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;
Expand All @@ -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;
}
Expand All @@ -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;
};
Expand Down
32 changes: 17 additions & 15 deletions lib/internal/crypto/diffiehellman.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
});
}
Expand All @@ -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
});
}
Expand All @@ -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);
}
Expand All @@ -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);
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -144,22 +146,22 @@ DiffieHellmanGroup.prototype.getPrivateKey =
dhGetPrivateKey;

function dhGetPrivateKey(encoding) {
const key = this._handle.getPrivateKey();
const key = this[kHandle].getPrivateKey();
encoding = encoding || getDefaultEncoding();
return encode(key, 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;
};

Expand All @@ -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;
Expand All @@ -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);
};
Expand Down
18 changes: 10 additions & 8 deletions lib/internal/crypto/hash.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ const {

const {
getDefaultEncoding,
kHandle,
legacyNativeHandle,
toBuf
} = require('internal/crypto/util');

Expand All @@ -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
};
Expand All @@ -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();
};

Expand All @@ -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;
};
Expand All @@ -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;
};
Expand All @@ -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
};
Expand All @@ -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;
};
Expand Down
16 changes: 9 additions & 7 deletions lib/internal/crypto/sig.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ const {
} = process.binding('constants').crypto;
const {
getDefaultEncoding,
kHandle,
legacyNativeHandle,
toBuf,
validateArrayBufferView,
} = require('internal/crypto/util');
Expand All @@ -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);
}
Expand All @@ -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;
};

Expand Down Expand Up @@ -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')
Expand All @@ -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);
}
Expand All @@ -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 = {
Expand Down