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 b9562a8916.

* 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>

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).

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(

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'