On Tue, May 31, 2016 at 12:08:57AM +0200, Arturo Borrero Gonzalez wrote:
> On 30 May 2016 at 21:47, Laura Garcia Liebana <nevola@xxxxxxxxx> wrote:
> > Add translation for multiport to nftables, which it's supported natively.
> >
> > Examples:
> >
> > $ sudo iptables-translate -t filter -A INPUT -p tcp -m multiport --dports 80,81 -j ACCEPT
> > nft add rule ip filter INPUT ip protocol tcp tcp dport { 80,81} counter accept
> >
> > $ sudo iptables-translate -t filter -A INPUT -p tcp -m multiport --dports 80:88 -j ACCEPT
> > nft add rule ip filter INPUT ip protocol tcp tcp dport { 80-88} counter accept
> >
> > $ sudo iptables-translate -t filter -A INPUT -p tcp -m multiport ! --dports 80:88 -j ACCEPT
> > nft add rule ip filter INPUT ip protocol tcp tcp dport != 80-88 counter accept
> >
>
> Lets clarify the syntax, this is valid:
>
> tcp dport 8000-8100
> tcp dport { 8000-8100}
> tcp dport { 8000-8100, 9000-9100}
>
> but they mean different things. It seems we should avoid the braces {}
> for the range case, otherwise we would be using a set with a single
> element.
>
Yes, you're right. I'll change it in order to allow port ranges without {}.
> However,
>
> tcp dport {8000,8100} <-- valid
> tcp dport 8000,8100 <-- invalid
>
> So we should always use braces {} in the non-range case.
>
> Same seems to apply in the case of inversion.
>
> so we end with this combinations:
>
> tcp dport {x,x}
> tcp dport != {x,x}
This is not supported.
> tcp dport x-x
> tcp dport != x-x
>
> BTW, related to this, there seems to be a bug in nftables:
>
> % nft add rule t c tcp dport != {80, 81}
> BUG: invalid expression type set
> nft: evaluate.c:1463: expr_evaluate_relational: Assertion `0' failed.
> Aborted
>
>
> > Signed-off-by: Laura Garcia Liebana <nevola@xxxxxxxxx>
> > ---
> > Changes in v2:
> > - Add curley brackets to lists and range of ports.
> >
> > extensions/libxt_multiport.c | 116 +++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 116 insertions(+)
> >
> > diff --git a/extensions/libxt_multiport.c b/extensions/libxt_multiport.c
> > index 03af5a9..25b5589 100644
> > --- a/extensions/libxt_multiport.c
> > +++ b/extensions/libxt_multiport.c
> > @@ -468,6 +468,118 @@ static void multiport_save6_v1(const void *ip_void,
> > __multiport_save_v1(match, ip->proto);
> > }
> >
> > +static int __multiport_xlate(const void *ip, const struct xt_entry_match *match,
> > + struct xt_xlate *xl, int numeric)
> > +{
> > + const struct xt_multiport *multiinfo
> > + = (const struct xt_multiport *)match->data;
> > + unsigned int i;
> > +
> > + switch (multiinfo->flags) {
> > + case XT_MULTIPORT_SOURCE:
> > + xt_xlate_add(xl, "sport ");
> > + break;
> > + case XT_MULTIPORT_DESTINATION:
> > + xt_xlate_add(xl, "dport ");
> > + break;
> > + case XT_MULTIPORT_EITHER:
> > + return 0;
> > + }
>
> this case XT_MULTIPORT_EITHER seems unsupported in nftables.
>
> Is there anything established to do in case we find an impossible
> translation? print a warning or something? I don't know right now.
> I guess we should avoid printing an invalid nftables rule as if the
> translation was 100% ok (which is not true in this case).
>
It was agreed to return 0 if the translation is not supported for
nftables. Currently, it does.
> Just wondering, I should check myself because I don't know this right now.
>
>
> > +
> > + if (multiinfo->count > 1)
> > + xt_xlate_add(xl, "{ ");
> > +
> > + for (i = 0; i < multiinfo->count; i++)
> > + xt_xlate_add(xl, "%s%u", i ? "," : "", multiinfo->ports[i]);
> > +
> > + if (multiinfo->count > 1)
> > + xt_xlate_add(xl, "}");
> > +
> > + xt_xlate_add(xl, " ");
> > +
> > + return 1;
> > +}
> > +
> > +static int multiport_xlate(const void *ip, const struct xt_entry_match *match,
> > + struct xt_xlate *xl, int numeric)
> > +{
> > + uint8_t proto = ((const struct ipt_ip *)ip)->proto;
> > +
> > + xt_xlate_add(xl, "%s ", proto_to_name(proto));
> > + return __multiport_xlate(ip, match, xl, numeric);
> > +}
> > +
> > +static int multiport_xlate6(const void *ip, const struct xt_entry_match *match,
> > + struct xt_xlate *xl, int numeric)
> > +{
> > + uint8_t proto = ((const struct ip6t_ip6 *)ip)->proto;
> > +
> > + xt_xlate_add(xl, "%s ", proto_to_name(proto));
> > + return __multiport_xlate(ip, match, xl, numeric);
> > +}
> > +
> > +static int __multiport_xlate_v1(const void *ip,
> > + const struct xt_entry_match *match,
> > + struct xt_xlate *xl, int numeric)
> > +{
> > + const struct xt_multiport_v1 *multiinfo
> > + = (const struct xt_multiport_v1 *)match->data;
> > + unsigned int i;
> > +
> > + switch (multiinfo->flags) {
> > + case XT_MULTIPORT_SOURCE:
> > + xt_xlate_add(xl, "sport ");
> > + break;
> > + case XT_MULTIPORT_DESTINATION:
> > + xt_xlate_add(xl, "dport ");
> > + break;
> > + case XT_MULTIPORT_EITHER:
> > + return 0;
> > + }
>
> same XT_MULTIPORT_EITHER here.
>
Same as before.
> > +
> > + if (multiinfo->invert) {
> > + xt_xlate_add(xl, "!= ");
> > + } else {
> > + if (multiinfo->count > 1)
> > + xt_xlate_add(xl, "{ ");
> > + }
>
> This if/else seems bogus. We only allow port sets if not inverting?
>
This should be now changed as we've to accept port ranges without {}.
> > +
> > + for (i = 0; i < multiinfo->count; i++) {
> > + xt_xlate_add(xl, "%s%u", i ? "," : "", multiinfo->ports[i]);
> > + if (i && multiinfo->invert)
> > + return 0;
>
> This return here could mean that we build an incomplete nftables rule
> (ie, missing '}')
>
Such condition cares that there is no translation in nftables for:
tcp dport != { 80,88 }
So the closed } doesn't even matter.
> > + if (multiinfo->pflags[i])
> > + xt_xlate_add(xl, "-%u", multiinfo->ports[++i]);
> > + }
> > +
If the rule needs such closed } , then it's controlled here:
> > + if (multiinfo->count > 1 && !multiinfo->invert)
> > + xt_xlate_add(xl, "}");
> > +
> > + xt_xlate_add(xl, " ");
> > +
> > + return 1;
> > +}
>
>
> --
> Arturo Borrero González
--
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
