containers / podman-security-bench

Apache License 2.0
39 stars 10 forks source link

Introducing whitelist checks #6

Closed badamowicz closed 2 years ago

badamowicz commented 2 years ago

Hi all! While working with checks mainly from chapters 4 and 5 of the CIS Docker Benchmark, we thought it might be a good idea to introduce a whitelist functionality for several of those checks. From chapters 4 and 5 we think these checks are possible candidates to be implemented with whitelists:

Checks from other chapters may apply as well.

For providing a starting point for further discussions I've implemented check 4.8 using a whitelist.

Implementation

I've introduced a new subdirectory whitelists which contains one file for every whitelist check. Example for check 4.8:

bin/mount
bin/su
bin/umount
etc/gshadow
etc/gshadow-
etc/nginx/conf.d/default.conf
etc/nginx/fastcgi_params
etc/nginx/mime.types
...

At runtime this file will be read with somthing like this

mapfile -t containerFiles < <(podman export "$c" | tar -tv 2>/dev/null | grep -E '^[-rwx].*(s|S).*\s[0-9]' | awk '{print $6}')
for file in "${containerFiles[@]}"; do
  non_wl_found="0"
  if ! grep -q "${file}" "whitelists/wl_${id}" 2>/dev/null; then
    non_wl_found=$file
  fi
done

and further processed. For details see the diff of this PR.

When running this check with a command like this

$  ./podman-security-bench.sh -c check_4_8 -i 03ab34d1e4e4

this will produce JSON output like in this example:

{
  "podmanbenchsecurity": "1.0.0",
  "start": 1654075081,
  "tests": [
        {
          "id": "4.8",
          "desc": "Ensure setuid and setgid permissions are removed (Manual)",
          "result": "WARN",
          "details": "None whitelisted files found:  03ab34d1e4e4/usr/bin/wall",
          "items": [
            "03ab34d1e4e4/usr/bin/wall"
          ]
        }
  ],
  "checks": 1,
  "score": 0,
  "end": 1654075121
}

Open Questions / Remarks

Conclusion

So basically this will allow to automate checks which are now considered to be executed only manually and I would like to start a discussion about it. If you think this is useful, I'd be happy to further contribute implementations for the other checks, at least those for chapters 4 and 5.

Looking forward to hearing your suggestions!

badamowicz commented 2 years ago

Anyone? ;-)

rhatdan commented 2 years ago

LGTM Any reason to keep this as a WIP?

rhatdan commented 2 years ago

This all seems like a good improvement to me. Allowing users to provide their own files to check as well as having a default list.

badamowicz commented 2 years ago

OK. Thanks for your feedback! Will implement now at least check 4.3 and 5.8 and then remove WIP. If someone can provide a list of whitelist candidates from the other chapters, I maybe could implement them as well. But can't promise.

badamowicz commented 2 years ago

Hi @rhatdan ! I've now only implemented checks 4.8 and 5.8. Check 4.3 is a special use case because the different Linux distributions ship with different package managers (apt, rpm, ...). I will have to think about it and file another PR for 4.3.

However, the other checks are ready to use and thoroughly tested on my local test envirnoment. So if there are no more suggestions from your side, I think this PR is ready for being merged.

(And btw: Thanks for providing this project at all! Saved us a lot of time!)

rhatdan commented 2 years ago

Is this checking images for file capabilities also? If so, should newuidmap and newgidmap be in the whitelist?

badamowicz commented 2 years ago

Is this checking images for file capabilities also? If so, should newuidmap and newgidmap be in the whitelist?

You mean check 4.8? Well, we could add newuidmap and newgidmap to the default whitelist. However since users are encouraged to provide their own whitelists (see README.md) we could also leave it up to them.

rhatdan commented 2 years ago

LGTM