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

[Roberto Sassu]

> -----Original Message-----
> From: owner-linux-security-module at vger.kernel.org [mailto:owner-linux-
> security-module at vger.kernel.org] On Behalf Of Mimi Zohar
> Sent: Thursday, April 23, 2020 4:34 AM
> To: Roberto Sassu <roberto.sassu at huawei.com>
> Cc: linux-integrity at vger.kernel.org; linux-security-module at vger.kernel.org;
> linux-kernel at vger.kernel.org; Silviu Vlasceanu
> <Silviu.Vlasceanu at huawei.com>; stable at vger.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 at vger.kernel.org
> > Fixes: 19f8a84713edc ("ima: measure and appraise the IMA policy itself")
> > Signed-off-by: Roberto Sassu <roberto.sassu at huawei.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.

In LOG or FIX mode, loading a policy with absolute path will succeed.
Maybe we should just keep the same behavior also when the policy
is directly written to securityfs.

Ok for the OFF mode, but probably this should be a separate patch.

Roberto

HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Li Jian, Shi Yanli


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