(no subject)

[Balazs Scheidler]

This is the userspace counterpart of "netfilter: nft_socket: add wildcard
support" posted a few minutes ago. 

Compared from v1:
  - it incorporating the changes requested by @Pablo and @Stefano.

It doesn't - yet - incorporate changing the type to "boolean", since that
would probably touch both "wildcard" and "transparent" I would do that in a
followup branch.

Cheers,
Bazsi

[PATCH nftables v2 1/5] socket: add support for "wildcard" key

[Balazs Scheidler]

iptables had a "-m socket --transparent" which didn't match sockets that are
bound to all addresses (e.g.  0.0.0.0 for ipv4, and ::0 for ipv6).  It was
possible to override this behavior by using --nowildcard, in which case it
did match zero bound sockets as well.

The issue is that nftables never included the wildcard check, so in effect
it behaved like "iptables -m socket --transparent --nowildcard" with no
means to exclude wildcarded listeners.

This is a problem as a user-space process that binds to 0.0.0.0:<port> that
enables IP_TRANSPARENT would effectively intercept traffic going in _any_
direction on the specific port, whereas in most cases, transparent proxies
would only need this for one specific address.

The solution is to add "socket wildcard" key to the nft_socket module, which
makes it possible to match on the wildcardness of a socket from
one's ruleset.

This is how to use it:

table inet haproxy {
	chain prerouting {
        	type filter hook prerouting priority -150; policy accept;
		socket transparent 1 socket wildcard 0 mark set 0x00000001
	}
}

This patch effectively depends on its counterpart in the kernel.

Signed-off-by: Balazs Scheidler <bazsi77@gmail.com>
---
 include/linux/netfilter/nf_tables.h | 2 ++
 src/evaluate.c                      | 4 +++-
 src/parser_bison.y                  | 2 ++
 src/parser_json.c                   | 2 ++
 src/scanner.l                       | 1 +
 src/socket.c                        | 6 ++++++
 6 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/include/linux/netfilter/nf_tables.h b/include/linux/netfilter/nf_tables.h
