From db7a633a4ff2da39f67f9ddf42931cc21052fda4 Mon Sep 17 00:00:00 2001 From: Ade Lee Date: Fri, 25 Jun 2021 13:11:11 -0400 Subject: [PATCH] Replace md5 for fips md5 is not an approved algorithm in FIPS mode, and trying to instantiate a hashlib.md5() will fail when the system is running in FIPS mode. md5 is allowed when in a non-security context. There is a plan to add a keyword parameter (usedforsecurity) to hashlib.md5() to annotate whether or not the instance is being used in a security context. In the case where it is not, the instantiation of md5 will be allowed. See https://bugs.python.org/issue9216 for more details. Some downstream python versions already support this parameter. To support these versions, a new encapsulation of md5() has been added to oslo_utils. See https://review.opendev.org/#/c/750031/ In this case, md5 is used to generate etags and to check file integrity when uploading certs. fingerprints when ssh keys are being generated and imported. Without this patch, these operations fail on FIPS enabled systems. Change-Id: Ib189c6f67946851d37c31a6a8d657460c15f491e --- lower-constraints.txt | 2 +- .../backends/agent/api_server/loadbalancer.py | 14 ++++++----- .../drivers/haproxy/rest_api_driver.py | 23 +++++++++++-------- .../backend/agent/api_server/test_server.py | 6 ++--- .../haproxy/test_rest_api_driver_0_5.py | 9 ++++---- .../haproxy/test_rest_api_driver_1_0.py | 9 ++++---- requirements.txt | 2 +- 7 files changed, 36 insertions(+), 29 deletions(-) diff --git a/lower-constraints.txt b/lower-constraints.txt index b3d0f6a219..564982451c 100644 --- a/lower-constraints.txt +++ b/lower-constraints.txt @@ -91,7 +91,7 @@ oslo.reports==1.18.0 oslo.serialization==2.28.1 oslo.service==1.30.0 oslo.upgradecheck==1.3.0 -oslo.utils==4.5.0 +oslo.utils==4.7.0 oslotest==3.2.0 packaging==20.4 paramiko==2.4.1 diff --git a/octavia/amphorae/backends/agent/api_server/loadbalancer.py b/octavia/amphorae/backends/agent/api_server/loadbalancer.py index b223e21435..12baf86275 100644 --- a/octavia/amphorae/backends/agent/api_server/loadbalancer.py +++ b/octavia/amphorae/backends/agent/api_server/loadbalancer.py @@ -12,7 +12,6 @@ # License for the specific language governing permissions and limitations # under the License. -import hashlib import io import os import re @@ -24,6 +23,7 @@ import flask import jinja2 from oslo_config import cfg from oslo_log import log as logging +from oslo_utils.secretutils import md5 import webob from werkzeug import exceptions @@ -56,7 +56,7 @@ SYSTEMD_TEMPLATE = JINJA_ENV.get_template(SYSTEMD_CONF) class Wrapped(object): def __init__(self, stream_): self.stream = stream_ - self.hash = hashlib.md5() # nosec + self.hash = md5(usedforsecurity=False) # nosec def read(self, line): block = self.stream.read(line) @@ -86,7 +86,8 @@ class Loadbalancer(object): cfg = file.read() resp = webob.Response(cfg, content_type='text/plain') resp.headers['ETag'] = ( - hashlib.md5(octavia_utils.b(cfg)).hexdigest()) # nosec + md5(octavia_utils.b(cfg), + usedforsecurity=False).hexdigest()) # nosec return resp def upload_haproxy_config(self, amphora_id, lb_id): @@ -415,9 +416,10 @@ class Loadbalancer(object): with open(cert_path, 'r') as crt_file: cert = crt_file.read() - md5 = hashlib.md5(octavia_utils.b(cert)).hexdigest() # nosec - resp = webob.Response(json=dict(md5sum=md5)) - resp.headers['ETag'] = md5 + md5sum = md5(octavia_utils.b(cert), + usedforsecurity=False).hexdigest() # nosec + resp = webob.Response(json=dict(md5sum=md5sum)) + resp.headers['ETag'] = md5sum return resp def delete_certificate(self, lb_id, filename): diff --git a/octavia/amphorae/drivers/haproxy/rest_api_driver.py b/octavia/amphorae/drivers/haproxy/rest_api_driver.py index b12b3a4bef..ff9fcda21a 100644 --- a/octavia/amphorae/drivers/haproxy/rest_api_driver.py +++ b/octavia/amphorae/drivers/haproxy/rest_api_driver.py @@ -21,6 +21,7 @@ import warnings from oslo_context import context as oslo_context from oslo_log import log as logging +from oslo_utils.secretutils import md5 import requests import simplejson from stevedore import driver as stevedore_driver @@ -468,20 +469,21 @@ class HaproxyAmphoraLoadBalancerDriver( if amphora and obj_id: for cert in certs: pem = cert_parser.build_pem(cert) - md5 = hashlib.md5(pem).hexdigest() # nosec + md5sum = md5(pem, usedforsecurity=False).hexdigest() # nosec name = '{id}.pem'.format(id=cert.id) cert_filename_list.append( os.path.join( CONF.haproxy_amphora.base_cert_dir, obj_id, name)) - self._upload_cert(amphora, obj_id, pem, md5, name) + self._upload_cert(amphora, obj_id, pem, md5sum, name) if certs: # Build and upload the crt-list file for haproxy crt_list = "\n".join(cert_filename_list) crt_list = f'{crt_list}\n'.encode('utf-8') - md5 = hashlib.md5(crt_list).hexdigest() # nosec + md5sum = md5(crt_list, + usedforsecurity=False).hexdigest() # nosec name = '{id}.pem'.format(id=listener.id) - self._upload_cert(amphora, obj_id, crt_list, md5, name) + self._upload_cert(amphora, obj_id, crt_list, md5sum, name) return {'tls_cert': tls_cert, 'sni_certs': sni_certs} def _process_secret(self, listener, secret_ref, amphora=None, obj_id=None): @@ -497,13 +499,13 @@ class HaproxyAmphoraLoadBalancerDriver( secret = secret.encode('utf-8') except AttributeError: pass - md5 = hashlib.md5(secret).hexdigest() # nosec + md5sum = md5(secret, usedforsecurity=False).hexdigest() # nosec id = hashlib.sha1(secret).hexdigest() # nosec name = '{id}.pem'.format(id=id) if amphora and obj_id: self._upload_cert( - amphora, obj_id, pem=secret, md5=md5, name=name) + amphora, obj_id, pem=secret, md5sum=md5sum, name=name) return name def _process_listener_pool_certs(self, listener, amphora, obj_id): @@ -536,10 +538,11 @@ class HaproxyAmphoraLoadBalancerDriver( pem = pem.encode('utf-8') except AttributeError: pass - md5 = hashlib.md5(pem).hexdigest() # nosec + md5sum = md5(pem, usedforsecurity=False).hexdigest() # nosec name = '{id}.pem'.format(id=tls_cert.id) if amphora and obj_id: - self._upload_cert(amphora, obj_id, pem=pem, md5=md5, name=name) + self._upload_cert(amphora, obj_id, pem=pem, + md5sum=md5sum, name=name) pool_cert_dict['client_cert'] = os.path.join( CONF.haproxy_amphora.base_cert_dir, obj_id, name) if pool.ca_tls_certificate_id: @@ -555,10 +558,10 @@ class HaproxyAmphoraLoadBalancerDriver( return pool_cert_dict - def _upload_cert(self, amp, listener_id, pem, md5, name): + def _upload_cert(self, amp, listener_id, pem, md5sum, name): try: if self.clients[amp.api_version].get_cert_md5sum( - amp, listener_id, name, ignore=(404,)) == md5: + amp, listener_id, name, ignore=(404,)) == md5sum: return except exc.NotFound: pass diff --git a/octavia/tests/functional/amphorae/backend/agent/api_server/test_server.py b/octavia/tests/functional/amphorae/backend/agent/api_server/test_server.py index ac15bf19ab..b5dc472273 100644 --- a/octavia/tests/functional/amphorae/backend/agent/api_server/test_server.py +++ b/octavia/tests/functional/amphorae/backend/agent/api_server/test_server.py @@ -12,7 +12,6 @@ # License for the specific language governing permissions and limitations # under the License. -import hashlib import os import random import socket @@ -23,6 +22,7 @@ from unittest import mock import fixtures from oslo_config import fixture as oslo_fixture from oslo_serialization import jsonutils +from oslo_utils.secretutils import md5 from oslo_utils import uuidutils from octavia.amphorae.backends.agent import api_server @@ -862,8 +862,8 @@ class TestServerTestCase(base.TestCase): rv = self.centos_app.get('/' + api_server.VERSION + '/loadbalancer/123/certificates/test.pem') self.assertEqual(200, rv.status_code) - self.assertEqual(dict(md5sum=hashlib.md5(octavia_utils. - b(CONTENT)).hexdigest()), + self.assertEqual(dict(md5sum=md5(octavia_utils.b(CONTENT), + usedforsecurity=False).hexdigest()), jsonutils.loads(rv.data.decode('utf-8'))) def test_ubuntu_upload_certificate_md5(self): diff --git a/octavia/tests/unit/amphorae/drivers/haproxy/test_rest_api_driver_0_5.py b/octavia/tests/unit/amphorae/drivers/haproxy/test_rest_api_driver_0_5.py index fecd5aa301..5e592bc288 100644 --- a/octavia/tests/unit/amphorae/drivers/haproxy/test_rest_api_driver_0_5.py +++ b/octavia/tests/unit/amphorae/drivers/haproxy/test_rest_api_driver_0_5.py @@ -17,6 +17,7 @@ from unittest import mock from oslo_config import cfg from oslo_config import fixture as oslo_fixture +from oslo_utils.secretutils import md5 from oslo_utils import uuidutils import requests import requests_mock @@ -342,7 +343,7 @@ class TestHaproxyAmphoraLoadBalancerDriverTest(base.TestCase): mock_oslo.return_value = fake_context self.driver.cert_manager.get_secret.reset_mock() self.driver.cert_manager.get_secret.return_value = fake_secret - ref_md5 = hashlib.md5(fake_secret).hexdigest() # nosec + ref_md5 = md5(fake_secret, usedforsecurity=False).hexdigest() # nosec ref_id = hashlib.sha1(fake_secret).hexdigest() # nosec ref_name = '{id}.pem'.format(id=ref_id) @@ -356,7 +357,7 @@ class TestHaproxyAmphoraLoadBalancerDriverTest(base.TestCase): fake_context, sample_listener.client_ca_tls_certificate_id) mock_upload_cert.assert_called_once_with( self.amp, sample_listener.id, pem=fake_secret, - md5=ref_md5, name=ref_name) + md5sum=ref_md5, name=ref_name) self.assertEqual(ref_name, result) @mock.patch('octavia.amphorae.drivers.haproxy.rest_api_driver.' @@ -406,7 +407,7 @@ class TestHaproxyAmphoraLoadBalancerDriverTest(base.TestCase): mock_load_certs.return_value = pool_data fake_pem = b'fake pem' mock_build_pem.return_value = fake_pem - ref_md5 = hashlib.md5(fake_pem).hexdigest() # nosec + ref_md5 = md5(fake_pem, usedforsecurity=False).hexdigest() # nosec ref_name = '{id}.pem'.format(id=pool_cert.id) ref_path = '{cert_dir}/{list_id}/{name}'.format( cert_dir=fake_cert_dir, list_id=sample_listener.id, name=ref_name) @@ -437,7 +438,7 @@ class TestHaproxyAmphoraLoadBalancerDriverTest(base.TestCase): mock_build_pem.assert_called_once_with(pool_cert) mock_upload_cert.assert_called_once_with( self.amp, sample_listener.id, pem=fake_pem, - md5=ref_md5, name=ref_name) + md5sum=ref_md5, name=ref_name) mock_secret.assert_has_calls(secret_calls) self.assertEqual(ref_result, result) diff --git a/octavia/tests/unit/amphorae/drivers/haproxy/test_rest_api_driver_1_0.py b/octavia/tests/unit/amphorae/drivers/haproxy/test_rest_api_driver_1_0.py index a845735d17..60ee82bb0d 100644 --- a/octavia/tests/unit/amphorae/drivers/haproxy/test_rest_api_driver_1_0.py +++ b/octavia/tests/unit/amphorae/drivers/haproxy/test_rest_api_driver_1_0.py @@ -17,6 +17,7 @@ from unittest import mock from oslo_config import cfg from oslo_config import fixture as oslo_fixture +from oslo_utils.secretutils import md5 from oslo_utils import uuidutils import requests import requests_mock @@ -343,7 +344,7 @@ class TestHaproxyAmphoraLoadBalancerDriverTest(base.TestCase): mock_oslo.return_value = fake_context self.driver.cert_manager.get_secret.reset_mock() self.driver.cert_manager.get_secret.return_value = fake_secret - ref_md5 = hashlib.md5(fake_secret).hexdigest() # nosec + ref_md5 = md5(fake_secret, usedforsecurity=False).hexdigest() # nosec ref_id = hashlib.sha1(fake_secret).hexdigest() # nosec ref_name = '{id}.pem'.format(id=ref_id) @@ -357,7 +358,7 @@ class TestHaproxyAmphoraLoadBalancerDriverTest(base.TestCase): fake_context, sample_listener.client_ca_tls_certificate_id) mock_upload_cert.assert_called_once_with( self.amp, sample_listener.id, pem=fake_secret, - md5=ref_md5, name=ref_name) + md5sum=ref_md5, name=ref_name) self.assertEqual(ref_name, result) @mock.patch('octavia.amphorae.drivers.haproxy.rest_api_driver.' @@ -407,7 +408,7 @@ class TestHaproxyAmphoraLoadBalancerDriverTest(base.TestCase): mock_load_certs.return_value = pool_data fake_pem = b'fake pem' mock_build_pem.return_value = fake_pem - ref_md5 = hashlib.md5(fake_pem).hexdigest() # nosec + ref_md5 = md5(fake_pem, usedforsecurity=False).hexdigest() # nosec ref_name = '{id}.pem'.format(id=pool_cert.id) ref_path = '{cert_dir}/{lb_id}/{name}'.format( cert_dir=fake_cert_dir, lb_id=sample_listener.load_balancer.id, @@ -439,7 +440,7 @@ class TestHaproxyAmphoraLoadBalancerDriverTest(base.TestCase): mock_build_pem.assert_called_once_with(pool_cert) mock_upload_cert.assert_called_once_with( self.amp, sample_listener.load_balancer.id, pem=fake_pem, - md5=ref_md5, name=ref_name) + md5sum=ref_md5, name=ref_name) mock_secret.assert_has_calls(secret_calls) self.assertEqual(ref_result, result) diff --git a/requirements.txt b/requirements.txt index 7e450f6960..6091dde299 100644 --- a/requirements.txt +++ b/requirements.txt @@ -26,7 +26,7 @@ oslo.policy>=3.6.2 # Apache-2.0 oslo.reports>=1.18.0 # Apache-2.0 oslo.serialization>=2.28.1 # Apache-2.0 oslo.upgradecheck>=1.3.0 # Apache-2.0 -oslo.utils>=4.5.0 # Apache-2.0 +oslo.utils>=4.7.0 # Apache-2.0 pyasn1!=0.2.3,>=0.1.8 # BSD pyasn1-modules>=0.0.6 # BSD python-barbicanclient>=4.5.2 # Apache-2.0