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

Add withClaim validation for custom claim validation #826

Closed
james-bw opened this issue Mar 30, 2022 · 4 comments · Fixed by #827
Closed

Add withClaim validation for custom claim validation #826

james-bw opened this issue Mar 30, 2022 · 4 comments · Fixed by #827
Assignees
Milestone

Comments

@james-bw
Copy link

OWASP has a list of things to improve security in JWT. One of those things regarding token sidejacking is validating custom claims against a random fingerprint. It would be great if this library provided a way to compare custom claims and values and validate the claim exists and the value is correct.

@Ocramius
Copy link
Collaborator

This is possible by writing custom Constraint implementations?

<?php
declare(strict_types=1);
namespace Lcobucci\JWT\Validation;
use Lcobucci\JWT\Token;
interface Constraint
{
/** @throws ConstraintViolation */
public function assert(Token $token): void;
}

In addition to that:

One of those things regarding token sidejacking is validating custom claims against a random fingerprint.

This is literally what a cryptographic signature does:

  1. a CSPRNG-based random fingerprint is generated
  2. then hashed/signed with a public key or a secret
  3. the receiving party validates the hash/signature with a public key or a secret

The whole linked chapter seems very weak, in this regard :|

@SvenRtbg
Copy link
Collaborator

I don't really like that OWASP page, too. It is assuming that tokens are used in a browser context, so that it is possible to add a cookie that has a string, and then make the token contain the hash of the string.

What about not allowing the token to be intercepted in the first place? This whole chapter sounds like it is trying to fix a situation that already went downhill, i.e. there is no reason to believe if an attacker is able to intercept the token, it's impossible to intercept any other cookie as well.

@james-bw
Copy link
Author

james-bw commented Apr 1, 2022

What about not allowing the token to be intercepted in the first place?

From a security perspective you have to mitigate risks. It's impossible to force clients (e.g. browsers) and server developers to never possibly use HTTP, which will always be exposed to man-in-the-middle attacks.

there is no reason to believe if an attacker is able to intercept the token, it's impossible to intercept any other cookie as well.

True, but it increases the complexity of the JWT defence, and therefore reduces the number of script kiddies who can execute an attack. Additionally, alternate options for fingerprinting (browser local storage, validating against an internal cache/redis lookup) can be used with this PR. I think addition of the validation of a claim is of value to the library whether or not the OWASP example is followed. The OWASP example is, I think, trying to add the concept of CSRF tokens to JWTs, which is a well-known security pattern.

@lcobucci
Copy link
Owner

lcobucci commented Apr 1, 2022

it increases the complexity of the JWT defence

That honestly sounds a bit like "security by obfuscation". However, I still believe the constraint is useful =)

@lcobucci lcobucci linked a pull request Aug 17, 2022 that will close this issue
@lcobucci lcobucci added this to the 4.2.0 milestone Aug 17, 2022
@lcobucci lcobucci self-assigned this Aug 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants