Skip to content

Conversation

@Arcelone
Copy link

@Arcelone Arcelone commented Sep 3, 2025

Add support for Cis benchmark v1.8.0

close #573

  • Add new test for section 2.7
  • Update Check 5.15
  • Update all the subset (v8 Grid and level_1)
  • Update Readme
  • Move to bash (some scripts weren't call with bash)

@nikjoesta
Copy link

Don't know if anyone still merges this or what the plans are. But I found an issue in the old benchmark script. It is ambiguously defined in the 1.6.0 specification as well. And this script does something completely weird. It is now clear in the 1.8.0 specification.

Long story short:
docker.socket -> docker.sock in 1.1.9 fixes it.

Longer story:
docker.socket with get_service_file finds the systemd path (like /lib/systemd/system/docker.socket) and not the socket (/run/docker.sock) itself cause its name is .sock not .socket. Therefore it doesn't overwrite the path and wants the auditing person to audit the systemd docker.socket file. Which is not what is intended from CIS. No harm in auditing that. But the real goal is to audit the docker.sock.

If you want me to look into it or do a PR (also for 1.6.0) please let me know.

@konstruktoid konstruktoid requested review from Copilot and konstruktoid and removed request for konstruktoid October 13, 2025 22:19
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds support for CIS benchmark v1.8.0, updating the Docker security testing framework. It introduces a new devicemapper storage driver check, updates the container restart policy check to be more comprehensive, and reorganizes test numbering to accommodate the new check.

  • Added new check 2.7 for devicemapper storage driver detection
  • Enhanced check 5.15 to validate both restart policy type and max attempts
  • Updated all test references and documentation to reflect new numbering scheme

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tests/2_docker_daemon_configuration.sh Added new check 2.7 for devicemapper storage driver and renumbered subsequent checks
tests/5_container_runtime.sh Enhanced check 5.15 to validate restart policy name in addition to max attempts
tests/1_host_configuration.sh Fixed description text from "docker.socket" to "docker.sock"
functions/functions_lib.sh Changed shebang to bash, added level1 function implementations, and updated CIS control mappings
docker-bench-security.sh Updated example commands to use bash instead of sh
README.md Updated version table, command examples, and added comprehensive test subset documentation
Dockerfile Updated base image, added bash package, and changed entrypoint to use bash

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

fi

if [ "$restart_policy" -gt "5" ]; then
if [ "$maxAttempts" -gt "5" ] || [ "$restart_policy" != "on-failure" ]; then
Copy link

Copilot AI Oct 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The numeric comparison may fail if $maxAttempts is empty or contains non-numeric values. Consider adding a check to ensure the variable contains a valid number before comparison.

Suggested change
if [ "$maxAttempts" -gt "5" ] || [ "$restart_policy" != "on-failure" ]; then
if { [ -n "$maxAttempts" ] && echo "$maxAttempts" | grep -Eq '^[0-9]+$' && [ "$maxAttempts" -gt "5" ]; } || [ "$restart_policy" != "on-failure" ]; then

Copilot uses AI. Check for mistakes.
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok with -n but not with regex validation since the API always return a number as per documentation.
BUT the doc and exemples are unclear if the value is :

  1. always returned
  2. 0 if the restart policy is different than "on-failure" in which cases the value don't make sense.

@Arcelone
Copy link
Author

@nikjoesta good catch, I didn't notice that. I did change the file name in the rule 1.1.9.
Tell me if it's what you were thinking about.

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

Successfully merging this pull request may close these issues.

Update for newest Benchmark v1.8.0

2 participants