search

dev-sec / cis-dil-benchmark

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

Use native severspec functions to check uid and gid of files #134

Closed spencer-cdw closed 1 year ago

spencer-cdw commented 1 year ago

CIS Check 1.4.1 throws the following error

expected: 0 got: (compared using `cmp` matcher)

Screen Shot 2022-11-02 at 11 51 22 AM

There are 10 instances in this repo where a file gid/uid should == 0. In 9 of those instances, the string syntax is used

its('uid') { should cmp 0 }
its('gid') { should cmp 0 }

This is the only instace where the symbol syntax is used

its(:uid) { should cmp 0 }
its(:gid) { should cmp 0 }
inspec --version
5.18.14

Similar comparison such as 1.5.2 uses a string and works as expected

https://github.com/dev-sec/cis-dil-benchmark/blob/master/controls/5_1_configure_cron.rb#L73-L74

Screen Shot 2022-11-02 at 11 50 40 AM

deric4 commented 1 year ago

This is a valid fix but took about about an hour diving into this because I noticed some unexpected behavior with this control that I don't know if its a regression or not.

The symbol syntax should behave the same as the string syntax, as long as the file actually exists. i.e when running against a docker an ubuntu:{focal, jammy} based container, none of the grub_conf.locations exist so I would expect the control to fail because of the describe.one block

https://github.com/dev-sec/cis-dil-benchmark/blob/ab97de3044961a674a775d5a10a842187b18a167/controls/1_4_secure_boot_settings.rb#L30-L44

but what I'm seeing when testing locally is that all of the files are being tested rather than just one of the files (both in container and VM), which I don't think is the desired behavior

https://github.com/dev-sec/cis-dil-benchmark/blob/ab97de3044961a674a775d5a10a842187b18a167/libraries/grubconf.rb#L7

@spencer-cdw can you provide some more detail about your testing environment (OS version, path of actual grub conf file, etc) as well as CLI output?

$ cat /etc/os-release
PRETTY_NAME="Ubuntu 22.04.1 LTS"
NAME="Ubuntu"
VERSION_ID="22.04"
VERSION="22.04.1 LTS (Jammy Jellyfish)"
VERSION_CODENAME=jammy
ID=ubuntu
ID_LIKE=debian
HOME_URL="https://www.ubuntu.com/"
SUPPORT_URL="https://help.ubuntu.com/"
BUG_REPORT_URL="https://bugs.launchpad.net/ubuntu/"
PRIVACY_POLICY_URL="https://www.ubuntu.co

$ cinc-auditor version
5.18.14 

$ cinc-auditor exec https://github.com/dev-sec/cis-dil-benchmark --controls=cis-dil-benchmark-1.4.1

[2022-11-03T01:47:03+00:00] WARN: URL target https://github.com/dev-sec/cis-dil-benchmark transformed to https://github.com/dev-sec/cis-dil-benchmark/archive/master.tar.gz. Consider using the git fetcher
[2022-11-03T01:47:05+00:00] WARN: Cannot find a UUID for your node.

