src: add comment support for objects

Enables specifying an optional comment when declaring named objects. The
comment is to be specified inside the object's block ({} block)

Relies on libnftnl exporting nftnl_obj_get_data and kernel space support
to store the comments.

For consistency, this patch makes the comment be printed first when
listing objects.

Adds a testcase importing all commented named objects except for secmark,
although it's supported.

Example: Adding a quota with a comment

> add table inet filter
> nft add quota inet filter q { over 1200 bytes \; comment "test_comment"\; }
> list ruleset

table inet filter {
	quota q {
		comment "test_comment"
		over 1200 bytes
	}
}

Signed-off-by: Jose M. Guisado Gomez <guigom@riseup.net>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

diff --git a/include/rule.h b/include/rule.h
index 56f1951f4d8958bb76e270f714e566a55e747fbd..837005b16eb9f5cd1dc27465a833f0939756b766 100644 (file)
--- a/include/rule.h
+++ b/include/rule.h
@@ -479,6 +479,7 @@ struct obj {
        struct handle                   handle;
        uint32_t                        type;
        unsigned int                    refcnt;
+       const char                      *comment;
        union {
                struct counter          counter;
                struct quota            quota;

diff --git a/src/mnl.c b/src/mnl.c
index cdcf94909d912e00acace58187b61433e04bacb8..ca4f4b2acda9108fe59724c71960bde0723f60fe 100644 (file)
--- a/src/mnl.c
+++ b/src/mnl.c
@@ -1189,6 +1189,7 @@ int mnl_nft_obj_add(struct netlink_ctx *ctx, struct cmd *cmd,
                    unsigned int flags)
 {
        struct obj *obj = cmd->object;
+       struct nftnl_udata_buf *udbuf;
        struct nftnl_obj *nlo;
        struct nlmsghdr *nlh;
 
@@ -1199,6 +1200,17 @@ int mnl_nft_obj_add(struct netlink_ctx *ctx, struct cmd *cmd,
        nftnl_obj_set_u32(nlo, NFTNL_OBJ_FAMILY, cmd->handle.family);
        nftnl_obj_set_u32(nlo, NFTNL_OBJ_TYPE, obj->type);
 
+       if (obj->comment) {
+               udbuf = nftnl_udata_buf_alloc(NFT_USERDATA_MAXLEN);
+               if (!udbuf)
+                       memory_allocation_error();
+               if (!nftnl_udata_put_strz(udbuf, NFTNL_UDATA_OBJ_COMMENT, obj->comment))
+                       memory_allocation_error();
+               nftnl_obj_set_data(nlo, NFTNL_OBJ_USERDATA, nftnl_udata_buf_data(udbuf),
+                                    nftnl_udata_buf_len(udbuf));
+               nftnl_udata_buf_free(udbuf);
+       }
+
        switch (obj->type) {
        case NFT_OBJECT_COUNTER:
                nftnl_obj_set_u64(nlo, NFTNL_OBJ_CTR_PKTS,

diff --git a/src/netlink.c b/src/netlink.c
index a107f492e8ede9a46f98b3e4ca3859e56113ba6f..6912b018ba5c864d3335cbf8031218f48f4e897b 100644 (file)
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -1314,11 +1314,33 @@ void netlink_dump_obj(struct nftnl_obj *nln, struct netlink_ctx *ctx)
        fprintf(fp, "\n");
 }
 
+static int obj_parse_udata_cb(const struct nftnl_udata *attr, void *data)
+{
+       unsigned char *value = nftnl_udata_get(attr);
+       uint8_t type = nftnl_udata_type(attr);
+       const struct nftnl_udata **tb = data;
+       uint8_t len = nftnl_udata_len(attr);
+
+       switch (type) {
+               case NFTNL_UDATA_OBJ_COMMENT:
+                       if (value[len - 1] != '\0')
+                               return -1;
+                       break;
+               default:
+                       return 0;
+       }
+       tb[type] = attr;
+       return 0;
+}
+
 struct obj *netlink_delinearize_obj(struct netlink_ctx *ctx,
                                    struct nftnl_obj *nlo)
 {
+       const struct nftnl_udata *ud[NFTNL_UDATA_OBJ_MAX + 1] = {};
+       const char *udata;
        struct obj *obj;
        uint32_t type;
+       uint32_t ulen;
 
        obj = obj_alloc(&netlink_location);
        obj->handle.family = nftnl_obj_get_u32(nlo, NFTNL_OBJ_FAMILY);
@@ -1328,6 +1350,15 @@ struct obj *netlink_delinearize_obj(struct netlink_ctx *ctx,
                xstrdup(nftnl_obj_get_str(nlo, NFTNL_OBJ_NAME));
        obj->handle.handle.id =
                nftnl_obj_get_u64(nlo, NFTNL_OBJ_HANDLE);
+       if (nftnl_obj_is_set(nlo, NFTNL_OBJ_USERDATA)) {
+               udata = nftnl_obj_get_data(nlo, NFTNL_OBJ_USERDATA, &ulen);
+               if (nftnl_udata_parse(udata, ulen, obj_parse_udata_cb, ud) < 0) {
+                       netlink_io_error(ctx, NULL, "Cannot parse userdata");
+                       return NULL;
+               }
+               if (ud[NFTNL_UDATA_OBJ_COMMENT])
+                       obj->comment = xstrdup(nftnl_udata_get(ud[NFTNL_UDATA_OBJ_COMMENT]));
+       }
 
        type = nftnl_obj_get_u32(nlo, NFTNL_OBJ_TYPE);
        switch (type) {

diff --git a/src/parser_bison.y b/src/parser_bison.y
index d938f5665a678201792e1fed8e4d9e923f69be05..7242c4c3d6f75883edb2e2caa2b5a714c49a75bd 100644 (file)
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -1006,10 +1006,18 @@ add_cmd                 :       TABLE           table_spec
                        {
                                $$ = cmd_alloc(CMD_ADD, CMD_OBJ_COUNTER, &$2, &@$, $3);
                        }
+                       |       COUNTER         obj_spec        counter_obj     '{' counter_block '}'
+                       {
+                               $$ = cmd_alloc(CMD_ADD, CMD_OBJ_COUNTER, &$2, &@$, $3);
+                       }
                        |       QUOTA           obj_spec        quota_obj       quota_config
                        {
                                $$ = cmd_alloc(CMD_ADD, CMD_OBJ_QUOTA, &$2, &@$, $3);
                        }
+                       |       QUOTA           obj_spec        quota_obj       '{' quota_block '}'
+                       {
+                               $$ = cmd_alloc(CMD_ADD, CMD_OBJ_QUOTA, &$2, &@$, $3);
+                       }
                        |       CT      HELPER  obj_spec        ct_obj_alloc    '{' ct_helper_block '}'
                        {
                                $$ = cmd_alloc_obj_ct(CMD_ADD, NFT_OBJECT_CT_HELPER, &$3, &@$, $4);
@@ -1026,14 +1034,26 @@ add_cmd                 :       TABLE           table_spec
                        {
                                $$ = cmd_alloc(CMD_ADD, CMD_OBJ_LIMIT, &$2, &@$, $3);
                        }
+                       |       LIMIT           obj_spec        limit_obj       '{' limit_block '}'
+                       {
+                               $$ = cmd_alloc(CMD_ADD, CMD_OBJ_LIMIT, &$2, &@$, $3);
+                       }
                        |       SECMARK         obj_spec        secmark_obj     secmark_config
                        {
                                $$ = cmd_alloc(CMD_ADD, CMD_OBJ_SECMARK, &$2, &@$, $3);
                        }
+                       |       SECMARK         obj_spec        secmark_obj     '{' secmark_block '}'
+                       {
+                               $$ = cmd_alloc(CMD_ADD, CMD_OBJ_SECMARK, &$2, &@$, $3);
+                       }
                        |       SYNPROXY        obj_spec        synproxy_obj    synproxy_config
                        {
                                $$ = cmd_alloc(CMD_ADD, CMD_OBJ_SYNPROXY, &$2, &@$, $3);
                        }
+                       |       SYNPROXY        obj_spec        synproxy_obj    '{' synproxy_block '}'
+                       {
+                               $$ = cmd_alloc(CMD_ADD, CMD_OBJ_SYNPROXY, &$2, &@$, $3);
+                       }
                        ;
 
 replace_cmd            :       RULE            ruleid_spec     rule
@@ -2039,6 +2059,10 @@ counter_block            :       /* empty */     { $$ = $<obj>-1; }
                        {
                                $$ = $1;
                        }
+                       |       counter_block     comment_spec
+                       {
+                               $<obj>1->comment = $2;
+                       }
                        ;
 
 quota_block            :       /* empty */     { $$ = $<obj>-1; }
@@ -2048,6 +2072,10 @@ quota_block              :       /* empty */     { $$ = $<obj>-1; }
                        {
                                $$ = $1;
                        }
+                       |       quota_block     comment_spec
+                       {
+                               $<obj>1->comment = $2;
+                       }
                        ;
 
 ct_helper_block                :       /* empty */     { $$ = $<obj>-1; }
@@ -2057,6 +2085,10 @@ ct_helper_block          :       /* empty */     { $$ = $<obj>-1; }
                        {
                                $$ = $1;
                        }
+                       |       ct_helper_block     comment_spec
+                       {
+                               $<obj>1->comment = $2;
+                       }
                        ;
 
 ct_timeout_block       :       /*empty */
@@ -2070,6 +2102,10 @@ ct_timeout_block :       /*empty */
                        {
                                $$ = $1;
                        }
+                       |       ct_timeout_block     comment_spec
+                       {
+                               $<obj>1->comment = $2;
+                       }
                        ;
 
 ct_expect_block                :       /*empty */      { $$ = $<obj>-1; }
@@ -2079,6 +2115,10 @@ ct_expect_block          :       /*empty */      { $$ = $<obj>-1; }
                        {
                                $$ = $1;
                        }
+                       |       ct_expect_block     comment_spec
+                       {
+                               $<obj>1->comment = $2;
+                       }
                        ;
 
 limit_block            :       /* empty */     { $$ = $<obj>-1; }
@@ -2088,6 +2128,10 @@ limit_block              :       /* empty */     { $$ = $<obj>-1; }
                        {
                                $$ = $1;
                        }
+                       |       limit_block     comment_spec
+                       {
+                               $<obj>1->comment = $2;
+                       }
                        ;
 
 secmark_block          :       /* empty */     { $$ = $<obj>-1; }
@@ -2097,6 +2141,10 @@ secmark_block            :       /* empty */     { $$ = $<obj>-1; }
                        {
                                $$ = $1;
                        }
+                       |       secmark_block     comment_spec
+                       {
+                               $<obj>1->comment = $2;
+                       }
                        ;
 
 synproxy_block         :       /* empty */     { $$ = $<obj>-1; }
@@ -2106,6 +2154,10 @@ synproxy_block           :       /* empty */     { $$ = $<obj>-1; }
                        {
                                $$ = $1;
                        }
+                       |       synproxy_block     comment_spec
+                       {
+                               $<obj>1->comment = $2;
+                       }
                        ;
 
 type_identifier                :       STRING  { $$ = $1; }

diff --git a/src/rule.c b/src/rule.c
index 2c4b5dbe2e9cc7514d6434dcabb4d787da4fc66e..dabb35791a164be64ff8d67bcf574e65b7e4d229 100644 (file)
--- a/src/rule.c
+++ b/src/rule.c
@@ -1893,6 +1893,7 @@ void obj_free(struct obj *obj)
 {
        if (--obj->refcnt > 0)
                return;
+       xfree(obj->comment);
        handle_free(&obj->handle);
        xfree(obj);
 }
@@ -1986,6 +1987,16 @@ static const char *synproxy_timestamp_to_str(const uint32_t flags)
         return "";
 }
 
+static void obj_print_comment(const struct obj *obj,
+                             struct print_fmt_options *opts,
+                             struct output_ctx *octx)
+{
+       if (obj->comment)
+               nft_print(octx, "%s%s%scomment \"%s\"",
+                         opts->nl, opts->tab, opts->tab,
+                         obj->comment);
+}
+
 static void obj_print_data(const struct obj *obj,
                           struct print_fmt_options *opts,
                           struct output_ctx *octx)
@@ -1995,6 +2006,8 @@ static void obj_print_data(const struct obj *obj,
                nft_print(octx, " %s {", obj->handle.obj.name);
                if (nft_output_handle(octx))
                        nft_print(octx, " # handle %" PRIu64, obj->handle.handle.id);
+
+               obj_print_comment(obj, opts, octx);
                nft_print(octx, "%s%s%s", opts->nl, opts->tab, opts->tab);
                if (nft_output_stateless(octx)) {
                        nft_print(octx, "packets 0 bytes 0");
@@ -2010,6 +2023,8 @@ static void obj_print_data(const struct obj *obj,
                nft_print(octx, " %s {", obj->handle.obj.name);
                if (nft_output_handle(octx))
                        nft_print(octx, " # handle %" PRIu64, obj->handle.handle.id);
+
+               obj_print_comment(obj, opts, octx);
                nft_print(octx, "%s%s%s", opts->nl, opts->tab, opts->tab);
                data_unit = get_rate(obj->quota.bytes, &bytes);
                nft_print(octx, "%s%" PRIu64 " %s",
@@ -2027,6 +2042,8 @@ static void obj_print_data(const struct obj *obj,
                nft_print(octx, " %s {", obj->handle.obj.name);
                if (nft_output_handle(octx))
                        nft_print(octx, " # handle %" PRIu64, obj->handle.handle.id);
+
+               obj_print_comment(obj, opts, octx);
                nft_print(octx, "%s%s%s", opts->nl, opts->tab, opts->tab);
                nft_print(octx, "\"%s\"%s", obj->secmark.ctx, opts->nl);
                break;
@@ -2034,6 +2051,8 @@ static void obj_print_data(const struct obj *obj,
                nft_print(octx, " %s {", obj->handle.obj.name);
                if (nft_output_handle(octx))
                        nft_print(octx, " # handle %" PRIu64, obj->handle.handle.id);
+
+               obj_print_comment(obj, opts, octx);
                nft_print(octx, "%s", opts->nl);
                nft_print(octx, "%s%stype \"%s\" protocol ",
                          opts->tab, opts->tab, obj->ct_helper.name);
@@ -2048,6 +2067,8 @@ static void obj_print_data(const struct obj *obj,
                nft_print(octx, " %s {", obj->handle.obj.name);
                if (nft_output_handle(octx))
                        nft_print(octx, " # handle %" PRIu64, obj->handle.handle.id);
+
+               obj_print_comment(obj, opts, octx);
                nft_print(octx, "%s", opts->nl);
                nft_print(octx, "%s%sprotocol ", opts->tab, opts->tab);
                print_proto_name_proto(obj->ct_timeout.l4proto, octx);
@@ -2063,6 +2084,8 @@ static void obj_print_data(const struct obj *obj,
                nft_print(octx, " %s {", obj->handle.obj.name);
                if (nft_output_handle(octx))
                        nft_print(octx, " # handle %" PRIu64, obj->handle.handle.id);
+
+               obj_print_comment(obj, opts, octx);
                nft_print(octx, "%s", opts->nl);
                nft_print(octx, "%s%sprotocol ", opts->tab, opts->tab);
                print_proto_name_proto(obj->ct_expect.l4proto, octx);
@@ -2091,6 +2114,8 @@ static void obj_print_data(const struct obj *obj,
                nft_print(octx, " %s {", obj->handle.obj.name);
                if (nft_output_handle(octx))
                        nft_print(octx, " # handle %" PRIu64, obj->handle.handle.id);
+
+               obj_print_comment(obj, opts, octx);
                nft_print(octx, "%s%s%s", opts->nl, opts->tab, opts->tab);
                switch (obj->limit.type) {
                case NFT_LIMIT_PKTS:
@@ -2128,6 +2153,8 @@ static void obj_print_data(const struct obj *obj,
                if (nft_output_handle(octx))
                        nft_print(octx, " # handle %" PRIu64, obj->handle.handle.id);
 
+               obj_print_comment(obj, opts, octx);
+
                if (flags & NF_SYNPROXY_OPT_MSS) {
                        nft_print(octx, "%s%s%s", opts->nl, opts->tab, opts->tab);
                        nft_print(octx, "mss %u", obj->synproxy.mss);

diff --git a/tests/shell/testcases/optionals/comments_objects_0 b/tests/shell/testcases/optionals/comments_objects_0
new file mode 100755 (executable)
index 0000000..7437c77
--- /dev/null
+++ b/tests/shell/testcases/optionals/comments_objects_0
@@ -0,0 +1,44 @@
+#!/bin/bash
+
+EXPECTED='table ip filter {
+       quota q {
+               over 1200 bytes
+               comment "test1"
+       }
+
+       counter c {
+               packets 0 bytes 0
+               comment "test2"
+       }
+
+       ct helper h {
+               type "sip" protocol tcp
+               l3proto ip
+               comment "test3"
+       }
+
+       ct expectation e {
+               protocol tcp
+               dport 666
+               timeout 100ms
+               size 96
+               l3proto ip
+               comment "test4"
+       }
+
+       limit l {
+               rate 400/hour
+               comment "test5"
+       }
+
+       synproxy s {
+               mss 1460
+               wscale 2
+               comment "test6"
+       }
+}
+'
+
+set -e
+
+$NFT -f - <<< "$EXPECTED"

diff --git a/tests/shell/testcases/optionals/dumps/comments_objects_0.nft b/tests/shell/testcases/optionals/dumps/comments_objects_0.nft
new file mode 100644 (file)
index 0000000..b760ced
--- /dev/null
+++ b/tests/shell/testcases/optionals/dumps/comments_objects_0.nft
@@ -0,0 +1,37 @@
+table ip filter {
+       quota q {
+               comment "test1"
+               over 1200 bytes
+       }
+
+       counter c {
+               comment "test2"
+               packets 0 bytes 0
+       }
+
+       ct helper h {
+               comment "test3"
+               type "sip" protocol tcp
+               l3proto ip
+       }
+
+       ct expectation e {
+               comment "test4"
+               protocol tcp
+               dport 666
+               timeout 100ms
+               size 96
+               l3proto ip
+       }
+
+       limit l {
+               comment "test5"
+               rate 400/hour
+       }
+
+       synproxy s {
+               comment "test6"
+               mss 1460
+               wscale 2
+       }
+}