Add sign_digest to EcdsaKeyPair to allow signing of pre-digested messages.#915
Add sign_digest to EcdsaKeyPair to allow signing of pre-digested messages.#915IanLuites wants to merge 1 commit intobriansmith:masterfrom
Conversation
IanLuites
changed the title
briansmith
left a comment
briansmith
left a comment
There was a problem hiding this comment.
Thanks for doing this. I'll get this merged right away if you make the changes I requested.
| digest: digest::Digest, | ||
| ) -> Result<signature::Signature, error::Unspecified> { | ||
| // Step 4 (out of order, already performed by caller). | ||
| if digest.algorithm() == self.alg.digest_alg { |
There was a problem hiding this comment.
Please use the early return style: if ... != ... { return Err(...); }
| ); | ||
| } | ||
|
|
||
| #[test] |
There was a problem hiding this comment.
Tests of the public API should be done in tests/ecdsa_tests.rs.
I suggest that you add a constructor pub(crate) try_from_test_vector to ring::digest::Digest that accepts a digest algorithm and a precomputed value. Then you can rewrite the existing tests to use your new function instead of calling sign_ directly as they currently do.
| /// generated by `rng`. | ||
| /// | ||
| /// The `digest` algorithm must match that of the signing algorithm. | ||
| pub fn sign_digest( |
There was a problem hiding this comment.
Please name this sign_digest_less_safe.
Please add a note like the following to the documentation: "In general, it is not safe to sign an arbitrary digest. Ensure that you only sign digests that you have computed yourself, or that you otherwise know are safe to sign. It could be a bad mistake to sign an attacker-controlled digest."
2 participants
I need to sign a relatively large message and was using
digest::Contextto piece wise stream and update the digest.I did however find out that
signdoes not accept adigest, but only an undigestedmessage.This PR would add a
sign_digestsibling, which verifies the digest is generated with the correct (matching) algorithm, and then signs it.