-
Notifications
You must be signed in to change notification settings - Fork 159
Support for RS384 and RS512 #235
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
Conversation
Hi! Thanks, but yes, unfortunately there's quite a lot more that goes into adding support for new algorithms. Take a look at commit e573f14 for an example (that includes some updates to internal utilities as well, but is a pretty fair representation of the scale). But I'm happy to go through the motions if there's demand for it. Is this PR motivated by a concrete demand for these features, or just wanting to contribute? I'm asking because I'm not aware of any WebAuthn authenticators that use these algorithms in practice. 🙂 |
It seems like I missed some places where the algorithm names are used, but looking at this commit it doesn't seem like a lot of work, especially considering that the COSE format is the same for all the RS... algorithms. I can easily introduce the changes that are still required, it still seems more or less trivial.
Mostly just wanting to contribute - my rationale was that it can't hurt to support more algorithms. I don't know if there is any authenticator out there right now that uses it - I do know WebAuthn4J has support for it though. |
Sorry for leaving this for so long, I've had to focus on other things for a while but now I've come back around to this. Thanks for the suggestion and contribution! I've made the additional changes needed for this, but not pushed them yet. We'll credit you for the contribution in the release notes, unless you prefer we don't - would you like to be credited by GitHub username or real name? |
Oh, thanks for getting back to me. To be honest, I had somewhat forgotten about this in the last months as I got caught up in other things. I'm happy the change made it further, though. |
`webauthn-server-core`: New features: - Added support for RS384 and RS512 signature algorithms. - Thanks to GitHub user JohnnyJayJay for the contribution, see #235 - Added `userHandle` field to `AssertionRequest` as part of the second bug fix below. `userHandle` is mutually exclusive with `username`. This was originally released in pre-release `1.12.3-RC3`, but was accidentally left out of the `1.12.3` release. Fixes: - During `RelyingParty.finishRegistration()` if an `attestationTrustSource` is configured, if the `aaguid` in the authenticator data is zero, the call to `AttestationTrustSource.findTrustRoots` will fall back to reading the AAGUID from the attestation certificate if possible. - Fixed bug in `RelyingParty.finishAssertion` where if `StartAssertionOptions.userHandle` was set, it did not propagate to `RelyingParty.finishAssertion` and caused an error saying username and user handle are both absent unless a user handle was returned by the authenticator. This was originally released in pre-release `1.12.3-RC3`, but was accidentally left out of the `1.12.3` release. - Fixed regression in `PublicKeyCredentialCreationOptions.toCredentialsCreateJson()`, which has not been emitting a `requireResidentKey` member since version `2.0.0`. This meant the JSON output was not backwards compatible with browsers that only support the Level 1 version of the WebAuthn spec. `webauthn-server-attestation`: Fixes: - `findEntries` and `findTrustRoots` methods in `FidoMetadataService` now attempt to read AAGUID from the attestation certificate if the `aaguid` argument is absent or zero. - Method `FidoMetadataService.Filters.allOf` now has `@SafeVarargs` annotation.
This PR makes a very small change to add support for the RSA PKCS#1 based signing algorithms that use SHA-384 or SHA-512 respectively.
I assume it can't hurt to support as many signing algorithms as possible and support for these is trivial, since they can be used exactly the same way as RS1 and RS256.
LMK if I missed something, e.g. if I still need to add tests somewhere.