[PATCH iproute2-next] iprule: warn about host bits in IPv4 rule prefixes

[PATCH iproute2-next] iprule: warn about host bits in IPv4 rule prefixes

[Zhixing Chen]

IPv4 policy rules match the from and to selectors according to the
supplied prefix length. Host bits outside the prefix do not make the rule
more specific, but ip rule currently accepts such prefixes silently.

Warn when an IPv4 from or to selector contains host bits. Keep sending the
request unchanged so existing scripts and kernel-visible behavior are not
affected.

Signed-off-by: Zhixing Chen <running910@gmail.com>
---

I noticed this after seeing rules such as:

  ip rule add from 192.168.0.147/24 lookup 2

The rule is valid, but it applies to the whole 192.168.0.0/24 prefix. My
understanding is that this form can be easy to misread as if the host
address mattered.

Changing the kernel dump would alter user-visible behavior, so this patch
only adds an iproute2-side warning and leaves the netlink request unchanged.
Existing scripts keep the same behavior.

---
 ip/iprule.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/ip/iprule.c b/ip/iprule.c
index b56b1b18..d358cfdf 100644
--- a/ip/iprule.c
+++ b/ip/iprule.c
@@ -276,6 +276,18 @@ static bool filter_nlmsg(struct nlmsghdr *n, struct rtattr **tb, int host_len)
 	return true;
 }
 
+static bool inet_prefix_host_bits_set(const inet_prefix *p)
+{
+	__u32 mask;
+
+	if (p->family != AF_INET || p->bitlen < 0 || p->bitlen >= 32)
+		return false;
+
+	mask = p->bitlen ? htonl(0xffffffff << (32 - p->bitlen)) : 0;
+
+	return p->data[0] & ~mask;
+}
+
 int print_rule(struct nlmsghdr *n, void *arg)
 {
 	FILE *fp = arg;
@@ -1000,6 +1012,10 @@ static int iprule_modify(int cmd, int argc, char **argv)
 
 			NEXT_ARG();
 			get_prefix(&dst, *argv, req.frh.family);
+			if (inet_prefix_host_bits_set(&dst))
+				fprintf(stderr,
+					"Warning: from prefix %s has host bits set\n",
+					*argv);
 			req.frh.src_len = dst.bitlen;
 			addattr_l(&req.n, sizeof(req), FRA_SRC,
 				  &dst.data, dst.bytelen);
@@ -1008,6 +1024,10 @@ static int iprule_modify(int cmd, int argc, char **argv)
 
 			NEXT_ARG();
 			get_prefix(&dst, *argv, req.frh.family);
+			if (inet_prefix_host_bits_set(&dst))
+				fprintf(stderr,
+					"Warning: to prefix %s has host bits set\n",
+					*argv);
 			req.frh.dst_len = dst.bitlen;
 			addattr_l(&req.n, sizeof(req), FRA_DST,
 				  &dst.data, dst.bytelen);

base-commit: fce739fa4f2ec83206d8d9435aa94ce22f09cdb1
-- 
2.34.1

Re: [PATCH iproute2-next] iprule: warn about host bits in IPv4 rule prefixes

[David Ahern]

> diff --git a/ip/iprule.c b/ip/iprule.c
> index b56b1b18..d358cfdf 100644
> --- a/ip/iprule.c
> +++ b/ip/iprule.c
> @@ -276,6 +276,18 @@ static bool filter_nlmsg(struct nlmsghdr *n, struct rtattr **tb, int host_len)
>  	return true;
>  }
>  
> +static bool inet_prefix_host_bits_set(const inet_prefix *p)
> +{
> +	__u32 mask;
> +
> +	if (p->family != AF_INET || p->bitlen < 0 || p->bitlen >= 32)
> +		return false;
> +
> +	mask = p->bitlen ? htonl(0xffffffff << (32 - p->bitlen)) : 0;
> +
> +	return p->data[0] & ~mask;

detects more than just host entries in the mask, no? 10.1.0.0/8 would
also throw a warning mesage, but that is not a host address