From 0f7e39fa1adc93ddbd28ce7bf600c80c8e945db4 Mon Sep 17 00:00:00 2001 From: "patchback[bot]" <45432694+patchback[bot]@users.noreply.github.com> Date: Sat, 22 May 2021 13:51:24 +0200 Subject: [PATCH] java_cert - fix incorrect certificate alias on pkcs12 import (#2560) (#2580) * fix wrong certificate alias used when importing pkcs12, modify error output, stdout is more relevant than stderr * add changelog fragment * fix changelog fragment (cherry picked from commit 8f083d5d85ddf4f98aee8221bf4cb3c4a721e7d6) Co-authored-by: absynth76 <58172580+absynth76@users.noreply.github.com> --- .../2560-java_cert-pkcs12-alias-bugfix.yml | 2 + plugins/modules/system/java_cert.py | 4 +- .../targets/java_cert/tasks/state_change.yml | 138 ++++++++++++------ 3 files changed, 98 insertions(+), 46 deletions(-) create mode 100644 changelogs/fragments/2560-java_cert-pkcs12-alias-bugfix.yml diff --git a/changelogs/fragments/2560-java_cert-pkcs12-alias-bugfix.yml b/changelogs/fragments/2560-java_cert-pkcs12-alias-bugfix.yml new file mode 100644 index 0000000000..471962d74f --- /dev/null +++ b/changelogs/fragments/2560-java_cert-pkcs12-alias-bugfix.yml @@ -0,0 +1,2 @@ +bugfixes: + - "java_cert - fix issue with incorrect alias used on PKCS#12 certificate import (https://github.com/ansible-collections/community.general/pull/2560)." diff --git a/plugins/modules/system/java_cert.py b/plugins/modules/system/java_cert.py index ad56358034..1c507f9277 100644 --- a/plugins/modules/system/java_cert.py +++ b/plugins/modules/system/java_cert.py @@ -278,7 +278,7 @@ def _export_public_cert_from_pkcs12(module, executable, pkcs_file, alias, passwo (export_rc, export_stdout, export_err) = module.run_command(export_cmd, data=password, check_rc=False) if export_rc != 0: - module.fail_json(msg="Internal module failure, cannot extract public certificate from pkcs12, error: %s" % export_err, + module.fail_json(msg="Internal module failure, cannot extract public certificate from pkcs12, error: %s" % export_stdout, rc=export_rc) with open(dest, 'w') as f: @@ -498,7 +498,7 @@ def main(): if pkcs12_path: # Extracting certificate with openssl - _export_public_cert_from_pkcs12(module, executable, pkcs12_path, cert_alias, pkcs12_pass, new_certificate) + _export_public_cert_from_pkcs12(module, executable, pkcs12_path, pkcs12_alias, pkcs12_pass, new_certificate) elif path: # Extracting the X509 digest is a bit easier. Keytool will print the PEM diff --git a/tests/integration/targets/java_cert/tasks/state_change.yml b/tests/integration/targets/java_cert/tasks/state_change.yml index 3c37fc6727..8cee41106f 100644 --- a/tests/integration/targets/java_cert/tasks/state_change.yml +++ b/tests/integration/targets/java_cert/tasks/state_change.yml @@ -4,52 +4,11 @@ args: creates: "{{ test_key_path }}" -- name: Create the test keystore - java_keystore: - name: placeholder - dest: "{{ test_keystore2_path }}" - password: "{{ test_keystore2_password }}" - private_key: "{{ lookup('file', '{{ test_key_path }}') }}" - certificate: "{{ lookup('file', '{{ test_cert_path }}') }}" - - name: Generate the self signed cert we will use for testing command: openssl req -x509 -newkey rsa:4096 -keyout '{{ test_key2_path }}' -out '{{ test_cert2_path }}' -days 365 -nodes -subj '/CN=localhost' args: creates: "{{ test_key2_path }}" -- name: | - Import the newly created certificate. This is our main test. - If the java_cert has been updated properly, then this task will report changed each time - since the module will be comparing the hash of the certificate instead of validating that the alias - simply exists - java_cert: - cert_alias: test_cert - cert_path: "{{ test_cert2_path }}" - keystore_path: "{{ test_keystore2_path }}" - keystore_pass: "{{ test_keystore2_password }}" - state: present - register: result_x509_changed - -- name: Verify the x509 status has changed - assert: - that: - - result_x509_changed is changed - -- name: | - We also want to make sure that the status doesnt change if we import the same cert - java_cert: - cert_alias: test_cert - cert_path: "{{ test_cert2_path }}" - keystore_path: "{{ test_keystore2_path }}" - keystore_pass: "{{ test_keystore2_password }}" - state: present - register: result_x509_succeeded - -- name: Verify the x509 status is ok - assert: - that: - - result_x509_succeeded is succeeded - - name: Create the pkcs12 archive from the test x509 cert command: > openssl pkcs12 @@ -70,6 +29,97 @@ -out {{ test_pkcs2_path }} -passout pass:"{{ test_keystore2_password }}" +- name: try to create the test keystore based on the just created pkcs12, keystore_create flag not enabled + java_cert: + cert_alias: test_pkcs12_cert + pkcs12_alias: test_pkcs12_cert + pkcs12_path: "{{ test_pkcs_path }}" + pkcs12_password: "{{ test_keystore2_password }}" + keystore_path: "{{ test_keystore2_path }}" + keystore_pass: "{{ test_keystore2_password }}" + ignore_errors: true + register: result_x509_changed + +- name: Verify the x509 status is failed + assert: + that: + - result_x509_changed is failed + +- name: Create the test keystore based on the just created pkcs12 + java_cert: + cert_alias: test_pkcs12_cert + pkcs12_alias: test_pkcs12_cert + pkcs12_path: "{{ test_pkcs_path }}" + pkcs12_password: "{{ test_keystore2_password }}" + keystore_path: "{{ test_keystore2_path }}" + keystore_pass: "{{ test_keystore2_password }}" + keystore_create: yes + +- name: try to import from pkcs12 a non existing alias + java_cert: + cert_alias: test_pkcs12_cert + pkcs12_alias: non_existing_alias + pkcs12_path: "{{ test_pkcs_path }}" + pkcs12_password: "{{ test_keystore2_password }}" + keystore_path: "{{ test_keystore2_path }}" + keystore_pass: "{{ test_keystore2_password }}" + keystore_create: yes + ignore_errors: yes + register: result_x509_changed + +- name: Verify the x509 status is failed + assert: + that: + - result_x509_changed is failed + +- name: import initial test certificate from file path + java_cert: + cert_alias: test_cert + cert_path: "{{ test_cert_path }}" + keystore_path: "{{ test_keystore2_path }}" + keystore_pass: "{{ test_keystore2_password }}" + keystore_create: yes + state: present + register: result_x509_changed + +- name: Verify the x509 status is changed + assert: + that: + - result_x509_changed is changed + +- name: | + Import the newly created certificate. This is our main test. + If the java_cert has been updated properly, then this task will report changed each time + since the module will be comparing the hash of the certificate instead of validating that the alias + simply exists + java_cert: + cert_alias: test_cert + cert_path: "{{ test_cert2_path }}" + keystore_path: "{{ test_keystore2_path }}" + keystore_pass: "{{ test_keystore2_password }}" + state: present + register: result_x509_changed + +- name: Verify the x509 status is changed + assert: + that: + - result_x509_changed is changed + +- name: | + We also want to make sure that the status doesnt change if we import the same cert + java_cert: + cert_alias: test_cert + cert_path: "{{ test_cert2_path }}" + keystore_path: "{{ test_keystore2_path }}" + keystore_pass: "{{ test_keystore2_password }}" + state: present + register: result_x509_succeeded + +- name: Verify the x509 status is ok + assert: + that: + - result_x509_succeeded is succeeded + - name: > Ensure the original pkcs12 cert is in the keystore java_cert: @@ -83,7 +133,7 @@ - name: | Perform the same test, but we will now be testing the pkcs12 functionality - If we add a different pkcs12 cert with the same alias, we should have a chnaged result, NOT the same + If we add a different pkcs12 cert with the same alias, we should have a changed result, NOT the same java_cert: cert_alias: test_pkcs12_cert pkcs12_alias: test_pkcs12_cert @@ -94,7 +144,7 @@ state: present register: result_pkcs12_changed -- name: Verify the pkcs12 status has changed +- name: Verify the pkcs12 status is changed assert: that: - result_pkcs12_changed is changed @@ -155,7 +205,7 @@ that: - result_x509_absent is changed -- name: Ensure we can remove the pkcs12 archive +- name: Ensure we can remove the certificate imported from pkcs12 archive java_cert: cert_alias: test_pkcs12_cert keystore_path: "{{ test_keystore2_path }}"