Outbound TCP missing packets.

[basil00]

(reported via email).

WinDivert seems to lose some outbound TCP packets. This shows up as "TCP previous segment not captured" errors in wireshark, and can cause TCP re-transmission (& slowing down the connection, sometimes significantly).

Preliminary investigation indicates that the missing packets are not even sent to WinDiverts WFP callout function. Investigation continues...

[GhalemB]

I did some debuging and found that It seems that some outbound TCP segments are received by the callout function divert_classify_callout( ... ). But FwpsQueryPacketInjectionState0 returns FWPS_PACKET_INJECTED_BY_SELF or FWPS_PACKET_PREVIOUSLY_INJECTED_BY_SELF. The returned packet_context is less than the context->priority. So, these segments go unfiltered :

if (packet_state == FWPS_PACKET_INJECTED_BY_SELF || packet_state == FWPS_PACKET_PREVIOUSLY_INJECTED_BY_SELF) { priority = (UINT32)packet_context; if (priority <= context->priority) { result->actionType = FWP_ACTION_CONTINUE; return; } }

[GhalemB]

Why such segments have an injection state FWPS_PACKET_INJECTED_BY_SELF or FWPS_PACKET_PREVIOUSLY_INJECTED_BY_SELF ?

[basil00]

These should be packets that were (re)injected by the user application using WinDivert. In WFP, the re-injected packets are indicated to the call-out once more, and if we capture them again, there'd be an infinite loop. The above code prevents this from happening. It's made a bit more complex because of priorities. If fresh packets are triggering the above code, then something is wrong.

[GhalemB]

Yes, I agree !

I have checked these packets through debugview (adding divert_filter() before), and apparently they are of interest ...

I tried to let them being queued but, the things are not going better, it creates another problem elsewhere (too slow divertSend operations).

Le Lundi 17 mars 2014 16h16, basil00 notifications@github.com a écrit :

These should be packets that were (re)injected by the user application using WinDivert. In WFP, the re-injected packets are indicated to the call-out once more, and if we capture them again, there'd be an infinite loop. The above code prevents this from happening. It's made a bit more complex because of priorities. If fresh packets are triggering the above code, then something is wrong. — Reply to this email directly or view it on GitHub.

[GhalemB]

I managed to fixe the bug (at least the first tests show it).

In fact, for some reason we are not able to parse all the NET_BUFFERs of a NET_BUFFER_LIST containing more than one NET_BUFFER (when WFP do fragmentation).

In stead using NET_BUFFER_LIST_NEXT_NBL working on NET_BUFFER_LISTs, I use NET_BUFFER_NEXT_NB working on NET_BUFFERs.

The things are going better, here is the modified code of divert_classify_callout :

