Possible bug with encrypted HMAC sessions

[David Verbrugge]

Using ibmtpm20tss-v1.6.0 I was trying to create primary objects using an encrypted HMAC only session. These objects were in the Owner hierarchy with empty password. This action was successful without the HMAC session. With HMAC command encrypt session I consistently received failure result for parameter 1 size error. Using SW TPM I found that the command encryption was not being applied by the TSS library resulting in the TPM decrypting plain text.

To make a long story short, I found what looks to be an error in TSS library code at TSSSessionsGetDecryptSession() and at TSSSessionsGetEncryptSession(). The 'for' loop in each of these functions is:
for (i = 0; (rc == 0) && (i < MAX_SESSION_NUM) && (sessionHandle[i] != TPM_RH_NULL) && (sessionHandle[i] != TPM_RS_PW); i++) {

I believe the for loop should read
for (i = 0; (rc == 0) && (i < MAX_SESSION_NUM) && (sessionHandle[i] != TPM_RH_NULL); i++) {

and below the 'for' loop, ahead of the if statement testing the sessionAttribute needs to be added an enclosing if statement to validate the session handle like so:
if (sessionHandle[i] != TPM_RS_PW) { // no decrypt for password sessions

Example:

    /* count the number of command decrypt sessions */
    *isDecrypt = 0; /* number of sessions with decrypt set */
    for (i = 0; (rc == 0) && (i < MAX_SESSION_NUM) && (sessionHandle[i] != TPM_RH_NULL); i++) {
    if (sessionHandle[i] != TPM_RS_PW) {  // no decrypt for password sessions
    if (sessionAttributes[i] & TPMA_SESSION_DECRYPT) {
        (*isDecrypt)++;      /* count number of decrypt sessions */
        *decryptSession = i; /* record which one it was */
    }
    }
    }

After I made the above changes locally, I was able to successfully utilize HMAC encrypted sessions.

Best regards,
David

[Ken Goldman]

1. I cannot reproduce this error. I used this trivial script:

startauthsession -se h
createprimary -hi o -se0 02000000 61

which seems to be your test case.  
Can you send me a short testcase, either C or a script?

2. Sorry, I can't see how your code logic is different from the existing code.  There are 4 AND terms.  Why would breaking one out (three and then one) make a difference?  Is there perhaps a () missing somewhere.

I.e., these seem logically equivalent:

for (i = 0; (rc == 0) && (i < MAX_SESSION_NUM) && (sessionHandle[i] != TPM_RH_NULL); i++) {
if (sessionHandle[i] != TPM_RS_PW) {
}
}

for (i = 0 ; (rc == 0) && (i < MAX_SESSION_NUM) &&
     (sessionHandle[i] != TPM_RH_NULL) &&
     (sessionHandle[i] != TPM_RS_PW) ;
 i++) {

[David Verbrugge]

Hi Ken,

Thank you for the quick response. I think the issue can be reproduced only when session[0] is of type TPM_RS_PW and session[1] is the HMAC session.

The difference in logic between original code my revision is that in the original, the for loop terminates immediate if the first session is of type 'TPM_RS_PW', which happens to be true in my use case . My revised version permits the for loop to pass over session index i==0 where sessionhandle[i]==TPM_RS_PW so it can find the HMAC session at i==1

I'm on an other task at present and will post a short C test case in the next couple of days.

[Ken Goldman]

I think you're right. That's an odd corner case, because normally one would use the first session for both authorization and encryption. If the second session is solely for encryption, there is no shared secret (unless it's salted or bound), so the encryption would be with a known key.
I'll write a test case for it in the next few days, but I believe that your patch is correct.

[Ken Goldman]

Fixed. This will be in the next release. Thanks.