Subject: Assigning low-numbered ports during connect()
To: None <tech-net@netbsd.org>
From: David Laight <david@l8s.co.uk>
List: tech-net
Date: 11/14/2005 22:24:17
I was writing some code to send UDP RPC requests and discovered that 
the code:
	sock = socket(d->ai->ai_family, SOCK_DGRAM, IPPROTO_UDP);
	range = IP_PORTRANGE_LOW;
	setsockopt(sock, IPPROTO_IP, IP_PORTRANGE, &range, sizeof(range));
	connect(sock, d->ai->ai_addr, d->ai->ai_addrlen);
will transmit a UDP request with a source port of zero.
(Whether executed by root or any other user.)

Without the setsockopt() a high numbered port is used (as expected).

Digging a little I discovered that in_pcbconnect() fails to pass the
'proc' pointer to in_pcbbind() so that it cannot allocate a low-numbered
port, and then fails to report the EACCESS error back to the user.
It then blindly transmits the UDP datagram from an unbound port.

Digging even further, it seems that when the IP_PORTRANGE_LOW stuff was
added in rev 1.45 (Jan 1998 - from FreeBSD) no one noticed that the
in_pcbbind() call in in_pcbconnect() could now fail (prior to that
in_pcbbind() would only allocate ports above 1024).  Later someone
made it return EAGAIN if all the local port were used - but still didn't
worry about the EACCESS return.

Before this ports below 1024 had to be explicitely bound - requiring
the applications to loop through the port numbers.

To behave sensibly the code needs to either:
1) Always allocate a high port
or
2) Allocate a low port for root and a high port for non-root
or
3) Allocate a low port for root and fail EACCESS for non-root
or
4) Always fail EACCESS

With the code above issing a bind() request is quite hard:
	static char ad[64];
	((struct sockaddr *)ad)->sa_family =
		    ((struct sockaddr *)d->ai->ai_addr)->sa_family;
	bind(sock, (void *)ad, d->ai->ai_addrlen);
seems to be enough, but you are close to needing to know the format
of the address buffer for the protocol.

The patch below implements (3) above.

	David

Index: netinet/in_pcb.c
===================================================================
RCS file: /cvsroot/src/sys/netinet/in_pcb.c,v
retrieving revision 1.100
diff -u -p -r1.100 in_pcb.c
--- netinet/in_pcb.c	29 May 2005 21:41:23 -0000	1.100
+++ netinet/in_pcb.c	14 Nov 2005 21:59:38 -0000
@@ -357,7 +357,7 @@ noname:
  * then pick one.
  */
 int
