Esys_TR_SetAuth should strip trailing 0's

[williamcroberts]

When the TPM calculates an HMAC, the trailing zeros are removed but tpm2-tss function Esys_TR_SetAuth doesn't remove them which causes the HMACs to be different. It should strip trailing zeros, this is causing a bug in systemd:

• https://github.com/systemd/systemd/issues/27716

[williamcroberts]

Probably also need to do this in the in-sensitive portions, I think ESAPI remembers those

[JuergenReppSIT]

@williamcroberts Before creating the PR I tested create primary with the following inSensitive:

  TPM2B_AUTH authValuePrimary = {
        .size = 6,
        .buffer = {1, 2, 3, 4, 5, 0}
    };

    TPM2B_SENSITIVE_CREATE inSensitivePrimary = {
        .size = 0,
        .sensitive = {
            .userAuth = authValuePrimary,
            .data = {
                 .size = 0,
                 .buffer = {0},
             },
        },
    };

and it worked with the new Esys_TR_SetAuth.

[williamcroberts]

We need to update some tests, left comments on your commit

[JuergenReppSIT]

We need to update some tests, left comments on your commit

I will update the integration tests to check the new Esys_TR_SetAuth.

Probably also need to do this in the in-sensitive portions, I think ESAPI remembers those

Yes ESAPI remembers those. ESAPI did use the HMAC key with the trailing zero for the HMAC computation if Esys_TR_SetAuth was not called. After a call of Esys_TR_SetAuth the HMAC key without the trailing zero is used. In both cases the same HMAC was computed by ESAPI (zero padding of the key for HMAC computation?) If ESYS_TR_PASSWORD is used instead of a session also both cases work. I wonder therefore what exactly was the error caused by ESYS_TR_SetAuth with a trailing zero?

[williamcroberts]

We need to update some tests, left comments on your commit

I will update the integration tests to check the new Esys_TR_SetAuth.

Probably also need to do this in the in-sensitive portions, I think ESAPI remembers those

Yes ESAPI remembers those. ESAPI did use the HMAC key with the trailing zero for the HMAC computation if Esys_TR_SetAuth was not called. After a call of Esys_TR_SetAuth the HMAC key without the trailing zero is used. In both cases the same HMAC was computed by ESAPI (zero padding of the key for HMAC computation?) If ESYS_TR_PASSWORD is used instead of a session also both cases work. I wonder therefore what exactly was the error caused by ESYS_TR_SetAuth with a trailing zero?

I actually tried to trigger this bug and couldn't, but @ddstreet reported it over here:

• https://github.com/systemd/systemd/issues/27716

[JuergenReppSIT]

I have added an integration test which shows that there is no difference between an auth value with and without a trailing zero. The error in systemd seems to be caused by an HMAC error in StartAuthSession:

71.581486] testsuite-70.sh[4385]: WARNING:esys:src/tss2-esys/api/Esys_StartAuthSession.c:391:Esys_StartAuthSession_Finish() Received TPM Error
[   71.582210] testsuite-70.sh[4385]: ERROR:esys:src/tss2-esys/api/Esys_StartAuthSession.c:136:Esys_StartAuthSession() Esys Finish ErrorCode (0x0000098e)

In Esys_StartAuth session no auth value is used to compute the HMAC.

[williamcroberts]

I have added an integration test which shows that there is no difference between an auth value with and without a trailing zero. The error in systemd seems to be caused by an HMAC error in StartAuthSession:

71.581486] testsuite-70.sh[4385]: WARNING:esys:src/tss2-esys/api/Esys_StartAuthSession.c:391:Esys_StartAuthSession_Finish() Received TPM Error
[   71.582210] testsuite-70.sh[4385]: ERROR:esys:src/tss2-esys/api/Esys_StartAuthSession.c:136:Esys_StartAuthSession() Esys Finish ErrorCode (0x0000098e)

In Esys_StartAuth session no auth value is used to compute the HMAC.

0x98e is:

tpm:session(1):the authorization HMAC check failed and DA counter incremented

Why are they getting this triggered, I don't think any primary object has an authValue

[williamcroberts]

@JuergenReppSIT I tested your patch, and I dropped your hunk in SetAuth:

 git diff --cached --name-only 
Makefile-test.am
test/integration/esys-check-auth-with-trailing-zero.int.c

And ran the test and it passed.

[ddstreet]

#include <tss2/tss2_esys.h>
#include <tss2/tss2_rc.h>
#include <stdio.h>
#include <stdlib.h>
#include <stdbool.h>

static const TPMT_SYM_DEF SESSION_TEMPLATE_SYM_AES_128_CFB = {
        .algorithm = TPM2_ALG_AES,
        .keyBits.aes = 128,
        .mode.aes = TPM2_ALG_CFB,
};

bool logrc(TSS2_RC rc, const char *msg) {
        printf("%s: %s\n", msg, Tss2_RC_Decode(rc));
        return rc == TSS2_RC_SUCCESS;
}

int main(int argc, char *argv[]) {
        ESYS_CONTEXT *c;
        TSS2_RC rc;

        if (argc < 2) {
                printf("Usage: %s <trailing byte>\n", argv[0]);
                return -1;
        }

        int trailing_byte = atoi(argv[1]);
        printf("Using trailing byte %d\n", trailing_byte);

        rc = Esys_Initialize(&c, NULL, NULL);
        if (!logrc(rc, "ESYS Initialized"))
                return -1;

        rc = Esys_Startup(c, TPM2_SU_CLEAR);
        if (!logrc(rc, "ESYS Started up"))
                return -1;

        ESYS_TR srk;
        rc = Esys_TR_FromTPMPublic(c, 0x81000001, ESYS_TR_NONE, ESYS_TR_NONE, ESYS_TR_NONE, &srk);
        if (!logrc(rc, "Got SRK"))
                return -1;

        TPM2B_PUBLIC template = {
                .size = sizeof(TPMT_PUBLIC),
                .publicArea = {
                        .type = TPM2_ALG_KEYEDHASH,
                        .nameAlg = TPM2_ALG_SHA256,
                        .parameters.keyedHashDetail.scheme.scheme = TPM2_ALG_NULL,
                        .unique.keyedHash.size = TPM2_SHA256_DIGEST_SIZE,
                },
        };
        TPM2B_SENSITIVE_CREATE sensitive = {
                .size = sizeof(TPMS_SENSITIVE_CREATE),
                .sensitive.data.size = 1,
                .sensitive.data.buffer[0] = 1,
                .sensitive.userAuth.size = 4,
                .sensitive.userAuth.buffer = { 1, 2, 3, (char) trailing_byte, },
        };
        TPM2B_PUBLIC *public = NULL;
        TPM2B_PRIVATE *private = NULL;
        rc = Esys_Create(c, srk, ESYS_TR_PASSWORD, ESYS_TR_NONE, ESYS_TR_NONE, &sensitive, &template, NULL, &(TPML_PCR_SELECTION) {}, &private, &public, NULL, NULL, NULL);
        if (!logrc(rc, "Created bind key"))
                return -1;

        ESYS_TR bind_key;
        rc = Esys_Load(c, srk, ESYS_TR_PASSWORD, ESYS_TR_NONE, ESYS_TR_NONE, private, public, &bind_key);
        if (!logrc(rc, "Loaded bind key"))
                return -1;

        TPM2B_AUTH bind_key_auth = {
                .size = 4,
                .buffer = { 1, 2, 3, (char) trailing_byte, },
        };
        rc = Esys_TR_SetAuth(c, bind_key, &bind_key_auth);
        if (!logrc(rc, "Set Auth"))
                return -1;

        ESYS_TR encryption_session;
        rc = Esys_StartAuthSession(c, srk, bind_key, ESYS_TR_NONE, ESYS_TR_NONE, ESYS_TR_NONE, NULL, TPM2_SE_HMAC, &SESSION_TEMPLATE_SYM_AES_128_CFB, TPM2_ALG_SHA256, &encryption_session);
        if (!logrc(rc, "Started encryption session"))
                return -1;

        const TPMA_SESSION sessionAttributes = TPMA_SESSION_DECRYPT | TPMA_SESSION_ENCRYPT | TPMA_SESSION_CONTINUESESSION;
        rc = Esys_TRSess_SetAttributes(c, encryption_session, sessionAttributes, 0xff);
        if (!logrc(rc, "Set encryption session attributes"))
                return -1;

        ESYS_TR policy_session;
        rc = Esys_StartAuthSession(c, srk, ESYS_TR_NONE, encryption_session, ESYS_TR_NONE, ESYS_TR_NONE, NULL, TPM2_SE_POLICY, &SESSION_TEMPLATE_SYM_AES_128_CFB, TPM2_ALG_SHA256, &policy_session);
        if (!logrc(rc, "Started policy session"))
                return -1;

        printf("Finished OK\n");

        return 0;
}

$ sudo ./test 1
Using trailing byte 1
ESYS Initialized: tpm:success
ESYS Started up: tpm:success
Got SRK: tpm:success
Created bind key: tpm:success
Loaded bind key: tpm:success
Set Auth: tpm:success
Started encryption session: tpm:success
Set encryption session attributes: tpm:success
Started policy session: tpm:success
Finished OK

