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

x509_certificate_convert does not fail on bad sources #809

Closed
andrewjroth opened this issue Oct 21, 2024 · 4 comments · Fixed by #830
Closed

x509_certificate_convert does not fail on bad sources #809

andrewjroth opened this issue Oct 21, 2024 · 4 comments · Fixed by #830

Comments

@andrewjroth
Copy link

SUMMARY

When using the module x509_certificate_convert, if the certificate source is bad, it will (incorrectly) report changed and the output will be empty/invalid.

ISSUE TYPE
  • Bug Report
COMPONENT NAME

x509_certificate_convert

ANSIBLE VERSION
ansible [core 2.13.13]
  config file = /<working_dir>/ansible.cfg
  configured module search path = ['/<working_dir>/plugins/modules']
  ansible python module location = /home/andrew/.pyenv/versions/3.8.19/lib/python3.8/site-packages/ansible
  ansible collection location = /<working_dir>/collections
  executable location = /home/andrew/.pyenv/versions/3.8.19/bin/ansible
  python version = 3.8.19 (default, Jul 18 2024, 16:20:35) [GCC 13.2.0]
  jinja version = 3.1.4
  libyaml = True
COLLECTION VERSION
# /<working_dir>/collections/ansible_collections
Collection       Version
---------------- -------
community.crypto 2.22.2 
CONFIGURATION
CALLBACKS_ENABLED(/<working_dir>/ansible.cfg) = ['profile_tasks']
COLLECTIONS_PATHS(/<working_dir>/ansible.cfg) = ['/<working_dir>/collections']
DEFAULT_FILTER_PLUGIN_PATH(/<working_dir>/ansible.cfg) = ['/<working_dir>/plugins/filters']
DEFAULT_LOAD_CALLBACK_PLUGINS(/<working_dir>/ansible.cfg) = True
DEFAULT_LOOKUP_PLUGIN_PATH(/<working_dir>/ansible.cfg) = ['/<working_dir>/plugins/lookup']
DEFAULT_MODULE_PATH(/<working_dir>/ansible.cfg) = ['/<working_dir>/plugins/modules']
DEFAULT_STDOUT_CALLBACK(/<working_dir>/ansible.cfg) = yaml
HOST_KEY_CHECKING(/<working_dir>/ansible.cfg) = False
OS / ENVIRONMENT

Ubuntu 24.04

STEPS TO REPRODUCE

Run example playbook:

- hosts: localhost
  gather_facts: false
  tasks:
    - name: Create Temporary File
      ansible.builtin.tempfile:
        state: file
      register: temp_file_result
    - name: Convert certificate
      community.crypto.x509_certificate_convert:
        src_path: "{{ temp_file_result.path }}"
        dest_path: "{{ temp_file_result.path }}.pem"
        format: pem
      register: cert_convert_result
    - debug:
        var: cert_convert_result
EXPECTED RESULTS

Playbook should fail on task "Convert certificate" because the input (src_path) is not a valid certificate.

ACTUAL RESULTS

Playbook completes successfully, with the task "Convert certificate" showing as changed.

PLAY [localhost] ************************************************************************************

TASK [Create Temporary File] ************************************************************************
Monday 21 October 2024  16:40:01 -0400 (0:00:00.011)       0:00:00.011 ******** 
changed: [localhost]

TASK [Convert certificates] *************************************************************************
Monday 21 October 2024  16:40:02 -0400 (0:00:00.292)       0:00:00.304 ******** 
changed: [localhost]

TASK [debug] ****************************************************************************************
Monday 21 October 2024  16:40:02 -0400 (0:00:00.433)       0:00:00.738 ******** 
ok: [localhost] => 
  cert_convert_result:
    changed: true
    failed: false

PLAY RECAP ******************************************************************************************
localhost                  : ok=3    changed=2    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0   
@felixfontein
Copy link
Contributor

Right now the module does not care about the certificate's contents. An empty or broken file is treated as a DER certificate and converted to PEM by Base64 encoding it and adding line-breaks and header/footer.

This allows the module to also handle certificates that cryptography cannot load, for example.

Maybe we should add a verify option or so which allows you to make sure it's a syntactically valid certificate (or more precisely: cryptography can load it).

@Matthew-Jenkins
Copy link

Matthew-Jenkins commented Dec 20, 2024

cryptography says this 'Our goal is for it to be your "cryptographic standard library". It supports Python 3.7+ and PyPy3 7.3.11+.'

So why would you want to use things cryptography doesn't support? If cryptography doesn't support something, wouldn't it be better to open an issue there? cryptography is fully able to load certs encoded using DER. You could make a loop of the various decoding functions until one works, fail out if none work then write it back in the format asked.

@felixfontein
Copy link
Contributor

Backwards compatibility, mainly.

@felixfontein
Copy link
Contributor

I created a PR that adds such an option in #830.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants