Bug 748 - Cloned TCP socket uses wrong source address
Cloned TCP socket uses wrong source address
Status: RESOLVED FIXED
Product: ns-3
Classification: Unclassified
Component: internet
ns-3-dev
All All
: P2 normal
Assigned To: Josh Pelkey
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-11-23 05:41 EST by Fabian Mauchle
Modified: 2010-02-04 14:11 EST (History)
5 users (show)

See Also:


Attachments
patch proposal (21.22 KB, patch)
2009-11-23 05:41 EST, Fabian Mauchle
Details | Diff
Example showing bug (3.93 KB, text/x-c++src)
2010-02-03 13:42 EST, Josh Pelkey
Details
patch proposal (22.72 KB, patch)
2010-02-03 15:02 EST, Josh Pelkey
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Fabian Mauchle 2009-11-23 05:41:50 EST
Created attachment 668 [details]
patch proposal

A listening TCP socket, to which a connection is opened, forks a new socket and determines the source address to use from the route lookup, instead of using the destination address to which the connection request originally has been sent to. This leaves the connection in a state where no communication is possible, as the endpoint of the forked socket does not match the address the client socket is sending packets to.

To solve this, it is required to pass the packet's destination address to the socket. This also requires changes to the Ipv4Endpoint API. 

The proposed patch includes changes to the TCP forking process and the Ipv4Endpoint API. Other protocols are only modified to match the new Ipv4Endpoint API to be able to compile it, but do not make explicit use of it.
Comment 1 Josh Pelkey 2010-02-02 15:08:54 EST
(In reply to comment #0)
> Created an attachment (id=668) [details]
> patch proposal
> 
> A listening TCP socket, to which a connection is opened, forks a new socket and
> determines the source address to use from the route lookup, instead of using
> the destination address to which the connection request originally has been
> sent to. This leaves the connection in a state where no communication is
> possible, as the endpoint of the forked socket does not match the address the
> client socket is sending packets to.
> 
> To solve this, it is required to pass the packet's destination address to the
> socket. This also requires changes to the Ipv4Endpoint API. 
> 
> The proposed patch includes changes to the TCP forking process and the
> Ipv4Endpoint API. Other protocols are only modified to match the new
> Ipv4Endpoint API to be able to compile it, but do not make explicit use of it.

Hi Fabian,

I think I follow this mostly.  Shouldn't the source address from the route lookup give you what you want?  I'm not saying that it is the right way to do it, but you suggested that it doesn't work.

Finally, the patch attached can no longer successfully be applied to ns-3-dev.  In any case, I can still follow what you've done from it.
Comment 2 Fabian Mauchle 2010-02-03 11:58:27 EST
(In reply to comment #1)
> Hi Fabian,
> I think I follow this mostly.  Shouldn't the source address from the route
> lookup give you what you want?  I'm not saying that it is the right way to do
> it, but you suggested that it doesn't work.
> Finally, the patch attached can no longer successfully be applied to ns-3-dev. 
> In any case, I can still follow what you've done from it.

Consider the following network:

    n0 ------------ n1 ---------- n2
ip:  1.2         1.1  2.1      2.2

n1 is a router and has a TCP socket listening on all interfaces.
n0 opens a conneciton to IP 2.1 (eg. the DNS entry points to this address)

on n1, the route lookup to answer the request would give IP 1.1 instead of 2.1 (to which the connect request was sent).
Comment 3 Josh Pelkey 2010-02-03 13:42:29 EST
Created attachment 747 [details]
Example showing bug

I made an example of the scenario Fabian described above.  The tcpdump clearly shows an issue:

    n0 ------------- n1 -------------- n2
10.1.3.1       .3.2     .2.1       .2.2

1.000000 IP 10.1.3.1.49153 > 10.1.2.1.50000: S 0:0(0) win 65535
1.004067 IP 10.1.3.2.50000 > 10.1.3.1.49153: S 0:0(0) ack 1 win 65535
1.004067 IP 10.1.3.1.49153 > 10.1.2.1.50000: . ack 1 win 65535
1.008192 IP 10.1.3.1.49153 > 10.1.2.1.50000: . 0:512(512) ack 1 win 65535
1.008635 IP 10.1.3.1.49153 > 10.1.2.1.50000: F 512:512(0) ack 1 win 65535
4.000000 IP 10.1.3.1.49153 > 10.1.2.1.50000: . 1:513(512) win 65535
4.004476 IP 10.1.2.1.50000 > 10.1.3.1.49153: R 0:0(0) ack 0 win 65535

So 3.1 sends to 2.1, but gets a SYN-ACK response from 3.2.  Since it gets a SYN-ACK, it ACKs and tries to send its data (only 1 packet).  2.1 doesn't reply and 3.1 must retransmit.  Finally 2.1 replies with a RST packet.
Comment 4 Josh Pelkey 2010-02-03 15:02:55 EST
Created attachment 749 [details]
patch proposal

I went through and re-worked this patch so that it can be applied to the current ns-3-dev.  The tcpdump now looks like this:

1.000000 IP 10.1.3.1.49153 > 10.1.2.1.50000: Flags [S], seq 0, win 65535, length 0
1.004067 IP 10.1.2.1.50000 > 10.1.3.1.49153: Flags [S.], seq 0, ack 1, win 65535, length 0
1.004067 IP 10.1.3.1.49153 > 10.1.2.1.50000: Flags [.], ack 1, win 65535, length 0
1.008192 IP 10.1.3.1.49153 > 10.1.2.1.50000: Flags [.], seq 1:513, ack 1, win 65535, length 512
1.008635 IP 10.1.3.1.49153 > 10.1.2.1.50000: Flags [F.], seq 513, ack 1, win 65535, length 0
1.012702 IP 10.1.2.1.50000 > 10.1.3.1.49153: Flags [.], ack 514, win 65535, length 0
1.212668 IP 10.1.2.1.50000 > 10.1.3.1.49153: Flags [.], ack 514, win 65535, length 0
3.002033 IP 10.1.2.1.50000 > 10.1.3.1.49153: Flags [F.], seq 1, ack 514, win 65535, length 0
3.002033 IP 10.1.3.1.49153 > 10.1.2.1.50000: Flags [.], ack 2, win 65535, length 0

I still need to run through it a little more to make sure it is all ok.
Comment 5 Tom Henderson 2010-02-03 16:00:28 EST
(In reply to comment #4)
> Created an attachment (id=749) [details]
> patch proposal
> 
> I went through and re-worked this patch so that it can be applied to the
> current ns-3-dev.  

+1 to avoid using the route lookup in this case, for the reason specified.  The addresses can't change from what was sent in the first SYN.
Comment 6 Josh Pelkey 2010-02-04 14:11:20 EST
changeset 805783c866fc, thanks Fabian.