Re: [RFC][PATCH 4/8] ima: Add digest_cache_measure and digest_cache_appraise boot-time policies

[Roberto Sassu <roberto.sassu@huaweicloud.com>]

On Thu, 2024-03-07 at 15:17 -0500, Mimi Zohar wrote:
> On Wed, 2024-02-14 at 15:35 +0100, Roberto Sassu wrote:
> > From: Roberto Sassu <roberto.sassu@huawei.com>
> > 
> > Specify the 'digest_cache_measure' boot-time policy with 'ima_policy=' in
> > the kernel command line
> 
> The 'built-in' policies may be specified on the boot command line.  Please
> update Subject line, to user the term "built-in" as well as here.

Ok, will do.

> >  to add the following rule at the beginning of the
> > IMA policy, before other rules:
> 
> Comments below...
> 
> > 
> > measure func=DIGEST_LIST_CHECK pcr=12
> > 
> > which will measure digest lists into PCR 12 (or the value in
> > CONFIG_IMA_DIGEST_CACHE_MEASURE_PCR_IDX).
> > 
> > 'digest_cache_measure' also adds 'digest_cache=content pcr=12' to the other
> > measure rules, if they have a compatible IMA hook. The PCR value still
> > comes from CONFIG_IMA_DIGEST_CACHE_MEASURE_PCR_IDX.
> > 
> > Specify 'digest_cache_appraise' to add the following rule at the beginning,
> > before other rules:
> > 
> > appraise func=DIGEST_LIST_CHECK appraise_type=imasig|modsig
> > 
> > which will appraise digest lists with IMA signatures or module-style
> > appended signatures.
> > 
> > 'digest_cache_appraise' also adds 'digest_cache=content' to the other
> > appraise rules, if they have a compatible IMA hook.
> > 
> > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> > ---
> >  .../admin-guide/kernel-parameters.txt         | 15 ++++++-
> >  security/integrity/ima/Kconfig                | 10 +++++
> >  security/integrity/ima/ima_policy.c           | 45 +++++++++++++++++++
> >  3 files changed, 69 insertions(+), 1 deletion(-)
> 
> [...]
>  
> > @@ -971,6 +1006,16 @@ void __init ima_init_policy(void)
> >  {
> >  	int build_appraise_entries, arch_entries;
> >  
> > +	/*
> > +	 * We need to load digest cache rules at the beginning, to avoid dont_
> > +	 * rules causing ours to not be reached.
> > +	 */
> 
> "lockdown" trusts IMA to measure and appraise kernel modules, if the rule
> exists.  Placing the digest_cache first breaks this trust.

The new rules don't prevent other rules to be reached, since they are
'do' and not 'don_t' rules.

If the kernel reads a file with file ID READING_MODULE, that would
still be matched by rules with 'func=MODULE_CHECK', even if there are
rules with 'func=DIGEST_LIST_CHECK', which will be instead matched when
there is a kernel read with file ID READING_DIGEST_LIST.

We can talk about the rule modification. Speaking of appraising kernel
modules, setting 'ima_policy=digest_cache_appraise' in the kernel
command line would have the effect of changing:

appraise func=MODULE_CHECK appraise_type=imasig|modsig

to:

appraise func=DIGEST_LIST_CHECK appraise_type=imasig|modsig
appraise func=MODULE_CHECK appraise_type=imasig|modsig digest_cache=content

The effect of this would be that, if the kernel does not have
security.ima or an appended signature, appraisal will be still
successful by verifying the signature (in the xattr or appended) of the
digest list, and looking up the digest of the kernel module in that
digest list.

> From a trusted and secure boot perspective, the architecture specific policy
> rules should not be ignored.

I'm still missing how the architecture-specific policy would be
ignored.

> Putting the digest_cache before any other rules
> will limit others from being able to use digest_cache.

Sorry, didn't understand.

Let me just remark that measuring/appraising a digest list is a
necessary condition for using the digest cache built from that digest
list.

Not doing that has the same effect of a negative digest lookup, even if
that digest was in the digest list.

> Instead of putting the digest_cache_{measure,appraise} built-in policies first,
> skip loading the dont_measure_rules.

It does not seem a good idea. We still want to avoid
measurements/appraisal in the pseudo filesystems.

Roberto

> Mimi
> 
> > +	if (ima_digest_cache_measure)
> > +		add_rules(&measure_digest_cache_rule, 1, IMA_DEFAULT_POLICY);
> > +
> > +	if (ima_digest_cache_appraise)
> > +		add_rules(&appraise_digest_cache_rule, 1, IMA_DEFAULT_POLICY);
> > +
> >  	/* if !ima_policy, we load NO default rules */
> >  	if (ima_policy)
> >  		add_rules(dont_measure_rules, ARRAY_SIZE(dont_measure_rules),