* [PATCH v2 01/13] docs: Warn about incomplete vtpmmgr TPM 2.0 support
2021-05-06 13:59 [PATCH v2 00/13] vtpmmgr: Some fixes - still incomplete Jason Andryuk
@ 2021-05-06 13:59 ` Jason Andryuk
2021-05-06 13:59 ` [PATCH v2 02/13] vtpmmgr: Print error code to aid debugging Jason Andryuk
` (11 subsequent siblings)
12 siblings, 0 replies; 32+ messages in thread
From: Jason Andryuk @ 2021-05-06 13:59 UTC (permalink / raw)
To: xen-devel; +Cc: Jason Andryuk, Ian Jackson, Wei Liu, Andrew Cooper
The vtpmmgr TPM 2.0 support is incomplete. Add a warning about that to
the documentation so others don't have to work through discovering it is
broken.
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
docs/man/xen-vtpmmgr.7.pod | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/docs/man/xen-vtpmmgr.7.pod b/docs/man/xen-vtpmmgr.7.pod
index af825a7ffe..875dcce508 100644
--- a/docs/man/xen-vtpmmgr.7.pod
+++ b/docs/man/xen-vtpmmgr.7.pod
@@ -222,6 +222,17 @@ XSM label, not the kernel.
=head1 Appendix B: vtpmmgr on TPM 2.0
+=head2 WARNING: Incomplete - cannot persist data
+
+TPM 2.0 support for vTPM manager is incomplete. There is no support for
+persisting an encryption key, so vTPM manager regenerates primary and secondary
+key handles each boot.
+
+Also, the vTPM manger group command implementation hardcodes TPM 1.2 commands.
+This means running manage-vtpmmgr.pl fails when the TPM 2.0 hardware rejects
+the TPM 1.2 commands. vTPM manager with TPM 2.0 cannot create groups and
+therefore cannot persist vTPM contents.
+
=head2 Manager disk image setup:
The vTPM Manager requires a disk image to store its encrypted data. The image
--
2.30.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH v2 02/13] vtpmmgr: Print error code to aid debugging
2021-05-06 13:59 [PATCH v2 00/13] vtpmmgr: Some fixes - still incomplete Jason Andryuk
2021-05-06 13:59 ` [PATCH v2 01/13] docs: Warn about incomplete vtpmmgr TPM 2.0 support Jason Andryuk
@ 2021-05-06 13:59 ` Jason Andryuk
2021-05-06 13:59 ` [PATCH v2 03/13] stubom: newlib: Enable C99 formats for %z Jason Andryuk
` (10 subsequent siblings)
12 siblings, 0 replies; 32+ messages in thread
From: Jason Andryuk @ 2021-05-06 13:59 UTC (permalink / raw)
To: xen-devel; +Cc: Jason Andryuk, Daniel De Graaf, Quan Xu, Samuel Thibault
tpm_get_error_name returns "Unknown Error Code" when an error string
is not defined. In that case, we should print the Error Code so it can
be looked up offline. tpm_get_error_name returns a const string, so
just have the two callers always print the error code so it is always
available.
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
---
stubdom/vtpmmgr/tpm.c | 2 +-
stubdom/vtpmmgr/tpm2.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/stubdom/vtpmmgr/tpm.c b/stubdom/vtpmmgr/tpm.c
index 779cddd64e..83b2bc16b2 100644
--- a/stubdom/vtpmmgr/tpm.c
+++ b/stubdom/vtpmmgr/tpm.c
@@ -109,7 +109,7 @@
UINT32 rsp_status; \
UNPACK_OUT(TPM_RSP_HEADER, &rsp_tag, &rsp_len, &rsp_status); \
if (rsp_status != TPM_SUCCESS) { \
- vtpmlogerror(VTPM_LOG_TPM, "Failed with return code %s\n", tpm_get_error_name(rsp_status)); \
+ vtpmlogerror(VTPM_LOG_TPM, "Failed with return code %s (%x)\n", tpm_get_error_name(rsp_status), rsp_status); \
status = rsp_status; \
goto abort_egress; \
} \
diff --git a/stubdom/vtpmmgr/tpm2.c b/stubdom/vtpmmgr/tpm2.c
index c9f1016ab5..655e6d164c 100644
--- a/stubdom/vtpmmgr/tpm2.c
+++ b/stubdom/vtpmmgr/tpm2.c
@@ -126,7 +126,7 @@
ptr = unpack_TPM_RSP_HEADER(ptr, \
&(tag), &(paramSize), &(status));\
if ((status) != TPM_SUCCESS){ \
- vtpmlogerror(VTPM_LOG_TPM, "Failed with return code %s\n", tpm_get_error_name(status));\
+ vtpmlogerror(VTPM_LOG_TPM, "Failed with return code %s (%x)\n", tpm_get_error_name(status), (status));\
goto abort_egress;\
}\
} while(0)
--
2.30.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH v2 03/13] stubom: newlib: Enable C99 formats for %z
2021-05-06 13:59 [PATCH v2 00/13] vtpmmgr: Some fixes - still incomplete Jason Andryuk
2021-05-06 13:59 ` [PATCH v2 01/13] docs: Warn about incomplete vtpmmgr TPM 2.0 support Jason Andryuk
2021-05-06 13:59 ` [PATCH v2 02/13] vtpmmgr: Print error code to aid debugging Jason Andryuk
@ 2021-05-06 13:59 ` Jason Andryuk
2021-05-06 13:59 ` [PATCH v2 04/13] vtpmmgr: Allow specifying srk_handle for TPM2 Jason Andryuk
` (9 subsequent siblings)
12 siblings, 0 replies; 32+ messages in thread
From: Jason Andryuk @ 2021-05-06 13:59 UTC (permalink / raw)
To: xen-devel; +Cc: Jason Andryuk, Ian Jackson, Wei Liu, Samuel Thibault
vtpmmgr was changed to print size_t with the %z modifier, but newlib
isn't compiled with %z support. So you get output like:
root seal: zu; sector of 13: zu
root: zu v=zu
itree: 36; sector of 112: zu
group: zu v=zu id=zu md=zu
group seal: zu; 5 in parent: zu; sector of 13: zu
vtpm: zu+zu; sector of 48: zu
Enable the C99 formats in newlib so vtpmmgr prints the numeric values.
Fixes 9379af08ccc0 "stubdom: vtpmmgr: Correctly format size_t with %z
when printing."
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
---
stubdom/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/stubdom/Makefile b/stubdom/Makefile
index 90d9ffcd9f..c6de5f68ae 100644
--- a/stubdom/Makefile
+++ b/stubdom/Makefile
@@ -105,7 +105,7 @@ cross-newlib: $(NEWLIB_STAMPFILE)
$(NEWLIB_STAMPFILE): mk-headers-$(XEN_TARGET_ARCH) newlib-$(NEWLIB_VERSION)
mkdir -p newlib-$(XEN_TARGET_ARCH)
( cd newlib-$(XEN_TARGET_ARCH) && \
- CC_FOR_TARGET="$(CC) $(TARGET_CPPFLAGS) $(TARGET_CFLAGS) $(NEWLIB_CFLAGS)" AR_FOR_TARGET=$(AR) LD_FOR_TARGET=$(LD) RANLIB_FOR_TARGET=$(RANLIB) ../newlib-$(NEWLIB_VERSION)/configure --prefix=$(CROSS_PREFIX) --verbose --target=$(GNU_TARGET_ARCH)-xen-elf --enable-newlib-io-long-long --disable-multilib && \
+ CC_FOR_TARGET="$(CC) $(TARGET_CPPFLAGS) $(TARGET_CFLAGS) $(NEWLIB_CFLAGS)" AR_FOR_TARGET=$(AR) LD_FOR_TARGET=$(LD) RANLIB_FOR_TARGET=$(RANLIB) ../newlib-$(NEWLIB_VERSION)/configure --prefix=$(CROSS_PREFIX) --verbose --target=$(GNU_TARGET_ARCH)-xen-elf --enable-newlib-io-long-long --enable-newlib-io-c99-formats --disable-multilib && \
$(MAKE) DESTDIR= && \
$(MAKE) DESTDIR= install )
--
2.30.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH v2 04/13] vtpmmgr: Allow specifying srk_handle for TPM2
2021-05-06 13:59 [PATCH v2 00/13] vtpmmgr: Some fixes - still incomplete Jason Andryuk
` (2 preceding siblings ...)
2021-05-06 13:59 ` [PATCH v2 03/13] stubom: newlib: Enable C99 formats for %z Jason Andryuk
@ 2021-05-06 13:59 ` Jason Andryuk
2021-05-06 21:35 ` Samuel Thibault
2021-05-10 11:56 ` Daniel P. Smith
2021-05-06 13:59 ` [PATCH v2 05/13] vtpmmgr: Move vtpmmgr_shutdown Jason Andryuk
` (8 subsequent siblings)
12 siblings, 2 replies; 32+ messages in thread
From: Jason Andryuk @ 2021-05-06 13:59 UTC (permalink / raw)
To: xen-devel
Cc: Jason Andryuk, Ian Jackson, Wei Liu, Daniel De Graaf, Quan Xu,
Samuel Thibault
Bypass taking ownership of the TPM2 if an srk_handle is specified.
This srk_handle must be usable with Null auth for the time being.
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
---
v2: Use "=" seperator
---
docs/man/xen-vtpmmgr.7.pod | 7 +++++++
stubdom/vtpmmgr/init.c | 11 ++++++++++-
2 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/docs/man/xen-vtpmmgr.7.pod b/docs/man/xen-vtpmmgr.7.pod
index 875dcce508..3286954568 100644
--- a/docs/man/xen-vtpmmgr.7.pod
+++ b/docs/man/xen-vtpmmgr.7.pod
@@ -92,6 +92,13 @@ Valid arguments:
=over 4
+=item srk_handle=<HANDLE>
+
+Specify a srk_handle for TPM 2.0. TPM 2.0 uses a key hierarchy, and
+this allow specifying the parent handle for vtpmmgr to create its own
+key under. Using this option bypasses vtpmmgr trying to take ownership
+of the TPM.
+
=item owner_auth=<AUTHSPEC>
=item srk_auth=<AUTHSPEC>
diff --git a/stubdom/vtpmmgr/init.c b/stubdom/vtpmmgr/init.c
index 1506735051..130e4f4bf6 100644
--- a/stubdom/vtpmmgr/init.c
+++ b/stubdom/vtpmmgr/init.c
@@ -302,6 +302,11 @@ int parse_cmdline_opts(int argc, char** argv, struct Opts* opts)
goto err_invalid;
}
}
+ else if(!strncmp(argv[i], "srk_handle=", 11)) {
+ if(sscanf(argv[i] + 11, "%x", &vtpm_globals.srk_handle) != 1) {
+ goto err_invalid;
+ }
+ }
else if(!strncmp(argv[i], "tpmdriver=", 10)) {
if(!strcmp(argv[i] + 10, "tpm_tis")) {
opts->tpmdriver = TPMDRV_TPM_TIS;
@@ -586,7 +591,11 @@ TPM_RESULT vtpmmgr2_create(void)
{
TPM_RESULT status = TPM_SUCCESS;
- TPMTRYRETURN(tpm2_take_ownership());
+ if ( vtpm_globals.srk_handle == 0 ) {
+ TPMTRYRETURN(tpm2_take_ownership());
+ } else {
+ tpm2_AuthArea_ctor(NULL, 0, &vtpm_globals.srk_auth_area);
+ }
/* create SK */
TPM2_Create_Params_out out;
--
2.30.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH v2 04/13] vtpmmgr: Allow specifying srk_handle for TPM2
2021-05-06 13:59 ` [PATCH v2 04/13] vtpmmgr: Allow specifying srk_handle for TPM2 Jason Andryuk
@ 2021-05-06 21:35 ` Samuel Thibault
2021-05-10 11:56 ` Daniel P. Smith
1 sibling, 0 replies; 32+ messages in thread
From: Samuel Thibault @ 2021-05-06 21:35 UTC (permalink / raw)
To: Jason Andryuk; +Cc: xen-devel, Ian Jackson, Wei Liu, Daniel De Graaf, Quan Xu
Jason Andryuk, le jeu. 06 mai 2021 09:59:14 -0400, a ecrit:
> Bypass taking ownership of the TPM2 if an srk_handle is specified.
>
> This srk_handle must be usable with Null auth for the time being.
>
> Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
> ---
> v2: Use "=" seperator
> ---
> docs/man/xen-vtpmmgr.7.pod | 7 +++++++
> stubdom/vtpmmgr/init.c | 11 ++++++++++-
> 2 files changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/docs/man/xen-vtpmmgr.7.pod b/docs/man/xen-vtpmmgr.7.pod
> index 875dcce508..3286954568 100644
> --- a/docs/man/xen-vtpmmgr.7.pod
> +++ b/docs/man/xen-vtpmmgr.7.pod
> @@ -92,6 +92,13 @@ Valid arguments:
>
> =over 4
>
> +=item srk_handle=<HANDLE>
> +
> +Specify a srk_handle for TPM 2.0. TPM 2.0 uses a key hierarchy, and
> +this allow specifying the parent handle for vtpmmgr to create its own
> +key under. Using this option bypasses vtpmmgr trying to take ownership
> +of the TPM.
> +
> =item owner_auth=<AUTHSPEC>
>
> =item srk_auth=<AUTHSPEC>
> diff --git a/stubdom/vtpmmgr/init.c b/stubdom/vtpmmgr/init.c
> index 1506735051..130e4f4bf6 100644
> --- a/stubdom/vtpmmgr/init.c
> +++ b/stubdom/vtpmmgr/init.c
> @@ -302,6 +302,11 @@ int parse_cmdline_opts(int argc, char** argv, struct Opts* opts)
> goto err_invalid;
> }
> }
> + else if(!strncmp(argv[i], "srk_handle=", 11)) {
> + if(sscanf(argv[i] + 11, "%x", &vtpm_globals.srk_handle) != 1) {
> + goto err_invalid;
> + }
> + }
> else if(!strncmp(argv[i], "tpmdriver=", 10)) {
> if(!strcmp(argv[i] + 10, "tpm_tis")) {
> opts->tpmdriver = TPMDRV_TPM_TIS;
> @@ -586,7 +591,11 @@ TPM_RESULT vtpmmgr2_create(void)
> {
> TPM_RESULT status = TPM_SUCCESS;
>
> - TPMTRYRETURN(tpm2_take_ownership());
> + if ( vtpm_globals.srk_handle == 0 ) {
> + TPMTRYRETURN(tpm2_take_ownership());
> + } else {
> + tpm2_AuthArea_ctor(NULL, 0, &vtpm_globals.srk_auth_area);
> + }
>
> /* create SK */
> TPM2_Create_Params_out out;
> --
> 2.30.2
>
--
Samuel
"...[Linux's] capacity to talk via any medium except smoke signals."
(By Dr. Greg Wettstein, Roger Maris Cancer Center)
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v2 04/13] vtpmmgr: Allow specifying srk_handle for TPM2
2021-05-06 13:59 ` [PATCH v2 04/13] vtpmmgr: Allow specifying srk_handle for TPM2 Jason Andryuk
2021-05-06 21:35 ` Samuel Thibault
@ 2021-05-10 11:56 ` Daniel P. Smith
1 sibling, 0 replies; 32+ messages in thread
From: Daniel P. Smith @ 2021-05-10 11:56 UTC (permalink / raw)
To: Jason Andryuk, xen-devel
Cc: Ian Jackson, Wei Liu, Daniel De Graaf, Quan Xu, Samuel Thibault
On 5/6/21 9:59 AM, Jason Andryuk wrote:
> Bypass taking ownership of the TPM2 if an srk_handle is specified.
>
> This srk_handle must be usable with Null auth for the time being.
>
> Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
> ---
Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com>
> v2: Use "=" seperator
> ---
> docs/man/xen-vtpmmgr.7.pod | 7 +++++++
> stubdom/vtpmmgr/init.c | 11 ++++++++++-
> 2 files changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/docs/man/xen-vtpmmgr.7.pod b/docs/man/xen-vtpmmgr.7.pod
> index 875dcce508..3286954568 100644
> --- a/docs/man/xen-vtpmmgr.7.pod
> +++ b/docs/man/xen-vtpmmgr.7.pod
> @@ -92,6 +92,13 @@ Valid arguments:
>
> =over 4
>
> +=item srk_handle=<HANDLE>
> +
> +Specify a srk_handle for TPM 2.0. TPM 2.0 uses a key hierarchy, and
> +this allow specifying the parent handle for vtpmmgr to create its own
> +key under. Using this option bypasses vtpmmgr trying to take ownership
> +of the TPM.
> +
> =item owner_auth=<AUTHSPEC>
>
> =item srk_auth=<AUTHSPEC>
> diff --git a/stubdom/vtpmmgr/init.c b/stubdom/vtpmmgr/init.c
> index 1506735051..130e4f4bf6 100644
> --- a/stubdom/vtpmmgr/init.c
> +++ b/stubdom/vtpmmgr/init.c
> @@ -302,6 +302,11 @@ int parse_cmdline_opts(int argc, char** argv, struct Opts* opts)
> goto err_invalid;
> }
> }
> + else if(!strncmp(argv[i], "srk_handle=", 11)) {
> + if(sscanf(argv[i] + 11, "%x", &vtpm_globals.srk_handle) != 1) {
> + goto err_invalid;
> + }
> + }
> else if(!strncmp(argv[i], "tpmdriver=", 10)) {
> if(!strcmp(argv[i] + 10, "tpm_tis")) {
> opts->tpmdriver = TPMDRV_TPM_TIS;
> @@ -586,7 +591,11 @@ TPM_RESULT vtpmmgr2_create(void)
> {
> TPM_RESULT status = TPM_SUCCESS;
>
> - TPMTRYRETURN(tpm2_take_ownership());
> + if ( vtpm_globals.srk_handle == 0 ) {
> + TPMTRYRETURN(tpm2_take_ownership());
> + } else {
> + tpm2_AuthArea_ctor(NULL, 0, &vtpm_globals.srk_auth_area);
> + }
>
> /* create SK */
> TPM2_Create_Params_out out;
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 05/13] vtpmmgr: Move vtpmmgr_shutdown
2021-05-06 13:59 [PATCH v2 00/13] vtpmmgr: Some fixes - still incomplete Jason Andryuk
` (3 preceding siblings ...)
2021-05-06 13:59 ` [PATCH v2 04/13] vtpmmgr: Allow specifying srk_handle for TPM2 Jason Andryuk
@ 2021-05-06 13:59 ` Jason Andryuk
2021-05-07 15:48 ` Daniel P. Smith
2021-05-06 13:59 ` [PATCH v2 06/13] vtpmmgr: Flush transient keys on shutdown Jason Andryuk
` (7 subsequent siblings)
12 siblings, 1 reply; 32+ messages in thread
From: Jason Andryuk @ 2021-05-06 13:59 UTC (permalink / raw)
To: xen-devel; +Cc: Jason Andryuk, Daniel De Graaf, Quan Xu, Samuel Thibault
Reposition vtpmmgr_shutdown so it can call flush_tpm2 without a forward
declaration.
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
---
stubdom/vtpmmgr/init.c | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/stubdom/vtpmmgr/init.c b/stubdom/vtpmmgr/init.c
index 130e4f4bf6..decf8e8b4d 100644
--- a/stubdom/vtpmmgr/init.c
+++ b/stubdom/vtpmmgr/init.c
@@ -503,20 +503,6 @@ egress:
return status;
}
-void vtpmmgr_shutdown(void)
-{
- /* Cleanup TPM resources */
- TPM_TerminateHandle(vtpm_globals.oiap.AuthHandle);
-
- /* Close tpmback */
- shutdown_tpmback();
-
- /* Close tpmfront/tpm_tis */
- close(vtpm_globals.tpm_fd);
-
- vtpmloginfo(VTPM_LOG_VTPM, "VTPM Manager stopped.\n");
-}
-
/* TPM 2.0 */
static void tpm2_AuthArea_ctor(const char *authValue, UINT32 authLen,
@@ -797,3 +783,17 @@ abort_egress:
egress:
return status;
}
+
+void vtpmmgr_shutdown(void)
+{
+ /* Cleanup TPM resources */
+ TPM_TerminateHandle(vtpm_globals.oiap.AuthHandle);
+
+ /* Close tpmback */
+ shutdown_tpmback();
+
+ /* Close tpmfront/tpm_tis */
+ close(vtpm_globals.tpm_fd);
+
+ vtpmloginfo(VTPM_LOG_VTPM, "VTPM Manager stopped.\n");
+}
--
2.30.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH v2 05/13] vtpmmgr: Move vtpmmgr_shutdown
2021-05-06 13:59 ` [PATCH v2 05/13] vtpmmgr: Move vtpmmgr_shutdown Jason Andryuk
@ 2021-05-07 15:48 ` Daniel P. Smith
0 siblings, 0 replies; 32+ messages in thread
From: Daniel P. Smith @ 2021-05-07 15:48 UTC (permalink / raw)
To: Jason Andryuk, xen-devel; +Cc: Daniel De Graaf, Quan Xu, Samuel Thibault
On 5/6/21 9:59 AM, Jason Andryuk wrote:
> Reposition vtpmmgr_shutdown so it can call flush_tpm2 without a forward
> declaration.
>
> Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
> Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
> ---
Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com>
> stubdom/vtpmmgr/init.c | 28 ++++++++++++++--------------
> 1 file changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/stubdom/vtpmmgr/init.c b/stubdom/vtpmmgr/init.c
> index 130e4f4bf6..decf8e8b4d 100644
> --- a/stubdom/vtpmmgr/init.c
> +++ b/stubdom/vtpmmgr/init.c
> @@ -503,20 +503,6 @@ egress:
> return status;
> }
>
> -void vtpmmgr_shutdown(void)
> -{
> - /* Cleanup TPM resources */
> - TPM_TerminateHandle(vtpm_globals.oiap.AuthHandle);
> -
> - /* Close tpmback */
> - shutdown_tpmback();
> -
> - /* Close tpmfront/tpm_tis */
> - close(vtpm_globals.tpm_fd);
> -
> - vtpmloginfo(VTPM_LOG_VTPM, "VTPM Manager stopped.\n");
> -}
> -
> /* TPM 2.0 */
>
> static void tpm2_AuthArea_ctor(const char *authValue, UINT32 authLen,
> @@ -797,3 +783,17 @@ abort_egress:
> egress:
> return status;
> }
> +
> +void vtpmmgr_shutdown(void)
> +{
> + /* Cleanup TPM resources */
> + TPM_TerminateHandle(vtpm_globals.oiap.AuthHandle);
> +
> + /* Close tpmback */
> + shutdown_tpmback();
> +
> + /* Close tpmfront/tpm_tis */
> + close(vtpm_globals.tpm_fd);
> +
> + vtpmloginfo(VTPM_LOG_VTPM, "VTPM Manager stopped.\n");
> +}
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 06/13] vtpmmgr: Flush transient keys on shutdown
2021-05-06 13:59 [PATCH v2 00/13] vtpmmgr: Some fixes - still incomplete Jason Andryuk
` (4 preceding siblings ...)
2021-05-06 13:59 ` [PATCH v2 05/13] vtpmmgr: Move vtpmmgr_shutdown Jason Andryuk
@ 2021-05-06 13:59 ` Jason Andryuk
2021-05-10 12:12 ` Daniel P. Smith
2021-05-06 13:59 ` [PATCH v2 07/13] vtpmmgr: Flush all transient keys Jason Andryuk
` (6 subsequent siblings)
12 siblings, 1 reply; 32+ messages in thread
From: Jason Andryuk @ 2021-05-06 13:59 UTC (permalink / raw)
To: xen-devel; +Cc: Jason Andryuk, Daniel De Graaf, Quan Xu, Samuel Thibault
Remove our key so it isn't left in the TPM for someone to come along
after vtpmmgr shutsdown.
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
---
stubdom/vtpmmgr/init.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/stubdom/vtpmmgr/init.c b/stubdom/vtpmmgr/init.c
index decf8e8b4d..56b4be85b3 100644
--- a/stubdom/vtpmmgr/init.c
+++ b/stubdom/vtpmmgr/init.c
@@ -792,6 +792,14 @@ void vtpmmgr_shutdown(void)
/* Close tpmback */
shutdown_tpmback();
+ if (hw_is_tpm2()) {
+ /* Blow away all stale handles left in the tpm*/
+ if (flush_tpm2() != TPM_SUCCESS) {
+ vtpmlogerror(VTPM_LOG_TPM,
+ "TPM2_FlushResources failed, continuing shutdown..\n");
+ }
+ }
+
/* Close tpmfront/tpm_tis */
close(vtpm_globals.tpm_fd);
--
2.30.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH v2 06/13] vtpmmgr: Flush transient keys on shutdown
2021-05-06 13:59 ` [PATCH v2 06/13] vtpmmgr: Flush transient keys on shutdown Jason Andryuk
@ 2021-05-10 12:12 ` Daniel P. Smith
0 siblings, 0 replies; 32+ messages in thread
From: Daniel P. Smith @ 2021-05-10 12:12 UTC (permalink / raw)
To: Jason Andryuk, xen-devel; +Cc: Daniel De Graaf, Quan Xu, Samuel Thibault
On 5/6/21 9:59 AM, Jason Andryuk wrote:
> Remove our key so it isn't left in the TPM for someone to come along
> after vtpmmgr shutsdown.
>
> Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
> Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
> ---
Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com>
> stubdom/vtpmmgr/init.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/stubdom/vtpmmgr/init.c b/stubdom/vtpmmgr/init.c
> index decf8e8b4d..56b4be85b3 100644
> --- a/stubdom/vtpmmgr/init.c
> +++ b/stubdom/vtpmmgr/init.c
> @@ -792,6 +792,14 @@ void vtpmmgr_shutdown(void)
> /* Close tpmback */
> shutdown_tpmback();
>
> + if (hw_is_tpm2()) {
> + /* Blow away all stale handles left in the tpm*/
> + if (flush_tpm2() != TPM_SUCCESS) {
> + vtpmlogerror(VTPM_LOG_TPM,
> + "TPM2_FlushResources failed, continuing shutdown..\n");
> + }
> + }
> +
> /* Close tpmfront/tpm_tis */
> close(vtpm_globals.tpm_fd);
>
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 07/13] vtpmmgr: Flush all transient keys
2021-05-06 13:59 [PATCH v2 00/13] vtpmmgr: Some fixes - still incomplete Jason Andryuk
` (5 preceding siblings ...)
2021-05-06 13:59 ` [PATCH v2 06/13] vtpmmgr: Flush transient keys on shutdown Jason Andryuk
@ 2021-05-06 13:59 ` Jason Andryuk
2021-05-10 12:19 ` Daniel P. Smith
2021-05-06 13:59 ` [PATCH v2 08/13] vtpmmgr: Shutdown more gracefully Jason Andryuk
` (5 subsequent siblings)
12 siblings, 1 reply; 32+ messages in thread
From: Jason Andryuk @ 2021-05-06 13:59 UTC (permalink / raw)
To: xen-devel; +Cc: Jason Andryuk, Daniel De Graaf, Quan Xu, Samuel Thibault
We're only flushing 2 transients, but there are 3 handles. Use <= to also
flush the third handle since TRANSIENT_LAST is inclusive
The number of transient handles/keys is hardware dependent, so this
should query for the limit. And assignment of handles is assumed to be
sequential from the minimum. That may not be guaranteed, but seems okay
with my tpm2.
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
---
v2 add "since TRANSIENT_LAST is inclusive" to commit message.
---
stubdom/vtpmmgr/init.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/stubdom/vtpmmgr/init.c b/stubdom/vtpmmgr/init.c
index 56b4be85b3..4ae34a4fcb 100644
--- a/stubdom/vtpmmgr/init.c
+++ b/stubdom/vtpmmgr/init.c
@@ -656,7 +656,7 @@ static TPM_RC flush_tpm2(void)
{
int i;
- for (i = TRANSIENT_FIRST; i < TRANSIENT_LAST; i++)
+ for (i = TRANSIENT_FIRST; i <= TRANSIENT_LAST; i++)
TPM2_FlushContext(i);
return TPM_SUCCESS;
--
2.30.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH v2 07/13] vtpmmgr: Flush all transient keys
2021-05-06 13:59 ` [PATCH v2 07/13] vtpmmgr: Flush all transient keys Jason Andryuk
@ 2021-05-10 12:19 ` Daniel P. Smith
0 siblings, 0 replies; 32+ messages in thread
From: Daniel P. Smith @ 2021-05-10 12:19 UTC (permalink / raw)
To: Jason Andryuk, xen-devel; +Cc: Daniel De Graaf, Quan Xu, Samuel Thibault
On 5/6/21 9:59 AM, Jason Andryuk wrote:
> We're only flushing 2 transients, but there are 3 handles. Use <= to also
> flush the third handle since TRANSIENT_LAST is inclusive
>
> The number of transient handles/keys is hardware dependent, so this
> should query for the limit. And assignment of handles is assumed to be
> sequential from the minimum. That may not be guaranteed, but seems okay
> with my tpm2.
>
> Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
> Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
> ---
> v2 add "since TRANSIENT_LAST is inclusive" to commit message.
> ---
Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com>
> stubdom/vtpmmgr/init.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/stubdom/vtpmmgr/init.c b/stubdom/vtpmmgr/init.c
> index 56b4be85b3..4ae34a4fcb 100644
> --- a/stubdom/vtpmmgr/init.c
> +++ b/stubdom/vtpmmgr/init.c
> @@ -656,7 +656,7 @@ static TPM_RC flush_tpm2(void)
> {
> int i;
>
> - for (i = TRANSIENT_FIRST; i < TRANSIENT_LAST; i++)
> + for (i = TRANSIENT_FIRST; i <= TRANSIENT_LAST; i++)
> TPM2_FlushContext(i);
>
> return TPM_SUCCESS;
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 08/13] vtpmmgr: Shutdown more gracefully
2021-05-06 13:59 [PATCH v2 00/13] vtpmmgr: Some fixes - still incomplete Jason Andryuk
` (6 preceding siblings ...)
2021-05-06 13:59 ` [PATCH v2 07/13] vtpmmgr: Flush all transient keys Jason Andryuk
@ 2021-05-06 13:59 ` Jason Andryuk
2021-05-06 14:04 ` Jason Andryuk
2021-05-10 12:42 ` Daniel P. Smith
2021-05-06 13:59 ` [PATCH v2 09/13] vtpmmgr: Support GetRandom passthrough on TPM 2.0 Jason Andryuk
` (4 subsequent siblings)
12 siblings, 2 replies; 32+ messages in thread
From: Jason Andryuk @ 2021-05-06 13:59 UTC (permalink / raw)
To: xen-devel
Cc: Jason Andryuk, Daniel De Graaf, Quan Xu, Samuel Thibault,
Samuel Thibault
vtpmmgr uses the default, weak app_shutdown, which immediately calls the
shutdown hypercall. This short circuits the vtpmmgr clean up logic. We
need to perform the clean up to actually Flush our key out of the tpm.
Setting do_shutdown is one step in that direction, but vtpmmgr will most
likely be waiting in tpmback_req_any. We need to call shutdown_tpmback
to cancel the wait inside tpmback and perform the shutdown.
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Samuel Thibault <samuel.thibaut@ens-lyon.org>
---
stubdom/vtpmmgr/vtpmmgr.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/stubdom/vtpmmgr/vtpmmgr.c b/stubdom/vtpmmgr/vtpmmgr.c
index 9fddaa24f8..46ea018921 100644
--- a/stubdom/vtpmmgr/vtpmmgr.c
+++ b/stubdom/vtpmmgr/vtpmmgr.c
@@ -67,11 +67,21 @@ int hw_is_tpm2(void)
return (hardware_version.hw_version == TPM2_HARDWARE) ? 1 : 0;
}
+static int do_shutdown;
+
+void app_shutdown(unsigned int reason)
+{
+ printk("Shutdown requested: %d\n", reason);
+ do_shutdown = 1;
+
+ shutdown_tpmback();
+}
+
void main_loop(void) {
tpmcmd_t* tpmcmd;
uint8_t respbuf[TCPA_MAX_BUFFER_LENGTH];
- while(1) {
+ while (!do_shutdown) {
/* Wait for requests from a vtpm */
vtpmloginfo(VTPM_LOG_VTPM, "Waiting for commands from vTPM's:\n");
if((tpmcmd = tpmback_req_any()) == NULL) {
--
2.30.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH v2 08/13] vtpmmgr: Shutdown more gracefully
2021-05-06 13:59 ` [PATCH v2 08/13] vtpmmgr: Shutdown more gracefully Jason Andryuk
@ 2021-05-06 14:04 ` Jason Andryuk
2021-05-10 12:42 ` Daniel P. Smith
1 sibling, 0 replies; 32+ messages in thread
From: Jason Andryuk @ 2021-05-06 14:04 UTC (permalink / raw)
To: xen-devel; +Cc: Daniel De Graaf, Quan Xu, Samuel Thibault
On Thu, May 6, 2021 at 10:00 AM Jason Andryuk <jandryuk@gmail.com> wrote:
>
> vtpmmgr uses the default, weak app_shutdown, which immediately calls the
> shutdown hypercall. This short circuits the vtpmmgr clean up logic. We
> need to perform the clean up to actually Flush our key out of the tpm.
>
> Setting do_shutdown is one step in that direction, but vtpmmgr will most
> likely be waiting in tpmback_req_any. We need to call shutdown_tpmback
> to cancel the wait inside tpmback and perform the shutdown.
>
> Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
> Reviewed-by: Samuel Thibault <samuel.thibaut@ens-lyon.org>
Whoops, this should be "Reviewed-by: Samuel Thibault
<samuel.thibault@ens-lyon.org>". The above is missing an "l" in the
last name.
-Jason
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 08/13] vtpmmgr: Shutdown more gracefully
2021-05-06 13:59 ` [PATCH v2 08/13] vtpmmgr: Shutdown more gracefully Jason Andryuk
2021-05-06 14:04 ` Jason Andryuk
@ 2021-05-10 12:42 ` Daniel P. Smith
1 sibling, 0 replies; 32+ messages in thread
From: Daniel P. Smith @ 2021-05-10 12:42 UTC (permalink / raw)
To: Jason Andryuk, xen-devel
Cc: Daniel De Graaf, Quan Xu, Samuel Thibault, Samuel Thibault
On 5/6/21 9:59 AM, Jason Andryuk wrote:
> vtpmmgr uses the default, weak app_shutdown, which immediately calls the
> shutdown hypercall. This short circuits the vtpmmgr clean up logic. We
> need to perform the clean up to actually Flush our key out of the tpm.
>
> Setting do_shutdown is one step in that direction, but vtpmmgr will most
> likely be waiting in tpmback_req_any. We need to call shutdown_tpmback
> to cancel the wait inside tpmback and perform the shutdown.
>
> Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
> Reviewed-by: Samuel Thibault <samuel.thibaut@ens-lyon.org>
> ---
Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com>
> stubdom/vtpmmgr/vtpmmgr.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/stubdom/vtpmmgr/vtpmmgr.c b/stubdom/vtpmmgr/vtpmmgr.c
> index 9fddaa24f8..46ea018921 100644
> --- a/stubdom/vtpmmgr/vtpmmgr.c
> +++ b/stubdom/vtpmmgr/vtpmmgr.c
> @@ -67,11 +67,21 @@ int hw_is_tpm2(void)
> return (hardware_version.hw_version == TPM2_HARDWARE) ? 1 : 0;
> }
>
> +static int do_shutdown;
> +
> +void app_shutdown(unsigned int reason)
> +{
> + printk("Shutdown requested: %d\n", reason);
> + do_shutdown = 1;
> +
> + shutdown_tpmback();
> +}
> +
> void main_loop(void) {
> tpmcmd_t* tpmcmd;
> uint8_t respbuf[TCPA_MAX_BUFFER_LENGTH];
>
> - while(1) {
> + while (!do_shutdown) {
> /* Wait for requests from a vtpm */
> vtpmloginfo(VTPM_LOG_VTPM, "Waiting for commands from vTPM's:\n");
> if((tpmcmd = tpmback_req_any()) == NULL) {
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 09/13] vtpmmgr: Support GetRandom passthrough on TPM 2.0
2021-05-06 13:59 [PATCH v2 00/13] vtpmmgr: Some fixes - still incomplete Jason Andryuk
` (7 preceding siblings ...)
2021-05-06 13:59 ` [PATCH v2 08/13] vtpmmgr: Shutdown more gracefully Jason Andryuk
@ 2021-05-06 13:59 ` Jason Andryuk
2021-05-06 21:40 ` Samuel Thibault
2021-05-10 12:51 ` Daniel P. Smith
2021-05-06 13:59 ` [PATCH v2 10/13] vtpmmgr: Remove bogus cast from TPM2_GetRandom Jason Andryuk
` (3 subsequent siblings)
12 siblings, 2 replies; 32+ messages in thread
From: Jason Andryuk @ 2021-05-06 13:59 UTC (permalink / raw)
To: xen-devel; +Cc: Jason Andryuk, Daniel De Graaf, Quan Xu, Samuel Thibault
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@gmail.com>
---
v2:
Add bounds and size checks
Whitespace fixup
---
stubdom/vtpmmgr/marshal.h | 15 ++++++++
stubdom/vtpmmgr/vtpm_cmd_handler.c | 61 +++++++++++++++++++++++++++++-
2 files changed, 75 insertions(+), 1 deletion(-)
diff --git a/stubdom/vtpmmgr/marshal.h b/stubdom/vtpmmgr/marshal.h
index dce19c6439..f1037a7976 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,
@@ -920,8 +929,14 @@ inline int unpack3_TPM_RQU_HEADER(BYTE* ptr, UINT32* pos, UINT32 max,
unpack3_UINT32(ptr, pos, max, ord);
}
+static
+inline int sizeof_TPM_RQU_GetRandom(BYTE* ptr) {
+ return sizeof_TPM_RQU_HEADER(ptr) + sizeof_UINT32(ptr);
+}
+
#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..c879b24c13 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,64 @@ 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;
+ const int max_rand_size = TCPA_MAX_BUFFER_LENGTH -
+ sizeof_TPM_RQU_GetRandom(tpmcmd->req);
+ UINT32 rand_offset;
+ UINT32 rand_size;
+ TPM_COMMAND_CODE ord;
+ BYTE *p;
+
+ if (tpmcmd->req_len != sizeof_TPM_RQU_GetRandom(tpmcmd->req)) {
+ status = TPM_BAD_PARAMETER;
+ tag = TPM_TAG_RQU_COMMAND;
+ goto abort_egress;
+ }
+
+ 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;
+ }
+
+ /* TPM_GetRandom req: <header><uint32 num bytes> */
+ unpack_UINT32(p, &rand_size);
+
+ /* Returning fewer bytes is acceptable per the spec. */
+ if (rand_size > max_rand_size)
+ rand_size = max_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);
+
+ 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,7 +901,7 @@ TPM_RESULT vtpmmgr_handle_cmd(
switch(ord) {
case TPM_ORD_GetRandom:
vtpmloginfo(VTPM_LOG_VTPM, "Passthrough: TPM_GetRandom\n");
- break;
+ return vtpmmgr_handle_getrandom(opaque, tpmcmd);
case TPM_ORD_PcrRead:
vtpmloginfo(VTPM_LOG_VTPM, "Passthrough: TPM_PcrRead\n");
// Quotes also need to be restricted to hide PCR values
--
2.30.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH v2 09/13] vtpmmgr: Support GetRandom passthrough on TPM 2.0
2021-05-06 13:59 ` [PATCH v2 09/13] vtpmmgr: Support GetRandom passthrough on TPM 2.0 Jason Andryuk
@ 2021-05-06 21:40 ` Samuel Thibault
2021-05-10 12:51 ` Daniel P. Smith
1 sibling, 0 replies; 32+ messages in thread
From: Samuel Thibault @ 2021-05-06 21:40 UTC (permalink / raw)
To: Jason Andryuk; +Cc: xen-devel, Daniel De Graaf, Quan Xu
Jason Andryuk, le jeu. 06 mai 2021 09:59:19 -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@gmail.com>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
> ---
> v2:
> Add bounds and size checks
> Whitespace fixup
> ---
> stubdom/vtpmmgr/marshal.h | 15 ++++++++
> stubdom/vtpmmgr/vtpm_cmd_handler.c | 61 +++++++++++++++++++++++++++++-
> 2 files changed, 75 insertions(+), 1 deletion(-)
>
> diff --git a/stubdom/vtpmmgr/marshal.h b/stubdom/vtpmmgr/marshal.h
> index dce19c6439..f1037a7976 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,
> @@ -920,8 +929,14 @@ inline int unpack3_TPM_RQU_HEADER(BYTE* ptr, UINT32* pos, UINT32 max,
> unpack3_UINT32(ptr, pos, max, ord);
> }
>
> +static
> +inline int sizeof_TPM_RQU_GetRandom(BYTE* ptr) {
> + return sizeof_TPM_RQU_HEADER(ptr) + sizeof_UINT32(ptr);
> +}
> +
> #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..c879b24c13 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,64 @@ 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;
> + const int max_rand_size = TCPA_MAX_BUFFER_LENGTH -
> + sizeof_TPM_RQU_GetRandom(tpmcmd->req);
> + UINT32 rand_offset;
> + UINT32 rand_size;
> + TPM_COMMAND_CODE ord;
> + BYTE *p;
> +
> + if (tpmcmd->req_len != sizeof_TPM_RQU_GetRandom(tpmcmd->req)) {
> + status = TPM_BAD_PARAMETER;
> + tag = TPM_TAG_RQU_COMMAND;
> + goto abort_egress;
> + }
> +
> + 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;
> + }
> +
> + /* TPM_GetRandom req: <header><uint32 num bytes> */
> + unpack_UINT32(p, &rand_size);
> +
> + /* Returning fewer bytes is acceptable per the spec. */
> + if (rand_size > max_rand_size)
> + rand_size = max_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);
> +
> + 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,7 +901,7 @@ TPM_RESULT vtpmmgr_handle_cmd(
> switch(ord) {
> case TPM_ORD_GetRandom:
> vtpmloginfo(VTPM_LOG_VTPM, "Passthrough: TPM_GetRandom\n");
> - break;
> + return vtpmmgr_handle_getrandom(opaque, tpmcmd);
> case TPM_ORD_PcrRead:
> vtpmloginfo(VTPM_LOG_VTPM, "Passthrough: TPM_PcrRead\n");
> // Quotes also need to be restricted to hide PCR values
> --
> 2.30.2
>
--
Samuel
<i8b4uUnderground> d-_-b
<BonyNoMore> how u make that inverted b?
<BonyNoMore> wait
<BonyNoMore> never mind
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v2 09/13] vtpmmgr: Support GetRandom passthrough on TPM 2.0
2021-05-06 13:59 ` [PATCH v2 09/13] vtpmmgr: Support GetRandom passthrough on TPM 2.0 Jason Andryuk
2021-05-06 21:40 ` Samuel Thibault
@ 2021-05-10 12:51 ` Daniel P. Smith
1 sibling, 0 replies; 32+ messages in thread
From: Daniel P. Smith @ 2021-05-10 12:51 UTC (permalink / raw)
To: xen-devel
On 5/6/21 9:59 AM, Jason Andryuk wrote:
> 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@gmail.com>
>
> ---
> v2:
> Add bounds and size checks
> Whitespace fixup
> ---
Reviewed by: Daniel P. Smith <dpsmith@apertussolutions.com>
> stubdom/vtpmmgr/marshal.h | 15 ++++++++
> stubdom/vtpmmgr/vtpm_cmd_handler.c | 61 +++++++++++++++++++++++++++++-
> 2 files changed, 75 insertions(+), 1 deletion(-)
>
> diff --git a/stubdom/vtpmmgr/marshal.h b/stubdom/vtpmmgr/marshal.h
> index dce19c6439..f1037a7976 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,
> @@ -920,8 +929,14 @@ inline int unpack3_TPM_RQU_HEADER(BYTE* ptr, UINT32* pos, UINT32 max,
> unpack3_UINT32(ptr, pos, max, ord);
> }
>
> +static
> +inline int sizeof_TPM_RQU_GetRandom(BYTE* ptr) {
> + return sizeof_TPM_RQU_HEADER(ptr) + sizeof_UINT32(ptr);
> +}
> +
> #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..c879b24c13 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,64 @@ 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;
> + const int max_rand_size = TCPA_MAX_BUFFER_LENGTH -
> + sizeof_TPM_RQU_GetRandom(tpmcmd->req);
> + UINT32 rand_offset;
> + UINT32 rand_size;
> + TPM_COMMAND_CODE ord;
> + BYTE *p;
> +
> + if (tpmcmd->req_len != sizeof_TPM_RQU_GetRandom(tpmcmd->req)) {
> + status = TPM_BAD_PARAMETER;
> + tag = TPM_TAG_RQU_COMMAND;
> + goto abort_egress;
> + }
> +
> + 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;
> + }
> +
> + /* TPM_GetRandom req: <header><uint32 num bytes> */
> + unpack_UINT32(p, &rand_size);
> +
> + /* Returning fewer bytes is acceptable per the spec. */
> + if (rand_size > max_rand_size)
> + rand_size = max_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);
> +
> + 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,7 +901,7 @@ TPM_RESULT vtpmmgr_handle_cmd(
> switch(ord) {
> case TPM_ORD_GetRandom:
> vtpmloginfo(VTPM_LOG_VTPM, "Passthrough: TPM_GetRandom\n");
> - break;
> + return vtpmmgr_handle_getrandom(opaque, tpmcmd);
> case TPM_ORD_PcrRead:
> vtpmloginfo(VTPM_LOG_VTPM, "Passthrough: TPM_PcrRead\n");
> // Quotes also need to be restricted to hide PCR values
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 10/13] vtpmmgr: Remove bogus cast from TPM2_GetRandom
2021-05-06 13:59 [PATCH v2 00/13] vtpmmgr: Some fixes - still incomplete Jason Andryuk
` (8 preceding siblings ...)
2021-05-06 13:59 ` [PATCH v2 09/13] vtpmmgr: Support GetRandom passthrough on TPM 2.0 Jason Andryuk
@ 2021-05-06 13:59 ` Jason Andryuk
2021-05-06 21:41 ` Samuel Thibault
2021-05-10 13:03 ` Daniel P. Smith
2021-05-06 13:59 ` [PATCH v2 11/13] vtpmmgr: Fix owner_auth & srk_auth parsing Jason Andryuk
` (2 subsequent siblings)
12 siblings, 2 replies; 32+ messages in thread
From: Jason Andryuk @ 2021-05-06 13:59 UTC (permalink / raw)
To: xen-devel; +Cc: Jason Andryuk, Daniel De Graaf, Quan Xu, Samuel Thibault
The UINT32 <-> UINT16 casting in TPM2_GetRandom is incorrect. Use a
local UINT16 as needed for the TPM hardware command and assign the
result.
Suggested-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
---
stubdom/vtpmmgr/tpm2.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/stubdom/vtpmmgr/tpm2.c b/stubdom/vtpmmgr/tpm2.c
index 655e6d164c..ebd06eac74 100644
--- a/stubdom/vtpmmgr/tpm2.c
+++ b/stubdom/vtpmmgr/tpm2.c
@@ -427,15 +427,22 @@ abort_egress:
TPM_RC TPM2_GetRandom(UINT32 * bytesRequested, BYTE * randomBytes)
{
+ UINT16 bytesReq;
TPM_BEGIN(TPM_ST_NO_SESSIONS, TPM_CC_GetRandom);
- ptr = pack_UINT16(ptr, (UINT16)*bytesRequested);
+ if (*bytesRequested > UINT16_MAX)
+ bytesReq = UINT16_MAX;
+ else
+ bytesReq = *bytesRequested;
+
+ ptr = pack_UINT16(ptr, bytesReq);
TPM_TRANSMIT();
TPM_UNPACK_VERIFY();
- ptr = unpack_UINT16(ptr, (UINT16 *)bytesRequested);
- ptr = unpack_TPM_BUFFER(ptr, randomBytes, *bytesRequested);
+ ptr = unpack_UINT16(ptr, &bytesReq);
+ *bytesRequested = bytesReq;
+ ptr = unpack_TPM_BUFFER(ptr, randomBytes, bytesReq);
abort_egress:
return status;
--
2.30.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH v2 10/13] vtpmmgr: Remove bogus cast from TPM2_GetRandom
2021-05-06 13:59 ` [PATCH v2 10/13] vtpmmgr: Remove bogus cast from TPM2_GetRandom Jason Andryuk
@ 2021-05-06 21:41 ` Samuel Thibault
2021-05-10 13:03 ` Daniel P. Smith
1 sibling, 0 replies; 32+ messages in thread
From: Samuel Thibault @ 2021-05-06 21:41 UTC (permalink / raw)
To: Jason Andryuk; +Cc: xen-devel, Daniel De Graaf, Quan Xu
Jason Andryuk, le jeu. 06 mai 2021 09:59:20 -0400, a ecrit:
> The UINT32 <-> UINT16 casting in TPM2_GetRandom is incorrect. Use a
> local UINT16 as needed for the TPM hardware command and assign the
> result.
>
> Suggested-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
> Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
> ---
> stubdom/vtpmmgr/tpm2.c | 13 ++++++++++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/stubdom/vtpmmgr/tpm2.c b/stubdom/vtpmmgr/tpm2.c
> index 655e6d164c..ebd06eac74 100644
> --- a/stubdom/vtpmmgr/tpm2.c
> +++ b/stubdom/vtpmmgr/tpm2.c
> @@ -427,15 +427,22 @@ abort_egress:
>
> TPM_RC TPM2_GetRandom(UINT32 * bytesRequested, BYTE * randomBytes)
> {
> + UINT16 bytesReq;
> TPM_BEGIN(TPM_ST_NO_SESSIONS, TPM_CC_GetRandom);
>
> - ptr = pack_UINT16(ptr, (UINT16)*bytesRequested);
> + if (*bytesRequested > UINT16_MAX)
> + bytesReq = UINT16_MAX;
> + else
> + bytesReq = *bytesRequested;
> +
> + ptr = pack_UINT16(ptr, bytesReq);
>
> TPM_TRANSMIT();
> TPM_UNPACK_VERIFY();
>
> - ptr = unpack_UINT16(ptr, (UINT16 *)bytesRequested);
> - ptr = unpack_TPM_BUFFER(ptr, randomBytes, *bytesRequested);
> + ptr = unpack_UINT16(ptr, &bytesReq);
> + *bytesRequested = bytesReq;
> + ptr = unpack_TPM_BUFFER(ptr, randomBytes, bytesReq);
>
> abort_egress:
> return status;
> --
> 2.30.2
>
--
Samuel
<N> (* If you have a precise idea of the intended use of the following code, please
<N> write to Eduardo.Gimenez@inria.fr and ask for the prize :-)
<N> -- Eduardo (11/8/97) *)
-+- N sur #ens-mim - et c'était un des développeurs -+-
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v2 10/13] vtpmmgr: Remove bogus cast from TPM2_GetRandom
2021-05-06 13:59 ` [PATCH v2 10/13] vtpmmgr: Remove bogus cast from TPM2_GetRandom Jason Andryuk
2021-05-06 21:41 ` Samuel Thibault
@ 2021-05-10 13:03 ` Daniel P. Smith
1 sibling, 0 replies; 32+ messages in thread
From: Daniel P. Smith @ 2021-05-10 13:03 UTC (permalink / raw)
To: Jason Andryuk, xen-devel; +Cc: Daniel De Graaf, Quan Xu, Samuel Thibault
On 5/6/21 9:59 AM, Jason Andryuk wrote:
> The UINT32 <-> UINT16 casting in TPM2_GetRandom is incorrect. Use a
> local UINT16 as needed for the TPM hardware command and assign the
> result.
>
> Suggested-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
> Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
> ---
Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com>
> stubdom/vtpmmgr/tpm2.c | 13 ++++++++++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/stubdom/vtpmmgr/tpm2.c b/stubdom/vtpmmgr/tpm2.c
> index 655e6d164c..ebd06eac74 100644
> --- a/stubdom/vtpmmgr/tpm2.c
> +++ b/stubdom/vtpmmgr/tpm2.c
> @@ -427,15 +427,22 @@ abort_egress:
>
> TPM_RC TPM2_GetRandom(UINT32 * bytesRequested, BYTE * randomBytes)
> {
> + UINT16 bytesReq;
> TPM_BEGIN(TPM_ST_NO_SESSIONS, TPM_CC_GetRandom);
>
> - ptr = pack_UINT16(ptr, (UINT16)*bytesRequested);
> + if (*bytesRequested > UINT16_MAX)
> + bytesReq = UINT16_MAX;
> + else
> + bytesReq = *bytesRequested;
> +
> + ptr = pack_UINT16(ptr, bytesReq);
>
> TPM_TRANSMIT();
> TPM_UNPACK_VERIFY();
>
> - ptr = unpack_UINT16(ptr, (UINT16 *)bytesRequested);
> - ptr = unpack_TPM_BUFFER(ptr, randomBytes, *bytesRequested);
> + ptr = unpack_UINT16(ptr, &bytesReq);
> + *bytesRequested = bytesReq;
> + ptr = unpack_TPM_BUFFER(ptr, randomBytes, bytesReq);
>
> abort_egress:
> return status;
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 11/13] vtpmmgr: Fix owner_auth & srk_auth parsing
2021-05-06 13:59 [PATCH v2 00/13] vtpmmgr: Some fixes - still incomplete Jason Andryuk
` (9 preceding siblings ...)
2021-05-06 13:59 ` [PATCH v2 10/13] vtpmmgr: Remove bogus cast from TPM2_GetRandom Jason Andryuk
@ 2021-05-06 13:59 ` Jason Andryuk
2021-05-06 21:41 ` Samuel Thibault
2021-05-10 13:18 ` Daniel P. Smith
2021-05-06 13:59 ` [PATCH v2 12/13] vtpmmgr: Check req_len before unpacking command Jason Andryuk
2021-05-06 13:59 ` [PATCH v2 13/13] vtpm: Correct timeout units and command duration Jason Andryuk
12 siblings, 2 replies; 32+ messages in thread
From: Jason Andryuk @ 2021-05-06 13:59 UTC (permalink / raw)
To: xen-devel; +Cc: Jason Andryuk, Daniel De Graaf, Quan Xu, Samuel Thibault
Argument parsing only matches to before ':' and then the string with
leading ':' is passed to parse_auth_string which fails to parse. Extend
the length to include the seperator in the match.
While here, switch the seperator to "=". The man page documented "="
and the other tpm.* arguments already use "=". Since it didn't work
before, we don't need to worry about backwards compatibility.
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
---
stubdom/vtpmmgr/init.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/stubdom/vtpmmgr/init.c b/stubdom/vtpmmgr/init.c
index 4ae34a4fcb..62dc5994de 100644
--- a/stubdom/vtpmmgr/init.c
+++ b/stubdom/vtpmmgr/init.c
@@ -289,16 +289,16 @@ int parse_cmdline_opts(int argc, char** argv, struct Opts* opts)
memcpy(vtpm_globals.srk_auth, WELLKNOWN_AUTH, sizeof(TPM_AUTHDATA));
for(i = 1; i < argc; ++i) {
- if(!strncmp(argv[i], "owner_auth:", 10)) {
- if((rc = parse_auth_string(argv[i] + 10, vtpm_globals.owner_auth)) < 0) {
+ if(!strncmp(argv[i], "owner_auth=", 11)) {
+ if((rc = parse_auth_string(argv[i] + 11, vtpm_globals.owner_auth)) < 0) {
goto err_invalid;
}
if(rc == 1) {
opts->gen_owner_auth = 1;
}
}
- else if(!strncmp(argv[i], "srk_auth:", 8)) {
- if((rc = parse_auth_string(argv[i] + 8, vtpm_globals.srk_auth)) != 0) {
+ else if(!strncmp(argv[i], "srk_auth=", 9)) {
+ if((rc = parse_auth_string(argv[i] + 9, vtpm_globals.srk_auth)) != 0) {
goto err_invalid;
}
}
--
2.30.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH v2 11/13] vtpmmgr: Fix owner_auth & srk_auth parsing
2021-05-06 13:59 ` [PATCH v2 11/13] vtpmmgr: Fix owner_auth & srk_auth parsing Jason Andryuk
@ 2021-05-06 21:41 ` Samuel Thibault
2021-05-10 13:18 ` Daniel P. Smith
1 sibling, 0 replies; 32+ messages in thread
From: Samuel Thibault @ 2021-05-06 21:41 UTC (permalink / raw)
To: Jason Andryuk; +Cc: xen-devel, Daniel De Graaf, Quan Xu
Jason Andryuk, le jeu. 06 mai 2021 09:59:21 -0400, a ecrit:
> Argument parsing only matches to before ':' and then the string with
> leading ':' is passed to parse_auth_string which fails to parse. Extend
> the length to include the seperator in the match.
>
> While here, switch the seperator to "=". The man page documented "="
> and the other tpm.* arguments already use "=". Since it didn't work
> before, we don't need to worry about backwards compatibility.
>
> Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
> ---
> stubdom/vtpmmgr/init.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/stubdom/vtpmmgr/init.c b/stubdom/vtpmmgr/init.c
> index 4ae34a4fcb..62dc5994de 100644
> --- a/stubdom/vtpmmgr/init.c
> +++ b/stubdom/vtpmmgr/init.c
> @@ -289,16 +289,16 @@ int parse_cmdline_opts(int argc, char** argv, struct Opts* opts)
> memcpy(vtpm_globals.srk_auth, WELLKNOWN_AUTH, sizeof(TPM_AUTHDATA));
>
> for(i = 1; i < argc; ++i) {
> - if(!strncmp(argv[i], "owner_auth:", 10)) {
> - if((rc = parse_auth_string(argv[i] + 10, vtpm_globals.owner_auth)) < 0) {
> + if(!strncmp(argv[i], "owner_auth=", 11)) {
> + if((rc = parse_auth_string(argv[i] + 11, vtpm_globals.owner_auth)) < 0) {
> goto err_invalid;
> }
> if(rc == 1) {
> opts->gen_owner_auth = 1;
> }
> }
> - else if(!strncmp(argv[i], "srk_auth:", 8)) {
> - if((rc = parse_auth_string(argv[i] + 8, vtpm_globals.srk_auth)) != 0) {
> + else if(!strncmp(argv[i], "srk_auth=", 9)) {
> + if((rc = parse_auth_string(argv[i] + 9, vtpm_globals.srk_auth)) != 0) {
> goto err_invalid;
> }
> }
> --
> 2.30.2
>
--
Samuel
c> ah (on trouve fluide glacial sur le net, ou il faut aller dans le monde reel ?)
s> dans le monde reel
c> zut
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v2 11/13] vtpmmgr: Fix owner_auth & srk_auth parsing
2021-05-06 13:59 ` [PATCH v2 11/13] vtpmmgr: Fix owner_auth & srk_auth parsing Jason Andryuk
2021-05-06 21:41 ` Samuel Thibault
@ 2021-05-10 13:18 ` Daniel P. Smith
1 sibling, 0 replies; 32+ messages in thread
From: Daniel P. Smith @ 2021-05-10 13:18 UTC (permalink / raw)
To: Jason Andryuk, xen-devel; +Cc: Daniel De Graaf, Quan Xu, Samuel Thibault
On 5/6/21 9:59 AM, Jason Andryuk wrote:
> Argument parsing only matches to before ':' and then the string with
> leading ':' is passed to parse_auth_string which fails to parse. Extend
> the length to include the seperator in the match.
>
> While here, switch the seperator to "=". The man page documented "="
> and the other tpm.* arguments already use "=". Since it didn't work
> before, we don't need to worry about backwards compatibility.
>
> Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
> ---
Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com>
> stubdom/vtpmmgr/init.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/stubdom/vtpmmgr/init.c b/stubdom/vtpmmgr/init.c
> index 4ae34a4fcb..62dc5994de 100644
> --- a/stubdom/vtpmmgr/init.c
> +++ b/stubdom/vtpmmgr/init.c
> @@ -289,16 +289,16 @@ int parse_cmdline_opts(int argc, char** argv, struct Opts* opts)
> memcpy(vtpm_globals.srk_auth, WELLKNOWN_AUTH, sizeof(TPM_AUTHDATA));
>
> for(i = 1; i < argc; ++i) {
> - if(!strncmp(argv[i], "owner_auth:", 10)) {
> - if((rc = parse_auth_string(argv[i] + 10, vtpm_globals.owner_auth)) < 0) {
> + if(!strncmp(argv[i], "owner_auth=", 11)) {
> + if((rc = parse_auth_string(argv[i] + 11, vtpm_globals.owner_auth)) < 0) {
> goto err_invalid;
> }
> if(rc == 1) {
> opts->gen_owner_auth = 1;
> }
> }
> - else if(!strncmp(argv[i], "srk_auth:", 8)) {
> - if((rc = parse_auth_string(argv[i] + 8, vtpm_globals.srk_auth)) != 0) {
> + else if(!strncmp(argv[i], "srk_auth=", 9)) {
> + if((rc = parse_auth_string(argv[i] + 9, vtpm_globals.srk_auth)) != 0) {
> goto err_invalid;
> }
> }
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 12/13] vtpmmgr: Check req_len before unpacking command
2021-05-06 13:59 [PATCH v2 00/13] vtpmmgr: Some fixes - still incomplete Jason Andryuk
` (10 preceding siblings ...)
2021-05-06 13:59 ` [PATCH v2 11/13] vtpmmgr: Fix owner_auth & srk_auth parsing Jason Andryuk
@ 2021-05-06 13:59 ` Jason Andryuk
2021-05-06 21:42 ` Samuel Thibault
2021-05-10 13:32 ` Daniel P. Smith
2021-05-06 13:59 ` [PATCH v2 13/13] vtpm: Correct timeout units and command duration Jason Andryuk
12 siblings, 2 replies; 32+ messages in thread
From: Jason Andryuk @ 2021-05-06 13:59 UTC (permalink / raw)
To: xen-devel; +Cc: Jason Andryuk, Daniel De Graaf, Quan Xu, Samuel Thibault
vtpm_handle_cmd doesn't ensure there is enough space before unpacking
the req buffer. Add a minimum size check. Called functions will have
to do their own checking if they need more data from the request.
The error case is tricky since abort_egress wants to rely with a
corresponding tag. Just hardcode TPM_TAG_RQU_COMMAND since the vtpm is
sending in malformed commands in the first place.
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
---
stubdom/vtpmmgr/vtpm_cmd_handler.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/stubdom/vtpmmgr/vtpm_cmd_handler.c b/stubdom/vtpmmgr/vtpm_cmd_handler.c
index c879b24c13..5586be6997 100644
--- a/stubdom/vtpmmgr/vtpm_cmd_handler.c
+++ b/stubdom/vtpmmgr/vtpm_cmd_handler.c
@@ -840,6 +840,12 @@ TPM_RESULT vtpmmgr_handle_cmd(
UINT32 size;
TPM_COMMAND_CODE ord;
+ if (tpmcmd->req_len < sizeof_TPM_RQU_HEADER(tpmcmd->req)) {
+ status = TPM_BAD_PARAMETER;
+ tag = TPM_TAG_RQU_COMMAND;
+ goto abort_egress;
+ }
+
unpack_TPM_RQU_HEADER(tpmcmd->req,
&tag, &size, &ord);
--
2.30.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH v2 12/13] vtpmmgr: Check req_len before unpacking command
2021-05-06 13:59 ` [PATCH v2 12/13] vtpmmgr: Check req_len before unpacking command Jason Andryuk
@ 2021-05-06 21:42 ` Samuel Thibault
2021-05-10 13:32 ` Daniel P. Smith
1 sibling, 0 replies; 32+ messages in thread
From: Samuel Thibault @ 2021-05-06 21:42 UTC (permalink / raw)
To: Jason Andryuk; +Cc: xen-devel, Daniel De Graaf, Quan Xu
Jason Andryuk, le jeu. 06 mai 2021 09:59:22 -0400, a ecrit:
> vtpm_handle_cmd doesn't ensure there is enough space before unpacking
> the req buffer. Add a minimum size check. Called functions will have
> to do their own checking if they need more data from the request.
>
> The error case is tricky since abort_egress wants to rely with a
> corresponding tag. Just hardcode TPM_TAG_RQU_COMMAND since the vtpm is
> sending in malformed commands in the first place.
>
> Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
> ---
> stubdom/vtpmmgr/vtpm_cmd_handler.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/stubdom/vtpmmgr/vtpm_cmd_handler.c b/stubdom/vtpmmgr/vtpm_cmd_handler.c
> index c879b24c13..5586be6997 100644
> --- a/stubdom/vtpmmgr/vtpm_cmd_handler.c
> +++ b/stubdom/vtpmmgr/vtpm_cmd_handler.c
> @@ -840,6 +840,12 @@ TPM_RESULT vtpmmgr_handle_cmd(
> UINT32 size;
> TPM_COMMAND_CODE ord;
>
> + if (tpmcmd->req_len < sizeof_TPM_RQU_HEADER(tpmcmd->req)) {
> + status = TPM_BAD_PARAMETER;
> + tag = TPM_TAG_RQU_COMMAND;
> + goto abort_egress;
> + }
> +
> unpack_TPM_RQU_HEADER(tpmcmd->req,
> &tag, &size, &ord);
>
> --
> 2.30.2
>
--
Samuel
j'etais en train de nettoyer ma souris et le coup est parti...
-+- s sur #ens-mim - et en plus c vrai... -+-
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v2 12/13] vtpmmgr: Check req_len before unpacking command
2021-05-06 13:59 ` [PATCH v2 12/13] vtpmmgr: Check req_len before unpacking command Jason Andryuk
2021-05-06 21:42 ` Samuel Thibault
@ 2021-05-10 13:32 ` Daniel P. Smith
1 sibling, 0 replies; 32+ messages in thread
From: Daniel P. Smith @ 2021-05-10 13:32 UTC (permalink / raw)
To: Jason Andryuk, xen-devel; +Cc: Daniel De Graaf, Quan Xu, Samuel Thibault
On 5/6/21 9:59 AM, Jason Andryuk wrote:
> vtpm_handle_cmd doesn't ensure there is enough space before unpacking
> the req buffer. Add a minimum size check. Called functions will have
> to do their own checking if they need more data from the request.
>
> The error case is tricky since abort_egress wants to rely with a
> corresponding tag. Just hardcode TPM_TAG_RQU_COMMAND since the vtpm is
> sending in malformed commands in the first place.
>
> Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
> ---
Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com>
> stubdom/vtpmmgr/vtpm_cmd_handler.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/stubdom/vtpmmgr/vtpm_cmd_handler.c b/stubdom/vtpmmgr/vtpm_cmd_handler.c
> index c879b24c13..5586be6997 100644
> --- a/stubdom/vtpmmgr/vtpm_cmd_handler.c
> +++ b/stubdom/vtpmmgr/vtpm_cmd_handler.c
> @@ -840,6 +840,12 @@ TPM_RESULT vtpmmgr_handle_cmd(
> UINT32 size;
> TPM_COMMAND_CODE ord;
>
> + if (tpmcmd->req_len < sizeof_TPM_RQU_HEADER(tpmcmd->req)) {
> + status = TPM_BAD_PARAMETER;
> + tag = TPM_TAG_RQU_COMMAND;
> + goto abort_egress;
> + }
> +
> unpack_TPM_RQU_HEADER(tpmcmd->req,
> &tag, &size, &ord);
>
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 13/13] vtpm: Correct timeout units and command duration
2021-05-06 13:59 [PATCH v2 00/13] vtpmmgr: Some fixes - still incomplete Jason Andryuk
` (11 preceding siblings ...)
2021-05-06 13:59 ` [PATCH v2 12/13] vtpmmgr: Check req_len before unpacking command Jason Andryuk
@ 2021-05-06 13:59 ` Jason Andryuk
2021-05-06 21:52 ` Samuel Thibault
2021-05-10 13:40 ` Daniel P. Smith
12 siblings, 2 replies; 32+ messages in thread
From: Jason Andryuk @ 2021-05-06 13:59 UTC (permalink / raw)
To: xen-devel; +Cc: Jason Andryuk, Ian Jackson, Wei Liu, Samuel Thibault
Add two patches:
vtpm-microsecond-duration.patch fixes the units for timeouts and command
durations.
vtpm-command-duration.patch increases the timeout linux uses to allow
commands to succeed.
Linux works around low timeouts, but not low durations. The second
patch allows commands to complete that often timeout with the lower
command durations.
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
---
stubdom/Makefile | 2 +
stubdom/vtpm-command-duration.patch | 52 +++++++++++++++++++++++++
stubdom/vtpm-microsecond-duration.patch | 52 +++++++++++++++++++++++++
3 files changed, 106 insertions(+)
create mode 100644 stubdom/vtpm-command-duration.patch
create mode 100644 stubdom/vtpm-microsecond-duration.patch
diff --git a/stubdom/Makefile b/stubdom/Makefile
index c6de5f68ae..06aa69d8bc 100644
--- a/stubdom/Makefile
+++ b/stubdom/Makefile
@@ -239,6 +239,8 @@ tpm_emulator-$(XEN_TARGET_ARCH): tpm_emulator-$(TPMEMU_VERSION).tar.gz
patch -d $@ -p1 < vtpm-implicit-fallthrough.patch
patch -d $@ -p1 < vtpm_TPM_ChangeAuthAsymFinish.patch
patch -d $@ -p1 < vtpm_extern.patch
+ patch -d $@ -p1 < vtpm-microsecond-duration.patch
+ patch -d $@ -p1 < vtpm-command-duration.patch
mkdir $@/build
cd $@/build; CC=${CC} $(CMAKE) .. -DCMAKE_C_FLAGS:STRING="-std=c99 -DTPM_NO_EXTERN $(TARGET_CPPFLAGS) $(TARGET_CFLAGS) -Wno-declaration-after-statement"
touch $@
diff --git a/stubdom/vtpm-command-duration.patch b/stubdom/vtpm-command-duration.patch
new file mode 100644
index 0000000000..6fdf2fc9be
--- /dev/null
+++ b/stubdom/vtpm-command-duration.patch
@@ -0,0 +1,52 @@
+From e7c976b5864e7d2649292d90ea60d5aea091a990 Mon Sep 17 00:00:00 2001
+From: Jason Andryuk <jandryuk@gmail.com>
+Date: Sun, 14 Mar 2021 12:46:34 -0400
+Subject: [PATCH 2/2] Increase command durations
+
+Wth Linux 5.4 xen-tpmfront and a Xen vtpm-stubdom, xen-tpmfront was
+failing commands with -ETIME:
+tpm tpm0: tpm_try_transmit: send(): error-62
+
+The vtpm was returning the data, but it was after the duration timeout
+in vtpm_send. Linux may have started being more stringent about timing?
+
+The vtpm-stubdom has a little delay since it writes its disk before
+returning the response.
+
+Anyway, the durations are rather low. When they were 1/10/1000 before
+converting to microseconds, Linux showed all three durations rounded to
+10000. Update them with values from a physical TPM1.2. These were
+taken from a WEC which was software downgraded from a TPM2 to a TPM1.2.
+They might be excessive, but I'd rather have a command succeed than
+return -ETIME.
+
+An IFX physical TPM1.2 uses:
+1000000
+1500000
+150000000
+
+Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
+---
+ tpm/tpm_data.c | 6 +++---
+ 1 file changed, 3 insertions(+), 3 deletions(-)
+
+diff --git a/tpm/tpm_data.c b/tpm/tpm_data.c
+index bebaf10..844afca 100644
+--- a/tpm/tpm_data.c
++++ b/tpm/tpm_data.c
+@@ -71,9 +71,9 @@ static void init_timeouts(void)
+ tpmData.permanent.data.tis_timeouts[1] = 2000000;
+ tpmData.permanent.data.tis_timeouts[2] = 750000;
+ tpmData.permanent.data.tis_timeouts[3] = 750000;
+- tpmData.permanent.data.cmd_durations[0] = 1000;
+- tpmData.permanent.data.cmd_durations[1] = 10000;
+- tpmData.permanent.data.cmd_durations[2] = 1000000;
++ tpmData.permanent.data.cmd_durations[0] = 3000000;
++ tpmData.permanent.data.cmd_durations[1] = 3000000;
++ tpmData.permanent.data.cmd_durations[2] = 600000000;
+ }
+
+ void tpm_init_data(void)
+--
+2.30.2
+
diff --git a/stubdom/vtpm-microsecond-duration.patch b/stubdom/vtpm-microsecond-duration.patch
new file mode 100644
index 0000000000..7a906e72c5
--- /dev/null
+++ b/stubdom/vtpm-microsecond-duration.patch
@@ -0,0 +1,52 @@
+From 5a510e0afd7c288e3f0fb3523ec749ba1366ad61 Mon Sep 17 00:00:00 2001
+From: Jason Andryuk <jandryuk@gmail.com>
+Date: Sun, 14 Mar 2021 12:42:10 -0400
+Subject: [PATCH 1/2] Use microseconds for timeouts and durations
+
+The timeout and duration fields should be in microseconds according to
+the spec.
+
+TPM_CAP_PROP_TIS_TIMEOUT:
+A 4 element array of UINT32 values each denoting the timeout value in
+microseconds for the following in this order:
+
+TPM_CAP_PROP_DURATION:
+A 3 element array of UINT32 values each denoting the duration value in
+microseconds of the duration of the three classes of commands:
+
+Linux will scale the timeouts up by 1000, but not the durations. Change
+the units for both sets as appropriate.
+
+Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
+---
+ tpm/tpm_data.c | 14 +++++++-------
+ 1 file changed, 7 insertions(+), 7 deletions(-)
+
+diff --git a/tpm/tpm_data.c b/tpm/tpm_data.c
+index a3a79ef..bebaf10 100644
+--- a/tpm/tpm_data.c
++++ b/tpm/tpm_data.c
+@@ -67,13 +67,13 @@ static void init_nv_storage(void)
+ static void init_timeouts(void)
+ {
+ /* for the timeouts we use the PC platform defaults */
+- tpmData.permanent.data.tis_timeouts[0] = 750;
+- tpmData.permanent.data.tis_timeouts[1] = 2000;
+- tpmData.permanent.data.tis_timeouts[2] = 750;
+- tpmData.permanent.data.tis_timeouts[3] = 750;
+- tpmData.permanent.data.cmd_durations[0] = 1;
+- tpmData.permanent.data.cmd_durations[1] = 10;
+- tpmData.permanent.data.cmd_durations[2] = 1000;
++ tpmData.permanent.data.tis_timeouts[0] = 750000;
++ tpmData.permanent.data.tis_timeouts[1] = 2000000;
++ tpmData.permanent.data.tis_timeouts[2] = 750000;
++ tpmData.permanent.data.tis_timeouts[3] = 750000;
++ tpmData.permanent.data.cmd_durations[0] = 1000;
++ tpmData.permanent.data.cmd_durations[1] = 10000;
++ tpmData.permanent.data.cmd_durations[2] = 1000000;
+ }
+
+ void tpm_init_data(void)
+--
+2.30.2
+
--
2.30.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH v2 13/13] vtpm: Correct timeout units and command duration
2021-05-06 13:59 ` [PATCH v2 13/13] vtpm: Correct timeout units and command duration Jason Andryuk
@ 2021-05-06 21:52 ` Samuel Thibault
2021-05-10 13:40 ` Daniel P. Smith
1 sibling, 0 replies; 32+ messages in thread
From: Samuel Thibault @ 2021-05-06 21:52 UTC (permalink / raw)
To: Jason Andryuk; +Cc: xen-devel, Ian Jackson, Wei Liu
Jason Andryuk, le jeu. 06 mai 2021 09:59:23 -0400, a ecrit:
> Add two patches:
> vtpm-microsecond-duration.patch fixes the units for timeouts and command
> durations.
> vtpm-command-duration.patch increases the timeout linux uses to allow
> commands to succeed.
>
> Linux works around low timeouts, but not low durations. The second
> patch allows commands to complete that often timeout with the lower
> command durations.
>
> Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
> ---
> stubdom/Makefile | 2 +
> stubdom/vtpm-command-duration.patch | 52 +++++++++++++++++++++++++
> stubdom/vtpm-microsecond-duration.patch | 52 +++++++++++++++++++++++++
> 3 files changed, 106 insertions(+)
> create mode 100644 stubdom/vtpm-command-duration.patch
> create mode 100644 stubdom/vtpm-microsecond-duration.patch
>
> diff --git a/stubdom/Makefile b/stubdom/Makefile
> index c6de5f68ae..06aa69d8bc 100644
> --- a/stubdom/Makefile
> +++ b/stubdom/Makefile
> @@ -239,6 +239,8 @@ tpm_emulator-$(XEN_TARGET_ARCH): tpm_emulator-$(TPMEMU_VERSION).tar.gz
> patch -d $@ -p1 < vtpm-implicit-fallthrough.patch
> patch -d $@ -p1 < vtpm_TPM_ChangeAuthAsymFinish.patch
> patch -d $@ -p1 < vtpm_extern.patch
> + patch -d $@ -p1 < vtpm-microsecond-duration.patch
> + patch -d $@ -p1 < vtpm-command-duration.patch
> mkdir $@/build
> cd $@/build; CC=${CC} $(CMAKE) .. -DCMAKE_C_FLAGS:STRING="-std=c99 -DTPM_NO_EXTERN $(TARGET_CPPFLAGS) $(TARGET_CFLAGS) -Wno-declaration-after-statement"
> touch $@
> diff --git a/stubdom/vtpm-command-duration.patch b/stubdom/vtpm-command-duration.patch
> new file mode 100644
> index 0000000000..6fdf2fc9be
> --- /dev/null
> +++ b/stubdom/vtpm-command-duration.patch
> @@ -0,0 +1,52 @@
> +From e7c976b5864e7d2649292d90ea60d5aea091a990 Mon Sep 17 00:00:00 2001
> +From: Jason Andryuk <jandryuk@gmail.com>
> +Date: Sun, 14 Mar 2021 12:46:34 -0400
> +Subject: [PATCH 2/2] Increase command durations
> +
> +Wth Linux 5.4 xen-tpmfront and a Xen vtpm-stubdom, xen-tpmfront was
> +failing commands with -ETIME:
> +tpm tpm0: tpm_try_transmit: send(): error-62
> +
> +The vtpm was returning the data, but it was after the duration timeout
> +in vtpm_send. Linux may have started being more stringent about timing?
> +
> +The vtpm-stubdom has a little delay since it writes its disk before
> +returning the response.
> +
> +Anyway, the durations are rather low. When they were 1/10/1000 before
> +converting to microseconds, Linux showed all three durations rounded to
> +10000. Update them with values from a physical TPM1.2. These were
> +taken from a WEC which was software downgraded from a TPM2 to a TPM1.2.
> +They might be excessive, but I'd rather have a command succeed than
> +return -ETIME.
> +
> +An IFX physical TPM1.2 uses:
> +1000000
> +1500000
> +150000000
> +
> +Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
> +---
> + tpm/tpm_data.c | 6 +++---
> + 1 file changed, 3 insertions(+), 3 deletions(-)
> +
> +diff --git a/tpm/tpm_data.c b/tpm/tpm_data.c
> +index bebaf10..844afca 100644
> +--- a/tpm/tpm_data.c
> ++++ b/tpm/tpm_data.c
> +@@ -71,9 +71,9 @@ static void init_timeouts(void)
> + tpmData.permanent.data.tis_timeouts[1] = 2000000;
> + tpmData.permanent.data.tis_timeouts[2] = 750000;
> + tpmData.permanent.data.tis_timeouts[3] = 750000;
> +- tpmData.permanent.data.cmd_durations[0] = 1000;
> +- tpmData.permanent.data.cmd_durations[1] = 10000;
> +- tpmData.permanent.data.cmd_durations[2] = 1000000;
> ++ tpmData.permanent.data.cmd_durations[0] = 3000000;
> ++ tpmData.permanent.data.cmd_durations[1] = 3000000;
> ++ tpmData.permanent.data.cmd_durations[2] = 600000000;
> + }
> +
> + void tpm_init_data(void)
> +--
> +2.30.2
> +
> diff --git a/stubdom/vtpm-microsecond-duration.patch b/stubdom/vtpm-microsecond-duration.patch
> new file mode 100644
> index 0000000000..7a906e72c5
> --- /dev/null
> +++ b/stubdom/vtpm-microsecond-duration.patch
> @@ -0,0 +1,52 @@
> +From 5a510e0afd7c288e3f0fb3523ec749ba1366ad61 Mon Sep 17 00:00:00 2001
> +From: Jason Andryuk <jandryuk@gmail.com>
> +Date: Sun, 14 Mar 2021 12:42:10 -0400
> +Subject: [PATCH 1/2] Use microseconds for timeouts and durations
> +
> +The timeout and duration fields should be in microseconds according to
> +the spec.
> +
> +TPM_CAP_PROP_TIS_TIMEOUT:
> +A 4 element array of UINT32 values each denoting the timeout value in
> +microseconds for the following in this order:
> +
> +TPM_CAP_PROP_DURATION:
> +A 3 element array of UINT32 values each denoting the duration value in
> +microseconds of the duration of the three classes of commands:
> +
> +Linux will scale the timeouts up by 1000, but not the durations. Change
> +the units for both sets as appropriate.
> +
> +Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
> +---
> + tpm/tpm_data.c | 14 +++++++-------
> + 1 file changed, 7 insertions(+), 7 deletions(-)
> +
> +diff --git a/tpm/tpm_data.c b/tpm/tpm_data.c
> +index a3a79ef..bebaf10 100644
> +--- a/tpm/tpm_data.c
> ++++ b/tpm/tpm_data.c
> +@@ -67,13 +67,13 @@ static void init_nv_storage(void)
> + static void init_timeouts(void)
> + {
> + /* for the timeouts we use the PC platform defaults */
> +- tpmData.permanent.data.tis_timeouts[0] = 750;
> +- tpmData.permanent.data.tis_timeouts[1] = 2000;
> +- tpmData.permanent.data.tis_timeouts[2] = 750;
> +- tpmData.permanent.data.tis_timeouts[3] = 750;
> +- tpmData.permanent.data.cmd_durations[0] = 1;
> +- tpmData.permanent.data.cmd_durations[1] = 10;
> +- tpmData.permanent.data.cmd_durations[2] = 1000;
> ++ tpmData.permanent.data.tis_timeouts[0] = 750000;
> ++ tpmData.permanent.data.tis_timeouts[1] = 2000000;
> ++ tpmData.permanent.data.tis_timeouts[2] = 750000;
> ++ tpmData.permanent.data.tis_timeouts[3] = 750000;
> ++ tpmData.permanent.data.cmd_durations[0] = 1000;
> ++ tpmData.permanent.data.cmd_durations[1] = 10000;
> ++ tpmData.permanent.data.cmd_durations[2] = 1000000;
> + }
> +
> + void tpm_init_data(void)
> +--
> +2.30.2
> +
> --
> 2.30.2
>
--
Samuel
Pour un père, autant mourir que de faire plein de calculs et pas s'occuper
de son fils
-+- y sur #ens-mim - sombres histoires de zombies -+-
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v2 13/13] vtpm: Correct timeout units and command duration
2021-05-06 13:59 ` [PATCH v2 13/13] vtpm: Correct timeout units and command duration Jason Andryuk
2021-05-06 21:52 ` Samuel Thibault
@ 2021-05-10 13:40 ` Daniel P. Smith
1 sibling, 0 replies; 32+ messages in thread
From: Daniel P. Smith @ 2021-05-10 13:40 UTC (permalink / raw)
To: Jason Andryuk, xen-devel; +Cc: Ian Jackson, Wei Liu, Samuel Thibault
On 5/6/21 9:59 AM, Jason Andryuk wrote:
> Add two patches:
> vtpm-microsecond-duration.patch fixes the units for timeouts and command
> durations.
> vtpm-command-duration.patch increases the timeout linux uses to allow
> commands to succeed.
>
> Linux works around low timeouts, but not low durations. The second
> patch allows commands to complete that often timeout with the lower
> command durations.
>
> Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
> ---
Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com>
> stubdom/Makefile | 2 +
> stubdom/vtpm-command-duration.patch | 52 +++++++++++++++++++++++++
> stubdom/vtpm-microsecond-duration.patch | 52 +++++++++++++++++++++++++
> 3 files changed, 106 insertions(+)
> create mode 100644 stubdom/vtpm-command-duration.patch
> create mode 100644 stubdom/vtpm-microsecond-duration.patch
>
> diff --git a/stubdom/Makefile b/stubdom/Makefile
> index c6de5f68ae..06aa69d8bc 100644
> --- a/stubdom/Makefile
> +++ b/stubdom/Makefile
> @@ -239,6 +239,8 @@ tpm_emulator-$(XEN_TARGET_ARCH): tpm_emulator-$(TPMEMU_VERSION).tar.gz
> patch -d $@ -p1 < vtpm-implicit-fallthrough.patch
> patch -d $@ -p1 < vtpm_TPM_ChangeAuthAsymFinish.patch
> patch -d $@ -p1 < vtpm_extern.patch
> + patch -d $@ -p1 < vtpm-microsecond-duration.patch
> + patch -d $@ -p1 < vtpm-command-duration.patch
> mkdir $@/build
> cd $@/build; CC=${CC} $(CMAKE) .. -DCMAKE_C_FLAGS:STRING="-std=c99 -DTPM_NO_EXTERN $(TARGET_CPPFLAGS) $(TARGET_CFLAGS) -Wno-declaration-after-statement"
> touch $@
> diff --git a/stubdom/vtpm-command-duration.patch b/stubdom/vtpm-command-duration.patch
> new file mode 100644
> index 0000000000..6fdf2fc9be
> --- /dev/null
> +++ b/stubdom/vtpm-command-duration.patch
> @@ -0,0 +1,52 @@
> +From e7c976b5864e7d2649292d90ea60d5aea091a990 Mon Sep 17 00:00:00 2001
> +From: Jason Andryuk <jandryuk@gmail.com>
> +Date: Sun, 14 Mar 2021 12:46:34 -0400
> +Subject: [PATCH 2/2] Increase command durations
> +
> +Wth Linux 5.4 xen-tpmfront and a Xen vtpm-stubdom, xen-tpmfront was
> +failing commands with -ETIME:
> +tpm tpm0: tpm_try_transmit: send(): error-62
> +
> +The vtpm was returning the data, but it was after the duration timeout
> +in vtpm_send. Linux may have started being more stringent about timing?
> +
> +The vtpm-stubdom has a little delay since it writes its disk before
> +returning the response.
> +
> +Anyway, the durations are rather low. When they were 1/10/1000 before
> +converting to microseconds, Linux showed all three durations rounded to
> +10000. Update them with values from a physical TPM1.2. These were
> +taken from a WEC which was software downgraded from a TPM2 to a TPM1.2.
> +They might be excessive, but I'd rather have a command succeed than
> +return -ETIME.
> +
> +An IFX physical TPM1.2 uses:
> +1000000
> +1500000
> +150000000
> +
> +Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
> +---
> + tpm/tpm_data.c | 6 +++---
> + 1 file changed, 3 insertions(+), 3 deletions(-)
> +
> +diff --git a/tpm/tpm_data.c b/tpm/tpm_data.c
> +index bebaf10..844afca 100644
> +--- a/tpm/tpm_data.c
> ++++ b/tpm/tpm_data.c
> +@@ -71,9 +71,9 @@ static void init_timeouts(void)
> + tpmData.permanent.data.tis_timeouts[1] = 2000000;
> + tpmData.permanent.data.tis_timeouts[2] = 750000;
> + tpmData.permanent.data.tis_timeouts[3] = 750000;
> +- tpmData.permanent.data.cmd_durations[0] = 1000;
> +- tpmData.permanent.data.cmd_durations[1] = 10000;
> +- tpmData.permanent.data.cmd_durations[2] = 1000000;
> ++ tpmData.permanent.data.cmd_durations[0] = 3000000;
> ++ tpmData.permanent.data.cmd_durations[1] = 3000000;
> ++ tpmData.permanent.data.cmd_durations[2] = 600000000;
> + }
> +
> + void tpm_init_data(void)
> +--
> +2.30.2
> +
> diff --git a/stubdom/vtpm-microsecond-duration.patch b/stubdom/vtpm-microsecond-duration.patch
> new file mode 100644
> index 0000000000..7a906e72c5
> --- /dev/null
> +++ b/stubdom/vtpm-microsecond-duration.patch
> @@ -0,0 +1,52 @@
> +From 5a510e0afd7c288e3f0fb3523ec749ba1366ad61 Mon Sep 17 00:00:00 2001
> +From: Jason Andryuk <jandryuk@gmail.com>
> +Date: Sun, 14 Mar 2021 12:42:10 -0400
> +Subject: [PATCH 1/2] Use microseconds for timeouts and durations
> +
> +The timeout and duration fields should be in microseconds according to
> +the spec.
> +
> +TPM_CAP_PROP_TIS_TIMEOUT:
> +A 4 element array of UINT32 values each denoting the timeout value in
> +microseconds for the following in this order:
> +
> +TPM_CAP_PROP_DURATION:
> +A 3 element array of UINT32 values each denoting the duration value in
> +microseconds of the duration of the three classes of commands:
> +
> +Linux will scale the timeouts up by 1000, but not the durations. Change
> +the units for both sets as appropriate.
> +
> +Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
> +---
> + tpm/tpm_data.c | 14 +++++++-------
> + 1 file changed, 7 insertions(+), 7 deletions(-)
> +
> +diff --git a/tpm/tpm_data.c b/tpm/tpm_data.c
> +index a3a79ef..bebaf10 100644
> +--- a/tpm/tpm_data.c
> ++++ b/tpm/tpm_data.c
> +@@ -67,13 +67,13 @@ static void init_nv_storage(void)
> + static void init_timeouts(void)
> + {
> + /* for the timeouts we use the PC platform defaults */
> +- tpmData.permanent.data.tis_timeouts[0] = 750;
> +- tpmData.permanent.data.tis_timeouts[1] = 2000;
> +- tpmData.permanent.data.tis_timeouts[2] = 750;
> +- tpmData.permanent.data.tis_timeouts[3] = 750;
> +- tpmData.permanent.data.cmd_durations[0] = 1;
> +- tpmData.permanent.data.cmd_durations[1] = 10;
> +- tpmData.permanent.data.cmd_durations[2] = 1000;
> ++ tpmData.permanent.data.tis_timeouts[0] = 750000;
> ++ tpmData.permanent.data.tis_timeouts[1] = 2000000;
> ++ tpmData.permanent.data.tis_timeouts[2] = 750000;
> ++ tpmData.permanent.data.tis_timeouts[3] = 750000;
> ++ tpmData.permanent.data.cmd_durations[0] = 1000;
> ++ tpmData.permanent.data.cmd_durations[1] = 10000;
> ++ tpmData.permanent.data.cmd_durations[2] = 1000000;
> + }
> +
> + void tpm_init_data(void)
> +--
> +2.30.2
> +
>
^ permalink raw reply [flat|nested] 32+ messages in thread