[PATCH v3 0/4] Avoid IANA-assigned port numbers

[PATCH v3 0/4] Avoid IANA-assigned port numbers

[Chuck Lever]

Following up on https://bugzilla.linux-nfs.org/show_bug.cgi?id=320 .

Here's a possible way to get libtirpc to avoid IANA-assigned port
numbers when a caller requests a dynamically-assigned port. This
approach also reduces the number of reserved ports used by long
running user space RPC daemons, and can be applied in combination
with changing bindresvport(3) to skip port numbers that appear in
/etc/services.

Changes since v2:
- make port assignment more random across serial callers and
  processes

Changes since RFC:
- fixed bugs
- re-organized the patches slightly
- clarified the patch descriptions

---

Chuck Lever (4):
      Add an internal helper for binding to a dynamically-assigned port
      Avoid choosing reserved ports in svc_tli_create(3)
      Avoid choosing reserved ports in clnt_tli_create(3)
      Avoid choosing reserved ports in legacy RPC APIs


 src/Makefile.am    |    5 +-
 src/binddynport.c  |  139 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/clnt_generic.c |    4 +
 src/rpc_soc.c      |   10 +---
 src/svc_generic.c  |   12 +---
 5 files changed, 153 insertions(+), 17 deletions(-)
 create mode 100644 src/binddynport.c

--
Chuck Lever

[PATCH v3 1/4] Add an internal helper for binding to a dynamically-assigned port

[Chuck Lever]

Create a helper function akin to bindresvport(3) that instead binds
to a dynamically assigned port using the rules in RFC 6335 Section 6
to avoid all IANA-assigned service port numbers.

This is intended to remain an internal helper for the time being, so
this commit provides no header declaration.

All internal bindresvport(3) call sites manufacture an INADDR_ANY-
type address to pass to bind(2), so the helper handles that as well,
to avoid code duplication. This means that callers do not need to
pass in a sockaddr. Only an open socket is required.

BugLink: https://bugzilla.linux-nfs.org/show_bug.cgi?id=320
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 src/Makefile.am   |    5 +-
 src/binddynport.c |  139 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 142 insertions(+), 2 deletions(-)
 create mode 100644 src/binddynport.c

diff --git a/src/Makefile.am b/src/Makefile.am
index fba2aa4..932414d 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -15,8 +15,9 @@ lib_LTLIBRARIES = libtirpc.la
 libtirpc_la_LDFLAGS = @LDFLAG_NOUNDEFINED@ -no-undefined -lpthread
 libtirpc_la_LDFLAGS += -version-info @LT_VERSION_INFO@
 
-libtirpc_la_SOURCES = auth_none.c auth_unix.c authunix_prot.c bindresvport.c clnt_bcast.c \
-        clnt_dg.c clnt_generic.c clnt_perror.c clnt_raw.c clnt_simple.c \
+libtirpc_la_SOURCES = auth_none.c auth_unix.c authunix_prot.c \
+        binddynport.c bindresvport.c \
+        clnt_bcast.c clnt_dg.c clnt_generic.c clnt_perror.c clnt_raw.c clnt_simple.c \
         clnt_vc.c rpc_dtablesize.c getnetconfig.c getnetpath.c getrpcent.c \
         getrpcport.c mt_misc.c pmap_clnt.c pmap_getmaps.c pmap_getport.c \
         pmap_prot.c pmap_prot2.c pmap_rmt.c rpc_prot.c rpc_commondata.c \
