IMA is not considering TPM registers 8-9 when calculating the boot
aggregate. When 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.
Signed-off-by: Maurizio Drocco <[email protected]>
---
security/integrity/ima/ima.h | 2 +-
security/integrity/ima/ima_crypto.c | 11 ++++++++++-
2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index df93ac258e01..9d94080bdad8 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 220b14920c37..6f0137bdaf61 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -809,7 +809,7 @@ static void ima_pcrread(u32 idx, struct tpm_digest *d)
static int ima_calc_boot_aggregate_tfm(char *digest, u16 alg_id,
struct crypto_shash *tfm)
{
- struct tpm_digest d = { .alg_id = alg_id, .digest = {0} };
+ struct tpm_digest d = { .alg_id = alg_id, .digest = {0} }, d0 = d;
int rc;
u32 i;
SHASH_DESC_ON_STACK(shash, tfm);
@@ -830,6 +830,15 @@ static int ima_calc_boot_aggregate_tfm(char *digest, u16 alg_id,
rc = crypto_shash_update(shash, d.digest,
crypto_shash_digestsize(tfm));
}
+ /* extend cumulative sha1 over tpm registers 8-9 */
+ for (i = TPM_PCR8; i < TPM_PCR10; i++) {
+ ima_pcrread(i, &d);
+ /* if not zero, accumulate with current aggregate */
+ if (memcmp(d.digest, d0.digest,
+ crypto_shash_digestsize(tfm) != 0))
+ rc = crypto_shash_update(shash, d.digest,
+ crypto_shash_digestsize(tfm));
+ }
if (!rc)
crypto_shash_final(shash, digest);
return rc;
--
2.17.1
Hi Maurizo,
On Thu, 2020-06-11 at 15:54 -0400, Maurizio Drocco wrote:
> IMA is not considering TPM registers 8-9 when calculating the boot
> aggregate. When 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.
>
> Signed-off-by: Maurizio Drocco <[email protected]>
Looks good. Just a minor comment below. Could you be a bit more
specific as to what is being measured into which PCR. Perhaps include
a reference to some doc or spec.
In order to test, ima-evm-utils needs to be updated as well. Could
you post the corresponding evmctl change? Please post the patch
against the ima-evm-utils next-testing branch.
> ---
> security/integrity/ima/ima.h | 2 +-
> security/integrity/ima/ima_crypto.c | 11 ++++++++++-
> 2 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index df93ac258e01..9d94080bdad8 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 220b14920c37..6f0137bdaf61 100644
> --- a/security/integrity/ima/ima_crypto.c
> +++ b/security/integrity/ima/ima_crypto.c
> @@ -809,7 +809,7 @@ static void ima_pcrread(u32 idx, struct tpm_digest *d)
> static int ima_calc_boot_aggregate_tfm(char *digest, u16 alg_id,
> struct crypto_shash *tfm)
> {
> - struct tpm_digest d = { .alg_id = alg_id, .digest = {0} };
> + struct tpm_digest d = { .alg_id = alg_id, .digest = {0} }, d0 = d;
> int rc;
> u32 i;
> SHASH_DESC_ON_STACK(shash, tfm);
> @@ -830,6 +830,15 @@ static int ima_calc_boot_aggregate_tfm(char *digest, u16 alg_id,
> rc = crypto_shash_update(shash, d.digest,
> crypto_shash_digestsize(tfm));
> }
> + /* extend cumulative sha1 over tpm registers 8-9 */
> + for (i = TPM_PCR8; i < TPM_PCR10; i++) {
> + ima_pcrread(i, &d);
> + /* if not zero, accumulate with current aggregate */
> + if (memcmp(d.digest, d0.digest,
> + crypto_shash_digestsize(tfm) != 0))
The formatting here is a bit off.
thanks,
Mimi
> + rc = crypto_shash_update(shash, d.digest,
> + crypto_shash_digestsize(tfm));
> + }
> if (!rc)
> crypto_shash_final(shash, digest);
> return rc;
Hi Maurizio,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on integrity/next-integrity]
[also build test WARNING on next-20200611]
[cannot apply to v5.7]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Maurizio-Drocco/extend-IMA-boot_aggregate-with-kernel-measurements/20200612-091504
base: https://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git next-integrity
config: x86_64-allyesconfig (attached as .config)
compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project 3b43f006294971b8049d4807110032169780e5b8)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install x86_64 cross compiling tool for clang build
# apt-get install binutils-x86-64-linux-gnu
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
All warnings (new ones prefixed by >>, old ones prefixed by <<):
>> security/integrity/ima/ima_crypto.c:838:35: warning: size argument in 'memcmp' call is a comparison [-Wmemsize-comparison]
crypto_shash_digestsize(tfm) != 0))
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
security/integrity/ima/ima_crypto.c:837:7: note: did you mean to compare the result of 'memcmp' instead?
if (memcmp(d.digest, d0.digest,
^
security/integrity/ima/ima_crypto.c:838:6: note: explicitly cast the argument to size_t to silence this warning
crypto_shash_digestsize(tfm) != 0))
^
(size_t)( )
1 warning generated.
vim +/memcmp +838 security/integrity/ima/ima_crypto.c
797
798 /*
799 * The boot_aggregate is a cumulative hash over TPM registers 0 - 7. With
800 * TPM 1.2 the boot_aggregate was based on reading the SHA1 PCRs, but with
801 * TPM 2.0 hash agility, TPM chips could support multiple TPM PCR banks,
802 * allowing firmware to configure and enable different banks.
803 *
804 * Knowing which TPM bank is read to calculate the boot_aggregate digest
805 * needs to be conveyed to a verifier. For this reason, use the same
806 * hash algorithm for reading the TPM PCRs as for calculating the boot
807 * aggregate digest as stored in the measurement list.
808 */
809 static int ima_calc_boot_aggregate_tfm(char *digest, u16 alg_id,
810 struct crypto_shash *tfm)
811 {
812 struct tpm_digest d = { .alg_id = alg_id, .digest = {0} }, d0 = d;
813 int rc;
814 u32 i;
815 SHASH_DESC_ON_STACK(shash, tfm);
816
817 shash->tfm = tfm;
818
819 pr_devel("calculating the boot-aggregate based on TPM bank: %04x\n",
820 d.alg_id);
821
822 rc = crypto_shash_init(shash);
823 if (rc != 0)
824 return rc;
825
826 /* cumulative sha1 over tpm registers 0-7 */
827 for (i = TPM_PCR0; i < TPM_PCR8; i++) {
828 ima_pcrread(i, &d);
829 /* now accumulate with current aggregate */
830 rc = crypto_shash_update(shash, d.digest,
831 crypto_shash_digestsize(tfm));
832 }
833 /* extend cumulative sha1 over tpm registers 8-9 */
834 for (i = TPM_PCR8; i < TPM_PCR10; i++) {
835 ima_pcrread(i, &d);
836 /* if not zero, accumulate with current aggregate */
837 if (memcmp(d.digest, d0.digest,
> 838 crypto_shash_digestsize(tfm) != 0))
839 rc = crypto_shash_update(shash, d.digest,
840 crypto_shash_digestsize(tfm));
841 }
842 if (!rc)
843 crypto_shash_final(shash, digest);
844 return rc;
845 }
846
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]
IMA is not considering TPM registers 8-9 when calculating the boot
aggregate. When 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.
Signed-off-by: Maurizio Drocco <[email protected]>
---
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 df93ac258e01..9d94080bdad8 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 220b14920c37..64f5e3151e18 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -809,7 +809,7 @@ static void ima_pcrread(u32 idx, struct tpm_digest *d)
static int ima_calc_boot_aggregate_tfm(char *digest, u16 alg_id,
struct crypto_shash *tfm)
{
- struct tpm_digest d = { .alg_id = alg_id, .digest = {0} };
+ struct tpm_digest d = { .alg_id = alg_id, .digest = {0} }, d0 = d;
int rc;
u32 i;
SHASH_DESC_ON_STACK(shash, tfm);
@@ -830,6 +830,19 @@ static int ima_calc_boot_aggregate_tfm(char *digest, u16 alg_id,
rc = crypto_shash_update(shash, d.digest,
crypto_shash_digestsize(tfm));
}
+ /*
+ * extend cumulative sha1 over tpm registers 8-9, which contain
+ * measurement for the kernel command line (reg. 8) and image (reg. 9)
+ * in a typical PCR allocation.
+ */
+ for (i = TPM_PCR8; i < TPM_PCR10; i++) {
+ ima_pcrread(i, &d);
+ /* if not zero, accumulate with current aggregate */
+ if (memcmp(d.digest, d0.digest,
+ crypto_shash_digestsize(tfm)) != 0)
+ rc = crypto_shash_update(shash, d.digest,
+ crypto_shash_digestsize(tfm));
+ }
if (!rc)
crypto_shash_final(shash, digest);
return rc;
--
2.17.1
> From: [email protected] [mailto:linux-integrity-
> [email protected]] On Behalf Of Maurizio Drocco
> Sent: Friday, June 12, 2020 4:38 PM
> IMA is not considering TPM registers 8-9 when calculating the boot
> aggregate. When 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.
>
> Signed-off-by: Maurizio Drocco <[email protected]>
> ---
> 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 df93ac258e01..9d94080bdad8 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 220b14920c37..64f5e3151e18 100644
> --- a/security/integrity/ima/ima_crypto.c
> +++ b/security/integrity/ima/ima_crypto.c
> @@ -809,7 +809,7 @@ static void ima_pcrread(u32 idx, struct tpm_digest *d)
> static int ima_calc_boot_aggregate_tfm(char *digest, u16 alg_id,
> struct crypto_shash *tfm)
> {
> - struct tpm_digest d = { .alg_id = alg_id, .digest = {0} };
> + struct tpm_digest d = { .alg_id = alg_id, .digest = {0} }, d0 = d;
> int rc;
> u32 i;
> SHASH_DESC_ON_STACK(shash, tfm);
> @@ -830,6 +830,19 @@ static int ima_calc_boot_aggregate_tfm(char
> *digest, u16 alg_id,
> rc = crypto_shash_update(shash, d.digest,
> crypto_shash_digestsize(tfm));
> }
> + /*
> + * extend cumulative sha1 over tpm registers 8-9, which contain
Hi Maurizio
with recent patches, boot_aggregate can be calculated from non-SHA1
PCR banks. I would replace with:
Extend cumulative digest over ...
Given that with this patch boot_aggregate is calculated differently,
shouldn't we call it boot_aggregate_v2 and enable it with a new
option?
Thanks
Roberto
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Li Jian, Shi Yanli
> + * measurement for the kernel command line (reg. 8) and image
> (reg. 9)
> + * in a typical PCR allocation.
> + */
> + for (i = TPM_PCR8; i < TPM_PCR10; i++) {
> + ima_pcrread(i, &d);
> + /* if not zero, accumulate with current aggregate */
> + if (memcmp(d.digest, d0.digest,
> + crypto_shash_digestsize(tfm)) != 0)
> + rc = crypto_shash_update(shash, d.digest,
> +
> crypto_shash_digestsize(tfm));
> + }
> if (!rc)
> crypto_shash_final(shash, digest);
> return rc;
> --
> 2.17.1
On Fri, 2020-06-12 at 15:11 +0000, Roberto Sassu wrote:
> with recent patches, boot_aggregate can be calculated from non-SHA1
> PCR banks. I would replace with:
>
> Extend cumulative digest over ...
>
> Given that with this patch boot_aggregate is calculated differently,
> shouldn't we call it boot_aggregate_v2 and enable it with a new
> option?
So here's the problem: if your current grub doesn't do any TPM
extensions (as most don't), then the two boot aggregates are the same
because PCRs 8 and 9 are zero and there's a test that doesn't add them
to the aggregate if they are zero. For these people its a nop so we
shouldn't force them to choose a different version of the same thing.
If, however, you're on a distribution where grub is automatically
measuring the kernel and command line into PCRs 8 and 9 (I think Fedora
32 does this), your boot aggregate will change. It strikes me in that
case we can call this a bug fix, since the boot aggregate isn't
properly binding to the previous measurements without PCRs 8 and 9. In
this case, do we want to allow people to select an option which doesn't
properly bind the IMA log to the boot measurements? That sounds like a
security hole to me.
However, since it causes a user visible difference in the grub already
measures case, do you have a current use case that would be affected?
As in are lots of people already running a distro with the TPM grub
updates and relying on the old boot aggregate?
James
> From: James Bottomley [mailto:[email protected]]
> Sent: Friday, June 12, 2020 7:14 PM
> On Fri, 2020-06-12 at 15:11 +0000, Roberto Sassu wrote:
> > with recent patches, boot_aggregate can be calculated from non-SHA1
> > PCR banks. I would replace with:
> >
> > Extend cumulative digest over ...
> >
> > Given that with this patch boot_aggregate is calculated differently,
> > shouldn't we call it boot_aggregate_v2 and enable it with a new
> > option?
>
> So here's the problem: if your current grub doesn't do any TPM
> extensions (as most don't), then the two boot aggregates are the same
> because PCRs 8 and 9 are zero and there's a test that doesn't add them
> to the aggregate if they are zero. For these people its a nop so we
> shouldn't force them to choose a different version of the same thing.
>
> If, however, you're on a distribution where grub is automatically
> measuring the kernel and command line into PCRs 8 and 9 (I think Fedora
> 32 does this), your boot aggregate will change. It strikes me in that
> case we can call this a bug fix, since the boot aggregate isn't
> properly binding to the previous measurements without PCRs 8 and 9. In
> this case, do we want to allow people to select an option which doesn't
> properly bind the IMA log to the boot measurements? That sounds like a
> security hole to me.
>
> However, since it causes a user visible difference in the grub already
> measures case, do you have a current use case that would be affected?
> As in are lots of people already running a distro with the TPM grub
> updates and relying on the old boot aggregate?
I don't know how many people would be affected. However, if an
attestation tool processes both measurement lists from unpatched kernels
and patched kernels, keeping the same name would be a problem as it
cannot be determined from the measurement list how boot_aggregate
was calculated.
Anyway, I agree this should be fixed. At least, I suggest to add a Fixes tag,
to ensure that this patch is applied to all stable kernels.
Roberto
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Li Jian, Shi Yanli
On Tue, 2020-06-16 at 17:29 +0000, Roberto Sassu wrote:
> > From: James Bottomley [mailto:[email protected]]
> > Sent: Friday, June 12, 2020 7:14 PM
> > On Fri, 2020-06-12 at 15:11 +0000, Roberto Sassu wrote:
> > > with recent patches, boot_aggregate can be calculated from non-SHA1
> > > PCR banks. I would replace with:
> > >
> > > Extend cumulative digest over ...
> > >
> > > Given that with this patch boot_aggregate is calculated differently,
> > > shouldn't we call it boot_aggregate_v2 and enable it with a new
> > > option?
> >
> > So here's the problem: if your current grub doesn't do any TPM
> > extensions (as most don't), then the two boot aggregates are the same
> > because PCRs 8 and 9 are zero and there's a test that doesn't add them
> > to the aggregate if they are zero. For these people its a nop so we
> > shouldn't force them to choose a different version of the same thing.
> >
> > If, however, you're on a distribution where grub is automatically
> > measuring the kernel and command line into PCRs 8 and 9 (I think Fedora
> > 32 does this), your boot aggregate will change. It strikes me in that
> > case we can call this a bug fix, since the boot aggregate isn't
> > properly binding to the previous measurements without PCRs 8 and 9. In
> > this case, do we want to allow people to select an option which doesn't
> > properly bind the IMA log to the boot measurements? That sounds like a
> > security hole to me.
> >
> > However, since it causes a user visible difference in the grub already
> > measures case, do you have a current use case that would be affected?
> > As in are lots of people already running a distro with the TPM grub
> > updates and relying on the old boot aggregate?
>
> I don't know how many people would be affected. However, if an
> attestation tool processes both measurement lists from unpatched kernels
> and patched kernels, keeping the same name would be a problem as it
> cannot be determined from the measurement list how boot_aggregate
> was calculated.
>
> Anyway, I agree this should be fixed. At least, I suggest to add a Fixes tag,
> to ensure that this patch is applied to all stable kernels.
The boot aggregate on existing systems would be sha1. Does it make
sense to limit this change to larger digests? Anyone backporting
support for larger digests would also need to backport this change as
well.
Mimi
> From: Mimi Zohar [mailto:[email protected]]
> Sent: Tuesday, June 16, 2020 8:11 PM
> On Tue, 2020-06-16 at 17:29 +0000, Roberto Sassu wrote:
> > > From: James Bottomley [mailto:[email protected]]
> > > Sent: Friday, June 12, 2020 7:14 PM
> > > On Fri, 2020-06-12 at 15:11 +0000, Roberto Sassu wrote:
> > > > with recent patches, boot_aggregate can be calculated from non-SHA1
> > > > PCR banks. I would replace with:
> > > >
> > > > Extend cumulative digest over ...
> > > >
> > > > Given that with this patch boot_aggregate is calculated differently,
> > > > shouldn't we call it boot_aggregate_v2 and enable it with a new
> > > > option?
> > >
> > > So here's the problem: if your current grub doesn't do any TPM
> > > extensions (as most don't), then the two boot aggregates are the same
> > > because PCRs 8 and 9 are zero and there's a test that doesn't add them
> > > to the aggregate if they are zero. For these people its a nop so we
> > > shouldn't force them to choose a different version of the same thing.
> > >
> > > If, however, you're on a distribution where grub is automatically
> > > measuring the kernel and command line into PCRs 8 and 9 (I think
> Fedora
> > > 32 does this), your boot aggregate will change. It strikes me in that
> > > case we can call this a bug fix, since the boot aggregate isn't
> > > properly binding to the previous measurements without PCRs 8 and 9.
> In
> > > this case, do we want to allow people to select an option which doesn't
> > > properly bind the IMA log to the boot measurements? That sounds like
> a
> > > security hole to me.
> > >
> > > However, since it causes a user visible difference in the grub already
> > > measures case, do you have a current use case that would be affected?
> > > As in are lots of people already running a distro with the TPM grub
> > > updates and relying on the old boot aggregate?
> >
> > I don't know how many people would be affected. However, if an
> > attestation tool processes both measurement lists from unpatched
> kernels
> > and patched kernels, keeping the same name would be a problem as it
> > cannot be determined from the measurement list how boot_aggregate
> > was calculated.
> >
> > Anyway, I agree this should be fixed. At least, I suggest to add a Fixes tag,
> > to ensure that this patch is applied to all stable kernels.
>
> The boot aggregate on existing systems would be sha1. Does it make
> sense to limit this change to larger digests? Anyone backporting
> support for larger digests would also need to backport this change as
> well.
Yes, it would be a safe choice.
Roberto
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Li Jian, Shi Yanli
IMA is not considering TPM registers 8-9 when calculating the boot
aggregate. When 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.
Signed-off-by: Maurizio Drocco <[email protected]>
---
security/integrity/ima/ima.h | 2 +-
security/integrity/ima/ima_crypto.c | 20 ++++++++++++++++++--
2 files changed, 19 insertions(+), 3 deletions(-)
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index df93ac258e01..9d94080bdad8 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 220b14920c37..299b23dad8d9 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -809,7 +809,7 @@ static void ima_pcrread(u32 idx, struct tpm_digest *d)
static int ima_calc_boot_aggregate_tfm(char *digest, u16 alg_id,
struct crypto_shash *tfm)
{
- struct tpm_digest d = { .alg_id = alg_id, .digest = {0} };
+ struct tpm_digest d = { .alg_id = alg_id, .digest = {0} }, d0 = d;
int rc;
u32 i;
SHASH_DESC_ON_STACK(shash, tfm);
@@ -823,13 +823,29 @@ 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);
+ /* if not zero, accumulate with current aggregate */
+ if (memcmp(d.digest, d0.digest,
+ crypto_shash_digestsize(tfm)) != 0)
+ rc = crypto_shash_update(shash, d.digest,
+ crypto_shash_digestsize(tfm));
+ }
+ }
if (!rc)
crypto_shash_final(shash, digest);
return rc;
--
2.17.1