From 40b3c9f62c840bbf73ea3331c6757bc35cb157c4 Mon Sep 17 00:00:00 2001 From: Rich Megginson Date: Tue, 28 Nov 2023 13:57:57 -0700 Subject: [PATCH 1/2] refactor: several role improvements When adding files to the trustdb, wait until the server recognizes that the trustdb is updated before returning. Do not use compiled C programs for testing trust, just use executable shell scripts. Set vars such as __fapolicyd_trust_supported et. al. based on os family and version rather than using distribution version. One reason is that CentOS Stream was excluded from many features but it should be included. Clean up after tests Other minor improvements Signed-off-by: Rich Megginson --- README.md | 4 +- defaults/main.yml | 17 ++-- examples/simple.yml | 1 - tasks/main.yml | 106 +++++++++++++++++-------- tasks/set_vars.yml | 32 ++++++++ templates/fapolicyd.conf.j2 | 4 +- tests/tests_trusted_execution.yml | 125 ++++++++++++------------------ vars/main.yml | 4 +- 8 files changed, 168 insertions(+), 125 deletions(-) diff --git a/README.md b/README.md index 9414d81..991a7b0 100644 --- a/README.md +++ b/README.md @@ -20,8 +20,8 @@ ansible-galaxy collection install -vv -r meta/collection-requirements.yml ### fapolicyd_setup_enable_service -Default `false` - if set to `true` the variable makes the service started -and enabled fapolicyd service after successful deployment. +Default `true` - if set to `false` the variable makes the service stopped +and disabled. ### fapolicyd_setup_trust diff --git a/defaults/main.yml b/defaults/main.yml index 0d1fe12..cbcbe08 100644 --- a/defaults/main.yml +++ b/defaults/main.yml @@ -4,25 +4,24 @@ # Enable or disable service after configuration # to allow testing and verification before use. # NB. See ima_evm_setup if planning to use IMA. -fapolicyd_setup_enable_service: false +fapolicyd_setup_enable_service: true # trust list for fapolicyd configuration file # default "rpmdb,file" -fapolicyd_setup_trust: "{{ '' if ansible_facts.distribution_version is - version('8.2', '<=') else 'rpmdb,file' }}" +fapolicyd_setup_trust: "{{ '' if __fapolicyd_trust_supported + else 'rpmdb,file' }}" # set integrity # default "none" # can be "none", "size", "sha256", "ima" # in case of ima, kernel's IMA has to be setup correctly -fapolicyd_setup_integrity: "{{ '' if ansible_facts.distribution_version is - version('8.3', '<=') else 'none' }}" +fapolicyd_setup_integrity: "{{ '' if __fapolicyd_integrity_supported + else 'none' }}" # set permissive mode fapolicyd_setup_permissive: false -# fapolicyd trust file managament +# fapolicyd trust file management # list of trusted files - -fapolicyd_add_trusted_file: "{{ '' if ansible_facts.distribution_version is - version('8.2', '<=') else [] }}" +fapolicyd_add_trusted_file: "{{ '' if __fapolicyd_trustfiles_supported + else [] }}" diff --git a/examples/simple.yml b/examples/simple.yml index cf06910..04c84ab 100644 --- a/examples/simple.yml +++ b/examples/simple.yml @@ -3,7 +3,6 @@ - name: Example fapolicyd role invocation hosts: all vars: - fapolicyd_setup_enable_service: true fapolicyd_setup_integrity: sha256 fapolicyd_setup_trust: rpmdb,file fapolicyd_add_trusted_file: diff --git a/tasks/main.yml b/tasks/main.yml index 1432222..c3d1c53 100644 --- a/tasks/main.yml +++ b/tasks/main.yml @@ -8,8 +8,7 @@ - Only Enterprise Linux >= 8.1 and Fedora are supported - System - {{ ansible_facts.os_family }} - Version - {{ ansible_facts.distribution_version }} - when: (ansible_facts.os_family != "RedHat") or - (ansible_facts.distribution_version is version("8.1", "<")) + when: not __fapolicyd_supported - name: Check trust compatibility fail: @@ -19,7 +18,7 @@ ignore_errors: true when: - fapolicyd_setup_trust | length > 0 - - ansible_facts.distribution_version is version("8.2", "<=") + - not __fapolicyd_trust_supported register: __failed_check_trust - name: Check integrity compatibility @@ -30,7 +29,7 @@ ignore_errors: true when: - fapolicyd_setup_integrity | length > 0 - - ansible_facts.distribution_version is version("8.3", "<=") + - not __fapolicyd_integrity_supported register: __failed_check_integrity - name: Check trust files compatibility @@ -41,7 +40,7 @@ ignore_errors: true when: - fapolicyd_add_trusted_file | length > 0 - - ansible_facts.distribution_version is version("8.3", "<=") + - not __fapolicyd_trustfiles_supported register: __failed_check_trusted_file - name: Check failed conditions @@ -52,18 +51,13 @@ - name: Install fapolicyd packages package: - name: "{{ __fapolicyd_packages }}" + name: "{{ __pkgs }}" state: present use: "{{ (__fapolicyd_is_ostree | d(false)) | ternary('ansible.posix.rhel_rpm_ostree', omit) }}" - -- name: Install fapolicyd-selinux packages - package: - name: "{{ __fapolicyd_selinux_packages }}" - state: present - use: "{{ (__fapolicyd_is_ostree | d(false)) | - ternary('ansible.posix.rhel_rpm_ostree', omit) }}" - when: ansible_facts.distribution_version is version("8.3", ">=") + vars: + __pkgs: "{{ __fapolicyd_packages + (__fapolicyd_selinux_packages + if __fapolicyd_selinux_supported else []) }}" - name: Copy fapolicyd configuration file template: @@ -79,39 +73,85 @@ check_mode: false changed_when: false when: - - ansible_facts.distribution_version is version("8.6", ">=") + - __fapolicyd_configcheck_supported | bool - __fapolicy_conf is changed -- name: Trustdb cleanup - command: fapolicyd-cli --file delete / - when: ansible_facts.distribution_version is version("8.3", ">=") - changed_when: true - failed_when: false - -- name: Add file to trustdb - command: fapolicyd-cli --file add {{ item | quote }} - loop: "{{ (fapolicyd_add_trusted_file is string) | - ternary([fapolicyd_add_trusted_file], fapolicyd_add_trusted_file) }}" - when: - - fapolicyd_add_trusted_file | length > 0 - - ansible_facts.distribution_version is version("8.3", ">=") - changed_when: true - - name: Start fapolicyd service service: name: "{{ __fapolicyd_services }}" - state: restarted + state: started enabled: true when: fapolicyd_setup_enable_service | bool ignore_errors: true + register: __fapolicyd_start + +- name: Restart fapolicyd service + service: + name: "{{ __fapolicyd_services }}" + state: restarted + enabled: true + when: + - fapolicyd_setup_enable_service | bool + - __fapolicy_conf is changed + ignore_errors: true register: __fapolicyd_restart - name: Check fapolicyd logs command: journalctl -n5 -u {{ __fapolicyd_services | quote }} register: __fapolicyd_results changed_when: false - when: __fapolicyd_restart is failed - failed_when: __fapolicyd_restart is failed + when: __fapolicyd_start is failed or __fapolicyd_restart is failed + failed_when: __fapolicyd_start is failed or __fapolicyd_restart is failed + +- name: Trustdb cleanup + command: fapolicyd-cli --file delete / + changed_when: true + failed_when: false + +- name: Add file to trustdb + command: fapolicyd-cli --file add {{ item | quote }} + loop: "{{ (fapolicyd_add_trusted_file is string) | + ternary([fapolicyd_add_trusted_file], fapolicyd_add_trusted_file) }}" + when: item | length > 0 + changed_when: true + +# The problem is that there is a race condition between calling +# fapolicyd-cli --update and when fapolicyd will actually enforce +# the policyd - so we have to look for the 'Updated' message in +# the fapolicyd logs. Also - I don't think we can move this into +# a script, because that script might be excluded by policy! +- name: Update fapolicyd db + when: fapolicyd_setup_enable_service | bool + shell: + cmd: | + set -euo pipefail + # get current journal cursor + cursor="" + while [ -z "$cursor" ]; do + sleep 1 + cursor="$(journalctl -u fapolicyd -n 0 --show-cursor | + awk '/^-- cursor:/ {print $3}')" || : + done + # update trustdb + fapolicyd-cli --update + # wait until we see the message 'Updated' - wait up to 30 seconds + starttime="$(date +%s)" + waittime=30 # seconds + endtime="$(expr "$starttime" + "$waittime")" + set +o pipefail # the read will always return a failure code at EOF + journalctl -u fapolicyd --no-tail -f --after-cursor "$cursor" | \ + while read -r line; do + if [[ "$line" =~ fapolicyd[^:\ ]*:\ Updated$ ]]; then + echo SUCCESS: trustdb is updated + exit 0 + fi + curtime="$(date +%s)" + if [ "$curtime" -gt "$endtime" ]; then + echo ERROR: trustdb not updated after "$waittime" seconds - exiting + exit 1 + fi + done + changed_when: true - name: Making sure fapolicyd does not run if it was set so service: diff --git a/tasks/set_vars.yml b/tasks/set_vars.yml index a57891f..23e1402 100644 --- a/tasks/set_vars.yml +++ b/tasks/set_vars.yml @@ -31,3 +31,35 @@ vars: __vars_file: "{{ role_path }}/vars/{{ item }}" when: __vars_file is file + +# fapolicyd only supported on EL +# NOTE - some RedHat os_family like CentOS stream and Fedora +# only have major version - in that case, assume the version is +# compatible with the highest released minor version of the +# major version e.g. CentOS stream 8 is the same as EL 8.9 +# or higher +- name: Set fapolicyd feature facts for OS versions + vars: + # use temp vars for readability + __major_ver: "{{ ansible_facts['distribution_major_version'] }}" + __ver: "{{ ansible_facts['distribution_version'] }}" + __distro_ver: "{{ (__major_ver == __ver) | + ternary(__major_ver ~ '.9999', __ver) }}" + __is_redhat: "{{ ansible_facts['os_family'] == 'RedHat' }}" + set_fact: + __fapolicyd_supported: "{{ __is_redhat and + __distro_ver is version('8.1', '>=') }}" + __fapolicyd_trust_supported: "{{ __is_redhat and + __distro_ver is version('8.3', '>=') }}" + __fapolicyd_integrity_supported: "{{ __is_redhat and + __distro_ver is version('8.4', '>=') }}" + __fapolicyd_trustfiles_supported: "{{ __is_redhat and + __distro_ver is version('8.4', '>=') }}" + __fapolicyd_selinux_supported: "{{ __is_redhat and + __distro_ver is version('8.3', '>=') }}" + __fapolicyd_configcheck_supported: "{{ __is_redhat and + __distro_ver is version('8.6', '>=') }}" + __fapolicyd_watch_fs_supported: "{{ __is_redhat and + __distro_ver is version('8.2', '>=') }}" + __fapolicyd_syslog_format_supported: "{{ __is_redhat and + __distro_ver is version('8.3', '>=') }}" diff --git a/templates/fapolicyd.conf.j2 b/templates/fapolicyd.conf.j2 index 4d1d93a..3b39264 100644 --- a/templates/fapolicyd.conf.j2 +++ b/templates/fapolicyd.conf.j2 @@ -15,7 +15,7 @@ detailed_report = 1 db_max_size = 50 subj_cache_size = 1549 obj_cache_size = 8191 -{% if ansible_facts.distribution_version is version("8.2", ">=") %} +{% if __fapolicyd_watch_fs_supported %} watch_fs = ext2,ext3,ext4,tmpfs,xfs,vfat,iso9660,btrfs {% endif %} @@ -23,7 +23,7 @@ watch_fs = ext2,ext3,ext4,tmpfs,xfs,vfat,iso9660,btrfs trust = {{ fapolicyd_setup_trust }} {% endif %} -{% if ansible_facts.distribution_version is version("8.3", ">=") %} +{% if __fapolicyd_syslog_format_supported %} syslog_format = rule,dec,perm,auid,pid,exe,:,path,ftype,trust {% endif %} diff --git a/tests/tests_trusted_execution.yml b/tests/tests_trusted_execution.yml index 072f953..6a2f7ec 100644 --- a/tests/tests_trusted_execution.yml +++ b/tests/tests_trusted_execution.yml @@ -4,14 +4,22 @@ tasks: - name: Run tests vars: - __test_fpd_binaries_dir: >- - {{ __fapolicyd_tmpdir.path }}/executable_binaries - __test_fpd_source_dir: "{{ __test_fpd_binaries_dir }}/source" - __test_fpd_source_file: "{{ __test_fpd_source_dir }}/main.c" - __test_fpd_exe1: "{{ __test_fpd_binaries_dir }}/exe1" - __test_fpd_exe2: "{{ __test_fpd_binaries_dir }}/exe2" + __test_fpd_exec_dir: >- + {{ __fapolicyd_tmpdir.path }}/executables + __test_fpd_exe1: "{{ __test_fpd_exec_dir }}/exe1" + __test_fpd_exe2: "{{ __test_fpd_exec_dir }}/exe2" __test_fpd_user: fapolicyd_test1_user block: + # NOTE - to debug a failing execution: + # - ensure the user has permission to read/execute the file + # and all directories leading to the file + # use a shell: like this: + # shell: | + # systemctl stop fapolicyd + # fapolicyd --debug-deny > /var/log/fapolicyd.log 2>&1 & + # run the command that causes the deny + # kill %1 + # cat /var/log/fapolicyd.log - name: Create temp test directory tempfile: path: /var/tmp @@ -19,57 +27,26 @@ state: directory register: __fapolicyd_tmpdir - - name: Create directories for executable binaries and source files + - name: Create directories for tests file: path: "{{ item }}" state: directory mode: "0755" loop: - "{{ __fapolicyd_tmpdir.path }}" - - "{{ __test_fpd_binaries_dir }}" - - "{{ __test_fpd_source_dir }}" + - "{{ __test_fpd_exec_dir }}" - - name: Create C source code (binary1) + - name: Create shell executables copy: content: | - int main(void) { - return 0; - } - dest: "{{ __test_fpd_source_file }}" - mode: "0644" - - - name: Determine if system is ostree and set flag - when: not __fapolicyd_is_ostree is defined - block: - - name: Check if system is ostree - stat: - path: /run/ostree-booted - register: __ostree_booted_stat - - - name: Set flag to indicate system is ostree - set_fact: - __fapolicyd_is_ostree: "{{ __ostree_booted_stat.stat.exists }}" - - - name: Install GCC and glibc-devel - package: - name: - - gcc - - glibc-devel - state: present - use: "{{ (__fapolicyd_is_ostree | d(false)) | - ternary('ansible.posix.rhel_rpm_ostree', omit) }}" - - - name: Compile C programs (exe1) - command: >- - gcc -o {{ __test_fpd_exe1 | quote }} - {{ __test_fpd_source_file | quote }} - changed_when: true - - - name: Compile C programs (exe2) - command: >- - gcc -g -o {{ __test_fpd_exe2 | quote }} - {{ __test_fpd_source_file | quote }} - changed_when: true + #/bin/bash + # this is item {{ item }} + exit 0 + dest: "{{ item }}" + mode: "0755" + loop: + - "{{ __test_fpd_exe1 }}" + - "{{ __test_fpd_exe2 }}" - name: Create a new user user: @@ -81,7 +58,6 @@ include_role: name: linux-system-roles.fapolicyd vars: - fapolicyd_setup_enable_service: true fapolicyd_setup_integrity: sha256 fapolicyd_setup_trust: rpmdb,file fapolicyd_add_trusted_file: @@ -119,13 +95,13 @@ changed_when: false failed_when: run_exe.rc != 126 - - name: Delete binary exe1 from trustdb - command: fapolicyd-cli -f delete {{ __test_fpd_exe1 | quote }} - changed_when: true - - - name: Update trustdb - command: fapolicyd-cli --update - changed_when: true + - name: Run the role again without test file + include_role: + name: linux-system-roles.fapolicyd + vars: + fapolicyd_setup_integrity: sha256 + fapolicyd_setup_trust: rpmdb,file + fapolicyd_add_trusted_file: [] - name: Run untrusted exe1 after removing from trustdb command: >- @@ -134,21 +110,13 @@ register: run_exe changed_when: false failed_when: run_exe.rc != 126 - - - name: Add binary exe1 to trustdb - command: fapolicyd-cli -f add {{ __test_fpd_exe1 | quote }} - changed_when: true - - - name: Update trustdb - command: fapolicyd-cli --update - changed_when: true - - - name: Run trusted exe1 - command: >- - su - {{ __test_fpd_user | quote }} -c - {{ __test_fpd_exe1 | quote }} - changed_when: true always: + - name: Shutdown fapolicyd + service: + name: fapolicyd + state: stopped + enabled: false + - name: Clean up temp directory file: path: "{{ __fapolicyd_tmpdir.path }}" @@ -158,9 +126,14 @@ user: name: "{{ __test_fpd_user }}" state: absent - - - name: Shutdown fapolicyd - service: - name: fapolicyd - state: stopped - enabled: false + register: __result + until: __result is success + ignore_errors: true # noqa ignore-errors + + # when user removal fails, it seems some process is + # still using the user - see which one it is + - name: Debug test user removal failure + command: ps -ef + when: __result is failed + failed_when: __result is failed + changed_when: false diff --git a/vars/main.yml b/vars/main.yml index 275c581..c07c20e 100644 --- a/vars/main.yml +++ b/vars/main.yml @@ -10,8 +10,8 @@ __fapolicyd_services: fapolicyd.service __fapolicyd_dir: /etc/fapolicyd __fapolicyd_conf: fapolicyd.conf -__fapolicyd_packages: fapolicyd -__fapolicyd_selinux_packages: fapolicyd-selinux +__fapolicyd_packages: [fapolicyd] +__fapolicyd_selinux_packages: [fapolicyd-selinux] # ansible_facts required by the role __fapolicyd_required_facts: From df13c2cef909ef7b9832bf6a7b14ce58c63bdbdd Mon Sep 17 00:00:00 2001 From: Rich Megginson Date: Mon, 4 Dec 2023 18:02:01 -0700 Subject: [PATCH 2/2] systemctl restart fapolicyd is the only reliable way --- tasks/main.yml | 38 +++++++++++++++++++++++--------------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/tasks/main.yml b/tasks/main.yml index c3d1c53..1a65556 100644 --- a/tasks/main.yml +++ b/tasks/main.yml @@ -115,11 +115,16 @@ when: item | length > 0 changed_when: true -# The problem is that there is a race condition between calling -# fapolicyd-cli --update and when fapolicyd will actually enforce -# the policyd - so we have to look for the 'Updated' message in -# the fapolicyd logs. Also - I don't think we can move this into -# a script, because that script might be excluded by policy! +# The problem is that there is a race condition between calling `systemctl +# restart fapolicyd`` and when fapolicyd will actually enforce the policy - so +# we have to look for the right string in the fapolicyd logs. Also - I don't +# think we can move this into a script, because that script might be excluded by +# policy! +# NOTE: I tried using `fapolicyd-cli --update` as recommended by the +# documentation but it does not seem to work in all cases e.g. on RHEL 8.8 if +# you are deleting entries but not adding entries, it seems to do nothing - the +# only reliable way to update the trustdb is to restart the daemon and check for +# "fapolicyd[...]: Starting to listen for events" in the journald output - name: Update fapolicyd db when: fapolicyd_setup_enable_service | bool shell: @@ -132,25 +137,28 @@ cursor="$(journalctl -u fapolicyd -n 0 --show-cursor | awk '/^-- cursor:/ {print $3}')" || : done - # update trustdb - fapolicyd-cli --update - # wait until we see the message 'Updated' - wait up to 30 seconds - starttime="$(date +%s)" + systemctl restart fapolicyd + search_str='fapolicyd[^:\ ]*:\ Starting to listen for events$' + # wait until we see the search_str - wait up to 30 seconds waittime=30 # seconds - endtime="$(expr "$starttime" + "$waittime")" + endtime="$(expr "$(date +%s)" + "$waittime")" set +o pipefail # the read will always return a failure code at EOF journalctl -u fapolicyd --no-tail -f --after-cursor "$cursor" | \ while read -r line; do - if [[ "$line" =~ fapolicyd[^:\ ]*:\ Updated$ ]]; then - echo SUCCESS: trustdb is updated + if [[ "$line" =~ $search_str ]]; then + echo INFO: trustdb is updated exit 0 fi - curtime="$(date +%s)" - if [ "$curtime" -gt "$endtime" ]; then - echo ERROR: trustdb not updated after "$waittime" seconds - exiting + done & pid=$! + while ps -p "$pid"; do + if [ "$(date +%s)" -gt "$endtime" ]; then + echo ERROR: failed to update the trustdb exit 1 fi + sleep 1 done + echo INFO: trustdb is updated + exit 0 # success changed_when: true - name: Making sure fapolicyd does not run if it was set so