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

Switch to jwcrypto #649

Open
tpazderka opened this issue Apr 12, 2019 · 13 comments
Open

Switch to jwcrypto #649

tpazderka opened this issue Apr 12, 2019 · 13 comments
Assignees

Comments

@tpazderka
Copy link
Collaborator

tpazderka commented Apr 12, 2019

jwcrypto seems to be a good replacement for pyjwkest. It looks maintained and based on pyca/cryptography so that is a good pre-requisite for our switch there.

I think I will try to switch at least something as an example for start and have it reviewed.

@tpazderka tpazderka changed the title Switch to python-jose Switch to jwcrypto Apr 12, 2019
@tpazderka
Copy link
Collaborator Author

python-jose does not support encryption as of now, so jwcrypto it is.

@schlenk
Copy link
Collaborator

schlenk commented Apr 12, 2019

Sounds ok.

The LGPL license shouldn't be an issue, or do we have any policies against it?

@tpazderka
Copy link
Collaborator Author

No, I don't think we have any policy against a license. LGPL is fine for integrating into projects with different licenses and is FSF approved so there shouldn't be any issues I am aware of.

I will give it a try and try to convert some of the methods/classes.

@schlenk
Copy link
Collaborator

schlenk commented Apr 12, 2019

For symmetric keys, there might be a surprise lurking in jwkest. jwk.SYMKey.encryption_key()
https://github.com/rohe/pyjwkest/blob/master/src/jwkest/jwk.py#L666

The openid spec for key derivation from the client_secret is implemented in jwkest, not in oic. So you probably need to move that method over to get interoperable results.

@tpazderka
Copy link
Collaborator Author

Good catch!

@tpazderka tpazderka self-assigned this Apr 12, 2019
@tpazderka
Copy link
Collaborator Author

I have a partial implementation without key derivation. But we depend on support for none algorithm, which is still WIP.

@schlenk
Copy link
Collaborator

schlenk commented Sep 30, 2019

Key derivation should be simple enough to copy from jkwest. Its just a bunch of sha256 calls anyway.

@schlenk
Copy link
Collaborator

schlenk commented Mar 18, 2020

did you make any progress on this (or have a fork/branch i can look at)? seems jwcrypto now allows the none stuff.

@tpazderka
Copy link
Collaborator Author

I have some progress in the use_pyjwt branch. It is not complete and has a lot of failing tests. I hope to get to it soon. Feel free to have a look.

@schlenk
Copy link
Collaborator

schlenk commented Jul 8, 2020

Decreased the test failures a bit in my local copy. It is really a bit messy, due to some concepts in keyio not fully baked and maybe even wrong (e.g. kid/iss handling for keys, JWS only talks about kid and a few other header attributes for key selection, not iss which is part of the payload). Should i push updates on my progress to the branch or shall i put it elsewhere?

@tpazderka
Copy link
Collaborator Author

Feel free to push to the branch. As long as we do not squash the commits, it should work out :)

The issue between kid/iss is that it would be possible to have keys with same kid from different issuers. But if we ignore the issuer bit, we can always try them all, not sure if there are any security implications.

I was wondering if redoing keyio shouldn't have been the first step :/

@candidtim
Copy link

I don't want to create a "+1" comment, but I am so much looking forward to this change. We have recently hit a bug caused pyjwkest, here is the relevant issue: IdentityPython/pyjwkest#100

In our case it means we have to disable the PYTHONOPTIMIZE option if we want to use pyoidc, which may very well be a blocking point for any production use.

This is just a single issue, but pyjwkest is no longer maintained, so I think it is reasonable to assume more may appear with time, especially should any vulnerabilities be found there. Last commit in pyjwkest was 3 years ago now...

@tpazderka
Copy link
Collaborator Author

Thanks for the comment. It is on my priority list right now and I will be looking into this shortly.

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

3 participants