* [PATCH lnf-ct 1/2] conntrack: labels: skip labels with non-alnum characters @ 2013-06-23 21:14 Florian Westphal 2013-06-23 21:14 ` [PATCH V2 lnf-ct 2/2] conntrack: snprintf: add connlabel format specifier Florian Westphal 0 siblings, 1 reply; 4+ messages in thread From: Florian Westphal @ 2013-06-23 21:14 UTC (permalink / raw) To: netfilter-devel; +Cc: Florian Westphal 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 <fw@strlen.de> --- 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(-) diff --git a/src/conntrack/labels.c b/src/conntrack/labels.c index 7393c42..7dfb780 100644 --- a/src/conntrack/labels.c +++ b/src/conntrack/labels.c @@ -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; -- 1.8.1.5 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH V2 lnf-ct 2/2] conntrack: snprintf: add connlabel format specifier 2013-06-23 21:14 [PATCH lnf-ct 1/2] conntrack: labels: skip labels with non-alnum characters Florian Westphal @ 2013-06-23 21:14 ` Florian Westphal 2013-06-25 15:39 ` Pablo Neira Ayuso 0 siblings, 1 reply; 4+ messages in thread From: Florian Westphal @ 2013-06-23 21:14 UTC (permalink / raw) To: netfilter-devel; +Cc: 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 <fw@strlen.de> --- 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(+) diff --git a/include/internal/prototypes.h b/include/internal/prototypes.h index 484deea..414d7f8 100644 --- a/include/internal/prototypes.h +++ b/include/internal/prototypes.h @@ -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, diff --git a/include/libnetfilter_conntrack/libnetfilter_conntrack.h b/include/libnetfilter_conntrack/libnetfilter_conntrack.h index 39dc24c..eedf85e 100644 --- a/include/libnetfilter_conntrack/libnetfilter_conntrack.h +++ b/include/libnetfilter_conntrack/libnetfilter_conntrack.h @@ -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, diff --git a/src/conntrack/snprintf_default.c b/src/conntrack/snprintf_default.c index 911faea..2ede0bc 100644 --- a/src/conntrack/snprintf_default.c +++ b/src/conntrack/snprintf_default.c @@ -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) +{ + 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, "%s,"); + + nfct_labelmap_destroy(map); + + BUFFER_SIZE(ret, size, len, offset); + + offset--; /* remove last , */ + size--; + ret = snprintf(buf + offset, len, " "); + BUFFER_SIZE(ret, size, len, offset); + + return size; +} + int __snprintf_conntrack_default(char *buf, unsigned int len, const struct nf_conntrack *ct, @@ -426,6 +481,11 @@ int __snprintf_conntrack_default(char *buf, BUFFER_SIZE(ret, size, len, offset); } + if ((flags & NFCT_OF_CONNLABELS) && test_bit(ATTR_CONNLABELS, ct->head.set)) { + ret = __snprintf_clabels(buf+offset, len, ct); + BUFFER_SIZE(ret, size, len, offset); + } + /* Delete the last blank space */ size--; diff --git a/src/conntrack/snprintf_xml.c b/src/conntrack/snprintf_xml.c index ad53075..27cda04 100644 --- a/src/conntrack/snprintf_xml.c +++ b/src/conntrack/snprintf_xml.c @@ -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>"); -- 1.8.1.5 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH V2 lnf-ct 2/2] conntrack: snprintf: add connlabel format specifier 2013-06-23 21:14 ` [PATCH V2 lnf-ct 2/2] conntrack: snprintf: add connlabel format specifier Florian Westphal @ 2013-06-25 15:39 ` Pablo Neira Ayuso 2013-06-25 19:43 ` Florian Westphal 0 siblings, 1 reply; 4+ messages in thread From: Pablo Neira Ayuso @ 2013-06-25 15:39 UTC (permalink / raw) To: Florian Westphal; +Cc: netfilter-devel On Sun, Jun 23, 2013 at 11:14:39PM +0200, Florian Westphal wrote: > 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 <fw@strlen.de> > --- > 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(+) > > diff --git a/include/internal/prototypes.h b/include/internal/prototypes.h > index 484deea..414d7f8 100644 > --- a/include/internal/prototypes.h > +++ b/include/internal/prototypes.h > @@ -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, > diff --git a/include/libnetfilter_conntrack/libnetfilter_conntrack.h b/include/libnetfilter_conntrack/libnetfilter_conntrack.h > index 39dc24c..eedf85e 100644 > --- a/include/libnetfilter_conntrack/libnetfilter_conntrack.h > +++ b/include/libnetfilter_conntrack/libnetfilter_conntrack.h > @@ -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, > diff --git a/src/conntrack/snprintf_default.c b/src/conntrack/snprintf_default.c > index 911faea..2ede0bc 100644 > --- a/src/conntrack/snprintf_default.c > +++ b/src/conntrack/snprintf_default.c > @@ -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) > +{ > + 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); ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH V2 lnf-ct 2/2] conntrack: snprintf: add connlabel format specifier 2013-06-25 15:39 ` Pablo Neira Ayuso @ 2013-06-25 19:43 ` Florian Westphal 0 siblings, 0 replies; 4+ messages in thread From: Florian Westphal @ 2013-06-25 19:43 UTC (permalink / raw) To: Pablo Neira Ayuso; +Cc: Florian Westphal, netfilter-devel Pablo Neira Ayuso <pablo@netfilter.org> wrote: > On Sun, Jun 23, 2013 at 11:14:39PM +0200, Florian Westphal wrote: > > 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. > > +static int > > +__snprintf_clabels(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; > > 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); Thats what I'll do. Thanks! ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-06-25 19:43 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-06-23 21:14 [PATCH lnf-ct 1/2] conntrack: labels: skip labels with non-alnum characters Florian Westphal 2013-06-23 21:14 ` [PATCH V2 lnf-ct 2/2] conntrack: snprintf: add connlabel format specifier Florian Westphal 2013-06-25 15:39 ` Pablo Neira Ayuso 2013-06-25 19:43 ` Florian Westphal
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).