[PATCH v2] IMA: Allow only ima-buf template for key measurement

[PATCH v2] IMA: Allow only ima-buf template for key measurement

[Lakshmi Ramasubramanian]

ima-buf is the default IMA template used for all buffer measurements.
Therefore, IMA policy rule for measuring keys need not specify
an IMA template.  But if a template is specified for key measurement
rule then it must be only ima-buf.

Update keys tests to not require a template to be specified for
key measurement rule, but if a template is specified verify it is
only ima-buf.

Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
---
 .../security/integrity/ima/tests/ima_keys.sh  | 20 ++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/testcases/kernel/security/integrity/ima/tests/ima_keys.sh b/testcases/kernel/security/integrity/ima/tests/ima_keys.sh
index c9eef4b68..b9bef4feb 100755
--- a/testcases/kernel/security/integrity/ima/tests/ima_keys.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_keys.sh
@@ -15,8 +15,7 @@ TST_CLEANUP=cleanup
 . ima_setup.sh
 
 FUNC_KEYCHECK='func=KEY_CHECK'
-TEMPLATE_BUF='template=ima-buf'
-REQUIRED_POLICY="^measure.*($FUNC_KEYCHECK.*$TEMPLATE_BUF|$TEMPLATE_BUF.*$FUNC_KEYCHECK)"
+REQUIRED_POLICY="^measure.*$FUNC_KEYCHECK"
 
 setup()
 {
@@ -28,12 +27,23 @@ cleanup()
 	tst_is_num $KEYRING_ID && keyctl clear $KEYRING_ID
 }
 
