Re: [PATCH v7 4/5] IMA: add a policy option to restrict xattr hash algorithms on appraisal

[Mimi Zohar <zohar@linux.ibm.com>]

Hi Simon,

On Wed, 2021-08-11 at 11:40 +0000, THOBY Simon wrote:
> +static unsigned int ima_parse_appraise_algos(char *arg)
> +{
> +	unsigned int res = 0;
> +	int idx;
> +	char *token;
> +
> +	while ((token = strsep(&arg, ",")) != NULL) {
> +		idx = match_string(hash_algo_name, HASH_ALGO__LAST, token);
> +
> +		if (idx < 0) {
> +			pr_err("unknown hash algorithm \"%s\"",
> +			       token);
> +			return 0;

Previous versions of this patch ignored unknown algorithms.  If not all
of the algorithms are defined in an older kernel, should loading the
policy fail?   As new IMA policy features are defined, older kernels
prevent loading newer policies with unknown features.   I hesitated to
equate the two scenarios.

> +		}
> +
> +		/* Add the hash algorithm to the 'allowed' bitfield */
> +		res |= (1U << idx);

This assumes that all the hash algorithms are enabled in the kernel,
but nothing checks that they are.  In validate_hash_algo(), either the
allowed_hashes is checked or the hash algorithm must be configured.  Do
we really want a total separation like this?

thanks,

Mimi

> +	}
> +
> +	return res;
> +}
> +