Skip to content

Commit

Permalink
openssl_pkcs12: do not crash when there's no certificate and/or priva…
Browse files Browse the repository at this point in the history
…te 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.
  • Loading branch information
felixfontein authored Sep 16, 2020
1 parent 1b3ff44 commit 7cdfdc1
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 5 deletions.
2 changes: 2 additions & 0 deletions changelogs/fragments/109-openssl_pkcs12-crash-no-cert-key.yml
Original file line number Diff line number Diff line change
@@ -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)."
12 changes: 7 additions & 5 deletions plugins/modules/openssl_pkcs12.py
Original file line number Diff line number Diff line change
Expand Up @@ -360,10 +360,12 @@ def parse(self):
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,
Expand Down Expand Up @@ -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)
Expand Down
24 changes: 24 additions & 0 deletions tests/integration/targets/openssl_pkcs12/tasks/impl.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -226,3 +249,4 @@
- ansible_pw1
- ansible_pw2
- ansible_pw3
- ansible_empty
7 changes: 7 additions & 0 deletions tests/integration/targets/openssl_pkcs12/tests/validate.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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')"

0 comments on commit 7cdfdc1

Please sign in to comment.