ecdsa: add hash length constraint #94 #102
Conversation
After some offline discussion, we decided it would be nice to have a separate specification that enforces the constraints on hash and curve size that don't affect the computation of the signature. Now the implementation is divided into two versions, one that enforces those constraints and one that doesn't. Also changes example file to be for a valid curve/hash combo.
|
|
||
| // NB: removed property |
There was a problem hiding this comment.
question: Should the "sign + verify works" property be in this module or in the unconstrained version?
| // We further convert it to an element in `Z n` to support the modular | ||
| // operations later in the protocol. | ||
| e = fromInteger (toInteger e') | ||
| // Documentation for the public interface of this API can be found in |
There was a problem hiding this comment.
question: Are we okay with omitting all the docs here? Should top-level docs live here instead of in the unconstrained version? (I left them in unconstrained because it seemed nice to have the docs next to the concrete implementation). I didn't want to duplicate them to avoid drift between the two versions.
There was a problem hiding this comment.
I think the reference makes sense in this case as I am also worried about drift.
| ⚠ This implementation is complete except for constraints on the size of the | ||
| input parameters ([FIPS-186-5] Section 6.1.1)! Using this implementation will | ||
| allow invalid combinations of parameters. Consider using `specification.cry` | ||
| instead. |
There was a problem hiding this comment.
note: These lines are the only new thing in this doc. Everything else was ported directly from specification.cry.
Making this PR because I want input on how to proceed.
I did not realize this previously, but the DSA standard has a note that requires the security level of the elliptic curve to be greater than the security level of the hash function (see Section 6.1.1 on page 30). This is reasonable in theory -- it would be an easy mistake to think you get security up to the parameter size of the elliptic curve but then have your hash function cut it much shorter.
I'm not sure that practical uses in the wild actually adhere to this requirement. For example, Rust Crypto has a method that associates a preferred digest to a curve but also allows signing a digest directly, and the latter I believe allows any length digest.
As another example, RFC 6979 includes test vectors for many combinations of curves and hash functions, including combinations that are not allowed with this constraint. Indeed, CI for this PR should fail because the first test vector I made (P384 + SHA256) does not meet the requirement.
I would appreciate feedback from internal and external folks about whether we want to encode this requirement into the standard.
If we do decide to encode this, we might want to increase the priority of #98 so that it's easier to instantiate valid curve+hash pairs.