Hi Mimi, > On Tue, 2024-12-31 at 11:00 +0100, Petr Vorel wrote: > > > Hi Petr, > > > On Fri, 2024-12-13 at 23:20 +0100, Petr Vorel wrote: > > > [snip] > > > > --- a/testcases/kernel/security/integrity/ima/tests/ima_setup.sh > > > > +++ b/testcases/kernel/security/integrity/ima/tests/ima_setup.sh > > > > @@ -1,7 +1,7 @@ > > > > #!/bin/sh > > > > # SPDX-License-Identifier: GPL-2.0-or-later > > > > # Copyright (c) 2009 IBM Corporation > > > > -# Copyright (c) 2018-2020 Petr Vorel <pvorel@xxxxxxx> > > > > +# Copyright (c) 2018-2024 Petr Vorel <pvorel@xxxxxxx> > > > > # Author: Mimi Zohar <zohar@xxxxxxxxxxxxx> > > > > TST_TESTFUNC="test" > > > > @@ -72,14 +72,20 @@ require_policy_readable() > > > > fi > > > > } > > > > -require_policy_writable() > > > > +check_policy_writable() > > > > { > > > > - local err="IMA policy already loaded and kernel not configured to enable multiple writes to it (need CONFIG_IMA_WRITE_POLICY=y)" > > > > - > > > > - [ -f $IMA_POLICY ] || tst_brk TCONF "$err" > > > > - # CONFIG_IMA_READ_POLICY > > > > + [ -f $IMA_POLICY ] || return 1 > > > > + # workaround for kernels < v4.18 without fix > > > > + # ffb122de9a60b ("ima: Reflect correct permissions for policy") > > > > echo "" 2> log > $IMA_POLICY > > > > - grep -q "Device or resource busy" log && tst_brk TCONF "$err" > > > > + grep -q "Device or resource busy" log && return 1 > > > > + return 0 > > > > +} > > > > + > > > > +require_policy_writable() > > > > +{ > > > > + check_policy_writable || tst_brk TCONF \ > > > > + "IMA policy already loaded and kernel not configured to enable multiple writes to it (need CONFIG_IMA_WRITE_POLICY=y)" > > > > } > > > > check_ima_policy_content() > > > > @@ -158,6 +164,34 @@ print_ima_config() > > > > tst_res TINFO "/proc/cmdline: $(cat /proc/cmdline)" > > > > } > > > > +load_ima_policy() > > > > +{ > > > > + local policy="$(ls $TST_DATAROOT/*.policy 2>/dev/null)" > > > > + > > > > + if [ "$LTP_IMA_LOAD_POLICY" != 1 -a "$policy" -a -f "$policy" ]; then > > > > + tst_res TINFO "NOTE: set LTP_IMA_LOAD_POLICY=1 to load policy for this test" > > > > + return > > > > + fi > > > > + > > > > + if [ -z "$policy" -o ! -f "$policy" ]; then > > > > + tst_res TINFO "no policy for this test" > > > > + LTP_IMA_LOAD_POLICY= > > > > + return > > > > + fi > > > > + > > > > + tst_res TINFO "trying to load '$policy' policy:" > > > > + cat $policy > > > > + if ! check_policy_writable; then > > > > + tst_res TINFO "WARNING: IMA policy already loaded and kernel not configured to enable multiple writes to it (need CONFIG_IMA_WRITE_POLICY=y), reboot required" > > > > + LTP_IMA_LOAD_POLICY= > > > > + return > > > > + fi > > > > + > > > > + cat "$policy" 2> log > $IMA_POLICY > > > > + if grep -q "Device or resource busy" log; then > > > > + tst_brk TBROK "Loading policy failed" > > > > + fi > > > To write to the IMA securityfs policy file, check_policy_writable() used "echo", > > > while here it's using "cat". "cat" fails when signed policies are required. > > > Perhaps add something like: > > > + > > > + if grep -q "write error: Permission denied" log; then > > > + tst_brk TBROK "Loading unsigned policy failed" > > > + fi > > +1, I'll add this extra check to v3. > > I suppose echo "" > /sys/kernel/security/ima/policy does not need this check. > The original method for loading an IMA policy was by cat'ing the policy rules. > Commit 7429b092811f ("ima: load policy using path") introduced the ability of > verifying the integrity of the policy itself. > echo <policy filepath> > /sys/kernel/security/ima/policy Thanks, I completely missed this already quite old method (v4.6). I guess I could use cat < /dev/null > /sys/kernel/security/ima/policy instead of echo "" > /sys/kernel/security/ima/policy Then "write error: Permission denied" check would not be needed, right? > > Do I understand correctly you talk about policy containing func=POLICY_CHECK [1]? > Yes. On a secure boot enabled system, the architecture specific policy might > require the IMA policy itself to be signed. > Snippet from ima_fs.c: > #if IS_ENABLED(CONFIG_INTEGRITY_MACHINE_KEYRING) && > IS_ENABLED(CONFIG_IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY) > "appraise func=POLICY_CHECK appraise_type=imasig", > #endif +1 > > Maybe there could be a test based on example [2]. > > echo /home/user/tmpfile > /sys/kernel/security/ima/policy > > cp tmpfile /sys/kernel/security/ima/policy > > cat tmpfile > /sys/kernel/security/ima/policy > All of the above will load a policy, assuming the policy itself doesn't need to > be signed. Only "echo /home/user/tmpfile > /sys/kernel/security/ima/policy" can > load a signed policy. > Loading a CA key (mokutil), signing (evmctl)[1] and loading (keyctl) an IMA > policy is probably beyond LTP. The purpose of this test would be to detect > whether policies need to be signed. > Going forward what's probably needed is a new package containing a set of pre- > defined sample custom policies, which are signed by the distro. > [1] Directions for signing and loading a custom policy, > https://ima-doc.readthedocs.io/en/latest/ima-utilities.html#sign-and-install-a-custom-policy Hopefully I find time to do some experiments with it soon. Kind regards, Petr > Thanks, > Mimi > > Kind regards, > > Petr > > [1] https://ima-doc.readthedocs.io/en/latest/policy-syntax.html#func-policy-check > > [2] https://ima-doc.readthedocs.io/en/latest/ima-policy.html#runtime-custom-policy > > > > +} > > > Mimi