Re: [PATCH v2 2/9] tpm: Avoid code bloat when not using EFI_TCG2_PROTOCOL
On Sun, Jun 23, 2024 at 03:52:12PM -0600, Simon Glass wrote: > Hi Tom, > > On Fri, 21 Jun 2024 at 16:12, Tom Rini wrote: > > > > On Fri, Jun 21, 2024 at 01:38:07PM -0600, Simon Glass wrote: > > > Hi Tom, > > > > > > On Fri, 21 Jun 2024 at 13:19, Tom Rini wrote: > > > > > > > > On Fri, Jun 21, 2024 at 11:55:42AM -0600, Simon Glass wrote: > > > > > Hi Tom, > > > > > > > > > > On Fri, 21 Jun 2024 at 10:05, Tom Rini wrote: > > > > [snip] > > > > > > Yes, I very much do not like guessing about 3 numbers instead of > > > > > > guessing about 1 number and using the standard mechanism we already > > > > > > have. Please use BOARD_SIZE_LIMIT as this is the standard mechanism > > > > > > to > > > > > > enforce size limits on U-Boot itself. > > > > > > > > > > If it were that easy I would have sent a patch :-) > > > > > > > > > > Here is the map for this board: > > > > > > > > > > ImagePosOffset Size Name > > > > > 0080 rom > > > > > ff80 ff80 1000 intel-descriptor > > > > > ff801000 ff801000 001ff000 intel-me > > > > > ffef ffef 000999f0 u-boot-with-ucode-ptr > > > > > fff899f0 fff899f0 5554 u-boot-dtb-with-ucode > > > > > fff8ef50 fff8ef50 u-boot-ucode > > > > > fff8ef50 fff8ef50 0571 fdtmap > > > > > fff9 fff9 0001 intel-vga > > > > > fffa fffa 0002fc94 intel-mrc > > > > > fffcfc94 fffcfc94 private-files > > > > > f800 f800 0070 x86-start16 > > > > > fff0 fff0 0005 x86-reset16 > > > > > fff8 fff8 0008 image-header > > > > > > > > > > What limit should I set on what? > > > > > > > > Is this a trick question? > > > > $ printf %d\\n $(( 0xfff9 - 0xffef)) > > > > 655360 > > > > > > > > Of course since we're less than that today, you can reduce it by > > > > whatever other magic numbers I'm not seeing but are part of your assumed > > > > sizes. > > > > > > That limit is on u-boot-nodtb.bin. Even with a size (for that file) of > > > 634816 it doesn't fit. I need to calculate a size based on the size of > > > the dtb and the microcode...which of course can change. > > > > Yes, and you're able to assume some size for them, which is what you > > put in the dts file? > > I just put in a limit for the blobs, whose sizes are known. > > > > > > > > - the U-Boot is the thing you are wanting to limit > > > > > - the dtb has microcode added > > > > > - the ucode is empty in this case > > > > > - the fdtmap is variable in size > > > > > > > > > > So this all seems a bit backwards. The actual limit is that > > > > > (u-boot-with-ucode-ptr + u-boot-dtb-with-ucode + u-boot-ucode + > > > > > fdtmap) fits in the space available. Note that some boards don't have > > > > > intel-vga or intel-mrc. > > > > > > > > > > With the other patch I sent I can have a sensible limit for all x86 > > > > > boards. > > > > > > Did you miss the comments above? > > > > No, I saw them. They're similar constraints to other systems. > > > > > > > > > > > And you can set the same sensible limit with the existing mechanism with > > > > the bonus of it not making x86 different from the rest? > > > > > > I understand that it is possible to set a limit for u-boot-nodtb.bin > > > but that is not accurate nor sufficient in the presence of blobs. The > > > solution belongs in Binman. > > > > Your series puts reasonable estimates on the size of the blobs and then > > will give a failure such as: > > binman: Node '/binman/rom/intel-vga': Offset 0xfff9 (4294508544) > > overlaps with previous entry '/binman/rom/fdtmap' ending at 0xfff902e1 > > (4294509281) > > Which is not as nice as (I just threw in a limit): > > u-boot-nodtb.bin exceeds file size limit: > > limit: 0x927c0 bytes > > actual: 0x9a810 bytes > > excess: 0x8050 bytes > > make[1]: *** [/home/trini/work/u-boot/u-boot/Makefile:1359: > > u-boot-nodtb.bin] Error 1 > > > > And tells us how much we need to get back size wise. Aside from when > > using the actual blobs (and in which case a real error will be shown > > when trying to use them), it's always about making an estimate on the > > part of the system that we control. > > Thanks for digging into this. > > I wasn't aware that the limit was just an estimate. Since it produces > a build error it seems to be enforced as a hard requirement. But I can > set a limit and we'll see how things go. > > I still like the binman block-size thing though. The error message > binman provides actually tells you what to do (reduce the space of the > things that are overflowing into intel-vga). So I'd like to add that > too. I think this is a reasonable compromise, thank you. -- Tom signature.asc Description: PGP signature
Re: [PATCH v2 2/9] tpm: Avoid code bloat when not using EFI_TCG2_PROTOCOL
Hi Tom, On Fri, 21 Jun 2024 at 16:12, Tom Rini wrote: > > On Fri, Jun 21, 2024 at 01:38:07PM -0600, Simon Glass wrote: > > Hi Tom, > > > > On Fri, 21 Jun 2024 at 13:19, Tom Rini wrote: > > > > > > On Fri, Jun 21, 2024 at 11:55:42AM -0600, Simon Glass wrote: > > > > Hi Tom, > > > > > > > > On Fri, 21 Jun 2024 at 10:05, Tom Rini wrote: > > > [snip] > > > > > Yes, I very much do not like guessing about 3 numbers instead of > > > > > guessing about 1 number and using the standard mechanism we already > > > > > have. Please use BOARD_SIZE_LIMIT as this is the standard mechanism to > > > > > enforce size limits on U-Boot itself. > > > > > > > > If it were that easy I would have sent a patch :-) > > > > > > > > Here is the map for this board: > > > > > > > > ImagePosOffset Size Name > > > > 0080 rom > > > > ff80 ff80 1000 intel-descriptor > > > > ff801000 ff801000 001ff000 intel-me > > > > ffef ffef 000999f0 u-boot-with-ucode-ptr > > > > fff899f0 fff899f0 5554 u-boot-dtb-with-ucode > > > > fff8ef50 fff8ef50 u-boot-ucode > > > > fff8ef50 fff8ef50 0571 fdtmap > > > > fff9 fff9 0001 intel-vga > > > > fffa fffa 0002fc94 intel-mrc > > > > fffcfc94 fffcfc94 private-files > > > > f800 f800 0070 x86-start16 > > > > fff0 fff0 0005 x86-reset16 > > > > fff8 fff8 0008 image-header > > > > > > > > What limit should I set on what? > > > > > > Is this a trick question? > > > $ printf %d\\n $(( 0xfff9 - 0xffef)) > > > 655360 > > > > > > Of course since we're less than that today, you can reduce it by > > > whatever other magic numbers I'm not seeing but are part of your assumed > > > sizes. > > > > That limit is on u-boot-nodtb.bin. Even with a size (for that file) of > > 634816 it doesn't fit. I need to calculate a size based on the size of > > the dtb and the microcode...which of course can change. > > Yes, and you're able to assume some size for them, which is what you > put in the dts file? I just put in a limit for the blobs, whose sizes are known. > > > > > - the U-Boot is the thing you are wanting to limit > > > > - the dtb has microcode added > > > > - the ucode is empty in this case > > > > - the fdtmap is variable in size > > > > > > > > So this all seems a bit backwards. The actual limit is that > > > > (u-boot-with-ucode-ptr + u-boot-dtb-with-ucode + u-boot-ucode + > > > > fdtmap) fits in the space available. Note that some boards don't have > > > > intel-vga or intel-mrc. > > > > > > > > With the other patch I sent I can have a sensible limit for all x86 > > > > boards. > > > > Did you miss the comments above? > > No, I saw them. They're similar constraints to other systems. > > > > > > > > And you can set the same sensible limit with the existing mechanism with > > > the bonus of it not making x86 different from the rest? > > > > I understand that it is possible to set a limit for u-boot-nodtb.bin > > but that is not accurate nor sufficient in the presence of blobs. The > > solution belongs in Binman. > > Your series puts reasonable estimates on the size of the blobs and then > will give a failure such as: > binman: Node '/binman/rom/intel-vga': Offset 0xfff9 (4294508544) > overlaps with previous entry '/binman/rom/fdtmap' ending at 0xfff902e1 > (4294509281) > Which is not as nice as (I just threw in a limit): > u-boot-nodtb.bin exceeds file size limit: > limit: 0x927c0 bytes > actual: 0x9a810 bytes > excess: 0x8050 bytes > make[1]: *** [/home/trini/work/u-boot/u-boot/Makefile:1359: u-boot-nodtb.bin] > Error 1 > > And tells us how much we need to get back size wise. Aside from when > using the actual blobs (and in which case a real error will be shown > when trying to use them), it's always about making an estimate on the > part of the system that we control. Thanks for digging into this. I wasn't aware that the limit was just an estimate. Since it produces a build error it seems to be enforced as a hard requirement. But I can set a limit and we'll see how things go. I still like the binman block-size thing though. The error message binman provides actually tells you what to do (reduce the space of the things that are overflowing into intel-vga). So I'd like to add that too. Regards, Simon
Re: [PATCH v2 2/9] tpm: Avoid code bloat when not using EFI_TCG2_PROTOCOL
On Fri, Jun 21, 2024 at 01:38:07PM -0600, Simon Glass wrote: > Hi Tom, > > On Fri, 21 Jun 2024 at 13:19, Tom Rini wrote: > > > > On Fri, Jun 21, 2024 at 11:55:42AM -0600, Simon Glass wrote: > > > Hi Tom, > > > > > > On Fri, 21 Jun 2024 at 10:05, Tom Rini wrote: > > [snip] > > > > Yes, I very much do not like guessing about 3 numbers instead of > > > > guessing about 1 number and using the standard mechanism we already > > > > have. Please use BOARD_SIZE_LIMIT as this is the standard mechanism to > > > > enforce size limits on U-Boot itself. > > > > > > If it were that easy I would have sent a patch :-) > > > > > > Here is the map for this board: > > > > > > ImagePosOffset Size Name > > > 0080 rom > > > ff80 ff80 1000 intel-descriptor > > > ff801000 ff801000 001ff000 intel-me > > > ffef ffef 000999f0 u-boot-with-ucode-ptr > > > fff899f0 fff899f0 5554 u-boot-dtb-with-ucode > > > fff8ef50 fff8ef50 u-boot-ucode > > > fff8ef50 fff8ef50 0571 fdtmap > > > fff9 fff9 0001 intel-vga > > > fffa fffa 0002fc94 intel-mrc > > > fffcfc94 fffcfc94 private-files > > > f800 f800 0070 x86-start16 > > > fff0 fff0 0005 x86-reset16 > > > fff8 fff8 0008 image-header > > > > > > What limit should I set on what? > > > > Is this a trick question? > > $ printf %d\\n $(( 0xfff9 - 0xffef)) > > 655360 > > > > Of course since we're less than that today, you can reduce it by > > whatever other magic numbers I'm not seeing but are part of your assumed > > sizes. > > That limit is on u-boot-nodtb.bin. Even with a size (for that file) of > 634816 it doesn't fit. I need to calculate a size based on the size of > the dtb and the microcode...which of course can change. Yes, and you're able to assume some size for them, which is what you put in the dts file? > > > - the U-Boot is the thing you are wanting to limit > > > - the dtb has microcode added > > > - the ucode is empty in this case > > > - the fdtmap is variable in size > > > > > > So this all seems a bit backwards. The actual limit is that > > > (u-boot-with-ucode-ptr + u-boot-dtb-with-ucode + u-boot-ucode + > > > fdtmap) fits in the space available. Note that some boards don't have > > > intel-vga or intel-mrc. > > > > > > With the other patch I sent I can have a sensible limit for all x86 > > > boards. > > Did you miss the comments above? No, I saw them. They're similar constraints to other systems. > > > > > And you can set the same sensible limit with the existing mechanism with > > the bonus of it not making x86 different from the rest? > > I understand that it is possible to set a limit for u-boot-nodtb.bin > but that is not accurate nor sufficient in the presence of blobs. The > solution belongs in Binman. Your series puts reasonable estimates on the size of the blobs and then will give a failure such as: binman: Node '/binman/rom/intel-vga': Offset 0xfff9 (4294508544) overlaps with previous entry '/binman/rom/fdtmap' ending at 0xfff902e1 (4294509281) Which is not as nice as (I just threw in a limit): u-boot-nodtb.bin exceeds file size limit: limit: 0x927c0 bytes actual: 0x9a810 bytes excess: 0x8050 bytes make[1]: *** [/home/trini/work/u-boot/u-boot/Makefile:1359: u-boot-nodtb.bin] Error 1 And tells us how much we need to get back size wise. Aside from when using the actual blobs (and in which case a real error will be shown when trying to use them), it's always about making an estimate on the part of the system that we control. -- Tom signature.asc Description: PGP signature
Re: [PATCH v2 2/9] tpm: Avoid code bloat when not using EFI_TCG2_PROTOCOL
Hi Tom, On Fri, 21 Jun 2024 at 13:19, Tom Rini wrote: > > On Fri, Jun 21, 2024 at 11:55:42AM -0600, Simon Glass wrote: > > Hi Tom, > > > > On Fri, 21 Jun 2024 at 10:05, Tom Rini wrote: > [snip] > > > Yes, I very much do not like guessing about 3 numbers instead of > > > guessing about 1 number and using the standard mechanism we already > > > have. Please use BOARD_SIZE_LIMIT as this is the standard mechanism to > > > enforce size limits on U-Boot itself. > > > > If it were that easy I would have sent a patch :-) > > > > Here is the map for this board: > > > > ImagePosOffset Size Name > > 0080 rom > > ff80 ff80 1000 intel-descriptor > > ff801000 ff801000 001ff000 intel-me > > ffef ffef 000999f0 u-boot-with-ucode-ptr > > fff899f0 fff899f0 5554 u-boot-dtb-with-ucode > > fff8ef50 fff8ef50 u-boot-ucode > > fff8ef50 fff8ef50 0571 fdtmap > > fff9 fff9 0001 intel-vga > > fffa fffa 0002fc94 intel-mrc > > fffcfc94 fffcfc94 private-files > > f800 f800 0070 x86-start16 > > fff0 fff0 0005 x86-reset16 > > fff8 fff8 0008 image-header > > > > What limit should I set on what? > > Is this a trick question? > $ printf %d\\n $(( 0xfff9 - 0xffef)) > 655360 > > Of course since we're less than that today, you can reduce it by > whatever other magic numbers I'm not seeing but are part of your assumed > sizes. That limit is on u-boot-nodtb.bin. Even with a size (for that file) of 634816 it doesn't fit. I need to calculate a size based on the size of the dtb and the microcode...which of course can change. > > > - the U-Boot is the thing you are wanting to limit > > - the dtb has microcode added > > - the ucode is empty in this case > > - the fdtmap is variable in size > > > > So this all seems a bit backwards. The actual limit is that > > (u-boot-with-ucode-ptr + u-boot-dtb-with-ucode + u-boot-ucode + > > fdtmap) fits in the space available. Note that some boards don't have > > intel-vga or intel-mrc. > > > > With the other patch I sent I can have a sensible limit for all x86 boards. Did you miss the comments above? > > And you can set the same sensible limit with the existing mechanism with > the bonus of it not making x86 different from the rest? I understand that it is possible to set a limit for u-boot-nodtb.bin but that is not accurate nor sufficient in the presence of blobs. The solution belongs in Binman. Regards, Simon
Re: [PATCH v2 2/9] tpm: Avoid code bloat when not using EFI_TCG2_PROTOCOL
On Fri, Jun 21, 2024 at 11:55:42AM -0600, Simon Glass wrote: > Hi Tom, > > On Fri, 21 Jun 2024 at 10:05, Tom Rini wrote: [snip] > > Yes, I very much do not like guessing about 3 numbers instead of > > guessing about 1 number and using the standard mechanism we already > > have. Please use BOARD_SIZE_LIMIT as this is the standard mechanism to > > enforce size limits on U-Boot itself. > > If it were that easy I would have sent a patch :-) > > Here is the map for this board: > > ImagePosOffset Size Name > 0080 rom > ff80 ff80 1000 intel-descriptor > ff801000 ff801000 001ff000 intel-me > ffef ffef 000999f0 u-boot-with-ucode-ptr > fff899f0 fff899f0 5554 u-boot-dtb-with-ucode > fff8ef50 fff8ef50 u-boot-ucode > fff8ef50 fff8ef50 0571 fdtmap > fff9 fff9 0001 intel-vga > fffa fffa 0002fc94 intel-mrc > fffcfc94 fffcfc94 private-files > f800 f800 0070 x86-start16 > fff0 fff0 0005 x86-reset16 > fff8 fff8 0008 image-header > > What limit should I set on what? Is this a trick question? $ printf %d\\n $(( 0xfff9 - 0xffef)) 655360 Of course since we're less than that today, you can reduce it by whatever other magic numbers I'm not seeing but are part of your assumed sizes. > - the U-Boot is the thing you are wanting to limit > - the dtb has microcode added > - the ucode is empty in this case > - the fdtmap is variable in size > > So this all seems a bit backwards. The actual limit is that > (u-boot-with-ucode-ptr + u-boot-dtb-with-ucode + u-boot-ucode + > fdtmap) fits in the space available. Note that some boards don't have > intel-vga or intel-mrc. > > With the other patch I sent I can have a sensible limit for all x86 boards. And you can set the same sensible limit with the existing mechanism with the bonus of it not making x86 different from the rest? -- Tom signature.asc Description: PGP signature
Re: [PATCH v2 2/9] tpm: Avoid code bloat when not using EFI_TCG2_PROTOCOL
Hi Tom, On Fri, 21 Jun 2024 at 10:05, Tom Rini wrote: > > On Fri, Jun 21, 2024 at 08:57:51AM -0600, Simon Glass wrote: > > Hi Tom, > > > > On Thu, 20 Jun 2024 at 17:19, Tom Rini wrote: > > > > > > On Thu, Jun 20, 2024 at 05:05:29PM -0600, Simon Glass wrote: > > > > Hi Tom, > > > > > > > > On Wed, 19 Jun 2024 at 09:32, Tom Rini wrote: > > > > > > > > > > On Tue, Jun 18, 2024 at 09:03:37PM -0600, Simon Glass wrote: > > > > > > Hi Tom, > > > > > > > > > > > > On Tue, 18 Jun 2024 at 08:15, Tom Rini wrote: > > > > > > > > > > > > > > On Tue, Jun 18, 2024 at 06:43:51AM -0600, Simon Glass wrote: > > > > > > > > Hi Tom, > > > > > > > > > > > > > > > > On Mon, 17 Jun 2024 at 11:16, Tom Rini > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > On Mon, Jun 17, 2024 at 07:53:22AM -0600, Simon Glass wrote: > > > > > > > > > > Hi, > > > > > > > > > > > > > > > > > > > > On Sat, 15 Jun 2024 at 01:03, Ilias Apalodimas > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > Hi Heinrich > > > > > > > > > > > > > > > > > > > > > > resending the reply, I accidentally sent half of the > > > > > > > > > > > message... > > > > > > > > > > > > > > > > > > > > > > On Fri, 14 Jun 2024 at 12:04, Heinrich Schuchardt > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > On 14.06.24 09:01, Ilias Apalodimas wrote: > > > > > > > > > > > > > On Fri, 14 Jun 2024 at 09:59, Heinrich Schuchardt > > > > > > > > > > > > > wrote: > > > > > > > > > > > > >> > > > > > > > > > > > > >> On 6/14/24 08:03, Ilias Apalodimas wrote: > > > > > > > > > > > > >>> Hi Simon, > > > > > > > > > > > > >>> > > > > > > > > > > > > >>> On Mon, 10 Jun 2024 at 17:59, Simon Glass > > > > > > > > > > > > >>> wrote: > > > > > > > > > > > > > > > > > > > > > > > > It does not make sense to enable all SHA > > > > > > > > > > > > algorithms unless they are > > > > > > > > > > > > needed. It bloats the code and in this case, > > > > > > > > > > > > causes chromebook_link to > > > > > > > > > > > > fail to build. That board does use the TPM, but > > > > > > > > > > > > not with measured boot, > > > > > > > > > > > > nor EFI. > > > > > > > > > > > > > > > > > > > > > > > > Since EFI_TCG2_PROTOCOL already selects these > > > > > > > > > > > > options, we just need to > > > > > > > > > > > > add them to MEASURED_BOOT as well. > > > > > > > > > > > > > > > > > > > > > > > > Note that the original commit combines refactoring > > > > > > > > > > > > and new features, > > > > > > > > > > > > which makes it hard to see what is going on. > > > > > > > > > > > > > > > > > > > > > > > > Fixes: 97707f12fda tpm: Support boot measurements > > > > > > > > > > > > Signed-off-by: Simon Glass > > > > > > > > > > > > --- > > > > > > > > > > > > > > > > > > > > > > > > Changes in v2: > > > > > > > > > > > > - Put the conditions under EFI_TCG2_PROTOCOL > > > > > > > > > > > > - Consider MEASURED_BOOT too > > > > > > > > > > > > > > > > > > > > > > > > boot/Kconfig | 4 > > > > > > > > > > > > lib/Kconfig | 4 > > > > > > > > > > > > 2 files changed, 4 insertions(+), 4 deletions(-) > > > > > > > > > > > > > > > > > > > > > > > > diff --git a/boot/Kconfig b/boot/Kconfig > > > > > > > > > > > > index 6f3096c15a6..b061891e109 100644 > > > > > > > > > > > > --- a/boot/Kconfig > > > > > > > > > > > > +++ b/boot/Kconfig > > > > > > > > > > > > @@ -734,6 +734,10 @@ config LEGACY_IMAGE_FORMAT > > > > > > > > > > > > config MEASURED_BOOT > > > > > > > > > > > > bool "Measure boot images and > > > > > > > > > > > > configuration when booting without EFI" > > > > > > > > > > > > depends on HASH && TPM_V2 > > > > > > > > > > > > + select SHA1 > > > > > > > > > > > > + select SHA256 > > > > > > > > > > > > + select SHA384 > > > > > > > > > > > > + select SHA512 > > > > > > > > > > > > help > > > > > > > > > > > > This option enables measurement of the > > > > > > > > > > > > boot process when booting > > > > > > > > > > > > without UEFI . Measurement involves > > > > > > > > > > > > creating cryptographic hashes > > > > > > > > > > > > diff --git a/lib/Kconfig b/lib/Kconfig > > > > > > > > > > > > index 189e6eb31aa..568892fce44 100644 > > > > > > > > > > > > --- a/lib/Kconfig > > > > > > > > > > > > +++ b/lib/Kconfig > > > > > > > > > > > > @@ -438,10 +438,6 @@ config TPM > > > > > > > > > > > > bool "Trusted Platform Module (TPM) > > > > > > > > > > > > Support" > > > > > > > > > > > > depends on DM > > > > > > > > > > > > imply DM_RNG > > > > > > > > > > > > - select SHA1 > > > > > > > > > > > > - select SHA256 > > >
Re: [PATCH v2 2/9] tpm: Avoid code bloat when not using EFI_TCG2_PROTOCOL
On Fri, Jun 21, 2024 at 08:57:51AM -0600, Simon Glass wrote: > Hi Tom, > > On Thu, 20 Jun 2024 at 17:19, Tom Rini wrote: > > > > On Thu, Jun 20, 2024 at 05:05:29PM -0600, Simon Glass wrote: > > > Hi Tom, > > > > > > On Wed, 19 Jun 2024 at 09:32, Tom Rini wrote: > > > > > > > > On Tue, Jun 18, 2024 at 09:03:37PM -0600, Simon Glass wrote: > > > > > Hi Tom, > > > > > > > > > > On Tue, 18 Jun 2024 at 08:15, Tom Rini wrote: > > > > > > > > > > > > On Tue, Jun 18, 2024 at 06:43:51AM -0600, Simon Glass wrote: > > > > > > > Hi Tom, > > > > > > > > > > > > > > On Mon, 17 Jun 2024 at 11:16, Tom Rini wrote: > > > > > > > > > > > > > > > > On Mon, Jun 17, 2024 at 07:53:22AM -0600, Simon Glass wrote: > > > > > > > > > Hi, > > > > > > > > > > > > > > > > > > On Sat, 15 Jun 2024 at 01:03, Ilias Apalodimas > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > Hi Heinrich > > > > > > > > > > > > > > > > > > > > resending the reply, I accidentally sent half of the > > > > > > > > > > message... > > > > > > > > > > > > > > > > > > > > On Fri, 14 Jun 2024 at 12:04, Heinrich Schuchardt > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > On 14.06.24 09:01, Ilias Apalodimas wrote: > > > > > > > > > > > > On Fri, 14 Jun 2024 at 09:59, Heinrich Schuchardt > > > > > > > > > > > > wrote: > > > > > > > > > > > >> > > > > > > > > > > > >> On 6/14/24 08:03, Ilias Apalodimas wrote: > > > > > > > > > > > >>> Hi Simon, > > > > > > > > > > > >>> > > > > > > > > > > > >>> On Mon, 10 Jun 2024 at 17:59, Simon Glass > > > > > > > > > > > >>> wrote: > > > > > > > > > > > > > > > > > > > > > > It does not make sense to enable all SHA algorithms > > > > > > > > > > > unless they are > > > > > > > > > > > needed. It bloats the code and in this case, causes > > > > > > > > > > > chromebook_link to > > > > > > > > > > > fail to build. That board does use the TPM, but not > > > > > > > > > > > with measured boot, > > > > > > > > > > > nor EFI. > > > > > > > > > > > > > > > > > > > > > > Since EFI_TCG2_PROTOCOL already selects these > > > > > > > > > > > options, we just need to > > > > > > > > > > > add them to MEASURED_BOOT as well. > > > > > > > > > > > > > > > > > > > > > > Note that the original commit combines refactoring > > > > > > > > > > > and new features, > > > > > > > > > > > which makes it hard to see what is going on. > > > > > > > > > > > > > > > > > > > > > > Fixes: 97707f12fda tpm: Support boot measurements > > > > > > > > > > > Signed-off-by: Simon Glass > > > > > > > > > > > --- > > > > > > > > > > > > > > > > > > > > > > Changes in v2: > > > > > > > > > > > - Put the conditions under EFI_TCG2_PROTOCOL > > > > > > > > > > > - Consider MEASURED_BOOT too > > > > > > > > > > > > > > > > > > > > > > boot/Kconfig | 4 > > > > > > > > > > > lib/Kconfig | 4 > > > > > > > > > > > 2 files changed, 4 insertions(+), 4 deletions(-) > > > > > > > > > > > > > > > > > > > > > > diff --git a/boot/Kconfig b/boot/Kconfig > > > > > > > > > > > index 6f3096c15a6..b061891e109 100644 > > > > > > > > > > > --- a/boot/Kconfig > > > > > > > > > > > +++ b/boot/Kconfig > > > > > > > > > > > @@ -734,6 +734,10 @@ config LEGACY_IMAGE_FORMAT > > > > > > > > > > > config MEASURED_BOOT > > > > > > > > > > > bool "Measure boot images and > > > > > > > > > > > configuration when booting without EFI" > > > > > > > > > > > depends on HASH && TPM_V2 > > > > > > > > > > > + select SHA1 > > > > > > > > > > > + select SHA256 > > > > > > > > > > > + select SHA384 > > > > > > > > > > > + select SHA512 > > > > > > > > > > > help > > > > > > > > > > > This option enables measurement of the > > > > > > > > > > > boot process when booting > > > > > > > > > > > without UEFI . Measurement involves > > > > > > > > > > > creating cryptographic hashes > > > > > > > > > > > diff --git a/lib/Kconfig b/lib/Kconfig > > > > > > > > > > > index 189e6eb31aa..568892fce44 100644 > > > > > > > > > > > --- a/lib/Kconfig > > > > > > > > > > > +++ b/lib/Kconfig > > > > > > > > > > > @@ -438,10 +438,6 @@ config TPM > > > > > > > > > > > bool "Trusted Platform Module (TPM) > > > > > > > > > > > Support" > > > > > > > > > > > depends on DM > > > > > > > > > > > imply DM_RNG > > > > > > > > > > > - select SHA1 > > > > > > > > > > > - select SHA256 > > > > > > > > > > > - select SHA384 > > > > > > > > > > > - select SHA512 > > > > > > > > > > > >>> > > > > > > > > > > > >>> I am not sure this is the right way to deal with your > > > > > > > > > > > >>> problem. > > > > > > > > > > > >>> The TPM main fu
Re: [PATCH v2 2/9] tpm: Avoid code bloat when not using EFI_TCG2_PROTOCOL
Hi Tom, On Thu, 20 Jun 2024 at 17:19, Tom Rini wrote: > > On Thu, Jun 20, 2024 at 05:05:29PM -0600, Simon Glass wrote: > > Hi Tom, > > > > On Wed, 19 Jun 2024 at 09:32, Tom Rini wrote: > > > > > > On Tue, Jun 18, 2024 at 09:03:37PM -0600, Simon Glass wrote: > > > > Hi Tom, > > > > > > > > On Tue, 18 Jun 2024 at 08:15, Tom Rini wrote: > > > > > > > > > > On Tue, Jun 18, 2024 at 06:43:51AM -0600, Simon Glass wrote: > > > > > > Hi Tom, > > > > > > > > > > > > On Mon, 17 Jun 2024 at 11:16, Tom Rini wrote: > > > > > > > > > > > > > > On Mon, Jun 17, 2024 at 07:53:22AM -0600, Simon Glass wrote: > > > > > > > > Hi, > > > > > > > > > > > > > > > > On Sat, 15 Jun 2024 at 01:03, Ilias Apalodimas > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > Hi Heinrich > > > > > > > > > > > > > > > > > > resending the reply, I accidentally sent half of the > > > > > > > > > message... > > > > > > > > > > > > > > > > > > On Fri, 14 Jun 2024 at 12:04, Heinrich Schuchardt > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > On 14.06.24 09:01, Ilias Apalodimas wrote: > > > > > > > > > > > On Fri, 14 Jun 2024 at 09:59, Heinrich Schuchardt > > > > > > > > > > > wrote: > > > > > > > > > > >> > > > > > > > > > > >> On 6/14/24 08:03, Ilias Apalodimas wrote: > > > > > > > > > > >>> Hi Simon, > > > > > > > > > > >>> > > > > > > > > > > >>> On Mon, 10 Jun 2024 at 17:59, Simon Glass > > > > > > > > > > >>> wrote: > > > > > > > > > > > > > > > > > > > > It does not make sense to enable all SHA algorithms > > > > > > > > > > unless they are > > > > > > > > > > needed. It bloats the code and in this case, causes > > > > > > > > > > chromebook_link to > > > > > > > > > > fail to build. That board does use the TPM, but not > > > > > > > > > > with measured boot, > > > > > > > > > > nor EFI. > > > > > > > > > > > > > > > > > > > > Since EFI_TCG2_PROTOCOL already selects these options, > > > > > > > > > > we just need to > > > > > > > > > > add them to MEASURED_BOOT as well. > > > > > > > > > > > > > > > > > > > > Note that the original commit combines refactoring and > > > > > > > > > > new features, > > > > > > > > > > which makes it hard to see what is going on. > > > > > > > > > > > > > > > > > > > > Fixes: 97707f12fda tpm: Support boot measurements > > > > > > > > > > Signed-off-by: Simon Glass > > > > > > > > > > --- > > > > > > > > > > > > > > > > > > > > Changes in v2: > > > > > > > > > > - Put the conditions under EFI_TCG2_PROTOCOL > > > > > > > > > > - Consider MEASURED_BOOT too > > > > > > > > > > > > > > > > > > > > boot/Kconfig | 4 > > > > > > > > > > lib/Kconfig | 4 > > > > > > > > > > 2 files changed, 4 insertions(+), 4 deletions(-) > > > > > > > > > > > > > > > > > > > > diff --git a/boot/Kconfig b/boot/Kconfig > > > > > > > > > > index 6f3096c15a6..b061891e109 100644 > > > > > > > > > > --- a/boot/Kconfig > > > > > > > > > > +++ b/boot/Kconfig > > > > > > > > > > @@ -734,6 +734,10 @@ config LEGACY_IMAGE_FORMAT > > > > > > > > > > config MEASURED_BOOT > > > > > > > > > > bool "Measure boot images and configuration > > > > > > > > > > when booting without EFI" > > > > > > > > > > depends on HASH && TPM_V2 > > > > > > > > > > + select SHA1 > > > > > > > > > > + select SHA256 > > > > > > > > > > + select SHA384 > > > > > > > > > > + select SHA512 > > > > > > > > > > help > > > > > > > > > > This option enables measurement of the > > > > > > > > > > boot process when booting > > > > > > > > > > without UEFI . Measurement involves > > > > > > > > > > creating cryptographic hashes > > > > > > > > > > diff --git a/lib/Kconfig b/lib/Kconfig > > > > > > > > > > index 189e6eb31aa..568892fce44 100644 > > > > > > > > > > --- a/lib/Kconfig > > > > > > > > > > +++ b/lib/Kconfig > > > > > > > > > > @@ -438,10 +438,6 @@ config TPM > > > > > > > > > > bool "Trusted Platform Module (TPM) Support" > > > > > > > > > > depends on DM > > > > > > > > > > imply DM_RNG > > > > > > > > > > - select SHA1 > > > > > > > > > > - select SHA256 > > > > > > > > > > - select SHA384 > > > > > > > > > > - select SHA512 > > > > > > > > > > >>> > > > > > > > > > > >>> I am not sure this is the right way to deal with your > > > > > > > > > > >>> problem. > > > > > > > > > > >>> The TPM main functionality is to measure and extend > > > > > > > > > > >>> PCRs, so sha > > > > > > > > > > >>> is really required. To make things even worse, you > > > > > > > > > > >>> don't know the PCR > > > > > > > > > > >>> banks that are enabled beforehand. This is a runtime > > > > > > > > > > >>>
Re: [PATCH v2 2/9] tpm: Avoid code bloat when not using EFI_TCG2_PROTOCOL
On Thu, Jun 20, 2024 at 05:05:29PM -0600, Simon Glass wrote: > Hi Tom, > > On Wed, 19 Jun 2024 at 09:32, Tom Rini wrote: > > > > On Tue, Jun 18, 2024 at 09:03:37PM -0600, Simon Glass wrote: > > > Hi Tom, > > > > > > On Tue, 18 Jun 2024 at 08:15, Tom Rini wrote: > > > > > > > > On Tue, Jun 18, 2024 at 06:43:51AM -0600, Simon Glass wrote: > > > > > Hi Tom, > > > > > > > > > > On Mon, 17 Jun 2024 at 11:16, Tom Rini wrote: > > > > > > > > > > > > On Mon, Jun 17, 2024 at 07:53:22AM -0600, Simon Glass wrote: > > > > > > > Hi, > > > > > > > > > > > > > > On Sat, 15 Jun 2024 at 01:03, Ilias Apalodimas > > > > > > > wrote: > > > > > > > > > > > > > > > > Hi Heinrich > > > > > > > > > > > > > > > > resending the reply, I accidentally sent half of the message... > > > > > > > > > > > > > > > > On Fri, 14 Jun 2024 at 12:04, Heinrich Schuchardt > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > On 14.06.24 09:01, Ilias Apalodimas wrote: > > > > > > > > > > On Fri, 14 Jun 2024 at 09:59, Heinrich Schuchardt > > > > > > > > > > wrote: > > > > > > > > > >> > > > > > > > > > >> On 6/14/24 08:03, Ilias Apalodimas wrote: > > > > > > > > > >>> Hi Simon, > > > > > > > > > >>> > > > > > > > > > >>> On Mon, 10 Jun 2024 at 17:59, Simon Glass > > > > > > > > > >>> wrote: > > > > > > > > > > > > > > > > > > It does not make sense to enable all SHA algorithms > > > > > > > > > unless they are > > > > > > > > > needed. It bloats the code and in this case, causes > > > > > > > > > chromebook_link to > > > > > > > > > fail to build. That board does use the TPM, but not with > > > > > > > > > measured boot, > > > > > > > > > nor EFI. > > > > > > > > > > > > > > > > > > Since EFI_TCG2_PROTOCOL already selects these options, > > > > > > > > > we just need to > > > > > > > > > add them to MEASURED_BOOT as well. > > > > > > > > > > > > > > > > > > Note that the original commit combines refactoring and > > > > > > > > > new features, > > > > > > > > > which makes it hard to see what is going on. > > > > > > > > > > > > > > > > > > Fixes: 97707f12fda tpm: Support boot measurements > > > > > > > > > Signed-off-by: Simon Glass > > > > > > > > > --- > > > > > > > > > > > > > > > > > > Changes in v2: > > > > > > > > > - Put the conditions under EFI_TCG2_PROTOCOL > > > > > > > > > - Consider MEASURED_BOOT too > > > > > > > > > > > > > > > > > > boot/Kconfig | 4 > > > > > > > > > lib/Kconfig | 4 > > > > > > > > > 2 files changed, 4 insertions(+), 4 deletions(-) > > > > > > > > > > > > > > > > > > diff --git a/boot/Kconfig b/boot/Kconfig > > > > > > > > > index 6f3096c15a6..b061891e109 100644 > > > > > > > > > --- a/boot/Kconfig > > > > > > > > > +++ b/boot/Kconfig > > > > > > > > > @@ -734,6 +734,10 @@ config LEGACY_IMAGE_FORMAT > > > > > > > > > config MEASURED_BOOT > > > > > > > > > bool "Measure boot images and configuration > > > > > > > > > when booting without EFI" > > > > > > > > > depends on HASH && TPM_V2 > > > > > > > > > + select SHA1 > > > > > > > > > + select SHA256 > > > > > > > > > + select SHA384 > > > > > > > > > + select SHA512 > > > > > > > > > help > > > > > > > > > This option enables measurement of the boot > > > > > > > > > process when booting > > > > > > > > > without UEFI . Measurement involves creating > > > > > > > > > cryptographic hashes > > > > > > > > > diff --git a/lib/Kconfig b/lib/Kconfig > > > > > > > > > index 189e6eb31aa..568892fce44 100644 > > > > > > > > > --- a/lib/Kconfig > > > > > > > > > +++ b/lib/Kconfig > > > > > > > > > @@ -438,10 +438,6 @@ config TPM > > > > > > > > > bool "Trusted Platform Module (TPM) Support" > > > > > > > > > depends on DM > > > > > > > > > imply DM_RNG > > > > > > > > > - select SHA1 > > > > > > > > > - select SHA256 > > > > > > > > > - select SHA384 > > > > > > > > > - select SHA512 > > > > > > > > > >>> > > > > > > > > > >>> I am not sure this is the right way to deal with your > > > > > > > > > >>> problem. > > > > > > > > > >>> The TPM main functionality is to measure and extend PCRs, > > > > > > > > > >>> so sha > > > > > > > > > >>> is really required. To make things even worse, you don't > > > > > > > > > >>> know the PCR > > > > > > > > > >>> banks that are enabled beforehand. This is a runtime > > > > > > > > > >>> config of the > > > > > > > > > >>> TPM. > > > > > > > > > >> > > > > > > > > > >> If neither MEASURED_BOOT nor EFI_TCG2_PROTOCOL is > > > > > > > > > >> selected, U-Boot > > > > > > > > > >> cannot extend PCRs. So it seems fine to let these two > > > > > > > > > >> select the > > >
Re: [PATCH v2 2/9] tpm: Avoid code bloat when not using EFI_TCG2_PROTOCOL
Hi Tom, On Wed, 19 Jun 2024 at 09:32, Tom Rini wrote: > > On Tue, Jun 18, 2024 at 09:03:37PM -0600, Simon Glass wrote: > > Hi Tom, > > > > On Tue, 18 Jun 2024 at 08:15, Tom Rini wrote: > > > > > > On Tue, Jun 18, 2024 at 06:43:51AM -0600, Simon Glass wrote: > > > > Hi Tom, > > > > > > > > On Mon, 17 Jun 2024 at 11:16, Tom Rini wrote: > > > > > > > > > > On Mon, Jun 17, 2024 at 07:53:22AM -0600, Simon Glass wrote: > > > > > > Hi, > > > > > > > > > > > > On Sat, 15 Jun 2024 at 01:03, Ilias Apalodimas > > > > > > wrote: > > > > > > > > > > > > > > Hi Heinrich > > > > > > > > > > > > > > resending the reply, I accidentally sent half of the message... > > > > > > > > > > > > > > On Fri, 14 Jun 2024 at 12:04, Heinrich Schuchardt > > > > > > > wrote: > > > > > > > > > > > > > > > > On 14.06.24 09:01, Ilias Apalodimas wrote: > > > > > > > > > On Fri, 14 Jun 2024 at 09:59, Heinrich Schuchardt > > > > > > > > > wrote: > > > > > > > > >> > > > > > > > > >> On 6/14/24 08:03, Ilias Apalodimas wrote: > > > > > > > > >>> Hi Simon, > > > > > > > > >>> > > > > > > > > >>> On Mon, 10 Jun 2024 at 17:59, Simon Glass > > > > > > > > >>> wrote: > > > > > > > > > > > > > > > > It does not make sense to enable all SHA algorithms unless > > > > > > > > they are > > > > > > > > needed. It bloats the code and in this case, causes > > > > > > > > chromebook_link to > > > > > > > > fail to build. That board does use the TPM, but not with > > > > > > > > measured boot, > > > > > > > > nor EFI. > > > > > > > > > > > > > > > > Since EFI_TCG2_PROTOCOL already selects these options, we > > > > > > > > just need to > > > > > > > > add them to MEASURED_BOOT as well. > > > > > > > > > > > > > > > > Note that the original commit combines refactoring and new > > > > > > > > features, > > > > > > > > which makes it hard to see what is going on. > > > > > > > > > > > > > > > > Fixes: 97707f12fda tpm: Support boot measurements > > > > > > > > Signed-off-by: Simon Glass > > > > > > > > --- > > > > > > > > > > > > > > > > Changes in v2: > > > > > > > > - Put the conditions under EFI_TCG2_PROTOCOL > > > > > > > > - Consider MEASURED_BOOT too > > > > > > > > > > > > > > > > boot/Kconfig | 4 > > > > > > > > lib/Kconfig | 4 > > > > > > > > 2 files changed, 4 insertions(+), 4 deletions(-) > > > > > > > > > > > > > > > > diff --git a/boot/Kconfig b/boot/Kconfig > > > > > > > > index 6f3096c15a6..b061891e109 100644 > > > > > > > > --- a/boot/Kconfig > > > > > > > > +++ b/boot/Kconfig > > > > > > > > @@ -734,6 +734,10 @@ config LEGACY_IMAGE_FORMAT > > > > > > > > config MEASURED_BOOT > > > > > > > > bool "Measure boot images and configuration when > > > > > > > > booting without EFI" > > > > > > > > depends on HASH && TPM_V2 > > > > > > > > + select SHA1 > > > > > > > > + select SHA256 > > > > > > > > + select SHA384 > > > > > > > > + select SHA512 > > > > > > > > help > > > > > > > > This option enables measurement of the boot > > > > > > > > process when booting > > > > > > > > without UEFI . Measurement involves creating > > > > > > > > cryptographic hashes > > > > > > > > diff --git a/lib/Kconfig b/lib/Kconfig > > > > > > > > index 189e6eb31aa..568892fce44 100644 > > > > > > > > --- a/lib/Kconfig > > > > > > > > +++ b/lib/Kconfig > > > > > > > > @@ -438,10 +438,6 @@ config TPM > > > > > > > > bool "Trusted Platform Module (TPM) Support" > > > > > > > > depends on DM > > > > > > > > imply DM_RNG > > > > > > > > - select SHA1 > > > > > > > > - select SHA256 > > > > > > > > - select SHA384 > > > > > > > > - select SHA512 > > > > > > > > >>> > > > > > > > > >>> I am not sure this is the right way to deal with your > > > > > > > > >>> problem. > > > > > > > > >>> The TPM main functionality is to measure and extend PCRs, > > > > > > > > >>> so sha > > > > > > > > >>> is really required. To make things even worse, you don't > > > > > > > > >>> know the PCR > > > > > > > > >>> banks that are enabled beforehand. This is a runtime config > > > > > > > > >>> of the > > > > > > > > >>> TPM. > > > > > > > > >> > > > > > > > > >> If neither MEASURED_BOOT nor EFI_TCG2_PROTOCOL is selected, > > > > > > > > >> U-Boot > > > > > > > > >> cannot extend PCRs. So it seems fine to let these two select > > > > > > > > >> the > > > > > > > > >> complete set of hashing algorithms. As Simon pointed out for > > > > > > > > >> EFI_TCG2_PROTOCOL this is already done in > > > > > > > > >> lib/efi_loader/Kconfig. > > > > > > > > > > > > > > > > > > It can. The cmd we have can extend those pcrs -- e.g tpm2 > > >
Re: [PATCH v2 2/9] tpm: Avoid code bloat when not using EFI_TCG2_PROTOCOL
On Tue, Jun 18, 2024 at 09:03:37PM -0600, Simon Glass wrote: > Hi Tom, > > On Tue, 18 Jun 2024 at 08:15, Tom Rini wrote: > > > > On Tue, Jun 18, 2024 at 06:43:51AM -0600, Simon Glass wrote: > > > Hi Tom, > > > > > > On Mon, 17 Jun 2024 at 11:16, Tom Rini wrote: > > > > > > > > On Mon, Jun 17, 2024 at 07:53:22AM -0600, Simon Glass wrote: > > > > > Hi, > > > > > > > > > > On Sat, 15 Jun 2024 at 01:03, Ilias Apalodimas > > > > > wrote: > > > > > > > > > > > > Hi Heinrich > > > > > > > > > > > > resending the reply, I accidentally sent half of the message... > > > > > > > > > > > > On Fri, 14 Jun 2024 at 12:04, Heinrich Schuchardt > > > > > > wrote: > > > > > > > > > > > > > > On 14.06.24 09:01, Ilias Apalodimas wrote: > > > > > > > > On Fri, 14 Jun 2024 at 09:59, Heinrich Schuchardt > > > > > > > > wrote: > > > > > > > >> > > > > > > > >> On 6/14/24 08:03, Ilias Apalodimas wrote: > > > > > > > >>> Hi Simon, > > > > > > > >>> > > > > > > > >>> On Mon, 10 Jun 2024 at 17:59, Simon Glass > > > > > > > >>> wrote: > > > > > > > > > > > > > > It does not make sense to enable all SHA algorithms unless > > > > > > > they are > > > > > > > needed. It bloats the code and in this case, causes > > > > > > > chromebook_link to > > > > > > > fail to build. That board does use the TPM, but not with > > > > > > > measured boot, > > > > > > > nor EFI. > > > > > > > > > > > > > > Since EFI_TCG2_PROTOCOL already selects these options, we > > > > > > > just need to > > > > > > > add them to MEASURED_BOOT as well. > > > > > > > > > > > > > > Note that the original commit combines refactoring and new > > > > > > > features, > > > > > > > which makes it hard to see what is going on. > > > > > > > > > > > > > > Fixes: 97707f12fda tpm: Support boot measurements > > > > > > > Signed-off-by: Simon Glass > > > > > > > --- > > > > > > > > > > > > > > Changes in v2: > > > > > > > - Put the conditions under EFI_TCG2_PROTOCOL > > > > > > > - Consider MEASURED_BOOT too > > > > > > > > > > > > > > boot/Kconfig | 4 > > > > > > > lib/Kconfig | 4 > > > > > > > 2 files changed, 4 insertions(+), 4 deletions(-) > > > > > > > > > > > > > > diff --git a/boot/Kconfig b/boot/Kconfig > > > > > > > index 6f3096c15a6..b061891e109 100644 > > > > > > > --- a/boot/Kconfig > > > > > > > +++ b/boot/Kconfig > > > > > > > @@ -734,6 +734,10 @@ config LEGACY_IMAGE_FORMAT > > > > > > > config MEASURED_BOOT > > > > > > > bool "Measure boot images and configuration when > > > > > > > booting without EFI" > > > > > > > depends on HASH && TPM_V2 > > > > > > > + select SHA1 > > > > > > > + select SHA256 > > > > > > > + select SHA384 > > > > > > > + select SHA512 > > > > > > > help > > > > > > > This option enables measurement of the boot > > > > > > > process when booting > > > > > > > without UEFI . Measurement involves creating > > > > > > > cryptographic hashes > > > > > > > diff --git a/lib/Kconfig b/lib/Kconfig > > > > > > > index 189e6eb31aa..568892fce44 100644 > > > > > > > --- a/lib/Kconfig > > > > > > > +++ b/lib/Kconfig > > > > > > > @@ -438,10 +438,6 @@ config TPM > > > > > > > bool "Trusted Platform Module (TPM) Support" > > > > > > > depends on DM > > > > > > > imply DM_RNG > > > > > > > - select SHA1 > > > > > > > - select SHA256 > > > > > > > - select SHA384 > > > > > > > - select SHA512 > > > > > > > >>> > > > > > > > >>> I am not sure this is the right way to deal with your problem. > > > > > > > >>> The TPM main functionality is to measure and extend PCRs, so > > > > > > > >>> sha > > > > > > > >>> is really required. To make things even worse, you don't know > > > > > > > >>> the PCR > > > > > > > >>> banks that are enabled beforehand. This is a runtime config > > > > > > > >>> of the > > > > > > > >>> TPM. > > > > > > > >> > > > > > > > >> If neither MEASURED_BOOT nor EFI_TCG2_PROTOCOL is selected, > > > > > > > >> U-Boot > > > > > > > >> cannot extend PCRs. So it seems fine to let these two select > > > > > > > >> the > > > > > > > >> complete set of hashing algorithms. As Simon pointed out for > > > > > > > >> EFI_TCG2_PROTOCOL this is already done in > > > > > > > >> lib/efi_loader/Kconfig. > > > > > > > > > > > > > > > > It can. The cmd we have can extend those pcrs -- e.g tpm2 > > > > > > > > pcr_extend 8 > > > > > > > > 0xb000 > > > > > > > > > > That's pretty normal for U-Boot though, since we want to avoid lots of > > > > > growth for things people might want control over. We can enable or > > > > > disable the SHA for the board, if this functionality is used outside >
Re: [PATCH v2 2/9] tpm: Avoid code bloat when not using EFI_TCG2_PROTOCOL
Hi Tom, On Tue, 18 Jun 2024 at 08:15, Tom Rini wrote: > > On Tue, Jun 18, 2024 at 06:43:51AM -0600, Simon Glass wrote: > > Hi Tom, > > > > On Mon, 17 Jun 2024 at 11:16, Tom Rini wrote: > > > > > > On Mon, Jun 17, 2024 at 07:53:22AM -0600, Simon Glass wrote: > > > > Hi, > > > > > > > > On Sat, 15 Jun 2024 at 01:03, Ilias Apalodimas > > > > wrote: > > > > > > > > > > Hi Heinrich > > > > > > > > > > resending the reply, I accidentally sent half of the message... > > > > > > > > > > On Fri, 14 Jun 2024 at 12:04, Heinrich Schuchardt > > > > > wrote: > > > > > > > > > > > > On 14.06.24 09:01, Ilias Apalodimas wrote: > > > > > > > On Fri, 14 Jun 2024 at 09:59, Heinrich Schuchardt > > > > > > > wrote: > > > > > > >> > > > > > > >> On 6/14/24 08:03, Ilias Apalodimas wrote: > > > > > > >>> Hi Simon, > > > > > > >>> > > > > > > >>> On Mon, 10 Jun 2024 at 17:59, Simon Glass > > > > > > >>> wrote: > > > > > > > > > > > > It does not make sense to enable all SHA algorithms unless > > > > > > they are > > > > > > needed. It bloats the code and in this case, causes > > > > > > chromebook_link to > > > > > > fail to build. That board does use the TPM, but not with > > > > > > measured boot, > > > > > > nor EFI. > > > > > > > > > > > > Since EFI_TCG2_PROTOCOL already selects these options, we just > > > > > > need to > > > > > > add them to MEASURED_BOOT as well. > > > > > > > > > > > > Note that the original commit combines refactoring and new > > > > > > features, > > > > > > which makes it hard to see what is going on. > > > > > > > > > > > > Fixes: 97707f12fda tpm: Support boot measurements > > > > > > Signed-off-by: Simon Glass > > > > > > --- > > > > > > > > > > > > Changes in v2: > > > > > > - Put the conditions under EFI_TCG2_PROTOCOL > > > > > > - Consider MEASURED_BOOT too > > > > > > > > > > > > boot/Kconfig | 4 > > > > > > lib/Kconfig | 4 > > > > > > 2 files changed, 4 insertions(+), 4 deletions(-) > > > > > > > > > > > > diff --git a/boot/Kconfig b/boot/Kconfig > > > > > > index 6f3096c15a6..b061891e109 100644 > > > > > > --- a/boot/Kconfig > > > > > > +++ b/boot/Kconfig > > > > > > @@ -734,6 +734,10 @@ config LEGACY_IMAGE_FORMAT > > > > > > config MEASURED_BOOT > > > > > > bool "Measure boot images and configuration when > > > > > > booting without EFI" > > > > > > depends on HASH && TPM_V2 > > > > > > + select SHA1 > > > > > > + select SHA256 > > > > > > + select SHA384 > > > > > > + select SHA512 > > > > > > help > > > > > > This option enables measurement of the boot > > > > > > process when booting > > > > > > without UEFI . Measurement involves creating > > > > > > cryptographic hashes > > > > > > diff --git a/lib/Kconfig b/lib/Kconfig > > > > > > index 189e6eb31aa..568892fce44 100644 > > > > > > --- a/lib/Kconfig > > > > > > +++ b/lib/Kconfig > > > > > > @@ -438,10 +438,6 @@ config TPM > > > > > > bool "Trusted Platform Module (TPM) Support" > > > > > > depends on DM > > > > > > imply DM_RNG > > > > > > - select SHA1 > > > > > > - select SHA256 > > > > > > - select SHA384 > > > > > > - select SHA512 > > > > > > >>> > > > > > > >>> I am not sure this is the right way to deal with your problem. > > > > > > >>> The TPM main functionality is to measure and extend PCRs, so > > > > > > >>> sha > > > > > > >>> is really required. To make things even worse, you don't know > > > > > > >>> the PCR > > > > > > >>> banks that are enabled beforehand. This is a runtime config of > > > > > > >>> the > > > > > > >>> TPM. > > > > > > >> > > > > > > >> If neither MEASURED_BOOT nor EFI_TCG2_PROTOCOL is selected, > > > > > > >> U-Boot > > > > > > >> cannot extend PCRs. So it seems fine to let these two select the > > > > > > >> complete set of hashing algorithms. As Simon pointed out for > > > > > > >> EFI_TCG2_PROTOCOL this is already done in lib/efi_loader/Kconfig. > > > > > > > > > > > > > > It can. The cmd we have can extend those pcrs -- e.g tpm2 > > > > > > > pcr_extend 8 > > > > > > > 0xb000 > > > > > > > > That's pretty normal for U-Boot though, since we want to avoid lots of > > > > growth for things people might want control over. We can enable or > > > > disable the SHA for the board, if this functionality is used outside > > > > of measured boot and tcg2, but someone is enabling the tpm command. > > > > > > > > > > > > > > > > So this patch should also consider CMD_TPM_V2 and CMD_TPM_V1. > > > > > > > > > > > > TPM v1 only needs SHA-1. > > > > > > > > > > I still prefer to imply all algos. > > > > > > > > 'imply' would be OK in this case a
Re: [PATCH v2 2/9] tpm: Avoid code bloat when not using EFI_TCG2_PROTOCOL
On Tue, Jun 18, 2024 at 06:43:51AM -0600, Simon Glass wrote: > Hi Tom, > > On Mon, 17 Jun 2024 at 11:16, Tom Rini wrote: > > > > On Mon, Jun 17, 2024 at 07:53:22AM -0600, Simon Glass wrote: > > > Hi, > > > > > > On Sat, 15 Jun 2024 at 01:03, Ilias Apalodimas > > > wrote: > > > > > > > > Hi Heinrich > > > > > > > > resending the reply, I accidentally sent half of the message... > > > > > > > > On Fri, 14 Jun 2024 at 12:04, Heinrich Schuchardt > > > > wrote: > > > > > > > > > > On 14.06.24 09:01, Ilias Apalodimas wrote: > > > > > > On Fri, 14 Jun 2024 at 09:59, Heinrich Schuchardt > > > > > > wrote: > > > > > >> > > > > > >> On 6/14/24 08:03, Ilias Apalodimas wrote: > > > > > >>> Hi Simon, > > > > > >>> > > > > > >>> On Mon, 10 Jun 2024 at 17:59, Simon Glass > > > > > >>> wrote: > > > > > > > > > > It does not make sense to enable all SHA algorithms unless they > > > > > are > > > > > needed. It bloats the code and in this case, causes > > > > > chromebook_link to > > > > > fail to build. That board does use the TPM, but not with > > > > > measured boot, > > > > > nor EFI. > > > > > > > > > > Since EFI_TCG2_PROTOCOL already selects these options, we just > > > > > need to > > > > > add them to MEASURED_BOOT as well. > > > > > > > > > > Note that the original commit combines refactoring and new > > > > > features, > > > > > which makes it hard to see what is going on. > > > > > > > > > > Fixes: 97707f12fda tpm: Support boot measurements > > > > > Signed-off-by: Simon Glass > > > > > --- > > > > > > > > > > Changes in v2: > > > > > - Put the conditions under EFI_TCG2_PROTOCOL > > > > > - Consider MEASURED_BOOT too > > > > > > > > > > boot/Kconfig | 4 > > > > > lib/Kconfig | 4 > > > > > 2 files changed, 4 insertions(+), 4 deletions(-) > > > > > > > > > > diff --git a/boot/Kconfig b/boot/Kconfig > > > > > index 6f3096c15a6..b061891e109 100644 > > > > > --- a/boot/Kconfig > > > > > +++ b/boot/Kconfig > > > > > @@ -734,6 +734,10 @@ config LEGACY_IMAGE_FORMAT > > > > > config MEASURED_BOOT > > > > > bool "Measure boot images and configuration when > > > > > booting without EFI" > > > > > depends on HASH && TPM_V2 > > > > > + select SHA1 > > > > > + select SHA256 > > > > > + select SHA384 > > > > > + select SHA512 > > > > > help > > > > > This option enables measurement of the boot process > > > > > when booting > > > > > without UEFI . Measurement involves creating > > > > > cryptographic hashes > > > > > diff --git a/lib/Kconfig b/lib/Kconfig > > > > > index 189e6eb31aa..568892fce44 100644 > > > > > --- a/lib/Kconfig > > > > > +++ b/lib/Kconfig > > > > > @@ -438,10 +438,6 @@ config TPM > > > > > bool "Trusted Platform Module (TPM) Support" > > > > > depends on DM > > > > > imply DM_RNG > > > > > - select SHA1 > > > > > - select SHA256 > > > > > - select SHA384 > > > > > - select SHA512 > > > > > >>> > > > > > >>> I am not sure this is the right way to deal with your problem. > > > > > >>> The TPM main functionality is to measure and extend PCRs, so > > > > > >>> sha > > > > > >>> is really required. To make things even worse, you don't know the > > > > > >>> PCR > > > > > >>> banks that are enabled beforehand. This is a runtime config of the > > > > > >>> TPM. > > > > > >> > > > > > >> If neither MEASURED_BOOT nor EFI_TCG2_PROTOCOL is selected, U-Boot > > > > > >> cannot extend PCRs. So it seems fine to let these two select the > > > > > >> complete set of hashing algorithms. As Simon pointed out for > > > > > >> EFI_TCG2_PROTOCOL this is already done in lib/efi_loader/Kconfig. > > > > > > > > > > > > It can. The cmd we have can extend those pcrs -- e.g tpm2 > > > > > > pcr_extend 8 > > > > > > 0xb000 > > > > > > That's pretty normal for U-Boot though, since we want to avoid lots of > > > growth for things people might want control over. We can enable or > > > disable the SHA for the board, if this functionality is used outside > > > of measured boot and tcg2, but someone is enabling the tpm command. > > > > > > > > > > > > > So this patch should also consider CMD_TPM_V2 and CMD_TPM_V1. > > > > > > > > > > TPM v1 only needs SHA-1. > > > > > > > > I still prefer to imply all algos. > > > > > > 'imply' would be OK in this case as I can disable it for that board. I > > > don't think it is in the spirit of U-Boot though. > > > > > > isn't someone checking the growth in U-Boot? Or do so few boards have > > > TPMs that it didn't register? The size growth was 3.2KB on > > > chromebook_link. > > > > As always, yes, nearly every PR (I don't check the on
Re: [PATCH v2 2/9] tpm: Avoid code bloat when not using EFI_TCG2_PROTOCOL
Hi Tom, On Mon, 17 Jun 2024 at 11:16, Tom Rini wrote: > > On Mon, Jun 17, 2024 at 07:53:22AM -0600, Simon Glass wrote: > > Hi, > > > > On Sat, 15 Jun 2024 at 01:03, Ilias Apalodimas > > wrote: > > > > > > Hi Heinrich > > > > > > resending the reply, I accidentally sent half of the message... > > > > > > On Fri, 14 Jun 2024 at 12:04, Heinrich Schuchardt > > > wrote: > > > > > > > > On 14.06.24 09:01, Ilias Apalodimas wrote: > > > > > On Fri, 14 Jun 2024 at 09:59, Heinrich Schuchardt > > > > > wrote: > > > > >> > > > > >> On 6/14/24 08:03, Ilias Apalodimas wrote: > > > > >>> Hi Simon, > > > > >>> > > > > >>> On Mon, 10 Jun 2024 at 17:59, Simon Glass wrote: > > > > > > > > It does not make sense to enable all SHA algorithms unless they are > > > > needed. It bloats the code and in this case, causes > > > > chromebook_link to > > > > fail to build. That board does use the TPM, but not with measured > > > > boot, > > > > nor EFI. > > > > > > > > Since EFI_TCG2_PROTOCOL already selects these options, we just > > > > need to > > > > add them to MEASURED_BOOT as well. > > > > > > > > Note that the original commit combines refactoring and new > > > > features, > > > > which makes it hard to see what is going on. > > > > > > > > Fixes: 97707f12fda tpm: Support boot measurements > > > > Signed-off-by: Simon Glass > > > > --- > > > > > > > > Changes in v2: > > > > - Put the conditions under EFI_TCG2_PROTOCOL > > > > - Consider MEASURED_BOOT too > > > > > > > > boot/Kconfig | 4 > > > > lib/Kconfig | 4 > > > > 2 files changed, 4 insertions(+), 4 deletions(-) > > > > > > > > diff --git a/boot/Kconfig b/boot/Kconfig > > > > index 6f3096c15a6..b061891e109 100644 > > > > --- a/boot/Kconfig > > > > +++ b/boot/Kconfig > > > > @@ -734,6 +734,10 @@ config LEGACY_IMAGE_FORMAT > > > > config MEASURED_BOOT > > > > bool "Measure boot images and configuration when booting > > > > without EFI" > > > > depends on HASH && TPM_V2 > > > > + select SHA1 > > > > + select SHA256 > > > > + select SHA384 > > > > + select SHA512 > > > > help > > > > This option enables measurement of the boot process > > > > when booting > > > > without UEFI . Measurement involves creating > > > > cryptographic hashes > > > > diff --git a/lib/Kconfig b/lib/Kconfig > > > > index 189e6eb31aa..568892fce44 100644 > > > > --- a/lib/Kconfig > > > > +++ b/lib/Kconfig > > > > @@ -438,10 +438,6 @@ config TPM > > > > bool "Trusted Platform Module (TPM) Support" > > > > depends on DM > > > > imply DM_RNG > > > > - select SHA1 > > > > - select SHA256 > > > > - select SHA384 > > > > - select SHA512 > > > > >>> > > > > >>> I am not sure this is the right way to deal with your problem. > > > > >>> The TPM main functionality is to measure and extend PCRs, so sha > > > > >>> is really required. To make things even worse, you don't know the > > > > >>> PCR > > > > >>> banks that are enabled beforehand. This is a runtime config of the > > > > >>> TPM. > > > > >> > > > > >> If neither MEASURED_BOOT nor EFI_TCG2_PROTOCOL is selected, U-Boot > > > > >> cannot extend PCRs. So it seems fine to let these two select the > > > > >> complete set of hashing algorithms. As Simon pointed out for > > > > >> EFI_TCG2_PROTOCOL this is already done in lib/efi_loader/Kconfig. > > > > > > > > > > It can. The cmd we have can extend those pcrs -- e.g tpm2 pcr_extend 8 > > > > > 0xb000 > > > > That's pretty normal for U-Boot though, since we want to avoid lots of > > growth for things people might want control over. We can enable or > > disable the SHA for the board, if this functionality is used outside > > of measured boot and tcg2, but someone is enabling the tpm command. > > > > > > > > > > So this patch should also consider CMD_TPM_V2 and CMD_TPM_V1. > > > > > > > > TPM v1 only needs SHA-1. > > > > > > I still prefer to imply all algos. > > > > 'imply' would be OK in this case as I can disable it for that board. I > > don't think it is in the spirit of U-Boot though. > > > > isn't someone checking the growth in U-Boot? Or do so few boards have > > TPMs that it didn't register? The size growth was 3.2KB on > > chromebook_link. > > As always, yes, nearly every PR (I don't check the ones that touch just > a single board for example) gets a world build before/after. In this > case I likely assumed that it was acceptable growth for enabling > features. It sounds like some of the chromebook boards need to be > setting the features to cause link failure if a size is exceeded? The problem is that some Intel platforms have binary blobs, so t
Re: [PATCH v2 2/9] tpm: Avoid code bloat when not using EFI_TCG2_PROTOCOL
On Mon, Jun 17, 2024 at 07:53:22AM -0600, Simon Glass wrote: > Hi, > > On Sat, 15 Jun 2024 at 01:03, Ilias Apalodimas > wrote: > > > > Hi Heinrich > > > > resending the reply, I accidentally sent half of the message... > > > > On Fri, 14 Jun 2024 at 12:04, Heinrich Schuchardt > > wrote: > > > > > > On 14.06.24 09:01, Ilias Apalodimas wrote: > > > > On Fri, 14 Jun 2024 at 09:59, Heinrich Schuchardt > > > > wrote: > > > >> > > > >> On 6/14/24 08:03, Ilias Apalodimas wrote: > > > >>> Hi Simon, > > > >>> > > > >>> On Mon, 10 Jun 2024 at 17:59, Simon Glass wrote: > > > > > > It does not make sense to enable all SHA algorithms unless they are > > > needed. It bloats the code and in this case, causes chromebook_link > > > to > > > fail to build. That board does use the TPM, but not with measured > > > boot, > > > nor EFI. > > > > > > Since EFI_TCG2_PROTOCOL already selects these options, we just need > > > to > > > add them to MEASURED_BOOT as well. > > > > > > Note that the original commit combines refactoring and new features, > > > which makes it hard to see what is going on. > > > > > > Fixes: 97707f12fda tpm: Support boot measurements > > > Signed-off-by: Simon Glass > > > --- > > > > > > Changes in v2: > > > - Put the conditions under EFI_TCG2_PROTOCOL > > > - Consider MEASURED_BOOT too > > > > > > boot/Kconfig | 4 > > > lib/Kconfig | 4 > > > 2 files changed, 4 insertions(+), 4 deletions(-) > > > > > > diff --git a/boot/Kconfig b/boot/Kconfig > > > index 6f3096c15a6..b061891e109 100644 > > > --- a/boot/Kconfig > > > +++ b/boot/Kconfig > > > @@ -734,6 +734,10 @@ config LEGACY_IMAGE_FORMAT > > > config MEASURED_BOOT > > > bool "Measure boot images and configuration when booting > > > without EFI" > > > depends on HASH && TPM_V2 > > > + select SHA1 > > > + select SHA256 > > > + select SHA384 > > > + select SHA512 > > > help > > > This option enables measurement of the boot process when > > > booting > > > without UEFI . Measurement involves creating > > > cryptographic hashes > > > diff --git a/lib/Kconfig b/lib/Kconfig > > > index 189e6eb31aa..568892fce44 100644 > > > --- a/lib/Kconfig > > > +++ b/lib/Kconfig > > > @@ -438,10 +438,6 @@ config TPM > > > bool "Trusted Platform Module (TPM) Support" > > > depends on DM > > > imply DM_RNG > > > - select SHA1 > > > - select SHA256 > > > - select SHA384 > > > - select SHA512 > > > >>> > > > >>> I am not sure this is the right way to deal with your problem. > > > >>> The TPM main functionality is to measure and extend PCRs, so sha > > > >>> is really required. To make things even worse, you don't know the PCR > > > >>> banks that are enabled beforehand. This is a runtime config of the > > > >>> TPM. > > > >> > > > >> If neither MEASURED_BOOT nor EFI_TCG2_PROTOCOL is selected, U-Boot > > > >> cannot extend PCRs. So it seems fine to let these two select the > > > >> complete set of hashing algorithms. As Simon pointed out for > > > >> EFI_TCG2_PROTOCOL this is already done in lib/efi_loader/Kconfig. > > > > > > > > It can. The cmd we have can extend those pcrs -- e.g tpm2 pcr_extend 8 > > > > 0xb000 > > That's pretty normal for U-Boot though, since we want to avoid lots of > growth for things people might want control over. We can enable or > disable the SHA for the board, if this functionality is used outside > of measured boot and tcg2, but someone is enabling the tpm command. > > > > > > > So this patch should also consider CMD_TPM_V2 and CMD_TPM_V1. > > > > > > TPM v1 only needs SHA-1. > > > > I still prefer to imply all algos. > > 'imply' would be OK in this case as I can disable it for that board. I > don't think it is in the spirit of U-Boot though. > > isn't someone checking the growth in U-Boot? Or do so few boards have > TPMs that it didn't register? The size growth was 3.2KB on > chromebook_link. As always, yes, nearly every PR (I don't check the ones that touch just a single board for example) gets a world build before/after. In this case I likely assumed that it was acceptable growth for enabling features. It sounds like some of the chromebook boards need to be setting the features to cause link failure if a size is exceeded? -- Tom signature.asc Description: PGP signature
Re: [PATCH v2 2/9] tpm: Avoid code bloat when not using EFI_TCG2_PROTOCOL
Hi, On Sat, 15 Jun 2024 at 01:03, Ilias Apalodimas wrote: > > Hi Heinrich > > resending the reply, I accidentally sent half of the message... > > On Fri, 14 Jun 2024 at 12:04, Heinrich Schuchardt wrote: > > > > On 14.06.24 09:01, Ilias Apalodimas wrote: > > > On Fri, 14 Jun 2024 at 09:59, Heinrich Schuchardt > > > wrote: > > >> > > >> On 6/14/24 08:03, Ilias Apalodimas wrote: > > >>> Hi Simon, > > >>> > > >>> On Mon, 10 Jun 2024 at 17:59, Simon Glass wrote: > > > > It does not make sense to enable all SHA algorithms unless they are > > needed. It bloats the code and in this case, causes chromebook_link to > > fail to build. That board does use the TPM, but not with measured boot, > > nor EFI. > > > > Since EFI_TCG2_PROTOCOL already selects these options, we just need to > > add them to MEASURED_BOOT as well. > > > > Note that the original commit combines refactoring and new features, > > which makes it hard to see what is going on. > > > > Fixes: 97707f12fda tpm: Support boot measurements > > Signed-off-by: Simon Glass > > --- > > > > Changes in v2: > > - Put the conditions under EFI_TCG2_PROTOCOL > > - Consider MEASURED_BOOT too > > > > boot/Kconfig | 4 > > lib/Kconfig | 4 > > 2 files changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/boot/Kconfig b/boot/Kconfig > > index 6f3096c15a6..b061891e109 100644 > > --- a/boot/Kconfig > > +++ b/boot/Kconfig > > @@ -734,6 +734,10 @@ config LEGACY_IMAGE_FORMAT > > config MEASURED_BOOT > > bool "Measure boot images and configuration when booting > > without EFI" > > depends on HASH && TPM_V2 > > + select SHA1 > > + select SHA256 > > + select SHA384 > > + select SHA512 > > help > > This option enables measurement of the boot process when > > booting > > without UEFI . Measurement involves creating cryptographic > > hashes > > diff --git a/lib/Kconfig b/lib/Kconfig > > index 189e6eb31aa..568892fce44 100644 > > --- a/lib/Kconfig > > +++ b/lib/Kconfig > > @@ -438,10 +438,6 @@ config TPM > > bool "Trusted Platform Module (TPM) Support" > > depends on DM > > imply DM_RNG > > - select SHA1 > > - select SHA256 > > - select SHA384 > > - select SHA512 > > >>> > > >>> I am not sure this is the right way to deal with your problem. > > >>> The TPM main functionality is to measure and extend PCRs, so sha > > >>> is really required. To make things even worse, you don't know the PCR > > >>> banks that are enabled beforehand. This is a runtime config of the > > >>> TPM. > > >> > > >> If neither MEASURED_BOOT nor EFI_TCG2_PROTOCOL is selected, U-Boot > > >> cannot extend PCRs. So it seems fine to let these two select the > > >> complete set of hashing algorithms. As Simon pointed out for > > >> EFI_TCG2_PROTOCOL this is already done in lib/efi_loader/Kconfig. > > > > > > It can. The cmd we have can extend those pcrs -- e.g tpm2 pcr_extend 8 > > > 0xb000 That's pretty normal for U-Boot though, since we want to avoid lots of growth for things people might want control over. We can enable or disable the SHA for the board, if this functionality is used outside of measured boot and tcg2, but someone is enabling the tpm command. > > > > So this patch should also consider CMD_TPM_V2 and CMD_TPM_V1. > > > > TPM v1 only needs SHA-1. > > I still prefer to imply all algos. 'imply' would be OK in this case as I can disable it for that board. I don't think it is in the spirit of U-Boot though. isn't someone checking the growth in U-Boot? Or do so few boards have TPMs that it didn't register? The size growth was 3.2KB on chromebook_link. > > > > > In cmd/tpm-v2.c do_tpm2_pcr_extend() and do_tpm_pcr_read() assume > > SHA256. Function tpm_pcr_extend() shows the same limitation. This bug > > should be fixed. But as is CMD_TPM_V2 seems only to require CONFIG_SHA256. > > Isn't [0] fixing this? > > [0] > https://source.denx.de/u-boot/u-boot/-/commit/89aa8463cdf3919ca4f04fc24ec8b154ff56d97e > Thanks > /Ilias > > > > Best regards > > > > Heinrich > > > > > > > > Regards > > > /Ilias > > >> > > >> Even if U-Boot does not support measured boot (EFI or non-EFI) we might > > >> still be using the TPMs RNG. > > >> > > >> Reviewed-by: Heinrich Schuchardt > > >> > > >>> > > >>>So this would make the TPM pretty useless. Can't you remove something > > >>> that doesn't break functionality? > > >>> > > >>> Thanks > > >>> /Ilias > > help > > This enables support for TPMs which can be used to provide > > security > > features for your board. The TPM can be connected via LPC > > or I2C > > -- > > >>
Re: [PATCH v2 2/9] tpm: Avoid code bloat when not using EFI_TCG2_PROTOCOL
Hi Heinrich resending the reply, I accidentally sent half of the message... On Fri, 14 Jun 2024 at 12:04, Heinrich Schuchardt wrote: > > On 14.06.24 09:01, Ilias Apalodimas wrote: > > On Fri, 14 Jun 2024 at 09:59, Heinrich Schuchardt > > wrote: > >> > >> On 6/14/24 08:03, Ilias Apalodimas wrote: > >>> Hi Simon, > >>> > >>> On Mon, 10 Jun 2024 at 17:59, Simon Glass wrote: > > It does not make sense to enable all SHA algorithms unless they are > needed. It bloats the code and in this case, causes chromebook_link to > fail to build. That board does use the TPM, but not with measured boot, > nor EFI. > > Since EFI_TCG2_PROTOCOL already selects these options, we just need to > add them to MEASURED_BOOT as well. > > Note that the original commit combines refactoring and new features, > which makes it hard to see what is going on. > > Fixes: 97707f12fda tpm: Support boot measurements > Signed-off-by: Simon Glass > --- > > Changes in v2: > - Put the conditions under EFI_TCG2_PROTOCOL > - Consider MEASURED_BOOT too > > boot/Kconfig | 4 > lib/Kconfig | 4 > 2 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/boot/Kconfig b/boot/Kconfig > index 6f3096c15a6..b061891e109 100644 > --- a/boot/Kconfig > +++ b/boot/Kconfig > @@ -734,6 +734,10 @@ config LEGACY_IMAGE_FORMAT > config MEASURED_BOOT > bool "Measure boot images and configuration when booting > without EFI" > depends on HASH && TPM_V2 > + select SHA1 > + select SHA256 > + select SHA384 > + select SHA512 > help > This option enables measurement of the boot process when > booting > without UEFI . Measurement involves creating cryptographic > hashes > diff --git a/lib/Kconfig b/lib/Kconfig > index 189e6eb31aa..568892fce44 100644 > --- a/lib/Kconfig > +++ b/lib/Kconfig > @@ -438,10 +438,6 @@ config TPM > bool "Trusted Platform Module (TPM) Support" > depends on DM > imply DM_RNG > - select SHA1 > - select SHA256 > - select SHA384 > - select SHA512 > >>> > >>> I am not sure this is the right way to deal with your problem. > >>> The TPM main functionality is to measure and extend PCRs, so sha > >>> is really required. To make things even worse, you don't know the PCR > >>> banks that are enabled beforehand. This is a runtime config of the > >>> TPM. > >> > >> If neither MEASURED_BOOT nor EFI_TCG2_PROTOCOL is selected, U-Boot > >> cannot extend PCRs. So it seems fine to let these two select the > >> complete set of hashing algorithms. As Simon pointed out for > >> EFI_TCG2_PROTOCOL this is already done in lib/efi_loader/Kconfig. > > > > It can. The cmd we have can extend those pcrs -- e.g tpm2 pcr_extend 8 > > 0xb000 > > So this patch should also consider CMD_TPM_V2 and CMD_TPM_V1. > > TPM v1 only needs SHA-1. I still prefer to imply all algos. > > In cmd/tpm-v2.c do_tpm2_pcr_extend() and do_tpm_pcr_read() assume > SHA256. Function tpm_pcr_extend() shows the same limitation. This bug > should be fixed. But as is CMD_TPM_V2 seems only to require CONFIG_SHA256. Isn't [0] fixing this? [0] https://source.denx.de/u-boot/u-boot/-/commit/89aa8463cdf3919ca4f04fc24ec8b154ff56d97e Thanks /Ilias > > Best regards > > Heinrich > > > > > Regards > > /Ilias > >> > >> Even if U-Boot does not support measured boot (EFI or non-EFI) we might > >> still be using the TPMs RNG. > >> > >> Reviewed-by: Heinrich Schuchardt > >> > >>> > >>>So this would make the TPM pretty useless. Can't you remove something > >>> that doesn't break functionality? > >>> > >>> Thanks > >>> /Ilias > help > This enables support for TPMs which can be used to provide > security > features for your board. The TPM can be connected via LPC or > I2C > -- > 2.34.1 > > >> >
Re: [PATCH v2 2/9] tpm: Avoid code bloat when not using EFI_TCG2_PROTOCOL
Hi Heinrich, On Fri, 14 Jun 2024 at 12:04, Heinrich Schuchardt wrote: > > On 14.06.24 09:01, Ilias Apalodimas wrote: > > On Fri, 14 Jun 2024 at 09:59, Heinrich Schuchardt > > wrote: > >> > >> On 6/14/24 08:03, Ilias Apalodimas wrote: > >>> Hi Simon, > >>> > >>> On Mon, 10 Jun 2024 at 17:59, Simon Glass wrote: > > It does not make sense to enable all SHA algorithms unless they are > needed. It bloats the code and in this case, causes chromebook_link to > fail to build. That board does use the TPM, but not with measured boot, > nor EFI. > > Since EFI_TCG2_PROTOCOL already selects these options, we just need to > add them to MEASURED_BOOT as well. > > Note that the original commit combines refactoring and new features, > which makes it hard to see what is going on. > > Fixes: 97707f12fda tpm: Support boot measurements > Signed-off-by: Simon Glass > --- > > Changes in v2: > - Put the conditions under EFI_TCG2_PROTOCOL > - Consider MEASURED_BOOT too > > boot/Kconfig | 4 > lib/Kconfig | 4 > 2 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/boot/Kconfig b/boot/Kconfig > index 6f3096c15a6..b061891e109 100644 > --- a/boot/Kconfig > +++ b/boot/Kconfig > @@ -734,6 +734,10 @@ config LEGACY_IMAGE_FORMAT > config MEASURED_BOOT > bool "Measure boot images and configuration when booting > without EFI" > depends on HASH && TPM_V2 > + select SHA1 > + select SHA256 > + select SHA384 > + select SHA512 > help > This option enables measurement of the boot process when > booting > without UEFI . Measurement involves creating cryptographic > hashes > diff --git a/lib/Kconfig b/lib/Kconfig > index 189e6eb31aa..568892fce44 100644 > --- a/lib/Kconfig > +++ b/lib/Kconfig > @@ -438,10 +438,6 @@ config TPM > bool "Trusted Platform Module (TPM) Support" > depends on DM > imply DM_RNG > - select SHA1 > - select SHA256 > - select SHA384 > - select SHA512 > >>> > >>> I am not sure this is the right way to deal with your problem. > >>> The TPM main functionality is to measure and extend PCRs, so sha > >>> is really required. To make things even worse, you don't know the PCR > >>> banks that are enabled beforehand. This is a runtime config of the > >>> TPM. > >> > >> If neither MEASURED_BOOT nor EFI_TCG2_PROTOCOL is selected, U-Boot > >> cannot extend PCRs. So it seems fine to let these two select the > >> complete set of hashing algorithms. As Simon pointed out for > >> EFI_TCG2_PROTOCOL this is already done in lib/efi_loader/Kconfig. > > > > It can. The cmd we have can extend those pcrs -- e.g tpm2 pcr_extend 8 > > 0xb000 > > So this patch should also consider CMD_TPM_V2 and CMD_TPM_V1. > > TPM v1 only needs SHA-1. I still prefer to leave the TPM in a working state tbh. > > In cmd/tpm-v2.c do_tpm2_pcr_extend() and do_tpm_pcr_read() assume > SHA256. Function tpm_pcr_extend() shows the same limitation. This bug > should be fixed. But as is CMD_TPM_V2 seems only to require CONFIG_SHA256. > > Best regards > > Heinrich > > > > > Regards > > /Ilias > >> > >> Even if U-Boot does not support measured boot (EFI or non-EFI) we might > >> still be using the TPMs RNG. > >> > >> Reviewed-by: Heinrich Schuchardt > >> > >>> > >>>So this would make the TPM pretty useless. Can't you remove something > >>> that doesn't break functionality? > >>> > >>> Thanks > >>> /Ilias > help > This enables support for TPMs which can be used to provide > security > features for your board. The TPM can be connected via LPC or > I2C > -- > 2.34.1 > > >> >
Re: [PATCH v2 2/9] tpm: Avoid code bloat when not using EFI_TCG2_PROTOCOL
On 14.06.24 09:01, Ilias Apalodimas wrote: On Fri, 14 Jun 2024 at 09:59, Heinrich Schuchardt wrote: On 6/14/24 08:03, Ilias Apalodimas wrote: Hi Simon, On Mon, 10 Jun 2024 at 17:59, Simon Glass wrote: It does not make sense to enable all SHA algorithms unless they are needed. It bloats the code and in this case, causes chromebook_link to fail to build. That board does use the TPM, but not with measured boot, nor EFI. Since EFI_TCG2_PROTOCOL already selects these options, we just need to add them to MEASURED_BOOT as well. Note that the original commit combines refactoring and new features, which makes it hard to see what is going on. Fixes: 97707f12fda tpm: Support boot measurements Signed-off-by: Simon Glass --- Changes in v2: - Put the conditions under EFI_TCG2_PROTOCOL - Consider MEASURED_BOOT too boot/Kconfig | 4 lib/Kconfig | 4 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/boot/Kconfig b/boot/Kconfig index 6f3096c15a6..b061891e109 100644 --- a/boot/Kconfig +++ b/boot/Kconfig @@ -734,6 +734,10 @@ config LEGACY_IMAGE_FORMAT config MEASURED_BOOT bool "Measure boot images and configuration when booting without EFI" depends on HASH && TPM_V2 + select SHA1 + select SHA256 + select SHA384 + select SHA512 help This option enables measurement of the boot process when booting without UEFI . Measurement involves creating cryptographic hashes diff --git a/lib/Kconfig b/lib/Kconfig index 189e6eb31aa..568892fce44 100644 --- a/lib/Kconfig +++ b/lib/Kconfig @@ -438,10 +438,6 @@ config TPM bool "Trusted Platform Module (TPM) Support" depends on DM imply DM_RNG - select SHA1 - select SHA256 - select SHA384 - select SHA512 I am not sure this is the right way to deal with your problem. The TPM main functionality is to measure and extend PCRs, so sha is really required. To make things even worse, you don't know the PCR banks that are enabled beforehand. This is a runtime config of the TPM. If neither MEASURED_BOOT nor EFI_TCG2_PROTOCOL is selected, U-Boot cannot extend PCRs. So it seems fine to let these two select the complete set of hashing algorithms. As Simon pointed out for EFI_TCG2_PROTOCOL this is already done in lib/efi_loader/Kconfig. It can. The cmd we have can extend those pcrs -- e.g tpm2 pcr_extend 8 0xb000 So this patch should also consider CMD_TPM_V2 and CMD_TPM_V1. TPM v1 only needs SHA-1. In cmd/tpm-v2.c do_tpm2_pcr_extend() and do_tpm_pcr_read() assume SHA256. Function tpm_pcr_extend() shows the same limitation. This bug should be fixed. But as is CMD_TPM_V2 seems only to require CONFIG_SHA256. Best regards Heinrich Regards /Ilias Even if U-Boot does not support measured boot (EFI or non-EFI) we might still be using the TPMs RNG. Reviewed-by: Heinrich Schuchardt So this would make the TPM pretty useless. Can't you remove something that doesn't break functionality? Thanks /Ilias help This enables support for TPMs which can be used to provide security features for your board. The TPM can be connected via LPC or I2C -- 2.34.1
Re: [PATCH v2 2/9] tpm: Avoid code bloat when not using EFI_TCG2_PROTOCOL
On Fri, 14 Jun 2024 at 09:59, Heinrich Schuchardt wrote: > > On 6/14/24 08:03, Ilias Apalodimas wrote: > > Hi Simon, > > > > On Mon, 10 Jun 2024 at 17:59, Simon Glass wrote: > >> > >> It does not make sense to enable all SHA algorithms unless they are > >> needed. It bloats the code and in this case, causes chromebook_link to > >> fail to build. That board does use the TPM, but not with measured boot, > >> nor EFI. > >> > >> Since EFI_TCG2_PROTOCOL already selects these options, we just need to > >> add them to MEASURED_BOOT as well. > >> > >> Note that the original commit combines refactoring and new features, > >> which makes it hard to see what is going on. > >> > >> Fixes: 97707f12fda tpm: Support boot measurements > >> Signed-off-by: Simon Glass > >> --- > >> > >> Changes in v2: > >> - Put the conditions under EFI_TCG2_PROTOCOL > >> - Consider MEASURED_BOOT too > >> > >> boot/Kconfig | 4 > >> lib/Kconfig | 4 > >> 2 files changed, 4 insertions(+), 4 deletions(-) > >> > >> diff --git a/boot/Kconfig b/boot/Kconfig > >> index 6f3096c15a6..b061891e109 100644 > >> --- a/boot/Kconfig > >> +++ b/boot/Kconfig > >> @@ -734,6 +734,10 @@ config LEGACY_IMAGE_FORMAT > >> config MEASURED_BOOT > >> bool "Measure boot images and configuration when booting without > >> EFI" > >> depends on HASH && TPM_V2 > >> + select SHA1 > >> + select SHA256 > >> + select SHA384 > >> + select SHA512 > >> help > >>This option enables measurement of the boot process when booting > >>without UEFI . Measurement involves creating cryptographic > >> hashes > >> diff --git a/lib/Kconfig b/lib/Kconfig > >> index 189e6eb31aa..568892fce44 100644 > >> --- a/lib/Kconfig > >> +++ b/lib/Kconfig > >> @@ -438,10 +438,6 @@ config TPM > >> bool "Trusted Platform Module (TPM) Support" > >> depends on DM > >> imply DM_RNG > >> - select SHA1 > >> - select SHA256 > >> - select SHA384 > >> - select SHA512 > > > > I am not sure this is the right way to deal with your problem. > > The TPM main functionality is to measure and extend PCRs, so sha > > is really required. To make things even worse, you don't know the PCR > > banks that are enabled beforehand. This is a runtime config of the > > TPM. > > If neither MEASURED_BOOT nor EFI_TCG2_PROTOCOL is selected, U-Boot > cannot extend PCRs. So it seems fine to let these two select the > complete set of hashing algorithms. As Simon pointed out for > EFI_TCG2_PROTOCOL this is already done in lib/efi_loader/Kconfig. It can. The cmd we have can extend those pcrs -- e.g tpm2 pcr_extend 8 0xb000 Regards /Ilias > > Even if U-Boot does not support measured boot (EFI or non-EFI) we might > still be using the TPMs RNG. > > Reviewed-by: Heinrich Schuchardt > > > > > So this would make the TPM pretty useless. Can't you remove something > > that doesn't break functionality? > > > > Thanks > > /Ilias > >> help > >>This enables support for TPMs which can be used to provide > >> security > >>features for your board. The TPM can be connected via LPC or I2C > >> -- > >> 2.34.1 > >> >
Re: [PATCH v2 2/9] tpm: Avoid code bloat when not using EFI_TCG2_PROTOCOL
On 6/14/24 08:03, Ilias Apalodimas wrote: Hi Simon, On Mon, 10 Jun 2024 at 17:59, Simon Glass wrote: It does not make sense to enable all SHA algorithms unless they are needed. It bloats the code and in this case, causes chromebook_link to fail to build. That board does use the TPM, but not with measured boot, nor EFI. Since EFI_TCG2_PROTOCOL already selects these options, we just need to add them to MEASURED_BOOT as well. Note that the original commit combines refactoring and new features, which makes it hard to see what is going on. Fixes: 97707f12fda tpm: Support boot measurements Signed-off-by: Simon Glass --- Changes in v2: - Put the conditions under EFI_TCG2_PROTOCOL - Consider MEASURED_BOOT too boot/Kconfig | 4 lib/Kconfig | 4 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/boot/Kconfig b/boot/Kconfig index 6f3096c15a6..b061891e109 100644 --- a/boot/Kconfig +++ b/boot/Kconfig @@ -734,6 +734,10 @@ config LEGACY_IMAGE_FORMAT config MEASURED_BOOT bool "Measure boot images and configuration when booting without EFI" depends on HASH && TPM_V2 + select SHA1 + select SHA256 + select SHA384 + select SHA512 help This option enables measurement of the boot process when booting without UEFI . Measurement involves creating cryptographic hashes diff --git a/lib/Kconfig b/lib/Kconfig index 189e6eb31aa..568892fce44 100644 --- a/lib/Kconfig +++ b/lib/Kconfig @@ -438,10 +438,6 @@ config TPM bool "Trusted Platform Module (TPM) Support" depends on DM imply DM_RNG - select SHA1 - select SHA256 - select SHA384 - select SHA512 I am not sure this is the right way to deal with your problem. The TPM main functionality is to measure and extend PCRs, so sha is really required. To make things even worse, you don't know the PCR banks that are enabled beforehand. This is a runtime config of the TPM. If neither MEASURED_BOOT nor EFI_TCG2_PROTOCOL is selected, U-Boot cannot extend PCRs. So it seems fine to let these two select the complete set of hashing algorithms. As Simon pointed out for EFI_TCG2_PROTOCOL this is already done in lib/efi_loader/Kconfig. Even if U-Boot does not support measured boot (EFI or non-EFI) we might still be using the TPMs RNG. Reviewed-by: Heinrich Schuchardt So this would make the TPM pretty useless. Can't you remove something that doesn't break functionality? Thanks /Ilias help This enables support for TPMs which can be used to provide security features for your board. The TPM can be connected via LPC or I2C -- 2.34.1
Re: [PATCH v2 2/9] tpm: Avoid code bloat when not using EFI_TCG2_PROTOCOL
Hi Simon, On Mon, 10 Jun 2024 at 17:59, Simon Glass wrote: > > It does not make sense to enable all SHA algorithms unless they are > needed. It bloats the code and in this case, causes chromebook_link to > fail to build. That board does use the TPM, but not with measured boot, > nor EFI. > > Since EFI_TCG2_PROTOCOL already selects these options, we just need to > add them to MEASURED_BOOT as well. > > Note that the original commit combines refactoring and new features, > which makes it hard to see what is going on. > > Fixes: 97707f12fda tpm: Support boot measurements > Signed-off-by: Simon Glass > --- > > Changes in v2: > - Put the conditions under EFI_TCG2_PROTOCOL > - Consider MEASURED_BOOT too > > boot/Kconfig | 4 > lib/Kconfig | 4 > 2 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/boot/Kconfig b/boot/Kconfig > index 6f3096c15a6..b061891e109 100644 > --- a/boot/Kconfig > +++ b/boot/Kconfig > @@ -734,6 +734,10 @@ config LEGACY_IMAGE_FORMAT > config MEASURED_BOOT > bool "Measure boot images and configuration when booting without EFI" > depends on HASH && TPM_V2 > + select SHA1 > + select SHA256 > + select SHA384 > + select SHA512 > help > This option enables measurement of the boot process when booting > without UEFI . Measurement involves creating cryptographic hashes > diff --git a/lib/Kconfig b/lib/Kconfig > index 189e6eb31aa..568892fce44 100644 > --- a/lib/Kconfig > +++ b/lib/Kconfig > @@ -438,10 +438,6 @@ config TPM > bool "Trusted Platform Module (TPM) Support" > depends on DM > imply DM_RNG > - select SHA1 > - select SHA256 > - select SHA384 > - select SHA512 I am not sure this is the right way to deal with your problem. The TPM main functionality is to measure and extend PCRs, so sha is really required. To make things even worse, you don't know the PCR banks that are enabled beforehand. This is a runtime config of the TPM. So this would make the TPM pretty useless. Can't you remove something that doesn't break functionality? Thanks /Ilias > help > This enables support for TPMs which can be used to provide security > features for your board. The TPM can be connected via LPC or I2C > -- > 2.34.1 >
[PATCH v2 2/9] tpm: Avoid code bloat when not using EFI_TCG2_PROTOCOL
It does not make sense to enable all SHA algorithms unless they are needed. It bloats the code and in this case, causes chromebook_link to fail to build. That board does use the TPM, but not with measured boot, nor EFI. Since EFI_TCG2_PROTOCOL already selects these options, we just need to add them to MEASURED_BOOT as well. Note that the original commit combines refactoring and new features, which makes it hard to see what is going on. Fixes: 97707f12fda tpm: Support boot measurements Signed-off-by: Simon Glass --- Changes in v2: - Put the conditions under EFI_TCG2_PROTOCOL - Consider MEASURED_BOOT too boot/Kconfig | 4 lib/Kconfig | 4 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/boot/Kconfig b/boot/Kconfig index 6f3096c15a6..b061891e109 100644 --- a/boot/Kconfig +++ b/boot/Kconfig @@ -734,6 +734,10 @@ config LEGACY_IMAGE_FORMAT config MEASURED_BOOT bool "Measure boot images and configuration when booting without EFI" depends on HASH && TPM_V2 + select SHA1 + select SHA256 + select SHA384 + select SHA512 help This option enables measurement of the boot process when booting without UEFI . Measurement involves creating cryptographic hashes diff --git a/lib/Kconfig b/lib/Kconfig index 189e6eb31aa..568892fce44 100644 --- a/lib/Kconfig +++ b/lib/Kconfig @@ -438,10 +438,6 @@ config TPM bool "Trusted Platform Module (TPM) Support" depends on DM imply DM_RNG - select SHA1 - select SHA256 - select SHA384 - select SHA512 help This enables support for TPMs which can be used to provide security features for your board. The TPM can be connected via LPC or I2C -- 2.34.1
