[PATCH nft] src: add cookie support for rules

[PATCH nft] src: add cookie support for rules

[Pablo Neira Ayuso]

This patch allows users to specify a unsigned 64-bit cookie for rules.
The userspace application assigns the cookie number for tracking the rule.
The cookie needs to be non-zero. This cookie value is only relevant to
userspace since this resides in the user data area.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
Phil, you suggested a cookie to track rules, here it is. A few notes:

- This patch is missing json support.
- No need for kernel update since the cookie is stored in the user data area.

 include/rule.h            |  3 ++-
 src/netlink_delinearize.c | 22 +++++++++++++++-------
 src/netlink_linearize.c   | 16 ++++++++++++----
 src/parser_bison.y        | 18 +++++++++++++++++-
 src/rule.c                |  2 ++
 src/scanner.l             |  1 +
 6 files changed, 49 insertions(+), 13 deletions(-)

diff --git a/include/rule.h b/include/rule.h
index 60eadfa3c9a2..0dd29625e613 100644
--- a/include/rule.h
+++ b/include/rule.h
@@ -275,8 +275,9 @@ struct rule {
 	struct location		location;
 	struct list_head	stmts;
 	unsigned int		num_stmts;
-	const char		*comment;
 	unsigned int		refcnt;
+	const char		*comment;
+	uint64_t		cookie;
 };
 
 extern struct rule *rule_alloc(const struct location *loc,
diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index 9e3ed53d09f1..7a25f5b0ddf4 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -2816,6 +2816,10 @@ static int parse_rule_udata_cb(const struct nftnl_udata *attr, void *data)
 		if (value[len - 1] != '\0')
 			return -1;
 		break;
+	case NFTNL_UDATA_RULE_COOKIE:
+		if (len != sizeof(uint64_t))
+			return -1;
+		break;
 	default:
 		return 0;
 	}
@@ -2823,24 +2827,28 @@ static int parse_rule_udata_cb(const struct nftnl_udata *attr, void *data)
 	return 0;
 }
 
-static char *nftnl_rule_get_comment(const struct nftnl_rule *nlr)
+static int nftnl_rule_get_userdata(const struct nftnl_rule *nlr, struct rule *rule)
 {
 	const struct nftnl_udata *tb[NFTNL_UDATA_RULE_MAX + 1] = {};
 	const void *data;
 	uint32_t len;
 
 	if (!nftnl_rule_is_set(nlr, NFTNL_RULE_USERDATA))
-		return NULL;
+		return 0;
 
 	data = nftnl_rule_get_data(nlr, NFTNL_RULE_USERDATA, &len);
 
 	if (nftnl_udata_parse(data, len, parse_rule_udata_cb, tb) < 0)
-		return NULL;
+		return -1;
 
-	if (!tb[NFTNL_UDATA_RULE_COMMENT])
-		return NULL;
+	if (tb[NFTNL_UDATA_RULE_COMMENT]) {
+		rule->comment =
+			xstrdup(nftnl_udata_get(tb[NFTNL_UDATA_RULE_COMMENT]));
+	}
+	if (tb[NFTNL_UDATA_RULE_COOKIE])
+		rule->cookie = nftnl_udata_get_u64(tb[NFTNL_UDATA_RULE_COOKIE]);
 
-	return xstrdup(nftnl_udata_get(tb[NFTNL_UDATA_RULE_COMMENT]));
+	return 0;
 }
 
 struct rule *netlink_delinearize_rule(struct netlink_ctx *ctx,
@@ -2866,7 +2874,7 @@ struct rule *netlink_delinearize_rule(struct netlink_ctx *ctx,
 	pctx->table = table_lookup(&h, &ctx->nft->cache);
 	assert(pctx->table != NULL);
 
-	pctx->rule->comment = nftnl_rule_get_comment(nlr);
+	nftnl_rule_get_userdata(nlr, pctx->rule);
 
 	nftnl_expr_foreach(nlr, netlink_parse_rule_expr, pctx);
 
diff --git a/src/netlink_linearize.c b/src/netlink_linearize.c
index 846df46b75fd..a5d86449d7f2 100644
--- a/src/netlink_linearize.c
+++ b/src/netlink_linearize.c
@@ -1516,6 +1516,7 @@ static void netlink_gen_stmt(struct netlink_linearize_ctx *ctx,
 void netlink_linearize_rule(struct netlink_ctx *ctx, struct nftnl_rule *nlr,
 			    const struct rule *rule)
 {
+	struct nftnl_udata_buf *udata = NULL;
 	struct netlink_linearize_ctx lctx;
 	const struct stmt *stmt;
 
@@ -1526,20 +1527,27 @@ void netlink_linearize_rule(struct netlink_ctx *ctx, struct nftnl_rule *nlr,
 	list_for_each_entry(stmt, &rule->stmts, list)
 		netlink_gen_stmt(&lctx, stmt);
 
-	if (rule->comment) {
-		struct nftnl_udata_buf *udata;
-
+	if (rule->comment || rule->cookie) {
 		udata = nftnl_udata_buf_alloc(NFT_USERDATA_MAXLEN);
 		if (!udata)
 			memory_allocation_error();
+	}
 
+	if (rule->comment) {
 		if (!nftnl_udata_put_strz(udata, NFTNL_UDATA_RULE_COMMENT,
 					  rule->comment))
 			memory_allocation_error();
+	}
+	if (rule->cookie) {
+		if (!nftnl_udata_put_u64(udata, NFTNL_UDATA_RULE_COOKIE,
+					 rule->cookie))
+			memory_allocation_error();
+	}
+
+	if (udata) {
 		nftnl_rule_set_data(nlr, NFTNL_RULE_USERDATA,
 				    nftnl_udata_buf_data(udata),
 				    nftnl_udata_buf_len(udata));
-
 		nftnl_udata_buf_free(udata);
 	}
 
diff --git a/src/parser_bison.y b/src/parser_bison.y
index 167c315810ed..9d982900eb94 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -543,6 +543,7 @@ int nft_lex(void *, void *, void *);
 %token POSITION			"position"
 %token INDEX			"index"
 %token COMMENT			"comment"
+%token COOKIE			"cookie"
 
 %token XML			"xml"
 %token JSON			"json"
@@ -568,7 +569,7 @@ int nft_lex(void *, void *, void *);
 %type <string>			identifier type_identifier string comment_spec
 %destructor { xfree($$); }	identifier type_identifier string comment_spec
 
-%type <val>			time_spec quota_used
+%type <val>			time_spec quota_used cookie_spec
 
 %type <expr>			data_type_expr data_type_atom_expr
 %destructor { expr_free($$); }  data_type_expr data_type_atom_expr
@@ -2482,6 +2483,12 @@ comment_spec		:	COMMENT		string
 			}
 			;
 
+cookie_spec		:	COOKIE		NUM
+			{
+				$$ = $2;
+			}
+			;
+
 ruleset_spec		:	/* empty */
 			{
 				memset(&$$, 0, sizeof($$));
@@ -2502,6 +2509,15 @@ rule			:	rule_alloc
 			{
 				$$->comment = $2;
 			}
+			|	rule_alloc	cookie_spec
+			{
+				$$->cookie = $2;
+			}
+			|	rule_alloc	cookie_spec comment_spec
+			{
+				$$->cookie = $2;
+				$$->comment = $3;
+			}
 			;
 
 rule_alloc		:	stmt_list
diff --git a/src/rule.c b/src/rule.c
index 6335aa2189ad..551a7dc01ce2 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -664,6 +664,8 @@ void rule_print(const struct rule *rule, struct output_ctx *octx)
 			nft_print(octx, " ");
 	}
 
+	if (rule->cookie)
+		nft_print(octx, " cookie %"PRIu64"", rule->cookie);
 	if (rule->comment)
 		nft_print(octx, " comment \"%s\"", rule->comment);
 
diff --git a/src/scanner.l b/src/scanner.l
index 45699c85d7d0..7117ebb918fc 100644
--- a/src/scanner.l
+++ b/src/scanner.l
@@ -300,6 +300,7 @@ addrstring	({macaddr}|{ip4addr}|{ip6addr})
 "position"		{ return POSITION; }
 "index"			{ return INDEX; }
 "comment"		{ return COMMENT; }
+"cookie"		{ return COOKIE; }
 
 "constant"		{ return CONSTANT; }
 "interval"		{ return INTERVAL; }
-- 
2.20.1

Re: [PATCH nft] src: add cookie support for rules

[Eric Garver]

On Tue, Aug 04, 2020 at 04:24:12PM +0200, Pablo Neira Ayuso wrote:
> This patch allows users to specify a unsigned 64-bit cookie for rules.
> The userspace application assigns the cookie number for tracking the rule.
> The cookie needs to be non-zero. This cookie value is only relevant to
> userspace since this resides in the user data area.
> 
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> ---
> Phil, you suggested a cookie to track rules, here it is. A few notes:
> 
> - This patch is missing json support.
> - No need for kernel update since the cookie is stored in the user data area.

It's also missing the ability to delete a rule using the cookie. I guess
this means userspace will have to fetch the ruleset and map a cookie to
rule handle in order to perform the delete.

    # nft add rule inet foobar input tcp dport 666 accept cookie 1234

    # nft list ruleset
    table inet foobar {
        chain input {
            tcp dport 666 accept cookie 1234
        }
    }

    # nft delete rule inet foobar input cookie 1234
    Error: syntax error, unexpected cookie, expecting handle
    delete rule inet foobar input cookie 1234
                                  ^^^^^^

Re: [PATCH nft] src: add cookie support for rules

[Pablo Neira Ayuso]

On Tue, Aug 04, 2020 at 01:38:05PM -0400, Eric Garver wrote:
> On Tue, Aug 04, 2020 at 04:24:12PM +0200, Pablo Neira Ayuso wrote:
> > This patch allows users to specify a unsigned 64-bit cookie for rules.
> > The userspace application assigns the cookie number for tracking the rule.
> > The cookie needs to be non-zero. This cookie value is only relevant to
> > userspace since this resides in the user data area.
> > 
> > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> > ---
> > Phil, you suggested a cookie to track rules, here it is. A few notes:
> > 
> > - This patch is missing json support.
> > - No need for kernel update since the cookie is stored in the user data area.
> 
> It's also missing the ability to delete a rule using the cookie. I guess
> this means userspace will have to fetch the ruleset and map a cookie to
> rule handle in order to perform the delete.

This cookie idea provides an alternative to skip the input and output
json string comparison, you have to combine it with --echo.

You will still need to use the handle to uniquely identify the rule by
processing the echo message (compare the cookie you set in the rule
that is sent to the kernel, instead of comparing strings).

The input and output json string comparison might be a problem in the
midterm. New netlink attributes and old kernels might result in
different input and output json string.

This is not aiming to provide a mechanism to delete rules by other
than the rule handle.