Re: [PATCH] ima: Fix return value of ima_write_policy()

[Mimi Zohar]

On Tue, 2020-04-21 at 11:04 +0200, Roberto Sassu wrote:
> Return datalen instead of zero if there is a rule to appraise the policy
> but that rule is not enforced.
> 
> Cc: stable@xxxxxxxxxxxxxxx
> Fixes: 19f8a84713edc ("ima: measure and appraise the IMA policy itself")
> Signed-off-by: Roberto Sassu <roberto.sassu@xxxxxxxxxx>
> ---
>  security/integrity/ima/ima_fs.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
> index a71e822a6e92..2c2ea814b954 100644
> --- a/security/integrity/ima/ima_fs.c
> +++ b/security/integrity/ima/ima_fs.c
> @@ -340,6 +340,8 @@ static ssize_t ima_write_policy(struct file *file, const char __user *buf,
>  				    1, 0);
>  		if (ima_appraise & IMA_APPRAISE_ENFORCE)
>  			result = -EACCES;
> +		else
> +			result = datalen;

In all other cases, where the IMA_APPRAISE_ENFORCE is not enabled we
allow the action. ÂHere we prevent loading the policy, but don't
return an error. ÂOne option, as you did, is return some indication
that the policy was not loaded. ÂAnother option would be to allow
loading the policy in LOG or FIX mode, but I don't think that would be
productive. ÂPerhaps differentiate between the LOG and FIX modes from
the OFF mode. ÂFor the LOG and FIX modes, perhaps return -EACCES as
well. ÂFor the OFF case, loading a policy with appraise rules should
not be permitted.

Mimi

>  	} else {
>  		result = ima_parse_add_rule(data);
>  	}