From 9c3566f524f748b9f7c98a36b3f2b1064cdba3ed Mon Sep 17 00:00:00 2001 From: Ben Grande Date: Mon, 22 Jul 2024 16:01:14 +0200 Subject: [PATCH 01/48] Delimit file names with null terminator --- usr/bin/permission-hardener | 55 ++++++++++++++++++------------------- 1 file changed, 26 insertions(+), 29 deletions(-) diff --git a/usr/bin/permission-hardener b/usr/bin/permission-hardener index 1d4c868..026e290 100755 --- a/usr/bin/permission-hardener +++ b/usr/bin/permission-hardener @@ -65,18 +65,18 @@ add_nosuid_statoverride_entry() { counter_actual=0 local dummy_line - while read -r dummy_line; do + while IFS="" read -r -d "" dummy_line; do true "DEBUG: test would evaluate parse" "${dummy_line}" should_be_counter=$((should_be_counter + 1)) done < <(find "${fso_to_process}" -perm /u=s,g=s -print0 | xargs -I{} -0 stat -c "%n %a %U %G" {}) local line - while read -r line; do + while IFS="" read -r -d "" line; do true "line: ${line}" counter_actual="$((counter_actual + 1))" local arr file_name existing_mode existing_owner existing_group - IFS=" " read -r -a arr <<< "${line}" + read -r -a arr <<< "${line}" file_name="${arr[0]}" existing_mode="${arr[1]}" existing_owner="${arr[2]}" @@ -153,7 +153,7 @@ add_nosuid_statoverride_entry() { local is_exact_whitelisted is_exact_whitelisted="" - for white_list_entry in ${exact_white_list}; do + for white_list_entry in "${exact_white_list[@]:-}"; do if test "${file_name}" = "${white_list_entry}"; then is_exact_whitelisted="true" ## Stop looping through the whitelist. @@ -163,7 +163,7 @@ add_nosuid_statoverride_entry() { local is_match_whitelisted is_match_whitelisted="" - for matchwhite_list_entry in ${match_white_list}; do + for matchwhite_list_entry in "${match_white_list[@]:-}"; do if echo "${file_name}" | grep --quiet --fixed-strings "${matchwhite_list_entry}"; then is_match_whitelisted="true" ## Stop looping through the match_white_list. @@ -173,7 +173,7 @@ add_nosuid_statoverride_entry() { local is_disable_whitelisted is_disable_whitelisted="" - for disablematch_list_entry in ${disable_white_list:-}; do + for disablematch_list_entry in "${disable_white_list[@]:-}"; do if echo "${file_name}" | grep --quiet --fixed-strings "${disablematch_list_entry}"; then is_disable_whitelisted="true" ## Stop looping through the disablewhitelist. @@ -234,10 +234,9 @@ add_nosuid_statoverride_entry() { ## Sanity test. if test ! "${should_be_counter}" = "${counter_actual}"; then - echo "INFO: fso_to_process: '${fso_to_process}' | counter_actual : '${counter_actual}'" - echo "INFO: fso_to_process: '${fso_to_process}' | should_be_counter: '${should_be_counter}'" + echo "INFO: file system object (parsed/wanted): '${fso_to_process}': (${counter_actual}/${should_be_counter})" + echo "ERROR: expected number of files to be parsed was not met." >&2 exit_code=202 - echo "ERROR: counter does not check out." >&2 fi } @@ -249,7 +248,7 @@ set_file_perms() { continue fi - if [[ "${line}" =~ ^# ]]; then + if [[ "${line}" =~ ^\s*# ]]; then continue fi @@ -291,23 +290,21 @@ set_file_perms() { local fso_without_trailing_slash fso_without_trailing_slash="${fso%/}" - if test "${mode_from_config}" = "disablewhitelist"; then - ## TODO: test/add white spaces inside file name support - disable_white_list+="${fso} " - continue - fi - - if test "${mode_from_config}" = "exactwhitelist"; then - ## TODO: test/add white spaces inside file name support - exact_white_list+="${fso} " - continue - fi - - if test "${mode_from_config}" = "matchwhitelist"; then - ## TODO: test/add white spaces inside file name support - match_white_list+="${fso} " - continue - fi + ## TODO: test/add white spaces inside file name support + 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 true "INFO: fso: '${fso}' - does not exist. This is likely normal." @@ -358,7 +355,7 @@ set_file_perms() { fi local arr file_name existing_mode existing_owner existing_group - IFS=" " read -r -a arr <<< "${stat_output}" + read -r -a arr <<< "${stat_output}" file_name="${arr[0]}" existing_mode="${arr[1]}" existing_owner="${arr[2]}" @@ -548,7 +545,7 @@ spare() { fi local line - while read -r line; do + while IFS="" read -r -d "" line; do ## example line: ## root root 4755 /usr/lib/eject/dmcrypt-get-device From 7ee1ea2cc7dd62feee3243d64b414130e68d35e9 Mon Sep 17 00:00:00 2001 From: Ben Grande Date: Mon, 22 Jul 2024 17:06:07 +0200 Subject: [PATCH 02/48] Unify functions that evaluate commands --- usr/bin/permission-hardener | 58 +++++++++++++++---------------------- 1 file changed, 24 insertions(+), 34 deletions(-) diff --git a/usr/bin/permission-hardener b/usr/bin/permission-hardener index 026e290..1011372 100755 --- a/usr/bin/permission-hardener +++ b/usr/bin/permission-hardener @@ -14,33 +14,23 @@ dpkg_admindir_parameter_existing_mode="--admindir ${store_dir}/existing_mode" dpkg_admindir_parameter_new_mode="--admindir ${store_dir}/new_mode" echo_wrapper_ignore() { - echo "INFO: run: $*" - "$@" 2>/dev/null || true -} - -echo_wrapper_silent_ignore() { - #echo "INFO: run: $@" + if test "${1}" = "verbose"; then + echo "INFO: run: $*" + fi + shift "$@" 2>/dev/null || true } echo_wrapper_audit() { - echo "INFO: run: $*" + if test "${1}" = "verbose"; then + echo "INFO: run: $*" + fi + shift return_code=0 "$@" || { return_code="$?" exit_code=203 - echo "ERROR: above command failed with exit code '${return_code}'! calling function name: '${FUNCNAME[1]}'" >&2 - } -} - -echo_wrapper_silent_audit() { - #echo "run (debugging): $@" - return_code=0 - "$@" || - { - return_code="$?" - exit_code=204 echo "ERROR: above command '$*' failed with exit code '${return_code}'! calling function name: '${FUNCNAME[1]}'" >&2 } } @@ -52,7 +42,7 @@ make_store_dir(){ } sanity_tests() { - echo_wrapper_silent_audit which \ + echo_wrapper_audit silent which \ capsh getcap setcap stat find dpkg-statoverride getent xargs grep 1>/dev/null } @@ -205,7 +195,7 @@ add_nosuid_statoverride_entry() { ## Save existing_mode in separate database. ## Not using --update as not intending to enforce existing_mode. # shellcheck disable=SC2086 - echo_wrapper_silent_audit dpkg-statoverride ${dpkg_admindir_parameter_existing_mode} --add "${existing_owner}" "${existing_group}" "${existing_mode}" "${file_name}" + 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. @@ -214,18 +204,18 @@ add_nosuid_statoverride_entry() { ## and re-add. ## Remove from real database. - echo_wrapper_silent_ignore dpkg-statoverride --remove "${file_name}" + echo_wrapper_ignore silent dpkg-statoverride --remove "${file_name}" ## Remove from separate database. # shellcheck disable=SC2086 - echo_wrapper_silent_ignore dpkg-statoverride ${dpkg_admindir_parameter_new_mode} --remove "${file_name}" + 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 dpkg-statoverride --add --update "${existing_owner}" "${existing_group}" "${new_mode}" "${file_name}" + 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_silent_audit dpkg-statoverride ${dpkg_admindir_parameter_new_mode} --add "${existing_owner}" "${existing_group}" "${new_mode}" "${file_name}" + echo_wrapper_audit silent dpkg-statoverride ${dpkg_admindir_parameter_new_mode} --add "${existing_owner}" "${existing_group}" "${new_mode}" "${file_name}" ## /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. @@ -414,20 +404,20 @@ set_file_perms() { ## Save existing_mode in separate database. ## Not using --update as not intending to enforce existing_mode. # shellcheck disable=SC2086 - echo_wrapper_silent_audit dpkg-statoverride ${dpkg_admindir_parameter_existing_mode} --add "${existing_owner}" "${existing_group}" "${existing_mode}" "${fso_without_trailing_slash}" + 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_silent_ignore dpkg-statoverride ${dpkg_admindir_parameter_new_mode} --remove "${fso_without_trailing_slash}" + 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_silent_ignore dpkg-statoverride --remove "${fso_without_trailing_slash}" - echo_wrapper_audit dpkg-statoverride --add --update "${owner_from_config}" "${group_from_config}" "${mode_from_config}" "${fso_without_trailing_slash}" + 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_silent_audit dpkg-statoverride ${dpkg_admindir_parameter_new_mode} --add "${owner_from_config}" "${group_from_config}" "${mode_from_config}" "${fso_without_trailing_slash}" + 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 true "There is no fso entry. Therefore add one." @@ -439,16 +429,16 @@ set_file_perms() { ## Save existing_mode in separate database. ## Not using --update as not intending to enforce existing_mode. # shellcheck disable=SC2086 - echo_wrapper_silent_audit dpkg-statoverride ${dpkg_admindir_parameter_existing_mode} --add "${existing_owner}" "${existing_group}" "${existing_mode}" "${fso_without_trailing_slash}" + 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 dpkg-statoverride --add --update "${owner_from_config}" "${group_from_config}" "${mode_from_config}" "${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_silent_audit dpkg-statoverride ${dpkg_admindir_parameter_new_mode} --add "${owner_from_config}" "${group_from_config}" "${mode_from_config}" "${fso_without_trailing_slash}" + 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 @@ -462,7 +452,7 @@ set_file_perms() { ## 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. - echo_wrapper_ignore setcap -r "${fso}" + echo_wrapper_ignore verbose setcap -r "${fso}" getcap_output="$(getcap "${fso}")" if test -n "${getcap_output}"; then exit_code=205 @@ -477,7 +467,7 @@ set_file_perms() { ## feature request: dpkg-statoverride: support for capabilities ## https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=502580 - echo_wrapper_audit setcap "${capability_from_config}+ep" "${fso}" + echo_wrapper_audit verbose setcap "${capability_from_config}+ep" "${fso}" fi done <"${config_file}" true "INFO: END parsing config_file: '${config_file}'" From 06fbcdac1de6f1830d911f05a4f7c14fd522fad4 Mon Sep 17 00:00:00 2001 From: Ben Grande Date: Tue, 23 Jul 2024 09:55:02 +0200 Subject: [PATCH 03/48] Prettify log messages --- usr/bin/permission-hardener | 107 +++++++++++++++++------------------- 1 file changed, 49 insertions(+), 58 deletions(-) diff --git a/usr/bin/permission-hardener b/usr/bin/permission-hardener index 1011372..6a78a28 100755 --- a/usr/bin/permission-hardener +++ b/usr/bin/permission-hardener @@ -31,7 +31,7 @@ echo_wrapper_audit() { { return_code="$?" exit_code=203 - echo "ERROR: above command '$*' failed with exit code '${return_code}'! calling function name: '${FUNCNAME[1]}'" >&2 + echo "ERROR: command '$*' failed with exit code '${return_code}'! calling function name: '${FUNCNAME[1]}'" >&2 } } @@ -56,13 +56,13 @@ add_nosuid_statoverride_entry() { local dummy_line while IFS="" read -r -d "" dummy_line; do - true "DEBUG: test would evaluate parse" "${dummy_line}" + true "DEBUG: test would parse line:" "${dummy_line}" should_be_counter=$((should_be_counter + 1)) done < <(find "${fso_to_process}" -perm /u=s,g=s -print0 | xargs -I{} -0 stat -c "%n %a %U %G" {}) local line while IFS="" read -r -d "" line; do - true "line: ${line}" + true "DEBUG: line: ${line}" counter_actual="$((counter_actual + 1))" local arr file_name existing_mode existing_owner existing_group @@ -73,23 +73,23 @@ add_nosuid_statoverride_entry() { existing_group="${arr[3]}" if test "${#arr[@]}" = 0; then - echo "ERROR: arr is empty. line: '${line}'" >&2 + echo "ERROR: line is empty: '${line}'" >&2 continue fi if test -z "${file_name}"; then - echo "ERROR: file_name is empty. line: '${line}'" >&2 + echo "ERROR: file name is empty. line: '${line}'" >&2 continue fi if test -z "${existing_mode}"; then - echo "ERROR: existing_mode is empty. line: '${line}'" >&2 + echo "ERROR: existing mode is empty. line: '${line}'" >&2 continue fi if test -z "${existing_owner}"; then - echo "ERROR: existing_owner is empty. line: '${line}'" >&2 + echo "ERROR: existing owner is empty. line: '${line}'" >&2 continue fi if test -z "${existing_group}"; then - echo "ERROR: existing_group is empty. line: '${line}'" >&2 + echo "ERROR: existing group is empty. line: '${line}'" >&2 continue fi @@ -99,12 +99,12 @@ add_nosuid_statoverride_entry() { if test -h "${file_name}"; then ## https://forums.whonix.org/t/disable-suid-binaries/7706/14 - true "skip symlink: ${file_name}" + true "DEBUG: skip symlink: ${file_name}" continue fi if test -d "${file_name}"; then - true "skip directory: ${file_name}" + true "DEBUG: skip directory: ${file_name}" continue fi @@ -171,26 +171,27 @@ add_nosuid_statoverride_entry() { fi done + clean_output="setuid=${setuid_output} setgid=${setsgid_output} existing_mode=${existing_mode} new_mode=${new_mode} file='${file_name}'" if test "${whitelists_disable_all:-}" = "true"; then - true "INFO: whitelists_disable_all=true - ${setuid_output} ${setsgid_output} found - file_name: '${file_name}' | existing_mode: '${existing_mode}'" + echo "INFO: whitelists_disable_all=true ${clean_output}" elif test "${is_disable_whitelisted}" = "true"; then - true "INFO: white list disabled - ${setuid_output} ${setsgid_output} found - file_name: '${file_name}' | existing_mode: '${existing_mode}'" + true "INFO: white list disabled ${clean_output}" else if test "${is_exact_whitelisted}" = "true"; then - true "INFO: SKIP whitelisted - ${setuid_output} ${setsgid_output} found - file_name: '${file_name}' | existing_mode: '${existing_mode}'" + true "INFO: is_exact_whitelisted=true ${clean_output}" continue fi if test "${is_match_whitelisted}" = "true"; then - true "INFO: SKIP matchwhitelisted - ${setuid_output} ${setsgid_output} found - file_name: '${file_name}' | existing_mode: '${existing_mode}' | matchwhite_list_entry: '${matchwhite_list_entry}'" + true "INFO: is_match_whitelisted=true ${clean_output} matchwhite_list_entry: '${matchwhite_list_entry}'" continue fi fi - echo "INFO: ${setuid_output} ${setsgid_output} found - file_name: '${file_name}' | existing_mode: '${existing_mode}' | new_mode: '${new_mode}'" + echo "INFO: ${clean_output}" # shellcheck disable=SC2086 if dpkg-statoverride ${dpkg_admindir_parameter_existing_mode} --list "${file_name}" >/dev/null; then - true "OK Existing mode already saved previously. Not saving again." + true "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. @@ -224,14 +225,14 @@ add_nosuid_statoverride_entry() { ## Sanity test. if test ! "${should_be_counter}" = "${counter_actual}"; then - echo "INFO: file system object (parsed/wanted): '${fso_to_process}': (${counter_actual}/${should_be_counter})" + echo "INFO: file (parsed/wanted): '${fso_to_process}': (${counter_actual}/${should_be_counter})" echo "ERROR: expected number of files to be parsed was not met." >&2 exit_code=202 fi } set_file_perms() { - true "INFO: START parsing config_file: '${config_file}'" + true "INFO: START parsing config file: '${config_file}'" local line while read -r line || test -n "${line}"; do if test -z "${line}"; then @@ -243,10 +244,10 @@ set_file_perms() { fi if [[ "${line}" =~ [0-9a-zA-Z/] ]]; then - true "OK line contains only white listed characters." + true "INFO: line contains only white listed characters." else exit_code=200 - echo "ERROR: cannot parse line with invalid character. line: '${line}'" >&2 + echo "ERROR: cannot parse line with invalid character in line: '${line}'" >&2 ## Safer to exit with error in this case. ## https://forums.whonix.org/t/disable-suid-binaries/7706/59 exit "${exit_code}" @@ -254,7 +255,7 @@ set_file_perms() { if test "${line}" = 'whitelists_disable_all=true'; then whitelists_disable_all=true - echo "INFO: whitelists_disable_all=true - all whitelists disabled." + echo "INFO: whitelists_disable_all=true" continue fi @@ -262,7 +263,7 @@ set_file_perms() { 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 - echo "ERROR: cannot parse. line: '${line}'" >&2 + echo "ERROR: cannot parse line: '${line}'" >&2 ## Debugging. du -hs /tmp || true echo "test -w /tmp: '$(test -w /tmp)'" >&2 || true @@ -297,7 +298,7 @@ set_file_perms() { esac if test ! -e "${fso}"; then - true "INFO: fso: '${fso}' - does not exist. This is likely normal." + true "INFO: file does not exist: '${fso}'" continue fi @@ -311,21 +312,21 @@ set_file_perms() { 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 - echo "ERROR: Mode '${mode_from_config}' is invalid!" >&2 + echo "ERROR: Invalid mode: '${mode_from_config}'" >&2 continue fi if test "${string_length_of_mode_from_config}" -lt "3"; then - echo "ERROR: Mode '${mode_from_config}' is invalid!" >&2 + echo "ERROR: Invalid mode: '${mode_from_config}'" >&2 continue fi if ! grep --quiet --fixed-strings "${owner_from_config}:" "${store_dir}/private/passwd"; then - echo "ERROR: owner_from_config '${owner_from_config}' does not exist!" >&2 + echo "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 - echo "ERROR: group_from_config '${group_from_config}' does not exist!" >&2 + echo "ERROR: group from config does not exist: '${group_from_config}'" >&2 continue fi @@ -340,7 +341,7 @@ set_file_perms() { local stat_output stat_output="" if ! stat_output="$(stat -c "%n %a %U %G" "${fso_without_trailing_slash}")"; then - echo "ERROR: failed to run 'stat' for fso_without_trailing_slash: '${fso_without_trailing_slash}'!" >&2 + echo "ERROR: failed to run 'stat' on file: '${fso_without_trailing_slash}'!" >&2 continue fi @@ -352,15 +353,15 @@ set_file_perms() { existing_group="${arr[3]}" if test "${#arr[@]}" = 0; then - echo "ERROR: arr is empty. stat_output: '${stat_output}' | line: '${line}'" >&2 + echo "ERROR: line is empty. Stat output: '${stat_output}', line: '${line}'" >&2 continue fi if test -z "${file_name}"; then - echo "ERROR: file_name is empty. stat_output: '${stat_output}' | line: '${line}'" >&2 + echo "ERROR: file name is empty. Stat output: '${stat_output}', line: '${line}'" >&2 continue fi if test -z "${existing_mode}"; then - echo "ERROR: existing_mode is empty. stat_output: '${stat_output}' | line: '${line}'" >&2 + echo "ERROR: existing mode is empty. Stat output: '${stat_output}', line: '${line}'" >&2 continue fi if test -z "${existing_owner}"; then @@ -368,7 +369,7 @@ set_file_perms() { continue fi if test -z "${existing_group}"; then - echo "ERROR: ${existing_group} is empty. stat_output: '${stat_output}' | line: '${line}'" >&2 + echo "ERROR: existing group is empty. Stat output: '${stat_output}', line: '${line}'" >&2 continue fi @@ -387,19 +388,19 @@ set_file_perms() { } if test "${dpkg_statoverride_list_exit_code}" = "0"; then - true "There is an fso entry. Check if owner/group/mode match." + true "INFO: There is an fso entry. Check if owner/group/mode matches." local grep_line grep_line="${owner_from_config} ${group_from_config} ${mode_for_grep} ${fso_without_trailing_slash}" if echo "${dpkg_statoverride_list_output}" | grep --quiet --fixed-strings "${grep_line}"; then - true "OK The owner/group/mode matches. No further action required." + true "INFO: The owner/group/mode matches. No further action required." else - true "The owner/group/mode do not match, therefore remove and re-add the entry to update it." + true "INFO: The owner/group/mode does not match, removing and re-adding the entry to update it." ## 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 - true "OK Existing mode already saved previously. No need to save again." + true "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. @@ -420,11 +421,11 @@ set_file_perms() { 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 - true "There is no fso entry. Therefore add one." + true "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 - true "OK Existing mode already saved previously. No need to save again." + true "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. @@ -456,12 +457,12 @@ set_file_perms() { getcap_output="$(getcap "${fso}")" if test -n "${getcap_output}"; then exit_code=205 - echo "ERROR: removing capabilities for fso '${fso}' failed!" >&2 + echo "ERROR: removing capabilities failed. File: '${fso}'" >&2 continue fi else if ! capsh --print | grep --fixed-strings "Bounding set" | grep --quiet "${capability_from_config}"; then - echo "ERROR: capability_from_config '${capability_from_config}' does not exist!" >&2 + echo "ERROR: capability from config does not exist: '${capability_from_config}'" >&2 continue fi @@ -470,7 +471,7 @@ set_file_perms() { echo_wrapper_audit verbose setcap "${capability_from_config}+ep" "${fso}" fi done <"${config_file}" - true "INFO: END parsing config_file: '${config_file}'" + true "INFO: END parsing config file: '${config_file}'" } parse_config_folder() { @@ -513,11 +514,8 @@ apply() { parse_config_folder echo "\ -INFO: To compare the current and previous permission modes: - Install 'meld' (or preferred diff tool) for comparison of file mode changes: +INFO: 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 - - Use 'meld' or another diff tool to view the differences: meld ${store_dir}/existing_mode/statoverride ${store_dir}/new_mode/statoverride" } @@ -545,7 +543,7 @@ spare() { echo "ERROR: cannot parse line: ${line}" >&2 continue fi - true "owner: '${owner}' group: '${group}' mode: '${mode}' file_name: '${file_name}'" + true "INFO: owner=${owner} group=${group} mode=${mode} file_name='${file_name}'" if test "${remove_file}" = "all"; then verbose="" @@ -573,7 +571,7 @@ spare() { # shellcheck disable=SC2086 chmod ${verbose} "${mode}" "${file_name}" || exit_code=203 else - echo "INFO: file_name: '${file_name}' - does not exist. This is likely normal." + echo "INFO: file doesn't exist: '${file_name}'" fi dpkg-statoverride --remove "${file_name}" &>/dev/null || true @@ -593,18 +591,11 @@ spare() { if test "$(cat "${store_dir}/remove_one")" = "false"; then echo "INFO: no file was removed. - File '${remove_file}' has not been removed from SUID Disabler and Permission Hardener during this invocation of this program. + File '${remove_file}' has not been removed from SUID Disabler and Permission Hardener during this invocation. This is expected if already done earlier. - Note: This is expected if already done earlier. - - Note: This program expects the full path to the file. Example: - $0 disable /usr/bin/newgrp - - The following syntax will not work: - $0 disable program-name - - The following example will not work: - $0 disable newgrp + 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 From aa99de68d307cd88462665424996d9b730ab5087 Mon Sep 17 00:00:00 2001 From: Ben Grande Date: Tue, 23 Jul 2024 18:46:47 +0200 Subject: [PATCH 04/48] Log output with defined levels --- usr/bin/permission-hardener | 140 ++++++++++++++++++++---------------- 1 file changed, 79 insertions(+), 61 deletions(-) diff --git a/usr/bin/permission-hardener b/usr/bin/permission-hardener index 6a78a28..9eff886 100755 --- a/usr/bin/permission-hardener +++ b/usr/bin/permission-hardener @@ -13,25 +13,33 @@ 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" +log_level=info +# shellcheck disable=SC1091 +source /usr/libexec/helper-scripts/log_run_die.sh + echo_wrapper_ignore() { if test "${1}" = "verbose"; then - echo "INFO: run: $*" + shift + log info "Run: $*" + else + shift fi - shift "$@" 2>/dev/null || true } echo_wrapper_audit() { if test "${1}" = "verbose"; then - echo "INFO: run: $*" + shift + log info "Run: $*" + else + shift fi - shift return_code=0 "$@" || { return_code="$?" exit_code=203 - echo "ERROR: command '$*' failed with exit code '${return_code}'! calling function name: '${FUNCNAME[1]}'" >&2 + log error "Command '$*' failed with exit code '${return_code}'! calling function name: '${FUNCNAME[1]}'" >&2 } } @@ -55,14 +63,13 @@ add_nosuid_statoverride_entry() { counter_actual=0 local dummy_line - while IFS="" read -r -d "" dummy_line; do - true "DEBUG: test would parse line:" "${dummy_line}" + while IFS="" read -r dummy_line; do + log info "Test would parse line: ${dummy_line}" should_be_counter=$((should_be_counter + 1)) done < <(find "${fso_to_process}" -perm /u=s,g=s -print0 | xargs -I{} -0 stat -c "%n %a %U %G" {}) local line - while IFS="" read -r -d "" line; do - true "DEBUG: line: ${line}" + while IFS="" read -r line; do counter_actual="$((counter_actual + 1))" local arr file_name existing_mode existing_owner existing_group @@ -73,23 +80,23 @@ add_nosuid_statoverride_entry() { existing_group="${arr[3]}" if test "${#arr[@]}" = 0; then - echo "ERROR: line is empty: '${line}'" >&2 + log error "Line is empty: '${line}'" >&2 continue fi if test -z "${file_name}"; then - echo "ERROR: file name is empty. line: '${line}'" >&2 + log error "File name is empty. line: '${line}'" >&2 continue fi if test -z "${existing_mode}"; then - echo "ERROR: existing mode is empty. line: '${line}'" >&2 + log error "Existing mode is empty. line: '${line}'" >&2 continue fi if test -z "${existing_owner}"; then - echo "ERROR: existing owner is empty. line: '${line}'" >&2 + log error "Existing owner is empty. line: '${line}'" >&2 continue fi if test -z "${existing_group}"; then - echo "ERROR: existing group is empty. line: '${line}'" >&2 + log error "Existing group is empty. line: '${line}'" >&2 continue fi @@ -99,12 +106,12 @@ add_nosuid_statoverride_entry() { if test -h "${file_name}"; then ## https://forums.whonix.org/t/disable-suid-binaries/7706/14 - true "DEBUG: skip symlink: ${file_name}" + log info "Skip symlink: ${file_name}" continue fi if test -d "${file_name}"; then - true "DEBUG: skip directory: ${file_name}" + log info "Skip directory: ${file_name}" continue fi @@ -144,6 +151,9 @@ add_nosuid_statoverride_entry() { local is_exact_whitelisted is_exact_whitelisted="" for white_list_entry in "${exact_white_list[@]:-}"; do + if test -z "${white_list_entry}"; then + continue + fi if test "${file_name}" = "${white_list_entry}"; then is_exact_whitelisted="true" ## Stop looping through the whitelist. @@ -154,6 +164,9 @@ add_nosuid_statoverride_entry() { local is_match_whitelisted is_match_whitelisted="" for matchwhite_list_entry in "${match_white_list[@]:-}"; do + if test -z "${matchwhite_list_entry}"; then + continue + fi if echo "${file_name}" | grep --quiet --fixed-strings "${matchwhite_list_entry}"; then is_match_whitelisted="true" ## Stop looping through the match_white_list. @@ -164,6 +177,9 @@ add_nosuid_statoverride_entry() { local is_disable_whitelisted is_disable_whitelisted="" for disablematch_list_entry in "${disable_white_list[@]:-}"; do + if test -z "${disablematch_list_entry}"; then + continue + fi if echo "${file_name}" | grep --quiet --fixed-strings "${disablematch_list_entry}"; then is_disable_whitelisted="true" ## Stop looping through the disablewhitelist. @@ -171,27 +187,26 @@ add_nosuid_statoverride_entry() { fi done + clean_output_prefix="Managing (S|G)UID of line:" clean_output="setuid=${setuid_output} setgid=${setsgid_output} existing_mode=${existing_mode} new_mode=${new_mode} file='${file_name}'" if test "${whitelists_disable_all:-}" = "true"; then - echo "INFO: whitelists_disable_all=true ${clean_output}" + log info "${clean_output_prefix} whitelists_disable_all=true ${clean_output}" elif test "${is_disable_whitelisted}" = "true"; then - true "INFO: white list disabled ${clean_output}" + log info "${clean_output_prefix} is_disable_whitelisted=true ${clean_output}" else if test "${is_exact_whitelisted}" = "true"; then - true "INFO: is_exact_whitelisted=true ${clean_output}" + log info "${clean_output_prefix} is_exact_whitelisted=true ${clean_output}" continue fi if test "${is_match_whitelisted}" = "true"; then - true "INFO: is_match_whitelisted=true ${clean_output} matchwhite_list_entry: '${matchwhite_list_entry}'" + log info "${clean_output_prefix} is_match_whitelisted=true matchwhite_list_entry=${matchwhite_list_entry} ${clean_output}" continue fi fi - echo "INFO: ${clean_output}" - # shellcheck disable=SC2086 if dpkg-statoverride ${dpkg_admindir_parameter_existing_mode} --list "${file_name}" >/dev/null; then - true "INFO: Existing mode already saved previously. Not saving again." + 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. @@ -225,14 +240,14 @@ add_nosuid_statoverride_entry() { ## Sanity test. if test ! "${should_be_counter}" = "${counter_actual}"; then - echo "INFO: file (parsed/wanted): '${fso_to_process}': (${counter_actual}/${should_be_counter})" - echo "ERROR: expected number of files to be parsed was not met." >&2 + 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() { - true "INFO: START parsing config file: '${config_file}'" + log info "START parsing config file: ${config_file}" local line while read -r line || test -n "${line}"; do if test -z "${line}"; then @@ -243,11 +258,9 @@ set_file_perms() { continue fi - if [[ "${line}" =~ [0-9a-zA-Z/] ]]; then - true "INFO: line contains only white listed characters." - else + if ! [[ "${line}" =~ [0-9a-zA-Z/] ]]; then exit_code=200 - echo "ERROR: cannot parse line with invalid character in line: '${line}'" >&2 + 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}" @@ -255,7 +268,7 @@ set_file_perms() { if test "${line}" = 'whitelists_disable_all=true'; then whitelists_disable_all=true - echo "INFO: whitelists_disable_all=true" + log info "whitelists_disable_all=true" continue fi @@ -263,7 +276,7 @@ set_file_perms() { 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 - echo "ERROR: cannot parse line: '${line}'" >&2 + log error "Cannot parse line: '${line}'" >&2 ## Debugging. du -hs /tmp || true echo "test -w /tmp: '$(test -w /tmp)'" >&2 || true @@ -272,6 +285,8 @@ set_file_perms() { 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. #echo "line: '${line}'" #echo "fso: '${fso}'" @@ -282,6 +297,7 @@ set_file_perms() { fso_without_trailing_slash="${fso%/}" ## TODO: test/add white spaces inside file name support + declare -g disable_white_list exact_white_list match_white_list case "${mode_from_config}" in disablewhitelist) disable_white_list+=("${fso}") @@ -298,7 +314,7 @@ set_file_perms() { esac if test ! -e "${fso}"; then - true "INFO: file does not exist: '${fso}'" + log warn "File does not exist: '${fso}'" continue fi @@ -312,21 +328,21 @@ set_file_perms() { 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 - echo "ERROR: Invalid mode: '${mode_from_config}'" >&2 + log error "Invalid mode: '${mode_from_config}'" >&2 continue fi if test "${string_length_of_mode_from_config}" -lt "3"; then - echo "ERROR: Invalid mode: '${mode_from_config}'" >&2 + log error "Invalid mode: '${mode_from_config}'" >&2 continue fi if ! grep --quiet --fixed-strings "${owner_from_config}:" "${store_dir}/private/passwd"; then - echo "ERROR: owner from config does not exist: '${owner_from_config}'" >&2 + 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 - echo "ERROR: group from config does not exist: '${group_from_config}'" >&2 + log error "Group from config does not exist: '${group_from_config}'" >&2 continue fi @@ -341,7 +357,7 @@ set_file_perms() { local stat_output stat_output="" if ! stat_output="$(stat -c "%n %a %U %G" "${fso_without_trailing_slash}")"; then - echo "ERROR: failed to run 'stat' on file: '${fso_without_trailing_slash}'!" >&2 + log error "Failed to run 'stat' on file: '${fso_without_trailing_slash}'!" >&2 continue fi @@ -353,23 +369,23 @@ set_file_perms() { existing_group="${arr[3]}" if test "${#arr[@]}" = 0; then - echo "ERROR: line is empty. Stat output: '${stat_output}', line: '${line}'" >&2 + log error "Line is empty. Stat output: '${stat_output}', line: '${line}'" >&2 continue fi if test -z "${file_name}"; then - echo "ERROR: file name is empty. Stat output: '${stat_output}', line: '${line}'" >&2 + log error "File name is empty. Stat output: '${stat_output}', line: '${line}'" >&2 continue fi if test -z "${existing_mode}"; then - echo "ERROR: existing mode is empty. Stat output: '${stat_output}', line: '${line}'" >&2 + log error "Existing mode is empty. Stat output: '${stat_output}', line: '${line}'" >&2 continue fi if test -z "${existing_owner}"; then - echo "ERROR: existing_owner is empty. stat_output: '${stat_output}' | line: '${line}'" >&2 + log error "Existing_owner is empty. stat_output: '${stat_output}' | line: '${line}'" >&2 continue fi if test -z "${existing_group}"; then - echo "ERROR: existing group is empty. Stat output: '${stat_output}', line: '${line}'" >&2 + log error "Existing group is empty. Stat output: '${stat_output}', line: '${line}'" >&2 continue fi @@ -388,19 +404,18 @@ set_file_perms() { } if test "${dpkg_statoverride_list_exit_code}" = "0"; then - true "INFO: There is an fso entry. Check if owner/group/mode matches." local grep_line grep_line="${owner_from_config} ${group_from_config} ${mode_for_grep} ${fso_without_trailing_slash}" if echo "${dpkg_statoverride_list_output}" | grep --quiet --fixed-strings "${grep_line}"; then - true "INFO: The owner/group/mode matches. No further action required." + log info "The owner/group/mode matches fso entry. No further action required." else - true "INFO: The owner/group/mode does not match, removing and re-adding the entry to update it." + 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 - true "INFO: Existing mode already saved previously. Not saving again." + 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. @@ -421,11 +436,11 @@ set_file_perms() { 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 - true "INFO: There is no fso entry, adding one." + 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 - true "INFO: Existing mode already saved previously. Not saving again." + 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. @@ -457,12 +472,12 @@ set_file_perms() { getcap_output="$(getcap "${fso}")" if test -n "${getcap_output}"; then exit_code=205 - echo "ERROR: removing capabilities failed. File: '${fso}'" >&2 + 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 - echo "ERROR: capability from config does not exist: '${capability_from_config}'" >&2 + log error "Capability from config does not exist: '${capability_from_config}'" >&2 continue fi @@ -470,8 +485,9 @@ set_file_perms() { ## https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=502580 echo_wrapper_audit verbose setcap "${capability_from_config}+ep" "${fso}" fi + done <"${config_file}" - true "INFO: END parsing config file: '${config_file}'" + log info "END parsing config file: ${config_file}" } parse_config_folder() { @@ -504,6 +520,7 @@ parse_config_folder() { /usr/local/etc/permission-hardening.d/*.conf do set_file_perms + done } @@ -513,8 +530,8 @@ apply() { sanity_tests parse_config_folder - echo "\ -INFO: To compare the current and previous permission modes, install 'meld' (or preferred diff tool) for comparison of file mode changes: + log info "\ +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" } @@ -529,21 +546,22 @@ spare() { 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 not applied before" return 0 fi local line - while IFS="" read -r -d "" line; do + 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 - echo "ERROR: cannot parse line: ${line}" >&2 + log error "Cannot parse line: ${line}" >&2 continue fi - true "INFO: owner=${owner} group=${group} mode=${mode} file_name='${file_name}'" + log info "Parsing line: owner=${owner} group=${group} mode=${mode} file_name='${file_name}'" if test "${remove_file}" = "all"; then verbose="" @@ -571,7 +589,7 @@ spare() { # shellcheck disable=SC2086 chmod ${verbose} "${mode}" "${file_name}" || exit_code=203 else - echo "INFO: file doesn't exist: '${file_name}'" + log warn "File does not exist: '${file_name}'" fi dpkg-statoverride --remove "${file_name}" &>/dev/null || true @@ -589,7 +607,7 @@ spare() { if test ! "${remove_file}" = "all"; then if test "$(cat "${store_dir}/remove_one")" = "false"; then - echo "INFO: no file was removed. + 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. @@ -617,7 +635,7 @@ spare() { check_root(){ if test "$(id -u)" != "0"; then - echo "ERROR: Not running as root, aborting." + log error "Not running as root, aborting." exit 1 fi } @@ -647,7 +665,7 @@ case "${1:-}" in esac if test "${exit_code}" != "0"; then - echo "ERROR: Exiting with non-zero exit code: '${exit_code}'" >&2 + log error "Exiting with non-zero exit code: '${exit_code}'" >&2 fi exit "${exit_code}" From 8be21b6eff40fdd3909ef63468463fc52e8bf45f Mon Sep 17 00:00:00 2001 From: Ben Grande Date: Tue, 23 Jul 2024 19:36:12 +0200 Subject: [PATCH 05/48] Handle newlines in file names --- usr/bin/permission-hardener | 45 +++++++++++++++++++++---------------- 1 file changed, 26 insertions(+), 19 deletions(-) diff --git a/usr/bin/permission-hardener b/usr/bin/permission-hardener index 9eff886..87f4307 100755 --- a/usr/bin/permission-hardener +++ b/usr/bin/permission-hardener @@ -63,40 +63,47 @@ add_nosuid_statoverride_entry() { counter_actual=0 local dummy_line - while IFS="" read -r dummy_line; do + while IFS="" read -r -d "" dummy_line; do log info "Test would parse line: ${dummy_line}" should_be_counter=$((should_be_counter + 1)) - done < <(find "${fso_to_process}" -perm /u=s,g=s -print0 | xargs -I{} -0 stat -c "%n %a %U %G" {}) + done < <(find "${fso_to_process}" -perm /u=s,g=s -print0) local line - while IFS="" read -r line; do + while IFS="" read -r -d "" line; do counter_actual="$((counter_actual + 1))" local arr file_name existing_mode existing_owner existing_group - read -r -a arr <<< "${line}" - file_name="${arr[0]}" - existing_mode="${arr[1]}" - existing_owner="${arr[2]}" - existing_group="${arr[3]}" + file_name="${line}" + stat_output="$(stat -c "%a %U %G" "${line}")" + read -r -a arr <<< "${stat_output}" + existing_mode="${arr[0]}" + existing_owner="${arr[1]}" + existing_group="${arr[2]}" if test "${#arr[@]}" = 0; then log error "Line is empty: '${line}'" >&2 continue fi if test -z "${file_name}"; then - log error "File name is empty. line: '${line}'" >&2 + log error "File name is empty in line: ${line}" >&2 continue fi if test -z "${existing_mode}"; then - log error "Existing mode is empty. line: '${line}'" >&2 + log error "Existing mode is empty in line: ${line}" >&2 continue fi if test -z "${existing_owner}"; then - log error "Existing owner is empty. line: '${line}'" >&2 + log error "Existing owner is empty in line: ${line}" >&2 continue fi if test -z "${existing_group}"; then - log error "Existing group is empty. line: '${line}'" >&2 + log error "Existing group is empty in line: ${line}" >&2 + continue + fi + + ## dpkg-statoverride: error: path may not contain newlines + if [[ "${file_name}" == *$'\n'* ]]; then + log warn "Skipping file name that contains newlines: ${file_name}" >&2 continue fi @@ -236,7 +243,7 @@ add_nosuid_statoverride_entry() { ## /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 - done < <(find "${fso_to_process}" -perm /u=s,g=s -print0 | xargs -I{} -0 stat -c "%n %a %U %G" {}) + done < <(find "${fso_to_process}" -perm /u=s,g=s -print0) ## Sanity test. if test ! "${should_be_counter}" = "${counter_actual}"; then @@ -356,17 +363,17 @@ set_file_perms() { local stat_output stat_output="" - if ! stat_output="$(stat -c "%n %a %U %G" "${fso_without_trailing_slash}")"; then + if ! stat_output="$(stat -c "%a %U %G" "${fso_without_trailing_slash}")"; then log error "Failed to run 'stat' on file: '${fso_without_trailing_slash}'!" >&2 continue fi local arr file_name existing_mode existing_owner existing_group read -r -a arr <<< "${stat_output}" - file_name="${arr[0]}" - existing_mode="${arr[1]}" - existing_owner="${arr[2]}" - existing_group="${arr[3]}" + file_name="${fso_without_trailing_slash}" + existing_mode="${arr[0]}" + existing_owner="${arr[1]}" + existing_group="${arr[2]}" if test "${#arr[@]}" = 0; then log error "Line is empty. Stat output: '${stat_output}', line: '${line}'" >&2 @@ -589,7 +596,7 @@ spare() { # shellcheck disable=SC2086 chmod ${verbose} "${mode}" "${file_name}" || exit_code=203 else - log warn "File does not exist: '${file_name}'" + log warn "File does not exist: ${file_name}" fi dpkg-statoverride --remove "${file_name}" &>/dev/null || true From 7200e9bd8c793f5ea30c3448fd03fbd38c6292b5 Mon Sep 17 00:00:00 2001 From: Patrick Schleizer Date: Wed, 24 Jul 2024 09:15:02 -0400 Subject: [PATCH 06/48] output --- usr/bin/permission-hardener | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/usr/bin/permission-hardener b/usr/bin/permission-hardener index 87f4307..2800e73 100755 --- a/usr/bin/permission-hardener +++ b/usr/bin/permission-hardener @@ -553,7 +553,7 @@ spare() { 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 not applied before" + true "DEBUG: Stat file does not exist, hardening was not applied before." return 0 fi From a077ae54ea050af8828813b781738cba24e27624 Mon Sep 17 00:00:00 2001 From: Patrick Schleizer Date: Wed, 24 Jul 2024 10:56:08 -0400 Subject: [PATCH 07/48] modify call of stat to use NUL delimiter for more robust string parsing --- usr/bin/permission-hardener | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/usr/bin/permission-hardener b/usr/bin/permission-hardener index 2800e73..36c21f1 100755 --- a/usr/bin/permission-hardener +++ b/usr/bin/permission-hardener @@ -74,11 +74,14 @@ add_nosuid_statoverride_entry() { local arr file_name existing_mode existing_owner existing_group file_name="${line}" - stat_output="$(stat -c "%a %U %G" "${line}")" - read -r -a arr <<< "${stat_output}" - existing_mode="${arr[0]}" - existing_owner="${arr[1]}" - existing_group="${arr[2]}" + ## Capture the stat output with fields separated by NUL characters. + ## Delimiter at the end to avoid the last field to be interpreted as having a newline. + stat_output=$(stat -c '%n\0%a\0%U\0%G\0%' "${line}") + readarray -d '\0' -t arr <<< "${stat_output}" + file_name_from_stat="${arr[0]}" + existing_mode="${arr[1]}" + existing_owner="${arr[2]}" + existing_group="${arr[3]}" if test "${#arr[@]}" = 0; then log error "Line is empty: '${line}'" >&2 @@ -363,7 +366,7 @@ set_file_perms() { local stat_output stat_output="" - if ! stat_output="$(stat -c "%a %U %G" "${fso_without_trailing_slash}")"; then + if ! stat_output="$(stat -c '%n\0%a\0%U\0%G\0%' "${fso_without_trailing_slash}")"; then log error "Failed to run 'stat' on file: '${fso_without_trailing_slash}'!" >&2 continue fi From 1cbda7998196dc04e83c48526d15f9ad5f11e6c9 Mon Sep 17 00:00:00 2001 From: Patrick Schleizer Date: Wed, 24 Jul 2024 10:57:13 -0400 Subject: [PATCH 08/48] check first if array is empty before parsing further --- usr/bin/permission-hardener | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/usr/bin/permission-hardener b/usr/bin/permission-hardener index 36c21f1..63786d3 100755 --- a/usr/bin/permission-hardener +++ b/usr/bin/permission-hardener @@ -77,16 +77,19 @@ add_nosuid_statoverride_entry() { ## Capture the stat output with fields separated by NUL characters. ## Delimiter at the end to avoid the last field to be interpreted as having a newline. stat_output=$(stat -c '%n\0%a\0%U\0%G\0%' "${line}") + readarray -d '\0' -t arr <<< "${stat_output}" - file_name_from_stat="${arr[0]}" - existing_mode="${arr[1]}" - existing_owner="${arr[2]}" - existing_group="${arr[3]}" if test "${#arr[@]}" = 0; then log error "Line is empty: '${line}'" >&2 continue fi + + file_name_from_stat="${arr[0]}" + existing_mode="${arr[1]}" + existing_owner="${arr[2]}" + existing_group="${arr[3]}" + if test -z "${file_name}"; then log error "File name is empty in line: ${line}" >&2 continue From b9dfe70a016e46e1f275918be19890526182cfa2 Mon Sep 17 00:00:00 2001 From: Patrick Schleizer Date: Wed, 24 Jul 2024 10:58:05 -0400 Subject: [PATCH 09/48] check first if file_name is empty --- usr/bin/permission-hardener | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/usr/bin/permission-hardener b/usr/bin/permission-hardener index 63786d3..c2c6129 100755 --- a/usr/bin/permission-hardener +++ b/usr/bin/permission-hardener @@ -73,7 +73,14 @@ add_nosuid_statoverride_entry() { counter_actual="$((counter_actual + 1))" local arr file_name existing_mode existing_owner existing_group + file_name="${line}" + + if test -z "${file_name}"; then + log error "File name is empty in line: ${line}" >&2 + continue + fi + ## Capture the stat output with fields separated by NUL characters. ## Delimiter at the end to avoid the last field to be interpreted as having a newline. stat_output=$(stat -c '%n\0%a\0%U\0%G\0%' "${line}") @@ -90,10 +97,6 @@ add_nosuid_statoverride_entry() { existing_owner="${arr[2]}" existing_group="${arr[3]}" - if test -z "${file_name}"; then - log error "File name is empty in line: ${line}" >&2 - continue - fi if test -z "${existing_mode}"; then log error "Existing mode is empty in line: ${line}" >&2 continue From ced02fb9e03e12c7d51923511e7d6a54b09a6274 Mon Sep 17 00:00:00 2001 From: Patrick Schleizer Date: Wed, 24 Jul 2024 11:01:24 -0400 Subject: [PATCH 10/48] add sanity test for file_name output from stat --- usr/bin/permission-hardener | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/usr/bin/permission-hardener b/usr/bin/permission-hardener index c2c6129..17d0abe 100755 --- a/usr/bin/permission-hardener +++ b/usr/bin/permission-hardener @@ -97,6 +97,15 @@ add_nosuid_statoverride_entry() { existing_owner="${arr[2]}" existing_group="${arr[3]}" + if [ ! "$file_name" = "$file_name_from_stat" ]; then + log error "\ +file_name is different from file_name_from_stat: +line: '${line}' +file_name '${file_name}' +file_name_from_stat: '${file_name_from_stat}'" >&2 + continue + fi + if test -z "${existing_mode}"; then log error "Existing mode is empty in line: ${line}" >&2 continue From a6e517736b83c124cf8cec52bac184612a29ad0d Mon Sep 17 00:00:00 2001 From: Patrick Schleizer Date: Wed, 24 Jul 2024 11:02:25 -0400 Subject: [PATCH 11/48] local stat_output --- usr/bin/permission-hardener | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/usr/bin/permission-hardener b/usr/bin/permission-hardener index 17d0abe..571d176 100755 --- a/usr/bin/permission-hardener +++ b/usr/bin/permission-hardener @@ -72,7 +72,7 @@ add_nosuid_statoverride_entry() { while IFS="" read -r -d "" line; do counter_actual="$((counter_actual + 1))" - local arr file_name existing_mode existing_owner existing_group + local arr file_name existing_mode existing_owner existing_group stat_output file_name="${line}" From d5366835112cc5fabef7ec46a9c582c08121cb14 Mon Sep 17 00:00:00 2001 From: Patrick Schleizer Date: Wed, 24 Jul 2024 11:03:28 -0400 Subject: [PATCH 12/48] local clean_output_prefix clean_output --- usr/bin/permission-hardener | 1 + 1 file changed, 1 insertion(+) diff --git a/usr/bin/permission-hardener b/usr/bin/permission-hardener index 571d176..978c6a9 100755 --- a/usr/bin/permission-hardener +++ b/usr/bin/permission-hardener @@ -212,6 +212,7 @@ file_name_from_stat: '${file_name_from_stat}'" >&2 fi done + local clean_output_prefix clean_output clean_output_prefix="Managing (S|G)UID of line:" clean_output="setuid=${setuid_output} setgid=${setsgid_output} existing_mode=${existing_mode} new_mode=${new_mode} file='${file_name}'" if test "${whitelists_disable_all:-}" = "true"; then From 00911df5c1de24960ad6d21b4cd99450f2d08a88 Mon Sep 17 00:00:00 2001 From: Patrick Schleizer Date: Wed, 24 Jul 2024 11:10:56 -0400 Subject: [PATCH 13/48] modify call of stat to use NUL delimiter for more robust string parsing --- usr/bin/permission-hardener | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/usr/bin/permission-hardener b/usr/bin/permission-hardener index 978c6a9..ed42723 100755 --- a/usr/bin/permission-hardener +++ b/usr/bin/permission-hardener @@ -72,7 +72,7 @@ add_nosuid_statoverride_entry() { while IFS="" read -r -d "" line; do counter_actual="$((counter_actual + 1))" - local arr file_name existing_mode existing_owner existing_group stat_output + local arr file_name file_name_from_stat existing_mode existing_owner existing_group stat_output file_name="${line}" @@ -88,7 +88,7 @@ add_nosuid_statoverride_entry() { readarray -d '\0' -t arr <<< "${stat_output}" if test "${#arr[@]}" = 0; then - log error "Line is empty: '${line}'" >&2 + log error "Line is empty. Stat output: '${stat_output}', line: '${line}'" >&2 continue fi @@ -387,12 +387,19 @@ set_file_perms() { continue fi - local arr file_name existing_mode existing_owner existing_group - read -r -a arr <<< "${stat_output}" + local arr file_name file_name_from_stat existing_mode existing_owner existing_group + readarray -d '\0' -t arr <<< "${stat_output}" file_name="${fso_without_trailing_slash}" - existing_mode="${arr[0]}" - existing_owner="${arr[1]}" - existing_group="${arr[2]}" + + if test "${#arr[@]}" = 0; then + log error "Line is empty: '${line}'" >&2 + continue + fi + + file_name_from_stat="${arr[0]}" + existing_mode="${arr[1]}" + existing_owner="${arr[2]}" + existing_group="${arr[3]}" if test "${#arr[@]}" = 0; then log error "Line is empty. Stat output: '${stat_output}', line: '${line}'" >&2 From 9712b5b4e3cff3eac8ef03b5e562ff89d74ef4b8 Mon Sep 17 00:00:00 2001 From: Patrick Schleizer Date: Wed, 24 Jul 2024 11:12:18 -0400 Subject: [PATCH 14/48] output --- usr/bin/permission-hardener | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/usr/bin/permission-hardener b/usr/bin/permission-hardener index ed42723..0ca91ec 100755 --- a/usr/bin/permission-hardener +++ b/usr/bin/permission-hardener @@ -392,7 +392,7 @@ set_file_perms() { file_name="${fso_without_trailing_slash}" if test "${#arr[@]}" = 0; then - log error "Line is empty: '${line}'" >&2 + log error "Line is empty. Stat output: '${stat_output}', line: '${line}'" >&2 continue fi @@ -414,7 +414,7 @@ set_file_perms() { continue 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: '${line}'" >&2 continue fi if test -z "${existing_group}"; then From 721392901be384014298f59deb57747b825c8b37 Mon Sep 17 00:00:00 2001 From: Patrick Schleizer Date: Wed, 24 Jul 2024 11:12:39 -0400 Subject: [PATCH 15/48] remove duplicate test --- usr/bin/permission-hardener | 4 ---- 1 file changed, 4 deletions(-) diff --git a/usr/bin/permission-hardener b/usr/bin/permission-hardener index 0ca91ec..933cb55 100755 --- a/usr/bin/permission-hardener +++ b/usr/bin/permission-hardener @@ -401,10 +401,6 @@ set_file_perms() { existing_owner="${arr[2]}" existing_group="${arr[3]}" - if test "${#arr[@]}" = 0; then - log error "Line is empty. Stat output: '${stat_output}', line: '${line}'" >&2 - continue - fi if test -z "${file_name}"; then log error "File name is empty. Stat output: '${stat_output}', line: '${line}'" >&2 continue From c9fd2ceb61ea176c731432f02a9fa40652fbddc8 Mon Sep 17 00:00:00 2001 From: Patrick Schleizer Date: Wed, 24 Jul 2024 11:13:35 -0400 Subject: [PATCH 16/48] downgrade warning of non-existing files to info to avoid all users by default getting a warning for expected non-existing files --- usr/bin/permission-hardener | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/usr/bin/permission-hardener b/usr/bin/permission-hardener index 933cb55..4689973 100755 --- a/usr/bin/permission-hardener +++ b/usr/bin/permission-hardener @@ -618,7 +618,7 @@ spare() { # shellcheck disable=SC2086 chmod ${verbose} "${mode}" "${file_name}" || exit_code=203 else - log warn "File does not exist: ${file_name}" + log info "File does not exist: ${file_name}" fi dpkg-statoverride --remove "${file_name}" &>/dev/null || true From 151ca659a9f5565744ff57f3b581c8c051def148 Mon Sep 17 00:00:00 2001 From: Patrick Schleizer Date: Wed, 24 Jul 2024 11:19:15 -0400 Subject: [PATCH 17/48] output --- usr/bin/permission-hardener | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/usr/bin/permission-hardener b/usr/bin/permission-hardener index 4689973..3e43dbf 100755 --- a/usr/bin/permission-hardener +++ b/usr/bin/permission-hardener @@ -64,7 +64,7 @@ add_nosuid_statoverride_entry() { local dummy_line while IFS="" read -r -d "" dummy_line; do - log info "Test would parse line: ${dummy_line}" + log info "Test would parse line: '${dummy_line}'" should_be_counter=$((should_be_counter + 1)) done < <(find "${fso_to_process}" -perm /u=s,g=s -print0) @@ -77,7 +77,7 @@ add_nosuid_statoverride_entry() { file_name="${line}" if test -z "${file_name}"; then - log error "File name is empty in line: ${line}" >&2 + log error "File name is empty in line: '${line}'" >&2 continue fi @@ -107,21 +107,21 @@ file_name_from_stat: '${file_name_from_stat}'" >&2 fi if test -z "${existing_mode}"; then - log error "Existing mode is empty in line: ${line}" >&2 + log error "Existing mode is empty in line: '${line}'" >&2 continue fi if test -z "${existing_owner}"; then - log error "Existing owner is empty in line: ${line}" >&2 + log error "Existing owner is empty in line: '${line}'" >&2 continue fi if test -z "${existing_group}"; then - log error "Existing group is empty in line: ${line}" >&2 + log error "Existing group is empty in line: '${line}'" >&2 continue fi ## dpkg-statoverride: error: path may not contain newlines if [[ "${file_name}" == *$'\n'* ]]; then - log warn "Skipping file name that contains newlines: ${file_name}" >&2 + log warn "Skipping file name that contains newlines: '${file_name}'" >&2 continue fi @@ -131,12 +131,12 @@ file_name_from_stat: '${file_name_from_stat}'" >&2 if test -h "${file_name}"; then ## https://forums.whonix.org/t/disable-suid-binaries/7706/14 - log info "Skip symlink: ${file_name}" + log info "Skip symlink: '${file_name}'" continue fi if test -d "${file_name}"; then - log info "Skip directory: ${file_name}" + log info "Skip directory: '${file_name}'" continue fi @@ -214,7 +214,7 @@ file_name_from_stat: '${file_name_from_stat}'" >&2 local clean_output_prefix clean_output clean_output_prefix="Managing (S|G)UID of line:" - clean_output="setuid=${setuid_output} setgid=${setsgid_output} existing_mode=${existing_mode} new_mode=${new_mode} file='${file_name}'" + clean_output="setuid='${setuid_output}' setgid='${setsgid_output}' 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 @@ -225,7 +225,7 @@ file_name_from_stat: '${file_name_from_stat}'" >&2 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}" + log info "${clean_output_prefix} is_match_whitelisted=true matchwhite_list_entry='${matchwhite_list_entry}' ${clean_output}" continue fi fi @@ -273,7 +273,7 @@ file_name_from_stat: '${file_name_from_stat}'" >&2 } set_file_perms() { - log info "START parsing config file: ${config_file}" + log info "START parsing config file: '${config_file}'" local line while read -r line || test -n "${line}"; do if test -z "${line}"; then @@ -286,7 +286,7 @@ set_file_perms() { if ! [[ "${line}" =~ [0-9a-zA-Z/] ]]; then exit_code=200 - log error "Line contains invalid characters: ${line}" >&2 + 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}" @@ -311,7 +311,7 @@ set_file_perms() { 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}" + 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. #echo "line: '${line}'" @@ -516,7 +516,7 @@ set_file_perms() { fi done <"${config_file}" - log info "END parsing config file: ${config_file}" + log info "END parsing config file: '${config_file}'" } parse_config_folder() { @@ -587,10 +587,10 @@ spare() { 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 + log error "Cannot parse line: '${line}'" >&2 continue fi - log info "Parsing line: owner=${owner} group=${group} mode=${mode} file_name='${file_name}'" + log info "Parsing line: owner='${owner}' group='${group}' mode='${mode}' file_name='${file_name}'" if test "${remove_file}" = "all"; then verbose="" @@ -618,7 +618,7 @@ spare() { # shellcheck disable=SC2086 chmod ${verbose} "${mode}" "${file_name}" || exit_code=203 else - log info "File does not exist: ${file_name}" + log info "File does not exist: '${file_name}'" fi dpkg-statoverride --remove "${file_name}" &>/dev/null || true From 3bf1f26c0bb271d63c16b314e4da040abf5b3713 Mon Sep 17 00:00:00 2001 From: Patrick Schleizer Date: Wed, 24 Jul 2024 11:20:26 -0400 Subject: [PATCH 18/48] downgrade warning of non-existing folders to info to avoid all users by default getting a warning for expected non-existing folders --- usr/bin/permission-hardener | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/usr/bin/permission-hardener b/usr/bin/permission-hardener index 3e43dbf..a71e9e6 100755 --- a/usr/bin/permission-hardener +++ b/usr/bin/permission-hardener @@ -340,7 +340,7 @@ set_file_perms() { esac if test ! -e "${fso}"; then - log warn "File does not exist: '${fso}'" + log info "File does not exist: '${fso}'" continue fi From 4a5312b3a9419c8b3e07dda2b650d5fbf9a38d34 Mon Sep 17 00:00:00 2001 From: Patrick Schleizer Date: Wed, 24 Jul 2024 11:27:51 -0400 Subject: [PATCH 19/48] output --- usr/bin/permission-hardener | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/usr/bin/permission-hardener b/usr/bin/permission-hardener index a71e9e6..b66db0f 100755 --- a/usr/bin/permission-hardener +++ b/usr/bin/permission-hardener @@ -13,14 +13,14 @@ 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" -log_level=info +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 info "Run: $*" + log notice "Run: $*" else shift fi @@ -30,7 +30,7 @@ echo_wrapper_ignore() { echo_wrapper_audit() { if test "${1}" = "verbose"; then shift - log info "Run: $*" + log notice "Run: $*" else shift fi @@ -559,7 +559,7 @@ apply() { sanity_tests parse_config_folder - log info "\ + 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" From cda0d26af7c057dab8edf4897f98c2e8f83e3d56 Mon Sep 17 00:00:00 2001 From: Patrick Schleizer Date: Wed, 24 Jul 2024 11:45:13 -0400 Subject: [PATCH 20/48] cannot use NULL inside a bash variable use custom delimiter instead --- usr/bin/permission-hardener | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/usr/bin/permission-hardener b/usr/bin/permission-hardener index b66db0f..0637ed9 100755 --- a/usr/bin/permission-hardener +++ b/usr/bin/permission-hardener @@ -12,6 +12,7 @@ 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#" log_level=notice # shellcheck disable=SC1091 @@ -81,11 +82,11 @@ add_nosuid_statoverride_entry() { continue fi - ## Capture the stat output with fields separated by NUL characters. ## Delimiter at the end to avoid the last field to be interpreted as having a newline. - stat_output=$(stat -c '%n\0%a\0%U\0%G\0%' "${line}") + stat_output=$(stat -c "%n${delimiter}%a${delimiter}%U${delimiter}%G${delimiter}%" "${line}") + stat_output_simple=$(stat -c '%n %a %U %G' "${line}") - readarray -d '\0' -t arr <<< "${stat_output}" + readarray -d "${delimiter}" -t arr <<< "${stat_output}" if test "${#arr[@]}" = 0; then log error "Line is empty. Stat output: '${stat_output}', line: '${line}'" >&2 @@ -382,13 +383,13 @@ set_file_perms() { local stat_output stat_output="" - if ! stat_output="$(stat -c '%n\0%a\0%U\0%G\0%' "${fso_without_trailing_slash}")"; then + if ! stat_output=$(stat -c "%n${delimiter}%a${delimiter}%U${delimiter}%G${delimiter}%" "${fso_without_trailing_slash}"); then log error "Failed to run 'stat' on file: '${fso_without_trailing_slash}'!" >&2 continue fi local arr file_name file_name_from_stat existing_mode existing_owner existing_group - readarray -d '\0' -t arr <<< "${stat_output}" + readarray -d "${delimiter}" -t arr <<< "${stat_output}" file_name="${fso_without_trailing_slash}" if test "${#arr[@]}" = 0; then From 6cadc70a96cd709fb7a94abcb14e7dd97c57fdb8 Mon Sep 17 00:00:00 2001 From: Patrick Schleizer Date: Wed, 24 Jul 2024 11:47:52 -0400 Subject: [PATCH 21/48] output --- usr/bin/permission-hardener | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/usr/bin/permission-hardener b/usr/bin/permission-hardener index 0637ed9..bea2abe 100755 --- a/usr/bin/permission-hardener +++ b/usr/bin/permission-hardener @@ -411,7 +411,7 @@ set_file_perms() { continue 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: '${line}'" >&2 continue fi if test -z "${existing_group}"; then From cc2b335ee692cc04a2c4e298902f3503927b2c50 Mon Sep 17 00:00:00 2001 From: Patrick Schleizer Date: Wed, 24 Jul 2024 11:48:32 -0400 Subject: [PATCH 22/48] cleanup --- usr/bin/permission-hardener | 1 - 1 file changed, 1 deletion(-) diff --git a/usr/bin/permission-hardener b/usr/bin/permission-hardener index bea2abe..2612b2a 100755 --- a/usr/bin/permission-hardener +++ b/usr/bin/permission-hardener @@ -84,7 +84,6 @@ add_nosuid_statoverride_entry() { ## Delimiter at the end to avoid the last field to be interpreted as having a newline. stat_output=$(stat -c "%n${delimiter}%a${delimiter}%U${delimiter}%G${delimiter}%" "${line}") - stat_output_simple=$(stat -c '%n %a %U %G' "${line}") readarray -d "${delimiter}" -t arr <<< "${stat_output}" From a16dd8474bf72c2b8c63adc7500140e89d19fedb Mon Sep 17 00:00:00 2001 From: Patrick Schleizer Date: Wed, 24 Jul 2024 11:50:30 -0400 Subject: [PATCH 23/48] sanity test --- usr/bin/permission-hardener | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/usr/bin/permission-hardener b/usr/bin/permission-hardener index 2612b2a..61090bf 100755 --- a/usr/bin/permission-hardener +++ b/usr/bin/permission-hardener @@ -99,6 +99,7 @@ add_nosuid_statoverride_entry() { if [ ! "$file_name" = "$file_name_from_stat" ]; then log error "\ +function add_nosuid_statoverride_entry: file_name is different from file_name_from_stat: line: '${line}' file_name '${file_name}' @@ -401,6 +402,16 @@ set_file_perms() { existing_owner="${arr[2]}" existing_group="${arr[3]}" + if [ ! "$file_name" = "$file_name_from_stat" ]; then + log error "\ +function set_file_perms: +file_name is different from file_name_from_stat: +line: '${line}' +file_name '${file_name}' +file_name_from_stat: '${file_name_from_stat}'" >&2 + continue + fi + if test -z "${file_name}"; then log error "File name is empty. Stat output: '${stat_output}', line: '${line}'" >&2 continue From 10c73b326f824f783169383888b9464965a53cbb Mon Sep 17 00:00:00 2001 From: Patrick Schleizer Date: Wed, 24 Jul 2024 12:07:26 -0400 Subject: [PATCH 24/48] fix delimiter parsing --- usr/bin/permission-hardener | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/usr/bin/permission-hardener b/usr/bin/permission-hardener index 61090bf..b843ad2 100755 --- a/usr/bin/permission-hardener +++ b/usr/bin/permission-hardener @@ -73,7 +73,7 @@ add_nosuid_statoverride_entry() { while IFS="" read -r -d "" line; do counter_actual="$((counter_actual + 1))" - local arr file_name file_name_from_stat existing_mode existing_owner existing_group stat_output + local arr file_name file_name_from_stat existing_mode existing_owner existing_group stat_output stat_output_newlined file_name="${line}" @@ -82,10 +82,9 @@ add_nosuid_statoverride_entry() { continue fi - ## Delimiter at the end to avoid the last field to be interpreted as having a newline. - stat_output=$(stat -c "%n${delimiter}%a${delimiter}%U${delimiter}%G${delimiter}%" "${line}") - - readarray -d "${delimiter}" -t arr <<< "${stat_output}" + stat_output=$(stat -c "%n${delimiter}%a${delimiter}%U${delimiter}%G${delimiter}" "${line}") + stat_output_newlined=$(printf '%s\n' "${stat_output//${delimiter}/$'\n'}") + readarray -t arr <<< "${stat_output_newlined}" if test "${#arr[@]}" = 0; then log error "Line is empty. Stat output: '${stat_output}', line: '${line}'" >&2 @@ -381,15 +380,16 @@ set_file_perms() { mode_for_grep="${mode_from_config:1}" fi - local stat_output + local stat_output stat_output_newlined stat_output="" - if ! stat_output=$(stat -c "%n${delimiter}%a${delimiter}%U${delimiter}%G${delimiter}%" "${fso_without_trailing_slash}"); then + if ! stat_output=$(stat -c "%n${delimiter}%a${delimiter}%U${delimiter}%G${delimiter}" "${fso_without_trailing_slash}"); then log error "Failed to run 'stat' on file: '${fso_without_trailing_slash}'!" >&2 continue fi + stat_output_newlined=$(printf '%s\n' "${stat_output//${delimiter}/$'\n'}") local arr file_name file_name_from_stat existing_mode existing_owner existing_group - readarray -d "${delimiter}" -t arr <<< "${stat_output}" + readarray -t arr <<< "${stat_output_newlined}" file_name="${fso_without_trailing_slash}" if test "${#arr[@]}" = 0; then From 4cc1289e89b341e15725d65e405e607ea4784f9f Mon Sep 17 00:00:00 2001 From: Patrick Schleizer Date: Wed, 24 Jul 2024 13:30:30 -0400 Subject: [PATCH 25/48] output --- usr/bin/permission-hardener | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/usr/bin/permission-hardener b/usr/bin/permission-hardener index b843ad2..cef71b9 100755 --- a/usr/bin/permission-hardener +++ b/usr/bin/permission-hardener @@ -160,6 +160,7 @@ file_name_from_stat: '${file_name_from_stat}'" >&2 setuid_or_setsgid=true fi if test -z "${setuid_or_setsgid}"; then + log info "Neither setuid nor setsgid. Skipping. file_name: '${file_name}'" continue fi @@ -177,10 +178,12 @@ file_name_from_stat: '${file_name_from_stat}'" >&2 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 @@ -190,10 +193,12 @@ file_name_from_stat: '${file_name_from_stat}'" >&2 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 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 @@ -203,10 +208,12 @@ file_name_from_stat: '${file_name_from_stat}'" >&2 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 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 @@ -230,6 +237,8 @@ file_name_from_stat: '${file_name_from_stat}'" >&2 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." @@ -277,6 +286,7 @@ set_file_perms() { local line while read -r line || test -n "${line}"; do if test -z "${line}"; then + true "DEBUG: line is empty. Skipping." continue fi @@ -498,6 +508,7 @@ file_name_from_stat: '${file_name_from_stat}'" >&2 fi fi if test -z "${capability_from_config}"; then + log info "capability_from_config is empty. Skipping. file_name: '${file_name}'" continue fi From 9231f058911ab9059e91c4c0c1677ef66b5bb666 Mon Sep 17 00:00:00 2001 From: Patrick Schleizer Date: Wed, 24 Jul 2024 13:31:49 -0400 Subject: [PATCH 26/48] todo --- usr/bin/permission-hardener | 3 +++ 1 file changed, 3 insertions(+) diff --git a/usr/bin/permission-hardener b/usr/bin/permission-hardener index cef71b9..1c21df2 100755 --- a/usr/bin/permission-hardener +++ b/usr/bin/permission-hardener @@ -6,6 +6,9 @@ ## 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 +## TODO: +## - unduplicate stat_output related source code + set -o errexit -o nounset -o pipefail exit_code=0 From 3b8a3f9b832ee1eee959fbcce8b5eed417d4712e Mon Sep 17 00:00:00 2001 From: Ben Grande Date: Thu, 25 Jul 2024 12:20:16 +0200 Subject: [PATCH 27/48] Unduplicate stat call --- usr/bin/permission-hardener | 187 ++++++++++++++++++------------------ 1 file changed, 91 insertions(+), 96 deletions(-) diff --git a/usr/bin/permission-hardener b/usr/bin/permission-hardener index 1c21df2..10fad42 100755 --- a/usr/bin/permission-hardener +++ b/usr/bin/permission-hardener @@ -6,9 +6,6 @@ ## 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 -## TODO: -## - unduplicate stat_output related source code - set -o errexit -o nounset -o pipefail exit_code=0 @@ -17,6 +14,7 @@ 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=SC2034 log_level=notice # shellcheck disable=SC1091 source /usr/libexec/helper-scripts/log_run_die.sh @@ -53,6 +51,78 @@ make_store_dir(){ 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}" + + declare -a arr + local file_name_from_stat existing_mode existing_owner existing_group stat_output stat_output_newlined + + if ! stat_output="$(stat -c "%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 + + stat_output_newlined="$(printf '%s\n' "${stat_output//${delimiter}/$'\n'}")" + readarray -t arr <<< "${stat_output_newlined}" + + if test "${#arr[@]}" = 0; then + log error "Line is empty. Stat output: '${stat_output}', 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 "\ +function ${FUNCNAME[1]}: +File name is different from file name received from stat: +File name '${file_name}' +File name from stat: '${file_name_from_stat}'" >&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 + + (IFS=$'\n'; echo "${arr[*]}") +} + sanity_tests() { echo_wrapper_audit silent which \ capsh getcap setcap stat find dpkg-statoverride getent xargs grep 1>/dev/null @@ -73,60 +143,21 @@ add_nosuid_statoverride_entry() { done < <(find "${fso_to_process}" -perm /u=s,g=s -print0) local line - while IFS="" read -r -d "" line; do - counter_actual="$((counter_actual + 1))" + while IFS="" read -r -d "" file_name; do + counter_actual=$((counter_actual + 1)) - local arr file_name file_name_from_stat existing_mode existing_owner existing_group stat_output stat_output_newlined - - file_name="${line}" - - if test -z "${file_name}"; then - log error "File name is empty in line: '${line}'" >&2 - continue - fi - - stat_output=$(stat -c "%n${delimiter}%a${delimiter}%U${delimiter}%G${delimiter}" "${line}") - stat_output_newlined=$(printf '%s\n' "${stat_output//${delimiter}/$'\n'}") - readarray -t arr <<< "${stat_output_newlined}" + declare -a arr + local existing_mode existing_owner existing_group + readarray -t arr < <(output_stat "${file_name}") + ## Above command creates a subshell that cannot be returned. if test "${#arr[@]}" = 0; then - log error "Line is empty. Stat output: '${stat_output}', line: '${line}'" >&2 continue fi - file_name_from_stat="${arr[0]}" - existing_mode="${arr[1]}" - existing_owner="${arr[2]}" - existing_group="${arr[3]}" - - if [ ! "$file_name" = "$file_name_from_stat" ]; then - log error "\ -function add_nosuid_statoverride_entry: -file_name is different from file_name_from_stat: -line: '${line}' -file_name '${file_name}' -file_name_from_stat: '${file_name_from_stat}'" >&2 - continue - fi - - if test -z "${existing_mode}"; then - log error "Existing mode is empty in line: '${line}'" >&2 - continue - fi - if test -z "${existing_owner}"; then - log error "Existing owner is empty in line: '${line}'" >&2 - continue - fi - if test -z "${existing_group}"; then - log error "Existing group is empty in line: '${line}'" >&2 - continue - fi - - ## dpkg-statoverride: error: path may not contain newlines - if [[ "${file_name}" == *$'\n'* ]]; then - log warn "Skipping file name that contains newlines: '${file_name}'" >&2 - continue - fi + existing_mode="${arr[0]}" + existing_owner="${arr[1]}" + existing_group="${arr[2]}" ## -h file True if file is a symbolic Link. ## -u file True if file has its set-user-id bit set. @@ -335,7 +366,6 @@ set_file_perms() { local fso_without_trailing_slash fso_without_trailing_slash="${fso%/}" - ## TODO: test/add white spaces inside file name support declare -g disable_white_list exact_white_list match_white_list case "${mode_from_config}" in disablewhitelist) @@ -393,54 +423,19 @@ set_file_perms() { mode_for_grep="${mode_from_config:1}" fi - local stat_output stat_output_newlined - stat_output="" - if ! stat_output=$(stat -c "%n${delimiter}%a${delimiter}%U${delimiter}%G${delimiter}" "${fso_without_trailing_slash}"); then - log error "Failed to run 'stat' on file: '${fso_without_trailing_slash}'!" >&2 - continue - fi - stat_output_newlined=$(printf '%s\n' "${stat_output//${delimiter}/$'\n'}") + declare -a arr + local existing_mode existing_owner existing_group - local arr file_name file_name_from_stat existing_mode existing_owner existing_group - readarray -t arr <<< "${stat_output_newlined}" file_name="${fso_without_trailing_slash}" - + readarray -t arr < <(output_stat "${file_name}") + ## Above command creates a subshell that cannot be returned from. if test "${#arr[@]}" = 0; then - log error "Line is empty. Stat output: '${stat_output}', line: '${line}'" >&2 continue fi - file_name_from_stat="${arr[0]}" - existing_mode="${arr[1]}" - existing_owner="${arr[2]}" - existing_group="${arr[3]}" - - if [ ! "$file_name" = "$file_name_from_stat" ]; then - log error "\ -function set_file_perms: -file_name is different from file_name_from_stat: -line: '${line}' -file_name '${file_name}' -file_name_from_stat: '${file_name_from_stat}'" >&2 - continue - fi - - if test -z "${file_name}"; then - log error "File name is empty. Stat output: '${stat_output}', line: '${line}'" >&2 - continue - fi - if test -z "${existing_mode}"; then - log error "Existing mode is empty. Stat output: '${stat_output}', line: '${line}'" >&2 - continue - fi - if test -z "${existing_owner}"; then - log error "Existing owner is empty. Stat output: '${stat_output}' | line: '${line}'" >&2 - continue - fi - if test -z "${existing_group}"; then - log error "Existing group is empty. Stat output: '${stat_output}', line: '${line}'" >&2 - continue - fi + existing_mode="${arr[0]}" + existing_owner="${arr[1]}" + existing_group="${arr[2]}" ## Check there is an entry for the fso. ## @@ -558,9 +553,9 @@ parse_config_folder() { ## '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) + passwd_file_contents_temp="$(getent passwd)" echo "${passwd_file_contents_temp}" | tee "${store_dir}/private/passwd" >/dev/null - group_file_contents_temp=$(getent group) + group_file_contents_temp="$(getent group)" echo "${group_file_contents_temp}" | tee "${store_dir}/private/group" >/dev/null #passwd_file_contents="$(cat "${store_dir}/private/passwd")" From 652a06c8e9f841e043cc5b5fb030b149cb70dc85 Mon Sep 17 00:00:00 2001 From: Ben Grande Date: Thu, 25 Jul 2024 12:37:21 +0200 Subject: [PATCH 28/48] Only print SUID or SGID values when set --- usr/bin/permission-hardener | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/usr/bin/permission-hardener b/usr/bin/permission-hardener index 10fad42..3196cfa 100755 --- a/usr/bin/permission-hardener +++ b/usr/bin/permission-hardener @@ -174,27 +174,23 @@ add_nosuid_statoverride_entry() { continue fi - local setuid setuid_output setsgid setsgid_output + local setuid setgid setuid="" - setuid_output="" if test -u "${file_name}"; then setuid=true - setuid_output="set-user-id" fi - setsgid="" - setsgid_output="" + setgid="" if test -g "${file_name}"; then - setsgid=true - setsgid_output="set-group-id" + setgid=true fi - local setuid_or_setsgid - setuid_or_setsgid="" - if test "${setuid}" = "true" || test "${setsgid}" = "true"; then - setuid_or_setsgid=true + 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_setsgid}"; then - log info "Neither setuid nor setsgid. Skipping. file_name: '${file_name}'" + if test -z "${setuid_or_setgid}"; then + log info "Neither setuid nor setgid. Skipping. file_name: '${file_name}'" continue fi @@ -255,7 +251,7 @@ add_nosuid_statoverride_entry() { local clean_output_prefix clean_output clean_output_prefix="Managing (S|G)UID of line:" - clean_output="setuid='${setuid_output}' setgid='${setsgid_output}' existing_mode='${existing_mode}' new_mode='${new_mode}' file='${file_name}'" + 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 From f616da7c0690fc0dffc21be59174ed8754ec55fb Mon Sep 17 00:00:00 2001 From: Patrick Schleizer Date: Fri, 26 Jul 2024 09:40:59 +0000 Subject: [PATCH 29/48] bumped changelog version --- changelog.upstream | 179 +++++++++++++++++++++++++++++++++++++++++++++ debian/changelog | 6 ++ 2 files changed, 185 insertions(+) diff --git a/changelog.upstream b/changelog.upstream index 32ec008..9095f50 100644 --- a/changelog.upstream +++ b/changelog.upstream @@ -1,3 +1,182 @@ +commit 9231f058911ab9059e91c4c0c1677ef66b5bb666 +Author: Patrick Schleizer +Date: Wed Jul 24 13:31:49 2024 -0400 + + todo + +commit 4cc1289e89b341e15725d65e405e607ea4784f9f +Author: Patrick Schleizer +Date: Wed Jul 24 13:30:30 2024 -0400 + + output + +commit 10c73b326f824f783169383888b9464965a53cbb +Author: Patrick Schleizer +Date: Wed Jul 24 12:07:26 2024 -0400 + + fix delimiter parsing + +commit a16dd8474bf72c2b8c63adc7500140e89d19fedb +Author: Patrick Schleizer +Date: Wed Jul 24 11:50:30 2024 -0400 + + sanity test + +commit cc2b335ee692cc04a2c4e298902f3503927b2c50 +Author: Patrick Schleizer +Date: Wed Jul 24 11:48:32 2024 -0400 + + cleanup + +commit 6cadc70a96cd709fb7a94abcb14e7dd97c57fdb8 +Author: Patrick Schleizer +Date: Wed Jul 24 11:47:52 2024 -0400 + + output + +commit cda0d26af7c057dab8edf4897f98c2e8f83e3d56 +Author: Patrick Schleizer +Date: Wed Jul 24 11:45:13 2024 -0400 + + cannot use NULL inside a bash variable + + use custom delimiter instead + +commit 4a5312b3a9419c8b3e07dda2b650d5fbf9a38d34 +Author: Patrick Schleizer +Date: Wed Jul 24 11:27:51 2024 -0400 + + output + +commit 3bf1f26c0bb271d63c16b314e4da040abf5b3713 +Author: Patrick Schleizer +Date: Wed Jul 24 11:20:26 2024 -0400 + + downgrade warning of non-existing folders to info + + to avoid all users by default getting a warning for expected non-existing folders + +commit 151ca659a9f5565744ff57f3b581c8c051def148 +Author: Patrick Schleizer +Date: Wed Jul 24 11:19:15 2024 -0400 + + output + +commit c9fd2ceb61ea176c731432f02a9fa40652fbddc8 +Author: Patrick Schleizer +Date: Wed Jul 24 11:13:35 2024 -0400 + + downgrade warning of non-existing files to info + + to avoid all users by default getting a warning for expected non-existing files + +commit 721392901be384014298f59deb57747b825c8b37 +Author: Patrick Schleizer +Date: Wed Jul 24 11:12:39 2024 -0400 + + remove duplicate test + +commit 9712b5b4e3cff3eac8ef03b5e562ff89d74ef4b8 +Author: Patrick Schleizer +Date: Wed Jul 24 11:12:18 2024 -0400 + + output + +commit 00911df5c1de24960ad6d21b4cd99450f2d08a88 +Author: Patrick Schleizer +Date: Wed Jul 24 11:10:56 2024 -0400 + + modify call of stat to use NUL delimiter + + for more robust string parsing + +commit d5366835112cc5fabef7ec46a9c582c08121cb14 +Author: Patrick Schleizer +Date: Wed Jul 24 11:03:28 2024 -0400 + + local clean_output_prefix clean_output + +commit a6e517736b83c124cf8cec52bac184612a29ad0d +Author: Patrick Schleizer +Date: Wed Jul 24 11:02:25 2024 -0400 + + local stat_output + +commit ced02fb9e03e12c7d51923511e7d6a54b09a6274 +Author: Patrick Schleizer +Date: Wed Jul 24 11:01:24 2024 -0400 + + add sanity test for file_name output from stat + +commit b9dfe70a016e46e1f275918be19890526182cfa2 +Author: Patrick Schleizer +Date: Wed Jul 24 10:58:05 2024 -0400 + + check first if file_name is empty + +commit 1cbda7998196dc04e83c48526d15f9ad5f11e6c9 +Author: Patrick Schleizer +Date: Wed Jul 24 10:57:13 2024 -0400 + + check first if array is empty before parsing further + +commit a077ae54ea050af8828813b781738cba24e27624 +Author: Patrick Schleizer +Date: Wed Jul 24 10:56:08 2024 -0400 + + modify call of stat to use NUL delimiter + + for more robust string parsing + +commit 7200e9bd8c793f5ea30c3448fd03fbd38c6292b5 +Author: Patrick Schleizer +Date: Wed Jul 24 09:15:02 2024 -0400 + + output + +commit 1b6161c2dcd9a0686503c84cda4c9f6a29fe4e02 +Merge: d2563ed 8be21b6 +Author: Patrick Schleizer +Date: Wed Jul 24 09:13:48 2024 -0400 + + Merge remote-tracking branch 'ben-grande/fuzz' + +commit 8be21b6eff40fdd3909ef63468463fc52e8bf45f +Author: Ben Grande +Date: Tue Jul 23 19:36:12 2024 +0200 + + Handle newlines in file names + +commit aa99de68d307cd88462665424996d9b730ab5087 +Author: Ben Grande +Date: Tue Jul 23 18:46:47 2024 +0200 + + Log output with defined levels + +commit 06fbcdac1de6f1830d911f05a4f7c14fd522fad4 +Author: Ben Grande +Date: Tue Jul 23 09:55:02 2024 +0200 + + Prettify log messages + +commit 7ee1ea2cc7dd62feee3243d64b414130e68d35e9 +Author: Ben Grande +Date: Mon Jul 22 17:06:07 2024 +0200 + + Unify functions that evaluate commands + +commit 9c3566f524f748b9f7c98a36b3f2b1064cdba3ed +Author: Ben Grande +Date: Mon Jul 22 16:01:14 2024 +0200 + + Delimit file names with null terminator + +commit d2563ed92317a029340dbb83f30da008b01325f2 +Author: Patrick Schleizer +Date: Sun Jul 21 10:40:14 2024 +0000 + + bumped changelog version + commit 64f8b2eb5870664fca06aa060f2f50af358ced55 Author: Patrick Schleizer Date: Sun Jul 21 06:36:22 2024 -0400 diff --git a/debian/changelog b/debian/changelog index 876e60b..06a9a4c 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,9 @@ +security-misc (3:38.4-1) unstable; urgency=medium + + * New upstream version (local package). + + -- Patrick Schleizer Fri, 26 Jul 2024 09:40:58 +0000 + security-misc (3:38.3-1) unstable; urgency=medium * New upstream version (local package). From 9f135231ccdc3f6eba27db2e1794eff23f03fc0f Mon Sep 17 00:00:00 2001 From: Patrick Schleizer Date: Fri, 26 Jul 2024 06:43:01 -0400 Subject: [PATCH 30/48] no longer disable Intel ME related kernel modules because that might break firmware updates This reverts commit 64f8b2eb5870664fca06aa060f2f50af358ced55. https://github.com/Kicksecure/security-misc/issues/239 --- etc/modprobe.d/30_security-misc_disable.conf | 24 ++++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/etc/modprobe.d/30_security-misc_disable.conf b/etc/modprobe.d/30_security-misc_disable.conf index b6cfcbe..d2408af 100644 --- a/etc/modprobe.d/30_security-misc_disable.conf +++ b/etc/modprobe.d/30_security-misc_disable.conf @@ -92,18 +92,18 @@ install gnss-usb /usr/bin/disabled-gps-by-security-misc ## https://github.com/Kicksecure/security-misc/pull/236#issuecomment-2229092813 ## https://github.com/Kicksecure/security-misc/issues/239 ## -install mei /usr/bin/disabled-intelme-by-security-misc -install mei-gsc /usr/bin/disabled-intelme-by-security-misc -install mei_gsc_proxy /usr/bin/disabled-intelme-by-security-misc -install mei_hdcp /usr/bin/disabled-intelme-by-security-misc -install mei-me /usr/bin/disabled-intelme-by-security-misc -install mei_phy /usr/bin/disabled-intelme-by-security-misc -install mei_pxp /usr/bin/disabled-intelme-by-security-misc -install mei-txe /usr/bin/disabled-intelme-by-security-misc -install mei-vsc /usr/bin/disabled-intelme-by-security-misc -install mei-vsc-hw /usr/bin/disabled-intelme-by-security-misc -install mei_wdt /usr/bin/disabled-intelme-by-security-misc -install microread_mei /usr/bin/disabled-intelme-by-security-misc +#install mei /usr/bin/disabled-intelme-by-security-misc +#install mei-gsc /usr/bin/disabled-intelme-by-security-misc +#install mei_gsc_proxy /usr/bin/disabled-intelme-by-security-misc +#install mei_hdcp /usr/bin/disabled-intelme-by-security-misc +#install mei-me /usr/bin/disabled-intelme-by-security-misc +#install mei_phy /usr/bin/disabled-intelme-by-security-misc +#install mei_pxp /usr/bin/disabled-intelme-by-security-misc +#install mei-txe /usr/bin/disabled-intelme-by-security-misc +#install mei-vsc /usr/bin/disabled-intelme-by-security-misc +#install mei-vsc-hw /usr/bin/disabled-intelme-by-security-misc +#install mei_wdt /usr/bin/disabled-intelme-by-security-misc +#install microread_mei /usr/bin/disabled-intelme-by-security-misc ## Intel Platform Monitoring Technology Telemetry (PMT): ## Disable some functionality of the Intel PMT components. From 9694cf0cd1a225c68d45814e0f4d6995659a0066 Mon Sep 17 00:00:00 2001 From: Patrick Schleizer Date: Fri, 26 Jul 2024 07:43:59 -0400 Subject: [PATCH 31/48] output --- usr/bin/permission-hardener | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/usr/bin/permission-hardener b/usr/bin/permission-hardener index 3196cfa..58f8918 100755 --- a/usr/bin/permission-hardener +++ b/usr/bin/permission-hardener @@ -112,7 +112,7 @@ File name from stat: '${file_name_from_stat}'" >&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: '${line}'" >&2 return 1 fi if test -z "${existing_group}"; then From 19f131c7426aaa5199504e75aba180a7771a2520 Mon Sep 17 00:00:00 2001 From: Patrick Schleizer Date: Fri, 26 Jul 2024 08:07:08 -0400 Subject: [PATCH 32/48] code simplification https://github.com/Kicksecure/security-misc/pull/251 --- usr/bin/permission-hardener | 37 +++++++++++-------------------------- 1 file changed, 11 insertions(+), 26 deletions(-) diff --git a/usr/bin/permission-hardener b/usr/bin/permission-hardener index 58f8918..98f4824 100755 --- a/usr/bin/permission-hardener +++ b/usr/bin/permission-hardener @@ -78,7 +78,7 @@ output_stat(){ block_newlines file "${file_name}" declare -a arr - local file_name_from_stat existing_mode existing_owner existing_group stat_output stat_output_newlined + local file_name_from_stat stat_output stat_output_newlined if ! stat_output="$(stat -c "%a${delimiter}%U${delimiter}%G${delimiter}%n${delimiter}" "${file_name}")"; then log error "Failed to run 'stat' on file: '${file_name}'!" >&2 @@ -119,8 +119,6 @@ File name from stat: '${file_name_from_stat}'" >&2 log error "Existing group is empty. Stat output: '${stat_output}', line: '${line}'" >&2 return 1 fi - - (IFS=$'\n'; echo "${arr[*]}") } sanity_tests() { @@ -146,18 +144,11 @@ add_nosuid_statoverride_entry() { while IFS="" read -r -d "" file_name; do counter_actual=$((counter_actual + 1)) - declare -a arr - local existing_mode existing_owner existing_group - - readarray -t arr < <(output_stat "${file_name}") - ## Above command creates a subshell that cannot be returned. - if test "${#arr[@]}" = 0; then - continue - fi - - existing_mode="${arr[0]}" - existing_owner="${arr[1]}" - existing_group="${arr[2]}" + ## 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. @@ -419,19 +410,13 @@ set_file_perms() { mode_for_grep="${mode_from_config:1}" fi - declare -a arr - local existing_mode existing_owner existing_group - file_name="${fso_without_trailing_slash}" - readarray -t arr < <(output_stat "${file_name}") - ## Above command creates a subshell that cannot be returned from. - if test "${#arr[@]}" = 0; then - continue - fi - existing_mode="${arr[0]}" - existing_owner="${arr[1]}" - existing_group="${arr[2]}" + ## sets: + ## exiting_mode + ## existing_owner + ## existing_group + output_stat "${file_name}" ## Check there is an entry for the fso. ## From 95722d6d7902367afb44175263a8628df9ad01b2 Mon Sep 17 00:00:00 2001 From: Patrick Schleizer Date: Fri, 26 Jul 2024 08:13:33 -0400 Subject: [PATCH 33/48] use long option name --- usr/bin/permission-hardener | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/usr/bin/permission-hardener b/usr/bin/permission-hardener index 98f4824..a3b68d2 100755 --- a/usr/bin/permission-hardener +++ b/usr/bin/permission-hardener @@ -80,7 +80,7 @@ output_stat(){ declare -a arr local file_name_from_stat stat_output stat_output_newlined - if ! stat_output="$(stat -c "%a${delimiter}%U${delimiter}%G${delimiter}%n${delimiter}" "${file_name}")"; then + 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 From 30f46790a4df7662926fa43d44ac34c3286dd590 Mon Sep 17 00:00:00 2001 From: Patrick Schleizer Date: Fri, 26 Jul 2024 08:21:21 -0400 Subject: [PATCH 34/48] use end of options whenever possible --- usr/bin/permission-hardener | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/usr/bin/permission-hardener b/usr/bin/permission-hardener index a3b68d2..412f3f2 100755 --- a/usr/bin/permission-hardener +++ b/usr/bin/permission-hardener @@ -80,7 +80,7 @@ output_stat(){ 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 --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 @@ -217,7 +217,7 @@ add_nosuid_statoverride_entry() { log info "matchwhite_list_entry unset. Skipping. file_name: '${file_name}'" continue fi - if echo "${file_name}" | grep --quiet --fixed-strings "${matchwhite_list_entry}"; then + if 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. @@ -232,7 +232,7 @@ add_nosuid_statoverride_entry() { log info "disablematch_list_entry unset. Skipping. file_name: '${file_name}'" continue fi - if echo "${file_name}" | grep --quiet --fixed-strings "${disablematch_list_entry}"; then + if 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. @@ -392,12 +392,12 @@ set_file_perms() { continue fi - if ! grep --quiet --fixed-strings "${owner_from_config}:" "${store_dir}/private/passwd"; then + 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 + 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 @@ -435,7 +435,7 @@ set_file_perms() { 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 echo "${dpkg_statoverride_list_output}" | grep --quiet --fixed-strings "${grep_line}"; then + if 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." @@ -498,22 +498,22 @@ set_file_perms() { ## 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. - echo_wrapper_ignore verbose setcap -r "${fso}" - getcap_output="$(getcap "${fso}")" + 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 + 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}" + echo_wrapper_audit verbose setcap "${capability_from_config}+ep" -- "${fso}" fi done <"${config_file}" @@ -530,7 +530,7 @@ parse_config_folder() { ## 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 + ## 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. From aac450f80836b03478b9e2632afc5a4519f9b37a Mon Sep 17 00:00:00 2001 From: Patrick Schleizer Date: Fri, 26 Jul 2024 08:22:04 -0400 Subject: [PATCH 35/48] refactoring --- usr/bin/permission-hardener | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/usr/bin/permission-hardener b/usr/bin/permission-hardener index 412f3f2..34ba038 100755 --- a/usr/bin/permission-hardener +++ b/usr/bin/permission-hardener @@ -122,8 +122,8 @@ File name from stat: '${file_name_from_stat}'" >&2 } sanity_tests() { - echo_wrapper_audit silent which \ - capsh getcap setcap stat find dpkg-statoverride getent xargs grep 1>/dev/null + echo_wrapper_audit silent \ + which capsh getcap setcap stat find dpkg-statoverride getent xargs grep 1>/dev/null } add_nosuid_statoverride_entry() { From 6f0551b944cbf83d82f7a1a554c4461bc971520b Mon Sep 17 00:00:00 2001 From: Patrick Schleizer Date: Fri, 26 Jul 2024 08:23:54 -0400 Subject: [PATCH 36/48] refactoring --- usr/bin/permission-hardener | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/usr/bin/permission-hardener b/usr/bin/permission-hardener index 34ba038..d84fd6e 100755 --- a/usr/bin/permission-hardener +++ b/usr/bin/permission-hardener @@ -123,7 +123,8 @@ File name from stat: '${file_name_from_stat}'" >&2 sanity_tests() { echo_wrapper_audit silent \ - which capsh getcap setcap stat find dpkg-statoverride getent xargs grep 1>/dev/null + which \ + capsh getcap setcap stat find dpkg-statoverride getent xargs grep 1>/dev/null } add_nosuid_statoverride_entry() { From 2b40ea75e9c3f679fd09ae331a56f294c3ac7607 Mon Sep 17 00:00:00 2001 From: Patrick Schleizer Date: Fri, 26 Jul 2024 08:24:23 -0400 Subject: [PATCH 37/48] cleanup --- usr/bin/permission-hardener | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/usr/bin/permission-hardener b/usr/bin/permission-hardener index d84fd6e..cb51993 100755 --- a/usr/bin/permission-hardener +++ b/usr/bin/permission-hardener @@ -124,7 +124,7 @@ File name from stat: '${file_name_from_stat}'" >&2 sanity_tests() { echo_wrapper_audit silent \ which \ - capsh getcap setcap stat find dpkg-statoverride getent xargs grep 1>/dev/null + capsh getcap setcap stat find dpkg-statoverride getent grep 1>/dev/null } add_nosuid_statoverride_entry() { From f2c9c2f5d1b59127b22fae4dd4b8bb7a6f98a485 Mon Sep 17 00:00:00 2001 From: Patrick Schleizer Date: Fri, 26 Jul 2024 08:26:16 -0400 Subject: [PATCH 38/48] output --- usr/bin/permission-hardener | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/usr/bin/permission-hardener b/usr/bin/permission-hardener index cb51993..a11e6a6 100755 --- a/usr/bin/permission-hardener +++ b/usr/bin/permission-hardener @@ -89,7 +89,7 @@ output_stat(){ readarray -t arr <<< "${stat_output_newlined}" if test "${#arr[@]}" = 0; then - log error "Line is empty. Stat output: '${stat_output}', line: '${line}'" >&2 + log error "Array is empty. Stat output: '${stat_output}', line: '${line}'" >&2 return 1 fi From 8e40c10c319a76e0256c8f135182b0ca7f532f85 Mon Sep 17 00:00:00 2001 From: Patrick Schleizer Date: Fri, 26 Jul 2024 08:31:17 -0400 Subject: [PATCH 39/48] comment --- usr/bin/permission-hardener | 2 ++ 1 file changed, 2 insertions(+) diff --git a/usr/bin/permission-hardener b/usr/bin/permission-hardener index a11e6a6..2e1345b 100755 --- a/usr/bin/permission-hardener +++ b/usr/bin/permission-hardener @@ -6,6 +6,8 @@ ## 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 From d96c0633d431dafd034ae8d1ae0ffbb59c49be4a Mon Sep 17 00:00:00 2001 From: Patrick Schleizer Date: Fri, 26 Jul 2024 08:39:11 -0400 Subject: [PATCH 40/48] more use of end of options --- usr/bin/permission-hardener | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/usr/bin/permission-hardener b/usr/bin/permission-hardener index 2e1345b..a7f02f4 100755 --- a/usr/bin/permission-hardener +++ b/usr/bin/permission-hardener @@ -220,7 +220,7 @@ add_nosuid_statoverride_entry() { log info "matchwhite_list_entry unset. Skipping. file_name: '${file_name}'" continue fi - if echo "${file_name}" | grep --quiet --fixed-strings -- "${matchwhite_list_entry}"; then + if 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. @@ -235,7 +235,7 @@ add_nosuid_statoverride_entry() { log info "disablematch_list_entry unset. Skipping. file_name: '${file_name}'" continue fi - if echo "${file_name}" | grep --quiet --fixed-strings -- "${disablematch_list_entry}"; then + if 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. @@ -339,7 +339,7 @@ set_file_perms() { log error "Cannot parse line: '${line}'" >&2 ## Debugging. du -hs /tmp || true - echo "test -w /tmp: '$(test -w /tmp)'" >&2 || true + 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}" @@ -438,7 +438,7 @@ set_file_perms() { 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 echo "${dpkg_statoverride_list_output}" | grep --quiet --fixed-strings -- "${grep_line}"; then + if 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." @@ -538,9 +538,9 @@ parse_config_folder() { ## receive SIGPIPE, which then fails the pipeline since 'set -o pipefail' is ## set for this script. passwd_file_contents_temp="$(getent passwd)" - echo "${passwd_file_contents_temp}" | tee "${store_dir}/private/passwd" >/dev/null + echo -- "${passwd_file_contents_temp}" | tee -- "${store_dir}/private/passwd" >/dev/null group_file_contents_temp="$(getent group)" - echo "${group_file_contents_temp}" | tee "${store_dir}/private/group" >/dev/null + 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")" @@ -603,9 +603,9 @@ spare() { if test "${remove_file}" = "${file_name}"; then verbose="--verbose" remove_one=true - echo "${remove_one}" | tee "${store_dir}/remove_one" >/dev/null + echo -- "${remove_one}" | tee -- "${store_dir}/remove_one" >/dev/null else - echo "false" | tee "${store_dir}/remove_one" >/dev/null + echo -- "false" | tee -- "${store_dir}/remove_one" >/dev/null continue fi fi @@ -674,7 +674,7 @@ check_root(){ } usage(){ - echo "Usage: ${0##*/} enable + echo -- "Usage: ${0##*/} enable ${0##*/} disable [FILE|all] Examples: From 05504b9ab251ae6e48b5d28eb5fdcd12d730ea8a Mon Sep 17 00:00:00 2001 From: Patrick Schleizer Date: Fri, 26 Jul 2024 08:40:10 -0400 Subject: [PATCH 41/48] minor --- usr/bin/permission-hardener | 1 + 1 file changed, 1 insertion(+) diff --git a/usr/bin/permission-hardener b/usr/bin/permission-hardener index a7f02f4..c12f13f 100755 --- a/usr/bin/permission-hardener +++ b/usr/bin/permission-hardener @@ -307,6 +307,7 @@ add_nosuid_statoverride_entry() { 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 From d144f68d1a06a1153c4178b2f6ba9643dededbb8 Mon Sep 17 00:00:00 2001 From: Patrick Schleizer Date: Fri, 26 Jul 2024 08:46:08 -0400 Subject: [PATCH 42/48] output --- usr/bin/permission-hardener | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/usr/bin/permission-hardener b/usr/bin/permission-hardener index c12f13f..72f404f 100755 --- a/usr/bin/permission-hardener +++ b/usr/bin/permission-hardener @@ -91,7 +91,12 @@ output_stat(){ readarray -t arr <<< "${stat_output_newlined}" if test "${#arr[@]}" = 0; then - log error "Array is empty. Stat output: '${stat_output}', line: '${line}'" >&2 + log error "Array is empty. +File name: '${file_name}' +Stat output: '${stat_output}' +stat_output_newlined: '${stat_output_newlined}' +line: '${line}' +" >&2 return 1 fi @@ -102,10 +107,11 @@ output_stat(){ if [ ! "$file_name" = "$file_name_from_stat" ]; then log error "\ -function ${FUNCNAME[1]}: File name is different from file name received from stat: -File name '${file_name}' -File name from stat: '${file_name_from_stat}'" >&2 +File name: '${file_name}' +File name from stat: '${file_name_from_stat}' +line: '${line}' +" >&2 return 1 fi From 0e661bc688c7222840c9d83fb3ccab6549b3ac11 Mon Sep 17 00:00:00 2001 From: Patrick Schleizer Date: Fri, 26 Jul 2024 08:49:14 -0400 Subject: [PATCH 43/48] output --- usr/bin/permission-hardener | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/usr/bin/permission-hardener b/usr/bin/permission-hardener index 72f404f..bdc5233 100755 --- a/usr/bin/permission-hardener +++ b/usr/bin/permission-hardener @@ -91,7 +91,7 @@ output_stat(){ readarray -t arr <<< "${stat_output_newlined}" if test "${#arr[@]}" = 0; then - log error "Array is empty. + log error "Array length is 0. File name: '${file_name}' Stat output: '${stat_output}' stat_output_newlined: '${stat_output_newlined}' From 82d401a7de58b74448113bed36c8f0cc073c7f82 Mon Sep 17 00:00:00 2001 From: Patrick Schleizer Date: Fri, 26 Jul 2024 08:52:42 -0400 Subject: [PATCH 44/48] sanity test --- usr/bin/permission-hardener | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/usr/bin/permission-hardener b/usr/bin/permission-hardener index bdc5233..dae1fd1 100755 --- a/usr/bin/permission-hardener +++ b/usr/bin/permission-hardener @@ -87,7 +87,28 @@ output_stat(){ 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 From ee037c01a1208b9247c3ae144fa3faa68657ffdb Mon Sep 17 00:00:00 2001 From: Patrick Schleizer Date: Fri, 26 Jul 2024 08:58:44 -0400 Subject: [PATCH 45/48] Skip file names starting with '--', because this would be interpreted by dpkg-statoverride as an option. --- usr/bin/permission-hardener | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/usr/bin/permission-hardener b/usr/bin/permission-hardener index dae1fd1..61298e6 100755 --- a/usr/bin/permission-hardener +++ b/usr/bin/permission-hardener @@ -79,6 +79,11 @@ output_stat(){ block_newlines file "${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 From 7e0f1a87010674c63963b70c87e903cf27b288ef Mon Sep 17 00:00:00 2001 From: Patrick Schleizer Date: Fri, 26 Jul 2024 09:08:04 -0400 Subject: [PATCH 46/48] dpkg-statoverride can actually handle '--file-name'. --- usr/bin/permission-hardener | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/usr/bin/permission-hardener b/usr/bin/permission-hardener index 61298e6..e643ada 100755 --- a/usr/bin/permission-hardener +++ b/usr/bin/permission-hardener @@ -79,10 +79,11 @@ output_stat(){ block_newlines file "${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 + ## dpkg-statoverride can acttually 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 From 794f6a25fa87a9d6d796b07ee06b690ea0badc92 Mon Sep 17 00:00:00 2001 From: Patrick Schleizer Date: Fri, 26 Jul 2024 09:08:29 -0400 Subject: [PATCH 47/48] comment --- usr/bin/permission-hardener | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/usr/bin/permission-hardener b/usr/bin/permission-hardener index e643ada..1c7a12e 100755 --- a/usr/bin/permission-hardener +++ b/usr/bin/permission-hardener @@ -79,7 +79,7 @@ output_stat(){ block_newlines file "${file_name}" - ## dpkg-statoverride can acttually handle '--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 From 6bbf176e3b91f842cf4cdeaf8cb1f4c60e159a0c Mon Sep 17 00:00:00 2001 From: Patrick Schleizer Date: Fri, 26 Jul 2024 09:33:45 -0400 Subject: [PATCH 48/48] consider end-of-options for `find` --- usr/bin/permission-hardener | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/usr/bin/permission-hardener b/usr/bin/permission-hardener index 1c7a12e..6dab5c6 100755 --- a/usr/bin/permission-hardener +++ b/usr/bin/permission-hardener @@ -174,7 +174,7 @@ add_nosuid_statoverride_entry() { while IFS="" read -r -d "" dummy_line; do log info "Test would parse line: '${dummy_line}'" should_be_counter=$((should_be_counter + 1)) - done < <(find "${fso_to_process}" -perm /u=s,g=s -print0) + done < <(printf -- "${fso_to_process}" | find -files0-from - -perm /u=s,g=s -print0) local line while IFS="" read -r -d "" file_name; do @@ -328,7 +328,7 @@ add_nosuid_statoverride_entry() { ## /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 - done < <(find "${fso_to_process}" -perm /u=s,g=s -print0) + done < <(printf -- "${fso_to_process}" | find -files0-from - -perm /u=s,g=s -print0) ## Sanity test. if test ! "${should_be_counter}" = "${counter_actual}"; then