diff --git a/changelogs/fragments/256-openssh_cert-adding-idempotency-option.yml b/changelogs/fragments/256-openssh_cert-adding-idempotency-option.yml new file mode 100644 index 000000000..d415fd8d5 --- /dev/null +++ b/changelogs/fragments/256-openssh_cert-adding-idempotency-option.yml @@ -0,0 +1,4 @@ +--- +minor_changes: + - openssh_cert - added ``regenerate`` option to validate additional certificate parameters which trigger + regeneration of an existing certificate (https://github.com/ansible-collections/community.crypto/pull/256). diff --git a/plugins/module_utils/openssh/backends/common.py b/plugins/module_utils/openssh/backends/common.py new file mode 100644 index 000000000..7dc952a0f --- /dev/null +++ b/plugins/module_utils/openssh/backends/common.py @@ -0,0 +1,42 @@ +# -*- coding: utf-8 -*- +# +# Copyright: (c) 2021, Andrew Pantuso (@ajpantuso) +# +# Ansible is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# Ansible is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Ansible. If not, see . + +from __future__ import absolute_import, division, print_function +__metaclass__ = type + +import os + + +def restore_on_failure(f): + def backup_and_restore(module, path, *args, **kwargs): + backup_file = module.backup_local(path) if os.path.exists(path) else None + + try: + f(module, path, *args, **kwargs) + except Exception: + if backup_file is not None: + module.atomic_move(backup_file, path) + raise + else: + module.add_cleanup_file(backup_file) + + return backup_and_restore + + +@restore_on_failure +def safe_atomic_move(module, path, destination): + module.atomic_move(path, destination) diff --git a/plugins/module_utils/openssh/certificate.py b/plugins/module_utils/openssh/certificate.py index 3e6514871..ad3a4f573 100644 --- a/plugins/module_utils/openssh/certificate.py +++ b/plugins/module_utils/openssh/certificate.py @@ -39,6 +39,7 @@ from hashlib import sha256 from ansible.module_utils import six +from ansible.module_utils.common.text.converters import to_text from ansible_collections.community.crypto.plugins.module_utils.crypto.support import convert_relative_to_datetime from ansible_collections.community.crypto.plugins.module_utils.openssh.utils import ( OpensshParser, @@ -74,6 +75,29 @@ _ALWAYS = datetime(1970, 1, 1) _FOREVER = datetime.max +_CRITICAL_OPTIONS = ( + 'force-command', + 'source-address', + 'verify-required', +) + +_DIRECTIVES = ( + 'clear', + 'no-x11-forwarding', + 'no-agent-forwarding', + 'no-port-forwarding', + 'no-pty', + 'no-user-rc', +) + +_EXTENSIONS = ( + 'permit-x11-forwarding', + 'permit-agent-forwarding', + 'permit-port-forwarding', + 'permit-pty', + 'permit-user-rc' +) + if six.PY3: long = int @@ -92,6 +116,9 @@ def __eq__(self, other): else: return self._valid_from == other._valid_from and self._valid_to == other._valid_to + def __ne__(self, other): + return not self == other + @property def validity_string(self): if not (self._valid_from == _ALWAYS and self._valid_to == _FOREVER): @@ -131,12 +158,14 @@ def format_datetime(dt, date_format): @staticmethod def to_datetime(time_string_or_timestamp): try: - if isinstance(time_string_or_timestamp, str): + if isinstance(time_string_or_timestamp, six.string_types): result = OpensshCertificateTimeParameters._time_string_to_datetime(time_string_or_timestamp.strip()) elif isinstance(time_string_or_timestamp, (long, int)): result = OpensshCertificateTimeParameters._timestamp_to_datetime(time_string_or_timestamp) else: - raise ValueError("Value must be of type (str, int, long) not %s" % type(time_string_or_timestamp)) + raise ValueError( + "Value must be of type (str, unicode, int, long) not %s" % type(time_string_or_timestamp) + ) except ValueError: raise return result @@ -174,6 +203,78 @@ def _time_string_to_datetime(time_string): return result +class OpensshCertificateOption(object): + def __init__(self, option_type, name, data): + if option_type not in ('critical', 'extension'): + raise ValueError("type must be either 'critical' or 'extension'") + + if not isinstance(name, six.string_types): + raise TypeError("name must be a string not %s" % type(name)) + + if not isinstance(data, six.string_types): + raise TypeError("data must be a string not %s" % type(data)) + + self._option_type = option_type + self._name = name.lower() + self._data = data + + def __eq__(self, other): + if not isinstance(other, type(self)): + return NotImplemented + + return all([ + self._option_type == other._option_type, + self._name == other._name, + self._data == other._data, + ]) + + def __hash__(self): + return hash((self._option_type, self._name, self._data)) + + def __ne__(self, other): + return not self == other + + def __str__(self): + if self._data: + return "%s=%s" % (self._name, self._data) + return self._name + + @property + def data(self): + return self._data + + @property + def name(self): + return self._name + + @property + def type(self): + return self._option_type + + @classmethod + def from_string(cls, option_string): + if not isinstance(option_string, six.string_types): + raise ValueError("option_string must be a string not %s" % type(option_string)) + option_type = None + + if ':' in option_string: + option_type, value = option_string.strip().split(':', 1) + if '=' in value: + name, data = value.split('=', 1) + else: + name, data = value, '' + elif '=' in option_string: + name, data = option_string.strip().split('=', 1) + else: + name, data = option_string.strip(), '' + + return cls( + option_type=option_type or get_option_type(name.lower()), + name=name, + data=data + ) + + @six.add_metaclass(abc.ABCMeta) class OpensshCertificateInfo: """Encapsulates all certificate information which is signed by a CA key""" @@ -402,7 +503,7 @@ def load(cls, path): @property def type_string(self): - return self._cert_info.type_string + return to_text(self._cert_info.type_string) @property def nonce(self): @@ -410,7 +511,7 @@ def nonce(self): @property def public_key(self): - return self._cert_info.public_key_fingerprint() + return to_text(self._cert_info.public_key_fingerprint()) @property def serial(self): @@ -422,11 +523,11 @@ def type(self): @property def key_id(self): - return self._cert_info.key_id + return to_text(self._cert_info.key_id) @property def principals(self): - return self._cert_info.principals + return [to_text(p) for p in self._cert_info.principals] @property def valid_after(self): @@ -438,11 +539,13 @@ def valid_before(self): @property def critical_options(self): - return self._cert_info.critical_options + return [ + OpensshCertificateOption('critical', to_text(n), to_text(d)) for n, d in self._cert_info.critical_options + ] @property def extensions(self): - return self._cert_info.extensions + return [OpensshCertificateOption('extension', to_text(n), to_text(d)) for n, d in self._cert_info.extensions] @property def reserved(self): @@ -450,7 +553,7 @@ def reserved(self): @property def signing_key(self): - return self._cert_info.signing_key_fingerprint() + return to_text(self._cert_info.signing_key_fingerprint()) @staticmethod def _parse_cert_info(pub_key_type, parser): @@ -484,14 +587,36 @@ def to_dict(self): 'principals': self.principals, 'valid_after': time_parameters.valid_from(date_format='human_readable'), 'valid_before': time_parameters.valid_to(date_format='human_readable'), - 'critical_options': self.critical_options, - 'extensions': [e[0] for e in self.extensions], + 'critical_options': [str(critical_option) for critical_option in self.critical_options], + 'extensions': [str(extension) for extension in self.extensions], 'reserved': self.reserved, 'public_key': self.public_key, 'signing_key': self.signing_key, } +def apply_directives(directives): + if any(d not in _DIRECTIVES for d in directives): + raise ValueError("directives must be one of %s" % ", ".join(_DIRECTIVES)) + + directive_to_option = { + 'no-x11-forwarding': OpensshCertificateOption('extension', 'permit-x11-forwarding', ''), + 'no-agent-forwarding': OpensshCertificateOption('extension', 'permit-agent-forwarding', ''), + 'no-port-forwarding': OpensshCertificateOption('extension', 'permit-port-forwarding', ''), + 'no-pty': OpensshCertificateOption('extension', 'permit-pty', ''), + 'no-user-rc': OpensshCertificateOption('extension', 'permit-user-rc', ''), + } + + if 'clear' in directives: + return [] + else: + return list(set(default_options()) - set(directive_to_option[d] for d in directives)) + + +def default_options(): + return [OpensshCertificateOption('extension', name, '') for name in _EXTENSIONS] + + def fingerprint(public_key): """Generates a SHA256 hash and formats output to resemble ``ssh-keygen``""" h = sha256() @@ -514,5 +639,34 @@ def get_cert_info_object(key_type): return cert_info +def get_option_type(name): + if name in _CRITICAL_OPTIONS: + result = 'critical' + elif name in _EXTENSIONS: + result = 'extension' + else: + raise ValueError("%s is not a valid option. " % name + + "Custom options must start with 'critical:' or 'extension:' to indicate type") + return result + + def is_relative_time_string(time_string): return time_string.startswith("+") or time_string.startswith("-") + + +def parse_option_list(option_list): + critical_options = [] + directives = [] + extensions = [] + + for option in option_list: + if option.lower() in _DIRECTIVES: + directives.append(option.lower()) + else: + option_object = OpensshCertificateOption.from_string(option) + if option_object.type == 'critical': + critical_options.append(option_object) + else: + extensions.append(option_object) + + return critical_options, list(set(extensions + apply_directives(directives))) diff --git a/plugins/modules/openssh_cert.py b/plugins/modules/openssh_cert.py index a518b5dfb..32f28e36e 100644 --- a/plugins/modules/openssh_cert.py +++ b/plugins/modules/openssh_cert.py @@ -34,6 +34,7 @@ force: description: - Should the certificate be regenerated even if it already exists and is valid. + - Equivalent to I(regenerate=always). type: bool default: false path: @@ -41,6 +42,26 @@ - Path of the file containing the certificate. type: path required: true + regenerate: + description: + - When C(never) the task will fail if a certificate already exists at I(path) and is unreadable. + Otherwise, a new certificate will only be generated if there is no existing certificate. + - When C(fail) the task will fail if a certificate already exists at I(path) and does not + match the module's options. + - When C(partial_idempotence) an existing certificate will be regenerated based on + I(serial), I(type), I(valid_from), I(valid_to), I(valid_at), and I(principals). + - When C(full_idempotence) I(identifier), I(options), I(public_key), and I(signing_key) + are also considered when compared against an existing certificate. + - C(always) is equivalent to I(force=true). + type: str + choices: + - never + - fail + - partial_idempotence + - full_idempotence + - always + default: partial_idempotence + version_added: 1.8.0 signing_key: description: - The path to the private openssh key that is used for signing the public key in order to generate the certificate. @@ -223,9 +244,12 @@ from ansible.module_utils.basic import AnsibleModule from ansible.module_utils.common.text.converters import to_native, to_text +from ansible_collections.community.crypto.plugins.module_utils.openssh.backends.common import safe_atomic_move + from ansible_collections.community.crypto.plugins.module_utils.openssh.certificate import ( OpensshCertificate, OpensshCertificateTimeParameters, + parse_option_list, ) from ansible_collections.community.crypto.plugins.module_utils.openssh.utils import ( @@ -243,11 +267,12 @@ def __init__(self, module): self.force = module.params['force'] self.identifier = module.params['identifier'] or "" - self.options = module.params['options'] + self.options = module.params['options'] or [] self.path = module.params['path'] self.pkcs11_provider = module.params['pkcs11_provider'] self.principals = module.params['principals'] or [] self.public_key = module.params['public_key'] + self.regenerate = module.params['regenerate'] if not self.force else 'always' self.serial_number = module.params['serial_number'] self.signing_key = module.params['signing_key'] self.state = module.params['state'] @@ -273,6 +298,8 @@ def __init__(self, module): try: self.original_data = OpensshCertificate.load(self.path) except (TypeError, ValueError) as e: + if self.regenerate in ('never', 'fail'): + self.module.fail_json(msg="Unable to read existing certificate: %s" % to_native(e)) self.module.warn("Unable to read existing certificate: %s" % to_native(e)) self._validate_parameters() @@ -281,31 +308,14 @@ def exists(self): return os.path.exists(self.path) def generate(self): - if not self._is_valid() or self.force: + if self._should_generate(): if not self.check_mode: - key_copy = os.path.join(self.module.tmpdir, os.path.basename(self.public_key)) - - try: - self.module.preserved_copy(self.public_key, key_copy) - except OSError as e: - self.module.fail_json(msg="Unable to stage temporary key: %s" % to_native(e)) - self.module.add_cleanup_file(key_copy) - - self.module.run_command(self._command_arguments(key_copy), environ_update=dict(TZ="UTC"), check_rc=True) - - temp_cert = os.path.splitext(key_copy)[0] + '-cert.pub' - self.module.add_cleanup_file(temp_cert) - backup_cert = self.module.backup_local(self.path) if self.exists() else None + temp_cert = self._generate_temp_certificate() try: - self.module.atomic_move(temp_cert, self.path) + safe_atomic_move(self.module, temp_cert, self.path) except OSError as e: - if backup_cert is not None: - self.module.atomic_move(backup_cert, self.path) self.module.fail_json(msg="Unable to write certificate to %s: %s" % (self.path, to_native(e))) - else: - if backup_cert is not None: - self.module.add_cleanup_file(backup_cert) try: self.data = OpensshCertificate.load(self.path) @@ -331,7 +341,10 @@ def result(self): result = {'changed': self.changed} if self.module._diff: - result['diff'] = self._generate_diff() + result['diff'] = { + 'before': get_cert_dict(self.original_data), + 'after': get_cert_dict(self.data) + } if self.state == 'present': result.update({ @@ -377,34 +390,86 @@ def _command_arguments(self, key_copy_path): return result - def _generate_diff(self): - before = self.original_data.to_dict() if self.original_data else {} - before.pop('nonce', None) - after = self.data.to_dict() if self.data else {} - after.pop('nonce', None) + def _compare_options(self): + try: + critical_options, extensions = parse_option_list(self.options) + except ValueError as e: + return self.module.fail_json(msg=to_native(e)) + + return all([ + set(self.original_data.critical_options) == set(critical_options), + set(self.original_data.extensions) == set(extensions) + ]) + + def _compare_time_parameters(self): + try: + original_time_parameters = OpensshCertificateTimeParameters( + valid_from=self.original_data.valid_after, + valid_to=self.original_data.valid_before + ) + except ValueError as e: + return self.module.fail_json(msg=to_native(e)) + + return all([ + original_time_parameters == self.time_parameters, + original_time_parameters.within_range(self.valid_at) + ]) + + def _generate_temp_certificate(self): + key_copy = os.path.join(self.module.tmpdir, os.path.basename(self.public_key)) + + try: + self.module.preserved_copy(self.public_key, key_copy) + except OSError as e: + self.module.fail_json(msg="Unable to stage temporary key: %s" % to_native(e)) + self.module.add_cleanup_file(key_copy) - return {'before': before, 'after': after} + self.module.run_command(self._command_arguments(key_copy), environ_update=dict(TZ="UTC"), check_rc=True) + + temp_cert = os.path.splitext(key_copy)[0] + '-cert.pub' + self.module.add_cleanup_file(temp_cert) + + return temp_cert def _get_cert_info(self): return self.module.run_command([self.ssh_keygen, '-Lf', self.path])[1] + def _get_key_fingerprint(self, path): + stdout = self.module.run_command([self.ssh_keygen, '-lf', path], check_rc=True)[1] + return stdout.split()[1] + def _is_valid(self): - if self.original_data: - try: - original_time_parameters = OpensshCertificateTimeParameters( - valid_from=self.original_data.valid_after, - valid_to=self.original_data.valid_before + partial_result = all([ + set(self.original_data.principals) == set(self.principals), + self.original_data.serial == self.serial_number if self.serial_number is not None else True, + self.original_data.type == self.type, + self._compare_time_parameters(), + ]) + + if self.regenerate == 'partial_idempotence': + return partial_result + + return partial_result and all([ + self._compare_options(), + self.original_data.key_id == self.identifier, + self.original_data.public_key == self._get_key_fingerprint(self.public_key), + self.original_data.signing_key == self._get_key_fingerprint(self.signing_key), + ]) + + def _should_generate(self): + if self.regenerate == 'never': + return self.original_data is None + elif self.regenerate == 'fail': + if self.original_data and not self._is_valid(): + self.module.fail_json( + msg="Certificate does not match the provided options.", + cert=get_cert_dict(self.original_data) ) - except ValueError as e: - return self.module.fail_json(msg=to_native(e)) - return all([ - self.original_data.type == self.type, - set(to_text(p) for p in self.original_data.principals) == set(self.principals), - self.original_data.serial == self.serial_number if self.serial_number is not None else True, - original_time_parameters == self.time_parameters, - original_time_parameters.within_range(self.valid_at) - ]) - return False + return self.original_data is None + elif self.regenerate in ('partial_idempotence', 'full_idempotence'): + return self.original_data is None or not self._is_valid() + else: + return True def _update_permissions(self): file_args = self.module.load_file_common_arguments(self.module.params) @@ -448,6 +513,15 @@ def format_cert_info(cert_info): return result +def get_cert_dict(data): + if data is None: + return {} + + result = data.to_dict() + result.pop('nonce') + return result + + def main(): module = AnsibleModule( argument_spec=dict( @@ -458,6 +532,11 @@ def main(): pkcs11_provider=dict(type='str'), principals=dict(type='list', elements='str'), public_key=dict(type='path'), + regenerate=dict( + type='str', + default='partial_idempotence', + choices=['never', 'fail', 'partial_idempotence', 'full_idempotence', 'always'] + ), signing_key=dict(type='path'), serial_number=dict(type='int'), state=dict(type='str', default='present', choices=['absent', 'present']), diff --git a/tests/integration/targets/openssh_cert/tasks/main.yml b/tests/integration/targets/openssh_cert/tasks/main.yml index c1897d6cf..c257a42c3 100644 --- a/tests/integration/targets/openssh_cert/tasks/main.yml +++ b/tests/integration/targets/openssh_cert/tasks/main.yml @@ -22,6 +22,12 @@ - name: Import key_idempotency tests import_tasks: ../tests/key_idempotency.yml + - name: Import options tests + import_tasks: ../tests/options_idempotency.yml + + - name: Import regenerate tests + import_tasks: ../tests/regenerate.yml + - name: Import remove tests import_tasks: ../tests/remove.yml when: not (ansible_facts['distribution'] == "CentOS" and ansible_facts['distribution_major_version'] == "6") diff --git a/tests/integration/targets/openssh_cert/tests/idempotency.yml b/tests/integration/targets/openssh_cert/tests/idempotency.yml index 3844a94ce..2e468a2f1 100644 --- a/tests/integration/targets/openssh_cert/tests/idempotency.yml +++ b/tests/integration/targets/openssh_cert/tests/idempotency.yml @@ -177,12 +177,17 @@ valid_from: "2001-01-21" valid_to: "2019-01-21" changed: false - # Options are currently not checked for idempotency purposes - test_name: Generate an OpenSSH user Certificate with no options (idempotent) type: user valid_from: "2001-01-21" valid_to: "2019-01-21" changed: false + - test_name: Generate an OpenSSH user Certificate with no options - full idempotency (idempotent) + type: user + valid_from: "2001-01-21" + valid_to: "2019-01-21" + regenerate: full_idempotence + changed: true - test_name: Generate cert without serial type: user valid_from: always @@ -228,13 +233,19 @@ valid_from: always valid_to: forever changed: false - # Identifiers are not included in idempotency checks so a new cert will not be generated - test_name: Generate cert with identifier type: user identifier: foo valid_from: always valid_to: forever changed: false + - test_name: Generate cert with identifier - full idempotency + type: user + identifier: foo + valid_from: always + valid_to: forever + regenerate: full_idempotence + changed: true - name: Execute idempotency tests openssh_cert: @@ -251,6 +262,7 @@ valid_at: "{{ test_case.valid_at | default(omit) }}" valid_from: "{{ test_case.valid_from | default(omit) }}" valid_to: "{{ test_case.valid_to | default(omit) }}" + regenerate: "{{ test_case.regenerate | default(omit) }}" check_mode: "{{ test_case.check_mode | default(omit) }}" register: idempotency_test_output loop: "{{ test_cases }}" diff --git a/tests/integration/targets/openssh_cert/tests/key_idempotency.yml b/tests/integration/targets/openssh_cert/tests/key_idempotency.yml index ecaeae7e7..b8e364348 100644 --- a/tests/integration/targets/openssh_cert/tests/key_idempotency.yml +++ b/tests/integration/targets/openssh_cert/tests/key_idempotency.yml @@ -40,12 +40,35 @@ valid_to: forever register: new_public_key_output -# Signing key and public key are not considered during idempotency checks -- name: Assert changes to public key or signing key results in no change +- name: Generate cert with new signing key - full idempotency + openssh_cert: + type: user + path: "{{ certificate_path }}" + public_key: "{{ public_key }}" + signing_key: "{{ new_signing_key }}" + valid_from: always + valid_to: forever + regenerate: full_idempotence + register: new_signing_key_full_idempotency_output + +- name: Generate cert with new pubic key - full idempotency + openssh_cert: + type: user + path: "{{ certificate_path }}" + public_key: "{{ new_public_key }}" + signing_key: "{{ new_signing_key }}" + valid_from: always + valid_to: forever + regenerate: full_idempotence + register: new_public_key_full_idempotency_output + +- name: Assert changes to public key or signing key results in no change unless idempotency=full assert: that: - new_signing_key_output is not changed - new_public_key_output is not changed + - new_signing_key_full_idempotency_output is changed + - new_public_key_full_idempotency_output is changed - name: Remove certificate openssh_cert: diff --git a/tests/integration/targets/openssh_cert/tests/options_idempotency.yml b/tests/integration/targets/openssh_cert/tests/options_idempotency.yml new file mode 100644 index 000000000..cb2c35e25 --- /dev/null +++ b/tests/integration/targets/openssh_cert/tests/options_idempotency.yml @@ -0,0 +1,102 @@ +#################################################################### +# WARNING: These are designed specifically for Ansible tests # +# and should not be used as examples of how to write Ansible roles # +#################################################################### + +- name: Generate cert with no options + openssh_cert: + type: user + path: "{{ certificate_path }}" + public_key: "{{ public_key }}" + signing_key: "{{ signing_key }}" + valid_from: always + valid_to: forever + options: + - clear + regenerate: full_idempotence + register: no_options + +- name: Generate cert with no options with explicit directives + openssh_cert: + type: user + path: "{{ certificate_path }}" + public_key: "{{ public_key }}" + signing_key: "{{ signing_key }}" + valid_from: always + valid_to: forever + options: + - no-user-rc + - no-x11-forwarding + - no-agent-forwarding + - no-port-forwarding + - no-pty + regenerate: full_idempotence + register: no_options_explicit_directives + +- name: Generate cert with explicit extension + openssh_cert: + type: user + path: "{{ certificate_path }}" + public_key: "{{ public_key }}" + signing_key: "{{ signing_key }}" + valid_from: always + valid_to: forever + options: + - clear + - permit-pty + regenerate: full_idempotence + register: explicit_extension_before + +- name: Generate cert with explicit extension (idempotency) + openssh_cert: + type: user + path: "{{ certificate_path }}" + public_key: "{{ public_key }}" + signing_key: "{{ signing_key }}" + valid_from: always + valid_to: forever + options: + - clear + - permit-pty + regenerate: full_idempotence + register: explicit_extension_after + +- name: Generate cert with explicit extension and corresponding directive + openssh_cert: + type: user + path: "{{ certificate_path }}" + public_key: "{{ public_key }}" + signing_key: "{{ signing_key }}" + valid_from: always + valid_to: forever + options: + - no-pty + - permit-pty + regenerate: full_idempotence + register: explicit_extension_and_directive + +- name: Generate cert with default options + openssh_cert: + type: user + path: "{{ certificate_path }}" + public_key: "{{ public_key }}" + signing_key: "{{ signing_key }}" + valid_from: always + valid_to: forever + regenerate: full_idempotence + register: default_options + +- name: Assert options results + assert: + that: + - no_options is changed + - no_options_explicit_directives is not changed + - explicit_extension_before is changed + - explicit_extension_after is not changed + - explicit_extension_and_directive is changed + - default_options is not changed + +- name: Remove certificate + openssh_cert: + path: "{{ certificate_path }}" + state: absent diff --git a/tests/integration/targets/openssh_cert/tests/regenerate.yml b/tests/integration/targets/openssh_cert/tests/regenerate.yml new file mode 100644 index 000000000..38f4f7fab --- /dev/null +++ b/tests/integration/targets/openssh_cert/tests/regenerate.yml @@ -0,0 +1,135 @@ +#################################################################### +# WARNING: These are designed specifically for Ansible tests # +# and should not be used as examples of how to write Ansible roles # +#################################################################### + +- set_fact: + test_cases: + - test_name: Generate certificate + type: user + signing_key: "{{ signing_key }}" + public_key: "{{ public_key }}" + path: "{{ certificate_path }}" + valid_from: always + valid_to: forever + regenerate: never + changed: true + - test_name: Regenerate never - same options + type: user + signing_key: "{{ signing_key }}" + public_key: "{{ public_key }}" + path: "{{ certificate_path }}" + valid_from: always + valid_to: forever + regenerate: never + changed: false + - test_name: Regenerate never - different options + type: user + signing_key: "{{ signing_key }}" + public_key: "{{ public_key }}" + path: "{{ certificate_path }}" + valid_from: always + valid_to: forever + options: + - clear + regenerate: never + changed: false + - test_name: Regenerate never with force + force: true + type: user + signing_key: "{{ signing_key }}" + public_key: "{{ public_key }}" + path: "{{ certificate_path }}" + valid_from: always + valid_to: forever + regenerate: never + changed: true + - test_name: Remove certificate + path: "{{ certificate_path }}" + state: absent + changed: true + - test_name: Regenerate fail - new certificate + type: user + signing_key: "{{ signing_key }}" + public_key: "{{ public_key }}" + path: "{{ certificate_path }}" + valid_from: always + valid_to: forever + regenerate: fail + changed: true + - test_name: Regenerate fail - same options + type: user + signing_key: "{{ signing_key }}" + public_key: "{{ public_key }}" + path: "{{ certificate_path }}" + valid_from: always + valid_to: forever + regenerate: fail + changed: false + - test_name: Regenerate fail - different options + type: user + signing_key: "{{ signing_key }}" + public_key: "{{ public_key }}" + path: "{{ certificate_path }}" + valid_from: always + valid_to: forever + options: + - clear + regenerate: fail + changed: false + ignore_errors: true + - test_name: Regenerate fail with force + force: true + type: user + signing_key: "{{ signing_key }}" + public_key: "{{ public_key }}" + path: "{{ certificate_path }}" + valid_from: always + valid_to: forever + regenerate: fail + changed: true + - test_name: Regenerate always + type: user + signing_key: "{{ signing_key }}" + public_key: "{{ public_key }}" + path: "{{ certificate_path }}" + valid_from: always + valid_to: forever + regenerate: always + changed: true + +- name: Execute regenerate tests + openssh_cert: + force: "{{ test_case.force | default(omit) }}" + options: "{{ test_case.options | default(omit) }}" + path: "{{ test_case.path | default(omit) }}" + public_key: "{{ test_case.public_key | default(omit) }}" + principals: "{{ test_case.principals | default(omit) }}" + regenerate: "{{ test_case.regenerate | default(omit) }}" + serial_number: "{{ test_case.serial_number | default(omit) }}" + signing_key: "{{ test_case.signing_key | default(omit) }}" + state: "{{ test_case.state | default(omit) }}" + type: "{{ test_case.type | default(omit) }}" + valid_at: "{{ test_case.valid_at | default(omit) }}" + valid_from: "{{ test_case.valid_from | default(omit) }}" + valid_to: "{{ test_case.valid_to | default(omit) }}" + check_mode: "{{ test_case.check_mode | default(omit) }}" + ignore_errors: "{{ test_case.ignore_errors | default(omit) }}" + register: regenerate_tests_output + loop: "{{ test_cases }}" + loop_control: + loop_var: test_case + +- name: Assert task statuses + assert: + that: + - result.changed == test_cases[index].changed + loop: "{{ regenerate_tests_output.results }}" + loop_control: + index_var: index + loop_var: result + +- name: Remove certificate + openssh_cert: + path: "{{ certificate_path }}" + state: absent diff --git a/tests/unit/plugins/module_utils/openssh/test_certificate.py b/tests/unit/plugins/module_utils/openssh/test_certificate.py index c9e2f4345..e6574ea2e 100644 --- a/tests/unit/plugins/module_utils/openssh/test_certificate.py +++ b/tests/unit/plugins/module_utils/openssh/test_certificate.py @@ -9,7 +9,9 @@ from ansible_collections.community.crypto.plugins.module_utils.openssh.certificate import ( OpensshCertificate, - OpensshCertificateTimeParameters + OpensshCertificateOption, + OpensshCertificateTimeParameters, + parse_option_list ) # Type: ssh-rsa-cert-v01@openssh.com user certificate @@ -42,7 +44,7 @@ b'Q7c8c/tNDaL7uqV46QQAAADcAAAAHc3NoLWRzcwAAAChaQ94wqca+KhkHtbkLpjvGsfu0Gy03SAb0+o11Shk/BXnK7N/cwEVD ' + b'ansible@ansible-host' ) -RSA_FINGERPRINT = b'SHA256:SvUwwUer4AwsdePYseJR3LcZS8lnKi6BqiL51Dop030' +RSA_FINGERPRINT = 'SHA256:SvUwwUer4AwsdePYseJR3LcZS8lnKi6BqiL51Dop030' # Type: ssh-dss-cert-v01@openssh.com user certificate # Public key: DSA-CERT SHA256:YCdJ2lYU+FSkWUud7zg1SJszprXoRGNU/GVcqXUjgC8 # Signing CA: ECDSA SHA256:w9lp4zGRJShhm4DzO3ulVm0BEcR0PMjrM6VanQo4C0w @@ -64,7 +66,7 @@ b'wvanQKM01uU73swNIt+ZFra9kRSi21xjzgMPn7U0AAABkAAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAABJAAAAIGmlKa/riG7+EpoW6dTJY6' + b'0N8BrEcniKgOxdRM1EPJ2DAAAAIQDnK4stvbvS+Bn0/42Was7uOfJtnLYXs5EuB2L3uejPcQ== ansible@ansible-host' ) -DSA_FINGERPRINT = b'SHA256:YCdJ2lYU+FSkWUud7zg1SJszprXoRGNU/GVcqXUjgC8' +DSA_FINGERPRINT = 'SHA256:YCdJ2lYU+FSkWUud7zg1SJszprXoRGNU/GVcqXUjgC8' # Type: ecdsa-sha2-nistp256-cert-v01@openssh.com user certificate # Public key: ECDSA-CERT SHA256:w9lp4zGRJShhm4DzO3ulVm0BEcR0PMjrM6VanQo4C0w # Signing CA: ED25519 SHA256:NP4JdfkCopbjwMepq0aPrpMz13cNmEd+uDOxC/j9N40 @@ -90,7 +92,7 @@ b'TUxOQAAAEAdp3eOLRN5t2wW29TBWbz604uuXg88jH4RA4HDhbRupa/x2rN3j6iZQ4VXPLA4JtdfIslHFkH6HUlxU8XsoJwP ' + b'ansible@ansible-host' ) -ECDSA_FINGERPRINT = b'SHA256:w9lp4zGRJShhm4DzO3ulVm0BEcR0PMjrM6VanQo4C0w' +ECDSA_FINGERPRINT = 'SHA256:w9lp4zGRJShhm4DzO3ulVm0BEcR0PMjrM6VanQo4C0w' # Type: ssh-ed25519-cert-v01@openssh.com user certificate # Public key: ED25519-CERT SHA256:NP4JdfkCopbjwMepq0aPrpMz13cNmEd+uDOxC/j9N40 # Signing CA: RSA SHA256:SvUwwUer4AwsdePYseJR3LcZS8lnKi6BqiL51Dop030 @@ -114,20 +116,20 @@ b'7WJpz3eypBJt4TglwRTJpp54IMN2CyDQm0N97x9ris8jQQHlCF2EgZp1u4aOiZJTSJ5d4hapO0uZwXOI9AIWy/lmx0/6jX07MWrs4iXpfiF' + b'5T4s6kEn7YW4SaJ0Z7xGp3V0vDOxh+jwHZGD5GM449Il6QxQwDY5BSJq+iMR467yaIjw2g8Kt4ZiU= ansible@ansible-host' ) -ED25519_FINGERPRINT = b'SHA256:NP4JdfkCopbjwMepq0aPrpMz13cNmEd+uDOxC/j9N40' +ED25519_FINGERPRINT = 'SHA256:NP4JdfkCopbjwMepq0aPrpMz13cNmEd+uDOxC/j9N40' # garbage INVALID_DATA = b'yDspTN+BJzvIK2Q+CRD3qBDVSi+YqSxwyz432VEaHKlXbuLURirY0QpuBCqgR6tCtWW5vEGkXKZ3' -VALID_OPTS = [(b'force-command', b'/usr/bin/csh')] -INVALID_OPTS = [(b'test', b'undefined')] +VALID_OPTS = [OpensshCertificateOption('critical', 'force-command', '/usr/bin/csh')] +INVALID_OPTS = [OpensshCertificateOption('critical', 'test', 'undefined')] VALID_EXTENSIONS = [ - (b'permit-X11-forwarding', b''), - (b'permit-agent-forwarding', b''), - (b'permit-port-forwarding', b''), - (b'permit-pty', b''), - (b'permit-user-rc', b''), + OpensshCertificateOption('extension', 'permit-x11-forwarding', ''), + OpensshCertificateOption('extension', 'permit-agent-forwarding', ''), + OpensshCertificateOption('extension', 'permit-port-forwarding', ''), + OpensshCertificateOption('extension', 'permit-pty', ''), + OpensshCertificateOption('extension', 'permit-user-rc', ''), ] -INVALID_EXTENSIONS = [(b'test', b'')] +INVALID_EXTENSIONS = [OpensshCertificateOption('extension', 'test', '')] VALID_TIME_PARAMETERS = [ (0, "always", "always", 0, @@ -177,15 +179,31 @@ ("2000-01-01 00:00:00", "2000-01-01 00:00:01", "2000-01-01 00:00:02"), ] +VALID_OPTIONS = [ + ("force-command=/usr/bin/csh", OpensshCertificateOption('critical', 'force-command', '/usr/bin/csh')), + ("Force-Command=/Usr/Bin/Csh", OpensshCertificateOption('critical', 'force-command', '/Usr/Bin/Csh')), + ("permit-x11-forwarding", OpensshCertificateOption('extension', 'permit-x11-forwarding', '')), + ("permit-X11-forwarding", OpensshCertificateOption('extension', 'permit-x11-forwarding', '')), + ("critical:foo=bar", OpensshCertificateOption('critical', 'foo', 'bar')), + ("extension:foo", OpensshCertificateOption('extension', 'foo', '')), +] + +INVALID_OPTIONS = [ + "foobar", + "foo=bar", + 'foo:bar=baz', + [], +] + def test_rsa_certificate(tmpdir): cert_file = tmpdir / 'id_rsa-cert.pub' cert_file.write(RSA_CERT_SIGNED_BY_DSA, mode='wb') cert = OpensshCertificate.load(str(cert_file)) - assert cert.key_id == b'test' + assert cert.key_id == 'test' assert cert.serial == 0 - assert cert.type_string == b'ssh-rsa-cert-v01@openssh.com' + assert cert.type_string == 'ssh-rsa-cert-v01@openssh.com' assert cert.public_key == RSA_FINGERPRINT assert cert.signing_key == DSA_FINGERPRINT @@ -196,7 +214,7 @@ def test_dsa_certificate(tmpdir): cert = OpensshCertificate.load(str(cert_file)) - assert cert.type_string == b'ssh-dss-cert-v01@openssh.com' + assert cert.type_string == 'ssh-dss-cert-v01@openssh.com' assert cert.public_key == DSA_FINGERPRINT assert cert.signing_key == ECDSA_FINGERPRINT assert cert.critical_options == [] @@ -208,7 +226,7 @@ def test_ecdsa_certificate(tmpdir): cert_file.write(ECDSA_CERT_SIGNED_BY_ED25519_VALID_OPTS) cert = OpensshCertificate.load(str(cert_file)) - assert cert.type_string == b'ecdsa-sha2-nistp256-cert-v01@openssh.com' + assert cert.type_string == 'ecdsa-sha2-nistp256-cert-v01@openssh.com' assert cert.public_key == ECDSA_FINGERPRINT assert cert.signing_key == ED25519_FINGERPRINT assert cert.critical_options == VALID_OPTS @@ -220,7 +238,7 @@ def test_ed25519_certificate(tmpdir): cert_file.write(ED25519_CERT_SIGNED_BY_RSA_INVALID_OPTS) cert = OpensshCertificate.load(str(cert_file)) - assert cert.type_string == b'ssh-ed25519-cert-v01@openssh.com' + assert cert.type_string == 'ssh-ed25519-cert-v01@openssh.com' assert cert.public_key == ED25519_FINGERPRINT assert cert.signing_key == RSA_FINGERPRINT assert cert.critical_options == INVALID_OPTS @@ -275,3 +293,56 @@ def test_valid_validity_test(valid_from, valid_to, valid_at): @pytest.mark.parametrize("valid_from,valid_to,valid_at", INVALID_VALIDITY_TEST) def test_invalid_validity_test(valid_from, valid_to, valid_at): assert not OpensshCertificateTimeParameters(valid_from, valid_to).within_range(valid_at) + + +@pytest.mark.parametrize("option_string,option_object", VALID_OPTIONS) +def test_valid_options(option_string, option_object): + assert OpensshCertificateOption.from_string(option_string) == option_object + + +@pytest.mark.parametrize("option_string", INVALID_OPTIONS) +def test_invalid_options(option_string): + with pytest.raises(ValueError): + OpensshCertificateOption.from_string(option_string) + + +def test_parse_option_list(): + critical_options, extensions = parse_option_list(['force-command=/usr/bin/csh']) + + critical_option_objects = [ + OpensshCertificateOption.from_string('force-command=/usr/bin/csh'), + ] + + extension_objects = [ + OpensshCertificateOption.from_string('permit-x11-forwarding'), + OpensshCertificateOption.from_string('permit-agent-forwarding'), + OpensshCertificateOption.from_string('permit-port-forwarding'), + OpensshCertificateOption.from_string('permit-user-rc'), + OpensshCertificateOption.from_string('permit-pty'), + ] + + assert set(critical_options) == set(critical_option_objects) + assert set(extensions) == set(extension_objects) + + +def test_parse_option_list_with_directives(): + critical_options, extensions = parse_option_list(['clear', 'no-pty', 'permit-pty', 'permit-user-rc']) + + extension_objects = [ + OpensshCertificateOption.from_string('permit-user-rc'), + OpensshCertificateOption.from_string('permit-pty'), + ] + + assert set(critical_options) == set() + assert set(extensions) == set(extension_objects) + + +def test_parse_option_list_case_sensitivity(): + critical_options, extensions = parse_option_list(['CLEAR', 'no-X11-forwarding', 'permit-X11-forwarding']) + + extension_objects = [ + OpensshCertificateOption.from_string('permit-x11-forwarding'), + ] + + assert set(critical_options) == set() + assert set(extensions) == set(extension_objects)