Re: [PATCH v2] extensions: libxt_multiport: Add translation to nft

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

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}
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).

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.

> +
> +       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?

> +
> +       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 '}')

> +               if (multiinfo->pflags[i])
> +                       xt_xlate_add(xl, "-%u", multiinfo->ports[++i]);
> +       }
> +
> +       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



[Index of Archives]     [Netfitler Users]     [LARTC]     [Bugtraq]     [Yosemite Forum]

  Powered by Linux