[IOT-2843] remove SVR restore behavior 03/23003/3
authorNathan Heldt-Sheller <nathan.heldt-sheller@intel.com>
Thu, 26 Oct 2017 03:06:56 +0000 (20:06 -0700)
committerNathan Heldt-Sheller <nathan.heldt-sheller@intel.com>
Thu, 26 Oct 2017 05:11:45 +0000 (22:11 -0700)
These functions were applied at incorrect times (e.g. if a normal
Update was rejected due to read-only properties during OTM, the entire
system would "restore" to a wrong state).  They were also wrong,
in that they restored some values, left others unchanged, and set
others to wrong values (e.g. presumed JustWorks OTM).

Also, the duplicate message logic was not being used to any consistent
effect and causing warnings.  It's also completely optional and so was
removed.

Change-Id: I23d23f946fbafe02cdc2d2ac6ac46abcedd1f149
Signed-off-by: Nathan Heldt-Sheller <nathan.heldt-sheller@intel.com>
resource/csdk/security/include/internal/doxmresource.h
resource/csdk/security/include/internal/pstatresource.h
resource/csdk/security/src/credresource.c
resource/csdk/security/src/doxmresource.c
resource/csdk/security/src/pstatresource.c

index cf96385..504e194 100644 (file)
@@ -238,12 +238,6 @@ void DeleteDoxmBinData(OicSecDoxm_t* doxm);
  */
 bool AreDoxmBinPropertyValuesEqual(OicSecDoxm_t* doxm1, OicSecDoxm_t* doxm2);
 
-/**
- * Function to restore doxm resurce to initial status.
- * This function will use in case of error while ownership transfer
- */
-void RestoreDoxmToInitState();
-
 #if defined(__WITH_DTLS__) && defined(MULTIPLE_OWNER)
 /**
  * Callback function to handle MOT DTLS handshake result.
index 5d54032..ed78e91 100644 (file)
@@ -113,12 +113,6 @@ OCStackResult CBORPayloadToPstat(const uint8_t *cborPayload, const size_t cborSi
  */
 void DeletePstatBinData(OicSecPstat_t* pstat);
 
-/**
- * Function to restore pstat resurce to initial status.
- * This function will use in case of error while ownership transfer
- */
-void RestorePstatToInitState();
-
 /**
  * Get the pstat.rowneruuid value for this device.
  *
index b8b08b5..0090ad7 100644 (file)
@@ -2121,7 +2121,7 @@ exit:
 #endif // __WITH_DTLS__ or __WITH_TLS__
 
 
-static OCEntityHandlerResult HandleNewCredential(OCEntityHandlerRequest *ehRequest, OicSecCred_t *cred, uint16_t previousMsgId)
+static OCEntityHandlerResult HandleNewCredential(OCEntityHandlerRequest *ehRequest, OicSecCred_t *cred)
 {
     OCEntityHandlerResult ret = OC_EH_INTERNAL_SERVER_ERROR;
 
@@ -2228,34 +2228,6 @@ static OCEntityHandlerResult HandleNewCredential(OCEntityHandlerRequest *ehReque
                 break;
             }
         }
-
-        if(OC_EH_CHANGED != ret)
-        {
-            /*
-                * If some error is occured while ownership transfer,
-                * ownership transfer related resource should be revert back to initial status.
-                */
-            const OicSecDoxm_t *ownershipDoxm =  GetDoxmResourceData();
-            if(ownershipDoxm)
-            {
-                if(!ownershipDoxm->owned)
-                {
-                    OIC_LOG(WARNING, TAG, "The operation failed during handle DOXM request");
-
-                    if((OC_ADAPTER_IP == ehRequest->devAddr.adapter && previousMsgId != ehRequest->messageID)
-                        || OC_ADAPTER_TCP == ehRequest->devAddr.adapter)
-                    {
-                        RestoreDoxmToInitState();
-                        RestorePstatToInitState();
-                        OIC_LOG(WARNING, TAG, "DOXM will be reverted.");
-                    }
-                }
-            }
-            else
-            {
-                OIC_LOG(ERROR, TAG, "Invalid DOXM resource");
-            }
-        }
     }
 #ifdef MULTIPLE_OWNER
     // In case SubOwner Credential
