Thanks for help on this issue! ---------- Forwarded message ---------- From: Stuart Henderson <[email protected]> Date: Wed, Jun 22, 2016 at 2:21 AM Subject: Re: tcp state transition in sloppy mode To: Jingmin Zhou <[email protected]>
Hi, Nice analysis. The PF mailing list is not very widely used any more, so you might not get a useful response here - it might be a good idea to post this to [email protected] to reach a wider audience. Best regards Stuart On 2016/06/21 14:15, Jingmin Zhou wrote: > Hi, > > Recently we ran into some issues with pf sloppy mode with regards to > some long standing TCP connections. While reading pf code, we feel > puzzled by the way pf handles TCP packets in sloppy mode. Here are > some of our analysis. > > Background: a TCP connection were established between two hosts (A > and B) before pf is enabled in sloppy mode. The connection is mostly > idle with periodic keep alive packets, i.e., PUSH-ACK and ACK packets > every few minutes. An pass rule matches this connection and allows > packet to pass through. > > When a PUSH-ACK packet of the connection was observed from host A->B, > the pass rule lets pf.c:pf_create_state function to create a new > state for the connection. In the state, s->src.state = TCPS_SYN_SENT, > and s->dst.state = TCPS_CLOSED (line 3563 and 3564 in the code below). > > > 3488 static __inline int > > 3489 pf_create_state(struct pf_pdesc *pd, struct pf_rule *r, struct > pf_rule *a, > > 3490 struct pf_rule *nr, struct pf_state_key **skw, struct > pf_state_key **sks, > > 3491 int *rewrite, struct pf_state **sm, int tag, struct pf_rule_ > slist *rules, > > 3492 struct pf_rule_actions *act, struct pf_src_node *sns > [PF_SN_MAX]) > > 3493 { > > ... > > > 3532 switch (pd->proto) { > > 3533 case IPPROTO_TCP: > > ... > > 3561 s->dst.seqhi = 1; > > 3562 s->dst.max_win = 1; > > 3563 s->src.state = TCPS_SYN_SENT; > > 3564 s->dst.state = TCPS_CLOSED; > > 3565 s->timeout = PFTM_TCP_FIRST_PACKET; > > 3566 break; > > > The second packet is an ACK packet from B to A. It eventually makes > pf enter into pf.c:pf_test_state function. A simplified code flow > is shown as below: > > > 4320 int > > 4321 pf_test_state(struct pf_pdesc *pd, struct pf_state **state, u > _short *reason) > > 4322 { > > ... > > 4338 STATE_LOOKUP(pd->kif, &key, pd->dir, *state, pd->m); > > 4339 > > 4340 if (pd->dir == (*state)->direction) { > > 4341 src = &(*state)->src; > > 4342 dst = &(*state)->dst; > > 4343 } else { > > 4344 src = &(*state)->dst; > > 4345 dst = &(*state)->src; > > 4346 } > > 4347 > > 4348 switch (pd->virtual_proto) { > > 4349 case IPPROTO_TCP: > > ... > > 4369 if ((*state)->state_flags & PFSTATE_SLOPPY) { > > 4370 if (pf_tcp_track_sloppy(pd, src, dst, state, reason) = > = > > 4371 PF_DROP) > > 4372 return (PF_DROP); > > 4373 } else { > > ... > > 4465 } > > > Please notice that src and dst are taken from state entry and are > swapped at line 4344 and 4345 before being passed into function > pf_tcp_track_sloppy. So the src has a state TCPS_CLOSED and dst > has a state TCPS_SYN_SENT in function pf_tcp_track_sloppy. > > Below, a simplified pf_tcp_track_sloppy is shown. As we can see, > the ACK packet from B to A will match condition at line 4168. So > the state now changes to ESTABLISHED (A->B.src) and CLOSED (A->B. > dst). > > > 4155 int > > 4156 pf_tcp_track_sloppy(struct pf_pdesc *pd, struct pf_state_peer > *src, > > 4157 struct pf_state_peer *dst, struct pf_state **state, u_short > *reason) > > 4158 { > > ... > > 4167 if (th->th_flags & TH_ACK) { > > 4168 if (dst->state == TCPS_SYN_SENT) { > > 4169 dst->state = TCPS_ESTABLISHED; > > 4170 if (src->state == TCPS_ESTABLISHED && > > 4171 !SLIST_EMPTY(&(*state)->src_nodes) && > > 4172 pf_src_connlimit(state)) { > > 4173 REASON_SET(reason, PFRES_SRCLIMIT); > > 4174 return (PF_DROP); > > 4175 } > > 4176 } else if (dst->state == TCPS_CLOSING) { > > 4177 dst->state = TCPS_FIN_WAIT_2; > > 4178 } else if (src->state == TCPS_SYN_SENT && > > 4179 dst->state < TCPS_SYN_SENT) { > > 4180 /* > > 4181 * Handle a special sloppy case where we only see one > > 4182 * half of the connection. If there is a ACK after > > 4183 * the initial SYN without ever seeing a packet from > > 4184 * the destination, set the connection to established. > > 4185 */ > > 4186 dst->state = src->state = TCPS_ESTABLISHED; > > 4187 if (!SLIST_EMPTY(&(*state)->src_nodes) && > > 4188 pf_src_connlimit(state)) { > > 4189 REASON_SET(reason, PFRES_SRCLIMIT); > > 4190 return (PF_DROP); > > 4191 } > > 4192 } else if (src->state == TCPS_CLOSING && > > 4193 dst->state == TCPS_ESTABLISHED && > > 4194 dst->seqlo == 0) { > > 4195 /* > > 4196 * Handle the closing of half connections where we > > 4197 * don't see the full bidirectional FIN/ACK+ACK > > 4198 * handshake. > > 4199 */ > > 4200 dst->state = TCPS_CLOSING; > > 4201 } > > 4202 } > > .. > > 4206 /* update expire time */ > > 4207 (*state)->expire = time_uptime; > > 4208 if (src->state >= TCPS_FIN_WAIT_2 && > > 4209 dst->state >= TCPS_FIN_WAIT_2) > > 4210 (*state)->timeout = PFTM_TCP_CLOSED; > > 4211 else if (src->state >= TCPS_CLOSING && > > 4212 dst->state >= TCPS_CLOSING) > > 4213 (*state)->timeout = PFTM_TCP_FIN_WAIT; > > 4214 else if (src->state < TCPS_ESTABLISHED || > > 4215 dst->state < TCPS_ESTABLISHED) > > 4216 (*state)->timeout = PFTM_TCP_OPENING; > > 4217 else if (src->state >= TCPS_CLOSING || > > 4218 dst->state >= TCPS_CLOSING) > > 4219 (*state)->timeout = PFTM_TCP_CLOSING; > > 4220 else > > 4221 (*state)->timeout = PFTM_TCP_ESTABLISHED; > > 4222 > > 4223 return (PF_PASS); > > 4224 } > > > There is a special condition at line 4178 and 4179, which only handles > two consecutive ACK packets from A to B. But it does not handle our > case > where we got A->B and then B->A ACK packets. > > Then the third ACK packet was observed from A to B. In function > tcp_track_sloppy, src->state is ESTABLISHED and dst->state is CLOSED. > The new packet does not match any ACK checks and the state of the > connection stays the same. > > The consequence? The state stays in ESTABLISHED (A->B.src) and CLOSED > (A->B.dst). The timeout is thus set to PFTM_TCP_OPENING (30 seconds) > by line 4216. So the state quickly expired and was removed from state > table. The next keep alive packet sequence triggers the same actions > again. > > It may not be a big issue but we feel a bit puzzled by the different > results of packet sequence. That is, two consecutive one way packets > will make the state into ESTABLISHED state in both directions, but > two way packets will make state into ESTABLISHED and CLOSED state > and hence expire quickly. Shall we handle the second case in a similar > way as the first case? > > Thank you very much for your time! > > Best wishes, > Jingmin > > >
