latchset / jwcrypto

Implements JWK,JWS,JWE specifications using python-cryptography
GNU Lesser General Public License v3.0
438 stars 118 forks source link

Improve ergonomics or document usage to emulate python-jose/PyJWT #343

Closed Nathan-Furnal closed 8 months ago

Nathan-Furnal commented 8 months ago

There's some discussion, for example in python-keycloak,to migrate away from python-jose as it isn't maintained anymore. I think jwcrypto fits the bill but without some extra security knowledge, it's hard to make the jump.

For example, the current way in the above library, to decode a JWT is:

jwt.decode(token, key, algorithms=algorithms, audience=self.client_id, **kwargs)

The public key is provided by keycloak. If we do something similar (in the API sense, not with correctness in mind) with jwcrypto we get:

jwt.JWT(jwt=token, key=key, algs=algorithms,...)

But this won't work, in this case for example that external public key is not even a JWK object but if we try to convert it (how?) then I could only get JWSInvalidSignature or something similar.

I think it would be an improvement to document how to use jwcrypto in that case and/or provide higher level functions which take the correct steps for a potential user.

Please do tell if this example is bad or unclear.

Thanks!

simo5 commented 8 months ago

What is missing from the last example here: https://jwcrypto.readthedocs.io/en/latest/jwt.html#examples ?

Nathan-Furnal commented 8 months ago

I'm going to take the test ran in python-keycloak, more precisely this line that decodes a JWT token with jose. I put a breakpoint right before that function returns. The items you'll find are:

The result of this is when the functions is called is:

{'exp': 1708619728, 'iat': 1708619428, 'jti': '4a02802f-b928-430c-bb7a-74a72de36e79', 'iss': 'http://localhost:8081/realms/8a1b3776-f3ef-49b9-9e45-ad1979be9e73', 'aud': 'account', 'sub': '9f4ac2db-febe-466c-b2c2-0b295c71b420', 'typ': 'Bearer', 'azp': 'bb2f2292-5ac6-4296-a7b5-c0a54f0a830e', 'session_state': '0d2f82dc-94ac-4ba8-a4e5-1f05da2acc90', 'acr': '1', 'realm_access': {'roles': ['offline_access', 'default-roles-8a1b3776-f3ef-49b9-9e45-ad1979be9e73', 'uma_authorization']}, 'resource_access': {'account': {'roles': ['manage-account', 'manage-account-links', 'view-profile']}}, 'scope': 'openid profile email', 'sid': '0d2f82dc-94ac-4ba8-a4e5-1f05da2acc90', 'email_verified': False, 'preferred_username': '61f1ad91-71fe-4200-9f94-5078bbb314f1', 'email': '61f1ad91-71fe-4200-9f94-5078bbb314f1@test.test'} With jwcrypto, I do the following:

jt.JWT(jwt=token, key=key, algs=algorithms)

and I get, *** jwcrypto.jws.InvalidJWSSignature: Verification failed for all signatures["Failed: [ValueError('Unrecognized key type')]"]

I think it'd be great to point at what a valid key type could be in the error message.

Now, I need to convert the key from a string to a valid JWK that will be usable by jwt.JWT and I'm not sure how to do that.

simo5 commented 8 months ago

The last example here shows how to import a public key from a PEM file, which is waht you seem to have: https://jwcrypto.readthedocs.io/en/latest/jwk.html#examples

If you have the blob already you can just pass in the blob, you do not have to .read() from a file.

can you show what formats are the other arguments of the pyJWT jwt.decode() function?

Nathan-Furnal commented 8 months ago

N.B: This is python-jose not PyJWT but they're pretty close in usage.

Thanks, this does bring me closer. The other arguments are called like this:

jwt.decode(token, key, algorithms=algorithms, audience=self.client_id, **kwargs)

The fonction looks like this in full:

def decode(token, key, algorithms=None, options=None, audience=None, issuer=None, subject=None, access_token=None)

