search

RustCrypto / signatures

Cryptographic signature algorithms: DSA, ECDSA, Ed25519
463 stars 106 forks source link

Non-conforming ECDSA signature generation #833

Closed emil-wire closed 2 months ago

emil-wire commented 2 months ago

Hello 👋 I'm Emil, Security Engineer at Wire (github.com/wireapp) an E2EE secure messenger. We recently conducted a security audit of our core-crypto library which implements the MLS standard and the external researchers found an issue in the P256 + RFC6979 implementation leading to non conforming signatures.

private key = 1888cb732fea4353451b856e4f2c0715163bcf4bb7788ab44f70ef9ff9727167
message = 049a27b521c74e81

Expected signature:

9ac51396b0c8d0a5a082b5d88b45760504c1e7c8ba1a49e8b40f4098ac74c757563050ecbfc45165503f9ef57b3039baae0000d6eebcecd922bb280b35d2db79

Actual signature using p256 version 0.13.2

5dcd6baac9e02e2351763333d40d73157d4fee343954c7a380c7fb688f9ef9609f473dcdbc0fbc2bcf020ac3cf5eeed9d88634c0014419a0bdc4c0f88341ca57

This seems to have been fixed in p256 0.14.0-pre.0 which produces the correct signature.

Since this doesn't have security implications for Wire but might have implications for other users, I'd suggest investigating the implementation.

Cheers & thank you for your work <3

PS: A more detailed writeup is available to @tarcieri

tarcieri commented 2 months ago

This was likely fixed in #777, which was an oversight. I could potentially backport it to older versions of ecdsa.

While it's a bug, it's one I don't believe to be security-critical in that the miscomputed k values are still uniformly random HMAC-DRBG output, and in that regard if someone were to use our buggy implementation as well as a correct one which produce two different k values for the same message, that's no different from signing the same message twice with two different (uniformly) randomly generated k values.

Security critical impacts would either involve reuse of k or producing a k which is not uniformly random, neither of which is happening here.

tarcieri commented 2 months ago

Hmm, I tried cherry-picking #777 onto ecdsa v0.16.9, but that didn't seem to address the issue, so I'm not sure there's a quick fix here.

There was some significant work on the rfc6979 crate to improve it and bring it more in line with the RFC, which also included API changes (#773, #781, #775).

I'm going to close this as "fixed on master" as I can't figure out an easy way to backport the fixes to v0.16.x.