Clean up RST ratelimiting. Previously, ratelimiting occured before tests

were performed to determine if the received packet should be reset. This
created erroneous ratelimiting and false alarms in some cases. The code
has now been reorganized so that the checks for validity come before
the call to badport_bandlim. Additionally, a few changes in the symbolic
names of the bandlim types have been made, as well as a clarification of
exactly which type each RST case falls under.

Submitted by: Mike Silbersack <silby@silby.com>

5 files changed, 109 insertions, 78 deletions

diff --git a/sys/netinet/icmp_var.h b/sys/netinet/icmp_var.h
index fd04f27..814e932 100644
--- a/sys/netinet/icmp_var.h
+++ b/sys/netinet/icmp_var.h
@@ -77,12 +77,13 @@ struct	icmpstat {
 #ifdef _KERNEL
 SYSCTL_DECL(_net_inet_icmp);
 extern int badport_bandlim __P((int));
-#define BANDLIM_UNREACH 0
-#define BANDLIM_RST_NOTOPEN 1
-#define BANDLIM_RST_OPEN 2
-#define BANDLIM_ECHO 3
-#define BANDLIM_TSTAMP 4
-#define BANDLIM_MAX 4 
+#define BANDLIM_UNLIMITED -1
+#define BANDLIM_ICMP_UNREACH 0
+#define BANDLIM_ICMP_ECHO 1 
+#define BANDLIM_ICMP_TSTAMP 2
+#define BANDLIM_RST_CLOSEDPORT 3 /* No connection, and no listeners */
+#define BANDLIM_RST_OPENPORT 4   /* No connection, listener */
+#define BANDLIM_MAX 4
 #endif
 
 #endif
diff --git a/sys/netinet/ip_icmp.c b/sys/netinet/ip_icmp.c
index 98758ca..6a277aa 100644
--- a/sys/netinet/ip_icmp.c
+++ b/sys/netinet/ip_icmp.c
@@ -464,7 +464,7 @@ icmp_input(m, off, proto)
 			break;
 		}
 		icp->icmp_type = ICMP_ECHOREPLY;
-		if (badport_bandlim(BANDLIM_ECHO) < 0)
+		if (badport_bandlim(BANDLIM_ICMP_ECHO) < 0)
 			goto freeit;
 		else
 			goto reflect;
@@ -482,7 +482,7 @@ icmp_input(m, off, proto)
 		icp->icmp_type = ICMP_TSTAMPREPLY;
 		icp->icmp_rtime = iptime();
 		icp->icmp_ttime = icp->icmp_rtime;	/* bogus, do later! */
-		if (badport_bandlim(BANDLIM_TSTAMP) < 0)
+		if (badport_bandlim(BANDLIM_ICMP_TSTAMP) < 0)
 			goto freeit;
 		else
 			goto reflect;
@@ -847,10 +847,10 @@ badport_bandlim(int which)
 	int dticks;
 	const char *bandlimittype[] = {
 		"Limiting icmp unreach response",
-		"Limiting closed port RST response",
-		"Limiting open port RST response",
 		"Limiting icmp ping response",
-		"Limiting icmp tstamp response"
+		"Limiting icmp tstamp response",
+		"Limiting closed port RST response",
+		"Limiting open port RST response"
 		};
 
 	/*
diff --git a/sys/netinet/tcp_input.c b/sys/netinet/tcp_input.c
index 07070ba..2c838c6 100644
--- a/sys/netinet/tcp_input.c
+++ b/sys/netinet/tcp_input.c
@@ -392,7 +392,7 @@ tcp_input(m, off0, proto)
 	struct ip6_hdr *ip6 = NULL;
 	int isipv6;
 #endif /* INET6 */
-	int rstreason = 0; /* For badport_bandlim accounting purposes */
+	int rstreason; /* For badport_bandlim accounting purposes */
 
 #ifdef INET6
 	isipv6 = (mtod(m, struct ip *)->ip_v == 6) ? 1 : 0;
