IdentityPython / JWTConnect-Python-CryptoJWT

An implementation of RFC 7515-18 using Cryptography
Apache License 2.0
13 stars 16 forks source link

Missing "x5c" key for serialized ECKey and others. #165

Open sallner opened 3 months ago

sallner commented 3 months ago

Hello and thanks for the good work on this library.

While I was using it I came across one inconsistency with respect to handling x5c certificates on various asymmetric keys. It looks that in principle https://github.com/IdentityPython/JWTConnect-Python-CryptoJWT/blob/69b064b776bbedeef8e20f5c499a0c60ed483015/src/cryptojwt/jwk/asym.py#L24 will store the x5c on the key instance but only for RSAKeys this is also included in serialize(). https://github.com/IdentityPython/JWTConnect-Python-CryptoJWT/blob/69b064b776bbedeef8e20f5c499a0c60ed483015/src/cryptojwt/jwk/rsa.py#L384-L385

Is this intentional? In principal I see no problem, that also other asymmetric keys could have a x5c certificate attached to it, in particular the ECKey. Would you consider this a reasonable change?

jschlyter commented 3 months ago

I believe it is reasonable to include x5c for all keys - please submit a PR (with tests)

sallner commented 3 months ago

Thanks for the positive feedback. I will try to sort out the CLA with my employer and come back to this ASAP.