Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 19 Dec 2000 22:14:44 +0100 (CET)
From:      jesper@skriver.dk
To:        FreeBSD-gnats-submit@freebsd.org
Cc:        jesper@skriver.dk
Subject:   kern/23655: react to ICMP administratively prohibited, part II
Message-ID:  <200012192114.eBJLEie00623@tam.skriver.dk>
Resent-Message-ID: <200012192120.eBJLK4H70927@freefall.freebsd.org>

next in thread | raw e-mail | index | archive | help

>Number:         23655
>Category:       kern
>Synopsis:       Fix/improvement of previously committed change.
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          update
>Submitter-Id:   current-users
>Arrival-Date:   Tue Dec 19 13:20:01 PST 2000
>Closed-Date:
>Last-Modified:
>Originator:     Jesper Skriver
>Release:        FreeBSD 5.0-CURRENT i386
>Organization:
Tele Danmark
>Environment:
System: FreeBSD tam 5.0-CURRENT FreeBSD 5.0-CURRENT #0: Tue Dec 19 21:25:48 CET 2000 root@tam:/usr/obj/usr/src/sys/TAM2 i386


>Description:
	Fix/improvement of my previous PR kern/23086 which PHK committed on 
saturday the 16th of december 2000.

That involved:

src/sys/netinet/tcp_subr.c ver 1.85
src/sys/netinet/tcp_var.h ver 1.62

To summarize:

PHK committed my original patch, that patch had the following
functionality:

- When a ICMP administrative prohibited is recieved, it zap's
  all TCP sessions in SYN-SENT state matching the source and destination
  IP addresses and TCP port numbers in the IP header + 8 bytes from the
  ICMP packet.
- It does not match against TCP sequence number
- disabled by default


The new code has this improved functionality:

- If the sysctl net.inet.tcp.icmp_admin_prohib_like_rst == 1 (default)
  it enables the below.
- When a ICMP administrative prohibited is recieved, it check is the
  IP header attached to the ICMP packet has any options set, if it has
  it ignores it. The reason for this is, if any options is set the extra
  8 bytes is no longer the first 8 bytes from the TCP header, source/
  destination ports and sequence number, which we need to find the
  right TCP session.
- Then it goes through the list of active TCP sessions, if it finds one
  with the same source/destination IP addresses and TCP port numbers, it
  checks if the sequence number it got from the ICMP packet is one of
  a unacknowledged packet from this sessions, if it's not, it ignores it.
- If the sysctl net.inet.tcp.icmp_like_rst_syn_sent_only == 1 (default)
  it will only zap connections in SYN-SENT state, if it's == 0 it will
  zap the connection regardless of current state.

>How-To-Repeat:
	The code in the tree currently doesn't check for TCP sequence numbers,
thus it's open for a potential DoS, the new code fixes this, and make the sysctl 
'net.inet.tcp.icmp_like_rst_syn_sent_only' control if this code reacts to TCP 
sessions only in SYN-SENT state, or to all TCP sessions regardless of state.

>Fix:

	Commit the below diff

diff -ru sys/netinet.old/in_pcb.c sys/netinet/in_pcb.c
--- sys/netinet.old/in_pcb.c	Sun Dec 17 18:57:24 2000
+++ sys/netinet/in_pcb.c	Tue Dec 19 21:18:58 2000
@@ -667,13 +667,14 @@
  * any errors for each matching socket.
  */
 void
-in_pcbnotify(head, dst, fport_arg, laddr, lport_arg, cmd, notify)
+in_pcbnotify(head, dst, fport_arg, laddr, lport_arg, cmd, notify, tcp_sequence)
 	struct inpcbhead *head;
 	struct sockaddr *dst;
 	u_int fport_arg, lport_arg;
 	struct in_addr laddr;
 	int cmd;
 	void (*notify) __P((struct inpcb *, int));
