diff options
author | Nathan Heldt-Sheller <nathan.heldt-sheller@intel.com> | 2017-05-20 13:42:52 -0700 |
---|---|---|
committer | Nathan Heldt-Sheller <nathan.heldt-sheller@intel.com> | 2017-05-21 18:18:17 +0000 |
commit | 048711628b037aec7cefa927bf4961e270bf524e (patch) | |
tree | ee68a190243fea3ec4e5627f85b3767bbaea5ab0 /resource/csdk | |
parent | 17430f70fe1b6e92943ddabdf094fa6bfc29b025 (diff) | |
download | iotivity-048711628b037aec7cefa927bf4961e270bf524e.tar.gz iotivity-048711628b037aec7cefa927bf4961e270bf524e.tar.bz2 iotivity-048711628b037aec7cefa927bf4961e270bf524e.zip |
[IOT-2293] [IOT-2192] reject POST to /acl2 Resource with ACL1 payload
/acl2 POST handler now calls a version-check-only optional CBORPayloadToAcl()
function and, if an acl1 payload is found, denies the POST request.
This update caught some unit test issues that were also corrected with
this patch.
Change-Id: I20d148ef037c82f5862fd9fec156bbb399ab7417
Signed-off-by: Nathan Heldt-Sheller <nathan.heldt-sheller@intel.com>
Reviewed-on: https://gerrit.iotivity.org/gerrit/20189
Tested-by: jenkins-iotivity <jenkins@iotivity.org>
Reviewed-by: Randeep Singh <randeep.s@samsung.com>
Reviewed-by: Dmitriy Zhuravlev <d.zhuravlev@samsung.com>
Diffstat (limited to 'resource/csdk')
-rw-r--r-- | resource/csdk/security/src/aclresource.c | 44 | ||||
-rw-r--r-- | resource/csdk/security/unittest/aclresourcetest.cpp | 11 |
2 files changed, 48 insertions, 7 deletions
diff --git a/resource/csdk/security/src/aclresource.c b/resource/csdk/security/src/aclresource.c index e5783b5e8..e8f5379bc 100644 --- a/resource/csdk/security/src/aclresource.c +++ b/resource/csdk/security/src/aclresource.c @@ -1335,11 +1335,19 @@ exit: } // This function converts CBOR format to ACL data. +// Callers should normally invoke "CBORPayloadToAcl()" unless wishing to check +// version of payload only. // Caller needs to invoke 'OICFree' on returned value when done using // TODO IOT-2220 this function is a prime example of why the SVR CBOR functions need // to be re-factored throughout. It's even worse with the addition of /acl2. -// note: This function is used in unit test hence not declared static, -OicSecAcl_t* CBORPayloadToAcl(const uint8_t *cborPayload, const size_t size) +// @param[in] cborPayload The CBOR data to be decoded into OicSecAcl_t +// @param[in] size The size of the CBOR data +// @param[out] versionCheck If included, this function will determine the version of +// ACL in the payload, assign to 'versionCheck', and return NULL +// without decoding the rest of the payload. If NULL, this function will complete +// decoding as normal, and will not assign a value to 'versionCheck'. +static OicSecAcl_t* CBORPayloadToAclVersionOpt(const uint8_t *cborPayload, const size_t size, + OicSecAclVersion_t *versionCheck) { if (NULL == cborPayload || 0 == size) { @@ -1383,12 +1391,27 @@ OicSecAcl_t* CBORPayloadToAcl(const uint8_t *cborPayload, const size_t size) OIC_LOG_V(DEBUG, TAG, "%s found %s tag.", __func__, tagName); if (0 == strcmp(tagName, OIC_JSON_ACLIST_NAME)) { + if (NULL != versionCheck) + { + OIC_LOG_V(DEBUG, TAG, "%s Found v1 ACL; assigning 'versionCheck' and returning NULL.", __func__); + *versionCheck = OIC_SEC_ACL_V1; + OICFree(acl); + return NULL; + } OIC_LOG_V(DEBUG, TAG, "%s decoding v1 ACL.", __func__); aclistVersion = OIC_SEC_ACL_V1; aclistTagJustFound = true; + } else if (0 == strcmp(tagName, OIC_JSON_ACLIST2_NAME)) { + if (NULL != versionCheck) + { + OIC_LOG_V(DEBUG, TAG, "%s Found v2 ACL; assigning 'versionCheck' and returning NULL.", __func__); + *versionCheck = OIC_SEC_ACL_V2; + OICFree(acl); + return NULL; + } OIC_LOG_V(DEBUG, TAG, "%s decoding v2 ACL.", __func__); aclistVersion = OIC_SEC_ACL_V2; aclistTagJustFound = true; @@ -1992,6 +2015,14 @@ exit: return acl; } +// This function converts CBOR format to ACL data. +// Caller needs to invoke 'OICFree' on returned value when done using +// note: This function is used in unit test hence not declared static. +OicSecAcl_t* CBORPayloadToAcl(const uint8_t *cborPayload, const size_t size) +{ + return CBORPayloadToAclVersionOpt(cborPayload, size, NULL); +} + #ifdef MULTIPLE_OWNER bool IsValidAclAccessForSubOwner(const OicUuid_t* uuid, const uint8_t *cborPayload, const size_t size) { @@ -2650,6 +2681,15 @@ static OCEntityHandlerResult HandleACLPostRequest(const OCEntityHandlerRequest * if (payload) { + // Clients should not POST v1 ACL to OCF 1.0 Server + OicSecAclVersion_t payloadVersionReceived = OIC_SEC_ACL_V1; + CBORPayloadToAclVersionOpt(payload, size, &payloadVersionReceived); + if (OIC_SEC_ACL_V2 != payloadVersionReceived) + { + OIC_LOG_V(WARNING, TAG, "%s /acl Resource is v2; POST of v1 ACL not acceptable.", __func__); + ehRet = OC_EH_NOT_ACCEPTABLE; + goto exit; + } OicSecAcl_t *newAcl = NULL; OIC_LOG(DEBUG, TAG, "ACL payload from POST request << "); OIC_LOG_BUFFER(DEBUG, TAG, payload, size); diff --git a/resource/csdk/security/unittest/aclresourcetest.cpp b/resource/csdk/security/unittest/aclresourcetest.cpp index fa946595e..b37a64202 100644 --- a/resource/csdk/security/unittest/aclresourcetest.cpp +++ b/resource/csdk/security/unittest/aclresourcetest.cpp @@ -111,7 +111,8 @@ static int GetNumberOfResource(const OicSecAce_t* ace) TEST(ACLResourceTest, CBORDefaultACLConversion) { - uint8_t defaultAclSub[] = { 0x2a }; + uint8_t defaultAclSub[] = {0x31, 0x31, 0x31, 0x31, 0x31, 0x31, 0x31, 0x31, + 0x31, 0x31, 0x31, 0x31, 0x31, 0x31, 0x31, 0x31}; uint8_t defaultAclOwnrs[] = {0x32, 0x32, 0x32, 0x32, 0x32, 0x32, 0x32, 0x32, 0x32, 0x32, 0x32, 0x32, 0x32, 0x32, 0x32, 0x32}; @@ -136,7 +137,7 @@ TEST(ACLResourceTest, CBORDefaultACLConversion) size_t defaultAclSize = 0; uint8_t *defaultPsStorage = NULL; - OCStackResult convRet = AclToCBORPayload(defaultAcl, OIC_SEC_ACL_V1, &defaultPsStorage, &defaultAclSize); + OCStackResult convRet = AclToCBORPayload(defaultAcl, OIC_SEC_ACL_V2, &defaultPsStorage, &defaultAclSize); EXPECT_EQ(OC_STACK_OK, convRet); ASSERT_TRUE(NULL != defaultPsStorage); EXPECT_NE(static_cast<size_t>(0), defaultAclSize); @@ -451,7 +452,7 @@ TEST(ACLResourceTest, ACLDeleteWithSingleResourceTest) //GET CBOR POST payload size_t size = 0; uint8_t *payload = NULL; - EXPECT_EQ(OC_STACK_OK, AclToCBORPayload(&acl, OIC_SEC_ACL_V1, &payload, &size)); + EXPECT_EQ(OC_STACK_OK, AclToCBORPayload(&acl, OIC_SEC_ACL_V2, &payload, &size)); ASSERT_TRUE(NULL != payload); // Security Payload @@ -511,7 +512,7 @@ TEST(ACLResourceTest, ACLDeleteWithMultiResourceTest) //GET CBOR POST payload size_t size = 0; uint8_t *payload = NULL; - EXPECT_EQ(OC_STACK_OK, AclToCBORPayload(&acl, OIC_SEC_ACL_V1, &payload, &size)); + EXPECT_EQ(OC_STACK_OK, AclToCBORPayload(&acl, OIC_SEC_ACL_V2, &payload, &size)); ASSERT_TRUE(NULL != payload); // Security Payload @@ -578,7 +579,7 @@ TEST(ACLResourceTest, ACLGetWithQueryTest) //GET CBOR POST payload size_t size = 0; uint8_t *payload = NULL; - EXPECT_EQ(OC_STACK_OK, AclToCBORPayload(&acl, OIC_SEC_ACL_V1, &payload, &size)); + EXPECT_EQ(OC_STACK_OK, AclToCBORPayload(&acl, OIC_SEC_ACL_V2, &payload, &size)); ASSERT_TRUE(NULL != payload); // Security Payload |