diff --git a/src/binddynport.c b/src/binddynport.c
new file mode 100644
index 0000000..062629a
--- /dev/null
+++ b/src/binddynport.c
@@ -0,0 +1,139 @@
+/*
+ * Copyright (c) 2018, Oracle America, Inc.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ * - Redistributions of source code must retain the above copyright notice,
+ *   this list of conditions and the following disclaimer.
+ * - Redistributions in binary form must reproduce the above copyright notice,
+ *   this list of conditions and the following disclaimer in the documentation
+ *   and/or other materials provided with the distribution.
+ * - Neither the name of "Oracle America, Inc." nor the names of its
+ *   contributors may be used to endorse or promote products derived
+ *   from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE
+ * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include <sys/types.h>
+#include <sys/socket.h>
+#include <sys/time.h>
+
+#include <netdb.h>
+#include <netinet/in.h>
+
+#include <stdlib.h>
+#include <unistd.h>
+#include <errno.h>
+#include <string.h>
+
+#include <rpc/rpc.h>
+
+#include "reentrant.h"
+#include "rpc_com.h"
+
+extern pthread_mutex_t port_lock;
+
+/*
+ * Dynamic port range as defined in RFC 6335 Section 6.
+ * This range avoids all IANA-assigned service port
+ * numbers.
+ */
+enum {
+	LOWPORT		= 49152,
+	ENDPORT		= 65534,
+	NPORTS		= ENDPORT - LOWPORT + 1,
+};
+
+/*
+ * Bind a socket to a dynamically-assigned IP port.
+ *
+ * @fd is an open but unbound socket.
+ *
+ * On each call, a port number is chosen at random from
+ * within the dynamic/private port range, even if the
+ * caller has CAP_NET_ADMIN_BIND.
+ *
+ * Returns 0 on success, -1 on failure. errno may be
+ * set to a non-determinant value.
+ *
+ * This function is re-entrant.
+ */
+int __binddynport(int fd)
+{
+	struct sockaddr_storage ss;
+#ifdef INET6
+	struct sockaddr_in6 *sin6;
+#endif
+	struct sockaddr_in *sin;
+	static unsigned int seed;
+	in_port_t port, *portp;
+	struct sockaddr *sap;
+	socklen_t salen;
+	int i, res;
+
+	if (__rpc_sockisbound(fd))
+		return 0;
+
+	res = -1;
+	sap = (struct sockaddr *)(void *)&ss;
+	salen = sizeof(ss);
+	memset(sap, 0, salen);
+
+	mutex_lock(&port_lock);
+
+	if (getsockname(fd, sap, &salen) == -1)
+		goto out;
+
+	switch (ss.ss_family) {
+	case AF_INET:
+		sin = (struct sockaddr_in *)(void *)&ss;
+		portp = &sin->sin_port;
+		salen = sizeof(struct sockaddr_in);
+		break;
+#ifdef INET6
+	case AF_INET6:
+		sin6 = (struct sockaddr_in6 *)(void *)&ss;
+		portp = &sin6->sin6_port;
+		salen = sizeof(struct sockaddr_in6);
+		break;
+#endif
+	default:
+		goto out;
+	}
+
+	if (!seed) {
+		struct timeval tv;
+
+		gettimeofday(&tv, NULL);
+		seed = tv.tv_usec * getpid();
+	}
+	port = (rand_r(&seed) % NPORTS) + LOWPORT;
+	for (i = 0; i < NPORTS; ++i) {
+		*portp = htons(port++);
+		res = bind(fd, sap, salen);
+		if (res >= 0) {
+			res = 0;
+			break;
+		}
+		if (errno != EADDRINUSE)
+			break;
+		if (port > ENDPORT)
+			port = LOWPORT;
+	}
+
+out:
+	mutex_unlock(&port_lock);
+	return res;
+}

[PATCH v3 2/4] Avoid choosing reserved ports in svc_tli_create(3)

[Chuck Lever]

Callers of svc_tli_create(3) can specify that an arbitrary port
number be dynamically assigned for the service listener being
created. svc_tli_create(3) tries bindresvport(3) first in this
case. bindresvport(3) chooses a reserved port if the caller has
CAP_NET_ADMIN_BIND privilege. If this fails, bind(2) is used to
assign a port number from the range above 1024.

This approach becomes a problem should bindresvport(3) or bind(2)
happen to choose the port number of a well-known service. If the
caller is a long-running service (like rpc.statd), the caller's
listener indefinitely blocks the IANA-assigned well-known service
for that port from starting.

