WICG / webcrypto-secure-curves

Proposal for the addition of Curve25519 and Curve448 to the Web Cryptography API
https://wicg.github.io/webcrypto-secure-curves/
Other
42 stars 4 forks source link

Make small-order checks in EdDSA optional? #27

Open twiss opened 1 month ago

twiss commented 1 month ago

@davidben from Chromium has expressed that they don't want to add checks beyond the ones that are already there in their implementation of Ed25519. So, we might want to make the small-order checks optional.

@galadran and @Frosne, since you have expressed the wish for having these checks, would you be OK with making them optional?

(I personally think it's a bit unfortunate that this would abandon the goal of having consistent verification results across all implementations, but I don't see another way of fulfilling everyone's wishes otherwise. Additionally, the implementations that have been shipped so far don't do all the checks, and it's probably best for the spec to reflect reality.)

panva commented 1 month ago

reflect reality

See wpt.fyi

Frosne commented 1 month ago

Hi,

I consider that these checks provide meaningful security for protocols using these primitives. These checks are relatively simple to add (for example, a patch implementing the main checks has only 50 LOC and most of that boilerplate).

If other implementations are unwilling to add them, I think we'd better be explicit in the standard that the checks aren't done despite the fact we know it leads to attacks on protocols (as described, for instance, in the following papers Prime, Order Please!, Seems Legit: Automated Analysis of Subtle Attacks on Protocols that Use Signatures, Taming the many EdDSAs).

twiss commented 1 month ago

Yeah. Although I personally agree, one could argue that the omission of these checks is an issue with RFC 8032, that should be addressed in the IETF rather than the W3C (similarly to the question of randomized signatures).

Would you (or someone at Mozilla) be willing to bring this up in the IETF and propose the inclusion of these checks in RFC8032bis, perhaps? (If not, I can also do so.)

Then, the solution here could also be to refer to "RFC8032 or its successors", and then if RFC8032bis specifies the checks they can be implemented based on that (and if eventually everyone does then we can always update the spec to only refer to the new RFC, so that they can be relied upon to be done).

javifernandez commented 1 month ago

I consider that these checks provide meaningful security for protocols using these primitives. These checks are relatively simple to add (for example, a patch implementing the main checks has only 50 LOC and most of that boilerplate).

It'd be useful to know @nmahendru opinion about the possibility of implementing these additional checks in WebKit.

nmahendru commented 1 month ago

I consider that these checks provide meaningful security for protocols using these primitives. These checks are relatively simple to add (for example, a patch implementing the main checks has only 50 LOC and most of that boilerplate).

It'd be useful to know @nmahendru opinion about the possibility of implementing these additional checks in WebKit.

We(everyone) should 100% either use a compliant library or add the checks over and above. Fix now and not regret later. I can add them to WebKit.

twiss commented 3 weeks ago

If other implementations are unwilling to add them, I think we'd better be explicit in the standard that the checks aren't done

I've opened a PR to at least document the fact that not every implementation does these checks, here: https://github.com/WICG/webcrypto-secure-curves/pull/31.

In the case of #28, it was proposed that we could merge Ed25519 into at least the editor's draft of the main Web Crypto spec with an open issue mentioned in the spec text, and then resolve it later once there's consensus in the IETF. Perhaps we could do the same for this issue?

javifernandez commented 3 days ago

I consider that these checks provide meaningful security for protocols using these primitives. These checks are relatively simple to add (for example, a patch implementing the main checks has only 50 LOC and most of that boilerplate).

It'd be useful to know @nmahendru opinion about the possibility of implementing these additional checks in WebKit.

We(everyone) should 100% either use a compliant library or add the checks over and above. Fix now and not regret later. I can add them to WebKit.

@davidben it'd be great to know Chrome's position on this. If we all agree, we could just close this issue and leave th espec text as it is, requiring checks for any small-order point.