Bogus issue - ignore

[indutny]

I assume that it works after all? ;) Thanks anyway!

[indutny]

I'm glad you are using elliptic!

[rongarret]

No, I’m still having a problem, I’m just not sure what the problem is. Elliptic seems to work, but signatures it produces to not verify against another implementation, and signatures produced by the other implementation don’t verify against elliptic. I thought I had found the problem, but it turns out I was just confused about reduced vs non-reduced bignums. So I’m still trying to find the smoking gun, and/or generate a reproducible example of the problem.

On Oct 27, 2016, at 8:38 PM, Fedor Indutny notifications@github.com wrote:

I assume that it works after all? ;) Thanks anyway!

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub, or mute the thread.

[indutny]

@rongarret alright, please let me know if you'll have a test case that I could use for reproducing it.

[rongarret]

Will do.

On Oct 27, 2016, at 10:32 PM, Fedor Indutny notifications@github.com wrote:

@rongarret alright, please let me know if you'll have a test case that I could use for reproducing it.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[arlolra]

Maybe the other implementation is hashing the message before signing it.

[rongarret]

On Oct 27, 2016, at 10:59 PM, Arlo Breault notifications@github.com wrote:

Maybe the other implementation is hashing the message before signing it

No, that�s not the problem. I tried pre-hashing the message in both implementations and I�m still getting different results in all cases.

But the problem does seem to have something to do with a difference in how the message parameter is handled. I am getting the same results in both implementations for the public key and for sig_R (which means that scalar multiplication is working) but different results for sig_S. But I�m signing empty messages, so that limits the number of things that could be going wrong :-)

rg

[fanatid]

@rongarret which curve you are using?

[rongarret]

P256

I think I’ve found the smoking gun. My implementation is able to reproduce the test vectors from:

https://tools.ietf.org/html/rfc6979

So the problem has to be in elliptic.js

Specifically, I can reproduce these two results:

With SHA-256, message = "sample": k = A6E3C57DD01ABE90086538398355DD4C3B17AA873382B0F24D6129493D8AAD60 r = EFD48B2AACB6A8FD1140DD9CD45E81D69D2C877B56AAF991C34D0EA84EAF3716 s = F7CB1C942D657C41D436C7A1B6E29F65F3E900DBB9AFF4064DC4AB2F843ACDA8

With SHA-256, message = "test": k = D16B6AE827F17175E040871A1C7EC3500192C4C92677336EC2537ACAEE0008E0 r = F1ABB023518351CD71D881567B1EA663ED3EFCF6C5132B354F28D3B0B7D38367 s = 019F4113742A2B14BD25926B49C649155F267E60D3814B4C0CC84250E46F0083

Using this key:

curve: NIST P-256

private key:

x = C9AFA9D845BA75166B5C215767B1D6934E50C3DB36E89B127B8A622B120F6721

On Oct 28, 2016, at 9:01 AM, Kirill Fomichev notifications@github.com wrote:

@rongarret which curve you are using?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[fanatid]

@rongarret I change elliptic for print k and run

const createHash = require('crypto').createHash
const elliptic = require('./')
const ec = new elliptic.ec('p256')

const msg = createHash('sha256').update('sample').digest()
const sign = ec.sign(msg, 'C9AFA9D845BA75166B5C215767B1D6934E50C3DB36E89B127B8A622B120F6721', 'hex')
console.log(`r: ${sign.r.toArrayLike(Buffer, 'be', 32).toString('hex')}`)
console.log(`s: ${sign.s.toArrayLike(Buffer, 'be', 32).toString('hex')}`)

k: a6e3c57dd01abe90086538398355dd4c3b17aa873382b0f24d6129493d8aad60 r: efd48b2aacb6a8fd1140dd9cd45e81d69d2c877b56aaf991c34d0ea84eaf3716 s: f7cb1c942d657c41d436c7a1b6e29f65f3e900dbb9aff4064dc4ab2f843acda8

and for test: k: d16b6ae827f17175e040871a1c7ec3500192c4c92677336ec2537acaee0008e0 r: f1abb023518351cd71d881567b1ea663ed3efcf6c5132b354f28d3b0b7d38367 s: 019f4113742a2b14bd25926b49c649155f267e60d3814b4c0cc84250e46f0083

k,r,s same as in test vectors

[indutny]

Appears to be working for me too:

'use strict';

const crypto = require('crypto');
const elliptic = require('./');

const hashed = crypto.createHash('sha256').update('sample').digest();

const curve = require('elliptic').ec('p256');

const key = Buffer.from('c9afa9d845ba75166b5c215767b1d6934e50c3db36e89b127b8a622b120f6721', 'hex');

console.log(curve.sign(hashed, key));

@rongarret are you hashing input before passing to it to .sign()?

[rongarret]

Ah. I was not hashing msg, just passing it in directly (per the example in the readme file). I could have sworn I tried hashing it and that didn’t work, but I must have been doing something else wrong and not noticed. I just now tried it again, and it is working. So thanks for getting me unwedged!

Is there a reason you don’t include the hash step as part of your signature code? It’s a required part of the ECDSA standard.

On Oct 28, 2016, at 9:37 AM, Kirill Fomichev notifications@github.com wrote:

@rongarret I change elliptic for print k and run

const createHash = require('crypto').createHash const elliptic = require('./') const ec = new elliptic.ec('p256')

const msg = createHash('sha256').update('sample').digest() const sign = ec.sign(msg, 'C9AFA9D845BA75166B5C215767B1D6934E50C3DB36E89B127B8A622B120F6721', 'hex') console.log(r: ${sign.r.toArrayLike(Buffer, 'be', 32).toString('hex')}) console.log(s: ${sign.s.toArrayLike(Buffer, 'be', 32).toString('hex')}) k: a6e3c57dd01abe90086538398355dd4c3b17aa873382b0f24d6129493d8aad60 r: efd48b2aacb6a8fd1140dd9cd45e81d69d2c877b56aaf991c34d0ea84eaf3716 s: f7cb1c942d657c41d436c7a1b6e29f65f3e900dbb9aff4064dc4ab2f843acda8

and for test: k: d16b6ae827f17175e040871a1c7ec3500192c4c92677336ec2537acaee0008e0 r: f1abb023518351cd71d881567b1ea663ed3efcf6c5132b354f28d3b0b7d38367 s: 019f4113742a2b14bd25926b49c649155f267e60d3814b4c0cc84250e46f0083

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[indutny]

@rongarret good to hear this! I'm open to any kind of changes that will clear up the ambiguity in readme, sorry for confusing you!

[rongarret]

That’s OK. Thanks for making this code available!

I would suggest doing one of the following:

1. Change key.sign so that it hashes msg
2. Change the documentation to make it clear that key.sign takes a hash of a message as an argument, not the message itself

Option 1 has the advantage of making the code more closely aligned with the standard, but the disadvantage of being non-backwards-compatible.

If you choose option 2 you might want to also consider checking the msg argument to make sure it has the expected length for a hash. One of the things that makes the current state of things so confusing is that the code is internally consistent (i.e. it will verify its own signatures) but the signatures it produces are not actually valid ECDSA signatures. The signature in your example is not the signature for [0,1,2,3,4,5,6,7,8,9,10], it’s the signature of a message whose SHA256 hash is [0,0,0…,1,2,3,4,5,6,7,8,9,10](or maybe it’s [0,1,2,3,4,5,6,7,8,9,10,0,0…0]?)

rg

On Oct 28, 2016, at 10:09 AM, Fedor Indutny notifications@github.com wrote:

@rongarret good to hear this! I'm open to any kind of changes that will clear up the ambiguity in readme, sorry for confusing you!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[indutny]

I'm up for option 2. I've intentionally made it the way it is right now, because it allows more efficient implementations in case of hashes known ahead of time.

[rongarret]

You have two entry points, key.sign_msg and key.sign_hash, then key.sign could dispatch on the kind of argument it receives. You’d need to wrap hashes so that they can be distinguished from messages. This may be more trouble than it’s worth. Changing the docs is good enough.

On Oct 28, 2016, at 10:40 AM, Fedor Indutny notifications@github.com wrote:

I'm up for option 2. I've intentionally made it the way it is right now, because it allows more efficient implementations in case of hashes known ahead of time.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[rongarret]

Or is this a bug?

b = [2, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 5] ... new ec('p256').keyFromPublic(b).getPublic().x.toString(16) ‘4fffffffafffffffffffffffffffffffb0000000000000000000000050’

[indutny]

@rongarret unlikely, looks like it just takes modulo over field order.

[rongarret]

@rongarret unlikely, looks like it just takes modulo over field order.

But the result of that should still be 5 :-)

In any case, the field order is:

FFFFFFFF00000000FFFFFFFFFFFFFFFFBCE6FAADA7179E84F3B9CAC2FC632551

The number in question is:

4FFFFFFFAFFFFFFFFFFFFFFFFFFFFFFFB0000000000000000000000050

So it's not even close.

On Oct 29, 2016, at 4:59 PM, Ron Garret ron@flownet.com wrote:

Or is this a bug?

b = [2, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 5] ... new ec('p256').keyFromPublic(b).getPublic().x.toString(16) ‘4fffffffafffffffffffffffffffffffb0000000000000000000000050’

[indutny]

@rongarret ok, I see it now. point.x is in reduction context (which is a Montgomery reduction for p256). You need to call point.getX() in order to get non-reduced one.

[rongarret]

Thanks!

On Oct 29, 2016, at 10:41 PM, Fedor Indutny notifications@github.com wrote:

@rongarret ok, I see it now. point.x is in reduction context (which is a Montgomery reduction for p256). You need to call point.getX() in order to get non-reduced one.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.