-
Notifications
You must be signed in to change notification settings - Fork 294
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
ECDSA: Add recovery ID to test cases #103
Comments
I looked that this issue just briefly. It seems that public key recovery has its own set of potential implementation issues, so that it would make sense to have its own test and test vector format. For example it is possible to generate invalid signatures with the property that public key recovery leads to a public key with a point at infinity. Obviously an implementation should properly handle such situations. Generating such a set of test vectors wouldn't be the main issue here. The first step would be to determine a test vector format that has sufficiently many use cases. There are a number of somewhat different protocols that all require public key recovery and hence it would be useful to have good coverage. Another question is finding a good place for a potentially new set of test vectors. I.e., it would be useful to have test vectors for specialized protocols all in the same place. The JCE interface does indeed not expose underlying EC arithmetic. BouncyCastle has some direct support, but having to rely on specific providers is of course not great. Test vector generation for the current test vectors does indeed try to generate a lot of edge cases. Hence the test vectors contain cases where recover ID 2 or 3 would be necessary. |
Having dedicated tests would be ideal indeed. In the meantime I tried generating the recover IDs but did not find an implementationation that is flexible enough which I can run with reasonable setup effort. The use case I'm most familiar with is Ethereum (which I guess is mostly the same as Ethereum). There you only require recover ID 0 and 1 since other IDs are considered invalid. It would be great to not run everything through DER but have Personally I'm interested to use them for ecdsa/secp256{k,r}1. But of course, the more use cases can be covered, the better. |
I had some time to look into public key recovery and have written some initial code to generate test vector. Since ECDSA verification through public key recovery use a different algorithm than ordinary ECDSA verification there are different kinds of edge cases that should be checked. One thing that needs to be done here is to define ECDSA variants, i.e., determine the following properties / parameters:
Concrete specifications (i.e. something resembling a standard) for ECDSA variants would be really helpful. I'm finding a number of small contradictions what is valid and it is often not clear if this is because of distinct protocols or misunderstandings. |
The |
I'm putting some experimental files here: https://github.com/bleichenbacher-daniel/Rooterberg/tree/main/ecdsa |
That's great stuff, thanks a lot for the effort! For my purpose the split r, s, revovery_id would be ideal. IMO the protocol specifics can be done by the user of the test vectors. My experience working with wycheproof was a bit painful as I had to write a custom DER encoder with all sorts of edge case handling just to use the test vectors but do not need DER anywhere in my project. The expected pubkey could be included in compressed and uncompressed form maybe? Not too hard to do this in the caller but if you include both that would make it more convenient. Ethereum users will need the uncompressed pubkey (65 bytes), my API needs the compressed representation (33 bytes). |
I did push some new test vectors with a new format. These are still experimental. I agree that tester should not have to parse DER encoding if their library is not using DER encoded signatures. Currently, there are test vectors that use P1363 encoding, which are easier to parse, but it would still make sense to add additional encodings. For standards that are well defined it would make sense to add additional encoding as well. Figuring out a convenient format is one of the goals here, I think. |
I implemented a prototype of some tests using these in here. |
Thanks a lot for the feedback. One of the open issues is indeed to collect information about different variant of ECDSA in use, so that it is possible to generate test vector files for given use cases. If there is a standard, RFC, BIP, etc. that describes which signatures are valid then it should be possible to generate test vector files for that given document or application. The test vectors do have a header describing the assumptions for the ECDSA test case: "testType": "EcdsaVerify", First testType describes the type of the test, which is verifying signatures here. I'll add other test vectors for deterministic signing (e.g with RFC 6979). If the header doesn't fit your application, then please ask for a new file. Generating the test vectors is far from being the biggest issue here. Finding out what variants exist is far more time consuming. I'm not too familiar with the crypto coin world and I easily mix up different proposals. |
I see, thanks for the extensive explanation. So for me the |
I hope this works better now. Eventually there should be some documentation listing protocols and corresponding parameters in order to simplify the selection of test vectors. Though I've just started collecting this kind of information. |
For testing an
secp256k1_recover_pubkey
implementation (recover pubkey from message and signature), it would be very helpful to have the recovery ID that is available during the signing process.The recovery ID is described here:
Right now I have it working by
It does not seem to to be exactly trivial to get it from the Java implementation due to the high level APIs. This was discussed here: https://stackoverflow.com/questions/49247397/how-to-extract-public-key-from-ecdsa-signature. But maybe you have some better idea how to approach this.
The text was updated successfully, but these errors were encountered: