Question on monkey-patching paramiko for FIPS
Hi all, As many have already seen, a number of changes have been merged in OpenStack as part of the effort to allow OpenStack to run on FIPS enabled systems. This effort has been captured in a proposed community goal. [1]. One of the requirements for this effort is that md5sum() not be used in a security related context. In fact, python 3.9 has been modified to raise an exception of hashlib.md5sum() is called on a FIPS enabled system, unless it is explicitly annotated with a usedforsecurity=False attribute [2]. We added a wrapper for md5sum in oslo.config to take advantage of this attribute. [3,4,5] Where we have less control is in libraries used by Openstack - and in particular, paramiko. Paramiko fails on FIPS enabled systems because of a call to md5sum() in get_fingerprint(). A patch has been submitted to fix this problem. [6]. Unfortunately, it takes a very long time for paramiko to fix issues. In order for us to make progress on FIPS testing, a small monkey-patch for paramiko was checked into tempest. [7]. Because this change was made to a test tool, this patch was relatively uncontroversial. A similar change has been found to be needed for manila [8]. I would expect that a similar change will be needed in other components that use paramiko to SSH to other systems (eg. cinder, neutron?) I suspect that the only reason this has not been detected in FIPS testing more widely yet is because the components that use paramiko for SSH are being tested in third party tests that do not, as yet, test FIPS. At the request of the manila team, I am bringing this monkey-patch to the attention of the wider OpenStack community to get feedback on the pros and cons of applying this monkey-patch. A couple things to note: 1. This monkey patch is quite small in scope and only needed until paramiko fixes the issue. 2. paramiko is not FIPS compliant, and so we will ultimately need to fix paramiko or replace it with a different library on FIPS enabled systems. When we do this, we would remove the monkey patch. Thanks, Ade Lee [1] https://opendev.org/openstack/governance/src/branch/master/goals/proposed/fi... [2] https://bugs.python.org/issue9216 [3] https://review.opendev.org/c/openstack/oslo.utils/+/750031 [4] Patches to various projects to use oslo.utils adapter for hashlib.md5 (as examples): glance: https://review.opendev.org/c/openstack/glance/+/756158 nova: https://review.opendev.org/c/openstack/nova/+/756434 nova: https://review.opendev.org/c/openstack/nova/+/777686 os-brick: https://review.opendev.org/c/openstack/os-brick/+/756151 oslo: https://review.opendev.org/c/openstack/oslo.versionedobjects/+/756153 tooz: https://review.opendev.org/c/openstack/tooz/+/756432 opensdk: https://review.opendev.org/c/openstack/openstacksdk/+/767411 octavia: https://review.opendev.org/c/openstack/octavia/+/798146 designate: https://review.opendev.org/c/openstack/designate/+/798157 glance_store: https://review.opendev.org/c/openstack/glance_store/+/756157 [5] Swift patch to handle hashlib.md5 https://review.opendev.org/c/openstack/swift/+/751966 [6] https://github.com/paramiko/paramiko/pull/1928 [7] https://review.opendev.org/c/openstack/tempest/+/822560 [8] https://review.opendev.org/c/openstack/manila/+/819375
Hi, On wtorek, 19 kwietnia 2022 22:59:03 CEST Ade Lee wrote:
Hi all,
As many have already seen, a number of changes have been merged in OpenStack as part of the effort to allow OpenStack to run on FIPS enabled systems. This effort has been captured in a proposed community goal. [1].
One of the requirements for this effort is that md5sum() not be used in a security related context. In fact, python 3.9 has been modified to raise an exception of hashlib.md5sum() is called on a FIPS enabled system, unless it is explicitly annotated with a usedforsecurity=False attribute [2]. We added a wrapper for md5sum in oslo.config to take advantage of this attribute. [3,4,5]
Where we have less control is in libraries used by Openstack - and in particular, paramiko. Paramiko fails on FIPS enabled systems because of a call to md5sum() in get_fingerprint(). A patch has been submitted to fix this problem. [6]. Unfortunately, it takes a very long time for paramiko to fix issues.
In order for us to make progress on FIPS testing, a small monkey-patch for paramiko was checked into tempest. [7]. Because this change was made to a test tool, this patch was relatively uncontroversial.
A similar change has been found to be needed for manila [8]. I would expect that a similar change will be needed in other components that use paramiko to SSH to other systems (eg. cinder, neutron?) I suspect that the only reason this has not been detected in FIPS testing more widely yet is because the components that use paramiko for SSH are being tested in third party tests that do not, as yet, test FIPS.
I just checked in Neutron and it seems that we are not using paramiko almost anywhere. It is used in neutron-tempest-plugin and in rally tests in neutron-vpnaas. Except that it's used in os-ken: https://opendev.org/openstack/os-ken/src/branch/master/os_ken/services/protocols/bgp/operator/ssh.py[1] but that's something what we in neutron are not using at all so that's why we didn't found it during the FIPS testing.
At the request of the manila team, I am bringing this monkey-patch to the attention of the wider OpenStack community to get feedback on the pros and cons of applying this monkey-patch.
A couple things to note: 1. This monkey patch is quite small in scope and only needed until paramiko fixes the issue. 2. paramiko is not FIPS compliant, and so we will ultimately need to fix paramiko or replace it with a different library on FIPS enabled systems. When we do this, we would remove the monkey patch.
IMHO it's ok to temporary use it in testing to be sure that everything else is working fine with FIPS (or fix any other issues which will be there). But in longer term I don't think we can say that e.g. Manila is FIPS compliant if it is using paramiko which isn't FIPS compliant. So it will need to be fixed on the paramiko side or Manila will need to move to some other lib to be FIPS compliant.
Thanks, Ade Lee
[1] https://opendev.org/openstack/governance/src/branch/master/goals/proposed/fi... [2] https://bugs.python.org/issue9216 [3] https://review.opendev.org/c/openstack/oslo.utils/+/750031 [4] Patches to various projects to use oslo.utils adapter for hashlib.md5 (as examples): glance: https://review.opendev.org/c/openstack/glance/+/756158 nova: https://review.opendev.org/c/openstack/nova/+/756434 nova: https://review.opendev.org/c/openstack/nova/+/777686 os-brick: https://review.opendev.org/c/openstack/os-brick/+/756151 oslo: https://review.opendev.org/c/openstack/oslo.versionedobjects/+/756153 tooz: https://review.opendev.org/c/openstack/tooz/+/756432 opensdk: https://review.opendev.org/c/openstack/openstacksdk/+/767411 octavia: https://review.opendev.org/c/openstack/octavia/+/798146 designate: https://review.opendev.org/c/openstack/designate/+/798157 glance_store: https://review.opendev.org/c/openstack/glance_store/+/756157 [5] Swift patch to handle hashlib.md5 https://review.opendev.org/c/openstack/swift/+/751966 [6] https://github.com/paramiko/paramiko/pull/1928 [7] https://review.opendev.org/c/openstack/tempest/+/822560 [8] https://review.opendev.org/c/openstack/manila/+/819375
-- Slawek Kaplonski Principal Software Engineer Red Hat -------- [1] https://opendev.org/openstack/os-ken/src/branch/master/os_ken/services/proto...
Hi Ade, We discussed this at our cinder meeting yesterday[1]. We do have occurrences of paramiko in our code[2] but they are mostly in our driver code where we are not testing compliance with FIPS. Currently, most of our third party CIs runs on Ubuntu and AFAIR, FIPS only works on CentOS so we can think about this when we start supporting FIPS for Ubuntu (as also discussed in Cinder PTG[3]). For the core cinder code, we don't have any usage of paramiko and the current jobs proposed[4] doesn't deal with third party drivers that are using paramiko. In conclusion, the FIPS compliance looks good from the Cinder side and we are planning to add documentation for new drivers about FIPS compliance so they're aware about it. [1] https://meetings.opendev.org/meetings/cinder/2022/cinder.2022-04-20-14.00.lo... [2] https://github.com/openstack/cinder/search?q=paramiko [3] https://etherpad.opendev.org/p/zed-ptg-cinder#L547 [4] https://review.opendev.org/c/openstack/cinder/+/790535 Thanks and regards Rajat Dhasmana On Wed, Apr 20, 2022 at 2:36 AM Ade Lee <alee@redhat.com> wrote:
Hi all,
As many have already seen, a number of changes have been merged in OpenStack as part of the effort to allow OpenStack to run on FIPS enabled systems. This effort has been captured in a proposed community goal. [1].
One of the requirements for this effort is that md5sum() not be used in a security related context. In fact, python 3.9 has been modified to raise an exception of hashlib.md5sum() is called on a FIPS enabled system, unless it is explicitly annotated with a usedforsecurity=False attribute [2]. We added a wrapper for md5sum in oslo.config to take advantage of this attribute. [3,4,5]
Where we have less control is in libraries used by Openstack - and in particular, paramiko. Paramiko fails on FIPS enabled systems because of a call to md5sum() in get_fingerprint(). A patch has been submitted to fix this problem. [6]. Unfortunately, it takes a very long time for paramiko to fix issues.
In order for us to make progress on FIPS testing, a small monkey-patch for paramiko was checked into tempest. [7]. Because this change was made to a test tool, this patch was relatively uncontroversial.
A similar change has been found to be needed for manila [8]. I would expect that a similar change will be needed in other components that use paramiko to SSH to other systems (eg. cinder, neutron?) I suspect that the only reason this has not been detected in FIPS testing more widely yet is because the components that use paramiko for SSH are being tested in third party tests that do not, as yet, test FIPS.
At the request of the manila team, I am bringing this monkey-patch to the attention of the wider OpenStack community to get feedback on the pros and cons of applying this monkey-patch.
A couple things to note: 1. This monkey patch is quite small in scope and only needed until paramiko fixes the issue. 2. paramiko is not FIPS compliant, and so we will ultimately need to fix paramiko or replace it with a different library on FIPS enabled systems. When we do this, we would remove the monkey patch.
Thanks, Ade Lee
[1] https://opendev.org/openstack/governance/src/branch/master/goals/proposed/fi... [2] https://bugs.python.org/issue9216 [3] https://review.opendev.org/c/openstack/oslo.utils/+/750031 [4] Patches to various projects to use oslo.utils adapter for hashlib.md5 (as examples): glance: https://review.opendev.org/c/openstack/glance/+/756158 nova: https://review.opendev.org/c/openstack/nova/+/756434 nova: https://review.opendev.org/c/openstack/nova/+/777686 os-brick: https://review.opendev.org/c/openstack/os-brick/+/756151 oslo: https://review.opendev.org/c/openstack/oslo.versionedobjects/+/756153 tooz: https://review.opendev.org/c/openstack/tooz/+/756432 opensdk: https://review.opendev.org/c/openstack/openstacksdk/+/767411 octavia: https://review.opendev.org/c/openstack/octavia/+/798146 designate: https://review.opendev.org/c/openstack/designate/+/798157 glance_store: https://review.opendev.org/c/openstack/glance_store/+/756157 [5] Swift patch to handle hashlib.md5 https://review.opendev.org/c/openstack/swift/+/751966 [6] https://github.com/paramiko/paramiko/pull/1928 [7] https://review.opendev.org/c/openstack/tempest/+/822560 [8] https://review.opendev.org/c/openstack/manila/+/819375
participants (3)
-
Ade Lee
-
Rajat Dhasmana
-
Slawek Kaplonski