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

Add support for OrderedDict to headers to make possible custom ordering #715

Closed
kadabusha opened this issue Dec 1, 2021 · 10 comments
Closed

Comments

@kadabusha
Copy link
Contributor

Headers order in jwt.encode() follows values hardcoded in api_jws.py:

        header = {'typ': self.header_typ, 'alg': algorithm}

and can not be changed.
An issue referenced to old #116 - new RFC 8225 has pretty strict requirements for headers - they should be ordered lexicographically (A-Z)

The signature of the PASSporT is created as specified by JWS
   ([RFC7515], Section 5.1, Steps 1 through 6).  PASSporT MUST use the
   JWS Protected Header.  For the JWS Payload and the JWS Protected
   Header, however, the lexicographic ordering and whitespace rules
   described in Sections 4 and 5 of this document, and the JSON
   serialization rules in Section 9 of this document, MUST be followed.

Expected Result

Values for headers should not override the configured ones

{'alg': 'ES256', 'ppt': 'shaken', 'typ': 'passport', 'url': 'example.com'}

Actual Result

No matter what order is used for headers, jwt.encode() always uses hardcoded typ/alg key names on the first places.

{'typ': 'passport', 'alg': 'ES256', 'ppt': 'shaken', 'url': 'example.com'}

Reproduction Steps

$ openssl req -new -x509 -nodes -newkey ec:<(openssl ecparam -name secp384r1) -keyout key.pem -out cert.crt -days 3650 -subj "/C=US/ST=Pennsylvania/L=Philadelphia/O=Example CA/CN=SHAKEN"
$ python
>>> import jwt
>>> from collections import OrderedDict
>>> key = open('domain.key').read()
>>> payload = {'attest': "my test1", 'dest': "3333", 'iat': "423dfd", 'orig': "321", 'origid': "123"}
>>> header = {'alg': 'ES256', 'ppt': 'shaken', 'typ': 'passport', 'url': 'example.com'}
>>> header_ordered = OrderedDict()
>>> header_ordered['alg'] = 'ES256'
>>> header_ordered['ppt'] = 'shaken'
>>> header_ordered['typ'] = 'passport'
>>> header_ordered['url'] = 'example.com'
>>> jwt.get_unverified_header(jwt.encode(payload, key, algorithm="ES256", headers=header))
{'typ': 'passport', 'alg': 'ES256', 'ppt': 'shaken', 'url': 'example.com'}
>>> jwt.get_unverified_header(jwt.encode(payload, key, algorithm="ES256", headers=header_ordered))
{'typ': 'passport', 'alg': 'ES256', 'ppt': 'shaken', 'url': 'example.com'}

System Information

$ python -m jwt.help
{
  "cryptography": {
    "version": "2.9.2"
  },
  "implementation": {
    "name": "CPython",
    "version": "3.8.12"
  },
  "platform": {
    "release": "11.3-RELEASE-p6",
    "system": "FreeBSD"
  },
  "pyjwt": {
    "version": "1.7.1"
  }
}
@auvipy
Copy link
Collaborator

auvipy commented Dec 12, 2021

please upgrade to latest pypi release & report again

@auvipy auvipy closed this as completed Dec 12, 2021
@kadabusha
Copy link
Contributor Author

Hi, @auvipy thanks for looking into this.
In order to confirm the issue with latest pypi release I used another test box with Debian 11 (bullseye), where installed latest python, cryptography and pyjwt. The misorder seems to be present there as well, please see below.

$ python -m jwt.help
{
  "cryptography": {
    "version": "36.0.0"
  },
  "implementation": {
    "name": "CPython",
    "version": "3.9.2"
  },
  "platform": {
    "release": "5.10.0-9-amd64",
    "system": "Linux"
  },
  "pyjwt": {
    "version": "2.3.0"
  }
}

$ python
Python 3.9.2 (default, Feb 28 2021, 17:03:44)
[GCC 10.2.1 20210110] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import jwt
>>> from collections import OrderedDict
>>> key = open('key.pem').read()
>>> payload = {'attest': "my test1", 'dest': "3333", 'iat': "423dfd", 'orig': "321", 'origid': "123"}
>>> header = {'alg': 'ES256', 'ppt': 'shaken', 'typ': 'passport', 'url': 'example.com'}
>>> header_ordered = OrderedDict()
>>> header_ordered['alg'] = 'ES256'
>>> header_ordered['ppt'] = 'shaken'
>>> header_ordered['typ'] = 'passport'
>>> header_ordered['url'] = 'example.com'
>>> jwt.get_unverified_header(jwt.encode(payload, key, algorithm="ES256", headers=header))
{'typ': 'passport', 'alg': 'ES256', 'ppt': 'shaken', 'url': 'example.com'}
>>> jwt.get_unverified_header(jwt.encode(payload, key, algorithm="ES256", headers=header_ordered))
{'typ': 'passport', 'alg': 'ES256', 'ppt': 'shaken', 'url': 'example.com'}

@auvipy auvipy reopened this Dec 13, 2021
@jpadilla
Copy link
Owner

I have no objections on introducing this to support RFC 8225. @kadabusha wanna work on it?

@kadabusha
Copy link
Contributor Author

Hi, @jpadilla
I'll check if I can workaround that - for my own test case I just removed typ from api_jws.py and passed it later in my code.
In case I'm able to get working code, I'll file a PR referencing this issue.

kadabusha added a commit to kadabusha/pyjwt that referenced this issue Dec 29, 2021
@kadabusha
Copy link
Contributor Author

@jpadilla the PR was filed here: #721
Seems to be as simple as add sort for keys on json.dumps()

@sabrina981 I don't quite follow your question.
RFC8225 is a standard that was written on top of JWT ones:

 "JSON Web Token (JWT)" [RFC7519], "JSON Web Signature (JWS)"
   [RFC7515], and other related specifications define a standard token
   format that can be used as a way of encapsulating claimed or asserted
   information with an associated digital signature using X.509-based
   certificates.

It is needed for particular implementations, e.g. one that is called Stir-Shaken in SIP world.
Even though the sorting is not mandated by JWT specification, it IS mandatory in the standard of 8225 implementation.
Hope this answers your question.

@github-actions
Copy link

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 7 days

@github-actions github-actions bot added the stale Issues without activity for more than 60 days label May 16, 2022
@kadabusha
Copy link
Contributor Author

An issue was still valid, PR waits for @jpadilla to review I assume:
#721

@github-actions github-actions bot removed the stale Issues without activity for more than 60 days label May 17, 2022
@github-actions
Copy link

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 7 days

@github-actions github-actions bot added the stale Issues without activity for more than 60 days label Jul 17, 2022
@kadabusha
Copy link
Contributor Author

Still valid.

@jpadilla jpadilla removed the stale Issues without activity for more than 60 days label Jul 19, 2022
auvipy pushed a commit that referenced this issue Jul 19, 2022
* Fix for headers disorder issue

Related issue #715

* Added comment with reference to issue

 Needed to trigger tests once more time.

* Fix for hardcoded value in docs after adding sort to jwt/api_jws.py

* Removed unneeded comment - issue #721
@kadabusha
Copy link
Contributor Author

PR was merged:
#721

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

4 participants
@jpadilla @auvipy @kadabusha and others