From 92b9d4ac1b97dc2eca0da45f6a99d25d530cc69c Mon Sep 17 00:00:00 2001 From: vmud213 Date: Thu, 13 Aug 2020 13:10:41 +0000 Subject: [PATCH] [Redfish] Enhance removing TLS certificates Enhances the TLS certificate removal logic so that it now accepts the list of TLS certificate files instead of fingerprints. Also enhances the logic of exporting TLS certificates if multiple certificates are present in a single file. Change-Id: Ida1f13c27cea1134a38f5f8465398c63272494fb --- proliantutils/ilo/client.py | 6 +- proliantutils/ilo/operations.py | 4 +- proliantutils/redfish/redfish.py | 85 ++++++-- proliantutils/tests/redfish/test_redfish.py | 227 +++++++++++++++++--- requirements.txt | 1 + 5 files changed, 280 insertions(+), 43 deletions(-) diff --git a/proliantutils/ilo/client.py b/proliantutils/ilo/client.py index 24930a38..b587f8ca 100644 --- a/proliantutils/ilo/client.py +++ b/proliantutils/ilo/client.py @@ -885,13 +885,15 @@ class IloClient(operations.IloOperations): def add_tls_certificate(self, cert_file_list): """Adds the TLS certificate to the iLO + :param cert_file_list: List of TLS certificate files :raises: IloError, on an error from iLO. """ return self._call_method('add_tls_certificate', cert_file_list) - def remove_tls_certificate(self, fp_list): + def remove_tls_certificate(self, cert_file_list): """Removes the TLS certificate from the iLO + :param cert_file_list: List of TLS certificate files :raises: IloError, on an error from iLO. """ - return self._call_method('remove_tls_certificate', fp_list) + return self._call_method('remove_tls_certificate', cert_file_list) diff --git a/proliantutils/ilo/operations.py b/proliantutils/ilo/operations.py index 25e0c6ad..60b6b13a 100644 --- a/proliantutils/ilo/operations.py +++ b/proliantutils/ilo/operations.py @@ -544,10 +544,10 @@ class IloOperations(object): """ raise exception.IloCommandNotSupportedError(ERRMSG) - def remove_tls_certificate(self, fp_list): + def remove_tls_certificate(self, cert_file_list): """Removes the TLS certificate from the iLO - :param fp_list: List of finger prints of the certificates + :param cert_file_list: List of TLS certificate files :raises: IloError, on an error from iLO. :raises: IloCommandNotSupportedError, if the command is not supported on the server. diff --git a/proliantutils/redfish/redfish.py b/proliantutils/redfish/redfish.py index 7d22189e..c03c62aa 100644 --- a/proliantutils/redfish/redfish.py +++ b/proliantutils/redfish/redfish.py @@ -14,9 +14,12 @@ __author__ = 'HPE' +from base64 import b64decode import json import re +from OpenSSL.crypto import FILETYPE_ASN1 +from OpenSSL.crypto import load_certificate from six.moves.urllib import parse import sushy from sushy.resources.system import mappings as sushy_map @@ -114,6 +117,9 @@ SUPPORTED_BOOT_MODE_MAP = { ilo_cons.SUPPORTED_BOOT_MODE_LEGACY_BIOS_AND_UEFI) } +_CERTIFICATE_PATTERN = ( + r'-----BEGIN CERTIFICATE-----.*?-----END CERTIFICATE-----') + LOG = log.get_logger(__name__) @@ -1339,17 +1345,33 @@ class RedfishOperations(operations.IloOperations): not supported on the server. """ sushy_system = self._get_sushy_system(PROLIANT_SYSTEM_ID) + if(self._is_boot_mode_uefi()): cert_list = [] for cert_file in cert_file_list: with open(cert_file, 'r') as f: data = json.dumps(f.read()) p = re.sub(r"\"", "", data) - q = re.sub(r"\\n", "\r\n", p) - r = q.rstrip() - cert = {} - cert['X509Certificate'] = r - cert_list.append(cert) + q = re.sub(r"\\n", "\r\n", p).rstrip() + + c_list = re.findall(_CERTIFICATE_PATTERN, q, re.DOTALL) + + if len(c_list) == 0: + LOG.warning("Could not find any valid certificate in " + "%(cert_file)s. Ignoring." % + {"cert_file": cert_file}) + continue + + for content in c_list: + cert = {} + cert['X509Certificate'] = content + cert_list.append(cert) + + if len(cert_list) == 0: + msg = (self._("No valid certificate in %(cert_file_list)s.") % + {"cert_file_list": cert_file_list}) + LOG.debug(msg) + raise exception.IloError(msg) cert_dict = {} cert_dict['NewCertificates'] = cert_list @@ -1366,28 +1388,61 @@ class RedfishOperations(operations.IloOperations): msg = 'TLS certificate cannot be upload in BIOS boot mode' raise exception.IloCommandNotSupportedInBiosError(msg) - def remove_tls_certificate(self, fp_list): + def remove_tls_certificate(self, cert_file_list): """Removes the TLS certificate from the iLO. - :param fp_list: List of finger prints of the TLS certificates + :param cert_file_list: List of TLS certificate files :raises: IloError, on an error from iLO. :raises: IloCommandNotSupportedError, if the command is not supported on the server. """ sushy_system = self._get_sushy_system(PROLIANT_SYSTEM_ID) + if(self._is_boot_mode_uefi()): - cert = {} + cert_dict = {} del_cert_list = [] - for fp in fp_list: - cert_fp = { - "FingerPrint": fp - } - del_cert_list.append(cert_fp) - cert.update({"DeleteCertificates": del_cert_list}) + for cert_file in cert_file_list: + with open(cert_file, 'r') as f: + data = json.dumps(f.read()) + p = re.sub(r"\"", "", data) + q = re.sub(r"\\n", "\r\n", p).rstrip() + + c_list = re.findall(_CERTIFICATE_PATTERN, q, re.DOTALL) + + if len(c_list) == 0: + LOG.warning("Could not find any valid certificate in " + "%(cert_file)s. Ignoring." % + {"cert_file": cert_file}) + continue + + for content in c_list: + pem_lines = [line.strip() for line in ( + content.strip().split('\n'))] + + try: + der_data = b64decode(''.join(pem_lines[1:-1])) + except ValueError: + LOG.warning("Illegal base64 encountered " + "in the certificate.") + else: + cert = load_certificate(FILETYPE_ASN1, der_data) + fp = cert.digest('sha256').decode('ascii') + cert_fp = { + "FingerPrint": fp + } + del_cert_list.append(cert_fp) + + if len(del_cert_list) == 0: + msg = (self._("No valid certificate in %(cert_file_list)s.") % + {"cert_file_list": cert_file_list}) + raise exception.IloError(msg) + + cert_dict.update({"DeleteCertificates": del_cert_list}) + try: (sushy_system.bios_settings.tls_config. - tls_config_settings.remove_tls_certificate(cert)) + tls_config_settings.remove_tls_certificate(cert_dict)) except sushy.exceptions.SushyError as e: msg = (self._("The Redfish controller has failed to remove " "TLS certificate. Error %(error)s") % diff --git a/proliantutils/tests/redfish/test_redfish.py b/proliantutils/tests/redfish/test_redfish.py index ac8ba086..08679846 100644 --- a/proliantutils/tests/redfish/test_redfish.py +++ b/proliantutils/tests/redfish/test_redfish.py @@ -13,7 +13,10 @@ # License for the specific language governing permissions and limitations # under the License. + +import builtins import collections +import io import json import ddt @@ -2028,39 +2031,98 @@ class RedfishOperationsTestCase(testtools.TestCase): self.rf_client.add_tls_certificate, data) + @mock.patch.object(builtins, 'open') @mock.patch.object(redfish.RedfishOperations, '_is_boot_mode_uefi') @mock.patch.object(redfish.RedfishOperations, '_get_sushy_system') def test_add_tls_certificate(self, get_sushy_system_mock, - _uefi_boot_mode_mock): + _uefi_boot_mode_mock, open_mock): _uefi_boot_mode_mock.return_value = True - cert_file = 'proliantutils/tests/redfish/json_samples/certfile.crt' - with open('proliantutils/tests/redfish/' - 'json_samples/certfile.crt', 'r') as f: - cert_data = f.read() + data = ( + "-----BEGIN CERTIFICATE-----\nMIID7TC\nCF" + "g879\n-----END CERTIFICATE-----\n" + "-----BEGIN CERTIFICATE-----\nKHY8UP\nGH" + "f792\n-----END CERTIFICATE-----\n" + ) - import re - cert_data = cert_data.rstrip() - ref_data = re.sub(r"\n", "\r\n", cert_data) + fd_mock = mock.MagicMock(spec=io.BytesIO) + open_mock.return_value = fd_mock + fd_mock.__enter__.return_value = fd_mock + fd_mock.read.return_value = data + c_l = [ + "-----BEGIN CERTIFICATE-----\r\nMIID7TC\r\nCF" + "g879\r\n-----END CERTIFICATE-----", + "-----BEGIN CERTIFICATE-----\r\nKHY8UP\r\nGH" + "f792\r\n-----END CERTIFICATE-----" + ] - data = { + expected_data = { "NewCertificates": [ { - "X509Certificate": ref_data + "X509Certificate": c_l[0], + }, + { + "X509Certificate": c_l[1], } ] } + cert_file = '/path/to/certfile' + self.rf_client.add_tls_certificate([cert_file]) (get_sushy_system_mock.return_value. bios_settings.tls_config.tls_config_settings. - add_tls_certificate.assert_called_once_with(data)) + add_tls_certificate.assert_called_once_with(expected_data)) + @mock.patch.object(builtins, 'open') + @mock.patch.object(redfish.RedfishOperations, '_is_boot_mode_uefi') + @mock.patch.object(redfish.RedfishOperations, '_get_sushy_system') + def test_add_tls_certificate_no_certificate(self, get_sushy_system_mock, + _uefi_boot_mode_mock, + open_mock): + _uefi_boot_mode_mock.return_value = True + data = ( + "-----UNFORMATED CERTIFICATE-----\nMIID7TC\nCF" + "g879\n-----END CERTIFICATE-----\n" + "-----UNFORMATED CERTIFICATE-----\nKHY8UP\nGH" + "f792\n-----END CERTIFICATE-----\n" + ) + + fd_mock = mock.MagicMock(spec=io.BytesIO) + open_mock.return_value = fd_mock + fd_mock.__enter__.return_value = fd_mock + fd_mock.read.return_value = data + cert_file = "/path/to/certfile" + + self.assertRaisesRegex( + exception.IloError, + "No valid certificate", + self.rf_client.add_tls_certificate, [cert_file]) + + (get_sushy_system_mock.return_value. + bios_settings.tls_config.tls_config_settings. + add_tls_certificate.assert_not_called()) + + @mock.patch.object(builtins, 'open') @mock.patch.object(redfish.RedfishOperations, '_is_boot_mode_uefi') @mock.patch.object(redfish.RedfishOperations, '_get_sushy_system') def test_add_tls_certificate_raises_ilo_error(self, get_sushy_system_mock, - _uefi_boot_mode_mock): + _uefi_boot_mode_mock, + open_mock): _uefi_boot_mode_mock.return_value = True - cert_file = 'proliantutils/tests/redfish/json_samples/certfile.crt' + + data = ( + "-----BEGIN CERTIFICATE-----\nMIID7TC\nCF" + "g879\n-----END CERTIFICATE-----\n" + "-----BEGIN CERTIFICATE-----\nKHY8UP\nGH" + "f792\n-----END CERTIFICATE-----\n" + ) + + fd_mock = mock.MagicMock(spec=io.BytesIO) + open_mock.return_value = fd_mock + fd_mock.__enter__.return_value = fd_mock + fd_mock.read.return_value = data + + cert_file = '/path/to/certfile' (get_sushy_system_mock.return_value. bios_settings.tls_config.tls_config_settings. add_tls_certificate.side_effect) = ( @@ -2071,26 +2133,143 @@ class RedfishOperationsTestCase(testtools.TestCase): 'The Redfish controller has failed to upload TLS certificate.', self.rf_client.add_tls_certificate, [cert_file]) + @mock.patch.object(redfish, 'load_certificate') + @mock.patch.object(redfish, 'b64decode') + @mock.patch.object(builtins, 'open') @mock.patch.object(redfish.RedfishOperations, '_is_boot_mode_uefi') @mock.patch.object(redfish.RedfishOperations, '_get_sushy_system') def test_remove_tls_certificate(self, get_sushy_system_mock, - _uefi_boot_mode_mock): + _uefi_boot_mode_mock, open_mock, + decode_mock, load_cert_mock): _uefi_boot_mode_mock.return_value = True - fp = ('FA:3A:68:C7:7E:ED:90:21:D2:FA:3E:54:6B:0C:14:D3:' - '2F:8D:43:50:F7:05:A7:0F:1C:68:35:DB:5C:D2:53:28') + decode_mock.side_effect = ['first decoded data', + 'second decoded data'] + cert1_mock = mock.MagicMock() + cert2_mock = mock.MagicMock() + load_cert_mock.side_effect = [cert1_mock, cert2_mock] + cert1_mock.digest.return_value.decode.return_value = "humptydumpty" + cert2_mock.digest.return_value.decode.return_value = "hickerydickery" - cert = {} - del_cert_list = [] - cert_fp = { - "FingerPrint": fp + data = ( + "-----BEGIN CERTIFICATE-----\nMIID7TC\nCF" + "g879\n-----END CERTIFICATE-----\n" + "-----BEGIN CERTIFICATE-----\nKHY8UP\nGH" + "f792\n-----END CERTIFICATE-----\n" + ) + + fd_mock = mock.MagicMock(spec=io.BytesIO) + open_mock.return_value = fd_mock + fd_mock.__enter__.return_value = fd_mock + fd_mock.read.return_value = data + + decode_calls = [ + mock.call('MIID7TCCFg879'), + mock.call('KHY8UPGHf792') + ] + load_cert_calls = [ + mock.call(redfish.FILETYPE_ASN1, 'first decoded data'), + mock.call(redfish.FILETYPE_ASN1, 'second decoded data') + ] + cert_file = '/path/to/certfile' + self.rf_client.remove_tls_certificate([cert_file]) + decode_mock.assert_has_calls(decode_calls) + load_cert_mock.assert_has_calls(load_cert_calls) + + expected_data = { + "DeleteCertificates": [ + { + "FingerPrint": "humptydumpty", + }, + { + "FingerPrint": "hickerydickery" + } + ] } - del_cert_list.append(cert_fp) - cert.update({"DeleteCertificates": del_cert_list}) - self.rf_client.remove_tls_certificate([fp]) (get_sushy_system_mock.return_value. bios_settings.tls_config.tls_config_settings. - remove_tls_certificate.assert_called_once_with(cert)) + remove_tls_certificate.assert_called_once_with(expected_data)) + + @mock.patch.object(redfish, 'load_certificate') + @mock.patch.object(redfish, 'b64decode') + @mock.patch.object(builtins, 'open') + @mock.patch.object(redfish.RedfishOperations, '_is_boot_mode_uefi') + @mock.patch.object(redfish.RedfishOperations, '_get_sushy_system') + def test_remove_tls_certificate_no_certificate(self, + get_sushy_system_mock, + _uefi_boot_mode_mock, + open_mock, decode_mock, + load_cert_mock): + + _uefi_boot_mode_mock.return_value = True + + data = ( + "-----UNFORMATED CERTIFICATE-----\nMIID7TC\nCF" + "g879\n-----END CERTIFICATE-----\n" + "-----UNFORMATED CERTIFICATE-----\nKHY8UP\nGH" + "f792\n-----END CERTIFICATE-----\n" + ) + + fd_mock = mock.MagicMock(spec=io.BytesIO) + open_mock.return_value = fd_mock + fd_mock.__enter__.return_value = fd_mock + fd_mock.read.return_value = data + + cert_file = '/path/to/certfile' + + self.assertRaisesRegex( + exception.IloError, + "No valid certificate", + self.rf_client.remove_tls_certificate, [cert_file]) + + decode_mock.assert_not_called() + load_cert_mock.assert_not_called() + + (get_sushy_system_mock.return_value. + bios_settings.tls_config.tls_config_settings. + remove_tls_certificate.assert_not_called()) + + @mock.patch.object(redfish, 'load_certificate') + @mock.patch.object(redfish, 'b64decode') + @mock.patch.object(builtins, 'open') + @mock.patch.object(redfish.RedfishOperations, '_is_boot_mode_uefi') + @mock.patch.object(redfish.RedfishOperations, '_get_sushy_system') + def test_remove_tls_certificate_raises_ilo_error(self, + get_sushy_system_mock, + _uefi_boot_mode_mock, + open_mock, decode_mock, + load_cert_mock): + _uefi_boot_mode_mock.return_value = True + decode_mock.side_effect = ['first decoded data', + 'second decoded data'] + cert1_mock = mock.MagicMock() + cert2_mock = mock.MagicMock() + load_cert_mock.side_effect = [cert1_mock, cert2_mock] + cert1_mock.digest.return_value.decode.return_value = "humptydumpty" + cert2_mock.digest.return_value.decode.return_value = "hickerydickery" + + data = ( + "-----BEGIN CERTIFICATE-----\nMIID7TC\nCF" + "g879\n-----END CERTIFICATE-----\n" + "-----BEGIN CERTIFICATE-----\nKHY8UP\nGH" + "f792\n-----END CERTIFICATE-----\n" + ) + + fd_mock = mock.MagicMock(spec=io.BytesIO) + open_mock.return_value = fd_mock + fd_mock.__enter__.return_value = fd_mock + fd_mock.read.return_value = data + + cert_file = '/path/to/certfile' + (get_sushy_system_mock.return_value. + bios_settings.tls_config.tls_config_settings. + remove_tls_certificate.side_effect) = ( + sushy.exceptions.SushyError) + + self.assertRaisesRegex( + exception.IloError, + 'The Redfish controller has failed to remove TLS certificate.', + self.rf_client.remove_tls_certificate, [cert_file]) @mock.patch.object(redfish.RedfishOperations, '_is_boot_mode_uefi') @mock.patch.object(redfish.RedfishOperations, '_get_sushy_system') diff --git a/requirements.txt b/requirements.txt index 0882a219..a9bdc1d6 100644 --- a/requirements.txt +++ b/requirements.txt @@ -10,3 +10,4 @@ pysnmp>=4.2.3,<5.0.0 # BSD # Redfish communication uses the Sushy library sushy>=3.1.0 +pyOpenSSL>=19.1.0