Re: [PATCH v2] tpm2_key_protector: dump PCRs on policy fail

[Gary Lin]

On Mon, Dec 16, 2024 at 05:28:34PM +0100, Daniel Kiper wrote:
> On Thu, Dec 12, 2024 at 02:11:24PM +0800, Gary Lin wrote:
> > PCR mismatching is one common cause of TPM key unsealing fail. Since the
> > system may be compromised, it is not safe to boot into OS to get the PCR
> > values and TPM eventlog for the further investigation.
> >
> > To provide some hints, GRUB now dumps PCRs on policy fail, so the user
> > can check the current PCR values. PCR 0~15 are chosen to cover the
> > firmware, bootloader, and OS.
> >
> > The sample output:
> >
> > PCR Mismatching! Check firmware and bootloader before typing passphrase!
> > TPM PCR [sha256]:
> >   00: 115c89bfa0e59e050cda5d2664031d225305f3582cf0c2afcb7c1f1ac2a7cf8d
> >   01: 079b3eadca25e10248daea4b1d508e5cfb703db28386be809a0b375c0a0a80a5
> >   02: 2cd8ec3de6a07e1fd39676100db57ba62372e820c19812fee55899f65746e192
> >   03: 9423b585d4eac05c97a0c06bca8898ad0ca519a6b810dcb91129bcdc10f4b112
> >   04: fa36bf5c9110d3891f040e2146d157484cd41123fa8faf4bc6b91db3d12b70ca
> >   05: 13e9ea9e38e5258e6ee2b6ae94a3cece0137490ef95c65caaac10cdf5e1bc40d
> >   06: 3ac10d749054a818806788f4e4eaa2fb4dd7d13ce0e99dc175145b63c34bb71c
> >   07: a6657a60f77928cad614a7ad153ab9ae0bed48e33b70348ae11a26762002b3bc
> >   08: 42e04f5bac1965535cb6bdb30c62bb199b1ba21d1ec6b22d0da159dfc925b8bb
> >   09: 5c83e8be79d4a432e6d409610de389ee6f1ac0c193f38d84a9ff94f360bd458b
> >   10: 0000000000000000000000000000000000000000000000000000000000000000
> >   11: 0000000000000000000000000000000000000000000000000000000000000000
> >   12: 0000000000000000000000000000000000000000000000000000000000000000
> >   13: 0000000000000000000000000000000000000000000000000000000000000000
> >   14: 894dd8e4ca1bb62e055f674f9390a39c4643ebdd1014702feef000c47e36a003
> >   15: 0000000000000000000000000000000000000000000000000000000000000000
> > error: failed to unseal sealed key (TPM2_Unseal: 0x99d).
> > error: no key protector provided a usable key for luks 
> > (af16e48f-746b-4a12-aae1-c14dcee429e0).
> 
> If you do this why do not add also a GRUB command to dump all PCRs,
> including DRTM ones.
> 
Sure, a new command would be helpful to inspect PCRs with GRUB shell.
I'll add the command in v3.

