Add Curve25519 #362
Add Curve25519 #362
Conversation
Also, fix section hierarchy.
| [[NIST-SP800-38A]] and <a href="#concept-contents-of-arraybuffer">the contents of | ||
| |plaintext|</a> as the input plaintext. | ||
| Perform the Ed25519 signing process, as specified in [[RFC8032]], | ||
| Section 5.1.6, with |message| as |M|, |
There was a problem hiding this comment.
Is there a use-case that someone can quote where the determinism of signatures as per the RFC 8032 is a requirement?
If there are none, It would be useful to add a note here that randomized signatures which verify fine as per the verify section in this spec, should also be permissible.
There are some enhancements(one of them) to the deterministic signing scheme in the RFC that might help better mitigate against side channel attacks, but as a side effect, make the signatures randomized, although still perfectly verifiable and thus usable.
There was a problem hiding this comment.
There are some use cases for deterministic signatures, e.g. RFC9162 (Certificate Transparency) has the use of deterministic ECDSA as an option (as well as Ed25519).
However, I think the more important reason to refer to RFC8032 here is that we should have a well-specified API, which behaves consistently across different implementations. If one implementation produces deterministic signatures and another produces randomized ones, it may cause unexpected interoperability issues, makes testing harder (since the expected behavior is not well-specified), and may become a fingerprinting target (i.e. be used to detect which implementation/platform it is).
And finally, the linked draft is expired, and it's not clear whether it will become an RFC in its current state. For example, one comment suggests to rename the randomized variant to REd25519, for example, rather than modifying the existing algorithm - which I personally agree with. That way, we can always add support for the randomized variant later, and applications can choose based on their use case.
There was a problem hiding this comment.
Any application relying on the fact that signatures are deterministic probably needs some rethinking. The only criteria should be, "Signatures are verifiable".(That seems to be true for RFC 9162 as well)
The draft I quoted has definitely expired but the point it highlights still stands. Below are some more examples of why randomized signatures have merit.
https://nielssamwel.nl/papers/ctrsa2018_wolfssl.pdf
In this paper we show that in use cases where power or electromagnetic leakage can be exploited, exactly the mechanism that makes EdDSA deterministic complicates its secure implementation.
https://www.romailler.ch/ddl/10.1109_FDTC.2017.12_eddsa.pdf
Since more and more embedded devices will implement EdDSA we analysed its resistance to fault attacks. We exploited the determinism of the algorithm to build a fault attack and we demonstrated its practicality
All that said, to re-state, I am not asking for forcing randomized signatures but to accept them as long as they are verifiable.
There was a problem hiding this comment.
All of those may be entirely valid points, but it would be better to discuss those topics in the CFRG, and progress the draft you linked, for example. But as it stands, Ed25519 is a deterministic signature algorithm, and we shouldn't unilaterally change that in Web Crypto.
There was a problem hiding this comment.
Also, REd25519 is probably not in the interest of improving security on the web. If you give options(random vs non random), you are expecting web developers to make a decision without fully understanding the implications. It would be hard to reason in favor of letting them decide vs the spec makes the decision for them.
There was a problem hiding this comment.
OK. Yeah, please do raise it with the CFRG, and could you please also suggest some text that you think should be changed/included in the spec? I wasn't being rhetorical in my last message, I'm really not sure what would be a reasonable way to specify this.
There was a problem hiding this comment.
As I proposed in the meeting yesterday, I could change the text to "as specified in [[RFC8032]] or its successors", or something like that? That way we don't have to wait for the consensus in the CFRG.
There was a problem hiding this comment.
I don't think that's sufficient. I think we should explicitly say that the contentious bits are implementation-defined until CFRG has fully considered them. It's reasonable to point to the drafts that exist for this. And we'll revise Web Crypto when that time comes to remove the implementation-defined bits.
That should probably include the issue @galadran raised in mozilla/standards-positions#271 (comment) as well although I'm not sure a draft exist for that.
Without such provisions it would be perfectly valid to write tests that would enshrine behavior of the current RFC which does not appear to be desirable.
There was a problem hiding this comment.
A better way of slicing this could be:
- Implementations should follow [[RFC8032]].
- Implementations must either reject or produce a signature that verifies.
And then we only test the "must".
There was a problem hiding this comment.
Apologies for the delayed response, I was off last week.
I don't think we should allow creating any signature that verifies, that would allow a rather wide range of behaviors even beyond what draft-irtf-cfrg-det-sigs-with-noise allows.
Perhaps we can informally say that we assume that draft-irtf-cfrg-det-sigs-with-noise will become a successor of RFC8032, and in anticipation of that, change the tests to allow randomized signatures (in addition to the change I suggested in my previous comment, ofc)?
| </dd> | ||
| <dt>Otherwise:</dt> | ||
| <dd> | ||
| Return an [= octet string containing =] the first |length| bits of |secret|. |
There was a problem hiding this comment.
Is there a use-case where web developers will want to use a secret less than 32 bytes in 2024?
If not, would it be reasonable to just remove the length parameter and always return 32 bytes ?
There was a problem hiding this comment.
I think there are legitimate use cases for shorter secrets, e.g., 128-bit or 192-bit symmetric keys. That being said, instead of using the output of X25519 directly, many protocols pass the output of X25519 through a hash function, often combined with additional data, in which case one truncates the output of the hash function and not the output of X25519.
In any case, the length parameter is a positional argument of the public deriveBits() function and cannot be removed. Theoretically, the X25519 implementation of the derive bits operation could reject lengths less than 32 bytes, but that seems like a rather artificial restriction and would be inconsistent with ECDH, for example.
There was a problem hiding this comment.
There has been some discussion on changing the handling of the length parameter of deriveBits for ECDH, e.g. #345 (comment) proposed to throw if the requested length is shorter than the full value. I believe Chromium is doing some measurements to see if that would be compatible with existing web applications. If so, we could change it for X25519 as well - but I do think it should be consistent with ECDH, indeed.
There was a problem hiding this comment.
Thanks for your comments. I get the point regarding consistency with similar existing interfaces.
|
We finally discussed about this PR in the last WebAppSec WG meeting. I don't think there was consensus on giving green light to merge this PR for now, although I don't think there are blockers either, but some people wanted to discuss a bit more in the PR about certain open issues. I'd try to summarize them in this comment:
My opinion is that only 1 and 2 might need to be addressed before merging the PR, perhaps introducing some freedom for implementers while there is not a CFRG resolution. I think 3 and 4 can be perfectly handled as issues of the WebCrypto APi spec draft. |
It seems Mozilla is implementing some improvements on NSS to handle non-cannonical and small-order points in EdDSa signatures. I have filed a similar report for BoringSSL; no idea what CryptoKit will do regarding this. It'd be great if the 3 engines show interoperable results on the tests defined for these cases. |
Hi, our plan is to reject all the small order points, whereas currently we have an ongoing discussion about the way to treat mixed-order points. |
This would make Firefox non interoperable with Safari and Chrome, according to the results of these tests. Given that Firefox's plan is aligned with the current Curve25519 draft, would Safari and Chrome reconsider their position on this issue ? |
|
Our opinion is based on the research presented in these papers: https://eprint.iacr.org/2019/779.pdf and https://eprint.iacr.org/2020/823.pdf. We will be very happy to discuss the matter if the other parties are willing to :) |
It'd be very important to have @davidben opinion on this issue. |
Reference the open issues in WICG/webcrypto-secure-curves around randomized signatures and small-order points in the spec text.
|
I've documented the open issues around Ed25519 (WICG/webcrypto-secure-curves#28 and WICG/webcrypto-secure-curves#27) here as well, reflecting WICG/webcrypto-secure-curves#30 and WICG/webcrypto-secure-curves#31. For the time being, @annevk & @nmahendru, does this resolve your objections around this PR? |
|
I think so, yes, thanks for working on that! |
There was a problem hiding this comment.
Should we state that "If performing the key generation operation results in an error, then throw an OperationError ."
for the step 2 of Generate Key Ed25519?
|
I'm not sure Ed25519 key generation can fail, can it? Generating the private key is just getting random bytes, which |
For instance, ECDH key generation could fail (according to the WebCrypto specification). Same thing - same random generation for private key followed by k* basePoint. |
There was a problem hiding this comment.
Thanks for the PR!
Just a couple of minor comments. Feel free to ignore them if you consider them unnecessary.
Set the [[usages]] internal slot of publicKey to be the usage intersection of usages and [ "verify" ].
Should not it be just "verify"? The same for the private key. Yes, I know that it's like this for ECDSA, but still.
If jwk does not meet the requirements of Section 2 of [RFC8037], then throw a DataError.
Should this bit be removed from >If the d field is present and put right before?
Let data be an instance of the privateKeyInfo ASN.1 structure defined in [RFC5208] with the following properties:
Set the version field to 0.
The spec says
version is the syntax version number, for compatibility with
future revisions of this document. It shall be 0 for this version
of the document.
Might mentioning 0 be skipped?
26.2 Registration
The order of the functions for ECDH and X25519 are different ( :D ). We should change ECDH registration operations to have the same order as X25519.
Yeah, if ECDH key generation can't fail, also in the implementations(?), then we could remove that step in a separate PR. (Though, there's a small wrinkle there in that it optionally calls out to an external specification that may specify additional curves, which I guess is allowed to return an error? Although even there I don't know why it would.)
I think we should be consistent here and require folks to explicitly pass
Right.. this step was copied from ECDH and was different there in both branches, but now it's identical in both branches in this draft. However, I think it's sort of conceptually equivalent to saying "if parsing the JWK {private,public} key fails...", so I sort of think it makes sense to have it only after you know whether it's a private or public key. But perhaps the wording should be updated a bit to make it clearer & more specific, and also to make the two branches different again. I'll take a stab at that.
Also this was copied from ECDH. I agree it's a bit superfluous, but it also seems harmless and.. perhaps better to be too explicit than not enough?
Yeah, I agree in general the order of the operations in the spec is a bit all over the place :D Indeed would be nice to make them consistent (and also linkable, btw) in the main spec at some point. |
|
Ok! |
|
@Frosne I've updated the language 😊 Let me know if it looks reasonable. Also, @annevk, @nmahendru and @davidben, it would be great to get one more review and/or an explicit approval as well, if you have time 😊 |
It is! |
|
Just a recap of the 3 issues we identified as blockers to merge this PR: 1- deriveBits interop - I think we can consider this as fixed with the last changes in the spec (eg, #345, #380) So @annevk, @nmahendru and @davidben do you think we could approve this PR now ? |
|
I'll merge this now given that we have 3 approvals and no objections, and also given that Curve25519 is relatively widely supported now (and Chrome also intends to ship X25519). The open issues around Ed25519 laid out in the spec text will be worked on in the IETF, and we can update the spec text here once that's done (e.g. by pointing to an RFC8032bis), as discussed. Thanks everyone for your patience and for the work on this! :) |
SHA: fe7d587 Reason: push, by twiss Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
This PR copies the text on X25519 and Ed25519 from https://github.com/WICG/webcrypto-secure-curves, and updates the tables, examples and references.
X448 and Ed448 are not yet included, due to there being fewer implementations and signs of interest, at the moment.
Closes #196.
The following implementers have shown interest:
The following tasks have been completed:
Implementation issues:
Preview | Diff