From 581ea1542e7466149796b9e5f6573f6a311fa26e Mon Sep 17 00:00:00 2001 From: Kyungsun Cho Date: Tue, 5 Apr 2016 23:31:14 +0900 Subject: [PATCH] [IOT-1076] Added missed file/cbor-handling bug-fix(s) on |psinterface.c| this change mainly includes as the following: - persistent storage close after using svr-db - moved or removed overlapped variables into its scope - added missed error-handling codes and so on [patch #1] initial commit [patch #2] updated commit message [patch #3-4] aligned iotivity coding guide Jira: https://jira.iotivity.org/browse/IOT-1076 Change-Id: Ib39dddb3ca428fbf856c1abf3d45f6b855460d73 Signed-off-by: Kyungsun Cho Reviewed-on: https://gerrit.iotivity.org/gerrit/7617 Reviewed-by: Ashwini Kumar Reviewed-by: Chul Lee Reviewed-by: Yonggoo Kang Tested-by: jenkins-iotivity Reviewed-by: Jongsung Lee Reviewed-by: Randeep Singh --- resource/csdk/security/src/psinterface.c | 218 ++++++++++++++++--------------- 1 file changed, 115 insertions(+), 103 deletions(-) diff --git a/resource/csdk/security/src/psinterface.c b/resource/csdk/security/src/psinterface.c index a3ec1b9..c10b097 100644 --- a/resource/csdk/security/src/psinterface.c +++ b/resource/csdk/security/src/psinterface.c @@ -17,21 +17,23 @@ // limitations under the License. // //-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-= + #ifdef WITH_ARDUINO #define __STDC_LIMIT_MACROS #endif #include #include -#include "ocstack.h" + +#include "cainterface.h" #include "logger.h" -#include "oic_malloc.h" #include "ocpayload.h" #include "ocpayloadcbor.h" +#include "ocstack.h" +#include "oic_malloc.h" #include "payload_logging.h" -#include "cainterface.h" -#include "secureresourcemanager.h" #include "resourcemanager.h" +#include "secureresourcemanager.h" #include "srmresourcestrings.h" #include "srmutility.h" @@ -41,130 +43,143 @@ const size_t DB_FILE_SIZE_BLOCK = 1023; /** - * Gets the Secure Virtual Database size. + * Gets the Secure Virtual Database size * - * @param ps pointer of OCPersistentStorage for the SVR name ("acl", "cred", "pstat" etc). + * @param ps - pointer of OCPersistentStorage for the Secure Virtual Resource(s) * - * @return total size of the SVR database. + * @return size_t - total size of the SVR database */ -static size_t GetSVRDatabaseSize(const OCPersistentStorage* ps) +static size_t GetSVRDatabaseSize(const OCPersistentStorage *ps) { - size_t size = 0; if (!ps) { - return size; + return 0; } - size_t bytesRead = 0; - char buffer[DB_FILE_SIZE_BLOCK]; - FILE* fp = ps->open(SVR_DB_DAT_FILE_NAME, "r"); + size_t size = 0; + char buffer[DB_FILE_SIZE_BLOCK]; // can not initialize with declaration + // but maybe not needed to initialize + FILE *fp = ps->open(SVR_DB_DAT_FILE_NAME, "rb"); if (fp) { + size_t bytesRead = 0; do { bytesRead = ps->read(buffer, 1, DB_FILE_SIZE_BLOCK, fp); size += bytesRead; - } while (bytesRead > 0); + } while (bytesRead); ps->close(fp); } return size; } +/** + * Gets the Secure Virtual Database from the Persistent Storage + * + * @param rsrcName - pointer of character string for the SVR name (e.g. "acl") + * @param data - pointer of the returned Secure Virtual Resource(s) + * @param size - pointer of the returned size of Secure Virtual Resource(s) + * + * @return OCStackResult - result of getting Secure Virtual Resource(s) + */ OCStackResult GetSecureVirtualDatabaseFromPS(const char *rsrcName, uint8_t **data, size_t *size) { - if (!data || !size) + OIC_LOG(DEBUG, TAG, "GetSecureVirtualDatabaseFromPS IN"); + if (!data || *data || !size) { return OC_STACK_INVALID_PARAM; } - OCStackResult ret = OC_STACK_ERROR; - *data = NULL; FILE *fp = NULL; - size_t fileSize = 0; uint8_t *fsData = NULL; + size_t fileSize = 0; + OCStackResult ret = OC_STACK_ERROR; OCPersistentStorage *ps = SRMGetPersistentStorageHandler(); VERIFY_NON_NULL(TAG, ps, ERROR); fileSize = GetSVRDatabaseSize(ps); - if (fileSize != 0) + OIC_LOG_V(DEBUG, TAG, "File Read Size: %zu", fileSize); + if (fileSize) { - OIC_LOG_V(DEBUG, TAG, "File Read Size: %zu", fileSize); - fsData = (uint8_t *)OICCalloc(1, fileSize); + fsData = (uint8_t *) OICCalloc(1, fileSize); VERIFY_NON_NULL(TAG, fsData, ERROR); - FILE *fp = ps->open(SVR_DB_DAT_FILE_NAME, "rb"); + fp = ps->open(SVR_DB_DAT_FILE_NAME, "rb"); VERIFY_NON_NULL(TAG, fp, ERROR); - size_t itemsRead = ps->read(fsData, 1, fileSize, fp); - if (itemsRead == fileSize) + if (ps->read(fsData, 1, fileSize, fp) == fileSize) { - VERIFY_NON_NULL(TAG, fsData, ERROR); - if (rsrcName != NULL) + if (rsrcName) { - CborParser parser = { .end = NULL }; - CborValue cbor = { .parser = NULL }; + CborParser parser; // will be initialized in |cbor_parser_init| + CborValue cbor; // will be initialized in |cbor_parser_init| cbor_parser_init(fsData, fileSize, 0, &parser, &cbor); - CborValue cborValue = { .parser = NULL }; - // CborError cborFindResult = cbor_value_enter_container(&cbor, &cborValue); + CborValue cborValue = {0}; CborError cborFindResult = cbor_value_map_find_value(&cbor, rsrcName, &cborValue); if (CborNoError == cborFindResult && cbor_value_is_byte_string(&cborValue)) { cborFindResult = cbor_value_dup_byte_string(&cborValue, data, size, NULL); - VERIFY_SUCCESS(TAG, cborFindResult == CborNoError, ERROR); + VERIFY_SUCCESS(TAG, CborNoError==cborFindResult, ERROR); ret = OC_STACK_OK; - goto exit; } + // in case of |else (...)|, svr_data not found } // return everything in case rsrcName is NULL else { *size = fileSize; - *data = (uint8_t *)OICCalloc(1, fileSize); + *data = (uint8_t *) OICCalloc(1, fileSize); VERIFY_NON_NULL(TAG, *data, ERROR); memcpy(*data, fsData, fileSize); + ret = OC_STACK_OK; } } } - else - { - ret = OC_STACK_OK; - } + OIC_LOG(DEBUG, TAG, "GetSecureVirtualDatabaseFromPS OUT"); exit: + if (fp) + { + ps->close(fp); + } OICFree(fsData); return ret; } -OCStackResult UpdateSecureResourceInPS(const char* rsrcName, const uint8_t* psPayload, size_t psSize) +/** + * Updates the Secure Virtual Resource(s) into the Persistent Storage. + * This function stores cbor-payload of each resource by appending resource name, + * and empty payload implies deleting the value + * + * @param rsrcName - pointer of character string for the SVR name (e.g. "acl") + * @param psPayload - pointer of the updated Secure Virtual Resource(s) + * @param psSize - the updated size of Secure Virtual Resource(s) + * + * @return OCStackResult - result of updating Secure Virtual Resource(s) + */ +OCStackResult UpdateSecureResourceInPS(const char *rsrcName, const uint8_t *psPayload, size_t psSize) { - OIC_LOG(DEBUG, TAG, "Entering UpdateSecureResourceInPS IN"); - /* - * This function stores cbor payload of each resource by appending resource name. - */ - // Empty payload implies deleting the value + OIC_LOG(DEBUG, TAG, "UpdateSecureResourceInPS IN"); if (!rsrcName) { return OC_STACK_INVALID_PARAM; } - OCStackResult ret = OC_STACK_ERROR; - - uint8_t *dbData = NULL; size_t dbSize = 0; - size_t cborSize = 0; + size_t outSize = 0; + uint8_t *dbData = NULL; uint8_t *outPayload = NULL; + uint8_t *aclCbor = NULL; uint8_t *pstatCbor = NULL; - uint8_t *amaclCbor = NULL; uint8_t *doxmCbor = NULL; + uint8_t *amaclCbor = NULL; uint8_t *svcCbor = NULL; uint8_t *credCbor = NULL; uint8_t *pconfCbor = NULL; int64_t cborEncoderResult = CborNoError; - CborEncoder encoder; - - ret = GetSecureVirtualDatabaseFromPS(NULL, &dbData, &dbSize); - if (dbData && dbSize != 0) + OCStackResult ret = GetSecureVirtualDatabaseFromPS(NULL, &dbData, &dbSize); + if (dbData && dbSize) { size_t aclCborLen = 0; size_t pstatCborLen = 0; @@ -174,48 +189,45 @@ OCStackResult UpdateSecureResourceInPS(const char* rsrcName, const uint8_t* psPa size_t credCborLen = 0; size_t pconfCborLen = 0; + // Gets each secure virtual resource from persistent storage + // this local scoping intended, for destroying large cbor instances after use { - CborValue cbor; - CborParser parser; + CborParser parser; // will be initialized in |cbor_parser_init| + CborValue cbor; // will be initialized in |cbor_parser_init| cbor_parser_init(dbData, dbSize, 0, &parser, &cbor); + CborValue curVal = {0}; CborError cborFindResult = CborNoError; - CborValue curVal; cborFindResult = cbor_value_map_find_value(&cbor, OIC_JSON_ACL_NAME, &curVal); if (CborNoError == cborFindResult && cbor_value_is_byte_string(&curVal)) { cborFindResult = cbor_value_dup_byte_string(&curVal, &aclCbor, &aclCborLen, NULL); VERIFY_CBOR_SUCCESS(TAG, cborFindResult, "Failed Finding ACL Name Value."); } - - cborFindResult = cbor_value_map_find_value(&cbor, OIC_JSON_DOXM_NAME, &curVal); - if (CborNoError == cborFindResult && cbor_value_is_byte_string(&curVal)) - { - cborFindResult = cbor_value_dup_byte_string(&curVal, &doxmCbor, &doxmCborLen, NULL); - VERIFY_CBOR_SUCCESS(TAG, cborFindResult, "Failed Finding DOXM Name Value."); - } - cborFindResult = cbor_value_map_find_value(&cbor, OIC_JSON_PSTAT_NAME, &curVal); if (CborNoError == cborFindResult && cbor_value_is_byte_string(&curVal)) { cborFindResult = cbor_value_dup_byte_string(&curVal, &pstatCbor, &pstatCborLen, NULL); VERIFY_CBOR_SUCCESS(TAG, cborFindResult, "Failed Finding PSTAT Name Value."); } - + cborFindResult = cbor_value_map_find_value(&cbor, OIC_JSON_DOXM_NAME, &curVal); + if (CborNoError == cborFindResult && cbor_value_is_byte_string(&curVal)) + { + cborFindResult = cbor_value_dup_byte_string(&curVal, &doxmCbor, &doxmCborLen, NULL); + VERIFY_CBOR_SUCCESS(TAG, cborFindResult, "Failed Finding DOXM Name Value."); + } cborFindResult = cbor_value_map_find_value(&cbor, OIC_JSON_AMACL_NAME, &curVal); if (CborNoError == cborFindResult && cbor_value_is_byte_string(&curVal)) { cborFindResult = cbor_value_dup_byte_string(&curVal, &amaclCbor, &amaclCborLen, NULL); VERIFY_CBOR_SUCCESS(TAG, cborFindResult, "Failed Finding AMACL Name Value."); } - cborFindResult = cbor_value_map_find_value(&cbor, OIC_JSON_SVC_NAME, &curVal); if (CborNoError == cborFindResult && cbor_value_is_byte_string(&curVal)) { cborFindResult = cbor_value_dup_byte_string(&curVal, &svcCbor, &svcCborLen, NULL); VERIFY_CBOR_SUCCESS(TAG, cborFindResult, "Failed Finding SVC Name Value."); } - cborFindResult = cbor_value_map_find_value(&cbor, OIC_JSON_CRED_NAME, &curVal); if (CborNoError == cborFindResult && cbor_value_is_byte_string(&curVal)) { @@ -228,74 +240,73 @@ OCStackResult UpdateSecureResourceInPS(const char* rsrcName, const uint8_t* psPa cborFindResult = cbor_value_dup_byte_string(&curVal, &pconfCbor, &pconfCborLen, NULL); VERIFY_CBOR_SUCCESS(TAG, cborFindResult, "Failed Finding PCONF Name Value."); } - } + // Updates the added |psPayload| with the existing secure virtual resource(s) + // this local scoping intended, for destroying large cbor instances after use { - size_t size = aclCborLen + pstatCborLen + doxmCborLen + amaclCborLen + svcCborLen - + credCborLen + pconfCborLen +psSize; - // This is arbitrary value that is added to cover the name of the resource, map addition and ending. - size += 255; + size_t size = aclCborLen + pstatCborLen + doxmCborLen + amaclCborLen + + svcCborLen + credCborLen + pconfCborLen + psSize + 255; + // This added '255' is arbitrary value that is added to cover the name of the resource, map addition and ending - outPayload = (uint8_t *)OICCalloc(1, size); + outPayload = (uint8_t *) OICCalloc(1, size); VERIFY_NON_NULL(TAG, outPayload, ERROR); + CborEncoder encoder; // will be initialized in |cbor_parser_init| cbor_encoder_init(&encoder, outPayload, size, 0); - - CborEncoder secRsrc; + CborEncoder secRsrc; // will be initialized in |cbor_encoder_create_map| cborEncoderResult |= cbor_encoder_create_map(&encoder, &secRsrc, CborIndefiniteLength); VERIFY_CBOR_SUCCESS(TAG, cborEncoderResult, "Failed Adding PS Map."); - if (psPayload) + if (psPayload && psSize) { cborEncoderResult |= cbor_encode_text_string(&secRsrc, rsrcName, strlen(rsrcName)); VERIFY_CBOR_SUCCESS(TAG, cborEncoderResult, "Failed Adding Value Tag"); cborEncoderResult |= cbor_encode_byte_string(&secRsrc, psPayload, psSize); VERIFY_CBOR_SUCCESS(TAG, cborEncoderResult, "Failed Adding Value."); } - - if (0 != strcmp(OIC_JSON_ACL_NAME, rsrcName) && aclCborLen > 0) + if (strcmp(OIC_JSON_ACL_NAME, rsrcName) && aclCborLen) { cborEncoderResult |= cbor_encode_text_string(&secRsrc, OIC_JSON_ACL_NAME, strlen(OIC_JSON_ACL_NAME)); VERIFY_CBOR_SUCCESS(TAG, cborEncoderResult, "Failed Adding ACL Name."); cborEncoderResult |= cbor_encode_byte_string(&secRsrc, aclCbor, aclCborLen); VERIFY_CBOR_SUCCESS(TAG, cborEncoderResult, "Failed Adding ACL Value."); } - if (0 != strcmp(OIC_JSON_PSTAT_NAME, rsrcName) && pstatCborLen > 0) + if (strcmp(OIC_JSON_PSTAT_NAME, rsrcName) && pstatCborLen) { cborEncoderResult |= cbor_encode_text_string(&secRsrc, OIC_JSON_PSTAT_NAME, strlen(OIC_JSON_PSTAT_NAME)); VERIFY_CBOR_SUCCESS(TAG, cborEncoderResult, "Failed Adding PSTAT Name."); cborEncoderResult |= cbor_encode_byte_string(&secRsrc, pstatCbor, pstatCborLen); VERIFY_CBOR_SUCCESS(TAG, cborEncoderResult, "Failed Adding PSTAT Value."); } - if (0 != strcmp(OIC_JSON_DOXM_NAME, rsrcName) && doxmCborLen > 0) + if (strcmp(OIC_JSON_DOXM_NAME, rsrcName) && doxmCborLen) { cborEncoderResult |= cbor_encode_text_string(&secRsrc, OIC_JSON_DOXM_NAME, strlen(OIC_JSON_DOXM_NAME)); VERIFY_CBOR_SUCCESS(TAG, cborEncoderResult, "Failed Adding Doxm Name."); cborEncoderResult |= cbor_encode_byte_string(&secRsrc, doxmCbor, doxmCborLen); VERIFY_CBOR_SUCCESS(TAG, cborEncoderResult, "Failed Adding Doxm Value."); } - if (0 != strcmp(OIC_JSON_AMACL_NAME, rsrcName) && amaclCborLen > 0) + if (strcmp(OIC_JSON_AMACL_NAME, rsrcName) && amaclCborLen) { cborEncoderResult |= cbor_encode_text_string(&secRsrc, OIC_JSON_AMACL_NAME, strlen(OIC_JSON_AMACL_NAME)); VERIFY_CBOR_SUCCESS(TAG, cborEncoderResult, "Failed Adding Amacl Name."); cborEncoderResult |= cbor_encode_byte_string(&secRsrc, amaclCbor, amaclCborLen); VERIFY_CBOR_SUCCESS(TAG, cborEncoderResult, "Failed Adding Amacl Value."); } - if (0 != strcmp(OIC_JSON_SVC_NAME, rsrcName) && svcCborLen > 0) + if (strcmp(OIC_JSON_SVC_NAME, rsrcName) && svcCborLen) { cborEncoderResult |= cbor_encode_text_string(&secRsrc, OIC_JSON_SVC_NAME, strlen(OIC_JSON_SVC_NAME)); VERIFY_CBOR_SUCCESS(TAG, cborEncoderResult, "Failed Adding SVC Name."); cborEncoderResult |= cbor_encode_byte_string(&secRsrc, svcCbor, svcCborLen); VERIFY_CBOR_SUCCESS(TAG, cborEncoderResult, "Failed Adding SVC Value."); } - if (0 != strcmp(OIC_JSON_CRED_NAME, rsrcName) && credCborLen > 0) + if (strcmp(OIC_JSON_CRED_NAME, rsrcName) && credCborLen) { cborEncoderResult |= cbor_encode_text_string(&secRsrc, OIC_JSON_CRED_NAME, strlen(OIC_JSON_CRED_NAME)); VERIFY_CBOR_SUCCESS(TAG, cborEncoderResult, "Failed Adding Cred Name."); cborEncoderResult |= cbor_encode_byte_string(&secRsrc, credCbor, credCborLen); VERIFY_CBOR_SUCCESS(TAG, cborEncoderResult, "Failed Adding Cred Value."); } - if (0 != strcmp(OIC_JSON_PCONF_NAME, rsrcName) && pconfCborLen > 0) + if (strcmp(OIC_JSON_PCONF_NAME, rsrcName) && pconfCborLen) { cborEncoderResult |= cbor_encode_text_string(&secRsrc, OIC_JSON_PCONF_NAME, strlen(OIC_JSON_PCONF_NAME)); VERIFY_CBOR_SUCCESS(TAG, cborEncoderResult, "Failed Adding Pconf Name."); @@ -305,41 +316,45 @@ OCStackResult UpdateSecureResourceInPS(const char* rsrcName, const uint8_t* psPa cborEncoderResult |= cbor_encoder_close_container(&encoder, &secRsrc); VERIFY_CBOR_SUCCESS(TAG, cborEncoderResult, "Failed Closing Array."); - - cborSize = encoder.ptr - outPayload; + outSize = encoder.ptr - outPayload; } } - else if (psPayload) + else if (psPayload && psSize) { size_t size = psSize + 255; - outPayload = (uint8_t *)OICCalloc(1, size); + // This added '255' is arbitrary value that is added to cover the name of the resource, map addition and ending + + outPayload = (uint8_t *) OICCalloc(1, size); VERIFY_NON_NULL(TAG, outPayload, ERROR); - CborEncoder encoder; + CborEncoder encoder; // will be initialized in |cbor_parser_init| cbor_encoder_init(&encoder, outPayload, size, 0); - - CborEncoder secRsrc ; + CborEncoder secRsrc; // will be initialized in |cbor_encoder_create_map| cborEncoderResult |= cbor_encoder_create_map(&encoder, &secRsrc, CborIndefiniteLength); + VERIFY_CBOR_SUCCESS(TAG, cborEncoderResult, "Failed Adding PS Map."); + cborEncoderResult |= cbor_encode_text_string(&secRsrc, rsrcName, strlen(rsrcName)); VERIFY_CBOR_SUCCESS(TAG, cborEncoderResult, "Failed Adding Value Tag"); cborEncoderResult |= cbor_encode_byte_string(&secRsrc, psPayload, psSize); VERIFY_CBOR_SUCCESS(TAG, cborEncoderResult, "Failed Adding Value."); + cborEncoderResult |= cbor_encoder_close_container(&encoder, &secRsrc); - cborSize = encoder.ptr - outPayload; + VERIFY_CBOR_SUCCESS(TAG, cborEncoderResult, "Failed Closing Array."); + outSize = encoder.ptr - outPayload; } - if (outPayload && cborSize > 0) + if (outPayload && outSize) { - OIC_LOG_V(DEBUG, TAG, "Writting in the file. %zd", cborSize); + OIC_LOG_V(DEBUG, TAG, "Writting in the file: %zu", outSize); OCPersistentStorage* ps = SRMGetPersistentStorageHandler(); if (ps) { - FILE *fp = ps->open(SVR_DB_DAT_FILE_NAME, "w+"); + FILE *fp = ps->open(SVR_DB_DAT_FILE_NAME, "wb"); if (fp) { - size_t numberItems = ps->write(outPayload, 1, cborSize, fp); - if (cborSize == numberItems) + size_t numberItems = ps->write(outPayload, 1, outSize, fp); + if (outSize == numberItems) { - OIC_LOG_V(DEBUG, TAG, "Written %zu bytes into SVR database file", cborSize); + OIC_LOG_V(DEBUG, TAG, "Written %zu bytes into SVR database file", outSize); ret = OC_STACK_OK; } else @@ -352,23 +367,20 @@ OCStackResult UpdateSecureResourceInPS(const char* rsrcName, const uint8_t* psPa { OIC_LOG(ERROR, TAG, "File open failed."); } - } - OIC_LOG_V(DEBUG, TAG, "Writing in the file . %zd", cborSize); } - OIC_LOG(DEBUG, TAG, "Exiting UpdateSecureResourceInPS OUT"); + OIC_LOG(DEBUG, TAG, "UpdateSecureResourceInPS OUT"); exit: OICFree(dbData); OICFree(outPayload); - OICFree(aclCbor); OICFree(pstatCbor); - OICFree(amaclCbor); OICFree(doxmCbor); + OICFree(amaclCbor); OICFree(svcCbor); OICFree(credCbor); - + OICFree(pconfCbor); return ret; } -- 2.7.4