All of lore.kernel.org
 help / color / mirror / Atom feed
From: Patrick McHardy <kaber@trash.net>
To: dccp@vger.kernel.org
Subject: Re: DCCP conntrack/NAT
Date: Tue, 08 Apr 2008 14:12:14 +0000	[thread overview]
Message-ID: <47FB7D3E.4060703@trash.net> (raw)
In-Reply-To: <47F64C0D.5080700@trash.net>

[-- Attachment #1: Type: text/plain, Size: 1288 bytes --]

Patrick McHardy wrote:
> Gerrit Renker wrote:
>>     State Transitions in the original direction
>>     ===========================================
>>
>>  * DCCP-Request:
>>    - in state Respond (sRS -> sRS), the Request is illegal (Respond 
>> is server state)
>
>>    - also, the CLOSEREQ state transition (sCR -> sIG) is illegal: 
>> Requests are sent      by clients only, and CLOSEREQ can only be 
>> entered by servers
>
> We track both sides, so we must also define which client packets
> are valid in which server state. This particular one is part of
> the unfinished resync feature. The firewall might be out of sync
> with both endpoints. If connection pickup is enabled it should
> let packets that might establish a new connection pass and resync
> when the other side responds with a valid Response. I need to
> think about this a bit more, but I've marked it with FIXME for
> now :)

I've added this patch on top to handle the out-of-sync case by
letting Requests pass in (almost) any state and resyncing when
seeing a valid Response.

Last TODO before the DCCP_LISTEN support is to fix connection
pickup for established connections. Since it doesn't see the
initial Request/Response it doesn't know which side has which
role and also can't properly pick an inital state.

