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

Security Vulnerability - Server Cache allows bypass of JWT Validation #399

Closed
ruiengana opened this issue Aug 19, 2021 · 13 comments · Fixed by #400
Closed

Security Vulnerability - Server Cache allows bypass of JWT Validation #399

ruiengana opened this issue Aug 19, 2021 · 13 comments · Fixed by #400

Comments

@ruiengana
Copy link

ruiengana commented Aug 19, 2021

When using JWT Validation, after a JWT is validated against certain Options (Issuer, Discovery, Audience) is get stored in plugin Introspection cache.

After token is in cache, you can use token in any API location configured with other Options (different Issuer, Discovery, Audience) that plugin will simply fetch cache hit and assume Options are still same.

Environment
  • lua-resty-openidc version (1.7.4)
  • OpenID Connect provider (Okta)
Expected behaviour

Imagine a JWT issued by A, with audience 1

And the following locations;

  • /api1 with configuration options to validate Bearer JWT from Issuer A and audience 1
  • /api2 with configuration options to validate Bearer JWT from Issuer B and audience 2

JWT should be able to verify with configuration on /api1 but fail verification with configuration from /api2.
This requires cache to be completely removed from openidc.jwt_verify()

Actual behaviour

Imagine a JWT issued by A, with audience 1

And the following locations;

  • /api1 setup to validate JWT from Issuer A with audience 1
  • /api2 setup to validate JWT from Issuer B with audience 2

If JWT is first used in /api1, because configuration is correct (Issuer, Audience), JWT result will be stored in openidc cache.
When using JWT in /api2, openidc will see a cache hit and directly validate JWT without even checking configuration options for /api2, this case Issuer B and audience 2 which doesn't match the JWT.

@ruiengana
Copy link
Author

One possible workaround is to change function that generates cache key. For example, change to be a function that receives token and options and hash everything together as use that as cache key. This way, cache would always be directly
linked to a context built up from token and configuration options.

@bodewig @hanikesn

@bodewig
Copy link
Collaborator

bodewig commented Aug 21, 2021

creating a hash for a table is a bit more complicated in Lua than one would hope for, but yes, this is the direction I've been thinking of myself. I haven't thought this through fully, but it might be possible to identify a proper subset of attributes that have to be the same for tokens to be valid for different opts tables - and the we could concatenate only these options and hash the result. This would be a fix, not a workaround :-)

A workaround is to disable the introspection cache.

@ruiengana
Copy link
Author

ruiengana commented Aug 22, 2021

@bodewig thank you for quick reply

Yes, introspection cache can be disabled and that would resolve Introspection uses cases, but for Bearer JWT Verification algorithm uses introspection cache independently of cache disable flag, meaning no option to workaround it (we had to start introspecting our JWTs with cache disabled)

Not sure if for Bearer JWT Verification use case it would be best to just remove cache completely.

Regarding configuration options to use in cache key I see only iss, aud and jti.

@ruiengana
Copy link
Author

ruiengana commented Sep 6, 2021

@bodewig any option to release a fix to completely remove cache in Bearer JWT Verification? This would make JWT Verification usable again.

@bodewig
Copy link
Collaborator

bodewig commented Sep 6, 2021

You control the cache via

lua_shared_dict introspection 10m;

