Skip to contentSkip to conversation

Conntrack TW->SS in Reply direction

PATCH Applied Marcelo Ricardo Leitner 3 participants 4 messages Sep 17, 2014 – Oct 22, 2014

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

Hi,

Any reason on why we don't allow TW->SS state change in reply direction? It is currently marked as (since ever it seems) in a comment:
* sTW -> sIV Reopened connection, but server may not do it.

That is true but just if the server role is never swapped between hosts and I don't see it violating any spec by allowing it..

Thing is, there is this application that needs a connection as a callback on a specified port range. This application is used between 2 servers, using conntrack and only allowing in new connections and its (related) packets. There will be a moment that the port that once was server for the callback, will now originate a callback to the other server.

As Linux stack uses TIME_WAIT timeout as 1min and conntrack as 2mins, there will be a moment on which the port selection allows using a port that, for conntrack, is still not allowed...

Reducing conntrack's TIME_WAIT timeout is more complicated because it may restrain the compatibility with other implementations, but allowing TW->SS imposes no harm?

Thanks,
Marcelo

#1 lore raw

PATCH 1/1 nf_conntrack_proto_tcp: allow server to become a client in TW handling

When a port that was used to listen for inbound connections gets closed and reused for outgoing connections (like rsh ends up doing for stderr flow), current we may reject the SYN/ACK packet for the new connection because tcp_conntracks states forbirds a port to become a client while there is still a TIME_WAIT entry in there for it.

As TCP may expire the TIME_WAIT socket in 60s and conntrack's timeout for it is 120s, there is a ~60s window that the application can end up opening a port that conntrack will end up blocking.

This patch fixes this by simply allowing such state transition: if we see a SYN, in TIME_WAIT state, on REPLY direction, move it to sSS. Note that the rest of the code already handles this situation, more specificly in tcp_packet(), first switch clause.

---
 net/netfilter/nf_conntrack_proto_tcp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
net/netfilter/nf_conntrack_proto_tcp.c +2 -2
index 44d1ea32570a07338dc39f34624bd823b6f76916..d87b6423ffb21e0f8f9b6ef25ef51c1cb5f54ad6 100644
@@ -213,7 +213,7 @@ static const u8 tcp_conntracks[2][6][TCP_CONNTRACK_MAX] = {
 	{
 /* REPLY */
 /* 	     sNO, sSS, sSR, sES, sFW, sCW, sLA, sTW, sCL, sS2	*/
-/*syn*/	   { sIV, sS2, sIV, sIV, sIV, sIV, sIV, sIV, sIV, sS2 },
+/*syn*/	   { sIV, sS2, sIV, sIV, sIV, sIV, sIV, sSS, sIV, sS2 },
 /*
  *	sNO -> sIV	Never reached.
  *	sSS -> sS2	Simultaneous open
@@ -223,7 +223,7 @@ static const u8 tcp_conntracks[2][6][TCP_CONNTRACK_MAX] = {
  *	sFW -> sIV
  *	sCW -> sIV
  *	sLA -> sIV
- *	sTW -> sIV	Reopened connection, but server may not do it.
+ *	sTW -> sSS	Reopened connection, but server may have switched role
  *	sCL -> sIV
  */
 /* 	     sNO, sSS, sSR, sES, sFW, sCW, sLA, sTW, sCL, sS2	*/
Signature · 1 line
1.9.3
Acked-by on v1 1/1
#2 lore raw

Re: [PATCH] nf_conntrack_proto_tcp: allow server to become a client in TW handling

Quoted 6 lines · Marcelo Ricardo Leitner

When a port that was used to listen for inbound connections gets closed and reused for outgoing connections (like rsh ends up doing for stderr flow), current we may reject the SYN/ACK packet for the new connection because tcp_conntracks states forbirds a port to become a client while there is still a TIME_WAIT entry in there for it.

Quoted 8 lines

As TCP may expire the TIME_WAIT socket in 60s and conntrack's timeout for it is 120s, there is a ~60s window that the application can end up opening a port that conntrack will end up blocking.

This patch fixes this by simply allowing such state transition: if we see a SYN, in TIME_WAIT state, on REPLY direction, move it to sSS. Note that the rest of the code already handles this situation, more specificly in tcp_packet(), first switch clause.

In those code branch if there was a valid FIN in either direction, we destroy the old connection and a new will be created. That way the rules about NEW connections will be applied, so the policies are not bypassed. Otherwise we just ignore the SYN packet, so if it's invalid, we'll catch the RST from the other side and destroy the conntrack entry. The event flow looks OK to me.

Quoted 1 line

Best regards,
Jozsef

Quoted 34 lines
---
 net/netfilter/nf_conntrack_proto_tcp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
net/netfilter/nf_conntrack_proto_tcp.c +2 -2
index 44d1ea32570a07338dc39f34624bd823b6f76916..d87b6423ffb21e0f8f9b6ef25ef51c1cb5f54ad6 100644
@@ -213,7 +213,7 @@ static const u8 tcp_conntracks[2][6][TCP_CONNTRACK_MAX] = {
 	{
 /* REPLY */
 /* 	     sNO, sSS, sSR, sES, sFW, sCW, sLA, sTW, sCL, sS2	*/
-/*syn*/	   { sIV, sS2, sIV, sIV, sIV, sIV, sIV, sIV, sIV, sS2 },
+/*syn*/	   { sIV, sS2, sIV, sIV, sIV, sIV, sIV, sSS, sIV, sS2 },
 /*
  *	sNO -> sIV	Never reached.
  *	sSS -> sS2	Simultaneous open
@@ -223,7 +223,7 @@ static const u8 tcp_conntracks[2][6][TCP_CONNTRACK_MAX] = {
  *	sFW -> sIV
  *	sCW -> sIV
  *	sLA -> sIV
- *	sTW -> sIV	Reopened connection, but server may not do it.
+ *	sTW -> sSS	Reopened connection, but server may have switched role
  *	sCL -> sIV
  */
 /* 	     sNO, sSS, sSR, sES, sFW, sCW, sLA, sTW, sCL, sS2	*/
Signature · 6 lines
1.9.3

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

-
E-mail : [email protected], [email protected] 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
maintainer Applied
#3 lore raw

Re: [PATCH] nf_conntrack_proto_tcp: allow server to become a client in TW handling

Quoted 24 lines · Jozsef Kadlecsik
Quoted 6 lines · Marcelo Ricardo Leitner

When a port that was used to listen for inbound connections gets closed and reused for outgoing connections (like rsh ends up doing for stderr flow), current we may reject the SYN/ACK packet for the new connection because tcp_conntracks states forbirds a port to become a client while there is still a TIME_WAIT entry in there for it.

Quoted 8 lines

As TCP may expire the TIME_WAIT socket in 60s and conntrack's timeout for it is 120s, there is a ~60s window that the application can end up opening a port that conntrack will end up blocking.

This patch fixes this by simply allowing such state transition: if we see a SYN, in TIME_WAIT state, on REPLY direction, move it to sSS. Note that the rest of the code already handles this situation, more specificly in tcp_packet(), first switch clause.

In those code branch if there was a valid FIN in either direction, we destroy the old connection and a new will be created. That way the rules about NEW connections will be applied, so the policies are not bypassed. Otherwise we just ignore the SYN packet, so if it's invalid, we'll catch the RST from the other side and destroy the conntrack entry. The event flow looks OK to me.

Quoted 3 lines
Quoted 1 line

Applied, thanks.