@@ -2347,7 +2319,6 @@ static OCEntityHandlerResult HandleNewCredential(OCEntityHandlerRequest *ehReque
         * list and updating svr database.
         */
     ret = (OC_STACK_OK == AddCredential(cred))? OC_EH_CHANGED : OC_EH_ERROR;
-    OC_UNUSED(previousMsgId);
     OC_UNUSED(ehRequest);
 #endif//__WITH_DTLS__ or __WITH_TLS__
 
@@ -2360,7 +2331,6 @@ static OCEntityHandlerResult HandlePostRequest(OCEntityHandlerRequest* ehRequest
     OIC_LOG(DEBUG, TAG, "HandleCREDPostRequest IN");
 
     OicSecDostype_t dos;
-    static uint16_t previousMsgId = 0;
     // Get binary representation of cbor
     OicSecCred_t *cred = NULL;
     OicUuid_t     *rownerId = NULL;
@@ -2387,7 +2357,7 @@ static OCEntityHandlerResult HandlePostRequest(OCEntityHandlerRequest* ehRequest
 
         LL_FOREACH_SAFE(cred, newCred, newCredTemp)
         {
-            ret = HandleNewCredential(ehRequest, newCred, previousMsgId);
+            ret = HandleNewCredential(ehRequest, newCred);
 
             if (OC_EH_CHANGED != ret)
             {
@@ -2425,13 +2395,6 @@ exit:
             DeleteCredList(cred);
         }
     }
-    else
-    {
-        if (OC_ADAPTER_IP == ehRequest->devAddr.adapter)
-        {
-            previousMsgId = ehRequest->messageID++;
-        }
-    }
 
     // Send response to request originator
     ret = ((SendSRMResponse(ehRequest, ret, NULL, 0)) == OC_STACK_OK) ?
index 159e49c..9779bf2 100644 (file)
@@ -1333,14 +1333,12 @@ OCEntityHandlerResult HandleDoxmPostRequestUpdatePS(bool fACE)
     }
 }
 
-OCEntityHandlerResult StartOTMJustWorks(OCEntityHandlerRequest *ehRequest,
-        bool isDuplicatedMsg)
+OCEntityHandlerResult StartOTMJustWorks(OCEntityHandlerRequest *ehRequest)
 {
     OIC_LOG_V(DEBUG, TAG, "%s: IN", __func__);
     OCEntityHandlerResult ehRet = OC_EH_OK;
 
 #if !(defined(__WITH_DTLS__) || defined(__WITH_TLS__))
-    OC_UNUSED(isDuplicatedMsg);
     OC_UNUSED(ehRequest);
 #endif // not __WITH_DTLS__ and not __WITH_TLS__
 
@@ -1368,8 +1366,7 @@ OCEntityHandlerResult StartOTMJustWorks(OCEntityHandlerRequest *ehRequest,
         OIC_LOG_V(INFO, TAG, "%s: ECDH_ANON CipherSuite is DISABLED", __func__);
 
         //In case of Mutual Verified Just-Works, verify mutualVerifNum
-        if (OIC_MV_JUST_WORKS == gDoxm->oxmSel && false == gDoxm->owned &&
-            false == isDuplicatedMsg)
+        if (OIC_MV_JUST_WORKS == gDoxm->oxmSel && false == gDoxm->owned)
         {
             uint8_t preMutualVerifNum[OWNER_PSK_LENGTH_128] = {0};
             uint8_t mutualVerifNum[MUTUAL_VERIF_NUM_LEN] = {0};
@@ -1423,14 +1420,12 @@ exit:
 }
 
 OCEntityHandlerResult HandleDoxmPostRequestRandomPin(OicSecDoxm_t *newDoxm,
-        OCEntityHandlerRequest *ehRequest,
-        bool isDuplicatedMsg)
+        OCEntityHandlerRequest *ehRequest)
 {
     OIC_LOG_V(DEBUG, TAG, "%s: IN", __func__);
     OCEntityHandlerResult ehRet = OC_EH_OK;
 
 #if !(defined(__WITH_DTLS__) || defined(__WITH_TLS__))
-    OC_UNUSED(isDuplicatedMsg);
     OC_UNUSED(ehRequest);
 #endif // not __WITH_DTLS__ and not __WITH_TLS__
 
@@ -1452,28 +1447,25 @@ OCEntityHandlerResult HandleDoxmPostRequestRandomPin(OicSecDoxm_t *newDoxm,
                                     (CATransportAdapter_t)ehRequest->devAddr.adapter);
         VERIFY_SUCCESS(TAG, caRes == CA_STATUS_OK, ERROR);
 
-        if (!isDuplicatedMsg)
+        char ranPin[OXM_RANDOM_PIN_MAX_SIZE + 1] = {0};
+        if (OC_STACK_OK == GeneratePin(ranPin, sizeof(ranPin)))
         {
-            char ranPin[OXM_RANDOM_PIN_MAX_SIZE + 1] = {0};
-            if (OC_STACK_OK == GeneratePin(ranPin, sizeof(ranPin)))
-            {
-                //Set the device id to derive temporal PSK
-                SetUuidForPinBasedOxm(&gDoxm->deviceID);
-
-                /**
-                 * Since PSK will be used directly by DTLS layer while PIN based ownership transfer,
-                 * Credential should not be saved into SVR.
-                 * For this reason, use a temporary get_psk_info callback to random PIN OxM.
-                 */
-                caRes = CAregisterPskCredentialsHandler(GetDtlsPskForRandomPinOxm);
-                VERIFY_SUCCESS(TAG, caRes == CA_STATUS_OK, ERROR);
-                ehRet = OC_EH_OK;
-            }
-            else
-            {
-                OIC_LOG(ERROR, TAG, "Failed to generate random PIN");
-                ehRet = OC_EH_ERROR;
-            }
+            //Set the device id to derive temporal PSK
+            SetUuidForPinBasedOxm(&gDoxm->deviceID);
+
+            /**
+             * Since PSK will be used directly by DTLS layer while PIN based ownership transfer,
+             * Credential should not be saved into SVR.
+             * For this reason, use a temporary get_psk_info callback to random PIN OxM.
+             */
+            caRes = CAregisterPskCredentialsHandler(GetDtlsPskForRandomPinOxm);
+            VERIFY_SUCCESS(TAG, caRes == CA_STATUS_OK, ERROR);
+            ehRet = OC_EH_OK;
+        }
+        else
+        {
+            OIC_LOG(ERROR, TAG, "Failed to generate random PIN");
+            ehRet = OC_EH_ERROR;
         }
 #endif // __WITH_DTLS__ or __WITH_TLS__
     }
@@ -1484,10 +1476,8 @@ OCEntityHandlerResult HandleDoxmPostRequestRandomPin(OicSecDoxm_t *newDoxm,
         memcpy(&(gDoxm->owner), &(newDoxm->owner), sizeof(OicUuid_t));
 
         // In case of random-pin based OTM, close the PIN display if callback is registered.
-        if (!isDuplicatedMsg)
-        {
-            ClosePinDisplay();
-        }
+        ClosePinDisplay();
+
     }
 #endif // __WITH_DTLS__ or __WITH_TLS__
     goto exit;
@@ -1498,16 +1488,14 @@ exit:
 
 #if defined(__WITH_DTLS__) || defined (__WITH_TLS__)
 OCEntityHandlerResult HandleDoxmPostRequestMfg(OicSecDoxm_t *newDoxm,
-        OCEntityHandlerRequest *ehRequest,
-        bool isDuplicatedMsg)
+        OCEntityHandlerRequest *ehRequest)
 {
     OIC_LOG_V(DEBUG, TAG, "%s: IN", __func__);
     OCEntityHandlerResult ehRet = OC_EH_OK;
 
-
         //In case of Confirm Manufacturer Cert, get user confirmation
         if (OIC_CON_MFG_CERT == newDoxm->oxmSel && false == newDoxm->owned &&
-            false == isDuplicatedMsg && !IsNilUuid(&newDoxm->owner))
+            !IsNilUuid(&newDoxm->owner))
         {
             if (OC_STACK_OK != VerifyOwnershipTransfer(NULL, USER_CONFIRM))
             {
@@ -1545,8 +1533,7 @@ exit:
 
 // Do OTM specific initiation steps
 OCEntityHandlerResult StartOwnershipTransfer(OicSecDoxm_t *newDoxm,
-        OCEntityHandlerRequest *ehRequest,
-        bool isDuplicatedMsg)
+        OCEntityHandlerRequest *ehRequest)
 {
     OIC_LOG_V(DEBUG, TAG, "%s: IN", __func__);
     OCEntityHandlerResult ehRet = OC_EH_OK;
@@ -1555,15 +1542,15 @@ OCEntityHandlerResult StartOwnershipTransfer(OicSecDoxm_t *newDoxm,
     {
         case OIC_JUST_WORKS:
         case OIC_MV_JUST_WORKS:
-            ehRet = StartOTMJustWorks(ehRequest, isDuplicatedMsg);
+            ehRet = StartOTMJustWorks(ehRequest);
             break;
         case OIC_RANDOM_DEVICE_PIN:
-            ehRet = HandleDoxmPostRequestRandomPin(newDoxm, ehRequest, isDuplicatedMsg);
+            ehRet = HandleDoxmPostRequestRandomPin(newDoxm, ehRequest);
             break;
 #if defined(__WITH_DTLS__) || defined (__WITH_TLS__)
         case OIC_MANUFACTURER_CERTIFICATE:
         case OIC_CON_MFG_CERT:
-            ehRet = HandleDoxmPostRequestMfg(newDoxm, ehRequest, isDuplicatedMsg);
+            ehRet = HandleDoxmPostRequestMfg(newDoxm, ehRequest);
             break;
 #endif // __WITH_DTLS__ or __WITH_TLS__
         default:
@@ -1579,8 +1566,6 @@ static OCEntityHandlerResult HandleDoxmPostRequest(OCEntityHandlerRequest *ehReq
 {
     OIC_LOG_V(DEBUG, TAG, "%s: IN", __func__);
     OCEntityHandlerResult ehRet = OC_EH_INTERNAL_SERVER_ERROR;
-    static uint16_t previousMsgId = 0;
-    bool isDuplicatedMsg = false;
     bool fACE = false;
     OicSecDoxm_t *newDoxm = NULL;
     bool roParsed = false;
@@ -1618,17 +1603,6 @@ static OCEntityHandlerResult HandleDoxmPostRequest(OCEntityHandlerRequest *ehReq
         goto exit;
     }
 
-    /*
-     * message ID is supported for CoAP over UDP only according to RFC 7252
-     * So we should check message ID to prevent duplicate request handling in case of OC_ADAPTER_IP.
-     * In case of other transport adapter, duplicate message check is not required.
-     */
-    if (OC_ADAPTER_IP == ehRequest->devAddr.adapter &&
-        previousMsgId == ehRequest->messageID)
-    {
-        isDuplicatedMsg = true;
-    }
-
     // Validate newDoxm->oxmsel first
     if (false == ValidateOxmsel(gDoxm->oxm, gDoxm->oxmLen, &newDoxm->oxmSel))
     {
@@ -1652,7 +1626,7 @@ static OCEntityHandlerResult HandleDoxmPostRequest(OCEntityHandlerRequest *ehReq
     if (oxmselParsed && (false == gDoxm->owned))
     {
         OIC_LOG_V(INFO, TAG, "%s: Device not owned and oxmsel Updated... starting OTM!", __func__);
-        ehRet = StartOwnershipTransfer(newDoxm, ehRequest, isDuplicatedMsg);
+        ehRet = StartOwnershipTransfer(newDoxm, ehRequest);
         VERIFY_SUCCESS(TAG, OC_EH_OK == ehRet, ERROR);
     }
 
@@ -1675,29 +1649,6 @@ static OCEntityHandlerResult HandleDoxmPostRequest(OCEntityHandlerRequest *ehReq
     ehRet = HandleDoxmPostRequestUpdatePS(fACE);
 
 exit:
-    if (OC_EH_OK != ehRet)
-    {
-        // If some error is occured during Update handler,
-        // revert /doxm and /pstat back to initial status.
-
-        if (gDoxm)
-        {
-            if (!isDuplicatedMsg)
-            {
-                RestoreDoxmToInitState();
-                RestorePstatToInitState();
-                OIC_LOG(WARNING, TAG, "DOXM will be reverted.");
-            }
-        }
-        else
-        {
-            OIC_LOG(ERROR, TAG, "Invalid DOXM resource.");
-        }
-    }
-    else
-    {
-        previousMsgId = ehRequest->messageID;
-    }
 
     //Send payload to request originator
     ehRet = ((SendSRMResponse(ehRequest, ehRet, NULL, 0)) == OC_STACK_OK) ?
@@ -2269,28 +2220,6 @@ exit:
 
 #endif //MULTIPLE_OWNER
 
-/**
- * Function to restore doxm resurce to initial status.
- * This function will use in case of error while ownership transfer
- */
-void RestoreDoxmToInitState()
-{
-    if(gDoxm)
-    {
-        OIC_LOG(INFO, TAG, "DOXM resource will revert back to initial status.");
-
-        OicUuid_t emptyUuid = {.id={0}};
-        memcpy(&(gDoxm->owner), &emptyUuid, sizeof(OicUuid_t));
-        gDoxm->owned = false;
-        gDoxm->oxmSel = OIC_JUST_WORKS;
-
-        if(!UpdatePersistentStorage(gDoxm))
-        {
-            OIC_LOG(ERROR, TAG, "Failed to revert DOXM in persistent storage");
-        }
-    }
-}
-
 OCStackResult SetDoxmSelfOwnership(const OicUuid_t* newROwner)
 {
     OCStackResult ret = OC_STACK_ERROR;
index 89f7c91..89804ff 100644 (file)
@@ -788,8 +788,6 @@ static OCEntityHandlerResult HandlePstatPostRequest(OCEntityHandlerRequest *ehRe
     OCEntityHandlerResult ehRet = OC_EH_ERROR;
     OIC_LOG(INFO, TAG, "HandlePstatPostRequest  processing POST request");
     OicSecPstat_t *pstat = NULL;
-    static uint16_t previousMsgId = 0;
-    bool isDuplicatedMsg = false;
 
     if (ehRequest->payload && NULL != gPstat)
     {
@@ -805,17 +803,6 @@ static OCEntityHandlerResult HandlePstatPostRequest(OCEntityHandlerRequest *ehRe
         // if CBOR parsing OK
         if (OC_STACK_OK == ret)
         {
-            /*
-             * message ID is supported for CoAP over UDP only according to RFC 7252
-             * So we should check message ID to prevent duplicate request handling in case of OC_ADAPTER_IP.
-             * In case of other transport adapter, duplicate message check is not required.
-             */
-            if (OC_ADAPTER_IP == ehRequest->devAddr.adapter &&
-                 previousMsgId == ehRequest->messageID)
-            {
-                isDuplicatedMsg = true;
-            }
-
             if (true == roParsed)
             {
                     OIC_LOG(ERROR, TAG, "Not acceptable request because of read-only properties");
@@ -905,42 +892,7 @@ static OCEntityHandlerResult HandlePstatPostRequest(OCEntityHandlerRequest *ehRe
         }
     }
 
-    exit:
-
-    // TODO [IOT-1796] This is another place error code returns need to be
-    // cleaned up.
-    if (OC_EH_OK != ehRet)
-    {
-        /*
-         * If some error is occured while ownership transfer,
-         * ownership transfer related resource should be revert back to initial status.
-         */
-        OIC_LOG(WARNING, TAG, "The operation failed during handle pstat POST request");
-        const OicSecDoxm_t* doxm = GetDoxmResourceData();
-        if (doxm)
-        {
-            if (!doxm->owned)
-            {
-                if (!isDuplicatedMsg)
-                {
-                    OIC_LOG(WARNING, TAG, "DOXM and PSTAT will be reverted.");
-                    RestoreDoxmToInitState();
-                    RestorePstatToInitState();
-                }
-            }
-        }
-        else
-        {
-           OIC_LOG(ERROR, TAG, "Invalid DOXM resource.");
-        }
-    }
-    else
-    {
-        if (ehRequest->devAddr.adapter == OC_ADAPTER_IP)
-        {
-            previousMsgId = ehRequest->messageID;
-        }
-    }
+exit:
 
     // Send response payload to request originator
     ehRet = ((SendSRMResponse(ehRequest, ehRet, NULL, 0)) == OC_STACK_OK) ?
@@ -1067,34 +1019,6 @@ OCStackResult DeInitPstatResource()
     return OCDeleteResource(gPstatHandle);
 }
 
-/**
- * Function to restore pstat resurce to initial status.
- * This function will use in case of error while ownership transfer
- */
-void RestorePstatToInitState()
-{
-    if(gPstat)
-    {
-        OIC_LOG(INFO, TAG, "PSTAT resource will revert back to initial status.");
-        gPstat->dos.state = DOS_RFOTM;
-        gPstat->dos.pending = false;
-        gPstat->cm = (OicSecDpm_t)(gPstat->cm | TAKE_OWNER);
-        OIC_LOG_V(INFO, TAG, "%s setting gPstat->tm = %u",
-            __func__, (OicSecDpm_t)(gPstat->tm & (~TAKE_OWNER)));
-        gPstat->tm = (OicSecDpm_t)(gPstat->tm & (~TAKE_OWNER));
-        gPstat->om = SINGLE_SERVICE_CLIENT_DRIVEN;
-        if(gPstat->sm && 0 < gPstat->smLen)
-        {
-            gPstat->sm[0] = SINGLE_SERVICE_CLIENT_DRIVEN;
-        }
-
-        if (!UpdatePersistentStorage(gPstat))
-        {
-            OIC_LOG(ERROR, TAG, "Failed to revert PSTAT in persistent storage");
-        }
-    }
-}
-
 OCStackResult GetPstatRownerId(OicUuid_t *rowneruuid)
 {
     if (gPstat && rowneruuid)