From b214121c888cd10d8d82bc90ac80a1d09c8f75e7 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 | 6 ++--- configure.ac | 5 ++-- src/kdfa.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++------ 3 files changed, 65 insertions(+), 12 deletions(-) diff --git a/Makefile.am b/Makefile.am index 34b753d2f..50a37ed50 100644 --- a/Makefile.am +++ b/Makefile.am @@ -34,9 +34,9 @@ ACLOCAL_AMFLAGS = -I m4 INCLUDE_DIRS = -I$(srcdir)/src -AM_CFLAGS = -DSAPI_CLIENT $(INCLUDE_DIRS) $(TPM20_TSS_CFLAGS) -AM_CXXFLAGS = -DSAPI_CLIENT $(INCLUDE_DIRS) $(TPM20_TSS_CXXFLAGS) -LDADD = src/libcommon.a $(TPM20_TSS_LIBS) $(TCTI_SOCK_LIBS) $(TCTI_DEV_LIBS) +AM_CFLAGS = -DSAPI_CLIENT $(INCLUDE_DIRS) $(TPM20_TSS_CFLAGS) $(CRYPTO_CFLAGS) +AM_CXXFLAGS = -DSAPI_CLIENT $(INCLUDE_DIRS) $(TPM20_TSS_CXXFLAGS) $(CRYPTO_CXXFLAGS) +LDADD = src/libcommon.a $(TPM20_TSS_LIBS) $(TCTI_SOCK_LIBS) $(TCTI_DEV_LIBS) $(CRYPTO_LIBS) TESTS = $(check_PROGRAMS) noinst_LIBRARIES = src/libcommon.a diff --git a/configure.ac b/configure.ac index 35257f905..b41dc6682 100644 --- a/configure.ac +++ b/configure.ac @@ -1,4 +1,4 @@ -AC_INIT([tpm2.0-tools], [1.1.0]) +AC_INIT([tpm2.0-tools], [1.1.1]) AC_CONFIG_MACRO_DIR([m4]) AC_PROG_CC AC_PROG_CXX @@ -9,7 +9,8 @@ AC_CONFIG_FILES([Makefile]) PKG_CHECK_MODULES([SAPI],[sapi]) PKG_CHECK_MODULES([TCTI_SOCK],[tcti-socket]) PKG_CHECK_MODULES([TCTI_DEV],[tcti-device]) -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)])], diff --git a/src/kdfa.c b/src/kdfa.c index a8e704370..7bba9e158 100644 --- a/src/kdfa.c +++ b/src/kdfa.c @@ -31,6 +31,26 @@ #include #include "changeEndian.h" +#include +#include + +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 KDFa( TPMI_ALG_HASH hashAlg, TPM2B *key, char *label, @@ -42,7 +62,7 @@ TPM_RC KDFa( TPMI_ALG_HASH hashAlg, TPM2B *key, char *label, 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; @@ -87,8 +107,23 @@ TPM_RC KDFa( TPMI_ALG_HASH hashAlg, TPM2B *key, char *label, i = 1; + const EVP_MD *md = tpm_algorithm_to_openssl_digest(hashAlg); + if (!md) { + fprintf(stderr, "Algorithm not supported for hmac: %x\n", 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) { + fprintf(stderr, "HMAC Init failed: %s\n", ERR_error_string(rc, NULL)); + return TPM_RC_MEMORY; + } + while( resultKey->t.size < bytes ) { + TPM2B_DIGEST tmpResult; // Inner loop i_Swizzled = CHANGE_ENDIAN_DWORD( i ); @@ -100,7 +135,7 @@ TPM_RC KDFa( TPMI_ALG_HASH hashAlg, TPM2B *key, char *label, bufferList[j++] = (TPM2B_DIGEST *)contextU; bufferList[j++] = (TPM2B_DIGEST *)contextV; bufferList[j++] = (TPM2B_DIGEST *)&(tpm2bBits.b); - bufferList[j++] = (TPM2B_DIGEST *)0; + bufferList[j] = (TPM2B_DIGEST *)0; #ifdef DEBUG OpenOutFile( &outFp ); for( j = 0; bufferList[j] != 0; j++ ) @@ -110,10 +145,24 @@ TPM_RC KDFa( TPMI_ALG_HASH hashAlg, TPM2B *key, char *label, } CloseOutFile( &outFp ); #endif - rval = (*HmacFunctionPtr )( hashAlg, key, (TPM2B **)&( bufferList[0] ), &tmpResult ); - if( rval != TPM_RC_SUCCESS ) - { - return( rval ); + + 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) { + fprintf(stderr, "HMAC Update failed: %s\n", 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) { + fprintf(stderr, "HMAC Final failed: %s\n", ERR_error_string(rc, NULL)); + rval = TPM_RC_MEMORY; + goto err; } ConcatSizedByteBuffer( resultKey, &(tmpResult.b) ); @@ -129,5 +178,8 @@ TPM_RC KDFa( TPMI_ALG_HASH hashAlg, TPM2B *key, char *label, CloseOutFile( &outFp ); #endif - return TPM_RC_SUCCESS; +err: + HMAC_CTX_cleanup(&ctx); + + return rval; }