Moreover, it seems that a reserved port is completely unnecessary
for listener sockets. It does not confer any extra privilege or
functionality to the listener socket, nor do remote clients infer
any extra privilege from a listener on a port number lower than
1024.

Therefore, remove the bindresvport step, and instead of invoking
bind(2) directly, use a mechanism which allocates the port number
from the dynamic port range described in RFC 6335 Section 6.

This also impacts all users of svc_tli_create(3) within the library,
such as svc_tp_create(3).

BugLink: https://bugzilla.linux-nfs.org/show_bug.cgi?id=320
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 src/svc_generic.c |   12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/src/svc_generic.c b/src/svc_generic.c
index 7aae796..52a56c2 100644
--- a/src/svc_generic.c
+++ b/src/svc_generic.c
@@ -53,6 +53,7 @@
 #include <rpc/svc.h>
 
 extern int __svc_vc_setflag(SVCXPRT *, int);
+extern int __binddynport(int fd);
 
 /*
  * The highest level interface for server creation.
@@ -220,15 +221,10 @@ svc_tli_create(fd, nconf, bindaddr, sendsz, recvsz)
 	 */
 	if (madefd || !__rpc_sockisbound(fd)) {
 		if (bindaddr == NULL) {
-			if (bindresvport(fd, NULL) < 0) {
-				memset(&ss, 0, sizeof ss);
-				ss.ss_family = si.si_af;
-				if (bind(fd, (struct sockaddr *)(void *)&ss,
-				    (socklen_t)si.si_alen) < 0) {
-					warnx(
+			if (__binddynport(fd) == -1) {
+				warnx(
 			"svc_tli_create: could not bind to anonymous port");
-					goto freedata;
-				}
+				goto freedata;
 			}
 			listen(fd, SOMAXCONN);
 		} else {

[PATCH v3 3/4] Avoid choosing reserved ports in clnt_tli_create(3)

[Chuck Lever]

Callers of clnt_tli_create(3) can specify that an arbitrary port
number be dynamically assigned for the client socket being created.
clnt_tli_create(3) tries bindresvport(3) first in this case.
bindresvport(3) chooses a reserved port if the caller has
CAP_NET_ADMIN_BIND privilege. If this fails, bind(2) is used to
assign a port number from the range above 1024.

This approach becomes a problem should bindresvport(3) or bind(2)
happen to choose the port number of a well-known service. If the
caller is a long-running service (like rpc.statd), it indefinitely
blocks the IANA-assigned well-known service for that port from
starting.

When using the AUTH_SYS authentication flavor, RPC services can use
the remote client's source port number to determine whether the
client is privileged, and thus the UID and GID numbers in the RPC
are trustworthy. However, it's pretty easy for a man-in-the-middle
to replace these values while the RPC is in flight. The source port
number is no guarantee of actual security.

Therefore, remove the bindresvport step, and instead of invoking
bind(2) directly, use a mechanism which allocates the port number
from the dynamic port range described in RFC 6335 Section 6.

This also impacts all users of clnt_tli_create(3) within the
library, such as clnt_tp_create(3), and the portmap/rpcbind clients.

BugLink: https://bugzilla.linux-nfs.org/show_bug.cgi?id=320
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 src/clnt_generic.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/clnt_generic.c b/src/clnt_generic.c
index 3f3dabf..e5a314f 100644
--- a/src/clnt_generic.c
+++ b/src/clnt_generic.c
@@ -47,6 +47,7 @@
 
 extern bool_t __rpc_is_local_host(const char *);
 int __rpc_raise_fd(int);
+extern int __binddynport(int fd);
 
 #ifndef NETIDLEN
 #define	NETIDLEN 32
@@ -340,7 +341,8 @@ clnt_tli_create(int fd, const struct netconfig *nconf,
 		servtype = nconf->nc_semantics;
 		if (!__rpc_fd2sockinfo(fd, &si))
 			goto err;
-		bindresvport(fd, NULL);
+		if (__binddynport(fd) == -1)
+			goto err;
 	} else {
 		if (!__rpc_fd2sockinfo(fd, &si))
 			goto err;

[PATCH v3 4/4] Avoid choosing reserved ports in legacy RPC APIs

[Chuck Lever]

Prevent a caller of legacy RPC client and server APIs from
dynamically allocating a well-known port number, when no port number
is provided. This is similar to recent changes to svc_tli_create(3)
and clnt_tli_create(3).

BugLink: https://bugzilla.linux-nfs.org/show_bug.cgi?id=320
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 src/rpc_soc.c |   10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/src/rpc_soc.c b/src/rpc_soc.c
index ed0892a..af6c482 100644
--- a/src/rpc_soc.c
+++ b/src/rpc_soc.c
@@ -67,6 +67,8 @@
 
 extern mutex_t	rpcsoc_lock;
 
+extern int __binddynport(int fd);
+
 static CLIENT *clnt_com_create(struct sockaddr_in *, rpcprog_t, rpcvers_t,
     int *, u_int, u_int, char *, int);
 static SVCXPRT *svc_com_create(int, u_int, u_int, char *);
@@ -145,7 +147,8 @@ clnt_com_create(raddr, prog, vers, sockp, sendsz, recvsz, tp, flags)
 	bindaddr.maxlen = bindaddr.len =  sizeof (struct sockaddr_in);
 	bindaddr.buf = raddr;
 
-	bindresvport(fd, NULL);
+	if (__binddynport(fd) == -1)
+		goto err;
 	cl = clnt_tli_create(fd, nconf, &bindaddr, prog, vers,
 				sendsz, recvsz);
 	if (cl) {
@@ -313,7 +316,6 @@ svc_com_create(fd, sendsize, recvsize, netid)
 	SVCXPRT *svc;
 	int madefd = FALSE;
 	int port;
-	struct sockaddr_in sin;
 
 	if ((nconf = __rpc_getconfip(netid)) == NULL) {
 		(void) syslog(LOG_ERR, "Could not get %s transport", netid);
@@ -330,10 +332,6 @@ svc_com_create(fd, sendsize, recvsize, netid)
 		madefd = TRUE;
 	}
 
-	memset(&sin, 0, sizeof sin);
-	sin.sin_family = AF_INET;
-	bindresvport(fd, &sin);
-	listen(fd, SOMAXCONN);
 	svc = svc_tli_create(fd, nconf, NULL, sendsize, recvsize);
 	(void) freenetconfigent(nconf);
 	if (svc == NULL) {

Re: [PATCH v3 0/4] Avoid IANA-assigned port numbers

[Steve Dickson]

On 02/25/2018 01:17 PM, Chuck Lever wrote:
> Following up on https://bugzilla.linux-nfs.org/show_bug.cgi?id=320 .
> 
> Here's a possible way to get libtirpc to avoid IANA-assigned port
> numbers when a caller requests a dynamically-assigned port. This
> approach also reduces the number of reserved ports used by long
> running user space RPC daemons, and can be applied in combination
> with changing bindresvport(3) to skip port numbers that appear in
> /etc/services.
> 
> Changes since v2:
> - make port assignment more random across serial callers and
>   processes
> 
> Changes since RFC:
> - fixed bugs
> - re-organized the patches slightly
> - clarified the patch descriptions
The series was committed... Thank you!

steved.
> 
> ---
> 
> Chuck Lever (4):
>       Add an internal helper for binding to a dynamically-assigned port
>       Avoid choosing reserved ports in svc_tli_create(3)
>       Avoid choosing reserved ports in clnt_tli_create(3)
>       Avoid choosing reserved ports in legacy RPC APIs
> 
> 
>  src/Makefile.am    |    5 +-
>  src/binddynport.c  |  139 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  src/clnt_generic.c |    4 +
>  src/rpc_soc.c      |   10 +---
>  src/svc_generic.c  |   12 +---
>  5 files changed, 153 insertions(+), 17 deletions(-)
>  create mode 100644 src/binddynport.c
> 
> --
> Chuck Lever
>