Implement Docker Bench Security tests as Bats test

[alexei-led]

Docker Bench Security scripts are very valuable, but there are some core problems with them.

1. I need a machine parsable test result and not only human readable. Bats can produce TAP result for all running tests. TAP is one of popular test result formats and there are tools, that know how to parse it easily.
2. I would like to see separate test result for every container on the Docker host for container level tests. Today, if there are 50 containers on host and test fails for only one, it's not easy to find which one (and do it automatically)
3. I also would like to be able to run only subset of all available tests on Docker host. Bats allows this use case too.

So, I've forked the docker-bench-security and added Bats framework (very small footprint), couple of helper scripts and converted all tests into Bats format (some tests code was reduced drastically too)

Content:

• bats_tests folder that contains Bats tests and templates
• there are two git submodules for Bats helper libraries under bats_tests\test_helper: bats-assert and bats-support
• run_test.sh - generates Bats tests from templates (for all containers on host) and executes specified (or all tests), it can produce a timestamp(ed) test result file in TAP format (wit -r flag)
• generate_tests.sh - a helper script that automatically creates valid Bats tests from predefined templates for all containers on Docker host
• bats.Dockerfile - Dockerfile that generates Docker image with all above
• I've also added a section into README.md file, that explain how to run Bats tests.

I did this change for our own need. We would like to be able to run Docker Security Bench tests on every machine in our Docker cluster. These tests run automatically when:

• new machine added to cluster
• host OS or Docker daemon updated
• new container is deployed to Docker host
• and once a day (crontab)

We also have a small service that collects all test results from all hosts and send them to another service (test analytics).

[GordonTheTurtle]

Please sign your commits following these rules:
https://github.com/docker/docker/blob/master/CONTRIBUTING.md#sign-your-work
The easiest way to do this is to amend the last commit:

$ git clone -b "bats" git@github.com:gaia-adm/docker-bench-security.git somewhere
$ cd somewhere
$ git rebase -i HEAD~24
editor opens
change each 'pick' to 'edit'
save the file and quit
$ git commit --amend -s --no-edit
$ git rebase --continue # and repeat the amend for each commit
$ git push -f

Ammending updates the existing PR. You DO NOT need to open a new one.

[GordonTheTurtle]

Please sign your commits following these rules:
https://github.com/docker/docker/blob/master/CONTRIBUTING.md#sign-your-work
The easiest way to do this is to amend the last commit:

$ git clone -b "bats" git@github.com:gaia-adm/docker-bench-security.git somewhere
$ cd somewhere
$ git rebase -i HEAD~24
editor opens
change each 'pick' to 'edit'
save the file and quit
$ git commit --amend -s --no-edit
$ git rebase --continue # and repeat the amend for each commit
$ git push -f

Ammending updates the existing PR. You DO NOT need to open a new one.

[konstruktoid]

Hi @alexei-led, and thanks for all this.
I'll do a quick check tonight and continue with testing later on.

[konstruktoid]

avaiable - typo

[diogomonica]

Merge issues.

[konstruktoid]

Merge conflict?

[diogomonica]

@alexei-led I don't think we should be adding this functionality here.

We're currently working on a new version of dockerbench here: https://github.com/diogomonica/actuary . Would love some help pushing it through the finish-line, and this is the exact kind of functionality that we're going towards.

[konstruktoid]

Since we're installing stuff, any way to verify what we've downloaded?

[konstruktoid]

@diogomonica @alexei-led I'm leaving this open for the time being then.

[alexei-led]

@konstruktoid Thank you for you comments. I've committed fixes, based on your review comments. Pls, take a look.

[alexei-led]

@diogomonica Diego, I really like your new project (actuary) - it's definitely going into the right direction: machine readable output and execution of test subset.
Still, there is a place to enhance (at least for me): be able to run container/image level tests as individual tests. If one container of 10s fails some test, I want to know which one and know it automatically: i.e. the same test passes for all other containers and fail only once.
It's also sometime more convenient to get standardized test results: test can either "fail", "error" (for unexpected errors) or "pass". "Info" status makes no sense, since it requires manual result inspection. But this is just my opinion.

[zuBux]

Hi @alexei-led, I'm writing most of Actuary's code at the moment. What you mention about containers has already been implemented. When you run a check that concerns containers, Actuary will print the container ID so you know which ones fail the test.

Test results are standardized already. We use "Pass", "Fail", "Skip" (if something goes wrong, so the audit can continue) and "Info". "Info" is used exactly for manual inspection. For example test "1.4 Remove all non-essential services from the host" prints all of the host's open ports. We can't know beforehand what's essential and what's not, so we put this under "Info".

You 're welcome (and everyone else of course) to test Actuary and report any issues you encounter or features you would like to see implemented :). Contributions are always welcome too!

[alexei-led]

Hi @zuBux, thank you for replay. Actuary looks very promising, I will take a deeper look.

WDYT, about allowing user to configure test parameters, to reduce required manual effort, for example: list of "trusted" users, max containers/host, required cgroups and etc?

[diogomonica]

@alexei-led @zuBux I think we could allow parameters as part of the config. It should be used sparingly though, it makes things more complex.

[zuBux]

@alexei-led @diogomonica Yes, adding parameters to checks makes things a lot more complex. I tried this approach with drydock and my thoughts are:

1. How do we decide which checks allow parameters and which don't?
2. Since Actuary looks for security best practices, how do we know that the check is still considered "best practice" after the user-supplied parameter value?

Unless we find multiple tests which could benefit from this feature, I believe we should avoid it. Instead we should focus more on readable code and reusable functions so users can add their own tests if they want (maybe a custom section?)

[alexei-led]

IMHO, any list or value, you present, to the user for further inspection, should and could be automated.
For example:

• list of "trusted" users, can be known in advance
• "image sprawl" test: why 100? is it some kind of magic number? why not 20? or 200? Let user select
• "exposed devices" check. why not to define a black/white list of devices allowed/forbidden to expose? Is it something you really need to go over for every "info" message?
  Maybe it's just my nature, but I like to automate things and do less manual tedious work.

[zuBux]

These are all good ideas, but I guess we 'll have to see if they truly add value after some time of testing. Also I think we should stop this conversation here, a PR of another project is not the place to do this :)