Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <20697475-1b31-4db7-d1e7-afe2d50e2bca@linux.ibm.com>
Date:   Fri, 29 Apr 2022 12:09:15 -0400
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.7.0
Subject: Re: [PATCH v8 2/7] ima: use IMA default hash algorithm for integrity
 violations
Content-Language: en-US
To:     Mimi Zohar <zohar@linux.ibm.com>, linux-integrity@vger.kernel.org
Cc:     Eric Biggers <ebiggers@kernel.org>, linux-fscrypt@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <20220429112601.1421947-1-zohar@linux.ibm.com>
 <20220429112601.1421947-3-zohar@linux.ibm.com>
From:   Stefan Berger <stefanb@linux.ibm.com>
In-Reply-To: <20220429112601.1421947-3-zohar@linux.ibm.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 4/29/22 07:25, Mimi Zohar wrote:
> Integrity file violations - ToM/ToU, open writers - are recorded in the IMA
> measurement list, containing 0x00's in both the template data and file data
> hash fields, but 0xFF's are actually extended into TPM PCRs.  Although the
> original 'ima' template data field ('d') is limited to 20 bytes, the 'd-ng'
> template digest field is not.
> 
> The violation file data hash template field ('d-ng') is unnecessarily hard
> coded to SHA1.  Instead of simply replacing the hard coded SHA1 hash
> algorithm with a larger hash algorithm, use the hash algorithm as defined
> in "ima_hash_algo".  ima_hash_algo is set to either the Kconfig IMA default
> hash algorithm or as defined on the boot command line (ima_hash=).
> 
> Including a non-SHA1 file data hash algorithm in the 'd-ng' field of
> violations is a cosmetic change.  The template data hash field, which is
> extended into the TPM PCRs, is not affected by this change and should not
> affect attestation of the IMA measurement list.
> 
> Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>

The effect seems to be that violations that have previously looked like 
this

10 0000000000000000000000000000000000000000 ima-ng 
sha1:0000000000000000000000000000000000000000 /var/log/audit/audit.log

now look like this:

10 0000000000000000000000000000000000000000 ima-ng 
sha256:0000000000000000000000000000000000000000 /var/log/audit/audit.log

evmctl [1.3.2] still works fine:

# evmctl ima_measurement --ignore-violations 
/sys/kernel/security/ima/binary_runtime_measurements
Matched per TPM bank calculated digest(s).


Tested-by: Stefan Berger <stefanb@linux.ibm.com>


> ---
>   security/integrity/ima/ima_template_lib.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c
> index e9d65f6fe2ae..4b6706f864d4 100644
> --- a/security/integrity/ima/ima_template_lib.c
> +++ b/security/integrity/ima/ima_template_lib.c
> @@ -370,7 +370,7 @@ int ima_eventdigest_init(struct ima_event_data *event_data,
>   int ima_eventdigest_ng_init(struct ima_event_data *event_data,
>   			    struct ima_field_data *field_data)
>   {
> -	u8 *cur_digest = NULL, hash_algo = HASH_ALGO_SHA1;
> +	u8 *cur_digest = NULL, hash_algo = ima_hash_algo;
>   	u32 cur_digestsize = 0;
>   
>   	if (event_data->violation)	/* recording a violation. */