From: Alexander Bluhm <alexander.bluhm@gmx.net>
Subject: Re: TCP port reuse at bind and connect
To: tech@openbsd.org
Date: Fri, 22 Mar 2024 11:46:34 +0100

On Thu, Mar 21, 2024 at 10:58:26PM +0100, Alexander Bluhm wrote:
> On top of that I found another problem.  in_pcbconnect() does not
> pass down the address it got from in_pcbselsrc() to in_pcbpickport().
> As a consequence local port numbers selected during connect(2) must
> be unique although they belong to different addresses.  This kind
> of uniqueness is not necessary and wastes more ports.
> 
> To solve this, I pass ina from in_pcbconnect() to in_pcbbind_locked().
> This does not interfere how wildcard sockets are matched with
> specific sockets during bind(2).  It only allows non-wildcard sockets
> to share a local port during connect(2).
> 
> I can split the latter bugfix from the diff if wanted.  The problem
> is just hard to see and test if all ports are random.

I think we should commit this part first.

ok?

bluhm

Index: netinet/in_pcb.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/in_pcb.c,v
diff -u -p -r1.296 in_pcb.c
--- netinet/in_pcb.c	29 Feb 2024 12:01:59 -0000	1.296
+++ netinet/in_pcb.c	22 Mar 2024 10:39:06 -0000
@@ -277,12 +277,12 @@ in_pcballoc(struct socket *so, struct in
 }
 
 int
-in_pcbbind_locked(struct inpcb *inp, struct mbuf *nam, struct proc *p)
+in_pcbbind_locked(struct inpcb *inp, struct mbuf *nam, const void *laddr,
+    struct proc *p)
 {
 	struct socket *so = inp->inp_socket;
 	u_int16_t lport = 0;
 	int wild = 0;
-	const void *laddr = &zeroin46_addr;
 	int error;
 
 	if (inp->inp_lport)
@@ -359,7 +359,7 @@ in_pcbbind(struct inpcb *inp, struct mbu
 
 	/* keep lookup, modification, and rehash in sync */
 	mtx_enter(&table->inpt_mtx);
-	error = in_pcbbind_locked(inp, nam, p);
+	error = in_pcbbind_locked(inp, nam, &zeroin46_addr, p);
 	mtx_leave(&table->inpt_mtx);
 
 	return error;
@@ -542,7 +542,7 @@ in_pcbconnect(struct inpcb *inp, struct 
 
 	if (inp->inp_laddr.s_addr == INADDR_ANY) {
 		if (inp->inp_lport == 0) {
-			error = in_pcbbind_locked(inp, NULL, curproc);
+			error = in_pcbbind_locked(inp, NULL, &ina, curproc);
 			if (error) {
 				mtx_leave(&table->inpt_mtx);
 				return (error);
Index: netinet/in_pcb.h
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/in_pcb.h,v
diff -u -p -r1.152 in_pcb.h
--- netinet/in_pcb.h	13 Feb 2024 12:22:09 -0000	1.152
+++ netinet/in_pcb.h	22 Mar 2024 10:39:37 -0000
@@ -307,7 +307,8 @@ extern int in_pcbnotifymiss;
 void	 in_init(void);
 void	 in_losing(struct inpcb *);
 int	 in_pcballoc(struct socket *, struct inpcbtable *, int);
-int	 in_pcbbind_locked(struct inpcb *, struct mbuf *, struct proc *);
+int	 in_pcbbind_locked(struct inpcb *, struct mbuf *, const void *,
+	    struct proc *);
 int	 in_pcbbind(struct inpcb *, struct mbuf *, struct proc *);
 int	 in_pcbaddrisavail(const struct inpcb *, struct sockaddr_in *, int,
 	    struct proc *);
Index: netinet6/in6_pcb.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/in6_pcb.c,v
diff -u -p -r1.141 in6_pcb.c
--- netinet6/in6_pcb.c	29 Feb 2024 12:01:59 -0000	1.141
+++ netinet6/in6_pcb.c	22 Mar 2024 10:40:47 -0000
@@ -313,7 +313,7 @@ in6_pcbconnect(struct inpcb *inp, struct
 
 	if (IN6_IS_ADDR_UNSPECIFIED(&inp->inp_laddr6)) {
 		if (inp->inp_lport == 0) {
-			error = in_pcbbind_locked(inp, NULL, curproc);
+			error = in_pcbbind_locked(inp, NULL, in6a, curproc);
 			if (error) {
 				mtx_leave(&table->inpt_mtx);
 				return (error);