[ovs-dev] datapath-windows: Do not drop Ip fragments less than MIN_FRAGMENT_SIZE

Previously ipfragment module would drop any fragments less than
MIN_FRAGMENT_SIZE (400 bytes), which was added to safeguard against the
vulnerability CVE-2000-0305. This check is incorrect, since minimum size
of the Ipfragment is 68 bytes (i.e. max length of Ip Header + 8 bytes of
L4 header). So Ip fragments less than MIN_FRAGMENT_SIZE (400 bytes) is not
guranted to be malformed or illegal.

To guard against security vulnerability CVE-2000-0305, for a given ip
datagram, ipfragments should be dropped only when number of smallest
fragments recieved reaches a certain threshold.

Signed-off-by: Anand Kumar <kumaranand@vmware.com>
---
 datapath-windows/ovsext/IpFragment.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

I guess you can also remove the define
(https://github.com/openvswitch/ovs/blob/master/datapath-windows/ovsext/IpFr
agment.c#L30)
since it is not used anywhere else.

Thanks,
Alin.

-----Mesaj original-----
De la: ovs-dev-bounces@openvswitch.org <ovs-dev-bounces@openvswitch.org> În
numele Anand Kumar
Trimis: Tuesday, March 6, 2018 1:21 AM
Către: dev@openvswitch.org
Subiect: [ovs-dev] [PATCH] datapath-windows: Do not drop Ip fragments less
than MIN_FRAGMENT_SIZE

Previously ipfragment module would drop any fragments less than
MIN_FRAGMENT_SIZE (400 bytes), which was added to safeguard against the
vulnerability CVE-2000-0305. This check is incorrect, since minimum size of
the Ipfragment is 68 bytes (i.e. max length of Ip Header + 8 bytes of
L4 header). So Ip fragments less than MIN_FRAGMENT_SIZE (400 bytes) is not
guranted to be malformed or illegal.

To guard against security vulnerability CVE-2000-0305, for a given ip
datagram, ipfragments should be dropped only when number of smallest
fragments recieved reaches a certain threshold.

Signed-off-by: Anand Kumar <kumaranand@vmware.com>
---
 datapath-windows/ovsext/IpFragment.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/datapath-windows/ovsext/IpFragment.c
b/datapath-windows/ovsext/IpFragment.c
index 3d5277a..da9d33a 100644
--- a/datapath-windows/ovsext/IpFragment.c
+++ b/datapath-windows/ovsext/IpFragment.c
@@ -275,10 +275,7 @@ OvsProcessIpv4Fragment(POVS_SWITCH_CONTEXT
switchContext,
     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;
--
2.9.3.windows.1

Thanks for the review. 

MIN_FRAGMENT_SIZE is used to determine maximum number of fragments that are allowed for an IP datagram. 
I will update the macro MAX_FRAGMENTS to compute the value based on MIN_FRAGMENT_SIZE.

Regards,
Anand Kumar

On 3/6/18, 5:43 AM, "aserdean@ovn.org" <aserdean@ovn.org> wrote:

    I guess you can also remove the define
    (https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_openvswitch_ovs_blob_master_datapath-2Dwindows_ovsext_IpFr&d=DwIFBA&c=uilaK90D4TOVoH58JNXRgQ&r=Q5z9tBe-nAOpE7LIHSPV8uy5-437agMXvkeHHMkR8Us&m=IO3JXN8xQplOxWRufcsmrjAad9LTMz362Yoy7M5ydI0&s=FWfFKSKtm6492BHmJ_HpEwBo_iYESHGduKFqDgo4ZOU&e=
    agment.c#L30)
    since it is not used anywhere else.
    
    Thanks,
    Alin.
    
    -----Mesaj original-----
    De la: ovs-dev-bounces@openvswitch.org <ovs-dev-bounces@openvswitch.org> În
    numele Anand Kumar
    Trimis: Tuesday, March 6, 2018 1:21 AM
    Către: dev@openvswitch.org
    Subiect: [ovs-dev] [PATCH] datapath-windows: Do not drop Ip fragments less
    than MIN_FRAGMENT_SIZE
    
    Previously ipfragment module would drop any fragments less than
    MIN_FRAGMENT_SIZE (400 bytes), which was added to safeguard against the
    vulnerability CVE-2000-0305. This check is incorrect, since minimum size of
    the Ipfragment is 68 bytes (i.e. max length of Ip Header + 8 bytes of
    L4 header). So Ip fragments less than MIN_FRAGMENT_SIZE (400 bytes) is not
    guranted to be malformed or illegal.
    
    To guard against security vulnerability CVE-2000-0305, for a given ip
    datagram, ipfragments should be dropped only when number of smallest
    fragments recieved reaches a certain threshold.
    
    Signed-off-by: Anand Kumar <kumaranand@vmware.com>

    ---
     datapath-windows/ovsext/IpFragment.c | 5 +----
     1 file changed, 1 insertion(+), 4 deletions(-)
    
    diff --git a/datapath-windows/ovsext/IpFragment.c
    b/datapath-windows/ovsext/IpFragment.c
    index 3d5277a..da9d33a 100644
    --- a/datapath-windows/ovsext/IpFragment.c
    +++ b/datapath-windows/ovsext/IpFragment.c
    @@ -275,10 +275,7 @@ OvsProcessIpv4Fragment(POVS_SWITCH_CONTEXT
    switchContext,
         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;
    --
    2.9.3.windows.1
    
    _______________________________________________
    dev mailing list
    dev@openvswitch.org
    https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwIFBA&c=uilaK90D4TOVoH58JNXRgQ&r=Q5z9tBe-nAOpE7LIHSPV8uy5-437agMXvkeHHMkR8Us&m=IO3JXN8xQplOxWRufcsmrjAad9LTMz362Yoy7M5ydI0&s=lZS0kDScEQSgdBhKx1EABsU7dS-a_f9IMQJv1aQar0I&e=