From 00ea3cc493b71ab4c96f5f66c8b9a019f0308fb79b8d28ca49374371b58d8d28 Mon Sep 17 00:00:00 2001 From: Matthias Gerstner Date: Thu, 29 Jun 2017 09:49:42 +0000 Subject: [PATCH] - 0002-kdfa-use-openssl-for-hmac-not-tpm.patch: fixed unexpected leak of cleartext password into the tpm when generating an HMAC in the context of tpm_kdfa() (key derivation function). OBS-URL: https://build.opensuse.org/package/show/security/tpm2.0-tools?expand=0&rev=23 --- 0002-kdfa-use-openssl-for-hmac-not-tpm.patch | 207 +++++++++++++++++++ tpm2.0-tools.changes | 7 + tpm2.0-tools.spec | 2 + 3 files changed, 216 insertions(+) create mode 100644 0002-kdfa-use-openssl-for-hmac-not-tpm.patch diff --git a/0002-kdfa-use-openssl-for-hmac-not-tpm.patch b/0002-kdfa-use-openssl-for-hmac-not-tpm.patch new file mode 100644 index 0000000..66019a3 --- /dev/null +++ b/0002-kdfa-use-openssl-for-hmac-not-tpm.patch @@ -0,0 +1,207 @@ +From c5d72beaab1cbbbe68271f4bc4b6670d69985157 Mon Sep 17 00:00:00 2001 +From: William Roberts +Date: Wed, 21 Jun 2017 09:32:32 -0700 +Subject: [PATCH] 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 +--- + Makefile.am | 4 +-- + configure.ac | 3 ++- + lib/tpm_kdfa.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++-------- + lib/tpm_kdfa.h | 2 +- + lib/tpm_session.c | 2 +- + 5 files changed, 72 insertions(+), 15 deletions(-) + +Index: tpm2.0-tools-2.0.0/Makefile.am +=================================================================== +--- tpm2.0-tools-2.0.0.orig/Makefile.am ++++ tpm2.0-tools-2.0.0/Makefile.am +@@ -35,10 +35,10 @@ 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) ++AM_CFLAGS := $(INCLUDE_DIRS) $(TPM20_TSS_CFLAGS) $(EXTRA_CFLAGS) $(CRYPTO_FLAGS) + AM_LDFLAGS := $(EXTRA_LDFLAGS) + +-LDADD = $(LIB_COMMON) $(TPM20_TSS_LIBS) $(TCTI_SOCK_LIBS) $(TCTI_DEV_LIBS) ++LDADD = $(LIB_COMMON) $(TPM20_TSS_LIBS) $(TCTI_SOCK_LIBS) $(TCTI_DEV_LIBS) $(CRYPTO_LIBS) + + sbin_PROGRAMS = \ + tools/tpm2_create \ +Index: tpm2.0-tools-2.0.0/configure.ac +=================================================================== +--- tpm2.0-tools-2.0.0.orig/configure.ac ++++ tpm2.0-tools-2.0.0/configure.ac +@@ -49,7 +49,8 @@ AS_IF( + [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)])], +Index: tpm2.0-tools-2.0.0/lib/tpm_kdfa.c +=================================================================== +--- tpm2.0-tools-2.0.0.orig/lib/tpm_kdfa.c ++++ tpm2.0-tools-2.0.0/lib/tpm_kdfa.c +@@ -27,20 +27,40 @@ + + #include + ++ #include ++#include ++ + #include "string-bytes.h" + #include "tpm_hmac.h" ++#include "log.h" ++ ++static const EVP_MD *tpm_algorithm_to_openssl_digest(TPMI_ALG_HASH algorithm) { + +-TPM_RC tpm_kdfa(TSS2_SYS_CONTEXT *sapi_context, TPMI_ALG_HASH hashAlg, ++ 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, + 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; + +@@ -64,8 +84,24 @@ TPM_RC tpm_kdfa(TSS2_SYS_CONTEXT *sapi_c + + 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 ); +@@ -77,21 +113,41 @@ TPM_RC tpm_kdfa(TSS2_SYS_CONTEXT *sapi_c + 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; + } +Index: tpm2.0-tools-2.0.0/lib/tpm_kdfa.h +=================================================================== +--- tpm2.0-tools-2.0.0.orig/lib/tpm_kdfa.h ++++ tpm2.0-tools-2.0.0/lib/tpm_kdfa.h +@@ -42,7 +42,7 @@ + * @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 ); + +Index: tpm2.0-tools-2.0.0/lib/tpm_session.c +=================================================================== +--- tpm2.0-tools-2.0.0.orig/lib/tpm_session.c ++++ tpm2.0-tools-2.0.0/lib/tpm_session.c +@@ -198,7 +198,7 @@ static TPM_RC StartAuthSession(TSS2_SYS_ + } + 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 ) ); + } + diff --git a/tpm2.0-tools.changes b/tpm2.0-tools.changes index 8914d1d..c73d1b4 100644 --- a/tpm2.0-tools.changes +++ b/tpm2.0-tools.changes @@ -1,3 +1,10 @@ +------------------------------------------------------------------- +Thu Jun 29 09:45:45 UTC 2017 - matthias.gerstner@suse.com + +- 0002-kdfa-use-openssl-for-hmac-not-tpm.patch: fixed unexpected leak of + cleartext password into the tpm when generating an HMAC in the context of + tpm_kdfa() (key derivation function). + ------------------------------------------------------------------- Tue Jun 20 08:35:29 UTC 2017 - matthias.gerstner@suse.com diff --git a/tpm2.0-tools.spec b/tpm2.0-tools.spec index 6fab3f9..a69cd75 100644 --- a/tpm2.0-tools.spec +++ b/tpm2.0-tools.spec @@ -29,6 +29,7 @@ Patch1: tpm2.0-tools-fix-gcc7.patch # this fixes an error with an unexpectedly large number of PCRS (bnc#1044419) # there's no release containing this fix yet Patch2: 0001-tpm2_listpcrs-use-TPM2_GetCapability-to-determine-PC.patch +Patch3: 0002-kdfa-use-openssl-for-hmac-not-tpm.patch BuildRequires: autoconf-archive BuildRequires: automake BuildRequires: gcc-c++ @@ -52,6 +53,7 @@ associated interfaces. %patch0 -p1 %patch1 -p1 %patch2 -p1 +%patch3 -p1 %build bash ./bootstrap