-
Notifications
You must be signed in to change notification settings - Fork 110
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
Unclear Docs - Which sign()
to use?
#864
Comments
After a bunch of reading (especially https://www.rfc-editor.org/rfc/rfc6979 ) it seems that sign/try_sign are fine. I think what's needed here is expansion of the docs to say "what" and "why". For example for try_sign it needs to say "this implement rfc6979 signatures. You want to use these in X situation". Similar for randomised variant, a "why" you might want it would really help. |
We generally don't expose user choices which allow users to shoot themselves in the foot via misuse. The traits all map to different signature algorithm APIs and the different parameters they take:
Generally the answer of what traits to use is going to be a combination of what APIs the desired signature algorithms support, and what input you have available. Note that not every algorithm that implements These traits have to be broad enough to cover every conceivable signature scheme with a common API, which is quite a challenge. Perhaps there could be a more general high-level overview of the traits in the rustdoc, but as you've discovered with RFC6979 things are quite nuanced and it's quite easy to get in the weeds with details of particular signature schemes that users who aren't expressly interested in those schemes will know or care about. |
Understandable, but by also having so many traits and not describing their purpose, it also still allows misuse. I was genuinely confused for some time when I realised all the different options.
An idea could be to document the combinations in the curve itself such as https://docs.rs/p256/latest/p256/struct.NistP256.html This could easily have an example here that says "when using with Signer/Verifier this can use X, Y, Z traits.". That way there is a clearer "okay, p256 with the DigestSigner trait is what I want" for example. But it also makes it clearer that the DigestSigner trait is still safe even though it doesn't have the rng (due to the rfc6979 deterministic randomisation) |
P-256 isn't a signature algorithm. ECDSA / P-256 is a signature algorithm. However, our implementation of ECDSA is generic. The trait impls are all in that crate: |
I know this, but the issue most users have is "I have to take all these different lego blocks and find how to put them together correctly". There isn't one place that says "here is a combination you should use". So that's why I suggested p-256 as the location for this, since it's the piece that has the most impact on how you hold the pieces as you perform the signature. All the other parts are generic. |
Reading the docs for a
SigningKey
there are a number of Signing traits. These include Signer, RandomizedSigner, PrehashSigner, RandomizedDigestSigner, DigestSigner.The issue it's that the docs aren't clear on how these fit together, or the security properties. The classing being the private key recovery of the ps3 due to lack of randomisation in signatures, but this crate doesn't seem to indicate that an rng is needed during
try_sign()
with the Signer trait or the DigestSigner variant.The docs should be clearer about what is the right trait to use - especially since there are potential security issues with using the wrong one.
The text was updated successfully, but these errors were encountered: