From 611f3ed3a98a7c697c333d1188cf34e8471ff0d6 Mon Sep 17 00:00:00 2001 From: "patchback[bot]" <45432694+patchback[bot]@users.noreply.github.com> Date: Mon, 5 Apr 2021 18:45:08 +0200 Subject: [PATCH] replace inline clear password by environment variable (#2177) (#2181) * replace inline clear password by environment variable on a per-command basis. * add changelog fragment * update related unit tests * Update changelogs/fragments/2177-java_keystore_1668_dont_expose_secrets_on_cmdline.yml Co-authored-by: Felix Fontein * fix unit test: force result without lambda Co-authored-by: Felix Fontein (cherry picked from commit eb851d420857981f2753869234e2c6fefa2a46f4) Co-authored-by: quidame --- ...tore_1668_dont_expose_secrets_on_cmdline.yml | 4 ++++ plugins/modules/system/java_keystore.py | 17 ++++++++++------- .../modules/system/test_java_keystore.py | 8 ++++---- 3 files changed, 18 insertions(+), 11 deletions(-) create mode 100644 changelogs/fragments/2177-java_keystore_1668_dont_expose_secrets_on_cmdline.yml diff --git a/changelogs/fragments/2177-java_keystore_1668_dont_expose_secrets_on_cmdline.yml b/changelogs/fragments/2177-java_keystore_1668_dont_expose_secrets_on_cmdline.yml new file mode 100644 index 0000000000..0d961a53ac --- /dev/null +++ b/changelogs/fragments/2177-java_keystore_1668_dont_expose_secrets_on_cmdline.yml @@ -0,0 +1,4 @@ +--- +security_fixes: + - "java_keystore - pass secret to keytool through an environment variable to not expose it as a + commandline argument (https://github.com/ansible-collections/community.general/issues/1668)." diff --git a/plugins/modules/system/java_keystore.py b/plugins/modules/system/java_keystore.py index 82bd03678c..feab757f58 100644 --- a/plugins/modules/system/java_keystore.py +++ b/plugins/modules/system/java_keystore.py @@ -146,8 +146,9 @@ def read_certificate_fingerprint(module, openssl_bin, certificate_path): def read_stored_certificate_fingerprint(module, keytool_bin, alias, keystore_path, keystore_password): - stored_certificate_fingerprint_cmd = [keytool_bin, "-list", "-alias", alias, "-keystore", keystore_path, "-storepass", keystore_password, "-v"] - (rc, stored_certificate_fingerprint_out, stored_certificate_fingerprint_err) = run_commands(module, stored_certificate_fingerprint_cmd) + stored_certificate_fingerprint_cmd = [keytool_bin, "-list", "-alias", alias, "-keystore", keystore_path, "-storepass:env", "STOREPASS", "-v"] + (rc, stored_certificate_fingerprint_out, stored_certificate_fingerprint_err) = run_commands( + module, stored_certificate_fingerprint_cmd, environ_update=dict(STOREPASS=keystore_password)) if rc != 0: if "keytool error: java.lang.Exception: Alias <%s> does not exist" % alias not in stored_certificate_fingerprint_out: return module.fail_json(msg=stored_certificate_fingerprint_out, @@ -168,8 +169,8 @@ def read_stored_certificate_fingerprint(module, keytool_bin, alias, keystore_pat return stored_certificate_match.group(1) -def run_commands(module, cmd, data=None, check_rc=True): - return module.run_command(cmd, check_rc=check_rc, data=data) +def run_commands(module, cmd, data=None, environ_update=None, check_rc=True): + return module.run_command(cmd, check_rc=check_rc, data=data, environ_update=environ_update) def create_path(): @@ -236,10 +237,12 @@ def create_jks(module, name, openssl_bin, keytool_bin, keystore_path, password, "-srckeystore", keystore_p12_path, "-srcstoretype", "pkcs12", "-alias", name, - "-deststorepass", password, - "-srcstorepass", password, + "-deststorepass:env", "STOREPASS", + "-srcstorepass:env", "STOREPASS", "-noprompt"] - (rc, import_keystore_out, import_keystore_err) = run_commands(module, import_keystore_cmd, data=None) + + (rc, import_keystore_out, import_keystore_err) = run_commands(module, import_keystore_cmd, data=None, + environ_update=dict(STOREPASS=password)) if rc == 0: update_jks_perm(module, keystore_path) return module.exit_json(changed=True, diff --git a/tests/unit/plugins/modules/system/test_java_keystore.py b/tests/unit/plugins/modules/system/test_java_keystore.py index 68863c149e..409e956799 100644 --- a/tests/unit/plugins/modules/system/test_java_keystore.py +++ b/tests/unit/plugins/modules/system/test_java_keystore.py @@ -71,14 +71,14 @@ class TestCreateJavaKeystore(ModuleTestCase): with patch('os.remove', return_value=True): self.create_path.side_effect = ['/tmp/tmpgrzm2ah7'] self.create_file.side_effect = ['/tmp/etacifitrec', '/tmp/yek_etavirp'] - self.run_commands.side_effect = lambda module, cmd, data: (0, '', '') + self.run_commands.side_effect = [(0, '', ''), (0, '', '')] create_jks(module, "test", "openssl", "keytool", "/path/to/keystore.jks", "changeit", "") module.exit_json.assert_called_once_with( changed=True, cmd=["keytool", "-importkeystore", "-destkeystore", "/path/to/keystore.jks", "-srckeystore", "/tmp/tmpgrzm2ah7", "-srcstoretype", "pkcs12", "-alias", "test", - "-deststorepass", "changeit", "-srcstorepass", "changeit", "-noprompt"], + "-deststorepass:env", "STOREPASS", "-srcstorepass:env", "STOREPASS", "-noprompt"], msg='', rc=0, stdout_lines='' @@ -173,7 +173,7 @@ class TestCreateJavaKeystore(ModuleTestCase): cmd=["keytool", "-importkeystore", "-destkeystore", "/path/to/keystore.jks", "-srckeystore", "/tmp/tmpgrzm2ah7", "-srcstoretype", "pkcs12", "-alias", "test", - "-deststorepass", "changeit", "-srcstorepass", "changeit", "-noprompt"], + "-deststorepass:env", "STOREPASS", "-srcstorepass:env", "STOREPASS", "-noprompt"], msg='', rc=1 ) @@ -306,7 +306,7 @@ class TestCertChanged(ModuleTestCase): self.run_commands.side_effect = [(0, 'foo: wxyz:9876:stuv', ''), (1, '', 'Oops')] cert_changed(module, "openssl", "keytool", "/path/to/keystore.jks", "changeit", 'foo') module.fail_json.assert_called_with( - cmd=["keytool", "-list", "-alias", "foo", "-keystore", "/path/to/keystore.jks", "-storepass", "changeit", "-v"], + cmd=["keytool", "-list", "-alias", "foo", "-keystore", "/path/to/keystore.jks", "-storepass:env", "STOREPASS", "-v"], msg='', err='Oops', rc=1