Redo the security update done in rev 1.54 of src/sys/netinet/tcp_subr.c

and 1.84 of src/sys/netinet/udp_usrreq.c

The changes broken down:

- remove 0 as a wildcard for addresses and port numbers in
  src/sys/netinet/in_pcb.c:in_pcbnotify()
- add src/sys/netinet/in_pcb.c:in_pcbnotifyall() used to notify
  all sessions with the specific remote address.
- change
  - src/sys/netinet/udp_usrreq.c:udp_ctlinput()
  - src/sys/netinet/tcp_subr.c:tcp_ctlinput()
  to use in_pcbnotifyall() to notify multiple sessions, instead of
  using in_pcbnotify() with 0 as src address and as port numbers.
- remove check for src port == 0 in
  - src/sys/netinet/tcp_subr.c:tcp_ctlinput()
  - src/sys/netinet/udp_usrreq.c:udp_ctlinput()
  as they are no longer needed.
- move handling of redirects and host dead from in_pcbnotify() to
  udp_ctlinput() and tcp_ctlinput(), so they will call
  in_pcbnotifyall() to notify all sessions with the specific
  remote address.

Approved by:	jlemon
Inspired by:    NetBSD

5 files changed, 95 insertions, 74 deletions

diff --git a/sys/netinet/in_pcb.c b/sys/netinet/in_pcb.c
index c114eca..0a8e952 100644
--- a/sys/netinet/in_pcb.c
+++ b/sys/netinet/in_pcb.c
@@ -78,8 +78,6 @@
 
 struct	in_addr zeroin_addr;
 
-static void	in_rtchange __P((struct inpcb *, int));
-
 /*
  * These configure the range of local port addresses assigned to
  * "unspecified" outgoing connections/packets/whatever.
@@ -693,20 +691,6 @@ in_pcbnotify(head, dst, fport_arg, laddr, lport_arg, cmd, notify, tcp_sequence,
 	if (faddr.s_addr == INADDR_ANY)
 		return;
 
-	/*
-	 * Redirects go to all references to the destination,
-	 * and use in_rtchange to invalidate the route cache.
-	 * Dead host indications: notify all references to the destination.
-	 * Otherwise, if we have knowledge of the local port and address,
-	 * deliver only to that socket.
-	 */
-	if (PRC_IS_REDIRECT(cmd) || cmd == PRC_HOSTDEAD) {
-		fport = 0;
-		lport = 0;
-		laddr.s_addr = 0;
-		if (cmd != PRC_HOSTDEAD)
-			notify = in_rtchange;
-	}
 	errno = inetctlerrmap[cmd];
 	s = splnet();
 	for (inp = LIST_FIRST(head); inp != NULL;) {
@@ -717,12 +701,11 @@ in_pcbnotify(head, dst, fport_arg, laddr, lport_arg, cmd, notify, tcp_sequence,
 		}
 #endif
 		if (inp->inp_faddr.s_addr != faddr.s_addr ||
-		    inp->inp_socket == 0 ||
-		    (lport && inp->inp_lport != lport) ||
-		    (laddr.s_addr && inp->inp_laddr.s_addr != laddr.s_addr) ||
-		    (fport && inp->inp_fport != fport)) {
-			inp = LIST_NEXT(inp, inp_list);
-			continue;
+		    inp->inp_socket == 0 || inp->inp_lport != lport ||
+		    inp->inp_laddr.s_addr != laddr.s_addr ||
+		    inp->inp_fport != fport) {
+				inp = LIST_NEXT(inp, inp_list);
+				continue;
 		}
 		/*
 		 * If tcp_seq_check is set, then skip sessions where
@@ -743,6 +726,45 @@ in_pcbnotify(head, dst, fport_arg, laddr, lport_arg, cmd, notify, tcp_sequence,
 	splx(s);
 }
 
+void
+in_pcbnotifyall(head, dst, cmd, notify)
+	struct inpcbhead *head;
+	struct sockaddr *dst;
+	int cmd;
+	void (*notify) __P((struct inpcb *, int));
+{
+	register struct inpcb *inp, *oinp;
+	struct in_addr faddr;
+	int errno, s;
+
+	if ((unsigned)cmd > PRC_NCMDS || dst->sa_family != AF_INET)
+		return;
+	faddr = ((struct sockaddr_in *)dst)->sin_addr;
+	if (faddr.s_addr == INADDR_ANY)
+		return;
+
+	errno = inetctlerrmap[cmd];
+	s = splnet();
+	for (inp = LIST_FIRST(head); inp != NULL;) {
+#ifdef INET6
+		if ((inp->inp_vflag & INP_IPV4) == 0) {
+			inp = LIST_NEXT(inp, inp_list);
+			continue;
+		}
+#endif
+		if (inp->inp_faddr.s_addr != faddr.s_addr ||
+			inp->inp_socket == 0) {
+				inp = LIST_NEXT(inp, inp_list);
+				continue;
+		}
+		oinp = inp;
+		inp = LIST_NEXT(inp, inp_list);
+		if (notify)
+			(*notify)(oinp, errno);
+	}
+	splx(s);
+}
+
 /*
  * Check for alternatives when higher level complains
  * about service problems.  For now, invalidate cached
@@ -781,7 +803,7 @@ in_losing(inp)
  * After a routing change, flush old routing
  * and allocate a (hopefully) better one.
  */
-static void
+void
 in_rtchange(inp, errno)
 	register struct inpcb *inp;
 	int errno;
diff --git a/sys/netinet/in_pcb.h b/sys/netinet/in_pcb.h
index 08b1627..7e4cfbe 100644
--- a/sys/netinet/in_pcb.h
+++ b/sys/netinet/in_pcb.h
@@ -274,6 +274,7 @@ extern int	ipport_hifirstauto;
 extern int	ipport_hilastauto;
 
 void	in_losing __P((struct inpcb *));
+void	in_rtchange __P((struct inpcb *, int));
 int	in_pcballoc __P((struct socket *, struct inpcbinfo *, struct proc *));
 int	in_pcbbind __P((struct inpcb *, struct sockaddr *, struct proc *));
 int	in_pcbconnect __P((struct inpcb *, struct sockaddr *, struct proc *));
@@ -292,6 +293,8 @@ struct inpcb *
 void	in_pcbnotify __P((struct inpcbhead *, struct sockaddr *,
 	    u_int, struct in_addr, u_int, int, void (*)(struct inpcb *, int),
 		u_int32_t, int));
+void	in_pcbnotifyall __P((struct inpcbhead *, struct sockaddr *,
+	    int, void (*)(struct inpcb *, int)));
 void	in_pcbrehash __P((struct inpcb *));
 int	in_setpeeraddr __P((struct socket *so, struct sockaddr **nam));
 int	in_setsockaddr __P((struct socket *so, struct sockaddr **nam));
diff --git a/sys/netinet/tcp_subr.c b/sys/netinet/tcp_subr.c
index ed33547..03b5870 100644
--- a/sys/netinet/tcp_subr.c
+++ b/sys/netinet/tcp_subr.c
@@ -1024,32 +1024,30 @@ tcp_ctlinput(cmd, sa, vip)
 		}
 	} else if (cmd == PRC_MSGSIZE)
 		notify = tcp_mtudisc;
-	else if (!PRC_IS_REDIRECT(cmd) &&
-		 ((unsigned)cmd > PRC_NCMDS || inetctlerrmap[cmd] == 0))
+	else if (PRC_IS_REDIRECT(cmd)) {
+		/*
+		 * Redirects go to all references to the destination,
+		 * and use in_rtchange to invalidate the route cache.
+		 */
+		ip = 0;
+		notify = in_rtchange;
+	} else if (cmd == PRC_HOSTDEAD)
+		/*
+		 * Dead host indications: notify all references to the
+		 * destination.
+		 */
+		ip = 0;
+	else if ((unsigned)cmd > PRC_NCMDS || inetctlerrmap[cmd] == 0)
 		return;
 	if (ip) {
 		th = (struct tcphdr *)((caddr_t)ip 
 				       + (IP_VHL_HL(ip->ip_vhl) << 2));
 		if (tcp_seq_check == 1)
 			tcp_sequence = ntohl(th->th_seq);
-		/*
-		 * Only call in_pcbnotify if the src port number != 0, as we 
-		 * treat 0 as a wildcard in src/sys/in_pbc.c:in_pcbnotify()
-		 *
-		 * It's sufficient to check for src|local port, as we'll have no
-		 * sessions with src|local port == 0
-		 *
-		 * Without this a attacker sending ICMP messages, where the attached 
-		 * IP header (+ 8 bytes) has the address and port numbers == 0, would
-		 * have the ICMP message applied to all sessions (modulo TCP sequence
-		 * number check).
-		 */
-		if (th->th_sport == 0)
-			return;
 		in_pcbnotify(&tcb, sa, th->th_dport, ip->ip_src, th->th_sport,
 			cmd, notify, tcp_sequence, tcp_seq_check);
 	} else
-		in_pcbnotify(&tcb, sa, 0, zeroin_addr, 0, cmd, notify, 0, 0);
+		in_pcbnotifyall(&tcb, sa, cmd, notify);
 }
 
 #ifdef INET6
