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

Audits check CLI arguments, not actual settings #2

Closed
rnelson0 opened this issue May 28, 2015 · 5 comments
Closed

Audits check CLI arguments, not actual settings #2

rnelson0 opened this issue May 28, 2015 · 5 comments

Comments

@rnelson0
Copy link

Lots of the tests (particularly in 2) look at CLI args to commands, like docker, i.e. https://github.com/diogomonica/docker-bench-security/blob/master/tests/2_docker_daemon_configuration.sh#L72 looks for -H via ps -ef | grep docker | grep "\-H".

It appears you could easily fake this out with liberal toying with the docker arguments. Reviewing them at https://docs.docker.com/reference/commandline/cli/, the pidfile stands out as an arbitrary string that could be abused. You could pass tests by creating the following pidfile: /var/tmp/lxcicc\=falselog-level\=\\\"debug\\\"iptables\=falseinsecure-registryregistry-mirror\\-Hdefault-ulimit (you cannot insert / even when escaped, so the tcp:// check would have to be faked with another argument.

Also, you're not looking at positions in the ps output. I believe you could exploit the lack of positional testing with something simple like a grep command that isn't given a 3rd argument:

$ grep "docker lxcicc\=falselog-level\=\\\"debug\\\"iptables\=falseinsecure-registryregistry-mirror\\-Htcp\:\/\/default-ulimittcp://"

# ps -ef | grep docker | grep "\-H"
user   32093 28062  0 21:29 pts/0    00:00:00 grep docker lxcicc\=falselog-level\=\"debug\"iptables\=falseinsecure-registryregistry-mirror\-Htcp\:\/\/default-ulimittcp://

I think if there are ways to test for the present of a listening network socket, for instance, rather than looking for tcp://, that would more accurately and consistently determine the state.

@rnelson0
Copy link
Author

Also, /bin/ps of ps may help avoid the possibility of PATH exploits. If that affects POSIX compliance (as convoluted as that may be) then detecting the correct location such as /bin/ps or /usr/bin/ps, etc., and storing that in a variable used where ps is used currently would provide similar protection.

@konstruktoid
Copy link
Collaborator

@rnelson0, see #48 regarding #2 (comment).

@rnelson0
Copy link
Author

@konstruktoid Some huge changes here. Looks like much has been improved, thanks to you and others! I think there are still some minor gaps, take a gander and see if I'm on track.

It looks like #14 helps with the reduction of PATH exploits (I think it's reasonable to assume that if someone injects a new binary in those directories, you cannot protect against it without a higher level framework).

In #48, get_command_line_args looks like a slight improvement but still misses positional checks. First, the -x option should be used to ensure it matches a command called docker, not one that includes docker. The grep patterns could be surrounded with better markers, perhaps ^<pattern>$ or ^--<pattern>$ as needed. I commented on these in the PR.

For options leading off with hyphens, that should prevent an issue. It's still possible that these arguments could be following a --pidfile argument which would now be on the previous line.I don't know each argument in depth to determine how this could be best addressed.

It looks like the search for tcp:// was removed, but is it possible to look at sockets rather than/in addition to cli args for that?

@konstruktoid
Copy link
Collaborator

Sorry for the late reply @rnelson0, and thanks for joining the discussion on #48.
I merged #48, so can we close this issue and possibly open a new if you want further improvements of the searches?

@rnelson0
Copy link
Author

rnelson0 commented Jul 1, 2015

Sounds good, thanks.

@rnelson0 rnelson0 closed this as completed Jul 1, 2015
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

No branches or pull requests

2 participants