From 97c72f88b7e4c2e3c9a28fff7aa112225536d953 Mon Sep 17 00:00:00 2001 From: Jon Ellis Date: Tue, 21 Jun 2022 11:41:24 +0100 Subject: [PATCH] Sudoers validate (#4794) * Use visudo to validate sudoers rules before use * Replace use of subprocess.Popen with module.run_command * Switch out apt for package * Check file mode when verifying file to determine whether something needs to change * Only install sudo package for debian and redhat environments (when testing) * Attempt to install sudo on FreeBSD too * Try just installing sudo for non-darwin machines * Don't validate file ownership * Attempt to install sudo on all platforms * Revert "Attempt to install sudo on all platforms" This reverts commit b9562a8916c1f3e57f097e74a3db0de8794f67df. * Remove file permissions changes from this PR * Add changelog fragment for 4794 sudoers validation * Add option to control when sudoers validation is used * Update changelog fragment Co-authored-by: Felix Fontein * Add version_added to validation property Co-authored-by: Felix Fontein * Also validate failed sudoers validation error message Co-authored-by: Felix Fontein * Make visudo not executable instead of trying to delete it * Update edge case validation * Write invalid sudoers file to alternative path to avoid breaking sudo * Don't try to remove or otherwise modify visudo on Darwin * Update plugins/modules/system/sudoers.py Co-authored-by: Felix Fontein * Remove trailing extra empty line to appease sanity checker Co-authored-by: Felix Fontein --- .../fragments/4794-sudoers-validation.yml | 2 + plugins/modules/system/sudoers.py | 32 +++++++++ .../targets/sudoers/tasks/main.yml | 69 ++++++++++++++++++- 3 files changed, 101 insertions(+), 2 deletions(-) create mode 100644 changelogs/fragments/4794-sudoers-validation.yml diff --git a/changelogs/fragments/4794-sudoers-validation.yml b/changelogs/fragments/4794-sudoers-validation.yml new file mode 100644 index 0000000000..32caacdc36 --- /dev/null +++ b/changelogs/fragments/4794-sudoers-validation.yml @@ -0,0 +1,2 @@ +minor_changes: + - sudoers - will attempt to validate the proposed sudoers rule using visudo if available, optionally skipped, or required (https://github.com/ansible-collections/community.general/pull/4794, https://github.com/ansible-collections/community.general/issues/4745). diff --git a/plugins/modules/system/sudoers.py b/plugins/modules/system/sudoers.py index d96716c7f9..3b46f108f8 100644 --- a/plugins/modules/system/sudoers.py +++ b/plugins/modules/system/sudoers.py @@ -65,6 +65,15 @@ options: - The name of the user for the sudoers rule. - This option cannot be used in conjunction with I(group). type: str + validation: + description: + - If C(absent), the sudoers rule will be added without validation. + - If C(detect) and visudo is available, then the sudoers rule will be validated by visudo. + - If C(required), visudo must be available to validate the sudoers rule. + type: str + default: detect + choices: [ absent, detect, required ] + version_added: 5.2.0 ''' EXAMPLES = ''' @@ -118,6 +127,8 @@ class Sudoers(object): FILE_MODE = 0o440 def __init__(self, module): + self.module = module + self.check_mode = module.check_mode self.name = module.params['name'] self.user = module.params['user'] @@ -128,6 +139,7 @@ class Sudoers(object): self.sudoers_path = module.params['sudoers_path'] self.file = os.path.join(self.sudoers_path, self.name) self.commands = module.params['commands'] + self.validation = module.params['validation'] def write(self): if self.check_mode: @@ -167,6 +179,20 @@ class Sudoers(object): runas_str = '({runas})'.format(runas=self.runas) if self.runas is not None else '' return "{owner} ALL={runas}{nopasswd} {commands}\n".format(owner=owner, runas=runas_str, nopasswd=nopasswd_str, commands=commands_str) + def validate(self): + if self.validation == 'absent': + return + + visudo_path = self.module.get_bin_path('visudo', required=self.validation == 'required') + if visudo_path is None: + return + + check_command = [visudo_path, '-c', '-f', '-'] + rc, stdout, stderr = self.module.run_command(check_command, data=self.content()) + + if rc != 0: + raise Exception('Failed to validate sudoers rule:\n{stdout}'.format(stdout=stdout)) + def run(self): if self.state == 'absent': if self.exists(): @@ -175,6 +201,8 @@ class Sudoers(object): else: return False + self.validate() + if self.exists() and self.matches(): return False @@ -209,6 +237,10 @@ def main(): 'choices': ['present', 'absent'], }, 'user': {}, + 'validation': { + 'default': 'detect', + 'choices': ['absent', 'detect', 'required'] + }, } module = AnsibleModule( diff --git a/tests/integration/targets/sudoers/tasks/main.yml b/tests/integration/targets/sudoers/tasks/main.yml index f3be2d8092..292aeec4e4 100644 --- a/tests/integration/targets/sudoers/tasks/main.yml +++ b/tests/integration/targets/sudoers/tasks/main.yml @@ -1,11 +1,16 @@ --- # Initialise environment -- name: Register sudoers.d directory +- name: Register variables set_fact: sudoers_path: /etc/sudoers.d alt_sudoers_path: /etc/sudoers_alt +- name: Install sudo package + ansible.builtin.package: + name: sudo + when: ansible_os_family != 'Darwin' + - name: Ensure sudoers directory exists ansible.builtin.file: path: "{{ sudoers_path }}" @@ -135,6 +140,52 @@ register: revoke_rule_1_stat +# Validation testing + +- name: Attempt command without full path to executable + community.general.sudoers: + name: edge-case-1 + state: present + user: alice + commands: systemctl + ignore_errors: true + register: edge_case_1 + + +- name: Attempt command without full path to executable, but disabling validation + community.general.sudoers: + name: edge-case-2 + state: present + user: alice + commands: systemctl + validation: absent + sudoers_path: "{{ alt_sudoers_path }}" + register: edge_case_2 + +- name: find visudo + command: + cmd: which visudo + register: which_visudo + when: ansible_os_family != 'Darwin' + +- name: Prevent visudo being executed + file: + path: "{{ which_visudo.stdout }}" + mode: '-x' + when: ansible_os_family != 'Darwin' + +- name: Attempt command without full path to executable, but enforcing validation with no visudo present + community.general.sudoers: + name: edge-case-3 + state: present + user: alice + commands: systemctl + validation: required + ignore_errors: true + when: ansible_os_family != 'Darwin' + register: edge_case_3 + + - name: Revoke non-existing rule community.general.sudoers: name: non-existing-rule @@ -175,8 +226,22 @@ - "rule_5_contents['content'] | b64decode == 'alice ALL=NOPASSWD: /usr/local/bin/command\n'" - "rule_6_contents['content'] | b64decode == 'alice ALL=(bob)NOPASSWD: /usr/local/bin/command\n'" -- name: Check stats +- name: Check revocation stat ansible.builtin.assert: that: - not revoke_rule_1_stat.stat.exists - not revoke_non_existing_rule_stat.stat.exists + +- name: Check edge case responses + ansible.builtin.assert: + that: + - edge_case_1 is failed + - "'Failed to validate sudoers rule' in edge_case_1.msg" + - edge_case_2 is not failed + +- name: Check missing validation edge case + ansible.builtin.assert: + that: + - edge_case_3 is failed + - "'Failed to find required executable' in edge_case_3.msg" + when: ansible_os_family != 'Darwin'