From 7bd617b2fbbb11ac30e4037be818d1093113e5be Mon Sep 17 00:00:00 2001 From: Karla Felix Date: Fri, 15 Jul 2022 14:46:20 -0300 Subject: [PATCH] Block ssl_ca certificates with same subject Block the addition of ssl_ca certificates with same subject name Test Plan: PASS: Attempted to install another certificate with same subject, and verified that it fails with an error. PASS: Generate and install a full iso and verified that columns subject and hash_subject were added to certificate table. PASS: Verified that when there is a subject name clash the command system certificate-install returns an error and the certificate that has the same subject PASS: Verified that the system shows an error when the subject field is emtpy for ssl_ca PASS: Verified that a new column subject shows up for command system certificate-list PASS: Verified that a new column subject shows up as a return to a successful system certificate-install command Depends-on: https://review.opendev.org/c/starlingx/ansible-playbooks/+/851894 Closes-bug: 1981100 Signed-off-by: Karla Felix Change-Id: I7ce11cc5dab6f686d360d01594ba100d07d2c2db --- .../cgtsclient/v1/certificate_shell.py | 10 +++-- .../sysinv/api/controllers/v1/certificate.py | 39 ++++++++++++++++--- .../sysinv/sysinv/sysinv/conductor/manager.py | 10 ++++- .../125_certificate_add_new_columns.py | 31 +++++++++++++++ .../sysinv/sysinv/db/sqlalchemy/models.py | 4 +- .../sysinv/sysinv/objects/certificate.py | 2 + 6 files changed, 84 insertions(+), 12 deletions(-) create mode 100644 sysinv/sysinv/sysinv/sysinv/db/sqlalchemy/migrate_repo/versions/125_certificate_add_new_columns.py diff --git a/sysinv/cgts-client/cgts-client/cgtsclient/v1/certificate_shell.py b/sysinv/cgts-client/cgts-client/cgtsclient/v1/certificate_shell.py index 7bfa02c8e3..9333e19d3e 100644 --- a/sysinv/cgts-client/cgts-client/cgtsclient/v1/certificate_shell.py +++ b/sysinv/cgts-client/cgts-client/cgtsclient/v1/certificate_shell.py @@ -1,5 +1,5 @@ # -# Copyright (c) 2018 Wind River Systems, Inc. +# Copyright (c) 2022 Wind River Systems, Inc. # # SPDX-License-Identifier: Apache-2.0 # @@ -22,7 +22,7 @@ PRIVATE_KEY_PATTERN = \ def _print_certificate_show(certificate): - fields = ['uuid', 'certtype', 'signature', 'start_date', 'expiry_date'] + fields = ['uuid', 'certtype', 'signature', 'start_date', 'expiry_date', 'subject'] if isinstance(certificate, dict): data = [(f, certificate.get(f, '')) for f in fields] details = ('details', certificate.get('details', '')) @@ -31,7 +31,6 @@ def _print_certificate_show(certificate): details = ('details', getattr(certificate, 'details', '')) if details[1]: data.append(details) - utils.print_tuple_list(data) @@ -49,8 +48,11 @@ def do_certificate_show(cc, args): def do_certificate_list(cc, args): """List certificates.""" certificates = cc.certificate.list() - fields = ['uuid', 'certtype', 'expiry_date'] + fields = ['uuid', 'certtype', 'expiry_date', 'subject'] field_labels = fields + for certificate in certificates: + if len(certificate.subject) > 20: + certificate.subject = certificate.subject[:20] + "..." utils.print_list(certificates, fields, field_labels, sortby=0) diff --git a/sysinv/sysinv/sysinv/sysinv/api/controllers/v1/certificate.py b/sysinv/sysinv/sysinv/sysinv/api/controllers/v1/certificate.py index 5e5cb3b22b..31d92bf3e7 100644 --- a/sysinv/sysinv/sysinv/sysinv/api/controllers/v1/certificate.py +++ b/sysinv/sysinv/sysinv/sysinv/api/controllers/v1/certificate.py @@ -16,7 +16,7 @@ # License for the specific language governing permissions and limitations # under the License. # -# Copyright (c) 2018 Wind River Systems, Inc. +# Copyright (c) 2018-2022 Wind River Systems, Inc. # import datetime @@ -104,6 +104,12 @@ class Certificate(base.APIBase): updated_at = wtypes.datetime.datetime + subject = wtypes.text + "Represents the subject of the certificate" + + hash_subject = wtypes.text + "Represents the hash of the certificate's subject" + def __init__(self, **kwargs): self.fields = list(objects.certificate.fields.keys()) for k in self.fields: @@ -126,7 +132,9 @@ class Certificate(base.APIBase): 'signature', 'details', 'start_date', - 'expiry_date']) + 'expiry_date', + 'subject', + 'hash_subject']) certificate.links = \ [link.Link.make_link('self', pecan.request.host_url, @@ -323,6 +331,9 @@ class CertificateController(rest.RestController): hash_issuers = [] cert_validity_error = None + + existing_certificates = pecan.request.dbapi.certificate_get_list() + for index, cert in enumerate(certs): msg = self._check_cert_validity(cert) if msg is not True: @@ -384,6 +395,24 @@ class CertificateController(rest.RestController): msg = "Cannot install non-CA type certificate as SSL " \ "CA certificate" return dict(success="", error=msg) + if cert.subject: + hash_subject = cutils.get_cert_subject_hash(cert) + duplicate_certificates = [certificate.uuid + for certificate in existing_certificates + if certificate.hash_subject + if hash_subject == int(certificate.hash_subject)] + if duplicate_certificates: + msg = "Cannot install certificate with same subject" \ + "\nPlease uninstall the following CA certs that have " \ + "the same subject first" + for uuid in duplicate_certificates: + msg += "\nUUID : %s" % uuid + LOG.error(msg) + return dict(success="", error=msg) + else: + msg = "Cannot install CA certificate without subject" + LOG.error(msg) + return dict(success="", error=msg) if mode == constants.CERT_MODE_OPENSTACK and index == 0: domain, msg = _check_endpoint_domain_exists() @@ -408,8 +437,6 @@ class CertificateController(rest.RestController): LOG.warn(msg) return dict(success="", error=str(e.value), body="", certificates={}) - certificates = pecan.request.dbapi.certificate_get_list() - # Create new or update existing certificates in sysinv with the # information returned from conductor manager. certificate_dicts = [] @@ -429,12 +456,14 @@ class CertificateController(rest.RestController): 'signature': inv_cert.get('signature'), 'start_date': inv_cert.get('not_valid_before'), 'expiry_date': inv_cert.get('not_valid_after'), + 'hash_subject': inv_cert.get('hash_subject'), + 'subject': inv_cert.get('subject'), } LOG.info("config_certificate values=%s" % values) # check to see if the installed cert exist in sysinv uuid = None - for certificate in certificates: + for certificate in existing_certificates: if mode == constants.CERT_MODE_SSL_CA: if inv_cert.get('signature') == certificate.signature: uuid = certificate.uuid diff --git a/sysinv/sysinv/sysinv/sysinv/conductor/manager.py b/sysinv/sysinv/sysinv/sysinv/conductor/manager.py index bcad8e02ac..4642107265 100644 --- a/sysinv/sysinv/sysinv/sysinv/conductor/manager.py +++ b/sysinv/sysinv/sysinv/sysinv/conductor/manager.py @@ -12983,6 +12983,8 @@ class ConductorManager(service.PeriodicService): # check if the cert is a CA cert is_ca = cutils.is_ca_cert(cert) + hash_subject = cutils.get_cert_subject_hash(cert) + signature = mode + '_' + str(cert.serial_number) if len(signature) > 255: LOG.info("Truncating certificate serial no %s" % signature) @@ -12992,7 +12994,8 @@ class ConductorManager(service.PeriodicService): cert_list.append({'cert': cert, 'is_ca': is_ca, 'public_bytes': public_bytes, - 'signature': signature}) + 'signature': signature, + 'hash_subject': hash_subject}) return cert_list, private_bytes @@ -13327,7 +13330,10 @@ class ConductorManager(service.PeriodicService): inv_cert = {'signature': cert.get('signature'), 'is_ca': cert.get('is_ca'), 'not_valid_before': cert.get('cert').not_valid_before, - 'not_valid_after': cert.get('cert').not_valid_after} + 'not_valid_after': cert.get('cert').not_valid_after, + 'hash_subject': cert.get('hash_subject'), + 'subject': cert.get('cert').subject.rfc4514_string() + } inv_certs.append(inv_cert) return inv_certs diff --git a/sysinv/sysinv/sysinv/sysinv/db/sqlalchemy/migrate_repo/versions/125_certificate_add_new_columns.py b/sysinv/sysinv/sysinv/sysinv/db/sqlalchemy/migrate_repo/versions/125_certificate_add_new_columns.py new file mode 100644 index 0000000000..a0d4f90401 --- /dev/null +++ b/sysinv/sysinv/sysinv/sysinv/db/sqlalchemy/migrate_repo/versions/125_certificate_add_new_columns.py @@ -0,0 +1,31 @@ +# vim: tabstop=4 shiftwidth=4 softtabstop=4 +# +# Copyright (c) 2022 Wind River Systems, Inc. +# +# SPDX-License-Identifier: Apache-2.0 +# + +from sqlalchemy import Column +from sqlalchemy import MetaData +from sqlalchemy import String +from sqlalchemy import Table + + +def upgrade(migrate_engine): + meta = MetaData() + meta.bind = migrate_engine + + # add column to certificate table + certificate = Table('certificate', meta, autoload=True) + + col_subject = Column('subject', String(255), nullable=True) + col_subject.create(certificate) + + col_hash_subject = Column('hash_subject', String(64), nullable=True) + col_hash_subject.create(certificate) + + +def downgrade(migrate_engine): + # As per other openstack components, downgrade is + # unsupported in this release. + raise NotImplementedError('SysInv database downgrade is unsupported.') diff --git a/sysinv/sysinv/sysinv/sysinv/db/sqlalchemy/models.py b/sysinv/sysinv/sysinv/sysinv/db/sqlalchemy/models.py index ba4a06f50f..81395d724d 100644 --- a/sysinv/sysinv/sysinv/sysinv/db/sqlalchemy/models.py +++ b/sysinv/sysinv/sysinv/sysinv/db/sqlalchemy/models.py @@ -15,7 +15,7 @@ # License for the specific language governing permissions and limitations # under the License. # -# Copyright (c) 2013-2021 Wind River Systems, Inc. +# Copyright (c) 2013-2022 Wind River Systems, Inc. # # SPDX-License-Identifier: Apache-2.0 # @@ -2018,6 +2018,8 @@ class certificate(Base): start_date = Column(DateTime(timezone=False)) expiry_date = Column(DateTime(timezone=False)) capabilities = Column(JSONEncodedDict) + subject = Column(String(255)) + hash_subject = Column(String(64)) class HelmOverrides(Base): diff --git a/sysinv/sysinv/sysinv/sysinv/objects/certificate.py b/sysinv/sysinv/sysinv/sysinv/objects/certificate.py index 0e326557ff..929714aeb9 100644 --- a/sysinv/sysinv/sysinv/sysinv/objects/certificate.py +++ b/sysinv/sysinv/sysinv/sysinv/objects/certificate.py @@ -24,6 +24,8 @@ class Certificate(base.SysinvObject): 'signature': utils.str_or_none, 'start_date': utils.datetime_or_str_or_none, 'expiry_date': utils.datetime_or_str_or_none, + 'subject': utils.str_or_none, + 'hash_subject': utils.str_or_none, } @base.remotable_classmethod