From fa423b8e8535cddd463b8a0d6dce6f7a79819d28 Mon Sep 17 00:00:00 2001 From: bugclerk <40872210+bugclerk@users.noreply.github.com> Date: Fri, 6 Sep 2024 04:01:11 -0700 Subject: [PATCH] NAS-129859 / 25.04 / Allow adding certificates to system's trusted store (by sonicaj) (#14430) * Add db migration to add trusted store column for certs (cherry picked from commit b988d5d54bb6a229f150eb29eb238b63c7c7a1a3) * Account for trusted store key in cert service (cherry picked from commit 7674f9f93ed1e075ca7256677a1559d73fe1be75) * Update generate ssl certs so that we add certs to trusted store (cherry picked from commit f1820b2fca0671fb0d9fceea1df8238ea56e15c7) * Add merge migration * Add ca/cert prefix when writing to trusted ca's path dir --------- Co-authored-by: Waqar Ahmed --- ...19-40_add_to_trust_store_field_for_cert.py | 24 +++++++++++++++++++ .../versions/25.04/2024-09-05_22-58_merge.py | 19 +++++++++++++++ .../etc_files/generate_ssl_certs.py | 11 +++++---- .../middlewared/plugins/crypto_/cert_entry.py | 2 +- .../crypto_/certificate_authorities.py | 1 - .../plugins/crypto_/certificates.py | 15 +++++++++--- 6 files changed, 62 insertions(+), 10 deletions(-) create mode 100644 src/middlewared/middlewared/alembic/versions/24.10/2024-09-04_19-40_add_to_trust_store_field_for_cert.py create mode 100644 src/middlewared/middlewared/alembic/versions/25.04/2024-09-05_22-58_merge.py diff --git a/src/middlewared/middlewared/alembic/versions/24.10/2024-09-04_19-40_add_to_trust_store_field_for_cert.py b/src/middlewared/middlewared/alembic/versions/24.10/2024-09-04_19-40_add_to_trust_store_field_for_cert.py new file mode 100644 index 0000000000000..e593c75948ec5 --- /dev/null +++ b/src/middlewared/middlewared/alembic/versions/24.10/2024-09-04_19-40_add_to_trust_store_field_for_cert.py @@ -0,0 +1,24 @@ +""" +Add add_to_trust_store field for certifacates + +Revision ID: c31881e67797 +Revises: 98c1ebde0079 +Create Date: 2024-09-04 19:40:16.801832+00:00 +""" +from alembic import op +import sqlalchemy as sa + + +revision = 'c31881e67797' +down_revision = '98c1ebde0079' +branch_labels = None +depends_on = None + + +def upgrade(): + with op.batch_alter_table('system_certificate', schema=None) as batch_op: + batch_op.add_column(sa.Column('cert_add_to_trusted_store', sa.Boolean(), nullable=False, server_default='0')) + + +def downgrade(): + pass diff --git a/src/middlewared/middlewared/alembic/versions/25.04/2024-09-05_22-58_merge.py b/src/middlewared/middlewared/alembic/versions/25.04/2024-09-05_22-58_merge.py new file mode 100644 index 0000000000000..2f7a37bd9c412 --- /dev/null +++ b/src/middlewared/middlewared/alembic/versions/25.04/2024-09-05_22-58_merge.py @@ -0,0 +1,19 @@ +"""Merge + +Revision ID: 7b618b9ca77d +Revises: 9f51d0be7b07, c31881e67797 +Create Date: 2024-09-04 00:35:59.547731+00:00 +""" + +revision = '7b618b9ca77d' +down_revision = ('9f51d0be7b07', 'c31881e67797') +branch_labels = None +depends_on = None + + +def upgrade(): + pass + + +def downgrade(): + pass diff --git a/src/middlewared/middlewared/etc_files/generate_ssl_certs.py b/src/middlewared/middlewared/etc_files/generate_ssl_certs.py index 91ee296731ff1..0c18a1e76512f 100644 --- a/src/middlewared/middlewared/etc_files/generate_ssl_certs.py +++ b/src/middlewared/middlewared/etc_files/generate_ssl_certs.py @@ -7,7 +7,7 @@ from middlewared.service import CallError, Service -def write_certificates(certs: list, cacerts: list) -> set: +def write_certificates(certs: list) -> set: expected_files = set() for cert in certs: if cert['chain_list']: @@ -31,9 +31,10 @@ def write_certificates(certs: list, cacerts: list) -> set: # to forcibly remove all locally-added CAs. trusted_cas_path = '/var/local/ca-certificates' shutil.rmtree(trusted_cas_path, ignore_errors=True) - for ca in filter(lambda c: c['chain_list'] and c['add_to_trusted_store'], cacerts): - with open(os.path.join(trusted_cas_path, f'{ca["name"]}.crt'), 'w') as f: - f.write('\n'.join(ca['chain_list'])) + for cert in filter(lambda c: c['chain_list'] and c['add_to_trusted_store'], certs): + cert_type = 'ca' if cert['cert_type'] == 'CA' else 'cert' + with open(os.path.join(trusted_cas_path, f'{cert_type}_{cert["name"]}.crt'), 'w') as f: + f.write('\n'.join(cert['chain_list'])) cp = subprocess.Popen('update-ca-certificates', stdout=subprocess.DEVNULL, stderr=subprocess.PIPE) err = cp.communicate()[1] @@ -73,7 +74,7 @@ def render(service: Service, middleware: Middleware) -> None: certs = middleware.call_sync('certificate.query') cas = middleware.call_sync('certificateauthority.query') - expected_files |= write_certificates(certs + cas, cas) + expected_files |= write_certificates(certs + cas) expected_files |= write_crls(cas, middleware) # We would like to remove certificates which have been deleted diff --git a/src/middlewared/middlewared/plugins/crypto_/cert_entry.py b/src/middlewared/middlewared/plugins/crypto_/cert_entry.py index 0fb8ccd2d6459..38be5d00f2997 100644 --- a/src/middlewared/middlewared/plugins/crypto_/cert_entry.py +++ b/src/middlewared/middlewared/plugins/crypto_/cert_entry.py @@ -49,6 +49,7 @@ Int('lifetime', null=True), Int('serial', null=True), Int('key_length', null=True), + Bool('add_to_trusted_store', default=False), Bool('chain', null=True), Bool('CA_type_existing'), Bool('CA_type_internal'), @@ -68,5 +69,4 @@ def get_ca_result_entry(): entry = copy.deepcopy(CERT_ENTRY) entry.name = 'certificateauthority_entry' - entry.attrs['add_to_trusted_store'] = Bool('add_to_trusted_store') return entry diff --git a/src/middlewared/middlewared/plugins/crypto_/certificate_authorities.py b/src/middlewared/middlewared/plugins/crypto_/certificate_authorities.py index a5eca60258885..a53e18cebfb20 100644 --- a/src/middlewared/middlewared/plugins/crypto_/certificate_authorities.py +++ b/src/middlewared/middlewared/plugins/crypto_/certificate_authorities.py @@ -203,7 +203,6 @@ def set_defaults(attr): ('edit', _set_enum('create_type')), ('edit', _set_cert_extensions_defaults('cert_extensions')), ('rm', {'name': 'dns_mapping'}), - ('add', Bool('add_to_trusted_store', default=False)), register=True ), ) diff --git a/src/middlewared/middlewared/plugins/crypto_/certificates.py b/src/middlewared/middlewared/plugins/crypto_/certificates.py index 2dd856dac6ecc..9e13f7b71d7e9 100644 --- a/src/middlewared/middlewared/plugins/crypto_/certificates.py +++ b/src/middlewared/middlewared/plugins/crypto_/certificates.py @@ -38,6 +38,7 @@ class CertificateModel(sa.Model): cert_renew_days = sa.Column(sa.Integer(), nullable=True, default=10) cert_acme_id = sa.Column(sa.ForeignKey('system_acmeregistration.id'), index=True, nullable=True) cert_revoked_date = sa.Column(sa.DateTime(), nullable=True) + cert_add_to_trusted_store = sa.Column(sa.Boolean(), default=False, nullable=False) class CertificateService(CRUDService): @@ -200,6 +201,7 @@ async def validate_common_attributes(self, data, schema_name): Str('digest_algorithm', enum=['SHA224', 'SHA256', 'SHA384', 'SHA512']), List('san', items=[Str('san')]), Ref('cert_extensions'), + Bool('add_to_trusted_store', default=False), register=True ), ) @@ -328,7 +330,7 @@ async def do_create(self, job, data): ).items() if k in [ 'name', 'certificate', 'CSR', 'privatekey', 'type', 'signedby', 'acme', 'acme_uri', - 'domains_authenticators', 'renew_days' + 'domains_authenticators', 'renew_days', 'add_to_trusted_store', ] } @@ -555,6 +557,7 @@ async def create_internal(self, job, data): 'certificate_update', Bool('revoked'), Int('renew_days', validators=[Range(min_=1, max_=30)]), + Bool('add_to_trusted_store'), Str('name'), ), ) @@ -595,7 +598,7 @@ async def do_update(self, job, id_, data): new.update(data) - if any(new.get(k) != old.get(k) for k in ('name', 'revoked', 'renew_days')): + if any(new.get(k) != old.get(k) for k in ('name', 'revoked', 'renew_days', 'add_to_trusted_store')): verrors = ValidationErrors() @@ -627,6 +630,12 @@ async def do_update(self, job, id_, data): 'Certificate has already been revoked and this cannot be reversed' ) + if not verrors and new['revoked'] and new['add_to_trusted_store']: + verrors.add( + 'certificate_update.add_to_trusted_store', + 'Revoked certificates cannot be added to system\'s trusted store' + ) + verrors.check() to_update = {'renew_days': new['renew_days']} if data.get('renew_days') else {} @@ -637,7 +646,7 @@ async def do_update(self, job, id_, data): 'datastore.update', self._config.datastore, id_, - {'name': new['name'], **to_update}, + {'name': new['name'], 'add_to_trusted_store': new['add_to_trusted_store'], **to_update}, {'prefix': self._config.datastore_prefix} )