@@ -642,13 +642,13 @@ findpcb:
 				goto drop;
 			}
 		}
-		rstreason = BANDLIM_RST_NOTOPEN;
-		goto maybedropwithreset;
+		rstreason = BANDLIM_RST_CLOSEDPORT;
+		goto dropwithreset;
 	}
 	tp = intotcpcb(inp);
 	if (tp == 0) {
-		rstreason = BANDLIM_RST_NOTOPEN;
-		goto maybedropwithreset;
+		rstreason = BANDLIM_RST_CLOSEDPORT;
+		goto dropwithreset;
 	}
 	if (tp->t_state == TCPS_CLOSED)
 		goto drop;
@@ -709,7 +709,8 @@ findpcb:
 				 */
 				if (thflags & TH_ACK) {
 					tcpstat.tcps_badsyn++;
-					goto maybedropwithreset;
+					rstreason = BANDLIM_RST_OPENPORT;
+					goto dropwithreset;
 				}
 				goto drop;
 			}
@@ -789,7 +790,8 @@ findpcb:
 				 */
 				if (thflags & TH_ACK) {
 					tcpstat.tcps_badsyn++;
-					goto maybedropwithreset;
+					rstreason = BANDLIM_RST_OPENPORT;
+					goto dropwithreset;
 				}
 				goto drop;
 			}
@@ -1015,8 +1017,10 @@ findpcb:
 
 		if (thflags & TH_RST)
 			goto drop;
-		if (thflags & TH_ACK)
-			goto maybedropwithreset;
+		if (thflags & TH_ACK) {
+			rstreason = BANDLIM_RST_OPENPORT;
+			goto dropwithreset;
+		}
 		if ((thflags & TH_SYN) == 0)
 			goto drop;
 		if (th->th_dport == th->th_sport) {
@@ -1208,8 +1212,10 @@ findpcb:
 	case TCPS_SYN_RECEIVED:
 		if ((thflags & TH_ACK) &&
 		    (SEQ_LEQ(th->th_ack, tp->snd_una) ||
-		     SEQ_GT(th->th_ack, tp->snd_max)))
-				goto maybedropwithreset;
+		     SEQ_GT(th->th_ack, tp->snd_max))) {
+				rstreason = BANDLIM_RST_OPENPORT;
+				goto dropwithreset;
+		}
 		break;
 
 	/*
@@ -1243,8 +1249,10 @@ findpcb:
 			 */
 			if (taop->tao_ccsent != 0)
 				goto drop;
-			else
+			else {
+				rstreason = BANDLIM_UNLIMITED;
 				goto dropwithreset;
+			}
 		}
 		if (thflags & TH_RST) {
 			if (thflags & TH_ACK)
@@ -1271,8 +1279,10 @@ findpcb:
 				if (tp->cc_send != to.to_ccecho) {
 					if (taop->tao_ccsent != 0)
 						goto drop;
-					else
+					else {
+						rstreason = BANDLIM_UNLIMITED;
 						goto dropwithreset;
+					}
 				}
 			} else
 				tp->t_flags &= ~TF_RCVD_CC;
@@ -1404,8 +1414,10 @@ trimthenstep6:
 		if ((thflags & TH_SYN) &&
 		    (to.to_flag & TOF_CC) && tp->cc_recv != 0) {
 			if (tp->t_state == TCPS_TIME_WAIT &&
-					(ticks - tp->t_starttime) > tcp_msl)
+					(ticks - tp->t_starttime) > tcp_msl) {
+				rstreason = BANDLIM_UNLIMITED;
 				goto dropwithreset;
+			}
 			if (CC_GT(to.to_cc, tp->cc_recv)) {
 				tp = tcp_close(tp);
 				goto findpcb;
@@ -1550,8 +1562,10 @@ trimthenstep6:
 	 * the sequence numbers haven't wrapped.  This is a partial fix
 	 * for the "LAND" DoS attack.
 	 */
-	if (tp->t_state == TCPS_SYN_RECEIVED && SEQ_LT(th->th_seq, tp->irs))
-		goto maybedropwithreset;
+	if (tp->t_state == TCPS_SYN_RECEIVED && SEQ_LT(th->th_seq, tp->irs)) {
+		rstreason = BANDLIM_RST_OPENPORT;
+		goto dropwithreset;
+	}
 
 	todrop = tp->rcv_nxt - th->th_seq;
 	if (todrop > 0) {
@@ -1607,6 +1621,7 @@ trimthenstep6:
 	    tp->t_state > TCPS_CLOSE_WAIT && tlen) {
 		tp = tcp_close(tp);
 		tcpstat.tcps_rcvafterclose++;
+		rstreason = BANDLIM_UNLIMITED;
 		goto dropwithreset;
 	}
 
@@ -1669,6 +1684,7 @@ trimthenstep6:
 	 */
 	if (thflags & TH_SYN) {
 		tp = tcp_drop(tp, ECONNRESET);
+		rstreason = BANDLIM_UNLIMITED;
 		goto dropwithreset;
 	}
 
@@ -2245,8 +2261,10 @@ dropafterack:
 	 */
 	if (tp->t_state == TCPS_SYN_RECEIVED && (thflags & TH_ACK) &&
 	    (SEQ_GT(tp->snd_una, th->th_ack) ||
-	     SEQ_GT(th->th_ack, tp->snd_max)) )
-		goto maybedropwithreset;
+	     SEQ_GT(th->th_ack, tp->snd_max)) ) {
+		rstreason = BANDLIM_RST_OPENPORT;
+		goto dropwithreset;
+	}
 #ifdef TCPDEBUG
 	if (so->so_options & SO_DEBUG)
 		tcp_trace(TA_DROP, ostate, tp, (void *)tcp_saveipgen,
@@ -2257,22 +2275,7 @@ dropafterack:
 	(void) tcp_output(tp);
 	return;
 
-
-	/*
-	 * Conditionally drop with reset or just drop depending on whether
-	 * we think we are under attack or not.
-	 */
-maybedropwithreset:
-	if (rstreason != BANDLIM_RST_NOTOPEN)
-		rstreason = BANDLIM_RST_OPEN;
-	if (badport_bandlim(rstreason) < 0)
-		goto drop;
-	/* fall through */
 dropwithreset:
-#ifdef TCP_RESTRICT_RST
-	if (restrict_rst)
-		goto drop;
-#endif
 	/*
 	 * Generate a RST, dropping incoming segment.
 	 * Make ACK acceptable to originator of segment.
@@ -2292,6 +2295,18 @@ dropwithreset:
 	    ip->ip_src.s_addr == htonl(INADDR_BROADCAST))
 		goto drop;
 	/* IPv6 anycast check is done at tcp6_input() */
+
+	/*
+	 * Perform bandwidth limiting (and RST blocking
+	 * if kernel is so configured.) 
+	 */
+#ifdef TCP_RESTRICT_RST
+	if (restrict_rst)
+		goto drop;
+#endif
+	if (badport_bandlim(rstreason) < 0)
+		goto drop;
+ 
 #ifdef TCPDEBUG
 	if (tp == 0 || (tp->t_inpcb->inp_socket->so_options & SO_DEBUG))
 		tcp_trace(TA_DROP, ostate, tp, (void *)tcp_saveipgen,
diff --git a/sys/netinet/tcp_reass.c b/sys/netinet/tcp_reass.c
index 07070ba..2c838c6 100644
--- a/sys/netinet/tcp_reass.c
+++ b/sys/netinet/tcp_reass.c
@@ -392,7 +392,7 @@ tcp_input(m, off0, proto)
 	struct ip6_hdr *ip6 = NULL;
 	int isipv6;
 #endif /* INET6 */
-	int rstreason = 0; /* For badport_bandlim accounting purposes */
+	int rstreason; /* For badport_bandlim accounting purposes */
 
 #ifdef INET6
 	isipv6 = (mtod(m, struct ip *)->ip_v == 6) ? 1 : 0;
@@ -642,13 +642,13 @@ findpcb:
 				goto drop;
 			}
 		}
