From: Kevin Kane Date: Thu, 10 Nov 2016 22:00:27 +0000 (-0800) Subject: [IOT-1525] Keep credential resource encrypted on-disk on Windows X-Git-Tag: 1.3.0~1049^2~19 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=6733dabb19c4bedb59e58b5224438fd9875a94ff;p=platform%2Fupstream%2Fiotivity.git [IOT-1525] Keep credential resource encrypted on-disk on Windows The Windows platform provides a way to encrypt and decrypt data with system-managed keys. Since the cred resource contains sensitive symmetric and private keys, keep this resource encrypted on disk. Change-Id: I88f3cab76f2e782ce778bc64830491822b7d9842 Signed-off-by: Kevin Kane Reviewed-on: https://gerrit.iotivity.org/gerrit/15033 Reviewed-by: Alex Kelley Tested-by: jenkins-iotivity Reviewed-by: Mike Fenelon Reviewed-by: Randeep Singh Reviewed-by: Dan Mihai Reviewed-by: Greg Zaverucha --- diff --git a/resource/csdk/SConscript b/resource/csdk/SConscript index 0c9446a..208a58b 100644 --- a/resource/csdk/SConscript +++ b/resource/csdk/SConscript @@ -119,7 +119,7 @@ if target_os in ['windows', 'msys_nt']: else: liboctbstack_env.Textfile(target = 'octbstack.def', source = [File('octbstack_product.def')]) - liboctbstack_env.AppendUnique(LIBS = ['ws2_32', 'advapi32', 'iphlpapi', 'bcrypt']) + liboctbstack_env.AppendUnique(LIBS = ['ws2_32', 'advapi32', 'iphlpapi', 'bcrypt', 'crypt32']) else: liboctbstack_env.AppendUnique(LIBS = ['m']) diff --git a/resource/csdk/security/provisioning/unittest/SConscript b/resource/csdk/security/provisioning/unittest/SConscript index e9831ce..7c560c5 100644 --- a/resource/csdk/security/provisioning/unittest/SConscript +++ b/resource/csdk/security/provisioning/unittest/SConscript @@ -84,6 +84,7 @@ if target_os in ['msys_nt', 'windows']: sptest_env.AppendUnique(LIBS = ['ws2_32', 'advapi32', 'bcrypt', + 'crypt32', 'octbstack_static', 'iphlpapi']) else: diff --git a/resource/csdk/security/src/credresource.c b/resource/csdk/security/src/credresource.c index 54180ea..cb1a052 100644 --- a/resource/csdk/security/src/credresource.c +++ b/resource/csdk/security/src/credresource.c @@ -66,6 +66,11 @@ #define TAG "OIC_SRM_CREDL" +#ifdef HAVE_WINDOWS_H +#include +#endif + + /** Max credential types number used for TLS */ #define MAX_TYPE 2 /** Default cbor payload size. This value is increased in case of CborErrorOutOfMemory. @@ -735,6 +740,11 @@ OCStackResult CBORPayloadToCred(const uint8_t *cborPayload, size_t size, CborError cborFindResult = CborNoError; cbor_parser_init(cborPayload, size, 0, &parser, &credCbor); + if (!cbor_value_is_container(&credCbor)) + { + return OC_STACK_ERROR; + } + OicSecCred_t *headCred = (OicSecCred_t *) OICCalloc(1, sizeof(OicSecCred_t)); // Enter CRED Root Map @@ -1266,6 +1276,43 @@ exit: return cred; } +#ifdef HAVE_WINDOWS_H +/* Helper for UpdatePersistentStorage. */ +static OCStackResult CopyPayload(uint8_t **payload, size_t *payloadSize, const DATA_BLOB *source) +{ + OCStackResult res = OC_STACK_OK; + + if (source->cbData > *payloadSize) + { + /* Need more memory to copy encrypted payload out. */ + OICClearMemory(*payload, *payloadSize); + OICFree(*payload); + *payload = OICMalloc(source->cbData); + + if (NULL == *payload) + { + OIC_LOG_V(ERROR, TAG, "Failed to OICMalloc for encrypted payload: %u", GetLastError()); + res = OC_STACK_NO_MEMORY; + } + } + else if (source->cbData < *payloadSize) + { + /* Zero portion of payload we won't overwrite with the encrypted version. The + * later call to OICClearMemory won't cover this part of the buffer. + */ + OICClearMemory(*payload + source->cbData, *payloadSize - source->cbData); + } + + if (OC_STACK_OK == res) + { + memcpy(*payload, source->pbData, source->cbData); + *payloadSize = source->cbData; + } + + return res; +} +#endif + static bool UpdatePersistentStorage(const OicSecCred_t *cred) { bool ret = false; @@ -1282,15 +1329,53 @@ static bool UpdatePersistentStorage(const OicSecCred_t *cred) int secureFlag = 0; OCStackResult res = CredToCBORPayload(cred, &payload, &size, secureFlag); +#ifdef HAVE_WINDOWS_H + /* On Windows, keep the credential resource encrypted on disk to protect symmetric and private keys. Only the + * current user on this system will be able to decrypt it later, to help prevent credential theft. + */ + if ((OC_STACK_OK == res) && payload) + { + DATA_BLOB decryptedPayload = { .cbData = size, .pbData = payload }; + DATA_BLOB encryptedPayload = { .cbData = 0, .pbData = NULL }; + + if (CryptProtectData( + &decryptedPayload, + NULL, + NULL, + NULL, + NULL, + CRYPTPROTECT_UI_FORBIDDEN, + &encryptedPayload)) + { + res = CopyPayload(&payload, &size, &encryptedPayload); + ret = (OC_STACK_OK == res); + + /* For the returned data from CryptProtectData, LocalFree must be used to free. Don't use OICFree. */ + if (NULL != LocalFree(encryptedPayload.pbData)) + { + OIC_LOG_V(ERROR, TAG, "LocalFree failed on output from CryptProtectData; memory may be corrupted or leaked. Last error: %u.", GetLastError()); + assert(!"LocalFree failed"); + } + } + else + { + OIC_LOG_V(ERROR, TAG, "Failed to CryptProtectData cred resource: %u", GetLastError()); + res = OC_STACK_ERROR; + ret = false; + } + } +#endif + if ((OC_STACK_OK == res) && payload) { if (OC_STACK_OK == UpdateSecureResourceInPS(OIC_JSON_CRED_NAME, payload, size)) { ret = true; } - OICClearMemory(payload, size); - OICFree(payload); } + + OICClearMemory(payload, size); + OICFree(payload); } else //Empty cred list { @@ -1299,6 +1384,7 @@ static bool UpdatePersistentStorage(const OicSecCred_t *cred) ret = true; } } + OIC_LOG(DEBUG, TAG, "OUT Cred UpdatePersistentStorage"); return ret; } @@ -2212,10 +2298,46 @@ OCStackResult InitCredResource() { OIC_LOG (DEBUG, TAG, "ReadSVDataFromPS failed"); } - if (data) + + if ((ret == OC_STACK_OK) && data) { // Read ACL resource from PS ret = CBORPayloadToCred(data, size, &gCred); + +#ifdef HAVE_WINDOWS_H + /* On Windows, if the credential payload isn't cleartext CBOR, it is encrypted. Decrypt and retry. */ + if (ret != OC_STACK_OK) + { + DATA_BLOB encryptedPayload = { .cbData = size, .pbData = data }; + DATA_BLOB decryptedPayload = { .cbData = 0, .pbData = NULL }; + + if (CryptUnprotectData( + &encryptedPayload, + NULL, + NULL, + NULL, + NULL, + CRYPTPROTECT_UI_FORBIDDEN, + &decryptedPayload)) + { + ret = CBORPayloadToCred(decryptedPayload.pbData, decryptedPayload.cbData, &gCred); + + /* For the returned data from CryptUnprotectData, LocalFree must be used to free. Don't use OICFree. */ + OICClearMemory(decryptedPayload.pbData, decryptedPayload.cbData); + if (NULL != LocalFree(decryptedPayload.pbData)) + { + OIC_LOG_V(ERROR, TAG, "LocalFree failed on output from CryptUnprotectData; memory may be corrupted or leaked. Last error: %u.", GetLastError()); + assert(!"LocalFree failed"); + } + } + else + { + /* Credential resource is corrupted, or we no longer have access to the encryption key to decrypt it. */ + OIC_LOG_V(ERROR, TAG, "Failed to CryptUnprotectData cred resource: %u", GetLastError()); + ret = OC_STACK_ERROR; + } + } +#endif } /* @@ -2610,9 +2732,6 @@ exit: OCStackResult SetCredRownerId(const OicUuid_t* newROwner) { OCStackResult ret = OC_STACK_ERROR; - uint8_t *cborPayload = NULL; - size_t size = 0; - int secureFlag = 0; OicUuid_t prevId = {.id={0}}; if(NULL == newROwner) @@ -2624,27 +2743,19 @@ OCStackResult SetCredRownerId(const OicUuid_t* newROwner) ret = OC_STACK_NO_RESOURCE; } - if(newROwner && gCred) + if (newROwner && gCred) { memcpy(prevId.id, gCred->rownerID.id, sizeof(prevId.id)); memcpy(gCred->rownerID.id, newROwner->id, sizeof(newROwner->id)); - // This added '256' is arbitrary value that is added to cover the name of the resource, map addition and ending - size = GetCredKeyDataSize(gCred); - size += (256 * OicSecCredCount(gCred)); - ret = CredToCBORPayload(gCred, &cborPayload, &size, secureFlag); - VERIFY_SUCCESS(TAG, OC_STACK_OK == ret, ERROR); - - ret = UpdateSecureResourceInPS(OIC_JSON_CRED_NAME, cborPayload, size); - VERIFY_SUCCESS(TAG, OC_STACK_OK == ret, ERROR); + VERIFY_SUCCESS(TAG, UpdatePersistentStorage(gCred), ERROR); - OICFree(cborPayload); + ret = OC_STACK_OK; } return ret; exit: - OICFree(cborPayload); memcpy(gCred->rownerID.id, prevId.id, sizeof(prevId.id)); return ret; } diff --git a/resource/csdk/security/tool/SConscript b/resource/csdk/security/tool/SConscript index 3de9ad0..3a0dbd3 100644 --- a/resource/csdk/security/tool/SConscript +++ b/resource/csdk/security/tool/SConscript @@ -51,7 +51,7 @@ tools_env.AppendUnique(RPATH = [tools_env.get('BUILD_DIR')]) if target_os in ['msys_nt', 'windows']: # octbstack.dll doesn't export all the functions called by this app, so use static LIBs instead. - tools_env.AppendUnique(LIBS = ['ws2_32', 'bcrypt', 'iphlpapi', + tools_env.AppendUnique(LIBS = ['ws2_32', 'bcrypt', 'iphlpapi', 'crypt32', 'coap', 'tinydtls', 'mbedtls', 'mbedx509', 'mbedcrypto', 'octbstack_static', 'ocsrm', 'connectivity_abstraction']) elif target_os in ['darwin']: diff --git a/resource/csdk/security/unittest/SConscript b/resource/csdk/security/unittest/SConscript index b284319..cf846af 100644 --- a/resource/csdk/security/unittest/SConscript +++ b/resource/csdk/security/unittest/SConscript @@ -73,7 +73,7 @@ if srmtest_env.get('MULTIPLE_OWNER') == '1': if target_os == 'windows': srmtest_env.AppendUnique(LINKFLAGS = ['/subsystem:CONSOLE']) - srmtest_env.AppendUnique(LIBS = ['advapi32', 'bcrypt', 'kernel32', 'ws2_32', 'iphlpapi', 'octbstack_static']) + srmtest_env.AppendUnique(LIBS = ['advapi32', 'bcrypt', 'crypt32', 'kernel32', 'ws2_32', 'iphlpapi', 'octbstack_static']) else: # TODO: Implement feature check. srmtest_env.AppendUnique(CPPDEFINES = ['HAVE_LOCALTIME_R']) diff --git a/resource/csdk/stack/test/SConscript b/resource/csdk/stack/test/SConscript index 1b4d6a5..5118d0f 100644 --- a/resource/csdk/stack/test/SConscript +++ b/resource/csdk/stack/test/SConscript @@ -59,7 +59,7 @@ if stacktest_env.get('LOGGING'): stacktest_env.AppendUnique(CPPDEFINES = ['TB_LOG']) if target_os in ['msys_nt', 'windows']: - stacktest_env.AppendUnique(LIBS = ['ws2_32', 'iphlpapi', 'kernel32', 'bcrypt', 'advapi32']) + stacktest_env.AppendUnique(LIBS = ['ws2_32', 'iphlpapi', 'kernel32', 'bcrypt', 'advapi32', 'crypt32']) else: stacktest_env.PrependUnique(LIBS = ['m']) diff --git a/resource/unittests/SConscript b/resource/unittests/SConscript index afaaeff..fbe6c94 100644 --- a/resource/unittests/SConscript +++ b/resource/unittests/SConscript @@ -47,7 +47,7 @@ unittests_env.PrependUnique(CPPPATH = [ ]) if target_os in ['windows']: - unittests_env.AppendUnique(LIBS = ['ws2_32', 'advapi32', 'iphlpapi', 'bcrypt']) + unittests_env.AppendUnique(LIBS = ['ws2_32', 'advapi32', 'iphlpapi', 'bcrypt', 'crypt32']) unittests_env.AppendUnique(CPPPATH = ['#extlibs/boost/boost']) if unittests_env.get('SECURED') == '1':