From 4147de8859841b3b55904b8c0275674ac53bb790 Mon Sep 17 00:00:00 2001 From: Erich Keane Date: Thu, 13 Aug 2015 13:18:12 -0700 Subject: [PATCH] More robust IP Dual-Mode filtering Update filtering in camessagehandler.c Filter duplicates based on last 4 messages. Filter both requests and responses, separately. Change-Id: I191436f91c733893bcbf12c9ffdb05bc841bfa2f Signed-off-by: John Light Signed-off-by: Erich Keane Reviewed-on: https://gerrit.iotivity.org/gerrit/1994 Tested-by: jenkins-iotivity Reviewed-by: Jon A. Cruz --- resource/csdk/connectivity/api/cacommon.h | 19 ++- resource/csdk/connectivity/src/camessagehandler.c | 134 +++++++++++---------- .../csdk/connectivity/src/ip_adapter/caipadapter.c | 1 + resource/csdk/stack/src/ocstack.c | 8 ++ 4 files changed, 98 insertions(+), 64 deletions(-) diff --git a/resource/csdk/connectivity/api/cacommon.h b/resource/csdk/connectivity/api/cacommon.h index b5beea1..9f72889 100644 --- a/resource/csdk/connectivity/api/cacommon.h +++ b/resource/csdk/connectivity/api/cacommon.h @@ -410,6 +410,20 @@ typedef struct uint16_t port; } CASocket_t; +#define HISTORYSIZE (4) + +typedef struct +{ + CATransportFlags_t flags; + uint16_t messageId; +} CAHistoryItem_t; + +typedef struct +{ + int nextIndex; + CAHistoryItem_t items[HISTORYSIZE]; +} CAHistory_t; + typedef struct { CATransportFlags_t clientFlags; @@ -437,12 +451,13 @@ typedef struct bool terminate; /**< the IP adapter needs to stop */ bool ipv6enabled; /**< IPv6 enabled by OCInit flags */ bool ipv4enabled; /**< IPv4 enabled by OCInit flags */ + bool dualstack; /**< IPv6 and IPv4 enabled */ } ip; struct calayer { - CATransportFlags_t previousRequestFlags; /**< address family filtering */ - uint16_t previousRequestMessageId; /**< address family filtering */ + CAHistory_t requestHistory; /**< filter IP family in requests */ + CAHistory_t responseHistory; /**< filter IP family in responses */ } ca; } CAGlobals_t; diff --git a/resource/csdk/connectivity/src/camessagehandler.c b/resource/csdk/connectivity/src/camessagehandler.c index 01037ca..3021d8d 100644 --- a/resource/csdk/connectivity/src/camessagehandler.c +++ b/resource/csdk/connectivity/src/camessagehandler.c @@ -82,7 +82,7 @@ static void CAProcessReceivedData(CAData_t *data); #endif static void CADestroyData(void *data, uint32_t size); static void CALogPayloadInfo(CAInfo_t *info); -static bool CADropSecondRequest(const CAEndpoint_t *endpoint, uint16_t messageId); +static bool CADropSecondMessage(CAHistory_t *history, const CAEndpoint_t *endpoint, uint16_t id); #ifdef WITH_BWT void CAAddDataToSendThread(CAData_t *data) @@ -123,8 +123,12 @@ static bool CAIsSelectedNetworkAvailable() static CAData_t* CAGenerateHandlerData(const CAEndpoint_t *endpoint, const void *data, CADataType_t dataType) { OIC_LOG(DEBUG, TAG, "CAGenerateHandlerData IN"); - CAInfo_t *info = NULL; - CAData_t *cadata = (CAData_t *) OICCalloc(1, sizeof(CAData_t)); + + CAResponseInfo_t* resInfo = NULL; + CARequestInfo_t* reqInfo = NULL; + CAErrorInfo_t *errorInfo = NULL; + + CAData_t *cadata = (CAData_t *) OICCalloc(1, sizeof (CAData_t)); if (!cadata) { OIC_LOG(ERROR, TAG, "memory allocation failed"); @@ -135,8 +139,7 @@ static CAData_t* CAGenerateHandlerData(const CAEndpoint_t *endpoint, const void if (!ep) { OIC_LOG(ERROR, TAG, "endpoint clone failed"); - OICFree(cadata); - return NULL; + goto errorexit; } OIC_LOG_V(DEBUG, TAG, "address : %s", ep->addr); @@ -144,97 +147,96 @@ static CAData_t* CAGenerateHandlerData(const CAEndpoint_t *endpoint, const void if(CA_RESPONSE_DATA == dataType) { - CAResponseInfo_t* resInfo = (CAResponseInfo_t*)OICCalloc(1, sizeof(CAResponseInfo_t)); + resInfo = (CAResponseInfo_t*)OICCalloc(1, sizeof (CAResponseInfo_t)); if (!resInfo) { OIC_LOG(ERROR, TAG, "memory allocation failed"); - OICFree(cadata); - CAFreeEndpoint(ep); - return NULL; + goto errorexit; } result = CAGetResponseInfoFromPDU(data, resInfo); if (CA_STATUS_OK != result) { OIC_LOG(ERROR, TAG, "CAGetResponseInfoFromPDU Failed"); - CAFreeEndpoint(ep); - CADestroyResponseInfoInternal(resInfo); - OICFree(cadata); - return NULL; + goto errorexit; + } + + if (CADropSecondMessage(&caglobals.ca.responseHistory, endpoint, resInfo->info.messageId)) + { + OIC_LOG(ERROR, TAG, "Second Response with same messageID, Drop it"); + goto errorexit; } + cadata->responseInfo = resInfo; - info = &resInfo->info; OIC_LOG(DEBUG, TAG, "Response Info :"); - CALogPayloadInfo(info); + CALogPayloadInfo(&resInfo->info); } else if (CA_REQUEST_DATA == dataType) { - CARequestInfo_t* reqInfo = (CARequestInfo_t*)OICCalloc(1, sizeof(CARequestInfo_t)); + reqInfo = (CARequestInfo_t*)OICCalloc(1, sizeof (CARequestInfo_t)); if (!reqInfo) { OIC_LOG(ERROR, TAG, "memory allocation failed"); - OICFree(cadata); - CAFreeEndpoint(ep); - return NULL; + goto errorexit; } result = CAGetRequestInfoFromPDU(data, reqInfo); if (CA_STATUS_OK != result) { OIC_LOG(ERROR, TAG, "CAGetRequestInfoFromPDU failed"); - CAFreeEndpoint(ep); - CADestroyRequestInfoInternal(reqInfo); - OICFree(cadata); - return NULL; + goto errorexit; } - if (CADropSecondRequest(endpoint, reqInfo->info.messageId)) + if (CADropSecondMessage(&caglobals.ca.requestHistory, endpoint, reqInfo->info.messageId)) { - OIC_LOG(ERROR, TAG, "Second Request with same Token, Drop it"); - CAFreeEndpoint(ep); - CADestroyRequestInfoInternal(reqInfo); - OICFree(cadata); - return NULL; + OIC_LOG(ERROR, TAG, "Second Request with same messageID, Drop it"); + goto errorexit; } cadata->requestInfo = reqInfo; - info = &reqInfo->info; OIC_LOG(DEBUG, TAG, "Request Info :"); - CALogPayloadInfo(info); - } + CALogPayloadInfo(&reqInfo->info); + } else if (CA_ERROR_DATA == dataType) { - CAErrorInfo_t *errorInfo = (CAErrorInfo_t *)OICCalloc(1, sizeof (CAErrorInfo_t)); + errorInfo = (CAErrorInfo_t *)OICCalloc(1, sizeof (CAErrorInfo_t)); if (!errorInfo) { OIC_LOG(ERROR, TAG, "Memory allocation failed!"); - OICFree(cadata); - CAFreeEndpoint(ep); - return NULL; + goto errorexit; } - CAResult_t result = CAGetErrorInfoFromPDU(data, errorInfo); + result = CAGetErrorInfoFromPDU(data, errorInfo); if (CA_STATUS_OK != result) { OIC_LOG(ERROR, TAG, "CAGetErrorInfoFromPDU failed"); - CAFreeEndpoint(ep); - OICFree(errorInfo); - OICFree(cadata); - return NULL; + goto errorexit; } cadata->errorInfo = errorInfo; - info = &errorInfo->info; OIC_LOG(DEBUG, TAG, "error Info :"); - CALogPayloadInfo(info); + CALogPayloadInfo(&errorInfo->info); + } + else + { + OIC_LOG_V(ERROR, TAG, "bad dataType: %d", dataType); + goto errorexit; } cadata->remoteEndpoint = ep; cadata->dataType = dataType; + OIC_LOG(DEBUG, TAG, "CAGenerateHandlerData OUT"); + return cadata; - OIC_LOG(DEBUG, TAG, "CAGenerateHandlerData OUT"); +errorexit: + CAFreeEndpoint(ep); + OICFree(cadata); + OICFree(resInfo); + OICFree(reqInfo); + OICFree(errorInfo); + return NULL; } static void CATimeoutCallback(const CAEndpoint_t *endpoint, const void *pdu, uint32_t size) @@ -548,44 +550,52 @@ static void CASendThreadProcess(void *threadData) /* * If a second message arrives with the same token and the other address * family, drop it. Typically, IPv6 beats IPv4, so the IPv4 message is dropped. - * This can be made more robust (for instance, another message could arrive - * in between), but it is good enough for now. */ -static bool CADropSecondRequest(const CAEndpoint_t *endpoint, uint16_t messageId) +static bool CADropSecondMessage(CAHistory_t *history, const CAEndpoint_t *ep, uint16_t id) { - if (!endpoint) + if (!ep) { return true; } - if (endpoint->adapter != CA_ADAPTER_IP) + if (ep->adapter != CA_ADAPTER_IP) + { + return false; + } + if (!caglobals.ip.dualstack) { return false; } bool ret = false; - CATransportFlags_t familyFlags = endpoint->flags & CA_IPFAMILY_MASK; + CATransportFlags_t familyFlags = ep->flags & CA_IPFAMILY_MASK; - if (messageId == caglobals.ca.previousRequestMessageId) + for (int i = 0; i < sizeof(history->items) / sizeof(history->items[0]); i++) { - if ((familyFlags ^ caglobals.ca.previousRequestFlags) == CA_IPFAMILY_MASK) + CAHistoryItem_t *item = &(history->items[i]); + if (id == item->messageId) { - if (familyFlags & CA_IPV6) - { - OIC_LOG(INFO, TAG, "IPv6 duplicate response ignored"); - } - else + if ((familyFlags ^ item->flags) == CA_IPFAMILY_MASK) { - OIC_LOG(INFO, TAG, "IPv4 duplicate response ignored"); + OIC_LOG_V(INFO, TAG, "IPv%c duplicate message ignored", + familyFlags & CA_IPV6 ? '6' : '4'); + ret = true; + break; } - ret = true; } } - caglobals.ca.previousRequestFlags = familyFlags; - caglobals.ca.previousRequestMessageId = messageId; + + history->items[history->nextIndex].flags = familyFlags; + history->items[history->nextIndex].messageId = id; + if (++history->nextIndex >= HISTORYSIZE) + { + history->nextIndex = 0; + } + return ret; } -static void CAReceivedPacketCallback(const CAEndpoint_t *remoteEndpoint, const void *data, uint32_t dataLen) +static void CAReceivedPacketCallback(const CAEndpoint_t *remoteEndpoint, const void *data, + uint32_t dataLen) { OIC_LOG(DEBUG, TAG, "IN"); VERIFY_NON_NULL_VOID(remoteEndpoint, TAG, "remoteEndpoint"); diff --git a/resource/csdk/connectivity/src/ip_adapter/caipadapter.c b/resource/csdk/connectivity/src/ip_adapter/caipadapter.c index 1579c03..33a0da9 100644 --- a/resource/csdk/connectivity/src/ip_adapter/caipadapter.c +++ b/resource/csdk/connectivity/src/ip_adapter/caipadapter.c @@ -239,6 +239,7 @@ static void CAInitializeIPGlobals() } caglobals.ip.ipv6enabled = flags & CA_IPV6; caglobals.ip.ipv4enabled = flags & CA_IPV4; + caglobals.ip.dualstack = caglobals.ip.ipv6enabled && caglobals.ip.ipv4enabled; } CAResult_t CAInitializeIP(CARegisterConnectivityCallback registerCallback, diff --git a/resource/csdk/stack/src/ocstack.c b/resource/csdk/stack/src/ocstack.c index 67d8de5..bca27b6 100644 --- a/resource/csdk/stack/src/ocstack.c +++ b/resource/csdk/stack/src/ocstack.c @@ -1589,6 +1589,14 @@ OCStackResult OCInit1(OCMode mode, OCTransportFlags serverFlags, OCTransportFlag } #endif +#ifdef RA_ADAPTER + if(!gRASetInfo) + { + OC_LOG(ERROR, TAG, PCF("Need to call OCSetRAInfo before calling OCInit")); + return OC_STACK_ERROR; + } +#endif + OCStackResult result = OC_STACK_ERROR; OC_LOG(INFO, TAG, PCF("Entering OCInit")); -- 2.7.4