search

IdentityPython / JWTConnect-Python-CryptoJWT

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

cryptography dep in CI #97

Closed peppelinux closed 3 years ago

peppelinux commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

rohe commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

peppelinux commented 3 years ago

https://pypi.org/simple/cryptography/

rohe commented 3 years ago

So someone screwed up at cryptography !

peppelinux commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

So we want to upgrade to cryptography 35?

rohe commented 3 years ago

Yes !

jschlyter commented 3 years ago

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

jschlyter commented 3 years ago

@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 commented 3 years ago

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

jschlyter commented 3 years ago

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

rohe commented 3 years ago

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

jschlyter commented 3 years ago

closed via #98