Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Curve25519 #362

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Add Curve25519 #362

wants to merge 3 commits into from

Conversation

twiss
Copy link
Member

@twiss twiss commented Feb 19, 2024

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

Copy link
Contributor

@tniessen tniessen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

[[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|,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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|.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your comments. I get the point regarding consistency with similar existing interfaces.

@javifernandez
Copy link

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:

  1. Apple wants the spec to allow randomized Ed25519 signatures, either explicitly in the WebCrypto spec or as resolution in the [RFC8032] the spec refers to (edit's preferred option).
  2. Clearer definition of the small-order checks of the EdDSA algorithm; we have recently added to the spec that the cofactorless algorithm should be used, to avoid the potential interoperability issues of leaving this open. Most of the engines' crypto libraries seem to implement this algorithm, though.
    2.1. The current spec draft requires "any" usage of small-order points to cause the signature's verification to fail, however, the engine's crypto libraries are not that exhaustive, according to the results of these tests. As far as I know, the feedback I've got from browsers vendors is to rely on the engine's crypto library and avoid implementing additional checks in the web API.
  3. Interoperability issues with the deriveBit's length operation (issues 329 and 322). These issues affects to other Key Derivation algorithms already present in the WebCrypto API specification, so it's not a specific issue of the Curve25519 based algorithms (X25519 in this case).
  4. Removal of the 'alg' field in the JWK format of the EdDSA key import and export operations. This was added to the spec back Aug 2023 and already covered by the corresponding WPT tests, but caused "some controversy" when trying to implement it in Chrome.

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.

@javifernandez
Copy link

2.1. The current spec draft requires "any" usage of small-order points to cause the signature's verification to fail, however, the engine's crypto libraries are not that exhaustive, according to the results of these tests. As far as I know, the feedback I've got from browsers vendors is to rely on the engine's crypto library and avoid implementing additional checks in the web API.

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.

@Frosne
Copy link

Frosne commented Apr 15, 2024

2.1. The current spec draft requires "any" usage of small-order points to cause the signature's verification to fail, however, the engine's crypto libraries are not that exhaustive, according to the results of these tests. As far as I know, the feedback I've got from browsers vendors is to rely on the engine's crypto library and avoid implementing additional checks in the web API.

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.

@javifernandez
Copy link

2.1. The current spec draft requires "any" usage of small-order points to cause the signature's verification to fail, however, the engine's crypto libraries are not that exhaustive, according to the results of these tests. As far as I know, the feedback I've got from browsers vendors is to rely on the engine's crypto library and avoid implementing additional checks in the web API.

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 ?

@Frosne
Copy link

Frosne commented Apr 15, 2024

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

@javifernandez
Copy link

2.1. The current spec draft requires "any" usage of small-order points to cause the signature's verification to fail, however, the engine's crypto libraries are not that exhaustive, according to the results of these tests. As far as I know, the feedback I've got from browsers vendors is to rely on the engine's crypto library and avoid implementing additional checks in the web API.

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 ?

It'd be very important to have @davidben opinion on this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Selected ECC curves are not secure
6 participants