openssl_pkcs12: Add idempotency checks

changelogs/fragments/54633-openssl_pkcs12_idempotency_fixes.yaml

@@ -0,0 +1,2 @@
+minor_changes:
+- "openssl_pkcs12 - Fixed idempotency checks, the module will regenerate the pkcs12 file if any of the parameters differ from the ones in the file. The ``ca_certificates`` parameter has been renamed to ``other_certificates``. "

docs/docsite/rst/porting_guides/porting_guide_2.8.rst

@@ -333,6 +333,8 @@ Noteworthy module changes
 * The ``win_dsc`` module will now validate the input options for a DSC resource. In previous versions invalid options
   would be ignored but are now not.
 
+* The ``openssl_pkcs12`` module will now regenerate the pkcs12 file if there are differences between the file on disk and the parameters passed to the module.
+
 Plugins
 =======
 

lib/ansible/modules/crypto/openssl_pkcs12.py

@@ -29,10 +29,11 @@
         type: str
         default: export
         choices: [ export, parse ]
-    ca_certificates:
+    other_certificates:
         description:
-            - List of CA certificate to include.
+            - List of other certificates to include. Pre 2.8 this parameter was called C(ca_certificates)
         type: list
+        aliases: [ ca_certificates ]
     certificate_path:
         description:
             - The path to read certificates and private keys from.
@@ -111,7 +112,7 @@
     friendly_name: raclette
     privatekey_path: /opt/certs/keys/key.pem
     certificate_path: /opt/certs/cert.pem
-    ca_certificates: /opt/certs/ca.pem
+    other_certificates: /opt/certs/ca.pem
     state: present
 
 - name: Change PKCS#12 file permission
@@ -121,7 +122,7 @@
     friendly_name: raclette
     privatekey_path: /opt/certs/keys/key.pem
     certificate_path: /opt/certs/cert.pem
-    ca_certificates: /opt/certs/ca.pem
+    other_certificates: /opt/certs/ca.pem
     state: present
     mode: '0600'
 
@@ -133,7 +134,7 @@
     friendly_name: raclette
     privatekey_path: /opt/certs/keys/key.pem
     certificate_path: /opt/certs/cert.pem
-    ca_certificates: /opt/certs/ca.pem
+    other_certificates: /opt/certs/ca.pem
     state: present
     mode: '0600'
     force: yes
@@ -201,7 +202,7 @@ def __init__(self, module):
             module.check_mode
         )
         self.action = module.params['action']
-        self.ca_certificates = module.params['ca_certificates']
+        self.other_certificates = module.params['other_certificates']
         self.certificate_path = module.params['certificate_path']
         self.friendly_name = module.params['friendly_name']
         self.iter_size = module.params['iter_size']
@@ -237,7 +238,50 @@ def _check_pkey_passphrase():
         if not state_and_perms:
             return state_and_perms
 
-        return _check_pkey_passphrase
+        if os.path.exists(self.path) and module.params['action'] == 'export':
+            dummy = self.generate(module)
+            self.src = self.path
+            try:
+                pkcs12_privatekey, pkcs12_certificate, pkcs12_other_certificates, pkcs12_friendly_name = self.parse()
+            except crypto.Error:
+                return False
+            if (pkcs12_privatekey is not None) and (self.privatekey_path is not None):
+                expected_pkey = crypto.dump_privatekey(crypto.FILETYPE_PEM,
+                                                       self.pkcs12.get_privatekey())
+                if pkcs12_privatekey != expected_pkey:
+                    return False
+            elif bool(pkcs12_privatekey) != bool(self.privatekey_path):
+                return False
+
+            if (pkcs12_certificate is not None) and (self.certificate_path is not None):
+
+                expected_cert = crypto.dump_certificate(crypto.FILETYPE_PEM,
+                                                        self.pkcs12.get_certificate())
+                if pkcs12_certificate != expected_cert:
+                    return False
+            elif bool(pkcs12_certificate) != bool(self.certificate_path):
+                return False
+
+            if (pkcs12_other_certificates is not None) and (self.other_certificates is not None):
+                expected_other_certs = [crypto.dump_certificate(crypto.FILETYPE_PEM,
+                                                                other_cert) for other_cert in self.pkcs12.get_ca_certificates()]
+                if set(pkcs12_other_certificates) != set(expected_other_certs):
+                    return False
+            elif bool(pkcs12_other_certificates) != bool(self.other_certificates):
+                return False
+
+            if pkcs12_privatekey:
+                # This check is required because pyOpenSSL will not return a firendly name
+                # if the private key is not set in the file
+                if ((self.pkcs12.get_friendlyname() is not None) and (pkcs12_friendly_name is not None)):
+                    if self.pkcs12.get_friendlyname() != pkcs12_friendly_name:
+                        return False
+                elif bool(self.pkcs12.get_friendlyname()) != bool(pkcs12_friendly_name):
+                    return False
+        else:
+            return False
+
+        return _check_pkey_passphrase()
 
     def dump(self):
         """Serialize the object into a dictionary."""
