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

Malformed JWTs due to different signing algorithms in headers and algorithm parameters #659

Closed
ashutosh1206 opened this issue May 16, 2021 · 3 comments · Fixed by #673
Closed

Comments

@ashutosh1206
Copy link

Generating JWTs (encode()) with alg key set in headers parameter along with a different algorithm parameter value results in malformed JWTs.

Example headers and algorithm values that will result in malformed tokens:

headers = {"typ": "JWT", "alg": "none"}
algorithm = "HS256"

Expected Result

A token signed with let's say HS256 algorithm should always contain alg value set as HS256 in the JWT header.

Actual Result

Tokens can be signed using an algorithm x (eg. HS256) with token header containing a different alg value

Reproduction Steps

import jwt
payload = {"test": "payload"}
headers = {"typ": "JWT", "alg": "none"}
print(jwt.encode(payload = payload, key = "secret", algorithm = "HS256", headers = headers))

Running the snippet above will give: eyJ0eXAiOiJKV1QiLCJhbGciOiJub25lIn0.eyJ0ZXN0IjoicGF5bG9hZCJ9.PySJLJ2Js8Z1K0acpZrgOhzHp0sea_N5rrNX1L_FJis. Header + payload in this case is signed using HS256, but the header value in the token is: {"typ":"JWT","alg":"none"}

Similarly,

import jwt
payload = {"test": "payload"}
headers = {"typ": "JWT", "alg": "RS256"}
jwt.encode(payload = payload, key = "secret", algorithm = "HS256", headers = headers)

will result in a token signed using HS256, with the token header being {"typ":"JWT","alg":"RS256"}

System Information

$ python -m jwt.help
{
  "cryptography": {
    "version": "3.4.7"
  },
  "implementation": {
    "name": "CPython",
    "version": "3.8.8"
  },
  "platform": {
    "release": "5.8.0-53-generic",
    "system": "Linux"
  },
  "pyjwt": {
    "version": "2.1.0"
  }
}

Root cause

algorithm (L#109 api_jws.py) variable is being used for signing, but header dict (L#93 api_jws.py) is being overwritten with headers method parameter in L#97 api_jws.py leading to alg value being overwritten to the value present in the method parameter headers.

I tried coming up with ways in which this bug could be abused by an adversary, but couldn't find any. And so, I don't think this qualifies as a security issue.

@De117
Copy link

De117 commented Jul 21, 2021

I just bumped into a related (usability) issue: using a non-default algorithm with a custom header. (I was doing OIDC asymmetric signatures.)

The algorithm parameter has a default, so you have to specify it twice:

key = ...
header = {"typ": "JWT", "alg": "PS256", "kid": "my-key-id"}
claims = {"foo": "bar"}

jws = jwt.encode(payload=claims, key=key, algorithm=header["alg"], headers=header)

Which wouldn't be needed if headers["alg"] is used in preference to "HS256":

jws = jwt.encode(payload=claims, key=key, headers=header)

It's cleaner code (for the user) and saner behaviour.
However, the parameter type would need to be changed to algorithm: Optional[str].

@dajiaji
Copy link
Contributor

dajiaji commented Jul 22, 2021

Hi, @De117 @ashutosh1206. I've sent a PR to fix this issue. Could you check it?

My PR supports the case mentioned by @De117.

key = ...
header = {"typ": "JWT", "alg": "PS256", "kid": "my-key-id"}
claims = {"foo": "bar"}

jws = jwt.encode(payload=claims, key=key, headers=header)

On the other hand, in the following @ashutosh1206 's case, alg=none will be preferred and returns error because key is set.

import jwt
payload = {"test": "payload"}
headers = {"typ": "JWT", "alg": "none"}
jwt.encode(payload = payload, key = "secret", algorithm = "HS256", headers = headers)

In any case, I think the consistent rule that headers["alg"] is preferred to algorithm is applied, making it easier to understand.

@De117
Copy link

De117 commented Jul 22, 2021

@dajiaji I had something slightly different in mind ("prefer headers["alg"] unless algorithm is explicitly given"), but your way ("always prefer headers["alg"]") is indeed simpler.

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.

3 participants