search

Yubico / java-webauthn-server

Server-side Web Authentication library for Java https://www.w3.org/TR/webauthn/#rp-operations
Other
457 stars 142 forks source link

Support for RS384 and RS512 #235

Closed JohnnyJayJay closed 1 year ago

JohnnyJayJay commented 1 year ago

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.

emlun commented 1 year ago

Hi! Thanks, but yes, unfortunately there's quite a lot more that goes into adding support for new algorithms. Take a look at commit e573f14c 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. :slightly_smiling_face:

JohnnyJayJay commented 1 year ago

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

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.

Is this PR motivated by a concrete demand for these features, or just wanting to contribute?

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.

emlun commented 1 year ago

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?

JohnnyJayJay commented 1 year ago

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. You can credit me using my GitHub username.