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

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

Merged
merged 6 commits into from
Nov 14, 2022

Conversation

spencer-cdw
Copy link
Contributor

@spencer-cdw spencer-cdw commented Nov 2, 2022

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
Copy link
Member

deric4 commented Nov 3, 2022

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

describe.one do
grub_conf.locations.each do |f|
describe file(f) do
it { should exist }
it { should_not be_readable.by 'group' }
it { should_not be_writable.by 'group' }
it { should_not be_executable.by 'group' }
it { should_not be_readable.by 'other' }
it { should_not be_writable.by 'other' }
it { should_not be_executable.by 'other' }
its(:gid) { should cmp 0 }
its(:uid) { should cmp 0 }
end
end
end

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

%w(/boot/grub/grub.conf /boot/grub/grub.cfg /boot/grub/menu.lst /boot/boot/grub/grub.conf /boot/boot/grub/grub.cfg /boot/boot/grub/menu.lst /boot/grub2/grub.cfg)


@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
Copy link
Contributor

schurzi commented Nov 3, 2022

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
Copy link
Contributor Author

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
Copy link
Contributor Author

spencer-cdw commented Nov 3, 2022

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
Copy link
Contributor Author

spencer-cdw commented Nov 3, 2022

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 #136

Signed-off-by: Spencer Owen <[email protected]>
Signed-off-by: Spencer Owen <[email protected]>
Signed-off-by: Spencer Owen <[email protected]>
@deric4
Copy link
Member

deric4 commented Nov 3, 2022

@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> 

Signed-off-by: Spencer Owen <[email protected]>
@spencer-cdw
Copy link
Contributor Author

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
Copy link
Member

deric4 commented Nov 4, 2022

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
Copy link
Contributor Author

spencer-cdw commented Nov 4, 2022

@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
Copy link
Contributor

schurzi commented Nov 8, 2022

@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
Copy link
Contributor Author

spencer-cdw commented Nov 8, 2022

@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 #136 and #137

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

@schurzi schurzi changed the title Fix gid error Use native inxpec functions to check uid and gid of files Nov 14, 2022
@schurzi schurzi changed the title Use native inxpec functions to check uid and gid of files Use native inspec functions to check uid and gid of files Nov 14, 2022
@schurzi schurzi changed the title Use native inspec functions to check uid and gid of files Use native severspec functions to check uid and gid of files Nov 14, 2022
@schurzi schurzi merged commit 44b9f82 into dev-sec:master Nov 14, 2022
@spencer-cdw spencer-cdw deleted the fix_gid branch November 14, 2022 18:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants