Yubico / java-webauthn-server

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

Misleading exception when validating malformed signature #234

Open JohnnyJayJay opened 2 years ago

JohnnyJayJay commented 2 years ago

The Crypto.verifySignature method used to validate assertion signatures currently throws a very general exception if any IllegalArgumentException or GeneralSecurityException occurs in the process. It seems to me that this is not intended to be caught in user code because it is sort of a non-recoverable thing ("either reconfigure your JVM because e.g. you're lacking some cryptographic provider or report this as a bug in the library" is what the exception message suggests).

However there is a case that is recoverable and should be equivalent to an invalid signature (i.e. signature.verify(bytes) returning false). That case is a malformed signature. I.e., the signature bytes are present but cannot be parsed according to the provided algorithm. in that case, signature.verify(bytes) will throw a SignatureException. IMO this exception should be caught separately and ultimately result in a AssertionFailedException on the API end rather than a RuntimeException. The goal of this would be to handle (possibly malicious) erroneous requests better and prevent them from throwing uncaught RuntimeExceptions that the library or your app isn't at fault for.

One open question is of course how that information should be conveyed to the caller of verifySignature. If verifySignature just returned false in case of a SignatureException on the verify call, you would certainly lose some information that could be useful in development/debugging.

emlun commented 2 years ago

Thanks for bringing this up! This seems like a good point, but we'll have to do some testing before we can say for certain if we want to make a change. My gut feeling is that this seems like a good idea, but I can't promise anything yet.