CVE-2017-7524: Insufficiently Protected Credentials (CWE-522)
value: tpm2-tools versions before 1.1.1 are vulnerable to a password leak due to transmitting password in plaintext from client to server when generating HMAC.
CWE description: The product transmits or stores authentication credentials, but it uses an insecure method that is susceptible to unauthorized interception and/or retrieval.
CVSS v3 base score: 7.5
Published: 2017-06-27T14:29Z
Repository: https://github.com/01org/tpm2.0-tools
Language: C
Files changed: Makefile.am, configure.ac, lib/tpm_kdfa.c, lib/tpm_kdfa.h, lib/tpm_session.c
Commit date: 2017-06-21 10:15:50 -0700
Commit message: kdfa: use openssl for hmac not tpm

While not reachable in the current code base tools, a potential
security bug lurked in tpm_kdfa().

If using that routine for an hmac authorization, the hmac was
calculated using the tpm. A user of an object wishing to
authenticate via hmac, would expect that the password is never
sent to the tpm. However, since the hmac calculation relies on
password, and is performed by the tpm, the password ends up
being sent in plain text to the tpm.

The fix is to use openssl to generate the hmac on the host.

Fixes: CVE-2017-7524

Signed-off-by: William Roberts <william.c.roberts@intel.com>

Diff: diff --git a/Makefile.am b/Makefile.am
index a3c43f8..c40c24b 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -33,14 +33,14 @@
 ACLOCAL_AMFLAGS = -I m4
 
 INCLUDE_DIRS = -I$(srcdir)/src -I$(srcdir)/lib
 LIB_COMMON := lib/libcommon.a
 
-AM_CFLAGS    := $(INCLUDE_DIRS) $(TPM20_TSS_CFLAGS) $(EXTRA_CFLAGS) $(TCTI_TABRMD_CFLAGS)
+AM_CFLAGS    := $(INCLUDE_DIRS) $(TPM20_TSS_CFLAGS) $(EXTRA_CFLAGS) $(TCTI_TABRMD_CFLAGS) $(CRYPTO_CFLAGS)
 AM_LDFLAGS   := $(EXTRA_LDFLAGS)
 
-LDADD = $(LIB_COMMON) $(TPM20_TSS_LIBS) $(TCTI_SOCK_LIBS) $(TCTI_DEV_LIBS) $(TCTI_TABRMD_LIBS)
+LDADD = $(LIB_COMMON) $(TPM20_TSS_LIBS) $(TCTI_SOCK_LIBS) $(TCTI_DEV_LIBS) $(TCTI_TABRMD_LIBS) $(CRYPTO_LIBS)
 
 sbin_PROGRAMS = \
     tools/tpm2_create \
     tools/tpm2_createprimary \
     tools/tpm2_load \
diff --git a/configure.ac b/configure.ac
index ce781ea..8ef3b17 100644
--- a/configure.ac
+++ b/configure.ac
@@ -67,11 +67,12 @@ AS_IF(
           "x$with_tcti_socket" != "xyes" -a \
           "x$with_tcti_tabrmd" != "xyes"],
     [AC_MSG_ERROR(
         [no TCTIs: at least one TCTI library must be enabled],
         [1])])
-PKG_CHECK_MODULES([CURL],[libcurl libcrypto])
+PKG_CHECK_MODULES([CRYPTO],[libcrypto])
+PKG_CHECK_MODULES([CURL],[libcurl])
 AC_ARG_ENABLE([unit],
             [AS_HELP_STRING([--enable-unit],
                             [build cmocka unit tests (default is no)])],
             [enable_unit=$enableval],
             [enable_unit=no])
diff --git a/lib/tpm_kdfa.c b/lib/tpm_kdfa.c
index 0ba8679..8198c9b 100644
--- a/lib/tpm_kdfa.c
+++ b/lib/tpm_kdfa.c
@@ -25,24 +25,44 @@
 // THE POSSIBILITY OF SUCH DAMAGE.
 //**********************************************************************;
 
 #include <sapi/tpm20.h>
 
