On Thu, Aug 22, 2024 at 04:03:35PM +0200, Daniel Kiper wrote: > On Fri, Jun 28, 2024 at 04:18:57PM +0800, Gary Lin via Grub-devel wrote: > > This commit adds the necessary TPM2 types and structs as the preparation > > for the TPM2 Software Stack (TSS2) support. The Marshal/Unmarshal > > functions are also added to handle the data structure to be submitted to > > TPM2 commands and to be received from the response. > > > > Cc: Stefan Berger <stef...@linux.ibm.com> > > Signed-off-by: Hernan Gatta <hega...@linux.microsoft.com> > > Signed-off-by: Gary Lin <g...@suse.com> > > --- > > grub-core/lib/tss2/tss2_mu.c | 1170 +++++++++++++++++++++++++++++ > > grub-core/lib/tss2/tss2_mu.h | 397 ++++++++++ > > grub-core/lib/tss2/tss2_structs.h | 768 +++++++++++++++++++ > > grub-core/lib/tss2/tss2_types.h | 404 ++++++++++ > > 4 files changed, 2739 insertions(+) > > create mode 100644 grub-core/lib/tss2/tss2_mu.c > > create mode 100644 grub-core/lib/tss2/tss2_mu.h > > create mode 100644 grub-core/lib/tss2/tss2_structs.h > > create mode 100644 grub-core/lib/tss2/tss2_types.h > > > > diff --git a/grub-core/lib/tss2/tss2_mu.c b/grub-core/lib/tss2/tss2_mu.c > > new file mode 100644 > > index 000000000..af7a75266 > > --- /dev/null > > +++ b/grub-core/lib/tss2/tss2_mu.c > > @@ -0,0 +1,1170 @@ > > +/* > > + * GRUB -- GRand Unified Bootloader > > + * Copyright (C) 2024 Free Software Foundation, Inc. > > + * Copyright (C) 2022 Microsoft Corporation > > Wrong order... > Sorry for that. I've revised the whole patch set to fix the order.
> > + * 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 <tss2_mu.h> > > + > > +void > > +grub_Tss2_MU_TPMS_AUTH_COMMAND_Marshal (grub_tpm2_buffer_t buffer, > > + const TPMS_AUTH_COMMAND* authCommand) > > s/const TPMS_AUTH_COMMAND*/ const TPMS_AUTH_COMMAND *authCommand/ > > As I can see this can be wrong in the whole patch set. Please fix it all over > the place... > Sure. > > +{ > > + 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)); > > s/const grub_uint8_t*/const grub_uint8_t */ > > Similar problem to the above. It has to be fixes... > Got it. > > + 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_Tss2_MU_TPM2B_Marshal (grub_tpm2_buffer_t buffer, > > + const grub_uint16_t size, > > + const grub_uint8_t* b) > > +{ > > + grub_tpm2_buffer_pack_u16 (buffer, size); > > + > > + for (grub_uint16_t i = 0; i < size; i++) > > + grub_tpm2_buffer_pack_u8 (buffer, b[i]); > > +} > > + > > +void > > +grub_Tss2_MU_TPMU_SYM_KEY_BITS_Marshal (grub_tpm2_buffer_t buffer, > > + const TPMI_ALG_SYM_OBJECT algorithm, > > + const TPMU_SYM_KEY_BITS *p) > > But here it is correct. Please be consistent... > > > +{ > > + switch (algorithm) > > + { > > + case TPM_ALG_AES: > > + case TPM_ALG_SM4: > > + case TPM_ALG_CAMELLIA: > > + case TPM_ALG_XOR: > > + grub_tpm2_buffer_pack_u16 (buffer, *((const grub_uint16_t*) p)); > > + break; > > + case TPM_ALG_NULL: > > + break; > > + default: > > + buffer->error = 1; > > + break; > > + } > > +} > > + > > +void > > +grub_Tss2_MU_TPMU_SYM_MODE_Marshal (grub_tpm2_buffer_t buffer, > > + const TPMI_ALG_SYM_OBJECT algorithm, > > + const TPMU_SYM_MODE *p) > > +{ > > + switch (algorithm) > > + { > > + case TPM_ALG_AES: > > + case TPM_ALG_SM4: > > + case TPM_ALG_CAMELLIA: > > + grub_tpm2_buffer_pack_u16 (buffer, *((const grub_uint16_t*) p)); > > + break; > > + case TPM_ALG_XOR: > > + case TPM_ALG_NULL: > > + break; > > + default: > > + buffer->error = 1; > > + break; > > + } > > +} > > + > > +void > > +grub_Tss2_MU_TPMT_SYM_DEF_Marshal (grub_tpm2_buffer_t buffer, > > + const TPMT_SYM_DEF *p) > > +{ > > + grub_tpm2_buffer_pack_u16 (buffer, p->algorithm); > > + grub_Tss2_MU_TPMU_SYM_KEY_BITS_Marshal (buffer, p->algorithm, > > &p->keyBits); > > + grub_Tss2_MU_TPMU_SYM_MODE_Marshal (buffer, p->algorithm, &p->mode); > > +} > > + > > +void > > +grub_Tss2_MU_TPMS_PCR_SELECTION_Marshal (grub_tpm2_buffer_t buffer, > > + const TPMS_PCR_SELECTION* pcrSelection) > > +{ > > + grub_tpm2_buffer_pack_u16 (buffer, pcrSelection->hash); > > + grub_tpm2_buffer_pack_u8 (buffer, pcrSelection->sizeOfSelect); > > + > > + for (grub_uint32_t i = 0; i < pcrSelection->sizeOfSelect; i++) > > + grub_tpm2_buffer_pack_u8 (buffer, pcrSelection->pcrSelect[i]); > > +} > > + > > +void > > +grub_Tss2_MU_TPML_PCR_SELECTION_Marshal (grub_tpm2_buffer_t buffer, > > + const TPML_PCR_SELECTION* pcrSelection) > > +{ > > + grub_tpm2_buffer_pack_u32 (buffer, pcrSelection->count); > > + > > + for (grub_uint32_t i = 0; i < pcrSelection->count; i++) > > Even if it works do not define variables in that way. Please do that at > the beginning of a function. > Ok. I've spotted the similar usage in several MU functions. Will fix them in the next version. > > + grub_Tss2_MU_TPMS_PCR_SELECTION_Marshal (buffer, > > + &pcrSelection->pcrSelections[i]); > > +} > > [...] > > > +void > > +grub_Tss2_MU_TPM2B_PUBLIC_Marshal (grub_tpm2_buffer_t buffer, > > + const TPM2B_PUBLIC *p) > > +{ > > + grub_uint32_t start; > > + grub_uint16_t size; > > + > > + if (p) > > + { > > + grub_tpm2_buffer_pack_u16 (buffer, p->size); > > + > > + start = buffer->size; > > + grub_Tss2_MU_TPMT_PUBLIC_Marshal (buffer, &p->publicArea); > > + size = grub_cpu_to_be16 (buffer->size - start); > > + grub_memcpy (&buffer->data[start - sizeof (grub_uint16_t)], &size, > > + sizeof (size)); > > Please reduce wrapping further where possible. And it is still possible > in many places... > Ok. > > + } > > + else > > + grub_tpm2_buffer_pack_u16 (buffer, 0); > > +} > > [...] > > > +void > > +grub_Tss2_MU_TPMU_SIGNATURE_Marshal (grub_tpm2_buffer_t buffer, > > + const TPMI_ALG_SIG_SCHEME sigAlg, > > + const TPMU_SIGNATURE *p) > > +{ > > + switch (sigAlg) > > + { > > + case TPM_ALG_RSASSA: > > + grub_Tss2_MU_TPMS_SIGNATURE_RSA_Marshal (buffer, (TPMS_SIGNATURE_RSA > > *)&p->rsassa); > > Missing space in cast... > > s/(TPMS_SIGNATURE_RSA *)&p->rsassa/(TPMS_SIGNATURE_RSA *) &p->rsassa/ > Ok. > > + break; > > + case TPM_ALG_RSAPSS: > > + grub_Tss2_MU_TPMS_SIGNATURE_RSA_Marshal (buffer, (TPMS_SIGNATURE_RSA > > *)&p->rsapss); > > Ditto and below... > > > + break; > > + case TPM_ALG_ECDSA: > > + grub_Tss2_MU_TPMS_SIGNATURE_ECC_Marshal (buffer, (TPMS_SIGNATURE_ECC > > *)&p->ecdsa); > > + break; > > + case TPM_ALG_ECDAA: > > + grub_Tss2_MU_TPMS_SIGNATURE_ECC_Marshal (buffer, (TPMS_SIGNATURE_ECC > > *)&p->ecdaa); > > + break; > > + case TPM_ALG_SM2: > > + grub_Tss2_MU_TPMS_SIGNATURE_ECC_Marshal (buffer, (TPMS_SIGNATURE_ECC > > *)&p->sm2); > > + break; > > + case TPM_ALG_ECSCHNORR: > > + grub_Tss2_MU_TPMS_SIGNATURE_ECC_Marshal (buffer, (TPMS_SIGNATURE_ECC > > *)&p->ecschnorr); > > + break; > > + case TPM_ALG_HMAC: > > + grub_Tss2_MU_TPMT_HA_Marshal (buffer, &p->hmac); > > + break; > > + case TPM_ALG_NULL: > > + break; > > + default: > > + buffer->error = 1; > > + break; > > + } > > +} > > [...] > > > diff --git a/grub-core/lib/tss2/tss2_mu.h b/grub-core/lib/tss2/tss2_mu.h > > new file mode 100644 > > index 000000000..041ee9956 > > --- /dev/null > > +++ b/grub-core/lib/tss2/tss2_mu.h > > @@ -0,0 +1,397 @@ > > +/* > > + * GRUB -- GRand Unified Bootloader > > + * Copyright (C) 2024 Free Software Foundation, Inc. > > + * Copyright (C) 2022 Microsoft Corporation > > Wrong order... > > > + * 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_MU_HEADER > > +#define GRUB_TPM2_MU_HEADER 1 > > + > > +#include <tss2_buffer.h> > > +#include <tss2_structs.h> > > + > > +void > > +grub_Tss2_MU_TPMS_AUTH_COMMAND_Marshal (grub_tpm2_buffer_t buffer, > > + const TPMS_AUTH_COMMAND* authCommand); > > Missing externs... > > [...] > Will add externs to all tss2 functions in the headers. > > diff --git a/grub-core/lib/tss2/tss2_structs.h > > b/grub-core/lib/tss2/tss2_structs.h > > new file mode 100644 > > index 000000000..9f9dc3d92 > > --- /dev/null > > +++ b/grub-core/lib/tss2/tss2_structs.h > > @@ -0,0 +1,768 @@ > > +/* > > + * GRUB -- GRand Unified Bootloader > > + * Copyright (C) 2024 Free Software Foundation, Inc. > > + * Copyright (C) 2022 Microsoft Corporation > > Wrong order... > > > + * 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 <tss2_types.h> > > + > > +/* TPMS_TAGGED_PROPERTY Structure */ > > +struct TPMS_TAGGED_PROPERTY > > +{ > > + TPM_PT property; > > + grub_uint32_t value; > > +}; > > +typedef struct TPMS_TAGGED_PROPERTY TPMS_TAGGED_PROPERTY; > > Please add "_t" to typedefs, i.e. > typedef struct TPMS_TAGGED_PROPERTY TPMS_TAGGED_PROPERTY_t. > > I think I asked for this last time. > I overlooked the comment. It's all renamed in my WIP branch. > [...] > > > +/* _PRIVATE Structure */ > > +struct _PRIVATE > > +{ > > + TPM2B_DIGEST integrityOuter; > > + TPM2B_DIGEST integrityInner; > > + TPM2B_SENSITIVE sensitive; > > +}; > > +typedef struct _PRIVATE _PRIVATE; > > I would not use so generic name... > > s/_PRIVATE/__TPM2B_PRIVATE/? At least... > That name is actually from the TPM2 SPEC and the intel-tss2 library follows the naming. Anyway, the '_PRIVATE' struct is not used by any TPM2 command, so it's totally fine to rename the struct without leading to any confusion. > > + > > +/* TPM2B_PRIVATE Structure */ > > +struct TPM2B_PRIVATE > > +{ > > + grub_uint16_t size; > > + grub_uint8_t buffer[sizeof(_PRIVATE)]; > > +}; > > +typedef struct TPM2B_PRIVATE TPM2B_PRIVATE; > > Feel free to add my RB when you fix all these minor issues. > Thanks. Gary Lin _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
