dev-sec / cis-dil-benchmark

CIS Distribution Independent Linux Benchmark - InSpec Profile
Apache License 2.0
149 stars 92 forks source link

cis-dil-benchmark-5.6 Ubuntu does not have group 'wheel' #138

Closed spencer-cdw closed 1 year ago

spencer-cdw commented 1 year ago

Ubuntu doesn't have a wheel group so test cis-dil-benchmark-5.6 will always fail

https://github.com/dev-sec/cis-dil-benchmark/blob/c845274efcf6e5f2e9307a780995a94c7bee0042/controls/5_4_user_accounts_and_environments.rb#L229-L244

Tested on ubuntu 18.04

docker run -it ubuntu/cinc-auditor bash -c 'cinc-auditor exec https://github.com/deric4/cis-dil-benchmark.git --controls=cis-dil-benchmark-5.6'    

.....

     ×  Groups with name == "wheel" is expected to exist
     expected Groups with name == "wheel" to exist

The equivalent group on ubuntu is sudo https://askubuntu.com/a/1036214

spencer-cdw commented 1 year ago

Verified fixed.

Before

     expected Groups with name == "wheel" to exist

After

     expected Groups with name == "sudo" to exist

Screenshot 2022-11-09 at 1 57 27 PM

schurzi commented 1 year ago

This also seems to be the case for Debian. https://unix.stackexchange.com/questions/4460/why-is-debian-not-creating-the-wheel-group-by-default https://salsa.debian.org/sudo-team/sudo/-/blob/master/etc/sudo.pp#L511-530

Can you check this too and maybe extend the fix to cover all Debian based distros?

schurzi commented 1 year ago

I am reading up on the CIS DIL benchmark. Maybe we should not change the group to sudo, because pam_wheel.so without extra options will always check for a group named wheel. It is not only sudo that is relevant here.

Starting to split hairs: In the benchmark it is not required for a group wheel to exist. The guide advises to verify the users in this group, so the semantics are a bit different.

deric4 commented 1 year ago

I am reading up on the CIS DIL benchmark. Maybe we should not change the group to sudo, because pam_wheel.so without extra options will always check for a group named wheel. It is not only sudo that is relevant here.

I agree with @schurzi on not changing the group to sudo. Two reasons:

  1. the man 8 pam_wheel on ubuntu 22.04 also says the default is wheel or gid:0
    DESCRIPTION
       The pam_wheel PAM module is used to enforce the so-called wheel group. By default it permits access to the target user if the applicant user is a member of the wheel group.
       If no group with this name exist, the module is using the group with the group-ID 0.
  2. Checking the CIS benchmark for ubuntu22.04 specifically (5.3.7) it kinda suggest to not specify a dedicated group for su since it's been "superseded" by sudo

but that still leaves a problem of the control failing on Debian based distros 🤔

spencer-cdw commented 1 year ago

Here is the 5.6 CIS description: https://secscan.acron.pl/centos7/5/6

I see a 2 phase approach

Phase 1: Fix check so it works on ubuntu Phase 2: Refactor/Add new check with additional conditional logic

Phase 1

# sudo code
if os == debian
  check if etc_groups includes 'wheel' or 'sudo' 

Phase 2

# sudo code
if os == debian
   if using pam_wheel module
     if group named wheel exists
       check all sudo users are part of a wheel group
      else
        check all sudo users are part of a group with 0

I'm mostly interested in limiting the scope to just phase 1 right now (so I can get the test passing on ubuntu), then coming back and improving the logic for phase 2 in another PR.

schurzi commented 1 year ago

@spencer-cdw I agree with your approach in general.

Two additions:

  1. Could you delete the group check completely for phase 1? Since this group can not exist without causing problems and is not required by the benchmark, this seems the cleanest approach.
  2. I will open up a follow-up issue that adds a feature for the intended implementation of the control.
spencer-cdw commented 1 year ago

Could you delete the group check completely for phase 1? Since this group can not exist without causing problems and is not required by the benchmark, this seems the cleanest approach.

Yes, pushed a new change. Thank you.