linux-kernel - Re: [PATCH] ima: Fix return value of ima_write

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 for Android: free password hash cracker in your pocket

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

Message-Id: <1587609266.5165.58.camel@linux.ibm.com>
Date:   Wed, 22 Apr 2020 22:34:26 -0400
From:   Mimi Zohar <zohar@...ux.ibm.com>
To:     Roberto Sassu <roberto.sassu@...wei.com>
Cc:     linux-integrity@...r.kernel.org,
        linux-security-module@...r.kernel.org,
        linux-kernel@...r.kernel.org, silviu.vlasceanu@...wei.com,
        stable@...r.kernel.org
Subject: Re: [PATCH] ima: Fix return value of ima_write_policy()

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@...r.kernel.org
> Fixes: 19f8a84713edc ("ima: measure and appraise the IMA policy itself")
> Signed-off-by: Roberto Sassu <roberto.sassu@...wei.com>
> ---
>  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);
>  	}