> > If the user happens to have the PCR values for key sealing, the PCR dump
> > can be used to identify the changed PCRs and narrow down the scope for
> > closer inspection.
> >
> > Please note that the PCR dump is trustworthy only if the GRUB binary is
> > authentic, so the user has to check the GRUB binary thoroughly before
> > using the PCR dump.
> >
> > Signed-off-by: Gary Lin <glin@suse.com>
> > Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
> > ---
> >  .../commands/tpm2_key_protector/module.c      | 101 +++++++++++++++++-
> >  1 file changed, 97 insertions(+), 4 deletions(-)
> >
> > diff --git a/grub-core/commands/tpm2_key_protector/module.c 
> > b/grub-core/commands/tpm2_key_protector/module.c
> > index 74e79a545..827f946ba 100644
> > --- a/grub-core/commands/tpm2_key_protector/module.c
> > +++ b/grub-core/commands/tpm2_key_protector/module.c
> > @@ -790,7 +790,7 @@ tpm2_protector_simple_policy_seq (const 
> > tpm2_protector_context_t *ctx,
> >
> >  static grub_err_t
> >  tpm2_protector_unseal (tpm2key_policy_t policy_seq, TPM_HANDLE_t 
> > sealed_handle,
> > -                  grub_uint8_t **key, grub_size_t *key_size)
> > +                  grub_uint8_t **key, grub_size_t *key_size, bool 
> > *dump_pcr)
> >  {
> >    TPMS_AUTH_COMMAND_t authCmd = {0};
> >    TPM2B_SENSITIVE_DATA_t data;
> > @@ -801,6 +801,8 @@ tpm2_protector_unseal (tpm2key_policy_t policy_seq, 
> > TPM_HANDLE_t sealed_handle,
> >    TPM_RC_t rc;
> >    grub_err_t err;
> >
> > +  *dump_pcr = false;
> > +
> >    /* Start Auth Session */
> >    nonceCaller.size = TPM_SHA256_DIGEST_SIZE;
> >    symmetric.algorithm = TPM_ALG_NULL;
> > @@ -820,6 +822,13 @@ tpm2_protector_unseal (tpm2key_policy_t policy_seq, 
> > TPM_HANDLE_t sealed_handle,
> >    rc = grub_tpm2_unseal (sealed_handle, &authCmd, &data, NULL);
> >    if (rc != TPM_RC_SUCCESS)
> >      {
> > +      /*
> > +       * Trigger PCR dump on policy fail
> > +       * TPM_RC_S (0x800) | TPM_RC_1 (0x100) | RC_FMT (0x80) | 
> > TPM_RC_POLICY_FAIL (0x1D)
> > +       */
> > +      if (rc == 0x99D)
> > +   *dump_pcr = true;
> > +
> >        err = grub_error (GRUB_ERR_BAD_DEVICE, "failed to unseal sealed key 
> > (TPM2_Unseal: 0x%x)", rc);
> >        goto error;
> >      }
> > @@ -845,6 +854,80 @@ tpm2_protector_unseal (tpm2key_policy_t policy_seq, 
> > TPM_HANDLE_t sealed_handle,
> >    return err;
> >  }
> >
> > +#define TPM_PCR_STR_SIZE (sizeof (TPMU_HA_t) * 2 + 1)
> > +
> > +static void
> > +tpm2_protector_get_pcr_str (const TPM_ALG_ID_t algo, grub_uint32_t index, 
> > char *pcr_str, grub_uint16_t buf_size)
> > +{
> > +  TPML_PCR_SELECTION_t pcr_sel = {
> > +    .count = 1,
> > +    .pcrSelections = {
> > +      {
> > +   .hash = algo,
> > +   .sizeOfSelect = 3,
> > +   .pcrSelect = {0}
> > +      },
> > +    }
> > +  };
> > +  TPML_DIGEST_t digest = {0};
> > +  grub_uint16_t i;
> > +  TPM_RC_t rc;
> > +
> > +  if (buf_size < TPM_PCR_STR_SIZE)
> > +    {
> > +      grub_snprintf (pcr_str, buf_size, "insufficient buffer");
> > +      return;
> > +    }
> > +
> > +  TPMS_PCR_SELECTION_SelectPCR (&pcr_sel.pcrSelections[0], index);
> > +
> > +  rc = grub_tpm2_pcr_read (NULL, &pcr_sel, NULL, NULL, &digest, NULL);
> > +  if (rc != TPM_RC_SUCCESS)
> > +    {
> > +      grub_snprintf (pcr_str, buf_size, "TPM2_PCR_Read: 0x%x", rc);
> > +      return;
> > +    }
> > +
> > +  /* Check the returned digest number and size */
> > +  if (digest.count != 1 || digest.digests[0].size > sizeof (TPMU_HA_t))
> > +    {
> > +      grub_snprintf (pcr_str, buf_size, "invalid digest");
> > +      return;
> > +    }
> > +
> > +  /* Print the digest to the buffer */
> > +  for (i = 0; i < digest.digests[0].size; i++)
> > +    grub_snprintf (pcr_str + 2 * i, buf_size - 2 * i, "%02x", 
> > digest.digests[0].buffer[i]);
> > +}
> > +
> > +static void
> > +tpm2_protector_dump_pcr (const tpm2_protector_context_t *ctx)
> > +{
> > +  const char *algo_name;
> > +  char pcr_str[TPM_PCR_STR_SIZE];
> > +  grub_uint8_t i;
> > +
> > +  if (ctx->bank == TPM_ALG_SHA1)
> > +    algo_name = "sha1";
> > +  else if (ctx->bank == TPM_ALG_SHA256)
> > +    algo_name = "sha256";
> > +  else if (ctx->bank == TPM_ALG_SHA384)
> > +    algo_name = "sha384";
> > +  else if (ctx->bank == TPM_ALG_SHA512)
> > +    algo_name = "sha512";
> > +  else
> > +    algo_name = "other";
> > +
> > +  grub_printf ("PCR Mismatching! Check firmware and bootloader before 
> > typing passphrase!\nTPM PCR [%s]:\n", algo_name);
> 
> This grub_printf() does not belong to this function. It should
> go to the tpm2_protector_unseal().
> 
There may be several policies in the policy sequence, so I try to
suppress the message until all policies fail.

> s/PCR Mismatching/PCR Mismatch/
> 
Will fix it in v3.

> > +  /* Print PCR 0~15 to cover Static Root of Trust Measurement (SRTM) */
> > +  for (i = 0; i <= 15; i++)
> 
> I would dump all PCRs including DRTM ones.
> 
Okay, will make it dump all PCRs.

> > +    {
> > +      tpm2_protector_get_pcr_str (ctx->bank, i, pcr_str, sizeof (pcr_str));
> > +      grub_printf ("  %02d: %s\n", i, pcr_str);
> > +    }
> > +}
> > +
> >  static grub_err_t
> >  tpm2_protector_srk_recover (const tpm2_protector_context_t *ctx,
> >                         grub_uint8_t **key, grub_size_t *key_size)
> > @@ -859,6 +942,7 @@ tpm2_protector_srk_recover (const 
> > tpm2_protector_context_t *ctx,
> >    tpm2key_policy_t policy_seq = NULL;
> >    tpm2key_authpolicy_t authpol = NULL;
> >    tpm2key_authpolicy_t authpol_seq = NULL;
> > +  bool dump_pcr = false;
> >    grub_err_t err;
> >
> >    /*
> > @@ -924,7 +1008,7 @@ tpm2_protector_srk_recover (const 
> > tpm2_protector_context_t *ctx,
> >    /* Iterate the authpolicy sequence to find one that unseals the key */
> >    FOR_LIST_ELEMENTS (authpol, authpol_seq)
> >      {
> > -      err = tpm2_protector_unseal (authpol->policy_seq, sealed_handle, 
> > key, key_size);
> > +      err = tpm2_protector_unseal (authpol->policy_seq, sealed_handle, 
> > key, key_size, &dump_pcr);
> >        if (err == GRUB_ERR_NONE)
> >          break;
> >
> > @@ -952,13 +1036,17 @@ tpm2_protector_srk_recover (const 
> > tpm2_protector_context_t *ctx,
> >         goto exit2;
> >     }
> >
> > -      err = tpm2_protector_unseal (policy_seq, sealed_handle, key, 
> > key_size);
> > +      err = tpm2_protector_unseal (policy_seq, sealed_handle, key, 
> > key_size, &dump_pcr);
> >      }
> >
> >    /* Pop error messages on success */
> >    if (err == GRUB_ERR_NONE)
> >      while (grub_error_pop ());
> >
> > +  /* Dump PCRs if necessary */
> > +  if (dump_pcr)
> 
> Even if it works I think we should use constants if they are defined. So,
>   if (dump_pcr == true)
>
Will fix it in v3.
 
Gary Lin

> > +    tpm2_protector_dump_pcr (ctx);
> > +
> >   exit2:
> >    grub_tpm2_flushcontext (sealed_handle);
> >
> > @@ -978,6 +1066,7 @@ tpm2_protector_nv_recover (const 
> > tpm2_protector_context_t *ctx,
> >  {
> >    TPM_HANDLE_t sealed_handle = ctx->nv;
> >    tpm2key_policy_t policy_seq = NULL;
> > +  bool dump_pcr = false;
> >    grub_err_t err;
> >
> >    /* Create a basic policy sequence based on the given PCR selection */
> > @@ -985,7 +1074,11 @@ tpm2_protector_nv_recover (const 
> > tpm2_protector_context_t *ctx,
> >    if (err != GRUB_ERR_NONE)
> >      goto exit;
> >
> > -  err = tpm2_protector_unseal (policy_seq, sealed_handle, key, key_size);
> > +  err = tpm2_protector_unseal (policy_seq, sealed_handle, key, key_size, 
> > &dump_pcr);
> > +
> > +  /* Dump PCRs if necessary */
> > +  if (dump_pcr)
> 
> Ditto.
> 
> > +    tpm2_protector_dump_pcr (ctx);
> >
> >   exit:
> >    grub_tpm2_flushcontext (sealed_handle);
> 
> Daniel