Skip to content

crypto: add generatePublicKey to ECDH, fix corner cases #1020

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

Closed
wants to merge 2 commits into from
Closed

crypto: add generatePublicKey to ECDH, fix corner cases #1020

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6c03548
crypto: add generatePublicKey to ECDH, fix corner cases
mbullington Mar 1, 2015
076917d
crypto: change function name
mbullington Mar 3, 2015
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
12 changes: 12 additions & 0 deletions doc/api/crypto.markdown
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -550,6 +550,18 @@ Format specifies point encoding and can be `'compressed'`, `'uncompressed'`, or
Encoding can be `'binary'`, `'hex'`, or `'base64'`. If no encoding is provided,
then a buffer is returned.

### ECDH.generatePublicKey([encoding[, format]])

Generates public EC Diffie-Hellman key value based on the provided private key
value. Returns the public key in the specified format and encoding.

Format specifies point encoding and can be `'compressed'`, `'uncompressed'`, or
`'hybrid'`. If no format is provided - the point will be returned in
`'uncompressed'` format.

Encoding can be `'binary'`, `'hex'`, or `'base64'`. If no encoding is provided,
then a buffer is returned.

### ECDH.computeSecret(other_public_key[, input_encoding][, output_encoding])

Computes the shared secret using `other_public_key` as the other
Expand Down
6 changes: 6 additions & 0 deletions lib/crypto.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -535,6 +535,12 @@ ECDH.prototype.generateKeys = function generateKeys(encoding, format) {
return this.getPublicKey(encoding, format);
};

ECDH.prototype.generatePublicKey = function generatePublicKey(encoding, format) {
this._handle.generatePublicKey();

return this.getPublicKey(encoding, format);
};

