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

separate out curves #2

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

mvayngrib
Copy link

No description provided.

@shea256
Copy link
Contributor

shea256 commented Sep 12, 2016

Thanks for the PR @mvayngrib! Love how you added a bunch of new curves.

I can merge this in soon but it'd be great if you could add some tests in with at least one of these curves before I do. Would you be up for that?

@mvayngrib
Copy link
Author

@shea256 do you remember where you got the OpenSSL reference values?

@shea256
Copy link
Contributor

shea256 commented Sep 12, 2016

I don't remember.

Here's one list thought: https://github.com/warner/python-ecdsa/blob/master/ecdsa/curves.py#L43

@shea256
Copy link
Contributor

shea256 commented Sep 12, 2016

I think I got it from an RFC but I'm not sure which one at the moment unfortunately.

@shea256
Copy link
Contributor

shea256 commented Sep 13, 2016

@mvayngrib Did you see the other curves in there?

@mvayngrib
Copy link
Author

mvayngrib commented Sep 15, 2016

@shea256 no, sadly i didn't. For now i'm just testing it by cross sign-verify native<=>elliptic, which passes (the code is non-deterministic, but since we're planning to use openssl references anyway, who cares).

also: i also added lazy-eval for creating instances of elliptic curves. They're very expensive to create so it's better not to pre-create them.

test('elliptic sign => native verify compat', function (t) {
    var data = 'some data'
    var algorithm = 'sha256'
    var hash = crypto.createHash(algorithm).update(data).digest()
    for (var name in aliases) {
        var curve = curves[name].curve
        var encoder = new KeyEncoder(name)
        var key = curve.genKeyPair()
        var sig = key.sign(hash).toDER('hex')
        var pubHex = key.getPublic('hex')
        var pub = encoder.encodePublic(pubHex, 'raw', 'pem')
        var verified = crypto.createVerify(algorithm).update(data).verify(pub, sig, 'hex')
        t.ok(verified)
    }

    t.end()
})

test('native sign => elliptic verify compat', function (t) {
    var data = 'some data'
    var algorithm = 'sha256'
    var hash = crypto.createHash(algorithm).update(data).digest()
    for (var name in aliases) {
        var curve = curves[name].curve
        var encoder = new KeyEncoder(name)
        var ecdh = crypto.createECDH(aliases[name])
        ecdh.generateKeys()
        var priv = ecdh.getPrivateKey()
        var pem = encoder.encodePrivate(priv, 'raw', 'pem')
        var sig = crypto.createSign(algorithm).update(data).sign(pem, 'hex')
        var verified = curve.keyFromPrivate(priv).verify(hash, sig)
        t.ok(verified)
    }

    t.end()
})

@shea256
Copy link
Contributor

shea256 commented Sep 16, 2016

@mvayngrib Thanks, great work!

I just incorporated your 2 commits and your tests in this latest PR:
#3

Please let me know what you think.

@CLAassistant
Copy link

CLAassistant commented Mar 8, 2020

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants