From 83d386795940099e0835c51f3522aae3d9217dc8 Mon Sep 17 00:00:00 2001 From: Aaron Rainbolt Date: Tue, 24 Dec 2024 20:14:57 -0600 Subject: [PATCH 1/4] Refactor permission-hardener to be more idempotent --- debian/security-misc.maintscript | 3 + usr/bin/permission-hardener | 1048 +++++++++-------- usr/bin/permission-hardener-old | 748 ++++++++++++ ....conf => 25_default_whitelist_passwd.conf} | 2 + 4 files changed, 1287 insertions(+), 514 deletions(-) create mode 100755 usr/bin/permission-hardener-old rename usr/lib/permission-hardener.d/{25_default_passwd.conf => 25_default_whitelist_passwd.conf} (91%) diff --git a/debian/security-misc.maintscript b/debian/security-misc.maintscript index adce7ef..5063263 100644 --- a/debian/security-misc.maintscript +++ b/debian/security-misc.maintscript @@ -106,3 +106,6 @@ rm_conffile /etc/default/grub.d/41_quiet.cfg ## moved to usability-misc rm_conffile /etc/dkms/framework.conf.d/30_security-misc.conf + +## renamed to reflect the fact that this uses a whitelist +rm_conffile /usr/lib/permission-hardener.d/25_default_passwd.conf diff --git a/usr/bin/permission-hardener b/usr/bin/permission-hardener index c88b54f..c8d115b 100755 --- a/usr/bin/permission-hardener +++ b/usr/bin/permission-hardener @@ -10,12 +10,6 @@ set -o errexit -o nounset -o pipefail -exit_code=0 -store_dir="/var/lib/permission-hardener" -dpkg_admindir_parameter_existing_mode="--admindir ${store_dir}/existing_mode" -dpkg_admindir_parameter_new_mode="--admindir ${store_dir}/new_mode" -delimiter="#permission-hardener-delimiter#" - # shellcheck disable=SC1091 source /usr/libexec/helper-scripts/safe_echo.sh # shellcheck disable=SC2034 @@ -34,6 +28,7 @@ echo_wrapper_ignore() { } echo_wrapper_audit() { + local return_code if test "${1}" = "verbose"; then shift log notice "Executing: $*" @@ -49,30 +44,23 @@ echo_wrapper_audit() { } } -make_store_dir(){ - mkdir --parents "${store_dir}/private" - mkdir --parents "${store_dir}/existing_mode" - mkdir --parents "${store_dir}/new_mode" -} - ## Some tools may fail on newlines and even variable assignment to array may ## fail if a variable that will be assigned to an array element contains ## characters that are used as delimiters. -block_newlines(){ +block_newlines() { local newline_variable newline_value - newline_variable="${1}" - newline_value="${2}" + newline_variable="${1:-}" + newline_value="${2:-}" ## dpkg-statoverride: error: path may not contain newlines - #if [[ "${newline_value}" == *$'\n'* ]]; then if [[ "${newline_value}" != "${newline_value//$'\n'/NEWLINE}" ]]; then log warn "Skipping ${newline_variable} that contains newlines: '${newline_value}'" >&2 return 1 fi } -output_stat(){ +output_stat() { local file_name - file_name="${1}" + file_name="${1:-}" if test -z "${file_name}"; then log error "File name is empty. file_name: '${file_name}'" >&2 @@ -81,16 +69,10 @@ output_stat(){ block_newlines file "${file_name}" - ## dpkg-statoverride can actually handle '--file-name'. -# if [[ $file_name == --* ]]; then -# log warn "File name starts with '--'. This would be interpreted by dpkg-statoverride as an option. Skipping. file_name: '${file_name}'" >&2 -# return 1 -# fi - declare -a arr local file_name_from_stat stat_output stat_output_newlined - if ! stat_output="$(stat --format="%a${delimiter}%U${delimiter}%G${delimiter}%n${delimiter}" -- "${file_name}")"; then + if ! stat_output="$(stat -L --format="%a${delimiter}%U${delimiter}%G${delimiter}%n${delimiter}" -- "${file_name}")"; then log error "Failed to run 'stat' on file: '${file_name}'!" >&2 return 1 fi @@ -100,7 +82,7 @@ output_stat(){ File name: '${file_name}' Stat output: '${stat_output}' stat_output_newlined: '${stat_output_newlined}' -line: '${line}' +line: '${processed_config_line}' " >&2 return 1 fi @@ -112,7 +94,7 @@ line: '${line}' File name: '${file_name}' Stat output: '${stat_output}' stat_output_newlined: '${stat_output_newlined}' -line: '${line}' +line: '${processed_config_line}' " >&2 return 1 fi @@ -124,7 +106,7 @@ line: '${line}' File name: '${file_name}' Stat output: '${stat_output}' stat_output_newlined: '${stat_output_newlined}' -line: '${line}' +line: '${processed_config_line}' " >&2 return 1 fi @@ -139,456 +121,178 @@ line: '${line}' File name is different from file name received from stat: File name: '${file_name}' File name from stat: '${file_name_from_stat}' -line: '${line}' +line: '${processed_config_line}' " >&2 return 1 fi if test -z "${existing_mode}"; then - log error "Existing mode is empty. Stat output: '${stat_output}', line: '${line}'" >&2 + log error "Existing mode is empty. Stat output: '${stat_output}', line: '${processed_config_line}'" >&2 return 1 fi if test -z "${existing_owner}"; then - log error "Existing owner is empty. Stat output: '${stat_output}', line: '${line}'" >&2 + log error "Existing owner is empty. Stat output: '${stat_output}', line: '${processed_config_line}'" >&2 return 1 fi if test -z "${existing_group}"; then - log error "Existing group is empty. Stat output: '${stat_output}', line: '${line}'" >&2 + log error "Existing group is empty. Stat output: '${stat_output}', line: '${processed_config_line}'" >&2 return 1 fi } -sanity_tests() { - echo_wrapper_audit silent \ - which \ - capsh getcap setcap stat find dpkg-statoverride getent grep 1>/dev/null +print_usage(){ + safe_echo "Usage: ${0##*/} enable + ${0##*/} disable [FILE|all] + +Examples: + ${0##*/} enable + ${0##*/} disable all + ${0##*/} disable /usr/bin/newgrp" >&2 } -add_nosuid_statoverride_entry() { - local fso_to_process - fso_to_process="${fso}" - local should_be_counter - should_be_counter=0 - local counter_actual - counter_actual=0 +## TODO: Validate input before you blindly trust it! +add_to_policy() { + local file_name file_mode file_owner file_group updated_entry policy_idx \ + file_capabilities + file_name="${1:-}" + file_mode="${2:-}" + file_owner="${3:-}" + file_group="${4:-}" + file_capabilities="${5:-}" + updated_entry=false - local dummy_line - while IFS="" read -r -d "" dummy_line; do - log info "Test would parse line: '${dummy_line}'" - should_be_counter=$((should_be_counter + 1)) - done < <(safe_echo_nonewline "${fso_to_process}" | find -files0-from - -perm /u=s,g=s -print0) - ## False positive on SC2185 (find without path argument) #1748 - ## https://github.com/koalaman/shellcheck/issues/1748 - ## - ## /usr/lib will hit ARG_MAX if using bash 'shopt -s globstar' and '/usr/lib/**'. - ## Using 'find' with '-perm /u=s,g=s' is faster and avoids ARG_MAX. - ## https://forums.whonix.org/t/disable-suid-binaries/7706/17 + for (( policy_idx=0; policy_idx < ${#policy_file_list[@]}; policy_idx++ )); do + if [ "${policy_file_list[policy_idx]}" = "${file_name}" ]; then + policy_mode_list[policy_idx]="${file_mode}" + policy_user_owner_list[policy_idx]="${file_owner}" + policy_group_owner_list[policy_idx]="${file_group}" + policy_capability_list[policy_idx]="${file_capabilities}" + updated_entry=true + break + fi + done - local line - while IFS="" read -r -d "" file_name; do - counter_actual=$((counter_actual + 1)) + if [ "${updated_entry}" != 'true' ]; then + policy_file_list+=( "${file_name}" ) + policy_mode_list+=( "${file_mode}" ) + policy_user_owner_list+=( "${file_owner}" ) + policy_group_owner_list+=( "${file_group}" ) + policy_capability_list+=( "${file_capabilities}" ) + fi +} + +check_nosuid_whitelist() { + local target_file match_white_list_entry + + target_file="${1:-}" + + ## Handle whitelists, if we're supposed to + if [ "${whitelists_disable_all}" = 'false' ]; then + ## literal matching is intentional here + # shellcheck disable=SC2076 + if ! [[ " ${policy_disable_white_list[*]} " =~ " ${target_file} " ]]; then + ## literal matching is intentional here too + # shellcheck disable=SC2076 + if [[ " ${policy_exact_white_list[*]} " =~ " ${target_file} " ]]; then + return 1 + fi + + for match_white_list_entry in "${policy_match_white_list[@]:-}"; do + if safe_echo "${target_file}" \ + | grep --quiet --fixed-strings -- "${match_white_list_entry}"; then + return 1 + fi + done + fi + fi + + return 0 +} + +load_early_nosuid_policy() { + local target_file find_list_item + + target_file="${1:-}" + + # shellcheck disable=SC2185 + while IFS="" read -r -d "" find_list_item; do + check_nosuid_whitelist "${find_list_item}" || continue ## sets: ## exiting_mode ## existing_owner ## existing_group - output_stat "${file_name}" + output_stat "${find_list_item}" ## -h file True if file is a symbolic Link. ## -u file True if file has its set-user-id bit set. ## -g file True if file has its set-group-id bit set. - if test -h "${file_name}"; then + if [ -h "${find_list_item}" ]; then ## https://forums.whonix.org/t/disable-suid-binaries/7706/14 - log info "Skip symlink: '${file_name}'" + log info "Skip symlink: '${find_list_item}'" continue fi - if test -d "${file_name}"; then - log info "Skip directory: '${file_name}'" + if [ -d "${find_list_item}" ]; then + log info "Skip directory: '${find_list_item}'" continue fi - local setuid setgid - setuid="" - if test -u "${file_name}"; then - setuid=true - fi - setgid="" - if test -g "${file_name}"; then - setgid=true - fi + ## Trim off the most significant digit of the mode, this discards S(U|G)ID + ## bits (and the sticky bit too but that doesn't matter on Linux) + ## + ## Actually, the old behavior is better here. + local new_mode + # new_mode="${existing_mode:1}" + new_mode='744' - local setuid_or_setgid - setuid_or_setgid="" - if test "${setuid}" = "true" || test "${setgid}" = "true"; then - setuid_or_setgid=true - fi - if test -z "${setuid_or_setgid}"; then - log info "Neither setuid nor setgid. Skipping. file_name: '${file_name}'" - continue - fi - - ## Remove suid / gid and execute permission for 'group' and 'others'. - ## Similar to: chmod og-ugx /path/to/filename - ## Removing execution permission is useful to make binaries such as 'su' - ## fail closed rather than fail open if suid was removed from these. - ## Do not remove read access since no security benefit and easier to - ## manually undo for users. - ## Are there suid or sgid binaries which are still useful if suid / sgid - ## has been removed from these? - new_mode="744" - - local is_exact_whitelisted - is_exact_whitelisted="" - for white_list_entry in "${exact_white_list[@]:-}"; do - if test -z "${white_list_entry}"; then - log info "white_list_entry unset. Skipping. file_name: '${file_name}'" - continue - fi - if test "${file_name}" = "${white_list_entry}"; then - is_exact_whitelisted="true" - log info "is_exact_whitelisted=true. Skipping. file_name: '${file_name}'" - ## Stop looping through the whitelist. - break - fi - done - - local is_match_whitelisted - is_match_whitelisted="" - for matchwhite_list_entry in "${match_white_list[@]:-}"; do - if test -z "${matchwhite_list_entry}"; then - log info "matchwhite_list_entry unset. Skipping. file_name: '${file_name}'" - continue - fi - if safe_echo "${file_name}" | grep --quiet --fixed-strings -- "${matchwhite_list_entry}"; then - is_match_whitelisted="true" - log info "is_match_whitelisted=true. Skipping. file_name: '${file_name}'" - ## Stop looping through the match_white_list. - break - fi - done - - local is_disable_whitelisted - is_disable_whitelisted="" - for disablematch_list_entry in "${disable_white_list[@]:-}"; do - if test -z "${disablematch_list_entry}"; then - log info "disablematch_list_entry unset. Skipping. file_name: '${file_name}'" - continue - fi - if safe_echo "${file_name}" | grep --quiet --fixed-strings -- "${disablematch_list_entry}"; then - is_disable_whitelisted="true" - log info "is_disable_whitelisted=true. Skipping. file_name: '${file_name}'" - ## Stop looping through the disablewhitelist. - break - fi - done - - local clean_output_prefix clean_output - clean_output_prefix="Managing (S|G)UID of line:" - clean_output="${setuid:+setuid='true'} ${setgid:+setgid='true'} existing_mode='${existing_mode}' new_mode='${new_mode}' file='${file_name}'" - if test "${whitelists_disable_all:-}" = "true"; then - log info "${clean_output_prefix} whitelists_disable_all=true ${clean_output}" - elif test "${is_disable_whitelisted}" = "true"; then - log info "${clean_output_prefix} is_disable_whitelisted=true ${clean_output}" - else - if test "${is_exact_whitelisted}" = "true"; then - log info "${clean_output_prefix} is_exact_whitelisted=true ${clean_output}" - continue - fi - if test "${is_match_whitelisted}" = "true"; then - log info "${clean_output_prefix} is_match_whitelisted=true matchwhite_list_entry='${matchwhite_list_entry}' ${clean_output}" - continue - fi - fi - - log notice "${clean_output_prefix} ${clean_output}" - - # shellcheck disable=SC2086 - if dpkg-statoverride ${dpkg_admindir_parameter_existing_mode} --list "${file_name}" >/dev/null; then - log info "Existing mode already saved previously. Not saving again." - else - ## Save existing_mode in separate database. - ## Not using --update as not intending to enforce existing_mode. - # shellcheck disable=SC2086 - echo_wrapper_audit silent dpkg-statoverride ${dpkg_admindir_parameter_existing_mode} --add "${existing_owner}" "${existing_group}" "${existing_mode}" "${file_name}" - fi - - ## No need to check "dpkg-statoverride --list" for existing entries. - ## If existing_mode was correct already, we would not have reached this - ## point. Since existing_mode is incorrect, remove from dpkg-statoverride - ## and re-add. - - ## Remove from real database. - echo_wrapper_ignore silent dpkg-statoverride --remove "${file_name}" - - ## Remove from separate database. - # shellcheck disable=SC2086 - echo_wrapper_ignore silent dpkg-statoverride ${dpkg_admindir_parameter_new_mode} --remove "${file_name}" - - ## Add to real database and use --update to make changes on disk. - echo_wrapper_audit verbose dpkg-statoverride --add --update "${existing_owner}" "${existing_group}" "${new_mode}" "${file_name}" - - ## Not using --update as this is only for recording. - # shellcheck disable=SC2086 - echo_wrapper_audit silent dpkg-statoverride ${dpkg_admindir_parameter_new_mode} --add "${existing_owner}" "${existing_group}" "${new_mode}" "${file_name}" - done < <(safe_echo_nonewline "${fso_to_process}" | find -files0-from - -perm /u=s,g=s -print0) - - ## Sanity test. - if test ! "${should_be_counter}" = "${counter_actual}"; then - log info "File (parsed/wanted): '${fso_to_process}': (${counter_actual}/${should_be_counter})" - log error "Expected number of files to be parsed was not met." >&2 - exit_code=202 - fi + add_to_policy "${find_list_item}" "${new_mode}" "${existing_owner}" \ + "${existing_group}" + done < <(safe_echo_nonewline "${target_file}" | find -files0-from - -perm /u=s,g=s -print0) } -set_file_perms() { - log info "START parsing config file: '${config_file}'" +load_late_nosuid_policy() { + local target_file state_idx state_file_item state_user_owner_item \ + state_group_owner_item - local line - while read -r line || test -n "${line}"; do - if test -z "${line}"; then - true "DEBUG: line is empty. Skipping." - continue - fi + target_file="${1:-}" + for (( state_idx=0; state_idx < ${#state_file_list[@]}; state_idx++ )); do + state_file_item="${state_file_list[state_idx]}" + state_user_owner_item="${state_user_owner_list[state_idx]}" + state_group_owner_item="${state_group_owner_list[state_idx]}" + check_nosuid_whitelist "${state_file_item}" || continue - if [[ "${line}" =~ ^\s*# ]]; then - continue - fi - - if ! [[ "${line}" =~ [0-9a-zA-Z/] ]]; then - exit_code=200 - log error "Line contains invalid characters: '${line}'" >&2 - ## Safer to exit with error in this case. - ## https://forums.whonix.org/t/disable-suid-binaries/7706/59 - exit "${exit_code}" - fi - - if test "${line}" = 'whitelists_disable_all=true'; then - whitelists_disable_all=true - log info "whitelists_disable_all=true" - continue - fi - - #global fso - local mode_from_config owner_from_config group_from_config capability_from_config - if ! read -r fso mode_from_config owner_from_config group_from_config capability_from_config <<<"${line}"; then - exit_code=201 - log error "Cannot parse line: '${line}'" >&2 - ## Debugging. - du -hs /tmp || true - safe_echo "test -w /tmp: '$(test -w /tmp)'" >&2 || true - ## Safer to exit with error in this case. - ## https://forums.whonix.org/t/disable-suid-binaries/7706/59 - exit "${exit_code}" - fi - - log info "Parsing line: fso='${fso}' mode_from_config='${mode_from_config}' owner_from_config='${owner_from_config}' group_from_config='${group_from_config}' capability_from_config='${capability_from_config}'" - - ## Debugging. - #safe_echo "line: '${line}'" - #safe_echo "fso: '${fso}'" - #safe_echo "mode_from_config: '${mode_from_config}'" - #safe_echo "owner_from_config: '${owner_from_config}'" - - local fso_without_trailing_slash - fso_without_trailing_slash="${fso%/}" - - declare -g disable_white_list exact_white_list match_white_list - case "${mode_from_config}" in - disablewhitelist) - disable_white_list+=("${fso}") - continue - ;; - exactwhitelist) - exact_white_list+=("${fso}") - continue - ;; - matchwhitelist) - match_white_list+=("${fso}") - continue - ;; - esac - - if test ! -e "${fso}"; then - log info "File does not exist: '${fso}'" - continue - fi - - ## Use dpkg-statoverride so permissions are not reset during upgrades. - - if test "${mode_from_config}" = "nosuid"; then - ## If mode_from_config is "nosuid" the config does not set owner and - ## group. Therefore do not enforce owner/group check. - add_nosuid_statoverride_entry - else - local string_length_of_mode_from_config - string_length_of_mode_from_config="${#mode_from_config}" - if test "${string_length_of_mode_from_config}" -gt "4"; then - log error "Invalid mode: '${mode_from_config}'" >&2 - continue - fi - if test "${string_length_of_mode_from_config}" -lt "3"; then - log error "Invalid mode: '${mode_from_config}'" >&2 + if [[ ${state_file_item} == ${target_file}* ]]; then + if [ -h "${state_file_item}" ]; then + ## https://forums.whonix.org/t/disable-suid-binaries/7706/14 + log info "Skip symlink: '${state_file_item}'" continue fi - if ! grep --quiet --fixed-strings -- "${owner_from_config}:" "${store_dir}/private/passwd"; then - log error "Owner from config does not exist: '${owner_from_config}'" >&2 + if [ -d "${state_file_item}" ]; then + log info "Skip directory: '${state_file_item}'" continue fi - if ! grep --quiet --fixed-strings -- "${group_from_config}:" "${store_dir}/private/group"; then - log error "Group from config does not exist: '${group_from_config}'" >&2 - continue - fi - - local mode_for_grep - mode_for_grep="${mode_from_config}" - first_character_of_mode_from_config="${mode_from_config::1}" - if test "${first_character_of_mode_from_config}" = "0"; then - ## Remove leading '0'. - mode_for_grep="${mode_from_config:1}" - fi - - file_name="${fso_without_trailing_slash}" - - ## sets: - ## exiting_mode - ## existing_owner - ## existing_group - output_stat "${file_name}" - - ## Check there is an entry for the fso. - ## - ## example: dpkg-statoverride --list | grep /home - ## output: - ## root root 755 /home - ## - ## dpkg-statoverride does not show leading '0'. - local dpkg_statoverride_list_output="" - local dpkg_statoverride_list_exit_code=0 - dpkg_statoverride_list_output="$(dpkg-statoverride --list "${fso_without_trailing_slash}")" || { - dpkg_statoverride_list_exit_code=$? - true - } - - if test "${dpkg_statoverride_list_exit_code}" = "0"; then - local grep_line - grep_line="${owner_from_config} ${group_from_config} ${mode_for_grep} ${fso_without_trailing_slash}" - if safe_echo "${dpkg_statoverride_list_output}" | grep --quiet --fixed-strings -- "${grep_line}"; then - log info "The owner/group/mode matches fso entry. No further action required." - else - log info "The owner/group/mode does not match fso entry, updating entry." - ## fso_without_trailing_slash instead of fso to prevent - ## "dpkg-statoverride: warning: stripping trailing /" - - # shellcheck disable=SC2086 - if dpkg-statoverride ${dpkg_admindir_parameter_existing_mode} --list "${fso_without_trailing_slash}" >/dev/null; then - log info "Existing mode already saved previously. Not saving again." - else - ## Save existing_mode in separate database. - ## Not using --update as not intending to enforce existing_mode. - # shellcheck disable=SC2086 - echo_wrapper_audit silent dpkg-statoverride ${dpkg_admindir_parameter_existing_mode} --add "${existing_owner}" "${existing_group}" "${existing_mode}" "${fso_without_trailing_slash}" - fi - - # shellcheck disable=SC2086 - echo_wrapper_ignore silent dpkg-statoverride ${dpkg_admindir_parameter_new_mode} --remove "${fso_without_trailing_slash}" - - ## Remove from and add to real database. - echo_wrapper_ignore silent dpkg-statoverride --remove "${fso_without_trailing_slash}" - echo_wrapper_audit verbose dpkg-statoverride --add --update "${owner_from_config}" "${group_from_config}" "${mode_from_config}" "${fso_without_trailing_slash}" - - ## Save in separate database. - ## Not using --update as this is only for saving. - # shellcheck disable=SC2086 - echo_wrapper_audit silent dpkg-statoverride ${dpkg_admindir_parameter_new_mode} --add "${owner_from_config}" "${group_from_config}" "${mode_from_config}" "${fso_without_trailing_slash}" - fi - else - log info "There is no fso entry, adding one." - - # shellcheck disable=SC2086 - if dpkg-statoverride ${dpkg_admindir_parameter_existing_mode} --list "${fso_without_trailing_slash}" >/dev/null; then - log info "Existing mode already saved previously. Not saving again." - else - ## Save existing_mode in separate database. - ## Not using --update as not intending to enforce existing_mode. - # shellcheck disable=SC2086 - echo_wrapper_audit silent dpkg-statoverride ${dpkg_admindir_parameter_existing_mode} --add "${existing_owner}" "${existing_group}" "${existing_mode}" "${fso_without_trailing_slash}" - fi - - ## Add to real database. - echo_wrapper_audit verbose dpkg-statoverride --add --update "${owner_from_config}" "${group_from_config}" "${mode_from_config}" "${fso_without_trailing_slash}" - - ## Save in separate database. - ## Not using --update as this is only for saving. - # shellcheck disable=SC2086 - echo_wrapper_audit silent dpkg-statoverride ${dpkg_admindir_parameter_new_mode} --add "${owner_from_config}" "${group_from_config}" "${mode_from_config}" "${fso_without_trailing_slash}" - fi + local new_mode + new_mode='744' + add_to_policy "${state_file_item}" "${new_mode}" \ + "${state_user_owner_item}" "${state_group_owner_item}" fi - if test -z "${capability_from_config}"; then - log info "capability_from_config is empty. Skipping. file_name: '${file_name}'" - continue - fi - - if test "${capability_from_config}" = "none"; then - ## https://forums.whonix.org/t/disable-suid-binaries/7706/45 - ## sudo setcap -r /bin/ping 2>/dev/null - ## Failed to set capabilities on file '/bin/ping' (No data available) - ## The value of the capability argument is not permitted for a file. Or - ## the file is not a regular (non-symlink) file - ## Therefore use echo_wrapper_ignore. - ## - ## NOTE: setcap does not support End-of-Options Marker ('--') yet. - ## setcap bug report: - ## setcap Command Does Not Support End-of-Options Marker ('--') - ## https://bugzilla.kernel.org/show_bug.cgi?id=219487 - echo_wrapper_ignore verbose setcap -r "${fso}" - getcap_output="$(getcap -- "${fso}")" - if test -n "${getcap_output}"; then - exit_code=205 - log error "Removing capabilities failed. File: '${fso}'" >&2 - continue - fi - else - if ! capsh --print | grep --fixed-strings -- "Bounding set" | grep --quiet -- "${capability_from_config}"; then - log error "Capability from config does not exist: '${capability_from_config}'" >&2 - continue - fi - - ## feature request: dpkg-statoverride: support for capabilities - ## https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=502580 - echo_wrapper_audit verbose setcap "${capability_from_config}+ep" -- "${fso}" - fi - - done <"${config_file}" - log info "END parsing config file: '${config_file}'" + done } -parse_config_folder() { - touch "${store_dir}/private/passwd" - chmod og-rwx "${store_dir}/private/passwd" - touch "${store_dir}/private/group" - chmod og-rwx "${store_dir}/private/group" +load_state() { + ## Config format: + ## path options + ## where options is one of: + ## user_owner group_owner filemode [capability-setting] + ## [nosuid|exactwhitelist|matchwhitelist|disablewhitelist] - local passwd_file_contents_temp - ## Query contents of password and group databases only once and buffer them - ## - ## If we don't buffer we sometimes get incorrect results when checking for - ## entries using 'if getent passwd | grep --quiet -- '^root:'; ...' since - ## 'grep' exits after the first match in this case causing 'getent' to - ## receive SIGPIPE, which then fails the pipeline since 'set -o pipefail' is - ## set for this script. - passwd_file_contents_temp="$(getent passwd)" - safe_echo "${passwd_file_contents_temp}" | tee -- "${store_dir}/private/passwd" >/dev/null - group_file_contents_temp="$(getent group)" - safe_echo "${group_file_contents_temp}" | tee -- "${store_dir}/private/group" >/dev/null + local config_file line bit_list file_path policy_nosuid_file_item - #passwd_file_contents="$(cat "${store_dir}/private/passwd")" - #group_file_contents="$(cat "${store_dir}/private/group")" - - shopt -s nullglob + ## Load configuration, deferring whitelist handling until later for config_file in \ /usr/lib/permission-hardener.d/*.conf \ /etc/permission-hardener.d/*.conf \ @@ -596,97 +300,314 @@ parse_config_folder() { /etc/permission-hardening.d/*.conf \ /usr/local/etc/permission-hardening.d/*.conf do - set_file_perms + if [ ! -f "${config_file}" ]; then + continue + fi + while read -r line; do + if [ -z "${line}" ]; then + true 'DEBUG: line is empty. Skipping.' + continue + fi + if [[ "${line}" =~ ^\s*# ]]; then + continue + fi + if ! [[ "${line}" =~ [0-9a-zA-Z/] ]]; then + exit_code=200 + log error "Line contains invalid characters: '${line}'" >&2 + ## Safer to exit with error in this case. + ## https://forums.whonix.org/t/disable-suid-binaries/7706/59 + exit "${exit_code}" + fi + if [ "${line}" = 'whitelists_disable_all=true' ]; then + whitelists_disable_all=true + log info "whitelists_disable_all=true" + continue + fi + + processed_config_line="${line}" + + IFS=' ' read -r -a bit_list <<< "${line}" + + if (( ${#bit_list[@]} < 2 )) \ + || (( ${#bit_list[@]} > 5 )) \ + || (( ${#bit_list[@]} == 3 )); then + exit_code=200 + log error "Line contains an invalid number of fields: '${line}'" >&2 + exit "${exit_code}" + fi + + # Strip trailing slash if appropriate + bit_list[0]="${bit_list[0]%/}" + file_path="${bit_list[0]}" + + case "${bit_list[1]}" in + 'exactwhitelist') + [ ! -e "${file_path}" ] && continue + policy_exact_white_list+=( "${file_path}" ) + continue + ;; + 'matchwhitelist') + policy_match_white_list+=( "${file_path}" ) + continue + ;; + 'disablewhitelist') + policy_disable_white_list+=( "${file_path}" ) + continue + ;; + 'nosuid') + [ ! -e "${file_path}" ] && continue + policy_nosuid_file_list+=( "${file_path}" ) + ;; + *) + [ ! -e "${file_path}" ] && continue + add_to_policy "${bit_list[@]}" + ;; + esac + done < "${config_file}" + done + + ## We have to handle nosuid files at the end since the whitelist arrays need + ## built first. + for policy_nosuid_file_item in "${policy_nosuid_file_list[@]}"; do + load_early_nosuid_policy "${policy_nosuid_file_item}" + done + + local line bit_list policy_file_item + + ## Load the state file from disk + if [ -f "${state_file}" ]; then + while read -r line; do + read -r -a bit_list <<< "${line}" + if (( ${#bit_list[@]} != 4 )); then + log info "Invalid number of fields in state file line: '${line}'. Skipping." + continue + fi + state_user_owner_list+=( "${bit_list[0]}" ) + state_group_owner_list+=( "${bit_list[1]}" ) + state_mode_list+=( "${bit_list[2]}" ) + state_file_list+=( "${bit_list[3]}" ) + done < "${state_file}" + fi + + ## Find any files in the policy that don't already have a matching file in + ## the state. Add those files to the state, and save them to the state file + ## as well. + for policy_file_item in "${policy_file_list[@]}"; do + # shellcheck disable=SC2076 + if [[ " ${state_file_list[*]} " =~ " ${policy_file_item} " ]]; then + continue + fi + output_stat "${policy_file_item}" + state_file_list+=( "${policy_file_item}" ) + state_user_owner_list+=( "${existing_owner}" ) + state_group_owner_list+=( "${existing_group}" ) + state_mode_list+=( "${existing_mode}" ) + # shellcheck disable=SC2086 + echo_wrapper_audit silent dpkg-statoverride \ + ${dpkg_admindir_parameter_existing_mode} \ + --add "${existing_owner}" "${existing_group}" "${existing_mode}" \ + "${policy_file_item}" + done + + for policy_nosuid_file_item in "${policy_nosuid_file_list[@]}"; do + load_late_nosuid_policy "${policy_nosuid_file_item}" done } -apply() { - check_root - make_store_dir - sanity_tests - parse_config_folder +apply_policy() { + local policy_idx did_state_update state_idx - log notice "\ -To compare the current and previous permission modes, install 'meld' (or preferred diff tool) for comparison of file mode changes: - sudo apt install --no-install-recommends meld - meld ${store_dir}/existing_mode/statoverride ${store_dir}/new_mode/statoverride" + ## Modify the in-memory state so that all items that the policy affects match + ## the policy. DO NOT save these changes to the state file! + for (( policy_idx=0; policy_idx < ${#policy_file_list[@]}; policy_idx++ )); do + did_state_update=false + for (( state_idx=0; state_idx < ${#state_file_list[@]}; state_idx++ )); do + if [ "${state_file_list[state_idx]}" = "${policy_file_list[policy_idx]}" ]; then + state_user_owner_list[state_idx]="${policy_user_owner_list[policy_idx]}" + state_group_owner_list[state_idx]="${policy_group_owner_list[policy_idx]}" + state_mode_list[state_idx]="${policy_mode_list[policy_idx]}" + did_state_update=true + break + fi + done + if [ "${did_state_update}" = 'false' ]; then + exit_code=206 + log error "File exists in policy but not in state! File: '${policy_file_list[policy_idx]}'" + exit "${exit_code}" + fi + done } -spare() { - check_root - make_store_dir +commit_policy() { + local policy_idx state_idx state_file_item \ + state_user_owner_item state_group_owner_item \ + state_mode_item orig_main_statoverride_db orig_new_statoverride_db \ + policy_file_item policy_capability_item - remove_file="${1}" - exit_code=0 - dpkg_admindir_parameter_existing_mode="--admindir ${store_dir}/existing_mode" - dpkg_admindir_parameter_new_mode="--admindir ${store_dir}/new_mode" + ## Check each file on the filesystem against the state, and update it if the + ## state does not match. Also ensure the consistency of the new_mode database + ## so that people can compare the original permissions of files with the new + ## permissions. + orig_main_statoverride_db="$(dpkg-statoverride --list)" || true + # shellcheck disable=SC2086 + orig_new_statoverride_db="$(dpkg-statoverride ${dpkg_admindir_parameter_new_mode} --list)" || true - if test ! -f "${store_dir}/existing_mode/statoverride"; then - true "DEBUG: Stat file does not exist, hardening was not applied before." + for (( state_idx=0; state_idx < ${#state_file_list[@]}; state_idx++ )); do + state_file_item="${state_file_list[state_idx]}" + state_user_owner_item="${state_user_owner_list[state_idx]}" + state_group_owner_item="${state_group_owner_list[state_idx]}" + state_mode_item="${state_mode_list[state_idx]}" + + ## Get rid of leading zeros, stat doesn't output them due to how we use it. + ## Using BASH_REMATCH is faster than sed. We capture all leading zeros into + ## one group, and the rest of the string into a second group. The second + ## group is the string we want. BASH_REMATCH[0] is the entire string, + ## BASH_REMATCH[1] is the first match that we want to discard, and + ## BASH_REMATCH[2] is the desired second group. + [[ "${state_mode_item}" =~ ^(0*)(.*) ]] || true; + state_mode_item="${BASH_REMATCH[2]}" + + output_stat "${state_file_item}" + + if [ "${existing_owner}" != "${state_user_owner_item}" ] \ + || [ "${existing_group}" != "${state_group_owner_item}" ] \ + || [ "${existing_mode}" != "${state_mode_item}" ]; then + if ! grep --quiet --fixed-strings -- "${state_user_owner_item}:" "${store_dir}/private/passwd"; then + log error "Owner from config does not exist: '${state_user_owner_item}'" >&2 + continue + fi + + if ! grep --quiet --fixed-strings -- "${state_group_owner_item}:" "${store_dir}/private/group"; then + log error "Group from config does not exist: '${state_group_owner_item}'" >&2 + continue + fi + # Remove and reapply in main list + if grep --quiet --fixed-strings \ + -- "${state_file_item}" <<< "${orig_main_statoverride_db}"; then + echo_wrapper_ignore silent dpkg-statoverride --remove \ + "${state_file_item}" + fi + echo_wrapper_audit verbose dpkg-statoverride --add --update \ + "${state_user_owner_item}" "${state_group_owner_item}" \ + "${state_mode_item}" "${state_file_item}" + + # Update item in secondary list + if grep --quiet --fixed-strings \ + -- "${state_file_item}" <<< "${orig_new_statoverride_db}"; then + # shellcheck disable=SC2086 + echo_wrapper_ignore silent dpkg-statoverride \ + ${dpkg_admindir_parameter_new_mode} --remove \ + "${state_file_item}" + fi + # shellcheck disable=SC2086 + echo_wrapper_audit verbose dpkg-statoverride \ + ${dpkg_admindir_parameter_new_mode} --add \ + "${state_user_owner_item}" "${state_group_owner_item}" \ + "${state_mode_item}" "${state_file_item}" + fi + done + + ## Apply capability hardening, dpkg-statoverride can't handle this so we have + ## to do this manually + for (( policy_idx=0; policy_idx < ${#policy_file_list[@]}; policy_idx++ )); do + policy_file_item="${policy_file_list[policy_idx]}" + policy_capability_item="${policy_capability_list[policy_idx]}" + if [ -z "${policy_capability_item}" ]; then + continue + fi + + if [ "${policy_capability_item}" = 'none' ]; then + echo_wrapper_ignore verbose setcap -r "${policy_file_item}" + if [ -n "$(getcap -- "${policy_file_item}")" ]; then + exit_code=205 + log error \ + "Removing capabilities failed. File: '${policy_file_item}'" >&2 + continue + fi + else + if ! capsh --print \ + | grep --fixed-strings -- "Bounding set" \ + | grep --quiet -- "${policy_capability_item}"; then + log error \ + "Capability from config does not exist: '${policy_capability_item}'" \ + >&2 + continue + fi + + ## feature request: dpkg-statoverride: support for capabilities + ## https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=502580 + echo_wrapper_audit verbose setcap "${policy_capability_item}+ep" \ + -- "${policy_file_item}" + fi + done +} + +undo_policy_for_file() { + local undo_file state_idx state_file_item did_undo \ + undo_all verbose orig_main_statoverride_db orig_new_statoverride_db \ + state_user_owner_item state_group_owner_item state_mode_item + + undo_file="${1}" + undo_all=false + verbose='--verbose' + if [ "${undo_file}" = 'all' ]; then + undo_all=true + verbose='' + fi + + if [ ! -f "${state_file}" ]; then + true 'DEBUG: State file does not exist, hardening was not applied before.' return 0 fi - local line - while read -r line; do - ## example line: - ## root root 4755 /usr/lib/eject/dmcrypt-get-device + did_undo=false - local owner group mode file_name - if ! read -r owner group mode file_name <<< "${line}"; then - exit_code=201 - log error "Cannot parse line: '${line}'" >&2 - continue + for (( state_idx=0; state_idx < ${#state_file_list[@]}; state_idx++ )); do + state_file_item="${state_file_list[state_idx]}" + if [ "${undo_all}" = 'true' ]; then + undo_file="${state_file_item}" fi - log info "Parsing line: owner='${owner}' group='${group}' mode='${mode}' file_name='${file_name}'" - if test "${remove_file}" = "all"; then - verbose="" - remove_one=false - else - if test "${remove_file}" = "${file_name}"; then - verbose="--verbose" - remove_one=true - safe_echo "${remove_one}" | tee -- "${store_dir}/remove_one" >/dev/null - else - safe_echo "false" | tee -- "${store_dir}/remove_one" >/dev/null - continue + if [ "${state_file_item}" = "${undo_file}" ]; then + orig_main_statoverride_db="$(dpkg-statoverride --list)" || true + # shellcheck disable=SC2086 + orig_new_statoverride_db="$(dpkg-statoverride ${dpkg_admindir_parameter_new_mode} --list)" || true + + if grep --quiet --fixed-strings \ + -- "${undo_file}" <<< "${orig_main_statoverride_db}"; then + echo_wrapper_ignore silent dpkg-statoverride --remove \ + "${undo_file}" fi - fi - if test "${remove_one}" = "true"; then - set -o xtrace - fi + if grep --quiet --fixed-strings \ + -- "${undo_file}" <<< "${orig_new_statoverride_db}"; then + # shellcheck disable=SC2086 + echo_wrapper_ignore silent dpkg-statoverride \ + ${dpkg_admindir_parameter_new_mode} --remove \ + "${undo_file}" + fi - if test -e "${file_name}"; then - # shellcheck disable=SC2086 - chown ${verbose} "${owner}:${group}" "${file_name}" || exit_code=202 - ## chmod need to be run after chown since chown removes suid. - ## https://unix.stackexchange.com/questions/53665/chown-removes-setuid-bit-bug-or-feature - # shellcheck disable=SC2086 - chmod ${verbose} "${mode}" "${file_name}" || exit_code=203 - else - log info "File does not exist: '${file_name}'" - fi - - dpkg-statoverride --remove "${file_name}" &>/dev/null || true - # shellcheck disable=SC2086 - dpkg-statoverride ${dpkg_admindir_parameter_existing_mode} --remove "${file_name}" &>/dev/null || true - # shellcheck disable=SC2086 - dpkg-statoverride ${dpkg_admindir_parameter_new_mode} --remove "${file_name}" &>/dev/null || true - - if test "${remove_one}" = "true"; then - set +o xtrace + if [ -e "${undo_file}" ]; then + state_user_owner_item="${state_user_owner_list[state_idx]}" + state_group_owner_item="${state_group_owner_list[state_idx]}" + state_mode_item="${state_mode_list[state_idx]}" + chown ${verbose} "${state_user_owner_item}:${state_group_owner_item}" \ + "${undo_file}" || exit_code=202 + ## chmod need to be run after chown since chown removes suid. + chmod ${verbose} "${state_mode_item}" "${undo_file}" || exit_code=203 + else + log info "File does not exist: '${undo_file}'" + fi + did_undo=true break fi + done - done < "${store_dir}/existing_mode/statoverride" + if ! [[ "${did_undo}" = 'false' ]]; then + log info "The specified file is not hardened, leaving unchanged. - if test ! "${remove_file}" = "all"; then - if test "$(cat "${store_dir}/remove_one")" = "false"; then - log info "No file was removed. - - File '${remove_file}' has not been removed from SUID Disabler and Permission Hardener during this invocation. This is expected if already done earlier. + File '${undo_file}' has not been removed from SUID Disabler and Permission Hardener during this invocation. This is expected if no policy was ever applied to the file before. This program expects the full path to the file. Example: $0 disable /usr/bin/newgrp # absolute path: works @@ -706,39 +627,138 @@ spare() { For completely disabling SUID Disabler and Permission Hardener: https://www.kicksecure.com/wiki/SUID_Disabler_and_Permission_Hardener#Disable_SUID_Disabler_and_Permission_Hardener" - fi fi } -check_root(){ - if test "$(id -u)" != "0"; then - log error "Not running as root, aborting." - exit 1 - fi +print_columns() { + local format_str bogus_str + format_str='' + for bogus_str in "$@"; do + format_str="${format_str}%s\t" + done + format_str="${format_str}\n" + # Using a dynamically generated format string on purpose. + # shellcheck disable=SC2059 + printf "${format_str}" "$@" } -usage(){ - safe_echo "Usage: ${0##*/} enable - ${0##*/} disable [FILE|all] +print_policy() { + local policy_idx -Examples: - ${0##*/} enable - ${0##*/} disable all - ${0##*/} disable /usr/bin/newgrp" >&2 - exit "${1}" + print_columns 'File' 'User' 'Group' 'Mode' 'Capabilities' + + for (( policy_idx=0; policy_idx < ${#policy_file_list[@]}; policy_idx++ )); do + print_columns \ + "${policy_file_list[policy_idx]}" \ + "${policy_user_owner_list[policy_idx]}" \ + "${policy_group_owner_list[policy_idx]}" \ + "${policy_mode_list[policy_idx]}" \ + "${policy_capability_list[policy_idx]}" + done } +print_state() { + local state_idx + + print_columns 'File' 'User' 'Group' 'Mode' + for (( state_idx=0; state_idx < ${#state_file_list[@]}; state_idx++ )); do + print_columns \ + "${state_file_list[state_idx]}" \ + "${state_user_owner_list[state_idx]}" \ + "${state_group_owner_list[state_idx]}" \ + "${state_mode_list[state_idx]}" + done +} + +## Constants +store_dir="/var/lib/permission-hardener" +state_file="${store_dir}/existing_mode/statoverride" +dpkg_admindir_parameter_existing_mode="--admindir ${store_dir}/existing_mode" +dpkg_admindir_parameter_new_mode="--admindir ${store_dir}/new_mode" +delimiter="#permission-hardener-delimiter#" + +## Global variables +policy_file_list=() +policy_user_owner_list=() +policy_group_owner_list=() +policy_mode_list=() +policy_capability_list=() +policy_exact_white_list=() +policy_match_white_list=() +policy_disable_white_list=() +policy_nosuid_file_list=() +state_file_list=() +state_user_owner_list=() +state_group_owner_list=() +state_mode_list=() +whitelists_disable_all=false +existing_mode='' +existing_owner='' +existing_group='' +processed_config_line='' +exit_code=0 + +## Setup and sanity checking +if [ "$(id -u)" != '0' ]; then + log error "Not running as root, aborting." + exit 1 +fi + +mkdir --parents "${store_dir}/private" +mkdir --parents "${store_dir}/existing_mode" +mkdir --parents "${store_dir}/new_mode" +touch "${store_dir}/private/passwd" +chmod og-rwx "${store_dir}/private/passwd" +touch "${store_dir}/private/group" +chmod og-rwx "${store_dir}/private/group" +getent passwd | sponge -- "${store_dir}/private/passwd" +getent group | sponge -- "${store_dir}/private/group" + +echo_wrapper_audit silent which capsh getcap setcap stat find \ + dpkg-statoverride getent grep 1>/dev/null + +## Command parsing and execution case "${1:-}" in - enable) shift; apply "$@";; + enable) + shift + load_state + apply_policy + commit_policy + ;; disable) shift case "${1:-}" in - "") usage 1;; - *) spare "${1}";; + "") + print_usage + exit 1 + ;; + *) + load_state + undo_policy_for_file "${1}" + ;; esac ;; - -h|--help) usage 0;; - *) usage 1;; + print-policy) + load_state + print_policy + ;; + print-state) + load_state + print_state + ;; + print-policy-applied-state) + load_state + apply_policy + print_state + ;; + -h|--help) + print_usage + exit 0 + ;; + *) + print_usage + exit 1 + ;; esac if test "${exit_code}" != "0"; then diff --git a/usr/bin/permission-hardener-old b/usr/bin/permission-hardener-old new file mode 100755 index 0000000..c88b54f --- /dev/null +++ b/usr/bin/permission-hardener-old @@ -0,0 +1,748 @@ +#!/bin/bash + +## Copyright (C) 2012 - 2024 ENCRYPTED SUPPORT LP +## See the file COPYING for copying conditions. + +## https://forums.whonix.org/t/disable-suid-binaries/7706 +## https://forums.whonix.org/t/re-mount-home-and-other-with-noexec-and-nosuid-among-other-useful-mount-options-for-better-security/7707 + +## dpkg-statoverride does not support end-of-options ("--"). + +set -o errexit -o nounset -o pipefail + +exit_code=0 +store_dir="/var/lib/permission-hardener" +dpkg_admindir_parameter_existing_mode="--admindir ${store_dir}/existing_mode" +dpkg_admindir_parameter_new_mode="--admindir ${store_dir}/new_mode" +delimiter="#permission-hardener-delimiter#" + +# shellcheck disable=SC1091 +source /usr/libexec/helper-scripts/safe_echo.sh +# shellcheck disable=SC2034 +log_level=notice +# shellcheck disable=SC1091 +source /usr/libexec/helper-scripts/log_run_die.sh + +echo_wrapper_ignore() { + if test "${1}" = "verbose"; then + shift + log notice "Executing: $*" + else + shift + fi + "$@" 2>/dev/null || true +} + +echo_wrapper_audit() { + if test "${1}" = "verbose"; then + shift + log notice "Executing: $*" + else + shift + fi + return_code=0 + "$@" || + { + return_code="$?" + exit_code=203 + log error "Command '$*' failed with exit code '${return_code}'! calling function name: '${FUNCNAME[1]}'" >&2 + } +} + +make_store_dir(){ + mkdir --parents "${store_dir}/private" + mkdir --parents "${store_dir}/existing_mode" + mkdir --parents "${store_dir}/new_mode" +} + +## Some tools may fail on newlines and even variable assignment to array may +## fail if a variable that will be assigned to an array element contains +## characters that are used as delimiters. +block_newlines(){ + local newline_variable newline_value + newline_variable="${1}" + newline_value="${2}" + ## dpkg-statoverride: error: path may not contain newlines + #if [[ "${newline_value}" == *$'\n'* ]]; then + if [[ "${newline_value}" != "${newline_value//$'\n'/NEWLINE}" ]]; then + log warn "Skipping ${newline_variable} that contains newlines: '${newline_value}'" >&2 + return 1 + fi +} + +output_stat(){ + local file_name + file_name="${1}" + + if test -z "${file_name}"; then + log error "File name is empty. file_name: '${file_name}'" >&2 + return 1 + fi + + block_newlines file "${file_name}" + + ## dpkg-statoverride can actually handle '--file-name'. +# if [[ $file_name == --* ]]; then +# log warn "File name starts with '--'. This would be interpreted by dpkg-statoverride as an option. Skipping. file_name: '${file_name}'" >&2 +# return 1 +# fi + + declare -a arr + local file_name_from_stat stat_output stat_output_newlined + + if ! stat_output="$(stat --format="%a${delimiter}%U${delimiter}%G${delimiter}%n${delimiter}" -- "${file_name}")"; then + log error "Failed to run 'stat' on file: '${file_name}'!" >&2 + return 1 + fi + + if [ "$stat_output" = "" ]; then + log error "stat_output is empty. +File name: '${file_name}' +Stat output: '${stat_output}' +stat_output_newlined: '${stat_output_newlined}' +line: '${line}' +" >&2 + return 1 + fi + + stat_output_newlined="$(printf '%s\n' "${stat_output//${delimiter}/$'\n'}")" + + if test "${stat_output_newlined}" = ""; then + log error "stat_output_newlined is empty. +File name: '${file_name}' +Stat output: '${stat_output}' +stat_output_newlined: '${stat_output_newlined}' +line: '${line}' +" >&2 + return 1 + fi + + readarray -t arr <<< "${stat_output_newlined}" + + if test "${#arr[@]}" = 0; then + log error "Array length is 0. +File name: '${file_name}' +Stat output: '${stat_output}' +stat_output_newlined: '${stat_output_newlined}' +line: '${line}' +" >&2 + return 1 + fi + + existing_mode="${arr[0]}" + existing_owner="${arr[1]}" + existing_group="${arr[2]}" + file_name_from_stat="${arr[3]}" + + if [ ! "$file_name" = "$file_name_from_stat" ]; then + log error "\ +File name is different from file name received from stat: +File name: '${file_name}' +File name from stat: '${file_name_from_stat}' +line: '${line}' +" >&2 + return 1 + fi + + if test -z "${existing_mode}"; then + log error "Existing mode is empty. Stat output: '${stat_output}', line: '${line}'" >&2 + return 1 + fi + if test -z "${existing_owner}"; then + log error "Existing owner is empty. Stat output: '${stat_output}', line: '${line}'" >&2 + return 1 + fi + if test -z "${existing_group}"; then + log error "Existing group is empty. Stat output: '${stat_output}', line: '${line}'" >&2 + return 1 + fi +} + +sanity_tests() { + echo_wrapper_audit silent \ + which \ + capsh getcap setcap stat find dpkg-statoverride getent grep 1>/dev/null +} + +add_nosuid_statoverride_entry() { + local fso_to_process + fso_to_process="${fso}" + local should_be_counter + should_be_counter=0 + local counter_actual + counter_actual=0 + + local dummy_line + while IFS="" read -r -d "" dummy_line; do + log info "Test would parse line: '${dummy_line}'" + should_be_counter=$((should_be_counter + 1)) + done < <(safe_echo_nonewline "${fso_to_process}" | find -files0-from - -perm /u=s,g=s -print0) + ## False positive on SC2185 (find without path argument) #1748 + ## https://github.com/koalaman/shellcheck/issues/1748 + ## + ## /usr/lib will hit ARG_MAX if using bash 'shopt -s globstar' and '/usr/lib/**'. + ## Using 'find' with '-perm /u=s,g=s' is faster and avoids ARG_MAX. + ## https://forums.whonix.org/t/disable-suid-binaries/7706/17 + + local line + while IFS="" read -r -d "" file_name; do + counter_actual=$((counter_actual + 1)) + + ## sets: + ## exiting_mode + ## existing_owner + ## existing_group + output_stat "${file_name}" + + ## -h file True if file is a symbolic Link. + ## -u file True if file has its set-user-id bit set. + ## -g file True if file has its set-group-id bit set. + + if test -h "${file_name}"; then + ## https://forums.whonix.org/t/disable-suid-binaries/7706/14 + log info "Skip symlink: '${file_name}'" + continue + fi + + if test -d "${file_name}"; then + log info "Skip directory: '${file_name}'" + continue + fi + + local setuid setgid + setuid="" + if test -u "${file_name}"; then + setuid=true + fi + setgid="" + if test -g "${file_name}"; then + setgid=true + fi + + local setuid_or_setgid + setuid_or_setgid="" + if test "${setuid}" = "true" || test "${setgid}" = "true"; then + setuid_or_setgid=true + fi + if test -z "${setuid_or_setgid}"; then + log info "Neither setuid nor setgid. Skipping. file_name: '${file_name}'" + continue + fi + + ## Remove suid / gid and execute permission for 'group' and 'others'. + ## Similar to: chmod og-ugx /path/to/filename + ## Removing execution permission is useful to make binaries such as 'su' + ## fail closed rather than fail open if suid was removed from these. + ## Do not remove read access since no security benefit and easier to + ## manually undo for users. + ## Are there suid or sgid binaries which are still useful if suid / sgid + ## has been removed from these? + new_mode="744" + + local is_exact_whitelisted + is_exact_whitelisted="" + for white_list_entry in "${exact_white_list[@]:-}"; do + if test -z "${white_list_entry}"; then + log info "white_list_entry unset. Skipping. file_name: '${file_name}'" + continue + fi + if test "${file_name}" = "${white_list_entry}"; then + is_exact_whitelisted="true" + log info "is_exact_whitelisted=true. Skipping. file_name: '${file_name}'" + ## Stop looping through the whitelist. + break + fi + done + + local is_match_whitelisted + is_match_whitelisted="" + for matchwhite_list_entry in "${match_white_list[@]:-}"; do + if test -z "${matchwhite_list_entry}"; then + log info "matchwhite_list_entry unset. Skipping. file_name: '${file_name}'" + continue + fi + if safe_echo "${file_name}" | grep --quiet --fixed-strings -- "${matchwhite_list_entry}"; then + is_match_whitelisted="true" + log info "is_match_whitelisted=true. Skipping. file_name: '${file_name}'" + ## Stop looping through the match_white_list. + break + fi + done + + local is_disable_whitelisted + is_disable_whitelisted="" + for disablematch_list_entry in "${disable_white_list[@]:-}"; do + if test -z "${disablematch_list_entry}"; then + log info "disablematch_list_entry unset. Skipping. file_name: '${file_name}'" + continue + fi + if safe_echo "${file_name}" | grep --quiet --fixed-strings -- "${disablematch_list_entry}"; then + is_disable_whitelisted="true" + log info "is_disable_whitelisted=true. Skipping. file_name: '${file_name}'" + ## Stop looping through the disablewhitelist. + break + fi + done + + local clean_output_prefix clean_output + clean_output_prefix="Managing (S|G)UID of line:" + clean_output="${setuid:+setuid='true'} ${setgid:+setgid='true'} existing_mode='${existing_mode}' new_mode='${new_mode}' file='${file_name}'" + if test "${whitelists_disable_all:-}" = "true"; then + log info "${clean_output_prefix} whitelists_disable_all=true ${clean_output}" + elif test "${is_disable_whitelisted}" = "true"; then + log info "${clean_output_prefix} is_disable_whitelisted=true ${clean_output}" + else + if test "${is_exact_whitelisted}" = "true"; then + log info "${clean_output_prefix} is_exact_whitelisted=true ${clean_output}" + continue + fi + if test "${is_match_whitelisted}" = "true"; then + log info "${clean_output_prefix} is_match_whitelisted=true matchwhite_list_entry='${matchwhite_list_entry}' ${clean_output}" + continue + fi + fi + + log notice "${clean_output_prefix} ${clean_output}" + + # shellcheck disable=SC2086 + if dpkg-statoverride ${dpkg_admindir_parameter_existing_mode} --list "${file_name}" >/dev/null; then + log info "Existing mode already saved previously. Not saving again." + else + ## Save existing_mode in separate database. + ## Not using --update as not intending to enforce existing_mode. + # shellcheck disable=SC2086 + echo_wrapper_audit silent dpkg-statoverride ${dpkg_admindir_parameter_existing_mode} --add "${existing_owner}" "${existing_group}" "${existing_mode}" "${file_name}" + fi + + ## No need to check "dpkg-statoverride --list" for existing entries. + ## If existing_mode was correct already, we would not have reached this + ## point. Since existing_mode is incorrect, remove from dpkg-statoverride + ## and re-add. + + ## Remove from real database. + echo_wrapper_ignore silent dpkg-statoverride --remove "${file_name}" + + ## Remove from separate database. + # shellcheck disable=SC2086 + echo_wrapper_ignore silent dpkg-statoverride ${dpkg_admindir_parameter_new_mode} --remove "${file_name}" + + ## Add to real database and use --update to make changes on disk. + echo_wrapper_audit verbose dpkg-statoverride --add --update "${existing_owner}" "${existing_group}" "${new_mode}" "${file_name}" + + ## Not using --update as this is only for recording. + # shellcheck disable=SC2086 + echo_wrapper_audit silent dpkg-statoverride ${dpkg_admindir_parameter_new_mode} --add "${existing_owner}" "${existing_group}" "${new_mode}" "${file_name}" + done < <(safe_echo_nonewline "${fso_to_process}" | find -files0-from - -perm /u=s,g=s -print0) + + ## Sanity test. + if test ! "${should_be_counter}" = "${counter_actual}"; then + log info "File (parsed/wanted): '${fso_to_process}': (${counter_actual}/${should_be_counter})" + log error "Expected number of files to be parsed was not met." >&2 + exit_code=202 + fi +} + +set_file_perms() { + log info "START parsing config file: '${config_file}'" + + local line + while read -r line || test -n "${line}"; do + if test -z "${line}"; then + true "DEBUG: line is empty. Skipping." + continue + fi + + if [[ "${line}" =~ ^\s*# ]]; then + continue + fi + + if ! [[ "${line}" =~ [0-9a-zA-Z/] ]]; then + exit_code=200 + log error "Line contains invalid characters: '${line}'" >&2 + ## Safer to exit with error in this case. + ## https://forums.whonix.org/t/disable-suid-binaries/7706/59 + exit "${exit_code}" + fi + + if test "${line}" = 'whitelists_disable_all=true'; then + whitelists_disable_all=true + log info "whitelists_disable_all=true" + continue + fi + + #global fso + local mode_from_config owner_from_config group_from_config capability_from_config + if ! read -r fso mode_from_config owner_from_config group_from_config capability_from_config <<<"${line}"; then + exit_code=201 + log error "Cannot parse line: '${line}'" >&2 + ## Debugging. + du -hs /tmp || true + safe_echo "test -w /tmp: '$(test -w /tmp)'" >&2 || true + ## Safer to exit with error in this case. + ## https://forums.whonix.org/t/disable-suid-binaries/7706/59 + exit "${exit_code}" + fi + + log info "Parsing line: fso='${fso}' mode_from_config='${mode_from_config}' owner_from_config='${owner_from_config}' group_from_config='${group_from_config}' capability_from_config='${capability_from_config}'" + + ## Debugging. + #safe_echo "line: '${line}'" + #safe_echo "fso: '${fso}'" + #safe_echo "mode_from_config: '${mode_from_config}'" + #safe_echo "owner_from_config: '${owner_from_config}'" + + local fso_without_trailing_slash + fso_without_trailing_slash="${fso%/}" + + declare -g disable_white_list exact_white_list match_white_list + case "${mode_from_config}" in + disablewhitelist) + disable_white_list+=("${fso}") + continue + ;; + exactwhitelist) + exact_white_list+=("${fso}") + continue + ;; + matchwhitelist) + match_white_list+=("${fso}") + continue + ;; + esac + + if test ! -e "${fso}"; then + log info "File does not exist: '${fso}'" + continue + fi + + ## Use dpkg-statoverride so permissions are not reset during upgrades. + + if test "${mode_from_config}" = "nosuid"; then + ## If mode_from_config is "nosuid" the config does not set owner and + ## group. Therefore do not enforce owner/group check. + add_nosuid_statoverride_entry + else + local string_length_of_mode_from_config + string_length_of_mode_from_config="${#mode_from_config}" + if test "${string_length_of_mode_from_config}" -gt "4"; then + log error "Invalid mode: '${mode_from_config}'" >&2 + continue + fi + if test "${string_length_of_mode_from_config}" -lt "3"; then + log error "Invalid mode: '${mode_from_config}'" >&2 + continue + fi + + if ! grep --quiet --fixed-strings -- "${owner_from_config}:" "${store_dir}/private/passwd"; then + log error "Owner from config does not exist: '${owner_from_config}'" >&2 + continue + fi + + if ! grep --quiet --fixed-strings -- "${group_from_config}:" "${store_dir}/private/group"; then + log error "Group from config does not exist: '${group_from_config}'" >&2 + continue + fi + + local mode_for_grep + mode_for_grep="${mode_from_config}" + first_character_of_mode_from_config="${mode_from_config::1}" + if test "${first_character_of_mode_from_config}" = "0"; then + ## Remove leading '0'. + mode_for_grep="${mode_from_config:1}" + fi + + file_name="${fso_without_trailing_slash}" + + ## sets: + ## exiting_mode + ## existing_owner + ## existing_group + output_stat "${file_name}" + + ## Check there is an entry for the fso. + ## + ## example: dpkg-statoverride --list | grep /home + ## output: + ## root root 755 /home + ## + ## dpkg-statoverride does not show leading '0'. + local dpkg_statoverride_list_output="" + local dpkg_statoverride_list_exit_code=0 + dpkg_statoverride_list_output="$(dpkg-statoverride --list "${fso_without_trailing_slash}")" || { + dpkg_statoverride_list_exit_code=$? + true + } + + if test "${dpkg_statoverride_list_exit_code}" = "0"; then + local grep_line + grep_line="${owner_from_config} ${group_from_config} ${mode_for_grep} ${fso_without_trailing_slash}" + if safe_echo "${dpkg_statoverride_list_output}" | grep --quiet --fixed-strings -- "${grep_line}"; then + log info "The owner/group/mode matches fso entry. No further action required." + else + log info "The owner/group/mode does not match fso entry, updating entry." + ## fso_without_trailing_slash instead of fso to prevent + ## "dpkg-statoverride: warning: stripping trailing /" + + # shellcheck disable=SC2086 + if dpkg-statoverride ${dpkg_admindir_parameter_existing_mode} --list "${fso_without_trailing_slash}" >/dev/null; then + log info "Existing mode already saved previously. Not saving again." + else + ## Save existing_mode in separate database. + ## Not using --update as not intending to enforce existing_mode. + # shellcheck disable=SC2086 + echo_wrapper_audit silent dpkg-statoverride ${dpkg_admindir_parameter_existing_mode} --add "${existing_owner}" "${existing_group}" "${existing_mode}" "${fso_without_trailing_slash}" + fi + + # shellcheck disable=SC2086 + echo_wrapper_ignore silent dpkg-statoverride ${dpkg_admindir_parameter_new_mode} --remove "${fso_without_trailing_slash}" + + ## Remove from and add to real database. + echo_wrapper_ignore silent dpkg-statoverride --remove "${fso_without_trailing_slash}" + echo_wrapper_audit verbose dpkg-statoverride --add --update "${owner_from_config}" "${group_from_config}" "${mode_from_config}" "${fso_without_trailing_slash}" + + ## Save in separate database. + ## Not using --update as this is only for saving. + # shellcheck disable=SC2086 + echo_wrapper_audit silent dpkg-statoverride ${dpkg_admindir_parameter_new_mode} --add "${owner_from_config}" "${group_from_config}" "${mode_from_config}" "${fso_without_trailing_slash}" + fi + else + log info "There is no fso entry, adding one." + + # shellcheck disable=SC2086 + if dpkg-statoverride ${dpkg_admindir_parameter_existing_mode} --list "${fso_without_trailing_slash}" >/dev/null; then + log info "Existing mode already saved previously. Not saving again." + else + ## Save existing_mode in separate database. + ## Not using --update as not intending to enforce existing_mode. + # shellcheck disable=SC2086 + echo_wrapper_audit silent dpkg-statoverride ${dpkg_admindir_parameter_existing_mode} --add "${existing_owner}" "${existing_group}" "${existing_mode}" "${fso_without_trailing_slash}" + fi + + ## Add to real database. + echo_wrapper_audit verbose dpkg-statoverride --add --update "${owner_from_config}" "${group_from_config}" "${mode_from_config}" "${fso_without_trailing_slash}" + + ## Save in separate database. + ## Not using --update as this is only for saving. + # shellcheck disable=SC2086 + echo_wrapper_audit silent dpkg-statoverride ${dpkg_admindir_parameter_new_mode} --add "${owner_from_config}" "${group_from_config}" "${mode_from_config}" "${fso_without_trailing_slash}" + fi + fi + if test -z "${capability_from_config}"; then + log info "capability_from_config is empty. Skipping. file_name: '${file_name}'" + continue + fi + + if test "${capability_from_config}" = "none"; then + ## https://forums.whonix.org/t/disable-suid-binaries/7706/45 + ## sudo setcap -r /bin/ping 2>/dev/null + ## Failed to set capabilities on file '/bin/ping' (No data available) + ## The value of the capability argument is not permitted for a file. Or + ## the file is not a regular (non-symlink) file + ## Therefore use echo_wrapper_ignore. + ## + ## NOTE: setcap does not support End-of-Options Marker ('--') yet. + ## setcap bug report: + ## setcap Command Does Not Support End-of-Options Marker ('--') + ## https://bugzilla.kernel.org/show_bug.cgi?id=219487 + echo_wrapper_ignore verbose setcap -r "${fso}" + getcap_output="$(getcap -- "${fso}")" + if test -n "${getcap_output}"; then + exit_code=205 + log error "Removing capabilities failed. File: '${fso}'" >&2 + continue + fi + else + if ! capsh --print | grep --fixed-strings -- "Bounding set" | grep --quiet -- "${capability_from_config}"; then + log error "Capability from config does not exist: '${capability_from_config}'" >&2 + continue + fi + + ## feature request: dpkg-statoverride: support for capabilities + ## https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=502580 + echo_wrapper_audit verbose setcap "${capability_from_config}+ep" -- "${fso}" + fi + + done <"${config_file}" + log info "END parsing config file: '${config_file}'" +} + +parse_config_folder() { + touch "${store_dir}/private/passwd" + chmod og-rwx "${store_dir}/private/passwd" + touch "${store_dir}/private/group" + chmod og-rwx "${store_dir}/private/group" + + local passwd_file_contents_temp + ## Query contents of password and group databases only once and buffer them + ## + ## If we don't buffer we sometimes get incorrect results when checking for + ## entries using 'if getent passwd | grep --quiet -- '^root:'; ...' since + ## 'grep' exits after the first match in this case causing 'getent' to + ## receive SIGPIPE, which then fails the pipeline since 'set -o pipefail' is + ## set for this script. + passwd_file_contents_temp="$(getent passwd)" + safe_echo "${passwd_file_contents_temp}" | tee -- "${store_dir}/private/passwd" >/dev/null + group_file_contents_temp="$(getent group)" + safe_echo "${group_file_contents_temp}" | tee -- "${store_dir}/private/group" >/dev/null + + #passwd_file_contents="$(cat "${store_dir}/private/passwd")" + #group_file_contents="$(cat "${store_dir}/private/group")" + + shopt -s nullglob + for config_file in \ + /usr/lib/permission-hardener.d/*.conf \ + /etc/permission-hardener.d/*.conf \ + /usr/local/etc/permission-hardener.d/*.conf \ + /etc/permission-hardening.d/*.conf \ + /usr/local/etc/permission-hardening.d/*.conf + do + set_file_perms + + done +} + +apply() { + check_root + make_store_dir + sanity_tests + parse_config_folder + + log notice "\ +To compare the current and previous permission modes, install 'meld' (or preferred diff tool) for comparison of file mode changes: + sudo apt install --no-install-recommends meld + meld ${store_dir}/existing_mode/statoverride ${store_dir}/new_mode/statoverride" +} + +spare() { + check_root + make_store_dir + + remove_file="${1}" + exit_code=0 + dpkg_admindir_parameter_existing_mode="--admindir ${store_dir}/existing_mode" + dpkg_admindir_parameter_new_mode="--admindir ${store_dir}/new_mode" + + if test ! -f "${store_dir}/existing_mode/statoverride"; then + true "DEBUG: Stat file does not exist, hardening was not applied before." + return 0 + fi + + local line + while read -r line; do + ## example line: + ## root root 4755 /usr/lib/eject/dmcrypt-get-device + + local owner group mode file_name + if ! read -r owner group mode file_name <<< "${line}"; then + exit_code=201 + log error "Cannot parse line: '${line}'" >&2 + continue + fi + log info "Parsing line: owner='${owner}' group='${group}' mode='${mode}' file_name='${file_name}'" + + if test "${remove_file}" = "all"; then + verbose="" + remove_one=false + else + if test "${remove_file}" = "${file_name}"; then + verbose="--verbose" + remove_one=true + safe_echo "${remove_one}" | tee -- "${store_dir}/remove_one" >/dev/null + else + safe_echo "false" | tee -- "${store_dir}/remove_one" >/dev/null + continue + fi + fi + + if test "${remove_one}" = "true"; then + set -o xtrace + fi + + if test -e "${file_name}"; then + # shellcheck disable=SC2086 + chown ${verbose} "${owner}:${group}" "${file_name}" || exit_code=202 + ## chmod need to be run after chown since chown removes suid. + ## https://unix.stackexchange.com/questions/53665/chown-removes-setuid-bit-bug-or-feature + # shellcheck disable=SC2086 + chmod ${verbose} "${mode}" "${file_name}" || exit_code=203 + else + log info "File does not exist: '${file_name}'" + fi + + dpkg-statoverride --remove "${file_name}" &>/dev/null || true + # shellcheck disable=SC2086 + dpkg-statoverride ${dpkg_admindir_parameter_existing_mode} --remove "${file_name}" &>/dev/null || true + # shellcheck disable=SC2086 + dpkg-statoverride ${dpkg_admindir_parameter_new_mode} --remove "${file_name}" &>/dev/null || true + + if test "${remove_one}" = "true"; then + set +o xtrace + break + fi + + done < "${store_dir}/existing_mode/statoverride" + + if test ! "${remove_file}" = "all"; then + if test "$(cat "${store_dir}/remove_one")" = "false"; then + log info "No file was removed. + + File '${remove_file}' has not been removed from SUID Disabler and Permission Hardener during this invocation. This is expected if already done earlier. + + This program expects the full path to the file. Example: + $0 disable /usr/bin/newgrp # absolute path: works + $0 disable newgrp # relative path: does not work + + To remove all: + $0 disable all + + This change might not be permanent. For full instructions, see: + https://www.kicksecure.com/wiki/SUID_Disabler_and_Permission_Hardener + + To view list of changed by SUID Disabler and Permission Hardener: + https://www.kicksecure.com/wiki/SUID_Disabler_and_Permission_Hardener#View_List_of_Permissions_Changed_by_SUID_Disabler_and_Permission_Hardener + + For re-enabling any specific SUID binary: + https://www.kicksecure.com/wiki/SUID_Disabler_and_Permission_Hardener#Re-Enable_Specific_SUID_Binaries + + For completely disabling SUID Disabler and Permission Hardener: + https://www.kicksecure.com/wiki/SUID_Disabler_and_Permission_Hardener#Disable_SUID_Disabler_and_Permission_Hardener" + fi + fi +} + +check_root(){ + if test "$(id -u)" != "0"; then + log error "Not running as root, aborting." + exit 1 + fi +} + +usage(){ + safe_echo "Usage: ${0##*/} enable + ${0##*/} disable [FILE|all] + +Examples: + ${0##*/} enable + ${0##*/} disable all + ${0##*/} disable /usr/bin/newgrp" >&2 + exit "${1}" +} + +case "${1:-}" in + enable) shift; apply "$@";; + disable) + shift + case "${1:-}" in + "") usage 1;; + *) spare "${1}";; + esac + ;; + -h|--help) usage 0;; + *) usage 1;; +esac + +if test "${exit_code}" != "0"; then + log error "Exiting with non-zero exit code: '${exit_code}'" >&2 +fi + +exit "${exit_code}" diff --git a/usr/lib/permission-hardener.d/25_default_passwd.conf b/usr/lib/permission-hardener.d/25_default_whitelist_passwd.conf similarity index 91% rename from usr/lib/permission-hardener.d/25_default_passwd.conf rename to usr/lib/permission-hardener.d/25_default_whitelist_passwd.conf index 6313e15..6c8369c 100644 --- a/usr/lib/permission-hardener.d/25_default_passwd.conf +++ b/usr/lib/permission-hardener.d/25_default_whitelist_passwd.conf @@ -10,5 +10,7 @@ # user logins with `su` and KScreenLocker # # See also: https://www.kicksecure.com/wiki/SUID_Disabler_and_Permission_Hardener#passwd +/usr/bin/passwd exactwhitelist +/bin/passwd exactwhitelist /usr/bin/passwd 0755 root root /bin/passwd 0755 root root From dbcb612517abbf8d162cfb31ba0585c518df8817 Mon Sep 17 00:00:00 2001 From: Aaron Rainbolt Date: Wed, 25 Dec 2024 19:48:28 -0600 Subject: [PATCH 2/4] Polish permission-hardener refactor --- usr/bin/permission-hardener | 302 ++++--- usr/bin/permission-hardener-old | 748 ------------------ .../25_default_whitelist_passwd.conf | 3 +- 3 files changed, 184 insertions(+), 869 deletions(-) delete mode 100755 usr/bin/permission-hardener-old diff --git a/usr/bin/permission-hardener b/usr/bin/permission-hardener index c8d115b..de03c7c 100755 --- a/usr/bin/permission-hardener +++ b/usr/bin/permission-hardener @@ -1,4 +1,5 @@ #!/bin/bash +# shellcheck disable=SC2076 ## Copyright (C) 2012 - 2024 ENCRYPTED SUPPORT LP ## See the file COPYING for copying conditions. @@ -8,32 +9,52 @@ ## dpkg-statoverride does not support end-of-options ("--"). +## SC2076 is disabled because ShellCheck seems to think that any use of +## [[ ... =~ ... ]] is supposed to be a regex match. But [[ '...' =~ '...' ]] +## works very well for literal matching, and it is used that way extensively +## throughout this script. + set -o errexit -o nounset -o pipefail -# shellcheck disable=SC1091 -source /usr/libexec/helper-scripts/safe_echo.sh +## Constants # shellcheck disable=SC2034 log_level=notice +store_dir="/var/lib/permission-hardener" +state_file="${store_dir}/existing_mode/statoverride" +dpkg_admindir_parameter_existing_mode="--admindir ${store_dir}/existing_mode" +dpkg_admindir_parameter_new_mode="--admindir ${store_dir}/new_mode" +delimiter="#permission-hardener-delimiter#" + +## Library imports +# shellcheck disable=SC1091 +source /usr/libexec/helper-scripts/safe_echo.sh # shellcheck disable=SC1091 source /usr/libexec/helper-scripts/log_run_die.sh +## Functions echo_wrapper_ignore() { - if test "${1}" = "verbose"; then + if [ "${1}" = 'verbose' ]; then shift log notice "Executing: $*" - else + elif [ "${1}" = 'silent' ]; then shift + else + log error "Unrecognized command '${1}'! calling function name: '${FUNCNAME[1]}'" >&2 + return fi "$@" 2>/dev/null || true } echo_wrapper_audit() { local return_code - if test "${1}" = "verbose"; then + if [ "${1}" = 'verbose' ]; then shift log notice "Executing: $*" - else + elif [ "${1}" = 'silent' ]; then shift + else + log error "Unrecognized command '${1}'! calling function name: '${FUNCNAME[1]}'" >&2 + return fi return_code=0 "$@" || @@ -59,25 +80,34 @@ block_newlines() { } output_stat() { - local file_name + local file_name stat_output stat_output_newlined + declare -a arr file_name="${1:-}" - if test -z "${file_name}"; then + if [ -z "${file_name}" ]; then log error "File name is empty. file_name: '${file_name}'" >&2 return 1 fi block_newlines file "${file_name}" - declare -a arr - local file_name_from_stat stat_output stat_output_newlined + if [ ! -e "${file_name}" ]; then + log info "File does not exist. file_name: '${file_name}'" >&2 + existing_mode='' + existing_owner='' + existing_group='' + file_name_from_stat='' + return 0 + fi - if ! stat_output="$(stat -L --format="%a${delimiter}%U${delimiter}%G${delimiter}%n${delimiter}" -- "${file_name}")"; then + if ! stat_output="$(stat -L \ + --format="%a${delimiter}%U${delimiter}%G${delimiter}%n${delimiter}" \ + -- "${file_name}")"; then log error "Failed to run 'stat' on file: '${file_name}'!" >&2 return 1 fi - if [ "$stat_output" = "" ]; then + if [ "$stat_output" = '' ]; then log error "stat_output is empty. File name: '${file_name}' Stat output: '${stat_output}' @@ -89,7 +119,7 @@ line: '${processed_config_line}' stat_output_newlined="$(printf '%s\n' "${stat_output//${delimiter}/$'\n'}")" - if test "${stat_output_newlined}" = ""; then + if [ "${stat_output_newlined}" = '' ]; then log error "stat_output_newlined is empty. File name: '${file_name}' Stat output: '${stat_output}' @@ -101,7 +131,7 @@ line: '${processed_config_line}' readarray -t arr <<< "${stat_output_newlined}" - if test "${#arr[@]}" = 0; then + if [ "${#arr[@]}" = '0' ]; then log error "Array length is 0. File name: '${file_name}' Stat output: '${stat_output}' @@ -116,7 +146,7 @@ line: '${processed_config_line}' existing_group="${arr[2]}" file_name_from_stat="${arr[3]}" - if [ ! "$file_name" = "$file_name_from_stat" ]; then + if [ "$file_name" != "$file_name_from_stat" ]; then log error "\ File name is different from file name received from stat: File name: '${file_name}' @@ -126,15 +156,15 @@ line: '${processed_config_line}' return 1 fi - if test -z "${existing_mode}"; then + if [ -z "${existing_mode}" ]; then log error "Existing mode is empty. Stat output: '${stat_output}', line: '${processed_config_line}'" >&2 return 1 fi - if test -z "${existing_owner}"; then + if [ -z "${existing_owner}" ]; then log error "Existing owner is empty. Stat output: '${stat_output}', line: '${processed_config_line}'" >&2 return 1 fi - if test -z "${existing_group}"; then + if [ -z "${existing_group}" ]; then log error "Existing group is empty. Stat output: '${stat_output}', line: '${processed_config_line}'" >&2 return 1 fi @@ -143,6 +173,9 @@ line: '${processed_config_line}' print_usage(){ safe_echo "Usage: ${0##*/} enable ${0##*/} disable [FILE|all] + ${0##*/} print-policy + ${0##*/} print-state + ${0##*/} print-policy-applied-state Examples: ${0##*/} enable @@ -150,7 +183,6 @@ Examples: ${0##*/} disable /usr/bin/newgrp" >&2 } -## TODO: Validate input before you blindly trust it! add_to_policy() { local file_name file_mode file_owner file_group updated_entry policy_idx \ file_capabilities @@ -187,25 +219,23 @@ check_nosuid_whitelist() { target_file="${1:-}" ## Handle whitelists, if we're supposed to - if [ "${whitelists_disable_all}" = 'false' ]; then - ## literal matching is intentional here - # shellcheck disable=SC2076 - if ! [[ " ${policy_disable_white_list[*]} " =~ " ${target_file} " ]]; then - ## literal matching is intentional here too - # shellcheck disable=SC2076 - if [[ " ${policy_exact_white_list[*]} " =~ " ${target_file} " ]]; then - return 1 - fi + [ "${whitelists_disable_all}" = 'true' ] && return 0 - for match_white_list_entry in "${policy_match_white_list[@]:-}"; do - if safe_echo "${target_file}" \ - | grep --quiet --fixed-strings -- "${match_white_list_entry}"; then - return 1 - fi - done - fi + ## literal matching is intentional here + [[ " ${policy_disable_white_list[*]} " =~ " ${target_file} " ]] && return 0 + + ## literal matching is intentional here too + if [[ " ${policy_exact_white_list[*]} " =~ " ${target_file} " ]]; then + return 1 fi + for match_white_list_entry in "${policy_match_white_list[@]:-}"; do + if safe_echo "${target_file}" \ + | grep --quiet --fixed-strings -- "${match_white_list_entry}"; then + return 1 + fi + done + return 0 } @@ -223,11 +253,11 @@ load_early_nosuid_policy() { ## existing_owner ## existing_group output_stat "${find_list_item}" + if [ -z "${file_name_from_stat}" ]; then + continue + fi - ## -h file True if file is a symbolic Link. - ## -u file True if file has its set-user-id bit set. - ## -g file True if file has its set-group-id bit set. - + ## -h file True if file is a symbolic link. if [ -h "${find_list_item}" ]; then ## https://forums.whonix.org/t/disable-suid-binaries/7706/14 log info "Skip symlink: '${find_list_item}'" @@ -239,58 +269,107 @@ load_early_nosuid_policy() { continue fi - ## Trim off the most significant digit of the mode, this discards S(U|G)ID - ## bits (and the sticky bit too but that doesn't matter on Linux) - ## - ## Actually, the old behavior is better here. + ## Remove suid / gid and execute permission for 'group' and 'others'. + ## Similar to: chmod og-ugx /path/to/filename + ## Removing execution permission is useful to make binaries such as 'su' + ## fail closed rather than fail open if suid was removed from these. + ## Do not remove read access since no security benefit and easier to + ## manually undo for users. + ## Are there suid or sgid binaries which are still useful if suid / sgid + ## has been removed from these? local new_mode - # new_mode="${existing_mode:1}" new_mode='744' add_to_policy "${find_list_item}" "${new_mode}" "${existing_owner}" \ "${existing_group}" - done < <(safe_echo_nonewline "${target_file}" | find -files0-from - -perm /u=s,g=s -print0) + done < <(safe_echo_nonewline "${target_file}" \ + | find -files0-from - -perm /u=s,g=s -print0) +} + +match_dir() { + local base_str match_str base_arr match_arr base_idx + + base_str="${1}" + match_str="${2}" + [[ "${base_str}" =~ '//' ]] && return 1 + [[ "${match_str}" =~ '//' ]] && return 1 + + IFS='/' read -r -a base_arr <<< "${base_str}" + IFS='/' read -r -a match_arr <<< "${match_str}" + (( ${#base_arr[@]} > ${#match_arr[@]} )) && return 1 + + for (( base_idx=0; base_idx < ${#base_arr[@]}; base_idx++ )); do + if [ "${base_arr[base_idx]}" != "${match_arr[base_idx]}" ]; then + return 1 + fi + done + + return 0 } load_late_nosuid_policy() { local target_file state_idx state_file_item state_user_owner_item \ - state_group_owner_item + state_group_owner_item new_mode target_file="${1:-}" for (( state_idx=0; state_idx < ${#state_file_list[@]}; state_idx++ )); do state_file_item="${state_file_list[state_idx]}" - state_user_owner_item="${state_user_owner_list[state_idx]}" - state_group_owner_item="${state_group_owner_list[state_idx]}" check_nosuid_whitelist "${state_file_item}" || continue - if [[ ${state_file_item} == ${target_file}* ]]; then - if [ -h "${state_file_item}" ]; then - ## https://forums.whonix.org/t/disable-suid-binaries/7706/14 - log info "Skip symlink: '${state_file_item}'" - continue - fi + ## If the "target file" matches the start of the state file name, that's + ## a likely match. + match_dir "${target_file}" "${state_file_item}" || continue - if [ -d "${state_file_item}" ]; then - log info "Skip directory: '${state_file_item}'" - continue - fi - - local new_mode - new_mode='744' - add_to_policy "${state_file_item}" "${new_mode}" \ - "${state_user_owner_item}" "${state_group_owner_item}" + if [ -h "${state_file_item}" ]; then + ## https://forums.whonix.org/t/disable-suid-binaries/7706/14 + log info "Skip symlink: '${state_file_item}'" + continue fi + + if [ -d "${state_file_item}" ]; then + log info "Skip directory: '${state_file_item}'" + continue + fi + + state_user_owner_item="${state_user_owner_list[state_idx]}" + state_group_owner_item="${state_group_owner_list[state_idx]}" + new_mode='744' + add_to_policy "${state_file_item}" "${new_mode}" \ + "${state_user_owner_item}" "${state_group_owner_item}" done } +load_state_without_policy() { + local line bit_list + + ## Load the state file from disk + if [ -f "${state_file}" ]; then + while read -r line; do + read -r -a bit_list <<< "${line}" + if (( ${#bit_list[@]} != 4 )); then + log info \ + "Invalid number of fields in state file line: '${line}'. Skipping." + continue + fi + state_user_owner_list+=( "${bit_list[0]}" ) + state_group_owner_list+=( "${bit_list[1]}" ) + state_mode_list+=( "${bit_list[2]}" ) + state_file_list+=( "${bit_list[3]}" ) + done < "${state_file}" + fi +} + load_state() { ## Config format: ## path options ## where options is one of: ## user_owner group_owner filemode [capability-setting] ## [nosuid|exactwhitelist|matchwhitelist|disablewhitelist] + ## + ## Additionally, the special value 'whitelists_disable_all=true' is understood + ## to mean that all whitelisting should be ignored. - local config_file line bit_list file_path policy_nosuid_file_item + local config_file line bit_list policy_nosuid_file_item policy_file_item ## Load configuration, deferring whitelist handling until later for config_file in \ @@ -303,14 +382,17 @@ load_state() { if [ ! -f "${config_file}" ]; then continue fi + while read -r line; do if [ -z "${line}" ]; then true 'DEBUG: line is empty. Skipping.' continue fi + if [[ "${line}" =~ ^\s*# ]]; then continue fi + if ! [[ "${line}" =~ [0-9a-zA-Z/] ]]; then exit_code=200 log error "Line contains invalid characters: '${line}'" >&2 @@ -339,28 +421,27 @@ load_state() { # Strip trailing slash if appropriate bit_list[0]="${bit_list[0]%/}" - file_path="${bit_list[0]}" case "${bit_list[1]}" in 'exactwhitelist') - [ ! -e "${file_path}" ] && continue - policy_exact_white_list+=( "${file_path}" ) + [ ! -e "${bit_list[0]}" ] && continue + policy_exact_white_list+=( "${bit_list[0]}" ) continue ;; 'matchwhitelist') - policy_match_white_list+=( "${file_path}" ) + policy_match_white_list+=( "${bit_list[0]}" ) continue ;; 'disablewhitelist') - policy_disable_white_list+=( "${file_path}" ) + policy_disable_white_list+=( "${bit_list[0]}" ) continue ;; 'nosuid') - [ ! -e "${file_path}" ] && continue - policy_nosuid_file_list+=( "${file_path}" ) + [ ! -e "${bit_list[0]}" ] && continue + policy_nosuid_file_list+=( "${bit_list[0]}" ) ;; *) - [ ! -e "${file_path}" ] && continue + [ ! -e "${bit_list[0]}" ] && continue add_to_policy "${bit_list[@]}" ;; esac @@ -373,32 +454,19 @@ load_state() { load_early_nosuid_policy "${policy_nosuid_file_item}" done - local line bit_list policy_file_item - - ## Load the state file from disk - if [ -f "${state_file}" ]; then - while read -r line; do - read -r -a bit_list <<< "${line}" - if (( ${#bit_list[@]} != 4 )); then - log info "Invalid number of fields in state file line: '${line}'. Skipping." - continue - fi - state_user_owner_list+=( "${bit_list[0]}" ) - state_group_owner_list+=( "${bit_list[1]}" ) - state_mode_list+=( "${bit_list[2]}" ) - state_file_list+=( "${bit_list[3]}" ) - done < "${state_file}" - fi + load_state_without_policy ## Find any files in the policy that don't already have a matching file in ## the state. Add those files to the state, and save them to the state file ## as well. for policy_file_item in "${policy_file_list[@]}"; do - # shellcheck disable=SC2076 if [[ " ${state_file_list[*]} " =~ " ${policy_file_item} " ]]; then continue fi output_stat "${policy_file_item}" + if [ -z "${file_name_from_stat}" ]; then + continue + fi state_file_list+=( "${policy_file_item}" ) state_user_owner_list+=( "${existing_owner}" ) state_group_owner_list+=( "${existing_group}" ) @@ -410,6 +478,7 @@ load_state() { "${policy_file_item}" done + ## Fix up nosuid policies using state information for policy_nosuid_file_item in "${policy_nosuid_file_list[@]}"; do load_late_nosuid_policy "${policy_nosuid_file_item}" done @@ -433,7 +502,8 @@ apply_policy() { done if [ "${did_state_update}" = 'false' ]; then exit_code=206 - log error "File exists in policy but not in state! File: '${policy_file_list[policy_idx]}'" + log error \ + "File exists in policy but not in state! File: '${policy_file_list[policy_idx]}'" exit "${exit_code}" fi done @@ -469,22 +539,24 @@ commit_policy() { state_mode_item="${BASH_REMATCH[2]}" output_stat "${state_file_item}" + if [ -z "${file_name_from_stat}" ]; then + continue + fi if [ "${existing_owner}" != "${state_user_owner_item}" ] \ || [ "${existing_group}" != "${state_group_owner_item}" ] \ || [ "${existing_mode}" != "${state_mode_item}" ]; then - if ! grep --quiet --fixed-strings -- "${state_user_owner_item}:" "${store_dir}/private/passwd"; then + if ! [[ "${passwd_file_contents}" =~ "${state_user_owner_item}:" ]]; then log error "Owner from config does not exist: '${state_user_owner_item}'" >&2 continue fi - if ! grep --quiet --fixed-strings -- "${state_group_owner_item}:" "${store_dir}/private/group"; then + if ! [[ "${group_file_contents}" =~ "${state_group_owner_item}:" ]]; then log error "Group from config does not exist: '${state_group_owner_item}'" >&2 continue fi - # Remove and reapply in main list - if grep --quiet --fixed-strings \ - -- "${state_file_item}" <<< "${orig_main_statoverride_db}"; then + ## Remove and reapply in main list + if [[ "${orig_main_statoverride_db}" =~ "${state_file_item}" ]]; then echo_wrapper_ignore silent dpkg-statoverride --remove \ "${state_file_item}" fi @@ -492,9 +564,8 @@ commit_policy() { "${state_user_owner_item}" "${state_group_owner_item}" \ "${state_mode_item}" "${state_file_item}" - # Update item in secondary list - if grep --quiet --fixed-strings \ - -- "${state_file_item}" <<< "${orig_new_statoverride_db}"; then + ## Update item in secondary list + if [[ "${orig_new_statoverride_db}" =~ "${state_file_item}" ]]; then # shellcheck disable=SC2086 echo_wrapper_ignore silent dpkg-statoverride \ ${dpkg_admindir_parameter_new_mode} --remove \ @@ -574,14 +645,12 @@ undo_policy_for_file() { # shellcheck disable=SC2086 orig_new_statoverride_db="$(dpkg-statoverride ${dpkg_admindir_parameter_new_mode} --list)" || true - if grep --quiet --fixed-strings \ - -- "${undo_file}" <<< "${orig_main_statoverride_db}"; then + if [[ "${orig_main_statoverride_db}" =~ "${undo_file}" ]]; then echo_wrapper_ignore silent dpkg-statoverride --remove \ "${undo_file}" fi - if grep --quiet --fixed-strings \ - -- "${undo_file}" <<< "${orig_new_statoverride_db}"; then + if [[ "${orig_new_statoverride_db}" =~ "${undo_file}" ]]; then # shellcheck disable=SC2086 echo_wrapper_ignore silent dpkg-statoverride \ ${dpkg_admindir_parameter_new_mode} --remove \ @@ -594,13 +663,16 @@ undo_policy_for_file() { state_mode_item="${state_mode_list[state_idx]}" chown ${verbose} "${state_user_owner_item}:${state_group_owner_item}" \ "${undo_file}" || exit_code=202 - ## chmod need to be run after chown since chown removes suid. + ## chmod needs to be run after chown since chown removes suid. chmod ${verbose} "${state_mode_item}" "${undo_file}" || exit_code=203 else log info "File does not exist: '${undo_file}'" fi did_undo=true - break + + if [ "${undo_all}" = 'false' ]; then + break + fi fi done @@ -637,7 +709,7 @@ print_columns() { format_str="${format_str}%s\t" done format_str="${format_str}\n" - # Using a dynamically generated format string on purpose. + ## Using a dynamically generated format string on purpose. # shellcheck disable=SC2059 printf "${format_str}" "$@" } @@ -670,13 +742,6 @@ print_state() { done } -## Constants -store_dir="/var/lib/permission-hardener" -state_file="${store_dir}/existing_mode/statoverride" -dpkg_admindir_parameter_existing_mode="--admindir ${store_dir}/existing_mode" -dpkg_admindir_parameter_new_mode="--admindir ${store_dir}/new_mode" -delimiter="#permission-hardener-delimiter#" - ## Global variables policy_file_list=() policy_user_owner_list=() @@ -696,6 +761,9 @@ existing_mode='' existing_owner='' existing_group='' processed_config_line='' +file_name_from_stat='' +passwd_file_contents="$(getent passwd)" +group_file_contents="$(getent group)" exit_code=0 ## Setup and sanity checking @@ -704,15 +772,8 @@ if [ "$(id -u)" != '0' ]; then exit 1 fi -mkdir --parents "${store_dir}/private" mkdir --parents "${store_dir}/existing_mode" mkdir --parents "${store_dir}/new_mode" -touch "${store_dir}/private/passwd" -chmod og-rwx "${store_dir}/private/passwd" -touch "${store_dir}/private/group" -chmod og-rwx "${store_dir}/private/group" -getent passwd | sponge -- "${store_dir}/private/passwd" -getent group | sponge -- "${store_dir}/private/group" echo_wrapper_audit silent which capsh getcap setcap stat find \ dpkg-statoverride getent grep 1>/dev/null @@ -733,7 +794,7 @@ case "${1:-}" in exit 1 ;; *) - load_state + load_state_without_policy undo_policy_for_file "${1}" ;; esac @@ -761,6 +822,7 @@ case "${1:-}" in ;; esac +## Exit if test "${exit_code}" != "0"; then log error "Exiting with non-zero exit code: '${exit_code}'" >&2 fi diff --git a/usr/bin/permission-hardener-old b/usr/bin/permission-hardener-old deleted file mode 100755 index c88b54f..0000000 --- a/usr/bin/permission-hardener-old +++ /dev/null @@ -1,748 +0,0 @@ -#!/bin/bash - -## Copyright (C) 2012 - 2024 ENCRYPTED SUPPORT LP -## See the file COPYING for copying conditions. - -## https://forums.whonix.org/t/disable-suid-binaries/7706 -## https://forums.whonix.org/t/re-mount-home-and-other-with-noexec-and-nosuid-among-other-useful-mount-options-for-better-security/7707 - -## dpkg-statoverride does not support end-of-options ("--"). - -set -o errexit -o nounset -o pipefail - -exit_code=0 -store_dir="/var/lib/permission-hardener" -dpkg_admindir_parameter_existing_mode="--admindir ${store_dir}/existing_mode" -dpkg_admindir_parameter_new_mode="--admindir ${store_dir}/new_mode" -delimiter="#permission-hardener-delimiter#" - -# shellcheck disable=SC1091 -source /usr/libexec/helper-scripts/safe_echo.sh -# shellcheck disable=SC2034 -log_level=notice -# shellcheck disable=SC1091 -source /usr/libexec/helper-scripts/log_run_die.sh - -echo_wrapper_ignore() { - if test "${1}" = "verbose"; then - shift - log notice "Executing: $*" - else - shift - fi - "$@" 2>/dev/null || true -} - -echo_wrapper_audit() { - if test "${1}" = "verbose"; then - shift - log notice "Executing: $*" - else - shift - fi - return_code=0 - "$@" || - { - return_code="$?" - exit_code=203 - log error "Command '$*' failed with exit code '${return_code}'! calling function name: '${FUNCNAME[1]}'" >&2 - } -} - -make_store_dir(){ - mkdir --parents "${store_dir}/private" - mkdir --parents "${store_dir}/existing_mode" - mkdir --parents "${store_dir}/new_mode" -} - -## Some tools may fail on newlines and even variable assignment to array may -## fail if a variable that will be assigned to an array element contains -## characters that are used as delimiters. -block_newlines(){ - local newline_variable newline_value - newline_variable="${1}" - newline_value="${2}" - ## dpkg-statoverride: error: path may not contain newlines - #if [[ "${newline_value}" == *$'\n'* ]]; then - if [[ "${newline_value}" != "${newline_value//$'\n'/NEWLINE}" ]]; then - log warn "Skipping ${newline_variable} that contains newlines: '${newline_value}'" >&2 - return 1 - fi -} - -output_stat(){ - local file_name - file_name="${1}" - - if test -z "${file_name}"; then - log error "File name is empty. file_name: '${file_name}'" >&2 - return 1 - fi - - block_newlines file "${file_name}" - - ## dpkg-statoverride can actually handle '--file-name'. -# if [[ $file_name == --* ]]; then -# log warn "File name starts with '--'. This would be interpreted by dpkg-statoverride as an option. Skipping. file_name: '${file_name}'" >&2 -# return 1 -# fi - - declare -a arr - local file_name_from_stat stat_output stat_output_newlined - - if ! stat_output="$(stat --format="%a${delimiter}%U${delimiter}%G${delimiter}%n${delimiter}" -- "${file_name}")"; then - log error "Failed to run 'stat' on file: '${file_name}'!" >&2 - return 1 - fi - - if [ "$stat_output" = "" ]; then - log error "stat_output is empty. -File name: '${file_name}' -Stat output: '${stat_output}' -stat_output_newlined: '${stat_output_newlined}' -line: '${line}' -" >&2 - return 1 - fi - - stat_output_newlined="$(printf '%s\n' "${stat_output//${delimiter}/$'\n'}")" - - if test "${stat_output_newlined}" = ""; then - log error "stat_output_newlined is empty. -File name: '${file_name}' -Stat output: '${stat_output}' -stat_output_newlined: '${stat_output_newlined}' -line: '${line}' -" >&2 - return 1 - fi - - readarray -t arr <<< "${stat_output_newlined}" - - if test "${#arr[@]}" = 0; then - log error "Array length is 0. -File name: '${file_name}' -Stat output: '${stat_output}' -stat_output_newlined: '${stat_output_newlined}' -line: '${line}' -" >&2 - return 1 - fi - - existing_mode="${arr[0]}" - existing_owner="${arr[1]}" - existing_group="${arr[2]}" - file_name_from_stat="${arr[3]}" - - if [ ! "$file_name" = "$file_name_from_stat" ]; then - log error "\ -File name is different from file name received from stat: -File name: '${file_name}' -File name from stat: '${file_name_from_stat}' -line: '${line}' -" >&2 - return 1 - fi - - if test -z "${existing_mode}"; then - log error "Existing mode is empty. Stat output: '${stat_output}', line: '${line}'" >&2 - return 1 - fi - if test -z "${existing_owner}"; then - log error "Existing owner is empty. Stat output: '${stat_output}', line: '${line}'" >&2 - return 1 - fi - if test -z "${existing_group}"; then - log error "Existing group is empty. Stat output: '${stat_output}', line: '${line}'" >&2 - return 1 - fi -} - -sanity_tests() { - echo_wrapper_audit silent \ - which \ - capsh getcap setcap stat find dpkg-statoverride getent grep 1>/dev/null -} - -add_nosuid_statoverride_entry() { - local fso_to_process - fso_to_process="${fso}" - local should_be_counter - should_be_counter=0 - local counter_actual - counter_actual=0 - - local dummy_line - while IFS="" read -r -d "" dummy_line; do - log info "Test would parse line: '${dummy_line}'" - should_be_counter=$((should_be_counter + 1)) - done < <(safe_echo_nonewline "${fso_to_process}" | find -files0-from - -perm /u=s,g=s -print0) - ## False positive on SC2185 (find without path argument) #1748 - ## https://github.com/koalaman/shellcheck/issues/1748 - ## - ## /usr/lib will hit ARG_MAX if using bash 'shopt -s globstar' and '/usr/lib/**'. - ## Using 'find' with '-perm /u=s,g=s' is faster and avoids ARG_MAX. - ## https://forums.whonix.org/t/disable-suid-binaries/7706/17 - - local line - while IFS="" read -r -d "" file_name; do - counter_actual=$((counter_actual + 1)) - - ## sets: - ## exiting_mode - ## existing_owner - ## existing_group - output_stat "${file_name}" - - ## -h file True if file is a symbolic Link. - ## -u file True if file has its set-user-id bit set. - ## -g file True if file has its set-group-id bit set. - - if test -h "${file_name}"; then - ## https://forums.whonix.org/t/disable-suid-binaries/7706/14 - log info "Skip symlink: '${file_name}'" - continue - fi - - if test -d "${file_name}"; then - log info "Skip directory: '${file_name}'" - continue - fi - - local setuid setgid - setuid="" - if test -u "${file_name}"; then - setuid=true - fi - setgid="" - if test -g "${file_name}"; then - setgid=true - fi - - local setuid_or_setgid - setuid_or_setgid="" - if test "${setuid}" = "true" || test "${setgid}" = "true"; then - setuid_or_setgid=true - fi - if test -z "${setuid_or_setgid}"; then - log info "Neither setuid nor setgid. Skipping. file_name: '${file_name}'" - continue - fi - - ## Remove suid / gid and execute permission for 'group' and 'others'. - ## Similar to: chmod og-ugx /path/to/filename - ## Removing execution permission is useful to make binaries such as 'su' - ## fail closed rather than fail open if suid was removed from these. - ## Do not remove read access since no security benefit and easier to - ## manually undo for users. - ## Are there suid or sgid binaries which are still useful if suid / sgid - ## has been removed from these? - new_mode="744" - - local is_exact_whitelisted - is_exact_whitelisted="" - for white_list_entry in "${exact_white_list[@]:-}"; do - if test -z "${white_list_entry}"; then - log info "white_list_entry unset. Skipping. file_name: '${file_name}'" - continue - fi - if test "${file_name}" = "${white_list_entry}"; then - is_exact_whitelisted="true" - log info "is_exact_whitelisted=true. Skipping. file_name: '${file_name}'" - ## Stop looping through the whitelist. - break - fi - done - - local is_match_whitelisted - is_match_whitelisted="" - for matchwhite_list_entry in "${match_white_list[@]:-}"; do - if test -z "${matchwhite_list_entry}"; then - log info "matchwhite_list_entry unset. Skipping. file_name: '${file_name}'" - continue - fi - if safe_echo "${file_name}" | grep --quiet --fixed-strings -- "${matchwhite_list_entry}"; then - is_match_whitelisted="true" - log info "is_match_whitelisted=true. Skipping. file_name: '${file_name}'" - ## Stop looping through the match_white_list. - break - fi - done - - local is_disable_whitelisted - is_disable_whitelisted="" - for disablematch_list_entry in "${disable_white_list[@]:-}"; do - if test -z "${disablematch_list_entry}"; then - log info "disablematch_list_entry unset. Skipping. file_name: '${file_name}'" - continue - fi - if safe_echo "${file_name}" | grep --quiet --fixed-strings -- "${disablematch_list_entry}"; then - is_disable_whitelisted="true" - log info "is_disable_whitelisted=true. Skipping. file_name: '${file_name}'" - ## Stop looping through the disablewhitelist. - break - fi - done - - local clean_output_prefix clean_output - clean_output_prefix="Managing (S|G)UID of line:" - clean_output="${setuid:+setuid='true'} ${setgid:+setgid='true'} existing_mode='${existing_mode}' new_mode='${new_mode}' file='${file_name}'" - if test "${whitelists_disable_all:-}" = "true"; then - log info "${clean_output_prefix} whitelists_disable_all=true ${clean_output}" - elif test "${is_disable_whitelisted}" = "true"; then - log info "${clean_output_prefix} is_disable_whitelisted=true ${clean_output}" - else - if test "${is_exact_whitelisted}" = "true"; then - log info "${clean_output_prefix} is_exact_whitelisted=true ${clean_output}" - continue - fi - if test "${is_match_whitelisted}" = "true"; then - log info "${clean_output_prefix} is_match_whitelisted=true matchwhite_list_entry='${matchwhite_list_entry}' ${clean_output}" - continue - fi - fi - - log notice "${clean_output_prefix} ${clean_output}" - - # shellcheck disable=SC2086 - if dpkg-statoverride ${dpkg_admindir_parameter_existing_mode} --list "${file_name}" >/dev/null; then - log info "Existing mode already saved previously. Not saving again." - else - ## Save existing_mode in separate database. - ## Not using --update as not intending to enforce existing_mode. - # shellcheck disable=SC2086 - echo_wrapper_audit silent dpkg-statoverride ${dpkg_admindir_parameter_existing_mode} --add "${existing_owner}" "${existing_group}" "${existing_mode}" "${file_name}" - fi - - ## No need to check "dpkg-statoverride --list" for existing entries. - ## If existing_mode was correct already, we would not have reached this - ## point. Since existing_mode is incorrect, remove from dpkg-statoverride - ## and re-add. - - ## Remove from real database. - echo_wrapper_ignore silent dpkg-statoverride --remove "${file_name}" - - ## Remove from separate database. - # shellcheck disable=SC2086 - echo_wrapper_ignore silent dpkg-statoverride ${dpkg_admindir_parameter_new_mode} --remove "${file_name}" - - ## Add to real database and use --update to make changes on disk. - echo_wrapper_audit verbose dpkg-statoverride --add --update "${existing_owner}" "${existing_group}" "${new_mode}" "${file_name}" - - ## Not using --update as this is only for recording. - # shellcheck disable=SC2086 - echo_wrapper_audit silent dpkg-statoverride ${dpkg_admindir_parameter_new_mode} --add "${existing_owner}" "${existing_group}" "${new_mode}" "${file_name}" - done < <(safe_echo_nonewline "${fso_to_process}" | find -files0-from - -perm /u=s,g=s -print0) - - ## Sanity test. - if test ! "${should_be_counter}" = "${counter_actual}"; then - log info "File (parsed/wanted): '${fso_to_process}': (${counter_actual}/${should_be_counter})" - log error "Expected number of files to be parsed was not met." >&2 - exit_code=202 - fi -} - -set_file_perms() { - log info "START parsing config file: '${config_file}'" - - local line - while read -r line || test -n "${line}"; do - if test -z "${line}"; then - true "DEBUG: line is empty. Skipping." - continue - fi - - if [[ "${line}" =~ ^\s*# ]]; then - continue - fi - - if ! [[ "${line}" =~ [0-9a-zA-Z/] ]]; then - exit_code=200 - log error "Line contains invalid characters: '${line}'" >&2 - ## Safer to exit with error in this case. - ## https://forums.whonix.org/t/disable-suid-binaries/7706/59 - exit "${exit_code}" - fi - - if test "${line}" = 'whitelists_disable_all=true'; then - whitelists_disable_all=true - log info "whitelists_disable_all=true" - continue - fi - - #global fso - local mode_from_config owner_from_config group_from_config capability_from_config - if ! read -r fso mode_from_config owner_from_config group_from_config capability_from_config <<<"${line}"; then - exit_code=201 - log error "Cannot parse line: '${line}'" >&2 - ## Debugging. - du -hs /tmp || true - safe_echo "test -w /tmp: '$(test -w /tmp)'" >&2 || true - ## Safer to exit with error in this case. - ## https://forums.whonix.org/t/disable-suid-binaries/7706/59 - exit "${exit_code}" - fi - - log info "Parsing line: fso='${fso}' mode_from_config='${mode_from_config}' owner_from_config='${owner_from_config}' group_from_config='${group_from_config}' capability_from_config='${capability_from_config}'" - - ## Debugging. - #safe_echo "line: '${line}'" - #safe_echo "fso: '${fso}'" - #safe_echo "mode_from_config: '${mode_from_config}'" - #safe_echo "owner_from_config: '${owner_from_config}'" - - local fso_without_trailing_slash - fso_without_trailing_slash="${fso%/}" - - declare -g disable_white_list exact_white_list match_white_list - case "${mode_from_config}" in - disablewhitelist) - disable_white_list+=("${fso}") - continue - ;; - exactwhitelist) - exact_white_list+=("${fso}") - continue - ;; - matchwhitelist) - match_white_list+=("${fso}") - continue - ;; - esac - - if test ! -e "${fso}"; then - log info "File does not exist: '${fso}'" - continue - fi - - ## Use dpkg-statoverride so permissions are not reset during upgrades. - - if test "${mode_from_config}" = "nosuid"; then - ## If mode_from_config is "nosuid" the config does not set owner and - ## group. Therefore do not enforce owner/group check. - add_nosuid_statoverride_entry - else - local string_length_of_mode_from_config - string_length_of_mode_from_config="${#mode_from_config}" - if test "${string_length_of_mode_from_config}" -gt "4"; then - log error "Invalid mode: '${mode_from_config}'" >&2 - continue - fi - if test "${string_length_of_mode_from_config}" -lt "3"; then - log error "Invalid mode: '${mode_from_config}'" >&2 - continue - fi - - if ! grep --quiet --fixed-strings -- "${owner_from_config}:" "${store_dir}/private/passwd"; then - log error "Owner from config does not exist: '${owner_from_config}'" >&2 - continue - fi - - if ! grep --quiet --fixed-strings -- "${group_from_config}:" "${store_dir}/private/group"; then - log error "Group from config does not exist: '${group_from_config}'" >&2 - continue - fi - - local mode_for_grep - mode_for_grep="${mode_from_config}" - first_character_of_mode_from_config="${mode_from_config::1}" - if test "${first_character_of_mode_from_config}" = "0"; then - ## Remove leading '0'. - mode_for_grep="${mode_from_config:1}" - fi - - file_name="${fso_without_trailing_slash}" - - ## sets: - ## exiting_mode - ## existing_owner - ## existing_group - output_stat "${file_name}" - - ## Check there is an entry for the fso. - ## - ## example: dpkg-statoverride --list | grep /home - ## output: - ## root root 755 /home - ## - ## dpkg-statoverride does not show leading '0'. - local dpkg_statoverride_list_output="" - local dpkg_statoverride_list_exit_code=0 - dpkg_statoverride_list_output="$(dpkg-statoverride --list "${fso_without_trailing_slash}")" || { - dpkg_statoverride_list_exit_code=$? - true - } - - if test "${dpkg_statoverride_list_exit_code}" = "0"; then - local grep_line - grep_line="${owner_from_config} ${group_from_config} ${mode_for_grep} ${fso_without_trailing_slash}" - if safe_echo "${dpkg_statoverride_list_output}" | grep --quiet --fixed-strings -- "${grep_line}"; then - log info "The owner/group/mode matches fso entry. No further action required." - else - log info "The owner/group/mode does not match fso entry, updating entry." - ## fso_without_trailing_slash instead of fso to prevent - ## "dpkg-statoverride: warning: stripping trailing /" - - # shellcheck disable=SC2086 - if dpkg-statoverride ${dpkg_admindir_parameter_existing_mode} --list "${fso_without_trailing_slash}" >/dev/null; then - log info "Existing mode already saved previously. Not saving again." - else - ## Save existing_mode in separate database. - ## Not using --update as not intending to enforce existing_mode. - # shellcheck disable=SC2086 - echo_wrapper_audit silent dpkg-statoverride ${dpkg_admindir_parameter_existing_mode} --add "${existing_owner}" "${existing_group}" "${existing_mode}" "${fso_without_trailing_slash}" - fi - - # shellcheck disable=SC2086 - echo_wrapper_ignore silent dpkg-statoverride ${dpkg_admindir_parameter_new_mode} --remove "${fso_without_trailing_slash}" - - ## Remove from and add to real database. - echo_wrapper_ignore silent dpkg-statoverride --remove "${fso_without_trailing_slash}" - echo_wrapper_audit verbose dpkg-statoverride --add --update "${owner_from_config}" "${group_from_config}" "${mode_from_config}" "${fso_without_trailing_slash}" - - ## Save in separate database. - ## Not using --update as this is only for saving. - # shellcheck disable=SC2086 - echo_wrapper_audit silent dpkg-statoverride ${dpkg_admindir_parameter_new_mode} --add "${owner_from_config}" "${group_from_config}" "${mode_from_config}" "${fso_without_trailing_slash}" - fi - else - log info "There is no fso entry, adding one." - - # shellcheck disable=SC2086 - if dpkg-statoverride ${dpkg_admindir_parameter_existing_mode} --list "${fso_without_trailing_slash}" >/dev/null; then - log info "Existing mode already saved previously. Not saving again." - else - ## Save existing_mode in separate database. - ## Not using --update as not intending to enforce existing_mode. - # shellcheck disable=SC2086 - echo_wrapper_audit silent dpkg-statoverride ${dpkg_admindir_parameter_existing_mode} --add "${existing_owner}" "${existing_group}" "${existing_mode}" "${fso_without_trailing_slash}" - fi - - ## Add to real database. - echo_wrapper_audit verbose dpkg-statoverride --add --update "${owner_from_config}" "${group_from_config}" "${mode_from_config}" "${fso_without_trailing_slash}" - - ## Save in separate database. - ## Not using --update as this is only for saving. - # shellcheck disable=SC2086 - echo_wrapper_audit silent dpkg-statoverride ${dpkg_admindir_parameter_new_mode} --add "${owner_from_config}" "${group_from_config}" "${mode_from_config}" "${fso_without_trailing_slash}" - fi - fi - if test -z "${capability_from_config}"; then - log info "capability_from_config is empty. Skipping. file_name: '${file_name}'" - continue - fi - - if test "${capability_from_config}" = "none"; then - ## https://forums.whonix.org/t/disable-suid-binaries/7706/45 - ## sudo setcap -r /bin/ping 2>/dev/null - ## Failed to set capabilities on file '/bin/ping' (No data available) - ## The value of the capability argument is not permitted for a file. Or - ## the file is not a regular (non-symlink) file - ## Therefore use echo_wrapper_ignore. - ## - ## NOTE: setcap does not support End-of-Options Marker ('--') yet. - ## setcap bug report: - ## setcap Command Does Not Support End-of-Options Marker ('--') - ## https://bugzilla.kernel.org/show_bug.cgi?id=219487 - echo_wrapper_ignore verbose setcap -r "${fso}" - getcap_output="$(getcap -- "${fso}")" - if test -n "${getcap_output}"; then - exit_code=205 - log error "Removing capabilities failed. File: '${fso}'" >&2 - continue - fi - else - if ! capsh --print | grep --fixed-strings -- "Bounding set" | grep --quiet -- "${capability_from_config}"; then - log error "Capability from config does not exist: '${capability_from_config}'" >&2 - continue - fi - - ## feature request: dpkg-statoverride: support for capabilities - ## https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=502580 - echo_wrapper_audit verbose setcap "${capability_from_config}+ep" -- "${fso}" - fi - - done <"${config_file}" - log info "END parsing config file: '${config_file}'" -} - -parse_config_folder() { - touch "${store_dir}/private/passwd" - chmod og-rwx "${store_dir}/private/passwd" - touch "${store_dir}/private/group" - chmod og-rwx "${store_dir}/private/group" - - local passwd_file_contents_temp - ## Query contents of password and group databases only once and buffer them - ## - ## If we don't buffer we sometimes get incorrect results when checking for - ## entries using 'if getent passwd | grep --quiet -- '^root:'; ...' since - ## 'grep' exits after the first match in this case causing 'getent' to - ## receive SIGPIPE, which then fails the pipeline since 'set -o pipefail' is - ## set for this script. - passwd_file_contents_temp="$(getent passwd)" - safe_echo "${passwd_file_contents_temp}" | tee -- "${store_dir}/private/passwd" >/dev/null - group_file_contents_temp="$(getent group)" - safe_echo "${group_file_contents_temp}" | tee -- "${store_dir}/private/group" >/dev/null - - #passwd_file_contents="$(cat "${store_dir}/private/passwd")" - #group_file_contents="$(cat "${store_dir}/private/group")" - - shopt -s nullglob - for config_file in \ - /usr/lib/permission-hardener.d/*.conf \ - /etc/permission-hardener.d/*.conf \ - /usr/local/etc/permission-hardener.d/*.conf \ - /etc/permission-hardening.d/*.conf \ - /usr/local/etc/permission-hardening.d/*.conf - do - set_file_perms - - done -} - -apply() { - check_root - make_store_dir - sanity_tests - parse_config_folder - - log notice "\ -To compare the current and previous permission modes, install 'meld' (or preferred diff tool) for comparison of file mode changes: - sudo apt install --no-install-recommends meld - meld ${store_dir}/existing_mode/statoverride ${store_dir}/new_mode/statoverride" -} - -spare() { - check_root - make_store_dir - - remove_file="${1}" - exit_code=0 - dpkg_admindir_parameter_existing_mode="--admindir ${store_dir}/existing_mode" - dpkg_admindir_parameter_new_mode="--admindir ${store_dir}/new_mode" - - if test ! -f "${store_dir}/existing_mode/statoverride"; then - true "DEBUG: Stat file does not exist, hardening was not applied before." - return 0 - fi - - local line - while read -r line; do - ## example line: - ## root root 4755 /usr/lib/eject/dmcrypt-get-device - - local owner group mode file_name - if ! read -r owner group mode file_name <<< "${line}"; then - exit_code=201 - log error "Cannot parse line: '${line}'" >&2 - continue - fi - log info "Parsing line: owner='${owner}' group='${group}' mode='${mode}' file_name='${file_name}'" - - if test "${remove_file}" = "all"; then - verbose="" - remove_one=false - else - if test "${remove_file}" = "${file_name}"; then - verbose="--verbose" - remove_one=true - safe_echo "${remove_one}" | tee -- "${store_dir}/remove_one" >/dev/null - else - safe_echo "false" | tee -- "${store_dir}/remove_one" >/dev/null - continue - fi - fi - - if test "${remove_one}" = "true"; then - set -o xtrace - fi - - if test -e "${file_name}"; then - # shellcheck disable=SC2086 - chown ${verbose} "${owner}:${group}" "${file_name}" || exit_code=202 - ## chmod need to be run after chown since chown removes suid. - ## https://unix.stackexchange.com/questions/53665/chown-removes-setuid-bit-bug-or-feature - # shellcheck disable=SC2086 - chmod ${verbose} "${mode}" "${file_name}" || exit_code=203 - else - log info "File does not exist: '${file_name}'" - fi - - dpkg-statoverride --remove "${file_name}" &>/dev/null || true - # shellcheck disable=SC2086 - dpkg-statoverride ${dpkg_admindir_parameter_existing_mode} --remove "${file_name}" &>/dev/null || true - # shellcheck disable=SC2086 - dpkg-statoverride ${dpkg_admindir_parameter_new_mode} --remove "${file_name}" &>/dev/null || true - - if test "${remove_one}" = "true"; then - set +o xtrace - break - fi - - done < "${store_dir}/existing_mode/statoverride" - - if test ! "${remove_file}" = "all"; then - if test "$(cat "${store_dir}/remove_one")" = "false"; then - log info "No file was removed. - - File '${remove_file}' has not been removed from SUID Disabler and Permission Hardener during this invocation. This is expected if already done earlier. - - This program expects the full path to the file. Example: - $0 disable /usr/bin/newgrp # absolute path: works - $0 disable newgrp # relative path: does not work - - To remove all: - $0 disable all - - This change might not be permanent. For full instructions, see: - https://www.kicksecure.com/wiki/SUID_Disabler_and_Permission_Hardener - - To view list of changed by SUID Disabler and Permission Hardener: - https://www.kicksecure.com/wiki/SUID_Disabler_and_Permission_Hardener#View_List_of_Permissions_Changed_by_SUID_Disabler_and_Permission_Hardener - - For re-enabling any specific SUID binary: - https://www.kicksecure.com/wiki/SUID_Disabler_and_Permission_Hardener#Re-Enable_Specific_SUID_Binaries - - For completely disabling SUID Disabler and Permission Hardener: - https://www.kicksecure.com/wiki/SUID_Disabler_and_Permission_Hardener#Disable_SUID_Disabler_and_Permission_Hardener" - fi - fi -} - -check_root(){ - if test "$(id -u)" != "0"; then - log error "Not running as root, aborting." - exit 1 - fi -} - -usage(){ - safe_echo "Usage: ${0##*/} enable - ${0##*/} disable [FILE|all] - -Examples: - ${0##*/} enable - ${0##*/} disable all - ${0##*/} disable /usr/bin/newgrp" >&2 - exit "${1}" -} - -case "${1:-}" in - enable) shift; apply "$@";; - disable) - shift - case "${1:-}" in - "") usage 1;; - *) spare "${1}";; - esac - ;; - -h|--help) usage 0;; - *) usage 1;; -esac - -if test "${exit_code}" != "0"; then - log error "Exiting with non-zero exit code: '${exit_code}'" >&2 -fi - -exit "${exit_code}" diff --git a/usr/lib/permission-hardener.d/25_default_whitelist_passwd.conf b/usr/lib/permission-hardener.d/25_default_whitelist_passwd.conf index 6c8369c..71d2298 100644 --- a/usr/lib/permission-hardener.d/25_default_whitelist_passwd.conf +++ b/usr/lib/permission-hardener.d/25_default_whitelist_passwd.conf @@ -7,7 +7,8 @@ # Keep the `passwd` utility executable to prevent issues with the # /usr/libexec/security-misc/pam-abort-on-locked-password script blocking -# user logins with `su` and KScreenLocker +# user logins with `su` and KScreenLocker. exactwhitelist is needed to keep +# the nosuid rule on /usr/bin from fighting with these rules. # # See also: https://www.kicksecure.com/wiki/SUID_Disabler_and_Permission_Hardener#passwd /usr/bin/passwd exactwhitelist From 717e6fcfbea38cef9d3e201cf2e2b725e3da2267 Mon Sep 17 00:00:00 2001 From: Aaron Rainbolt Date: Mon, 30 Dec 2024 19:23:20 -0600 Subject: [PATCH 3/4] Post-review improvements to permission-hardener --- usr/bin/permission-hardener | 61 ++++++++++++++++++------------------- 1 file changed, 30 insertions(+), 31 deletions(-) diff --git a/usr/bin/permission-hardener b/usr/bin/permission-hardener index de03c7c..bcd8180 100755 --- a/usr/bin/permission-hardener +++ b/usr/bin/permission-hardener @@ -107,7 +107,7 @@ output_stat() { return 1 fi - if [ "$stat_output" = '' ]; then + if [ -z "$stat_output" ]; then log error "stat_output is empty. File name: '${file_name}' Stat output: '${stat_output}' @@ -119,7 +119,7 @@ line: '${processed_config_line}' stat_output_newlined="$(printf '%s\n' "${stat_output//${delimiter}/$'\n'}")" - if [ "${stat_output_newlined}" = '' ]; then + if [ -z "${stat_output_newlined}" ]; then log error "stat_output_newlined is empty. File name: '${file_name}' Stat output: '${stat_output}' @@ -225,9 +225,7 @@ check_nosuid_whitelist() { [[ " ${policy_disable_white_list[*]} " =~ " ${target_file} " ]] && return 0 ## literal matching is intentional here too - if [[ " ${policy_exact_white_list[*]} " =~ " ${target_file} " ]]; then - return 1 - fi + [[ " ${policy_exact_white_list[*]} " =~ " ${target_file} " ]] && return 1 for match_white_list_entry in "${policy_match_white_list[@]:-}"; do if safe_echo "${target_file}" \ @@ -286,6 +284,9 @@ load_early_nosuid_policy() { | find -files0-from - -perm /u=s,g=s -print0) } +## If the "target file" matches the start of the state file name, that's a +## likely match. This is used by load_late_nosuid_policy for detecting info +## about files that need SUID-locked that are in the state. match_dir() { local base_str match_str base_arr match_arr base_idx @@ -316,8 +317,6 @@ load_late_nosuid_policy() { state_file_item="${state_file_list[state_idx]}" check_nosuid_whitelist "${state_file_item}" || continue - ## If the "target file" matches the start of the state file name, that's - ## a likely match. match_dir "${target_file}" "${state_file_item}" || continue if [ -h "${state_file_item}" ]; then @@ -340,21 +339,21 @@ load_late_nosuid_policy() { } load_state_without_policy() { - local line bit_list + local line field_list ## Load the state file from disk if [ -f "${state_file}" ]; then while read -r line; do - read -r -a bit_list <<< "${line}" - if (( ${#bit_list[@]} != 4 )); then + read -r -a field_list <<< "${line}" + if (( ${#field_list[@]} != 4 )); then log info \ "Invalid number of fields in state file line: '${line}'. Skipping." continue fi - state_user_owner_list+=( "${bit_list[0]}" ) - state_group_owner_list+=( "${bit_list[1]}" ) - state_mode_list+=( "${bit_list[2]}" ) - state_file_list+=( "${bit_list[3]}" ) + state_user_owner_list+=( "${field_list[0]}" ) + state_group_owner_list+=( "${field_list[1]}" ) + state_mode_list+=( "${field_list[2]}" ) + state_file_list+=( "${field_list[3]}" ) done < "${state_file}" fi } @@ -369,7 +368,7 @@ load_state() { ## Additionally, the special value 'whitelists_disable_all=true' is understood ## to mean that all whitelisting should be ignored. - local config_file line bit_list policy_nosuid_file_item policy_file_item + local config_file line field_list policy_nosuid_file_item policy_file_item ## Load configuration, deferring whitelist handling until later for config_file in \ @@ -393,7 +392,7 @@ load_state() { continue fi - if ! [[ "${line}" =~ [0-9a-zA-Z/] ]]; then + if ! [[ "${line}" =~ ^[-0-9a-zA-Z._/[:space:]]*$ ]]; then exit_code=200 log error "Line contains invalid characters: '${line}'" >&2 ## Safer to exit with error in this case. @@ -409,40 +408,40 @@ load_state() { processed_config_line="${line}" - IFS=' ' read -r -a bit_list <<< "${line}" + IFS=' ' read -r -a field_list <<< "${line}" - if (( ${#bit_list[@]} < 2 )) \ - || (( ${#bit_list[@]} > 5 )) \ - || (( ${#bit_list[@]} == 3 )); then + if (( ${#field_list[@]} != 2 )) \ + && (( ${#field_list[@]} != 4 )) \ + && (( ${#field_list[@]} != 5 )); then exit_code=200 log error "Line contains an invalid number of fields: '${line}'" >&2 exit "${exit_code}" fi # Strip trailing slash if appropriate - bit_list[0]="${bit_list[0]%/}" + field_list[0]="${field_list[0]%/}" - case "${bit_list[1]}" in + case "${field_list[1]}" in 'exactwhitelist') - [ ! -e "${bit_list[0]}" ] && continue - policy_exact_white_list+=( "${bit_list[0]}" ) + [ ! -e "${field_list[0]}" ] && continue + policy_exact_white_list+=( "${field_list[0]}" ) continue ;; 'matchwhitelist') - policy_match_white_list+=( "${bit_list[0]}" ) + policy_match_white_list+=( "${field_list[0]}" ) continue ;; 'disablewhitelist') - policy_disable_white_list+=( "${bit_list[0]}" ) + policy_disable_white_list+=( "${field_list[0]}" ) continue ;; 'nosuid') - [ ! -e "${bit_list[0]}" ] && continue - policy_nosuid_file_list+=( "${bit_list[0]}" ) + [ ! -e "${field_list[0]}" ] && continue + policy_nosuid_file_list+=( "${field_list[0]}" ) ;; *) - [ ! -e "${bit_list[0]}" ] && continue - add_to_policy "${bit_list[@]}" + [ ! -e "${field_list[0]}" ] && continue + add_to_policy "${field_list[@]}" ;; esac done < "${config_file}" @@ -661,7 +660,7 @@ undo_policy_for_file() { state_user_owner_item="${state_user_owner_list[state_idx]}" state_group_owner_item="${state_group_owner_list[state_idx]}" state_mode_item="${state_mode_list[state_idx]}" - chown ${verbose} "${state_user_owner_item}:${state_group_owner_item}" \ + chown ${verbose} -- "${state_user_owner_item}:${state_group_owner_item}" \ "${undo_file}" || exit_code=202 ## chmod needs to be run after chown since chown removes suid. chmod ${verbose} "${state_mode_item}" "${undo_file}" || exit_code=203 From 93ebf176c5f38bd268e5394e01421e46b9ae7dff Mon Sep 17 00:00:00 2001 From: Aaron Rainbolt Date: Thu, 2 Jan 2025 20:41:40 -0500 Subject: [PATCH 4/4] Make the main field count check in permission-hardener a bit more elegant --- usr/bin/permission-hardener | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/usr/bin/permission-hardener b/usr/bin/permission-hardener index 3ed62b3..11bdfb9 100755 --- a/usr/bin/permission-hardener +++ b/usr/bin/permission-hardener @@ -410,13 +410,14 @@ load_state() { IFS=' ' read -r -a field_list <<< "${line}" - if (( ${#field_list[@]} != 2 )) \ - && (( ${#field_list[@]} != 4 )) \ - && (( ${#field_list[@]} != 5 )); then - exit_code=200 - log error "Line contains an invalid number of fields: '${line}'" >&2 - exit "${exit_code}" - fi + case "${#field_list[@]}" in + 2|4|5) true;; + *) + exit_code=200 + log error "Line contains an invalid number of fields: '${line}'" >&2 + exit "${exit_code}" + ;; + esac # Strip trailing slash if appropriate field_list[0]="${field_list[0]%/}"