[-- Attachment #2: x --]
[-- Type: text/plain, Size: 5224 bytes --]

diff --git a/include/linux/netfilter/nf_conntrack_dccp.h b/include/linux/netfilter/nf_conntrack_dccp.h
index 41ffdf8..40dcc82 100644
--- a/include/linux/netfilter/nf_conntrack_dccp.h
+++ b/include/linux/netfilter/nf_conntrack_dccp.h
@@ -30,6 +30,8 @@ enum ct_dccp_roles {
 struct nf_ct_dccp {
 	u_int8_t	role[IP_CT_DIR_MAX];
 	u_int8_t	state;
+	u_int8_t	last_pkt;
+	u_int8_t	last_dir;
 	u_int64_t	handshake_seq;
 };
 
diff --git a/net/netfilter/nf_conntrack_proto_dccp.c b/net/netfilter/nf_conntrack_proto_dccp.c
index a89113d..96bd70c 100644
--- a/net/netfilter/nf_conntrack_proto_dccp.c
+++ b/net/netfilter/nf_conntrack_proto_dccp.c
@@ -147,10 +147,10 @@ dccp_state_table[CT_DCCP_ROLE_MAX + 1][DCCP_PKT_SYNCACK + 1][CT_DCCP_MAX + 1] =
 		 * sRQ -> sRQ		Retransmitted Request or reincarnation
 		 * sRS -> sRS		Retransmitted Request (apparently Response
 		 * 			got lost after we saw it) or reincarnation
-		 * sPO -> sIG		Request during PARTOPEN state, server will ignore it
-		 * sOP -> sIG		Request during OPEN state: server will ignore it
-		 * sCR -> sIG		FIXME MUST respond with Close to CloseReq (8.3.)
-		 * sCG -> sIG
+		 * sPO -> sIG		Ignore, conntrack might be out of sync
+		 * sOP -> sIG		Ignore, conntrack might be out of sync
+		 * sCR -> sIG		Ignore, conntrack might be out of sync
+		 * sCG -> sIG		Ignore, conntrack might be out of sync
 		 * sTW -> sRQ		Reincarnation
 		 *
 		 *	sNO, sRQ, sRS, sPO. sOP, sCR, sCG, sTW, */
@@ -158,10 +158,18 @@ dccp_state_table[CT_DCCP_ROLE_MAX + 1][DCCP_PKT_SYNCACK + 1][CT_DCCP_MAX + 1] =
 		},
 		[DCCP_PKT_RESPONSE] = {
 		/*
-		 * A Response from the client is always invalid.
+		 * sNO -> sIV		Invalid
+		 * sRQ -> sIG		Ignore, might be response to ignored Request
+		 * sRS -> sIG		Ignore, might be response to ignored Request
+		 * sPO -> sIG		Ignore, might be response to ignored Request
+		 * sOP -> sIG		Ignore, might be response to ignored Request
+		 * sCR -> sIG		Ignore, might be response to ignored Request
+		 * sCG -> sIG		Ignore, might be response to ignored Request
+		 * sTW -> sIV		Invalid, reincarnation in reverse direction
+		 *			goes through sRQ
 		 *
 		 *	sNO, sRQ, sRS, sPO, sOP, sCR, sCG, sTW */
-			sIV, sIV, sIV, sIV, sIV, sIV, sIV, sIV,
+			sIV, sIG, sIG, sIG, sIG, sIG, sIG, sIV,
 		},
 		[DCCP_PKT_ACK] = {
 		/*
@@ -258,20 +266,17 @@ dccp_state_table[CT_DCCP_ROLE_MAX + 1][DCCP_PKT_SYNCACK + 1][CT_DCCP_MAX + 1] =
 	[CT_DCCP_ROLE_SERVER] = {
 		[DCCP_PKT_REQUEST] = {
 		/*
-		 * A Request from the server is only valid for reopening a
-		 * connection in TIMEWAIT state.
-		 *
-		 * sNO -> sIV
-		 * sRQ -> sIV
-		 * sRS -> sIV
-		 * sPO -> sIV
-		 * sOP -> sIV
-		 * sCR -> sIV
-		 * sCG -> sIV
+		 * sNO -> sIV		Invalid
+		 * sRQ -> sIG		Ignore, conntrack might be out of sync
+		 * sRS -> sIG		Ignore, conntrack might be out of sync
+		 * sPO -> sIG		Ignore, conntrack might be out of sync
+		 * sOP -> sIG		Ignore, conntrack might be out of sync
+		 * sCR -> sIG		Ignore, conntrack might be out of sync
+		 * sCG -> sIG		Ignore, conntrack might be out of sync
 		 * sTW -> sRQ		Reincarnation, must reverse roles
 		 *
 		 *	sNO, sRQ, sRS, sPO, sOP, sCR, sCG, sTW */
-			sIV, sIV, sIV, sIV, sIV, sIV, sIV, sRQ
+			sIV, sIG, sIG, sIG, sIG, sIG, sIG, sRQ
 		},
 		[DCCP_PKT_RESPONSE] = {
 		/*
@@ -279,13 +284,13 @@ dccp_state_table[CT_DCCP_ROLE_MAX + 1][DCCP_PKT_SYNCACK + 1][CT_DCCP_MAX + 1] =
 		 * sRQ -> sRS		Response to clients Request
 		 * sRS -> sRS		Retransmitted Response (8.1.3. SHOULD NOT)
 		 * sPO -> sIG		Response to an ignored Request or late retransmit
-		 * sOP -> sIG		Invalid
-		 * sCR -> sIG		Invalid
-		 * sCG -> sIG		Invalid
-		 * sTW -> sIG		Invalid
+		 * sOP -> sIG		Ignore, might be response to ignored Request
+		 * sCR -> sIG		Ignore, might be response to ignored Request
+		 * sCG -> sIG		Ignore, might be response to ignored Request
+		 * sTW -> sIV		Invalid, Request from client in sTW moves to sRQ
 		 *
 		 *	sNO, sRQ, sRS, sPO, sOP, sCR, sCG, sTW */
-			sIV, sRS, sRS, sIG, sIG, sIG, sIG, sIG
+			sIV, sRS, sRS, sIG, sIG, sIG, sIG, sIV
 		},
 		[DCCP_PKT_ACK] = {
 		/*
@@ -503,6 +508,20 @@ static int dccp_packet(struct nf_conn *ct, const struct sk_buff *skb,
 			set_bit(IPS_ASSURED_BIT, &ct->status);
 		break;
 	case CT_DCCP_IGNORE:
+		/*
+		 * Connection tracking might be out of sync, so we ignore
+		 * packets that might establish a new connection and resync
+		 * if the server responds with a valid Response.
+		 */
+		if (ct->proto.dccp.last_dir == !dir &&
+		    ct->proto.dccp.last_pkt == DCCP_PKT_REQUEST &&
+		    type == DCCP_PKT_RESPONSE) {
+			ct->proto.dccp.role[!dir] = CT_DCCP_ROLE_CLIENT;
+			ct->proto.dccp.role[dir] = CT_DCCP_ROLE_SERVER;
+			ct->proto.dccp.handshake_seq = dccp_hdr_seq(dh);
+			new_state = CT_DCCP_RESPOND;
+			break;
+		}
 		write_unlock_bh(&dccp_lock);
 		if (LOG_INVALID(IPPROTO_DCCP))
 			nf_log_packet(pf, 0, skb, NULL, NULL, NULL,
@@ -516,6 +535,8 @@ static int dccp_packet(struct nf_conn *ct, const struct sk_buff *skb,
 		return -NF_ACCEPT;
 	}
 
+	ct->proto.dccp.last_dir = dir;
+	ct->proto.dccp.last_pkt = type;
 	ct->proto.dccp.state = new_state;
 	write_unlock_bh(&dccp_lock);
 	nf_ct_refresh_acct(ct, ctinfo, skb, dccp_timeout[new_state]);

WARNING: multiple messages have this Message-ID (diff)
From: Patrick McHardy <kaber@trash.net>
To: Gerrit Renker <gerrit@erg.abdn.ac.uk>,
	dccp@vger.kernel.org,
	Netfilter Development Mailinglist
	<netfilter-devel@vger.kernel.org>
Subject: Re: DCCP conntrack/NAT
Date: Tue, 08 Apr 2008 16:12:14 +0200	[thread overview]
Message-ID: <47FB7D3E.4060703@trash.net> (raw)
In-Reply-To: <47FAA40D.7010905@trash.net>

[-- Attachment #1: Type: text/plain, Size: 1288 bytes --]

Patrick McHardy wrote:
> Gerrit Renker wrote:
>>     State Transitions in the original direction
>>     ===========================================
>>
>>  * DCCP-Request:
>>    - in state Respond (sRS -> sRS), the Request is illegal (Respond 
>> is server state)
>
>>    - also, the CLOSEREQ state transition (sCR -> sIG) is illegal: 
>> Requests are sent      by clients only, and CLOSEREQ can only be 
>> entered by servers
>
> We track both sides, so we must also define which client packets
> are valid in which server state. This particular one is part of
> the unfinished resync feature. The firewall might be out of sync
> with both endpoints. If connection pickup is enabled it should
> let packets that might establish a new connection pass and resync
> when the other side responds with a valid Response. I need to
> think about this a bit more, but I've marked it with FIXME for
> now :)

I've added this patch on top to handle the out-of-sync case by
letting Requests pass in (almost) any state and resyncing when
seeing a valid Response.

Last TODO before the DCCP_LISTEN support is to fix connection
pickup for established connections. Since it doesn't see the
initial Request/Response it doesn't know which side has which
role and also can't properly pick an inital state.

[-- Attachment #2: x --]
[-- Type: text/plain, Size: 5224 bytes --]

diff --git a/include/linux/netfilter/nf_conntrack_dccp.h b/include/linux/netfilter/nf_conntrack_dccp.h
index 41ffdf8..40dcc82 100644
--- a/include/linux/netfilter/nf_conntrack_dccp.h
+++ b/include/linux/netfilter/nf_conntrack_dccp.h
@@ -30,6 +30,8 @@ enum ct_dccp_roles {
 struct nf_ct_dccp {
 	u_int8_t	role[IP_CT_DIR_MAX];
 	u_int8_t	state;
+	u_int8_t	last_pkt;
+	u_int8_t	last_dir;
 	u_int64_t	handshake_seq;
 };
 
diff --git a/net/netfilter/nf_conntrack_proto_dccp.c b/net/netfilter/nf_conntrack_proto_dccp.c
index a89113d..96bd70c 100644
--- a/net/netfilter/nf_conntrack_proto_dccp.c
+++ b/net/netfilter/nf_conntrack_proto_dccp.c
@@ -147,10 +147,10 @@ dccp_state_table[CT_DCCP_ROLE_MAX + 1][DCCP_PKT_SYNCACK + 1][CT_DCCP_MAX + 1] =
 		 * sRQ -> sRQ		Retransmitted Request or reincarnation
 		 * sRS -> sRS		Retransmitted Request (apparently Response
 		 * 			got lost after we saw it) or reincarnation
-		 * sPO -> sIG		Request during PARTOPEN state, server will ignore it
-		 * sOP -> sIG		Request during OPEN state: server will ignore it
-		 * sCR -> sIG		FIXME MUST respond with Close to CloseReq (8.3.)
-		 * sCG -> sIG
+		 * sPO -> sIG		Ignore, conntrack might be out of sync
+		 * sOP -> sIG		Ignore, conntrack might be out of sync
+		 * sCR -> sIG		Ignore, conntrack might be out of sync
+		 * sCG -> sIG		Ignore, conntrack might be out of sync
 		 * sTW -> sRQ		Reincarnation
 		 *
 		 *	sNO, sRQ, sRS, sPO. sOP, sCR, sCG, sTW, */
@@ -158,10 +158,18 @@ dccp_state_table[CT_DCCP_ROLE_MAX + 1][DCCP_PKT_SYNCACK + 1][CT_DCCP_MAX + 1] =
 		},
 		[DCCP_PKT_RESPONSE] = {
 		/*
-		 * A Response from the client is always invalid.
+		 * sNO -> sIV		Invalid
+		 * sRQ -> sIG		Ignore, might be response to ignored Request
+		 * sRS -> sIG		Ignore, might be response to ignored Request
+		 * sPO -> sIG		Ignore, might be response to ignored Request
+		 * sOP -> sIG		Ignore, might be response to ignored Request
+		 * sCR -> sIG		Ignore, might be response to ignored Request
+		 * sCG -> sIG		Ignore, might be response to ignored Request
+		 * sTW -> sIV		Invalid, reincarnation in reverse direction
+		 *			goes through sRQ
 		 *
 		 *	sNO, sRQ, sRS, sPO, sOP, sCR, sCG, sTW */
-			sIV, sIV, sIV, sIV, sIV, sIV, sIV, sIV,
+			sIV, sIG, sIG, sIG, sIG, sIG, sIG, sIV,
 		},
 		[DCCP_PKT_ACK] = {
 		/*
@@ -258,20 +266,17 @@ dccp_state_table[CT_DCCP_ROLE_MAX + 1][DCCP_PKT_SYNCACK + 1][CT_DCCP_MAX + 1] =
 	[CT_DCCP_ROLE_SERVER] = {
 		[DCCP_PKT_REQUEST] = {
 		/*
-		 * A Request from the server is only valid for reopening a
-		 * connection in TIMEWAIT state.
-		 *
-		 * sNO -> sIV
-		 * sRQ -> sIV
-		 * sRS -> sIV
-		 * sPO -> sIV
-		 * sOP -> sIV
-		 * sCR -> sIV
-		 * sCG -> sIV
+		 * sNO -> sIV		Invalid
+		 * sRQ -> sIG		Ignore, conntrack might be out of sync
+		 * sRS -> sIG		Ignore, conntrack might be out of sync
+		 * sPO -> sIG		Ignore, conntrack might be out of sync
+		 * sOP -> sIG		Ignore, conntrack might be out of sync
+		 * sCR -> sIG		Ignore, conntrack might be out of sync
+		 * sCG -> sIG		Ignore, conntrack might be out of sync
 		 * sTW -> sRQ		Reincarnation, must reverse roles
 		 *
 		 *	sNO, sRQ, sRS, sPO, sOP, sCR, sCG, sTW */
-			sIV, sIV, sIV, sIV, sIV, sIV, sIV, sRQ
+			sIV, sIG, sIG, sIG, sIG, sIG, sIG, sRQ
 		},
 		[DCCP_PKT_RESPONSE] = {
 		/*
@@ -279,13 +284,13 @@ dccp_state_table[CT_DCCP_ROLE_MAX + 1][DCCP_PKT_SYNCACK + 1][CT_DCCP_MAX + 1] =
 		 * sRQ -> sRS		Response to clients Request
 		 * sRS -> sRS		Retransmitted Response (8.1.3. SHOULD NOT)
 		 * sPO -> sIG		Response to an ignored Request or late retransmit
-		 * sOP -> sIG		Invalid
-		 * sCR -> sIG		Invalid
-		 * sCG -> sIG		Invalid
-		 * sTW -> sIG		Invalid
+		 * sOP -> sIG		Ignore, might be response to ignored Request
+		 * sCR -> sIG		Ignore, might be response to ignored Request
+		 * sCG -> sIG		Ignore, might be response to ignored Request
+		 * sTW -> sIV		Invalid, Request from client in sTW moves to sRQ
 		 *
 		 *	sNO, sRQ, sRS, sPO, sOP, sCR, sCG, sTW */
-			sIV, sRS, sRS, sIG, sIG, sIG, sIG, sIG
+			sIV, sRS, sRS, sIG, sIG, sIG, sIG, sIV
 		},
 		[DCCP_PKT_ACK] = {
 		/*
@@ -503,6 +508,20 @@ static int dccp_packet(struct nf_conn *ct, const struct sk_buff *skb,
 			set_bit(IPS_ASSURED_BIT, &ct->status);
 		break;
 	case CT_DCCP_IGNORE:
+		/*
+		 * Connection tracking might be out of sync, so we ignore
+		 * packets that might establish a new connection and resync
+		 * if the server responds with a valid Response.
+		 */
+		if (ct->proto.dccp.last_dir == !dir &&
+		    ct->proto.dccp.last_pkt == DCCP_PKT_REQUEST &&
+		    type == DCCP_PKT_RESPONSE) {
+			ct->proto.dccp.role[!dir] = CT_DCCP_ROLE_CLIENT;
+			ct->proto.dccp.role[dir] = CT_DCCP_ROLE_SERVER;
+			ct->proto.dccp.handshake_seq = dccp_hdr_seq(dh);
+			new_state = CT_DCCP_RESPOND;
+			break;
+		}
 		write_unlock_bh(&dccp_lock);
 		if (LOG_INVALID(IPPROTO_DCCP))
 			nf_log_packet(pf, 0, skb, NULL, NULL, NULL,
@@ -516,6 +535,8 @@ static int dccp_packet(struct nf_conn *ct, const struct sk_buff *skb,
 		return -NF_ACCEPT;
 	}
 
+	ct->proto.dccp.last_dir = dir;
+	ct->proto.dccp.last_pkt = type;
 	ct->proto.dccp.state = new_state;
 	write_unlock_bh(&dccp_lock);
 	nf_ct_refresh_acct(ct, ctinfo, skb, dccp_timeout[new_state]);

  parent reply	other threads:[~2008-04-08 14:12 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-04-04 15:41 DCCP conntrack/NAT Patrick McHardy
2008-04-04 15:41 ` Patrick McHardy
2008-04-04 19:59 ` Jan Engelhardt
2008-04-04 19:59   ` Jan Engelhardt
2008-04-05 10:09 ` Pablo Neira Ayuso
2008-04-05 10:09   ` Pablo Neira Ayuso
2008-04-05 10:12 ` Pablo Neira Ayuso
2008-04-05 10:12   ` Pablo Neira Ayuso
2008-04-06  0:28 ` Patrick McHardy
2008-04-06  0:28   ` Patrick McHardy
2008-04-07 21:50 ` Gerrit Renker
2008-04-07 21:50   ` Gerrit Renker
2008-04-07 22:45 ` Patrick McHardy
2008-04-07 22:45   ` Patrick McHardy
2008-04-08  9:27 ` Gerrit Renker
2008-04-08  9:27   ` Gerrit Renker
2008-04-08 10:30 ` Patrick McHardy
2008-04-08 10:30   ` Patrick McHardy
2008-04-08 10:33 ` Patrick McHardy
2008-04-08 10:33   ` Patrick McHardy
2008-04-08 11:18 ` Patrick McHardy
2008-04-08 11:18   ` Patrick McHardy
2008-04-08 13:38 ` Gerrit Renker
2008-04-08 13:38   ` Gerrit Renker
2008-04-08 14:12 ` Patrick McHardy [this message]
2008-04-08 14:12   ` Patrick McHardy
2008-04-08 14:26 ` Patrick McHardy
2008-04-08 14:26   ` Patrick McHardy
2008-04-08 16:21 ` Patrick McHardy
2008-04-08 16:21   ` Patrick McHardy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=47FB7D3E.4060703@trash.net \
    --to=kaber@trash.net \
    --cc=dccp@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.