data_reg: xml: fix bytes movements

PATCH Applied Arturo Borrero 2 participants 7 messages Jun 3, 2013 – Jun 5, 2013 netfilter

lore

Share & tools

lore

Arturo Borrero

author

2013-06-03 20:44 UTC #0 lore raw

PATCH 1/5 data_reg: xml: fix bytes movements

There was bad bits movements and calcules when XML printing/parsing.

Signed-off-by Arturo Borrero Gonzalez <[email protected]>

---
 src/expr/data_reg.c |   12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

src/expr/data_reg.c +8 -4

index 74ebe76..7d26175 100644
@@ -15,6 +15,7 @@
 #include <limits.h>
 #include <arpa/inet.h>
 #include <errno.h>
+#include <netinet/in.h>
 
 #include <libmnl/libmnl.h>
 #include <linux/netfilter.h>

@@ -205,7 +206,7 @@ static int nft_data_reg_value_xml_parse(union nft_data_reg *reg, char *xml)
 		reg->val[i] = utmp;
 	}
 
-	reg->len = sizeof(reg->val);
+	reg->len = len*sizeof(reg->val[0]);
 
 	mxmlDelete(tree);
 	return 0;

@@ -259,8 +260,9 @@ int nft_data_reg_value_snprintf_xml(char *buf, size_t size,
 				    union nft_data_reg *reg, uint32_t flags)
 {
 	int len = size, offset = 0, ret, i, j;
+	uint32_t be;
 	uint8_t *tmp;
-	int data_len = reg->len/sizeof(uint32_t);
+	int data_len = reg->len/sizeof(reg->val[0]);
 
 	ret = snprintf(buf, len, "<data_reg type=\"value\">");
 	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);

