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] Algorithm Confusion Through kid Header #440

Closed
paragonie-security opened this issue Aug 19, 2021 · 1 comment
Closed

[SECURITY] Algorithm Confusion Through kid Header #440

paragonie-security opened this issue Aug 19, 2021 · 1 comment
Assignees

Comments

@paragonie-security
Copy link

paragonie-security commented Aug 19, 2021

Initially, JWT.decode correctly rejects tokens with an alg header that isn't included in allowed_algorithms.

raise(JWT::IncorrectAlgorithm, 'Expected a different algorithm') unless options_includes_algo_in_header?

However, this check can be bypassed if multiple keys are permitted, through the kid header mechanism:

@key = find_key(&@keyfinder) if @keyfinder
@key = ::JWT::JWK::KeyFinder.new(jwks: @options[:jwks]).key_for(header['kid']) if @options[:jwks]

raise ::JWT::DecodeError, 'No key id (kid) found from token headers' unless kid
jwk = resolve_key(kid)
raise ::JWT::DecodeError, 'No keys found in jwks' if jwks_keys.empty?
raise ::JWT::DecodeError, "Could not find public key for kid #{kid}" unless jwk
::JWT::JWK.import(jwk).keypair

This is identical to the problem in firebase/php-jwt#351 https://seclists.org/fulldisclosure/2021/Aug/14

To fix this issue: Keys MUST be stored, in memory, as both the raw key bytes and the specific algorithm the key is expected to be used with. After fetching a key, this algorithm MUST be validated against the allowed_algorithms list.

Note: This particular sharp edge isn't covered by the JWT Best Practices RFC.

@anakinj anakinj self-assigned this Aug 20, 2021
@anakinj
Copy link
Member

anakinj commented Aug 20, 2021

I see the potential issue here. I think the gem is already indirectly doing a type check on the given JWKs when they are imported. So if a RSA public key is picked from the JWKs it cannot be used as the HMAC secret.

The keys are translated into the following Ruby types based on the kty property of the JWK:

EC  -> OpenSSL::PKey::EC
OCT -> String
RSA -> OpenSSL::PKey::RSA

The error handling is not pretty in this scenario but the verification will fail with some type related error.

Here is an example where we try to fool the keyfinder to fetch a RSA key for a HMAC check, this just illustrates that mixing RSA keys with HMAC verification will always fail.

hmac_jwk = JWT::JWK.new('secret')
rsa_jwk  = JWT::JWK.new(OpenSSL::PKey::RSA.new(2048))
jwks     = { keys: [hmac_jwk.export, rsa_jwk.export] }

signed_token = JWT.encode({'foo' => 'bar'}, rsa_jwk.export[:n], 'HS256', { kid: rsa_jwk.kid })
JWT.decode(signed_token, nil, true, algorithms: ['HS256'], jwks: jwks)

The decoding process will not get to the verification part because the given signing key is of the wrong type. It will eventually result in a TypeError (no implicit conversion of OpenSSL::PKey::RSA into String)

Could be something I'm missing here and would love to se a POC of a way to fool the verification.

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