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 <[email protected]>
> >
> > 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 <[email protected]>
> > Signed-off-by: Hernan Gatta <[email protected]>
> > Signed-off-by: Gary Lin <[email protected]>
> > ---
> > .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;
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
[email protected]
https://lists.gnu.org/mailman/listinfo/grub-devel