The options arg is a dict, the docs says:

 """Verifies a JWT string's signature and validates reserved claims.

    Args:
        token (str): A signed JWS to be verified.
        key (str or dict): A key to attempt to verify the payload with. Can be
            individual JWK or JWK set.
        algorithms (str or list): Valid algorithms that should be used to verify the JWS.
        audience (str): The intended audience of the token.  If the "aud" claim is
            included in the claim set, then the audience must be included and must equal
            the provided claim.
        issuer (str or iterable): Acceptable value(s) for the issuer of the token.
            If the "iss" claim is included in the claim set, then the issuer must be
            given and the claim in the token must be among the acceptable values.
        subject (str): The subject of the token.  If the "sub" claim is
            included in the claim set, then the subject must be included and must equal
            the provided claim.
        access_token (str): An access token string. If the "at_hash" claim is included in the
            claim set, then the access_token must be included, and it must match
            the "at_hash" claim.
        options (dict): A dictionary of options for skipping validation steps.

            defaults = {
                'verify_signature': True,
                'verify_aud': True,
                'verify_iat': True,
                'verify_exp': True,
                'verify_nbf': True,
                'verify_iss': True,
                'verify_sub': True,
                'verify_jti': True,
                'verify_at_hash': True,
                'require_aud': False,
                'require_iat': False,
                'require_exp': False,
                'require_nbf': False,
                'require_iss': False,
                'require_sub': False,
                'require_jti': False,
                'require_at_hash': False,
                'leeway': 0,
            }

    Returns:
        dict: The dict representation of the claims set, assuming the signature is valid
            and all requested data validation passes.

    Raises:
        JWTError: If the signature is invalid in any way.
        ExpiredSignatureError: If the signature has expired.
        JWTClaimsError: If any claim is invalid in any way.

EDIT: I suppose this is all things we can pass in the header?

simo5 commented 8 months ago

Sorry I was not clear, I did not ask what the docs for the function are, but what your application is actually passing as arguments in the example

In jwcrypto for example you pass the claims in a dict

Here is a link to the docs before generation broke (fixing as we speak): https://jwcrypto.readthedocs.io/en/v1.5.0/jwt.html

For example to validate a specific issuer you would pass in a check_claims dict with the issuer to check like this: { 'iss': '' }

In your case I see an audience claim so: .., check_claims={ 'aud': self.client_id }, ..

Assuming self.client_id is a properly formatted string for audience as specified in RFC 7519

simo5 commented 8 months ago

You can see more examples on how to pass claims to check in https://github.com/latchset/jwcrypto/blob/main/jwcrypto/tests.py

Nathan-Furnal commented 8 months ago

Yes indeed! The python-keycloak lib also seems to pass arguments like "verify_aud" (the ones which are put in the options arg of python-jose). Is there a place to put them in jwcrypto?

For example, verify_aud is set to False during the tests in python-keycloak and I cannot emulate this behavior.

simo5 commented 8 months ago

it depends on what they mean by that, verify_aud is not a standard attribute, if I had to guess the verify_aud is what goes in the check_claims dict as the aud claim, if not you'll have to read their documentation to understand what it is.

simo5 commented 8 months ago

ahh it seems that those verify_ = True/False are ways to enforce or skip specific claims checks

If you do not want to check 'aud' you can simply not pass it to check_claims, only exp and nbf are implicitly check, because they are dates and the library can get the current date easily, for all other claims you have to pass values to check against via check_claims, and if you pass nothing nothing is checked

Nathan-Furnal commented 8 months ago

It's what I ended up doing, I'll probably parse their options dict to add/remove fields that I pass to check_claims.

Nathan-Furnal commented 8 months ago

Last question, is there an easy way to create a private key in a single string like the PEM format? Right now I get a dict when I do key.generate_private_key() but it'd need to be a single string to not change the API.

simo5 commented 8 months ago

key.export() will give you a standard json string with both private/public part (or just the secret for a symmetric key).

So for example: jwk.JWK.generate(kty='oct', size=256).export()

simo5 commented 8 months ago

Again check out the JWK exmples in the docs