[PATCH] Fix xprt_bindresvport

[PATCH] Fix xprt_bindresvport

[Olaf Kirch]

[-- Attachment #1: Type: text/plain, Size: 604 bytes --]

# Subject: Fix xprt_bindresvport
# 
# xprt_bindresvport would grab ports in the range 1-800, which can
# conflict with all sorts of network services. What we really want
# to do is select from a range N - 1023.
# 
# The patch below changes xprt_bindresvport to select ports from
# 650-1023 by default (631 is cups, which we better avoid).
# It also adds syscontrols to allow the admin to use a different
# port range.
# 
# Signed-off-by: Olaf Kirch <okir@suse.de>
-- 
Olaf Kirch   |  --- o --- Nous sommes du soleil we love when we play
okir@suse.de |    / | \   sol.dhoop.naytheet.ah kin.ir.samse.qurax

[-- Attachment #2: sunrpc-bindresvport --]
[-- Type: text/plain, Size: 4259 bytes --]

From: Olaf Kirch <okir@suse.de>
Subject: Fix sunrpc xprt_bindresvport

xprt_bindresvport would grab ports in the range 1-800, which can
conflict with all sorts of network services. What we really want
to do is select from a range N - 1023.

The patch below changes xprt_bindresvport to select ports from
650-1023 by default (631 is cups, which we better avoid).
It also adds syscontrols to allow the admin to use different
port range.

Signed-off-by: Olaf Kirch <okir@suse.de>

Index: linux-2.6.10/include/linux/sunrpc/debug.h
===================================================================
--- linux-2.6.10.orig/include/linux/sunrpc/debug.h	2004-10-18 23:55:36.000000000 +0200
+++ linux-2.6.10/include/linux/sunrpc/debug.h	2005-02-23 17:18:55.000000000 +0100
@@ -94,6 +94,8 @@ enum {
 	CTL_NLMDEBUG,
 	CTL_SLOTTABLE_UDP,
 	CTL_SLOTTABLE_TCP,
+	CTL_MIN_RESVPORT,
+	CTL_MAX_RESVPORT,
 };
 
 #endif /* _LINUX_SUNRPC_DEBUG_H_ */
Index: linux-2.6.10/include/linux/sunrpc/xprt.h
===================================================================
--- linux-2.6.10.orig/include/linux/sunrpc/xprt.h	2005-02-23 17:17:55.000000000 +0100
+++ linux-2.6.10/include/linux/sunrpc/xprt.h	2005-02-23 17:19:21.000000000 +0100
@@ -224,6 +224,9 @@ void			xprt_sock_setbufsize(struct rpc_x
 					(test_and_clear_bit(XPRT_CONNECT, &(xp)->sockstate))
 #define xprt_clear_connected(xp)	(clear_bit(XPRT_CONNECT, &(xp)->sockstate))
 
+extern unsigned int	xprt_min_resvport;
+extern unsigned int	xprt_max_resvport;
+
 #endif /* __KERNEL__*/
 
 #endif /* _LINUX_SUNRPC_XPRT_H */
Index: linux-2.6.10/net/sunrpc/sysctl.c
===================================================================
--- linux-2.6.10.orig/net/sunrpc/sysctl.c	2005-02-23 17:17:55.000000000 +0100
+++ linux-2.6.10/net/sunrpc/sysctl.c	2005-02-23 17:19:35.000000000 +0100
@@ -29,6 +29,10 @@ unsigned int	nfs_debug;
 unsigned int	nfsd_debug;
 unsigned int	nlm_debug;
 
+unsigned int	xprt_min_resvport = 650;
+unsigned int	xprt_max_resvport = 1023;
+
+
 #ifdef RPC_DEBUG
 
 static struct ctl_table_header *sunrpc_table_header;
@@ -121,6 +125,8 @@ done:
 
 static unsigned int min_slot_table_size = RPC_MIN_SLOT_TABLE;
 static unsigned int max_slot_table_size = RPC_MAX_SLOT_TABLE;
+static unsigned int xprt_min_resvport_limit = 1;
+static unsigned int xprt_max_resvport_limit = 65535;
 
 static ctl_table debug_table[] = {
 	{
@@ -156,6 +162,28 @@ static ctl_table debug_table[] = {
 		.proc_handler	= &proc_dodebug
 	}, 
 	{
+		.ctl_name	= CTL_MIN_RESVPORT,
+		.procname	= "min_resvport",
+		.data		= &xprt_min_resvport,
+		.maxlen		= sizeof(unsigned int),
+		.mode		= 0644,
+		.proc_handler	= &proc_dointvec_minmax,
+		.strategy	= &sysctl_intvec,
+		.extra1		= &xprt_min_resvport_limit,
+		.extra2		= &xprt_max_resvport_limit
+	},
+	{
+		.ctl_name	= CTL_MAX_RESVPORT,
+		.procname	= "max_resvport",
+		.data		= &xprt_max_resvport,
+		.maxlen		= sizeof(unsigned int),
+		.mode		= 0644,
+		.proc_handler	= &proc_dointvec_minmax,
+		.strategy	= &sysctl_intvec,
+		.extra1		= &xprt_min_resvport_limit,
+		.extra2		= &xprt_max_resvport_limit
+	},
+	{
 		.ctl_name	= CTL_SLOTTABLE_UDP,
 		.procname	= "udp_slot_table_entries",
 		.data		= &xprt_udp_slot_table_entries,
Index: linux-2.6.10/net/sunrpc/xprt.c
===================================================================
--- linux-2.6.10.orig/net/sunrpc/xprt.c	2005-02-23 17:17:55.000000000 +0100
+++ linux-2.6.10/net/sunrpc/xprt.c	2005-02-23 17:18:08.000000000 +0100
@@ -75,7 +75,6 @@
 
 #define XPRT_MAX_BACKOFF	(8)
 #define XPRT_IDLE_TIMEOUT	(5*60*HZ)
-#define XPRT_MAX_RESVPORT	(800)
 
 /*
  * Local functions
@@ -1492,7 +1491,7 @@ xprt_setup(int proto, struct sockaddr_in
 	xprt->timer.function = xprt_init_autodisconnect;
 	xprt->timer.data = (unsigned long) xprt;
 	xprt->last_used = jiffies;
-	xprt->port = XPRT_MAX_RESVPORT;
+	xprt->port = xprt_max_resvport;
 
 	/* Set timeout parameters */
 	if (to) {
@@ -1540,8 +1539,10 @@ static inline int xprt_bindresvport(stru
 			xprt->port = port;
 			return 0;
 		}
-		if (--port == 0)
-			port = XPRT_MAX_RESVPORT;
+		if (port < xprt_min_resvport)
+			port = xprt_max_resvport;
+		else
+			port--;
 	} while (err == -EADDRINUSE && port != xprt->port);
 
 	printk("RPC: Can't bind to reserved port (%d).\n", -err);

Re: [PATCH] Fix xprt_bindresvport

[Trond Myklebust]

m=C3=A5 den 11.07.2005 Klokka 11:25 (+0200) skreiv Olaf Kirch:
> # Subject: Fix xprt_bindresvport
> #=20
> # xprt_bindresvport would grab ports in the range 1-800, which can
> # conflict with all sorts of network services. What we really want
> # to do is select from a range N - 1023.
> #=20
> # The patch below changes xprt_bindresvport to select ports from
> # 650-1023 by default (631 is cups, which we better avoid).
> # It also adds syscontrols to allow the admin to use a different
> # port range.

Yep. I'll try to fold this in for 2.6.14.

Note that when we're using RPCSEC_GSS, we ought to drop the reserved
port requirement altogether (and make sure that the server always
exports with the "insecure" option by default).

Cheers,
  Trond



-------------------------------------------------------
This SF.Net email is sponsored by the 'Do More With Dual!' webinar happening
July 14 at 8am PDT/11am EDT. We invite you to explore the latest in dual
core and dual graphics technology at this free one hour event hosted by HP, 
AMD, and NVIDIA.  To register visit http://www.hp.com/go/dualwebinar
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [PATCH] Fix xprt_bindresvport

[Steve Dickson]

Olaf Kirch wrote:
>
>  #endif /* _LINUX_SUNRPC_XPRT_H */
> Index: linux-2.6.10/net/sunrpc/sysctl.c
> ===================================================================
> --- linux-2.6.10.orig/net/sunrpc/sysctl.c	2005-02-23 17:17:55.000000000 +0100
> +++ linux-2.6.10/net/sunrpc/sysctl.c	2005-02-23 17:19:35.000000000 +0100
> @@ -29,6 +29,10 @@ unsigned int	nfs_debug;
>  unsigned int	nfsd_debug;
>  unsigned int	nlm_debug;
>  
> +unsigned int	xprt_min_resvport = 650;
> +unsigned int	xprt_max_resvport = 1023;
> +
> +
>  #ifdef RPC_DEBUG
>  
>  static struct ctl_table_header *sunrpc_table_header;
> @@ -121,6 +125,8 @@ done:
>  
>  static unsigned int min_slot_table_size = RPC_MIN_SLOT_TABLE;
>  static unsigned int max_slot_table_size = RPC_MAX_SLOT_TABLE;
> +static unsigned int xprt_min_resvport_limit = 1;
> +static unsigned int xprt_max_resvport_limit = 65535;
Question, why is 65535 one being used here instead of something
like 1023 (or PROT_SOCK-1)? Since since inet_bind() will only
succeed with a source ports that are less than PROT_SOCK, so it
may not make sense to allow the user to set the max reserver port
to a value greater than PROT_SOCK-1, true?

steved.



-------------------------------------------------------
SF.Net email is sponsored by: Discover Easy Linux Migration Strategies
from IBM. Find simple to follow Roadmaps, straightforward articles,
informative Webcasts and more! Get everything you need to get up to
speed, fast. http://ads.osdn.com/?ad_id=7477&alloc_id=16492&op=click
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [PATCH] Fix xprt_bindresvport

[Olaf Kirch]

On Thu, Jul 14, 2005 at 12:00:11PM -0400, Steve Dickson wrote:
> Question, why is 65535 one being used here instead of something
> like 1023 (or PROT_SOCK-1)? Since since inet_bind() will only
> succeed with a source ports that are less than PROT_SOCK, so it
> may not make sense to allow the user to set the max reserver port
> to a value greater than PROT_SOCK-1, true?

I don't understand. Of course you can bind to any socket up to 65535,
not just 1023.
And in an environment where the admin doesn't care for privileged
vs unprivileged ports, it's good to give him the choice of allowing
the kernel to bind to any port.

Olaf
-- 
Olaf Kirch   |  --- o --- Nous sommes du soleil we love when we play
okir@suse.de |    / | \   sol.dhoop.naytheet.ah kin.ir.samse.qurax


-------------------------------------------------------
SF.Net email is sponsored by: Discover Easy Linux Migration Strategies
from IBM. Find simple to follow Roadmaps, straightforward articles,
informative Webcasts and more! Get everything you need to get up to
speed, fast. http://ads.osdn.com/?ad_id=7477&alloc_id=16492&op=click
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [PATCH] Fix xprt_bindresvport

[Steve Dickson]

Olaf Kirch wrote:
> On Thu, Jul 14, 2005 at 12:00:11PM -0400, Steve Dickson wrote:
> 
>>Question, why is 65535 one being used here instead of something
>>like 1023 (or PROT_SOCK-1)? Since since inet_bind() will only
>>succeed with a source ports that are less than PROT_SOCK, so it
>>may not make sense to allow the user to set the max reserver port
>>to a value greater than PROT_SOCK-1, true?
> 
> 
> I don't understand. Of course you can bind to any socket up to 65535,
> not just 1023.
Understood... but ports > 1023 are not considered privileged and
connections that are not using privilege ports will not be accepted
by a number of RPC daemon..

> And in an environment where the admin doesn't care for privileged
> vs unprivileged ports, it's good to give him the choice of allowing
> the kernel to bind to any port.
If such an environment exists, then I agree... but I don't think this
is the norm... I'm pretty sure a lot of RPC daemons check for
privileged port by default... Something I don't think admits can
turn off...

Don't get me wrong, I really like the idea of being able to define
the range of privileged ports the kernel can use (I think it will be
very handy), but by definition a privileged port is between 1 and 1023
and by setting the max port to 1023 we would be maintaining that
definition...

steved.


-------------------------------------------------------
SF.Net email is sponsored by: Discover Easy Linux Migration Strategies
from IBM. Find simple to follow Roadmaps, straightforward articles,
informative Webcasts and more! Get everything you need to get up to
speed, fast. http://ads.osdn.com/?ad_id=7477&alloc_id=16492&op=click
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [PATCH] Fix xprt_bindresvport

[Peter Åstrand]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1259 bytes --]

On Sun, 17 Jul 2005, Steve Dickson wrote:

>> And in an environment where the admin doesn't care for privileged
>> vs unprivileged ports, it's good to give him the choice of allowing
>> the kernel to bind to any port.
> If such an environment exists, then I agree... but I don't think this
> is the norm... I'm pretty sure a lot of RPC daemons check for
> privileged port by default... Something I don't think admits can
> turn off...

Novell Netware is one system which does not check for privileged ports. 
Also, when using SSH tunnels, typically you do not use privileged ports. 
(With unfs3 this can actually be done in a secure way by using the one 
time password feature.)


> Don't get me wrong, I really like the idea of being able to define
> the range of privileged ports the kernel can use (I think it will be
> very handy), but by definition a privileged port is between 1 and 1023
> and by setting the max port to 1023 we would be maintaining that
> definition...

I agree with that definition, but I also agree with Olaf Kirch: Adding a 
"noprivports" option is not as elegant.

-- 
Peter Åstrand		Chief Developer
Cendio			www.thinlinc.com
Teknikringen 3		www.cendio.se
583 30 Linköping        Phone: +46-13-21 46 00

RE: [PATCH] Fix xprt_bindresvport

[Lever, Charles]

> Olaf Kirch wrote:
> > On Thu, Jul 14, 2005 at 12:00:11PM -0400, Steve Dickson wrote:
> >=20
> >>Question, why is 65535 one being used here instead of something
> >>like 1023 (or PROT_SOCK-1)? Since since inet_bind() will only
> >>succeed with a source ports that are less than PROT_SOCK, so it
> >>may not make sense to allow the user to set the max reserver port
> >>to a value greater than PROT_SOCK-1, true?
> >=20
> >=20
> > I don't understand. Of course you can bind to any socket up=20
> to 65535,
> > not just 1023.
> Understood... but ports > 1023 are not considered privileged and
> connections that are not using privilege ports will not be accepted
> by a number of RPC daemon..

in addition, part of AUTH_SYS security (such as it is) is connecting via
a privileged port.

a better solution, which trond recently suggested, is to disable
xprt->resvport for AUTH_GSS-secured transports, but leave the other
security flavors as they are now.


-------------------------------------------------------
SF.Net email is sponsored by: Discover Easy Linux Migration Strategies
from IBM. Find simple to follow Roadmaps, straightforward articles,
informative Webcasts and more! Get everything you need to get up to
speed, fast. http://ads.osdn.com/?ad_id=7477&alloc_id=16492&op=click
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [PATCH] Fix xprt_bindresvport

[Olaf Kirch]

On Sun, Jul 17, 2005 at 09:02:20AM -0700, Lever, Charles wrote:
> in addition, part of AUTH_SYS security (such as it is) is connecting via
> a privileged port.
> 
> a better solution, which trond recently suggested, is to disable
> xprt->resvport for AUTH_GSS-secured transports, but leave the other
> security flavors as they are now.

I'm fine with capping the max port at 1023. I just thought it was
an elegant way to allow admins to dispense with the privport nonsense
altogether, if they want. The alternative would have been a mount
option "noprivports" or similar.

Olaf
-- 
Olaf Kirch   |  --- o --- Nous sommes du soleil we love when we play
okir@suse.de |    / | \   sol.dhoop.naytheet.ah kin.ir.samse.qurax


-------------------------------------------------------
SF.Net email is sponsored by: Discover Easy Linux Migration Strategies
from IBM. Find simple to follow Roadmaps, straightforward articles,
informative Webcasts and more! Get everything you need to get up to
speed, fast. http://ads.osdn.com/?ad_id=7477&alloc_id=16492&op=click
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs