Skip to content
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

Base64 decoder should not ignore illegal characters #875

Open
laurids opened this issue Nov 30, 2023 · 1 comment
Open

Base64 decoder should not ignore illegal characters #875

laurids opened this issue Nov 30, 2023 · 1 comment
Milestone

Comments

@laurids
Copy link

laurids commented Nov 30, 2023

Is your feature request related to a problem? Please describe.
I would like to challenge the behavior described here: https://github.com/jwtk/jjwt#adding-invalid-characters
The base64 decoder implementation does not follow best practice, because it ignores non-alphabet characters at the start and end of the string.
It is best practice to reject strings with illegal base64 characters, both according to rfc4648, and it is the most common expectation, since the standard java.util.Base64 decoders do so.
In our project, we need to parse JWT access tokens in the server code, but we also need to pass the raw JWT to another component, which is not written in Java, so it uses a different parser. That parser will fail on illegal base64 characters, but we would like to fail as soon as possible.
Why would you need to be lenient regarding non-base64-characters? If an authorization server issues JWTs with illegal base64 characters, would it not be fair to reject them and instead notify them to fix their code!?

Describe the solution you'd like
The base64 decoder implementation to throw an exception when encountering a non-alphabet character. Such as the standard java.util.Base64 decoders do - they throw IllegalArgumentException with a message like "Illegal base64 character".

Describe alternatives you've considered
Edit: We could use the standard java.util.Base64 URL safe decoder by setting it as described in https://github.com/jwtk/jjwt#custom-base64

Additional context
According to rfc4648#section-3.3

Implementations MUST reject the encoded data if it contains characters outside the base alphabet when interpreting base-encoded data, unless the specification referring to this document explicitly states otherwise.

According to rfc4648#section-12

If non-alphabet characters are ignored, instead of causing rejection of the entire encoding (as recommended), a covert channel that can be used to "leak" information is made possible. The ignored characters could also be used for other nefarious purposes, such as to avoid a string equality comparison or to trigger implementation bugs. The implications of ignoring non-alphabet characters should be understood in applications that do not follow the recommended practice.

@lhazlewood
Copy link
Contributor

Hi @laurids! This was a design decision in the early days of JJWT in the spirit of the "robustness principle". The main reason being that illegal Base64 characters in the beginning or the end from people trying to 'trick' signature comparisons doesn't actually change the underlying signature, so there is no reason to fail: if the signature is confirmed successfully (from a cryptographic perspective), people trying to 'play games' doesn't impact the security properties of the underlying signature/mac algorithm (which would otherwise be 'security by obscurity' and therefore useless).

We can keep this issue open to track changing the behavior, but this implementation has been around for 8-9 years now; odds are high that existing usages depend on this behavior, so we'd probably only be able to change this on a major release or next significant point release.

As you mention, at least until we feel we can/should change the code, the easiest workaround is to use a (fairly simple) custom Base64Url decoder per https://github.com/jwtk/jjwt#custom-base64.

Thanks for creating the issue!

@lhazlewood lhazlewood added this to the 1.0 milestone Feb 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants