Commit aad182df authored by js126.lee's avatar js126.lee Committed by Randeep

Resolved security issue-IOT1003 & IOT1013

https://jira.iotivity.org/browse/IOT-1013
[PM C]OCProvisionPairwiseDevices returns OC_STACK_OK instead of
OC_STACK_INVALID_PARAM for provisioning same device

https://jira.iotivity.org/browse/IOT-1003
[PM C] APIs returns OC_STACK_INVALID_PARAM instead of
 OC_STACK_INVALID_CALLBACK while CB = NULL

-Patch 1: Resolved security Issue on 1.1-RC1
-Patch 2: Apply MR.Cho's review comment

Change-Id: I1a2d0265e756da41cffbcc60042e4f35de2dbcf5
Signed-off-by: default avatarjs126.lee <js126.lee@samsung.com>
Reviewed-on: https://gerrit.iotivity.org/gerrit/6219Reviewed-by: default avatarKyungsun Cho <goodsun.cho@samsung.com>
Reviewed-by: default avatarYonggoo Kang <ygace.kang@samsung.com>
Reviewed-by: default avatarChul Lee <chuls.lee@samsung.com>
Tested-by: default avatarjenkins-iotivity <jenkins-iotivity@opendaylight.org>
Reviewed-by: Randeep's avatarRandeep Singh <randeep.s@samsung.com>
parent bedf2f7a
......@@ -114,7 +114,7 @@ OCStackResult OCSetOwnerTransferCallbackData(OicSecOxm_t oxm, OTMCallbackData_t*
{
if(NULL == callbackData)
{
return OC_STACK_INVALID_PARAM;
return OC_STACK_INVALID_CALLBACK ;
}
return OTMSetOwnershipTransferCallbackData(oxm, callbackData);
......@@ -128,7 +128,11 @@ OCStackResult OCDoOwnershipTransfer(void* ctx,
{
return OC_STACK_INVALID_PARAM;
}
if (!resultCallback)
{
OIC_LOG(INFO, TAG, "OCDoOwnershipTransfer : NULL Callback");
return OC_STACK_INVALID_CALLBACK;
}
return OTMDoOwnershipTransfer(ctx, targetDevices, resultCallback);
}
......@@ -215,11 +219,21 @@ OCStackResult OCUnlinkDevices(void* ctx,
OCUuidList_t* idList = NULL;
size_t numOfDev = 0;
if (!pTargetDev1 || !pTargetDev2 || !resultCallback)
if (!pTargetDev1 || !pTargetDev2 || !pTargetDev1->doxm || !pTargetDev2->doxm)
{
OIC_LOG(ERROR, TAG, "OCUnlinkDevices : NULL parameters");
return OC_STACK_INVALID_PARAM;
}
if (!resultCallback)
{
OIC_LOG(INFO, TAG, "OCUnlinkDevices : NULL Callback");
return OC_STACK_INVALID_CALLBACK;
}
if (0 == memcmp(&pTargetDev1->doxm->deviceID, &pTargetDev2->doxm->deviceID, sizeof(OicUuid_t)))
{
OIC_LOG(INFO, TAG, "OCUnlinkDevices : Same device ID");
return OC_STACK_INVALID_PARAM;
}
// Get linked devices with the first device.
OCStackResult res = PDMGetLinkedDevices(&(pTargetDev1->doxm->deviceID), &idList, &numOfDev);
......@@ -277,11 +291,16 @@ OCStackResult OCRemoveDevice(void* ctx, unsigned short waitTimeForOwnedDeviceDis
{
OIC_LOG(INFO, TAG, "IN OCRemoveDevice");
OCStackResult res = OC_STACK_ERROR;
if (!pTargetDev || !resultCallback || 0 == waitTimeForOwnedDeviceDiscovery)
if (!pTargetDev || 0 == waitTimeForOwnedDeviceDiscovery)
{
OIC_LOG(INFO, TAG, "OCRemoveDevice : Invalied parameters");
return OC_STACK_INVALID_PARAM;
}
if (!resultCallback)
{
OIC_LOG(INFO, TAG, "OCRemoveDevice : NULL Callback");
return OC_STACK_INVALID_CALLBACK;
}
// Send DELETE requests to linked devices
OCStackResult resReq = OC_STACK_ERROR; // Check that we have to wait callback or not.
......@@ -551,16 +570,26 @@ OCStackResult OCProvisionPairwiseDevices(void* ctx, OicSecCredType_t type, size_
OCProvisionResultCB resultCallback)
{
if (!pDev1 || !pDev2 || !resultCallback)
if (!pDev1 || !pDev2 || !pDev1->doxm || !pDev2->doxm)
{
OIC_LOG(ERROR, TAG, "OCProvisionPairwiseDevices : Invalid parameters");
return OC_STACK_INVALID_PARAM;
}
if (!resultCallback)
{
OIC_LOG(INFO, TAG, "OCProvisionPairwiseDevices : NULL Callback");
return OC_STACK_INVALID_CALLBACK;
}
if (!(keySize == OWNER_PSK_LENGTH_128 || keySize == OWNER_PSK_LENGTH_256))
{
OIC_LOG(INFO, TAG, "OCProvisionPairwiseDevices : Invalid key size");
return OC_STACK_INVALID_PARAM;
}
if (0 == memcmp(&pDev1->doxm->deviceID, &pDev2->doxm->deviceID, sizeof(OicUuid_t)))
{
OIC_LOG(INFO, TAG, "OCProvisionPairwiseDevices : Same device ID");
return OC_STACK_INVALID_PARAM;
}
OIC_LOG(DEBUG, TAG, "Checking link in DB");
bool linkExists = true;
......
......@@ -629,12 +629,21 @@ OCStackResult SRPProvisionCredentials(void *ctx, OicSecCredType_t type, size_t k
const OCProvisionDev_t *pDev2,
OCProvisionResultCB resultCallback)
{
VERIFY_NON_NULL(TAG, pDev1, ERROR, OC_STACK_INVALID_PARAM);
if (SYMMETRIC_PAIR_WISE_KEY == type)
if (!pDev1 || !pDev2 || !pDev1->doxm || !pDev2->doxm)
{
VERIFY_NON_NULL(TAG, pDev2, ERROR, OC_STACK_INVALID_PARAM);
OIC_LOG(INFO, TAG, "SRPUnlinkDevices : NULL parameters");
return OC_STACK_INVALID_PARAM;
}
if (!resultCallback)
{
OIC_LOG(INFO, TAG, "SRPUnlinkDevices : NULL Callback");
return OC_STACK_INVALID_CALLBACK;
}
if (0 == memcmp(&pDev1->doxm->deviceID, &pDev2->doxm->deviceID, sizeof(OicUuid_t)))
{
OIC_LOG(INFO, TAG, "SRPUnlinkDevices : Same device ID");
return OC_STACK_INVALID_PARAM;
}
VERIFY_NON_NULL(TAG, resultCallback, ERROR, OC_STACK_INVALID_CALLBACK);
if (SYMMETRIC_PAIR_WISE_KEY == type &&
!(OWNER_PSK_LENGTH_128 == keySize || OWNER_PSK_LENGTH_256 == keySize))
......@@ -1298,11 +1307,22 @@ OCStackResult SRPUnlinkDevices(void* ctx,
{
OIC_LOG(INFO, TAG, "IN SRPUnlinkDevices");
if (!pTargetDev1 || !pTargetDev2 || !resultCallback)
if (!pTargetDev1 || !pTargetDev2 || !pTargetDev1->doxm || !pTargetDev2->doxm)
{
OIC_LOG(INFO, TAG, "SRPUnlinkDevices : NULL parameters");
return OC_STACK_INVALID_PARAM;
}
if (!resultCallback)
{
OIC_LOG(INFO, TAG, "SRPUnlinkDevices : NULL Callback");
return OC_STACK_INVALID_CALLBACK;
}
if (0 == memcmp(&pTargetDev1->doxm->deviceID, &pTargetDev2->doxm->deviceID, sizeof(OicUuid_t)))
{
OIC_LOG(INFO, TAG, "SRPUnlinkDevices : Same device ID");
return OC_STACK_INVALID_PARAM;
}
OIC_LOG(INFO, TAG, "Unlinking following devices: ");
PMPrintOCProvisionDev(pTargetDev1);
PMPrintOCProvisionDev(pTargetDev2);
......@@ -1560,11 +1580,16 @@ OCStackResult SRPRemoveDevice(void* ctx, unsigned short waitTimeForOwnedDeviceDi
{
OIC_LOG(INFO, TAG, "IN SRPRemoveDevice");
if (!pTargetDev || !resultCallback || 0 == waitTimeForOwnedDeviceDiscovery)
if (!pTargetDev || 0 == waitTimeForOwnedDeviceDiscovery)
{
OIC_LOG(INFO, TAG, "SRPRemoveDevice : NULL parameters");
return OC_STACK_INVALID_PARAM;
}
if (!resultCallback)
{
OIC_LOG(INFO, TAG, "SRPRemoveDevice : NULL Callback");
return OC_STACK_INVALID_CALLBACK;
}
// Declare variables in here to handle error cases with goto statement.
OCProvisionDev_t* pOwnedDevList = NULL;
......
......@@ -20,6 +20,41 @@
#include "gtest/gtest.h"
#include "ocprovisioningmanager.h"
static OicSecAcl_t acl1;
static OicSecAcl_t acl2;
static OCProvisionDev_t pDev1;
static OCProvisionDev_t pDev2;
static OicSecCredType_t credType = SYMMETRIC_PAIR_WISE_KEY;
static OicSecOxm_t oicSecDoxmJustWorks = OIC_JUST_WORKS;
static OicSecOxm_t oicSecDoxmRandomPin = OIC_RANDOM_DEVICE_PIN;
static OicSecDoxm_t defaultDoxm1 =
{
NULL, /* OicUrn_t *oxmType */
0, /* size_t oxmTypeLen */
&oicSecDoxmJustWorks, /* uint16_t *oxm */
1, /* size_t oxmLen */
OIC_JUST_WORKS, /* uint16_t oxmSel */
SYMMETRIC_PAIR_WISE_KEY,/* OicSecCredType_t sct */
false, /* bool owned */
{{0}}, /* OicUuid_t deviceID */
false, /* bool dpc */
{{0}}, /* OicUuid_t owner */
};
static OicSecDoxm_t defaultDoxm2 =
{
NULL, /* OicUrn_t *oxmType */
0, /* size_t oxmTypeLen */
&oicSecDoxmRandomPin, /* uint16_t *oxm */
1, /* size_t oxmLen */
OIC_RANDOM_DEVICE_PIN, /* uint16_t oxmSel */
SYMMETRIC_PAIR_WISE_KEY,/* OicSecCredType_t sct */
false, /* bool owned */
{{0}}, /* OicUuid_t deviceID */
false, /* bool dpc */
{{0}}, /* OicUuid_t owner */
};
static void provisioningCB (void* UNUSED1, int UNUSED2, OCProvisionResult_t *UNUSED3, bool UNUSED4)
{
//dummy callback
......@@ -29,25 +64,70 @@ static void provisioningCB (void* UNUSED1, int UNUSED2, OCProvisionResult_t *UNU
(void) UNUSED4;
}
TEST(OCProvisionPairwiseDevicesTest, NullDevice1)
{
pDev1.doxm = &defaultDoxm1;
uint8_t deviceId1[] = {0x64, 0x65, 0x76, 0x69, 0x63, 0x65, 0x49, 0x64};
memcpy(pDev1.doxm->deviceID.id, deviceId1, sizeof(deviceId1));
pDev2.doxm = &defaultDoxm2;
uint8_t deviceId2[] = {0x64, 0x65, 0x76, 0x69, 0x63, 0x65, 0x49, 0x63};
memcpy(pDev2.doxm->deviceID.id, deviceId2, sizeof(deviceId2));
EXPECT_EQ(OC_STACK_INVALID_PARAM, OCProvisionPairwiseDevices(NULL, credType,
OWNER_PSK_LENGTH_128, NULL, &acl1,
&pDev2, &acl2, &provisioningCB));
}
TEST(OCProvisionPairwiseDevicesTest, NullDevice2)
{
EXPECT_EQ(OC_STACK_INVALID_PARAM, OCProvisionPairwiseDevices(NULL, credType,
OWNER_PSK_LENGTH_128, &pDev1, &acl1,
NULL, &acl2, &provisioningCB));
}
TEST(OCProvisionPairwiseDevicesTest, SamelDeviceId)
{
EXPECT_EQ(OC_STACK_INVALID_PARAM, OCProvisionPairwiseDevices(NULL, credType,
OWNER_PSK_LENGTH_128, &pDev1, &acl1,
&pDev1, &acl2, &provisioningCB));
}
TEST(OCProvisionPairwiseDevicesTest, NullCallback)
{
EXPECT_EQ(OC_STACK_INVALID_CALLBACK, OCProvisionPairwiseDevices(NULL, credType,
OWNER_PSK_LENGTH_128, &pDev1, &acl1,
&pDev2, &acl2, NULL));
}
TEST(OCProvisionPairwiseDevicesTest, InvalidKeySize)
{
EXPECT_EQ(OC_STACK_INVALID_PARAM, OCProvisionPairwiseDevices(NULL, credType,
0, &pDev1, &acl1,
&pDev2, &acl2 ,&provisioningCB));
}
TEST(OCUnlinkDevicesTest, NullDevice1)
{
OCProvisionDev_t dev2;
EXPECT_EQ(OC_STACK_INVALID_PARAM, OCUnlinkDevices(NULL, NULL, &dev2, provisioningCB));
EXPECT_EQ(OC_STACK_INVALID_PARAM, OCUnlinkDevices(NULL, NULL, &pDev2, provisioningCB));
}
TEST(OCUnlinkDevicesTest, NullDevice2)
{
OCProvisionDev_t dev1;
EXPECT_EQ(OC_STACK_INVALID_PARAM, OCUnlinkDevices(NULL, &dev1, NULL, provisioningCB));
EXPECT_EQ(OC_STACK_INVALID_PARAM, OCUnlinkDevices(NULL, &pDev1, NULL, provisioningCB));
}
TEST(OCUnlinkDevicesTest, NullCallback)
{
OCProvisionDev_t dev1;
OCProvisionDev_t dev2;
EXPECT_EQ(OC_STACK_INVALID_PARAM, OCUnlinkDevices(NULL, &dev1, &dev2, NULL));
EXPECT_EQ(OC_STACK_INVALID_CALLBACK, OCUnlinkDevices(NULL, &pDev1, &pDev2, NULL));
}
TEST(OCUnlinkDevicesTest, SamelDeviceId)
{
EXPECT_EQ(OC_STACK_INVALID_PARAM, OCUnlinkDevices(NULL,&pDev1, &pDev1, &provisioningCB));
}
TEST(OCRemoveDeviceTest, NullTargetDevice)
{
unsigned short waitTime = 10 ;
......@@ -57,15 +137,13 @@ TEST(OCRemoveDeviceTest, NullTargetDevice)
TEST(OCRemoveDeviceTest, NullResultCallback)
{
unsigned short waitTime = 10;
OCProvisionDev_t dev1;
EXPECT_EQ(OC_STACK_INVALID_PARAM, OCRemoveDevice(NULL, waitTime, &dev1, NULL));
EXPECT_EQ(OC_STACK_INVALID_CALLBACK, OCRemoveDevice(NULL, waitTime, &pDev1, NULL));
}
TEST(OCRemoveDeviceTest, ZeroWaitTime)
{
unsigned short waitTime = 0;
OCProvisionDev_t dev1;
EXPECT_EQ(OC_STACK_INVALID_PARAM, OCRemoveDevice(NULL, waitTime, &dev1, provisioningCB));
EXPECT_EQ(OC_STACK_INVALID_PARAM, OCRemoveDevice(NULL, waitTime, &pDev1, provisioningCB));
}
TEST(OCGetDevInfoFromNetworkTest, NullUnOwnedDeviceInfo)
......
......@@ -26,6 +26,35 @@ static OCProvisionDev_t pDev2;
static OicSecCredType_t credType = SYMMETRIC_PAIR_WISE_KEY;
static OCProvisionDev_t selectedDeviceInfo;
static OicSecPconf_t pconf;
static OicSecOxm_t oicSecDoxmJustWorks = OIC_JUST_WORKS;
static OicSecOxm_t oicSecDoxmRandomPin = OIC_RANDOM_DEVICE_PIN;
static OicSecDoxm_t defaultDoxm1 =
{
NULL, /* OicUrn_t *oxmType */
0, /* size_t oxmTypeLen */
&oicSecDoxmJustWorks, /* uint16_t *oxm */
1, /* size_t oxmLen */
OIC_JUST_WORKS, /* uint16_t oxmSel */
SYMMETRIC_PAIR_WISE_KEY,/* OicSecCredType_t sct */
false, /* bool owned */
{{0}}, /* OicUuid_t deviceID */
false, /* bool dpc */
{{0}}, /* OicUuid_t owner */
};
static OicSecDoxm_t defaultDoxm2 =
{
NULL, /* OicUrn_t *oxmType */
0, /* size_t oxmTypeLen */
&oicSecDoxmRandomPin, /* uint16_t *oxm */
1, /* size_t oxmLen */
OIC_RANDOM_DEVICE_PIN, /* uint16_t oxmSel */
SYMMETRIC_PAIR_WISE_KEY,/* OicSecCredType_t sct */
false, /* bool owned */
{{0}}, /* OicUuid_t deviceID */
false, /* bool dpc */
{{0}}, /* OicUuid_t owner */
};
static void provisioningCB (void* UNUSED1, int UNUSED2, OCProvisionResult_t *UNUSED3, bool UNUSED4)
{
......@@ -38,6 +67,14 @@ static void provisioningCB (void* UNUSED1, int UNUSED2, OCProvisionResult_t *UNU
TEST(SRPProvisionACLTest, NullDeviceInfo)
{
pDev1.doxm = &defaultDoxm1;
uint8_t deviceId1[] = {0x64, 0x65, 0x76, 0x69, 0x63, 0x65, 0x49, 0x64};
memcpy(pDev1.doxm->deviceID.id, deviceId1, sizeof(deviceId1));
pDev2.doxm = &defaultDoxm2;
uint8_t deviceId2[] = {0x64, 0x65, 0x76, 0x69, 0x63, 0x65, 0x49, 0x63};
memcpy(pDev2.doxm->deviceID.id, deviceId2, sizeof(deviceId2));
EXPECT_EQ(OC_STACK_INVALID_PARAM, SRPProvisionACL(NULL, NULL, &acl, &provisioningCB));
}
......@@ -58,6 +95,13 @@ TEST(SRPProvisionCredentialsTest, NullDevice1)
&pDev2, &provisioningCB));
}
TEST(SRPProvisionCredentialsTest, SamelDeviceId)
{
EXPECT_EQ(OC_STACK_INVALID_PARAM, SRPProvisionCredentials(NULL, credType,
OWNER_PSK_LENGTH_128, &pDev1,
&pDev1, &provisioningCB));
}
TEST(SRPProvisionCredentialsTest, NullCallback)
{
EXPECT_EQ(OC_STACK_INVALID_CALLBACK, SRPProvisionCredentials(NULL, credType,
......@@ -74,21 +118,22 @@ TEST(SRPProvisionCredentialsTest, InvalidKeySize)
TEST(SRPUnlinkDevicesTest, NullDevice1)
{
OCProvisionDev_t dev2;
EXPECT_EQ(OC_STACK_INVALID_PARAM, SRPUnlinkDevices(NULL, NULL, &dev2, provisioningCB));
EXPECT_EQ(OC_STACK_INVALID_PARAM, SRPUnlinkDevices(NULL, NULL, &pDev2, provisioningCB));
}
TEST(SRPUnlinkDevicesTest, NullDevice2)
{
OCProvisionDev_t dev1;
EXPECT_EQ(OC_STACK_INVALID_PARAM, SRPUnlinkDevices(NULL, &dev1, NULL, provisioningCB));
EXPECT_EQ(OC_STACK_INVALID_PARAM, SRPUnlinkDevices(NULL, &pDev1, NULL, provisioningCB));
}
TEST(SRPUnlinkDevicesTest, SamelDeviceId)
{
EXPECT_EQ(OC_STACK_INVALID_PARAM, SRPUnlinkDevices(NULL, &pDev1, &pDev1, provisioningCB));
}
TEST(SRPUnlinkDevicesTest, NullCallback)
{
OCProvisionDev_t dev1;
OCProvisionDev_t dev2;
EXPECT_EQ(OC_STACK_INVALID_PARAM, SRPUnlinkDevices(NULL, &dev1, &dev2, NULL));
EXPECT_EQ(OC_STACK_INVALID_CALLBACK, SRPUnlinkDevices(NULL, &pDev1, &pDev2, NULL));
}
TEST(SRPRemoveDeviceTest, NullTargetDevice)
......@@ -101,7 +146,7 @@ TEST(SRPRemoveDeviceTest, NullResultCallback)
{
unsigned short waitTime = 10;
OCProvisionDev_t dev1;
EXPECT_EQ(OC_STACK_INVALID_PARAM, SRPRemoveDevice(NULL, waitTime, &dev1, NULL));
EXPECT_EQ(OC_STACK_INVALID_CALLBACK, SRPRemoveDevice(NULL, waitTime, &dev1, NULL));
}
TEST(SRPRemoveDeviceTest, ZeroWaitTime)
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment