Can't make FTPPortmapper work correct, detailed problem definition

[Eddie Kohler]

Hi Tomasz,

I've looked into your problem and found that it is in fact a bug. :( But
it had a simple fix; patch appended.

The bug was this: The ACK adjustment was determined by looking at the raw
ACK number; but that is wrong; you should look at (ACK - DELTA) and compare
that to TRIGGER. Explanation: TCPRewriter allows you to change the sequence
numbers a connection uses. It expresses this as a TRIGGER (sequence number)
plus a DELTA (adjustment). The basic idea is that all sequence numbers from
TRIGGER onwards should be adjusted by DELTA. For instance, say TRIGGER=5,
DELTA=-2. That means that the rewriter changes a SEQ of 5 into a SEQ of 3.
So an ACK for 3 should be changed to an ACK for 5. But obviously !(3 >= 5)
-- so the adjustment wouldn't previously take place. With the new formula,
though, (3 - -2) >= 5 is true, so we're all set.

Thanks for the bug report, and let us know if you find any other problems!

Eddie


diff -u -u -w -r1.29 tcprewriter.cc
--- elements/tcpudp/tcprewriter.cc      6 Oct 2002 18:09:13 -0000       1.29
+++ elements/tcpudp/tcprewriter.cc      9 Feb 2003 23:05:46 -0000
@@ -77,16 +77,14 @@
   // update sequence numbers
   uint32_t csum_delta = _udp_csum_delta;

-  uint32_t oldval = ntohl(tcph->th_seq);
-  uint32_t newval = htonl(oldval + delta_for(oldval));
+  uint32_t newval = htonl(new_seq(ntohl(tcph->th_seq)));
   if (tcph->th_seq != newval) {
     csum_delta += (~tcph->th_seq >> 16) + (~tcph->th_seq & 0xFFFF)
       + (newval >> 16) + (newval & 0xFFFF);
     tcph->th_seq = newval;
   }

-  oldval = ntohl(tcph->th_ack);
-  newval = htonl(oldval - reverse()->delta_for(oldval));
+  newval = htonl(reverse()->new_ack(ntohl(tcph->th_ack)));
   if (tcph->th_ack != newval) {
     csum_delta += (~tcph->th_ack >> 16) + (~tcph->th_ack & 0xFFFF)
       + (newval >> 16) + (newval & 0xFFFF);
diff -u -u -w -r1.13 tcprewriter.hh
--- elements/tcpudp/tcprewriter.hh      6 Aug 2002 00:52:05 -0000       1.13
+++ elements/tcpudp/tcprewriter.hh      9 Feb 2003 23:05:46 -0000
@@ -83,7 +83,8 @@
     TCPMapping *reverse() const                { return static_cast<TCPMapping *>(_reverse); }

     int update_seqno_delta(tcp_seq_t old_seqno, int32_t delta);
-    int32_t delta_for(tcp_seq_t) const;
+    tcp_seq_t new_seq(tcp_seq_t) const;
+    tcp_seq_t new_ack(tcp_seq_t) const;

     void apply(WritablePacket *p);

@@ -150,10 +151,17 @@
     return 0;
 }

-inline int32_t
-TCPRewriter::TCPMapping::delta_for(tcp_seq_t seqno) const
+inline tcp_seq_t
+TCPRewriter::TCPMapping::new_seq(tcp_seq_t seqno) const
 {
-  return (SEQ_GEQ(seqno, _trigger) ? _delta : _old_delta);
+  return seqno + (SEQ_GEQ(seqno, _trigger) ? _delta : _old_delta);
+}
+
+inline tcp_seq_t
+TCPRewriter::TCPMapping::new_ack(tcp_seq_t ackno) const
+{
+  tcp_seq_t mod_ackno = ackno - _delta;
+  return (SEQ_GEQ(mod_ackno, _trigger) ? mod_ackno : ackno - _old_delta);
 }

 CLICK_ENDDECLS