+	u_int32_t tcp_sequence;
 {
 	register struct inpcb *inp, *oinp;
 	struct in_addr faddr;
@@ -714,6 +715,15 @@
 		    (lport && inp->inp_lport != lport) ||
 		    (laddr.s_addr && inp->inp_laddr.s_addr != laddr.s_addr) ||
 		    (fport && inp->inp_fport != fport)) {
+			inp = inp->inp_list.le_next;
+			continue;
+		}
+		/*
+		 * If tcp_sequence is set, then skip sessions where
+		 * the sequence number is not one of a unacknowledged
+		 * packet.
+		 */
+		if ((tcp_sequence) && (tcp_seq_vs_sess(inp, tcp_sequence) == 0)) {
 			inp = inp->inp_list.le_next;
 			continue;
 		}
diff -ru sys/netinet.old/in_pcb.h sys/netinet/in_pcb.h
--- sys/netinet.old/in_pcb.h	Sun Dec 17 18:57:24 2000
+++ sys/netinet/in_pcb.h	Sun Dec 17 22:47:39 2000
@@ -290,7 +290,7 @@
 			       struct in_addr, u_int, struct in_addr, u_int,
 			       int, struct ifnet *));
 void	in_pcbnotify __P((struct inpcbhead *, struct sockaddr *,
-	    u_int, struct in_addr, u_int, int, void (*)(struct inpcb *, int)));
+	    u_int, struct in_addr, u_int, int, void (*)(struct inpcb *, int), u_int32_t));
 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 -ru sys/netinet.old/tcp_subr.c sys/netinet/tcp_subr.c
--- sys/netinet.old/tcp_subr.c	Sun Dec 17 18:57:24 2000
+++ sys/netinet/tcp_subr.c	Tue Dec 19 21:18:00 2000
@@ -139,9 +139,20 @@
  * as required by rfc1122 section 3.2.2.1
  */
  
-static int	icmp_admin_prohib_like_rst = 0;
+static int	icmp_admin_prohib_like_rst = 1;
 SYSCTL_INT(_net_inet_tcp, OID_AUTO, icmp_admin_prohib_like_rst, CTLFLAG_RW,
-	&icmp_admin_prohib_like_rst, 0, "Treat ICMP administratively prohibited messages like TCP RST, rfc1122 section 3.2.2.1");
+	&icmp_admin_prohib_like_rst, 0, 
+	"Treat ICMP administratively prohibited messages like TCP RST, rfc1122 section 3.2.2.1");
+
+/*
+ * When icmp_admin_prohib_like_rst is enabled, only act on
+ * sessions in SYN-SENT state
+ */
+
+static int	icmp_like_rst_syn_sent_only = 1;
+SYSCTL_INT(_net_inet_tcp, OID_AUTO, icmp_like_rst_syn_sent_only, CTLFLAG_RW,
+	&icmp_like_rst_syn_sent_only, 0, 
+	"When icmp_admin_prohib_like_rst is enabled, only act on sessions in SYN-SENT state");
 
 static void	tcp_cleartaocache __P((void));
 static void	tcp_notify __P((struct inpcb *, int));
@@ -967,10 +978,19 @@
 	register struct ip *ip = vip;
 	register struct tcphdr *th;
 	void (*notify) __P((struct inpcb *, int)) = tcp_notify;
+	tcp_seq tcp_sequence = 0;
 
 	if (cmd == PRC_QUENCH)
 		notify = tcp_quench;
-	else if ((icmp_admin_prohib_like_rst == 1) && (cmd == PRC_UNREACH_PORT) && (ip))
+	else if ((icmp_admin_prohib_like_rst == 1) && (cmd == PRC_UNREACH_PORT) && 
+			(ip) && ((IP_VHL_HL(ip->ip_vhl) << 2) == sizeof(struct ip)))
+		/*
+		 * Only go here if the length of the IP header in the ICMP packet
+		 * is 20 bytes, that is it doesn't have options, if it does have
+		 * options, we will not have the first 8 bytes of the TCP header,
+		 * and thus we cannot match against TCP source/destination port
+		 * numbers and TCP sequence number.
+		 */
 		notify = tcp_drop_syn_sent;
 	else if (cmd == PRC_MSGSIZE)
 		notify = tcp_mtudisc;
