[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 <kkane@microsoft.com>
Reviewed-on: https://gerrit.iotivity.org/gerrit/15033
Reviewed-by: Alex Kelley <alexke@microsoft.com>
Tested-by: jenkins-iotivity <jenkins-iotivity@opendaylight.org>
Reviewed-by: Mike Fenelon <mike.fenelon@microsoft.com>
Reviewed-by: Randeep Singh <randeep.s@samsung.com>
Reviewed-by: Dan Mihai <Daniel.Mihai@microsoft.com>
Reviewed-by: Greg Zaverucha <gregz@microsoft.com>

diff --git a/resource/csdk/SConscript b/resource/csdk/SConscript
index 0c9446a..208a58b 100644 (file)
--- 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')])
 
        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'])
 
 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 (file)
--- 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',
     sptest_env.AppendUnique(LIBS = ['ws2_32',
                                     'advapi32',
                                     'bcrypt',

+                                    'crypt32',

                                     'octbstack_static',
                                     'iphlpapi'])
 else:
                                     'octbstack_static',
                                     'iphlpapi'])
 else:

diff --git a/resource/csdk/security/src/credresource.c b/resource/csdk/security/src/credresource.c
index 54180ea..cb1a052 100644 (file)
--- a/resource/csdk/security/src/credresource.c
+++ b/resource/csdk/security/src/credresource.c
@@ -66,6 +66,11 @@
 
 #define TAG  "OIC_SRM_CREDL"
 
 
 #define TAG  "OIC_SRM_CREDL"
 

+#ifdef HAVE_WINDOWS_H
+#include <wincrypt.h>
+#endif
+
+

 /** Max credential types number used for TLS */
 #define MAX_TYPE 2
 /** Default cbor payload size. This value is increased in case of CborErrorOutOfMemory.
 /** 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);
 
     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
     OicSecCred_t *headCred = (OicSecCred_t *) OICCalloc(1, sizeof(OicSecCred_t));
 
     // Enter CRED Root Map


@@ -1266,6 +1276,43 @@ exit:
     return cred;
 }
 
     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;
 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);
 
         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;
             }
         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
     {
     }
     else //Empty cred list
     {


@@ -1299,6 +1384,7 @@ static bool UpdatePersistentStorage(const OicSecCred_t *cred)
             ret = true;
         }
     }
             ret = true;
         }
     }

+

     OIC_LOG(DEBUG, TAG, "OUT Cred UpdatePersistentStorage");
     return ret;
 }
     OIC_LOG(DEBUG, TAG, "OUT Cred UpdatePersistentStorage");
     return ret;
 }


@@ -2212,10 +2298,46 @@ OCStackResult InitCredResource()
     {
         OIC_LOG (DEBUG, TAG, "ReadSVDataFromPS failed");
     }
     {
         OIC_LOG (DEBUG, TAG, "ReadSVDataFromPS failed");
     }

-    if (data)
+
+    if ((ret == OC_STACK_OK) && data)

     {
         // Read ACL resource from PS
         ret = CBORPayloadToCred(data, size, &gCred);
     {
         // 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;
 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)
     OicUuid_t prevId = {.id={0}};
 
     if(NULL == newROwner)


@@ -2624,27 +2743,19 @@ OCStackResult SetCredRownerId(const OicUuid_t* newROwner)
         ret = OC_STACK_NO_RESOURCE;
     }
 
         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));
 
     {
         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:
     }
 
     return ret;
 
 exit:

-    OICFree(cborPayload);

     memcpy(gCred->rownerID.id, prevId.id, sizeof(prevId.id));
     return ret;
 }
     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 (file)
--- 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.
 
 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']:
                                    '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 (file)
--- 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'])
 
 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'])
 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 (file)
--- 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(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'])
 
 else:
        stacktest_env.PrependUnique(LIBS = ['m'])
 

diff --git a/resource/unittests/SConscript b/resource/unittests/SConscript
index afaaeff..fbe6c94 100644 (file)
--- a/resource/unittests/SConscript
+++ b/resource/unittests/SConscript
@@ -47,7 +47,7 @@ unittests_env.PrependUnique(CPPPATH = [
                ])
 
 if target_os in ['windows']:
                ])
 
 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':
        unittests_env.AppendUnique(CPPPATH = ['#extlibs/boost/boost'])
 
 if unittests_env.get('SECURED') == '1':