Skip to content

Conversation

HamdaanAliQuatil
Copy link
Collaborator

No description provided.

@HamdaanAliQuatil
Copy link
Collaborator Author

Split from #114

Comment on lines 39 to 41
/// Use [EllipticCurve.p256] for the P-256 curve.
/// Use [EllipticCurve.p384] for the P-384 curve.
/// Use [EllipticCurve.p521] for the P-521 curve.
Copy link
Member

Choose a reason for hiding this comment

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

I think we should remove this, or find something useful to say about each curve.

That might be hard.

If we want to say something, we could find some NIST recommendations for which curve to use, or link to keylength.com.

Like how we did in: https://pub.dev/documentation/webcrypto/latest/webcrypto/RsassaPkcs1V15PrivateKey/generateKey.html

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

imo we won't be able to do something very similar to RsassaPkcs1V15PrivateKey since we do not have curves that are discouraged and we only support 3 curves as of now.

I'm considering the option of removing them.

I found SafeCurves which says that P-256 is secure but lacks some nice-to-have features.
Some critics say Why SafeCurves is a misnomer, that the end user does not need to know such details and SafeCurves only cater to folks who want to design their own cryptographic protocols/ Learn from mistakes.

I found a NIST recommendation on the Choices of Key Lengths, Underlying Fields, Curves, and Base Points https://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-186.pdf

but I can't see a way to possibly add this information for 2 reasons:

  1. We are not providing the user an option to use P-192
  2. The doc talks about the security strength of each curve:

In general, if an elliptic curve has a basepoint of order n, then the security strength will be
approximately one-half of the bit length of n.

imo we can add a comment about this and specify why it should concern an end user but we should probably avoid mentioning anything that a layman cannot understand.

Copy link
Member

Choose a reason for hiding this comment

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

but we should probably avoid mentioning anything that a layman cannot understand.

So long as we have sufficient sources, I think we should assume that the reader of this package is somewhat versed in cryptography -- or at-least that's it's someone willing to actually learn about it.

If you're not willing to learn about cryptography, you probably shouldn't use this package, but rather use something simpler. This is intentionally pretty low level.


On the flip side, let's land this as is, we can always improve the documentation.

@HamdaanAliQuatil HamdaanAliQuatil requested a review from jonasfj May 28, 2024 17:37
@jonasfj jonasfj merged commit 2b1d99e into google:master May 29, 2024
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.

2 participants