@@ -980,10 +1000,12 @@
 	if (ip) {
 		th = (struct tcphdr *)((caddr_t)ip 
 				       + (IP_VHL_HL(ip->ip_vhl) << 2));
+		if (notify == tcp_drop_syn_sent)
+			tcp_sequence = ntohl(th->th_seq);
 		in_pcbnotify(&tcb, sa, th->th_dport, ip->ip_src, th->th_sport,
-			cmd, notify);
+			cmd, notify, tcp_sequence);
 	} else
-		in_pcbnotify(&tcb, sa, 0, zeroin_addr, 0, cmd, notify);
+		in_pcbnotify(&tcb, sa, 0, zeroin_addr, 0, cmd, notify, 0);
 }
 
 #ifdef INET6
@@ -1070,6 +1092,30 @@
 #endif /* INET6 */
 
 /*
+ * Check if the supplied TCP sequence number is a sequence number
+ * for a sent but unacknowledged packet on the given TCP session.
+ */
+int
+tcp_seq_vs_sess(inp, tcp_sequence)
+	struct inpcb *inp;
+	tcp_seq tcp_sequence;
+{
+	struct tcpcb *tp = intotcpcb(inp);
+	/*
+	 * If the sequence number is less than that of the last 
+	 * unacknowledged packet, or greater than that of the 
+	 * last sent, the given sequence number is not that
+	 * of a sent but unacknowledged packet for this session.
+	 */
+	if (SEQ_LT(tcp_sequence, tp->snd_una) ||
+			SEQ_GT(tcp_sequence, tp->snd_max)) {
+		return(0);
+	} else {
+		return(1);
+	}
+}
+
+/*
  * When a source quench is received, close congestion window
  * to one segment.  We will gradually open it again as we proceed.
  */
@@ -1086,7 +1132,9 @@
 
 /*
  * When a ICMP unreachable is recieved, drop the
- * TCP connection, but only if in SYN_SENT
+ * TCP connection, depending on the sysctl
+ * icmp_like_rst_syn_sent_only, it only drops
+ * the session if it's in SYN-SENT state
  */
 void
 tcp_drop_syn_sent(inp, errno)
@@ -1094,8 +1142,9 @@
 	int errno;
 {
 	struct tcpcb *tp = intotcpcb(inp);
-	if((tp) && (tp->t_state == TCPS_SYN_SENT))
-			tcp_drop(tp, errno);
+	if((tp) && ((icmp_like_rst_syn_sent_only == 0) || 
+			(tp->t_state == TCPS_SYN_SENT)))
+		tcp_drop(tp, errno);
 }
 
 /*
diff -ru sys/netinet.old/tcp_var.h sys/netinet/tcp_var.h
--- sys/netinet.old/tcp_var.h	Sun Dec 17 18:57:24 2000
+++ sys/netinet/tcp_var.h	Tue Dec 19 20:16:54 2000
@@ -392,6 +392,7 @@
 struct tcpcb *
 	 tcp_newtcpcb __P((struct inpcb *));
 int	 tcp_output __P((struct tcpcb *));
+int	 tcp_seq_vs_sess __P((struct inpcb *, tcp_seq));
 void	 tcp_quench __P((struct inpcb *, int));
 void	 tcp_respond __P((struct tcpcb *, void *,
 	    struct tcphdr *, struct mbuf *, tcp_seq, tcp_seq, int));
diff -ru sys/netinet.old/udp_usrreq.c sys/netinet/udp_usrreq.c
--- sys/netinet.old/udp_usrreq.c	Sun Dec 17 18:57:24 2000
+++ sys/netinet/udp_usrreq.c	Sun Dec 17 19:59:53 2000
@@ -512,9 +512,9 @@
 	if (ip) {
 		uh = (struct udphdr *)((caddr_t)ip + (ip->ip_hl << 2));
 		in_pcbnotify(&udb, sa, uh->uh_dport, ip->ip_src, uh->uh_sport,
-			cmd, udp_notify);
+			cmd, udp_notify, 0);
 	} else
-		in_pcbnotify(&udb, sa, 0, zeroin_addr, 0, cmd, udp_notify);
+		in_pcbnotify(&udb, sa, 0, zeroin_addr, 0, cmd, udp_notify, 0);
 }
 
 static int

>Release-Note:
>Audit-Trail:
>Unformatted:


To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-bugs" in the body of the message




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200012192114.eBJLEie00623>