diff mbox

[nf-next] netfilter: xt_socket: add XT_SOCKET_NOWILDCARD flag

Message ID 1370300249.24311.190.camel@edumazet-glaptop
State Changes Requested
Delegated to: Pablo Neira
Headers show

Commit Message

Eric Dumazet June 3, 2013, 10:57 p.m. UTC 
From: Eric Dumazet <edumazet@google.com>

xt_socket module can be a nice replacement to conntrack module
in some cases (SYN filtering for example)

But it lacks the ability to match the 3rd packet of TCP
handshake (ACK coming from the client).

Add a XT_SOCKET_NOWILDCARD flag to disable the wildcard mechanism

iptables -I INPUT -p tcp --syn -j SYN_CHAIN
iptables -I INPUT -m socket -j ACCEPT


Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Patrick McHardy <kaber@trash.net>
Cc: Jesper Dangaard Brouer <brouer@redhat.com>
---
 include/uapi/linux/netfilter/xt_socket.h |    1 +
 net/netfilter/xt_socket.c                |   14 ++++++++++----
 2 files changed, 11 insertions(+), 4 deletions(-)



--
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

Comments

Jesper Dangaard Brouer June 4, 2013, 9:10 a.m. UTC | #1 
On Mon, 03 Jun 2013 15:57:29 -0700
Eric Dumazet <eric.dumazet@gmail.com> wrote:

> From: Eric Dumazet <edumazet@google.com>
> 
> xt_socket module can be a nice replacement to conntrack module
> in some cases (SYN filtering for example)
> 
> But it lacks the ability to match the 3rd packet of TCP
> handshake (ACK coming from the client).
> 
> Add a XT_SOCKET_NOWILDCARD flag to disable the wildcard mechanism

Sorry, but I'm not sure I understand your description.

What is the effect of adding the XT_SOCKET_NOWILDCARD flag?
It almost sound like it adds the ability to match the 3rd packet of TCP
handshake (ACK coming from the client), is that the case?
Eric Dumazet June 4, 2013, 1:46 p.m. UTC | #2 
On Tue, 2013-06-04 at 11:10 +0200, Jesper Dangaard Brouer wrote:
> On Mon, 03 Jun 2013 15:57:29 -0700
> Eric Dumazet <eric.dumazet@gmail.com> wrote:
> 
> > From: Eric Dumazet <edumazet@google.com>
> > 
> > xt_socket module can be a nice replacement to conntrack module
> > in some cases (SYN filtering for example)
> > 
> > But it lacks the ability to match the 3rd packet of TCP
> > handshake (ACK coming from the client).
> > 
> > Add a XT_SOCKET_NOWILDCARD flag to disable the wildcard mechanism
> 
> Sorry, but I'm not sure I understand your description.
> 
> What is the effect of adding the XT_SOCKET_NOWILDCARD flag?
> It almost sound like it adds the ability to match the 3rd packet of TCP
> handshake (ACK coming from the client), is that the case?
> 

Well, if the found socket happens to be a LISTEN socket, we ignore the
socket if it was bound to 0.0.0.0

Thats the wildcard thing in xt_socket. Not clear why its there, but
thing is : we apparently have to keep this behavior by default.

So yes, the ACK packet from the client is not matched by current
xt_socket.

After my patch, it is matched.

I CCed you because you mentioned using conntrack for SYN filtering :
xt_socket can be a way to do the same thing without the conntrack
overhead, for locally terminated traffic.



--
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
Jesper Dangaard Brouer June 4, 2013, 2:30 p.m. UTC | #3 
On Tue, 04 Jun 2013 06:46:57 -0700
Eric Dumazet <eric.dumazet@gmail.com> wrote:

> On Tue, 2013-06-04 at 11:10 +0200, Jesper Dangaard Brouer wrote:
> > On Mon, 03 Jun 2013 15:57:29 -0700
> > Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > 
> > > From: Eric Dumazet <edumazet@google.com>
> > > 
> > > xt_socket module can be a nice replacement to conntrack module
> > > in some cases (SYN filtering for example)
> > > 
> > > But it lacks the ability to match the 3rd packet of TCP
> > > handshake (ACK coming from the client).
> > > 
> > > Add a XT_SOCKET_NOWILDCARD flag to disable the wildcard mechanism
> > 
> > Sorry, but I'm not sure I understand your description.
> > 
> > What is the effect of adding the XT_SOCKET_NOWILDCARD flag?
> > It almost sound like it adds the ability to match the 3rd packet of
> > TCP handshake (ACK coming from the client), is that the case?
> > 
> 
> Well, if the found socket happens to be a LISTEN socket, we ignore the
> socket if it was bound to 0.0.0.0
> 
> Thats the wildcard thing in xt_socket. Not clear why its there, but
> thing is : we apparently have to keep this behavior by default.
> 
> So yes, the ACK packet from the client is not matched by current
> xt_socket.
> 
> After my patch, it is matched.
> 
> I CCed you because you mentioned using conntrack for SYN filtering :
> xt_socket can be a way to do the same thing without the conntrack
> overhead, for locally terminated traffic.

