| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <87o6yrju35.fsf@nvidia.com>
Date: Mon, 24 Feb 2025 15:03:25 +0100
From: Petr Machata <petrm@...dia.com>
To: Ido Schimmel <idosch@...dia.com>
CC: <netdev@...r.kernel.org>, <stephen@...workplumber.org>,
<dsahern@...il.com>, <gnault@...hat.com>, <petrm@...dia.com>
Subject: Re: [PATCH iproute2-next 4/5] iprule: Add port mask support
Ido Schimmel <idosch@...dia.com> writes:
> Add port mask support, allowing users to specify a source or destination
> port with an optional mask. Example:
>
> # ip rule add sport 80 table 100
> # ip rule add sport 90/0xffff table 200
> # ip rule add dport 1000-2000 table 300
> # ip rule add sport 0x123/0xfff table 400
> # ip rule add dport 0x4/0xff table 500
> # ip rule add dport 0x8/0xf table 600
> # ip rule del dport 0x8/0xf table 600
>
> In non-JSON output, the mask is not printed in case of exact match:
>
> $ ip rule show
> 0: from all lookup local
> 32761: from all dport 0x4/0xff lookup 500
> 32762: from all sport 0x123/0xfff lookup 400
> 32763: from all dport 1000-2000 lookup 300
> 32764: from all sport 90 lookup 200
> 32765: from all sport 80 lookup 100
> 32766: from all lookup main
> 32767: from all lookup default
>
> Dump can be filtered by port value and mask:
>
> $ ip rule show sport 80
> 32765: from all sport 80 lookup 100
> $ ip rule show sport 90
> 32764: from all sport 90 lookup 200
> $ ip rule show sport 0x123/0x0fff
> 32762: from all sport 0x123/0xfff lookup 400
> $ ip rule show dport 4/0xff
> 32761: from all dport 0x4/0xff lookup 500
>
> In JSON output, the port mask is printed as an hexadecimal string to be
> consistent with other masks. The port value is printed as an integer in
> order not to break existing scripts:
>
> $ ip -j -p rule show sport 0x123/0xfff table 400
> [ {
> "priority": 32762,
> "src": "all",
> "sport": 291,
> "sport_mask": "0xfff",
> "table": "400"
> } ]
>
> The mask attribute is only sent to the kernel in case of inexact match
> so that iproute2 will continue working with kernels that do not support
> the attribute.
>
> Signed-off-by: Ido Schimmel <idosch@...dia.com>
Two minor suggestions and a couple notes to self below. Looks OK overall.
> ---
> ip/iprule.c | 103 +++++++++++++++++++++++++++++++++++++-----
> man/man8/ip-rule.8.in | 14 +++---
> 2 files changed, 100 insertions(+), 17 deletions(-)
>
> diff --git a/ip/iprule.c b/ip/iprule.c
> index 64d389bebb76..fbe69a3b6293 100644
> --- a/ip/iprule.c
> +++ b/ip/iprule.c
> @@ -23,6 +23,8 @@
> #include "ip_common.h"
> #include "json_print.h"
>
> +#define PORT_MAX_MASK 0xFFFF
> +
> enum list_action {
> IPRULE_LIST,
> IPRULE_FLUSH,
> @@ -44,8 +46,8 @@ static void usage(void)
> " [ iif STRING ] [ oif STRING ] [ pref NUMBER ] [ l3mdev ]\n"
> " [ uidrange NUMBER-NUMBER ]\n"
> " [ ipproto PROTOCOL ]\n"
> - " [ sport [ NUMBER | NUMBER-NUMBER ]\n"
> - " [ dport [ NUMBER | NUMBER-NUMBER ] ]\n"
> + " [ sport [ NUMBER[/MASK] | NUMBER-NUMBER ]\n"
> + " [ dport [ NUMBER[/MASK] | NUMBER-NUMBER ] ]\n"
> " [ dscp DSCP ] [ flowlabel FLOWLABEL[/MASK] ]\n"
> "ACTION := [ table TABLE_ID ]\n"
> " [ protocol PROTO ]\n"
> @@ -80,6 +82,7 @@ static struct
> int protocolmask;
> struct fib_rule_port_range sport;
> struct fib_rule_port_range dport;
> + __u16 sport_mask, dport_mask;
> __u8 ipproto;
> } filter;
>
> @@ -186,8 +189,9 @@ static bool filter_nlmsg(struct nlmsghdr *n, struct rtattr **tb, int host_len)
> return false;
> }
>
> - if (filter.sport.start) {
> + if (filter.sport_mask) {
OK, sport_mask now implies sport.start because of the changes in
iprule_port_parse().
> const struct fib_rule_port_range *r;
> + __u16 sport_mask = PORT_MAX_MASK;
>
> if (!tb[FRA_SPORT_RANGE])
> return false;
> @@ -196,10 +200,16 @@ static bool filter_nlmsg(struct nlmsghdr *n, struct rtattr **tb, int host_len)
> if (r->start != filter.sport.start ||
> r->end != filter.sport.end)
> return false;
> +
> + if (tb[FRA_SPORT_MASK])
> + sport_mask = rta_getattr_u16(tb[FRA_SPORT_MASK]);
> + if (filter.sport_mask != sport_mask)
> + return false;
> }
>
> - if (filter.dport.start) {
> + if (filter.dport_mask) {
> const struct fib_rule_port_range *r;
> + __u16 dport_mask = PORT_MAX_MASK;
>
> if (!tb[FRA_DPORT_RANGE])
> return false;
> @@ -208,6 +218,11 @@ static bool filter_nlmsg(struct nlmsghdr *n, struct rtattr **tb, int host_len)
> if (r->start != filter.dport.start ||
> r->end != filter.dport.end)
> return false;
> +
> + if (tb[FRA_DPORT_MASK])
> + dport_mask = rta_getattr_u16(tb[FRA_DPORT_MASK]);
> + if (filter.dport_mask != dport_mask)
> + return false;
> }
>
> if (filter.tun_id) {
> @@ -390,7 +405,26 @@ int print_rule(struct nlmsghdr *n, void *arg)
> struct fib_rule_port_range *r = RTA_DATA(tb[FRA_SPORT_RANGE]);
>
> if (r->start == r->end) {
> - print_uint(PRINT_ANY, "sport", " sport %u", r->start);
> + if (tb[FRA_SPORT_MASK]) {
> + __u16 mask;
> +
> + mask = rta_getattr_u16(tb[FRA_SPORT_MASK]);
> + print_uint(PRINT_JSON, "sport", NULL, r->start);
> + print_0xhex(PRINT_JSON, "sport_mask", NULL,
> + mask);
> + if (mask == PORT_MAX_MASK) {
> + print_uint(PRINT_FP, NULL, " sport %u",
> + r->start);
> + } else {
> + print_0xhex(PRINT_FP, NULL,
> + " sport %#x", r->start);
Looks good, for JSON we always emit as uint, for FP we emit uint in
backward-compatible scenarios.
> + print_0xhex(PRINT_FP, NULL, "/%#x",
> + mask);
> + }
> + } else {
> + print_uint(PRINT_ANY, "sport", " sport %u",
> + r->start);
Hm, yeah, and on an old kernel we don't even get the mask in JSON.
Makes sense.
> + }
> } else {
> print_uint(PRINT_ANY, "sport_start", " sport %u",
> r->start);
> @@ -402,7 +436,26 @@ int print_rule(struct nlmsghdr *n, void *arg)
> struct fib_rule_port_range *r = RTA_DATA(tb[FRA_DPORT_RANGE]);
>
> if (r->start == r->end) {
> - print_uint(PRINT_ANY, "dport", " dport %u", r->start);
> + if (tb[FRA_DPORT_MASK]) {
> + __u16 mask;
> +
> + mask = rta_getattr_u16(tb[FRA_DPORT_MASK]);
> + print_uint(PRINT_JSON, "dport", NULL, r->start);
> + print_0xhex(PRINT_JSON, "dport_mask", NULL,
> + mask);
> + if (mask == 0xFFFF) {
> + print_uint(PRINT_FP, NULL, " dport %u",
> + r->start);
> + } else {
> + print_0xhex(PRINT_FP, NULL,
> + " dport %#x", r->start);
> + print_0xhex(PRINT_FP, NULL, "/%#x",
> + mask);
> + }
> + } else {
> + print_uint(PRINT_ANY, "dport", " dport %u",
> + r->start);
> + }
> } else {
> print_uint(PRINT_ANY, "dport_start", " dport %u",
> r->start);
> @@ -600,10 +653,13 @@ static int flush_rule(struct nlmsghdr *n, void *arg)
> return 0;
> }
>
> -static void iprule_port_parse(char *arg, struct fib_rule_port_range *r)
> +static void iprule_port_parse(char *arg, struct fib_rule_port_range *r,
> + __u16 *mask)
> {
> char *sep;
>
> + *mask = PORT_MAX_MASK;
> +
> sep = strchr(arg, '-');
> if (sep) {
> *sep = '\0';
> @@ -617,6 +673,21 @@ static void iprule_port_parse(char *arg, struct fib_rule_port_range *r)
> return;
> }
>
> + sep = strchr(arg, '/');
> + if (sep) {
> + *sep = '\0';
> +
> + if (get_u16(&r->start, arg, 0))
> + invarg("invalid port", arg);
> +
> + r->end = r->start;
> +
> + if (get_u16(mask, sep + 1, 0))
> + invarg("invalid mask", sep + 1);
> +
> + return;
> + }
> +
> if (get_u16(&r->start, arg, 0))
> invarg("invalid port", arg);
>
I think this duplicates the port number parsing unnecessarily. How
about:
+ sep = strchr(arg, '/');
+ if (sep) {
+ *sep = '\0';
+ if (get_u16(mask, sep + 1, 0))
+ invarg("invalid mask", sep + 1);
+ }
+
if (get_u16(&r->start, arg, 0))
invarg("invalid port", arg);
> @@ -770,10 +841,12 @@ static int iprule_list_flush_or_save(int argc, char **argv, int action)
> filter.ipproto = ipproto;
> } else if (strcmp(*argv, "sport") == 0) {
> NEXT_ARG();
> - iprule_port_parse(*argv, &filter.sport);
> + iprule_port_parse(*argv, &filter.sport,
> + &filter.sport_mask);
> } else if (strcmp(*argv, "dport") == 0) {
> NEXT_ARG();
> - iprule_port_parse(*argv, &filter.dport);
> + iprule_port_parse(*argv, &filter.dport,
> + &filter.dport_mask);
> } else if (strcmp(*argv, "dscp") == 0) {
> __u32 dscp;
>
> @@ -1043,18 +1116,26 @@ static int iprule_modify(int cmd, int argc, char **argv)
> addattr8(&req.n, sizeof(req), FRA_IP_PROTO, ipproto);
> } else if (strcmp(*argv, "sport") == 0) {
> struct fib_rule_port_range r;
> + __u16 sport_mask;
>
> NEXT_ARG();
> - iprule_port_parse(*argv, &r);
> + iprule_port_parse(*argv, &r, &sport_mask);
> addattr_l(&req.n, sizeof(req), FRA_SPORT_RANGE, &r,
> sizeof(r));
> + if (sport_mask != PORT_MAX_MASK)
> + addattr16(&req.n, sizeof(req), FRA_SPORT_MASK,
> + sport_mask);
> } else if (strcmp(*argv, "dport") == 0) {
> struct fib_rule_port_range r;
> + __u16 dport_mask;
>
> NEXT_ARG();
> - iprule_port_parse(*argv, &r);
> + iprule_port_parse(*argv, &r, &dport_mask);
> addattr_l(&req.n, sizeof(req), FRA_DPORT_RANGE, &r,
> sizeof(r));
> + if (dport_mask != PORT_MAX_MASK)
> + addattr16(&req.n, sizeof(req), FRA_DPORT_MASK,
> + dport_mask);
> } else if (strcmp(*argv, "dscp") == 0) {
> __u32 dscp;
>
> diff --git a/man/man8/ip-rule.8.in b/man/man8/ip-rule.8.in
> index 6fc741d4f470..4945ccd55076 100644
> --- a/man/man8/ip-rule.8.in
> +++ b/man/man8/ip-rule.8.in
> @@ -52,10 +52,10 @@ ip-rule \- routing policy database management
> .B ipproto
> .IR PROTOCOL " ] [ "
> .BR sport " [ "
> -.IR NUMBER " | "
> +.IR NUMBER\fR[\fB/\fIMASK "] | "
> .IR NUMBER "-" NUMBER " ] ] [ "
> .BR dport " [ "
> -.IR NUMBER " | "
> +.IR NUMBER\fR[\fB/\fIMASK "] | "
> .IR NUMBER "-" NUMBER " ] ] [ "
> .B tun_id
> .IR TUN_ID " ] [ "
> @@ -270,12 +270,14 @@ value to match.
> select the ip protocol value to match.
>
> .TP
> -.BI sport " NUMBER | NUMBER-NUMBER"
> -select the source port value to match. supports port range.
> +.BI sport " NUMBER\fR[\fB/\fIMASK\fR] | NUMBER-NUMBER"
> +select the source port value to match with an optional mask. supports port
> +range.
s/supports/Supports/. And below.
>
> .TP
> -.BI dport " NUMBER | NUMBER-NUMBER"
> -select the destination port value to match. supports port range.
> +.BI dport " NUMBER\fR[\fB/\fIMASK\fR] | NUMBER-NUMBER"
> +select the destination port value to match with an optional mask. supports port
> +range.
>
> .TP
> .BI priority " PREFERENCE"
Powered by blists - more mailing lists