From 6135c3d0e5a9d0f651c6891c1b77b58a8ba7f614 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Lesimple?= Date: Tue, 18 Jul 2023 17:28:35 +0200 Subject: [PATCH] fix: enhance test 99.1.3 speed for large /etc/sudoers.d folders (#188) Signed-off-by: Stephane Lesimple --- bin/hardening/99.1.3_acc_sudoers_no_all.sh | 79 +++++++------------- tests/hardening/99.1.3_acc_sudoers_no_all.sh | 13 ---- 2 files changed, 26 insertions(+), 66 deletions(-) diff --git a/bin/hardening/99.1.3_acc_sudoers_no_all.sh b/bin/hardening/99.1.3_acc_sudoers_no_all.sh index 0037e5f..6de076c 100755 --- a/bin/hardening/99.1.3_acc_sudoers_no_all.sh +++ b/bin/hardening/99.1.3_acc_sudoers_no_all.sh @@ -23,70 +23,50 @@ DIRECTORY="/etc/sudoers.d" # improves readability in audit report REGEX="ALL = \( ALL( : ALL)? \)( NOPASSWD:)? ALL" EXCEPT="" -MAX_FILES_TO_LOG=0 # This function will be called if the script status is on enabled / audit mode audit() { # expand spaces to [[:space:]]* # shellcheck disable=2001 REGEX="$(echo "$REGEX" | sed 's/ /[[:space:]]*/g')" + matched_files="" - local skiplog - skiplog=0 - if [ $MAX_FILES_TO_LOG != 0 ]; then - # if we have more than $MAX_FILES_TO_LOG files in $DIRECTORY, we'll reduce - # logging in the loop, to avoid flooding the logs and getting timed out - local nbfiles - # shellcheck disable=2012 # (find is too slow and calls fstatat() for each file) - nbfiles=$(ls -f "$DIRECTORY" | wc -l) - if [ "$nbfiles" -gt "$MAX_FILES_TO_LOG" ]; then - skiplog=1 - info "Found $nbfiles files in $DIRECTORY (> $MAX_FILES_TO_LOG), we won't log every file we check" - fi + # check for pattern in $FILE + if $SUDO_CMD grep -Eq "$REGEX" "$FILE"; then + # Will log/warn below, once we've scanned everything, + # because we must also check for patterns that are excused + matched_files="$FILE" + elif [ $? -gt 1 ]; then + # ret > 1 means other grep error we must report + crit "Couldn't grep for pattern in $FILE" + else + # no match, it's ok + : fi - FILES="" - if $SUDO_CMD [ ! -r "$FILE" ]; then - crit "$FILE is not readable" - return - fi - FILES="$FILE" + # check for pattern in whole $DIRECTORY if $SUDO_CMD [ ! -d "$DIRECTORY" ]; then debug "$DIRECTORY does not exist" elif $SUDO_CMD [ ! -x "$DIRECTORY" ]; then crit "Cannot browse $DIRECTORY" else - FILES="$FILES $($SUDO_CMD ls -1 $DIRECTORY | sed s=^=$DIRECTORY/=)" + info "Will check for $($SUDO_CMD ls -f "$DIRECTORY" | wc -l) files within $DIRECTORY" + matched_files="$matched_files $($SUDO_CMD grep -REl "$REGEX" "$DIRECTORY" || true)" fi - for file in $FILES; do - if $SUDO_CMD [ ! -r "$file" ]; then - debug "$file is not readable, but it might just have disappeared since we've listed the folder contents, re-check that it exists" - if $SUDO_CMD [ -e "$file" ]; then - crit "$file is not readable" - else - debug "$file has disappeared, ignore it" + + # now check for pattern exceptions, and crit for each file otherwise + for file in $matched_files; do + RET=$($SUDO_CMD grep -E "$REGEX" "$file" | sed 's/\t/#/g;s/ /#/g') + for line in $RET; do + if grep -q "$(echo "$line" | cut -d '#' -f 1)" <<<"$EXCEPT"; then + # shellcheck disable=2001 + ok "$(echo "$line" | sed 's/#/ /g') is present in $file but was EXCUSED because $(echo "$line" | cut -d '#' -f 1) is part of exceptions." continue fi - else - if ! $SUDO_CMD grep -E "$REGEX" "$file" &>/dev/null; then - if [ $skiplog = 0 ]; then - ok "There is no carte-blanche sudo permission in $file" - fi - else - RET=$($SUDO_CMD grep -E "$REGEX" "$file" | sed 's/\t/#/g;s/ /#/g') - for line in $RET; do - if grep -q "$(echo "$line" | cut -d '#' -f 1)" <<<"$EXCEPT"; then - # shellcheck disable=2001 - ok "$(echo "$line" | sed 's/#/ /g') is present in $file but was EXCUSED because $(echo "$line" | cut -d '#' -f 1) is part of exceptions." - continue - fi - # shellcheck disable=2001 - crit "$(echo "$line" | sed 's/#/ /g') is present in $file" - done - fi - fi + # shellcheck disable=2001 + crit "$(echo "$line" | sed 's/#/ /g') is present in $file" + done done - } # This function will be called if the script status is on enabled mode @@ -101,13 +81,6 @@ status=audit # Put EXCEPTION account names here, space separated EXCEPT="root %root %sudo %wheel" - -# If we find more than this amount of files in sudoers.d/, -# we'll reduce the logging in the loop to avoid getting -# timed out because we spend too much time logging. -# Using 0 disables this feature and will never reduce the -# logging, regardless of the number of files. -MAX_FILES_TO_LOG=0 EOF } # This function will check config parameters required diff --git a/tests/hardening/99.1.3_acc_sudoers_no_all.sh b/tests/hardening/99.1.3_acc_sudoers_no_all.sh index 632ce8f..b0f47c2 100644 --- a/tests/hardening/99.1.3_acc_sudoers_no_all.sh +++ b/tests/hardening/99.1.3_acc_sudoers_no_all.sh @@ -28,19 +28,6 @@ test_audit() { register_test contain "[ OK ] jeantestuser ALL = (ALL) NOPASSWD:ALL is present in /etc/sudoers.d/jeantestuser but was EXCUSED because jeantestuser is part of exceptions" run userexcept /opt/debian-cis/bin/hardening/"${script}".sh --audit-all - # testing the MAX_FILES_TO_LOG config option - echo 'MAX_FILES_TO_LOG=1' >>/opt/debian-cis/etc/conf.d/"${script}".cfg - describe Testing with MAX_FILES_TO_LOG=1 - register_test retvalshouldbe 0 - register_test contain "won't log every file we check" - run maxlogfiles_1 /opt/debian-cis/bin/hardening/"${script}".sh --audit-all - - echo 'MAX_FILES_TO_LOG=9999' >>/opt/debian-cis/etc/conf.d/"${script}".cfg - describe Testing with MAX_FILES_TO_LOG=9999 - register_test retvalshouldbe 0 - register_test contain "There is no carte-blanche sudo permission in" - run maxlogfiles_9999 /opt/debian-cis/bin/hardening/"${script}".sh --audit-all - rm -f /etc/sudoers.d/jeantestuser userdel jeantestuser }