netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* nftables: secmark support
@ 2019-10-22 15:57 Christian Göttsche
  2019-10-22 17:34 ` Pablo Neira Ayuso
  0 siblings, 1 reply; 8+ messages in thread
From: Christian Göttsche @ 2019-10-22 15:57 UTC (permalink / raw)
  To: netfilter-devel

Hi,
I am trying to finally get secmark with nftables to work.
The kernel[1][2] and libnftnl[3] parts are done.
For the nft front-end I think some things need a further change than
already introduced[4].

1.
I found no way to store the secmark label into the connection tracking
state and thereby set the label on established,related packets.
Using a patch[5] it works with the following syntax:
(Note: The patch will currently probably not apply to current master,
due to [6])

    [... define secmarks and port maps ...]
    chain input {
        type filter hook input priority 0;
        ct state new meta secmark set tcp dport map @secmapping_in
        ct state new ip protocol icmp meta secmark set "icmp_server"
        ct state new ip6 nexthdr icmpv6 meta secmark set "icmp_server"
        ct state new ct secmark_raw set meta secmark_raw
        ct state established,related meta secmark_raw set ct secmark_raw
    }
    chain output {
        type filter hook output priority 0;
        ct state new meta secmark set tcp dport map @secmapping_out
        ct state new ip protocol icmp meta secmark set "icmp_client"
        ct state new ip6 nexthdr icmpv6 meta secmark set "icmp_client"
        ct state new ct secmark_raw set meta secmark_raw
        ct state established,related meta secmark_raw set ct secmark_raw
    }

2.
The rules in 1. are not idempotent. The output of 'nft list ruleset' is:

    chain input {
        type filter hook input priority filter; policy accept;
        ct state new secmark name tcp dport map @secmapping_in
        ct state new ip protocol icmp secmark name "icmp_server"
        ct state new ip6 nexthdr ipv6-icmp secmark name "icmp_server"
        ct state new ct secmark set secmark
        ct state established,related secmark set ct secmark
    }
    chain output {
        type filter hook output priority filter; policy accept;
        ct state new secmark name tcp dport map @secmapping_out
        ct state new ip protocol icmp secmark name "icmp_client"
        ct state new ip6 nexthdr ipv6-icmp secmark name "icmp_client"
        ct state new ct secmark set secmark
        ct state established,related secmark set ct secmark
    }

What are the code locations to fix?

3.
The patch also adds the ability to reset secmarks.
Is there a way to query the kernel about the actual secid (to verify
the reset works)?

4.
Maybe I can contribute a howto for wiki.nftables.org. What is the
preferred format?

Best regards,
     Christian Göttsche


[1] https://github.com/torvalds/linux/commit/fb961945457f5177072c968aa38fee910ab893b9
[2] https://github.com/torvalds/linux/commit/b473a1f5ddee5f73392c387940f4fbcbabfc3431
[3] https://git.netfilter.org/libnftnl/commit/?id=aaf20ad0dc22d2ebcad1b2c43288e984f0efe2c3
[4] https://git.netfilter.org/nftables/commit/?id=3bc84e5c1fdd1ff011af9788fe174e0514c2c9ea
[5] https://salsa.debian.org/cgzones-guest/pkg-nftables/blob/master/debian/patches/0004-secmark-add-missing-pieces.patch
[6] https://git.netfilter.org/nftables/commit/?id=998142c71d095d79488495ea545a704213fa0ba0

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: nftables: secmark support
  2019-10-22 15:57 nftables: secmark support Christian Göttsche
@ 2019-10-22 17:34 ` Pablo Neira Ayuso
  2019-10-28 14:27   ` Christian Göttsche
  0 siblings, 1 reply; 8+ messages in thread
From: Pablo Neira Ayuso @ 2019-10-22 17:34 UTC (permalink / raw)
  To: Christian Göttsche; +Cc: netfilter-devel

On Tue, Oct 22, 2019 at 05:57:25PM +0200, Christian Göttsche wrote:
> Hi,
> I am trying to finally get secmark with nftables to work.
> The kernel[1][2] and libnftnl[3] parts are done.
> For the nft front-end I think some things need a further change than
> already introduced[4].
> 
> 1.
> I found no way to store the secmark label into the connection tracking
> state and thereby set the label on established,related packets.
> Using a patch[5] it works with the following syntax:
> (Note: The patch will currently probably not apply to current master,
> due to [6])
> 
>     [... define secmarks and port maps ...]
>     chain input {
>         type filter hook input priority 0;
>         ct state new meta secmark set tcp dport map @secmapping_in
>         ct state new ip protocol icmp meta secmark set "icmp_server"
>         ct state new ip6 nexthdr icmpv6 meta secmark set "icmp_server"
>         ct state new ct secmark_raw set meta secmark_raw
>         ct state established,related meta secmark_raw set ct secmark_raw

So your concern is the need for this extra secmark_raw, correct?

This is what your patch [6] does, right? If you don't mind to rebase
it I can have a look if I can propose you something else than this new
keyword.

>     }
>     chain output {
>         type filter hook output priority 0;
>         ct state new meta secmark set tcp dport map @secmapping_out
>         ct state new ip protocol icmp meta secmark set "icmp_client"
>         ct state new ip6 nexthdr icmpv6 meta secmark set "icmp_client"
>         ct state new ct secmark_raw set meta secmark_raw
>         ct state established,related meta secmark_raw set ct secmark_raw
>     }
> 
> 2.
> The rules in 1. are not idempotent. The output of 'nft list ruleset' is:
> 
>     chain input {
>         type filter hook input priority filter; policy accept;
>         ct state new secmark name tcp dport map @secmapping_in
>         ct state new ip protocol icmp secmark name "icmp_server"
>         ct state new ip6 nexthdr ipv6-icmp secmark name "icmp_server"
>         ct state new ct secmark set secmark
>         ct state established,related secmark set ct secmark

This is the listing after you add ruleset in 1., correct?

>     }
>     chain output {
>         type filter hook output priority filter; policy accept;
>         ct state new secmark name tcp dport map @secmapping_out
>         ct state new ip protocol icmp secmark name "icmp_client"
>         ct state new ip6 nexthdr ipv6-icmp secmark name "icmp_client"
>         ct state new ct secmark set secmark
>         ct state established,related secmark set ct secmark
>     }
> 
> What are the code locations to fix?
> 
> 3.
> The patch also adds the ability to reset secmarks.
> Is there a way to query the kernel about the actual secid (to verify
> the reset works)?

What do you mean by "reset secmarks", example please.

> 4.
> Maybe I can contribute a howto for wiki.nftables.org. What is the
> preferred format?

That would be great indeed.

Sorry for the many questions!

[...]
> [1] https://github.com/torvalds/linux/commit/fb961945457f5177072c968aa38fee910ab893b9
> [2] https://github.com/torvalds/linux/commit/b473a1f5ddee5f73392c387940f4fbcbabfc3431
> [3] https://git.netfilter.org/libnftnl/commit/?id=aaf20ad0dc22d2ebcad1b2c43288e984f0efe2c3
> [4] https://git.netfilter.org/nftables/commit/?id=3bc84e5c1fdd1ff011af9788fe174e0514c2c9ea
> [5] https://salsa.debian.org/cgzones-guest/pkg-nftables/blob/master/debian/patches/0004-secmark-add-missing-pieces.patch
> [6] https://git.netfilter.org/nftables/commit/?id=998142c71d095d79488495ea545a704213fa0ba0

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: nftables: secmark support
  2019-10-22 17:34 ` Pablo Neira Ayuso
