[nf-next,RFC] netfilter: nf_conntrack_tcp: reset entry only from CLOSE and TIME_WAIT states

Before this patch, if TCP state is < TIME_WAIT, we skip this logic.
Along time, we got new states over the TIME_WAIT value, such as
SYN_SENT2, RETRANS and UNACK, that can trigger this conntrack entry
reset, however this check was never updated.

My understanding is that we should only exercise the conntrack reset
codepath if the existing packet is triggering a transition to SYN_SENT
state while we were in TIME_WAIT or CLOSE states.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
While reviewing TCP tracking code, I noticed this. This is just narrowing
down this codepath so it only works with TIME_WAIT and CLOSE states.

 net/netfilter/nf_conntrack_proto_tcp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Hi Pablo,

On Tue, 20 Mar 2018, Pablo Neira Ayuso wrote:

> Before this patch, if TCP state is < TIME_WAIT, we skip this logic. 
> Along time, we got new states over the TIME_WAIT value, such as 
> SYN_SENT2, RETRANS and UNACK, that can trigger this conntrack entry 
> reset, however this check was never updated.
> 
> My understanding is that we should only exercise the conntrack reset 
> codepath if the existing packet is triggering a transition to SYN_SENT 
> state while we were in TIME_WAIT or CLOSE states.

RETRANS and UNACK are not TCP states, just mnemonics to help assign 
special timeout values.

SYN_SENT2 is a TCP state which is the second one in the case of 
simultaneous open but from there there's no way back to reach SYN_SENT 
again.

So I think the original condition is all right. However, if you believe 
the patch makes it more clean and less error-prone, then just apply it.

Best regards,
Jozsef

> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> ---
> While reviewing TCP tracking code, I noticed this. This is just narrowing
> down this codepath so it only works with TIME_WAIT and CLOSE states.
> 
>  net/netfilter/nf_conntrack_proto_tcp.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/net/netfilter/nf_conntrack_proto_tcp.c b/net/netfilter/nf_conntrack_proto_tcp.c
> index e97cdc1cf98c..c765d83db574 100644
> --- a/net/netfilter/nf_conntrack_proto_tcp.c
> +++ b/net/netfilter/nf_conntrack_proto_tcp.c
> @@ -827,7 +827,8 @@ static int tcp_packet(struct nf_conn *ct,
>  
>  	switch (new_state) {
>  	case TCP_CONNTRACK_SYN_SENT:
> -		if (old_state < TCP_CONNTRACK_TIME_WAIT)
> +		if (old_state != TCP_CONNTRACK_TIME_WAIT &&
> +		    old_state != TCP_CONNTRACK_CLOSE)
>  			break;
>  		/* RFC 1122: "When a connection is closed actively,
>  		 * it MUST linger in TIME-WAIT state for a time 2xMSL
> -- 
> 2.11.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-
E-mail  : kadlec@blackhole.kfki.hu, kadlecsik.jozsef@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
          H-1525 Budapest 114, POB. 49, Hungary
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html