[PATCH 0/3] Fix umount.nfs

[PATCH 0/3] Fix umount.nfs

[Chuck Lever]

Hi Steve-

This short series addresses a couple of problems found with
umount.nfs.  See RH bugzilla 492598.

After this series is applied, umount.nfs will send UMNT requests from
a privileged source port.  This appears to match the behavior of
umount.nfs before nfs-utils 1.1.5, and is required when the "secure"
export option is used on the server.

A minor problem with the value returned by nfs_mount_protocol() is
also addressed.  This might affect umount.nfs and NFSv2/v3
version/protocol fallback by sending MNT and UMNT requests using
the wrong transport protocol.

---

Chuck Lever (3):
      umount.nfs: Fix return value of nfs_mount_protocol()
      umount.nfs: Use a privileged port when sending UMNT requests
      support: Provide an API for creating a privileged RPC client


 support/include/nfsrpc.h |   11 +++-
 support/nfs/rpc_socket.c |  140 ++++++++++++++++++++++++++++++++++++++++++++--
 utils/mount/network.c    |    4 +
 3 files changed, 145 insertions(+), 10 deletions(-)

-- 
Chuck Lever <chuck.lever@oracle.com>

[PATCH 1/3] support: Provide an API for creating a privileged RPC client

[Chuck Lever]

We needed to guarantee that some RPC programs, such as PMAP, got an
unprivileged port, to prevent exhausting the local privileged port
space sending RPC requests that don't need such privileges.
nfs_get_rpcclient() provides that feature.

However, some RPC programs, such as MNT and UMNT, require a privileged
port.  So, let's provide an additional API for this that also supports
IPv6 and setting a destination port.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 support/include/nfsrpc.h |   11 +++-
 support/nfs/rpc_socket.c |  140 ++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 143 insertions(+), 8 deletions(-)

diff --git a/support/include/nfsrpc.h b/support/include/nfsrpc.h
index 097debb..543c35b 100644
--- a/support/include/nfsrpc.h
+++ b/support/include/nfsrpc.h
@@ -24,6 +24,7 @@
 #define __NFS_UTILS_NFSRPC_H
 
 #include <rpc/types.h>
