diff --git a/changelogs/fragments/62991-openssl_dhparam-cryptography-backend.yml b/changelogs/fragments/62991-openssl_dhparam-cryptography-backend.yml new file mode 100644 index 00000000000..d9d1f7fc294 --- /dev/null +++ b/changelogs/fragments/62991-openssl_dhparam-cryptography-backend.yml @@ -0,0 +1,2 @@ +minor_changes: +- "openssl_dhparam - now supports a ``cryptography``-based backend. Auto-detection can be overwritten with the ``select_crypto_backend`` option." diff --git a/lib/ansible/module_utils/crypto.py b/lib/ansible/module_utils/crypto.py index ecac0d878aa..be91041f4c1 100644 --- a/lib/ansible/module_utils/crypto.py +++ b/lib/ansible/module_utils/crypto.py @@ -30,6 +30,7 @@ from __future__ import absolute_import, division, print_function __metaclass__ = type +import sys from distutils.version import LooseVersion try: @@ -1883,3 +1884,22 @@ def quick_is_not_prime(n): # TODO: maybe do some iterations of Miller-Rabin to increase confidence # (https://en.wikipedia.org/wiki/Miller%E2%80%93Rabin_primality_test) return False + + +python_version = (sys.version_info[0], sys.version_info[1]) +if python_version >= (2, 7) or python_version >= (3, 1): + # Ansible still supports Python 2.6 on remote nodes + def count_bits(no): + no = abs(no) + if no == 0: + return 0 + return no.bit_length() +else: + # Slow, but works + def count_bits(no): + no = abs(no) + count = 0 + while no > 0: + no >>= 1 + count += 1 + return count diff --git a/lib/ansible/modules/crypto/openssl_dhparam.py b/lib/ansible/modules/crypto/openssl_dhparam.py index 4cefb10823b..4e7dca01399 100644 --- a/lib/ansible/modules/crypto/openssl_dhparam.py +++ b/lib/ansible/modules/crypto/openssl_dhparam.py @@ -22,10 +22,14 @@ description: - "Please note that the module regenerates existing DH params if they don't match the module's options. If you are concerned that this could overwrite your existing DH params, consider using the I(backup) option." + - The module can use the cryptography Python library, or the C(openssl) executable. + By default, it tries to detect which one is available. This can be overridden + with the I(select_crypto_backend) option. requirements: - - OpenSSL + - Either cryptography >= 2.0 + - Or OpenSSL binary C(openssl) author: -- Thom Wiggers (@thomwiggers) + - Thom Wiggers (@thomwiggers) options: state: description: @@ -56,6 +60,16 @@ options: type: bool default: no version_added: "2.8" + select_crypto_backend: + description: + - Determines which crypto backend to use. + - The default choice is C(auto), which tries to use C(cryptography) if available, and falls back to C(openssl). + - If set to C(openssl), will try to use the OpenSSL C(openssl) executable. + - If set to C(cryptography), will try to use the L(cryptography,https://cryptography.io/) library. + type: str + default: auto + choices: [ auto, cryptography, openssl ] + version_added: '2.10' extends_documentation_fragment: - files seealso: @@ -100,19 +114,40 @@ backup_file: sample: /path/to/dhparams.pem.2019-03-09@11:22~ ''' +import abc import os import re import tempfile +import traceback +from distutils.version import LooseVersion -from ansible.module_utils.basic import AnsibleModule +from ansible.module_utils.basic import AnsibleModule, missing_required_lib from ansible.module_utils._text import to_native +from ansible.module_utils import crypto as crypto_utils + + +MINIMAL_CRYPTOGRAPHY_VERSION = '2.0' + +CRYPTOGRAPHY_IMP_ERR = None +try: + import cryptography + import cryptography.exceptions + import cryptography.hazmat.backends + import cryptography.hazmat.primitives.asymmetric.dh + import cryptography.hazmat.primitives.serialization + CRYPTOGRAPHY_VERSION = LooseVersion(cryptography.__version__) +except ImportError: + CRYPTOGRAPHY_IMP_ERR = traceback.format_exc() + CRYPTOGRAPHY_FOUND = False +else: + CRYPTOGRAPHY_FOUND = True class DHParameterError(Exception): pass -class DHParameter(object): +class DHParameterBase(object): def __init__(self, module): self.state = module.params['state'] @@ -120,32 +155,22 @@ class DHParameter(object): self.size = module.params['size'] self.force = module.params['force'] self.changed = False - self.openssl_bin = module.get_bin_path('openssl', True) self.backup = module.params['backup'] self.backup_file = None + @abc.abstractmethod + def _do_generate(self, module): + """Actually generate the DH params.""" + pass + def generate(self, module): - """Generate a keypair.""" + """Generate DH params.""" changed = False # ony generate when necessary if self.force or not self._check_params_valid(module): - # create a tempfile - fd, tmpsrc = tempfile.mkstemp() - os.close(fd) - module.add_cleanup_file(tmpsrc) # Ansible will delete the file on exit - # openssl dhparam -out - command = [self.openssl_bin, 'dhparam', '-out', tmpsrc, str(self.size)] - rc, dummy, err = module.run_command(command, check_rc=False) - if rc != 0: - raise DHParameterError(to_native(err)) - if self.backup: - self.backup_file = module.backup_local(self.path) - try: - module.atomic_move(tmpsrc, self.path) - except Exception as e: - module.fail_json(msg="Failed to write to file %s: %s" % (self.path, str(e))) + self._do_generate(module) changed = True # fix permissions (checking force not necessary as done above) @@ -171,6 +196,70 @@ class DHParameter(object): return False return self._check_params_valid(module) and self._check_fs_attributes(module) + @abc.abstractmethod + def _check_params_valid(self, module): + """Check if the params are in the correct state""" + pass + + def _check_fs_attributes(self, module): + """Checks (and changes if not in check mode!) fs attributes""" + file_args = module.load_file_common_arguments(module.params) + attrs_changed = module.set_fs_attributes_if_different(file_args, False) + + return not attrs_changed + + def dump(self): + """Serialize the object into a dictionary.""" + + result = { + 'size': self.size, + 'filename': self.path, + 'changed': self.changed, + } + if self.backup_file: + result['backup_file'] = self.backup_file + + return result + + +class DHParameterAbsent(DHParameterBase): + + def __init__(self, module): + super(DHParameterAbsent, self).__init__(module) + + def _do_generate(self, module): + """Actually generate the DH params.""" + pass + + def _check_params_valid(self, module): + """Check if the params are in the correct state""" + pass + + +class DHParameterOpenSSL(DHParameterBase): + + def __init__(self, module): + super(DHParameterOpenSSL, self).__init__(module) + self.openssl_bin = module.get_bin_path('openssl', True) + + def _do_generate(self, module): + """Actually generate the DH params.""" + # create a tempfile + fd, tmpsrc = tempfile.mkstemp() + os.close(fd) + module.add_cleanup_file(tmpsrc) # Ansible will delete the file on exit + # openssl dhparam -out + command = [self.openssl_bin, 'dhparam', '-out', tmpsrc, str(self.size)] + rc, dummy, err = module.run_command(command, check_rc=False) + if rc != 0: + raise DHParameterError(to_native(err)) + if self.backup: + self.backup_file = module.backup_local(self.path) + try: + module.atomic_move(tmpsrc, self.path) + except Exception as e: + module.fail_json(msg="Failed to write to file %s: %s" % (self.path, str(e))) + def _check_params_valid(self, module): """Check if the params are in the correct state""" command = [self.openssl_bin, 'dhparam', '-check', '-text', '-noout', '-in', self.path] @@ -193,25 +282,43 @@ class DHParameter(object): return bits == self.size - def _check_fs_attributes(self, module): - """Checks (and changes if not in check mode!) fs attributes""" - file_args = module.load_file_common_arguments(module.params) - attrs_changed = module.set_fs_attributes_if_different(file_args, False) - return not attrs_changed - - def dump(self): - """Serialize the object into a dictionary.""" +class DHParameterCryptography(DHParameterBase): - result = { - 'size': self.size, - 'filename': self.path, - 'changed': self.changed, - } - if self.backup_file: - result['backup_file'] = self.backup_file + def __init__(self, module): + super(DHParameterCryptography, self).__init__(module) + self.crypto_backend = cryptography.hazmat.backends.default_backend() + + def _do_generate(self, module): + """Actually generate the DH params.""" + # Generate parameters + params = cryptography.hazmat.primitives.asymmetric.dh.generate_parameters( + generator=2, + key_size=self.size, + backend=self.crypto_backend, + ) + # Serialize parameters + result = params.parameter_bytes( + encoding=cryptography.hazmat.primitives.serialization.Encoding.PEM, + format=cryptography.hazmat.primitives.serialization.ParameterFormat.PKCS3, + ) + # Write result + if self.backup: + self.backup_file = module.backup_local(self.path) + crypto_utils.write_file(module, result) - return result + def _check_params_valid(self, module): + """Check if the params are in the correct state""" + # Load parameters + try: + with open(self.path, 'rb') as f: + data = f.read() + params = self.crypto_backend.load_pem_parameters(data) + except Exception as dummy: + return False + # Check parameters + bits = crypto_utils.count_bits(params.parameter_numbers().p) + return bits == self.size def main(): @@ -224,6 +331,7 @@ def main(): force=dict(type='bool', default=False), path=dict(type='path', required=True), backup=dict(type='bool', default=False), + select_crypto_backend=dict(type='str', default='auto', choices=['auto', 'cryptography', 'openssl']), ), supports_check_mode=True, add_file_common_args=True, @@ -236,9 +344,31 @@ def main(): msg="The directory '%s' does not exist or the file is not a directory" % base_dir ) - dhparam = DHParameter(module) - - if dhparam.state == 'present': + if module.params['state'] == 'present': + backend = module.params['select_crypto_backend'] + if backend == 'auto': + # Detection what is possible + can_use_cryptography = CRYPTOGRAPHY_FOUND and CRYPTOGRAPHY_VERSION >= LooseVersion(MINIMAL_CRYPTOGRAPHY_VERSION) + can_use_openssl = module.get_bin_path('openssl', False) is not None + + # First try cryptography, then OpenSSL + if can_use_cryptography: + backend = 'cryptography' + elif can_use_openssl: + backend = 'openssl' + + # Success? + if backend == 'auto': + module.fail_json(msg=("Can't detect either the required Python library cryptography (>= {0}) " + "or the OpenSSL binary openssl").format(MINIMAL_CRYPTOGRAPHY_VERSION)) + + if backend == 'openssl': + dhparam = DHParameterOpenSSL(module) + elif backend == 'cryptography': + if not CRYPTOGRAPHY_FOUND: + module.fail_json(msg=missing_required_lib('cryptography >= {0}'.format(MINIMAL_CRYPTOGRAPHY_VERSION)), + exception=CRYPTOGRAPHY_IMP_ERR) + dhparam = DHParameterCryptography(module) if module.check_mode: result = dhparam.dump() @@ -250,6 +380,7 @@ def main(): except DHParameterError as exc: module.fail_json(msg=to_native(exc)) else: + dhparam = DHParameterAbsent(module) if module.check_mode: result = dhparam.dump() diff --git a/test/integration/targets/openssl_dhparam/meta/main.yml b/test/integration/targets/openssl_dhparam/meta/main.yml new file mode 100644 index 00000000000..800aff64284 --- /dev/null +++ b/test/integration/targets/openssl_dhparam/meta/main.yml @@ -0,0 +1,2 @@ +dependencies: + - setup_openssl diff --git a/test/integration/targets/openssl_dhparam/tasks/impl.yml b/test/integration/targets/openssl_dhparam/tasks/impl.yml new file mode 100644 index 00000000000..dcef8da59d5 --- /dev/null +++ b/test/integration/targets/openssl_dhparam/tasks/impl.yml @@ -0,0 +1,97 @@ +--- +# The tests for this module generate unsafe parameters for testing purposes; +# otherwise tests would be too slow. Use sizes of at least 2048 in production! +- name: "[{{ select_crypto_backend }}] Generate parameter" + openssl_dhparam: + size: 768 + path: '{{ output_dir }}/dh768.pem' + select_crypto_backend: "{{ select_crypto_backend }}" + +- name: "[{{ select_crypto_backend }}] Don't regenerate parameters with no change" + openssl_dhparam: + size: 768 + path: '{{ output_dir }}/dh768.pem' + select_crypto_backend: "{{ select_crypto_backend }}" + register: dhparam_changed + +- name: "[{{ select_crypto_backend }}] Generate parameters with size option" + openssl_dhparam: + path: '{{ output_dir }}/dh512.pem' + size: 512 + select_crypto_backend: "{{ select_crypto_backend }}" + +- name: "[{{ select_crypto_backend }}] Don't regenerate parameters with size option and no change" + openssl_dhparam: + path: '{{ output_dir }}/dh512.pem' + size: 512 + select_crypto_backend: "{{ select_crypto_backend }}" + register: dhparam_changed_512 + +- copy: + src: '{{ output_dir }}/dh768.pem' + remote_src: yes + dest: '{{ output_dir }}/dh512.pem' + +- name: "[{{ select_crypto_backend }}] Re-generate if size is different" + openssl_dhparam: + path: '{{ output_dir }}/dh512.pem' + size: 512 + select_crypto_backend: "{{ select_crypto_backend }}" + register: dhparam_changed_to_512 + +- name: "[{{ select_crypto_backend }}] Force re-generate parameters with size option" + openssl_dhparam: + path: '{{ output_dir }}/dh512.pem' + size: 512 + force: yes + select_crypto_backend: "{{ select_crypto_backend }}" + register: dhparam_changed_force + +- name: "[{{ select_crypto_backend }}] Create broken params" + copy: + dest: "{{ output_dir }}/dhbroken.pem" + content: "broken" +- name: "[{{ select_crypto_backend }}] Regenerate broken params" + openssl_dhparam: + path: '{{ output_dir }}/dhbroken.pem' + size: 512 + force: yes + select_crypto_backend: "{{ select_crypto_backend }}" + register: output_broken + +- name: "[{{ select_crypto_backend }}] Generate params" + openssl_dhparam: + path: '{{ output_dir }}/dh_backup.pem' + size: 512 + backup: yes + select_crypto_backend: "{{ select_crypto_backend }}" + register: dhparam_backup_1 +- name: "[{{ select_crypto_backend }}] Generate params (idempotent)" + openssl_dhparam: + path: '{{ output_dir }}/dh_backup.pem' + size: 512 + backup: yes + select_crypto_backend: "{{ select_crypto_backend }}" + register: dhparam_backup_2 +- name: "[{{ select_crypto_backend }}] Generate params (change)" + openssl_dhparam: + path: '{{ output_dir }}/dh_backup.pem' + size: 512 + force: yes + backup: yes + select_crypto_backend: "{{ select_crypto_backend }}" + register: dhparam_backup_3 +- name: "[{{ select_crypto_backend }}] Generate params (remove)" + openssl_dhparam: + path: '{{ output_dir }}/dh_backup.pem' + state: absent + backup: yes + select_crypto_backend: "{{ select_crypto_backend }}" + register: dhparam_backup_4 +- name: "[{{ select_crypto_backend }}] Generate params (remove, idempotent)" + openssl_dhparam: + path: '{{ output_dir }}/dh_backup.pem' + state: absent + backup: yes + select_crypto_backend: "{{ select_crypto_backend }}" + register: dhparam_backup_5 diff --git a/test/integration/targets/openssl_dhparam/tasks/main.yml b/test/integration/targets/openssl_dhparam/tasks/main.yml index bf788abbd8c..d8c106b3a28 100644 --- a/test/integration/targets/openssl_dhparam/tasks/main.yml +++ b/test/integration/targets/openssl_dhparam/tasks/main.yml @@ -1,88 +1,38 @@ --- -- block: - # This module generates unsafe parameters for testing purposes - # otherwise tests would be too slow - - name: Generate parameter - openssl_dhparam: - size: 768 - path: '{{ output_dir }}/dh768.pem' +# The tests for this module generate unsafe parameters for testing purposes; +# otherwise tests would be too slow. Use sizes of at least 2048 in production! - - name: Don't regenerate parameters with no change - openssl_dhparam: - size: 768 - path: '{{ output_dir }}/dh768.pem' - register: dhparam_changed +- name: Run module with backend autodetection + openssl_dhparam: + path: '{{ output_dir }}/dh_backend_selection.pem' + size: 512 - - name: Generate parameters with size option - openssl_dhparam: - path: '{{ output_dir }}/dh512.pem' - size: 512 +- block: + - name: Running tests with OpenSSL backend + include_tasks: impl.yml - - name: Don't regenerate parameters with size option and no change - openssl_dhparam: - path: '{{ output_dir }}/dh512.pem' - size: 512 - register: dhparam_changed_512 + - include_tasks: ../tests/validate.yml - - copy: - src: '{{ output_dir }}/dh768.pem' - remote_src: yes - dest: '{{ output_dir }}/dh512.pem' + vars: + select_crypto_backend: openssl + # when: openssl_version.stdout is version('1.0.0', '>=') - - name: Re-generate if size is different - openssl_dhparam: - path: '{{ output_dir }}/dh512.pem' - size: 512 - register: dhparam_changed_to_512 +- name: Remove output directory + file: + path: "{{ output_dir }}" + state: absent - - name: Force re-generate parameters with size option - openssl_dhparam: - path: '{{ output_dir }}/dh512.pem' - size: 512 - force: yes - register: dhparam_changed_force +- name: Re-create output directory + file: + path: "{{ output_dir }}" + state: directory - - name: Create broken params - copy: - dest: "{{ output_dir }}/dhbroken.pem" - content: "broken" - - name: Regenerate broken params - openssl_dhparam: - path: '{{ output_dir }}/dhbroken.pem' - size: 512 - force: yes - register: output_broken +- block: + - name: Running tests with cryptography backend + include_tasks: impl.yml - - name: Generate params - openssl_dhparam: - path: '{{ output_dir }}/dh_backup.pem' - size: 512 - backup: yes - register: dhparam_backup_1 - - name: Generate params (idempotent) - openssl_dhparam: - path: '{{ output_dir }}/dh_backup.pem' - size: 512 - backup: yes - register: dhparam_backup_2 - - name: Generate params (change) - openssl_dhparam: - path: '{{ output_dir }}/dh_backup.pem' - size: 512 - force: yes - backup: yes - register: dhparam_backup_3 - - name: Generate params (remove) - openssl_dhparam: - path: '{{ output_dir }}/dh_backup.pem' - state: absent - backup: yes - register: dhparam_backup_4 - - name: Generate params (remove, idempotent) - openssl_dhparam: - path: '{{ output_dir }}/dh_backup.pem' - state: absent - backup: yes - register: dhparam_backup_5 + - include_tasks: ../tests/validate.yml - - import_tasks: ../tests/validate.yml + vars: + select_crypto_backend: cryptography + when: cryptography_version.stdout is version('2.0', '>=') diff --git a/test/integration/targets/openssl_dhparam/tests/validate.yml b/test/integration/targets/openssl_dhparam/tests/validate.yml index b4a2857255f..a670ab9ceff 100644 --- a/test/integration/targets/openssl_dhparam/tests/validate.yml +++ b/test/integration/targets/openssl_dhparam/tests/validate.yml @@ -1,29 +1,29 @@ --- -- name: Validate generated params +- name: "[{{ select_crypto_backend }}] Validate generated params" shell: 'openssl dhparam -in {{ output_dir }}/{{ item }}.pem -noout -check' with_items: - dh768 - dh512 -- name: Get bit size of 768 +- name: "[{{ select_crypto_backend }}] Get bit size of 768" shell: 'openssl dhparam -noout -in {{ output_dir }}/dh768.pem -text | head -n1 | sed -ne "s@.*(\\([[:digit:]]\{1,\}\\) bit).*@\\1@p"' register: bit_size_dhparam -- name: Check bit size of default +- name: "[{{ select_crypto_backend }}] Check bit size of default" assert: that: - bit_size_dhparam.stdout == "768" -- name: Get bit size of 512 +- name: "[{{ select_crypto_backend }}] Get bit size of 512" shell: 'openssl dhparam -noout -in {{ output_dir }}/dh512.pem -text | head -n1 | sed -ne "s@.*(\\([[:digit:]]\{1,\}\\) bit).*@\\1@p"' register: bit_size_dhparam_512 -- name: Check bit size of default +- name: "[{{ select_crypto_backend }}] Check bit size of default" assert: that: - bit_size_dhparam_512.stdout == "512" -- name: Check if changed works correctly +- name: "[{{ select_crypto_backend }}] Check if changed works correctly" assert: that: - dhparam_changed is not changed @@ -31,12 +31,12 @@ - dhparam_changed_to_512 is changed - dhparam_changed_force is changed -- name: Verify that broken params will be regenerated +- name: "[{{ select_crypto_backend }}] Verify that broken params will be regenerated" assert: that: - output_broken is changed -- name: Check backup +- name: "[{{ select_crypto_backend }}] Check backup" assert: that: - dhparam_backup_1 is changed