From: Pablo Neira Ayuso <pablo@netfilter.org>
To: "Carlos Falgueras García" <carlosfg@riseup.net>
Cc: netfilter-devel@vger.kernel.org, kaber@trash.net
Subject: Re: [PATCH 2/3 v2] libnftnl: rule: Change the "userdata" attribute to use new TLV buffer.
Date: Wed, 2 Mar 2016 19:41:29 +0100 [thread overview]
Message-ID: <20160302184129.GC1351@salvia> (raw)
In-Reply-To: <1456763140-15121-2-git-send-email-carlosfg@riseup.net>
On Mon, Feb 29, 2016 at 05:25:39PM +0100, Carlos Falgueras García wrote:
> Now is it possible to store multiple variable length user data into a rule.
> Modify XML and JSON parsers to support this new feature.
>
> Signed-off-by: Carlos Falgueras García <carlosfg@riseup.net>
> ---
> include/json.h | 7 ++
> include/utils.h | 2 +
> include/xml.h | 5 ++
> src/jansson.c | 41 +++++++++
> src/mxml.c | 39 ++++++++
> src/rule.c | 271 ++++++++++++++++++++++++++++++++++++++++++++++++++------
> src/utils.c | 45 ++++++++++
> 7 files changed, 385 insertions(+), 25 deletions(-)
>
> diff --git a/include/json.h b/include/json.h
> index bd70cec..8b64f00 100644
> --- a/include/json.h
> +++ b/include/json.h
> @@ -3,6 +3,7 @@
>
> #ifdef JSON_PARSING
> #include <jansson.h>
> +#include <libnftnl/attr.h>
> #include <stdbool.h>
> #include "common.h"
>
> @@ -51,6 +52,12 @@ int nftnl_jansson_parse_elem(struct nftnl_set *s, json_t *tree,
>
> int nftnl_data_reg_json_parse(union nftnl_data_reg *reg, json_t *data,
> struct nftnl_parse_err *err);
> +
> +int nftnl_jansson_attr_parse(struct nftnl_attrbuf *attrbuf,
> + json_t *root,
> + struct nftnl_parse_err *err,
> + struct nftnl_set_list *set_list);
> +
> #else
> #define json_t void
> #endif
> diff --git a/include/utils.h b/include/utils.h
> index 0087dbb..1bbabff 100644
> --- a/include/utils.h
> +++ b/include/utils.h
> @@ -69,6 +69,8 @@ enum nftnl_type {
> int nftnl_strtoi(const char *string, int base, void *number, enum nftnl_type type);
> int nftnl_get_value(enum nftnl_type type, void *val, void *out);
>
> +char *str2value(const char *str, size_t strlen);
> +
> const char *nftnl_verdict2str(uint32_t verdict);
> int nftnl_str2verdict(const char *verdict, int *verdict_num);
>
> diff --git a/include/xml.h b/include/xml.h
> index 7b33a83..a43ea57 100644
> --- a/include/xml.h
> +++ b/include/xml.h
> @@ -4,6 +4,7 @@
> #ifdef XML_PARSING
> #include <mxml.h>
> #include "common.h"
> +#include <libnftnl/attr.h>
>
> #define NFTNL_XML_MAND 0
> #define NFTNL_XML_OPT (1 << 0)
> @@ -51,6 +52,10 @@ int nftnl_mxml_set_parse(mxml_node_t *tree, struct nftnl_set *s,
>
> int nftnl_data_reg_xml_parse(union nftnl_data_reg *reg, mxml_node_t *tree,
> struct nftnl_parse_err *err);
> +
> +int nftnl_mxml_attr_parse(struct nftnl_attrbuf *attrbuf, mxml_node_t *tree,
> + uint32_t mxml_flags, uint16_t flags,
> + struct nftnl_parse_err *err);
> #else
> #define mxml_node_t void
> #endif
> diff --git a/src/jansson.c b/src/jansson.c
> index 3476ed2..9fff7b5 100644
> --- a/src/jansson.c
> +++ b/src/jansson.c
> @@ -19,6 +19,7 @@
> #include <libnftnl/set.h>
>
> #include <libnftnl/expr.h>
> +#include <libnftnl/attr.h>
> #include <linux/netfilter/nf_tables.h>
>
> #ifdef JSON_PARSING
> @@ -276,4 +277,44 @@ int nftnl_jansson_set_elem_parse(struct nftnl_set_elem *e, json_t *root,
>
> return 0;
> }
> +
> +int nftnl_jansson_attr_parse(struct nftnl_attrbuf *attrbuf,
> + json_t *root,
> + struct nftnl_parse_err *err,
> + struct nftnl_set_list *set_list)
> +{
> + int ret = 0;
> + uint8_t type;
> + uint8_t len;
> + const char *value_str;
> + char *value = NULL;
> +
> + ret = nftnl_jansson_parse_val(root, "type", NFTNL_TYPE_U8, &type, err);
> + if (ret != 0)
> + return -1;
> +
> + ret = nftnl_jansson_parse_val(root, "length", NFTNL_TYPE_U8, &len, err);
> + if (ret != 0)
> + return -1;
> +
> + value_str = nftnl_jansson_parse_str(root, "value", err);
> + if (ret != 0)
> + return -1;
> +
> + if (strlen(value_str) != 2*len)
> + return -1;
> +
> + value = str2value(value_str, 2*len);
> + if (!value) {
> + perror("nftnl_jansson_attr_parse");
> + exit(EXIT_FAILURE);
You shouldn't call exit() in a library.
Please, pass up the error code as return value.
> + }
> +
> + if (!nftnl_attr_put_check(attrbuf, type, len, (void *)value))
> + ret = -1;
> +
> + free(value);
> + return ret;
> +}
> +
> #endif
> diff --git a/src/mxml.c b/src/mxml.c
> index 51dbf1b..793a89d 100644
> --- a/src/mxml.c
> +++ b/src/mxml.c
> @@ -229,4 +229,43 @@ int nftnl_mxml_family_parse(mxml_node_t *tree, const char *node_name,
>
> return family;
> }
> +
> +int nftnl_mxml_attr_parse(struct nftnl_attrbuf *attrbuf, mxml_node_t *tree,
> + uint32_t mxml_flags, uint16_t flags,
> + struct nftnl_parse_err *err)
> +{
> + int ret = 0;
> + uint8_t len;
> + uint8_t type;
> + const char *value_str;
> + char *value = NULL;
> +
> + if (nftnl_mxml_num_parse(tree, "type", mxml_flags, BASE_DEC, &type,
> + NFTNL_TYPE_U8, flags, err) < 0)
> + return -1;
> +
> + if (nftnl_mxml_num_parse(tree, "length", mxml_flags, BASE_DEC, &len,
> + NFTNL_TYPE_U8, flags, err) < 0)
> + return -1;
> +
> + value_str = nftnl_mxml_str_parse(tree, "value", mxml_flags, flags, err);
> + if (value_str == NULL)
> + return -1;
> +
> + if (strlen(value_str) != 2*len)
> + return -1;
> +
> + value = str2value(value_str, 2*len);
> + if (!value) {
> + perror("nftnl_mxml_attr_parse");
> + exit(EXIT_FAILURE);
> + }
> +
> + if (!nftnl_attr_put_check(attrbuf, type, len, (void *)value))
> + ret = -1;
> +
> + free(value);
> + return ret;
> +}
> +
> #endif
> diff --git a/src/rule.c b/src/rule.c
> index 3a32bf6..4a5dc8c 100644
> --- a/src/rule.c
> +++ b/src/rule.c
> @@ -19,7 +19,6 @@
> #include <netinet/in.h>
> #include <errno.h>
> #include <inttypes.h>
> -#include <ctype.h>
>
> #include <libmnl/libmnl.h>
> #include <linux/netfilter/nfnetlink.h>
> @@ -28,6 +27,7 @@
> #include <libnftnl/rule.h>
> #include <libnftnl/set.h>
> #include <libnftnl/expr.h>
> +#include <libnftnl/attr.h>
>
> struct nftnl_rule {
> struct list_head head;
> @@ -38,10 +38,7 @@ struct nftnl_rule {
> const char *chain;
> uint64_t handle;
> uint64_t position;
> - struct {
> - void *data;
> - uint32_t len;
> - } user;
> + struct nftnl_attrbuf *userdata;
> struct {
> uint32_t flags;
> uint32_t proto;
> @@ -50,6 +47,15 @@ struct nftnl_rule {
> struct list_head expr_list;
> };
>
> +static size_t nftnl_rule_snprintf_data2str(char *buf, size_t size,
> + const void *data, size_t datalen);
> +static size_t nftnl_rule_snprintf_default_attr(char *buf, size_t size,
> + const struct nftnl_attr *attr);
> +static size_t nftnl_rule_snprintf_xml_attr(char *buf, size_t size,
> + const struct nftnl_attr *attr);
> +static size_t nftnl_rule_snprintf_json_attr(char *buf, size_t size,
> + const struct nftnl_attr *attr);
> +
> struct nftnl_rule *nftnl_rule_alloc(void)
> {
> struct nftnl_rule *r;
> @@ -75,6 +81,8 @@ void nftnl_rule_free(struct nftnl_rule *r)
> xfree(r->table);
> if (r->chain != NULL)
> xfree(r->chain);
> + if (r->flags & (1 << NFTNL_RULE_USERDATA))
> + nftnl_attrbuf_free(r->userdata);
>
> xfree(r);
> }
> @@ -162,8 +170,12 @@ void nftnl_rule_set_data(struct nftnl_rule *r, uint16_t attr,
> r->position = *((uint64_t *)data);
> break;
> case NFTNL_RULE_USERDATA:
> - r->user.data = (void *)data;
> - r->user.len = data_len;
> + (r->userdata = nftnl_attrbuf_alloc(data_len));
> + if (!r->userdata) {
> + perror("nftnl_rule_set_data - userdata");
> + exit(EXIT_FAILURE);
> + }
> + nftnl_attrbuf_copy_data(r->userdata, data, data_len);
> break;
> }
> r->flags |= (1 << attr);
> @@ -221,8 +233,8 @@ const void *nftnl_rule_get_data(const struct nftnl_rule *r, uint16_t attr,
> *data_len = sizeof(uint64_t);
> return &r->position;
> case NFTNL_RULE_USERDATA:
> - *data_len = r->user.len;
> - return r->user.data;
> + *data_len = nftnl_attrbuf_get_len(r->userdata);
> + return (void *)nftnl_attrbuf_get_data(r->userdata);
> }
> return NULL;
> }
> @@ -288,8 +300,9 @@ void nftnl_rule_nlmsg_build_payload(struct nlmsghdr *nlh, struct nftnl_rule *r)
> if (r->flags & (1 << NFTNL_RULE_POSITION))
> mnl_attr_put_u64(nlh, NFTA_RULE_POSITION, htobe64(r->position));
> if (r->flags & (1 << NFTNL_RULE_USERDATA)) {
> - mnl_attr_put(nlh, NFTA_RULE_USERDATA, r->user.len,
> - r->user.data);
> + mnl_attr_put(nlh, NFTA_RULE_USERDATA,
> + nftnl_attrbuf_get_len(r->userdata),
> + nftnl_attrbuf_get_data(r->userdata));
> }
>
> if (!list_empty(&r->expr_list)) {
> @@ -447,19 +460,17 @@ int nftnl_rule_nlmsg_parse(const struct nlmsghdr *nlh, struct nftnl_rule *r)
> r->flags |= (1 << NFTNL_RULE_POSITION);
> }
> if (tb[NFTA_RULE_USERDATA]) {
> + uint16_t udata_size;
> const void *udata =
> mnl_attr_get_payload(tb[NFTA_RULE_USERDATA]);
>
> - if (r->user.data)
> - xfree(r->user.data);
> -
> - r->user.len = mnl_attr_get_payload_len(tb[NFTA_RULE_USERDATA]);
> + udata_size = mnl_attr_get_payload_len(tb[NFTA_RULE_USERDATA]);
>
> - r->user.data = malloc(r->user.len);
> - if (r->user.data == NULL)
> + (r->userdata = nftnl_attrbuf_alloc(udata_size));
> + if (!r->userdata)
> return -1;
> + nftnl_attrbuf_copy_data(r->userdata, udata, udata_size);
>
> - memcpy(r->user.data, udata, r->user.len);
> r->flags |= (1 << NFTNL_RULE_USERDATA);
> }
>
> @@ -481,6 +492,7 @@ int nftnl_jansson_parse_rule(struct nftnl_rule *r, json_t *tree,
> uint64_t uval64;
> uint32_t uval32;
> int i, family;
> + struct nftnl_attrbuf *attrbuf;
>
> root = nftnl_jansson_get_node(tree, "rule", err);
> if (root == NULL)
> @@ -557,6 +569,31 @@ int nftnl_jansson_parse_rule(struct nftnl_rule *r, json_t *tree,
> nftnl_rule_add_expr(r, e);
> }
>
Please, wrap these code below into a function, eg. nftnl_jansson_udata_parse()
> + array = json_object_get(root, "userdata");
> + if (array == NULL) {
> + err->error = NFTNL_PARSE_EMISSINGNODE;
> + err->node_name = "userdata";
> + goto err;
> + }
> +
> + attrbuf = nftnl_attrbuf_alloc(NFT_USERDATA_MAXLEN);
> + if (!attrbuf) {
> + perror("nftnl_jansson_parse_rule");
> + exit(EXIT_FAILURE);
> + }
> +
> + for (i = 0; i < json_array_size(array); ++i) {
> + if (nftnl_jansson_attr_parse(attrbuf,
> + json_array_get(array, i),
> + err,
> + set_list) < 0)
> + goto err;
> + }
> +
> + nftnl_rule_set_data(r, NFTNL_RULE_USERDATA,
> + nftnl_attrbuf_get_data(attrbuf),
> + nftnl_attrbuf_get_len(attrbuf));
> +
> return 0;
> err:
> return -1;
> @@ -592,7 +629,7 @@ int nftnl_mxml_rule_parse(mxml_node_t *tree, struct nftnl_rule *r,
> struct nftnl_parse_err *err,
> struct nftnl_set_list *set_list)
> {
> - mxml_node_t *node;
> + mxml_node_t *node, *node_ud;
> struct nftnl_expr *e;
> const char *table, *chain;
> int family;
> @@ -649,6 +686,35 @@ int nftnl_mxml_rule_parse(mxml_node_t *tree, struct nftnl_rule *r,
> nftnl_rule_add_expr(r, e);
> }
>
> + node_ud = mxmlFindElement(tree, tree, "userdata", NULL, NULL,
> + MXML_DESCEND);
> + if (node_ud) {
Please, wrap these code below into a function, eg. nftnl_mxml_udata_parse()
> + struct nftnl_attrbuf *attrbuf;
> +
> + attrbuf = nftnl_attrbuf_alloc(NFT_USERDATA_MAXLEN);
> + if (!attrbuf) {
> + perror("nftnl_mxml_rule_parse");
> + exit(EXIT_FAILURE);
> + }
> +
> + /* Iterate over attributes */
> + for (
> + node = mxmlFindElement(node_ud, node_ud, "attr", NULL,
> + NULL, MXML_DESCEND);
> + node != NULL;
> + node = mxmlFindElement(node, node_ud, "attr", NULL,
> + NULL, MXML_DESCEND)
> + ) {
> + if (nftnl_mxml_attr_parse(attrbuf, node,
> + MXML_DESCEND_FIRST, r->flags, err) < 0)
> + return -1;
> + }
> +
> + nftnl_rule_set_data(r, NFTNL_RULE_USERDATA,
> + nftnl_attrbuf_get_data(attrbuf),
> + nftnl_attrbuf_get_len(attrbuf));
> + }
> +
> return 0;
> }
> #endif
> @@ -711,6 +777,21 @@ int nftnl_rule_parse_file(struct nftnl_rule *r, enum nftnl_parse_type type,
> }
> EXPORT_SYMBOL_ALIAS(nftnl_rule_parse_file, nft_rule_parse_file);
>
> +static size_t nftnl_rule_snprintf_data2str(char *buf, size_t size,
> + const void *data, size_t datalen)
> +{
> + int i;
> + size_t ret, len = size, offset = 0;
> + const unsigned char *str = data;
> +
> + for (i = 0; i < datalen; i++) {
> + ret = snprintf(buf+offset, len, "%02X", str[i]);
> + SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
> + }
> +
> + return offset;
> +}
> +
> static int nftnl_rule_snprintf_json(char *buf, size_t size, struct nftnl_rule *r,
> uint32_t type, uint32_t flags)
> {
> @@ -757,6 +838,31 @@ static int nftnl_rule_snprintf_json(char *buf, size_t size, struct nftnl_rule *r
> SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
> }
>
> + if (r->flags & (1 << NFTNL_RULE_USERDATA)) {
> + const struct nftnl_attr *attr;
> +
> + ret = snprintf(buf+offset, len, "\"userdata\":[");
> + SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
> +
> + nftnl_attr_for_each(attr, r->userdata) {
> + ret = nftnl_rule_snprintf_json_attr(buf+offset, len,
> + attr);
> + SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
> +
> + ret = snprintf(buf+offset, len, ",");
> + SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
> + }
> + /* delete last comma */
> + buf[offset-1] = '\0';
> + offset--;
> + size--;
> + len++;
> +
> + ret = snprintf(buf+offset, len, "],\n");
> + SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
> +
> + }
> +
> ret = snprintf(buf+offset, len, "\"expr\":[");
> SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
>
> @@ -849,17 +955,123 @@ static int nftnl_rule_snprintf_xml(char *buf, size_t size, struct nftnl_rule *r,
> SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
>
> }
> +
> + if (r->flags & (1 << NFTNL_RULE_USERDATA)) {
> + const struct nftnl_attr *attr;
> +
> + ret = snprintf(buf+offset, len, "<userdata>");
> + SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
> +
> + nftnl_attr_for_each(attr, r->userdata) {
> + ret = snprintf(buf+offset, len, "<attr>");
> + SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
> +
> + ret = nftnl_rule_snprintf_xml_attr(buf+offset, len,
> + attr);
> + SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
> +
> + ret = snprintf(buf+offset, len, "</attr>");
> + SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
> + }
> +
> + ret = snprintf(buf+offset, len, "</userdata>");
> + SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
> + }
> +
> ret = snprintf(buf+offset, len, "</rule>");
> SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
>
> return offset;
> }
>
> +static size_t nftnl_rule_snprintf_xml_attr(char *buf, size_t size,
> + const struct nftnl_attr *attr)
[...]
> +static size_t nftnl_rule_snprintf_json_attr(char *buf, size_t size,
> + const struct nftnl_attr *attr)
You can consolidate these two functions into one using the NFTNL_BUF_*
API.
> diff --git a/src/utils.c b/src/utils.c
> index ba36bc4..0cac4b6 100644
> --- a/src/utils.c
> +++ b/src/utils.c
> @@ -139,6 +139,51 @@ int nftnl_strtoi(const char *string, int base, void *out, enum nftnl_type type)
> return ret;
> }
>
> +static int hex2char(char *out, char c)
> +{
> + /* numbers */
> + if (c >= 0x30 && c <= 0x39)
> + *out = c - 0x30;
> + /* lowercase characters */
> + else if (c >= 0x61 && c <= 0x66)
> + *out = c - 0x61 + 10;
> + /* uppercase characters */
> + else if (c >= 0x41 && c <= 0x46)
> + *out = c - 0x41 + 10;
> + else {
> + errno = EINVAL;
> + return 0;
> + }
You can just print data in hexadecimal, so we can get rid of this.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2016-03-02 18:41 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-02-29 16:25 [PATCH 1/3 v2] libnftnl: Implement new buffer of TLV objects Carlos Falgueras García
2016-02-29 16:25 ` [PATCH 2/3 v2] libnftnl: rule: Change the "userdata" attribute to use new TLV buffer Carlos Falgueras García
2016-03-02 18:41 ` Pablo Neira Ayuso [this message]
2016-03-08 9:58 ` Carlos Falgueras García
2016-03-08 10:12 ` Pablo Neira Ayuso
2016-02-29 16:25 ` [PATCH 3/3 v2] nftables: rule: Change the field "rule->comment" for an nftnl_attrbuf Carlos Falgueras García
2016-03-02 18:37 ` Pablo Neira Ayuso
2016-03-08 9:58 ` Carlos Falgueras García
2016-03-08 10:13 ` Pablo Neira Ayuso
2016-03-02 18:32 ` [PATCH 1/3 v2] libnftnl: Implement new buffer of TLV objects Pablo Neira Ayuso
2016-03-08 9:57 ` Carlos Falgueras García
2016-03-08 10:10 ` Pablo Neira Ayuso
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160302184129.GC1351@salvia \
--to=pablo@netfilter.org \
--cc=carlosfg@riseup.net \
--cc=kaber@trash.net \
--cc=netfilter-devel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.