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

jwks_uri shouldn't be a requirement for checking signatures #182

Closed
xushangning opened this issue Oct 9, 2019 · 6 comments · Fixed by #373
Closed

jwks_uri shouldn't be a requirement for checking signatures #182

xushangning opened this issue Oct 9, 2019 · 6 comments · Fixed by #373

Comments

@xushangning
Copy link

Since I receive no responses from my previous PR #150, here I describe the issues with jwks_uri in the code to see if anybody is interested.

So basically, JWT supports two signing algorithms for signatures: RS256/384/512 and HS256/384/512, among many others. As RS256 (RSA Signature with SHA-256) is asymmetric, every time you verify the signature, you have to fetch the server's public keys from the location pointed to by jwks_uri. That's why jwks_uri is considered necessary in #72.

But the situation is entirely different for HS256 (HMAC with SHA-256), a symmetric algorithm, so it doesn't have public/private key pairs. The signature can be checked when supplied with a secret. Most of the time, the secret is implemented as the client secret. In this case, the server doesn't need to supply a set of public keys through jwks_uri.

The code below will check for the existence of jwks_uri no matter which type of signatures is in use:

// Verify the signature
if ($this->canVerifySignatures()) {
if (!$this->getProviderConfigValue('jwks_uri')) {
throw new OpenIDConnectClientException ('Unable to verify signature due to no jwks_uri being defined');
}

Also, in verifyJWTsignature, jwks_uri is a hard requirement:

$payload = implode('.', $parts);
$jwks = json_decode($this->fetchURL($this->getProviderConfigValue('jwks_uri')));
if ($jwks === NULL) {
throw new OpenIDConnectClientException('Error decoding JSON from jwks_uri');
}

I can write a PR like #150 again for master. Sorry for the mention @jumbojett :)

Reference: Signing Algorithms

@Hesesses
Copy link

I'm having the same problem, any ideas how to fix this?

@telemmaite
Copy link

telemmaite commented Jan 11, 2021

This library is made to conform to OpenID connect standard, not just plain exchange/verification of jwt tokens.
jwks_uri is REQUIRED in the OpenID Provider Metadata https://openid.net/specs/openid-connect-discovery-1_0.html.

So if your OpeinID provider does not have jwks_uri then it is misconfigured, or maybe you don`t need this library at all but just a JWT sign/verify library as https://github.com/firebase/php-jwt

@xushangning
Copy link
Author

This library is made to conform to OpenID connect standard, not just plain exchange/verification of jwt tokens.
jwks_uri is REQUIRED in the OpenID Provider Metadata https://openid.net/specs/openid-connect-discovery-1_0.html.

The two claims are both correct, but they don't lead to your conclusion, because the OIDC's official website states that Discovery itself is optional [1]. jwks_uri is only required for OIDC Discovery.

As always, my reading of the standard may be wrong, so feel free to raise objections.

@xushangning
Copy link
Author

I'm having the same problem, any ideas how to fix this?

You can see how I modified an older version of this library in PR #150. I have a fork at xushangning/OpenID-Connect-PHP with the PR applied, but it was also an old version and not maintained.

@DeepDiver1975
Copy link
Collaborator

#308 will solve this as a side quest

@singpolyma
Copy link

#308 almost solved this, but it still has a redundant check for jwks_uri presence in the verifySignatures method. The presence of that is checked when needed inside the verifyJWTSignature method so I think the check can just be deleted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants