-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Cross tests with libecc #1112
Cross tests with libecc #1112
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice :)
Do you have some instructions on how to build this together with libecc, so that people can play around with it?
I think the next step would be to try to produce the same signature with libsecp256k1 and libecc, and see if they match exactly. This should work out as libecc also uses deterministic ECDSA with RFC6979, so it should be the exact same algorithm. (If not, we need to see if they do something differently, and you may need to switch to their low-level function https://github.com/ANSSI-FR/libecc/blob/master/src/sig/fuzzing_ecdsa.c#L31.)
|
||
ec_pub_key ec_pubkey; | ||
CHECK(ec_pub_key_import_from_aff_buf(&ec_pubkey, (const ec_params *)¶ms, | ||
pubkey_buf + 1, pubkey_len - 1, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this intentional given the lines above, where you already modify pubkey_buf
and pubkey_len
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh. That's a careless mistake I always make. After I successfully ran the test (without first modifying pubkey_buf
and pubkey_len
), I thought modifying them first would "look" better. So I did it and forgot to run the test again to check.
I turns out that the code doesn't even compile with pubkey_buf += 1
, so the "better looking" modification is actually worse. I'll correct the mistake. My bad!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My bad!
No worries, we all have bugs in our draft code. ^^
src/tests.c
Outdated
CHECK(get_sig_by_name("ECDSA", &sig_mapping) == 0); | ||
|
||
pubkey_buf += 1; // this is needed because libecc only supports uncompressed points | ||
pubkey_len -= 1; // and does not recoginze B0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is B0?
Are you sure libecc doesn't support compressed keys? that would surprise me.
and typo: "recoginze" ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After reading https://github.com/ANSSI-FR/libecc/blob/master/src/sig/ec_key.h several times, I concluded that libecc
supports importing projective or affine coordinates, in either a "structured" or an "unstructured" format. By structured they mean that the buffer includes the curve parameters, the signature algorithm, etc., in addition to the key. So I thought the simplest way to port libsecp
's public key to libecc
is through an unstructured affine buffer.
By B0 I just mean the first byte :). I took this terminology from here. I was puzzled why the public key produced by libsecp
is 65 bytes long when a key is really two 32-byte values. So I googled a bit and found that "uncompressed keys" have an extra 04
at the beginning.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you confusing jacobian/projective coordinates with compressed/uncompressed encoding?
- Affine coordinates are tuples (x, y) on the curve.
- Projective coordinates are tuples (X, Y, Z) representing the affine coordinate (X/Z, Y/Z).
- Jacobian coordinates are tuples (X, Y, Z) representing the affine coordinates (X/Z^2, Y/Z^3).
SECG encoding of public keys is (all representing affine coordinates):
- Compressed encoding
- (0x02 || x) for even y
- (0x03 || x) for odd y
- Uncompressed encoding
- (0x04 || x || y)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for succinctly summarizing the different presentations :), but I don't think I'm confusing them. I believe I've used the correct formats/encodings/presentations in the code..? The only mistake I've done is offsetting pubkey_buf
twice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@BerryYoghurt Now you updated the comment to "does not recognize B0 = 04". But that wouldn't that mean it supports only compressed and not only uncompressed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@real-or-random For byte-by-byte comparison I'll do that, but when signing with libsecp and verifying with libecc, isn't it more logical to give libecc access only to the public key? I mean, the verifier doesn't normally have the private key, they're supposed to read the public key and the message from the signer then check the signature. Right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps it's better to refer to this format supported by libecc as "raw public keys" or something like that; "uncompressed" is likely to be understood as SECG's (0x04 || X || Y) format.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@real-or-random For byte-by-byte comparison I'll do that, but when signing with libsecp and verifying with libecc, isn't it more logical to give libecc access only to the public key? I mean, the verifier doesn't normally have the private key, they're supposed to read the public key and the message from the signer then check the signature. Right?
So yes, in the real world, the signer obviously wouldn't reveal the secret key.
For our purpose of cross-testing however, this is fine. If libecc recomputes the public key, then the cross-test would also cover the computation of the public key from the secret key. So if there's a difference between the two libs in that algorithm, the test would also catch it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@real-or-random I was convinced of cross-testing the key generation algorithm, and it is now done in the same testcase that compares signatures byte by byte. However, after writing the new testcase, I am still reluctant to derive the public key from the private key in the other two -- for two reasons.
First, it's not much simpler (code-wise) to import the private key instead of the public key. The only difference is avoiding the header byte.
Second, since it's already done in test_compare_libecc_secp256k1
, I think having both is better, more or less.
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, I think if we verify with libsecp256k1 and libecc in test_compare_libecc_secp256k1
, then we won't need the other two tests anymore.
edit: Let me try to explain in more detail. So in general, I see that it makes sense to have multiple test cases. But one of the goals here should be efficiency because later on we want to run many iterations in a loop. In that sense
- sign with libecc
- sign with libsecp256k1
- check sigs are identical
- verify with libecc
- verify with libsecp256k1
seems a rather efficient way of doing tests, i.e., we get a lot of "test value" per running time.
The way I build them is a bit hacky and I am actually looking for a better way. But here it is:
EDIT: modified to accommodate deterministic ECDSA |
…c-cross-tests ci fails
By this, do you mean comparing the signatures byte by byte? |
Yes, the serialized representations should be identical byte by byte. |
I am having some difficulty in including What I thought I could do:
But the problem I'm facing right now is that most (if not all) I think there must be a way of "plugging in" Any help would be appreciated! |
Hm, I'm not sure I can follow. So why would you want to forward-declare those? So the usual way to use a C library is include its header in your source file and then later link against the compiled library. So you would
Or if item 1 adds complexity, first skip it and use the existing Or is the problem that you need access to internal functions or types of libecc? |
Closing due to lack of activity. Cross-testing is tracked in #691. |
See #1108
TODO:
test_secp256k1_sign_libecc_verify()
. (I still need to figure out how to try it without the whole ci overhead)libecc
and verify withlibsecp256k1