saltstack-formulas / sudoers-formula

sudoers-formula

Set up sudo and the sudoers included files.

Table of Contents

• General notes
• Contributing to this repo
• Available states
  • sudoers
  • sudoers.included
• Testing
  • Requirements
  • bin/kitchen converge
  • bin/kitchen verify
  • bin/kitchen destroy
  • bin/kitchen test
  • bin/kitchen login

General notes

See the full SaltStack Formulas installation and usage instructions.

If you are interested in writing or contributing to formulas, please pay attention to the Writing Formula Section.

If you want to use this formula, please pay attention to the FORMULA file and/or git tag, which contains the currently released version. This formula is versioned according to Semantic Versioning.

See Formula Versioning Section for more details.

Contributing to this repo

Commit message formatting is significant!!

Please see How to contribute for more details.

Available states

• sudoers
• sudoers.included

sudoers

Set up the sudoers file

sudoers.included

Set up an additional sudoers included file.

Testing

Linux testing is done with kitchen-salt.

Requirements

• Ruby
• Docker

$ gem install bundler
$ bundle install
$ bin/kitchen test [platform]

Where [platform] is the platform name defined in kitchen.yml, e.g. debian-9-2019-2-py3.

bin/kitchen converge

Creates the docker instance and runs the sudoers main state, ready for testing.

bin/kitchen verify

Runs the inspec tests on the actual instance.

bin/kitchen destroy

Removes the docker instance.

bin/kitchen test

Runs all of the stages above in one go: i.e. destroy + converge + verify + destroy.

bin/kitchen login

Gives you SSH access to the instance for manual testing.

sudoers-formula's Issues

Can not figure out how to add and env_keep line to an included file.

I have file that I'm trying to include. The non-salt version looks like this:

jenkins` ALL=NOPASSWD: /usr/sbin/cowbuilder, /usr/sbin/chroot, /usr/bin/pbuilder
Defaults env_keep+="DEB_* DIST ARCH ADT BUILDBRANCHSTR APPVERSIONSTR  DEB_KEEP_BUILD_ENV"

I have figured out how to write the first line in a pillar, but can get the syntax right for the env_keep+= line. Does anyone have any suggestions?

Currently I am trying a pillar like this:

sudoers:
  includedir: /etc/sudoers.d
  included_files:
    /etc/sudoers.d/91-jenkins:
      users:
        jenkins:
          - 'ALL=NOPASSWD: /usr/sbin/cowbuilder'
          - 'All=NOPASSWD: /usr/sbin/chroot'
          - 'ALL=NOPASSWD: /usr/bin/pbuilder'
      defaults:
        - env_keep='"DEB_* DIST ARCH ADT BUILDBRANCHSTR APPVERSIONSTR  DEB_KEEP_BUILD_ENV"'

I have tried several other versions of this, but get the error:

Unable to manage file: Jinja variable 'list object' has no attribute 'get'
Without the "env_keep" line the pillar is processed properly.

Error using sudoers.included

I am experiencing errors with the sudoers.included state related to the 'sudoers' variable in the /etc/sudoers template. Here is my configuration:

In top.sls, sudoers is applied globally, and sudoers.included is applied to the host experiencing the error:
top.sls

base:
  '*':
    - sudoers
  'myhost':
    - sudoers.included

The pillar data for myhost is as follows:

sudoers:
  groups:
    itops: 'ALL=(ALL) ALL'
    sudo: 'ALL=(ALL) ALL'
  includedir: /etc/sudoers.d
  included_files:
    /etc/sudoers.d/git-salt:
      - users:
        - git: 'ALL= /usr/bin/salt-call'

However when salt is called, I receive the following error:

          ID: /etc/sudoers.d/git-salt
    Function: file.managed
      Result: False
     Comment: Unable to manage file: Jinja variable 'list object' has no attribute 'get'; line 18

              ---
              [...]
                  {%- set users = sudoers.get('users', {}) %}
                  {%- set groups = sudoers.get('groups', {}) %}
                {%- endif %}
                {%- set includedir = sudoers.get('includedir', '/etc/sudoers.d') -%}
              {%- else %}
                {%- set defaults = sudoers.get('defaults', []) %}    <======================
                {%- set users = sudoers.get('users', {}) %}
                {%- set groups = sudoers.get('groups', {}) %}
                {%- set includedir = sudoers.get('includedir', None) %}
              {%- endif %}
              {%- set aliases = sudoers.get('aliases', {}) %}
              [...]
              ---

From looking at the code, it seems that indeed sudoers is not initialized if the file is included, which it is from sudoers.included. sudoers.included does initialize sudoers from pillar, but I'm not sure how or if the context gets passed between the included file.

purge_includedir doesn not seem to work

Your setup

Formula commit hash / release tag

v0.23.4

Versions reports (master & minion)

Salt Version:
          Salt: 3003
 
Dependency Versions:
          cffi: Not Installed
      cherrypy: Not Installed
      dateutil: 2.7.3
     docker-py: Not Installed
         gitdb: Not Installed
     gitpython: Not Installed
        Jinja2: 2.10.1
       libgit2: 0.28.3
      M2Crypto: Not Installed
          Mako: Not Installed
       msgpack: 0.6.2
  msgpack-pure: Not Installed
  mysql-python: Not Installed
     pycparser: Not Installed
      pycrypto: Not Installed
  pycryptodome: 3.6.1
        pygit2: 1.0.3
        Python: 3.8.5 (default, Jan 27 2021, 15:41:15)
  python-gnupg: 0.4.5
        PyYAML: 5.3.1
         PyZMQ: 18.1.1
         smmap: Not Installed
       timelib: Not Installed
       Tornado: 4.5.3
           ZMQ: 4.3.2
 
System Versions:
          dist: ubuntu 20.04 focal
        locale: utf-8
       machine: x86_64
       release: 5.4.0-73-generic
        system: Linux
       version: Ubuntu 20.04 focal

Pillar / config used

sudoers:
  purge_includedir: true
  groups:
    sudo:
    - 'ALL=(ALL) NOPASSWD: ALL'

---

Bug details

Describe the bug

The purge_includedir option doesn't do anything when set to true

Steps to reproduce the bug

1. Set option
2. Add a random file to /etc/sudoers.d/
3. Execute salt
4. Watch how the file is not deleted

Expected behaviour

Extra files to disappear

Attempts to fix the bug

I made sure the correct value is exported in Pillar:

    sudoers:
        ----------
        groups:
            ----------
            sudo:
                - ALL=(ALL) NOPASSWD: ALL
        purge_includedir:
            True

Additional context

I think this might be related to saltstack/salt#26605 ?

[BUG] Rule ordering

Your setup

Formula commit hash / release tag

Versions reports (master & minion)

Pillar / config used

---

Bug details

Describe the bug

Currently, rules are run through dictsort, which causes issues with users/groups which match multiple rules. Sudo takes the last matching rule - which may not always be desirable - for example if one wants to have NOPASSWD for a specific command take priority over a general password enforced rule.

Example:

sudoers:
  groups:
    wheel:
      - 'ALL=(ALL) ALL'
    hypervisor.cluster-admins:
      - >-
        {{ grains['host'] }}=(root) NOPASSWD:
        /sbin/multipath -f [[\:alnum\:]]*,

Returns:

%hypervisor.cluster-admins falkor21=(root) NOPASSWD: /sbin/multipath -f [[\:alnum\:]]*
%wheel ALL=(ALL) ALL

It should be vice versa, as now, for users who are in both groups (wheel and hypervisor.cluster-admins) the NOPASSWD rule never matches, as the wheel rule takes priority.

I'm not sure if just removing dictsort is the right solution either though, I think it requires some other logic.

Steps to reproduce the bug

Expected behaviour

Attempts to fix the bug

Additional context

Wrong brackets in files/sudoers - line 13

Shouldn't lines 13 and 24 have curly brackets in files/sudoers?

      {%- set generic_defaults = defaults.get('generic', []) %}

to

      {%- set generic_defaults = defaults.get('generic', {}) %}

Both ways however, I get error

      ID: /etc/sudoers
Function: file.managed
  Result: False
 Comment: Unable to manage file: Jinja variable 'generic_defaults' is undefined

Rework how users are added to sudo files to ensure they are generated the same way here and in the users formula

This PR: saltstack-formulas/users-formula#42 (comment) was created as a workaround because users are created in both /etc/sudoers, and /etc/sudoers.d/. If this was standardized between the two formulas, that would be good. We might actually just need to pull this out of the users formula.

[FEATURE] sudo.included.clean state

Describe alternatives you've considered

Add sudo.included.clean state to reverse sudo.included state.

Finalise regression fix #70 (`755` & InSpec test)

CC: @kmosher @jynolen @daks.

#70 was merged quickly due to a regression but it appears that the directory mode should have been set to 755:

sudoers-formula/sudoers/included.sls

Line 16 in 9c7ef40

- mode: 750

@daks Mentioned an InSpec test that could also be added:

#70 (comment)

Not sure why I set it to 440 but in fact on Debian 9 or 10 it's 755 so no problem for me to merge this PR as soon as possible.

One improvement could be to add a basic test on this directory mode, here https://github.com/saltstack-formulas/sudoers-formula/blob/master/test/integration/default/controls/config.rb, with something like

  describe directory('/etc/sudoers.d/') do
    it { should be_owned_by 'root' }
    it { should be_grouped_into 'root' }
    its('mode') { should cmp '0755' }
  end

A better one (but I'm not sure I know how to run it) would be to set some NOPASSWD sudo rules and try to use it.

Example usage

Could someone give actual example of how to add a user to the sudoers file? What are the arguments for example? Or is that supposed to be obvious from the pillar? Thanks :)

Adding support for netgroups

Hello,

While recently using sudoers-formula on a machine at work, I learned that it doesn't seem to support specifying netgroup information as mentioned in the sudoers man page. To that end, I've created PR #44 to add support for this. There's probably not a lot of people that need this, but I know at least one. :-)

Tagging

Is there a reason why the releases to master is not tagged? Use case is, I am looking to pin a particular version I would want to use in our saltstack infra.

Does not support mac os

Salt supports mac os minions, but this formula does not ?

includedir directive breaks sudoers on older versions if directory does not exist

I'm not sure one would want to bother as this should affect very few distributions.

On RHEL5/CentOS5, the /etc/sudoers.d directory does not exist by default, and /etc/sudoers.d is by default set as an includedir by the formula.

The bug in older versions of sudo means that the sudoers file will be seen as corrupt, leaving sudo broken when this directory does not exist.

A solution would be to have the formula create the directory, bypassing this issue. Then again, given the limited scope of this issue, it might not be worth doing anything about it.

I'm not sure if the issue remains present on sudo bundled with newer distributions, as they usually have a sudoers.d directory by default, thus making this a non issue for them in any case.

Pillar data is being ignored

I have a top.sls file:

base:
  '*':
    - sudoers

And as a test I have a pillar file in /srv/salt/pillar/sudoers/init.sls

sudoers:
  users:
    johndoe:
      - 'All=(ALL) ALL'

When I run the highstate I see this output:

pbp:
----------
          ID: sudo
    Function: pkg.installed
      Result: True
     Comment: Package sudo is already installed
     Started: 14:48:45.512514
    Duration: 441.472 ms
     Changes:   
----------
          ID: /etc/sudoers
    Function: file.managed
      Result: True
     Comment: The file /etc/sudoers is in the correct state
     Started: 14:48:45.955057
    Duration: 46.428 ms
     Changes:   

Summary
------------
Succeeded: 2
Failed:    0
------------
Total states run:     2

On my minion it shows a sudoes file that is managed by salt, but it does not have the additional users in my pillar data. I realize this is may be a fundamental error, but I've tried several things and can't seem to get the pillar data included.

Thanks,
Robert

error after upgrading to 2017 version of master

After upgrading salt-master to 2017, this formula keeps getting this error, at least on ubuntu:

 ----------
          ID: /etc/sudoers
    Function: file.managed
      Result: False
     Comment: Failed to commit change: [Errno 2] No such file or directory: /tmp/__salt.tmp.cboCne
     Started: 23:51:11.292604
    Duration: 1441.796 ms
     Changes:
              ----------
              diff:
                  Replace text file with binary file
----------

filter sudoers file by host specifics

The sudoers file currently includes all pillar data. this means that a host sudoers file will include all the aliases for example even if it does not use them. It would be nice to have the sudoers file tailored to the host to isolate issues. for instance, now if I go in and add a new hosts aliase and make a mistake in the pillar so that salt cant catch it because its yaml correct but sudoers file wont parse it (it happened believe me) this will break all sudoers files in all hosts. but if the aliases are added only in the files that use them this will limit the impact of a mistake like this.
How would one go about doing so? is it possible to have multiple sudoers pillar files and filter what goes into the sudoers file based on host its going on?

Possible Pull Request for sudoers mac/syntax/sudoers.d

Hi,

I saw there is now a sudoers formula, which is great.

I made one a while back, and I was wondering if there is interest to merge it in. It offers the following benefits:

• syntax-safe: no changes are made before syntax is known to pass
  • This prevents locking sudo due to syntax errors
  • files are uploaded to separate folder, syntax checked there, then moved in-place
• use of /etc/sudoers.d: each sudo function can be put in a separate file
• works for macOS (MacBook Retina doesn't run Linux yet)

If it is likely that a pull request would be approved, I could create it.