-
Notifications
You must be signed in to change notification settings - Fork 160
Add support for Ed25519 and Ed448 #429
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
base: main
Are you sure you want to change the base?
Conversation
Test Results2 271 tests 2 263 ✅ 1m 0s ⏱️ Results for commit a515d73. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright! 🙂 we'll also need corresponding constants in PublicKeyCredentialParameters
, and to add Ed448
to the default preferredPubkeyParams
(also in RelyingPartyV2
). I'd say to add it after ES512
, since Ed448
and ES512
are at the same security level and the current default prefers ES256
over EdDSA
.
Then we also need some tests! 🙂
-
First off, that
Ed448
is included in emittedpubKeyCredParams
when available. You can duplicate this test and replace theEdDSA
constant references (but not theKeyFactory
argument, I think) withEd448
:Line 4280 in 5cbebf2
it("EdDSA, when available.") { RelyingPartyRegistrationSpec
) -
We also need some tests of using Ed448:
Line 2417 in 5cbebf2
it("a generated Ed25519 key.") { So I guess for that I'll need to show you how the
RegistrationTestDataGenerator
works 😄 (of course if we can find some real examples that's great too, but I don't know of any popular authenticator implementations off the top of my head.)You can run that class in your IDE, and it will go through a hardcoded list of
RegistrationTestData
instances and callregenerate()
on each of them. Then for each test data index it'll print the index in the list, and then a block of code to paste over thenew RegistrationTestData(...)
for that test data set. After that you can run the code formatter and commit the new or updated test data. This way we have a set of test vectors that are stable even after changes to the code that originally generated them, but which can be easily updated whenever needed. Most of these are also listed in thedefaultSettingsValidExamples
constant, which is used inRelyingPartyRegistrationSpec
to check that all of those test cases work with the default settings.So for this we can start by duplicating the
val BasicAttestationEdDsa
asval BasicAttestationEd448
and just update the constant inregenerate()
, then add that to the list inregenerateTestData
and todefaultSettingsValidExamples
, and then runRegistrationTestDataGenerator
to generate the test case. For this we'll probably need to add some switch cases inTestAuthenticator.scala
(regenerate()
callscreateBasicAttestedCredential
which eventually ends up ingenerateKeypair
, and you'll also need to update theKeyPairGenerator argument
further down that call chain) andWebauthnCodecs
too.I realize that's a lot to dig into, so let me know if you need any help and/or guidance with it 😄
* The signature scheme Ed25519 as defined in <a | ||
* href="https://www.ietf.org/archive/id/draft-ietf-jose-fully-specified-algorithms-13.html#name-edwards-curve-digital-signa">Fully-Specified | ||
* Algorithms for JOSE and COSE</a> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The link to jose-fully-specified-algorithms is good, but should probably be in the @see
list instead (ah, and it already is!) since it's not the primary definition of the signature scheme; that's still RFC 8032 like above.
Also, since the upstream outcome (see #319 (comment)) was to recommend against using -19
, we should flag that here. I don't think we need to annoy users with a @Deprecated
annotation though, something like this should do:
* The signature scheme Ed25519 as defined in <a | |
* href="https://www.ietf.org/archive/id/draft-ietf-jose-fully-specified-algorithms-13.html#name-edwards-curve-digital-signa">Fully-Specified | |
* Algorithms for JOSE and COSE</a> | |
* The signature scheme Ed25519 as defined in <a | |
* href="https://www.ietf.org/archive/id/draft-ietf-jose-fully-specified-algorithms-13.html#name-edwards-curve-digital-signa">Fully-Specified | |
* Algorithms for JOSE and COSE</a> | |
* | |
* This value is NOT RECOMMENDED, see the <a href="https://w3c.github.io/webauthn/#dom-publickeycredentialcreationoptions-pubkeycredparams">documentation of <code>pubKeyCredParams</code></a>. Use {@link EdDSA} instead or in addition. |
* The signature scheme Ed448 as defined in <a | ||
* href="https://www.ietf.org/archive/id/draft-ietf-jose-fully-specified-algorithms-13.html#name-edwards-curve-digital-signa">Fully-Specified | ||
* Algorithms for JOSE and COSE</a> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's refer to RFC 8032 as the primary source here too.
No description provided.