| 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
| ||
|
Message-Id: <1581373420.5585.920.camel@linux.ibm.com>
Date: Mon, 10 Feb 2020 17:23:40 -0500
From: Mimi Zohar <zohar@...ux.ibm.com>
To: Roberto Sassu <roberto.sassu@...wei.com>,
James.Bottomley@...senPartnership.com,
jarkko.sakkinen@...ux.intel.com
Cc: linux-integrity@...r.kernel.org,
linux-security-module@...r.kernel.org,
linux-kernel@...r.kernel.org, silviu.vlasceanu@...wei.com,
stable@...r.kernel.org
Subject: Re: [PATCH v3 2/8] ima: Switch to ima_hash_algo for boot aggregate
On Mon, 2020-02-10 at 11:00 +0100, Roberto Sassu wrote:
> boot_aggregate is the first entry of IMA measurement list. Its purpose is
> to link pre-boot measurements to IMA measurements. As IMA was designed to
> work with a TPM 1.2, the SHA1 PCR bank was always selected.
>
> Currently, even if a TPM 2.0 is used, the SHA1 PCR bank is selected.
> However, the assumption that the SHA1 PCR bank is always available is not
> correct, as PCR banks can be selected with the PCR_Allocate() TPM command.
>
> This patch tries to use ima_hash_algo as hash algorithm for boot_aggregate.
> If no PCR bank uses that algorithm, the patch tries to find the SHA256 PCR
> bank (which is mandatory in the TCG PC Client specification).
Up to here, the patch description matches the code.
> If also this
> bank is not found, the patch selects the first one. If the TPM algorithm
> of that bank is not mapped to a crypto ID, boot_aggregate is set to zero.
This comment and the one inline are left over from previous version.
>
> Changelog
>
> v1:
> - add Mimi's comments
> - if there is no PCR bank for the IMA default algorithm use SHA256
> (suggested by James Bottomley)
>
> Cc: stable@...r.kernel.org # 5.1.x
> Fixes: 879b589210a9 ("tpm: retrieve digest size of unknown algorithms with PCR read")
> Reported-by: Jerry Snitselaar <jsnitsel@...hat.com>
> Suggested-by: James Bottomley <James.Bottomley@...senPartnership.com>
> Signed-off-by: Roberto Sassu <roberto.sassu@...wei.com>
Thanks, Roberto. This patch is dependent on 1/8. As soon as there's
a topic branch, I'll queue this, removing the extraneous comments.
Mimi
> ---
> security/integrity/ima/ima_crypto.c | 45 +++++++++++++++++++++++++----
> security/integrity/ima/ima_init.c | 22 ++++++++++----
> 2 files changed, 56 insertions(+), 11 deletions(-)
>
> diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
> index 73044fc6a952..f2f41a2bc3d4 100644
> --- a/security/integrity/ima/ima_crypto.c
> +++ b/security/integrity/ima/ima_crypto.c
> @@ -655,18 +655,29 @@ static void __init ima_pcrread(u32 idx, struct tpm_digest *d)
> }
>
> /*
> - * Calculate the boot aggregate hash
> + * The boot_aggregate is a cumulative hash over TPM registers 0 - 7. With
> + * TPM 1.2 the boot_aggregate was based on reading the SHA1 PCRs, but with
> + * TPM 2.0 hash agility, TPM chips could support multiple TPM PCR banks,
> + * allowing firmware to configure and enable different banks.
> + *
> + * Knowing which TPM bank is read to calculate the boot_aggregate digest
> + * needs to be conveyed to a verifier. For this reason, use the same
> + * hash algorithm for reading the TPM PCRs as for calculating the boot
> + * aggregate digest as stored in the measurement list.
> */
> -static int __init ima_calc_boot_aggregate_tfm(char *digest,
> +static int __init ima_calc_boot_aggregate_tfm(char *digest, u16 alg_id,
> struct crypto_shash *tfm)
> {
> - struct tpm_digest d = { .alg_id = TPM_ALG_SHA1, .digest = {0} };
> + struct tpm_digest d = { .alg_id = alg_id, .digest = {0} };
> int rc;
> u32 i;
> SHASH_DESC_ON_STACK(shash, tfm);
>
> shash->tfm = tfm;
>
> + pr_devel("calculating the boot-aggregate based on TPM bank: %04x\n",
> + d.alg_id);
> +
> rc = crypto_shash_init(shash);
> if (rc != 0)
> return rc;
> @@ -675,7 +686,8 @@ static int __init ima_calc_boot_aggregate_tfm(char *digest,
> for (i = TPM_PCR0; i < TPM_PCR8; i++) {
> ima_pcrread(i, &d);
> /* now accumulate with current aggregate */
> - rc = crypto_shash_update(shash, d.digest, TPM_DIGEST_SIZE);
> + rc = crypto_shash_update(shash, d.digest,
> + crypto_shash_digestsize(tfm));
> }
> if (!rc)
> crypto_shash_final(shash, digest);
> @@ -685,14 +697,35 @@ static int __init ima_calc_boot_aggregate_tfm(char *digest,
> int __init ima_calc_boot_aggregate(struct ima_digest_data *hash)
> {
> struct crypto_shash *tfm;
> - int rc;
> + u16 crypto_id, alg_id;
> + int rc, i, bank_idx = 0;
> +
> + for (i = 0; i < ima_tpm_chip->nr_allocated_banks; i++) {
> + crypto_id = ima_tpm_chip->allocated_banks[i].crypto_id;
> + if (crypto_id == hash->algo) {
> + bank_idx = i;
> + break;
> + }
> +
> + if (crypto_id == HASH_ALGO_SHA256)
> + bank_idx = i;
> + }
> +
> + if (bank_idx == 0 &&
> + ima_tpm_chip->allocated_banks[0].crypto_id == HASH_ALGO__LAST) {
> + pr_err("No suitable TPM algorithm for boot aggregate\n");
> + return 0;
> + }
> +
> + hash->algo = ima_tpm_chip->allocated_banks[bank_idx].crypto_id;
>
> tfm = ima_alloc_tfm(hash->algo);
> if (IS_ERR(tfm))
> return PTR_ERR(tfm);
>
> hash->length = crypto_shash_digestsize(tfm);
> - rc = ima_calc_boot_aggregate_tfm(hash->digest, tfm);
> + alg_id = ima_tpm_chip->allocated_banks[bank_idx].alg_id;
> + rc = ima_calc_boot_aggregate_tfm(hash->digest, alg_id, tfm);
>
> ima_free_tfm(tfm);
>
> diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
> index 5d55ade5f3b9..fbd7a8e28a6b 100644
> --- a/security/integrity/ima/ima_init.c
> +++ b/security/integrity/ima/ima_init.c
> @@ -27,7 +27,7 @@ struct tpm_chip *ima_tpm_chip;
> /* Add the boot aggregate to the IMA measurement list and extend
> * the PCR register.
> *
> - * Calculate the boot aggregate, a SHA1 over tpm registers 0-7,
> + * Calculate the boot aggregate, a hash over tpm registers 0-7,
> * assuming a TPM chip exists, and zeroes if the TPM chip does not
> * exist. Add the boot aggregate measurement to the measurement
> * list and extend the PCR register.
> @@ -51,15 +51,27 @@ static int __init ima_add_boot_aggregate(void)
> int violation = 0;
> struct {
> struct ima_digest_data hdr;
> - char digest[TPM_DIGEST_SIZE];
> + char digest[TPM_MAX_DIGEST_SIZE];
> } hash;
>
> memset(iint, 0, sizeof(*iint));
> memset(&hash, 0, sizeof(hash));
> iint->ima_hash = &hash.hdr;
> - iint->ima_hash->algo = HASH_ALGO_SHA1;
> - iint->ima_hash->length = SHA1_DIGEST_SIZE;
> -
> + iint->ima_hash->algo = ima_hash_algo;
> + iint->ima_hash->length = hash_digest_size[ima_hash_algo];
> +
> + /*
> + * With TPM 2.0 hash agility, TPM chips could support multiple TPM
> + * PCR banks, allowing firmware to configure and enable different
> + * banks. The SHA1 bank is not necessarily enabled.
> + *
> + * Use the same hash algorithm for reading the TPM PCRs as for
> + * calculating the boot aggregate digest. Preference is given to
> + * the configured IMA default hash algorithm. Otherwise, use the
> + * TPM required banks - SHA256 for TPM 2.0, SHA1 for TPM 1.2. If
> + * SHA256 is not available, use the first PCR bank and if that is
> + * not mapped to a crypto ID, set boot_aggregate to zero.
The last line of the above comment is left over from the previous
version.
> + */
> if (ima_tpm_chip) {
> result = ima_calc_boot_aggregate(&hash.hdr);
> if (result < 0) {
Powered by blists - more mailing lists