Skip to content

Commit

Permalink
ec: validate that a point before deriving keys
Browse files Browse the repository at this point in the history
This update checks to make sure that the public key passed in to
ECDH is a point that actually exists on the curve. This is
important to prevent a twist attack that can be used to reveal
the private key of a party in an ECDH operation over a number of
occurances.

For more details on the attack see this blog post:
https://github.com/christianlundkvist/blog/blob/master/2020_05_26_secp256k1_twist_attacks/secp256k1_twist_attacks.md

CVE: CVE-2020-28498
  • Loading branch information
kdenhartog authored and indutny committed Feb 2, 2021
1 parent e71b2d9 commit 441b742
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 0 deletions.
3 changes: 3 additions & 0 deletions lib/elliptic/ec/key.js
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,9 @@ KeyPair.prototype._importPublic = function _importPublic(key, enc) {

// ECDH
KeyPair.prototype.derive = function derive(pub) {
if(!pub.validate()) {

This comment has been minimized.

Copy link
@thejohnfreeman

thejohnfreeman Feb 22, 2021

Why wrap in an if statement? Why not just assert?

This comment has been minimized.

Copy link
@kdenhartog

kdenhartog Jun 16, 2022

Author Contributor

Seems like a good optimization to me

This comment has been minimized.

Copy link
@coolaj86

coolaj86 Jul 13, 2022

Even faster if skipping assert and just doing throw new Error('public point not validated'). 🚀

assert(pub.validate(), 'public point not validated');
}
return pub.mul(this.priv).getX();
};

Expand Down
14 changes: 14 additions & 0 deletions test/ecdh-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,17 @@ describe('ECDH', function() {
test('ed25519');
test('secp256k1');
});

describe('ECDH twist attack', () => {
it('should be able to prevent a twist attack for secp256k1', () => {
var bobEcdh = new elliptic.ec('secp256k1');
var malloryEcdh = new elliptic.ec('secp256k1');
var bob = bobEcdh.genKeyPair();
// This is a bad point that shouldn't be able to be passed to derive.
// If a bad point can be passed it's possible to perform a twist attack.
var mallory = malloryEcdh.keyFromPublic({ x: 14, y: 16 });
assert.throws(function () {
bob.derive(mallory.getPublic());
});
});
});

1 comment on commit 441b742

@Trealent777
Copy link

Choose a reason for hiding this comment

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

test/ecdh-test.js

Please sign in to comment.