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

"Incorrect padding" errors in some IDAs with version 8.8.0 #346

Closed
15 tasks done
timmc-edx opened this issue May 18, 2023 · 2 comments
Closed
15 tasks done

"Incorrect padding" errors in some IDAs with version 8.8.0 #346

timmc-edx opened this issue May 18, 2023 · 2 comments
Assignees

Comments

@timmc-edx
Copy link
Contributor

timmc-edx commented May 18, 2023

This is showing up in enterprise-subsidy after the upgrade to 8.8.0:

_verify_jwt_signature(token, jwt_issuer, decode_symmetric_token=decode_symmetric_token)
File "/edx/venvs/enterprise-subsidy/lib/python3.8/site-packages/edx_rest_framework_extensions/auth/jwt/decoder.py", line 197, in _verify_jwt_signature
key_set = _get_signing_jwk_key_set(jwt_issuer, add_symmetric_keys=False)
File "/edx/venvs/enterprise-subsidy/lib/python3.8/site-packages/edx_rest_framework_extensions/auth/jwt/decoder.py", line 328, in _get_signing_jwk_key_set
key_set.extend(PyJWKSet.from_json(signing_jwk_set).keys)
File "/edx/venvs/enterprise-subsidy/lib/python3.8/site-packages/jwt/api_jwk.py", line 114, in from_json
return PyJWKSet.from_dict(obj)
File "/edx/venvs/enterprise-subsidy/lib/python3.8/site-packages/jwt/api_jwk.py", line 109, in from_dict
return PyJWKSet(keys)
File "/edx/venvs/enterprise-subsidy/lib/python3.8/site-packages/jwt/api_jwk.py", line 96, in __init__
self.keys.append(PyJWK(key))
File "/edx/venvs/enterprise-subsidy/lib/python3.8/site-packages/jwt/api_jwk.py", line 60, in __init__
self.key = self.Algorithm.from_jwk(self._jwk_data)
File "/edx/venvs/enterprise-subsidy/lib/python3.8/site-packages/jwt/algorithms.py", line 473, in from_jwk
from_base64url_uint(obj["n"]),
File "/edx/venvs/enterprise-subsidy/lib/python3.8/site-packages/jwt/utils.py", line 53, in from_base64url_uint
data = base64url_decode(force_bytes(val))
File "/edx/venvs/enterprise-subsidy/lib/python3.8/site-packages/jwt/utils.py", line 33, in base64url_decode
return base64.urlsafe_b64decode(input_bytes)
File "/usr/lib/python3.8/base64.py", line 133, in urlsafe_b64decode
return b64decode(s)
File "/usr/lib/python3.8/base64.py", line 87, in b64decode
return binascii.a2b_base64(s)binascii.Error: Incorrect padding

A/C:

@timmc-edx
Copy link
Contributor Author

timmc-edx commented May 18, 2023

The issue seems to occur when PyJWTSet tries to load a modulus value (the n in the JSON) which is Base64-encoded but lacks padding chars (=). pyjwkest does not have this problem.

Here's what I think is going on:

  • Python's base64.urlsafe_b64_decode requires padding chars, even though the spec seems to indicate that padding chars should be optional for urlsafe mode.
  • pyjwkest was blindly adding a few padding characters onto the Base64 before decoding it in order to work around this: https://github.com/IdentityPython/pyjwkest/blob/880fe789ff74a6b927151ddba6e9796fe88a7ce1/src/jwkest/__init__.py#L123
  • PyJWT instead uses the length of the string to decide how many padding chars to add before passing the base64 to Python's stdlib: https://github.com/jpadilla/pyjwt/blob/2.6.0/jwt/utils.py#L25
  • Some of the keysets in our config files have hard-wrapped the modulus value, inside a YAML single-quoted string. YAML converts single newlines in a single-quoted string into spaces, so that the n value in the JSON contains spaces.
  • base64.urlsafe_b64decode is ignoring the spaces, but when PyJWT goes to count the chars to decide how much padding to add, it is including the spaces. So it ends up putting the wrong amount of padding on.

Conclusions:

  • We should fix our config files
  • We should mention this in the release notes of Quince (and Palm?), as well as of edx-drf-extensions 8.8.0
  • Possibly, we should file an issue with PyJWT to do the padding differently

@timmc-edx
Copy link
Contributor Author

timmc-edx commented May 23, 2023

Declining to file an issue with PyJWT. From looking at specs, I think we can regard these few 2U configs as just corrupted, and that they had only been working by accident. At best, PyJWT might throw a different error, but I'm not sure it's worth it.

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

1 participant