Skip to content

Commit

Permalink
pf: fix syncookies in conjunction with tcp fast port reuse
Browse files Browse the repository at this point in the history
Basic scenario: we have a closed connection (In TCPS_FIN_WAIT_2), and
get a new connection (i.e. SYN) re-using the tuple.

Without syncookies we look at the SYN, and completely unlink the old,
closed state on the SYN.
With syncookies we send a generated SYN|ACK back, and drop the SYN,
never looking at the state table.

So when the ACK (i.e. the third step in the three way handshake for
connection setup) turns up, we’ve not actually removed the old state, so
we find it, and don’t do the syncookie dance, or allow the new
connection to get set up.

Explicitly check for this in pf_test_state_tcp(). If we find a state in
TCPS_FIN_WAIT_2 and the syncookie is valid we delete the existing state
so we can set up the new state.
Note that when we verify the syncookie in pf_test_state_tcp() we don't
decrement the number of half-open connections to avoid an incorrect
double decrement.

MFC after:      2 weeks
Differential Revision:  https://reviews.freebsd.org/D37919

(cherry picked from commit 9c041b4)
  • Loading branch information
kprovost authored and fichtner committed Jan 30, 2023
1 parent 6b25257 commit 0b02f48
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 7 deletions.
1 change: 1 addition & 0 deletions sys/net/pfvar.h
Original file line number Diff line number Diff line change
Expand Up @@ -2108,6 +2108,7 @@ int pf_set_syncookies(struct pfioc_nv *);
int pf_synflood_check(struct pf_pdesc *);
void pf_syncookie_send(struct mbuf *m, int off,
struct pf_pdesc *);
bool pf_syncookie_check(struct pf_pdesc *);
u_int8_t pf_syncookie_validate(struct pf_pdesc *);
struct mbuf * pf_syncookie_recreate_syn(uint8_t, int,
struct pf_pdesc *);
Expand Down
8 changes: 5 additions & 3 deletions sys/netpfil/pf/pf.c
Original file line number Diff line number Diff line change
Expand Up @@ -4976,9 +4976,11 @@ pf_test_state_tcp(struct pf_kstate **state, int direction, struct pfi_kkif *kif,
if ((action = pf_synproxy(pd, state, reason)) != PF_PASS)
return (action);

if (((th->th_flags & (TH_SYN|TH_ACK)) == TH_SYN) &&
dst->state >= TCPS_FIN_WAIT_2 &&
src->state >= TCPS_FIN_WAIT_2) {
if (dst->state >= TCPS_FIN_WAIT_2 &&
src->state >= TCPS_FIN_WAIT_2 &&
(((th->th_flags & (TH_SYN|TH_ACK)) == TH_SYN) ||
((th->th_flags & (TH_SYN|TH_ACK|TH_RST)) == TH_ACK &&
pf_syncookie_check(pd) && pd->dir == PF_IN))) {
if (V_pf_status.debug >= PF_DEBUG_MISC) {
printf("pf: state reuse ");
pf_print_state(*state);
Expand Down
23 changes: 19 additions & 4 deletions sys/netpfil/pf/pf_syncookies.c
Original file line number Diff line number Diff line change
Expand Up @@ -300,8 +300,8 @@ pf_syncookie_send(struct mbuf *m, int off, struct pf_pdesc *pd)
1);
}

uint8_t
pf_syncookie_validate(struct pf_pdesc *pd)
bool
pf_syncookie_check(struct pf_pdesc *pd)
{
uint32_t hash, ack, seq;
union pf_syncookie cookie;
Expand All @@ -314,14 +314,29 @@ pf_syncookie_validate(struct pf_pdesc *pd)
cookie.cookie = (ack & 0xff) ^ (ack >> 24);

/* we don't know oddeven before setting the cookie (union) */
if (atomic_load_64(&V_pf_status.syncookies_inflight[cookie.flags.oddeven])
if (atomic_load_64(&V_pf_status.syncookies_inflight[cookie.flags.oddeven])
== 0)
return (0);
return (0);

hash = pf_syncookie_mac(pd, cookie, seq);
if ((ack & ~0xff) != (hash & ~0xff))
return (false);

return (true);
}

uint8_t
pf_syncookie_validate(struct pf_pdesc *pd)
{
uint32_t ack;
union pf_syncookie cookie;

if (! pf_syncookie_check(pd))
return (0);

ack = ntohl(pd->hdr.tcp.th_ack) - 1;
cookie.cookie = (ack & 0xff) ^ (ack >> 24);

counter_u64_add(V_pf_status.lcounters[KLCNT_SYNCOOKIES_VALID], 1);
atomic_add_64(&V_pf_status.syncookies_inflight[cookie.flags.oddeven], -1);

Expand Down

0 comments on commit 0b02f48

Please sign in to comment.