linux-kernel - Re: [RFC][PATCH 8/8] ima: Detect if digest cache changed since last measurement/appraisal

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ddb1c28356fb8a4dcca9bff6dc206802d7981bb8.camel@linux.ibm.com>
Date: Fri, 08 Mar 2024 12:35:05 -0500
From: Mimi Zohar <zohar@...ux.ibm.com>
To: Roberto Sassu <roberto.sassu@...weicloud.com>, corbet@....net,
        dmitry.kasatkin@...il.com, eric.snowberg@...cle.com,
        paul@...l-moore.com, jmorris@...ei.org, serge@...lyn.com
Cc: linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org,
        linux-integrity@...r.kernel.org, linux-security-module@...r.kernel.org,
        wufan@...ux.microsoft.com, pbrobinson@...il.com, zbyszek@...waw.pl,
        hch@....de, mjg59@...f.ucam.org, pmatilai@...hat.com, jannh@...gle.com,
        dhowells@...hat.com, jikos@...nel.org, mkoutny@...e.com,
        ppavlu@...e.com, petr.vorel@...il.com, petrtesarik@...weicloud.com,
        mzerqung@...inter.de, kgold@...ux.ibm.com,
        Roberto Sassu <roberto.sassu@...wei.com>
Subject: Re: [RFC][PATCH 8/8] ima: Detect if digest cache changed since last
 measurement/appraisal

Hi Roberto,

> b/security/integrity/ima/ima_main.c
> index a66522a22cbc..e1b2f5737753 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -301,6 +301,15 @@ static int process_measurement(struct file *file, const
> struct cred *cred,
>  		}
>  	}
>  
> +	/* Check if digest cache changed since last measurement/appraisal. */
> +	if (iint->digest_cache &&
> +	    digest_cache_changed(inode, iint->digest_cache)) {
> +		iint->flags &= ~IMA_DONE_MASK;
> +		iint->measured_pcrs = 0;
> +		digest_cache_put(iint->digest_cache);
> +		iint->digest_cache = NULL;
> +	}
> +
>  	/* Determine if already appraised/measured based on bitmask
>  	 * (IMA_MEASURE, IMA_MEASURED, IMA_XXXX_APPRAISE, IMA_XXXX_APPRAISED,
>  	 *  IMA_AUDIT, IMA_AUDITED)
> @@ -371,8 +380,15 @@ static int process_measurement(struct file *file, const
> struct cred *cred,
>  	 * Since we allow IMA policy rules without func=, we have to enforce
>  	 * this restriction here.
>  	 */
> -	if (rc == 0 && policy_mask && func != DIGEST_LIST_CHECK)
> -		digest_cache = digest_cache_get(file_dentry(file));
> +	if (rc == 0 && policy_mask && func != DIGEST_LIST_CHECK) {
> +		if (!iint->digest_cache) {
> +			/* Released by ima_iint_free(). */
> +			digest_cache = digest_cache_get(file_dentry(file));
> +			iint->digest_cache = digest_cache;
> +		} else {
> +			digest_cache = iint->digest_cache;
> +		}

Simple cleanup:
		if (!iint->digest_cache)
			iint->digest_cache =digest_cache_get(file_dentry(file));

		digest_cache = iint->digest_cache;

> +	}
>  
>  	if (digest_cache) {
>  		found = digest_cache_lookup(file_dentry(file), digest_cache,
> @@ -386,8 +402,6 @@ static int process_measurement(struct file *file, const
> struct cred *cred,
>  			if (verif_mask_ptr)
>  				allow_mask = policy_mask & *verif_mask_ptr;
>  		}
> -
> -		digest_cache_put(digest_cache);

Keeping a reference to the digest_cache list for each file in the iint cache
until the file is re-accessed, might take a while to free.

I'm wondering if it necessary to keep a reference to the digest_cache.  Or is it
possible to just compare the existing iint->digest_cache pointer with the
current digest_cache pointer?

thanks,

Mimi

>  	}
>  
>  	if (action & IMA_MEASURE)