* [PATCH v4 1/7] ima: Switch to ima_hash_algo for boot aggregate [not found] <20200325104712.25694-1-roberto.sassu@huawei.com> @ 2020-03-25 10:47 ` Roberto Sassu 2020-04-02 23:53 ` Mimi Zohar 2020-03-25 10:47 ` [PATCH v4 2/7] ima: Evaluate error in init_ima() Roberto Sassu 1 sibling, 1 reply; 6+ messages in thread From: Roberto Sassu @ 2020-03-25 10:47 UTC (permalink / raw) To: zohar, James.Bottomley Cc: linux-integrity, linux-security-module, linux-kernel, silviu.vlasceanu, Roberto Sassu, stable 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 even if a TPM 2.0 with support for stronger hash algorithms is available. This patch first tries to find a PCR bank with the IMA default hash algorithm. If it does not find it, it selects the SHA256 PCR bank for TPM 2.0 and SHA1 for TPM 1.2. Ultimately, it selects SHA1 also for TPM 2.0 if the SHA256 PCR bank is not found. If none of the PCR banks above can be found, boot_aggregate file digest is filled with zeros, as for TPM bypass, making it impossible to perform a remote attestation of the system. Changelog v3: - Remove option to select the first PCR bank and select SHA1 as fallback choice also for TPM 2.0 (suggested by Mimi) v1: - add Mimi's comments - if there is no PCR bank for the IMA default hash algorithm use SHA256 (suggested by James Bottomley) Cc: stable@vger.kernel.org # 5.1.x Fixes: 879b589210a9 ("tpm: retrieve digest size of unknown algorithms with PCR read") Reported-by: Jerry Snitselaar <jsnitsel@redhat.com> Suggested-by: James Bottomley <James.Bottomley@HansenPartnership.com> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com> --- security/integrity/ima/ima_crypto.c | 47 +++++++++++++++++++++++++---- security/integrity/ima/ima_init.c | 22 +++++++++++--- 2 files changed, 58 insertions(+), 11 deletions(-) diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c index 423c84f95a14..8e445a671225 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,37 @@ 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 = -1; + + 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 == -1 && crypto_id == HASH_ALGO_SHA1) + bank_idx = i; + } + + if (bank_idx == -1) { + 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 567468188a61..fc1e1002b48d 100644 --- a/security/integrity/ima/ima_init.c +++ b/security/integrity/ima/ima_init.c @@ -25,7 +25,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. @@ -49,15 +49,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 + * TCG required banks - SHA256 for TPM 2.0, SHA1 for TPM 1.2. + * Ultimately select SHA1 also for TPM 2.0 if the SHA256 PCR bank + * is not found. + */ if (ima_tpm_chip) { result = ima_calc_boot_aggregate(&hash.hdr); if (result < 0) { -- 2.17.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v4 1/7] ima: Switch to ima_hash_algo for boot aggregate 2020-03-25 10:47 ` [PATCH v4 1/7] ima: Switch to ima_hash_algo for boot aggregate Roberto Sassu @ 2020-04-02 23:53 ` Mimi Zohar 2020-05-08 4:54 ` Jerry Snitselaar 0 siblings, 1 reply; 6+ messages in thread From: Mimi Zohar @ 2020-04-02 23:53 UTC (permalink / raw) To: Roberto Sassu, James.Bottomley, Jerry Snitselaar Cc: linux-integrity, linux-security-module, linux-kernel, silviu.vlasceanu, stable Hi Roberto, On Wed, 2020-03-25 at 11:47 +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 even if a > TPM 2.0 with support for stronger hash algorithms is available. > > This patch first tries to find a PCR bank with the IMA default hash > algorithm. If it does not find it, it selects the SHA256 PCR bank for > TPM 2.0 and SHA1 for TPM 1.2. Ultimately, it selects SHA1 also for TPM 2.0 > if the SHA256 PCR bank is not found. > > If none of the PCR banks above can be found, boot_aggregate file digest is > filled with zeros, as for TPM bypass, making it impossible to perform a > remote attestation of the system. > > Cc: stable@vger.kernel.org # 5.1.x > Fixes: 879b589210a9 ("tpm: retrieve digest size of unknown algorithms with PCR read") > Reported-by: Jerry Snitselaar <jsnitsel@redhat.com> > Suggested-by: James Bottomley <James.Bottomley@HansenPartnership.com> > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com> Thank you! This patch set is now queued in next-integrity-testing during the open window. Jerry, I assume this works for you. Could we get your tag? thanks! Mimi ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4 1/7] ima: Switch to ima_hash_algo for boot aggregate 2020-04-02 23:53 ` Mimi Zohar @ 2020-05-08 4:54 ` Jerry Snitselaar 2020-05-08 17:29 ` Mimi Zohar 0 siblings, 1 reply; 6+ messages in thread From: Jerry Snitselaar @ 2020-05-08 4:54 UTC (permalink / raw) To: Mimi Zohar Cc: Roberto Sassu, James.Bottomley, linux-integrity, linux-security-module, linux-kernel, silviu.vlasceanu, stable On Thu Apr 02 20, Mimi Zohar wrote: >Hi Roberto, > >On Wed, 2020-03-25 at 11:47 +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 even if a >> TPM 2.0 with support for stronger hash algorithms is available. >> >> This patch first tries to find a PCR bank with the IMA default hash >> algorithm. If it does not find it, it selects the SHA256 PCR bank for >> TPM 2.0 and SHA1 for TPM 1.2. Ultimately, it selects SHA1 also for TPM 2.0 >> if the SHA256 PCR bank is not found. >> >> If none of the PCR banks above can be found, boot_aggregate file digest is >> filled with zeros, as for TPM bypass, making it impossible to perform a >> remote attestation of the system. >> >> Cc: stable@vger.kernel.org # 5.1.x >> Fixes: 879b589210a9 ("tpm: retrieve digest size of unknown algorithms with PCR read") >> Reported-by: Jerry Snitselaar <jsnitsel@redhat.com> >> Suggested-by: James Bottomley <James.Bottomley@HansenPartnership.com> >> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com> > >Thank you! This patch set is now queued in next-integrity-testing >during the open window. Jerry, I assume this works for you. Could we >get your tag? > >thanks! > >Mimi > Hi Mimi, Yes, I no longer get the errors with this patch. Tested-by: Jerry Snitselaar <jsnitsel@redhat.com> Regards, Jerry ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4 1/7] ima: Switch to ima_hash_algo for boot aggregate 2020-05-08 4:54 ` Jerry Snitselaar @ 2020-05-08 17:29 ` Mimi Zohar 0 siblings, 0 replies; 6+ messages in thread From: Mimi Zohar @ 2020-05-08 17:29 UTC (permalink / raw) To: Jerry Snitselaar Cc: Roberto Sassu, James.Bottomley, linux-integrity, linux-security-module, linux-kernel, silviu.vlasceanu, stable, Stephen Rothwell, Madhuparna Bhowmik On Thu, 2020-05-07 at 21:54 -0700, Jerry Snitselaar wrote: > On Thu Apr 02 20, Mimi Zohar wrote: > >Hi Roberto, > > > >On Wed, 2020-03-25 at 11:47 +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 even if a > >> TPM 2.0 with support for stronger hash algorithms is available. > >> > >> This patch first tries to find a PCR bank with the IMA default hash > >> algorithm. If it does not find it, it selects the SHA256 PCR bank for > >> TPM 2.0 and SHA1 for TPM 1.2. Ultimately, it selects SHA1 also for TPM 2.0 > >> if the SHA256 PCR bank is not found. > >> > >> If none of the PCR banks above can be found, boot_aggregate file digest is > >> filled with zeros, as for TPM bypass, making it impossible to perform a > >> remote attestation of the system. > >> > >> Cc: stable@vger.kernel.org # 5.1.x > >> Fixes: 879b589210a9 ("tpm: retrieve digest size of unknown algorithms with PCR read") > >> Reported-by: Jerry Snitselaar <jsnitsel@redhat.com> > >> Suggested-by: James Bottomley <James.Bottomley@HansenPartnership.com> > >> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com> > > > >Thank you! This patch set is now queued in next-integrity-testing > >during the open window. Jerry, I assume this works for you. Could we > >get your tag? > > > > Yes, I no longer get the errors with this patch. > > > Tested-by: Jerry Snitselaar <jsnitsel@redhat.com> Thanks, Jerry. I really do appreciate receiving your tag. Not all, but a lot of subsystems, do not rebase their branch, at least once it is in linux-next. Adding tags is considered rebasing. For this reason, I've started staging patches in the next-integrity- testing branch, before moving them to next-integrity. thanks, Mimi ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v4 2/7] ima: Evaluate error in init_ima() [not found] <20200325104712.25694-1-roberto.sassu@huawei.com> 2020-03-25 10:47 ` [PATCH v4 1/7] ima: Switch to ima_hash_algo for boot aggregate Roberto Sassu @ 2020-03-25 10:47 ` Roberto Sassu 2020-03-27 0:07 ` James Morris 1 sibling, 1 reply; 6+ messages in thread From: Roberto Sassu @ 2020-03-25 10:47 UTC (permalink / raw) To: zohar, James.Bottomley Cc: linux-integrity, linux-security-module, linux-kernel, silviu.vlasceanu, Roberto Sassu, stable Evaluate error in init_ima() before register_blocking_lsm_notifier() and return if not zero. Cc: stable@vger.kernel.org # 5.3.x Fixes: b16942455193 ("ima: use the lsm policy update notifier") Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com> --- security/integrity/ima/ima_main.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 9d0abedeae77..f96f151294e6 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -792,6 +792,9 @@ static int __init init_ima(void) error = ima_init(); } + if (error) + return error; + error = register_blocking_lsm_notifier(&ima_lsm_policy_notifier); if (error) pr_warn("Couldn't register LSM notifier, error %d\n", error); -- 2.17.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v4 2/7] ima: Evaluate error in init_ima() 2020-03-25 10:47 ` [PATCH v4 2/7] ima: Evaluate error in init_ima() Roberto Sassu @ 2020-03-27 0:07 ` James Morris 0 siblings, 0 replies; 6+ messages in thread From: James Morris @ 2020-03-27 0:07 UTC (permalink / raw) To: Roberto Sassu Cc: zohar, James.Bottomley, linux-integrity, linux-security-module, linux-kernel, silviu.vlasceanu, stable On Wed, 25 Mar 2020, Roberto Sassu wrote: > Evaluate error in init_ima() before register_blocking_lsm_notifier() and > return if not zero. > > Cc: stable@vger.kernel.org # 5.3.x > Fixes: b16942455193 ("ima: use the lsm policy update notifier") > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com> > --- > security/integrity/ima/ima_main.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c > index 9d0abedeae77..f96f151294e6 100644 > --- a/security/integrity/ima/ima_main.c > +++ b/security/integrity/ima/ima_main.c > @@ -792,6 +792,9 @@ static int __init init_ima(void) > error = ima_init(); > } > > + if (error) > + return error; > + > error = register_blocking_lsm_notifier(&ima_lsm_policy_notifier); > if (error) > pr_warn("Couldn't register LSM notifier, error %d\n", error); > Reviewed-by: James Morris <jamorris@linux.microsoft.com> -- James Morris <jmorris@namei.org> ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-05-08 17:29 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20200325104712.25694-1-roberto.sassu@huawei.com>
2020-03-25 10:47 ` [PATCH v4 1/7] ima: Switch to ima_hash_algo for boot aggregate Roberto Sassu
2020-04-02 23:53 ` Mimi Zohar
2020-05-08 4:54 ` Jerry Snitselaar
2020-05-08 17:29 ` Mimi Zohar
2020-03-25 10:47 ` [PATCH v4 2/7] ima: Evaluate error in init_ima() Roberto Sassu
2020-03-27 0:07 ` James Morris
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).