From 4589605d424846c5e9b36fa4442acfd07eac1cce Mon Sep 17 00:00:00 2001 From: Aleksey Volkov Date: Mon, 28 Aug 2017 08:50:32 +0300 Subject: [PATCH] [IOT-2641] /cred resource rownerid fix This changes includes some refactoring of credential resource design: each credential structure instance has own rowner id value and it was changed to common rowner value. IOT-2641 depends on it. Tested with CT1.7.9.3 and 1.7.9.1, 1.7.4.1 json2cbor and svrdbeditor sources should be fixed accordingly by their owners Change-Id: I50afae10ac9f702c86d321dcf758525968f7bc31 Signed-off-by: Aleksey Volkov --- .../experimental/securevirtualresourcetypes.h | 3 +- .../security/include/internal/security_internals.h | 2 +- resource/csdk/security/src/credresource.c | 86 +++++++++------------- resource/csdk/security/tool/json2cbor.c | 4 +- .../tool/svrdbeditor_src/svrdbeditorcred.c | 6 +- .../csdk/security/unittest/credentialresource.cpp | 24 +++--- 6 files changed, 57 insertions(+), 68 deletions(-) diff --git a/resource/csdk/security/include/experimental/securevirtualresourcetypes.h b/resource/csdk/security/include/experimental/securevirtualresourcetypes.h index 5f483e1..ee70905 100644 --- a/resource/csdk/security/include/experimental/securevirtualresourcetypes.h +++ b/resource/csdk/security/include/experimental/securevirtualresourcetypes.h @@ -562,9 +562,8 @@ struct OicSecCred #endif /* __WITH_DTLS__ or __WITH_TLS__*/ OicSecKey_t privateData; // 6:R:S:N:oic.sec.key char *period; // 7:R:S:N:String - OicUuid_t rownerID; // 8:R:S:Y:oic.uuid #ifdef MULTIPLE_OWNER - OicUuid_t *eownerID; //9:R:S:N:oic.uuid + OicUuid_t *eownerID; //8:R:S:N:oic.uuid #endif //MULTIPLE_OWNER OicSecCred_t *next; }; diff --git a/resource/csdk/security/include/internal/security_internals.h b/resource/csdk/security/include/internal/security_internals.h index 3a805df..a5802e9 100644 --- a/resource/csdk/security/include/internal/security_internals.h +++ b/resource/csdk/security/include/internal/security_internals.h @@ -114,7 +114,7 @@ OCStackResult CreateCredResource(); * @return ::OC_STACK_OK if conversion is successful, else ::OC_STACK_ERROR if unsuccessful. */ OCStackResult CBORPayloadToCred(const uint8_t *cborPayload, size_t size, - OicSecCred_t **secCred); + OicSecCred_t **secCred, OicUuid_t **rownerId); /** * This internal method is used to create '/oic/sec/doxm' resource. diff --git a/resource/csdk/security/src/credresource.c b/resource/csdk/security/src/credresource.c index 0a48d73..cbc7584 100644 --- a/resource/csdk/security/src/credresource.c +++ b/resource/csdk/security/src/credresource.c @@ -94,6 +94,7 @@ static const uint8_t ROLEID_MAP_SIZE = 1; static OicSecCred_t *gCred = NULL; static OCResourceHandle gCredHandle = NULL; +static OicUuid_t gRownerId = { .id = { 0 } }; typedef enum CredCompareResult{ CRED_CMP_EQUAL = 0, @@ -698,7 +699,7 @@ OCStackResult CredToCBORPayload(const OicSecCred_t *credS, uint8_t **cborPayload cbor_encoder_init(&encoder, outPayload, cborLen, 0); - size_t credRootMapSize = (NULL == cred)? CRED_EMPTY_ROOT_MAP_SIZE : CRED_ROOT_MAP_SIZE; + size_t credRootMapSize = CRED_ROOT_MAP_SIZE; // Create CRED Root Map (creds, rownerid) cborEncoderResult = cbor_encoder_create_map(&encoder, &credRootMap, credRootMapSize); @@ -901,16 +902,13 @@ OCStackResult CredToCBORPayload(const OicSecCred_t *credS, uint8_t **cborPayload cborEncoderResult = cbor_encode_text_string(&credRootMap, OIC_JSON_ROWNERID_NAME, strlen(OIC_JSON_ROWNERID_NAME)); VERIFY_CBOR_SUCCESS(TAG, cborEncoderResult, "Failed Adding rownerid Name."); - OicUuid_t emptyUuid = { .id = { 0 } }; - const OicUuid_t* rownerID = (NULL == cred)? &emptyUuid : &cred->rownerID; - ret = ConvertUuidToStr(rownerID, &rowner); + ret = ConvertUuidToStr(&gRownerId, &rowner); VERIFY_SUCCESS(TAG, ret == OC_STACK_OK, ERROR); cborEncoderResult = cbor_encode_text_string(&credRootMap, rowner, strlen(rowner)); VERIFY_CBOR_SUCCESS(TAG, cborEncoderResult, "Failed Addding rownerid Value."); OICFree(rowner); } - if (cred != NULL) { //RT -- Mandatory CborEncoder rtArray; @@ -986,7 +984,7 @@ exit: } OCStackResult CBORPayloadToCred(const uint8_t *cborPayload, size_t size, - OicSecCred_t **secCred) + OicSecCred_t **secCred, OicUuid_t **rownerid) { if (NULL == cborPayload || NULL == secCred || NULL != *secCred || 0 == size) { @@ -1261,17 +1259,14 @@ OCStackResult CBORPayloadToCred(const uint8_t *cborPayload, size_t size, char *stRowner = NULL; cborFindResult = cbor_value_dup_text_string(&CredRootMap, &stRowner, &len, NULL); VERIFY_CBOR_SUCCESS(TAG, cborFindResult, "Failed Finding Rownerid Value."); - - ret = ConvertStrToUuid(stRowner, &headCred->rownerID); + *rownerid = (OicUuid_t *) OICCalloc(1, sizeof(OicUuid_t)); + VERIFY_NOT_NULL(TAG, *rownerid, ERROR); + ret = ConvertStrToUuid(stRowner, *rownerid); //Because cbor using malloc directly //It is required to use free() instead of OICFree free(stRowner); VERIFY_SUCCESS(TAG, (ret == OC_STACK_OK), ERROR); } - else if (NULL != gCred) - { - memcpy(&(headCred->rownerID), &(gCred->rownerID), sizeof(OicUuid_t)); - } //Because cbor using malloc directly //It is required to use free() instead of OICFree free(tagName); @@ -1308,20 +1303,23 @@ bool IsValidCredentialAccessForSubOwner(const OicUuid_t* uuid, const uint8_t *cb { OicSecCred_t* cred = NULL; bool isValidCred = false; + OicUuid_t *rownerId = NULL; OIC_LOG_BUFFER(DEBUG, TAG, cborPayload, size); VERIFY_NOT_NULL(TAG, uuid, ERROR); VERIFY_NOT_NULL(TAG, cborPayload, ERROR); VERIFY_SUCCESS(TAG, 0 != size, ERROR); - VERIFY_SUCCESS(TAG, OC_STACK_OK == CBORPayloadToCred(cborPayload, size, &cred), ERROR); + VERIFY_SUCCESS(TAG, OC_STACK_OK == CBORPayloadToCred(cborPayload, size, &cred, &rownerId), ERROR); VERIFY_NOT_NULL(TAG, cred, ERROR); VERIFY_NOT_NULL(TAG, cred->eownerID, ERROR); + VERIFY_NOT_NULL(TAG, rownerId, ERROR); VERIFY_SUCCESS(TAG, (memcmp(cred->eownerID->id, uuid->id, sizeof(uuid->id)) == 0), ERROR); isValidCred = true; exit: + OICFree(rownerId); DeleteCredList(cred); return isValidCred; @@ -1335,6 +1333,7 @@ OicSecCred_t * GenerateCredential(const OicUuid_t * subject, OicSecCredType_t cr { OIC_LOG(DEBUG, TAG, "IN GenerateCredential"); + OC_UNUSED(rownerID); (void)publicData; OCStackResult ret = OC_STACK_ERROR; @@ -1373,8 +1372,6 @@ OicSecCred_t * GenerateCredential(const OicUuid_t * subject, OicSecCredType_t cr cred->privateData.encoding = privateData->encoding; } - VERIFY_NOT_NULL(TAG, rownerID, ERROR); - memcpy(&cred->rownerID, rownerID, sizeof(OicUuid_t)); #ifdef MULTIPLE_OWNER if(eownerID) @@ -1838,14 +1835,6 @@ OCStackResult AddCredential(OicSecCred_t * newCred) OIC_LOG(DEBUG, TAG, "Adding New Cred"); LL_APPEND(gCred, newCred); - if (gCred != NULL) - { - OicUuid_t emptyOwner = { .id = {0} }; - if (memcmp(&(newCred->rownerID), &emptyOwner, sizeof(OicUuid_t)) != 0) - { - memcpy(&(gCred->rownerID), &(newCred->rownerID), sizeof(OicUuid_t)); - } - } saveToDB: @@ -2439,12 +2428,6 @@ static OCEntityHandlerResult HandleNewCredential(OCEntityHandlerRequest *ehReque { if(IsEmptyCred(cred)) { - if(memcmp(cred->rownerID.id, emptyUuid.id, sizeof(emptyUuid.id)) != 0) - { - OIC_LOG(INFO, TAG, "CRED's rowner will be updated."); - if (gCred != NULL) - { - memcpy(gCred->rownerID.id, cred->rownerID.id, sizeof(cred->rownerID.id)); if (UpdatePersistentStorage(gCred)) { ret = OC_EH_CHANGED; @@ -2453,12 +2436,6 @@ static OCEntityHandlerResult HandleNewCredential(OCEntityHandlerRequest *ehReque { ret = OC_EH_ERROR; } - } - } - else - { - ret = OC_EH_ERROR; - } } else { @@ -2495,6 +2472,7 @@ static OCEntityHandlerResult HandlePostRequest(OCEntityHandlerRequest* ehRequest static uint16_t previousMsgId = 0; // Get binary representation of cbor OicSecCred_t *cred = NULL; + OicUuid_t *rownerId = NULL; uint8_t *payload = (((OCSecurityPayload*)ehRequest->payload)->securityData); size_t size = (((OCSecurityPayload*)ehRequest->payload)->payloadSize); @@ -2509,7 +2487,7 @@ static OCEntityHandlerResult HandlePostRequest(OCEntityHandlerRequest* ehRequest goto exit; } - res = CBORPayloadToCred(payload, size, &cred); + res = CBORPayloadToCred(payload, size, &cred, &rownerId); if (OC_STACK_OK == res) { @@ -2541,9 +2519,14 @@ static OCEntityHandlerResult HandlePostRequest(OCEntityHandlerRequest* ehRequest break; } } + if (OC_EH_CHANGED == ret && NULL != rownerId) + { + memcpy(&gRownerId, rownerId, sizeof(OicUuid_t)); + } } exit: + OICFree(rownerId); if (OC_EH_CHANGED != ret) { if (NULL != cred) @@ -2698,6 +2681,7 @@ OCStackResult InitCredResource() { OCStackResult ret = OC_STACK_ERROR; OicSecCred_t* cred = NULL; + OicUuid_t *rownerId = NULL; //Read Cred resource from PS uint8_t *data = NULL; @@ -2712,7 +2696,7 @@ OCStackResult InitCredResource() if ((ret == OC_STACK_OK) && data) { // Read Cred resource from PS - ret = CBORPayloadToCred(data, size, &gCred); + ret = CBORPayloadToCred(data, size, &gCred, &rownerId); #ifdef HAVE_WINDOWS_H /* On Windows, if the credential payload isn't cleartext CBOR, it is encrypted. Decrypt and retry. */ @@ -2742,7 +2726,7 @@ OCStackResult InitCredResource() CRYPTPROTECT_UI_FORBIDDEN, &decryptedPayload)) { - ret = CBORPayloadToCred(decryptedPayload.pbData, decryptedPayload.cbData, &gCred); + ret = CBORPayloadToCred(decryptedPayload.pbData, decryptedPayload.cbData, &gCred, &rownerId); /* For the returned data from CryptUnprotectData, LocalFree must be used to free. Don't use OICFree. */ OICClearMemory(decryptedPayload.pbData, decryptedPayload.cbData); @@ -2800,9 +2784,13 @@ OCStackResult InitCredResource() } } - if (0 == memcmp(&gCred->rownerID, &emptyUuid, sizeof(OicUuid_t))) + if (NULL == rownerId || 0 == memcmp(rownerId, &emptyUuid, sizeof(OicUuid_t))) { - memcpy(&gCred->rownerID, &deviceID, sizeof(OicUuid_t)); + memcpy(&gRownerId, &deviceID, sizeof(OicUuid_t)); + } + else + { + memcpy(&gRownerId, rownerId, sizeof(OicUuid_t)); } if (!UpdatePersistentStorage(gCred)) @@ -2817,6 +2805,7 @@ exit: OIC_LOG(DEBUG, TAG, "OUT InitCredResource."); OICClearMemory(data, size); OICFree(data); + OICFree(rownerId); return ret; } @@ -2874,7 +2863,6 @@ OicSecCred_t* GetCredEntryByCredId(const uint16_t credId) cred->credId = tmpCred->credId; cred->credType = tmpCred->credType; memcpy(cred->subject.id, tmpCred->subject.id , sizeof(cred->subject.id)); - memcpy(cred->rownerID.id, tmpCred->rownerID.id , sizeof(cred->rownerID.id)); if (tmpCred->period) { cred->period = OICStrdup(tmpCred->period); @@ -3219,15 +3207,11 @@ OCStackResult SetCredRownerId(const OicUuid_t* newROwner) { ret = OC_STACK_INVALID_PARAM; } - if(NULL == gCred) - { - ret = OC_STACK_NO_RESOURCE; - } - if (newROwner && gCred) + if (newROwner) { - memcpy(prevId.id, gCred->rownerID.id, sizeof(prevId.id)); - memcpy(gCred->rownerID.id, newROwner->id, sizeof(newROwner->id)); + memcpy(prevId.id, gRownerId.id, sizeof(prevId.id)); + memcpy(gRownerId.id, newROwner->id, sizeof(newROwner->id)); VERIFY_SUCCESS(TAG, UpdatePersistentStorage(gCred), ERROR); @@ -3237,15 +3221,15 @@ OCStackResult SetCredRownerId(const OicUuid_t* newROwner) return ret; exit: - memcpy(gCred->rownerID.id, prevId.id, sizeof(prevId.id)); + memcpy(gRownerId.id, prevId.id, sizeof(prevId.id)); return ret; } OCStackResult GetCredRownerId(OicUuid_t *rowneruuid) { - if (gCred && rowneruuid) + if (rowneruuid) { - memcpy(&(rowneruuid->id), &(gCred->rownerID.id), sizeof(rowneruuid->id)); + memcpy(&(rowneruuid->id), &(gRownerId.id), sizeof(rowneruuid->id)); return OC_STACK_OK; } return OC_STACK_ERROR; diff --git a/resource/csdk/security/tool/json2cbor.c b/resource/csdk/security/tool/json2cbor.c index 9d56f03..90799cf 100644 --- a/resource/csdk/security/tool/json2cbor.c +++ b/resource/csdk/security/tool/json2cbor.c @@ -1327,13 +1327,13 @@ OicSecCred_t *JSONToCredBin(const char *jsonStr) } // rownerid - cJSON *jsonCredObj = cJSON_GetObjectItem(jsonCredMap, OIC_JSON_ROWNERID_NAME); +/* cJSON *jsonCredObj = cJSON_GetObjectItem(jsonCredMap, OIC_JSON_ROWNERID_NAME); VERIFY_NOT_NULL(TAG, jsonCredObj, ERROR); VERIFY_SUCCESS(TAG, cJSON_String == jsonCredObj->type, ERROR); ret = ConvertStrToUuid(jsonCredObj->valuestring, &headCred->rownerID); VERIFY_SUCCESS(TAG, OC_STACK_OK == ret, ERROR); ret = OC_STACK_OK; - +*/ exit: cJSON_Delete(jsonRoot); if (OC_STACK_OK != ret) diff --git a/resource/csdk/security/tool/svrdbeditor_src/svrdbeditorcred.c b/resource/csdk/security/tool/svrdbeditor_src/svrdbeditorcred.c index 627aec9..9a03ec9 100644 --- a/resource/csdk/security/tool/svrdbeditor_src/svrdbeditorcred.c +++ b/resource/csdk/security/tool/svrdbeditor_src/svrdbeditorcred.c @@ -67,6 +67,7 @@ void RefreshCred() OicSecCred_t *credList = NULL; OicSecCred_t *cred = NULL; OicSecCred_t *tmpCred = NULL; + OicUuid_t *rownerId = NULL; //Load security resouce data from SVR DB. ocResult = GetSecureVirtualDatabaseFromPS(OIC_JSON_CRED_NAME, &secPayload, &payloadSize); if (OC_STACK_OK != ocResult) @@ -76,7 +77,7 @@ void RefreshCred() } if (secPayload && 0 != payloadSize) { - ocResult = CBORPayloadToCred(secPayload, payloadSize, &credList); + ocResult = CBORPayloadToCred(secPayload, payloadSize, &credList, &rownerId); if (OC_STACK_OK != ocResult) { PRINT_ERR("CBORPayloadToCred : %d", ocResult); @@ -85,6 +86,7 @@ void RefreshCred() } } OICFree(secPayload); + OICFree(rownerId); DeInitCredResource(); //Add the loaded credentials into gCred of CredResource module in order to use the credential management mechanism. @@ -358,7 +360,7 @@ void PrintCredList(const OicSecCred_t *creds) if (!isEmptyList) { PRINT_PROG("%15s : ", OIC_JSON_ROWNERID_NAME); - PrintUuid(&(creds->rownerID)); +// PrintUuid(&(creds->rownerID)); } else { diff --git a/resource/csdk/security/unittest/credentialresource.cpp b/resource/csdk/security/unittest/credentialresource.cpp index 731a8db..56412b1 100644 --- a/resource/csdk/security/unittest/credentialresource.cpp +++ b/resource/csdk/security/unittest/credentialresource.cpp @@ -57,7 +57,6 @@ OicSecCred_t * getCredList() VERIFY_NOT_NULL(TAG, cred->privateData.data, ERROR); OICStrcpy((char *)cred->privateData.data, strlen("My private Key11")+1,"My private Key11"); // use |memcpy| for copying full-lengthed UUID without null termination - memcpy(cred->rownerID.id, "aaaaaaaaaaaaaaaa", sizeof(cred->rownerID.id)); cred->next = (OicSecCred_t*)OICCalloc(1, sizeof(*cred->next)); VERIFY_NOT_NULL(TAG, cred->next, ERROR); cred->next->credId = 5678; @@ -80,7 +79,6 @@ OicSecCred_t * getCredList() OICStrcpy(cred->next->publicData.data, sz,"My Public Key123"); #endif // use |memcpy| for copying full-lengthed UUID without null termination - memcpy(cred->next->rownerID.id, "bbbbbbbbbbbbbbbb", sizeof(cred->next->rownerID.id)); return cred; @@ -113,7 +111,6 @@ static void printCred(const OicSecCred_t * cred) OIC_LOG_V(INFO, TAG, "cred->publicData.data = %s", credTmp1->publicData.data); } #endif /* __WITH_DTLS__ */ - OIC_LOG_V(INFO, TAG, "cred->rownerID = %s", credTmp1->rownerID.id); } } @@ -254,9 +251,12 @@ TEST(CredResourceTest, CBORPayloadToCredVALID) ASSERT_TRUE(NULL != payload); OicSecCred_t *cred2 = NULL; - EXPECT_EQ(OC_STACK_OK, CBORPayloadToCred(payload, size, &cred2)); + OicUuid_t *rownerId = NULL; + EXPECT_EQ(OC_STACK_OK, CBORPayloadToCred(payload, size, &cred2, &rownerId)); OICFree(payload); ASSERT_TRUE(cred2 != NULL); + ASSERT_TRUE(rownerId != NULL); + OICFree(rownerId); DeleteCredList(cred2); } @@ -276,12 +276,14 @@ TEST(CredResourceTest, CBORPayloadToCredSecureVALID) ASSERT_TRUE(NULL != payload); OicSecCred_t *cred2 = NULL; - EXPECT_EQ(OC_STACK_OK, CBORPayloadToCred(payload, size, &cred2)); + OicUuid_t *rownerId = NULL; + EXPECT_EQ(OC_STACK_OK, CBORPayloadToCred(payload, size, &cred2, &rownerId)); ASSERT_TRUE(cred2 != NULL); ASSERT_TRUE(NULL == cred2->privateData.data); ASSERT_TRUE(0 == cred2->privateData.len); OICFree(payload); + OICFree(rownerId); DeleteCredList(cred1); DeleteCredList(cred2); @@ -290,14 +292,16 @@ TEST(CredResourceTest, CBORPayloadToCredSecureVALID) TEST(CredResourceTest, CBORPayloadToCredNULL) { OicSecCred_t *cred = NULL; - EXPECT_EQ(OC_STACK_INVALID_PARAM, CBORPayloadToCred(NULL, 0, NULL)); + OicUuid_t *rownerId = NULL; + EXPECT_EQ(OC_STACK_INVALID_PARAM, CBORPayloadToCred(NULL, 0, NULL, NULL)); uint8_t *cborPayload = (uint8_t *) OICCalloc(1, 10); ASSERT_TRUE(NULL != cborPayload); - EXPECT_EQ(OC_STACK_INVALID_PARAM, CBORPayloadToCred(NULL, 0, &cred)); - EXPECT_EQ(OC_STACK_INVALID_PARAM, CBORPayloadToCred(cborPayload, 0, NULL)); + EXPECT_EQ(OC_STACK_INVALID_PARAM, CBORPayloadToCred(NULL, 0, &cred, &rownerId)); + EXPECT_EQ(OC_STACK_INVALID_PARAM, CBORPayloadToCred(cborPayload, 0, NULL, NULL)); cred = getCredList(); - EXPECT_EQ(OC_STACK_INVALID_PARAM, CBORPayloadToCred(cborPayload, 0, &cred)); + EXPECT_EQ(OC_STACK_INVALID_PARAM, CBORPayloadToCred(cborPayload, 0, &cred, &rownerId)); DeleteCredList(cred); + OICFree(rownerId); OICFree(cborPayload); } @@ -388,5 +392,5 @@ TEST(CredAddTmpPskWithPINTest, NullSubject) #endif // __WITH_DTLS__ or __WITH_TLS__ TEST(CredCBORPayloadToCredTest, NullPayload) { - EXPECT_EQ(OC_STACK_INVALID_PARAM, CBORPayloadToCred(NULL, 0, NULL)); + EXPECT_EQ(OC_STACK_INVALID_PARAM, CBORPayloadToCred(NULL, 0, NULL, NULL)); } -- 2.7.4