ansible-hardening: false positive for sudo NOPASSWD check

[Troy Engel (RAX)]

The RHEL7STIG checks for the word 'nopasswd' in /etc/sudoers here:

* https://github.com/openstack/ansible-hardening/blob/master/tasks/rhel7stig/auth.yml#L103

This causes a false positive on a stick RHEL7 server, as there's a commented out example from Red Hat like so present in the /etc/sudoers file:

## Same thing without a password
# %wheel ALL=(ALL) NOPASSWD: ALL

This results in:

TASK [ansible-hardening : Check for 'nopasswd' in sudoers files] ***************
task path: ansible-hardening/tasks/rhel7stig/auth.yml:103
ok: [SERVERNAME] => {"changed": false, "cmd": "grep -ir nopasswd /etc/sudoers /etc/sudoers.d/ || echo 'not found'", "delta": "0:00:00.004764", "end": "2017-07-03 16:34:09.278939", "rc": 0, "start": "2017-07-03 16:34:09.274175", "stderr": "", "stderr_lines": [], "stdout": "/etc/sudoers:# %wheel\tALL=(ALL)\tNOPASSWD: ALL", "stdout_lines": ["/etc/sudoers:# %wheel\tALL=(ALL)\tNOPASSWD: ALL"]}

TASK [ansible-hardening : V-71947 - Users must provide a password for privilege escalation.] ***
task path: ansible-hardening/tasks/rhel7stig/auth.yml:112
ok: [SERVERNAME] => {
    "msg": "The 'NOPASSWD' directive was found in the sudoers configuration files. Remove the directive to ensure that all users must provide a password to run commands as the root user.\n"
}

We should be more smart in ignoring commented out lines, especially since it's delivered this way from the vendor as a default.

[Jean-Philippe Evrard (jean-philippe-evrard)]

We should probably replace
https://github.com/openstack/ansible-hardening/blob/f422da8599c6d8f64ebfefbf0a0aa711ea1f9569/tasks/rhel7stig/auth.yml#L104

by

shell: egrep -ir ^nopasswd /etc/sudoers /etc/sudoers.d/ || echo 'not found'

or something like that?

[Troy Engel (RAX) (terackspace)]

I believe horrible config editing practices are allowed (you can indent a line with spaces and it's still respected), so I might recommend a slightly more robust version:

 egrep -vr "^([[:space:]]*)?(#|$)" test.sudo | grep -i nopasswd || echo 'not found'

That's just a rough first draft, with the /etc/sudoers from RHEL7 copied and then I added this trash to it:

xyzzy ALL=(ALL) ALL
  xyzzy ALL=(ALL) ALL
  # xyzzy ALL=(ALL) ALL
  # xyzzy ALL=(ALL) NOPASSWD:ALL
 xyzzy ALL=(ALL) NOPASSWD:ALL
xyzzy ALL=(ALL) NOPASSWD:ALL

Tested:
$ visudo -c -f test.sudo
test.sudo: parsed OK

Here's just the left hand side of the egrep:

$ egrep -vr "^([[:space:]]*)?(#|$)" test.sudo
Defaults !visiblepw
Defaults always_set_home
Defaults env_reset
Defaults env_keep = "COLORS DISPLAY HOSTNAME HISTSIZE INPUTRC KDEDIR LS_COLORS"
Defaults env_keep += "MAIL PS1 PS2 QTDIR USERNAME LANG LC_ADDRESS LC_CTYPE"
Defaults env_keep += "LC_COLLATE LC_IDENTIFICATION LC_MEASUREMENT LC_MESSAGES"
Defaults env_keep += "LC_MONETARY LC_NAME LC_NUMERIC LC_PAPER LC_TELEPHONE"
Defaults env_keep += "LC_TIME LC_ALL LANGUAGE LINGUAS _XKB_CHARSET XAUTHORITY"
Defaults secure_path = /sbin:/bin:/usr/sbin:/usr/bin
root ALL=(ALL) ALL
%wheel ALL=(ALL) ALL
xyzzy ALL=(ALL) ALL
  xyzzy ALL=(ALL) ALL
 xyzzy ALL=(ALL) NOPASSWD:ALL
xyzzy ALL=(ALL) NOPASSWD:ALL

Maybe do it backwards?

$ grep -ir nopasswd test.sudo | egrep -v "^([[:space:]]*)?(#|$)"
 xyzzy ALL=(ALL) NOPASSWD:ALL
xyzzy ALL=(ALL) NOPASSWD:ALL

...something like that?

[Troy Engel (RAX) (terackspace)]

Possible alternate idea -- the visudo command can create a JSON parsed output of this file, since multiple items that are redefined can end up overwriting each other this seems safer. The user control block that contains a NOPASSWD:ALL looks like this:

        {
            "User_List": [
                { "username": "xyzzy" }
            ],
            "Host_List": [
                { "hostalias": "ALL" }
            ],
            "Cmnd_Specs": [
                {
                    "runasusers": [
                        { "runasalias": "ALL" }
                    ],
                    "Options": [
                        { "authenticate": false },
                        { "setenv": true }
                    ],
                    "Commands": [
                        { "cmndalias": "ALL" }
                    ]
                }
            ]
        }

So I simply do this (since we can't be assured any sort of cli JSON parser like js or jview is installed to do it more JSON-y):

$ visudo -f test.sudo -x /dev/stdout | grep '"authenticate": false'
                        { "authenticate": false },

Given this, it might make more sense to pipe this to 'python -mjson ' (sic) as a fancy one-liner and try to use the pythonic way to find this data with a natural JSON parser. Food for thought!

[Major Hayden (rackerhacker)]

I like the way you think, Troy. ;)

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/481226

[Major Hayden (rackerhacker)]

There's no export to JSON option with the version of sudo on CentOS 7, so I've adapted Troy's regex and pushed a patch.

[Troy Engel (RAX) (terackspace)]

Major the second /etc/sudoers.d/ directory seems to have been dropped from your patch (in the grep), was that intentional? We definitely want to scan everything in that subdir...

[Major Hayden (rackerhacker)]

Good catch. I'll get that fixed up today.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/481226
Committed: https://git.openstack.org/cgit/openstack/ansible-hardening/commit/?id=e112b92c64eddcf699ca452474fc19730e63104e
Submitter: Jenkins
Branch: master

commit e112b92c64eddcf699ca452474fc19730e63104e
Author: Major Hayden <email address hidden>
Date: Mon Jul 10 09:39:59 2017 -0500

    Fix grep for sudoers w/o password

    The grep task that looks for sudoers that have the NOPASSWD option
    had false positives when the line began with a space or comment
    character. This patch fixes the regex to account for those.

    Closes-Bug: 1702182
    Change-Id: Iaf6e388cff1243838acd2edb02d48dda174410be

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/ansible-hardening 16.0.0.0rc2 release candidate.

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/ansible-hardening 17.0.0.0b1 development milestone.