@@ -272,15 +274,17 @@ int nft_data_reg_value_snprintf_xml(char *buf, size_t size,
 		ret = snprintf(buf+offset, len, "<data%d>0x", i);
 		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
 
-		tmp = (uint8_t *)&reg->val[i];
+		be = htonl(reg->val[i]);
+		tmp = (uint8_t *)&be;
 
-		for (j=0; j<sizeof(int); j++) {
+		for (j = 0; j < sizeof(uint32_t); j++) {
 			ret = snprintf(buf+offset, len, "%.02x", tmp[j]);
 			SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
 		}
 
 		ret = snprintf(buf+offset, len, "</data%d>", i);
 		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+
 	}
 
 	ret = snprintf(buf+offset, len, "</data_reg>");

Arturo Borrero

author

2013-06-03 20:44 UTC #1 lore raw

PATCH 2/5 rule: fix snprintf return value

This cause some chained snprintf to fail.

Signed-off-by Arturo Borrero Gonzalez <[email protected]>

---
 src/rule.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

src/rule.c +2 -2

index f0208d9..a91d21d 100644
@@ -676,7 +676,7 @@ static int nft_rule_snprintf_xml(char *buf, size_t size, struct nft_rule *r,
 	ret = snprintf(buf+offset, len, "</rule>");
 	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
 
-	return ret;
+	return offset;
 }
 
 static int nft_rule_snprintf_default(char *buf, size_t size, struct nft_rule *r,

@@ -699,7 +699,7 @@ static int nft_rule_snprintf_default(char *buf, size_t size, struct nft_rule *r,
 		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
 	}
 
-	return ret;
+	return offset;
 }
 
 int nft_rule_snprintf(char *buf, size_t size, struct nft_rule *r,

Pablo Neira Ayuso

maintainer Applied

2013-06-05 03:09 UTC #2 lore raw

Re: [libnftables PATCH 2/5] rule: fix snprintf return value

Quoted 1 line · Arturo Borrero

This cause some chained snprintf to fail.

Applied, thanks.

Arturo Borrero

author

2013-06-03 20:44 UTC #3 lore raw

PATCH 3/5 src: xml: set errno to EINVAL when invalid parsing

In reply to Arturo Borrero

This patch sets errno to EINVAL when the XML parsing fails due to a bad format, a missing node or something.

Signed-off-by Arturo Borrero Gonzalez <[email protected]>

---
 src/chain.c |  141 +++++++++++++++++++++++++----------------------------------
 src/rule.c  |  129 ++++++++++++++++++++++--------------------------------
 src/table.c |   58 ++++++++++--------------
 3 files changed, 137 insertions(+), 191 deletions(-)

src/chain.c +60 ~~-81~~

index 71d84a9..8dc9a49 100644
@@ -467,73 +467,61 @@ static int nft_chain_xml_parse(struct nft_chain *c, char *xml)
 
 	/* Load the tree */
 	tree = mxmlLoadString(NULL, xml, MXML_OPAQUE_CALLBACK);
-	if (tree == NULL)
+	if (tree == NULL) {
+		errno = EINVAL;
 		return -1;
+	}
 
 	/* Validate this is a <chain> node */
-	if (strcmp(tree->value.opaque, "chain") != 0) {
-		mxmlDelete(tree);
-		return -1;
-	}
+	if (strcmp(tree->value.opaque, "chain") != 0)
+		goto err;
 
 	/* Validate version */
-	if (mxmlElementGetAttr(tree, "version") == NULL) {
-		mxmlDelete(tree);
-		return -1;
-	}
+	if (mxmlElementGetAttr(tree, "version") == NULL)
+		goto err;
+
 	tmp = strtoll(mxmlElementGetAttr(tree, "version"), &endptr, 10);
-	if (tmp == LLONG_MAX || *endptr || tmp != NFT_CHAIN_XML_VERSION) {
-		mxmlDelete(tree);
-		return -1;
-	}
+	if (tmp == LLONG_MAX || *endptr || tmp != NFT_CHAIN_XML_VERSION)
+		goto err;
 
 	/* Get and set <chain name="xxx" ... >*/
-	if (mxmlElementGetAttr(tree, "name") == NULL) {
-		mxmlDelete(tree);
-		return -1;
-	}
+	if (mxmlElementGetAttr(tree, "name") == NULL)
+		goto err;
+
 	strncpy(c->name, mxmlElementGetAttr(tree, "name"),
 		NFT_CHAIN_MAXNAMELEN);
 	c->flags |= (1 << NFT_CHAIN_ATTR_NAME);
 
 	/* Get and set <chain handle="x" ... >*/
-	if (mxmlElementGetAttr(tree, "handle") == NULL) {
-		mxmlDelete(tree);
-		return -1;
-	}
+	if (mxmlElementGetAttr(tree, "handle") == NULL)
+		goto err;
 
 	utmp = strtoull(mxmlElementGetAttr(tree, "handle"), &endptr, 10);
-	if (utmp == UINT64_MAX || utmp < 0 || *endptr) {
-		mxmlDelete(tree);
-		return -1;
-	}
+	if (utmp == UINT64_MAX || utmp < 0 || *endptr)
+		goto err;
 
 	c->handle = utmp;
 	c->flags |= (1 << NFT_CHAIN_ATTR_HANDLE);
 
 	/* Get and set <chain bytes="x" ... >*/
-	if (mxmlElementGetAttr(tree, "bytes") == NULL) {
-		mxmlDelete(tree);
-		return -1;
-	}
+	if (mxmlElementGetAttr(tree, "bytes") == NULL)
+		goto err;
+
 	utmp = strtoull(mxmlElementGetAttr(tree, "bytes"), &endptr, 10);
-	if (utmp == UINT64_MAX || utmp < 0 || *endptr) {
-		mxmlDelete(tree);
-		return -1;
-	}
+	if (utmp == UINT64_MAX || utmp < 0 || *endptr)
+		goto err;
+
 	c->bytes = utmp;
 	c->flags |= (1 << NFT_CHAIN_ATTR_BYTES);
 
 	/* Get and set <chain packets="x" ... > */
-	if (mxmlElementGetAttr(tree, "packets") == NULL) {
-		mxmlDelete(tree);
-		return -1;
-	}
+	if (mxmlElementGetAttr(tree, "packets") == NULL)
+		goto err;
+
 	utmp = strtoull(mxmlElementGetAttr(tree, "packets"), &endptr, 10);
-	if (utmp == UINT64_MAX || utmp < 0 || *endptr) {
-		mxmlDelete(tree);
-		return -1;
-	}
+	if (utmp == UINT64_MAX || utmp < 0 || *endptr)
+		goto err;
+
 	c->packets = utmp;
 	c->flags |= (1 << NFT_CHAIN_ATTR_PACKETS);

@@ -543,10 +531,8 @@ static int nft_chain_xml_parse(struct nft_chain *c, char *xml)
 
 	/* Get and set <type> */
 	node = mxmlFindElement(tree, tree, "type", NULL, NULL, MXML_DESCEND);
-	if (node == NULL) {
-		mxmlDelete(tree);
-		return -1;
-	}
+	if (node == NULL)
+		goto err;
 
 	if (c->type)
 		free(c->type);

@@ -556,10 +542,9 @@ static int nft_chain_xml_parse(struct nft_chain *c, char *xml)
 
 	/* Get and set <table> */
 	node = mxmlFindElement(tree, tree, "table", NULL, NULL,	MXML_DESCEND);
-	if (node == NULL) {
-		mxmlDelete(tree);
-		return -1;
-	}
+	if (node == NULL)
+		goto err;
+
 	if (c->table)
 		free(c->table);

@@ -568,15 +553,12 @@ static int nft_chain_xml_parse(struct nft_chain *c, char *xml)
 
 	/* Get and set <prio> */
 	node = mxmlFindElement(tree, tree, "prio", NULL, NULL, MXML_DESCEND);
-	if (node == NULL) {
-		mxmlDelete(tree);
-		return -1;
-	}
+	if (node == NULL)
+		goto err;
+
 	tmp = strtoll(node->child->value.opaque, &endptr, 10);
-	if (tmp > INT32_MAX || tmp < INT32_MIN || *endptr) {
-		mxmlDelete(tree);
-		return -1;
-	}
+	if (tmp > INT32_MAX || tmp < INT32_MIN || *endptr)
+		goto err;
 
 	memcpy(&c->prio, &tmp, sizeof(c->prio));
 	c->flags |= (1 << NFT_CHAIN_ATTR_PRIO);

@@ -587,51 +569,48 @@ static int nft_chain_xml_parse(struct nft_chain *c, char *xml)
 	/* Get and set <hooknum> */
 	node = mxmlFindElement(tree, tree, "hooknum", NULL, NULL,
 			       MXML_DESCEND);
-	if (node == NULL) {
-		mxmlDelete(tree);
-		return -1;
-	}
+	if (node == NULL)
+		goto err;
+
 	utmp = strtoull(node->child->value.opaque, &endptr, 10);
-	if (utmp > UINT32_MAX || utmp < 0 || *endptr) {
-		mxmlDelete(tree);
-		return -1;
-	}
+	if (utmp > UINT32_MAX || utmp < 0 || *endptr)
+		goto err;
 
 	memcpy(&c->hooknum, &utmp, sizeof(c->hooknum));
 	c->flags |= (1 << NFT_CHAIN_ATTR_HOOKNUM);
 
 	/* Get and set <policy> */
 	node = mxmlFindElement(tree, tree, "policy", NULL, NULL, MXML_DESCEND);
-	if (node == NULL) {
-		mxmlDelete(tree);
-		return -1;
-	}
+	if (node == NULL)
+		goto err;
+
 	utmp = strtoull(node->child->value.opaque, &endptr, 10);
-	if (utmp > UINT32_MAX || utmp < 0 || *endptr) {
-		mxmlDelete(tree);
-		return -1;
-	}
+	if (utmp > UINT32_MAX || utmp < 0 || *endptr)
+		goto err;
 
 	c->policy = (uint32_t)utmp;
 	c->flags |= (1 << NFT_CHAIN_ATTR_POLICY);
 
 	/* Get and set <family> */
 	node = mxmlFindElement(tree, tree, "family", NULL, NULL, MXML_DESCEND);
-	if (node == NULL) {
-		mxmlDelete(tree);
-		return -1;
-	}
+	if (node == NULL)
+		goto err;
+
 	utmp = strtoull(node->child->value.opaque, &endptr, 10);
-	if (utmp > UINT8_MAX || utmp < 0 || *endptr) {
-		mxmlDelete(tree);
-		return -1;
-	}
+	if (utmp > UINT8_MAX || utmp < 0 || *endptr)
+		goto err;
 
 	c->family = (uint32_t)utmp;
 	c->flags |= (1 << NFT_CHAIN_ATTR_FAMILY);
 
 	mxmlDelete(tree);
 	return 0;
+
+err:
+	/* The XML format is invalid */
+	errno = EINVAL;
+	mxmlDelete(tree);
+	return -1;
 #else
 	errno = EOPNOTSUPP;
 	return -1;

src/rule.c +52 ~~-77~~

206 lines not on this page · raw message

src/table.c +25 ~~-33~~

95 lines not on this page · raw message

Pablo Neira Ayuso

maintainer

2013-06-05 03:38 UTC #4 lore raw

Re: [libnftables PATCH 3/5] src: xml: set errno to EINVAL when invalid parsing

Quoted 1 line · Arturo Borrero

This patch sets errno to EINVAL when the XML parsing fails due to a bad format, a missing node or something.

I like this patch but it doesn't apply cleanly without the versioning stuff.

Since the versioning looks more tricky, can you rebase this patch upon the current tree and resend?

Thanks.

Arturo Borrero

author

2013-06-03 20:44 UTC #5 lore raw

PATCH 4/5 expr: xml: don't print target&match info

In reply to Arturo Borrero

As is not supported in parsing, don't print at all target&match <info> attribute.

However, I think this could be easily supported, with the drawback of having a XML file full of binary data.

Signed-off-by Arturo Borrero Gonzalez <[email protected]>

---
 src/expr/match.c  |   12 +-----------
 src/expr/target.c |   12 +-----------
 2 files changed, 2 insertions(+), 22 deletions(-)

src/expr/match.c +1 ~~-11~~

index edb78ea..4f8d8fc 100644
@@ -245,22 +245,12 @@ static int nft_rule_expr_match_snprintf_xml(char *buf, size_t len,
 					    struct nft_expr_match *mt)
 {
 	int ret, size=len;
-	int i;
 	int offset = 0;
-	uint8_t *data = (uint8_t *)mt->data;
 
-	ret = snprintf(buf, len, "<name>%s</name><rev>%u</rev><info>0x",
+	ret = snprintf(buf, len, "<name>%s</name><rev>%u</rev>",
 				mt->name, mt->rev);
 	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
 
-	for (i=0; i < mt->data_len; i++) {
-	        ret = snprintf(buf+offset, len, "%x", data[i] & 0xff);
-		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
-	}
-
-	ret = snprintf(buf+offset, len, "</info>");
-	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
-
 	return offset;
 }

src/expr/target.c +1 ~~-11~~

index 6652c47..bcdf034 100644
@@ -249,22 +249,12 @@ int nft_rule_exp_target_snprintf_xml(char *buf, size_t len,
 				struct nft_expr_target *tg)
 {
 	int ret, size=len;
-	int i;
 	int offset = 0;
-	uint8_t *data = (uint8_t *)tg->data;
 
-	ret = snprintf(buf, len, "<name>%s</name><rev>%u</rev><info>0x",
+	ret = snprintf(buf, len, "<name>%s</name><rev>%u</rev>",
 			tg->name, tg->rev);
 	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
 
-	for (i=0; i < tg->data_len; i++) {
-		ret = snprintf(buf+offset, len, "%x", data[i] & 0xff);
-		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
-	}
-
-	ret = snprintf(buf+offset, len, "</info>");
-	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
-
 	return offset;
 }

Arturo Borrero

author

2013-06-03 20:45 UTC #6 lore raw

PATCH 5/5 examples: get XML ruleset

In reply to Arturo Borrero

This example code prints out the current ruleset in XML format. In a near future, when sets are XML-handled this example will be updated.

Take this as a proposal. There are many ways to print out the ruleset in XML.
* Nesting or not rules in each chain.
* Nesting or not each chain in his corresponding table.
* Using other naming scheme for top level elements, like '<nftables>' and/or '<ruleset>'.

Also note that ATM there is no implementation to parse the whole ruleset but objects (table/chain/rule).

Signed-off-by Arturo Borrero Gonzalez <[email protected]>

---
 examples/Makefile.am           |    4 +
 examples/nft-ruleset-xml-get.c |  265 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 269 insertions(+)
 create mode 100644 examples/nft-ruleset-xml-get.c

examples/Makefile.am +4 -0

index dcf798a..49488b8 100644
@@ -13,6 +13,7 @@ check_PROGRAMS = nft-table-add		\
 		 nft-rule-xml-add	\
 		 nft-rule-del		\
 		 nft-rule-get		\
+		 nft-ruleset-xml-get	\
 		 nft-events		\
 		 nft-set-add		\
 		 nft-set-get		\

@@ -61,6 +62,9 @@ nft_rule_del_LDADD = ../src/libnftables.la ${LIBMNL_LIBS} ${LIBXML_LIBS}
 nft_rule_get_SOURCES = nft-rule-get.c
 nft_rule_get_LDADD = ../src/libnftables.la ${LIBMNL_LIBS} ${LIBXML_LIBS}
 
+nft_ruleset_xml_get_SOURCES = nft-ruleset-xml-get.c
+nft_ruleset_xml_get_LDADD = ../src/libnftables.la ${LIBMNL_LIBS} ${LIBXML_LIBS}
+
 nft_events_SOURCES = nft-events.c
 nft_events_LDADD = ../src/libnftables.la ${LIBMNL_LIBS} ${LIBXML_LIBS}

examples/nft-ruleset-xml-get.c new file +265 -0

265 lines not on this page · raw message