From 6ee238d9617b6d6f30333bda64ec4247a46fbd97 Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Tue, 4 Jan 2022 07:00:09 +0100 Subject: [PATCH] certificate_complete_chain: avoid infinite loops, and double roots when root certificate was already part of chain (#360) * Avoid infinite loops, and double roots when root certificate was already part of chain. * Refactor tests for readability. --- .../360-certificate_complete_chain-loop.yml | 3 + plugins/modules/certificate_complete_chain.py | 18 ++ .../certificate_complete_chain/tasks/main.yml | 179 ++++++++++++------ 3 files changed, 146 insertions(+), 54 deletions(-) create mode 100644 changelogs/fragments/360-certificate_complete_chain-loop.yml diff --git a/changelogs/fragments/360-certificate_complete_chain-loop.yml b/changelogs/fragments/360-certificate_complete_chain-loop.yml new file mode 100644 index 00000000..86c36852 --- /dev/null +++ b/changelogs/fragments/360-certificate_complete_chain-loop.yml @@ -0,0 +1,3 @@ +bugfixes: + - "certificate_complete_chain - do not hang when infinite loop is found (https://github.com/ansible-collections/community.crypto/issues/355, https://github.com/ansible-collections/community.crypto/pull/360)." + - "certificate_complete_chain - do not append root twice if the chain already ends with a root certificate (https://github.com/ansible-collections/community.crypto/pull/360)." diff --git a/plugins/modules/certificate_complete_chain.py b/plugins/modules/certificate_complete_chain.py index 8c96ba9e..5a6c02bc 100644 --- a/plugins/modules/certificate_complete_chain.py +++ b/plugins/modules/certificate_complete_chain.py @@ -238,12 +238,14 @@ class CertificateSet(object): self.module = module self.certificates = set() self.certificate_by_issuer = dict() + self.certificate_by_cert = dict() def _load_file(self, path): certs = load_PEM_list(self.module, path, fail_on_error=False) for cert in certs: self.certificates.add(cert) self.certificate_by_issuer[cert.cert.subject] = cert + self.certificate_by_cert[cert.cert] = cert def load(self, path): ''' @@ -275,6 +277,16 @@ def format_cert(cert): return str(cert.cert) +def check_cycle(module, occured_certificates, next): + ''' + Make sure that next is not in occured_certificates so far, and add it. + ''' + next_cert = next.cert + if next_cert in occured_certificates: + module.fail_json(msg='Found cycle while building certificate chain') + occured_certificates.add(next_cert) + + def main(): module = AnsibleModule( argument_spec=dict( @@ -313,13 +325,19 @@ def main(): # Try to complete chain current = chain[-1] completed = [] + occured_certificates = set([cert.cert for cert in chain]) + if current.cert in roots.certificate_by_cert: + # Don't try to complete the chain when it's already ending with a root certificate + current = None while current: root = roots.find_parent(current) if root: + check_cycle(module, occured_certificates, root) completed.append(root) break intermediate = intermediates.find_parent(current) if intermediate: + check_cycle(module, occured_certificates, intermediate) completed.append(intermediate) current = intermediate else: diff --git a/tests/integration/targets/certificate_complete_chain/tasks/main.yml b/tests/integration/targets/certificate_complete_chain/tasks/main.yml index a50d2178..f6035a73 100644 --- a/tests/integration/targets/certificate_complete_chain/tasks/main.yml +++ b/tests/integration/targets/certificate_complete_chain/tasks/main.yml @@ -15,73 +15,144 @@ src: '{{ role_path }}/files/' dest: '{{ remote_tmp_dir }}/files/' - - name: Find root for cert 1 using directory - certificate_complete_chain: - input_chain: '{{ lookup("file", "cert1-fullchain.pem", rstrip=False) }}' - root_certificates: - - '{{ remote_tmp_dir }}/files/roots/' - register: cert1_root - - name: Verify root for cert 1 - assert: - that: - - cert1_root.complete_chain | join('') == (lookup('file', 'cert1.pem', rstrip=False) ~ lookup('file', 'cert1-chain.pem', rstrip=False) ~ lookup('file', 'cert1-root.pem', rstrip=False)) - - cert1_root.root == lookup('file', 'cert1-root.pem', rstrip=False) + - block: + - name: Find root for cert 1 using directory + certificate_complete_chain: + input_chain: '{{ fullchain | trim }}' + root_certificates: + - '{{ remote_tmp_dir }}/files/roots/' + register: cert1_root + - name: Verify root for cert 1 + assert: + that: + - cert1_root.complete_chain | join('') == (fullchain ~ root) + - cert1_root.root == root + vars: + fullchain: "{{ lookup('file', 'cert1-fullchain.pem', rstrip=False) }}" + root: "{{ lookup('file', 'cert1-root.pem', rstrip=False) }}" - - name: Find rootchain for cert 1 using intermediate and root PEM + - block: + - name: Find rootchain for cert 1 using intermediate and root PEM + certificate_complete_chain: + input_chain: '{{ cert }}' + intermediate_certificates: + - '{{ remote_tmp_dir }}/files/cert1-chain.pem' + root_certificates: + - '{{ remote_tmp_dir }}/files/roots.pem' + register: cert1_rootchain + - name: Verify rootchain for cert 1 + assert: + that: + - cert1_rootchain.complete_chain | join('') == (cert ~ chain ~ root) + - cert1_rootchain.chain[:-1] | join('') == chain + - cert1_rootchain.root == root + vars: + cert: "{{ lookup('file', 'cert1.pem', rstrip=False) }}" + chain: "{{ lookup('file', 'cert1-chain.pem', rstrip=False) }}" + root: "{{ lookup('file', 'cert1-root.pem', rstrip=False) }}" + + - block: + - name: Find root for cert 2 using directory + certificate_complete_chain: + input_chain: "{{ fullchain | trim }}" + root_certificates: + - '{{ remote_tmp_dir }}/files/roots/' + register: cert2_root + - name: Verify root for cert 2 + assert: + that: + - cert2_root.complete_chain | join('') == (fullchain ~ root) + - cert2_root.root == root + vars: + fullchain: "{{ lookup('file', 'cert2-fullchain.pem', rstrip=False) }}" + root: "{{ lookup('file', 'cert2-root.pem', rstrip=False) }}" + + - block: + - name: Find rootchain for cert 2 using intermediate and root PEM + certificate_complete_chain: + input_chain: '{{ cert }}' + intermediate_certificates: + - '{{ remote_tmp_dir }}/files/cert2-chain.pem' + root_certificates: + - '{{ remote_tmp_dir }}/files/roots.pem' + register: cert2_rootchain + - name: Verify rootchain for cert 2 + assert: + that: + - cert2_rootchain.complete_chain | join('') == (cert ~ chain ~ root) + - cert2_rootchain.chain[:-1] | join('') == chain + - cert2_rootchain.root == root + vars: + cert: "{{ lookup('file', 'cert2.pem', rstrip=False) }}" + chain: "{{ lookup('file', 'cert2-chain.pem', rstrip=False) }}" + root: "{{ lookup('file', 'cert2-root.pem', rstrip=False) }}" + + - block: + - name: Find alternate rootchain for cert 2 using intermediate and root PEM + certificate_complete_chain: + input_chain: '{{ cert }}' + intermediate_certificates: + - '{{ remote_tmp_dir }}/files/cert2-altchain.pem' + root_certificates: + - '{{ remote_tmp_dir }}/files/roots.pem' + register: cert2_rootchain_alt + - name: Verify rootchain for cert 2 + assert: + that: + - cert2_rootchain_alt.complete_chain | join('') == (cert ~ chain ~ root) + - cert2_rootchain_alt.chain[:-1] | join('') == chain + - cert2_rootchain_alt.root == root + vars: + cert: "{{ lookup('file', 'cert2.pem', rstrip=False) }}" + chain: "{{ lookup('file', 'cert2-altchain.pem', rstrip=False) }}" + root: "{{ lookup('file', 'cert2-altroot.pem', rstrip=False) }}" + + - block: + - name: Find alternate rootchain for cert 2 when complete chain is already presented to the module + certificate_complete_chain: + input_chain: '{{ cert ~ chain ~ root }}' + root_certificates: + - '{{ remote_tmp_dir }}/files/roots.pem' + register: cert2_complete_chain + - name: Verify rootchain for cert 2 + assert: + that: + - cert2_complete_chain.complete_chain | join('') == (cert ~ chain ~ root) + - cert2_complete_chain.chain == [] + - cert2_complete_chain.root == root + vars: + cert: "{{ lookup('file', 'cert2.pem', rstrip=False) }}" + chain: "{{ lookup('file', 'cert2-altchain.pem', rstrip=False) }}" + root: "{{ lookup('file', 'cert2-altroot.pem', rstrip=False) }}" + + - name: Check failure when no intermediate certificate can be found certificate_complete_chain: - input_chain: '{{ lookup("file", "cert1.pem", rstrip=False) }}' + input_chain: '{{ lookup("file", "cert2.pem", rstrip=True) }}' intermediate_certificates: - '{{ remote_tmp_dir }}/files/cert1-chain.pem' root_certificates: - '{{ remote_tmp_dir }}/files/roots.pem' - register: cert1_rootchain - - name: Verify rootchain for cert 1 + register: cert2_no_intermediate + ignore_errors: true + - name: Verify failure assert: that: - - cert1_rootchain.complete_chain | join('') == (lookup('file', 'cert1.pem', rstrip=False) ~ lookup('file', 'cert1-chain.pem', rstrip=False) ~ lookup('file', 'cert1-root.pem', rstrip=False)) - - cert1_rootchain.chain[:-1] | join('') == lookup('file', 'cert1-chain.pem', rstrip=False) - - cert1_rootchain.root == lookup('file', 'cert1-root.pem', rstrip=False) + - cert2_no_intermediate is failed + - "cert2_no_intermediate.msg.startswith('Cannot complete chain. Stuck at certificate ')" - - name: Find root for cert 2 using directory + - name: Check failure when infinite loop is found certificate_complete_chain: - input_chain: '{{ lookup("file", "cert2-fullchain.pem", rstrip=False) }}' - root_certificates: - - '{{ remote_tmp_dir }}/files/roots/' - register: cert2_root - - name: Verify root for cert 2 - assert: - that: - - cert2_root.complete_chain | join('') == (lookup('file', 'cert2.pem', rstrip=False) ~ lookup('file', 'cert2-chain.pem', rstrip=False) ~ lookup('file', 'cert2-root.pem', rstrip=False)) - - cert2_root.root == lookup('file', 'cert2-root.pem', rstrip=False) - - - name: Find rootchain for cert 2 using intermediate and root PEM - certificate_complete_chain: - input_chain: '{{ lookup("file", "cert2.pem", rstrip=False) }}' + input_chain: '{{ lookup("file", "cert2-fullchain.pem", rstrip=True) }}' intermediate_certificates: - - '{{ remote_tmp_dir }}/files/cert2-chain.pem' - root_certificates: - '{{ remote_tmp_dir }}/files/roots.pem' - register: cert2_rootchain - - name: Verify rootchain for cert 2 + root_certificates: + - '{{ remote_tmp_dir }}/files/cert1-chain.pem' + register: cert2_infinite_loop + ignore_errors: true + - name: Verify failure assert: that: - - cert2_rootchain.complete_chain | join('') == (lookup('file', 'cert2.pem', rstrip=False) ~ lookup('file', 'cert2-chain.pem', rstrip=False) ~ lookup('file', 'cert2-root.pem', rstrip=False)) - - cert2_rootchain.chain[:-1] | join('') == lookup('file', 'cert2-chain.pem', rstrip=False) - - cert2_rootchain.root == lookup('file', 'cert2-root.pem', rstrip=False) - - - name: Find alternate rootchain for cert 2 using intermediate and root PEM - certificate_complete_chain: - input_chain: '{{ lookup("file", "cert2.pem", rstrip=True) }}' - intermediate_certificates: - - '{{ remote_tmp_dir }}/files/cert2-altchain.pem' - root_certificates: - - '{{ remote_tmp_dir }}/files/roots.pem' - register: cert2_rootchain_alt - - name: Verify rootchain for cert 2 - assert: - that: - - cert2_rootchain_alt.complete_chain | join('') == (lookup('file', 'cert2.pem', rstrip=False) ~ lookup('file', 'cert2-altchain.pem', rstrip=False) ~ lookup('file', 'cert2-altroot.pem', rstrip=False)) - - cert2_rootchain_alt.chain[:-1] | join('') == lookup('file', 'cert2-altchain.pem', rstrip=False) - - cert2_rootchain_alt.root == lookup('file', 'cert2-altroot.pem', rstrip=False) + - cert2_infinite_loop is failed + - "cert2_infinite_loop.msg == 'Found cycle while building certificate chain'" when: cryptography_version.stdout is version('1.5', '>=')