Date: Sat, 11 Aug 2018 17:42:06 +0200 From: Andreas Longwitz <longwitz@incore.de> To: freebsd-net@freebsd.org Subject: Re: natd sends wrong sequence number when a retransmitted PASV packet comes in Message-ID: <5B6F03CE.8090105@incore.de> In-Reply-To: <5B6B2725.9030306@incore.de> References: <5B6B2725.9030306@incore.de>
next in thread | previous in thread | raw e-mail | index | archive | help
At first I have to correct a typo in my former post: "must have sequence
number 465" (not 455).
In the meantime I have build a complete test environment to debug this
problem and I am rather sure, there is a bug in natd.
For every ftp connection natd creates a little table (struct
ack_data_record ack) of size N_LINK_TCP_DATA to save the information
about altered length for TCP packets. This table is filled in function
AddSeq() in a circular list each time when "227 Entering Passive Mode"
is sent out to the ftp client. In TcpAliasOut there is a lookup in this
table by calling GetDelteSeqOut() to get the correct delta value to
create the value of seq for the outgoing packet. GetDeltaSeqOut searches
the table in a sequential manner and looks for the biggest ack_old value
not bigger than the actual seq and returns delta from the matching table
line. As long as all ack_old in the table are different, there is no
problem with this procedure. But if for what reason ever one of the "227
Entering" packets must be retransmitted, then we have two entries in the
table with the same value of ack_old and maybe different delta because
of port numbers with different decimal lenght. In this case if the delta
information of the original "227 Entering" packet is placed in the table
before the information of the replayed "227 Entering" packet the
function GetDeltaSeqOut() returns for the next outgoing packet the wrong
old delta value and the connection to the ftp client breaks.
I think the function GetDeltaSeqOut() should also treat the table in a
circular manner, so we can search the table from newer to older entries.
Especially if there are identical ack_new values in the table, we can
take the delta value of the last entry. The following patch solved the
problem for me:
--- alias_db.c.orig 2015-07-03 16:40:03.000000000 +0200
+++ alias_db.c 2018-08-10 23:31:09.242105000 +0200
@@ -2031,14 +2035,18 @@
packet size was altered is searched.
*/
- int i;
+ int i, j;
int delta, seq_diff_min;
delta = 0;
seq_diff_min = -1;
- for (i = 0; i < N_LINK_TCP_DATA; i++) {
+ i = lnk->data.tcp->state.index;
+ for (j = 0; j < N_LINK_TCP_DATA; j++) {
struct ack_data_record x;
+ if (i == 0)
+ i = N_LINK_TCP_DATA;
+ i--;
x = lnk->data.tcp->ack[i];
if (x.active == 1) {
int seq_diff;
Please can somebody review this patch. I am not sure if the same
modification should also be done in the function GetDeltaAckIn().
--
Andreas Longwitz
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?5B6F03CE.8090105>