$ sudo ./test 0
Using trailing byte 0
ESYS Initialized: tpm:success
ESYS Started up: tpm:success
Got SRK: tpm:success
Created bind key: tpm:success
Loaded bind key: tpm:success
Set Auth: tpm:success
Started encryption session: tpm:success
Set encryption session attributes: tpm:success
WARNING:esys:src/tss2-esys/api/Esys_StartAuthSession.c:391:Esys_StartAuthSession_Finish() Received TPM Error 
ERROR:esys:src/tss2-esys/api/Esys_StartAuthSession.c:136:Esys_StartAuthSession() Esys Finish ErrorCode (0x0000098e) 
Started policy session: tpm:session(1):the authorization HMAC check failed and DA counter incremented

[williamcroberts]

Thanks @ddstreet . I don't know why my test wasn't reproducing it, but running off of your code and changing it to the way I had it, it also reproduced (see below).

Now we at least now have solid reproducers. I guess the question is now what should we do inside of ESAPI. If we do something like in systemd and set the last byte to 0xFF if it's a nul byte, existing keys wouldn't work. If we zero trim, existing keys would just start working with sessions. I am in favor of zero trimming in ESAPI. For systemd, as you point out, existing keys wouldn't work anyways, so just setting to 0xFF seems sane. @JuergenReppSIT any thoughts?

#include <tss2/tss2_esys.h>
#include <tss2/tss2_rc.h>
#include <stdio.h>
#include <stdlib.h>
#include <stdbool.h>

static const TPMT_SYM_DEF SESSION_TEMPLATE_SYM_AES_128_CFB = {
        .algorithm = TPM2_ALG_AES,
        .keyBits.aes = 128,
        .mode.aes = TPM2_ALG_CFB,
};

bool logrc(TSS2_RC rc, const char *msg) {
        printf("%s: %s\n", msg, Tss2_RC_Decode(rc));
        return rc == TSS2_RC_SUCCESS;
}

int main(int argc, char *argv[]) {
        ESYS_CONTEXT *c;
        TSS2_RC rc;

        if (argc < 2) {
                printf("Usage: %s <trailing byte>\n", argv[0]);
                return -1;
        }

        int trailing_byte = atoi(argv[1]);
        printf("Using trailing byte %d\n", trailing_byte);

        rc = Esys_Initialize(&c, NULL, NULL);
        if (!logrc(rc, "ESYS Initialized"))
                return -1;

        rc = Esys_Startup(c, TPM2_SU_CLEAR);
        if (!logrc(rc, "ESYS Started up"))
                return -1;

        TPM2B_PUBLIC template = {
            .publicArea = {
                .type = TPM2_ALG_RSA,
                .nameAlg = TPM2_ALG_SHA256,
                .objectAttributes = (TPMA_OBJECT_USERWITHAUTH |
                                     TPMA_OBJECT_DECRYPT |
                                     TPMA_OBJECT_FIXEDTPM |
                                     TPMA_OBJECT_FIXEDPARENT |
                                     TPMA_OBJECT_SENSITIVEDATAORIGIN |
                                     TPMA_OBJECT_RESTRICTED),
                .authPolicy = {
                     .size = 0,
                 },
                 .parameters.rsaDetail = {
                      .symmetric = {
                          .algorithm = TPM2_ALG_AES,
                          .keyBits.aes = 128,
                          .mode.aes = TPM2_ALG_CFB},
                      .scheme = {
                           .scheme = TPM2_ALG_NULL
                       },
                      .keyBits = 2048,
                      .exponent = 0,
                  },
                .unique.rsa = {
                     .size = 0,
                     .buffer = {},
                 },
            },
        };

        TPM2B_SENSITIVE_CREATE sensitive = {
                .sensitive.userAuth.size = 4,
                .sensitive.userAuth.buffer = { 1, 2, 3, (char) trailing_byte, },
        };

        TPM2B_DATA outside_info = { 0 };
        TPML_PCR_SELECTION creation_pcr = { 0 };

        ESYS_TR srk;
        rc = Esys_CreatePrimary(c, ESYS_TR_RH_OWNER,
                ESYS_TR_PASSWORD, ESYS_TR_NONE, ESYS_TR_NONE,
                &sensitive, &template, &outside_info, &creation_pcr,
                &srk, NULL, NULL, NULL, NULL);
        if (!logrc(rc, "ESYS Started up"))
                return -1;

        ESYS_TR encryption_session;
        rc = Esys_StartAuthSession(c, srk, srk, ESYS_TR_NONE, ESYS_TR_NONE, ESYS_TR_NONE, NULL, TPM2_SE_HMAC, &SESSION_TEMPLATE_SYM_AES_128_CFB, TPM2_ALG_SHA256, &encryption_session);
        if (!logrc(rc, "Started encryption session"))
                return -1;

        const TPMA_SESSION sessionAttributes = TPMA_SESSION_DECRYPT | TPMA_SESSION_ENCRYPT | TPMA_SESSION_CONTINUESESSION;
        rc = Esys_TRSess_SetAttributes(c, encryption_session, sessionAttributes, 0xff);
        if (!logrc(rc, "Set encryption session attributes"))
                return -1;

        TPM2B_PUBLIC *public = NULL;
        TPM2B_PRIVATE *private = NULL;
        rc = Esys_Create(c, srk, encryption_session, ESYS_TR_NONE, ESYS_TR_NONE, &sensitive, &template, NULL, &(TPML_PCR_SELECTION) {}, &private, &public, NULL, NULL, NULL);
        if (!logrc(rc, "Created bind key"))
                return -1;

        ESYS_TR bind_key;
        rc = Esys_Load(c, srk, encryption_session, ESYS_TR_NONE, ESYS_TR_NONE, private, public, &bind_key);
        if (!logrc(rc, "Loaded bind key"))
                return -1;

        printf("Finished OK\n");

        return 0;
}

