[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v17 10/20] tpm2: Add TPM Software Stack (TSS)
|
From: |
Gary Lin |
|
Subject: |
Re: [PATCH v17 10/20] tpm2: Add TPM Software Stack (TSS) |
|
Date: |
Wed, 19 Jun 2024 14:41:13 +0800 |
On Tue, Jun 18, 2024 at 03:30:03PM +0200, Daniel Kiper wrote:
> On Fri, Jun 14, 2024 at 02:45:43PM +0800, Gary Lin wrote:
> > From: Hernan Gatta <hegatta@linux.microsoft.com>
> >
> > A Trusted Platform Module (TPM) Software Stack (TSS) provides logic to
> > compose and submit TPM commands and parse reponses.
> >
> > A limited number of TPM commands may be accessed via the EFI TCG2
> > protocol. This protocol exposes functionality that is primarily geared
> > toward TPM usage within the context of Secure Boot. For all other TPM
> > commands, however, such as sealing and unsealing, this protocol does not
> > provide any help, with the exception of passthrough command submission.
> >
> > The SubmitCommand method allows a caller to send raw commands to the
> > system's TPM and to receive the corresponding response. These
> > command/response pairs are formatted using the TPM wire protocol. To
> > construct commands in this way, and to parse the TPM's response, it is
> > necessary to, first, possess knowledge of the various TPM structures, and,
> > second, of the TPM wire protocol itself.
> >
> > As such, this patch includes a set of header files that define the
> > necessary TPM structures and TSS functions, implementations of various
> > TPM2_* functions (inventoried below), and logic to write and read command
> > and response buffers, respectively, using the TPM wire protocol.
> >
> > Functions: TPM2_Create, TPM2_CreatePrimary, TPM2_EvictControl,
> > TPM2_FlushContext, TPM2_Load, TPM2_PCR_Read, TPM2_PolicyGetDigest,
> > TPM2_PolicyPCR, TPM2_ReadPublic, TPM2_StartAuthSession, TPM2_Unseal,
> > TPM2_LoadExternal, TPM2_Hash, TPM2_VerifySignature,
> > TPM2_PolicyAuthorize, TPM2_TestParms
> >
> > Signed-off-by: Hernan Gatta <hegatta@linux.microsoft.com>
> > Signed-off-by: Gary Lin <glin@suse.com>
> > Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
> > ---
> > grub-core/tpm2/buffer.c | 145 +++
> > grub-core/tpm2/mu.c | 1168 ++++++++++++++++++++++++
> > grub-core/tpm2/tcg2.c | 143 +++
> > grub-core/tpm2/tpm2.c | 1048 +++++++++++++++++++++
> > include/grub/tpm2/buffer.h | 65 ++
> > include/grub/tpm2/internal/functions.h | 156 ++++
> > include/grub/tpm2/internal/structs.h | 768 ++++++++++++++++
> > include/grub/tpm2/internal/types.h | 403 ++++++++
> > include/grub/tpm2/mu.h | 396 ++++++++
> > include/grub/tpm2/tcg2.h | 34 +
> > include/grub/tpm2/tpm2.h | 34 +
> > 11 files changed, 4360 insertions(+)
> > create mode 100644 grub-core/tpm2/buffer.c
> > create mode 100644 grub-core/tpm2/mu.c
> > create mode 100644 grub-core/tpm2/tcg2.c
> > create mode 100644 grub-core/tpm2/tpm2.c
> > create mode 100644 include/grub/tpm2/buffer.h
> > create mode 100644 include/grub/tpm2/internal/functions.h
> > create mode 100644 include/grub/tpm2/internal/structs.h
> > create mode 100644 include/grub/tpm2/internal/types.h
> > create mode 100644 include/grub/tpm2/mu.h
> > create mode 100644 include/grub/tpm2/tcg2.h
> > create mode 100644 include/grub/tpm2/tpm2.h
> >
> > diff --git a/grub-core/tpm2/buffer.c b/grub-core/tpm2/buffer.c
> > new file mode 100644
> > index 000000000..cb9f29497
> > --- /dev/null
> > +++ b/grub-core/tpm2/buffer.c
>
> I think this together with other TPM2 driver files should go to the
> grub-core/commands/efi/tpm2 directory.
>
The TPM2 stack is not EFI only. The only EFI related code is in
grub-core/tpm2/tcg2.c which mainly implements how the TPM2 commands to
be submitted. I'd propose to move them to grub-core/commands/tpm2 and
rename tcg2.c to tcg2-efi.c.
> > @@ -0,0 +1,145 @@
> > +/*
> > + * GRUB -- GRand Unified Bootloader
> > + * Copyright (C) 2022 Microsoft Corporation
> > + *
> > + * GRUB is free software: you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation, either version 3 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * GRUB is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with GRUB. If not, see <http://www.gnu.org/licenses/>.
> > + */
> > +
> > +#include <grub/misc.h>
> > +#include <grub/tpm2/buffer.h>
> > +
> > +void grub_tpm2_buffer_init (grub_tpm2_buffer_t buffer)
> > +{
> > + grub_memset (buffer->data, 0xDD, sizeof (buffer->data));
>
> If you init the buffer->data with 0xDD instead of 0 then it begs for
> a comment. And s/0xDD/0xdd/...
>
It should be 0. I'll fix in v18.
> > + buffer->size = 0;
> > + buffer->offset = 0;
> > + buffer->cap = sizeof (buffer->data);
> > + buffer->error = 0;
> > +}
> > +
> > +void
> > +grub_tpm2_buffer_pack (grub_tpm2_buffer_t buffer, const void* data,
> > + grub_size_t size)
> > +{
> > + grub_uint32_t r = buffer->cap - buffer->size;
> > +
> > + if (buffer->error)
> > + return;
> > +
> > + if (size > r)
> > + {
> > + buffer->error = 1;
> > + return;
> > + }
> > +
> > + grub_memcpy (&buffer->data[buffer->size], (void*) data, size);
> > + buffer->size += size;
> > +}
> > +
> > +void
> > +grub_tpm2_buffer_pack_u8 (grub_tpm2_buffer_t buffer, grub_uint8_t value)
> > +{
> > + grub_tpm2_buffer_pack (buffer, (const char*) &value, sizeof (value));
> > +}
> > +
> > +void
> > +grub_tpm2_buffer_pack_u16 (grub_tpm2_buffer_t buffer, grub_uint16_t value)
> > +{
> > + grub_uint16_t tmp = grub_cpu_to_be16 (value);
>
> I would add an empty line here.
>
> > + grub_tpm2_buffer_pack (buffer, (const char*) &tmp, sizeof (tmp));
> > +}
> > +
> > +void
> > +grub_tpm2_buffer_pack_u32 (grub_tpm2_buffer_t buffer, grub_uint32_t value)
> > +{
> > + grub_uint32_t tmp = grub_cpu_to_be32 (value);
>
> Ditto and below please if needed...
>
I'll go through the whole patch and fix the similar issue.
> > + grub_tpm2_buffer_pack (buffer, (const char*) &tmp, sizeof (tmp));
> > +}
>
> [...]
>
> > diff --git a/grub-core/tpm2/mu.c b/grub-core/tpm2/mu.c
> > new file mode 100644
> > index 000000000..10ed71c04
> > --- /dev/null
> > +++ b/grub-core/tpm2/mu.c
>
> I can imagine where it comes from but I think it should be efi.c instead
> of mu.c.
>
No, it's not from the MU firmware but stands for Marshal/Unmarshal.
The similar naming policy from tpm2-tss:
https://github.com/tpm2-software/tpm2-tss/blob/master/include/tss2/tss2_mu.h
> > @@ -0,0 +1,1168 @@
> > +/*
> > + * GRUB -- GRand Unified Bootloader
> > + * Copyright (C) 2022 Microsoft Corporation
> > + *
> > + * GRUB is free software: you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation, either version 3 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * GRUB is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with GRUB. If not, see <http://www.gnu.org/licenses/>.
> > + */
> > +
> > +#include <grub/misc.h>
> > +#include <grub/tpm2/mu.h>
> > +
> > +void
> > +grub_tpm2_mu_TPMS_AUTH_COMMAND_Marshal (grub_tpm2_buffer_t buffer,
> > + const TPMS_AUTH_COMMAND* authCommand)
>
> s/grub_tpm2_mu_TPMS_AUTH_COMMAND_Marshal/grub_efi_tpm2_TPMS_AUTH_COMMAND_Marshal/
>
> I would change "grub_tpm2_mu_" prefix to "grub_efi_tpm2_" everywhere.
>
As mentioned above, it's not EFI related, so the renaming is not
necessary.
> > +{
> > + grub_uint32_t start;
> > + grub_uint32_t tmp;
> > +
> > + grub_tpm2_buffer_pack_u32 (buffer, 0);
> > + start = buffer->size;
> > +
> > + grub_tpm2_buffer_pack_u32 (buffer, authCommand->sessionHandle);
> > +
> > + grub_tpm2_buffer_pack_u16 (buffer, authCommand->nonce.size);
> > + grub_tpm2_buffer_pack (buffer, authCommand->nonce.buffer,
> > + authCommand->nonce.size);
> > +
> > + grub_tpm2_buffer_pack_u8 (buffer,
> > + *((const grub_uint8_t*)
> > &authCommand->sessionAttributes));
> > +
> > + grub_tpm2_buffer_pack_u16 (buffer, authCommand->hmac.size);
> > + grub_tpm2_buffer_pack (buffer, authCommand->hmac.buffer,
> > + authCommand->hmac.size);
> > +
> > + tmp = grub_cpu_to_be32 (buffer->size - start);
> > + grub_memcpy (&buffer->data[start - sizeof (grub_uint32_t)], &tmp,
> > + sizeof (tmp));
> > +}
> > +
> > +void
> > +grub_tpm2_mu_TPM2B_Marshal (grub_tpm2_buffer_t buffer,
> > + const grub_uint16_t size,
> > + const grub_uint8_t* b)
>
> s/grub_tpm2_mu_TPM2B_Marshal/grub_efi_tpm2_TPM2B_Marshal/ and similar
> changes below please...
>
> > +{
> > + grub_tpm2_buffer_pack_u16 (buffer, size);
> > +
> > + for (grub_uint16_t i = 0; i < size; i++)
> > + grub_tpm2_buffer_pack_u8 (buffer, b[i]);
> > +}
>
> [...]
>
> > +static void
> > +__tpm2_mu_TPM2B_BUFFER_Unmarshal (grub_tpm2_buffer_t buffer,
> > + TPM2B* p, grub_uint16_t bound)
>
> I think this...
>
> > +{
> > + grub_tpm2_buffer_unpack_u16 (buffer, &p->size);
> > +
> > + if (p->size > bound)
> > + {
> > + buffer->error = 1;
> > + return;
> > + }
> > +
> > + grub_tpm2_buffer_unpack (buffer, &p->buffer, p->size);
> > +}
> > +
> > +#define TPM2B_BUFFER_UNMARSHAL(buffer, type, data) \
> > + __tpm2_mu_TPM2B_BUFFER_Unmarshal(buffer, (TPM2B *)data, sizeof(type) -
> > sizeof(grub_uint16_t))
>
> ... and this...
>
> > +
> > +void
> > +grub_tpm2_mu_TPMS_AUTH_RESPONSE_Unmarshal (grub_tpm2_buffer_t buffer,
> > + TPMS_AUTH_RESPONSE* p)
>
> ... should go behind this...
>
> > +{
> > + grub_uint8_t tmp;
> > + grub_uint32_t tmp32;
> > +
> > + grub_tpm2_buffer_unpack_u16 (buffer, &p->nonce.size);
> > +
> > + if (p->nonce.size)
> > + grub_tpm2_buffer_unpack (buffer, &p->nonce.buffer, p->nonce.size);
> > +
> > + grub_tpm2_buffer_unpack_u8 (buffer, &tmp);
> > + tmp32 = tmp;
> > + grub_memcpy (&p->sessionAttributes, &tmp32, sizeof (grub_uint32_t));
> > +
> > + grub_tpm2_buffer_unpack_u16 (buffer, &p->hmac.size);
> > +
> > + if (p->hmac.size)
> > + grub_tpm2_buffer_unpack (buffer, &p->hmac.buffer, p->hmac.size);
> > +}
> > +
> > +void
> > +grub_tpm2_mu_TPM2B_DIGEST_Unmarshal (grub_tpm2_buffer_t buffer,
> > + TPM2B_DIGEST* digest)
> > +{
> > + TPM2B_BUFFER_UNMARSHAL (buffer, TPM2B_DIGEST, digest);
> > +}
> > +
> > +void
> > +grub_tpm2_mu_TPM2B_NONCE_Unmarshal (grub_tpm2_buffer_t buffer,
> > + TPM2B_NONCE* nonce)
> > +{
> > + TPM2B_BUFFER_UNMARSHAL (buffer, TPM2B_NONCE, nonce);
> > +}
>
> [...]
>
> > diff --git a/grub-core/tpm2/tcg2.c b/grub-core/tpm2/tcg2.c
> > new file mode 100644
> > index 000000000..9e4b7f565
> > --- /dev/null
> > +++ b/grub-core/tpm2/tcg2.c
> > @@ -0,0 +1,143 @@
> > +/*
> > + * GRUB -- GRand Unified Bootloader
> > + * Copyright (C) 2022 Microsoft Corporation
> > + *
> > + * GRUB is free software: you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation, either version 3 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * GRUB is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with GRUB. If not, see <http://www.gnu.org/licenses/>.
> > + */
> > +
> > +#include <grub/efi/api.h>
> > +#include <grub/efi/efi.h>
> > +#include <grub/efi/tpm.h>
> > +#include <grub/mm.h>
> > +#include <grub/tpm2/tcg2.h>
> > +
> > +static grub_err_t
> > +grub_tcg2_get_caps (grub_efi_tpm2_protocol_t *protocol, int *tpm2,
> > + grub_size_t *max_output_size)
> > +{
> > + grub_efi_status_t status;
> > +
>
> I would drop this empty line. Please fix similar things in the other
> places too.
>
Sure. I'll check the patch to remove the unnecessary lines.
> > + static int has_caps = 0;
> > + static EFI_TCG2_BOOT_SERVICE_CAPABILITY caps =
> > + {
> > + .Size = (grub_uint8_t) sizeof (caps)
> > + };
> > +
> > + if (has_caps)
> > + goto exit;
> > +
> > + status = protocol->get_capability (protocol, &caps);
> > + if (status != GRUB_EFI_SUCCESS || !caps.TPMPresentFlag)
> > + return GRUB_ERR_FILE_NOT_FOUND;
> > +
> > + has_caps = 1;
> > +
> > +exit:
>
> Missing space before label. Please fix similar things in the other
> places too.
>
Ok, I'll add the missing spaces to the labels.
> > + if (tpm2)
>
> I prefer
> if (tpm2 != NULL)
>
> > + *tpm2 = caps.TPMPresentFlag;
> > + if (max_output_size)
>
> Ditto... Especially here short form can be confusing...
> Please fix similar problems everywhere.
>
Got it.
> > + *max_output_size = caps.MaxResponseSize;
> > +
> > + return GRUB_ERR_NONE;
> > +}
> > +
> > +static grub_err_t
> > +grub_tcg2_get_protocol (grub_efi_tpm2_protocol_t **protocol)
> > +{
> > + static grub_guid_t tpm2_guid = EFI_TPM2_GUID;
> > + static grub_efi_tpm2_protocol_t *tpm2_protocol = NULL;
> > +
> > + int tpm2;
> > + grub_efi_handle_t *handles;
> > + grub_efi_uintn_t num_handles;
> > + grub_efi_handle_t tpm2_handle;
> > + grub_err_t err = GRUB_ERR_FILE_NOT_FOUND;
> > +
> > + if (tpm2_protocol)
> > + {
> > + *protocol = tpm2_protocol;
> > + return GRUB_ERR_NONE;
> > + }
> > +
> > + handles = grub_efi_locate_handle (GRUB_EFI_BY_PROTOCOL, &tpm2_guid, NULL,
> > + &num_handles);
> > + if (!handles || !num_handles)
> > + return err;
> > +
> > + tpm2_handle = handles[0];
> > +
> > + tpm2_protocol = grub_efi_open_protocol (tpm2_handle, &tpm2_guid,
> > + GRUB_EFI_OPEN_PROTOCOL_GET_PROTOCOL);
> > + if (!tpm2_protocol)
> > + goto exit;
> > +
> > + err = grub_tcg2_get_caps (tpm2_protocol, &tpm2, NULL);
> > + if (err || !tpm2)
> > + goto exit;
> > +
> > + *protocol = tpm2_protocol;
> > + err = GRUB_ERR_NONE;
> > +
> > +exit:
> > + grub_free (handles);
> > + return err;
> > +}
>
> [...]
>
> > diff --git a/include/grub/tpm2/buffer.h b/include/grub/tpm2/buffer.h
> > new file mode 100644
> > index 000000000..87dcd8d6c
> > --- /dev/null
> > +++ b/include/grub/tpm2/buffer.h
> > @@ -0,0 +1,65 @@
> > +/*
> > + * GRUB -- GRand Unified Bootloader
> > + * Copyright (C) 2022 Microsoft Corporation
> > + *
> > + * GRUB is free software: you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation, either version 3 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * GRUB is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with GRUB. If not, see <http://www.gnu.org/licenses/>.
> > + */
> > +
> > +#ifndef GRUB_TPM2_BUFFER_HEADER
> > +#define GRUB_TPM2_BUFFER_HEADER 1
> > +
> > +#include <grub/types.h>
> > +
> > +#define GRUB_TPM2_BUFFER_CAPACITY 4096
> > +
> > +struct grub_tpm2_buffer
> > +{
> > + grub_uint8_t data[GRUB_TPM2_BUFFER_CAPACITY];
> > + grub_size_t size;
> > + grub_size_t offset;
> > + grub_size_t cap;
> > + int error;
>
> The int does not look right for me here.
Yeah, grub_bool_t is better for this variable.
>
> > +};
> > +typedef struct grub_tpm2_buffer *grub_tpm2_buffer_t;
> > +
> > +void
> > +grub_tpm2_buffer_init (grub_tpm2_buffer_t buffer);
> > +
> > +void
> > +grub_tpm2_buffer_pack (grub_tpm2_buffer_t buffer, const void* data,
> > + grub_size_t size);
> > +
> > +void
> > +grub_tpm2_buffer_pack_u8 (grub_tpm2_buffer_t buffer, grub_uint8_t value);
> > +
> > +void
> > +grub_tpm2_buffer_pack_u16 (grub_tpm2_buffer_t buffer, grub_uint16_t value);
> > +
> > +void
> > +grub_tpm2_buffer_pack_u32 (grub_tpm2_buffer_t buffer, grub_uint32_t value);
> > +
> > +void
> > +grub_tpm2_buffer_unpack (grub_tpm2_buffer_t buffer, void* data,
> > + grub_size_t size);
> > +
> > +void
> > +grub_tpm2_buffer_unpack_u8 (grub_tpm2_buffer_t buffer, grub_uint8_t*
> > value);
> > +
> > +void
> > +grub_tpm2_buffer_unpack_u16 (grub_tpm2_buffer_t buffer, grub_uint16_t*
> > value);
> > +
> > +void
> > +grub_tpm2_buffer_unpack_u32 (grub_tpm2_buffer_t buffer, grub_uint32_t*
> > value);
>
> Is not all that stuff internal thing? If yes it should not go to the
> include/grub/tpm2 dir.
>
Currently, the tpm2 module is the only user. I'll move them to the tpm2
dir.
> [...]
>
> > diff --git a/include/grub/tpm2/internal/structs.h
> > b/include/grub/tpm2/internal/structs.h
> > new file mode 100644
> > index 000000000..c615d71e9
> > --- /dev/null
> > +++ b/include/grub/tpm2/internal/structs.h
> > @@ -0,0 +1,768 @@
> > +/*
> > + * GRUB -- GRand Unified Bootloader
> > + * Copyright (C) 2022 Microsoft Corporation
> > + *
> > + * GRUB is free software: you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation, either version 3 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * GRUB is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with GRUB. If not, see <http://www.gnu.org/licenses/>.
> > + */
> > +
> > +#ifndef GRUB_TPM2_INTERNAL_STRUCTS_HEADER
> > +#define GRUB_TPM2_INTERNAL_STRUCTS_HEADER 1
> > +
> > +#include <grub/tpm2/internal/types.h>
> > +
>
> [...]
>
> > +/* TPMA_SESSION Structure */
> > +struct TPMA_SESSION
> > +{
> > + unsigned int continueSession:1;
> > + unsigned int auditExclusive:1;
> > + unsigned int auditReset:1;
> > + unsigned int reserved1:2;
> > + unsigned int decrypt:1;
> > + unsigned int encrypt:1;
> > + unsigned int audit:1;
> > + unsigned int reserved:24;
>
> s/unsigned int/grub_uint32_t/?
>
Hmmm, I overlooked this struct. Per TPM2 spec, TPMA_SESSION is actually
UINT8. However, the unpack function covered the mistake so the TPM2
commands still work. Will fix it in v18.
> > +};
> > +typedef struct TPMA_SESSION TPMA_SESSION;
>
> [...]
>
> > +/* TPMA_OBJECT Structure */
> > +struct TPMA_OBJECT
> > +{
> > + unsigned int reserved1:1;
> > + unsigned int fixedTPM:1;
> > + unsigned int stClear:1;
> > + unsigned int reserved2:1;
> > + unsigned int fixedParent:1;
> > + unsigned int sensitiveDataOrigin:1;
> > + unsigned int userWithAuth:1;
> > + unsigned int adminWithPolicy:1;
> > + unsigned int reserved3:2;
> > + unsigned int noDA:1;
> > + unsigned int encryptedDuplication:1;
> > + unsigned int reserved4:4;
> > + unsigned int restricted:1;
> > + unsigned int decrypt:1;
> > + unsigned int sign:1;
> > + unsigned int reserved5:13;
>
> s/unsigned int/grub_uint64_t/?
>
It's UINT32. Will fix it in v18.
> > +};
> > +typedef struct TPMA_OBJECT TPMA_OBJECT;
>
> [...]
>
> > +/* TPMA_LOCALITY Structure */
> > +struct TPMA_LOCALITY
> > +{
> > + unsigned char TPM_LOC_ZERO:1;
> > + unsigned char TPM_LOC_ONE:1;
> > + unsigned char TPM_LOC_TWO:1;
> > + unsigned char TPM_LOC_THREE:1;
> > + unsigned char TPM_LOC_FOUR:1;
> > + unsigned char Extended:3;
>
> s/unsigned char/grub_uint8_t/?
>
> And similar things below...
>
Ok, I'll check the header to fix those types...
> > +};
> > +typedef struct TPMA_LOCALITY TPMA_LOCALITY;
>
> [...]
>
> > diff --git a/include/grub/tpm2/tcg2.h b/include/grub/tpm2/tcg2.h
> > new file mode 100644
> > index 000000000..553b3fd93
> > --- /dev/null
> > +++ b/include/grub/tpm2/tcg2.h
> > @@ -0,0 +1,34 @@
> > +/*
> > + * GRUB -- GRand Unified Bootloader
> > + * Copyright (C) 2022 Microsoft Corporation
> > + *
> > + * GRUB is free software: you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation, either version 3 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * GRUB is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with GRUB. If not, see <http://www.gnu.org/licenses/>.
> > + */
> > +
> > +#ifndef GRUB_TPM2_TCG2_HEADER
> > +#define GRUB_TPM2_TCG2_HEADER 1
> > +
> > +#include <grub/err.h>
> > +#include <grub/types.h>
> > +
> > +grub_err_t
> > +grub_tcg2_get_max_output_size (grub_size_t *size);
> > +
> > +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);
> > +
> > +#endif /* ! GRUB_TPM2_TCG2_HEADER */
> > diff --git a/include/grub/tpm2/tpm2.h b/include/grub/tpm2/tpm2.h
> > new file mode 100644
> > index 000000000..cfdc9edcd
> > --- /dev/null
> > +++ b/include/grub/tpm2/tpm2.h
> > @@ -0,0 +1,34 @@
> > +/*
> > + * GRUB -- GRand Unified Bootloader
> > + * Copyright (C) 2022 Microsoft Corporation
>
> I think you should add
> Copyright (C) 2024 Free Software Foundation, Inc.
> everywhere in this patch. Maybe in the others too.
>
Will do that in v18.
> > + * GRUB is free software: you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation, either version 3 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * GRUB is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with GRUB. If not, see <http://www.gnu.org/licenses/>.
> > + */
> > +
> > +#ifndef GRUB_TPM2_TPM2_HEADER
> > +#define GRUB_TPM2_TPM2_HEADER 1
> > +
> > +#include <grub/tpm2/internal/types.h>
> > +#include <grub/tpm2/internal/structs.h>
> > +#include <grub/tpm2/internal/functions.h>
>
> You put all three headers into "internal" directory and then you make
> all that stuff public. Something is off here. Please export to the rest
> of the GRUB only minimal set of TPM2 functions. Additionally, all names,
> functions, types, structs, variables, constants, etc. exported outside
> of a compilation unit should be prefixed with "grub_" or "GRUB_" respectively.
> All typedefs should end with "_t". In general please be in line with
> the GRUB coding style.
>
> > +/* Well-Known Windows SRK handle */
> > +#define TPM2_SRK_HANDLE 0x81000001
> > +
> > +typedef struct TPM2_SEALED_KEY {
>
> Please be inline with the GRUB coding style. So, define struct first and
> then typedef.
>
Ok, I'll fix them.
Gary Lin
> > + TPM2B_PUBLIC public;
> > + TPM2B_PRIVATE private;
> > +} TPM2_SEALED_KEY;
>
> Daniel
- [PATCH v17 07/20] asn1_test: test module for libtasn1, (continued)
Re: [PATCH v17 10/20] tpm2: Add TPM Software Stack (TSS), Daniel Kiper, 2024/06/18
[PATCH v17 11/20] key_protector: Add TPM2 Key Protector, Gary Lin, 2024/06/14