ECDH.prototype.getPublicKey = function getPublicKey(encoding, format) {
var f;
if (format) {
Expand Down
38 changes: 27 additions & 11 deletions src/node_crypto.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4025,6 +4025,7 @@ void ECDH::Initialize(Environment* env, Handle<Object> target) {
t->InstanceTemplate()->SetInternalFieldCount(1);

env->SetProtoMethod(t, "generateKeys", GenerateKeys);
env->SetProtoMethod(t, "generatePublicKey", GeneratePublicKey);
env->SetProtoMethod(t, "computeSecret", ComputeSecret);
env->SetProtoMethod(t, "getPublicKey", GetPublicKey);
env->SetProtoMethod(t, "getPrivateKey", GetPrivateKey);
Expand Down Expand Up @@ -4062,10 +4063,33 @@ void ECDH::GenerateKeys(const FunctionCallbackInfo<Value>& args) {

if (!EC_KEY_generate_key(ecdh->key_))
return env->ThrowError("Failed to generate EC_KEY");

ecdh->generated_ = true;
}
Copy link
Member

Choose a reason for hiding this comment

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

Why?

Copy link
Member

Choose a reason for hiding this comment

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

I mean, perhaps it should be set from GeneratePublicKey too? Or be a flag field with two possible flags: GeneratedPrivate and GeneratedPublic?

Copy link
Author

Choose a reason for hiding this comment

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

If I have something like the following code sample, getPrivateKey() will return an error if you haven't generated the keys, which is a pain for imported keys. I'd have to generate a keypair, then overwrite the keys (wasting a keypair just to toggle the bool).

var ecdh = crypto.createECDH(curve);
ecdh.setPrivateKey(privateKey);
ecdh.setPublicKey(publicKey);

ecdh.getPrivateKey(); // error

I could also import only the private key then use generatePublicKey to toggle the bool, but the getPrivateKey/getPublicKey functionality with the bool initially can be unexpected behavior.

Perhaps computeSecret could just check for the private key and throw an error based on that?

Copy link
Member

Choose a reason for hiding this comment

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

Ok, if setPrivateKey will set both Private and Public flag - will that work?


void ECDH::GeneratePublicKey(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);

ECDH* ecdh = Unwrap<ECDH>(args.Holder());

const BIGNUM* priv = EC_KEY_get0_private_key(ecdh->key_);
if (priv == nullptr)
return env->ThrowError("Failed to get ECDH private key");

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

if(EC_POINT_mul(ecdh->group_, pub, priv, nullptr, nullptr, nullptr) <= 0) {
Copy link
Member

Choose a reason for hiding this comment

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

Style: space after 'if' (here and below.)

EC_POINT_free(pub);
return env->ThrowError("Failed to generate ECDH public key");
}

if(!EC_KEY_set_public_key(ecdh->key_, pub)) {
EC_POINT_free(pub);
return env->ThrowError("Failed to convert EC_POINT to a public key");
}

EC_POINT_free(pub);
}

EC_POINT* ECDH::BufferToPoint(char* data, size_t len) {
EC_POINT* pub;
Expand Down Expand Up @@ -4095,7 +4119,6 @@ EC_POINT* ECDH::BufferToPoint(char* data, size_t len) {
return nullptr;
}


void ECDH::ComputeSecret(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);

Expand Down Expand Up @@ -4124,7 +4147,6 @@ void ECDH::ComputeSecret(const FunctionCallbackInfo<Value>& args) {
args.GetReturnValue().Set(Buffer::Use(env, out, out_len));
}


void ECDH::GetPublicKey(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);

Expand All @@ -4133,9 +4155,6 @@ void ECDH::GetPublicKey(const FunctionCallbackInfo<Value>& args) {

ECDH* ecdh = Unwrap<ECDH>(args.Holder());

if (!ecdh->generated_)
return env->ThrowError("You should generate ECDH keys first");

const EC_POINT* pub = EC_KEY_get0_public_key(ecdh->key_);
if (pub == nullptr)
return env->ThrowError("Failed to get ECDH public key");
Expand Down Expand Up @@ -4168,9 +4187,6 @@ void ECDH::GetPrivateKey(const FunctionCallbackInfo<Value>& args) {

ECDH* ecdh = Unwrap<ECDH>(args.Holder());

if (!ecdh->generated_)
return env->ThrowError("You should generate ECDH keys first");

const BIGNUM* b = EC_KEY_get0_private_key(ecdh->key_);
if (b == nullptr)
return env->ThrowError("Failed to get ECDH private key");
Expand Down Expand Up @@ -4224,7 +4240,7 @@ void ECDH::SetPublicKey(const FunctionCallbackInfo<Value>& args) {
int r = EC_KEY_set_public_key(ecdh->key_, pub);
EC_POINT_free(pub);
if (!r)
return env->ThrowError("Failed to convert BN to a private key");
return env->ThrowError("Failed to convert EC_POINT to a public key");
}


Expand Down
3 changes: 1 addition & 2 deletions src/node_crypto.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -635,7 +635,6 @@ class ECDH : public BaseObject {
protected:
ECDH(Environment* env, v8::Local<v8::Object> wrap, EC_KEY* key)
: BaseObject(env, wrap),
generated_(false),
key_(key),
group_(EC_KEY_get0_group(key_)) {
MakeWeak<ECDH>(this);
Expand All @@ -644,6 +643,7 @@ class ECDH : public BaseObject {

static void New(const v8::FunctionCallbackInfo<v8::Value>& args);
static void GenerateKeys(const v8::FunctionCallbackInfo<v8::Value>& args);
static void GeneratePublicKey(const v8::FunctionCallbackInfo<v8::Value>& args);
static void ComputeSecret(const v8::FunctionCallbackInfo<v8::Value>& args);
static void GetPrivateKey(const v8::FunctionCallbackInfo<v8::Value>& args);
static void SetPrivateKey(const v8::FunctionCallbackInfo<v8::Value>& args);
Expand All @@ -652,7 +652,6 @@ class ECDH : public BaseObject {

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

bool generated_;
EC_KEY* key_;
const EC_GROUP* group_;
};
Expand Down
6 changes: 6 additions & 0 deletions test/parallel/test-crypto-dh.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,3 +178,9 @@ ecdh4.setPublicKey(ecdh1.getPublicKey());
assert.throws(function() {
ecdh4.setPublicKey(ecdh3.getPublicKey());
});

// ECDH should be able to generate public key from private key
var ecdh5 = crypto.createECDH('prime256v1');
ecdh5.setPrivateKey(ecdh4.getPrivateKey());

assert(ecdh5.generatePublicKey('hex') === ecdh5.getPublicKey('hex'));
Copy link
Member

Choose a reason for hiding this comment

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

Can you use assert.equal here?