crypto: better docs for cases where peer's public key is invalid
changes in c++ are in the computeSecret function, but the thrown exception that was moved to JS land was in BufferToPoint function, here i let the allocation error be thrown so the only value returned is the nullptr that i use later to catch the error in computeSecret, to then construct the exception in JS land. an ERR_CRYPTO_ECDH_INVALID_PUBLIC_KEY error was added to errors.js and with that, subsequent changes to docs and tests were made. PR-URL: https://github.com/nodejs/node/pull/16849 Refs: https://www.iacr.org/archive/pkc2003/25670211/25670211.pdf Fixes: https://github.com/nodejs/node/issues/16625 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
This commit is contained in:
parent
31e0dbc0c7
commit
845633a7c6
@ -651,7 +651,11 @@ added: v0.11.14
|
||||
changes:
|
||||
- version: v6.0.0
|
||||
pr-url: https://github.com/nodejs/node/pull/5522
|
||||
description: The default `inputEncoding` changed from `binary` to `utf8`.
|
||||
description: The default `inputEncoding` changed from `binary` to `utf8`
|
||||
- version: REPLACEME
|
||||
pr-url: https://github.com/nodejs/node/pull/16849
|
||||
description: Changed error format to better support invalid public key
|
||||
error
|
||||
-->
|
||||
- `otherPublicKey` {string | Buffer | TypedArray | DataView}
|
||||
- `inputEncoding` {string}
|
||||
@ -668,6 +672,12 @@ provided, `otherPublicKey` is expected to be a [`Buffer`][], `TypedArray`, or
|
||||
If `outputEncoding` is given a string will be returned; otherwise a
|
||||
[`Buffer`][] is returned.
|
||||
|
||||
`ecdh.computeSecret` will throw an
|
||||
`ERR_CRYPTO_ECDH_INVALID_PUBLIC_KEY` error when `otherPublicKey`
|
||||
lies outside of the elliptic curve. Since `otherPublicKey` is
|
||||
usually supplied from a remote user over an insecure network,
|
||||
its recommended for developers to handle this exception accordingly.
|
||||
|
||||
### ecdh.generateKeys([encoding[, format]])
|
||||
<!-- YAML
|
||||
added: v0.11.14
|
||||
|
@ -676,6 +676,13 @@ of OpenSSL being used.
|
||||
An invalid value for the `format` argument was passed to the `crypto.ECDH()`
|
||||
class `getPublicKey()` method.
|
||||
|
||||
<a id="ERR_CRYPTO_ECDH_INVALID_PUBLIC_KEY"></a>
|
||||
### ERR_CRYPTO_ECDH_INVALID_PUBLIC_KEY
|
||||
|
||||
An invalid value for the `key` argument has been passed to the
|
||||
`crypto.ECDH()` class `computeSecret()` method. It means that the public
|
||||
key lies outside of the elliptic curve.
|
||||
|
||||
<a id="ERR_CRYPTO_ENGINE_UNKNOWN"></a>
|
||||
### ERR_CRYPTO_ENGINE_UNKNOWN
|
||||
|
||||
|
@ -96,6 +96,8 @@ function dhComputeSecret(key, inEnc, outEnc) {
|
||||
inEnc = inEnc || encoding;
|
||||
outEnc = outEnc || encoding;
|
||||
var ret = this._handle.computeSecret(toBuf(key, inEnc));
|
||||
if (typeof ret === 'string')
|
||||
throw new errors.Error('ERR_CRYPTO_ECDH_INVALID_PUBLIC_KEY');
|
||||
if (outEnc && outEnc !== 'buffer')
|
||||
ret = ret.toString(outEnc);
|
||||
return ret;
|
||||
|
@ -281,6 +281,8 @@ E('ERR_CPU_USAGE', 'Unable to obtain cpu usage %s');
|
||||
E('ERR_CRYPTO_CUSTOM_ENGINE_NOT_SUPPORTED',
|
||||
'Custom engines not supported by this OpenSSL');
|
||||
E('ERR_CRYPTO_ECDH_INVALID_FORMAT', 'Invalid ECDH format: %s');
|
||||
E('ERR_CRYPTO_ECDH_INVALID_PUBLIC_KEY',
|
||||
'Public key is not valid for specified curve');
|
||||
E('ERR_CRYPTO_ENGINE_UNKNOWN', 'Engine "%s" was not found');
|
||||
E('ERR_CRYPTO_FIPS_FORCED',
|
||||
'Cannot set FIPS mode, it was forced with --force-fips at startup.');
|
||||
|
@ -5220,7 +5220,6 @@ EC_POINT* ECDH::BufferToPoint(char* data, size_t len) {
|
||||
len,
|
||||
nullptr);
|
||||
if (!r) {
|
||||
env()->ThrowError("Failed to translate Buffer to a EC_POINT");
|
||||
goto fatal;
|
||||
}
|
||||
|
||||
@ -5247,8 +5246,12 @@ void ECDH::ComputeSecret(const FunctionCallbackInfo<Value>& args) {
|
||||
|
||||
EC_POINT* pub = ecdh->BufferToPoint(Buffer::Data(args[0]),
|
||||
Buffer::Length(args[0]));
|
||||
if (pub == nullptr)
|
||||
if (pub == nullptr) {
|
||||
args.GetReturnValue().Set(
|
||||
FIXED_ONE_BYTE_STRING(env->isolate(),
|
||||
"ERR_CRYPTO_ECDH_INVALID_PUBLIC_KEY"));
|
||||
return;
|
||||
}
|
||||
|
||||
// NOTE: field_size is in bits
|
||||
int field_size = EC_GROUP_get_degree(ecdh->group_);
|
||||
|
@ -222,9 +222,13 @@ if (availableCurves.has('prime256v1') && availableCurves.has('secp256k1')) {
|
||||
const ecdh3 = crypto.createECDH('secp256k1');
|
||||
const key3 = ecdh3.generateKeys();
|
||||
|
||||
assert.throws(() => {
|
||||
ecdh2.computeSecret(key3, 'latin1', 'buffer');
|
||||
}, /^Error: Failed to translate Buffer to a EC_POINT$/);
|
||||
common.expectsError(
|
||||
() => ecdh2.computeSecret(key3, 'latin1', 'buffer'),
|
||||
{
|
||||
code: 'ERR_CRYPTO_ECDH_INVALID_PUBLIC_KEY',
|
||||
type: Error,
|
||||
message: 'Public key is not valid for specified curve'
|
||||
});
|
||||
|
||||
// ECDH should allow .setPrivateKey()/.setPublicKey()
|
||||
const ecdh4 = crypto.createECDH('prime256v1');
|
||||
@ -331,9 +335,13 @@ if (availableCurves.has('prime256v1') && availableHashes.has('sha256')) {
|
||||
const invalidKey = Buffer.alloc(65);
|
||||
invalidKey.fill('\0');
|
||||
curve.generateKeys();
|
||||
assert.throws(() => {
|
||||
curve.computeSecret(invalidKey);
|
||||
}, /^Error: Failed to translate Buffer to a EC_POINT$/);
|
||||
common.expectsError(
|
||||
() => curve.computeSecret(invalidKey),
|
||||
{
|
||||
code: 'ERR_CRYPTO_ECDH_INVALID_PUBLIC_KEY',
|
||||
type: Error,
|
||||
message: 'Public key is not valid for specified curve'
|
||||
});
|
||||
// Check that signing operations are not impacted by the above error.
|
||||
const ecPrivateKey =
|
||||
'-----BEGIN EC PRIVATE KEY-----\n' +
|
||||
|
Loading…
x
Reference in New Issue
Block a user