From 7cdfdc1bfb2bd5c0d154c4c2270f158301a26ee8 Mon Sep 17 00:00:00 2001
From: Felix Fontein <felix@fontein.de>
Date: Wed, 16 Sep 2020 11:25:24 +0200
Subject: [PATCH] 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.
---
 .../109-openssl_pkcs12-crash-no-cert-key.yml  |  2 ++
 plugins/modules/openssl_pkcs12.py             | 12 ++++++----
 .../targets/openssl_pkcs12/tasks/impl.yml     | 24 +++++++++++++++++++
 .../targets/openssl_pkcs12/tests/validate.yml |  7 ++++++
 4 files changed, 40 insertions(+), 5 deletions(-)
 create mode 100644 changelogs/fragments/109-openssl_pkcs12-crash-no-cert-key.yml

diff --git a/changelogs/fragments/109-openssl_pkcs12-crash-no-cert-key.yml b/changelogs/fragments/109-openssl_pkcs12-crash-no-cert-key.yml
new file mode 100644
index 00000000..a97631b4
--- /dev/null
+++ b/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)."
diff --git a/plugins/modules/openssl_pkcs12.py b/plugins/modules/openssl_pkcs12.py
index 94e2929e..b02fa1d8 100644
--- a/plugins/modules/openssl_pkcs12.py
+++ b/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)
diff --git a/tests/integration/targets/openssl_pkcs12/tasks/impl.yml b/tests/integration/targets/openssl_pkcs12/tasks/impl.yml
index f169f470..e062772b 100644
--- a/tests/integration/targets/openssl_pkcs12/tasks/impl.yml
+++ b/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
diff --git a/tests/integration/targets/openssl_pkcs12/tests/validate.yml b/tests/integration/targets/openssl_pkcs12/tests/validate.yml
index baff2282..1f380b3a 100644
--- a/tests/integration/targets/openssl_pkcs12/tests/validate.yml
+++ b/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')"