cryptography icon indicating copy to clipboard operation
cryptography copied to clipboard

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

Open socketpair opened this issue 1 year ago • 14 comments

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.

socketpair avatar May 12 '24 19:05 socketpair

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

alex avatar May 12 '24 19:05 alex

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 avatar May 13 '24 06:05 socketpair

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

socketpair avatar May 13 '24 06:05 socketpair

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())

woodruffw avatar May 15 '24 20:05 woodruffw

ed25519ph supports context data too does it not?

reaperhulk avatar May 15 '24 21:05 reaperhulk

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?)

woodruffw avatar May 15 '24 22:05 woodruffw

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: @.***>

alex avatar May 15 '24 22:05 alex

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")

woodruffw avatar May 15 '24 22:05 woodruffw

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: @.***>

alex avatar May 15 '24 22:05 alex

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.

woodruffw avatar May 15 '24 22:05 woodruffw

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.

reaperhulk avatar May 15 '24 23:05 reaperhulk

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)

woodruffw avatar May 15 '24 23:05 woodruffw

@woodruffw do you still care about this? 😄

reaperhulk avatar May 18 '25 17:05 reaperhulk

@woodruffw do you still care about this? 😄

It's not as prominent on my radar anymore, since Sigstore appears to be largely settling on "normal" ed25519 🙂

woodruffw avatar May 18 '25 18:05 woodruffw