Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

endless loop when root certificate is found in intermediate certificates set #355

Closed
JensHeinrich opened this issue Dec 29, 2021 · 4 comments · Fixed by #360
Closed

endless loop when root certificate is found in intermediate certificates set #355

JensHeinrich opened this issue Dec 29, 2021 · 4 comments · Fixed by #360
Labels
bug Something isn't working

Comments

@JensHeinrich
Copy link
Contributor

SUMMARY

When a root certificate is found inside the intermediate certificates set, it is it's own parent and therefor the loop never ends

ISSUE TYPE
  • Bug Report
COMPONENT NAME

plugins/modules/certificate_complete_chain.py

ANSIBLE VERSION
ansible 2.10.8
  config file = None
  configured module search path = ['/home/jheinrich/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python3/dist-packages/ansible
  executable location = /usr/bin/ansible
  python version = 3.9.7 (default, Sep 10 2021, 14:59:43) [GCC 11.2.0]

COLLECTION VERSION
Collection       Version
---------------- -------
community.crypto 2.0.2  

CONFIGURATION
ANSIBLE_NOCOWS(/home/jheinrich/Development/PROJECT/ansible.cfg) = True
COLLECTIONS_PATHS(/home/jheinrich/Development/PROJECT/ansible.cfg) = ['/home/jheinrich/Development/PROJECT']
COMMAND_WARNINGS(/home/jheinrich/Development/PROJECT/ansible.cfg) = False
DEFAULT_BECOME(/home/jheinrich/Development/PROJECT/ansible.cfg) = True
DEFAULT_BECOME_ASK_PASS(/home/jheinrich/Development/PROJECT/ansible.cfg) = True
DEFAULT_BECOME_METHOD(/home/jheinrich/Development/PROJECT/ansible.cfg) = sudo
DEFAULT_HOST_LIST(/home/jheinrich/Development/PROJECT/ansible.cfg) = ['/home/jheinrich/Development/PROJECT/environments']
DEFAULT_LOOKUP_PLUGIN_PATH(/home/jheinrich/Development/PROJECT/ansible.cfg) = ['/usr/share/ansible/plugins/lookup']
DEFAULT_VAULT_PASSWORD_FILE(/home/jheinrich/Development/PROJECT/ansible.cfg) = /home/jheinrich/vault_ub.key
INTERPRETER_PYTHON(/home/jheinrich/Development/PROJECT/ansible.cfg) = /usr/bin/python3
OS / ENVIRONMENT
STEPS TO REPRODUCE
- hosts: localhost
  tasks:
        - name: concatenate certificate chain
          community.crypto.certificate_complete_chain:
            input_chain: "{{ lookup(
              'file',
              'path/to/certificate'
              ) }}"
            # The following human error surfaced the "bug"
            intermediate_certificates:
              - "/etc/ssl/root_certificates/"
            root_certificates:
              - "/etc/ssl/intermediate_certificates"
EXPECTED RESULTS

Module to fail giving a 'Cannot complete chain. Stuck at certificate {0}' message

ACTUAL RESULTS

Not ending ansible task as the loop in line 315 of the file is never finished


@JensHeinrich
Copy link
Contributor Author

I do see two easy ways of fixing this, but I would like @felixfontein input:

Is a root certificate really it's own parent?
If not, the is_parent function could to the check and return False resulting in a failure
If it is, a check could be added in the if intermediate block ending the loop and returning it, probably with a warning.
The same check could also be used to fail the module though

@felixfontein felixfontein added the bug Something isn't working label Dec 29, 2021
@felixfontein
Copy link
Contributor

@JensHeinrich I guess it is and it is not :-) In any case, I would not fix it in that function, but instead add a generic loop detection, to avoid the same issue happening with cycles longer than one. Basically create a set of all certificates so far in the chain (the actual certificate + all intermediates), and if a new parent found is already in that set, stop with an error.

@JensHeinrich
Copy link
Contributor Author

Maybe just use a dedicated class and fail the append if it is already in there?
Otherwise this sounds good to me

@felixfontein
Copy link
Contributor

resolved_by_pr #360

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants