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

Undocumented breaking change in v3 regarding base64url padding strictness #482

Closed
daltones opened this issue Sep 21, 2023 · 3 comments · Fixed by #497
Closed

Undocumented breaking change in v3 regarding base64url padding strictness #482

daltones opened this issue Sep 21, 2023 · 3 comments · Fixed by #497
Assignees
Labels
Compliance Compliance with applicable specifications
Milestone

Comments

@daltones
Copy link

daltones commented Sep 21, 2023

Description

I'm in the process of migrating web-token/jwt-* packages from v2 to v3 and a test case of my project showed that base64url decoding isn't padding-strict anymore.

To give more context: we have our own component to create or verify JWS tokens, but that is just a wrapper for web-token/jwt-signature functionality. And that component's test cases also cover malformed tokens.

The test that is not passing refers to a JWS token with valid header and signature parts and a payload part that is not valid base64url content regarding padding.

  • In v2, JWSSerializerManager::unserialize() throws InvalidArgumentException: Unsupported input.
  • In v3, JWSSerializerManager::unserialize() returns fine with some payload.

I noticed that this behavior change was introduced in ab78fbb, in which several Base64Url::decode() calls were simply replaced by Base64UrlSafe::decode() calls.

The problem is that Base64Url (from spomky-labs/base64url) is padding-strict since it uses PHP's built-in base64_decode(), and Base64UrlSafe (from paragonie/constant_time_encoding) is not padding-strict by default (there is a flag on the ::decode() method).

@Spomky Spomky self-assigned this Sep 21, 2023
@Spomky Spomky added the Compliance Compliance with applicable specifications label Sep 21, 2023
@Spomky Spomky added this to the 3.2.9 milestone Sep 21, 2023
@Spomky
Copy link
Member

Spomky commented Sep 21, 2023

Hi,

I agree. The encoding should not deviute from the specification.
This will be fixed in the current minor version.

@Spomky Spomky linked a pull request Jan 4, 2024 that will close this issue
@Spomky
Copy link
Member

Spomky commented Jan 4, 2024

Addresse in #497.
Will be tagged soon.

@Spomky Spomky closed this as completed Jan 4, 2024
Copy link
Contributor

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Compliance Compliance with applicable specifications
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants