From a21c5ea3d68e282b39f3f52e889d123a2f05dc7d Mon Sep 17 00:00:00 2001 From: Gene Wood Date: Mon, 10 Nov 2025 12:24:26 +0000 Subject: [PATCH] Add check if simple_test private key is being used * Add check if the user is using the simple_test private key * Change unit tests to use simple_example instead of simple_test * Break up large check_update_private_key_and_ca_crt method Fixes #10 --- hsm_orchestrator/__init__.py | 34 ++++++++++++++---- tests/files/example.cnf | 2 +- tests/setup.py | 10 +++--- tests/test_check.py | 68 ++++++++++++++++++++++++++++++++++-- tests/test_pull.py | 8 +++-- tests/test_push.py | 4 ++- 6 files changed, 107 insertions(+), 19 deletions(-) diff --git a/hsm_orchestrator/__init__.py b/hsm_orchestrator/__init__.py index 4474728..411f1f5 100644 --- a/hsm_orchestrator/__init__.py +++ b/hsm_orchestrator/__init__.py @@ -115,8 +115,8 @@ def get_openssl_cnf_config(self) -> None: "", inline_comment ) - def check_update_private_key_and_ca_crt(self) -> None: - """Validate and update the OpenSSL 'private_key' and 'certificate' values. + def check_update_private_key(self) -> None: + """Validate and update the OpenSSL 'private_key' value. Prompts the user if the referenced files are missing or invalid and writes updates back to the OpenSSL configuration file. @@ -129,10 +129,6 @@ def check_update_private_key_and_ca_crt(self) -> None: private_key_name = self.openssl_config[ self.openssl_config["ca"]["default_ca"] ].get("private_key") - # 'certificate' OpenSSL config value is equivalent to the "-cert" CLI argument - ca_crt_name = self.openssl_config[self.openssl_config["ca"]["default_ca"]].get( - "certificate" - ) ca_dir = Path(self.repo_dir / Path("certificate-authorities")) possible_private_key_names = [x.name for x in ca_dir.iterdir() if x.is_dir()] @@ -150,6 +146,12 @@ def check_update_private_key_and_ca_crt(self) -> None: f" '{private_key_name}' doesn't map to a directory in the" " hsm/certificate-authorities/ directory." ) + elif private_key_name == "simple_test": + prompt_for_private_key = Confirm.ask( + f"The 'private_key' in the {self.cnf_file} file is set to 'simple_test'" + " which is a test private key. [q]Would you like to change it to" + " something different?[/q]" + ) if prompt_for_private_key: private_key_name = Prompt.ask( "[q]What would you like to change the 'private_key' value in the" @@ -161,7 +163,24 @@ def check_update_private_key_and_ca_crt(self) -> None: ] = private_key_name self.openssl_config.write() + def check_update_ca_crt(self) -> None: + """Validate and update the OpenSSL 'certificate' value. + + Prompts the user if the referenced files are missing or invalid and writes + updates back to the OpenSSL configuration file. + + :returns: None + + """ + ca_crt_name = self.openssl_config[self.openssl_config["ca"]["default_ca"]].get( + "certificate" + ) + ca_dir = Path(self.repo_dir / Path("certificate-authorities")) + private_key_name = self.openssl_config[ + self.openssl_config["ca"]["default_ca"] + ].get("private_key") private_key_path = Path(ca_dir / Path(private_key_name)) + prompt_for_certificate = False if not ca_crt_name: prompt_for_certificate = True @@ -331,7 +350,8 @@ def check_update_cnf_file(self) -> None: " 'ca' section which is required." ) self.check_update_start_end_date() - self.check_update_private_key_and_ca_crt() + self.check_update_private_key() + self.check_update_ca_crt() self.check_update_unique_subject() self.check_ca_files() diff --git a/tests/files/example.cnf b/tests/files/example.cnf index 5756535..2d35e43 100644 --- a/tests/files/example.cnf +++ b/tests/files/example.cnf @@ -22,7 +22,7 @@ serial = $dir/serial # The current serial number crlnumber = $dir/crlnumber # the current crl number # must be commented out to leave a V1 CRL crl = $dir/crl.pem # The current CRL -private_key = simple_test # The private key +private_key = simple_example # The private key x509_extensions = mozilla_amo_intermediate_ca # The extensions to add to the cert diff --git a/tests/setup.py b/tests/setup.py index 197452e..818fc96 100644 --- a/tests/setup.py +++ b/tests/setup.py @@ -114,17 +114,17 @@ def set_up_environment( Path(repo_dir / "certs_issued").mkdir() Path(repo_dir / "certs_issued" / "test").mkdir() Path(repo_dir / "certificate-authorities").mkdir() - Path(repo_dir / "certificate-authorities" / "simple_test").mkdir() - Path(repo_dir / "certificate-authorities" / "simple_test" / "test").mkdir() + Path(repo_dir / "certificate-authorities" / "simple_example").mkdir() + Path(repo_dir / "certificate-authorities" / "simple_example" / "test").mkdir() Path( - repo_dir / "certificate-authorities" / "simple_test" / "test" / "test.crt" + repo_dir / "certificate-authorities" / "simple_example" / "test" / "test.crt" ).touch() with Path( - repo_dir / "certificate-authorities" / "simple_test" / "test" / "serial" + repo_dir / "certificate-authorities" / "simple_example" / "test" / "serial" ).open("w") as f: f.write("01") with Path( - repo_dir / "certificate-authorities" / "simple_test" / "test" / "index.txt" + repo_dir / "certificate-authorities" / "simple_example" / "test" / "index.txt" ).open("w") as f: f.write( "V\t22511013200827Z\t\t01\tunknown\t/C=US/O=Mozilla Corporation/OU=Mozilla" diff --git a/tests/test_check.py b/tests/test_check.py index 92f1320..f65a1cf 100644 --- a/tests/test_check.py +++ b/tests/test_check.py @@ -500,7 +500,7 @@ def test_missing_private_key_setting(tmp_path, datafiles, monkeypatch): result = runner.invoke( main, ["check", "--skip-git-fetch", "--config", env["orchestrator_config_file"]], - input="simple_test\n", + input="simple_example\n", ) re_search(r"You must set the 'private_key' value in the .* file", result.output) @@ -524,7 +524,71 @@ def test_wrong_private_key_value(tmp_path, datafiles, monkeypatch): result = runner.invoke( main, ["check", "--skip-git-fetch", "--config", env["orchestrator_config_file"]], - input="simple_test\n", + input="simple_example\n", + ) + re_search( + r"What would you like to change the 'private_key' value in the" + r" .*example.cnf to\? \[", + result.output, + ) + + +@pytest.mark.datafiles(FIXTURE_DIR / "example.csr", FIXTURE_DIR / "example.cnf") +def test_using_test_private_key(tmp_path, datafiles, monkeypatch): + runner = CliRunner() + with runner.isolated_filesystem(tmp_path): + env = set_up_environment(tmp_path, datafiles, monkeypatch) + repo_dir = env["repo_dir"] + Path(repo_dir / "certificate-authorities" / "simple_test").mkdir() + Path(repo_dir / "certificate-authorities" / "simple_test" / "test").mkdir() + Path( + repo_dir / "certificate-authorities" / "simple_test" / "test" / "test.crt" + ).touch() + with Path( + repo_dir / "certificate-authorities" / "simple_test" / "test" / "serial" + ).open("w") as f: + f.write("01") + with Path( + repo_dir / "certificate-authorities" / "simple_test" / "test" / "index.txt" + ).open("w") as f: + f.write( + "V\t22511013200827Z\t\t01\tunknown\t/C=US/O=Mozilla" + " Corporation/OU=Mozilla AMO Production Signing Service/CN=test" + ) + with ( + Path(datafiles / "example.cnf").open("r") as in_file, + env["cnf_file"].open("w") as out_file, + ): + for line in in_file: + if line.startswith("private_key"): + out_file.write("private_key = simple_test # The private key\n") + else: + out_file.write(line) + result = runner.invoke( + main, + ["check", "--skip-git-fetch", "--config", env["orchestrator_config_file"]], + input="n\n", + ) + re_search( + r"The 'private_key' in the .* file is set to 'simple_test' which is a test" + r" private key\. Would you like to change it to something different\?", + result.output, + ) + re_search( + r"What would you like to change the 'private_key' value in the" + r" .*example.cnf to\? \[", + result.output, + reverse=True, + ) + result = runner.invoke( + main, + ["check", "--skip-git-fetch", "--config", env["orchestrator_config_file"]], + input="y\nsimple_example\n", + ) + re_search( + r"The 'private_key' in the .* file is set to 'simple_test' which is a test" + r" private key\. Would you like to change it to something different\?", + result.output, ) re_search( r"What would you like to change the 'private_key' value in the" diff --git a/tests/test_pull.py b/tests/test_pull.py index 471bdea..f18d8da 100644 --- a/tests/test_pull.py +++ b/tests/test_pull.py @@ -192,12 +192,12 @@ def test_file_actions_table_output(tmp_path, datafiles, monkeypatch): result_lines, ) re_search( - r"repo[/\\]certificate-authorities[/\\]simple_test[/\\]test *:" + r"repo[/\\]certificate-authorities[/\\]simple_example[/\\]test *:" r" usb[/\\]serial$", result_lines, ) re_search( - r"repo[/\\]certificate-authorities[/\\]simple_test[/\\]test *:" + r"repo[/\\]certificate-authorities[/\\]simple_example[/\\]test *:" r" usb[/\\]index\.txt$", result_lines, ) @@ -243,7 +243,9 @@ def test_file_actions(tmp_path, datafiles, monkeypatch): ).exists() assert Path(env["usb_mount_point"] / "unrelated-directory").exists() - ca_path = env["repo_dir"] / "certificate-authorities" / "simple_test" / "test" + ca_path = ( + env["repo_dir"] / "certificate-authorities" / "simple_example" / "test" + ) assert Path(ca_path / "serial").exists() assert Path(ca_path / "index.txt").exists() cert_path = env["repo_dir"] / "certs_issued" / "test" diff --git a/tests/test_push.py b/tests/test_push.py index 4651bd8..ae48ef0 100644 --- a/tests/test_push.py +++ b/tests/test_push.py @@ -35,7 +35,9 @@ def test_selecting_usb_stick(tmp_path, datafiles, monkeypatch): @pytest.mark.datafiles(FIXTURE_DIR / "example.csr", FIXTURE_DIR / "example.cnf") -def test_selecting_usb_stick_with_unsupported_filesystem(tmp_path, datafiles, monkeypatch): +def test_selecting_usb_stick_with_unsupported_filesystem( + tmp_path, datafiles, monkeypatch +): runner = CliRunner() with runner.isolated_filesystem(tmp_path): env = set_up_environment(tmp_path, datafiles, create_usb_stick=False)