[PATCH net-next] net: Fail explicit bind to local reserved ports

[PATCH net-next] net: Fail explicit bind to local reserved ports

[Subash Abhinov Kasiviswanathan]

Reserved ports may have some special use cases which are not suitable
for use by general userspace applications. Currently, ports specified
in ip_local_reserved_ports will not be returned only in case of
automatic port assignment.

In some cases, it maybe required to prevent the host from assigning
the ports even in case of explicit binds. Consider the case of a
transparent proxy where packets are being redirected. In case a socket
matches this connection, packets from this application would be
incorrectly sent to one of the endpoints.

Add a boolean sysctl flag 'reserved_port_bind'. Default value is 1
which preserves the existing behavior. Setting the value to 0 will
prevent userspace applications from binding to these ports even when
they are explicitly requested.

Cc: Sean Tranchetti <stranche@codeaurora.org>
Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
---
 Documentation/networking/ip-sysctl.txt | 5 +++++
 include/net/netns/ipv4.h               | 2 ++
 net/ipv4/af_inet.c                     | 3 +++
 net/ipv4/inet_connection_sock.c        | 7 +++++++
 net/ipv4/sysctl_net_ipv4.c             | 7 +++++++
 net/ipv4/udp.c                         | 5 +++++
 6 files changed, 29 insertions(+)

diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
index 49e95f4..8a9d649 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -945,6 +945,11 @@ ip_unprivileged_port_start - INTEGER
 
 	Default: 1024
 
+reserved_port_bind - BOOLEAN
+	If set, allows explicit bind requests to applications requesting
+	any port within the range of ip_local_reserved_ports.
+	Default: 1
+
 ip_nonlocal_bind - BOOLEAN
 	If set, allows processes to bind() to non-local IP addresses,
 	which can be quite useful - but may break some applications.
diff --git a/include/net/netns/ipv4.h b/include/net/netns/ipv4.h
index c0c0791..0941369 100644
--- a/include/net/netns/ipv4.h
+++ b/include/net/netns/ipv4.h
@@ -107,6 +107,8 @@ struct netns_ipv4 {
 #ifdef CONFIG_NET_L3_MASTER_DEV
 	int sysctl_raw_l3mdev_accept;
 #endif
+	int sysctl_reserved_port_bind;
+
 	int sysctl_tcp_early_demux;
 	int sysctl_udp_early_demux;
 
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 70f92aa..e1ad45d 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -1814,6 +1814,9 @@ static __net_init int inet_init_net(struct net *net)
 	net->ipv4.ip_local_ports.range[0] =  32768;
 	net->ipv4.ip_local_ports.range[1] =  60999;
 
+	/* Allow explicit binding to reserved ports */
+	net->ipv4.sysctl_reserved_port_bind = 1;
+
 	seqlock_init(&net->ipv4.ping_group_range.lock);
 	/*
 	 * Sane defaults - nobody may create ping sockets.
diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index f5c163d..6dda979 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -307,6 +307,13 @@ int inet_csk_get_port(struct sock *sk, unsigned short snum)
 	head = &hinfo->bhash[inet_bhashfn(net, port,
 					  hinfo->bhash_size)];
 	spin_lock_bh(&head->lock);
+
+	if (inet_is_local_reserved_port(net, snum) &&
+	    !net->ipv4.sysctl_reserved_port_bind) {
+		ret = 1;
+		goto fail_unlock;
+	}
+
 	inet_bind_bucket_for_each(tb, &head->chain)
 		if (net_eq(ib_net(tb), net) && tb->l3mdev == l3mdev &&
 		    tb->port == port)
diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
index 59ded25..557fdec 100644
--- a/net/ipv4/sysctl_net_ipv4.c
+++ b/net/ipv4/sysctl_net_ipv4.c
@@ -742,6 +742,13 @@ static int proc_fib_multipath_hash_policy(struct ctl_table *table, int write,
 		.proc_handler	= proc_do_large_bitmap,
 	},
 	{
+		.procname       = "reserved_port_bind",
+		.data           = &init_net.ipv4.sysctl_reserved_port_bind,
+		.maxlen         = sizeof(int),
+		.mode           = 0644,
+		.proc_handler   = proc_dointvec
+	},
+	{
 		.procname	= "ip_no_pmtu_disc",
 		.data		= &init_net.ipv4.sysctl_ip_no_pmtu_disc,
 		.maxlen		= sizeof(int),
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index d88821c..59a4274 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -274,6 +274,11 @@ int udp_lib_get_port(struct sock *sk, unsigned short snum,
 	} else {
 		hslot = udp_hashslot(udptable, net, snum);
 		spin_lock_bh(&hslot->lock);
+
+		if (inet_is_local_reserved_port(net, snum) &&
+		    !net->ipv4.sysctl_reserved_port_bind)
+			goto fail_unlock;
+
 		if (hslot->count > 10) {
 			int exist;
 			unsigned int slot2 = udp_sk(sk)->udp_portaddr_hash ^ snum;
-- 
1.9.1

Re: [PATCH net-next] net: Fail explicit bind to local reserved ports

[David Miller]

From: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
Date: Wed, 28 Aug 2019 21:26:54 -0600

> Reserved ports may have some special use cases which are not suitable
> for use by general userspace applications. Currently, ports specified
> in ip_local_reserved_ports will not be returned only in case of
> automatic port assignment.
> 
> In some cases, it maybe required to prevent the host from assigning
> the ports even in case of explicit binds. Consider the case of a
> transparent proxy where packets are being redirected. In case a socket
> matches this connection, packets from this application would be
> incorrectly sent to one of the endpoints.
> 
> Add a boolean sysctl flag 'reserved_port_bind'. Default value is 1
> which preserves the existing behavior. Setting the value to 0 will
> prevent userspace applications from binding to these ports even when
> they are explicitly requested.
> 
> Cc: Sean Tranchetti <stranche@codeaurora.org>
> Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>

I don't know how happy I am about this.  Whatever sets up the transparent
proxy business can block any attempt to communicate over these ports.

Also, protocols like SCTP need the new handling too.

Re: [PATCH net-next] net: Fail explicit bind to local reserved ports

[Subash Abhinov Kasiviswanathan]

> I don't know how happy I am about this.  Whatever sets up the 
> transparent
> proxy business can block any attempt to communicate over these ports.
> 
> Also, protocols like SCTP need the new handling too.

Hi David

The purpose of this patch was to allow the transparent proxy application
to block the specific socket ranges to prevent the communication on the
specific ports.

Dropping packets for this particular port using iptables could lead to
applications on the system getting stuck without getting a socket error.
If bind fails explicitly, the application can atleast retry for some 
other
port.
Is there some alternate existing mechanism to achieve this already?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project