* [PATCH AUTOSEL 5.7 03/30] ima: extend boot_aggregate with kernel measurements [not found] <20200708154116.3199728-1-sashal@kernel.org> @ 2020-07-08 15:40 ` Sasha Levin 2020-07-08 16:13 ` Mimi Zohar 0 siblings, 1 reply; 16+ messages in thread From: Sasha Levin @ 2020-07-08 15:40 UTC (permalink / raw) To: linux-kernel, stable Cc: Maurizio Drocco, Bruno Meneguele, Mimi Zohar, Sasha Levin, linux-integrity, linux-security-module From: Maurizio Drocco <maurizio.drocco@ibm.com> [ Upstream commit 20c59ce010f84300f6c655d32db2610d3433f85c ] Registers 8-9 are used to store measurements of the kernel and its command line (e.g., grub2 bootloader with tpm module enabled). IMA should include them in the boot aggregate. Registers 8-9 should be only included in non-SHA1 digests to avoid ambiguity. Signed-off-by: Maurizio Drocco <maurizio.drocco@ibm.com> Reviewed-by: Bruno Meneguele <bmeneg@redhat.com> Tested-by: Bruno Meneguele <bmeneg@redhat.com> (TPM 1.2, TPM 2.0) Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> Signed-off-by: Sasha Levin <sashal@kernel.org> --- security/integrity/ima/ima.h | 2 +- security/integrity/ima/ima_crypto.c | 15 ++++++++++++++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index 495e28bd488e6..844a55225ede0 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -30,7 +30,7 @@ enum ima_show_type { IMA_SHOW_BINARY, IMA_SHOW_BINARY_NO_FIELD_LEN, IMA_SHOW_BINARY_OLD_STRING_FMT, IMA_SHOW_ASCII }; -enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8 }; +enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8, TPM_PCR10 = 10 }; /* digest size for IMA, fits SHA1 or MD5 */ #define IMA_DIGEST_SIZE SHA1_DIGEST_SIZE diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c index fb27174806ba4..e0738d1d143d7 100644 --- a/security/integrity/ima/ima_crypto.c +++ b/security/integrity/ima/ima_crypto.c @@ -682,13 +682,26 @@ static int ima_calc_boot_aggregate_tfm(char *digest, u16 alg_id, if (rc != 0) return rc; - /* cumulative sha1 over tpm registers 0-7 */ + /* cumulative digest over TPM registers 0-7 */ for (i = TPM_PCR0; i < TPM_PCR8; i++) { ima_pcrread(i, &d); /* now accumulate with current aggregate */ rc = crypto_shash_update(shash, d.digest, crypto_shash_digestsize(tfm)); } + /* + * Extend cumulative digest over TPM registers 8-9, which contain + * measurement for the kernel command line (reg. 8) and image (reg. 9) + * in a typical PCR allocation. Registers 8-9 are only included in + * non-SHA1 boot_aggregate digests to avoid ambiguity. + */ + if (alg_id != TPM_ALG_SHA1) { + for (i = TPM_PCR8; i < TPM_PCR10; i++) { + ima_pcrread(i, &d); + rc = crypto_shash_update(shash, d.digest, + crypto_shash_digestsize(tfm)); + } + } if (!rc) crypto_shash_final(shash, digest); return rc; -- 2.25.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH AUTOSEL 5.7 03/30] ima: extend boot_aggregate with kernel measurements 2020-07-08 15:40 ` [PATCH AUTOSEL 5.7 03/30] ima: extend boot_aggregate with kernel measurements Sasha Levin @ 2020-07-08 16:13 ` Mimi Zohar 2020-07-09 1:27 ` Sasha Levin 0 siblings, 1 reply; 16+ messages in thread From: Mimi Zohar @ 2020-07-08 16:13 UTC (permalink / raw) To: Sasha Levin, linux-kernel, stable Cc: Maurizio Drocco, Bruno Meneguele, linux-integrity, linux-security-module Hi Sasha, On Wed, 2020-07-08 at 11:40 -0400, Sasha Levin wrote: > From: Maurizio Drocco <maurizio.drocco@ibm.com> > > [ Upstream commit 20c59ce010f84300f6c655d32db2610d3433f85c ] > > Registers 8-9 are used to store measurements of the kernel and its > command line (e.g., grub2 bootloader with tpm module enabled). IMA > should include them in the boot aggregate. Registers 8-9 should be > only included in non-SHA1 digests to avoid ambiguity. Prior to Linux 5.8, the SHA1 template data hashes were padded before being extended into the TPM. Support for calculating and extending the per TPM bank template data digests is only being upstreamed in Linux 5.8. How will attestation servers know whether to include PCRs 8 & 9 in the the boot_aggregate calculation? Now, there is a direct relationship between the template data SHA1 padded digest not including PCRs 8 & 9, and the new per TPM bank template data digest including them. Mimi ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH AUTOSEL 5.7 03/30] ima: extend boot_aggregate with kernel measurements 2020-07-08 16:13 ` Mimi Zohar @ 2020-07-09 1:27 ` Sasha Levin 2020-11-29 13:17 ` Mimi Zohar 0 siblings, 1 reply; 16+ messages in thread From: Sasha Levin @ 2020-07-09 1:27 UTC (permalink / raw) To: Mimi Zohar Cc: linux-kernel, stable, Maurizio Drocco, Bruno Meneguele, linux-integrity, linux-security-module On Wed, Jul 08, 2020 at 12:13:13PM -0400, Mimi Zohar wrote: >Hi Sasha, > >On Wed, 2020-07-08 at 11:40 -0400, Sasha Levin wrote: >> From: Maurizio Drocco <maurizio.drocco@ibm.com> >> >> [ Upstream commit 20c59ce010f84300f6c655d32db2610d3433f85c ] >> >> Registers 8-9 are used to store measurements of the kernel and its >> command line (e.g., grub2 bootloader with tpm module enabled). IMA >> should include them in the boot aggregate. Registers 8-9 should be >> only included in non-SHA1 digests to avoid ambiguity. > >Prior to Linux 5.8, the SHA1 template data hashes were padded before >being extended into the TPM. Support for calculating and extending >the per TPM bank template data digests is only being upstreamed in >Linux 5.8. > >How will attestation servers know whether to include PCRs 8 & 9 in the >the boot_aggregate calculation? Now, there is a direct relationship >between the template data SHA1 padded digest not including PCRs 8 & 9, >and the new per TPM bank template data digest including them. Got it, I'll drop it then, thank you! -- Thanks, Sasha ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH AUTOSEL 5.7 03/30] ima: extend boot_aggregate with kernel measurements 2020-07-09 1:27 ` Sasha Levin @ 2020-11-29 13:17 ` Mimi Zohar 2020-12-01 0:21 ` Sasha Levin 2020-12-11 3:10 ` Tyler Hicks 0 siblings, 2 replies; 16+ messages in thread From: Mimi Zohar @ 2020-11-29 13:17 UTC (permalink / raw) To: Sasha Levin Cc: linux-kernel, stable, Maurizio Drocco, Bruno Meneguele, linux-integrity, linux-security-module Hi Sasha, On Wed, 2020-07-08 at 21:27 -0400, Sasha Levin wrote: > On Wed, Jul 08, 2020 at 12:13:13PM -0400, Mimi Zohar wrote: > >Hi Sasha, > > > >On Wed, 2020-07-08 at 11:40 -0400, Sasha Levin wrote: > >> From: Maurizio Drocco <maurizio.drocco@ibm.com> > >> > >> [ Upstream commit 20c59ce010f84300f6c655d32db2610d3433f85c ] > >> > >> Registers 8-9 are used to store measurements of the kernel and its > >> command line (e.g., grub2 bootloader with tpm module enabled). IMA > >> should include them in the boot aggregate. Registers 8-9 should be > >> only included in non-SHA1 digests to avoid ambiguity. > > > >Prior to Linux 5.8, the SHA1 template data hashes were padded before > >being extended into the TPM. Support for calculating and extending > >the per TPM bank template data digests is only being upstreamed in > >Linux 5.8. > > > >How will attestation servers know whether to include PCRs 8 & 9 in the > >the boot_aggregate calculation? Now, there is a direct relationship > >between the template data SHA1 padded digest not including PCRs 8 & 9, > >and the new per TPM bank template data digest including them. > > Got it, I'll drop it then, thank you! After re-thinking this over, I realized that the attestation server can verify the "boot_aggregate" based on the quoted PCRs without knowing whether padded SHA1 hashes or per TPM bank hash values were extended into the TPM[1], but non-SHA1 boot aggregate values [2] should always include PCRs 8 & 9. Any place commit 6f1a1d103b48 was backported [2], this commit 20c59ce010f8 ("ima: extend boot_aggregate with kernel measurements") should be backported as well. thanks, Mimi [1] commit 1ea973df6e21 ("ima: Calculate and extend PCR with digests in ima_template_entry") [2] commit 6f1a1d103b48 ("ima: Switch to ima_hash_algo for boot aggregate") ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH AUTOSEL 5.7 03/30] ima: extend boot_aggregate with kernel measurements 2020-11-29 13:17 ` Mimi Zohar @ 2020-12-01 0:21 ` Sasha Levin 2020-12-01 3:13 ` Mimi Zohar 2020-12-11 3:10 ` Tyler Hicks 1 sibling, 1 reply; 16+ messages in thread From: Sasha Levin @ 2020-12-01 0:21 UTC (permalink / raw) To: Mimi Zohar Cc: linux-kernel, stable, Maurizio Drocco, Bruno Meneguele, linux-integrity, linux-security-module On Sun, Nov 29, 2020 at 08:17:38AM -0500, Mimi Zohar wrote: >Hi Sasha, > >On Wed, 2020-07-08 at 21:27 -0400, Sasha Levin wrote: >> On Wed, Jul 08, 2020 at 12:13:13PM -0400, Mimi Zohar wrote: >> >Hi Sasha, >> > >> >On Wed, 2020-07-08 at 11:40 -0400, Sasha Levin wrote: >> >> From: Maurizio Drocco <maurizio.drocco@ibm.com> >> >> >> >> [ Upstream commit 20c59ce010f84300f6c655d32db2610d3433f85c ] >> >> >> >> Registers 8-9 are used to store measurements of the kernel and its >> >> command line (e.g., grub2 bootloader with tpm module enabled). IMA >> >> should include them in the boot aggregate. Registers 8-9 should be >> >> only included in non-SHA1 digests to avoid ambiguity. >> > >> >Prior to Linux 5.8, the SHA1 template data hashes were padded before >> >being extended into the TPM. Support for calculating and extending >> >the per TPM bank template data digests is only being upstreamed in >> >Linux 5.8. >> > >> >How will attestation servers know whether to include PCRs 8 & 9 in the >> >the boot_aggregate calculation? Now, there is a direct relationship >> >between the template data SHA1 padded digest not including PCRs 8 & 9, >> >and the new per TPM bank template data digest including them. >> >> Got it, I'll drop it then, thank you! > >After re-thinking this over, I realized that the attestation server can >verify the "boot_aggregate" based on the quoted PCRs without knowing >whether padded SHA1 hashes or per TPM bank hash values were extended >into the TPM[1], but non-SHA1 boot aggregate values [2] should always >include PCRs 8 & 9. > >Any place commit 6f1a1d103b48 was backported [2], this commit >20c59ce010f8 ("ima: extend boot_aggregate with kernel measurements") >should be backported as well. Which kernels should it apply to? 5.7 is EOL now, so I looked at 5.4 but it doesn't apply cleanly there. -- Thanks, Sasha ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH AUTOSEL 5.7 03/30] ima: extend boot_aggregate with kernel measurements 2020-12-01 0:21 ` Sasha Levin @ 2020-12-01 3:13 ` Mimi Zohar 2020-12-02 23:53 ` Sasha Levin 0 siblings, 1 reply; 16+ messages in thread From: Mimi Zohar @ 2020-12-01 3:13 UTC (permalink / raw) To: Sasha Levin Cc: linux-kernel, stable, Maurizio Drocco, Bruno Meneguele, linux-integrity, linux-security-module On Mon, 2020-11-30 at 19:21 -0500, Sasha Levin wrote: > On Sun, Nov 29, 2020 at 08:17:38AM -0500, Mimi Zohar wrote: > >Hi Sasha, > > > >On Wed, 2020-07-08 at 21:27 -0400, Sasha Levin wrote: > >> On Wed, Jul 08, 2020 at 12:13:13PM -0400, Mimi Zohar wrote: > >> >Hi Sasha, > >> > > >> >On Wed, 2020-07-08 at 11:40 -0400, Sasha Levin wrote: > >> >> From: Maurizio Drocco <maurizio.drocco@ibm.com> > >> >> > >> >> [ Upstream commit 20c59ce010f84300f6c655d32db2610d3433f85c ] > >> >> > >> >> Registers 8-9 are used to store measurements of the kernel and its > >> >> command line (e.g., grub2 bootloader with tpm module enabled). IMA > >> >> should include them in the boot aggregate. Registers 8-9 should be > >> >> only included in non-SHA1 digests to avoid ambiguity. > >> > > >> >Prior to Linux 5.8, the SHA1 template data hashes were padded before > >> >being extended into the TPM. Support for calculating and extending > >> >the per TPM bank template data digests is only being upstreamed in > >> >Linux 5.8. > >> > > >> >How will attestation servers know whether to include PCRs 8 & 9 in the > >> >the boot_aggregate calculation? Now, there is a direct relationship > >> >between the template data SHA1 padded digest not including PCRs 8 & 9, > >> >and the new per TPM bank template data digest including them. > >> > >> Got it, I'll drop it then, thank you! > > > >After re-thinking this over, I realized that the attestation server can > >verify the "boot_aggregate" based on the quoted PCRs without knowing > >whether padded SHA1 hashes or per TPM bank hash values were extended > >into the TPM[1], but non-SHA1 boot aggregate values [2] should always > >include PCRs 8 & 9. > > > >Any place commit 6f1a1d103b48 was backported [2], this commit > >20c59ce010f8 ("ima: extend boot_aggregate with kernel measurements") > >should be backported as well. > > Which kernels should it apply to? 5.7 is EOL now, so I looked at 5.4 but > it doesn't apply cleanly there. For 5.4, both "git cherry-pick" and "git am --3way" for 20c59ce010f8 seem to work. thanks, Mimi ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH AUTOSEL 5.7 03/30] ima: extend boot_aggregate with kernel measurements 2020-12-01 3:13 ` Mimi Zohar @ 2020-12-02 23:53 ` Sasha Levin 0 siblings, 0 replies; 16+ messages in thread From: Sasha Levin @ 2020-12-02 23:53 UTC (permalink / raw) To: Mimi Zohar Cc: linux-kernel, stable, Maurizio Drocco, Bruno Meneguele, linux-integrity, linux-security-module On Mon, Nov 30, 2020 at 10:13:02PM -0500, Mimi Zohar wrote: >On Mon, 2020-11-30 at 19:21 -0500, Sasha Levin wrote: >> On Sun, Nov 29, 2020 at 08:17:38AM -0500, Mimi Zohar wrote: >> >Hi Sasha, >> > >> >On Wed, 2020-07-08 at 21:27 -0400, Sasha Levin wrote: >> >> On Wed, Jul 08, 2020 at 12:13:13PM -0400, Mimi Zohar wrote: >> >> >Hi Sasha, >> >> > >> >> >On Wed, 2020-07-08 at 11:40 -0400, Sasha Levin wrote: >> >> >> From: Maurizio Drocco <maurizio.drocco@ibm.com> >> >> >> >> >> >> [ Upstream commit 20c59ce010f84300f6c655d32db2610d3433f85c ] >> >> >> >> >> >> Registers 8-9 are used to store measurements of the kernel and its >> >> >> command line (e.g., grub2 bootloader with tpm module enabled). IMA >> >> >> should include them in the boot aggregate. Registers 8-9 should be >> >> >> only included in non-SHA1 digests to avoid ambiguity. >> >> > >> >> >Prior to Linux 5.8, the SHA1 template data hashes were padded before >> >> >being extended into the TPM. Support for calculating and extending >> >> >the per TPM bank template data digests is only being upstreamed in >> >> >Linux 5.8. >> >> > >> >> >How will attestation servers know whether to include PCRs 8 & 9 in the >> >> >the boot_aggregate calculation? Now, there is a direct relationship >> >> >between the template data SHA1 padded digest not including PCRs 8 & 9, >> >> >and the new per TPM bank template data digest including them. >> >> >> >> Got it, I'll drop it then, thank you! >> > >> >After re-thinking this over, I realized that the attestation server can >> >verify the "boot_aggregate" based on the quoted PCRs without knowing >> >whether padded SHA1 hashes or per TPM bank hash values were extended >> >into the TPM[1], but non-SHA1 boot aggregate values [2] should always >> >include PCRs 8 & 9. >> > >> >Any place commit 6f1a1d103b48 was backported [2], this commit >> >20c59ce010f8 ("ima: extend boot_aggregate with kernel measurements") >> >should be backported as well. >> >> Which kernels should it apply to? 5.7 is EOL now, so I looked at 5.4 but >> it doesn't apply cleanly there. > >For 5.4, both "git cherry-pick" and "git am --3way" for 20c59ce010f8 >seem to work. You're right, I've grabbed it too. Thanks! -- Thanks, Sasha ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH AUTOSEL 5.7 03/30] ima: extend boot_aggregate with kernel measurements 2020-11-29 13:17 ` Mimi Zohar 2020-12-01 0:21 ` Sasha Levin @ 2020-12-11 3:10 ` Tyler Hicks 2020-12-11 11:01 ` Mimi Zohar 1 sibling, 1 reply; 16+ messages in thread From: Tyler Hicks @ 2020-12-11 3:10 UTC (permalink / raw) To: Mimi Zohar Cc: Sasha Levin, linux-kernel, stable, Maurizio Drocco, Bruno Meneguele, linux-integrity, linux-security-module On 2020-11-29 08:17:38, Mimi Zohar wrote: > Hi Sasha, > > On Wed, 2020-07-08 at 21:27 -0400, Sasha Levin wrote: > > On Wed, Jul 08, 2020 at 12:13:13PM -0400, Mimi Zohar wrote: > > >Hi Sasha, > > > > > >On Wed, 2020-07-08 at 11:40 -0400, Sasha Levin wrote: > > >> From: Maurizio Drocco <maurizio.drocco@ibm.com> > > >> > > >> [ Upstream commit 20c59ce010f84300f6c655d32db2610d3433f85c ] > > >> > > >> Registers 8-9 are used to store measurements of the kernel and its > > >> command line (e.g., grub2 bootloader with tpm module enabled). IMA > > >> should include them in the boot aggregate. Registers 8-9 should be > > >> only included in non-SHA1 digests to avoid ambiguity. > > > > > >Prior to Linux 5.8, the SHA1 template data hashes were padded before > > >being extended into the TPM. Support for calculating and extending > > >the per TPM bank template data digests is only being upstreamed in > > >Linux 5.8. > > > > > >How will attestation servers know whether to include PCRs 8 & 9 in the > > >the boot_aggregate calculation? Now, there is a direct relationship > > >between the template data SHA1 padded digest not including PCRs 8 & 9, > > >and the new per TPM bank template data digest including them. > > > > Got it, I'll drop it then, thank you! > > After re-thinking this over, I realized that the attestation server can > verify the "boot_aggregate" based on the quoted PCRs without knowing > whether padded SHA1 hashes or per TPM bank hash values were extended > into the TPM[1], but non-SHA1 boot aggregate values [2] should always > include PCRs 8 & 9. I'm still not clear on how an attestation server would know to include PCRs 8 and 9 after this change came through a stable kernel update. It doesn't seem like something appropriate for stable since it requires code changes to attestation servers to handle the change. I know this has already been released in some stable releases, so I'm too late, but perhaps I'm missing something. Tyler > > Any place commit 6f1a1d103b48 was backported [2], this commit > 20c59ce010f8 ("ima: extend boot_aggregate with kernel measurements") > should be backported as well. > > thanks, > > Mimi > > [1] commit 1ea973df6e21 ("ima: Calculate and extend PCR with digests in ima_template_entry") > [2] commit 6f1a1d103b48 ("ima: Switch to ima_hash_algo for boot aggregate") > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH AUTOSEL 5.7 03/30] ima: extend boot_aggregate with kernel measurements 2020-12-11 3:10 ` Tyler Hicks @ 2020-12-11 11:01 ` Mimi Zohar 2020-12-11 17:46 ` James Bottomley 2020-12-14 16:42 ` Tyler Hicks 0 siblings, 2 replies; 16+ messages in thread From: Mimi Zohar @ 2020-12-11 11:01 UTC (permalink / raw) To: Tyler Hicks Cc: Sasha Levin, linux-kernel, stable, Maurizio Drocco, Bruno Meneguele, linux-integrity, linux-security-module On Thu, 2020-12-10 at 21:10 -0600, Tyler Hicks wrote: > On 2020-11-29 08:17:38, Mimi Zohar wrote: > > Hi Sasha, > > > > On Wed, 2020-07-08 at 21:27 -0400, Sasha Levin wrote: > > > On Wed, Jul 08, 2020 at 12:13:13PM -0400, Mimi Zohar wrote: > > > >Hi Sasha, > > > > > > > >On Wed, 2020-07-08 at 11:40 -0400, Sasha Levin wrote: > > > >> From: Maurizio Drocco <maurizio.drocco@ibm.com> > > > >> > > > >> [ Upstream commit 20c59ce010f84300f6c655d32db2610d3433f85c ] > > > >> > > > >> Registers 8-9 are used to store measurements of the kernel and its > > > >> command line (e.g., grub2 bootloader with tpm module enabled). IMA > > > >> should include them in the boot aggregate. Registers 8-9 should be > > > >> only included in non-SHA1 digests to avoid ambiguity. > > > > > > > >Prior to Linux 5.8, the SHA1 template data hashes were padded before > > > >being extended into the TPM. Support for calculating and extending > > > >the per TPM bank template data digests is only being upstreamed in > > > >Linux 5.8. > > > > > > > >How will attestation servers know whether to include PCRs 8 & 9 in the > > > >the boot_aggregate calculation? Now, there is a direct relationship > > > >between the template data SHA1 padded digest not including PCRs 8 & 9, > > > >and the new per TPM bank template data digest including them. > > > > > > Got it, I'll drop it then, thank you! > > > > After re-thinking this over, I realized that the attestation server can > > verify the "boot_aggregate" based on the quoted PCRs without knowing > > whether padded SHA1 hashes or per TPM bank hash values were extended > > into the TPM[1], but non-SHA1 boot aggregate values [2] should always > > include PCRs 8 & 9. > > I'm still not clear on how an attestation server would know to include > PCRs 8 and 9 after this change came through a stable kernel update. It > doesn't seem like something appropriate for stable since it requires > code changes to attestation servers to handle the change. > > I know this has already been released in some stable releases, so I'm > too late, but perhaps I'm missing something. The point of adding PCRs 8 & 9 only to non-SHA1 boot_aggregate values was to avoid affecting existing attestation servers. The intention was when attestation servers added support for the non-sha1 boot_aggregate values, they'd also include PCRs 8 & 9. The existing SHA1 boot_aggregate value remains PCRs 0 - 7. To prevent this or something similar from happening again, what should have been the proper way of including PCRs 8 & 9? thanks, Mimi ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH AUTOSEL 5.7 03/30] ima: extend boot_aggregate with kernel measurements 2020-12-11 11:01 ` Mimi Zohar @ 2020-12-11 17:46 ` James Bottomley 2020-12-13 2:22 ` Mimi Zohar 2020-12-14 16:42 ` Tyler Hicks 1 sibling, 1 reply; 16+ messages in thread From: James Bottomley @ 2020-12-11 17:46 UTC (permalink / raw) To: Mimi Zohar, Tyler Hicks Cc: Sasha Levin, linux-kernel, stable, Maurizio Drocco, Bruno Meneguele, linux-integrity, linux-security-module On Fri, 2020-12-11 at 06:01 -0500, Mimi Zohar wrote: > On Thu, 2020-12-10 at 21:10 -0600, Tyler Hicks wrote: > > On 2020-11-29 08:17:38, Mimi Zohar wrote: > > > Hi Sasha, > > > > > > On Wed, 2020-07-08 at 21:27 -0400, Sasha Levin wrote: > > > > On Wed, Jul 08, 2020 at 12:13:13PM -0400, Mimi Zohar wrote: > > > > > Hi Sasha, > > > > > > > > > > On Wed, 2020-07-08 at 11:40 -0400, Sasha Levin wrote: > > > > > > From: Maurizio Drocco <maurizio.drocco@ibm.com> > > > > > > > > > > > > [ Upstream commit 20c59ce010f84300f6c655d32db2610d3433f85c > > > > > > ] > > > > > > > > > > > > Registers 8-9 are used to store measurements of the kernel > > > > > > and its command line (e.g., grub2 bootloader with tpm > > > > > > module enabled). IMA should include them in the boot > > > > > > aggregate. Registers 8-9 should be only included in non- > > > > > > SHA1 digests to avoid ambiguity. > > > > > > > > > > Prior to Linux 5.8, the SHA1 template data hashes were padded > > > > > before being extended into the TPM. Support for calculating > > > > > and extending the per TPM bank template data digests is only > > > > > being upstreamed in Linux 5.8. > > > > > > > > > > How will attestation servers know whether to include PCRs 8 & > > > > > 9 in the the boot_aggregate calculation? Now, there is a > > > > > direct relationship between the template data SHA1 padded > > > > > digest not including PCRs 8 & 9, and the new per TPM bank > > > > > template data digest including them. > > > > > > > > Got it, I'll drop it then, thank you! > > > > > > After re-thinking this over, I realized that the attestation > > > server can verify the "boot_aggregate" based on the quoted PCRs > > > without knowing whether padded SHA1 hashes or per TPM bank hash > > > values were extended into the TPM[1], but non-SHA1 boot aggregate > > > values [2] should always include PCRs 8 & 9. > > > > I'm still not clear on how an attestation server would know to > > include PCRs 8 and 9 after this change came through a stable kernel > > update. It doesn't seem like something appropriate for stable since > > it requires code changes to attestation servers to handle the > > change. > > > > I know this has already been released in some stable releases, so > > I'm too late, but perhaps I'm missing something. > > The point of adding PCRs 8 & 9 only to non-SHA1 boot_aggregate values > was to avoid affecting existing attestation servers. The intention > was when attestation servers added support for the non-sha1 > boot_aggregate values, they'd also include PCRs 8 & 9. The existing > SHA1 boot_aggregate value remains PCRs 0 - 7. > > To prevent this or something similar from happening again, what > should have been the proper way of including PCRs 8 & 9? Just to be pragmatic: this is going to happen again. Shim is already measuring the Mok variables through PCR 14, so if we want an accurate boot aggregate, we're going to have to include PCR 14 as well (or persuade shim to measure through a PCR we're already including, which isn't impossible since I think shim should be measuring the Mok variables using the EV_EFI_VARIABLE_DRIVER_CONFIG event and, since it affects secure boot policy, that does argue it should be measured through PCR 7). James ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH AUTOSEL 5.7 03/30] ima: extend boot_aggregate with kernel measurements 2020-12-11 17:46 ` James Bottomley @ 2020-12-13 2:22 ` Mimi Zohar 2020-12-28 19:28 ` Ken Goldman 0 siblings, 1 reply; 16+ messages in thread From: Mimi Zohar @ 2020-12-13 2:22 UTC (permalink / raw) To: James Bottomley, Tyler Hicks Cc: Sasha Levin, linux-kernel, stable, Maurizio Drocco, Bruno Meneguele, linux-integrity, linux-security-module On Fri, 2020-12-11 at 09:46 -0800, James Bottomley wrote: > On Fri, 2020-12-11 at 06:01 -0500, Mimi Zohar wrote: > > On Thu, 2020-12-10 at 21:10 -0600, Tyler Hicks wrote: > > > On 2020-11-29 08:17:38, Mimi Zohar wrote: > > > > Hi Sasha, > > > > > > > > On Wed, 2020-07-08 at 21:27 -0400, Sasha Levin wrote: > > > > > On Wed, Jul 08, 2020 at 12:13:13PM -0400, Mimi Zohar wrote: > > > > > > Hi Sasha, > > > > > > > > > > > > On Wed, 2020-07-08 at 11:40 -0400, Sasha Levin wrote: > > > > > > > From: Maurizio Drocco <maurizio.drocco@ibm.com> > > > > > > > > > > > > > > [ Upstream commit 20c59ce010f84300f6c655d32db2610d3433f85c > > > > > > > ] > > > > > > > > > > > > > > Registers 8-9 are used to store measurements of the kernel > > > > > > > and its command line (e.g., grub2 bootloader with tpm > > > > > > > module enabled). IMA should include them in the boot > > > > > > > aggregate. Registers 8-9 should be only included in non- > > > > > > > SHA1 digests to avoid ambiguity. > > > > > > > > > > > > Prior to Linux 5.8, the SHA1 template data hashes were padded > > > > > > before being extended into the TPM. Support for calculating > > > > > > and extending the per TPM bank template data digests is only > > > > > > being upstreamed in Linux 5.8. > > > > > > > > > > > > How will attestation servers know whether to include PCRs 8 & > > > > > > 9 in the the boot_aggregate calculation? Now, there is a > > > > > > direct relationship between the template data SHA1 padded > > > > > > digest not including PCRs 8 & 9, and the new per TPM bank > > > > > > template data digest including them. > > > > > > > > > > Got it, I'll drop it then, thank you! > > > > > > > > After re-thinking this over, I realized that the attestation > > > > server can verify the "boot_aggregate" based on the quoted PCRs > > > > without knowing whether padded SHA1 hashes or per TPM bank hash > > > > values were extended into the TPM[1], but non-SHA1 boot aggregate > > > > values [2] should always include PCRs 8 & 9. > > > > > > I'm still not clear on how an attestation server would know to > > > include PCRs 8 and 9 after this change came through a stable kernel > > > update. It doesn't seem like something appropriate for stable since > > > it requires code changes to attestation servers to handle the > > > change. > > > > > > I know this has already been released in some stable releases, so > > > I'm too late, but perhaps I'm missing something. > > > > The point of adding PCRs 8 & 9 only to non-SHA1 boot_aggregate values > > was to avoid affecting existing attestation servers. The intention > > was when attestation servers added support for the non-sha1 > > boot_aggregate values, they'd also include PCRs 8 & 9. The existing > > SHA1 boot_aggregate value remains PCRs 0 - 7. > > > > To prevent this or something similar from happening again, what > > should have been the proper way of including PCRs 8 & 9? > > Just to be pragmatic: this is going to happen again. Shim is already > measuring the Mok variables through PCR 14, so if we want an accurate > boot aggregate, we're going to have to include PCR 14 as well (or > persuade shim to measure through a PCR we're already including, which > isn't impossible since I think shim should be measuring the Mok > variables using the EV_EFI_VARIABLE_DRIVER_CONFIG event and, since it > affects secure boot policy, that does argue it should be measured > through PCR 7). Ok. Going forward, it sounds like we need to define a new "boot_aggregate" record. One that contains a version number and PCR mask. Mimi ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH AUTOSEL 5.7 03/30] ima: extend boot_aggregate with kernel measurements 2020-12-13 2:22 ` Mimi Zohar @ 2020-12-28 19:28 ` Ken Goldman 2020-12-29 2:01 ` Mimi Zohar 0 siblings, 1 reply; 16+ messages in thread From: Ken Goldman @ 2020-12-28 19:28 UTC (permalink / raw) To: Mimi Zohar, James Bottomley, Tyler Hicks Cc: Sasha Levin, linux-kernel, stable, Maurizio Drocco, Bruno Meneguele, linux-integrity, linux-security-module On 12/12/2020 9:22 PM, Mimi Zohar wrote: > Ok. Going forward, it sounds like we need to define a new > "boot_aggregate" record. One that contains a version number and PCR > mask. Just BTW, there is a TCG standard for a TPM 2.0 PCR mask that works well. There is also a standard for an event log version number. It is the first event of a TPM 2.0 event log. It is strange. One useful field, though, is a mapping between the algorithm ID (e.g., sha256 is 0x000b) and the digest size (e.g., 32 bytes). This permits a parser to parse a log even when it encounters an unknown digest algorithm. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH AUTOSEL 5.7 03/30] ima: extend boot_aggregate with kernel measurements 2020-12-28 19:28 ` Ken Goldman @ 2020-12-29 2:01 ` Mimi Zohar 0 siblings, 0 replies; 16+ messages in thread From: Mimi Zohar @ 2020-12-29 2:01 UTC (permalink / raw) To: Ken Goldman, James Bottomley, Tyler Hicks Cc: Sasha Levin, linux-kernel, stable, Maurizio Drocco, Bruno Meneguele, linux-integrity, linux-security-module On Mon, 2020-12-28 at 14:28 -0500, Ken Goldman wrote: > On 12/12/2020 9:22 PM, Mimi Zohar wrote: > > Ok. Going forward, it sounds like we need to define a new > > "boot_aggregate" record. One that contains a version number and PCR > > mask. > > Just BTW, there is a TCG standard for a TPM 2.0 PCR mask that works > well. Sounds good. > > There is also a standard for an event log version number. It is > the first event of a TPM 2.0 event log. It is strange. Ok > > One useful field, though, is a mapping between the algorithm ID (e.g., > sha256 is 0x000b) and the digest size (e.g., 32 bytes). This permits > a parser to parse a log even when it encounters an unknown digest > algorithm. The template data is prefixed with the template data length. The problem is verifying the boot_aggregate, not parsing the log. thanks, Mimi ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH AUTOSEL 5.7 03/30] ima: extend boot_aggregate with kernel measurements 2020-12-11 11:01 ` Mimi Zohar 2020-12-11 17:46 ` James Bottomley @ 2020-12-14 16:42 ` Tyler Hicks 2021-01-12 15:35 ` Tyler Hicks 1 sibling, 1 reply; 16+ messages in thread From: Tyler Hicks @ 2020-12-14 16:42 UTC (permalink / raw) To: Mimi Zohar Cc: Sasha Levin, linux-kernel, stable, Maurizio Drocco, Bruno Meneguele, linux-integrity, linux-security-module On 2020-12-11 06:01:54, Mimi Zohar wrote: > On Thu, 2020-12-10 at 21:10 -0600, Tyler Hicks wrote: > > On 2020-11-29 08:17:38, Mimi Zohar wrote: > > > Hi Sasha, > > > > > > On Wed, 2020-07-08 at 21:27 -0400, Sasha Levin wrote: > > > > On Wed, Jul 08, 2020 at 12:13:13PM -0400, Mimi Zohar wrote: > > > > >Hi Sasha, > > > > > > > > > >On Wed, 2020-07-08 at 11:40 -0400, Sasha Levin wrote: > > > > >> From: Maurizio Drocco <maurizio.drocco@ibm.com> > > > > >> > > > > >> [ Upstream commit 20c59ce010f84300f6c655d32db2610d3433f85c ] > > > > >> > > > > >> Registers 8-9 are used to store measurements of the kernel and its > > > > >> command line (e.g., grub2 bootloader with tpm module enabled). IMA > > > > >> should include them in the boot aggregate. Registers 8-9 should be > > > > >> only included in non-SHA1 digests to avoid ambiguity. > > > > > > > > > >Prior to Linux 5.8, the SHA1 template data hashes were padded before > > > > >being extended into the TPM. Support for calculating and extending > > > > >the per TPM bank template data digests is only being upstreamed in > > > > >Linux 5.8. > > > > > > > > > >How will attestation servers know whether to include PCRs 8 & 9 in the > > > > >the boot_aggregate calculation? Now, there is a direct relationship > > > > >between the template data SHA1 padded digest not including PCRs 8 & 9, > > > > >and the new per TPM bank template data digest including them. > > > > > > > > Got it, I'll drop it then, thank you! > > > > > > After re-thinking this over, I realized that the attestation server can > > > verify the "boot_aggregate" based on the quoted PCRs without knowing > > > whether padded SHA1 hashes or per TPM bank hash values were extended > > > into the TPM[1], but non-SHA1 boot aggregate values [2] should always > > > include PCRs 8 & 9. > > > > I'm still not clear on how an attestation server would know to include > > PCRs 8 and 9 after this change came through a stable kernel update. It > > doesn't seem like something appropriate for stable since it requires > > code changes to attestation servers to handle the change. > > > > I know this has already been released in some stable releases, so I'm > > too late, but perhaps I'm missing something. > > The point of adding PCRs 8 & 9 only to non-SHA1 boot_aggregate values > was to avoid affecting existing attestation servers. The intention was > when attestation servers added support for the non-sha1 boot_aggregate > values, they'd also include PCRs 8 & 9. The existing SHA1 > boot_aggregate value remains PCRs 0 - 7. AFAIK, there's nothing that prevents the non-SHA1 TPM 2.0 PCR banks from being used even before v5.8, albeit with zero padded SHA1 digests. Existing attestation servers that already support that configuration are broken by this stable backport. > To prevent this or something similar from happening again, what should > have been the proper way of including PCRs 8 & 9? I don't think that commits like 6f1a1d103b48 ("ima: Switch to ima_hash_algo for boot aggregate") and 20c59ce010f8 ("ima: extend boot_aggregate with kernel measurements") should be backported to stable. Including PCRs 8 and 9 definitely makes sense to include in the boot_aggregate value but limiting such a change to "starting in 5.8", rather than "starting in 5.8 and 5.4.82", is the safer approach when attestation server modifications are required. Tyler > > thanks, > > Mimi > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH AUTOSEL 5.7 03/30] ima: extend boot_aggregate with kernel measurements 2020-12-14 16:42 ` Tyler Hicks @ 2021-01-12 15:35 ` Tyler Hicks 2021-01-12 16:56 ` Mimi Zohar 0 siblings, 1 reply; 16+ messages in thread From: Tyler Hicks @ 2021-01-12 15:35 UTC (permalink / raw) To: Mimi Zohar Cc: Sasha Levin, linux-kernel, stable, Maurizio Drocco, Bruno Meneguele, linux-integrity, linux-security-module On 2020-12-14 10:42:24, Tyler Hicks wrote: > On 2020-12-11 06:01:54, Mimi Zohar wrote: > > On Thu, 2020-12-10 at 21:10 -0600, Tyler Hicks wrote: > > > On 2020-11-29 08:17:38, Mimi Zohar wrote: > > > > Hi Sasha, > > > > > > > > On Wed, 2020-07-08 at 21:27 -0400, Sasha Levin wrote: > > > > > On Wed, Jul 08, 2020 at 12:13:13PM -0400, Mimi Zohar wrote: > > > > > >Hi Sasha, > > > > > > > > > > > >On Wed, 2020-07-08 at 11:40 -0400, Sasha Levin wrote: > > > > > >> From: Maurizio Drocco <maurizio.drocco@ibm.com> > > > > > >> > > > > > >> [ Upstream commit 20c59ce010f84300f6c655d32db2610d3433f85c ] > > > > > >> > > > > > >> Registers 8-9 are used to store measurements of the kernel and its > > > > > >> command line (e.g., grub2 bootloader with tpm module enabled). IMA > > > > > >> should include them in the boot aggregate. Registers 8-9 should be > > > > > >> only included in non-SHA1 digests to avoid ambiguity. > > > > > > > > > > > >Prior to Linux 5.8, the SHA1 template data hashes were padded before > > > > > >being extended into the TPM. Support for calculating and extending > > > > > >the per TPM bank template data digests is only being upstreamed in > > > > > >Linux 5.8. > > > > > > > > > > > >How will attestation servers know whether to include PCRs 8 & 9 in the > > > > > >the boot_aggregate calculation? Now, there is a direct relationship > > > > > >between the template data SHA1 padded digest not including PCRs 8 & 9, > > > > > >and the new per TPM bank template data digest including them. > > > > > > > > > > Got it, I'll drop it then, thank you! > > > > > > > > After re-thinking this over, I realized that the attestation server can > > > > verify the "boot_aggregate" based on the quoted PCRs without knowing > > > > whether padded SHA1 hashes or per TPM bank hash values were extended > > > > into the TPM[1], but non-SHA1 boot aggregate values [2] should always > > > > include PCRs 8 & 9. > > > > > > I'm still not clear on how an attestation server would know to include > > > PCRs 8 and 9 after this change came through a stable kernel update. It > > > doesn't seem like something appropriate for stable since it requires > > > code changes to attestation servers to handle the change. > > > > > > I know this has already been released in some stable releases, so I'm > > > too late, but perhaps I'm missing something. > > > > The point of adding PCRs 8 & 9 only to non-SHA1 boot_aggregate values > > was to avoid affecting existing attestation servers. The intention was > > when attestation servers added support for the non-sha1 boot_aggregate > > values, they'd also include PCRs 8 & 9. The existing SHA1 > > boot_aggregate value remains PCRs 0 - 7. > > AFAIK, there's nothing that prevents the non-SHA1 TPM 2.0 PCR banks from > being used even before v5.8, albeit with zero padded SHA1 digests. > Existing attestation servers that already support that configuration are > broken by this stable backport. To wrap up this thread, I think the last thing to address is if this commit should be reverted from stable kernels? Do you have any thoughts about that, Mimi? Tyler > > > To prevent this or something similar from happening again, what should > > have been the proper way of including PCRs 8 & 9? > > I don't think that commits like 6f1a1d103b48 ("ima: Switch to > ima_hash_algo for boot aggregate") and 20c59ce010f8 ("ima: extend > boot_aggregate with kernel measurements") should be backported to > stable. > > Including PCRs 8 and 9 definitely makes sense to include in the > boot_aggregate value but limiting such a change to "starting in 5.8", > rather than "starting in 5.8 and 5.4.82", is the safer approach when > attestation server modifications are required. > > Tyler > > > > > thanks, > > > > Mimi > > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH AUTOSEL 5.7 03/30] ima: extend boot_aggregate with kernel measurements 2021-01-12 15:35 ` Tyler Hicks @ 2021-01-12 16:56 ` Mimi Zohar 0 siblings, 0 replies; 16+ messages in thread From: Mimi Zohar @ 2021-01-12 16:56 UTC (permalink / raw) To: Tyler Hicks, Jerry Snitselaar Cc: Sasha Levin, linux-kernel, stable, Maurizio Drocco, Bruno Meneguele, linux-integrity, linux-security-module Hi Tyler, On Tue, 2021-01-12 at 09:35 -0600, Tyler Hicks wrote: > On 2020-12-14 10:42:24, Tyler Hicks wrote: > > On 2020-12-11 06:01:54, Mimi Zohar wrote: > > > On Thu, 2020-12-10 at 21:10 -0600, Tyler Hicks wrote: > > > > On 2020-11-29 08:17:38, Mimi Zohar wrote: > > > > > Hi Sasha, > > > > > > > > > > On Wed, 2020-07-08 at 21:27 -0400, Sasha Levin wrote: > > > > > > On Wed, Jul 08, 2020 at 12:13:13PM -0400, Mimi Zohar wrote: > > > > > > >Hi Sasha, > > > > > > > > > > > > > >On Wed, 2020-07-08 at 11:40 -0400, Sasha Levin wrote: > > > > > > >> From: Maurizio Drocco <maurizio.drocco@ibm.com> > > > > > > >> > > > > > > >> [ Upstream commit 20c59ce010f84300f6c655d32db2610d3433f85c ] > > > > > > >> > > > > > > >> Registers 8-9 are used to store measurements of the kernel and its > > > > > > >> command line (e.g., grub2 bootloader with tpm module enabled). IMA > > > > > > >> should include them in the boot aggregate. Registers 8-9 should be > > > > > > >> only included in non-SHA1 digests to avoid ambiguity. > > > > > > > > > > > > > >Prior to Linux 5.8, the SHA1 template data hashes were padded before > > > > > > >being extended into the TPM. Support for calculating and extending > > > > > > >the per TPM bank template data digests is only being upstreamed in > > > > > > >Linux 5.8. > > > > > > > > > > > > > >How will attestation servers know whether to include PCRs 8 & 9 in the > > > > > > >the boot_aggregate calculation? Now, there is a direct relationship > > > > > > >between the template data SHA1 padded digest not including PCRs 8 & 9, > > > > > > >and the new per TPM bank template data digest including them. > > > > > > > > > > > > Got it, I'll drop it then, thank you! > > > > > > > > > > After re-thinking this over, I realized that the attestation server can > > > > > verify the "boot_aggregate" based on the quoted PCRs without knowing > > > > > whether padded SHA1 hashes or per TPM bank hash values were extended > > > > > into the TPM[1], but non-SHA1 boot aggregate values [2] should always > > > > > include PCRs 8 & 9. > > > > > > > > I'm still not clear on how an attestation server would know to include > > > > PCRs 8 and 9 after this change came through a stable kernel update. It > > > > doesn't seem like something appropriate for stable since it requires > > > > code changes to attestation servers to handle the change. > > > > > > > > I know this has already been released in some stable releases, so I'm > > > > too late, but perhaps I'm missing something. > > > > > > The point of adding PCRs 8 & 9 only to non-SHA1 boot_aggregate values > > > was to avoid affecting existing attestation servers. The intention was > > > when attestation servers added support for the non-sha1 boot_aggregate > > > values, they'd also include PCRs 8 & 9. The existing SHA1 > > > boot_aggregate value remains PCRs 0 - 7. > > > > AFAIK, there's nothing that prevents the non-SHA1 TPM 2.0 PCR banks from > > being used even before v5.8, albeit with zero padded SHA1 digests. > > Existing attestation servers that already support that configuration are > > broken by this stable backport. > To wrap up this thread, I think the last thing to address is if this > commit should be reverted from stable kernels? Do you have any thoughts > about that, Mimi? > > > > > > To prevent this or something similar from happening again, what should > > > have been the proper way of including PCRs 8 & 9? > > > > I don't think that commits like 6f1a1d103b48 ("ima: Switch to > > ima_hash_algo for boot aggregate") and 20c59ce010f8 ("ima: extend > > boot_aggregate with kernel measurements") should be backported to > > stable. > > > > Including PCRs 8 and 9 definitely makes sense to include in the > > boot_aggregate value but limiting such a change to "starting in 5.8", > > rather than "starting in 5.8 and 5.4.82", is the safer approach when > > attestation server modifications are required. As I recall, commit 6f1a1d103b48 ("ima: Switch to ima_hash_algo for boot aggregate") was backported to address TPMs without SHA1 support, as reported by Jerry. Mimi ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2021-01-12 16:58 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <20200708154116.3199728-1-sashal@kernel.org> 2020-07-08 15:40 ` [PATCH AUTOSEL 5.7 03/30] ima: extend boot_aggregate with kernel measurements Sasha Levin 2020-07-08 16:13 ` Mimi Zohar 2020-07-09 1:27 ` Sasha Levin 2020-11-29 13:17 ` Mimi Zohar 2020-12-01 0:21 ` Sasha Levin 2020-12-01 3:13 ` Mimi Zohar 2020-12-02 23:53 ` Sasha Levin 2020-12-11 3:10 ` Tyler Hicks 2020-12-11 11:01 ` Mimi Zohar 2020-12-11 17:46 ` James Bottomley 2020-12-13 2:22 ` Mimi Zohar 2020-12-28 19:28 ` Ken Goldman 2020-12-29 2:01 ` Mimi Zohar 2020-12-14 16:42 ` Tyler Hicks 2021-01-12 15:35 ` Tyler Hicks 2021-01-12 16:56 ` Mimi Zohar
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).