-		rstreason = BANDLIM_RST_NOTOPEN;
-		goto maybedropwithreset;
+		rstreason = BANDLIM_RST_CLOSEDPORT;
+		goto dropwithreset;
 	}
 	tp = intotcpcb(inp);
 	if (tp == 0) {
-		rstreason = BANDLIM_RST_NOTOPEN;
-		goto maybedropwithreset;
+		rstreason = BANDLIM_RST_CLOSEDPORT;
+		goto dropwithreset;
 	}
 	if (tp->t_state == TCPS_CLOSED)
 		goto drop;
@@ -709,7 +709,8 @@ findpcb:
 				 */
 				if (thflags & TH_ACK) {
 					tcpstat.tcps_badsyn++;
-					goto maybedropwithreset;
+					rstreason = BANDLIM_RST_OPENPORT;
+					goto dropwithreset;
 				}
 				goto drop;
 			}
@@ -789,7 +790,8 @@ findpcb:
 				 */
 				if (thflags & TH_ACK) {
 					tcpstat.tcps_badsyn++;
-					goto maybedropwithreset;
+					rstreason = BANDLIM_RST_OPENPORT;
+					goto dropwithreset;
 				}
 				goto drop;
 			}
@@ -1015,8 +1017,10 @@ findpcb:
 
 		if (thflags & TH_RST)
 			goto drop;
-		if (thflags & TH_ACK)
-			goto maybedropwithreset;
+		if (thflags & TH_ACK) {
+			rstreason = BANDLIM_RST_OPENPORT;
+			goto dropwithreset;
+		}
 		if ((thflags & TH_SYN) == 0)
 			goto drop;
 		if (th->th_dport == th->th_sport) {
@@ -1208,8 +1212,10 @@ findpcb:
 	case TCPS_SYN_RECEIVED:
 		if ((thflags & TH_ACK) &&
 		    (SEQ_LEQ(th->th_ack, tp->snd_una) ||
-		     SEQ_GT(th->th_ack, tp->snd_max)))
-				goto maybedropwithreset;
+		     SEQ_GT(th->th_ack, tp->snd_max))) {
+				rstreason = BANDLIM_RST_OPENPORT;
+				goto dropwithreset;
+		}
 		break;
 
 	/*
@@ -1243,8 +1249,10 @@ findpcb:
 			 */
 			if (taop->tao_ccsent != 0)
 				goto drop;
-			else
+			else {
+				rstreason = BANDLIM_UNLIMITED;
 				goto dropwithreset;
+			}
 		}
 		if (thflags & TH_RST) {
 			if (thflags & TH_ACK)
@@ -1271,8 +1279,10 @@ findpcb:
 				if (tp->cc_send != to.to_ccecho) {
 					if (taop->tao_ccsent != 0)
 						goto drop;
-					else
+					else {
+						rstreason = BANDLIM_UNLIMITED;
 						goto dropwithreset;
+					}
 				}
 			} else
 				tp->t_flags &= ~TF_RCVD_CC;