-in_pcbconnect(void *v, struct mbuf *nam)
+in_pcbconnect(void *v, struct mbuf *nam, struct proc *p)
 {
 	struct inpcb *inp = v;
 	struct in_ifaddr *ia = NULL;
@@ -427,15 +427,14 @@ in_pcbconnect(void *v, struct mbuf *nam)
 		return (EADDRINUSE);
 	if (in_nullhost(inp->inp_laddr)) {
 		if (inp->inp_lport == 0) {
-			error = in_pcbbind(inp, (struct mbuf *)0,
-			    (struct proc *)0);
+			error = in_pcbbind(inp, NULL, p);
 			/*
 			 * This used to ignore the return value
 			 * completely, but we need to check for
 			 * ephemeral port shortage.
-			 * XXX Should we check for other errors, too?
+			 * And attempts to request low ports if not root.
 			 */
-			if (error == EAGAIN)
+			if (error != 0)
 				return (error);
 		}
 		inp->inp_laddr = ifaddr->sin_addr;
Index: netinet/in_pcb.h
===================================================================
RCS file: /cvsroot/src/sys/netinet/in_pcb.h,v
retrieving revision 1.39
diff -u -p -r1.39 in_pcb.h
--- netinet/in_pcb.h	12 Feb 2005 12:31:07 -0000	1.39
+++ netinet/in_pcb.h	14 Nov 2005 21:59:38 -0000
@@ -118,7 +118,7 @@ struct inpcb {
 void	in_losing(struct inpcb *);
 int	in_pcballoc(struct socket *, void *);
 int	in_pcbbind(void *, struct mbuf *, struct proc *);
-int	in_pcbconnect(void *, struct mbuf *);
+int	in_pcbconnect(void *, struct mbuf *, struct proc *);
 void	in_pcbdetach(void *);
 void	in_pcbdisconnect(void *);
 void	in_pcbinit(struct inpcbtable *, int, int);
Index: netinet/tcp_input.c
===================================================================
RCS file: /cvsroot/src/sys/netinet/tcp_input.c,v
retrieving revision 1.236
diff -u -p -r1.236 tcp_input.c
--- netinet/tcp_input.c	12 Aug 2005 14:41:00 -0000	1.236
+++ netinet/tcp_input.c	14 Nov 2005 21:59:39 -0000
@@ -3735,7 +3735,7 @@ syn_cache_get(struct sockaddr *src, stru
 	am->m_len = src->sa_len;
 	bcopy(src, mtod(am, caddr_t), src->sa_len);
 	if (inp) {
-		if (in_pcbconnect(inp, am)) {
+		if (in_pcbconnect(inp, am, NULL)) {
 			(void) m_free(am);
 			goto resetandabort;
 		}
@@ -3756,7 +3756,7 @@ syn_cache_get(struct sockaddr *src, stru
 				&sin6->sin6_addr.s6_addr32[3],
 				sizeof(sin6->sin6_addr.s6_addr32[3]));
 		}
-		if (in6_pcbconnect(in6p, am)) {
+		if (in6_pcbconnect(in6p, am, NULL)) {
 			(void) m_free(am);
 			goto resetandabort;
 		}
Index: netinet/tcp_usrreq.c
===================================================================
RCS file: /cvsroot/src/sys/netinet/tcp_usrreq.c,v
retrieving revision 1.111
diff -u -p -r1.111 tcp_usrreq.c
--- netinet/tcp_usrreq.c	7 Sep 2005 17:58:13 -0000	1.111
+++ netinet/tcp_usrreq.c	14 Nov 2005 21:59:39 -0000
@@ -385,7 +385,7 @@ tcp_usrreq(struct socket *so, int req,
 				if (error)
 					break;
 			}
-			error = in_pcbconnect(inp, nam);
+			error = in_pcbconnect(inp, nam, p);
 		}
 #endif
 #ifdef INET6
@@ -396,7 +396,7 @@ tcp_usrreq(struct socket *so, int req,
 				if (error)
 					break;
 			}
-			error = in6_pcbconnect(in6p, nam);
+			error = in6_pcbconnect(in6p, nam, p);
 			if (!error) {
 				/* mapped addr case */
 				if (IN6_IS_ADDR_V4MAPPED(&in6p->in6p_faddr))
Index: netinet/udp_usrreq.c
===================================================================
RCS file: /cvsroot/src/sys/netinet/udp_usrreq.c,v
retrieving revision 1.142
diff -u -p -r1.142 udp_usrreq.c
--- netinet/udp_usrreq.c	3 Sep 2005 18:01:07 -0000	1.142
+++ netinet/udp_usrreq.c	14 Nov 2005 21:59:39 -0000
@@ -1226,7 +1226,7 @@ udp_usrreq(struct socket *so, int req, s
 		break;
 
 	case PRU_CONNECT:
-		error = in_pcbconnect(inp, nam);
+		error = in_pcbconnect(inp, nam, p);
 		if (error)
 			break;
 		soisconnected(so);
@@ -1268,7 +1268,7 @@ udp_usrreq(struct socket *so, int req, s
 				error = EISCONN;
 				goto die;
 			}
-			error = in_pcbconnect(inp, nam);
+			error = in_pcbconnect(inp, nam, p);
 			if (error)
 				goto die;
 		} else {
Index: netinet6/in6_pcb.c
===================================================================
RCS file: /cvsroot/src/sys/netinet6/in6_pcb.c,v
retrieving revision 1.67
diff -u -p -r1.67 in6_pcb.c
--- netinet6/in6_pcb.c	29 May 2005 21:43:51 -0000	1.67
+++ netinet6/in6_pcb.c	14 Nov 2005 21:59:40 -0000
@@ -344,9 +344,10 @@ in6_pcbbind(v, nam, p)
  * then pick one.
  */
 int
-in6_pcbconnect(v, nam)
+in6_pcbconnect(v, nam, p)
 	void *v;
 	struct mbuf *nam;
+	struct proc *p;
 {
 	struct in6pcb *in6p = v;
 	struct in6_addr *in6a = NULL;
@@ -449,8 +450,9 @@ in6_pcbconnect(v, nam)
 	     in6p->in6p_laddr.s6_addr32[3] == 0))
 	{
 		if (in6p->in6p_lport == 0) {
-			(void)in6_pcbbind(in6p, (struct mbuf *)0,
-			    (struct proc *)0);
+			error = in6_pcbbind(in6p, (struct mbuf *)0, p);
+			if (error != 0)
+				return error;
 		}
 		in6p->in6p_laddr = *in6a;
 	}
Index: netinet6/in6_pcb.h
===================================================================
RCS file: /cvsroot/src/sys/netinet6/in6_pcb.h,v
retrieving revision 1.26
diff -u -p -r1.26 in6_pcb.h
--- netinet6/in6_pcb.h	29 May 2005 21:43:51 -0000	1.26
+++ netinet6/in6_pcb.h	14 Nov 2005 21:59:40 -0000
@@ -152,7 +152,7 @@ void	in6_losing __P((struct in6pcb *));
 void	in6_pcbinit __P((struct inpcbtable *, int, int));
 int	in6_pcballoc __P((struct socket *, void *));
 int	in6_pcbbind __P((void *, struct mbuf *, struct proc *));
-int	in6_pcbconnect __P((void *, struct mbuf *));
+int	in6_pcbconnect __P((void *, struct mbuf *, struct proc *));
 void	in6_pcbdetach __P((struct in6pcb *));
 void	in6_pcbdisconnect __P((struct in6pcb *));
 struct	in6pcb *in6_pcblookup_port __P((struct inpcbtable *, struct in6_addr *,
Index: netinet6/udp6_usrreq.c
===================================================================
RCS file: /cvsroot/src/sys/netinet6/udp6_usrreq.c,v
retrieving revision 1.70
diff -u -p -r1.70 udp6_usrreq.c
--- netinet6/udp6_usrreq.c	28 Aug 2005 21:01:02 -0000	1.70
+++ netinet6/udp6_usrreq.c	14 Nov 2005 21:59:40 -0000
@@ -336,7 +336,7 @@ udp6_usrreq(so, req, m, addr6, control, 
 			break;
 		}
 		s = splsoftnet();
-		error = in6_pcbconnect(in6p, addr6);
+		error = in6_pcbconnect(in6p, addr6, p);
 		splx(s);
 		if (error == 0)
 			soisconnected(so);
-- 
David Laight: david@l8s.co.uk