Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Date:   Wed, 23 Feb 2022 16:50:57 -0800
From:   Eric Biggers <ebiggers@kernel.org>
To:     Mimi Zohar <zohar@linux.ibm.com>
Cc:     linux-integrity@vger.kernel.org,
        Stefan Berger <stefanb@linux.ibm.com>,
        linux-fscrypt@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v5 6/8] ima: define signature version 3
Message-ID: <YhbWcRfo/Cz8EyNw@sol.localdomain>
References: <20220211214310.119257-1-zohar@linux.ibm.com>
 <20220211214310.119257-7-zohar@linux.ibm.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220211214310.119257-7-zohar@linux.ibm.com>
Precedence: bulk

On Fri, Feb 11, 2022 at 04:43:08PM -0500, Mimi Zohar wrote:
> To disambiguate the signed data stored in the 'security.ima' xattr,
> define signature version 3 as the hash of the ima_file_id structure.
> 
> Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
> ---
>  security/integrity/digsig.c           |  3 ++-
>  security/integrity/ima/ima_appraise.c | 35 +++++++++++++++++++++++++++
>  security/integrity/integrity.h        | 20 +++++++++++++--
>  3 files changed, 55 insertions(+), 3 deletions(-)
> 
> diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
> index 3b06a01bd0fd..fd8f77d92a62 100644
> --- a/security/integrity/digsig.c
> +++ b/security/integrity/digsig.c
> @@ -74,7 +74,8 @@ int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,
>  		/* v1 API expect signature without xattr type */
>  		return digsig_verify(keyring, sig + 1, siglen - 1, digest,
>  				     digestlen);
> -	case 2:
> +	case 2: /* regular file data hash based sginature */
> +	case 3: /* struct ima_file_id data base signature */
>  		return asymmetric_verify(keyring, sig, siglen, digest,
>  					 digestlen);
>  	}
> diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
> index 17232bbfb9f9..c2b429c141a7 100644
> --- a/security/integrity/ima/ima_appraise.c
> +++ b/security/integrity/ima/ima_appraise.c
> @@ -225,6 +225,33 @@ int ima_read_xattr(struct dentry *dentry,
>  	return ret;
>  }
>  
> +/*
> + * calc_file_id_hash - calculate the hash of the ima_file_id struct data
> + * @type: xattr type [enum evm_ima_xattr_type]
> + * @algo: hash algorithm [enum hash_algo]
> + * @digest: pointer to the digest to be hashed
> + * @hash: (out) pointer to the hash
> + *
> + * IMA signature version 3 disambiguates the data that is signed by
> + * indirectly signing the hash of the ima_file_id structure data.
> + *
> + * Return 0 on success, error code otherwise.
> + */
> +static int calc_file_id_hash(enum evm_ima_xattr_type type,
> +			     enum hash_algo algo, const u8 *digest,
> +			     struct ima_digest_data *hash)
> +{
> +	struct ima_file_id file_id = {.hash_algorithm = algo};
> +	uint unused = HASH_MAX_DIGESTSIZE - hash_digest_size[algo];
> +
> +	memcpy(file_id.hash, digest, hash_digest_size[algo]);
> +
> +	hash->algo = algo;
> +	hash->length = hash_digest_size[algo];
> +
> +	return ima_calc_buffer_hash(&file_id, sizeof(file_id) - unused, hash);
> +}
> +

calc_file_id_hash() isn't used in this patch.

Perhaps this patch should be merged with the following one?  I don't understand
the point of this patch on its own.

- Eric