+#include <rpc/clnt.h>
 
 /*
  * Conventional RPC program numbers
@@ -54,7 +55,7 @@
 extern rpcprog_t	nfs_getrpcbyname(const rpcprog_t, const char *table[]);
 
 /*
- * Acquire an RPC CLIENT *
+ * Acquire an RPC CLIENT * with an ephemeral source port
  */
 extern CLIENT		*nfs_get_rpcclient(const struct sockaddr *,
 				const socklen_t, const unsigned short,
@@ -62,6 +63,14 @@ extern CLIENT		*nfs_get_rpcclient(const struct sockaddr *,
 				struct timeval *);
 
 /*
+ * Acquire an RPC CLIENT * with a privileged source port
+ */
+extern CLIENT		*nfs_get_priv_rpcclient( const struct sockaddr *,
+				const socklen_t, const unsigned short,
+				const rpcprog_t, const rpcvers_t,
+				struct timeval *);
+
+/*
  * Convert a socket address to a universal address
  */
 extern char		*nfs_sockaddr2universal(const struct sockaddr *,
diff --git a/support/nfs/rpc_socket.c b/support/nfs/rpc_socket.c
index 2b11e35..85e6064 100644
--- a/support/nfs/rpc_socket.c
+++ b/support/nfs/rpc_socket.c
@@ -132,6 +132,58 @@ static int nfs_bind(const int sock, const sa_family_t family)
 	return -1;
 }
 
+#ifdef IPV6_SUPPORT
+
+/*
+ * Bind a socket using an unused privileged source port.
+ *
+ * Returns zero on success, or returns -1 on error.  errno is
+ * set to reflect the nature of the error.
+ */
+static int nfs_bindresvport(const int sock, const sa_family_t family)
+{
+	struct sockaddr_in sin = {
+		.sin_family		= AF_INET,
+		.sin_addr.s_addr	= htonl(INADDR_ANY),
+	};
+	struct sockaddr_in6 sin6 = {
+		.sin6_family		= AF_INET6,
+		.sin6_addr		= IN6ADDR_ANY_INIT,
+	};
+
+	switch (family) {
+	case AF_INET:
+		return bindresvport_sa(sock, (struct sockaddr *)&sin,
+					(socklen_t)sizeof(sin));
+	case AF_INET6:
+		return bindresvport_sa(sock, (struct sockaddr *)&sin6,
+					(socklen_t)sizeof(sin6));
+	}
+
+	errno = EAFNOSUPPORT;
+	return -1;
+}
+
+#else	/* !IPV6_SUPPORT */
+
+/*
+ * Bind a socket using an unused privileged source port.
+ *
+ * Returns zero on success, or returns -1 on error.  errno is
+ * set to reflect the nature of the error.
+ */
+static int nfs_bindresvport(const int sock, const sa_family_t family)
+{
+	if (family != AF_INET) {
+		errno = EAFNOSUPPORT;
+		return -1;
+	}
+
+	return bindresvport(sock, NULL);
+}
+
+#endif	/* !IPV6_SUPPORT */
+
 /*
  * Perform a non-blocking connect on the socket fd.
  *
@@ -218,7 +270,8 @@ static CLIENT *nfs_get_udpclient(const struct sockaddr *sap,
 				 const socklen_t salen,
 				 const rpcprog_t program,
 				 const rpcvers_t version,
-				 struct timeval *timeout)
+				 struct timeval *timeout,
+				 const int resvport)
 {
 	CLIENT *client;
 	int ret, sock;
@@ -245,7 +298,10 @@ static CLIENT *nfs_get_udpclient(const struct sockaddr *sap,
 		return NULL;
 	}
 
-	ret = nfs_bind(sock, sap->sa_family);
+	if (resvport)
+		ret = nfs_bindresvport(sock, sap->sa_family);
+	else
+		ret = nfs_bind(sock, sap->sa_family);
 	if (ret < 0) {
 		rpc_createerr.cf_stat = RPC_SYSTEMERROR;
 		rpc_createerr.cf_error.re_errno = errno;
@@ -294,7 +350,8 @@ static CLIENT *nfs_get_tcpclient(const struct sockaddr *sap,
 				 const socklen_t salen,
 				 const rpcprog_t program,
 				 const rpcvers_t version,
-				 struct timeval *timeout)
+				 struct timeval *timeout,
+				 const int resvport)
 {
 	CLIENT *client;
 	int ret, sock;
@@ -321,7 +378,10 @@ static CLIENT *nfs_get_tcpclient(const struct sockaddr *sap,
 		return NULL;
 	}
 
-	ret = nfs_bind(sock, sap->sa_family);
+	if (resvport)
+		ret = nfs_bindresvport(sock, sap->sa_family);
+	else
+		ret = nfs_bind(sock, sap->sa_family);
 	if (ret < 0) {
 		rpc_createerr.cf_stat = RPC_SYSTEMERROR;
 		rpc_createerr.cf_error.re_errno = errno;
@@ -365,7 +425,8 @@ static CLIENT *nfs_get_tcpclient(const struct sockaddr *sap,
  * @timeout: pointer to request timeout (must not be NULL)
  *
  * Set up an RPC client for communicating with an RPC program @program
- * and @version on the server @sap over @transport.
+ * and @version on the server @sap over @transport.  An unprivileged
+ * source port is used.
  *
  * Returns a pointer to a prepared RPC client if successful, and
  * @timeout is initialized; caller must destroy a non-NULL returned RPC
@@ -405,10 +466,75 @@ CLIENT *nfs_get_rpcclient(const struct sockaddr *sap,
 
 	switch (transport) {
 	case IPPROTO_TCP:
-		return nfs_get_tcpclient(sap, salen, program, version, timeout);
+		return nfs_get_tcpclient(sap, salen, program, version,
+						timeout, 0);
+	case 0:
+	case IPPROTO_UDP:
+		return nfs_get_udpclient(sap, salen, program, version,
+						timeout, 0);
+	}
+
+	rpc_createerr.cf_stat = RPC_UNKNOWNPROTO;
+	return NULL;
+}
+
+/**
+ * nfs_get_priv_rpcclient - acquire an RPC client
+ * @sap: pointer to socket address of RPC server
+ * @salen: length of socket address
+ * @transport: IPPROTO_ value of transport protocol to use
+ * @program: RPC program number
+ * @version: RPC version number
+ * @timeout: pointer to request timeout (must not be NULL)
+ *
+ * Set up an RPC client for communicating with an RPC program @program
+ * and @version on the server @sap over @transport.  A privileged
+ * source port is used.
+ *
+ * Returns a pointer to a prepared RPC client if successful, and
+ * @timeout is initialized; caller must destroy a non-NULL returned RPC
+ * client.  Otherwise returns NULL, and rpc_createerr.cf_stat is set to
+ * reflect the error.
+ */
+CLIENT *nfs_get_priv_rpcclient(const struct sockaddr *sap,
+			       const socklen_t salen,
+			       const unsigned short transport,
+			       const rpcprog_t program,
+			       const rpcvers_t version,
+			       struct timeval *timeout)
+{
+	struct sockaddr_in *sin = (struct sockaddr_in *)sap;
+	struct sockaddr_in6 *sin6 = (struct sockaddr_in6 *)sap;
+
+	switch (sap->sa_family) {
+	case AF_LOCAL:
+		return nfs_get_localclient(sap, salen, program,
+						version, timeout);
+	case AF_INET:
+		if (sin->sin_port == 0) {
+			rpc_createerr.cf_stat = RPC_UNKNOWNADDR;
+			return NULL;
+		}
+		break;
+	case AF_INET6:
+		if (sin6->sin6_port == 0) {
+			rpc_createerr.cf_stat = RPC_UNKNOWNADDR;
+			return NULL;
+		}
+		break;
+	default:
+		rpc_createerr.cf_stat = RPC_UNKNOWNHOST;
+		return NULL;
+	}
+
+	switch (transport) {
+	case IPPROTO_TCP:
+		return nfs_get_tcpclient(sap, salen, program, version,
+						timeout, 1);
 	case 0:
 	case IPPROTO_UDP:
-		return nfs_get_udpclient(sap, salen, program, version, timeout);
+		return nfs_get_udpclient(sap, salen, program, version,
+						timeout, 1);
 	}
 
 	rpc_createerr.cf_stat = RPC_UNKNOWNPROTO;

[PATCH 2/3] umount.nfs: Use a privileged port when sending UMNT requests

[Chuck Lever]

Turns out we do actually need to use a privileged port for UMNT.  The
Linux rpc.mountd complains if an ephemeral source port is used:

  Apr 17 15:52:19 ingres mountd[2061]: refused unmount request from
    192.168.0.59 for /export (/export): illegal port 60932

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 utils/mount/network.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/utils/mount/network.c b/utils/mount/network.c
index bcd0c0f..234e7f3 100644
--- a/utils/mount/network.c
+++ b/utils/mount/network.c
@@ -869,7 +869,7 @@ int nfs_advise_umount(const struct sockaddr *sap, const socklen_t salen,
 	memcpy(saddr, sap, salen);
 	nfs_set_port(saddr, mnt_pmap.pm_port);
 
-	client = nfs_get_rpcclient(saddr, salen, mnt_pmap.pm_prot,
+	client = nfs_get_priv_rpcclient(saddr, salen, mnt_pmap.pm_prot,
 					mnt_pmap.pm_prog, mnt_pmap.pm_vers,
 					&timeout);
 	if (client == NULL)

[PATCH 3/3] umount.nfs: Fix return value of nfs_mount_protocol()

[Chuck Lever]

Fix a copy-paste error introduced in nfs_mount_protocol().  It should
return an IPPROTO_ number, not an NFS version number.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 utils/mount/network.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/utils/mount/network.c b/utils/mount/network.c
index 234e7f3..72f4b84 100644
--- a/utils/mount/network.c
+++ b/utils/mount/network.c
@@ -1337,7 +1337,7 @@ static unsigned short nfs_mount_protocol(struct mount_options *options)
 			return IPPROTO_UDP;
 	}
 
-	return nfs_nfs_version(options);
+	return nfs_nfs_protocol(options);
 }
 
 /*

Re: [PATCH 0/3] Fix umount.nfs

[Steve Dickson]

Chuck Lever wrote:
> Hi Steve-
> 
> This short series addresses a couple of problems found with
> umount.nfs.  See RH bugzilla 492598.
> 
> After this series is applied, umount.nfs will send UMNT requests from
> a privileged source port.  This appears to match the behavior of
> umount.nfs before nfs-utils 1.1.5, and is required when the "secure"
> export option is used on the server.
> 
> A minor problem with the value returned by nfs_mount_protocol() is
> also addressed.  This might affect umount.nfs and NFSv2/v3
> version/protocol fallback by sending MNT and UMNT requests using
> the wrong transport protocol.
> 
> ---
> 
> Chuck Lever (3):
>       umount.nfs: Fix return value of nfs_mount_protocol()
>       umount.nfs: Use a privileged port when sending UMNT requests
>       support: Provide an API for creating a privileged RPC client
> 
> 
>  support/include/nfsrpc.h |   11 +++-
>  support/nfs/rpc_socket.c |  140 ++++++++++++++++++++++++++++++++++++++++++++--
>  utils/mount/network.c    |    4 +
>  3 files changed, 145 insertions(+), 10 deletions(-)
> 
All three patched tested and committed.

steved.