From d0b39271b348e03e1e74a072c744ab59e501dc6c Mon Sep 17 00:00:00 2001
From: "patchback[bot]" <45432694+patchback[bot]@users.noreply.github.com>
Date: Tue, 21 Jun 2022 12:52:21 +0200
Subject: [PATCH] Sudoers validate (#4794) (#4866)
* 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 <felix@fontein.de>
* Add version_added to validation property
Co-authored-by: Felix Fontein <felix@fontein.de>
* Also validate failed sudoers validation error message
Co-authored-by: Felix Fontein <felix@fontein.de>
* 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 <felix@fontein.de>
* Remove trailing extra empty line to appease sanity checker
Co-authored-by: Felix Fontein <felix@fontein.de>
(cherry picked from commit 97c72f88b7e4c2e3c9a28fff7aa112225536d953)
Co-authored-by: Jon Ellis <ellis.jp@gmail.com>
---
.../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'