linux-integrity.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Mimi Zohar <zohar@linux.ibm.com>
To: THOBY Simon <Simon.THOBY@viveris.fr>,
	"dmitry.kasatkin@gmail.com" <dmitry.kasatkin@gmail.com>,
	"linux-integrity@vger.kernel.org"
	<linux-integrity@vger.kernel.org>,
	BARVAUX Didier <Didier.BARVAUX@viveris.fr>
Subject: Re: [PATCH v3 2/4] IMA: add support to restrict the hash algorithms used for file appraisal
Date: Tue, 27 Jul 2021 11:29:26 -0400	[thread overview]
Message-ID: <1664c5d77e148eb387717428c55d2ef4f2482732.camel@linux.ibm.com> (raw)
In-Reply-To: <20210727102307.552052-3-simon.thoby@viveris.fr>

Hi Simon,

On Tue, 2021-07-27 at 10:23 +0000, THOBY Simon wrote:
> This patch plugs in support for restricting the hash algorithms
> accepted for protecting the security.ima xattr when appraising
> files.

The first sentence should provide the motivation for the patch.  For
example, start this paragraph by saying that the hash algorithms are
currently not restricted.  Then continue with "Restrict ..." or maybe
"Provide the plumbing to restrict ...".

> 
> Each ima policy rule can have a list of allowed hash algorithms,
> and a file matching the policy but whose IMA hash is
> not explicitly in the list will not pass appraisal.

Belongs in the patch associated with the IMA policy "appraise_hash"
rule option.
> 
> This do not apply only to IMA in hash mode, it also works with digital
> signatures, in which case it checks that the hash (which was then
> signed by the trusted private key) have been generated with one of
> the algortihms whitelisted for this specific rule.

Instead of phrasing this paragraph in the negative, the content could
be combined with the first paragraph in the positive.   For example,
"Restrict the permitted set of hash algorithms used for verifying file
signatures stored in security.ima xattr."

> 
> Signed-off-by: Simon Thoby <simon.thoby@viveris.fr>
> ---

<snip>

> @@ -327,6 +329,20 @@ static int process_measurement(struct file *file, const struct cred *cred,
>  
>  	hash_algo = ima_get_hash_algo(xattr_value, xattr_len);
>  
> +	/* Ensure that the digest was generated using an allowed algorithm */
> +	if (appraisal_allowed_hashes &&
> +	    !(appraisal_allowed_hashes & (1U << hash_algo))) {
> +		rc = -EACCES;
> +
> +		if (!pathbuf)	/* ima_rdwr_violation possibly pre-fetched */
> +			pathname = ima_d_path(&file->f_path, &pathbuf, filename);
> +
> +		integrity_audit_msg(AUDIT_INTEGRITY_DATA, file_inode(file),
> +			pathname, "collect_data", "forbidden-hash-algorithm", rc, 0);
> +
> +		goto out_locked;
> +	}
> +

This doesn't look like the right place for checking the hash algorithm.
IMA may be configured differently on different systems.  Some might
only enable measurement without appraisal, appraisal without
measurement, or both.  Only appraisal returns a failure and prevents
read, execute, mmap, etc.   The hash algorithm check probably should be
deferred to appraisal.  Placing the test here would skip measurements.

thanks,

Mimi

>  	rc = ima_collect_measurement(iint, file, buf, size, hash_algo, modsig);
>  	if (rc != 0 && rc != -EBADF && rc != -EINVAL)
>  		goto out_locked;



  reply	other threads:[~2021-07-27 15:29 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-27 10:23 [PATCH v3 0/4] IMA: restrict the accepted digest algorithms for the security.ima xattr THOBY Simon
2021-07-27 10:23 ` [PATCH v3 1/4] IMA: block writes of the security.ima xattr with unsupported algorithms THOBY Simon
2021-07-27 14:04   ` Mimi Zohar
2021-07-27 10:23 ` [PATCH v3 2/4] IMA: add support to restrict the hash algorithms used for file appraisal THOBY Simon
2021-07-27 15:29   ` Mimi Zohar [this message]
2021-07-27 10:23 ` [PATCH v3 3/4] IMA: add a policy option to restrict xattr hash algorithms on appraisal THOBY Simon
2021-07-27 16:24   ` Mimi Zohar
2021-07-27 10:23 ` [PATCH v3 4/4] IMA: introduce a new policy option func=SETXATTR_CHECK THOBY Simon
2021-07-27 11:24   ` THOBY Simon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1664c5d77e148eb387717428c55d2ef4f2482732.camel@linux.ibm.com \
    --to=zohar@linux.ibm.com \
    --cc=Didier.BARVAUX@viveris.fr \
    --cc=Simon.THOBY@viveris.fr \
    --cc=dmitry.kasatkin@gmail.com \
    --cc=linux-integrity@vger.kernel.org \
    --subject='Re: [PATCH v3 2/4] IMA: add support to restrict the hash algorithms used for file appraisal' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).