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

Issue for item 3.9 to 3.14 on v1.3.5 #408

Closed
zshrine opened this issue Dec 10, 2019 · 8 comments
Closed

Issue for item 3.9 to 3.14 on v1.3.5 #408

zshrine opened this issue Dec 10, 2019 · 8 comments

Comments

@zshrine
Copy link

zshrine commented Dec 10, 2019

Hi,

I have configured my daemon.json as per below snippet.

 "tlscacert": "/var/lib/docker/certs/ca.pem", 
 "tlscert": "/var/lib/docker/certs/server-cert.pem", 
 "tlskey": "/var/lib/docker/certs/server-key.pem",

When I run a scan on the base host, my log shows the below.

{"id": "3.9", "desc": "Ensure that TLS CA certificate file ownership is set to root:root", "result": "INFO", "details": "No TLS CA certificate found"},
{"id": "3.10", "desc": "Ensure that TLS CA certificate file permissions are set to 444 or more restrictive", "result": "INFO", "details": "No TLS CA certificate found"},
{"id": "3.11", "desc": "Ensure that Docker server certificate file ownership is set to root:root", "result": "INFO", "details": "No TLS Server certificate found"},
{"id": "3.12", "desc": "Ensure that Docker server certificate file permissions are set to 444 or more restrictive", "result": "INFO", "details": "No TLS Server certificate found"},
{"id": "3.13", "desc": "Ensure that Docker server certificate key file ownership is set to root:root", "result": "INFO", "details": "No TLS Key found"},
{"id": "3.14", "desc": "Ensure that Docker server certificate key file permissions are set to 400", "result": "INFO", "details": "No TLS Key found"},

I did an echo of $tlscacert and got the below.

# echo $tlscacert 
tlscacert:/var/lib/docker/certs/ca.pem

In 3_docker_daemon_configuration_files.sh, line 263 if [ -f "$tlscacert" ]; then will definitely fail since it is checking for the full string of tlscacert:/var/lib/docker/certs/ca.pem instead of just /var/lib/docker/certs/ca.pem.

Thanks.

@konstruktoid
Copy link
Collaborator

Thanks for reporting this @zshrine, I'll have a look as soon as possible.

@zshrine
Copy link
Author

zshrine commented Dec 11, 2019

Thank you! I modified the script as per below to only get the json value.
tlscacert=$(get_docker_configuration_file_args 'tlscacert' | cut -d ":" -f 2)

@konstruktoid
Copy link
Collaborator

I'm not getting the same results as you, sorry to say.
Did the following modification:

diff --git a/tests/3_docker_daemon_configuration_files.sh b/tests/3_docker_daemon_configuration_files.sh
index ed9d418..04cacc6 100644
--- a/tests/3_docker_daemon_configuration_files.sh
+++ b/tests/3_docker_daemon_configuration_files.sh
@@ -257,8 +257,10 @@ check_3_9() {
   totalChecks=$((totalChecks + 1))
   if [ -n "$(get_docker_configuration_file_args 'tlscacert')" ]; then
     tlscacert=$(get_docker_configuration_file_args 'tlscacert')
+    echo "*** get_docker_configuration_file_args $tlscacert"
   else
     tlscacert=$(get_docker_effective_command_line_args '--tlscacert' | sed -n 's/.*tlscacert=\([^s]\)/\1/p' | sed 's/--/ --/g' | cut -d " " -f 1)
+    echo "*** command_line_args $tlscacert"
   fi
   if [ -f "$tlscacert" ]; then
     if [ "$(stat -c %u%g "$tlscacert")" -eq 00 ]; then
@@ -289,8 +291,10 @@ check_3_10() {
   totalChecks=$((totalChecks + 1))
   if [ -n "$(get_docker_configuration_file_args 'tlscacert')" ]; then
     tlscacert=$(get_docker_configuration_file_args 'tlscacert')
+    echo "*** get_docker_configuration_file_args $tlscacert"
   else
     tlscacert=$(get_docker_effective_command_line_args '--tlscacert' | sed -n 's/.*tlscacert=\([^s]\)/\1/p' | sed 's/--/ --/g' | cut -d " " -f 1)
+    echo "*** command_line_args $tlscacert"
   fi
   if [ -f "$tlscacert" ]; then
     if [ "$(stat -c %a $tlscacert)" -eq 444 ] || [ "$(stat -c %a $tlscacert)" -eq 400 ]; then

and the result was correct:

[INFO] 3 - Docker daemon configuration files
[PASS] 3.1  - Ensure that docker.service file ownership is set to root:root
[...]
[PASS] 3.8  - Ensure that registry certificate file permissions are set to 444 or more restrictive
*** get_docker_configuration_file_args /etc/docker/certs.d/ca-centos.pem
[PASS] 3.9  - Ensure that TLS CA certificate file ownership is set to root:root
*** get_docker_configuration_file_args /etc/docker/certs.d/ca-centos.pem
[PASS] 3.10  - Ensure that TLS CA certificate file permissions are set to 444 or more restrictive
[...]
$ sudo cat /etc/docker/daemon.json 
{
"icc": false,
"tls": true,
"tlscacert": "/etc/docker/certs.d/ca-centos.pem",
"tlscert": "/etc/docker/certs.d/server-centos-cert.pem",
"tlskey": "/etc/docker/certs.d/server-centos-key.pem",
"tlsverify": true
}

@zshrine
Copy link
Author

zshrine commented Dec 12, 2019

My apologies. The daemon.json i provided was wrong.

The actual daemon.json is as below:

"tlscacert":"/var/lib/docker/certs/ca.pem",

I have tested according to your sample above and it is parsing the value correctly. Seems like the space missing between json key and value is what causes the values to be parsed wrongly in my environment. In my daemon.json, there was no space between the key and value.

konstruktoid added a commit to konstruktoid/docker-bench-security that referenced this issue Dec 12, 2019
Signed-off-by: Thomas Sjögren <konstruktoid@users.noreply.github.com>
@konstruktoid
Copy link
Collaborator

Could you test #409?

@zshrine
Copy link
Author

zshrine commented Dec 13, 2019

I tested. It is working as per expected. Thanks.

# grep tls /etc/docker/daemon.json
"tls": true,
"tlsverify": true,
"tlscacert":"/var/lib/docker/certs/ca.pem",
"tlscert":"/var/lib/docker/certs/server-cert.pem",
"tlskey":"/var/lib/docker/certs/server-key.pem",

The json results for 3.9 to 3.14

{"id": "3.9", "desc": "Ensure that TLS CA certificate file ownership is set to root:root", "result": "PASS"},
{"id": "3.10", "desc": "Ensure that TLS CA certificate file permissions are set to 444 or more restrictive", "result": "PASS"},
{"id": "3.11", "desc": "Ensure that Docker server certificate file ownership is set to root:root", "result": "PASS"},
{"id": "3.12", "desc": "Ensure that Docker server certificate file permissions are set to 444 or more restrictive", "result": "PASS"},
{"id": "3.13", "desc": "Ensure that Docker server certificate key file ownership is set to root:root", "result": "PASS"},
{"id": "3.14", "desc": "Ensure that Docker server certificate key file permissions are set to 400", "result": "PASS"},

konstruktoid added a commit that referenced this issue Dec 13, 2019
@konstruktoid
Copy link
Collaborator

Merged.

@konstruktoid
Copy link
Collaborator

Closing since it seems to work after merge.

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