tpm2-software / tpm2-tss

OSS implementation of the TCG TPM2 Software Stack (TSS2)
https://tpm2-software.github.io
BSD 2-Clause "Simplified" License
755 stars 366 forks source link

Unsealing with session and password returns incorrect data (suspected reply decryption issue) #2574

Open JonathonHall-Purism opened 1 year ago

JonathonHall-Purism commented 1 year ago

Hi,

We have a secret sealed using a policy that includes the object's authorization value. (We want unsealing the object to require both PCRs and the auth value, so we build an appropriate policy and set flags, but I stripped out the PCR auth and flags from the repro.)

Unsealing the secret returns successfully, but the returned data is not correct. I think tpm2-tss is decrypting the response incorrectly. Configuring the policy session used to unseal with --disable-encrypt works around the issue, but then the response is sent in the clear by the TPM, encryption would be nice to avoid bus-sniffing. (If I'm wrong and should move this to tpm2-tools, just let me know!)

Script to reproduce, simplified as much as I could: test-seal-unseal-policy-password-simplified.sh.gz

I can reproduce on swtpm (in my case, running in qemu using swtpm for the virtual TPM), and on an Infineon SLB9665 hardware TPM.

Non-working output from the end of the script - the data has the right length but is not correct:

+ echo sealed:
sealed:
+ xxd /tmp/seal_data.bin
00000000: 5450 4d32 2073 6561 6c20 7465 7374 0a    TPM2 seal test.
+ echo unsealed:
unsealed:
+ xxd /tmp/unsealed_data.bin
00000000: 2831 601a db55 13f4 283b 2692 d9fd 2a    (1`..U..(;&...*

Running the script with --workaround disables encryption in the policy session and works:

+ echo sealed:
sealed:
+ xxd /tmp/seal_data.bin
00000000: 5450 4d32 2073 6561 6c20 7465 7374 0a    TPM2 seal test.
+ echo unsealed:
unsealed:
+ xxd /tmp/unsealed_data.bin
00000000: 5450 4d32 2073 6561 6c20 7465 7374 0a    TPM2 seal test.

I haven't worked through all the docs on how the reply decryption works yet and haven't seen how that's selected in tpm2-tss. I saw that ESYS_CONTEXT contains things about the session for encryption, but not decryption. If anyone could point me in the right direction, I would appreciate it!

JonathonHall-Purism commented 1 year ago

@AndreasFuchsTPM , @williamcroberts , @JuergenReppSIT - would any of you have any ideas on this issue? I'm happy to work up a fix but some pointers on where to look would be very helpful!

williamcroberts commented 1 year ago

@AndreasFuchsTPM , @williamcroberts , @JuergenReppSIT - would any of you have any ideas on this issue? I'm happy to work up a fix but some pointers on where to look would be very helpful!

I'm able to reproduce, let me look this morning real quick.

JuergenReppSIT commented 1 year ago

I also could reproduce the error. I checked the encryption in the simulator and saw that the session key was the the same in the simulator and in the tool command. But the nonces used to compute the key for symmetric encryption with KDFa were different which caused the wrong encryption (ContextU had 32 bytes, ContextV 0 bytes in the simulator) But i'm afraid i won't be able to continue troubleshooting until next week

williamcroberts commented 1 year ago

I'm seeing the same thing: Simulator side: CryptKDFa input nonceCaller(0): 0x18...0x0 CryptKDFa input nonceTPM(32): 0xcb...0x6f

tpm2-tss side: nonceCaller (32): 0x0a ... 0x67 nonceTPM (32): 0xdc ... 0x5c

I modified the simulator ibmtpm1628 in src/CryptUtil.c function ParmEncryptSym and had it dump the values.

Just for the heck of it, I ran it against swtpm and got the same issue, so the issue is definitely in the tpm2-tss IIUC.

williamcroberts commented 1 year ago

I see nonceTPM coming back at:

TSS2_RC
iesys_check_response(ESYS_CONTEXT * esys_context)
{
    TSS2_RC r;
    const uint8_t *rpBuffer;
    size_t rpBuffer_size;
    TSS2L_SYS_AUTH_RESPONSE rspAuths;
    HASH_TAB_ITEM rp_hash_tab[3];
    uint8_t rpHashNum = 0;

    if (esys_context->authsCount == 0) {
        LOG_TRACE("No auths to verify");
        return TSS2_RC_SUCCESS;
    }

    r = Tss2_Sys_GetRspAuths(esys_context->sys, &rspAuths);
    return_if_error(r, "Error: GetRspAuths");

After that if you check rspAuths[0] it's correct.

Looking at:

TSS2_RC
iesys_check_rp_hmacs(ESYS_CONTEXT * esys_context,
                     TSS2L_SYS_AUTH_RESPONSE * rspAuths,
                     HASH_TAB_ITEM rp_hash_tab[3],
                     uint8_t rpHashNum)
{
    TSS2_RC r;

    for (int i = 0; i < rspAuths->count; i++) {
        RSRC_NODE_T *session = esys_context->session_tab[i];
        if (session == NULL)
            continue;

        IESYS_SESSION *rsrc_session = &session->rsrc.misc.rsrc_session;
        if (rsrc_session->type_policy_session == POLICY_PASSWORD) {
            /* A policy password session has no auth value */
            if (rspAuths->auths[i].hmac.size != 0) {
                LOG_ERROR("PolicyPassword session's HMAC must be 0-length.");
                return TSS2_ESYS_RC_RSP_AUTH_FAILED;
            }
            continue;
        }

The continue skips.... looks like nonces are never updated in the code below

williamcroberts commented 1 year ago

If I drop the continue, the authHmac fails...

williamcroberts commented 1 year ago

OOOH I see why this is failing.

TL;DR PolicyPassword replaces the hmac with the password (why using policypassword is bad and leads to weirdness).

I'll update the script... but in general it looks like we should be checking for this unintended sidefect inthe ESAPI code perhaps? Ie if you have a policy session AND it's policy password is invoked AND you have or attempt to enable session encryption/decryption fail?

This explains the empty nonce on the TPM side.

williamcroberts commented 1 year ago

Attached is a working script, but this is where you specify a session for policypassword and a session for parameter encryption/decryption using the -S option. You should also use a session like this to seal your secret so folks don't see it on the way to the TPM.

test-seal-unseal-policy-password-simplified.sh.txt

I'll leave adding the robustness for checks (perhaps) in ESAPI if it makes sense to @JuergenReppSIT or @AndreasFuchsTPM to whomever else.

JonathonHall-Purism commented 1 year ago

Wow, thank you @williamcroberts ! I'll integrate this back in Heads. Will get it on the seal too, thanks for the suggestion.

I'll leave this open for the discussion about robustness checks, if there is anything else I can provide to help, just ping me!