[ovs-dev] [PATCH] conntrack: invalid packet should not modify ct state
Zang MingJie
zealot0630 at gmail.com
Fri Sep 14 08:46:47 UTC 2018
>> > Did you notice this check ?
>> >
>> > if (src->state < CT_DPIF_TCPS_SYN_SENT) {
>> > /* First packet from this end. Set its state */
>>
>> Yes, this is exactly where we found the problem. If first reply packet
>> is invalid, it masses all following packets.
>
>
>
> Based on your description in the commit message:
> " Currently an invalid packet can change the seq number while the
connection is in SEQ_RECV state."
> we don't fall into this condition since SEQ_RECV >
CT_DPIF_TCPS_SYN_SENT, right ?
>
> If you did not mean "SEQ_RECV", maybe you meant something else ?
> What the value of "src->state" where you saw an issue ?
SYN_RECV is our server side tcp state, ct table track either side tcp state
separately, the problem happens in this situation:
client | ct.src ct.dst | server
packet: syn ->
state : SYN_SEND | SYN_SEND CLOSED | SYN_RECV
packet: <- malformed packet
state: SYN_SEND | SYN_SEND SYN_SEND | SYN_RECV <-updated to wrong state
packet: <- syn+ack <-invalid
malformed packet will set wrong seq_lo and seq_hi to the state, preventing
following packets pass ct.
>
>>
>>
>> >
>> >
>> >
>> >>
>> >>
>> >> Signed-off-by: Zang MingJie <zealot0630 at gmail.com>
>> >> ---
>> >> lib/conntrack-tcp.c | 10 ++++++++--
>> >> 1 file changed, 8 insertions(+), 2 deletions(-)
>> >>
>> >> diff --git a/lib/conntrack-tcp.c b/lib/conntrack-tcp.c
>> >> index 86d313d28..7443a3293 100644
>> >> --- a/lib/conntrack-tcp.c
>> >> +++ b/lib/conntrack-tcp.c
>> >> @@ -151,9 +151,9 @@ tcp_conn_update(struct conn *conn_, struct
conntrack_bucket *ctb,
>> >> struct conn_tcp *conn = conn_tcp_cast(conn_);
>> >> struct tcp_header *tcp = dp_packet_l4(pkt);
>> >> /* The peer that sent 'pkt' */
>> >> - struct tcp_peer *src = &conn->peer[reply ? 1 : 0];
>> >> + struct tcp_peer orig_src, *src = &conn->peer[reply ? 1 : 0];
>> >> /* The peer that should receive 'pkt' */
>> >> - struct tcp_peer *dst = &conn->peer[reply ? 0 : 1];
>> >> + struct tcp_peer orig_dst, *dst = &conn->peer[reply ? 0 : 1];
>> >> uint8_t sws = 0, dws = 0;
>> >> uint16_t tcp_flags = TCP_FLAGS(tcp->tcp_ctl);
>> >>
>> >> @@ -187,6 +187,10 @@ tcp_conn_update(struct conn *conn_, struct
conntrack_bucket *ctb,
>> >> dws = TCP_MAX_WSCALE;
>> >> }
>> >>
>> >> +
>> >> + orig_src = *src;
>> >> + orig_dst = *dst;
>> >> +
>> >> /*
>> >> * Sequence tracking algorithm from Guido van Rooij's paper:
>> >> * http://www.madison-gurkha.com/publications/tcp_filtering/
>> >> @@ -385,6 +389,8 @@ tcp_conn_update(struct conn *conn_, struct
conntrack_bucket *ctb,
>> >> src->state = dst->state = CT_DPIF_TCPS_TIME_WAIT;
>> >> }
>> >> } else {
>> >> + *src = orig_src;
>> >> + *dst = orig_dst;
>> >> return CT_UPDATE_INVALID;
>> >> }
>> >>
>> >> --
>> >> 2.19.0.rc1
>> >>
>> >> _______________________________________________
>> >> dev mailing list
>> >> dev at openvswitch.org
>> >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>> >
>> >
>
>
More information about the dev
mailing list