Profile:   CIS Distribution Independent Linux Benchmark Profile (cis-dil-benchmark)
Version:   0.4.13
Target:    local://
Target ID: 

  ×  cis-dil-benchmark-1.4.1: Ensure permissions on bootloader config are configured (21 failed)
     ×  File /boot/grub/grub.conf is expected to exist
     expected File /boot/grub/grub.conf to exist
     ✔  File /boot/grub/grub.conf is expected not to be readable by group
     ✔  File /boot/grub/grub.conf is expected not to be writable by group
     ✔  File /boot/grub/grub.conf is expected not to be executable by group
     ✔  File /boot/grub/grub.conf is expected not to be readable by other
     ✔  File /boot/grub/grub.conf is expected not to be writable by other
     ✔  File /boot/grub/grub.conf is expected not to be executable by other
     ×  File /boot/grub/grub.conf gid is expected to cmp == 0

     expected: 0
          got: 

     (compared using `cmp` matcher)

     ×  File /boot/grub/grub.conf uid is expected to cmp == 0

     expected: 0
          got: 

     (compared using `cmp` matcher)

     ×  File /boot/grub/grub.cfg is expected to exist
     expected File /boot/grub/grub.cfg to exist
     ✔  File /boot/grub/grub.cfg is expected not to be readable by group
     ✔  File /boot/grub/grub.cfg is expected not to be writable by group
     ✔  File /boot/grub/grub.cfg is expected not to be executable by group
     ✔  File /boot/grub/grub.cfg is expected not to be readable by other
     ✔  File /boot/grub/grub.cfg is expected not to be writable by other
     ✔  File /boot/grub/grub.cfg is expected not to be executable by other
     ×  File /boot/grub/grub.cfg gid is expected to cmp == 0

     expected: 0
          got: 

     (compared using `cmp` matcher)

     ×  File /boot/grub/grub.cfg uid is expected to cmp == 0

     expected: 0
          got: 

     (compared using `cmp` matcher)

     ×  File /boot/grub/menu.lst is expected to exist
     expected File /boot/grub/menu.lst to exist
     ✔  File /boot/grub/menu.lst is expected not to be readable by group
     ✔  File /boot/grub/menu.lst is expected not to be writable by group
     ✔  File /boot/grub/menu.lst is expected not to be executable by group
     ✔  File /boot/grub/menu.lst is expected not to be readable by other
     ✔  File /boot/grub/menu.lst is expected not to be writable by other
     ✔  File /boot/grub/menu.lst is expected not to be executable by other
     ×  File /boot/grub/menu.lst gid is expected to cmp == 0

     expected: 0
          got: 

     (compared using `cmp` matcher)

     ×  File /boot/grub/menu.lst uid is expected to cmp == 0

     expected: 0
          got: 

     (compared using `cmp` matcher)

     ×  File /boot/boot/grub/grub.conf is expected to exist
     expected File /boot/boot/grub/grub.conf to exist
     ✔  File /boot/boot/grub/grub.conf is expected not to be readable by group
     ✔  File /boot/boot/grub/grub.conf is expected not to be writable by group
     ✔  File /boot/boot/grub/grub.conf is expected not to be executable by group
     ✔  File /boot/boot/grub/grub.conf is expected not to be readable by other
     ✔  File /boot/boot/grub/grub.conf is expected not to be writable by other
     ✔  File /boot/boot/grub/grub.conf is expected not to be executable by other
     ×  File /boot/boot/grub/grub.conf gid is expected to cmp == 0

     expected: 0
          got: 

     (compared using `cmp` matcher)

     ×  File /boot/boot/grub/grub.conf uid is expected to cmp == 0

     expected: 0
          got: 

     (compared using `cmp` matcher)

     ×  File /boot/boot/grub/grub.cfg is expected to exist
     expected File /boot/boot/grub/grub.cfg to exist
     ✔  File /boot/boot/grub/grub.cfg is expected not to be readable by group
     ✔  File /boot/boot/grub/grub.cfg is expected not to be writable by group
     ✔  File /boot/boot/grub/grub.cfg is expected not to be executable by group
     ✔  File /boot/boot/grub/grub.cfg is expected not to be readable by other
     ✔  File /boot/boot/grub/grub.cfg is expected not to be writable by other
     ✔  File /boot/boot/grub/grub.cfg is expected not to be executable by other
     ×  File /boot/boot/grub/grub.cfg gid is expected to cmp == 0

     expected: 0
          got: 

     (compared using `cmp` matcher)

     ×  File /boot/boot/grub/grub.cfg uid is expected to cmp == 0

     expected: 0
          got: 

     (compared using `cmp` matcher)

     ×  File /boot/boot/grub/menu.lst is expected to exist
     expected File /boot/boot/grub/menu.lst to exist
     ✔  File /boot/boot/grub/menu.lst is expected not to be readable by group
     ✔  File /boot/boot/grub/menu.lst is expected not to be writable by group
     ✔  File /boot/boot/grub/menu.lst is expected not to be executable by group
     ✔  File /boot/boot/grub/menu.lst is expected not to be readable by other
     ✔  File /boot/boot/grub/menu.lst is expected not to be writable by other
     ✔  File /boot/boot/grub/menu.lst is expected not to be executable by other
     ×  File /boot/boot/grub/menu.lst gid is expected to cmp == 0

     expected: 0
          got: 

     (compared using `cmp` matcher)

     ×  File /boot/boot/grub/menu.lst uid is expected to cmp == 0

     expected: 0
          got: 

     (compared using `cmp` matcher)

     ×  File /boot/grub2/grub.cfg is expected to exist
     expected File /boot/grub2/grub.cfg to exist
     ✔  File /boot/grub2/grub.cfg is expected not to be readable by group
     ✔  File /boot/grub2/grub.cfg is expected not to be writable by group
     ✔  File /boot/grub2/grub.cfg is expected not to be executable by group
     ✔  File /boot/grub2/grub.cfg is expected not to be readable by other
     ✔  File /boot/grub2/grub.cfg is expected not to be writable by other
     ✔  File /boot/grub2/grub.cfg is expected not to be executable by other
     ×  File /boot/grub2/grub.cfg gid is expected to cmp == 0

     expected: 0
          got: 

     (compared using `cmp` matcher)

     ×  File /boot/grub2/grub.cfg uid is expected to cmp == 0

     expected: 0
          got: 

     (compared using `cmp` matcher)

Profile Summary: 0 successful controls, 1 control failure, 0 controls skipped
Test Summary: 42 successful, 21 failures, 0 skipped
schurzi commented 1 year ago

I think I would like to use the specialized inspec functions for this, like in our other baselines:

https://github.com/dev-sec/ssh-baseline/blob/30aa061cd6e2e8d8a73f5d2ea7c9655ed148b95d/controls/sshd_spec.rb#L82-L83

it { should be_owned_by sshd_custom_user }
it { should be_grouped_into os.darwin? ? 'wheel' : sshd_custom_user }

see: https://serverspec.org/resource_types.html#file

Would you like to change this on all occasions?

spencer-cdw commented 1 year ago

Would you like to change this on all occasions?

Certainly. I've just pushed an update to the branch that refactors all of them.

spencer-cdw commented 1 year ago

Additional information on my environment.

I have a docker container with packer/inspec/ansible installed

inspec = 5.18.14
packer = 1.8.2
FROM bitnami/node:18.7.0-debian-11-r0
ARG INSPEC_VERSION=5.18.14
ARG PACKER_VERSION=1.8.2

USER root
RUN wget "https://packages.chef.io/files/stable/inspec/${INSPEC_VERSION}/debian/11/inspec_${INSPEC_VERSION}-1_amd64.deb" -O /tmp/inspec.deb && \
    dpkg -i /tmp/inspec.deb && \
    rm -rf /tmp/inspec.deb && \
    apt-get install -y libreadline-dev libreadline8 && \
    inspec --chef-license=accept

RUN wget https://releases.hashicorp.com/packer/${PACKER_VERSION}/packer_${PACKER_VERSION}_linux_amd64.zip -O /tmp/packer.zip && \
    unzip /tmp/packer.zip -d /usr/local/bin && \
    rm -rfv /tmp/packer.zip && \
    packer --version
cat /etc/os-release 
PRETTY_NAME="Debian GNU/Linux 11 (bullseye)"
NAME="Debian GNU/Linux"
VERSION_ID="11"
VERSION="11 (bullseye)"
VERSION_CODENAME=bullseye
ID=debian
HOME_URL="https://www.debian.org/"
SUPPORT_URL="https://www.debian.org/support"
BUG_REPORT_URL="https://bugs.debian.org/"

Packer is building 18.04/20.04/22.04 ubuntu images on aws/gcp/azure

Running cinc-auditor exec is a little tricky and I haven't figured out how to do that yet since these images only exist for a few minutes then are destroyed.

I am seeing the same behavior where the describe.one does not appear to be working for me. (I noticed that behavior a while ago, before I made this change so I don't think it is a regression)

spencer-cdw commented 1 year ago

The new syntax is working:

Screen Shot 2022-11-03 at 11 13 12 AM

I've created a new issue to look more into the describe.one bug https://github.com/dev-sec/cis-dil-benchmark/issues/136

deric4 commented 1 year ago

@spencer-cdw

Running cinc-auditor exec is a little tricky and I haven't figured out how to do that yet since these images only exist for a few minutes then are destroyed.

In the past I've either used the packer build -debug or adding a breakpoint provisioner to debug locally (haven't found an easy way to do this in a CI pipeline unfortunately)

I think I would like to use the specialized inspec functions for this, like in our other baselines:

https://github.com/dev-sec/ssh-baseline/blob/30aa061cd6e2e8d8a73f5d2ea7c9655ed148b95d/controls/sshd_spec.rb#L82-L83

it { should be_owned_by sshd_custom_user } it { should be_grouped_into os.darwin? ? 'wheel' : sshd_custom_user } see: https://serverspec.org/resource_types.html#file

@schurzi is there a special function for determine gid and uid for a file resource? The server spec link seems to be only for the user resource.

Do you know of a way to list all of the supported functions for a specific resource? I've always just kind of just trial and error'd things based of the class methods.

$ cinc-auditor shell
Welcome to the interactive InSpec Shell
To find out how to use it, type: help

You are currently running on:

    Name:      ubuntu
    Families:  debian, linux, unix, os
    Release:   22.04
    Arch:      x86_64

inspec> file('/etc/os-release').class.superclass.instance_methods(false).sort
=> [:allowed?,
 :basename,
 :block_device?,
 :character_device?,
 :contain,
 :content,
 :directory?,
 :executable?,
 :exist?,
 :file,
 :file?,
 :file_version,
 :gid,
 :group,
 :grouped_into?,
 :immutable?,
 :link_path,
 :linked_to?,
 :md5sum,
 :mode,
 :mode?,
 :more_permissive_than?,
 :mount_options,
 :mounted?,
 :mtime,
 :owned_by?,
 :owner,
 :path,
 :pipe?,
 :product_version,
 :readable?,
 :selinux_label,
 :setgid?,
 :setuid?,
 :sgid,
 :sha256sum,
 :shallow_link_path,
 :size,
 :socket?,
 :source,
 :source_path,
 :sticky,
 :sticky?,
 :suid,
 :symlink?,
 :to_s,
 :type,
 :uid,
 :version?,
 :writable?]
inspec>
spencer-cdw commented 1 year ago

At this point. I'm seeing success in the HTML report, but still seeing failures in the JSON report.

Before

Screen Shot 2022-11-03 at 10 03 25 PM Screen Shot 2022-11-03 at 9 01 49 AM

After

Screen Shot 2022-11-03 at 10 07 29 PM

Screen Shot 2022-11-03 at 10 07 46 PM

deric4 commented 1 year ago

I was able to filter the extra tests by adding the following:

  describe.one do
    grub_conf.locations.each do |f|

      next unless file(f).exist?

      describe file(f) do

https://github.com/deric4/cis-dil-benchmark/blob/6c0e40c3f12e8f7503c238fbd7d7dd2627f0b37b/controls/1_4_secure_boot_settings.rb#L33

The only potential problem is that the entire control is considered skipped if none of the files in grub_conf.locations exist, which would be a change in behavior from the current implementation where the control reports as failing.

additional environment setup if grub conf file doesn't exist (i.e. docker container)

FROM ubuntu:jammy as base

RUN : \
  && set -eu \
  && apt-get -q update \
  && DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends \
    ca-certificates \
    curl \
    git \
  && apt-get clean \
  && rm -rf /var/lib/apt/lists/* \
  && :

FROM base as cinc-auditor-bin

ARG CINC_AUDITOR_VER=5.18.14

WORKDIR /workspace

RUN : \
  && set -eu \

  # only here to avoid error message from inspec
  && git init \
  && . /etc/os-release \
  && curl \
       --fail-early \
       --silent \
       --show-error \
       --location \
       --remote-name \
       "http://downloads.cinc.sh/files/stable/cinc-auditor/${CINC_AUDITOR_VER}/${ID}/${VERSION_ID}/cinc-auditor_${CINC_AUDITOR_VER}-1_$(dpkg --print-architecture).deb" \
  && dpkg -i "cinc-auditor_${CINC_AUDITOR_VER}-1_$(dpkg --print-architecture).deb" \
  && rm *.deb \
  # this is only needed for the container
  && mkdir -p /boot/grub \
  && touch /boot/grub/grub.cfg && chmod go-r /boot/grub/grub.cfg \
  && :

build

$ docker build -t ubuntu/cinc-auditor . --target cinc-auditor-bin
...
<build output>
...

run

$ docker run -it ubuntu/cinc-auditor bash -c 'cinc-auditor exec https://github.com/deric4/cis-dil-benchmark.git --controls=cis-dil-benchmark-1.4.1'
[2022-11-04T12:05:57+00:00] WARN: Cannot find a UUID for your node.

Profile:   CIS Distribution Independent Linux Benchmark Profile (cis-dil-benchmark)
Version:   0.4.13
Target:    local://
Target ID: 

  ✔  cis-dil-benchmark-1.4.1: Ensure permissions on bootloader config are configured
     ✔  File /boot/grub/grub.cfg is expected to exist
     ✔  File /boot/grub/grub.cfg is expected not to be readable by group
     ✔  File /boot/grub/grub.cfg is expected not to be writable by group
     ✔  File /boot/grub/grub.cfg is expected not to be executable by group
     ✔  File /boot/grub/grub.cfg is expected not to be readable by other
     ✔  File /boot/grub/grub.cfg is expected not to be writable by other
     ✔  File /boot/grub/grub.cfg is expected not to be executable by other
     ✔  File /boot/grub/grub.cfg gid is expected to cmp == 0
     ✔  File /boot/grub/grub.cfg uid is expected to cmp == 0

Profile Summary: 1 successful control, 0 control failures, 0 controls skipped
Test Summary: 9 successful, 0 failures, 0 skipped
spencer-cdw commented 1 year ago

@deric4 Great reproduction case.

I reproduced your test. I also removed the following line from the docker container and confirmed that 0 tests were executed.&& touch /boot/grub/grub.cfg && chmod go-r /boot/grub/grub.cfg \

describe.one does not do what I think it should be doing. I would assume that this would return a failure. 

docker run -it ubuntu/cinc-auditor bash -c 'cinc-auditor exec https://github.com/deric4/cis-dil-benchmark.git --controls=cis-dil-benchmark-1.4.1'
[2022-11-04T21:27:25+00:00] WARN: Cannot find a UUID for your node.

Profile:   CIS Distribution Independent Linux Benchmark Profile (cis-dil-benchmark)
Version:   0.4.13
Target:    local://
Target ID:

  ✔  cis-dil-benchmark-1.4.1: Ensure permissions on bootloader config are configured
     ✔  File /boot/grub/grub.cfg is expected to exist
     ✔  File /boot/grub/grub.cfg is expected not to be readable by group
     ✔  File /boot/grub/grub.cfg is expected not to be writable by group
     ✔  File /boot/grub/grub.cfg is expected not to be executable by group
     ✔  File /boot/grub/grub.cfg is expected not to be readable by other
     ✔  File /boot/grub/grub.cfg is expected not to be writable by other
     ✔  File /boot/grub/grub.cfg is expected not to be executable by other
     ✔  File /boot/grub/grub.cfg gid is expected to cmp == 0
     ✔  File /boot/grub/grub.cfg uid is expected to cmp == 0

Profile Summary: 1 successful control, 0 control failures, 0 controls skipped
Test Summary: 9 successful, 0 failures, 0 skipped
docker run -it ubuntu/cinc-auditor bash -c 'cinc-auditor exec https://github.com/deric4/cis-dil-benchmark.git --controls=cis-dil-benchmark-1.4.1'
[2022-11-04T21:28:10+00:00] WARN: Cannot find a UUID for your node.

Profile:   CIS Distribution Independent Linux Benchmark Profile (cis-dil-benchmark)
Version:   0.4.13
Target:    local://
Target ID:

     No tests executed.

Test Summary: 0 successful, 0 failures, 0 skipped
schurzi commented 1 year ago

@schurzi is there a special function for determine gid and uid for a file resource? The server spec link seems to be only for the user resource.

I linked the file resource and as far as I am aware, all these methods work with files. I do not know, if there is a direct way to access gid and uid , but I would propose we structure our tests in a way to use be_owned_by and be_grouped_into.

Do you know of a way to list all of the supported functions for a specific resource? I've always just kind of just trial and error'd things based of the class methods.

I always use the documentation site I linked. Your way of listing the instance methods is also valid and very creative. :D

@spencer-cdw I'd like to merge this now as is. We should solve the other problems in separate issues/PRs. Is the current state complete from your point of view?

spencer-cdw commented 1 year ago

@schurzi

Is the current state complete from your point of view?

Yes, Before if a file didn't exist, it would fail with error

  ×  cis-dil-benchmark-1.7.1.4: Ensure permissions on /etc/motd are configured (3 failed)
     ×  File /etc/motd group is expected to eq "root"

     expected: "root"
          got: nil

     (compared using ==)

Now if a file doesn't exist, it fails with error

expected File xxx to be truthy, got false

Screenshot 2022-11-08 at 12 24 36 PM

This is a cleaner failure (in my opinion) and uses the built in be_grouped_into and be_owned_by native resources, instead of the gid comparison.

Note: There was 1 instance where a check was literally looking to see if the root users uid ==0. I omitted that instance from the refactor.

I think this is good to merge as is. We can figure out why some tests aren't being skipped in this related issue https://github.com/dev-sec/cis-dil-benchmark/issues/136 and https://github.com/dev-sec/cis-dil-benchmark/pull/137

Because that behavior/bug is the same both before and after this change, I feel confident that this isn't introducing any regressions.