or something similar in your nginx.conf. If you completely remove this configuration, the cache should be disabled (I haven't tested that, mind you).

@bodewig
Copy link
Collaborator

bodewig commented Sep 19, 2021

I completely overlooked opts.introspection_cache_ignore that completely disables the cache. You can use that selectively for specific locations.

@bodewig
Copy link
Collaborator

bodewig commented Sep 19, 2021

and then I realized neither jwt_verify nor bearer_jwt_verify honored that setting - fixed that part in master.

@bodewig
Copy link
Collaborator

bodewig commented Sep 19, 2021

@ruiengana I was trying to improve the actual caching problem and the first thing I realized was that (bearer_)jwt_verify and introspect share the same cache, which is a bit strange. Most likely one will not use introspect on JWTs, but you could and in that case you really want to invoke the introspection endpoint and not geth back the JWT stored by jwt_verify. This is pretty easy to fix.

The next point is what kind of validation jwt_verify actually performs. It verifies the signature - so we need to take the configuration for expected algorithms and the none alg into account - parses the JWT and then runs the verifier you use as last arguments of the function. This probably is where you verify the audience and other things. Rather than trying to come up with a clever way to derive a cache key from the validators, it is probably easier to run the validators on the cached result - if there is one.

@bodewig
Copy link
Collaborator

bodewig commented Sep 19, 2021

it is probably easier to run the validators on the cached result

actually it is not, at least not without verifying the cryptographic signature as well, as lua-resty-jwt doesn't allow to separate the two functions. And if we re-verify the signature, we can simply omit the cache in the first place. I'll need to rethink what is the best approach here.

@ruiengana
Copy link
Author

@ruiengana I was trying to improve the actual caching problem and the first thing I realized was that (bearer_)jwt_verify and introspect share the same cache, which is a bit strange. Most likely one will not use introspect on JWTs, but you could and in that case you really want to invoke the introspection endpoint and not geth back the JWT stored by jwt_verify. This is pretty easy to fix.

The next point is what kind of validation jwt_verify actually performs. It verifies the signature - so we need to take the configuration for expected algorithms and the none alg into account - parses the JWT and then runs the verifier you use as last arguments of the function. This probably is where you verify the audience and other things. Rather than trying to come up with a clever way to derive a cache key from the validators, it is probably easier to run the validators on the cached result - if there is one.

We had to start introspecting JWT tokens because JWT verify didn't had option to disable cache usage. With your latest commit we can remove that workaround and go back to use JWT Verify on JWTs (still keep using disable introspection cache option).

It is quite strange that cache structure is shared between Introspection and JWT Verifier, don't understand the reason behind it since they are quite different in nature.

@ruiengana
Copy link
Author

Regarding JWT algorithms and JWT Bearer validation there is a good article that explains the risks of None algorithm and current JWT libraries implementations.

See https://auth0.com/blog/critical-vulnerabilities-in-json-web-token-libraries/

In my view we need to have a allow and/or deny algorithms list options for JWT Bearer validation.

@bodewig
Copy link
Collaborator

bodewig commented Sep 19, 2021

Regarding JWT algorithms and JWT Bearer validation there is a good article that explains the risks of None algorithm

we are very much aware of this - and thus do no allow none by default. :-)

You are looking for opts.token_signing_alg_values_expected which has been available since 1.5.4.

bodewig added a commit that referenced this issue Sep 22, 2021
see #399

Signed-off-by: Stefan Bodewig <stefan.bodewig@innoq.com>
bodewig added a commit that referenced this issue Sep 22, 2021
see #399

Signed-off-by: Stefan Bodewig <stefan.bodewig@innoq.com>
bodewig added a commit that referenced this issue Sep 23, 2021
see #399

Signed-off-by: Stefan Bodewig <stefan.bodewig@innoq.com>
bodewig added a commit that referenced this issue Sep 23, 2021
see #399

Signed-off-by: Stefan Bodewig <stefan.bodewig@innoq.com>
bodewig added a commit that referenced this issue Sep 23, 2021
see #399

Signed-off-by: Stefan Bodewig <stefan.bodewig@innoq.com>
bodewig added a commit that referenced this issue Sep 23, 2021
see #399

Signed-off-by: Stefan Bodewig <stefan.bodewig@innoq.com>
@bodewig
Copy link
Collaborator

bodewig commented Sep 24, 2021

I've split the caches used for introspection and JWT verification and so the option's name has changed. Also I've introduced a new option that allows you to select cache segments. This way you can get a separate cache namespace per like-configured location.

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

Successfully merging a pull request may close this issue.

2 participants