From ef6f47b83dc4d36093c7b561578c3b5858f81e25 Mon Sep 17 00:00:00 2001
From: Dan Mihai <Daniel.Mihai@microsoft.com>
Date: Thu, 23 Feb 2017 13:53:42 -0800
Subject: [PATCH] [IOT-1845] Fix session leak

1. Close OCDoOwnershipTransfer session after successful Ownership
   Transfer (i.e., from ReadyForNomalStatusHandler).

2. Use OC_STACK_UNAUTHORIZED_REQ instead of the CA result returned
   by CAcloseSslConnection when calling SetResult - because SetResult
   expects an OC result as parameter.

Change-Id: I075f7fe168a35a0ab6f5c74f1faa12108156727b
Signed-off-by: Dan Mihai <Daniel.Mihai@microsoft.com>
Reviewed-on: https://gerrit.iotivity.org/gerrit/17485
Tested-by: jenkins-iotivity <jenkins@iotivity.org>
Reviewed-by: Greg Zaverucha <gregz@microsoft.com>
Reviewed-by: Phil Coval <philippe.coval@osg.samsung.com>
Reviewed-by: Kevin Kane <kkane@microsoft.com>
---
 .../provisioning/src/ownershiptransfermanager.c    | 68 +++++++++++++---------
 1 file changed, 40 insertions(+), 28 deletions(-)

diff --git a/resource/csdk/security/provisioning/src/ownershiptransfermanager.c b/resource/csdk/security/provisioning/src/ownershiptransfermanager.c
index c460e6c..666d8d0 100755
--- a/resource/csdk/security/provisioning/src/ownershiptransfermanager.c
+++ b/resource/csdk/security/provisioning/src/ownershiptransfermanager.c
@@ -75,6 +75,7 @@
 #include "pkix_interface.h"
 #include "oxmverifycommon.h"
 #include "psinterface.h"
+#include "ocstackinternal.h"
 
 #define TAG "OIC_OTM"
 
@@ -194,6 +195,28 @@ static OxmAllowTableIdx_t GetOxmAllowTableIdx(OicSecOxm_t oxm)
 }
 
 /**
+ * Internal API to close the secure Ownership Transfer session.
+ */
+static bool CloseSslConnection(const OCProvisionDev_t *selectedDeviceInfo)
+{
+    bool success = true;
+
+    CAEndpoint_t endpoint;
+    CopyDevAddrToEndpoint(&selectedDeviceInfo->endpoint, &endpoint);
+    endpoint.port = selectedDeviceInfo->securePort;
+    CAResult_t caResult = CAcloseSslConnection(&endpoint);
+
+    if (CA_STATUS_OK != caResult)
+    {
+        OIC_LOG_V(ERROR, TAG, "%s: Failed to close DTLS session, caResult = %d",
+            __func__, caResult);
+        success = false;
+    }
+
+    return success;
+}
+
+/**
  * Function to select appropriate  provisioning method.
  *
  * @param[in] supportedMethods   Array of supported methods
@@ -474,15 +497,7 @@ static void SetResult(OTMContext_t* otmCtx, const OCStackResult res)
                 {
                     OIC_LOG(WARNING, TAG, "Internal error in PDMDeleteDevice");
                 }
-
-                CAEndpoint_t endpoint;
-                memcpy(&endpoint, &(otmCtx->selectedDeviceInfo->endpoint), sizeof(CAEndpoint_t));
-                endpoint.port = otmCtx->selectedDeviceInfo->securePort;
-
-                if (CA_STATUS_OK != CAcloseSslConnection(&endpoint))
-                {
-                    OIC_LOG(WARNING, TAG, "Failed to close Secure session");
-                }
+                CloseSslConnection(otmCtx->selectedDeviceInfo);
             }
         }
     }
@@ -1241,7 +1256,8 @@ static OCStackApplicationResult OwnerAclHandler(void *ctx, OCDoHandle UNUSED,
             res = PostOwnershipInformation(otmCtx);
             if(OC_STACK_OK != res)
             {
-                OIC_LOG_V(ERROR, TAG, "%s: Failed to update the ownership information of the new device, res = %d",
+                OIC_LOG_V(ERROR, TAG, "%s: Failed to update the ownership information"
+                    "of the new device, res = %d",
                     __func__, res);
                 SetResult(otmCtx, res);
             }
@@ -1256,15 +1272,10 @@ static OCStackApplicationResult OwnerAclHandler(void *ctx, OCDoHandle UNUSED,
         selectedDeviceInfo->ownerAclUnauthorizedRequest = true;
 
         //Close the temporal secure session and re-connect using the owner credential
-        CAEndpoint_t* endpoint = (CAEndpoint_t *)&selectedDeviceInfo->endpoint;
-        endpoint->port = selectedDeviceInfo->securePort;
-        CAResult_t caResult = CAcloseSslConnection(endpoint);
-
-        if(CA_STATUS_OK != caResult)
+        if(!CloseSslConnection(selectedDeviceInfo))
         {
-            OIC_LOG_V(ERROR, TAG, "%s: Failed to close DTLS session, caResult = %d",
-                __func__, caResult);
-            SetResult(otmCtx, caResult);
+            //Cannot make progress reliably, so return the error code from the previous request.
+            SetResult(otmCtx, OC_STACK_UNAUTHORIZED_REQ);
         }
         else
         {
@@ -1413,16 +1424,17 @@ static OCStackApplicationResult ReadyForNomalStatusHandler(void *ctx, OCDoHandle
         OIC_LOG(INFO, TAG, "Device state is in Ready for Normal Operation.");
         OCStackResult res = PDMSetDeviceState(&otmCtx->selectedDeviceInfo->doxm->deviceID,
                                               PDM_DEVICE_ACTIVE);
-         if (OC_STACK_OK == res)
-         {
-                OIC_LOG_V(INFO, TAG, "Add device's UUID in PDM_DB");
-                SetResult(otmCtx, OC_STACK_OK);
-                return OC_STACK_DELETE_TRANSACTION;
-         }
-          else
-         {
-              OIC_LOG(ERROR, TAG, "Ownership transfer is complete but adding information to DB is failed.");
-         }
+        if (OC_STACK_OK == res)
+        {
+            CloseSslConnection(otmCtx->selectedDeviceInfo);
+            OIC_LOG_V(INFO, TAG, "Add device's UUID in PDM_DB");
+            SetResult(otmCtx, OC_STACK_OK);
+            return OC_STACK_DELETE_TRANSACTION;
+        }
+        else
+        {
+            OIC_LOG(ERROR, TAG, "Ownership transfer is complete but adding information to DB is failed.");
+        }
     }
     else
     {
-- 
2.7.4