JWS signature verification broken when `algorithms` is a plain string

[vivienm]

Hello!

I'm having a small issue with functions jwt.decode and jws.verify in a corner-case scenario.

I'm playing with a handcrafted, invalid JWT where the "alg" header has been set to true (not allowed to share, sorry!):

$ echo "$JWT" | cut -d '.' -f 1 | base64 -d
{"alg":true,"typ":"JWT"}

When trying to decode or verify it, it fails with a TypeError. I'd have expected a JWSError instead:

>>> jws.verify(os.environ["JWT"], "", algorithms="HS256")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File ".../jose/jwt.py", line 142, in decode
    payload = jws.verify(token, key, algorithms, verify=verify_signature)
  File ".../jose/jws.py", line 73, in verify
    _verify_signature(signing_input, header, signature, key, algorithms)
  File ".../jose/jws.py", line 256, in _verify_signature
    if algorithms is not None and alg not in algorithms:
TypeError: 'in <string>' requires string as left operand, not bool

This happens when the argument algorithms is a string. With a list, this seems to behave normally:

>>> jws.verify(os.environ["JWT"], "", algorithms=["HS256"])
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File ".../jose/jws.py", line 73, in verify
    _verify_signature(signing_input, header, signature, key, algorithms)
  File ".../jose/jws.py", line 257, in _verify_signature
    raise JWSError("The specified alg value is not allowed")
jose.exceptions.JWSError: The specified alg value is not allowed

Still, according to docs, both strings and lists of strings are valid types for algorithms:

https://github.com/mpdavis/python-jose/blob/96474ecfb6ad3ce16f41b0814ab5126d58725e2a/jose/jws.py#L48-L55

I believe the root issue is here:

https://github.com/mpdavis/python-jose/blob/96474ecfb6ad3ce16f41b0814ab5126d58725e2a/jose/jws.py#L250-L257

algorithms may be a plain string here (I have not seen any list normalization before this code is reached). So the expression alg not in algorithms tries to assess whether a boolean (in my scenario) belongs to a string, and fails with an unexpected TypeError.

Even in a normal scenario, with alg being a string, I'm not sure this code is correct either. If algorithms is a plain string, the test will assess whether alg is a substring of algorithms. This looks wrong: its both too permissive and differs from the list behavior.

I'm not familiar with this project, but I believe a fix may look like

     if algorithms is not None:
         if isinstance(algorithms, str):
             algorithms=[algorithms]
         if alg not in algorithms:
             raise JWSError("The specified alg value is not allowed") 

[almartmart]

The point here might be that "alg" header is expecting to contain a string rather than a boolean. Same with the string value, as it is expecting a valid algorithm. Expecting a proper behaviour on a library might require a proper usage of it, so I don't think this code needs to be added. It may just be a good contribution to the Documentation.

[vivienm]

I beg to differ: JWTs are typically sent by clients, and should be considered as untrusted input. I agree the JWT is invalid, but I think a proper exception should be raised, not a generic, undocumented one.