netdev - [PATCH net-next 3/7] ipv6: add RTM

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
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Thu, 27 Sep 2018 19:58:53 +0200
From:   Christian Brauner <christian@...uner.io>
To:     jbenc@...hat.com, davem@...emloft.net, dsahern@...il.com,
        stephen@...workplumber.org, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org
Cc:     Christian Brauner <christian@...uner.io>
Subject: [PATCH net-next 3/7] ipv6: add RTM_GETADDR2

Various userspace programs (e.g. iproute2) have sent RTM_GETADDR
requests with struct ifinfomsg. This is wrong and should have been
struct ifaddrmsg all along as mandated by the manpages. However, dump
requests so far didn't parse the netlink message that was sent and
succeeded even when a wrong struct was passed along.

Currently, the message is parsed under the assumption that the correct
struct ifaddrmsg is sent down. If the parsing fails the kernel will
still fulfill the request to preserve backwards compatability but a
rate-limited message that there were leftover bytes after parsing the
message is recorded in dmesg. It has been argued that this is
unacceptable [1].

But various new features that got and will get added to RTM_GETADDR make
it necessary to definitely know what header was passed along.
This is currently not possible without resorting to (likely unreliable)
hacks such as introducing a nested attribute that ensures that
RTM_GETADDR which pass along properties such as IFA_TARGET_NETNSID
always exceed RTM_GETADDR requests that send down the wrong struct
ifinfomsg [2]. Basically, multiple approaches to solve this have been
shut down. Furthermore, the API expressed via RTM_GETADDR is apparently
frozen [3] so the wiggle room at this point seems very much zero.

The correct solution at this point seems to me to introduce a new
RTM_GETADDR2 request. This way we can parse the message and fail hard if
the struct is not struct ifaddrmsg and can safely extend it in the
future. Userspace tools that rely on the buggy RTM_GETADDR API will
still keep working without even having to see any log messages and new
userspace tools that want to make user of new features can make use of
the new RTM_GETADDR2 requests.

[1]: https://lists.openwall.net/netdev/2018/09/25/59
[2]: https://lists.openwall.net/netdev/2018/09/25/75
[3]: https://lists.openwall.net/netdev/2018/09/26/166

Signed-off-by: Christian Brauner <christian@...uner.io>
Cc: David Ahern <dsahern@...il.com>
Cc: Jiri Benc <jbenc@...hat.com>
Cc: Stephen Hemminger <stephen@...workplumber.org>
---
 net/ipv6/addrconf.c | 30 ++++++++++++++++++++++++------
 1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index a9a317322388..6e76e5cc76c4 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -5003,7 +5003,7 @@ static int in6_dump_addrs(struct inet6_dev *idev, struct sk_buff *skb,
 }
 
 static int inet6_dump_addr(struct sk_buff *skb, struct netlink_callback *cb,
-			   enum addr_type_t type)
+			   enum addr_type_t type, int rtm_version)
 {
 	struct net *net = sock_net(skb->sk);
 	struct nlattr *tb[IFA_MAX+1];
@@ -5020,8 +5020,14 @@ static int inet6_dump_addr(struct sk_buff *skb, struct netlink_callback *cb,
 	s_idx = idx = cb->args[1];
 	s_ip_idx = ip_idx = cb->args[2];
 
-	if (nlmsg_parse(cb->nlh, sizeof(struct ifaddrmsg), tb, IFA_MAX,
-			ifa_ipv6_policy, NULL) >= 0) {
+	if (rtm_version == RTM_GETADDR2) {
+		int err;
+
+		err = nlmsg_parse(cb->nlh, sizeof(struct ifaddrmsg), tb,
+				  IFA_MAX, ifa_ipv6_policy, NULL);
+		if (err < 0)
+			return err;
+
 		if (tb[IFA_TARGET_NETNSID]) {
 			netnsid = nla_get_s32(tb[IFA_TARGET_NETNSID]);
 
@@ -5068,14 +5074,21 @@ static int inet6_dump_ifaddr(struct sk_buff *skb, struct netlink_callback *cb)
 {
 	enum addr_type_t type = UNICAST_ADDR;
 
-	return inet6_dump_addr(skb, cb, type);
+	return inet6_dump_addr(skb, cb, type, RTM_GETADDR);
+}
+
+static int inet6_dump_ifaddr2(struct sk_buff *skb, struct netlink_callback *cb)
+{
+	enum addr_type_t type = UNICAST_ADDR;
+
+	return inet6_dump_addr(skb, cb, type, RTM_GETADDR2);
 }
 
 static int inet6_dump_ifmcaddr(struct sk_buff *skb, struct netlink_callback *cb)
 {
 	enum addr_type_t type = MULTICAST_ADDR;
 
-	return inet6_dump_addr(skb, cb, type);
+	return inet6_dump_addr(skb, cb, type, RTM_GETMULTICAST);
 }
 
 
@@ -5083,7 +5096,7 @@ static int inet6_dump_ifacaddr(struct sk_buff *skb, struct netlink_callback *cb)
 {
 	enum addr_type_t type = ANYCAST_ADDR;
 
-	return inet6_dump_addr(skb, cb, type);
+	return inet6_dump_addr(skb, cb, type, RTM_GETANYCAST);
 }
 
 static int inet6_rtm_getaddr(struct sk_buff *in_skb, struct nlmsghdr *nlh,
@@ -6833,6 +6846,11 @@ int __init addrconf_init(void)
 				   RTNL_FLAG_DOIT_UNLOCKED);
 	if (err < 0)
 		goto errout;
+	err = rtnl_register_module(THIS_MODULE, PF_INET6, RTM_GETADDR2,
+				   inet6_rtm_getaddr, inet6_dump_ifaddr2,
+				   RTNL_FLAG_DOIT_UNLOCKED);
+	if (err < 0)
+		goto errout;
 	err = rtnl_register_module(THIS_MODULE, PF_INET6, RTM_GETMULTICAST,
 				   NULL, inet6_dump_ifmcaddr, 0);
 	if (err < 0)
-- 
2.17.1