Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

community.general.sudoers should do a syntax check #4745

Closed
1 task done
s-hamann opened this issue May 29, 2022 · 5 comments · Fixed by #4794
Closed
1 task done

community.general.sudoers should do a syntax check #4745

s-hamann opened this issue May 29, 2022 · 5 comments · Fixed by #4794
Labels
feature This issue/PR relates to a feature request module module plugins plugin (any type) system

Comments

@s-hamann
Copy link
Contributor

Summary

The sudoers module currently allows writing broken sudoers files, for example when commands is not an absolute path. This kind of user error can easily be detected by running visudo -c -f ... on the new file. I think it makes sense for the sudoers module to run this check.

Some errors, such as duplicate aliases, can only be detected in the context of the other sudoers files. According to this post a more comprehensive check can be done by running a full configuration check visudo -c after installing the new file. However, I'm not sure if this is worth implementing. The sudoers module does not currently seem to be capable of producing syntax error that can not be detected by a simple visudo -c -f ....

Issue Type

Feature Idea

Component Name

sudoers

Additional Information

Slightly modified example task from the documentation:

- name: >-
    Allow the alice user to run sudo /bin/systemctl restart my-service or
    sudo /bin/systemctl reload my-service, but a password is required
  community.general.sudoers:
    name: alice-service
    user: alice
    commands:
      - systemctl restart my-service

sudo commands complain about the syntax error:

visudo -c -f /etc/sudoers.d/alice-service
alice-service:1:21: syntax error
alice ALL=NOPASSWD: systemctl restart my-service
                    ^~~~~~~~~

Code of Conduct

  • I agree to follow the Ansible Code of Conduct
@ansibullbot
Copy link
Collaborator

Files identified in the description:

If these files are incorrect, please update the component name section of the description or use the !component bot command.

click here for bot help

@ansibullbot
Copy link
Collaborator

cc @JonEllis @JonEllis0
click here for bot help

@ansibullbot ansibullbot added feature This issue/PR relates to a feature request module module plugins plugin (any type) system labels May 29, 2022
@repcsi
Copy link

repcsi commented May 30, 2022

For me on RHEL 8 boxes the "visudo -c "check command reports that the file permissions are bad on files that were created by the sudoers module. New files are created with 644. Should I create a new issue for this?

# visudo -c 
/etc/sudoers: parsed OK
/etc/sudores.d/some-new-rule: bad permissions, should be mode 0440

# umask
0022

# ls -l /etc/sudores.d/some-new-rule
-rw-r--r--. 1 root root 340 May 30 2020 some-new-rule

@felixfontein
Copy link
Collaborator

@repcsi since it's not related to the current issue, yes please.

@JonEllis
Copy link
Contributor

Great suggestion. Validation sounds like a good addition to me.

This was referenced Nov 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature This issue/PR relates to a feature request module module plugins plugin (any type) system
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants