search

ros2 / sros2

tools to generate and distribute keys for SROS 2
Apache License 2.0
90 stars 46 forks source link

Use modern PKCS7 to sign the certificate bytes. #290

Closed clalancette closed 9 months ago

clalancette commented 10 months ago

Using the SSLBinding leads to a warning in newer versions of pycryptography. Luckily, there is a supported API called pkcs7 that allows us to do the same thing. Even better, this API is supported since pycryptography 3.2, so this should work on both Ubuntu 22.04 and Ubuntu 24.04 without warnings.

clalancette commented 10 months ago

CI:

clalancette commented 10 months ago

OK, we need to keep the fallback path for Windows. I've done that in 389dc4c, here is new CI:

marcoag commented 9 months ago

I gave it a try and works for me. Thanks for the much needed change!

mikaelarguedas commented 6 months ago

@clalancette thanks for this!

A couple follow-up questions

clalancette commented 6 months ago

can the issue https://github.com/ros2/sros2/issues/239 be closed now that this is merged ?

Yep! Done now.

  • should this be backported to iron and humble ?

I don't think we particularly need it there. We are still using the older version of pycryptography there, which has the old API.

No, we still need it on Windows, even on Jazzy and Rolling. We are still using the older version of pycryptography there.

mikaelarguedas commented 6 months ago

I don't think we particularly need it there. We are still using the older version of pycryptography there, which has the old API.

Ok sorry for catching the train on route so long after. My understanding from reading the issues was that the reason for this PR was to support modern versions of cryptography as on Windows the version was too recent and caused breakage on Iron https://github.com/ros2/ros2_documentation/issues/4069. So if this API is not made avialable on Iron, that means this issue is still open and users are still force to downgrade cryptography on their system https://github.com/ros2/ros2_documentation/blob/fa244408830942eb45b32a31ae6b11d79320571a/source/Releases/Release-Iron-Irwini.rst?plain=1#L849 is that correct ?

No, we still need it on Windows, even on Jazzy and Rolling. We are still using the older version of pycryptography there.

Is there something specific holding us back here ?

It looks like cryptography is compatible with python 3.8, the system one on Ubuntu 24.04 is 41.0.7-4build3 on ubuntu 22.04 3.4.8-1ubuntu2.2 on windows it's installed from pip (so way above the faulty 3.1.0 version)

clalancette commented 6 months ago

Is there something specific holding us back here ?

Yes, Windows Debug.

We have an ancient version of pycryptography that we have built by hand, and have not updated in a long time; you can see it get installed here: https://github.com/ros2/ci/blob/3435d04acf17872b07861e81265bc34d7ef40c72/ros2_batch_job/__main__.py#L493

The correct solution here would be to update that debug wheel to a modern pycryptography version, but a) it needs to build against Python 3.8 for the other reasons mentioned, and b) when I poked at it a couple of months ago, I couldn't figure out how to make it build at all. If you have time to look into that, it would be appreciated.