containers / podman-security-bench

Apache License 2.0
39 stars 10 forks source link

Feature/check 4 3 #7

Closed badamowicz closed 2 years ago

badamowicz commented 2 years ago

Hi, all! As announced in this PR, here comes the implementation of check_4_3. Let me explain a few things why I did it this way.

Whitelist vs. Blacklist

In the beginning I thought it was a good idea to also use a whitelist for 4.3, but it wasn't at all. Different distributions will provide different package managers and those in turn will provide different formats of the output of the package list. This rendered it almost impossible to provide a reasonable whitelist. So I changed my mind and also intruduced blacklist which made it pretty easy to achieve the goal.

New directory structure

I removed the former whitelist directory and an originally introduced blacklist directory and instead now only use a single directory named default-lists which contains both blacklists and whitelists. Not a big issue, but makes the handling of the files a bit easier.

New function

I've introduced a new function get_list_cmd() which provides the command for listing packages for their respective distribution type. Currently it looks like this:

get_list_cmd() {
  distro="${1}"

  case "$distro" in
    fedora,rhel)
      echo -n "rpm -qa"
      return
      ;;
    debian|ubuntu)
      echo -n "apt list --installed"
      return
      ;;
    *)
      echo -n "rpm -qa"
      return
      ;;
  esac
}

There is obviously more room left for other distribution types as you can see. So feel free to contribute more types if you thinks that's necessary. I wasn't able to evaluate all available distros in my local environment.

Looking forward to your suggestions! Bernd

badamowicz commented 2 years ago

Encountered a bug in check 5_8 if the whitelist is empty. Don't merge so far. Will provide a fix.

badamowicz commented 2 years ago

Bug is fixed in 6f3dcce. It actually only occured if the lists of ports inside the container was empty.

badamowicz commented 2 years ago

Converted this to a draft because after internal discussions about check 4.3 we think it might be a better solution to use a whitelist instead of a blacklist. So I will have to re-think the implementation.

badamowicz commented 2 years ago

Completely re-implemented 4.3. No more whitelists/blacklists. Instead only allow files.

rhatdan commented 2 years ago

Nice work, can you squash your commits and rewrite the commit message on github.

badamowicz commented 2 years ago

Nice work, can you squash your commits and rewrite the commit message on github.

Thanks! All squashed to a single commit now.