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 #95

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

[SECURITY] Algorithm Confusion Through kid Header #95

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

Comments

@paragonie-security
Copy link

paragonie-security commented Aug 20, 2021

Your JWS implementation correctly rejects invalid algorithms.

elsif algorithms.blank? || Array(algorithms).include?(alg&.to_sym)

However, when a kid header is present, it fetches the key after this algorithm check.

json-jwt/lib/json/jws.rb

Lines 124 to 140 in a2b4c15

public_key_or_secret = with_jwk_support public_key_or_secret
case
when hmac?
secure_compare sign(signature_base_string, public_key_or_secret), signature
when rsa?
public_key = public_key_or_secret
public_key.verify digest, signature, signature_base_string
when rsa_pss?
public_key = public_key_or_secret
public_key.verify_pss digest, signature, signature_base_string, salt_length: :digest, mgf1_hash: digest
when ecdsa?
public_key = public_key_or_secret
verify_ecdsa_group! public_key
public_key.verify digest, raw_to_asn1(signature, public_key), signature_base_string
else
raise UnexpectedAlgorithm.new('Unknown Signature Algorithm')
end

def with_jwk_support(key)
case key
when JSON::JWK
key.to_key
when JSON::JWK::Set
key.detect do |jwk|
jwk[:kid] && jwk[:kid] == kid
end&.to_key or raise JWK::Set::KidNotFound
else
key
end
end

When JWKs are used, this algorithm check isn't congruently applied to the keys.

json-jwt/lib/json/jwk.rb

Lines 40 to 51 in a2b4c15

def to_key
case
when rsa?
to_rsa_key
when ec?
to_ec_key
when oct?
self[:k]
else
raise UnknownAlgorithm.new('Unknown Key Type')
end
end

Therefore, if someone initializes a JWK or JWK::Set with different algorithm types, it's possible to swap the alg header and get the wrong key for a given algorithm. In extreme cases, this can lead to a cryptographic integrity bypass (reminiscent of the HS256/RS256 issue from years ago).

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 algorithms array.

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

@nov
Copy link
Owner

nov commented Aug 20, 2021 via email

@nov nov closed this as completed Dec 12, 2021
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