+ #include <openssl/err.h>
+#include <openssl/hmac.h>
+
 #include "string-bytes.h"
 #include "tpm_hmac.h"
+#include "log.h"
+
+static const EVP_MD *tpm_algorithm_to_openssl_digest(TPMI_ALG_HASH algorithm) {
+
+    switch(algorithm) {
+    case TPM_ALG_SHA1:
+        return EVP_sha1();
+    case ALG_SHA256_VALUE:
+        return EVP_sha256();
+    case TPM_ALG_SHA384:
+        return EVP_sha384();
+    case TPM_ALG_SHA512:
+        return EVP_sha512();
+    default:
+        return NULL;
+    }
+    /* no return, not possible */
+}
 
-TPM_RC tpm_kdfa(TSS2_SYS_CONTEXT *sapi_context, TPMI_ALG_HASH hashAlg,
+TPM_RC tpm_kdfa(TPMI_ALG_HASH hashAlg,
         TPM2B *key, char *label, TPM2B *contextU, TPM2B *contextV, UINT16 bits,
         TPM2B_MAX_BUFFER  *resultKey )
 {
-    TPM2B_DIGEST tmpResult;
     TPM2B_DIGEST tpm2bLabel, tpm2bBits, tpm2b_i_2;
     UINT8 *tpm2bBitsPtr = &tpm2bBits.t.buffer[0];
     UINT8 *tpm2b_i_2Ptr = &tpm2b_i_2.t.buffer[0];
     TPM2B_DIGEST *bufferList[8];
     UINT32 bitsSwizzled, i_Swizzled;
-    TPM_RC rval;
+    TPM_RC rval = TPM_RC_SUCCESS;
     int i, j;
     UINT16 bytes = bits / 8;
 
     resultKey->t .size = 0;
 
@@ -62,12 +82,28 @@ TPM_RC tpm_kdfa(TSS2_SYS_CONTEXT *sapi_context, TPMI_ALG_HASH hashAlg,
 
     resultKey->t.size = 0;
 
     i = 1;
 
+    const EVP_MD *md = tpm_algorithm_to_openssl_digest(hashAlg);
+    if (!md) {
+        LOG_ERR("Algorithm not supported for hmac: %x", hashAlg);
+        return TPM_RC_HASH;
+    }
+
+    HMAC_CTX ctx;
+    HMAC_CTX_init(&ctx);
+    int rc = HMAC_Init_ex(&ctx, key->buffer, key->size, md, NULL);
+    if (!rc) {
+        LOG_ERR("HMAC Init failed: %s", ERR_error_string(rc, NULL));
+        return TPM_RC_MEMORY;
+    }
+
+    // TODO Why is this a loop? It appears to only execute once.
     while( resultKey->t.size < bytes )
     {
+        TPM2B_DIGEST tmpResult;
         // Inner loop
 
         i_Swizzled = string_bytes_endian_convert_32( i );
         *(UINT32 *)tpm2b_i_2Ptr = i_Swizzled;
 
@@ -75,23 +111,43 @@ TPM_RC tpm_kdfa(TSS2_SYS_CONTEXT *sapi_context, TPMI_ALG_HASH hashAlg,
         bufferList[j++] = (TPM2B_DIGEST *)&(tpm2b_i_2.b);
         bufferList[j++] = (TPM2B_DIGEST *)&(tpm2bLabel.b);
         bufferList[j++] = (TPM2B_DIGEST *)contextU;
         bufferList[j++] = (TPM2B_DIGEST *)contextV;
         bufferList[j++] = (TPM2B_DIGEST *)&(tpm2bBits.b);
-        bufferList[j++] = (TPM2B_DIGEST *)0;
-        rval = tpm_hmac(sapi_context, hashAlg, key, (TPM2B **)&( bufferList[0] ), &tmpResult );
-        if( rval != TPM_RC_SUCCESS )
-        {
-            return( rval );
+        bufferList[j] = (TPM2B_DIGEST *)0;
+
+        int c;
+        for(c=0; c < j; c++) {
+            TPM2B_DIGEST *digest = bufferList[c];
+            int rc =  HMAC_Update(&ctx, digest->b.buffer, digest->b.size);
+            if (!rc) {
+                LOG_ERR("HMAC Update failed: %s", ERR_error_string(rc, NULL));
+                rval = TPM_RC_MEMORY;
+                goto err;
+            }
+        }
+
+        unsigned size = sizeof(tmpResult.t.buffer);
+        int rc = HMAC_Final(&ctx, tmpResult.t.buffer, &size);
+        if (!rc) {
+            LOG_ERR("HMAC Final failed: %s", ERR_error_string(rc, NULL));
+            rval = TPM_RC_MEMORY;
+            goto err;
         }
 
+        tmpResult.t.size = size;
+
         bool res = string_bytes_concat_buffer(resultKey, &(tmpResult.b));
         if (!res) {
-            return TSS2_SYS_RC_BAD_VALUE;
+            rval = TSS2_SYS_RC_BAD_VALUE;
+            goto err;
         }
     }
 
     // Truncate the result to the desired size.
     resultKey->t.size = bytes;
 
-    return TPM_RC_SUCCESS;
+err:
+    HMAC_CTX_cleanup(&ctx);
+
+    return rval;
 }
diff --git a/lib/tpm_kdfa.h b/lib/tpm_kdfa.h
index 68413bd..e26b111 100644
--- a/lib/tpm_kdfa.h
+++ b/lib/tpm_kdfa.h
@@ -40,10 +40,10 @@
  * @param contextV
  * @param bits
  * @param resultKey
  * @return
  */
-TPM_RC tpm_kdfa(TSS2_SYS_CONTEXT *sapi_context, TPMI_ALG_HASH hashAlg,
+TPM_RC tpm_kdfa(TPMI_ALG_HASH hashAlg,
         TPM2B *key, char *label, TPM2B *contextU, TPM2B *contextV,
         UINT16 bits, TPM2B_MAX_BUFFER *resultKey );
 
 #endif /* SRC_TPM_KDFA_H_ */
diff --git a/lib/tpm_session.c b/lib/tpm_session.c
index 414f1f7..f13e7ef 100644
--- a/lib/tpm_session.c
+++ b/lib/tpm_session.c
@@ -196,11 +196,11 @@ static TPM_RC StartAuthSession(TSS2_SYS_CONTEXT *sapi_context, SESSION *session
             {
                 session->sessionKey.t.size = 0;
             }
             else
             {
-                rval = tpm_kdfa(sapi_context, session->authHash, &(key.b), label, &( session->nonceNewer.b ),
+                rval = tpm_kdfa(session->authHash, &(key.b), label, &( session->nonceNewer.b ),
                         &( session->nonceOlder.b ), bytes * 8, (TPM2B_MAX_BUFFER *)&( session->sessionKey ) );
             }
 
             if( rval != TPM_RC_SUCCESS )
             {

Vulnerable code: AM_CFLAGS    := $(INCLUDE_DIRS) $(TPM20_TSS_CFLAGS) $(EXTRA_CFLAGS) $(TCTI_TABRMD_CFLAGS)
LDADD = $(LIB_COMMON) $(TPM20_TSS_LIBS) $(TCTI_SOCK_LIBS) $(TCTI_DEV_LIBS) $(TCTI_TABRMD_LIBS)
PKG_CHECK_MODULES([CURL],[libcurl libcrypto])
TPM_RC tpm_kdfa(TSS2_SYS_CONTEXT *sapi_context, TPMI_ALG_HASH hashAlg,
    TPM2B_DIGEST tmpResult;
    TPM_RC rval;
        bufferList[j++] = (TPM2B_DIGEST *)0;
        rval = tpm_hmac(sapi_context, hashAlg, key, (TPM2B **)&( bufferList[0] ), &tmpResult );
        if( rval != TPM_RC_SUCCESS )
        {
            return( rval );
            return TSS2_SYS_RC_BAD_VALUE;
    return TPM_RC_SUCCESS;
TPM_RC tpm_kdfa(TSS2_SYS_CONTEXT *sapi_context, TPMI_ALG_HASH hashAlg,
                rval = tpm_kdfa(sapi_context, session->authHash, &(key.b), label, &( session->nonceNewer.b ),
Fixed code: AM_CFLAGS    := $(INCLUDE_DIRS) $(TPM20_TSS_CFLAGS) $(EXTRA_CFLAGS) $(TCTI_TABRMD_CFLAGS) $(CRYPTO_CFLAGS)
LDADD = $(LIB_COMMON) $(TPM20_TSS_LIBS) $(TCTI_SOCK_LIBS) $(TCTI_DEV_LIBS) $(TCTI_TABRMD_LIBS) $(CRYPTO_LIBS)
PKG_CHECK_MODULES([CRYPTO],[libcrypto])
PKG_CHECK_MODULES([CURL],[libcurl])
 #include <openssl/err.h>
#include <openssl/hmac.h>

#include "log.h"

static const EVP_MD *tpm_algorithm_to_openssl_digest(TPMI_ALG_HASH algorithm) {

    switch(algorithm) {
    case TPM_ALG_SHA1:
        return EVP_sha1();
    case ALG_SHA256_VALUE:
        return EVP_sha256();
    case TPM_ALG_SHA384:
        return EVP_sha384();
    case TPM_ALG_SHA512:
        return EVP_sha512();
    default:
        return NULL;
    }
    /* no return, not possible */
}
TPM_RC tpm_kdfa(TPMI_ALG_HASH hashAlg,
    TPM_RC rval = TPM_RC_SUCCESS;
    const EVP_MD *md = tpm_algorithm_to_openssl_digest(hashAlg);
    if (!md) {
        LOG_ERR("Algorithm not supported for hmac: %x", hashAlg);
        return TPM_RC_HASH;
    }

    HMAC_CTX ctx;
    HMAC_CTX_init(&ctx);
    int rc = HMAC_Init_ex(&ctx, key->buffer, key->size, md, NULL);
    if (!rc) {
        LOG_ERR("HMAC Init failed: %s", ERR_error_string(rc, NULL));
        return TPM_RC_MEMORY;
    }

    // TODO Why is this a loop? It appears to only execute once.
        TPM2B_DIGEST tmpResult;
        bufferList[j] = (TPM2B_DIGEST *)0;

        int c;
        for(c=0; c < j; c++) {
            TPM2B_DIGEST *digest = bufferList[c];
            int rc =  HMAC_Update(&ctx, digest->b.buffer, digest->b.size);
            if (!rc) {
                LOG_ERR("HMAC Update failed: %s", ERR_error_string(rc, NULL));
                rval = TPM_RC_MEMORY;
                goto err;
            }
        }

        unsigned size = sizeof(tmpResult.t.buffer);
        int rc = HMAC_Final(&ctx, tmpResult.t.buffer, &size);
        if (!rc) {
            LOG_ERR("HMAC Final failed: %s", ERR_error_string(rc, NULL));
            rval = TPM_RC_MEMORY;
            goto err;
        tmpResult.t.size = size;

            rval = TSS2_SYS_RC_BAD_VALUE;
            goto err;
err:
    HMAC_CTX_cleanup(&ctx);

    return rval;
TPM_RC tpm_kdfa(TPMI_ALG_HASH hashAlg,
                rval = tpm_kdfa(session->authHash, &(key.b), label, &( session->nonceNewer.b ),