Re: [PATCH v11 13/20] util/grub-protect: Add new tool

[Gary Lin]

On Mon, Apr 15, 2024 at 05:40:53PM +0800, Gary Lin wrote:
> On Fri, Apr 12, 2024 at 04:52:02PM -0400, Stefan Berger wrote:
> > 
> > 
> > On 4/12/24 04:39, Gary Lin via Grub-devel wrote:
> > > From: Hernan Gatta <hegatta@linux.microsoft.com>
> > > 
> > > To utilize the key protectors framework, there must be a way to protect
> > > full-disk encryption keys in the first place. The grub-protect tool
> > > includes support for the TPM2 key protector but other protectors that
> > > require setup ahead of time can be supported in the future.
> > > 
> > > For the TPM2 key protector, the intended flow is for a user to have a
> > > LUKS 1 or LUKS 2-protected fully-encrypted disk. The user then creates a
> > > new LUKS key file, say by reading /dev/urandom into a file, and creates
> > > a new LUKS key slot for this key. Then, the user invokes the grub-protect
> > > tool to seal this key file to a set of PCRs using the system's TPM 2.0.
> > > The resulting sealed key file is stored in an unencrypted partition such
> > > as the EFI System Partition (ESP) so that GRUB may read it. The user also
> > > has to ensure the cryptomount command is included in GRUB's boot script
> > > and that it carries the requisite key protector (-P) parameter.
> > > 
> > > Sample usage:
> > > 
> > > $ dd if=/dev/urandom of=luks-key bs=1 count=32
> > > $ sudo cryptsetup luksAddKey /dev/sdb1 luks-key --pbkdf=pbkdf2 
> > > --hash=sha512
> > > 
> > > To seal the key with TPM 2.0 Key File (recommended):
> > > 
> > > $ sudo grub-protect --action=add \
> > >                      --protector=tpm2 \
> > >               --tpm2-pcrs=0,2,4,7,9 \
> > >                      --tpm2key \
> > >                      --tpm2-keyfile=luks-key \
> > >                      --tpm2-outfile=/boot/efi/boot/grub2/sealed.tpm
> > > 
> > > Or, to seal the key with the raw sealed key:
> > > 
> > > $ sudo grub-protect --action=add \
> > >                      --protector=tpm2 \
> > >               --tpm2-pcrs=0,2,4,7,9 \
> > >                      --tpm2-keyfile=luks-key \
> > >                      --tpm2-outfile=/boot/efi/boot/grub2/sealed.key
> > > 
> > > Then, in the boot script, for TPM 2.0 Key File:
> > > 
> > > tpm2_key_protector_init --tpm2key=(hd0,gpt1)/boot/grub2/sealed.tpm
> > > cryptomount -u <SDB1_UUID> -P tpm2
> > > 
> > > Or, for the raw sealed key:
> > > 
> > > tpm2_key_protector_init --keyfile=(hd0,gpt1)/boot/grub2/sealed.key 
> > > --pcrs=0,2,4,7,9
> > > cryptomount -u <SDB1_UUID> -P tpm2
> > > 
> > > The benefit of using TPM 2.0 Key File is that the PCR set is already
> > > written in the key file, so there is no need to specify PCRs when
> > > invoking tpm2_key_protector_init.
> > > 
> > > Signed-off-by: Hernan Gatta <hegatta@linux.microsoft.com>
> > > Signed-off-by: Gary Lin <glin@suse.com>
> > > ---
> > >   .gitignore          |    2 +
> > >   Makefile.util.def   |   22 +
> > >   configure.ac        |   30 +
> > >   util/grub-protect.c | 1396 +++++++++++++++++++++++++++++++++++++++++++
> > >   4 files changed, 1450 insertions(+)
> > >   create mode 100644 util/grub-protect.c
> > > 
[...]
> > > +  /* Create SRK */
> > > +  authCommand.sessionHandle = TPM_RS_PW;
> > > +  inPublic.publicArea.type = args->srk_type.type;
> > > +  inPublic.publicArea.nameAlg  = TPM_ALG_SHA256;
> > > +  inPublic.publicArea.objectAttributes.restricted = 1;
> > > +  inPublic.publicArea.objectAttributes.userWithAuth = 1;
> > > +  inPublic.publicArea.objectAttributes.decrypt = 1;
> > > +  inPublic.publicArea.objectAttributes.fixedTPM = 1;
> > > +  inPublic.publicArea.objectAttributes.fixedParent = 1;
> > > +  inPublic.publicArea.objectAttributes.sensitiveDataOrigin = 1;
> > > +  inPublic.publicArea.objectAttributes.noDA = 1;
> > > +
> > > +  switch (args->srk_type.type)
> > > +    {
> > > +    case TPM_ALG_RSA:
> > > +      inPublic.publicArea.parameters.rsaDetail.symmetric.algorithm = 
> > > TPM_ALG_AES;
> > > +      inPublic.publicArea.parameters.rsaDetail.symmetric.keyBits.aes = 
> > > 128;
> > > +      inPublic.publicArea.parameters.rsaDetail.symmetric.mode.aes = 
> > > TPM_ALG_CFB;
> > > +      inPublic.publicArea.parameters.rsaDetail.scheme.scheme = 
> > > TPM_ALG_NULL;
> > > +      inPublic.publicArea.parameters.rsaDetail.keyBits = 
> > > args->srk_type.detail.rsa_bits;
> > 
> > Same comment here about pairing RSA3072 with AES-256 and SHA-512 maybe
> > (since SHA 384 isn't supported here).
> > 
> It's sad that we don't have native SHA384 support due to the outdated
> libgcrypt :(
> 
I revised the code and found We actually don't need libgcrypt here. When
writing the patches to support authorized policy, TPM2_Hash() was introduced,
and we can replace grub_crypto_hash() with TPM2_Hash() to enable SHA-384
digest calculation.

Gary Lin

> > Rest LGTM.
> > 
> Thanks for reviewing the patch!
> 
> Gary Lin
>