* 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).