pyca / cryptography

cryptography is a package designed to expose cryptographic primitives and recipes to Python developers.
https://cryptography.io
Other
6.56k stars 1.51k forks source link

Add support of Ed25519ph (i.e. ed25519 + SHA512) #10969

Open socketpair opened 4 months ago

socketpair commented 4 months ago

https://www.openssl.org/docs/man3.3/man7/Ed25519.html:

The instances Ed25519ph, Ed448ph are referred to as HashEdDSA schemes. For these two instances, the sign and verify procedures do not require access to the complete message; they operate on a hash of the message. For Ed25519ph, the hash function is SHA512. For Ed448ph, the hash function is SHAKE256 with an output length of 512 bits.

alex commented 4 months ago

We'll need to figure out the right API design for this, but I think we'd be happy to add it.

socketpair commented 4 months ago

Same as hashing API, but, sign/verify instead of .final() ? Pass key in constructor or the latest function - is a question. I think, the last. Because one may want to save state (actually of hasher) and we must not save keys in sthe state.

socketpair commented 4 months ago

Or, possibly reimplement hash + sign/verify with constant combinations. And use standard API like ECDSA.

woodruffw commented 4 months ago

I'm also selfishly interested in Ed29919ph support 🙂

I like @socketpair's second proposal, since it keeps these prehash variants consistent with other prehash variants already supported by Cryptography.

With that being said: ed25519ph isn't referentially transparent like ECDSA prehashing is, so users might be surprised that pk.sign(data, prehash=True) != pk.sign(hash(data), prehash=False). But maybe this isn't a huge issue.

TL;DR: I think an API like this would make sense:

pk = Ed25519PrivateKey.generate()

# option 1
pk.sign(b"hash", prehashed=True)

# option 2, reject if the HashAlgorithm isn't a supported one
pk.sign(b"hash", prehash_algorithm=Hashes.SHA512())
reaperhulk commented 4 months ago

ed25519ph supports context data too does it not?

woodruffw commented 4 months ago

ed25519ph supports context data too does it not?

Yep, it does -- RFC 8032 says that the context is up to 255 octets (and is empty by default)

Given that, maybe this isn't trivial to shoehorn into the pre-existing "prehashed" pattern 😅. Instead, it could be something like this:

pk = Ed25519PrivateKey.generate()

pk.sign(b"hash", variant=Ed25519Ph(context=b"foo"))

This would also allow for an Ed25518Ctx variant as well, per RFC 8032. Then again, variant is a pretty bad name (flavor? instance?)

alex commented 4 months ago

I'm inclined to think it should be sign_prrhahed or something

On Wed, May 15, 2024, 6:14 PM William Woodruff @.***> wrote:

ed25519ph supports context data too does it not?

Yep, it does -- RFC 8032 says that the context is up to 255 octets (and is empty by default)

Given that, maybe this isn't trivial to shoehorn into the pre-existing "prehashed" pattern 😅. Instead, it could be something like this:

pk = Ed25519PrivateKey.generate() pk.sign(b"hash", variant=Ed25519Ph(context=b"foo"))

This would also allow for an Ed25518Ctx variant as well, per RFC 8032. Then again, variant is a pretty bad name (flavor? instance?)

— Reply to this email directly, view it on GitHub https://github.com/pyca/cryptography/issues/10969#issuecomment-2113554893, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAAGBDXVKDWAOBKVK2KCKLZCPM4RAVCNFSM6AAAAABHTBFMB6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMJTGU2TIOBZGM . You are receiving this because you commented.Message ID: @.***>

woodruffw commented 4 months ago

I'm inclined to think it should be sign_prrhahed or something

That works for my use case :slightly_smiling_face: -- something like this?

# context defaults to b""
pk.sign_prehashed(b"hash", context=b"foo")
alex commented 4 months ago

Question about whether you pass a hash always or is there a way to pass data, see all the other sign methods with prehashrd

On Wed, May 15, 2024, 6:42 PM William Woodruff @.***> wrote:

I'm inclined to think it should be sign_prrhahed or something

That works for my use case 🙂 -- something like this?

context defaults to b""pk.sign_prehashed(b"hash", context=b"foo")

— Reply to this email directly, view it on GitHub https://github.com/pyca/cryptography/issues/10969#issuecomment-2113600916, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAAGBG3QGWOCI2X7AK34Z3ZCPQGTAVCNFSM6AAAAABHTBFMB6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMJTGYYDAOJRGY . You are receiving this because you commented.Message ID: @.***>

woodruffw commented 4 months ago

Question about whether you pass a hash always or is there a way to pass data, see all the other sign methods with prehashrd

My intuition is hash always, since that's what the ECDSA and DSA prehashed variants do.

reaperhulk commented 4 months ago

If someone thinks of ed25519ph as a distinct algorithm rather than a particular use case of ed25519 they'll find it confusing since you must hash the data yourself before signing or verification and use a different signing method. That may be fine, in which case

def sign_prehashed(self, digest: bytes, context: Option[bytes]):

would be an acceptable API. However, if we want to allow one-shot ed25519ph then things get weirder. We could expand sign_prehashed to take two kwargs, digest or data, but I don't really like that. Which leads us back to thinking about what changes to sign might look like.

woodruffw commented 4 months ago

If someone thinks of ed25519ph as a distinct algorithm rather than a particular use case of ed25519 they'll find it confusing since you must hash the data yourself before signing or verification and use a different signing method.

This is how RFC 8032 talks about Ed22219ph and Ed25519ctx, at least -- they're distinct algorithms within different internal constructions when compared to Ed25519, so (IMO) it makes sense for them to have a separate API.

The absence of a one-shot prehashed API is arguably also consistent with the RFC's advice, which is to not use the prehashed variant at all unless a legacy API requires operating only on digests. So having it be digest-only would be a consistent nudge, if that influences thoughts here :slightly_smiling_face:

(From: https://datatracker.ietf.org/doc/html/rfc8032#section-8.5)