@@ -1404,8 +1414,10 @@ trimthenstep6:
 		if ((thflags & TH_SYN) &&
 		    (to.to_flag & TOF_CC) && tp->cc_recv != 0) {
 			if (tp->t_state == TCPS_TIME_WAIT &&
-					(ticks - tp->t_starttime) > tcp_msl)
+					(ticks - tp->t_starttime) > tcp_msl) {
+				rstreason = BANDLIM_UNLIMITED;
 				goto dropwithreset;
+			}
 			if (CC_GT(to.to_cc, tp->cc_recv)) {
 				tp = tcp_close(tp);
 				goto findpcb;
@@ -1550,8 +1562,10 @@ trimthenstep6:
 	 * the sequence numbers haven't wrapped.  This is a partial fix
 	 * for the "LAND" DoS attack.
 	 */
-	if (tp->t_state == TCPS_SYN_RECEIVED && SEQ_LT(th->th_seq, tp->irs))
-		goto maybedropwithreset;
+	if (tp->t_state == TCPS_SYN_RECEIVED && SEQ_LT(th->th_seq, tp->irs)) {
+		rstreason = BANDLIM_RST_OPENPORT;
+		goto dropwithreset;
+	}
 
 	todrop = tp->rcv_nxt - th->th_seq;
 	if (todrop > 0) {
@@ -1607,6 +1621,7 @@ trimthenstep6:
 	    tp->t_state > TCPS_CLOSE_WAIT && tlen) {
 		tp = tcp_close(tp);
 		tcpstat.tcps_rcvafterclose++;
+		rstreason = BANDLIM_UNLIMITED;
 		goto dropwithreset;
 	}
 
@@ -1669,6 +1684,7 @@ trimthenstep6:
 	 */
 	if (thflags & TH_SYN) {
 		tp = tcp_drop(tp, ECONNRESET);
+		rstreason = BANDLIM_UNLIMITED;
 		goto dropwithreset;
 	}
 
@@ -2245,8 +2261,10 @@ dropafterack:
 	 */
 	if (tp->t_state == TCPS_SYN_RECEIVED && (thflags & TH_ACK) &&
 	    (SEQ_GT(tp->snd_una, th->th_ack) ||
-	     SEQ_GT(th->th_ack, tp->snd_max)) )
-		goto maybedropwithreset;
+	     SEQ_GT(th->th_ack, tp->snd_max)) ) {
+		rstreason = BANDLIM_RST_OPENPORT;
+		goto dropwithreset;
+	}
 #ifdef TCPDEBUG
 	if (so->so_options & SO_DEBUG)
 		tcp_trace(TA_DROP, ostate, tp, (void *)tcp_saveipgen,
@@ -2257,22 +2275,7 @@ dropafterack:
 	(void) tcp_output(tp);
 	return;
 
-
-	/*
-	 * Conditionally drop with reset or just drop depending on whether
-	 * we think we are under attack or not.
-	 */
-maybedropwithreset:
-	if (rstreason != BANDLIM_RST_NOTOPEN)
-		rstreason = BANDLIM_RST_OPEN;
-	if (badport_bandlim(rstreason) < 0)
-		goto drop;
-	/* fall through */
 dropwithreset:
-#ifdef TCP_RESTRICT_RST
-	if (restrict_rst)
-		goto drop;
-#endif
 	/*
 	 * Generate a RST, dropping incoming segment.
 	 * Make ACK acceptable to originator of segment.
@@ -2292,6 +2295,18 @@ dropwithreset:
 	    ip->ip_src.s_addr == htonl(INADDR_BROADCAST))
 		goto drop;
 	/* IPv6 anycast check is done at tcp6_input() */
+
+	/*
+	 * Perform bandwidth limiting (and RST blocking
+	 * if kernel is so configured.) 
+	 */
+#ifdef TCP_RESTRICT_RST
+	if (restrict_rst)
+		goto drop;
+#endif
+	if (badport_bandlim(rstreason) < 0)
+		goto drop;
+ 
 #ifdef TCPDEBUG
 	if (tp == 0 || (tp->t_inpcb->inp_socket->so_options & SO_DEBUG))
 		tcp_trace(TA_DROP, ostate, tp, (void *)tcp_saveipgen,
diff --git a/sys/netinet/udp_usrreq.c b/sys/netinet/udp_usrreq.c
index 5c92f62..468064f 100644
--- a/sys/netinet/udp_usrreq.c
+++ b/sys/netinet/udp_usrreq.c
@@ -353,7 +353,7 @@ udp_input(m, off, proto)
 			udpstat.udps_noportbcast++;
 			goto bad;
 		}
-		if (badport_bandlim(BANDLIM_UNREACH) < 0)
+		if (badport_bandlim(BANDLIM_ICMP_UNREACH) < 0)
 			goto bad;
 		if (blackhole)
 			goto bad;