linux-kernel - Re: [PATCH v3] IMA: Add audit log for failure conditions

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <1591722292.5567.28.camel@linux.ibm.com>
Date:   Tue, 09 Jun 2020 13:04:52 -0400
From:   Mimi Zohar <zohar@...ux.ibm.com>
To:     Lakshmi Ramasubramanian <nramas@...ux.microsoft.com>,
        paul@...l-moore.com
Cc:     linux-integrity@...r.kernel.org, linux-audit@...hat.com,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] IMA: Add audit log for failure conditions

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

> +	}