Re: [PATCH v19 00/33] Automatic Disk Unlock with TPM2

[Gary Lin]

On Mon, Sep 16, 2024 at 01:42:18PM -0400, Stefan Berger wrote:
> 
> 
> On 9/15/24 11:35 PM, Gary Lin wrote:
> > On Mon, Sep 16, 2024 at 10:24:03AM +0800, Gary Lin wrote:
> > > On Fri, Sep 13, 2024 at 10:32:39AM -0400, Stefan Berger wrote:
> > > > 
> > > > 
> > > > On 9/6/24 5:10 AM, Gary Lin wrote:
> > > > > GIT repo for v19: https://github.com/lcp/grub2/tree/tpm2-unlock-v19
> > > > > 
> > > > > This patch series is based on "Automatic TPM Disk Unlock"(*1) posted 
> > > > > by
> > > > > Hernan Gatta to introduce the key protector framework and TPM2 stack
> > > > > to GRUB2, and this could be a useful feature for the systems to
> > > > > implement full disk encryption.
> > > > > 
> > > > > To support TPM 2.0 Key File format(*2), patch 1~7,9-16 are grabbed 
> > > > > from
> > > > > Daniel Axtens's "appended signature secure boot support" (*3) to 
> > > > > import
> > > > > libtasn1 into grub2. Besides, the libtasn1 version is upgraded to
> > > > > 4.19.0 instead of 4.16.0 in the original patch.
> > > > 
> > > > 
> > > > I was going to try it out now (on a ppc64 machine) but fail to 
> > > > configure it.
> > > > The configure and build work on tip of master.
> > > > 
> > > > > git clean -xdf ; ./bootstrap && ./configure --prefix=/usr
> > > > [...]
> > > > Using python3...
> > > > Importing unicode...
> > > > Importing libgcrypt...
> > > > Importing libtasn1...
> > > > cp: cannot stat 'grub-core/lib/libtasn1/lib/*.[ch]': No such file or
> > > > directory
> > > That's weird. The second patch, "libtasn1: import libtasn1-4.19.0",
> > > imports the libtasn1 files into grub-core/lib/libtasn1/, and those
> > > source files are supposed to exist when applying the patch mentioned
> > > below.
> > > 
> > > I'll do a thorough check for that...
> > > 
> > I successfully built the patches on a freshly-cloned grub git repo.
> > Since you mentioned ppc64, I wonder if it's caused by the conflicts with
> > the PowerPC Secure Boot patches?
> 
> I took your series from the mailing list with the b4 tool. For some reason
> 02/33 is missing there, maybe because it is too big.
> 
> https://lore.kernel.org/grub-devel/20240916033543.gzfture5q4ljuw4b@GaryLaptop/T/#t
> 
> I checked out your repo branch and there I can configure but then run into
> this issue here:
> 
> tests/asn1/tests/Test_overflow.c: In function ‘test_overflow’:
> tests/asn1/tests/Test_overflow.c:48:50: error: left shift of negative value
> [-Werror=shift-negative-value]
>    48 |       unsigned long num = ((long) GRUB_UINT_MAX) << 2;
>       |                                                  ^~
> cc1: all warnings being treated as errors
> 
> It's the cast to 'long' that this gcc complains about. If I remove the cast
> then it works.
> 
Hmm the cast is actually inherited from the original test from
libtasn1, and my patch just replaces UINT_MAX with GRUB_UINT_MAX.

The original patch from Daniel Axtens replaces the if statement with
'#if':

-  if (LONG_MAX > INT_MAX)
+#if (GRUB_LONG_MAX > GRUB_INT_MAX)
+     {
+-      unsigned long num = ((long) UINT_MAX) << 2;
++      unsigned long num = ((long) GRUB_UINT_MAX) << 2;
...
+     }
++#endif

Maybe I should restore that part to avoid the cast error in some
architectures.

Gary Lin

> $ gcc --version
> gcc (Ubuntu 9.4.0-1ubuntu1~20.04.2) 9.4.0
> Copyright (C) 2019 Free Software Foundation, Inc.
> This is free software; see the source for copying conditions.  There is NO
> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
>