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

Vulnerability in the JWT token validation #564

Closed
phindmarsh opened this issue Apr 15, 2015 · 4 comments
Closed

Vulnerability in the JWT token validation #564

phindmarsh opened this issue Apr 15, 2015 · 4 comments

Comments

@phindmarsh
Copy link
Contributor

Recently a 'critical vulnerability' with JWT was disclosed by Auth0.

It essentially boils down to modifying the algorithm header value and tricking a server using rsa signing to use the public key has an hmac secret instead, allowing an attacker who knows the servers public key to craft arbitrary tokens and have auth bypass. See the summary on the Auth0 blog.

The solution to this is to pass the allowed algorithms when decoding a user-specified token, rather than trusting the value in the token header. This is usually fine since the server validating tokens should know what methods have been used to generate them.

The php-jwt lib has applied a patch for this which likely needs to be ported into the embedded JWT class of this library (see commits firebase/php-jwt@10918f2 and firebase/php-jwt@b2c2be6). Alternatively, this lib could depend on firebase/php-jwt directly.

What are your thoughts on either updating the embedded lib, or depending on the php-jwt lib instead? I'm happy to submit a PR either way.

@phindmarsh
Copy link
Contributor Author

/cc @bshaffer I'd be keen to address this if you have an opinion.

@bshaffer
Copy link
Owner

Hey @phindmarsh! I do not take this lightly, and am sorry for not responding earlier. This is a critical change, and if you are truly willing to submit a PR, I would love to go with whichever option you find more appropriate.

Patching the library itself would be easier, as bringing in firebase/php-jwt will require updating the documentation (because we would want it to be an optional dependency) and making other core updates. But, as you are more familiar with the problem itself, what is your preference?

@phindmarsh
Copy link
Contributor Author

No worries, I'm happy to push a PR for this today.

I agree about patching the library rather than pulling in firebase/php-jwt, although I do wonder if that is something to be considered for a later time (if only because that lib gets more eyes from a security context). Our OAuth server implementation uses it instead of the built in one at the recommendation of a security audit.

I'll get a PR together applying a fix for the internal JWT class, and we can move the migration to firebase/php-jwt to another discussion.

@bshaffer
Copy link
Owner

Great! Yes, I agree it is ideal to use firebase's jwt library at some
point.
On Tue, Apr 21, 2015 at 3:11 PM Patrick Hindmarsh notifications@github.com
wrote:

No worries, I'm happy to push a PR for this today.

I agree about patching the library rather than pulling in firebase/php-jwt,
although I do wonder if that is something to be considered for a later time
(if only because that lib gets more eyes from a security context). Our
OAuth server implementation uses it instead of the built in one at the
recommendation of a security audit.

I'll get a PR together applying a fix for the internal JWT class, and we
can move the migration to firebase/php-jwt to another discussion.


Reply to this email directly or view it on GitHub
#564 (comment)
.

phindmarsh added a commit to phindmarsh/oauth2-server-php that referenced this issue Apr 21, 2015
JWT::decode() now takes an array of $allowed_algorithms to prevent tricking the server into decoding using a different algorithm than intended.

Addresses bshaffer#564
phindmarsh added a commit to phindmarsh/oauth2-server-php that referenced this issue Apr 22, 2015
JWT::decode() now takes an array of $allowed_algorithms to prevent tricking the server into decoding using a different algorithm than intended.

Addresses bshaffer#564
phindmarsh added a commit to phindmarsh/oauth2-server-php that referenced this issue Apr 22, 2015
JWT::decode() now takes an array of $allowed_algorithms to prevent tricking the server into decoding using a different algorithm than intended.

Addresses bshaffer#564
phindmarsh added a commit to phindmarsh/oauth2-server-php that referenced this issue Apr 22, 2015
JWT::decode() now takes an array of $allowed_algorithms to prevent tricking the server into decoding using a different algorithm than intended.

Addresses bshaffer#564
bshaffer pushed a commit that referenced this issue Apr 22, 2015
JWT::decode() now takes an array of $allowed_algorithms to prevent tricking the server into decoding using a different algorithm than intended.

Addresses #564
bshaffer pushed a commit that referenced this issue Apr 22, 2015
JWT::decode() now takes an array of $allowed_algorithms to prevent tricking the server into decoding using a different algorithm than intended.

Addresses #564
bshaffer pushed a commit that referenced this issue Apr 22, 2015
JWT::decode() now takes an array of $allowed_algorithms to prevent tricking the server into decoding using a different algorithm than intended.

Addresses #564
phindmarsh added a commit to phindmarsh/oauth2-server-php that referenced this issue Apr 22, 2015
JWT::decode() now takes an array of $allowed_algorithms to prevent tricking the server into decoding using a different algorithm than intended.

Addresses bshaffer#564
phindmarsh added a commit to phindmarsh/oauth2-server-php that referenced this issue Apr 22, 2015
An alternate patch that maintains backwards compatibility with existing interfaces, but is unsafe for users who call JWT::decode themselves without passing the allowed arguments array.
phindmarsh added a commit to phindmarsh/oauth2-server-php that referenced this issue Apr 22, 2015
An alternate patch that maintains backwards compatibility with existing interfaces, but is unsafe for users who call JWT::decode themselves without passing the allowed arguments array.
phindmarsh added a commit to phindmarsh/oauth2-server-php that referenced this issue Apr 23, 2015
An alternate patch that maintains backwards compatibility with existing interfaces, but is unsafe for users who call JWT::decode themselves without passing the allowed arguments array.
@bshaffer bshaffer closed this as completed May 4, 2015
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