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.

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

crypto: ECDH convertKey to convert public keys between different formats #19080

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions doc/api/crypto.md
Original file line number Diff line number Diff line change
Expand Up @@ -656,6 +656,54 @@ assert.strictEqual(aliceSecret.toString('hex'), bobSecret.toString('hex'));
// OK
```

### ECDH.convertKey(key, curve[, inputEncoding[, outputEncoding[, format]]])
<!-- YAML
added: REPLACEME
-->

- `key` {string | Buffer | TypedArray | DataView}
- `curve` {string}
- `inputEncoding` {string}
- `outputEncoding` {string}
- `format` {string} Defaults to `uncompressed`.

Converts the EC Diffie-Hellman public key specified by `key` and `curve` to the
format specified by `format`. The `format` argument specifies point encoding
and can be `'compressed'`, `'uncompressed'` or `'hybrid'`. The supplied key is
interpreted using the specified `inputEncoding`, and the returned key is encoded
using the specified `outputEncoding`. Encodings can be `'latin1'`, `'hex'`,
or `'base64'`.

Use [`crypto.getCurves()`][] to obtain a list of available curve names.
On recent OpenSSL releases, `openssl ecparam -list_curves` will also display
the name and description of each available elliptic curve.

If `format` is not specified the point will be returned in `'uncompressed'`
format.

If the `inputEncoding` is not provided, `key` is expected to be a [`Buffer`][],
`TypedArray`, or `DataView`.

Example (uncompressing a key):

```js
const { ECDH } = require('crypto');

const ecdh = ECDH('secp256k1');
ecdh.generateKeys();

const compressedKey = ecdh.getPublicKey('hex', 'compressed');

const uncompressedKey = ECDH.convertKey(compressedKey,
'secp256k1',
'hex',
'hex',
'uncompressed');

// the converted key and the uncompressed public key should be the same
console.log(uncompressedKey === ecdh.getPublicKey('hex'));
```

### ecdh.computeSecret(otherPublicKey[, inputEncoding][, outputEncoding])
<!-- YAML
added: v0.11.14
Expand Down
82 changes: 50 additions & 32 deletions lib/internal/crypto/diffiehellman.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ const {
const {
DiffieHellman: _DiffieHellman,
DiffieHellmanGroup: _DiffieHellmanGroup,
ECDH: _ECDH
ECDH: _ECDH,
ECDHConvertKey: _ECDHConvertKey
} = process.binding('crypto');
const {
POINT_CONVERSION_COMPRESSED,
Expand Down Expand Up @@ -84,11 +85,9 @@ DiffieHellmanGroup.prototype.generateKeys =
dhGenerateKeys;

function dhGenerateKeys(encoding) {
var keys = this._handle.generateKeys();
const keys = this._handle.generateKeys();
encoding = encoding || getDefaultEncoding();
if (encoding && encoding !== 'buffer')
keys = keys.toString(encoding);
return keys;
return encode(keys, encoding);
}


Expand All @@ -100,12 +99,10 @@ function dhComputeSecret(key, inEnc, outEnc) {
const encoding = getDefaultEncoding();
inEnc = inEnc || encoding;
outEnc = outEnc || encoding;
var ret = this._handle.computeSecret(toBuf(key, inEnc));
const ret = this._handle.computeSecret(toBuf(key, inEnc));
if (typeof ret === 'string')
throw new ERR_CRYPTO_ECDH_INVALID_PUBLIC_KEY();
if (outEnc && outEnc !== 'buffer')
ret = ret.toString(outEnc);
return ret;
return encode(ret, outEnc);
}


Expand All @@ -114,11 +111,9 @@ DiffieHellmanGroup.prototype.getPrime =
dhGetPrime;

function dhGetPrime(encoding) {
var prime = this._handle.getPrime();
const prime = this._handle.getPrime();
encoding = encoding || getDefaultEncoding();
if (encoding && encoding !== 'buffer')
prime = prime.toString(encoding);
return prime;
return encode(prime, encoding);
}


Expand All @@ -127,11 +122,9 @@ DiffieHellmanGroup.prototype.getGenerator =
dhGetGenerator;

function dhGetGenerator(encoding) {
var generator = this._handle.getGenerator();
const generator = this._handle.getGenerator();
encoding = encoding || getDefaultEncoding();
if (encoding && encoding !== 'buffer')
generator = generator.toString(encoding);
return generator;
return encode(generator, encoding);
}


Expand All @@ -140,11 +133,9 @@ DiffieHellmanGroup.prototype.getPublicKey =
dhGetPublicKey;

function dhGetPublicKey(encoding) {
var key = this._handle.getPublicKey();
const key = this._handle.getPublicKey();
encoding = encoding || getDefaultEncoding();
if (encoding && encoding !== 'buffer')
key = key.toString(encoding);
return key;
return encode(key, encoding);
}


Expand All @@ -153,11 +144,9 @@ DiffieHellmanGroup.prototype.getPrivateKey =
dhGetPrivateKey;

function dhGetPrivateKey(encoding) {
var key = this._handle.getPrivateKey();
const key = this._handle.getPrivateKey();
encoding = encoding || getDefaultEncoding();
if (encoding && encoding !== 'buffer')
key = key.toString(encoding);
return key;
return encode(key, encoding);
}


Expand Down Expand Up @@ -197,7 +186,40 @@ ECDH.prototype.generateKeys = function generateKeys(encoding, format) {
};

ECDH.prototype.getPublicKey = function getPublicKey(encoding, format) {
var f;
const f = getFormat(format);
const key = this._handle.getPublicKey(f);
encoding = encoding || getDefaultEncoding();
return encode(key, encoding);
};

ECDH.convertKey = function convertKey(key, curve, inEnc, outEnc, format) {
if (typeof key !== 'string' && !isArrayBufferView(key)) {
throw new ERR_INVALID_ARG_TYPE(
'key',
['string', 'Buffer', 'TypedArray', 'DataView']
);
}

if (typeof curve !== 'string') {
throw new ERR_INVALID_ARG_TYPE('curve', 'string');
}

const encoding = getDefaultEncoding();
inEnc = inEnc || encoding;
outEnc = outEnc || encoding;
const f = getFormat(format);
const convertedKey = _ECDHConvertKey(toBuf(key, inEnc), curve, f);
return encode(convertedKey, outEnc);
};

function encode(buffer, encoding) {
if (encoding && encoding !== 'buffer')
buffer = buffer.toString(encoding);
return buffer;
}

function getFormat(format) {
let f;
if (format) {
if (format === 'compressed')
f = POINT_CONVERSION_COMPRESSED;
Expand All @@ -211,12 +233,8 @@ ECDH.prototype.getPublicKey = function getPublicKey(encoding, format) {
} else {
f = POINT_CONVERSION_UNCOMPRESSED;
}
var key = this._handle.getPublicKey(f);
encoding = encoding || getDefaultEncoding();
if (encoding && encoding !== 'buffer')
key = key.toString(encoding);
return key;
};
return f;
}

module.exports = {
DiffieHellman,
Expand Down
83 changes: 72 additions & 11 deletions src/node_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4684,31 +4684,31 @@ void ECDH::GenerateKeys(const FunctionCallbackInfo<Value>& args) {
}


EC_POINT* ECDH::BufferToPoint(char* data, size_t len) {
EC_POINT* ECDH::BufferToPoint(Environment* env,
const EC_GROUP* group,
char* data,
size_t len) {
EC_POINT* pub;
int r;

pub = EC_POINT_new(group_);
pub = EC_POINT_new(group);
if (pub == nullptr) {
env()->ThrowError("Failed to allocate EC_POINT for a public key");
env->ThrowError("Failed to allocate EC_POINT for a public key");
return nullptr;
}

r = EC_POINT_oct2point(
group_,
group,
pub,
reinterpret_cast<unsigned char*>(data),
len,
Copy link
Member

Choose a reason for hiding this comment

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

Since you're making changes here, you might want to get rid of the goto by moving the fatal: block inside the if statement.

Copy link
Contributor Author

@wuweiweiwu wuweiweiwu Mar 4, 2018

Choose a reason for hiding this comment

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

Done!

nullptr);
if (!r) {
goto fatal;
EC_POINT_free(pub);
return nullptr;
}

return pub;

fatal:
EC_POINT_free(pub);
return nullptr;
}


Expand All @@ -4725,7 +4725,9 @@ void ECDH::ComputeSecret(const FunctionCallbackInfo<Value>& args) {
if (!ecdh->IsKeyPairValid())
return env->ThrowError("Invalid key pair");

EC_POINT* pub = ecdh->BufferToPoint(Buffer::Data(args[0]),
EC_POINT* pub = ECDH::BufferToPoint(env,
ecdh->group_,
Buffer::Data(args[0]),
Buffer::Length(args[0]));
if (pub == nullptr) {
args.GetReturnValue().Set(
Expand Down Expand Up @@ -4874,7 +4876,9 @@ void ECDH::SetPublicKey(const FunctionCallbackInfo<Value>& args) {

MarkPopErrorOnReturn mark_pop_error_on_return;

EC_POINT* pub = ecdh->BufferToPoint(Buffer::Data(args[0].As<Object>()),
EC_POINT* pub = ECDH::BufferToPoint(env,
ecdh->group_,
Buffer::Data(args[0].As<Object>()),
Buffer::Length(args[0].As<Object>()));
if (pub == nullptr)
return env->ThrowError("Failed to convert Buffer to EC_POINT");
Expand Down Expand Up @@ -5550,6 +5554,61 @@ void ExportChallenge(const FunctionCallbackInfo<Value>& args) {
args.GetReturnValue().Set(outString);
}


// Convert the input public key to compressed, uncompressed, or hybrid formats.
void ConvertKey(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);

CHECK_EQ(args.Length(), 3);

size_t len = Buffer::Length(args[0]);
if (len == 0)
return args.GetReturnValue().SetEmptyString();

node::Utf8Value curve(env->isolate(), args[1]);

int nid = OBJ_sn2nid(*curve);
if (nid == NID_undef)
return env->ThrowTypeError("Invalid ECDH curve name");

EC_GROUP* group = EC_GROUP_new_by_curve_name(nid);
if (group == nullptr)
return env->ThrowError("Failed to get EC_GROUP");

EC_POINT* pub = ECDH::BufferToPoint(env,
group,
Buffer::Data(args[0]),
len);

std::shared_ptr<void> cleanup(nullptr, [group, pub] (...) {
Copy link
Contributor Author

@wuweiweiwu wuweiweiwu Mar 4, 2018

Choose a reason for hiding this comment

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

@bnoordhuis I am not sure if I am using shared_ptr correctly. I created it before the pub === nullptr check so that returns. the delete function will be called

Copy link
Member

Choose a reason for hiding this comment

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

@wuweiweiwu we also have OnScopeLeave in util.h, you could use that if you prefer? It might be a bit more self-documenting?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@addaleax I will look at that! Is there a preference using onScopeLeave v. shared_ptr?

Copy link
Member

Choose a reason for hiding this comment

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

@wuweiweiwu The former is pretty new as an utility -- but basically, use whichever makes sense to you, or lets the code seem more readable. :)

(Also, probably doesn't matter much now that this has landed, unless you want to send another PR right behind this. ¯\_(ツ)_/¯)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good! I'll go over both so I am more familiar

EC_GROUP_free(group);
EC_POINT_free(pub);
});

if (pub == nullptr)
return env->ThrowError("Failed to convert Buffer to EC_POINT");

point_conversion_form_t form =
static_cast<point_conversion_form_t>(args[2]->Uint32Value());

int size = EC_POINT_point2oct(group, pub, form, nullptr, 0, nullptr);
if (size == 0)
return env->ThrowError("Failed to get public key length");

unsigned char* out = node::Malloc<unsigned char>(size);

int r = EC_POINT_point2oct(group, pub, form, out, size, nullptr);
if (r != size) {
free(out);
return env->ThrowError("Failed to get public key");
}

Local<Object> buf =
Buffer::New(env, reinterpret_cast<char*>(out), size).ToLocalChecked();
args.GetReturnValue().Set(buf);
}


void TimingSafeEqual(const FunctionCallbackInfo<Value>& args) {
CHECK(Buffer::HasInstance(args[0]));
CHECK(Buffer::HasInstance(args[1]));
Expand Down Expand Up @@ -5692,6 +5751,8 @@ void InitCrypto(Local<Object> target,
env->SetMethod(target, "certVerifySpkac", VerifySpkac);
env->SetMethod(target, "certExportPublicKey", ExportPublicKey);
env->SetMethod(target, "certExportChallenge", ExportChallenge);

env->SetMethod(target, "ECDHConvertKey", ConvertKey);
#ifndef OPENSSL_NO_ENGINE
env->SetMethod(target, "setEngine", SetEngine);
#endif // !OPENSSL_NO_ENGINE
Expand Down
6 changes: 4 additions & 2 deletions src/node_crypto.h
Original file line number Diff line number Diff line change
Expand Up @@ -625,6 +625,10 @@ class ECDH : public BaseObject {
}

static void Initialize(Environment* env, v8::Local<v8::Object> target);
static EC_POINT* BufferToPoint(Environment* env,
const EC_GROUP* group,
char* data,
size_t len);

protected:
ECDH(Environment* env, v8::Local<v8::Object> wrap, EC_KEY* key)
Expand All @@ -643,8 +647,6 @@ class ECDH : public BaseObject {
static void GetPublicKey(const v8::FunctionCallbackInfo<v8::Value>& args);
static void SetPublicKey(const v8::FunctionCallbackInfo<v8::Value>& args);

EC_POINT* BufferToPoint(char* data, size_t len);

bool IsKeyPairValid();
bool IsKeyValidForCurve(const BIGNUM* private_key);

Expand Down
Loading