| 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: <c77c748a-5cfa-bb99-b13e-5de3ee845511@cumulusnetworks.com>
Date: Sun, 25 Feb 2018 17:04:07 +0200
From: Nikolay Aleksandrov <nikolay@...ulusnetworks.com>
To: Roopa Prabhu <roopa@...ulusnetworks.com>, davem@...emloft.net,
netdev@...r.kernel.org
Cc: dsa@...ulusnetworks.com, idosch@...lanox.com
Subject: Re: [PATCH net-next 1/5] net: fib_rules: support for match on
ip_proto, sport and dport
On 25/02/18 07:44, Roopa Prabhu wrote:
> From: Roopa Prabhu <roopa@...ulusnetworks.com>
>
> uapi for ip_proto, sport and dport range match
> in fib rules.
>
> Signed-off-by: Roopa Prabhu <roopa@...ulusnetworks.com>
> ---
> include/net/fib_rules.h | 31 +++++++++++++-
> include/uapi/linux/fib_rules.h | 8 ++++
> net/core/fib_rules.c | 94 +++++++++++++++++++++++++++++++++++++++++-
> 3 files changed, 130 insertions(+), 3 deletions(-)
>
You should probably update validate_rulemsg() as well, these aren't added in the per-proto
policies and nothing validates if the attribute data is actually there. Maybe I'm missing
something obvious, but it looks like many other FRA_ attributes don't have such checks.
> diff --git a/include/net/fib_rules.h b/include/net/fib_rules.h
> index b3d2162..6d99202 100644
> --- a/include/net/fib_rules.h
> +++ b/include/net/fib_rules.h
> @@ -11,6 +11,11 @@
> #include <net/rtnetlink.h>
> #include <net/fib_notifier.h>
>
> +struct fib_port_range {
> + __u16 start;
> + __u16 end;
> +};
> +
> struct fib_kuid_range {
> kuid_t start;
> kuid_t end;
> @@ -27,7 +32,7 @@ struct fib_rule {
> u8 action;
> u8 l3mdev;
> u8 proto;
> - /* 1 byte hole, try to use */
> + u8 ip_proto;
> u32 target;
> __be64 tun_id;
> struct fib_rule __rcu *ctarget;
> @@ -40,6 +45,8 @@ struct fib_rule {
> char iifname[IFNAMSIZ];
> char oifname[IFNAMSIZ];
> struct fib_kuid_range uid_range;
> + struct fib_port_range sport_range;
> + struct fib_port_range dport_range;
> struct rcu_head rcu;
> };
>
> @@ -144,6 +151,28 @@ static inline u32 frh_get_table(struct fib_rule_hdr *frh, struct nlattr **nla)
> return frh->table;
> }
>
> +static inline bool fib_rule_port_inrange(struct fib_port_range *a,
> + __be16 port)
> +{
> + if (!a->start)
> + return true;
Can start be == 0 ?
IIUC this check is unnecessary because when you're adding the new rule,
you do a check for start > 0 so it shouldn't be possible to be 0.
> + return ntohs(port) >= a->start &&
> + ntohs(port) <= a->end;
> +}
> +
> +static inline bool fib_rule_port_range_valid(const struct fib_port_range *a)
> +{
> + return a->start > 0 && a->end < 0xffff &&
> + a->start <= a->end;
nit: alignment (also can be on a single line)
> +}
> +
> +static inline bool fib_rule_port_range_compare(struct fib_port_range *a,
> + struct fib_port_range *b)
> +{
> + return a->start == b->start &&
> + a->end == b->end;
nit: alignment (also can be on a single line)
> +}
> +
> struct fib_rules_ops *fib_rules_register(const struct fib_rules_ops *,
> struct net *);
> void fib_rules_unregister(struct fib_rules_ops *);
> diff --git a/include/uapi/linux/fib_rules.h b/include/uapi/linux/fib_rules.h
> index 77d90ae..232df14 100644
> --- a/include/uapi/linux/fib_rules.h
> +++ b/include/uapi/linux/fib_rules.h
> @@ -35,6 +35,11 @@ struct fib_rule_uid_range {
> __u32 end;
> };
>
> +struct fib_rule_port_range {
> + __u16 start;
> + __u16 end;
> +};
> +
> enum {
> FRA_UNSPEC,
> FRA_DST, /* destination address */
> @@ -59,6 +64,9 @@ enum {
> FRA_L3MDEV, /* iif or oif is l3mdev goto its table */
> FRA_UID_RANGE, /* UID range */
> FRA_PROTOCOL, /* Originator of the rule */
> + FRA_IP_PROTO, /* ip proto */
> + FRA_SPORT_RANGE, /* sport */
> + FRA_DPORT_RANGE, /* dport */
> __FRA_MAX
> };
>
> diff --git a/net/core/fib_rules.c b/net/core/fib_rules.c
> index a6aea80..5008235 100644
> --- a/net/core/fib_rules.c
> +++ b/net/core/fib_rules.c
> @@ -33,6 +33,10 @@ bool fib_rule_matchall(const struct fib_rule *rule)
> if (!uid_eq(rule->uid_range.start, fib_kuid_range_unset.start) ||
> !uid_eq(rule->uid_range.end, fib_kuid_range_unset.end))
> return false;
> + if (fib_rule_port_range_valid(&rule->sport_range))
> + return false;
> + if (fib_rule_port_range_valid(&rule->dport_range))
> + return false;
> return true;
> }
> EXPORT_SYMBOL_GPL(fib_rule_matchall);
> @@ -221,6 +225,12 @@ static int nla_put_uid_range(struct sk_buff *skb, struct fib_kuid_range *range)
> return nla_put(skb, FRA_UID_RANGE, sizeof(out), &out);
> }
>
> +static int nla_put_port_range(struct sk_buff *skb, int attrtype,
> + struct fib_port_range *range)
> +{
> + return nla_put(skb, attrtype, sizeof(*range), range);
> +}
> +
> static int fib_rule_match(struct fib_rule *rule, struct fib_rules_ops *ops,
> struct flowi *fl, int flags,
> struct fib_lookup_arg *arg)
> @@ -425,6 +435,17 @@ static int rule_exists(struct fib_rules_ops *ops, struct fib_rule_hdr *frh,
> !uid_eq(r->uid_range.end, rule->uid_range.end))
> continue;
>
> + if (r->ip_proto != rule->ip_proto)
> + continue;
> +
> + if (!fib_rule_port_range_compare(&r->sport_range,
> + &rule->sport_range))
> + continue;
> +
> + if (!fib_rule_port_range_compare(&r->dport_range,
> + &rule->dport_range))
> + continue;
> +
> if (!ops->compare(r, frh, tb))
> continue;
> return 1;
> @@ -432,6 +453,20 @@ static int rule_exists(struct fib_rules_ops *ops, struct fib_rule_hdr *frh,
> return 0;
> }
>
> +static int nla_get_port_range(struct nlattr *pattr,
> + struct fib_port_range *port_range)
> +{
> + const struct fib_port_range *pr = nla_data(pattr);
> +
> + if (!fib_rule_port_range_valid(pr))
> + return -EINVAL;
> +
> + port_range->start = pr->start;
> + port_range->end = pr->end;
> +
> + return 0;
> +}
> +
> int fib_nl_newrule(struct sk_buff *skb, struct nlmsghdr *nlh,
> struct netlink_ext_ack *extack)
> {
> @@ -569,6 +604,23 @@ int fib_nl_newrule(struct sk_buff *skb, struct nlmsghdr *nlh,
> rule->uid_range = fib_kuid_range_unset;
> }
>
> + if (tb[FRA_IP_PROTO])
> + rule->ip_proto = nla_get_u8(tb[FRA_IP_PROTO]);
> +
> + if (tb[FRA_SPORT_RANGE]) {
> + err = nla_get_port_range(tb[FRA_SPORT_RANGE],
> + &rule->sport_range);
> + if (err)
> + goto errout_free;
> + }
> +
> + if (tb[FRA_DPORT_RANGE]) {
> + err = nla_get_port_range(tb[FRA_DPORT_RANGE],
> + &rule->dport_range);
> + if (err)
> + goto errout_free;
> + }
> +
> if ((nlh->nlmsg_flags & NLM_F_EXCL) &&
> rule_exists(ops, frh, tb, rule)) {
> err = -EEXIST;
> @@ -638,6 +690,8 @@ int fib_nl_delrule(struct sk_buff *skb, struct nlmsghdr *nlh,
> struct fib_rule *rule, *r;
> struct nlattr *tb[FRA_MAX+1];
> struct fib_kuid_range range;
> + struct fib_port_range *sprange = NULL;
> + struct fib_port_range *dprange = NULL;
> int err = -EINVAL;
>
> if (nlh->nlmsg_len < nlmsg_msg_size(sizeof(*frh)))
> @@ -667,6 +721,22 @@ int fib_nl_delrule(struct sk_buff *skb, struct nlmsghdr *nlh,
> range = fib_kuid_range_unset;
> }
>
> + if (tb[FRA_SPORT_RANGE]) {
> + sprange = nla_data(tb[FRA_SPORT_RANGE]);
> + if (!fib_rule_port_range_valid(sprange)) {
> + err = -EINVAL;
> + goto errout;
> + }
> + }
> +
> + if (tb[FRA_DPORT_RANGE]) {
> + dprange = nla_data(tb[FRA_DPORT_RANGE]);
> + if (!fib_rule_port_range_valid(dprange)) {
> + err = -EINVAL;
> + goto errout;
> + }
> + }
> +
> list_for_each_entry(rule, &ops->rules_list, list) {
> if (tb[FRA_PROTOCOL] &&
> (rule->proto != nla_get_u8(tb[FRA_PROTOCOL])))
> @@ -712,6 +782,18 @@ int fib_nl_delrule(struct sk_buff *skb, struct nlmsghdr *nlh,
> !uid_eq(rule->uid_range.end, range.end)))
> continue;
>
> + if (tb[FRA_IP_PROTO] &&
> + (rule->ip_proto != nla_get_u8(tb[FRA_IP_PROTO])))
nit: no need for the extra braces
> + continue;
> +
> + if (sprange &&
> + !fib_rule_port_range_compare(&rule->sport_range, sprange))
> + continue;
> +
> + if (dprange &&
> + !fib_rule_port_range_compare(&rule->dport_range, dprange))
> + continue;
> +
> if (!ops->compare(rule, frh, tb))
> continue;
>
> @@ -790,7 +872,10 @@ static inline size_t fib_rule_nlmsg_size(struct fib_rules_ops *ops,
> + nla_total_size(4) /* FRA_FWMASK */
> + nla_total_size_64bit(8) /* FRA_TUN_ID */
> + nla_total_size(sizeof(struct fib_kuid_range))
> - + nla_total_size(1); /* FRA_PROTOCOL */
> + + nla_total_size(1) /* FRA_PROTOCOL */
> + + nla_total_size(1) /* FRA_IP_PROTO */
> + + nla_total_size(sizeof(struct fib_port_range)) /* FRA_SPORT_RANGE */
> + + nla_total_size(sizeof(struct fib_port_range)); /* FRA_DPORT_RANGE */
>
> if (ops->nlmsg_payload)
> payload += ops->nlmsg_payload(rule);
> @@ -855,7 +940,12 @@ static int fib_nl_fill_rule(struct sk_buff *skb, struct fib_rule *rule,
> (rule->l3mdev &&
> nla_put_u8(skb, FRA_L3MDEV, rule->l3mdev)) ||
> (uid_range_set(&rule->uid_range) &&
> - nla_put_uid_range(skb, &rule->uid_range)))
> + nla_put_uid_range(skb, &rule->uid_range)) ||
> + (fib_rule_port_range_valid(&rule->sport_range) &&
> + nla_put_port_range(skb, FRA_SPORT_RANGE, &rule->sport_range)) ||
> + (fib_rule_port_range_valid(&rule->dport_range) &&
> + nla_put_port_range(skb, FRA_DPORT_RANGE, &rule->dport_range)) ||
> + (rule->ip_proto && nla_put_u8(skb, FRA_IP_PROTO, rule->ip_proto)))
> goto nla_put_failure;
>
> if (rule->suppress_ifgroup != -1) {
>
Powered by blists - more mailing lists