On Fri, Oct 25, 2024 at 03:35:11PM +0800, Gary Lin wrote:
> On Thu, Oct 24, 2024 at 05:11:00PM +0200, Daniel Kiper wrote:
> > On Mon, Oct 21, 2024 at 04:07:03PM +0800, Gary Lin wrote:
> > > From: Hernan Gatta <hega...@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/efi/grub/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/efi/grub/sealed.key
> > >
> > > Then, in the boot script, for TPM 2.0 Key File:
> > >
> > > tpm2_key_protector_init --tpm2key=(hd0,gpt1)/efi/grub/sealed.tpm
> > > cryptomount -u <SDB1_UUID> -P tpm2
> > >
> > > Or, for the raw sealed key:
> > >
> > > tpm2_key_protector_init --keyfile=(hd0,gpt1)/efi/grub/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.
> > >
> > > Cc: Stefan Berger <stef...@linux.ibm.com>
> > > Signed-off-by: Hernan Gatta <hega...@linux.microsoft.com>
> > > Signed-off-by: Gary Lin <g...@suse.com>
> > > ---
> > > .gitignore | 2 +
> > > Makefile.util.def | 26 +
> > > configure.ac | 30 +
> > > docs/man/grub-protect.h2m | 4 +
> > > util/grub-protect.c | 1394 +++++++++++++++++++++++++++++++++++++
> > > 5 files changed, 1456 insertions(+)
> > > create mode 100644 docs/man/grub-protect.h2m
> > > create mode 100644 util/grub-protect.c
> >
> > [...]
> >
> > > +static grub_err_t
> > > +protect_write_file (const char *filepath, void *buffer, size_t
> > > buffer_size)
> > > +{
> > > + grub_err_t err;
> > > + FILE *f;
> > > +
> > > + f = fopen (filepath, "wb");
> > > + if (f == NULL)
> > > + return GRUB_ERR_FILE_NOT_FOUND;
> > > +
> > > + if (fwrite (buffer, buffer_size, 1, f) != 1)
> > > + {
> > > + err = GRUB_ERR_WRITE_ERROR;
> > > + goto exit;
> > > + }
> > > +
> > > + err = GRUB_ERR_NONE;
> > > +
> > > + exit:
> > > + fclose (f);
> > > +
> > > + return err;
> > > +}
> > > +
> > > +grub_err_t
> > > +grub_tcg2_get_max_output_size (grub_size_t *size)
> > > +{
> > > + if (size == NULL)
> > > + return GRUB_ERR_BAD_ARGUMENT;
> > > +
> > > + *size = GRUB_TPM2_BUFFER_CAPACITY;
> > > +
> > > + return GRUB_ERR_NONE;
> > > +}
> > > +
> > > +grub_err_t
> > > +grub_tcg2_submit_command (grub_size_t input_size, grub_uint8_t *input,
> > > + grub_size_t output_size, grub_uint8_t *output)
> > > +{
> > > + static const grub_size_t header_size = sizeof (grub_uint16_t) +
> > > + (2 * sizeof(grub_uint32_t));
> >
> > AIUI this is somehow related to the TPM structs defined elsewhere. So,
> > I think these structs should have defined header as a separate struct
> > (probably preferred). Then you could use, e.g., sizeof (tpm_header) and be
> > done. Or you can define a constant next to TPM structs as you do here.
> > Of course it would be nice if the constant has relevant name and/or
> > a comment...
> >
> I'll define a TPM command header in tss2_structs.h like this:
>
> struct TPM_COMMAND_HEADER
> {
> grub_uint16_t tag;
> grub_uint32_t command_size;
> TPM_CC_t command_code;
> };
> typedef struct TPM_COMMAND_HEADER TPM_COMMAND_HEADER_t;
>
Correction:
The header size check is for the response, so the struct is actually
TPM_RESPONSE_HEADER:
struct TPM_RESPONSE_HEADER
{
grub_uint16_t tag;
grub_uint32_t response_size;
TPM_RC_t response_code;
};
typedef struct TPM_RESPONSE_HEADER TPM_RESPONSE_HEADER_t;
Gary Lin
> This would make the code more comprehensive.
>
>
> > > + if (write (protector_tpm2_fd, input, input_size) != input_size)
> > > + return GRUB_ERR_BAD_DEVICE;
> > > +
> > > + if (read (protector_tpm2_fd, output, output_size) < header_size)
> > > + return GRUB_ERR_BAD_DEVICE;
> > > +
> > > + return GRUB_ERR_NONE;
> > > +}
> >
> > [...]
> >
> > > +static grub_err_t
> > > +protect_tpm2_export_tpm2key (const protect_args_t *args,
> > > + tpm2_sealed_key_t *sealed_key)
> > > +{
> > > + const char *sealed_key_oid = "2.23.133.10.1.5";
> >
> > Please name this OID in a comment or define properly named constant.
> >
> The OID is from the TPM 2.0 Key File spec. I'll add a '#define' for it
> and a proper comment.
>
> > > + asn1_node asn1_def = NULL;
> > > + asn1_node tpm2key = NULL;
> > > + grub_uint32_t parent;
> > > + grub_uint32_t cmd_code;
> > > + struct grub_tpm2_buffer pol_buf;
> > > + TPML_PCR_SELECTION_t pcr_sel = {
> > > + .count = 1,
> > > + .pcrSelections = {
> > > + {
> > > + .hash = args->tpm2_bank,
> > > + .sizeOfSelect = 3,
> > > + .pcrSelect = {0}
> > > + },
> > > + }
> > > + };
> > > + struct grub_tpm2_buffer pub_buf;
> > > + struct grub_tpm2_buffer priv_buf;
> > > + void *der_buf = NULL;
> > > + int der_buf_size = 0;
> > > + int i;
> > > + int ret;
> > > + grub_err_t err;
> > > +
> > > + for (i = 0; i < args->tpm2_pcr_count; i++)
> > > + TPMS_PCR_SELECTION_SelectPCR (&pcr_sel.pcrSelections[0],
> > > args->tpm2_pcrs[i]);
> > > +
> > > + /*
> > > + * Prepare the parameters for TPM_CC_PolicyPCR:
> > > + * empty pcrDigest and the user selected PCRs
> > > + */
> > > + grub_tpm2_buffer_init (&pol_buf);
> > > + grub_tpm2_buffer_pack_u16 (&pol_buf, 0);
> > > + grub_Tss2_MU_TPML_PCR_SELECTION_Marshal (&pol_buf, &pcr_sel);
> > > +
> > > + grub_tpm2_buffer_init (&pub_buf);
> > > + grub_Tss2_MU_TPM2B_PUBLIC_Marshal (&pub_buf, &sealed_key->public);
> > > + grub_tpm2_buffer_init (&priv_buf);
> > > + grub_Tss2_MU_TPM2B_Marshal (&priv_buf, sealed_key->private.size,
> > > + sealed_key->private.buffer);
> > > + if (pub_buf.error != 0 || priv_buf.error != 0)
> > > + return GRUB_ERR_BAD_ARGUMENT;
> > > +
> > > + ret = asn1_array2tree (tpm2key_asn1_tab, &asn1_def, NULL);
> > > + if (ret != ASN1_SUCCESS)
> > > + return GRUB_ERR_BAD_ARGUMENT;
> > > +
> > > + ret = asn1_create_element (asn1_def, "TPM2KEY.TPMKey" , &tpm2key);
> > > + if (ret != ASN1_SUCCESS)
> > > + return GRUB_ERR_BAD_ARGUMENT;
> > > +
> > > + /* Set 'type' to "sealed key" */
> > > + ret = asn1_write_value (tpm2key, "type", sealed_key_oid, 1);
> > > + if (ret != ASN1_SUCCESS)
> > > + {
> > > + fprintf (stderr, "Failed to set 'type': 0x%u\n", ret);
> > > + err = GRUB_ERR_BAD_ARGUMENT;
> > > + goto error;
> > > + }
> > > +
> > > + /* Set 'emptyAuth' to TRUE */
> > > + ret = asn1_write_value (tpm2key, "emptyAuth", "TRUE", 1);
> > > + if (ret != ASN1_SUCCESS)
> > > + {
> > > + fprintf (stderr, "Failed to set 'emptyAuth': 0x%x\n", ret);
> > > + err = GRUB_ERR_BAD_ARGUMENT;
> > > + goto error;
> > > + }
> > > +
> > > + /* Set 'policy' */
> > > + ret = asn1_write_value (tpm2key, "policy", "NEW", 1);
> > > + if (ret != ASN1_SUCCESS)
> > > + {
> > > + fprintf (stderr, "Failed to set 'policy': 0x%x\n", ret);
> > > + err = GRUB_ERR_BAD_ARGUMENT;
> > > + goto error;
> > > + }
> > > + cmd_code = grub_cpu_to_be32 (TPM_CC_PolicyPCR);
> > > + ret = asn1_write_value (tpm2key, "policy.?LAST.CommandCode", &cmd_code,
> > > + sizeof (cmd_code));
> > > + if (ret != ASN1_SUCCESS)
> > > + {
> > > + fprintf (stderr, "Failed to set 'policy CommandCode': 0x%x\n",
> > > ret);
> > > + err = GRUB_ERR_BAD_ARGUMENT;
> > > + goto error;
> > > + }
> > > + ret = asn1_write_value (tpm2key, "policy.?LAST.CommandPolicy",
> > > &pol_buf.data,
> > > + pol_buf.size);
> > > + if (ret != ASN1_SUCCESS)
> > > + {
> > > + fprintf (stderr, "Failed to set 'policy CommandPolicy': 0x%x\n",
> > > ret);
> > > + err = GRUB_ERR_BAD_ARGUMENT;
> > > + goto error;
> > > + }
> > > +
> > > + /* Remove 'secret' */
> > > + ret = asn1_write_value (tpm2key, "secret", NULL, 0);
> > > + if (ret != ASN1_SUCCESS)
> > > + {
> > > + fprintf (stderr, "Failed to remove 'secret': 0x%x\n", ret);
> > > + err = GRUB_ERR_BAD_ARGUMENT;
> > > + goto error;
> > > + }
> > > +
> > > + /* Remove 'authPolicy' */
> > > + ret = asn1_write_value (tpm2key, "authPolicy", NULL, 0);
> > > + if (ret != ASN1_SUCCESS)
> > > + {
> > > + fprintf (stderr, "Failed to remove 'authPolicy': 0x%x\n", ret);
> > > + err = GRUB_ERR_BAD_ARGUMENT;
> > > + goto error;
> > > + }
> > > +
> > > + /* Remove 'description' */
> > > + ret = asn1_write_value (tpm2key, "description", NULL, 0);
> > > + if (ret != ASN1_SUCCESS)
> > > + {
> > > + fprintf (stderr, "Failed to remove 'description': 0x%x\n", ret);
> > > + err = GRUB_ERR_BAD_ARGUMENT;
> > > + goto error;
> > > + }
> > > +
> > > + /*
> > > + * Use the SRK handle as the parent handle if specified
> > > + * Otherwise, Use TPM_RH_OWNER as the default parent handle
> > > + */
> > > + if (args->tpm2_srk != 0)
> > > + parent = grub_cpu_to_be32 (args->tpm2_srk);
> > > + else
> > > + parent = grub_cpu_to_be32 (TPM_RH_OWNER);
> > > + ret = asn1_write_value (tpm2key, "parent", &parent, sizeof (parent));
> > > + if (ret != ASN1_SUCCESS)
> > > + {
> > > + fprintf (stderr, "Failed to set 'parent': 0x%x\n", ret);
> > > + err = GRUB_ERR_BAD_ARGUMENT;
> > > + goto error;
> > > + }
> > > +
> > > + /*
> > > + * Set 'rsaParent' to TRUE if the RSA SRK is specified and the SRK
> > > + * handle is not persistent. Otherwise, remove 'rsaParent'.
> > > + */
> > > + if (args->tpm2_srk == 0 && args->srk_type.type == TPM_ALG_RSA)
> > > + ret = asn1_write_value (tpm2key, "rsaParent", "TRUE", 1);
> > > + else
> > > + ret = asn1_write_value (tpm2key, "rsaParent", NULL, 0);
> > > +
> > > + if (ret != ASN1_SUCCESS)
> > > + {
> > > + fprintf (stderr, "Failed to set 'rsaParent': 0x%x\n", ret);
> > > + err = GRUB_ERR_BAD_ARGUMENT;
> > > + goto error;
> > > + }
> > > +
> > > + /* Set the pubkey */
> > > + ret = asn1_write_value (tpm2key, "pubkey", pub_buf.data, pub_buf.size);
> > > + if (ret != ASN1_SUCCESS)
> > > + {
> > > + fprintf (stderr, "Failed to set 'pubkey': 0x%x\n", ret);
> > > + err = GRUB_ERR_BAD_ARGUMENT;
> > > + goto error;
> > > + }
> > > +
> > > + /* Set the privkey */
> > > + ret = asn1_write_value (tpm2key, "privkey", priv_buf.data,
> > > priv_buf.size);
> > > + if (ret != ASN1_SUCCESS)
> > > + {
> > > + fprintf (stderr, "Failed to set 'privkey': 0x%x\n", ret);
> > > + err = GRUB_ERR_BAD_ARGUMENT;
> > > + goto error;
> > > + }
> > > +
> > > + /* Create the DER binary */
> > > + der_buf_size = 0;
> > > + ret = asn1_der_coding (tpm2key, "", NULL, &der_buf_size, NULL);
> > > + if (ret != ASN1_MEM_ERROR)
> > > + {
> > > + fprintf (stderr, "Failed to get DER size: 0x%x\n", ret);
> > > + err = GRUB_ERR_BAD_ARGUMENT;
> > > + goto error;
> > > + }
> > > +
> > > + der_buf = grub_malloc (der_buf_size);
> > > + if (der_buf == NULL)
> > > + {
> > > + fprintf (stderr, "Failed to allocate memory for DER encoding\n");
> > > + err = GRUB_ERR_OUT_OF_MEMORY;
> > > + goto error;
> > > + }
> > > +
> > > + ret = asn1_der_coding (tpm2key, "", der_buf, &der_buf_size, NULL);
> > > + if (ret != ASN1_SUCCESS)
> > > + {
> > > + fprintf (stderr, "DER coding error: 0x%x\n", ret);
> > > + err = GRUB_ERR_BAD_ARGUMENT;
> > > + goto error;
> > > + }
> > > +
> > > + err = protect_write_file (args->tpm2_outfile, der_buf, der_buf_size);
> > > + if (err != GRUB_ERR_NONE)
> > > + fprintf (stderr, "Could not write tpm2key file (Error: %u).\n",
> > > errno);
> > > +
> > > + error:
> > > + grub_free (der_buf);
> > > +
> > > + if (tpm2key)
> > > + asn1_delete_structure (&tpm2key);
> > > +
> > > + return err;
> > > +}
> >
> > [...]
> >
> > > +static grub_err_t
> > > +protect_tpm2_args_verify (protect_args_t *args)
> > > +{
> > > + switch (args->action)
> > > + {
> > > + case PROTECT_ACTION_ADD:
> > > + if (args->args & PROTECT_ARG_TPM2_EVICT)
> > > + {
> > > + fprintf (stderr, N_("--tpm2-evict is invalid when --action is
> > > 'add'.\n"));
> > > + return GRUB_ERR_BAD_ARGUMENT;
> > > + }
> > > +
> > > + if (args->tpm2_keyfile == NULL)
> > > + {
> > > + fprintf (stderr, N_("--tpm2-keyfile must be specified.\n"));
> > > + return GRUB_ERR_BAD_ARGUMENT;
> > > + }
> > > +
> > > + if (args->tpm2_outfile == NULL)
> > > + {
> > > + fprintf (stderr, N_("--tpm2-outfile must be specified.\n"));
> > > + return GRUB_ERR_BAD_ARGUMENT;
> > > + }
> > > +
> > > + if (args->tpm2_device == NULL)
> > > + args->tpm2_device = "/dev/tpm0";
> >
> > When you move this "if" to the beginning of function or its end then you
> > can do this check once and drop one below...
> >
> Got it.
>
> > > + if (args->tpm2_pcr_count == 0)
> > > + {
> > > + args->tpm2_pcrs[0] = 7;
> > > + args->tpm2_pcr_count = 1;
> > > + }
> > > +
> > > + if (args->srk_type.type == TPM_ALG_ERROR)
> > > + {
> > > + args->srk_type.type = TPM_ALG_ECC;
> > > + args->srk_type.detail.ecc_curve = TPM_ECC_NIST_P256;
> > > + }
> > > +
> > > + if (args->tpm2_bank == TPM_ALG_ERROR)
> > > + args->tpm2_bank = TPM_ALG_SHA256;
> > > +
> > > + break;
> > > +
> > > + case PROTECT_ACTION_REMOVE:
> > > + if (args->args & PROTECT_ARG_TPM2_ASYMMETRIC)
> > > + {
> > > + fprintf (stderr, N_("--tpm2-asymmetric is invalid when --action is
> > > 'remove'.\n"));
> > > + return GRUB_ERR_BAD_ARGUMENT;
> > > + }
> > > +
> > > + if (args->args & PROTECT_ARG_TPM2_BANK)
> > > + {
> > > + fprintf (stderr, N_("--tpm2-bank is invalid when --action is
> > > 'remove'.\n"));
> > > + return GRUB_ERR_BAD_ARGUMENT;
> > > + }
> > > +
> > > + if (args->args & PROTECT_ARG_TPM2_KEYFILE)
> > > + {
> > > + fprintf (stderr, N_("--tpm2-keyfile is invalid when --action is
> > > 'remove'.\n"));
> > > + return GRUB_ERR_BAD_ARGUMENT;
> > > + }
> > > +
> > > + if (args->args & PROTECT_ARG_TPM2_OUTFILE)
> > > + {
> > > + fprintf (stderr, N_("--tpm2-outfile is invalid when --action is
> > > 'remove'.\n"));
> > > + return GRUB_ERR_BAD_ARGUMENT;
> > > + }
> > > +
> > > + if (args->args & PROTECT_ARG_TPM2_PCRS)
> > > + {
> > > + fprintf (stderr, N_("--tpm2-pcrs is invalid when --action is
> > > 'remove'.\n"));
> > > + return GRUB_ERR_BAD_ARGUMENT;
> > > + }
> > > +
> > > + if (args->tpm2_srk == 0)
> > > + {
> > > + fprintf (stderr, N_("--tpm2-srk is not specified when --action is
> > > 'remove'.\n"));
> > > + return GRUB_ERR_BAD_ARGUMENT;
> > > + }
> > > +
> > > + if (args->tpm2_device == NULL)
> > > + args->tpm2_device = "/dev/tpm0";
> >
> > ... I mean from here...
> >
> > > + break;
> > > +
> > > + default:
> > > + fprintf (stderr, N_("The TPM2 key protector only supports the
> > > following actions: add, remove.\n"));
> > > + return GRUB_ERR_BAD_ARGUMENT;
> > > + }
> > > +
> > > + return GRUB_ERR_NONE;
> > > +}
> >
> > [...]
> >
> > > +static grub_err_t
> > > +protect_args_verify (protect_args_t *args)
> > > +{
> > > + if (args->action == PROTECT_ACTION_ERROR)
> > > + {
> > > + fprintf (stderr, N_("--action is mandatory.\n"));
> > > + return GRUB_ERR_BAD_ARGUMENT;
> > > + }
> > > +
> > > + /* At the moment, the only configurable key protector is the TPM2 one,
> > > so it
> > > + * is the only key protector supported by this tool. */
> >
> > Wrong coding style for the comment...
> >
> Will fix it in the next version.
>
> > > + if (args->protector != PROTECT_TYPE_TPM2)
> > > + {
> > > + fprintf (stderr, N_("--protector is mandatory and only 'tpm2' is
> > > currently supported.\n"));
> > > + return GRUB_ERR_BAD_ARGUMENT;
> > > + }
> > > +
> > > + switch (args->protector)
> > > + {
> > > + case PROTECT_TYPE_TPM2:
> > > + return protect_tpm2_args_verify (args);
> > > + default:
> > > + return GRUB_ERR_BAD_ARGUMENT;
> > > + }
> > > +
> > > + return GRUB_ERR_NONE;
> > > +}
> >
> > [...]
> >
> > > +int
> > > +main (int argc, char *argv[])
> > > +{
> > > + grub_err_t err;
> >
> > s/grub_err_t/int/
> >
> > > + protect_args_t args = {0};
> > > +
> > > + if (argp_parse (&protect_argp, argc, argv, 0, 0, &args) != 0)
> > > + {
> > > + fprintf (stderr, N_("Could not parse arguments.\n"));
> > > + return GRUB_ERR_BAD_ARGUMENT;
> >
> > You expose GRUB internals to the user space and mix types. There is no
> > guarantee GRUB_ERR_BAD_ARGUMENT value will not change in the future.
> > So, I think you should return EXIT_FAILURE here.
> >
> Will fix it in the next version.
>
> > > + }
> > > +
> > > + protect_init (&argc, &argv);
> > > +
> > > + err = protect_args_verify (&args);
> >
> > Ditto... The EXIT_SUCCESS and EXIT_FAILURE are your friends...
> >
> > > + if (err != GRUB_ERR_NONE)
> > > + goto exit;
> > > +
> > > + err = protect_dispatch (&args);
> >
> > Ditto...
> >
> > I did not check other patches but if you do the same thing elsewhere
> > please fix it.
> >
> Will check other patches.
>
> > > + if (err != GRUB_ERR_NONE)
> > > + goto exit;
> > > +
> > > + exit:
> > > + protect_fini ();
> > > +
>
> > > + return err;
> I would like to handle 'err' like this:
>
> if (err != GRUB_ERR_NONE)
> return EXIT_FAILURE;
>
> return EXIT_SUCCESS;
>
>
> Gary Lin
>
> > > +}
> >
> > Daniel
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