index 1341b52f..10be073a 100644
--- a/include/linux/netfilter/nf_tables.h
+++ b/include/linux/netfilter/nf_tables.h
@@ -996,10 +996,12 @@ enum nft_socket_attributes {
  *
  * @NFT_SOCKET_TRANSPARENT: Value of the IP(V6)_TRANSPARENT socket option
  * @NFT_SOCKET_MARK: Value of the socket mark
+ * @NFT_SOCKET_WILDCARD: Whether the socket is zero-bound (e.g. 0.0.0.0 or ::0)
  */
 enum nft_socket_keys {
 	NFT_SOCKET_TRANSPARENT,
 	NFT_SOCKET_MARK,
+	NFT_SOCKET_WILDCARD,
 	__NFT_SOCKET_MAX
 };
 #define NFT_SOCKET_MAX	(__NFT_SOCKET_MAX - 1)
diff --git a/src/evaluate.c b/src/evaluate.c
index 320a464f..e1992e2a 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -1999,8 +1999,10 @@ static int expr_evaluate_meta(struct eval_ctx *ctx, struct expr **exprp)
 static int expr_evaluate_socket(struct eval_ctx *ctx, struct expr **expr)
 {
 	int maxval = 0;
+	enum nft_socket_keys key = (*expr)->socket.key;
 
-	if((*expr)->socket.key == NFT_SOCKET_TRANSPARENT)
+	if (key == NFT_SOCKET_TRANSPARENT ||
+	    key == NFT_SOCKET_WILDCARD)
 		maxval = 1;
 	__expr_set_context(&ctx->ectx, (*expr)->dtype, (*expr)->byteorder,
 			   (*expr)->len, maxval);
diff --git a/src/parser_bison.y b/src/parser_bison.y
index 95adc48f..d938f566 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -213,6 +213,7 @@ int nft_lex(void *, void *, void *);
 
 %token SOCKET			"socket"
 %token TRANSPARENT		"transparent"
+%token WILDCARD			"wildcard"
 
 %token TPROXY			"tproxy"
 
@@ -4595,6 +4596,7 @@ socket_expr		:	SOCKET	socket_key
 
 socket_key 		: 	TRANSPARENT	{ $$ = NFT_SOCKET_TRANSPARENT; }
 			|	MARK		{ $$ = NFT_SOCKET_MARK; }
+			|	WILDCARD	{ $$ = NFT_SOCKET_WILDCARD; }
 			;
 
 offset_opt		:	/* empty */	{ $$ = 0; }
diff --git a/src/parser_json.c b/src/parser_json.c
index 59347168..ac89166e 100644
--- a/src/parser_json.c
+++ b/src/parser_json.c
@@ -427,6 +427,8 @@ static struct expr *json_parse_socket_expr(struct json_ctx *ctx,
 		keyval = NFT_SOCKET_TRANSPARENT;
 	else if (!strcmp(key, "mark"))
 		keyval = NFT_SOCKET_MARK;
+	else if (!strcmp(key, "wildcard"))
+		keyval = NFT_SOCKET_WILDCARD;
 
 	if (keyval == -1) {
 		json_error(ctx, "Invalid socket key value.");
diff --git a/src/scanner.l b/src/scanner.l
index 45699c85..9e6464f9 100644
--- a/src/scanner.l
+++ b/src/scanner.l
@@ -268,6 +268,7 @@ addrstring	({macaddr}|{ip4addr}|{ip6addr})
 
 "socket"		{ return SOCKET; }
 "transparent"		{ return TRANSPARENT;}
+"wildcard"		{ return WILDCARD; }
 
 "tproxy"		{ return TPROXY; }
 
diff --git a/src/socket.c b/src/socket.c
index d78a163a..673e5d0f 100644
--- a/src/socket.c
+++ b/src/socket.c
@@ -26,6 +26,12 @@ const struct socket_template socket_templates[] = {
 		.len		= 4 * BITS_PER_BYTE,
 		.byteorder	= BYTEORDER_HOST_ENDIAN,
 	},
+	[NFT_SOCKET_WILDCARD] = {
+		.token		= "wildcard",
+		.dtype		= &integer_type,
+		.len		= BITS_PER_BYTE,
+		.byteorder	= BYTEORDER_HOST_ENDIAN,
+	},
 };
 
 static void socket_expr_print(const struct expr *expr, struct output_ctx *octx)
-- 
2.17.1

[PATCH nftables v2 2/5] src/scanner.l: fix whitespace issue for the TRANSPARENT keyword

[Balazs Scheidler]

Signed-off-by: Balazs Scheidler <bazsi77@gmail.com>
---
 src/scanner.l | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/scanner.l b/src/scanner.l
index 9e6464f9..7afd9bfb 100644
--- a/src/scanner.l
+++ b/src/scanner.l
@@ -267,7 +267,7 @@ addrstring	({macaddr}|{ip4addr}|{ip6addr})
 "trace"			{ return TRACE; }
 
 "socket"		{ return SOCKET; }
-"transparent"		{ return TRANSPARENT;}
+"transparent"		{ return TRANSPARENT; }
 "wildcard"		{ return WILDCARD; }
 
 "tproxy"		{ return TPROXY; }
-- 
2.17.1

[PATCH nftables v2 3/5] doc: added documentation on "socket wildcard"

[Balazs Scheidler]

Signed-off-by: Balazs Scheidler <bazsi77@gmail.com>
---
 doc/primary-expression.txt | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/doc/primary-expression.txt b/doc/primary-expression.txt
index a9c39cbb..e87e8cc2 100644
--- a/doc/primary-expression.txt
+++ b/doc/primary-expression.txt
@@ -195,7 +195,7 @@ raw prerouting meta ipsec exists accept
 SOCKET EXPRESSION
 ~~~~~~~~~~~~~~~~~
 [verse]
-*socket* {*transparent* | *mark*}
+*socket* {*transparent* | *mark* | *wildcard*}
 
 Socket expression can be used to search for an existing open TCP/UDP socket and
 its attributes that can be associated with a packet. It looks for an established
@@ -209,15 +209,20 @@ or non-zero bound listening socket (possibly with a non-local address).
 Value of the IP_TRANSPARENT socket option in the found socket. It can be 0 or 1.|
 boolean (1 bit)
 |mark| Value of the socket mark (SOL_SOCKET, SO_MARK). | mark
+|wildcard|
+Indicates whether the socket is wildcard-bound (e.g. 0.0.0.0 or ::0). |
+boolean (1 bit)
 |==================
 
 .Using socket expression
 ------------------------
-# Mark packets that correspond to a transparent socket
+# Mark packets that correspond to a transparent socket. "socket wildcard 0"
+# means that zero-bound listener sockets are NOT matched (which is usually
+# exactly what you want).
 table inet x {
     chain y {
 	type filter hook prerouting priority -150; policy accept;
-        socket transparent 1 mark set 0x00000001 accept
+        socket transparent 1 socket wildcard 0 mark set 0x00000001 accept
     }
 }
 
-- 
2.17.1

[PATCH nftables v2 4/5] tests: added "socket wildcard" testcases

[Balazs Scheidler]

Signed-off-by: Balazs Scheidler <bazsi77@gmail.com>
---
 tests/py/inet/socket.t         |  4 ++++
 tests/py/inet/socket.t.json    | 29 +++++++++++++++++++++++++++++
 tests/py/inet/socket.t.payload | 29 +++++++++++++++++++++++++++++
 3 files changed, 62 insertions(+)

diff --git a/tests/py/inet/socket.t b/tests/py/inet/socket.t
index 91846e8e..05e9ebb4 100644
--- a/tests/py/inet/socket.t
+++ b/tests/py/inet/socket.t
@@ -9,3 +9,7 @@ socket transparent 1;ok
 socket transparent 2;fail
 
 socket mark 0x00000005;ok
+
+socket wildcard 0;ok
+socket wildcard 1;ok
+socket wildcard 2;fail
diff --git a/tests/py/inet/socket.t.json b/tests/py/inet/socket.t.json
index 99d6e248..fa48e79d 100644
--- a/tests/py/inet/socket.t.json
+++ b/tests/py/inet/socket.t.json
@@ -43,3 +43,32 @@
     }
 ]
 
+# socket wildcard 0
+[
+    {
+        "match": {
+            "left": {
+                "socket": {
+                    "key": "wildcard"
+                }
+            },
+            "op": "==",
+            "right": 0
+        }
+    }
+]
+
+# socket wildcard 1
+[
+    {
+        "match": {
+            "left": {
+                "socket": {
+                    "key": "wildcard"
+                }
+            },
+            "op": "==",
+            "right": 1
+        }
+    }
+]
diff --git a/tests/py/inet/socket.t.payload b/tests/py/inet/socket.t.payload
index 687b7a45..79fcea79 100644
--- a/tests/py/inet/socket.t.payload
+++ b/tests/py/inet/socket.t.payload
@@ -43,3 +43,32 @@ inet sockin sockchain
   [ socket load mark => reg 1 ]
   [ cmp eq reg 1 0x00000005 ]
 
+# socket wildcard 0
+ip sockip4 sockchain
+  [ socket load wildcard => reg 1 ]
+  [ cmp eq reg 1 0x00000000 ]
+
+# socket wildcard 0
+ip6 sockip6 sockchain
+  [ socket load wildcard => reg 1 ]
+  [ cmp eq reg 1 0x00000000 ]
+
+# socket wildcard 0
+inet sockin sockchain
+  [ socket load wildcard => reg 1 ]
+  [ cmp eq reg 1 0x00000000 ]
+
+# socket wildcard 1
+ip sockip4 sockchain
+  [ socket load wildcard => reg 1 ]
+  [ cmp eq reg 1 0x00000001 ]
+
+# socket wildcard 1
+ip6 sockip6 sockchain
+  [ socket load wildcard => reg 1 ]
+  [ cmp eq reg 1 0x00000001 ]
+
+# socket wildcard 1
+inet sockin sockchain
+  [ socket load wildcard => reg 1 ]
+  [ cmp eq reg 1 0x00000001 ]
-- 
2.17.1

[PATCH nftables v2 5/5] tests: allow tests/monitor to use a custom nft executable

[Balazs Scheidler]

Signed-off-by: Balazs Scheidler <bazsi77@gmail.com>
---
 tests/monitor/run-tests.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/monitor/run-tests.sh b/tests/monitor/run-tests.sh
index ffb833a7..5a736fc6 100755
--- a/tests/monitor/run-tests.sh
+++ b/tests/monitor/run-tests.sh
@@ -1,7 +1,7 @@
 #!/bin/bash
 
 cd $(dirname $0)
-nft=../../src/nft
+nft=${NFT:-../../src/nft}
 debug=false
 test_json=false
 
-- 
2.17.1

Re: [PATCH nftables v2 1/5] socket: add support for "wildcard" key

[Pablo Neira Ayuso]

On Sat, Aug 29, 2020 at 09:04:01AM +0200, Balazs Scheidler wrote:
> iptables had a "-m socket --transparent" which didn't match sockets that are
> bound to all addresses (e.g.  0.0.0.0 for ipv4, and ::0 for ipv6).  It was
> possible to override this behavior by using --nowildcard, in which case it
> did match zero bound sockets as well.
> 
> The issue is that nftables never included the wildcard check, so in effect
> it behaved like "iptables -m socket --transparent --nowildcard" with no
> means to exclude wildcarded listeners.
> 
> This is a problem as a user-space process that binds to 0.0.0.0:<port> that
> enables IP_TRANSPARENT would effectively intercept traffic going in _any_
> direction on the specific port, whereas in most cases, transparent proxies
> would only need this for one specific address.
> 
> The solution is to add "socket wildcard" key to the nft_socket module, which
> makes it possible to match on the wildcardness of a socket from
> one's ruleset.
> 
> This is how to use it:
> 
> table inet haproxy {
> 	chain prerouting {
>         	type filter hook prerouting priority -150; policy accept;
> 		socket transparent 1 socket wildcard 0 mark set 0x00000001
> 	}
> }
> 
> This patch effectively depends on its counterpart in the kernel.

Applied, thanks.

Re: [PATCH nftables v2 2/5] src/scanner.l: fix whitespace issue for the TRANSPARENT keyword

[Pablo Neira Ayuso]

Applied, thanks.

Re: [PATCH nftables v2 3/5] doc: added documentation on "socket wildcard"

[Pablo Neira Ayuso]

Applied, thanks.

Re: [PATCH nftables v2 4/5] tests: added "socket wildcard" testcases

[Pablo Neira Ayuso]

Also applied, thanks.

Re: [PATCH nftables v2 5/5] tests: allow tests/monitor to use a custom nft executable

[Pablo Neira Ayuso]

On Sat, Aug 29, 2020 at 09:04:05AM +0200, Balazs Scheidler wrote:
> Signed-off-by: Balazs Scheidler <bazsi77@gmail.com>
> ---
>  tests/monitor/run-tests.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/monitor/run-tests.sh b/tests/monitor/run-tests.sh
> index ffb833a7..5a736fc6 100755
> --- a/tests/monitor/run-tests.sh
> +++ b/tests/monitor/run-tests.sh
> @@ -1,7 +1,7 @@
>  #!/bin/bash
>  
>  cd $(dirname $0)
> -nft=../../src/nft
> +nft=${NFT:-../../src/nft}
>  debug=false
>  test_json=false

IIRC, Stefano mentioned this might break valgrind due to lack of
quotes?

Thanks.

Re: [PATCH nftables v2 5/5] tests: allow tests/monitor to use a custom nft executable

[Stefano Brivio]

On Sat, 29 Aug 2020 13:18:50 +0200
Pablo Neira Ayuso <pablo@netfilter.org> wrote:

> On Sat, Aug 29, 2020 at 09:04:05AM +0200, Balazs Scheidler wrote:
> > Signed-off-by: Balazs Scheidler <bazsi77@gmail.com>
> > ---
> >  tests/monitor/run-tests.sh | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/tests/monitor/run-tests.sh b/tests/monitor/run-tests.sh
> > index ffb833a7..5a736fc6 100755
> > --- a/tests/monitor/run-tests.sh
> > +++ b/tests/monitor/run-tests.sh
> > @@ -1,7 +1,7 @@
> >  #!/bin/bash
> >  
> >  cd $(dirname $0)
> > -nft=../../src/nft
> > +nft=${NFT:-../../src/nft}
> >  debug=false
> >  test_json=false  
> 
> IIRC, Stefano mentioned this might break valgrind due to lack of
> quotes?

Wait, this is just for monitor/run-tests.sh now. The problem was on the
change proposed for shell/run_tests.sh, which wasn't needed because
it already supports passing a different command, and is not in this
version.

For monitor/run-tests.sh, I think that will need some fixing anyway (if
we want to support wrappers at all). So this change itself just
improves things.

-- 
Stefano

Re: [PATCH nftables v2 5/5] tests: allow tests/monitor to use a custom nft executable

[Pablo Neira Ayuso]

On Sat, Aug 29, 2020 at 02:24:31PM +0200, Stefano Brivio wrote:
> On Sat, 29 Aug 2020 13:18:50 +0200
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> 
> > On Sat, Aug 29, 2020 at 09:04:05AM +0200, Balazs Scheidler wrote:
> > > Signed-off-by: Balazs Scheidler <bazsi77@gmail.com>
> > > ---
> > >  tests/monitor/run-tests.sh | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/tests/monitor/run-tests.sh b/tests/monitor/run-tests.sh
> > > index ffb833a7..5a736fc6 100755
> > > --- a/tests/monitor/run-tests.sh
> > > +++ b/tests/monitor/run-tests.sh
> > > @@ -1,7 +1,7 @@
> > >  #!/bin/bash
> > >  
> > >  cd $(dirname $0)
> > > -nft=../../src/nft
> > > +nft=${NFT:-../../src/nft}
> > >  debug=false
> > >  test_json=false  
> > 
> > IIRC, Stefano mentioned this might break valgrind due to lack of
> > quotes?
> 
> Wait, this is just for monitor/run-tests.sh now. The problem was on the
> change proposed for shell/run_tests.sh, which wasn't needed because
> it already supports passing a different command, and is not in this
> version.
> 
> For monitor/run-tests.sh, I think that will need some fixing anyway (if
> we want to support wrappers at all). So this change itself just
> improves things.

Thanks for explaining, patch is applied.