diff --git a/sys/netinet/tcp_timewait.c b/sys/netinet/tcp_timewait.c
index ed33547..03b5870 100644
--- a/sys/netinet/tcp_timewait.c
+++ b/sys/netinet/tcp_timewait.c
@@ -1024,32 +1024,30 @@ tcp_ctlinput(cmd, sa, vip)
 		}
 	} else if (cmd == PRC_MSGSIZE)
 		notify = tcp_mtudisc;
-	else if (!PRC_IS_REDIRECT(cmd) &&
-		 ((unsigned)cmd > PRC_NCMDS || inetctlerrmap[cmd] == 0))
+	else if (PRC_IS_REDIRECT(cmd)) {
+		/*
+		 * Redirects go to all references to the destination,
+		 * and use in_rtchange to invalidate the route cache.
+		 */
+		ip = 0;
+		notify = in_rtchange;
+	} else if (cmd == PRC_HOSTDEAD)
+		/*
+		 * Dead host indications: notify all references to the
+		 * destination.
+		 */
+		ip = 0;
+	else if ((unsigned)cmd > PRC_NCMDS || inetctlerrmap[cmd] == 0)
 		return;
 	if (ip) {
 		th = (struct tcphdr *)((caddr_t)ip 
 				       + (IP_VHL_HL(ip->ip_vhl) << 2));
 		if (tcp_seq_check == 1)
 			tcp_sequence = ntohl(th->th_seq);
-		/*
-		 * Only call in_pcbnotify if the src port number != 0, as we 
-		 * treat 0 as a wildcard in src/sys/in_pbc.c:in_pcbnotify()
-		 *
-		 * It's sufficient to check for src|local port, as we'll have no
-		 * sessions with src|local port == 0
-		 *
-		 * Without this a attacker sending ICMP messages, where the attached 
-		 * IP header (+ 8 bytes) has the address and port numbers == 0, would
-		 * have the ICMP message applied to all sessions (modulo TCP sequence
-		 * number check).
-		 */
-		if (th->th_sport == 0)
-			return;
 		in_pcbnotify(&tcb, sa, th->th_dport, ip->ip_src, th->th_sport,
 			cmd, notify, tcp_sequence, tcp_seq_check);
 	} else
-		in_pcbnotify(&tcb, sa, 0, zeroin_addr, 0, cmd, notify, 0, 0);
+		in_pcbnotifyall(&tcb, sa, cmd, notify);
 }
 
 #ifdef INET6
diff --git a/sys/netinet/udp_usrreq.c b/sys/netinet/udp_usrreq.c
index 60c80d2..2e7fe89 100644
--- a/sys/netinet/udp_usrreq.c
+++ b/sys/netinet/udp_usrreq.c
@@ -506,29 +506,29 @@ udp_ctlinput(cmd, sa, vip)
 {
 	register struct ip *ip = vip;
 	register struct udphdr *uh;
+	void (*notify) __P((struct inpcb *, int)) = udp_notify;
 
-	if (!PRC_IS_REDIRECT(cmd) &&
-	    ((unsigned)cmd >= PRC_NCMDS || inetctlerrmap[cmd] == 0))
+	if (PRC_IS_REDIRECT(cmd)) {
+		/*
+		 * Redirects go to all references to the destination,
+		 * and use in_rtchange to invalidate the route cache.
+		 */
+		ip = 0;
+		notify = in_rtchange;
+	} else if (cmd == PRC_HOSTDEAD)
+		/*
+		 * Dead host indications: notify all references to the
+		 * destination.
+		 */
+		ip = 0;
+	else if ((unsigned)cmd >= PRC_NCMDS || inetctlerrmap[cmd] == 0)
 		return;
 	if (ip) {
 		uh = (struct udphdr *)((caddr_t)ip + (ip->ip_hl << 2));
-		/*
-		 * Only call in_pcbnotify if the src port number != 0, as we 
-		 * treat 0 as a wildcard in src/sys/in_pbc.c:in_pcbnotify()
-		 *
-		 * It's sufficient to check for src|local port, as we'll have no 
-		 * sessions with src|local port == 0
-		 *
-		 * Without this a attacker sending ICMP messages, where the attached    
-		 * IP header (+ 8 bytes) has the address and port numbers == 0, would
-		 * have the ICMP message applied to all sessions.
-		 */
-		if (uh->uh_sport == 0)
-			return;
 		in_pcbnotify(&udb, sa, uh->uh_dport, ip->ip_src, uh->uh_sport,
-			cmd, udp_notify, 0, 0);
+			cmd, notify, 0, 0);
 	} else
-		in_pcbnotify(&udb, sa, 0, zeroin_addr, 0, cmd, udp_notify, 0, 0);
+		in_pcbnotifyall(&udb, sa, cmd, notify);
 }
 
 static int