tink-crypto / tink

Tink is a multi-language, cross-platform, open source library that provides cryptographic APIs that are secure, easy to use correctly, and hard(er) to misuse.
https://developers.google.com/tink
Apache License 2.0
13.47k stars 1.18k forks source link

Golang: Descriptive JWT verification errors #645

Closed josephmgardner closed 1 year ago

josephmgardner commented 1 year ago

Is your feature request related to a problem?

When verifying signed JWTs with tink for Golang, a verification failure returns a generic "verification failed" error, without specifying the reason why (for example: token has expired, issuer is incorrect). It would be useful to be able to examine the failure reason, both for testing/debugging purposes and to provide useful information to any clients requesting token validation.

What sort of feature would you like to see?

Specific error types for different failure conditions (for example, like the jwx library would be great.

Alternatively, wrapping the underlying cause of the failure and returning a wrapped error would also be sufficient.

Have you considered any alternative solutions?

Alternate solutions:

Thank you!

juergw commented 1 year ago

Thanks for bringing this up. I agree that this not ideal. The error string should be more descriptive We already do this in Java, and I think we can do something similar in Go.

But I think (at least for now) we don't want to introduce specific error types. The reason for this is that the user should usually not take different actions on the error output, the JWT should always be rejected. Also, distinguishing errors often does not work as well as one would expect. For example, if we want to distinguish an expiration error from a invalid signature error, then this works fine until the verification key gets rotated and removed. Then, the expiration error will suddenly become an invalid signature error. This might cause unexpected and hard to debug problems if the user depends on this.

juergw commented 1 year ago

Since https://github.com/google/tink/commit/1aa2eccfec0a9faf98125e64b4318e43cc1ddccb, the error message returned in JWT verification should now be more descriptive.

josephmgardner commented 1 year ago

Looks good, thanks a lot @juergw! Happy for this to be closed if you are.

josephmgardner commented 1 year ago

Hi again @juergw, I've been thinking about this some more and looked back at your initial reply (which I'm afraid I missed at the time - apologies).

Given that verification now returns different error strings, it is possible for users to distinguish failure cases by parsing the error string. Since this is possible, would it be cleaner to formalise it with error types after all?

I think there are some legitimate use cases for being able to easily distinguish them - for example, being able to log attempted verification of tokens with bad signatures vs expired (the latter being an expected case).

Thanks again for looking into this.

juergw commented 1 year ago

Parsing the error is certainly not what we want the user to do.

We will think about it.

juergw commented 1 year ago

I have now added this function in Golang: https://github.com/google/tink/commit/23005cee999d0ac23b145735b6b5153e0f684322