check_2_3 doesnt appear to account for log-level default value

[spedersen-emailage]

CIS Docker Benchmark v1.6.0 recommendation 2.3, "Ensure the logging level is set to 'info' (Manual)," states that log-level should be set to info.

check_2_3() accounts for log-level being explicitly set via command-line options -l and --log-level and checks the contents of several possible config files, but doesn't appear to take into account that the default log-level for Docker is already info.

docker-bench-security/tests/2_docker_daemon_configuration.sh

Lines 46 to 81 in 2311026

	check_2_3() {
	local id="2.3"
	local desc="Ensure the logging level is set to 'info' (Scored)"
	local remediation="Ensure that the Docker daemon configuration file has the following configuration included log-level: info. Alternatively, run the Docker daemon as following: dockerd --log-level=info"
	local remediationImpact="None."
	local check="$id - $desc"
	starttestjson "$id" "$desc"
	if get_docker_configuration_file_args 'log-level' >/dev/null 2>&1; then
	if get_docker_configuration_file_args 'log-level' | grep info >/dev/null 2>&1; then
	pass -s "$check"
	logcheckresult "PASS"
	return
	fi
	if [ -z "$(get_docker_configuration_file_args 'log-level')" ]; then
	pass -s "$check"
	logcheckresult "PASS"
	return
	fi
	warn -s "$check"
	logcheckresult "WARN"
	return
	fi
	if get_docker_effective_command_line_args '-l'; then
	if get_docker_effective_command_line_args '-l' | grep "info" >/dev/null 2>&1; then
	pass -s "$check"
	logcheckresult "PASS"
	return
	fi
	warn -s "$check"
	logcheckresult "WARN"
	return
	fi
	pass -s "$check"
	logcheckresult "PASS"
	}

$ dockerd --help|grep -i log
      --log-driver string                       Default driver for container logs (default "json-file")
      --log-format string                       Set the logging format ("text"|"json") (default "text")
  -l, --log-level string                        Set the logging level ("debug"|"info"|"warn"|"error"|"fatal") (default "info")
      --log-opt map                             Default log driver options for containers (default map[])
      --raw-logs                                Full timestamps without ANSI coloring

Reference: https://docs.docker.com/reference/cli/dockerd/

Since this is a default value, it's not set explicitly and doesn't appear in any config files. In order for this check to be more accurate, should it assume that if no alternate value for log-level is found, the default is used, and the check has passed?

Of course, a work-around to pass the check is to explicitly set the log-level to info.

Thoughts?

[konstruktoid]

yeah, you're absolutley correct. if log-level isn't set, it should pass as well

[konstruktoid]

are you seen an situation where it doesn't work?

docker-bench-security/tests/2_docker_daemon_configuration.sh

Line 60 in 2311026

if [ -z "$(get_docker_configuration_file_args 'log-level')" ]; then

should pass if no option is set.

[spedersen-emailage]

@konstruktoid Yeah, I have a test instance where it fails.

I found the problem ... the result of get_docker_configuration_file_args 'log-level' isn't null so doesn't pass a check with -z ... it actually contains the string "null" instead.

It's the output from jq:

# jq --monochrome-output --raw-output ".[\"log-level\"]" "/etc/docker/daemon.json"
null

docker-bench-security/functions/helper_lib.sh

Line 126 in 2311026

jq --monochrome-output --raw-output ".[\"${OPTION}\"]" "$CONFIG_FILE"

:(

Hacked up the check_2_3 function to confirm:

check_2_3() {
  local id="2.3"
  local desc="Ensure the logging level is set to 'info' (Scored)"
  local remediation="Ensure that the Docker daemon configuration file has the following configuration included log-level: info. Alternatively, run the Docker daemon as following: dockerd --log-level=info"
  local remediationImpact="None."
  local check="$id - $desc"
  starttestjson "$id" "$desc"

  real_null=""

  echo "test0: $(get_docker_configuration_file_args 'userland-proxy')"
  echo "test1: $(get_docker_configuration_file_args 'log-level')"
  echo "test1a: $(get_docker_configuration_file_args 'log-level'|wc -c)"
  echo "test1b: $(get_docker_configuration_file_args 'log-level'|wc -m)"
  [ -z $(get_docker_configuration_file_args 'log-info') ] && echo "test1c: null" || echo "test1c: not null"
  echo "test1d: $real_null"
  [ -z $real_null ] && echo "test1e: this is a real null" || echo "test1e: this is not a real null"

  if get_docker_configuration_file_args 'log-level' >/dev/null 2>&1; then
 
    echo "test2: first if"

    if get_docker_configuration_file_args 'log-level' | grep info >/dev/null 2>&1; then
      
      echo "test3: second if"

      pass -s "$check"
      logcheckresult "PASS"
      return
    fi

    if [ -z "$(get_docker_configuration_file_args 'log-level')" ]; then
      
      echo "test4: third if"
  
      pass -s "$check"
      logcheckresult "PASS"
      return
    fi

    echo "test5: failed here"

    warn -s "$check"
    logcheckresult "WARN"
    return
  fi
  if get_docker_effective_command_line_args '-l'; then
    if get_docker_effective_command_line_args '-l' | grep "info" >/dev/null 2>&1; then
      pass -s "$check"
      logcheckresult "PASS"
      return
    fi
    warn -s "$check"
    logcheckresult "WARN"
    return
  fi
  pass -s "$check"
  logcheckresult "PASS"
}

Results:

# ./docker-bench-security.sh -c check_2_3
# --------------------------------------------------------------------------------------------
# Docker Bench for Security v1.6.0
#
# Docker, Inc. (c) 2015-2024
#
# Checks for dozens of common best-practices around deploying Docker containers in production.
# Based on the CIS Docker Benchmark 1.6.0.
# --------------------------------------------------------------------------------------------

Initializing 2024-05-08T15:05:15+00:00

Section A - Check results
test0: false
test1: null
test1a: 5
test1b: 5
test1c: not null
test1d: 
test1e: this is a real null
test2: first if
test5: failed here
[WARN] 2.3 - Ensure the logging level is set to 'info' (Scored)

Section C - Score

[INFO] Checks: 1
[INFO] Score: -1

[konstruktoid]

nice catch, want to send a PR?

[spedersen-emailage]

Could add // empty to the end of the JQ query, which would return nothing if the value of the object is false or if the object doesn't exist. That would be OK for this specific use case, but get_docker_configuration_file_args is used elsewhere and I see a number of true/false checks, which would break.

Example:

# cat /etc/docker/daemon.json 
{
  "no-new-privileges": true,
  "userland-proxy": false
}
# jq --monochrome-output --raw-output ".[\"log-level\"] // empty" "/etc/docker/daemon.json"
# jq --monochrome-output --raw-output ".[\"userland-proxy\"] // empty" "/etc/docker/daemon.json"
# jq --monochrome-output --raw-output ".[\"no-new-privileges\"] // empty" "/etc/docker/daemon.json"
true
# 

• log-level doesn't exist, so no value is returned.
• userland-proxy is false so no value is returned.
• no-new-privileges is true so true is returned.

That would be the simplest solution, but would require a refactor of all other tests that use the function.

[spedersen-emailage]

Found a solution that appears to work. I can submit a PR, but I'm not able to do any sort of regression testing to see if the change impacts any other tests.

jq --monochrome-output --raw-output "if has(\"${OPTION}\") then .[\"${OPTION}\"] else \"\" end" "$CONFIG_FILE"

It checks if the specified option exists and outputs the value of that option if it does, or an empty string if it doesn't. An empty string is the same as null in bash, so it works.

[konstruktoid]

Found a solution that appears to work. I can submit a PR, but I'm not able to do any sort of regression testing to see if the change impacts any other tests.

jq --monochrome-output --raw-output "if has(\"${OPTION}\") then .[\"${OPTION}\"] else \"\" end" "$CONFIG_FILE"

It checks if the specified option exists and outputs the value of that option if it does, or an empty string if it doesn't. An empty string is the same as null in bash, so it works.

do some basic testing and then submit a PR, and we'll go from there

[spedersen-emailage]

Haven't been able to get back to this, but should be able to soon.

[spedersen-emailage]

#553

[konstruktoid]

Thanks @spedersen-emailage, PR merged

[spedersen-emailage]

Pulled the latest version of master and tested; looks good! Thanks!