Conversation
Before this change, parallel mypy processes implicitly shared a single cache directory, while mypy assumes its cache dir is exclusive to the running process and Python and mypy version, etc. This causes random crashes due to cache corruption when running ‘tox run-parallel’. Use separate cache dirs to avoid this problem.
A few tests were accidentally part of ‘TestOKPAlgorithms’ even though they have nothing to do with OKP algorithms. They are actually tests for RSA and HMAC, so name them as such.
|
note that i've included 2 preparatory commits. if maintainers prefer, i can spin those into separate pull requests. just let me know! |
This adds support for ‘Ed25519’ and ‘Ed448’ fully-specified algorithms as specified in RFC 9864 <https://datatracker.ietf.org/doc/html/rfc9864>, ‘Fully-Specified Algorithms for JSON Object Signing and Encryption (JOSE) and CBOR Object Signing and Encryption (COSE)’. Technically these are the same as the already supported ‘EdDSA’ algorithm, but with the used curve made explicit. When a fully-specified algorithm value is used, the actual key is validated against the expected curve type. The original polymorphic ‘EdDSA’ identifier (which doesn't specify the exact curve but is otherwise the same) keeps working as before. Updated docs accordingly, and organised the overview of supported algorithms per ‘family’ to make it easier to read. Implementation notes: - OKPAlgorithm now takes an optional ‘expected_curve’ argument, similar to ECAlgorithm. - OKPAlgorithm.prepare_key() now checks the expected curve. This method now uses a single code path for validation, and curve validation is just a few if/else statements, without a separate helper (cf. ECAlgorithm._validate_curve() for EC keys which has a more complex control flow). - The PyJWK constructor only handled ‘crv: Ed25519’ but not ‘crv: Ed448’. This seems to be an oversight from when Ed448 was added, because before that only Ed25519 was supported. Now it handles both. - Example EdDSA keys for testing now include the ‘alg’ parameter, as recommended by RFC 9864 §7 (Security Considerations). Tests using these fixtures cover variants such as correct, incorrect, and missing ‘alg’ values. Closes jpadilla#1190.
8199ee7 to
3d3e878
Compare
|
LGTM. |
jpadilla
left a comment
jpadilla
left a comment
There was a problem hiding this comment.
Security classification: security-boundary-affected. The algorithm-choice, OKP key-material, and asymmetric-JWK paths were reviewed. One compatibility issue needs correction before approval.
| if not crv: | ||
| raise InvalidKeyError(f"crv is not found: {self._jwk_data}") | ||
| if crv == "Ed25519": | ||
| if crv in ("Ed25519", "Ed448"): |
There was a problem hiding this comment.
[P2] Support fully specified tokens with algorithm-less OKP JWKs
When alg is omitted, this branch binds every OKP key to EdDSA. The later exact PyJWK binding then rejects a valid Ed25519 or Ed448 token before signature verification, even though RFC 9864 section 5 permits the JWK alg member to be omitted. I reproduced this for both curves through PyJWK; the same key-selection path is used by PyJWKClient. Please distinguish an explicit JWK algorithm constraint from an inferred default so an algorithm-less key can verify the caller-allowed, curve-compatible fully specified identifier while retaining legacy EdDSA support. Add PyJWK/PyJWKClient controls for both curves and both legacy and fully specified identifiers.
There was a problem hiding this comment.
Minimal repro for both Ed25519 and Ed448:
- Load a matching OKP public JWK without its optional
algmember. - Construct it with
PyJWK.from_dict(). - Verify a valid token whose header uses the curve's fully specified algorithm.
Expected: verification succeeds because the curve matches and the caller explicitly allowed that algorithm.
Actual:
Ed25519 bound-as EdDSA
InvalidAlgorithmError: Token algorithm 'Ed25519' does not match the key's algorithm 'EdDSA'
Ed448 bound-as EdDSA
InvalidAlgorithmError: Token algorithm 'Ed448' does not match the key's algorithm 'EdDSA'
Matching controls succeed when the JWK includes alg, when PyJWK receives an explicit algorithm override, or when the token uses legacy EdDSA. RFC 9864 section 5 permits the JWK alg member to be omitted.
This branch has not been deployed
3 participants
This adds support for ‘Ed25519’ and ‘Ed448’ fully-specified algorithms
as specified in RFC 9864 https://datatracker.ietf.org/doc/html/rfc9864,
‘Fully-Specified Algorithms for JSON Object Signing and
Encryption (JOSE) and CBOR Object Signing and Encryption (COSE)’.
Technically these are the same as the already supported ‘EdDSA’
algorithm, but with the used curve made explicit. When a fully-specified
algorithm value is used, the actual key is validated against the
expected curve type.
The original polymorphic ‘EdDSA’ identifier (which doesn't specify the
exact curve but is otherwise the same) keeps working as before.
Updated docs accordingly, and organised the overview of supported
algorithms per ‘family’ to make it easier to read.
Implementation notes:
OKPAlgorithm now takes an optional ‘expected_curve’ argument, similar
to ECAlgorithm.
OKPAlgorithm.prepare_key() now checks the expected curve. This method
now uses a single code path for validation, and curve validation is
just a few if/else statements, without a separate helper (cf.
ECAlgorithm._validate_curve() for EC keys which has a more complex
control flow).
The PyJWK constructor only handled ‘crv: Ed25519’ but not ‘crv:
Ed448’. This seems to be an oversight from when Ed448 was added,
because before that only Ed25519 was supported. Now it handles both.
Example EdDSA keys for testing now include the ‘alg’ parameter, as
recommended by RFC 9864 §7 (Security Considerations). Tests using
these fixtures cover variants such as correct, incorrect, and missing
‘alg’ values.
Closes #1190.