$ ./nullbyte2 1
Using trailing byte 1
ESYS Initialized: tpm:success
ESYS Started up: tpm:success
ESYS Started up: tpm:success
Started encryption session: tpm:success
Set encryption session attributes: tpm:success
Created bind key: tpm:success
Loaded bind key: tpm:success
Finished OK

$ ./nullbyte2 0
Using trailing byte 0
ESYS Initialized: tpm:success
ESYS Started up: tpm:success
ESYS Started up: tpm:success
Started encryption session: tpm:success
Set encryption session attributes: tpm:success
WARNING:esys:src/tss2-esys/api/Esys_Create.c:399:Esys_Create_Finish() Received TPM Error 
ERROR:esys:src/tss2-esys/api/Esys_Create.c:134:Esys_Create() Esys Finish ErrorCode (0x0000098e) 
Created bind key: tpm:session(1):the authorization HMAC check failed and DA counter incremented

[JuergenReppSIT]

@ddstreet Also thanks for the example. It works with the draft PR #2665 But if Esys_TR_SetAuth is not called the error still occurs. I will try to fix it in ESAPI.

[williamcroberts]

@ddstreet Also thanks for the example. It works with the draft PR #2665 But if Esys_TR_SetAuth is not called the error still occurs. I will try to fix it in ESAPI.

Ha I just mentioned that on the bug tracker. Thanks @JuergenReppSIT

[JuergenReppSIT]

@ddstreet sorry I forgot, that Esys_Create does not create an esys object. The object will be created by Esys_Load. So the call of Esys_TR_SetAuth is needed for keys.

[williamcroberts]

@ddstreet sorry I forgot, that Esys_Create does not create an esys object. The object will be created by Esys_Load. So the call of Esys_TR_SetAuth is needed for keys.

Oh I could see how that's the case. Could we add a feature where the ESAPI context is able to look it up by name instead of handle during Esys_Load()?

[JuergenReppSIT]

To enable such a feature it would be necessary in Esys_Create to compute the name from the public key and store it in a table of entries (name, authvalue) in the esys context. But it would not possible to delete such an entry, because Esys_Load can be executed several times with this key.

[williamcroberts]

That shouldn't be too much overhead IMO

[JuergenReppSIT]

But a long running application which creates many keys would increase this table and only with Esys_Finalize could the table could be freed.

[williamcroberts]

But a long running application which creates many keys would increase this table and only with Esys_Finalize could the table could be freed.

You could add a delete method, but im also thinking it changes how Create vs load from disk flow, currently something like this works.

TPM2B_PUBLIC *pub = NULL;
TPM2B_PRIVATE *priv = NULL;
if (create) {
   Esys_Create(pub, priv);
else {
  load_from_disk(pub, priv);

ESYS_TR tr = ESYS_TR_NONE;
Esys_Load(pub, priv, &tr)

// Get Auth
Esys_TR_SetAuth(tr, "auth");

// More logic

This code would continue to work, but you don't need to call the SetAuth on the Create path, so it could be moved into only the load_from_disk path. But that means folks might be like, why do I have to call it on one path versus the other?

With that said, I'm OK with leaving at is versus complicating the nuances and code here.