Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH] ima: Fix return value of ima_write_policy()
From:   Mimi Zohar <zohar@linux.ibm.com>
To:     Roberto Sassu <roberto.sassu@huawei.com>
Cc:     linux-integrity@vger.kernel.org,
        linux-security-module@vger.kernel.org,
        linux-kernel@vger.kernel.org, silviu.vlasceanu@huawei.com,
        stable@vger.kernel.org
Date:   Wed, 22 Apr 2020 22:34:26 -0400
In-Reply-To: <20200421090442.22693-1-roberto.sassu@huawei.com>
References: <20200421090442.22693-1-roberto.sassu@huawei.com>
Content-Type: text/plain; charset="UTF-8"
Mime-Version: 1.0
Content-Transfer-Encoding: 8bit
Message-Id: <1587609266.5165.58.camel@linux.ibm.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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

Mimi

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