@ 2019-10-28 14:27   ` Christian Göttsche
  2019-11-18 16:44     ` Christian Göttsche
  0 siblings, 1 reply; 8+ messages in thread
From: Christian Göttsche @ 2019-10-28 14:27 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel

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

> >     [... define secmarks and port maps ...]
> >     chain input {
> >         type filter hook input priority 0;
> >         ct state new meta secmark set tcp dport map @secmapping_in
> >         ct state new ip protocol icmp meta secmark set "icmp_server"
> >         ct state new ip6 nexthdr icmpv6 meta secmark set "icmp_server"
> >         ct state new ct secmark_raw set meta secmark_raw
> >         ct state established,related meta secmark_raw set ct secmark_raw
>
> So your concern is the need for this extra secmark_raw, correct?

Exactly, cause i want to store the kernel internal secid in the packet
state to match it on est,rel packets.
Otherwise I got "Counter expression must be constant" and other errors.

> This is what your patch [6] does, right? If you don't mind to rebase
> it I can have a look if I can propose you something else than this new
> keyword.

Attached at the end (base on 707ad229d48f2ba7920541527b755b155ddedcda)

> This is the listing after you add ruleset in 1., correct?

Yes

> > 3.
> > The patch also adds the ability to reset secmarks.
> > Is there a way to query the kernel about the actual secid (to verify
> > the reset works)?
>
> What do you mean by "reset secmarks", example please.

Reseting secmarks intends to renew the association between the secmark
string and the kernel internal secid.
To keep it in sync after e.g. a SELinux policy reload, without
restarting the whole firewall, resetting counters etc..



From c559cb37e09526e02da02724017d0f921a03a1c1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com>
Date: Mon, 28 Oct 2019 15:12:29 +0100
Subject: [PATCH] add secmark_raw for storing secmark id in packet state

---
 src/ct.c           |  2 ++
 src/evaluate.c     |  2 ++
 src/meta.c         |  3 +++
 src/parser_bison.y | 37 +++++++++++++++++++++++++++++--------
 src/rule.c         |  6 ++++++
 src/scanner.l      |  1 +
 6 files changed, 43 insertions(+), 8 deletions(-)

diff --git a/src/ct.c b/src/ct.c
index ed458e6..9e6a835 100644
--- a/src/ct.c
+++ b/src/ct.c
@@ -299,6 +299,8 @@ const struct ct_template ct_templates[__NFT_CT_MAX] = {
                           BYTEORDER_BIG_ENDIAN, 128),
     [NFT_CT_DST_IP6]    = CT_TEMPLATE("ip6 daddr", &ip6addr_type,
                           BYTEORDER_BIG_ENDIAN, 128),
+    [NFT_CT_SECMARK]    = CT_TEMPLATE("secmark", &integer_type,
+                          BYTEORDER_HOST_ENDIAN, 32),
 };

 static void ct_print(enum nft_ct_keys key, int8_t dir, uint8_t nfproto,
diff --git a/src/evaluate.c b/src/evaluate.c
index a56cd2a..1b2f5e3 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -3944,8 +3944,10 @@ static int cmd_evaluate_reset(struct eval_ctx
*ctx, struct cmd *cmd)
     switch (cmd->obj) {
     case CMD_OBJ_COUNTER:
     case CMD_OBJ_QUOTA:
+    case CMD_OBJ_SECMARK:
     case CMD_OBJ_COUNTERS:
     case CMD_OBJ_QUOTAS:
+    case CMD_OBJ_SECMARKS:
         if (cmd->handle.table.name == NULL)
             return 0;
         if (table_lookup(&cmd->handle, &ctx->nft->cache) == NULL)
diff --git a/src/meta.c b/src/meta.c
index f54b818..8093d67 100644
--- a/src/meta.c
+++ b/src/meta.c
@@ -709,6 +709,8 @@ const struct meta_template meta_templates[] = {
     [NFT_META_TIME_HOUR]    = META_TEMPLATE("hour", &hour_type,
                         4 * BITS_PER_BYTE,
                         BYTEORDER_HOST_ENDIAN),
+    [NFT_META_SECMARK]    = META_TEMPLATE("secmark", &integer_type,
+                        32, BYTEORDER_HOST_ENDIAN),
 };

 static bool meta_key_is_unqualified(enum nft_meta_keys key)
@@ -720,6 +722,7 @@ static bool meta_key_is_unqualified(enum nft_meta_keys key)
     case NFT_META_OIFNAME:
     case NFT_META_IIFGROUP:
     case NFT_META_OIFGROUP:
+    case NFT_META_SECMARK:
         return true;
     default:
         return false;
diff --git a/src/parser_bison.y b/src/parser_bison.y
index 11f0dc8..16fcea2 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -479,6 +479,7 @@ int nft_lex(void *, void *, void *);

 %token SECMARK            "secmark"
 %token SECMARKS            "secmarks"
+%token SECMARK_RAW        "secmark_raw"

 %token NANOSECOND        "nanosecond"
 %token MICROSECOND        "microsecond"
@@ -748,7 +749,7 @@ int nft_lex(void *, void *, void *);

 %type <expr>            meta_expr
 %destructor { expr_free($$); }    meta_expr
-%type <val>            meta_key    meta_key_qualified
meta_key_unqualified    numgen_type
+%type <val>            meta_key    meta_key_qualified
meta_key_unqualified    meta_key_object    numgen_type

 %type <expr>            socket_expr
 %destructor { expr_free($$); } socket_expr
@@ -1365,6 +1366,18 @@ reset_cmd        :    COUNTERS    ruleset_spec
             {
                 $$ = cmd_alloc(CMD_RESET, CMD_OBJ_QUOTA, &$2, &@$, NULL);
             }
+            |    SECMARKS    ruleset_spec
+            {
+                $$ = cmd_alloc(CMD_RESET, CMD_OBJ_SECMARKS, &$2, &@$, NULL);
+            }
+            |    SECMARKS    TABLE    table_spec
+            {
+                $$ = cmd_alloc(CMD_RESET, CMD_OBJ_SECMARKS, &$3, &@$, NULL);
+            }
+            |       SECMARK        obj_spec
+            {
+                $$ = cmd_alloc(CMD_RESET, CMD_OBJ_SECMARK, &$2, &@$, NULL);
+            }
             ;

 flush_cmd        :    TABLE        table_spec
@@ -4123,7 +4136,7 @@ meta_key_qualified    :    LENGTH        { $$ =
NFT_META_LEN; }
             |    PROTOCOL    { $$ = NFT_META_PROTOCOL; }
             |    PRIORITY    { $$ = NFT_META_PRIORITY; }
             |    RANDOM        { $$ = NFT_META_PRANDOM; }
-            |    SECMARK        { $$ = NFT_META_SECMARK; }
+            |    SECMARK_RAW    { $$ = NFT_META_SECMARK; }
             ;

 meta_key_unqualified    :    MARK        { $$ = NFT_META_MARK; }
@@ -4152,7 +4165,18 @@ meta_key_unqualified    :    MARK        { $$ =
NFT_META_MARK; }
             |       HOUR        { $$ = NFT_META_TIME_HOUR; }
             ;

+meta_key_object        :    SECMARK        { $$ = NFT_META_SECMARK; }
+            ;
+
 meta_stmt        :    META    meta_key    SET    stmt_expr
+            {
+                $$ = meta_stmt_alloc(&@$, $2, $4);
+            }
+            |    meta_key_unqualified    SET    stmt_expr
+            {
+                $$ = meta_stmt_alloc(&@$, $1, $3);
+            }
+            |    META meta_key_object    SET    stmt_expr
             {
                 switch ($2) {
                 case NFT_META_SECMARK:
@@ -4161,14 +4185,10 @@ meta_stmt        :    META    meta_key    SET
  stmt_expr
                     $$->objref.expr = $4;
                     break;
                 default:
-                    $$ = meta_stmt_alloc(&@$, $2, $4);
-                    break;
+                    erec_queue(error(&@2, "invalid meta object name
'%s'\n", $2), state->msgs);
+                    YYERROR;
                 }
             }
-            |    meta_key_unqualified    SET    stmt_expr
-            {
-                $$ = meta_stmt_alloc(&@$, $1, $3);
-            }
             |    META    STRING    SET    stmt_expr
             {
                 struct error_record *erec;
@@ -4354,6 +4374,7 @@ ct_key            :    L3PROTOCOL    { $$ =
NFT_CT_L3PROTOCOL; }
             |    PROTO_DST    { $$ = NFT_CT_PROTO_DST; }
             |    LABEL        { $$ = NFT_CT_LABELS; }
             |    EVENT        { $$ = NFT_CT_EVENTMASK; }
+            |    SECMARK_RAW    { $$ = NFT_CT_SECMARK; }
             |    ct_key_dir_optional
             ;

diff --git a/src/rule.c b/src/rule.c
index 64756bc..dbbec5e 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -2454,6 +2454,12 @@ static int do_command_reset(struct netlink_ctx
*ctx, struct cmd *cmd)
     case CMD_OBJ_QUOTA:
         type = NFT_OBJECT_QUOTA;
         break;
+    case CMD_OBJ_SECMARKS:
+        dump = true;
+        /* fall through */
+    case CMD_OBJ_SECMARK:
+        type = NFT_OBJECT_SECMARK;
+        break;
     default:
         BUG("invalid command object type %u\n", cmd->obj);
     }
diff --git a/src/scanner.l b/src/scanner.l
index 3de5a9e..feaa691 100644
--- a/src/scanner.l
+++ b/src/scanner.l
@@ -591,6 +591,7 @@ addrstring    ({macaddr}|{ip4addr}|{ip6addr})

 "secmark"        { return SECMARK; }
 "secmarks"        { return SECMARKS; }
+"secmark_raw"        { return SECMARK_RAW; }

 {addrstring}        {
                 yylval->string = xstrdup(yytext);
-- 
2.24.0.rc1

[-- Attachment #2: 0001-add-secmark_raw-for-storing-secmark-id-in-packet-sta.patch --]
[-- Type: text/x-patch, Size: 5736 bytes --]

From c559cb37e09526e02da02724017d0f921a03a1c1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com>
Date: Mon, 28 Oct 2019 15:12:29 +0100
Subject: [PATCH] add secmark_raw for storing secmark id in packet state

---
 src/ct.c           |  2 ++
 src/evaluate.c     |  2 ++
 src/meta.c         |  3 +++
 src/parser_bison.y | 37 +++++++++++++++++++++++++++++--------
 src/rule.c         |  6 ++++++
 src/scanner.l      |  1 +
 6 files changed, 43 insertions(+), 8 deletions(-)

diff --git a/src/ct.c b/src/ct.c
index ed458e6..9e6a835 100644
--- a/src/ct.c
+++ b/src/ct.c
@@ -299,6 +299,8 @@ const struct ct_template ct_templates[__NFT_CT_MAX] = {
 					      BYTEORDER_BIG_ENDIAN, 128),
 	[NFT_CT_DST_IP6]	= CT_TEMPLATE("ip6 daddr", &ip6addr_type,
 					      BYTEORDER_BIG_ENDIAN, 128),
+	[NFT_CT_SECMARK]	= CT_TEMPLATE("secmark", &integer_type,
+					      BYTEORDER_HOST_ENDIAN, 32),
 };
 
 static void ct_print(enum nft_ct_keys key, int8_t dir, uint8_t nfproto,
diff --git a/src/evaluate.c b/src/evaluate.c
index a56cd2a..1b2f5e3 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -3944,8 +3944,10 @@ static int cmd_evaluate_reset(struct eval_ctx *ctx, struct cmd *cmd)
 	switch (cmd->obj) {
 	case CMD_OBJ_COUNTER:
 	case CMD_OBJ_QUOTA:
+	case CMD_OBJ_SECMARK:
 	case CMD_OBJ_COUNTERS:
 	case CMD_OBJ_QUOTAS:
+	case CMD_OBJ_SECMARKS:
 		if (cmd->handle.table.name == NULL)
 			return 0;
 		if (table_lookup(&cmd->handle, &ctx->nft->cache) == NULL)
diff --git a/src/meta.c b/src/meta.c
index f54b818..8093d67 100644
--- a/src/meta.c
+++ b/src/meta.c
@@ -709,6 +709,8 @@ const struct meta_template meta_templates[] = {
 	[NFT_META_TIME_HOUR]	= META_TEMPLATE("hour", &hour_type,
 						4 * BITS_PER_BYTE,
 						BYTEORDER_HOST_ENDIAN),
+	[NFT_META_SECMARK]	= META_TEMPLATE("secmark", &integer_type,
+						32, BYTEORDER_HOST_ENDIAN),
 };
 
 static bool meta_key_is_unqualified(enum nft_meta_keys key)
@@ -720,6 +722,7 @@ static bool meta_key_is_unqualified(enum nft_meta_keys key)
 	case NFT_META_OIFNAME:
 	case NFT_META_IIFGROUP:
 	case NFT_META_OIFGROUP:
+	case NFT_META_SECMARK:
 		return true;
 	default:
 		return false;
diff --git a/src/parser_bison.y b/src/parser_bison.y
index 11f0dc8..16fcea2 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -479,6 +479,7 @@ int nft_lex(void *, void *, void *);
 
 %token SECMARK			"secmark"
 %token SECMARKS			"secmarks"
+%token SECMARK_RAW		"secmark_raw"
 
 %token NANOSECOND		"nanosecond"
 %token MICROSECOND		"microsecond"
@@ -748,7 +749,7 @@ int nft_lex(void *, void *, void *);
 
 %type <expr>			meta_expr
 %destructor { expr_free($$); }	meta_expr
-%type <val>			meta_key	meta_key_qualified	meta_key_unqualified	numgen_type
+%type <val>			meta_key	meta_key_qualified	meta_key_unqualified	meta_key_object	numgen_type
 
 %type <expr>			socket_expr
 %destructor { expr_free($$); } socket_expr
@@ -1365,6 +1366,18 @@ reset_cmd		:	COUNTERS	ruleset_spec
 			{
 				$$ = cmd_alloc(CMD_RESET, CMD_OBJ_QUOTA, &$2, &@$, NULL);
 			}
+			|	SECMARKS	ruleset_spec
+			{
+				$$ = cmd_alloc(CMD_RESET, CMD_OBJ_SECMARKS, &$2, &@$, NULL);
+			}
+			|	SECMARKS	TABLE	table_spec
+			{
+				$$ = cmd_alloc(CMD_RESET, CMD_OBJ_SECMARKS, &$3, &@$, NULL);
+			}
+			|       SECMARK		obj_spec
+			{
+				$$ = cmd_alloc(CMD_RESET, CMD_OBJ_SECMARK, &$2, &@$, NULL);
+			}
 			;
 
 flush_cmd		:	TABLE		table_spec
@@ -4123,7 +4136,7 @@ meta_key_qualified	:	LENGTH		{ $$ = NFT_META_LEN; }
 			|	PROTOCOL	{ $$ = NFT_META_PROTOCOL; }
 			|	PRIORITY	{ $$ = NFT_META_PRIORITY; }
 			|	RANDOM		{ $$ = NFT_META_PRANDOM; }
-			|	SECMARK		{ $$ = NFT_META_SECMARK; }
+			|	SECMARK_RAW	{ $$ = NFT_META_SECMARK; }
 			;
 
 meta_key_unqualified	:	MARK		{ $$ = NFT_META_MARK; }
@@ -4152,7 +4165,18 @@ meta_key_unqualified	:	MARK		{ $$ = NFT_META_MARK; }
 			|       HOUR		{ $$ = NFT_META_TIME_HOUR; }
 			;
 
+meta_key_object		:	SECMARK		{ $$ = NFT_META_SECMARK; }
+			;
+
 meta_stmt		:	META	meta_key	SET	stmt_expr
+			{
+				$$ = meta_stmt_alloc(&@$, $2, $4);
+			}
+			|	meta_key_unqualified	SET	stmt_expr
+			{
+				$$ = meta_stmt_alloc(&@$, $1, $3);
+			}
+			|	META meta_key_object	SET	stmt_expr
 			{
 				switch ($2) {
 				case NFT_META_SECMARK:
@@ -4161,14 +4185,10 @@ meta_stmt		:	META	meta_key	SET	stmt_expr
 					$$->objref.expr = $4;
 					break;
 				default:
-					$$ = meta_stmt_alloc(&@$, $2, $4);
-					break;
+					erec_queue(error(&@2, "invalid meta object name '%s'\n", $2), state->msgs);
+					YYERROR;
 				}
 			}
-			|	meta_key_unqualified	SET	stmt_expr
-			{
-				$$ = meta_stmt_alloc(&@$, $1, $3);
-			}
 			|	META	STRING	SET	stmt_expr
 			{
 				struct error_record *erec;
@@ -4354,6 +4374,7 @@ ct_key			:	L3PROTOCOL	{ $$ = NFT_CT_L3PROTOCOL; }
 			|	PROTO_DST	{ $$ = NFT_CT_PROTO_DST; }
 			|	LABEL		{ $$ = NFT_CT_LABELS; }
 			|	EVENT		{ $$ = NFT_CT_EVENTMASK; }
+			|	SECMARK_RAW	{ $$ = NFT_CT_SECMARK; }
 			|	ct_key_dir_optional
 			;
 
diff --git a/src/rule.c b/src/rule.c
index 64756bc..dbbec5e 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -2454,6 +2454,12 @@ static int do_command_reset(struct netlink_ctx *ctx, struct cmd *cmd)
 	case CMD_OBJ_QUOTA:
 		type = NFT_OBJECT_QUOTA;
 		break;
+	case CMD_OBJ_SECMARKS:
+		dump = true;
+		/* fall through */
+	case CMD_OBJ_SECMARK:
+		type = NFT_OBJECT_SECMARK;
+		break;
 	default:
 		BUG("invalid command object type %u\n", cmd->obj);
 	}
diff --git a/src/scanner.l b/src/scanner.l
index 3de5a9e..feaa691 100644
--- a/src/scanner.l
+++ b/src/scanner.l
@@ -591,6 +591,7 @@ addrstring	({macaddr}|{ip4addr}|{ip6addr})
 
 "secmark"		{ return SECMARK; }
 "secmarks"		{ return SECMARKS; }
+"secmark_raw"		{ return SECMARK_RAW; }
 
 {addrstring}		{
 				yylval->string = xstrdup(yytext);
-- 
2.24.0.rc1


^ permalink raw reply related	[flat|nested] 8+ messages in thread
* Re: nftables: secmark support
  2019-10-28 14:27   ` Christian Göttsche
@ 2019-11-18 16:44     ` Christian Göttsche
  2019-11-18 18:18       ` Pablo Neira Ayuso
  0 siblings, 1 reply; 8+ messages in thread
From: Christian Göttsche @ 2019-11-18 16:44 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel

Am Mo., 28. Okt. 2019 um 15:27 Uhr schrieb Christian Göttsche
<cgzones@googlemail.com>:
> > This is what your patch [6] does, right? If you don't mind to rebase
> > it I can have a look if I can propose you something else than this new
> > keyword.
>
> Attached at the end (base on 707ad229d48f2ba7920541527b755b155ddedcda)

friendly ping; any progress?

rebased against 4a382ec54a8c09df1a625ddc7d32fc06257c596d at
https://paste.debian.net/1116802/

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: nftables: secmark support
  2019-11-18 16:44     ` Christian Göttsche
@ 2019-11-18 18:18       ` Pablo Neira Ayuso
  2019-11-18 18:30         ` Pablo Neira Ayuso
  0 siblings, 1 reply; 8+ messages in thread
From: Pablo Neira Ayuso @ 2019-11-18 18:18 UTC (permalink / raw)
  To: Christian Göttsche; +Cc: netfilter-devel

Hi Christian,

On Mon, Nov 18, 2019 at 05:44:07PM +0100, Christian Göttsche wrote:
> Am Mo., 28. Okt. 2019 um 15:27 Uhr schrieb Christian Göttsche
> <cgzones@googlemail.com>:
> > > This is what your patch [6] does, right? If you don't mind to rebase
> > > it I can have a look if I can propose you something else than this new
> > > keyword.
> >
> > Attached at the end (base on 707ad229d48f2ba7920541527b755b155ddedcda)
> 
> friendly ping; any progress?
> 
> rebased against 4a382ec54a8c09df1a625ddc7d32fc06257c596d at
> https://paste.debian.net/1116802/

Thanks for following up on this. A few comments on your patch:

1) I would replace secmark_raw by secmark instead. I think we should
   hide this assymmetry to the user. I would suggest you also extend
   the evaluation phase, ie. expr_evaluate_meta() and expr_evaluate_ct()
   to bail out in case the user tries to match on the raw packet / ct
   secmark ID. IIRC, the only usecase for this raw ID is to save and
   to restore the secmark from/to the packet to/from the conntrack
   object.

And a few minor issues:

2) Please remove meta_key_unqualified chunk.

        meta_key_unqualified    SET stmt_expr

3) Remove the reset command chunk too:

--- a/src/rule.c
+++ b/src/rule.c
@@ -2539,6 +2539,12 @@ static int do_command_reset(struct netlink_ctx
*ctx, struct cmd *cmd)
        case CMD_OBJ_QUOTA:
                type = NFT_OBJECT_QUOTA;
                break;
+       case CMD_OBJ_SECMARKS:
+               dump = true;
+               /* fall through */
+       case CMD_OBJ_SECMARK:
+               type = NFT_OBJECT_SECMARK;
+               break;
        default:
                BUG("invalid command object type %u\n", cmd->obj);
        }

Thanks.

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: nftables: secmark support
  2019-11-18 18:18       ` Pablo Neira Ayuso
@ 2019-11-18 18:30         ` Pablo Neira Ayuso
  2019-11-19 19:02           ` Christian Göttsche
  0 siblings, 1 reply; 8+ messages in thread
From: Pablo Neira Ayuso @ 2019-11-18 18:30 UTC (permalink / raw)
  To: Christian Göttsche; +Cc: netfilter-devel

On Mon, Nov 18, 2019 at 07:18:49PM +0100, Pablo Neira Ayuso wrote:
> Hi Christian,
> 
> On Mon, Nov 18, 2019 at 05:44:07PM +0100, Christian Göttsche wrote:
> > Am Mo., 28. Okt. 2019 um 15:27 Uhr schrieb Christian Göttsche
> > <cgzones@googlemail.com>:
> > > > This is what your patch [6] does, right? If you don't mind to rebase
> > > > it I can have a look if I can propose you something else than this new
> > > > keyword.
> > >
> > > Attached at the end (base on 707ad229d48f2ba7920541527b755b155ddedcda)
> > 
> > friendly ping; any progress?
> > 
> > rebased against 4a382ec54a8c09df1a625ddc7d32fc06257c596d at
> > https://paste.debian.net/1116802/
> 
> Thanks for following up on this. A few comments on your patch:
> 
> 1) I would replace secmark_raw by secmark instead. I think we should
>    hide this assymmetry to the user. I would suggest you also extend
>    the evaluation phase, ie. expr_evaluate_meta() and expr_evaluate_ct()
>    to bail out in case the user tries to match on the raw packet / ct
>    secmark ID. IIRC, the only usecase for this raw ID is to save and
>    to restore the secmark from/to the packet to/from the conntrack
>    object.
> 
> And a few minor issues:
> 
> 2) Please remove meta_key_unqualified chunk.
> 
>         meta_key_unqualified    SET stmt_expr

I mean, this update (moving the location of this rule) is not
necessary, right? Thanks.

> 3) Remove the reset command chunk too:
> 
> --- a/src/rule.c
> +++ b/src/rule.c
> @@ -2539,6 +2539,12 @@ static int do_command_reset(struct netlink_ctx
> *ctx, struct cmd *cmd)
>         case CMD_OBJ_QUOTA:
>                 type = NFT_OBJECT_QUOTA;
>                 break;
> +       case CMD_OBJ_SECMARKS:
> +               dump = true;
> +               /* fall through */
> +       case CMD_OBJ_SECMARK:
> +               type = NFT_OBJECT_SECMARK;
> +               break;
>         default:
>                 BUG("invalid command object type %u\n", cmd->obj);
>         }
> 
> Thanks.

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: nftables: secmark support
  2019-11-18 18:30         ` Pablo Neira Ayuso
@ 2019-11-19 19:02           ` Christian Göttsche
  2019-11-19 19:40             ` Pablo Neira Ayuso
  0 siblings, 1 reply; 8+ messages in thread
From: Christian Göttsche @ 2019-11-19 19:02 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel

> > 1) I would replace secmark_raw by secmark instead. I think we should
> >    hide this assymmetry to the user. I would suggest you also extend
> >    the evaluation phase, ie. expr_evaluate_meta() and expr_evaluate_ct()
> >    to bail out in case the user tries to match on the raw packet / ct
> >    secmark ID. IIRC, the only usecase for this raw ID is to save and
> >    to restore the secmark from/to the packet to/from the conntrack
> >    object.
> >
> > And a few minor issues:
> >
> > 2) Please remove meta_key_unqualified chunk.
> >
> >         meta_key_unqualified    SET stmt_expr
>
> I mean, this update (moving the location of this rule) is not
> necessary, right? Thanks.

Without these, I am stuck with

$ ./src/nft -c -f files/examples/secmark.nft
files/examples/secmark.nft:64:49-58: Error: Counter expression must be constant
                ct state established,related meta secmark set ct secmark
                                                              ^^^^^^^^^^

using https://salsa.debian.org/cgzones-guest/nftables/compare/master...secmark_v2

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: nftables: secmark support
  2019-11-19 19:02           ` Christian Göttsche
@ 2019-11-19 19:40             ` Pablo Neira Ayuso
  0 siblings, 0 replies; 8+ messages in thread
From: Pablo Neira Ayuso @ 2019-11-19 19:40 UTC (permalink / raw)
  To: Christian Göttsche; +Cc: netfilter-devel

On Tue, Nov 19, 2019 at 08:02:10PM +0100, Christian Göttsche wrote:
> > > 1) I would replace secmark_raw by secmark instead. I think we should
> > >    hide this assymmetry to the user. I would suggest you also extend
> > >    the evaluation phase, ie. expr_evaluate_meta() and expr_evaluate_ct()
> > >    to bail out in case the user tries to match on the raw packet / ct
> > >    secmark ID. IIRC, the only usecase for this raw ID is to save and
> > >    to restore the secmark from/to the packet to/from the conntrack
> > >    object.
> > >
> > > And a few minor issues:
> > >
> > > 2) Please remove meta_key_unqualified chunk.
> > >
> > >         meta_key_unqualified    SET stmt_expr
> >
> > I mean, this update (moving the location of this rule) is not
> > necessary, right? Thanks.
>
> Without these, I am stuck with
>
> $ ./src/nft -c -f files/examples/secmark.nft
> files/examples/secmark.nft:64:49-58: Error: Counter expression must be constant
>                 ct state established,related meta secmark set ct secmark
>                                                               ^^^^^^^^^^

meta_stmt               :       META    meta_key        SET stmt_expr
                        {
                                switch ($2) {
                                case NFT_META_SECMARK:
                                        $$ = objref_stmt_alloc(&@$);
                                        $$->objref.type = NFT_OBJECT_SECMARK;
                                        $$->objref.expr = $4;

Check for what type of expression you have on $4 from the parser code.
If this is EXPR_META or EXPR_CT, then this is restoring a value. If
that is the case, then you have to use meta_stmt_alloc(), not
objref_stmt_alloc(), since this is not a reference to object.

^ permalink raw reply	[flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-11-19 19:40 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-22 15:57 nftables: secmark support Christian Göttsche
2019-10-22 17:34 ` Pablo Neira Ayuso
2019-10-28 14:27   ` Christian Göttsche
2019-11-18 16:44     ` Christian Göttsche
2019-11-18 18:18       ` Pablo Neira Ayuso
2019-11-18 18:30         ` Pablo Neira Ayuso
2019-11-19 19:02           ` Christian Göttsche
2019-11-19 19:40             ` Pablo Neira Ayuso

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).