openssl_pkcs12: do not crash when there's no certificate and/or private key in existing PKCS#12 file (#109)

* Do not crash when PKCS#12 file contains no private key and/or main certificate.

* Add changelog fragment.

* Call getters only once each, check explicitly for None.

* Add test.

* Also 'parse' correctly PKCS#12 file with no private key.

changelogs/fragments/109-openssl_pkcs12-crash-no-cert-key.yml

@@ -0,0 +1,2 @@
+bugfixes:
+- "openssl_pkcs12 - do not crash when reading PKCS#12 file which has no private key and/or no main certificate (https://github.com/ansible-collections/community.crypto/issues/103)."

plugins/modules/openssl_pkcs12.py

@@ -360,10 +360,12 @@ class Pkcs(OpenSSLObject):
                 pkcs12_content = pkcs12_fh.read()
             p12 = crypto.load_pkcs12(pkcs12_content,
                                      self.passphrase)
-            pkey = crypto.dump_privatekey(crypto.FILETYPE_PEM,
-                                          p12.get_privatekey())
-            crt = crypto.dump_certificate(crypto.FILETYPE_PEM,
-                                          p12.get_certificate())
+            pkey = p12.get_privatekey()
+            if pkey is not None:
+                pkey = crypto.dump_privatekey(crypto.FILETYPE_PEM, pkey)
+            crt = p12.get_certificate()
+            if crt is not None:
+                crt = crypto.dump_certificate(crypto.FILETYPE_PEM, crt)
             other_certs = []
             if p12.get_ca_certificates() is not None:
                 other_certs = [crypto.dump_certificate(crypto.FILETYPE_PEM,
@@ -444,7 +446,7 @@ def main():
                     changed = True
                 else:
                     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)))
+                    dump_content = ''.join([to_native(pem) for pem in [pkey, cert] + other_certs if pem is not None])
                     pkcs12.write(module, to_bytes(dump_content))
 
             file_args = module.load_file_common_arguments(module.params)

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

@@ -213,6 +213,29 @@
       state: absent
       backup: true
     register: p12_backup_5
+  - name: Generate 'empty' PKCS#12 file
+    openssl_pkcs12:
+      path: '{{ output_dir }}/ansible_empty.p12'
+      friendly_name: abracadabra
+      ca_certificates:
+      - '{{ output_dir }}/ansible2.crt'
+      - '{{ output_dir }}/ansible3.crt'
+      state: present
+    register: p12_empty
+  - name: Generate 'empty' PKCS#12 file (idempotent)
+    openssl_pkcs12:
+      path: '{{ output_dir }}/ansible_empty.p12'
+      friendly_name: abracadabra
+      ca_certificates:
+      - '{{ output_dir }}/ansible2.crt'
+      - '{{ output_dir }}/ansible3.crt'
+      state: present
+    register: p12_empty_idem
+  - name: Generate 'empty' PKCS#12 file (parse)
+    openssl_pkcs12:
+      src: '{{ output_dir }}/ansible_empty.p12'
+      path: '{{ output_dir }}/ansible_empty.pem'
+      action: parse
   - import_tasks: ../tests/validate.yml
   always:
   - name: Delete PKCS#12 file
@@ -226,3 +249,4 @@
     - ansible_pw1
     - ansible_pw2
     - ansible_pw3
+    - ansible_empty

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

@@ -55,3 +55,10 @@
       - p12_backup_5 is not changed
       - p12_backup_5.backup_file is undefined
       - p12_backup_4.pkcs12 is none
+
+- name: Check 'empty' file
+  assert:
+    that:
+      - p12_empty is changed
+      - p12_empty_idem is not changed
+      - "lookup('file', output_dir ~ '/ansible_empty.pem') == lookup('file', output_dir ~ '/ansible3.crt') ~ '\n' ~ lookup('file', output_dir ~ '/ansible2.crt')"