From 8aceea34bec253f1c891f41ddf3811f26db83aba Mon Sep 17 00:00:00 2001 From: Wayonb Date: Tue, 19 Nov 2024 23:52:43 -0500 Subject: [PATCH] [tools/shoestring] fix: renew certs generates a new ca cert problem: renew certificates command creates a new ca cert without renew-ca option solution: only generate a new ca cert with the renew-ca option --- .../shoestring/commands/renew_certificates.py | 6 ++- .../shoestring/internal/CertificateFactory.py | 18 ++++++++- .../tests/commands/test_renew_certificates.py | 20 ++++++++++ .../tests/internal/test_CertificateFactory.py | 37 +++++++++++++++++++ 4 files changed, 78 insertions(+), 3 deletions(-) diff --git a/tools/shoestring/shoestring/commands/renew_certificates.py b/tools/shoestring/shoestring/commands/renew_certificates.py index f8458f216..ed02ddd7f 100644 --- a/tools/shoestring/shoestring/commands/renew_certificates.py +++ b/tools/shoestring/shoestring/commands/renew_certificates.py @@ -17,7 +17,11 @@ async def run_main(args): openssl_executor = OpensslExecutor(os.environ.get('OPENSSL_EXECUTABLE', 'openssl')) with CertificateFactory(openssl_executor, ca_key_path, config.node.ca_password) as factory: - factory.generate_ca_certificate(config.node.ca_common_name) + if args.renew_ca: + factory.generate_ca_certificate(config.node.ca_common_name) + else: # health node full certificate check needs current ca cert to pass + factory.reuse_ca_certificate(config.node.ca_common_name, directories.certificates) + factory.generate_random_node_private_key() factory.generate_node_certificate(config.node.node_common_name) factory.create_node_certificate_chain() diff --git a/tools/shoestring/shoestring/internal/CertificateFactory.py b/tools/shoestring/shoestring/internal/CertificateFactory.py index d77b8e312..adc0f7b0f 100644 --- a/tools/shoestring/shoestring/internal/CertificateFactory.py +++ b/tools/shoestring/shoestring/internal/CertificateFactory.py @@ -61,8 +61,8 @@ def _generate_random_private_key(self, name): '-algorithm', 'ed25519' ]) - def generate_ca_certificate(self, ca_cn, days=7300): - """Generates a CA certificate.""" + def _prepare_ca_certificate(self, ca_cn): + """Prepare CA certificate environment.""" if not ca_cn: raise RuntimeError('CA common name cannot be empty') @@ -106,6 +106,20 @@ def generate_ca_certificate(self, ca_cn, days=7300): with open('index.txt', 'wt', encoding='utf8') as outfile: outfile.write('') + def reuse_ca_certificate(self, ca_cn, ca_cert_path): + """Setup current CA certificate.""" + + # prepare CA config + self._prepare_ca_certificate(ca_cn) + + shutil.copy(ca_cert_path / 'ca.crt.pem', '.') + + def generate_ca_certificate(self, ca_cn, days=7300): + """Generates a CA certificate.""" + + # prepare CA config + self._prepare_ca_certificate(ca_cn) + # actually generate CA certificate self.openssl_executor.dispatch(self._add_ca_password([ 'req', diff --git a/tools/shoestring/tests/commands/test_renew_certificates.py b/tools/shoestring/tests/commands/test_renew_certificates.py index df6782472..6eb4530d8 100644 --- a/tools/shoestring/tests/commands/test_renew_certificates.py +++ b/tools/shoestring/tests/commands/test_renew_certificates.py @@ -29,6 +29,18 @@ def _create_configuration(output_directory, ca_password, ca_common_name, node_co filename=filename) +def _load_binary_file_data(filename): + with open(filename, 'rb') as infile: + return infile.read() + + +def _assert_node_full_certificate(ca_certificate_filepath, node_certificate_filepath, certificates_path): + node_full_crt_data = _load_binary_file_data(certificates_path / 'node.full.crt.pem') + node_crt_data = _load_binary_file_data(node_certificate_filepath) + ca_crt_data = _load_binary_file_data(ca_certificate_filepath) + assert node_full_crt_data == node_crt_data + ca_crt_data + + async def _assert_can_renew_node_certificate(ca_password=None): # Arrange: with tempfile.TemporaryDirectory() as output_directory: @@ -76,6 +88,9 @@ async def _assert_can_renew_node_certificate(ca_password=None): create_openssl_executor().dispatch(['verify', '-CAfile', ca_certificate_path, ca_certificate_path]) assert ca_certificate_last_modified_time == ca_certificate_path.stat().st_mtime + # verify that node.full == node.crt + ca.crt + _assert_node_full_certificate(ca_certificate_path, node_certificate_path, preparer.directories.certificates) + async def test_can_renew_node_certificate(): await _assert_can_renew_node_certificate() @@ -90,6 +105,8 @@ async def test_can_renew_node_certificate_with_ca_password(): # region CA and node certificates renewal async def _assert_can_renew_ca_and_node_certificates(ca_password=None, use_relative_path=None): + # pylint: disable=too-many-locals + # Arrange: with tempfile.TemporaryDirectory() as output_directory: config_filepath_1 = _create_configuration(output_directory, ca_password, 'ORIGINAL CA CN', 'ORIGINAL NODE CN', '1.shoestring.ini') @@ -137,6 +154,9 @@ async def _assert_can_renew_ca_and_node_certificates(ca_password=None, use_relat create_openssl_executor().dispatch(['verify', '-CAfile', ca_certificate_path, ca_certificate_path]) assert ca_certificate_last_modified_time != ca_certificate_path.stat().st_mtime + # verify that node.full == node.crt + ca.crt + _assert_node_full_certificate(ca_certificate_path, node_certificate_path, preparer.directories.certificates) + async def test_can_renew_ca_and_node_certificates(): await _assert_can_renew_ca_and_node_certificates() diff --git a/tools/shoestring/tests/internal/test_CertificateFactory.py b/tools/shoestring/tests/internal/test_CertificateFactory.py index 52a587823..f18b1bc79 100644 --- a/tools/shoestring/tests/internal/test_CertificateFactory.py +++ b/tools/shoestring/tests/internal/test_CertificateFactory.py @@ -1,6 +1,7 @@ import datetime import os import re +import shutil import tempfile import unittest from pathlib import Path @@ -269,6 +270,42 @@ def test_cannot_generate_node_certificate_without_common_name(self): with self.assertRaisesRegex(RuntimeError, 'Node common name cannot be empty'): factory.generate_node_certificate('') + def test_can_reuse_ca_certificate(self): + def _load_binary_file_data(filename): + with open(filename, 'rb') as infile: + return infile.read() + + # Arrange: + with tempfile.TemporaryDirectory() as package_directory: + ca_certificate_filepath = Path(package_directory) / 'ca.crt.pem' + + # - create a CA private key + with tempfile.TemporaryDirectory() as certificate_directory: + ca_private_key_path = self._create_ca_private_key(certificate_directory) + + with CertificateFactory(self._create_executor(), ca_private_key_path) as factory: + # create CA certificate + ca_common_name = 'my CA common name' + factory.generate_ca_certificate(ca_common_name) + factory.package(package_directory) + + # Sanity: + shutil.rmtree(certificate_directory) + assert not ca_private_key_path.exists() + + # Act: + with CertificateFactory(self._create_executor(), ca_private_key_path) as renew_factory: + renew_factory.reuse_ca_certificate(ca_common_name, Path(package_directory)) + + # Assert: + assert Path('ca.crt.pem').exists() + assert Path('ca.cnf').exists() + assert ca_certificate_filepath.exists() + + original_ca_crt_data = _load_binary_file_data(ca_certificate_filepath) + copied_ca_crt_data = _load_binary_file_data('ca.crt.pem') + assert original_ca_crt_data == copied_ca_crt_data + # endregion # region packaging