static void divert_classify_callout(IN UINT8 direction, IN UINT32 if_idx,     IN UINT32 sub_if_idx, IN BOOL isipv4,     IN const FWPS_INCOMING_VALUES0 fixed_vals,     IN const FWPS_INCOMING_METADATA_VALUES0 meta_vals, IN OUT void data,     const FWPS_FILTER0 filter, IN UINT64 flow_context,     OUT FWPS_CLASSIFY_OUT0 *result) {     FWPS_PACKET_INJECTION_STATE packet_state;     HANDLE packet_context;     UINT32 priority;     PNET_BUFFER_LIST buffers, buffers_fst, buffers_itr, buffers1;     PNET_BUFFER buffer, buffer1, buffers_fst1, buffers_itr1;     BOOL outbound;     context_t context;     packet_t packet;     BOOL injected = FALSE;

    DEBUG("divert classify callout\n");

    // Basic checks:     if (!(result->rights & FWPS_RIGHT_ACTION_WRITE) || data == NULL)     {         return;     }

    context = (context_t)filter->context;     buffers = (PNET_BUFFER_LIST)data;     buffers1 = buffers;     buffer1 = NET_BUFFER_LIST_FIRST_NB(buffers1);     buffer = NET_BUFFER_LIST_FIRST_NB(buffers);

    // this code helped me to check whether a NET_BUFFER_LIST contains more than one NET_BUFFER.     // Here, we are able to parse the NET_BUFFER_LIST through NET_BUFFER_NEXT_NB but not with    //NET_BUFFER_LIST_NEXT_NBL     while(buffers1 != NULL || buffer1 != NULL)     {                 DEBUG("divert classify callout0110");                if(buffer1 != NULL){             DEBUG("divert classify callout0111");                buffer1 = NET_BUFFER_NEXT_NB(buffer1);         }         if(buffers1 != NULL){             DEBUG("divert classify callout0112");                    buffers1 = NET_BUFFER_LIST_NEXT_NBL(buffers1);         }     }    

           if (isipv4)     {         packet_state = FwpsQueryPacketInjectionState0(inject_handle, buffers,             &packet_context);     }     else     {         packet_state = FwpsQueryPacketInjectionState0(injectv6_handle,             buffers, &packet_context);     }     if (!divert_context_verify(context, DIVERT_CONTEXT_STATE_OPEN))     {         result->actionType = FWP_ACTION_CONTINUE;         return;     }     if (packet_state == FWPS_PACKET_INJECTED_BY_SELF ||         packet_state == FWPS_PACKET_PREVIOUSLY_INJECTED_BY_SELF)     {         //buffers1 = NdisAllocateCloneNetBufferList(buffers, pool_handle, NULL, NDIS_CLONE_FLAGS_USE_ORIGINAL_MDLS);         //buffers = buffers;         //divert_filter(NET_BUFFER_LIST_FIRST_NB(buffers), if_idx, sub_if_idx, direction == DIVERT_DIRECTION_OUTBOUND,         //    context->filter);

        //result->actionType = FWP_ACTION_PERMIT;         injected = TRUE;         priority = (UINT32)packet_context;         DEBUG("divert classify callout004 %04x %04x", priority, context->priority);         if (priority <= context->priority)         {                     DEBUG("divert classify callout005");

            result->actionType = FWP_ACTION_PERMIT;             return;         }         //priority = 0;     }     else     {         priority = 0;     }

    /      * This code is complicated by the fact the a single NET_BUFFER_LIST      * may contain several NET_BUFFER structures.  Each NET_BUFFER needs to      * be filtered independently.  To achieve this we do the following:      * 1) First check if any NET_BUFFER passes the filter.      * 2) If no, then CONTINUE the entire NET_BUFFER_LIST.      * 3) Else, split the NET_BUFFER_LIST into individual NET_BUFFERs; and          either queue or re-inject based on the filter.      */

    // Find the first NET_BUFFER we need to queue:         outbound = (direction == DIVERT_DIRECTION_OUTBOUND);

    buffers_fst1 = NET_BUFFER_LIST_FIRST_NB(buffers);     do     {                 if (divert_filter(buffers_fst1, if_idx, sub_if_idx, outbound,             context->filter))         {             break;         }         buffers_fst1 = NET_BUFFER_NEXT_NB(buffers_fst1);     }     while (buffers_fst1 != NULL);

    // No NET_BUFFER needs to be queued, permit the entire NET_BUFFER_LIST:     if (buffers_fst1 == NULL)     {         result->actionType = FWP_ACTION_CONTINUE;         return;     }

    if ((context->flags & DIVERT_FLAG_SNIFF) == 0)     {         // Re-inject all packets up to 'buffers_fst'         buffers_itr1 = NET_BUFFER_LIST_FIRST_NB(buffers);         while (buffers_itr1 != buffers_fst1)         {             //buffer = NET_BUFFER_LIST_FIRST_NB(buffers_itr1);             if (!divert_reinject_packet(context, direction, isipv4, if_idx,                     sub_if_idx, priority, buffers, buffers_itr1))             {                 goto divert_classify_callout_exit;             }             buffers_itr1 = NET_BUFFER_NEXT_NB(buffers_itr1);         }     }     else     {         buffers_itr1 = buffers_fst1;     }

    // Queue buffers_itr = buffers_fst, which matched our filter.     //buffer = NET_BUFFER_LIST_FIRST_NB(buffers_itr);     if (!divert_queue_packet(context, buffers, buffers_itr1, direction, if_idx,             sub_if_idx))     {         goto divert_classify_callout_exit;     }     buffers_itr1 = NET_BUFFER_NEXT_NB(buffers_itr1);

    // Queue or re-inject remaining packets.     while (buffers_itr1 != NULL)     {         //buffer = NET_BUFFER_LIST_FIRST_NB(buffers_itr);         if (divert_filter(buffers_itr1, if_idx, sub_if_idx, outbound,             context->filter))         {             if (!divert_queue_packet(context, buffers, buffers_itr1, direction,                     if_idx, sub_if_idx))             {                 goto divert_classify_callout_exit;             }         }         else if ((context->flags & DIVERT_FLAG_SNIFF) == 0)         {             if (!divert_reinject_packet(context, direction, isipv4, if_idx,                     sub_if_idx, priority, buffers, buffers_itr1))             {                 goto divert_classify_callout_exit;             }         }         buffers_itr1 = NET_BUFFER_NEXT_NB(buffers_itr1);     }

    if ((context->flags & DIVERT_FLAG_DROP) == 0)     {         DEBUG("divert classify callout17\n");         divert_read_service(context);     }

divert_classify_callout_exit:

    if ((context->flags & DIVERT_FLAG_SNIFF) != 0)     {         result->actionType = FWP_ACTION_CONTINUE;     }     else     {         //if(injected == TRUE)         //    result->actionType = FWP_ACTION_PERMIT;         //else             result->actionType = FWP_ACTION_BLOCK;         result->flags |= FWPS_CLASSIFY_OUT_FLAG_ABSORB;         result->rights &= ~FWPS_RIGHT_ACTION_WRITE;     } }

Le Lundi 17 mars 2014 16h21, boudour ghalem boudour_ghalem@yahoo.fr a écrit :

Yes, I agree !

I have checked these packets through debugview (adding divert_filter() before), and apparently they are of interest ...

I tried to let them being queued but, the things are not going better, it creates another problem elsewhere (too slow divertSend operations).

Le Lundi 17 mars 2014 16h16, basil00 notifications@github.com a écrit :

These should be packets that were (re)injected by the user application using WinDivert. In WFP, the re-injected packets are indicated to the call-out once more, and if we capture them again, there'd be an infinite loop. The above code prevents this from happening. It's made a bit more complex because of priorities. If fresh packets are triggering the above code, then something is wrong. — Reply to this email directly or view it on GitHub.

[basil00]

I will try to confirm and get back in a few days.

[basil00]

In stead using NET_BUFFER_LIST_NEXT_NBL working on NET_BUFFER_LISTs, I use NET_BUFFER_NEXT_NB working on NET_BUFFERs.

OK, I have run some tests and this does appear to be the source of the problem. I will work on a fix asap.

[basil00]

I've committed a patch that seems to solve the problem (at least for the preliminary tests). I need to do more testing, and then will incorporate in the next release.

[basil00]

This problem appears to have been fixed, so the issue is closed. A new version of WinDivert should be released shortly (~1 week).