Hi
I think pf sequence number tracking is too strict by one octet.
The bug is triggered by a TCP packet with the FIN bit set and
containing data that fits exactly into the announced window.
This packet announces a window of 1024 octets as scaling factor is 3:
00:58:30.250388 10.188.50.50.45397 > 10.188.50.10.13330: . ack 130049 win 128
<nop,nop,timestamp 110573613 3971688291> (DF)
The other side sends 1024 octes data and FIN:
00:58:30.251669 10.188.50.10.13330 > 10.188.50.50.45397: FP 130049:131073(1024)
ack 1 win 2048 <nop,nop,timestamp 3971688291 110573613>
The packet counts 1025 stream length to pf, 1024 data + 1 FIN.
These 1025 octets do not fit into the 1024 window so pf drops the
packet. Our stack generates and accepts the packet as only the
data part has to fit into the socket buffer.
To fix this, pf should only use the data lenght for window comparison.
ok?
bluhm
Index: net/pf.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf.c,v
retrieving revision 1.817
diff -u -p -r1.817 pf.c
--- net/pf.c 23 Nov 2012 18:35:25 -0000 1.817
+++ net/pf.c 14 Jan 2013 01:40:38 -0000
@@ -4013,7 +4013,7 @@ pf_tcp_track_full(struct pf_pdesc *pd, s
{
struct tcphdr *th = pd->hdr.tcp;
u_int16_t win = ntohs(th->th_win);
- u_int32_t ack, end, seq, orig_seq;
+ u_int32_t ack, end, data_end, seq, orig_seq;
u_int8_t sws, dws;
int ackskew;
@@ -4076,6 +4076,7 @@ pf_tcp_track_full(struct pf_pdesc *pd, s
}
}
}
+ data_end = end;
if (th->th_flags & TH_FIN)
end++;
@@ -4106,6 +4107,7 @@ pf_tcp_track_full(struct pf_pdesc *pd, s
end = seq + pd->p_len;
if (th->th_flags & TH_SYN)
end++;
+ data_end = end;
if (th->th_flags & TH_FIN)
end++;
}
@@ -4152,7 +4154,7 @@ pf_tcp_track_full(struct pf_pdesc *pd, s
#define MAXACKWINDOW (0xffff + 1500) /* 1500 is an arbitrary fudge factor */
- if (SEQ_GEQ(src->seqhi, end) &&
+ if (SEQ_GEQ(src->seqhi, data_end) &&
/* Last octet inside other's window space */
SEQ_GEQ(seq, src->seqlo - (dst->max_win << dws)) &&
/* Retrans: not more than one window back */
@@ -4223,7 +4225,7 @@ pf_tcp_track_full(struct pf_pdesc *pd, s
} else if ((dst->state < TCPS_SYN_SENT ||
dst->state >= TCPS_FIN_WAIT_2 ||
src->state >= TCPS_FIN_WAIT_2) &&
- SEQ_GEQ(src->seqhi + MAXACKWINDOW, end) &&
+ SEQ_GEQ(src->seqhi + MAXACKWINDOW, data_end) &&
/* Within a window forward of the originating packet */
SEQ_GEQ(seq, src->seqlo - MAXACKWINDOW)) {
/* Within a window backward of the originating packet */
@@ -4313,12 +4315,13 @@ pf_tcp_track_full(struct pf_pdesc *pd, s
pd->dir == PF_IN ? "in" : "out",
pd->dir == (*state)->direction ? "fwd" : "rev");
addlog("pf: State failure on: %c %c %c %c | %c %c\n",
- SEQ_GEQ(src->seqhi, end) ? ' ' : '1',
+ SEQ_GEQ(src->seqhi, data_end) ? ' ' : '1',
SEQ_GEQ(seq, src->seqlo - (dst->max_win << dws)) ?
' ': '2',
(ackskew >= -MAXACKWINDOW) ? ' ' : '3',
(ackskew <= (MAXACKWINDOW << sws)) ? ' ' : '4',
- SEQ_GEQ(src->seqhi + MAXACKWINDOW, end) ?' ' :'5',
+ SEQ_GEQ(src->seqhi + MAXACKWINDOW, data_end) ?
+ ' ' :'5',
SEQ_GEQ(seq, src->seqlo - MAXACKWINDOW) ?' ' :'6');
}
REASON_SET(reason, PFRES_BADSTATE);
