[PATCH nftables 0/4] socket: add support for "wildcard" key

[PATCH nftables 0/4] socket: add support for "wildcard" key

[Balazs Scheidler]

NOTE: this depends on a kernel patch, so please merge that before this can
be merged.  Also, apart from build testing and running the binaries on an
unpatched kernel (and confirming the netlink payload is formatted as it
should be) this is untested.

This series adds the nftables side of "socket wildcard" a new expression
that extracts whether the associated socket is bound to the ANY address or
not.

iptables originally had this behavior by default when using "-m socket
--transparent", but this was missing from nftables.


Also, the last patch in the series allows one to override the "nft"
executable used by the tests.

[PATCH nftables 1/4] 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>
---
 src/evaluate.c     | 5 ++++-
 src/parser_bison.y | 2 ++
 src/parser_json.c  | 2 ++
 src/scanner.l      | 1 +
 src/socket.c       | 6 ++++++
 5 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/src/evaluate.c b/src/evaluate.c
index b64ed3c0..28dade8a 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -1999,8 +1999,11 @@ 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 d4e99417..fff941e5 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"
 
@@ -4591,6 +4592,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..90b36615 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

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

[Stefano Brivio]

On Sat, 22 Aug 2020 08:22:00 +0200
Balazs Scheidler <bazsi77@gmail.com> 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.
> 
> Signed-off-by: Balazs Scheidler <bazsi77@gmail.com>
> ---
>  src/evaluate.c     | 5 ++++-
>  src/parser_bison.y | 2 ++
>  src/parser_json.c  | 2 ++
>  src/scanner.l      | 1 +
>  src/socket.c       | 6 ++++++
>  5 files changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/src/evaluate.c b/src/evaluate.c
> index b64ed3c0..28dade8a 100644
> --- a/src/evaluate.c
> +++ b/src/evaluate.c
> @@ -1999,8 +1999,11 @@ 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;

The empty line before this isn't needed: it's another declaration.

>  
> -	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 d4e99417..fff941e5 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"
>  
> @@ -4591,6 +4592,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..90b36615 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;}

For consistency, { return WILDCARD; } (TRANSPARENT is an exception).

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

You could also use boolean_type for this, see e.g. the meta ipsec
attribute.

-- 
Stefano

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

[Pablo Neira Ayuso]

Hi Balazs,

One more comment for your upcoming v2 ;-)

On Sat, Aug 22, 2020 at 08:22:00AM +0200, Balazs Scheidler wrote:
[...]
>  src/evaluate.c     | 5 ++++-
>  src/parser_bison.y | 2 ++
>  src/parser_json.c  | 2 ++
>  src/scanner.l      | 1 +
>  src/socket.c       | 6 ++++++
>  5 files changed, 15 insertions(+), 1 deletion(-)

Please, update include/linux/netfilter/nf_tables.h to add
NFT_SOCKET_WILDCARD.

Thanks.

[PATCH nftables 2/4] 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..6d3383ed 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 weather 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

Re: [PATCH nftables 2/4] doc: added documentation on "socket wildcard"

[Stefano Brivio]

On Sat, 22 Aug 2020 08:22:01 +0200
Balazs Scheidler <bazsi77@gmail.com> wrote:

> @@ -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 weather the socket is wildcard-bound (e.g. 0.0.0.0 or ::0). |

s/weather/whether/.

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

"zero-bound" would be a bit clearer (and consistent with the rest).

-- 
Stefano

[PATCH nftables 3/4] tests: added "socked 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

Re: [PATCH nftables 3/4] tests: added "socked wildcard" testcases

[Stefano Brivio]

On Sat, 22 Aug 2020 08:22:02 +0200
Balazs Scheidler <bazsi77@gmail.com> wrote:

> Signed-off-by: Balazs Scheidler <bazsi77@gmail.com>

Nit, in case you re-post: s/socked/socket/.

-- 
Stefano

[PATCH nftables 4/4] tests: allow tests to use a custom nft executable

[Balazs Scheidler]

Signed-off-by: Balazs Scheidler <bazsi77@gmail.com>
---
 tests/monitor/run-tests.sh | 2 +-
 tests/shell/run-tests.sh   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

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
 
diff --git a/tests/shell/run-tests.sh b/tests/shell/run-tests.sh
index 943f8877..5233ba86 100755
--- a/tests/shell/run-tests.sh
+++ b/tests/shell/run-tests.sh
@@ -2,7 +2,7 @@
 
 # Configuration
 TESTDIR="./$(dirname $0)/testcases"
-SRC_NFT="$(dirname $0)/../../src/nft"
+SRC_NFT=${NFT:-../../src/nft}
 DIFF=$(which diff)
 
 msg_error() {
-- 
2.17.1

Re: [PATCH nftables 4/4] tests: allow tests to use a custom nft executable

[Stefano Brivio]

Hi Balazs,

On Sat, 22 Aug 2020 08:22:03 +0200
Balazs Scheidler <bazsi77@gmail.com> wrote:

> diff --git a/tests/shell/run-tests.sh b/tests/shell/run-tests.sh
> index 943f8877..5233ba86 100755
> --- a/tests/shell/run-tests.sh
> +++ b/tests/shell/run-tests.sh
> @@ -2,7 +2,7 @@
>  
>  # Configuration
>  TESTDIR="./$(dirname $0)/testcases"
> -SRC_NFT="$(dirname $0)/../../src/nft"
> +SRC_NFT=${NFT:-../../src/nft}

This isn't needed (and lacks quotes, won't work with a wrapper, e.g.
valgrind). It's already possible to pass a different nft executable
because later we have:

	[ -z "$NFT" ] && NFT=$SRC_NFT

...now, you could in theory replace this assignment with the one you
proposed, but I think a SRC_NFT="../../src/nft" variable is more obvious
to configure compared to NFT="${NFT:../../src/nft}".

-- 
Stefano