@@ -254,13 +298,12 @@ def dump(self):
 
     def generate(self, module):
         """Generate PKCS#12 file archive."""
-
         self.pkcs12 = crypto.PKCS12()
 
-        if self.ca_certificates:
-            ca_certs = [crypto_utils.load_certificate(ca_cert) for ca_cert
-                        in self.ca_certificates]
-            self.pkcs12.set_ca_certificates(ca_certs)
+        if self.other_certificates:
+            other_certs = [crypto_utils.load_certificate(other_cert) for other_cert
+                           in self.other_certificates]
+            self.pkcs12.set_ca_certificates(other_certs)
 
         if self.certificate_path:
             self.pkcs12.set_certificate(crypto_utils.load_certificate(
@@ -278,20 +321,14 @@ def generate(self, module):
             except crypto_utils.OpenSSLBadPassphraseError as exc:
                 raise PkcsError(exc)
 
-        if self.backup:
-            self.backup_file = module.backup_local(self.path)
-        crypto_utils.write_file(
-            module,
-            self.pkcs12.export(self.passphrase, self.iter_size, self.maciter_size),
-            0o600
-        )
+        return self.pkcs12.export(self.passphrase, self.iter_size, self.maciter_size)
 
     def remove(self, module):
         if self.backup:
             self.backup_file = module.backup_local(self.path)
         super(Pkcs, self).remove(module)
 
-    def parse(self, module):
+    def parse(self):
         """Read PKCS#12 file."""
 
         try:
@@ -303,17 +340,29 @@ def parse(self, module):
                                           p12.get_privatekey())
             crt = crypto.dump_certificate(crypto.FILETYPE_PEM,
                                           p12.get_certificate())
+            other_certs = []
+            if p12.get_ca_certificates() is not None:
+                other_certs = [crypto.dump_certificate(crypto.FILETYPE_PEM,
+                                                       other_cert) for other_cert in p12.get_ca_certificates()]
 
-            crypto_utils.write_file(module, b'%s%s' % (pkey, crt))
+            friendly_name = p12.get_friendlyname()
+
+            return (pkey, crt, other_certs, friendly_name)
 
         except IOError as exc:
             raise PkcsError(exc)
 
+    def write(self, module, content, mode=None):
+        """Write the PKCS#12 file."""
+        if self.backup:
+            self.backup_file = module.backup_local(self.path)
+        crypto_utils.write_file(module, content, mode)
+
 
 def main():
     argument_spec = dict(
         action=dict(type='str', default='export', choices=['export', 'parse']),
-        ca_certificates=dict(type='list', elements='path'),
+        other_certificates=dict(type='list', elements='path', aliases=['ca_certificates']),
         certificate_path=dict(type='path'),
         force=dict(type='bool', default=False),
         friendly_name=dict(type='str', aliases=['name']),
@@ -363,10 +412,13 @@ def main():
                 if module.params['action'] == 'export':
                     if not module.params['friendly_name']:
                         module.fail_json(msg='Friendly_name is required')
-                    pkcs12.generate(module)
+                    pkcs12_content = pkcs12.generate(module)
+                    pkcs12.write(module, pkcs12_content, 0o600)
                     changed = True
                 else:
-                    pkcs12.parse(module)
+                    pkey, cert, other_certs, friendly_name = pkcs12.parse()
+                    dump_content = '%s%s%s' % (to_native(pkey), to_native(cert), to_native(b''.join(other_certs)))
+                    pkcs12.write(module, to_bytes(dump_content))
 
             file_args = module.load_file_common_arguments(module.params)
             if module.set_fs_attributes_if_different(file_args, changed):

test/integration/targets/openssl_pkcs12/tasks/impl.yml

@@ -1,21 +1,48 @@
 ---
 - block:
-  - name: 'Generate privatekey with'
+  - name: 'Generate privatekey'
     openssl_privatekey:
       path: "{{ output_dir }}/ansible_pkey.pem"
 
-  - name: 'Generate CSR with'
+  - name: 'Generate privatekey2'
+    openssl_privatekey:
+      path: "{{ output_dir }}/ansible_pkey2.pem"
+
+  - name: 'Generate privatekey3'
+    openssl_privatekey:
+      path: "{{ output_dir }}/ansible_pkey3.pem"
+
+  - name: 'Generate CSR'
     openssl_csr:
       path: "{{ output_dir }}/ansible.csr"
       privatekey_path: "{{ output_dir }}/ansible_pkey.pem"
       commonName: 'www.ansible.com'
 
+  - name: 'Generate CSR 2'
+    openssl_csr:
+      path: "{{ output_dir }}/ansible2.csr"
+      privatekey_path: "{{ output_dir }}/ansible_pkey2.pem"
+      commonName: 'www2.ansible.com'
+
+  - name: 'Generate CSR 3'
+    openssl_csr:
+      path: "{{ output_dir }}/ansible3.csr"
+      privatekey_path: "{{ output_dir }}/ansible_pkey3.pem"
+      commonName: 'www3.ansible.com'
+
   - name: 'Generate certificate'
     openssl_certificate:
-      path: "{{ output_dir }}/ansible.crt"
-      privatekey_path: "{{ output_dir }}/ansible_pkey.pem"
-      csr_path: "{{ output_dir }}/ansible.csr"
+      path: "{{ output_dir }}/{{ item.name }}.crt"
+      privatekey_path: "{{ output_dir }}/{{ item.pkey }}"
+      csr_path: "{{ output_dir }}/{{ item.name }}.csr"
       provider: selfsigned
+    loop:
+      - name: ansible
+        pkey: ansible_pkey.pem
+      - name: ansible2
+        pkey: ansible_pkey2.pem
+      - name: ansible3
+        pkey: ansible_pkey3.pem
 
   - name: 'Generate PKCS#12 file'
     openssl_pkcs12:
@@ -26,6 +53,15 @@
       state: present
     register: p12_standard
 
+  - name: 'Generate PKCS#12 file again, idempotency'
+    openssl_pkcs12:
+      path: "{{ output_dir }}/ansible.p12"
+      friendly_name: 'abracadabra'
+      privatekey_path: "{{ output_dir }}/ansible_pkey.pem"
+      certificate_path: "{{ output_dir }}/ansible.crt"
+      state: present
+    register: p12_standard_idempotency
+
   - name: 'Generate PKCS#12 file (force)'
     openssl_pkcs12:
       path: "{{ output_dir }}/ansible.p12"
@@ -54,6 +90,37 @@
       action: 'parse'
       state: 'present'
 
+  - name: 'Generate PKCS#12 file with multiple certs'
+    openssl_pkcs12:
+      path: "{{ output_dir }}/ansible_multi_certs.p12"
+      friendly_name: 'abracadabra'
+      privatekey_path: "{{ output_dir }}/ansible_pkey.pem"
+      certificate_path: "{{ output_dir }}/ansible.crt"
+      ca_certificates:
+        - "{{ output_dir }}/ansible2.crt"
+        - "{{ output_dir }}/ansible3.crt"
+      state: present
+    register: p12_multiple_certs
+
+  - name: 'Generate PKCS#12 file with multiple certs, again (idempotency)'
+    openssl_pkcs12:
+      path: "{{ output_dir }}/ansible_multi_certs.p12"
+      friendly_name: 'abracadabra'
+      privatekey_path: "{{ output_dir }}/ansible_pkey.pem"
+      certificate_path: "{{ output_dir }}/ansible.crt"
+      ca_certificates:
+        - "{{ output_dir }}/ansible2.crt"
+        - "{{ output_dir }}/ansible3.crt"
+      state: present
+    register: p12_multiple_certs_idempotency
+
+  - name: 'Dump PKCS#12 with multiple certs'
+    openssl_pkcs12:
+      src: "{{ output_dir }}/ansible_multi_certs.p12"
+      path: "{{ output_dir }}/ansible_parse_multi_certs.pem"
+      action: 'parse'
+      state: 'present'
+
   - name: Generate privatekey with password
     openssl_privatekey:
       path: '{{ output_dir }}/privatekeypw.pem'
@@ -163,10 +230,11 @@
   - name: 'Delete PKCS#12 file'
     openssl_pkcs12:
       state: absent
-      path: '{{ output_dir }}/ansible.p12'
+      path: '{{ output_dir }}/{{ item }}.p12'
     loop:
       - 'ansible'
       - 'ansible_no_pkey'
+      - 'ansible_multi_certs'
       - 'ansible_pw1'
       - 'ansible_pw2'
       - 'ansible_pw3'

test/integration/targets/openssl_pkcs12/tests/validate.yml

@@ -1,9 +1,3 @@
----
-- name: 'Install pexpect'
-  pip:
-    name: 'pexpect'
-    state: 'present'
-
 - name: 'Validate PKCS#12'
   command: "openssl pkcs12 -info -in {{ output_dir }}/ansible.p12 -nodes -passin pass:''"
   register: p12
@@ -12,6 +6,10 @@
   command: "openssl pkcs12 -info -in {{ output_dir }}/ansible_no_pkey.p12 -nodes -passin pass:''"
   register: p12_validate_no_pkey
 
+- name: 'Validate PKCS#12 with multiple certs'
+  shell: "openssl pkcs12 -info -in {{ output_dir }}/ansible_multi_certs.p12 -nodes -passin pass:'' | grep subject"
+  register: p12_validate_multi_certs
+
 - name: 'Validate PKCS#12 (assert)'
   assert:
     that:
@@ -21,6 +19,11 @@
       - p12_validate_no_pkey.stdout_lines[-1] == '-----END CERTIFICATE-----'
       - p12_force.changed
       - p12_force_and_mode.mode == '0644' and p12_force_and_mode.changed
+      - not p12_standard_idempotency.changed
+      - not p12_multiple_certs_idempotency.changed
+      - "'www.' in p12_validate_multi_certs.stdout"
+      - "'www2.' in p12_validate_multi_certs.stdout"
+      - "'www3.' in p12_validate_multi_certs.stdout"
 
 - name: Check passphrase on private key
   assert: