[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH 9/9] vtpmmgr: Support GetRandom passthrough on TPM 2.0
On Tue, May 4, 2021 at 9:33 AM Samuel Thibault <samuel.thibault@xxxxxxxxxxxx> wrote: > > Jason Andryuk, le mar. 04 mai 2021 08:48:42 -0400, a ecrit: > > GetRandom passthrough currently fails when using vtpmmgr with a hardware > > TPM 2.0. > > vtpmmgr (8): INFO[VTPM]: Passthrough: TPM_GetRandom > > vtpm (12): vtpm_cmd.c:120: Error: TPM_GetRandom() failed with error code > > (30) > > > > When running on TPM 2.0 hardware, vtpmmgr needs to convert the TPM 1.2 > > TPM_ORD_GetRandom into a TPM2 TPM_CC_GetRandom command. Besides the > > differing ordinal, the TPM 1.2 uses 32bit sizes for the request and > > response (vs. 16bit for TPM2). > > > > Place the random output directly into the tpmcmd->resp and build the > > packet around it. This avoids bouncing through an extra buffer, but the > > header has to be written after grabbing the random bytes so we have the > > number of bytes to include in the size. > > > > Signed-off-by: Jason Andryuk <jandryuk@xxxxxxxxx> > > --- > > stubdom/vtpmmgr/marshal.h | 10 +++++++ > > stubdom/vtpmmgr/vtpm_cmd_handler.c | 48 ++++++++++++++++++++++++++++++ > > 2 files changed, 58 insertions(+) > > > > diff --git a/stubdom/vtpmmgr/marshal.h b/stubdom/vtpmmgr/marshal.h > > index dce19c6439..20da22af09 100644 > > --- a/stubdom/vtpmmgr/marshal.h > > +++ b/stubdom/vtpmmgr/marshal.h > > @@ -890,6 +890,15 @@ inline int sizeof_TPM_AUTH_SESSION(const > > TPM_AUTH_SESSION* auth) { > > return rv; > > } > > > > +static > > +inline int sizeof_TPM_RQU_HEADER(BYTE* ptr) { > > + int rv = 0; > > + rv += sizeof_UINT16(ptr); > > + rv += sizeof_UINT32(ptr); > > + rv += sizeof_UINT32(ptr); > > + return rv; > > +} > > + > > static > > inline BYTE* pack_TPM_RQU_HEADER(BYTE* ptr, > > TPM_TAG tag, > > @@ -923,5 +932,6 @@ inline int unpack3_TPM_RQU_HEADER(BYTE* ptr, UINT32* > > pos, UINT32 max, > > #define pack_TPM_RSP_HEADER(p, t, s, r) pack_TPM_RQU_HEADER(p, t, s, r) > > #define unpack_TPM_RSP_HEADER(p, t, s, r) unpack_TPM_RQU_HEADER(p, t, s, r) > > #define unpack3_TPM_RSP_HEADER(p, l, m, t, s, r) unpack3_TPM_RQU_HEADER(p, > > l, m, t, s, r) > > +#define sizeof_TPM_RSP_HEADER(p) sizeof_TPM_RQU_HEADER(p) > > > > #endif > > diff --git a/stubdom/vtpmmgr/vtpm_cmd_handler.c > > b/stubdom/vtpmmgr/vtpm_cmd_handler.c > > index 2ac14fae77..7ca1d9df94 100644 > > --- a/stubdom/vtpmmgr/vtpm_cmd_handler.c > > +++ b/stubdom/vtpmmgr/vtpm_cmd_handler.c > > @@ -47,6 +47,7 @@ > > #include "vtpm_disk.h" > > #include "vtpmmgr.h" > > #include "tpm.h" > > +#include "tpm2.h" > > #include "tpmrsa.h" > > #include "tcg.h" > > #include "mgmt_authority.h" > > @@ -772,6 +773,52 @@ static int vtpmmgr_permcheck(struct tpm_opaque *opq) > > return 1; > > } > > > > +TPM_RESULT vtpmmgr_handle_getrandom(struct tpm_opaque *opaque, > > + tpmcmd_t* tpmcmd) > > +{ > > + TPM_RESULT status = TPM_SUCCESS; > > + TPM_TAG tag; > > + UINT32 size; > > + UINT32 rand_offset; > > + UINT32 rand_size; > > + TPM_COMMAND_CODE ord; > > + BYTE *p; > > + > > + p = unpack_TPM_RQU_HEADER(tpmcmd->req, &tag, &size, &ord); > > + > > + if (!hw_is_tpm2()) { > > + size = TCPA_MAX_BUFFER_LENGTH; > > + TPMTRYRETURN(TPM_TransmitData(tpmcmd->req, tpmcmd->req_len, > > + tpmcmd->resp, &size)); > > + tpmcmd->resp_len = size; > > + > > + return TPM_SUCCESS; > > + } > > > We need to check for the size of the request before unpacking (which > doesn't check for it), don't we? Yes, good catch. vtpmmgr_handle_cmd doesn't check either. > > + /* TPM_GetRandom req: <header><uint32 num bytes> */ > > + unpack_UINT32(p, &rand_size); > > + > > + /* Call TPM2_GetRandom but return a TPM_GetRandom response. */ > > + /* TPM_GetRandom resp: <header><uint32 num bytes><num random bytes> */ > > + rand_offset = sizeof_TPM_RSP_HEADER(tpmcmd->resp) + > > + sizeof_UINT32(tpmcmd->resp); > > There is a spurious indentation here, at first sight I even thought it > was part of the comment. Sorry about that - it was 8 spaces instead of a tab. > We also need to check that rand_size is not too large? > - that the returned data won't overflow tpmcmd->resp + rand_offset > - that it fits in a UINT16 Yes, will do. > Also, TPM2_GetRandom casts bytesRequested into UINT16*, that's bogus, it > should use a local UINT16 variable and assign *bytesRequested. Good catch. I'll do that in a new patch. > > + TPMTRYRETURN(TPM2_GetRandom(&rand_size, tpmcmd->resp + rand_offset)); > > + > > + p = pack_TPM_RSP_HEADER(tpmcmd->resp, TPM_TAG_RSP_COMMAND, > > + rand_offset + rand_size, status); > > + p = pack_UINT32(p, rand_size); > > + tpmcmd->resp_len = rand_offset + rand_size; > > + > > + return status; > > + > > +abort_egress: > > + tpmcmd->resp_len = VTPM_COMMAND_HEADER_SIZE; > > + pack_TPM_RSP_HEADER(tpmcmd->resp, tag + 3, tpmcmd->resp_len, status); > > + > > + return status; > > +} > > + > > TPM_RESULT vtpmmgr_handle_cmd( > > struct tpm_opaque *opaque, > > tpmcmd_t* tpmcmd) > > @@ -842,6 +889,7 @@ TPM_RESULT vtpmmgr_handle_cmd( > > switch(ord) { > > case TPM_ORD_GetRandom: > > vtpmloginfo(VTPM_LOG_VTPM, "Passthrough: > > TPM_GetRandom\n"); > > + return vtpmmgr_handle_getrandom(opaque, tpmcmd); > > break; > > Drop the break, then. I would say also move (or drop) the log, like the > other cases. Will drop the break. I would just leave the log since it matches the other cases in this case statement. But I can remove it if you want. Regards, Jason
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |