diff options
author | Anand Kumar <kumaranand@vmware.com> | 2017-06-09 19:54:45 -0700 |
---|---|---|
committer | Ben Pfaff <blp@ovn.org> | 2017-07-10 11:14:55 -0700 |
commit | b34cd6119aa1ce50d910252202e5eaa13b5fce5e (patch) | |
tree | ed06dafb654091201a809284b306f3dfa31f23f1 /datapath-windows/ovsext | |
parent | ef666482cb8ab034a2aec3cec6ae07bd7b62136b (diff) | |
download | openvswitch-b34cd6119aa1ce50d910252202e5eaa13b5fce5e.tar.gz |
datapath-windows: Add validations in fragmentation module
- Minimum valid fragment size is 400 bytes, any fragment smaller
is likely to be intentionally crafted (CVE-2000-0305).
- Validate maximum length of an Ip datagram
- Added counters to keep track of number of fragments for a given
Ip datagram.
Signed-off-by: Anand Kumar <kumaranand@vmware.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: Alin Gabriel Serdean <aserdean@cloudbasesolutions.com>
Acked-by: Sairam Venugopal <vsairam@vmware.com>
Diffstat (limited to 'datapath-windows/ovsext')
-rw-r--r-- | datapath-windows/ovsext/Actions.c | 2 | ||||
-rw-r--r-- | datapath-windows/ovsext/IpFragment.c | 41 | ||||
-rw-r--r-- | datapath-windows/ovsext/IpFragment.h | 2 |
3 files changed, 32 insertions, 13 deletions
diff --git a/datapath-windows/ovsext/Actions.c b/datapath-windows/ovsext/Actions.c index 3ea066b85..ebe8264aa 100644 --- a/datapath-windows/ovsext/Actions.c +++ b/datapath-windows/ovsext/Actions.c @@ -2181,7 +2181,7 @@ OvsDoExecuteActions(POVS_SWITCH_CONTEXT switchContext, if (status != NDIS_STATUS_SUCCESS) { /* Pending NBLs are consumed by Defragmentation. */ if (status != NDIS_STATUS_PENDING) { - OVS_LOG_ERROR("CT Action failed"); + OVS_LOG_ERROR("CT Action failed status = %lu", status); dropReason = L"OVS-conntrack action failed"; } else { /* We added a new pending NBL to be consumed later. diff --git a/datapath-windows/ovsext/IpFragment.c b/datapath-windows/ovsext/IpFragment.c index 0874cb99d..e601a1538 100644 --- a/datapath-windows/ovsext/IpFragment.c +++ b/datapath-windows/ovsext/IpFragment.c @@ -25,6 +25,10 @@ #undef OVS_DBG_MOD #endif #define OVS_DBG_MOD OVS_DBG_IPFRAG +/* Based on MIN_FRAGMENT_SIZE.*/ +#define MAX_FRAGMENTS 164 +#define MIN_FRAGMENT_SIZE 400 +#define MAX_IPDATAGRAM_SIZE 65535 /* Function declarations */ static VOID OvsIpFragmentEntryCleaner(PVOID data); @@ -146,8 +150,9 @@ OvsIpv4Reassemble(POVS_SWITCH_CONTEXT switchContext, IPHdr *ipHdr, *newIpHdr; CHAR *ethBuf[sizeof(EthHdr)]; CHAR *packetBuf; - UINT16 ipHdrLen, packetLen, packetHeader; + UINT16 ipHdrLen, packetHeader; POVS_FRAGMENT_LIST head = NULL; + UINT32 packetLen; curNb = NET_BUFFER_LIST_FIRST_NB(*curNbl); ASSERT(NET_BUFFER_NEXT_NB(curNb) == NULL); @@ -161,7 +166,10 @@ OvsIpv4Reassemble(POVS_SWITCH_CONTEXT switchContext, if (ipHdr == NULL) { return NDIS_STATUS_INVALID_PACKET; } - ipHdrLen = (UINT16)(ipHdr->ihl * 4); + ipHdrLen = ipHdr->ihl * 4; + if (ipHdrLen + entry->totalLen > MAX_IPDATAGRAM_SIZE) { + return NDIS_STATUS_INVALID_LENGTH; + } packetLen = ETH_HEADER_LENGTH + ipHdrLen + entry->totalLen; packetBuf = (CHAR*)OvsAllocateMemoryWithTag(packetLen, OVS_IPFRAG_POOL_TAG); @@ -185,7 +193,10 @@ OvsIpv4Reassemble(POVS_SWITCH_CONTEXT switchContext, packetHeader = ETH_HEADER_LENGTH + ipHdrLen; head = entry->head; while (head) { - ASSERT((packetHeader + head->offset) <= packetLen); + if ((UINT32)(packetHeader + head->offset) > packetLen) { + status = NDIS_STATUS_INVALID_DATA; + goto cleanup; + } NdisMoveMemory(packetBuf + packetHeader + head->offset, head->pbuff, head->len); head = head->next; @@ -197,10 +208,6 @@ OvsIpv4Reassemble(POVS_SWITCH_CONTEXT switchContext, status = NDIS_STATUS_RESOURCES; } - OvsFreeMemoryWithTag(packetBuf, OVS_IPFRAG_POOL_TAG); - /* Timeout the entry so that clean up thread deletes it .*/ - entry->expiration -= IPFRAG_ENTRY_TIMEOUT; - /* Complete the fragment NBL */ ctx = (POVS_BUFFER_CONTEXT)NET_BUFFER_LIST_CONTEXT_DATA_START(*curNbl); if (ctx->flags & OVS_BUFFER_NEED_COMPLETE) { @@ -214,6 +221,9 @@ OvsIpv4Reassemble(POVS_SWITCH_CONTEXT switchContext, ctx = (POVS_BUFFER_CONTEXT)NET_BUFFER_LIST_CONTEXT_DATA_START(*newNbl); ctx->mru = entry->mru; *curNbl = *newNbl; +cleanup: + OvsFreeMemoryWithTag(packetBuf, OVS_IPFRAG_POOL_TAG); + entry->markedForDelete = TRUE; return status; } /* @@ -259,12 +269,15 @@ OvsProcessIpv4Fragment(POVS_SWITCH_CONTEXT switchContext, if (ipHdr == NULL) { return NDIS_STATUS_INVALID_PACKET; } - ipHdrLen = (UINT16)(ipHdr->ihl * 4); + ipHdrLen = ipHdr->ihl * 4; payloadLen = ntohs(ipHdr->tot_len) - ipHdrLen; offset = ntohs(ipHdr->frag_off) & IP_OFFSET; offset <<= 3; flags = ntohs(ipHdr->frag_off) & IP_MF; - + /* Only the last fragment can be of smaller size.*/ + if (flags && ntohs(ipHdr->tot_len) < MIN_FRAGMENT_SIZE) { + return NDIS_STATUS_INVALID_LENGTH; + } /*Copy fragment specific fields. */ fragKey.protocol = ipHdr->protocol; fragKey.id = ipHdr->id; @@ -318,6 +331,7 @@ OvsProcessIpv4Fragment(POVS_SWITCH_CONTEXT switchContext, entry->mru = ETH_HEADER_LENGTH + ipHdrLen + payloadLen; entry->recvdLen += fragStorage->len; entry->head = entry->tail = fragStorage; + entry->numFragments = 1; if (!flags) { entry->totalLen = offset + payloadLen; } @@ -337,8 +351,9 @@ OvsProcessIpv4Fragment(POVS_SWITCH_CONTEXT switchContext, /* Acquire the entry lock. */ NdisAcquireSpinLock(&(entry->lockObj)); NdisGetCurrentSystemTime((LARGE_INTEGER *)¤tTime); - if (currentTime > entry->expiration) { - /* Expired entry. */ + if (currentTime > entry->expiration || entry->numFragments == MAX_FRAGMENTS) { + /* Mark the entry for delete. */ + entry->markedForDelete = TRUE; goto fragment_error; } POVS_FRAGMENT_LIST next = entry->head; @@ -393,6 +408,7 @@ found: /*Update Maximum recieved Unit */ entry->mru = entry->mru > (ETH_HEADER_LENGTH + ipHdrLen + payloadLen) ? entry->mru : (ETH_HEADER_LENGTH + ipHdrLen + payloadLen); + entry->numFragments++; if (!flags) { entry->totalLen = offset + payloadLen; } @@ -404,6 +420,7 @@ found: return status; } fragment_error: + status = NDIS_STATUS_INVALID_PACKET; /* Release the entry lock. */ NdisReleaseSpinLock(&(entry->lockObj)); payload_copy_error: @@ -460,7 +477,7 @@ static VOID OvsIpFragmentEntryDelete(POVS_IPFRAG_ENTRY entry, BOOLEAN checkExpiry) { NdisAcquireSpinLock(&(entry->lockObj)); - if (checkExpiry) { + if (!entry->markedForDelete && checkExpiry) { UINT64 currentTime; NdisGetCurrentSystemTime((LARGE_INTEGER *)¤tTime); if (entry->expiration > currentTime) { diff --git a/datapath-windows/ovsext/IpFragment.h b/datapath-windows/ovsext/IpFragment.h index e650399e6..cd5b96033 100644 --- a/datapath-windows/ovsext/IpFragment.h +++ b/datapath-windows/ovsext/IpFragment.h @@ -37,6 +37,8 @@ typedef struct _OVS_IPFRAG_KEY { typedef struct _OVS_IPFRAG_ENTRY { NDIS_SPIN_LOCK lockObj; /* To access the entry. */ + BOOLEAN markedForDelete; + UINT8 numFragments; UINT16 totalLen; UINT16 recvdLen; UINT16 mru; |