Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote:
> On Tue, Jun 18, 2013 at 10:54:22PM +0200, Florian Westphal wrote:
> > by default, nfct_snprintf will not resolve connlabel names, as they're
> > system specific. This adds a fmt attribute to resolve names accordingly.
> >
> > output looks like this:
> > ... mark=0 use=1 labels=eth0-in (0),eth1-in (1)
> > or
> > <labels>
> > <label name="eth0-in" bit="0"/>
> > <label name="eth1-in" bit="1"/>
> > </labels>
>
> I think that the bit field is meaningless, so I would just export the
> strings. I'd use:
>
> <labels>
> <label>eth0-in</label>
> <label>eth1-in</label>
> <labels>
>
> I'd rather use elements, attributes are not easily extensible IMO.
Okay, I'll change it accordingly. In case translation fails i'll put
the number in there, so you'd get <label>(42)</label> if bit 42 is set
but has no mapping entry.
> > Major stupidity:
> > As names can be anything, we need to be careful, especially when
> > creating XML output. Only alphanumerical label names are printed at
> > this time.
>
> Hm, the string is enclosed between commas, isn't that enough to avoid
> troubles?
What if connlabel.conf contains line like
0 foo<bar>&;!"
> > a) the chosen xml representation
> > b) wheter it makes sense to limit valid charactes in the label
> > names this way (e.g., should we care? should be disallow that
> > in iptables as well, etc).
> > c) if this patch makes sense in the first place, e.g, can't use
> > alternate mapping file for labels with nfct_snprintf
>
> Regarding c) I think it makes sense. You can add some
> nfct_set_connlabel_file interface to set where to find it, it's a bit
> cheating but should resolve the issue.
Yuck ;-)
Lets ignore it for now, adding that would break thread-safety.
I guess one alternative would be to add
nfct_labelmap_snprintf(char *buf,
size_t sz,
const struct nfct_labelmap *,
const struct nfct_bitmask *, int flags);
When a use case presents itself.
> > +/*
> > + * Labels can have any name, there are no restrictions.
> > + * We will only print alpha numerical ones; else parsers
> > + * could choke when putinng "&>" and friends into output
> > + * (especially when using XML format). ASCII machines only.
> > + */
> > +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;
>
> is isalnum() instead?
I was intimidated by locale interactions of isalnum...
Cheers,
Florian
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
