[Click] AggregateIPFlows NAT Issue . . .

[Eddie Kohler]

Hi John,

Thanks very much for reporting this issue.  A fix, which goes beyond your fix 
in that it also supports fragments, has been checked in.

However, I can't reproduce the problem you report with IPRewriter, even when 
IP's src and dst are the same.  Can you send a test case?

Eddie


John Russell Lane wrote:
> I've run into a minor issue with AggregateIPFlows and NAT where the
> outbound host is the local machine (i.e., where the source IP ==
> destination IP).
> 
> The problem is that AggregateIPFlows does not properly tag a "reverse"
> flow when the source and destination are the same IP address.  That
> is, both the "forward" and "reverse" flow end up being tagged as new
> ("forward").  The reason for this is that the key formed to identify
> both "forward" and "reverse" endpoint flows is essentially something
> like:
> 
>   (SRC_IP < DST_IP) ? <SRC_IP, DST_IP> : <DST_IP, SRC_IP>
> 
> When a "reply" packet comes in, this forms an ordering over the IP
> address pairs.  Ports are then "flipped" to match the given order,
> stored in another structure and later used to identify an individual
> flow between the endpoints.  However, when SRC_IP == DST_IP, this
> "tie" is not handled, the port flipping (which had relied solely on
> the IP address ordering) is not done and the resulting ports used to
> lookup the flow differ for the inbound and outbound packets.
> 
> Though it is a bit of a hack, the following patch resolves this issue
> in AggregateIPFlows by simply handling SRC_IP == DST_IP as a special
> case, examining the given ports in that case and indicating the flip
> should be performed when SRC_PRT > DST_PRT.  This should ensure that a
> first packet and return packet share the same (port-based)
> flow-identifying key on inbound and outbound packets from the same
> host.
> 
> However, in confirming that this patch works (with 1.5/1.6 via git), I
> also had to add a workaround for IPRewriter.  The problem there is
> that TCP checksums are not computed correctly under the above
> conditions (SRC_IP == DST_IP).  Adding SetIPChecksum / SetTCPChecksum
> manually (downstream from the rewrite element) fixes this; so it seems
> the same problem lies somewhere within the map used in IPRewriter.
> I'm pressed for time at the moment and this got me moving again; if no
> one cares to, I'll hopefully have a look at a better fix in a few
> weeks.  :-)
> 
> If I've missed something, please let me know.  Thanks!
> 
> jrl.
> 
> ---Begin Patch---
> diff -ru src/elements/analysis/aggregateipflows.cc
> src.nat_fix/elements/analysis/aggregateipflows.cc
> --- src/elements/analysis/aggregateipflows.cc   2007-12-07
> 11:16:12.000000000 +0900
> +++ src.nat_fix/elements/analysis/aggregateipflows.cc   2007-11-23
> 04:20:18.000000000 +0900
> @@ -529,6 +529,17 @@
>         return handle_fragment(p, paint, m, hpinfo);
> 
>      uint32_t ports = *(reinterpret_cast<const uint32_t *>(udp_ptr));
> +    if (hosts.a == hosts.b) {
> +      const uint16_t * sport = reinterpret_cast<const uint16_t *>(udp_ptr),
> +                     * dport = reinterpret_cast<const uint16_t
> *>(udp_ptr+sizeof(uint16_t));
> +      // This causes a flip below in order to form a source-destination
> +      // invariant ordering over address/port pairs even when the host pair
> +      // is equal.  Without this, one flow ends up being created for the
> +      // forward packet and another is created for reply packets because
> +      // the source-destination port ordering is only dependent upon the IPs
> +      // ("ties" are not handled).
> +      if (*sport > *dport) paint ^= 1;
> +    }
>      if (paint & 1)
>         ports = flip_ports(ports);
>      FlowInfo *finfo = find_flow_info(m, hpinfo, ports, paint & 1, p);
> _______________________________________________
> click mailing list
> click at amsterdam.lcs.mit.edu
> https://amsterdam.lcs.mit.edu/mailman/listinfo/click