Skip to content

cryptography dep in CI #97

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

Closed
peppelinux opened this issue Oct 27, 2021 · 17 comments
Closed

cryptography dep in CI #97

peppelinux opened this issue Oct 27, 2021 · 17 comments
Assignees

Comments

@peppelinux
Copy link
Member

In oidcmsg CI we have the problem:
https://github.com/IdentityPython/JWTConnect-Python-OidcMsg/runs/4022769174?check_suite_focus=true#step:4:255

other users told me that they have too, with their setup.
IS there something to improve in the poetry lock or any other action to solve this?

@rohe
Copy link
Contributor

rohe commented Oct 27, 2021

Isn't this an issue for JWTConnect-Python-OidcMsg and not for JWTConnect-Python-CryptoJWT?

Anyway, why poetry.lock is a problem is weird since we're not using poetry for OidcMsg.

@peppelinux
Copy link
Member Author

good question, take a look to oidcmsg CI logs and your conclusion, I'm ready to move this issue to oidcmsg

@rohe
Copy link
Contributor

rohe commented Oct 27, 2021

It seems the root of the problem is indeed in CryptoJWT.
Why it has

[tool.poetry.dependencies]
python = "^3.6.2"
cryptography = "^3.4.6"

in pyproject.toml and

[[package]]
name = "cryptography"
version = "3.4.8"

in poetry.lock I don't know.

Did Cryptography leap from 3.4.X to 35.0.0 ??

@rohe
Copy link
Contributor

rohe commented Oct 27, 2021

Turns out they jumped from 3.4.8 to 35.0.0 .
Wonder if someone made an error ? That it should have been 3.5.0 ??

@peppelinux
Copy link
Member Author

peppelinux commented Oct 27, 2021

anyway, this issue was detected from both collegues and oidcmsg CI, sounds like something that must be patched indeed

@peppelinux
Copy link
Member Author

@rohe
Copy link
Contributor

rohe commented Oct 28, 2021

So someone screwed up at cryptography !

@peppelinux
Copy link
Member Author

or ... is it a poetry bug?
it looks too much strange that that rule matches with 35...

it's time to have a brand new release of cryptojwt if you agree, we had many minor changes in the master branch and that would be a good moment to fix definitively this dependency problem

@rohe
Copy link
Contributor

rohe commented Oct 28, 2021

I don't think it's a poetry bug.
I locally changed the cryptography dependency to be 35.0.0.
Unfortunately running the tests I get a bunch of errors.
All of them seems to have something to do with the X509 support.
Since @jschlyter did all/most of that I would appreciate if he could fix whatever is amiss.

@jschlyter jschlyter self-assigned this Oct 28, 2021
@jschlyter
Copy link
Collaborator

So we want to upgrade to cryptography 35?

@rohe
Copy link
Contributor

rohe commented Oct 28, 2021

Yes !

@jschlyter
Copy link
Collaborator

@rohe it seems some of the test vectors are no longer accepted by Cryptography, see results in #98

@jschlyter
Copy link
Collaborator

@rohe regression due to:

"BACKWARDS INCOMPATIBLE: The X.509 certificate parser no longer allows negative serial numbers. RFC 5280 has always prohibited these."

The Microsoft test certificates has negative serial numbers. Can we simply replace them with correct ones?

@rohe
Copy link
Contributor

rohe commented Oct 30, 2021

I'd support that !
Just put that comment in the README file.

@jschlyter
Copy link
Collaborator

Can you update the test vectors @rohe? You found them in the first place :-)

@rohe
Copy link
Contributor

rohe commented Oct 30, 2021

I did ? Can't remember but I'll try.

@jschlyter
Copy link
Collaborator

closed via #98

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