[PATCH 0/2] tpm: Defend against TPM sending unexpected short packets

[Stefan Berger]

This series of patches addresses issues that may arise if a TPM sends
unexpected short packets.

Stefan

Stefan Berger (2):
  tpm: Require a response to have minimum size of a valid response
    header
  tcgbios: Check for enough bytes returned from TPM2_GetCapability

src/hw/tpm_drivers.c |  3 ++-
 src/tcgbios.c        | 13 +++++++++++--
 2 files changed, 13 insertions(+), 3 deletions(-)

[PATCH 1/2] tpm: Require a response to have minimum size of a valid response header

[Stefan Berger]

Defend against a broken TPM 1.2 or TPM 2.0 that doesn't send at least
a full response header in the response but less than 10 bytes.

Signed-off-by: Stefan Berger 
---
 src/hw/tpm_drivers.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/hw/tpm_drivers.c b/src/hw/tpm_drivers.c
index e4770b3..2b5753c 100644
--- a/src/hw/tpm_drivers.c
+++ b/src/hw/tpm_drivers.c
@@ -620,7 +620,8 @@ tpmhw_transmit(u8 locty, struct tpm_req_header *req,
         return -1;

irc = td->readresp(respbuffer, respbufferlen);
-    if (irc != 0)
+    if (irc != 0 ||
+        *respbufferlen < sizeof(struct tpm_rsp_header))
         return -1;

td->ready();

[PATCH 2/2] tcgbios: Check for enough bytes returned from TPM2_GetCapability

[Stefan Berger]

When querying a TPM 2.0 for its PCRs, make sure that we get enough bytes
from it in a response that did not indicate a failure. Basically we are
defending against a TPM 2.0 sending responses that are not compliant to
the specs.

Signed-off-by: Stefan Berger 
---
 src/tcgbios.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/src/tcgbios.c b/src/tcgbios.c
index 2e503f9..95c1e94 100644
--- a/src/tcgbios.c
+++ b/src/tcgbios.c
@@ -481,8 +481,17 @@ tpm20_get_pcrbanks(void)
     if (ret)
         return ret;

-    u32 size = be32_to_cpu(trg->hdr.totlen) -
-                           offsetof(struct tpm2_res_getcapability, data);
+    /* defend against (broken) TPM sending packets that are too short */
+    u32 resplen = be32_to_cpu(trg->hdr.totlen);
+    if (resplen <= offsetof(struct tpm2_res_getcapability, data))
+        return -1;
+
+    u32 size = resplen - offsetof(struct tpm2_res_getcapability, data);
+    /* we need a valid tpml_pcr_selection up to and including sizeOfSelect */
+    if (size < offsetof(struct tpml_pcr_selection, selections) +
+               offsetof(struct tpms_pcr_selection, pcrSelect))
+        return -1;
+
     tpm20_pcr_selection = malloc_high(size);
     if (tpm20_pcr_selection) {
         memcpy(tpm20_pcr_selection, &trg->data, size);

[PATCH 1/2] tpm: Require a response to have minimum size of a valid response header

[Marc-André Lureau]

On Thu, Nov 7, 2019 at 1:51 AM Stefan Berger  wrote:
> Defend against a broken TPM 1.2 or TPM 2.0 that doesn't send at least
> a full response header in the response but less than 10 bytes.
>
> Signed-off-by: Stefan Berger

Reviewed-by: Marc-André Lureau
> src/hw/tpm_drivers.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/src/hw/tpm_drivers.c b/src/hw/tpm_drivers.c
> index e4770b3..2b5753c 100644
> --- a/src/hw/tpm_drivers.c
> +++ b/src/hw/tpm_drivers.c
> @@ -620,7 +620,8 @@ tpmhw_transmit(u8 locty, struct tpm_req_header *req,
>          return -1;
>  irc = td->readresp(respbuffer, respbufferlen);
> if (irc != 0)
>
> if (irc != 0 ||
>    *respbufferlen < sizeof(struct tpm_rsp_header))
>    return -1;
>
> td->ready();

[PATCH 2/2] tcgbios: Check for enough bytes returned from TPM2_GetCapability

[Marc-André Lureau]

On Thu, Nov 7, 2019 at 3:14 AM Stefan Berger  wrote:
> When querying a TPM 2.0 for its PCRs, make sure that we get enough bytes
> from it in a response that did not indicate a failure. Basically we are
> defending against a TPM 2.0 sending responses that are not compliant to
> the specs.
>
> Signed-off-by: Stefan Berger

Reviewed-by: Marc-André Lureau
> src/tcgbios.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/src/tcgbios.c b/src/tcgbios.c
> index 2e503f9..95c1e94 100644
> --- a/src/tcgbios.c
> +++ b/src/tcgbios.c
> @@ -481,8 +481,17 @@ tpm20_get_pcrbanks(void)
>      if (ret)
>          return ret;
> u32 size = be32_to_cpu(trg->hdr.totlen) -
>                       offsetof(struct tpm2_res_getcapability, data);
> /* defend against (broken) TPM sending packets that are too short */
> u32 resplen = be32_to_cpu(trg->hdr.totlen);
> if (resplen <= offsetof(struct tpm2_res_getcapability, data))
>    return -1;
> u32 size = resplen - offsetof(struct tpm2_res_getcapability, data);
> /* we need a valid tpml_pcr_selection up to and including sizeOfSelect */
> if (size < offsetof(struct tpml_pcr_selection, selections) +
>           offsetof(struct tpms_pcr_selection, pcrSelect))
>    return -1;
> tpm20_pcr_selection = malloc_high(size);
>    if (tpm20_pcr_selection) {
>        memcpy(tpm20_pcr_selection, &trg->data, size);

[PATCH 1/2] tpm: Require a response to have minimum size of a valid response header

[Philippe Mathieu-Daudé]

On 11/6/19 10:35 PM, Stefan Berger wrote:
> Defend against a broken TPM 1.2 or TPM 2.0 that doesn't send at least
> a full response header in the response but less than 10 bytes.
> Signed-off-by: Stefan Berger
>
> src/hw/tpm_drivers.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/src/hw/tpm_drivers.c b/src/hw/tpm_drivers.c
> index e4770b3..2b5753c 100644
> --- a/src/hw/tpm_drivers.c
> +++ b/src/hw/tpm_drivers.c
> @@ -620,7 +620,8 @@ tpmhw_transmit(u8 locty, struct tpm_req_header *req,
>           return -1;
>   irc = td->readresp(respbuffer, respbufferlen);
> if (irc != 0)
>
> if (irc != 0 ||
>    *respbufferlen < sizeof(struct tpm_rsp_header))
>     return -1;
>
> td->ready();

Reviewed-by: Philippe Mathieu-Daudé

[PATCH 2/2] tcgbios: Check for enough bytes returned from TPM2_GetCapability

[Philippe Mathieu-Daudé]

On 11/6/19 10:36 PM, Stefan Berger wrote:
> When querying a TPM 2.0 for its PCRs, make sure that we get enough bytes
> from it in a response that did not indicate a failure. Basically we are
> defending against a TPM 2.0 sending responses that are not compliant to
> the specs.
> Signed-off-by: Stefan Berger
>
> src/tcgbios.c | 13 +++++++++++--
>   1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/src/tcgbios.c b/src/tcgbios.c
> index 2e503f9..95c1e94 100644
> --- a/src/tcgbios.c
> +++ b/src/tcgbios.c
> @@ -481,8 +481,17 @@ tpm20_get_pcrbanks(void)
>       if (ret)
>           return ret;
> u32 size = be32_to_cpu(trg->hdr.totlen) -
>                       offsetof(struct tpm2_res_getcapability, data);
> /* defend against (broken) TPM sending packets that are too short */
> u32 resplen = be32_to_cpu(trg->hdr.totlen);
> if (resplen <= offsetof(struct tpm2_res_getcapability, data))
>    return -1;
> u32 size = resplen - offsetof(struct tpm2_res_getcapability, data);
> /* we need a valid tpml_pcr_selection up to and including sizeOfSelect */
> if (size < offsetof(struct tpml_pcr_selection, selections) +
>           offsetof(struct tpms_pcr_selection, pcrSelect))
>    return -1;
> tpm20_pcr_selection = malloc_high(size);
> if (tpm20_pcr_selection) {
>     memcpy(tpm20_pcr_selection, &trg->data, size);

Reviewed-by: Philippe Mathieu-Daudé

[Kevin O'Connor]

On Wed, Nov 06, 2019 at 04:35:58PM -0500, Stefan Berger wrote:
> This series of patches addresses issues that may arise if a TPM sends
> unexpected short packets.
>
> Stefan

Thanks.  I committed this series.

-Kevin