+check_policy_template()
+{
+	while read line; do
+	if ( echo $line | grep -q 'template=') && ( ! echo $line | grep -q 'template=ima-buf' ); then
+		tst_res TCONF "only template=ima-buf can be specified for KEY_CHECK"
+		return 1
+	fi
+	done < $TST_TMPDIR/policy.txt
+	return 0
+}
+
 check_keys_policy()
 {
 	local pattern="$1"
 
 	if ! grep -E "$pattern" $TST_TMPDIR/policy.txt; then
-		tst_res TCONF "IMA policy must specify $pattern, $FUNC_KEYCHECK, $TEMPLATE_BUF"
+		tst_res TCONF "IMA policy must specify $pattern, $FUNC_KEYCHECK"
 		return 1
 	fi
 	return 0
@@ -49,6 +59,8 @@ test1()
 
 	tst_res TINFO "verify key measurement for keyrings and templates specified in IMA policy"
 
+	check_policy_template || return
+
 	check_keys_policy "$pattern" > $tmp_file || return
 	keycheck_lines=$(cat $tmp_file)
 	keyrings=$(for i in $keycheck_lines; do echo "$i" | grep "keyrings" | \
@@ -101,6 +113,8 @@ test2()
 
 	tst_res TINFO "verify measurement of certificate imported into a keyring"
 
+	check_policy_template || return
+
 	check_keys_policy "$pattern" >/dev/null || return
 
 	KEYRING_ID=$(keyctl newring $keyring_name @s) || \
-- 
2.30.1

Re: [PATCH v2] IMA: Allow only ima-buf template for key measurement

[Petr Vorel]

Hi Lakshmi, Mimi,

> ima-buf is the default IMA template used for all buffer measurements.
> Therefore, IMA policy rule for measuring keys need not specify
> an IMA template.  But if a template is specified for key measurement
> rule then it must be only ima-buf.

> Update keys tests to not require a template to be specified for
> key measurement rule, but if a template is specified verify it is
> only ima-buf.

Reviewed-by: Petr Vorel <pvorel@suse.cz>

Just a double check does it always work without template=ima-buf for all kernel versions?
Or only for kernels with dea87d0889dd ("ima: select ima-buf template for buffer measurement")
i.e. v5.11-rc1 or backport?

Also, don't we want to change also keycheck.policy?
Currently it contains:
measure func=KEY_CHECK keyrings=.ima|.evm|.builtin_trusted_keys|.blacklist|key_import_test template=ima-buf
Do we want to drop template=ima-buf to test the default value? Or have two rules
(one with template=ima-buf, other w/a?)

Mimi, any comment on this?

Kind regards,
Petr

Re: [PATCH v2] IMA: Allow only ima-buf template for key measurement

[Lakshmi Ramasubramanian]

On 3/16/21 7:07 AM, Petr Vorel wrote:
> Hi Lakshmi, Mimi,
> 
>> ima-buf is the default IMA template used for all buffer measurements.
>> Therefore, IMA policy rule for measuring keys need not specify
>> an IMA template.  But if a template is specified for key measurement
>> rule then it must be only ima-buf.
> 
>> Update keys tests to not require a template to be specified for
>> key measurement rule, but if a template is specified verify it is
>> only ima-buf.
> 
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
Thanks Petr.

> 
> Just a double check does it always work without template=ima-buf for all kernel versions?
> Or only for kernels with dea87d0889dd ("ima: select ima-buf template for buffer measurement")
> i.e. v5.11-rc1 or backport?
The above change is required. Prior to this change, template has to be 
specified in the policy, otherwise the default template would be used.

> 
> Also, don't we want to change also keycheck.policy?
> Currently it contains:
> measure func=KEY_CHECK keyrings=.ima|.evm|.builtin_trusted_keys|.blacklist|key_import_test template=ima-buf
> Do we want to drop template=ima-buf to test the default value? Or have two rules
> (one with template=ima-buf, other w/a?)
Good point.

I will send you the v3 patch - with two rules: one with template=buf and 
other without a template, like the following example:

measure func=KEY_CHECK 
keyrings=.builtin_trusted_keys|.blacklist|key_import_test template=ima-buf

measure func=KEY_CHECK keyrings=.ima|.evm

  -lakshmi

> 
> Mimi, any comment on this?
> 
> Kind regards,
> Petr
> 

Re: [PATCH v2] IMA: Allow only ima-buf template for key measurement

[Petr Vorel]

Hi Lakshmi,

> > Just a double check does it always work without template=ima-buf for all kernel versions?
> > Or only for kernels with dea87d0889dd ("ima: select ima-buf template for buffer measurement")
> > i.e. v5.11-rc1 or backport?
> The above change is required. Prior to this change, template has to be
> specified in the policy, otherwise the default template would be used.
The default template is ima-ng, right?
From what you write I understand that "measure func=KEY_CHECK
keyrings=.ima|.evm" will work only on newer kernel, thus we should always use
template=ima-buf as the policy example so that it's working also on that few
kernels between <v5.6,v5.10> (which have IMA key functionality, but not
dea87d0889dd), right?

But we should mention that in the README.md.

Kind regards,
Petr

> > Also, don't we want to change also keycheck.policy?
> > Currently it contains:
> > measure func=KEY_CHECK keyrings=.ima|.evm|.builtin_trusted_keys|.blacklist|key_import_test template=ima-buf
> > Do we want to drop template=ima-buf to test the default value? Or have two rules
> > (one with template=ima-buf, other w/a?)
> Good point.

> I will send you the v3 patch - with two rules: one with template=buf and
> other without a template, like the following example:

> measure func=KEY_CHECK
> keyrings=.builtin_trusted_keys|.blacklist|key_import_test template=ima-buf

> measure func=KEY_CHECK keyrings=.ima|.evm

>  -lakshmi

Re: [PATCH v2] IMA: Allow only ima-buf template for key measurement

[Lakshmi Ramasubramanian]

On 3/16/21 10:21 AM, Petr Vorel wrote:
Hi Petr,

> 
>>> Just a double check does it always work without template=ima-buf for all kernel versions?
>>> Or only for kernels with dea87d0889dd ("ima: select ima-buf template for buffer measurement")
>>> i.e. v5.11-rc1 or backport?
>> The above change is required. Prior to this change, template has to be
>> specified in the policy, otherwise the default template would be used.
> The default template is ima-ng, right?
Yes: ima-ng is the default template.

>>From what you write I understand that "measure func=KEY_CHECK
> keyrings=.ima|.evm" will work only on newer kernel, thus we should always use
> template=ima-buf as the policy example so that it's working also on that few
> kernels between <v5.6,v5.10> (which have IMA key functionality, but not
> dea87d0889dd), right?
Yes: In the kernels between v5.6 and v5.10, ima-buf template needs to be 
specified in the policy for KEY_CHECK.

> 
> But we should mention that in the README.md.
> 
Agreed - will update the README.md

thanks,
  -lakshmi

Re: [PATCH v2] IMA: Allow only ima-buf template for key measurement

[Petr Vorel]

Hi Lakshmi,

> > > > Just a double check does it always work without template=ima-buf for all kernel versions?
> > > > Or only for kernels with dea87d0889dd ("ima: select ima-buf template for buffer measurement")
> > > > i.e. v5.11-rc1 or backport?
> > > The above change is required. Prior to this change, template has to be
> > > specified in the policy, otherwise the default template would be used.
> > The default template is ima-ng, right?
> Yes: ima-ng is the default template.

> > > From what you write I understand that "measure func=KEY_CHECK
> > keyrings=.ima|.evm" will work only on newer kernel, thus we should always use
> > template=ima-buf as the policy example so that it's working also on that few
> > kernels between <v5.6,v5.10> (which have IMA key functionality, but not
> > dea87d0889dd), right?
> Yes: In the kernels between v5.6 and v5.10, ima-buf template needs to be
> specified in the policy for KEY_CHECK.

OK, thus your original version - i.e. don't require template=ima-buf,
but keep it in policy example is the best approach.

> > But we should mention that in the README.md.

> Agreed - will update the README.md
Thanks!

Kind regards,
Petr

> thanks,
>  -lakshmi