Re: [PATCH v18 20/25] tpm2_key_protector: Implement NV index

[Gary Lin]

On Fri, Aug 30, 2024 at 06:03:16PM +0200, Daniel Kiper wrote:
> On Fri, Jun 28, 2024 at 04:19:03PM +0800, Gary Lin via Grub-devel wrote:
> > From: Patrick Colp <patrick.colp@oracle.com>
> >
> > Currently with the TPM2 protector, only SRK mode is supported and
> > NV index support is just a stub. Implement the NV index option.
> >
> > Note: This only extends support on the unseal path. grub2_protect
> > has not been updated. tpm2-tools can be used to insert a key into
> > the NV index.
> >
> > An example of inserting a key using tpm2-tools:
> >
> >   # Get random key.
> >   tpm2_getrandom 32 > key.dat
> >
> >   # Create primary object.
> >   tpm2_createprimary -C o -g sha256 -G ecc -c primary.ctx
> >
> >   # Create policy object. `pcrs.dat` contains the PCR values to seal 
> > against.
> >   tpm2_startauthsession -S session.dat
> >   tpm2_policypcr -S session.dat -l sha256:7,11 -f pcrs.dat -L policy.dat
> >   tpm2_flushcontext session.dat
> >
> >   # Seal key into TPM.
> >   cat key.dat | tpm2_create -C primary.ctx -u key.pub -r key.priv -L 
> > policy.dat -i-
> >   tpm2_load -C primary.ctx -u key.pub -r key.priv -n sealing.name -c 
> > sealing.ctx
> >   tpm2_evictcontrol -C o -c sealing.ctx 0x81000000
> >
> > Then to unseal the key in grub, add this to grub.cfg:
> >
> >   tpm2_key_protector_init --mode=nv --nvindex=0x81000000 --pcrs=7,11
> >   cryptomount -u <UUID> --protector tpm2
> 
> Please add this to the GRUB documentation.
> 
Sure.

> > Signed-off-by: Patrick Colp <patrick.colp@oracle.com>
> > Signed-off-by: Gary Lin <glin@suse.com>
> > Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
> > ---
> >  .../commands/tpm2_key_protector/module.c      | 27 ++++++++++++++++---
> >  1 file changed, 23 insertions(+), 4 deletions(-)
> >
> > diff --git a/grub-core/commands/tpm2_key_protector/module.c 
> > b/grub-core/commands/tpm2_key_protector/module.c
> > index a98109c43..ae412e6f4 100644
> > --- a/grub-core/commands/tpm2_key_protector/module.c
> > +++ b/grub-core/commands/tpm2_key_protector/module.c
> > @@ -981,11 +981,30 @@ grub_tpm2_protector_srk_recover (const struct 
> > grub_tpm2_protector_context *ctx,
> >  }
> >
> >  static grub_err_t
> > -grub_tpm2_protector_nv_recover (const struct grub_tpm2_protector_context 
> > *ctx __attribute__ ((unused)),
> > -                           grub_uint8_t **key __attribute__ ((unused)),
> > -                           grub_size_t *key_size __attribute__ ((unused)))
> > +grub_tpm2_protector_nv_recover (const struct grub_tpm2_protector_context 
> > *ctx,
> > +                           grub_uint8_t **key, grub_size_t *key_size)
> >  {
> > -  return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET, N_("NV Index mode is 
> > not implemented yet"));
> > +  TPM_HANDLE sealed_handle = ctx->nv;
> > +  tpm2key_policy_t policy_seq = NULL;
> > +  grub_err_t err;
> > +
> > +  /* Create a basic policy sequence based on the given PCR selection */
> > +  err = grub_tpm2_protector_simple_policy_seq (ctx, &policy_seq);
> > +  if (err != GRUB_ERR_NONE)
> > +    goto exit;
> > +
> > +  err = grub_tpm2_protector_unseal (policy_seq, sealed_handle, key, 
> > key_size);
> > +
> > +  /* Pop error messages on success */
> > +  if (err == GRUB_ERR_NONE)
> > +    while (grub_error_pop ());
> 
> Hmmm... Why does unseal success clear whole error stack?
> If it is correct it begs for comment here.
> 
It's mirrored from srk mode which may goes through several policy
sequences. For nvindex, it only tries one policy sequence, so, yes, it's
redundant to pop grub errors.

Gary Lin