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;
Message-ID: <1591722292.5567.28.camel@linux.ibm.com>
Subject: Re: [PATCH v3] IMA: Add audit log for failure conditions
From:   Mimi Zohar <zohar@linux.ibm.com>
To:     Lakshmi Ramasubramanian <nramas@linux.microsoft.com>,
        paul@paul-moore.com
Cc:     linux-integrity@vger.kernel.org, linux-audit@redhat.com,
        linux-kernel@vger.kernel.org
Date:   Tue, 09 Jun 2020 13:04:52 -0400
In-Reply-To: <20200608215343.4491-1-nramas@linux.microsoft.com>
References: <20200608215343.4491-1-nramas@linux.microsoft.com>
Content-Type: text/plain; charset="UTF-8"
Mime-Version: 1.0
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Mon, 2020-06-08 at 14:53 -0700, Lakshmi Ramasubramanian wrote:
> The final log statement in process_buffer_measurement() for failure
> condition is at debug level. This does not log the message unless
> the system log level is raised which would significantly increase
> the messages in the system log. Change this log message to an audit
> message for better triaging failures in the function.

We need to differentiate between emitting the error number for
debugging purposes and auditing integrity failures.  The purpose of
this patch should be to audit integrity failures.  Please update the
patch description.

(Including the "errno" as Steve suggested would be fine.)

> 
>  out:
> -	if (ret < 0)
> -		pr_devel("%s: failed, result: %d\n", __func__, ret);
> +	if (ret < 0) {
> +		snprintf(measurement_audit_cause, AUDIT_CAUSE_LEN_MAX,
> +			 "%s(%d)", audit_cause, ret);

Please remove this.

> +
> +		switch (func) {
> +		case KEXEC_CMDLINE:
> +			op = "measuring_kexec_cmdline";
> +			break;
> +		case KEY_CHECK:
> +			op = "measuring_keys";
> +			break;
> +		default:
> +			op = "measuring_blacklisted_hash";
> +			break;
> +		}

Instead of a switch, define a string array based on func.

> +
> +		integrity_audit_msg(AUDIT_INTEGRITY_PCR, NULL, eventname,
> +				    op, measurement_audit_cause, ret, 0);

Use "audit_cause".

thanks,

Mimi

> +	}