Thank you for Cc'ing me.  I didn't realize that the module could be
used in this manor.  Much appreciated! :-)

--
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
Eric Dumazet June 20, 2013, 8:38 a.m. UTC | #4 
On Mon, 2013-06-03 at 15:57 -0700, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
> 
> xt_socket module can be a nice replacement to conntrack module
> in some cases (SYN filtering for example)
> 
> But it lacks the ability to match the 3rd packet of TCP
> handshake (ACK coming from the client).
> 
> Add a XT_SOCKET_NOWILDCARD flag to disable the wildcard mechanism
> 
> iptables -I INPUT -p tcp --syn -j SYN_CHAIN
> iptables -I INPUT -m socket -j ACCEPT
> 
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Patrick McHardy <kaber@trash.net>
> Cc: Jesper Dangaard Brouer <brouer@redhat.com>
> ---

Hi Pablo, is there any problem with this patch ?

Thanks


--
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
Pablo Neira Ayuso June 20, 2013, 9:55 a.m. UTC | #5 
On Thu, Jun 20, 2013 at 01:38:35AM -0700, Eric Dumazet wrote:
> On Mon, 2013-06-03 at 15:57 -0700, Eric Dumazet wrote:
> > From: Eric Dumazet <edumazet@google.com>
> > 
> > xt_socket module can be a nice replacement to conntrack module
> > in some cases (SYN filtering for example)
> > 
> > But it lacks the ability to match the 3rd packet of TCP
> > handshake (ACK coming from the client).
> > 
> > Add a XT_SOCKET_NOWILDCARD flag to disable the wildcard mechanism
> > 
> > iptables -I INPUT -p tcp --syn -j SYN_CHAIN
> > iptables -I INPUT -m socket -j ACCEPT
> > 
> > 
> > Signed-off-by: Eric Dumazet <edumazet@google.com>
> > Cc: Patrick McHardy <kaber@trash.net>
> > Cc: Jesper Dangaard Brouer <brouer@redhat.com>
> > ---
> 
> Hi Pablo, is there any problem with this patch ?

user-space part is missing (or at least I didn't manage to find the
patch).

We've been adding new revision for such a changes. I know it's a bit
too much, but we want to make sure that a user does not set this
option from user-space and it's silently ignored by iptables.
--
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
Eric Dumazet June 20, 2013, 10:19 a.m. UTC | #6 
On Thu, 2013-06-20 at 11:55 +0200, Pablo Neira Ayuso wrote:

> user-space part is missing (or at least I didn't manage to find the
> patch).

OK, I'll send the user-space, I was not sure if you wanted too.

(Stephen always want kernel part being applied before iproute2 changes)

> 
> We've been adding new revision for such a changes. I know it's a bit
> too much, but we want to make sure that a user does not set this
> option from user-space and it's silently ignored by iptables.

No problem, will send a v2.

Thanks


--
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
diff mbox

Patch

diff --git a/include/uapi/linux/netfilter/xt_socket.h b/include/uapi/linux/netfilter/xt_socket.h
index 26d7217..be1994fb 100644
--- a/include/uapi/linux/netfilter/xt_socket.h
+++ b/include/uapi/linux/netfilter/xt_socket.h
@@ -5,6 +5,7 @@ 
 
 enum {
 	XT_SOCKET_TRANSPARENT = 1 << 0,
+	XT_SOCKET_NOWILDCARD = 1 << 1,
 };
 
 struct xt_socket_mtinfo1 {
diff --git a/net/netfilter/xt_socket.c b/net/netfilter/xt_socket.c
index 0270424..9843314 100644
--- a/net/netfilter/xt_socket.c
+++ b/net/netfilter/xt_socket.c
@@ -163,8 +163,11 @@  socket_match(const struct sk_buff *skb, struct xt_action_param *par,
 		bool wildcard;
 		bool transparent = true;
 
-		/* Ignore sockets listening on INADDR_ANY */
-		wildcard = (sk->sk_state != TCP_TIME_WAIT &&
+		/* Ignore sockets listening on INADDR_ANY,
+		 * unless XT_SOCKET_NOWILDCARD is set
+		 */
+		wildcard = (!(info->flags & XT_SOCKET_NOWILDCARD) &&
+			    sk->sk_state != TCP_TIME_WAIT &&
 			    inet_sk(sk)->inet_rcv_saddr == 0);
 
 		/* Ignore non-transparent sockets,
@@ -302,8 +305,11 @@  socket_mt6_v1(const struct sk_buff *skb, struct xt_action_param *par)
 		bool wildcard;
 		bool transparent = true;
 
-		/* Ignore sockets listening on INADDR_ANY */
-		wildcard = (sk->sk_state != TCP_TIME_WAIT &&
+		/* Ignore sockets listening on INADDR_ANY
+		 * unless XT_SOCKET_NOWILDCARD is set
+		 */
+		wildcard = (!(info->flags & XT_SOCKET_NOWILDCARD) &&
+			    sk->sk_state != TCP_TIME_WAIT &&
 			    ipv6_addr_any(&inet6_sk(sk)->rcv_saddr));
 
 		/* Ignore non-transparent sockets,