conntrack: labels: skip labels with non-alnum characters

PATCH New Florian Westphal 2 participants 4 messages Jun 23, 2013 – Jun 25, 2013 netfilter

lore

Share & tools

lore

Florian Westphal

author maintainer

2013-06-23 21:16 UTC #0 lore raw

PATCH 1/2 conntrack: labels: skip labels with non-alnum characters

Can always lift this restriction later but for now enforce strict label naming.

This is mainly to make sure that e.g. using
conntrack ... -o xml,connlabels

will output the expected format, without nasty surprises.

Signed-off-by Florian Westphal <[email protected]>

---
I've split this into a separate patch since it has noting to do with the nfct_snprintf change.

 src/conntrack/labels.c | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

src/conntrack/labels.c +27 -1

index 7393c42..7dfb780 100644
@@ -1,3 +1,4 @@
+#include <stdbool.h>
 #include <stdint.h>
 
 #include "internal/internal.h"

@@ -184,6 +185,30 @@ static struct nfct_labelmap *map_alloc(void)
 	return map;
 }
 
+/*
+ * We will only accept alpha numerical labels; else
+ * parses might choke on output when label named
+ * "foo;<&bar" exists.  ASCII machines only.
+ *
+ * Avoids libc isalnum() etc. to avoid issues with locale
+ * settings.
+ */
+static bool label_is_sane(const char *label)
+{
+	for (;*label; label++) {
+		if (*label >= 'a' && *label <= 'z')
+			continue;
+		if (*label >= 'A' && *label <= 'Z')
+			continue;
+		if (*label >= '0' && *label <= '9')
+			continue;
+		if (*label == ' ' || *label == '-')
+			continue;
+		return false;
+	}
+	return true;
+}
+
 struct nfct_labelmap *__labelmap_new(const char *name)
 {
 	struct nfct_labelmap *map;

@@ -219,7 +244,8 @@ struct nfct_labelmap *__labelmap_new(const char *name)
 		end = trim_label(end);
 		if (!end)
 			continue;
-		if (map_insert(map, end, bit) == 0) {
+
+		if (label_is_sane(end) && map_insert(map, end, bit) == 0) {
 			added++;
 			if (maxbit < bit)
 				maxbit = bit;

Signature · 1 line

1.8.1.5

Florian Westphal

author maintainer

2013-06-23 21:16 UTC #1 lore raw

PATCH v2 2/2 conntrack: snprintf: add connlabel format specifier

By default, nfct_snprintf will not print connlabels, as they're system specific and can easily generate lots of output.

This adds a fmt attribute to print connlabel names.

output looks like this:
... mark=0 use=1 labels=eth0-in,eth1-in
or
<labels>
<label>eth0-in</label>
<label>eth1-in</label>
</labels>

Signed-off-by Florian Westphal <[email protected]>

---
Pablo, I _think_ this is pretty much what you had in mind, if I misunderstood anything please let me know.

Change since V1:
- avoid changing current nfct_snprint default output
- print only name of a label, needs NFCT_OF_CONNLABELS output format

 include/internal/prototypes.h                      |  1 +
 .../libnetfilter_conntrack.h                       |  3 ++
 src/conntrack/snprintf_default.c                   | 60 ++++++++++++++++++++++
 src/conntrack/snprintf_xml.c                       | 37 +++++++++++++
 4 files changed, 101 insertions(+)

include/internal/prototypes.h +1 -0

index 484deea..414d7f8 100644
@@ -15,6 +15,7 @@ int __snprintf_protocol(char *buf, unsigned int len, const struct nf_conntrack *
 int __snprintf_proto(char *buf, unsigned int len, const struct __nfct_tuple *tuple);
 int __snprintf_conntrack_default(char *buf, unsigned int len, const struct nf_conntrack *ct, const unsigned int msg_type, const unsigned int flags);
 int __snprintf_conntrack_xml(char *buf, unsigned int len, const struct nf_conntrack *ct, const unsigned int msg_type, const unsigned int flags);
+int __snprintf_connlabels(char *buf, unsigned int len, struct nfct_labelmap *map, const struct nfct_bitmask *b, const char *fmt);
 
 enum __nfct_addr {
 	__ADDR_SRC = 0,

include/libnetfilter_conntrack/libnetfilter_conntrack.h +3 -0

index 39dc24c..eedf85e 100644
@@ -389,6 +389,9 @@ enum {
 
 	NFCT_OF_TIMESTAMP_BIT = 3,
 	NFCT_OF_TIMESTAMP = (1 << NFCT_OF_TIMESTAMP_BIT),
+
+	NFCT_OF_CONNLABELS_BIT = 4,
+	NFCT_OF_CONNLABELS = (1 << NFCT_OF_CONNLABELS_BIT),
 };
 
 extern int nfct_snprintf(char *buf,

src/conntrack/snprintf_default.c +60 -0

72 lines not on this page · raw message

src/conntrack/snprintf_xml.c +37 -0

index ad53075..27cda04 100644
@@ -348,6 +348,36 @@ static int __snprintf_tuple_xml(char *buf,
 	return size;
 }
 
+static int
+__snprintf_clabels_xml(char *buf, unsigned int len,
+		       const struct nf_conntrack *ct)
+{
+	const struct nfct_bitmask *b = nfct_get_attr(ct, ATTR_CONNLABELS);
+	struct nfct_labelmap *map;
+	int ret, size = 0, offset = 0;
+
+	if (!b)
+		return 0;
+
+	map = nfct_labelmap_new(NULL);
+	if (!map)
+		return 0;
+
+	ret = snprintf(buf, len, "<labels>");
+	BUFFER_SIZE(ret, size, len, offset);
+
+	ret = __snprintf_connlabels(buf + offset, len, map, b, "<label>%s</label>");
+
+	nfct_labelmap_destroy(map);
+
+	BUFFER_SIZE(ret, size, len, offset);
+
+	ret = snprintf(buf + offset, len, "</labels>");
+	BUFFER_SIZE(ret, size, len, offset);
+
+	return size;
+}
+
 int __snprintf_conntrack_xml(char *buf,
 			     unsigned int len,
 			     const struct nf_conntrack *ct,

@@ -390,6 +420,7 @@ int __snprintf_conntrack_xml(char *buf,
 	    test_bit(ATTR_USE, ct->head.set) ||
 	    test_bit(ATTR_STATUS, ct->head.set) ||
 	    test_bit(ATTR_ID, ct->head.set) ||
+	    test_bit(ATTR_CONNLABELS, ct->head.set) ||
 	    test_bit(ATTR_TIMESTAMP_START, ct->head.set) ||
 	    test_bit(ATTR_TIMESTAMP_STOP, ct->head.set)) {
 		ret = snprintf(buf+offset, len,

@@ -432,6 +463,11 @@ int __snprintf_conntrack_xml(char *buf,
 		BUFFER_SIZE(ret, size, len, offset);
 	}
 
+	if ((flags & NFCT_OF_CONNLABELS) && test_bit(ATTR_CONNLABELS, ct->head.set)) {
+		ret = __snprintf_clabels_xml(buf+offset, len, ct);
+		BUFFER_SIZE(ret, size, len, offset);
+	}
+
 	if (test_bit(ATTR_SECMARK, ct->head.set)) {
 		ret = snprintf(buf+offset, len, 
 				"<secmark>%u</secmark>", ct->secmark);

@@ -510,6 +546,7 @@ int __snprintf_conntrack_xml(char *buf,
 	    test_bit(ATTR_USE, ct->head.set) ||
 	    test_bit(ATTR_STATUS, ct->head.set) ||
 	    test_bit(ATTR_ID, ct->head.set) ||
+	    test_bit(ATTR_CONNLABELS, ct->head.set) ||
 	    test_bit(ATTR_TIMESTAMP_START, ct->head.set) ||
 	    test_bit(ATTR_TIMESTAMP_STOP, ct->head.set)) {
 	    	ret = snprintf(buf+offset, len, "</meta>");

Signature · 1 line

1.8.1.5

Pablo Neira Ayuso

maintainer

2013-06-25 15:40 UTC #2 lore raw

Re: [PATCH V2 lnf-ct 2/2] conntrack: snprintf: add connlabel format specifier

Quoted 89 lines · Florian Westphal

By default, nfct_snprintf will not print connlabels, as they're system specific and can easily generate lots of output.

This adds a fmt attribute to print connlabel names.

output looks like this:
... mark=0 use=1 labels=eth0-in,eth1-in
or
<labels>
<label>eth0-in</label>
<label>eth1-in</label>
</labels>

Signed-off-by Florian Westphal <[email protected]>

---
Pablo, I _think_ this is pretty much what you had in mind, if I misunderstood anything please let me know.

Change since V1:
- avoid changing current nfct_snprint default output
- print only name of a label, needs NFCT_OF_CONNLABELS output format

 include/internal/prototypes.h                      |  1 +
 .../libnetfilter_conntrack.h                       |  3 ++
 src/conntrack/snprintf_default.c                   | 60 ++++++++++++++++++++++
 src/conntrack/snprintf_xml.c                       | 37 +++++++++++++
 4 files changed, 101 insertions(+)

include/internal/prototypes.h +1 -0

index 484deea..414d7f8 100644
@@ -15,6 +15,7 @@ int __snprintf_protocol(char *buf, unsigned int len, const struct nf_conntrack *
 int __snprintf_proto(char *buf, unsigned int len, const struct __nfct_tuple *tuple);
 int __snprintf_conntrack_default(char *buf, unsigned int len, const struct nf_conntrack *ct, const unsigned int msg_type, const unsigned int flags);
 int __snprintf_conntrack_xml(char *buf, unsigned int len, const struct nf_conntrack *ct, const unsigned int msg_type, const unsigned int flags);
+int __snprintf_connlabels(char *buf, unsigned int len, struct nfct_labelmap *map, const struct nfct_bitmask *b, const char *fmt);
 
 enum __nfct_addr {
 	__ADDR_SRC = 0,

include/libnetfilter_conntrack/libnetfilter_conntrack.h +3 -0

index 39dc24c..eedf85e 100644
@@ -389,6 +389,9 @@ enum {
 
 	NFCT_OF_TIMESTAMP_BIT = 3,
 	NFCT_OF_TIMESTAMP = (1 << NFCT_OF_TIMESTAMP_BIT),
+
+	NFCT_OF_CONNLABELS_BIT = 4,
+	NFCT_OF_CONNLABELS = (1 << NFCT_OF_CONNLABELS_BIT),
 };
 
 extern int nfct_snprintf(char *buf,

src/conntrack/snprintf_default.c +27 -0

index 911faea..2ede0bc 100644
@@ -288,6 +288,61 @@ __snprintf_helper_name(char *buf, unsigned int len, const struct nf_conntrack *c
 	return (snprintf(buf, len, "helper=%s ", ct->helper_name));
 }
 
+int
+__snprintf_connlabels(char *buf, unsigned int len,
+		      struct nfct_labelmap *map,
+		      const struct nfct_bitmask *b, const char *fmt)
+{
+	unsigned int i, max;
+	int ret, size = 0, offset = 0;
+
+	max = nfct_bitmask_maxbit(b);
+	for (i = 0; i <= max && len; i++) {
+		const char *name;
+		if (!nfct_bitmask_test_bit(b, i))
+			continue;
+		name = nfct_labelmap_get_name(map, i);
+		if (!name || strcmp(name, "") == 0)
+			continue;
+
+		ret = snprintf(buf + offset, len, fmt, name);
+		BUFFER_SIZE(ret, size, len, offset);
+	}
+	return size;
+}
+
+static int
+__snprintf_clabels(char *buf, unsigned int len,
+		   const struct nf_conntrack *ct)
+{

src/conntrack/snprintf_default.c __snprintf_helper_name(char *buf, unsigned int len, const struct nf_conntrack *c · line 327

Quoted 10 lines

src/conntrack/snprintf_default.c +10 -0

+	const struct nfct_bitmask *b = nfct_get_attr(ct, ATTR_CONNLABELS);
+	struct nfct_labelmap *map;
+	int ret, size = 0, offset = 0;
+
+	if (!b)
+		return 0;
+
+	map = nfct_labelmap_new(NULL);
+	if (!map)
+		return 0;

This opens and parses the map file for each conntrack, it would be expensive. I think it's better provide more control to the client regarding the load of the mapping, it's more flexible.

Consider adding:

int nfct_snprintf_connlabel(char *buf, unsigned int len,
const struct nf_conntrack *ct, const struct nfct_labelmap *labelmap);

Florian Westphal

author maintainer

2013-06-25 19:43 UTC #3 lore raw

Re: [PATCH V2 lnf-ct 2/2] conntrack: snprintf: add connlabel format specifier

Quoted 24 lines · Pablo Neira Ayuso

Quoted 8 lines · Florian Westphal
By default, nfct_snprintf will not print connlabels, as they're system specific and can easily generate lots of output.
This adds a fmt attribute to print connlabel names.
src/conntrack/snprintf_default.c +4 -0
+static int
+__snprintf_clabels(char *buf, unsigned int len,
+		   const struct nf_conntrack *ct)
+{
src/conntrack/snprintf_default.c __snprintf_helper_name(char *buf, unsigned int len, const struct nf_conntrack *c · line 327
Quoted 10 lines
src/conntrack/snprintf_default.c +10 -0
+	const struct nfct_bitmask *b = nfct_get_attr(ct, ATTR_CONNLABELS);
+	struct nfct_labelmap *map;
+	int ret, size = 0, offset = 0;
+
+	if (!b)
+		return 0;
+
+	map = nfct_labelmap_new(NULL);
+	if (!map)
+		return 0;
This opens and parses the map file for each conntrack, it would be expensive. I think it's better provide more control to the client regarding the load of the mapping, it's more flexible.

Quoted 5 lines

Consider adding:
int nfct_snprintf_connlabel(char *buf, unsigned int len,
const struct nf_conntrack *ct, const struct nfct_labelmap *labelmap);

Thats what I'll do.

Thanks!