Date: Mon, 31 Aug 1998 11:00:03 -0700 (PDT) From: Archie Cobbs <archie@whistle.com> To: freebsd-hackers@FreeBSD.ORG Subject: Re: FreeBSD's RST validation Message-ID: <199808311800.LAA06751@bubba.whistle.com> In-Reply-To: <199808311306.GAA27281@salsa.gv.tsc.tdk.com> from Don Lewis at "Aug 31, 98 06:06:36 am"
next in thread | previous in thread | raw e-mail | index | archive | help
Would some -hackers comment on this bug (posted to freebsd-security)?
Thanks,
-Archie
Don Lewis writes:
> On Aug 30, 5:21pm, Tristan Horn wrote:
> } Subject: FreeBSD's RST validation
> } --ZRyEpB+iJ+qUx0kp
> } Content-Type: multipart/mixed; boundary=qGV0fN9tzfkG3CxV
> }
> }
> } --qGV0fN9tzfkG3CxV
> } Content-Type: text/plain; charset=us-ascii
> }
> } RFC 793, pages 36-39 (chapter 3.5) describes closing connections with
> } TCP. Page 37 is of particular interest:
> }
> } Reset Processing
> }
> } In all states except SYN-SENT, all reset (RST) segments are validated
> } by checking their SEQ-fields. A reset is valid if its sequence number
> } is in the window. In the SYN-SENT state (a RST received in response
> } to an initial SYN), the RST is acceptable if the ACK field
> } acknowledges the SYN.
> }
> } Unfortunately, FreeBSD (2.2.5, 2.2.6, 2.2.7, 3.0) does not appear to
> } validate RST segments to this extent. In other words, only the packets'
> } IP/port pairs are checked.
>
> Back in December 1997, I posted the following patch for the LAND attack
> and also implemented stricter RST validation. The variation of the
> LAND fix in the first two chunks of this patch was implemented (you'll
> have to look carefully at the code to find the second chunk), but I don't
> believe the rest of the fixes in this patch were applied.
>
> I've been running a version of this patch altered for 2.1.x since December
> without problems. If you remove the first two chunks of this patch, it
> will apply cleanly to the 2.2-stable version of tcp_input.c, though I have
> no idea if it will work ...
>
> ----------------- Cut Here --------------------------
> --- tcp_input.c.2_2 Mon Dec 1 16:49:21 1997
> +++ tcp_input.c Wed Dec 3 02:21:45 1997
> @@ -318,19 +318,6 @@
> #endif /* TUBA_INCLUDE */
>
> /*
> - * Reject attempted self-connects. XXX This actually masks
> - * a bug elsewhere, since self-connect should work.
> - * However, a urrently-active DoS attack in the Internet
> - * sends a phony self-connect request which causes an infinite
> - * loop.
> - */
> - if (ti->ti_src.s_addr == ti->ti_dst.s_addr
> - && ti->ti_sport == ti->ti_dport) {
> - tcpstat.tcps_badsyn++;
> - goto drop;
> - }
> -
> - /*
> * Check that TCP offset makes sense,
> * pull out TCP options and adjust length. XXX
> */
> @@ -654,6 +641,24 @@
> if (m->m_flags & (M_BCAST|M_MCAST) ||
> IN_MULTICAST(ntohl(ti->ti_dst.s_addr)))
> goto drop;
> +
> + /*
> + * Reject attempted self-connects.
> + *
> + * Doing the test here should prevent the "LAND" DoS
> + * attack without affecting legitimate self-connects
> + * which will occur in the SYN-SENT state.
> + *
> + * In the dropafterack code below we'll also fix the real
> + * bug in the SYN-RECEIVED state that causes the infinite
> + * loop since it can also be used to generate ACK storms.
> + */
> + if (ti->ti_src.s_addr == ti->ti_dst.s_addr
> + && ti->ti_sport == ti->ti_dport) {
> + tcpstat.tcps_badsyn++;
> + goto drop;
> + }
> +
> am = m_get(M_DONTWAIT, MT_SONAME); /* XXX */
> if (am == NULL)
> goto drop;
> @@ -962,17 +967,99 @@
>
> /*
> * States other than LISTEN or SYN_SENT.
> - * First check timestamp, if present.
> + * First check the RST flag and sequence number since reset segments
> + * are exempt from the timestamp and connection count tests. This
> + * fixes a bug introduced by the Stevens, vol. 2, p. 960 bugfix
> + * below which allowed reset segments in half the sequence space
> + * to fall though and be processed (which gives forged reset
> + * segments with a random sequence number a 50 percent chance of
> + * killing a connection).
> + * Then check timestamp, if present.
> * Then check the connection count, if present.
> * Then check that at least some bytes of segment are within
> * receive window. If segment begins before rcv_nxt,
> * drop leading data (and SYN); if nothing left, just ack.
> *
> + *
> + * If the RST bit is set, check the sequence number to see
> + * if this is a valid reset segment.
> + * RFC 793 page 37:
> + * In all states except SYN-SENT, all reset (RST) segments
> + * are validated by checking their SEQ-fields. A reset is
> + * valid if its sequence number is in the window.
> + * Note: this does not take into account delayed ACKs, so
> + * we should test against last_ack_sent instead of rcv_nxt.
> + * Also, it does not make sense to allow reset segments with
> + * sequence numbers greater than last_ack_sent to be processed
> + * since these sequence numbers are just the acknowledgement
> + * numbers in our outgoing packets being echoed back at us,
> + * and these acknowledgement numbers are monotonically
> + * increasing.
> + * If we have multiple segments in flight, the intial reset
> + * segment sequence numbers will be to the left of last_ack_sent,
> + * but they will eventually catch up.
> + * In any case, it never made sense to trim reset segments to
> + * fit the receive window since RFC 1122 says:
> + * 4.2.2.12 RST Segment: RFC-793 Section 3.4
> + *
> + * A TCP SHOULD allow a received RST segment to include data.
> + *
> + * DISCUSSION
> + * It has been suggested that a RST segment could contain
> + * ASCII text that encoded and explained the cause of the
> + * RST. No standard has yet been established for such
> + * data.
> + *
> + * If the reset segment passes the sequence number test examine
> + * the state:
> + * SYN_RECEIVED STATE:
> + * If passive open, return to LISTEN state.
> + * If active open, inform user that connection was refused.
> + * ESTABLISHED, FIN_WAIT_1, FIN_WAIT2, CLOSE_WAIT STATES:
> + * Inform user that connection was reset, and close tcb.
> + * CLOSING, LAST_ACK, TIME_WAIT STATES
> + * Close the tcb.
> + * TIME_WAIT state:
> + * Drop the segment - see Stevens, vol. 2, p. 964 and
> + * RFC 1337.
> + */
> + if (tiflags&TH_RST) {
> + if (tp->last_ack_sent == ti->ti_seq) {
> + switch (tp->t_state) {
> +
> + case TCPS_SYN_RECEIVED:
> + so->so_error = ECONNREFUSED;
> + goto close;
> +
> + case TCPS_ESTABLISHED:
> + case TCPS_FIN_WAIT_1:
> + case TCPS_FIN_WAIT_2:
> + case TCPS_CLOSE_WAIT:
> + so->so_error = ECONNRESET;
> + close:
> + tp->t_state = TCPS_CLOSED;
> + tcpstat.tcps_drops++;
> + tp = tcp_close(tp);
> + break;
> +
> + case TCPS_CLOSING:
> + case TCPS_LAST_ACK:
> + tp = tcp_close(tp);
> + break;
> +
> + case TCPS_TIME_WAIT:
> + break;
> + }
> + }
> + goto drop;
> + }
> +
> + /*
> * RFC 1323 PAWS: If we have a timestamp reply on this segment
> * and it's less than ts_recent, drop it.
> */
> - if ((to.to_flag & TOF_TS) != 0 && (tiflags & TH_RST) == 0 &&
> - tp->ts_recent && TSTMP_LT(to.to_tsval, tp->ts_recent)) {
> + if ((to.to_flag & TOF_TS) != 0 && tp->ts_recent &&
> + TSTMP_LT(to.to_tsval, tp->ts_recent)) {
>
> /* Check to see if ts_recent is over 24 days old. */
> if ((int)(tcp_now - tp->ts_recent_age) > TCP_PAWS_IDLE) {
> @@ -1003,10 +1090,19 @@
> * RST segments do not have to comply with this.
> */
> if ((tp->t_flags & (TF_REQ_CC|TF_RCVD_CC)) == (TF_REQ_CC|TF_RCVD_CC) &&
> - ((to.to_flag & TOF_CC) == 0 || tp->cc_recv != to.to_cc) &&
> - (tiflags & TH_RST) == 0)
> + ((to.to_flag & TOF_CC) == 0 || tp->cc_recv != to.to_cc))
> goto dropafterack;
>
> + /*
> + * In the SYN-RECEIVED state, validate that the packet belongs to
> + * this connection before trimming the data to fit the receive
> + * window. Check the sequence number versus IRS since we know
> + * 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(ti->ti_seq, tp->irs))
> + goto dropwithreset;
> +
> todrop = tp->rcv_nxt - ti->ti_seq;
> if (todrop > 0) {
> if (tiflags & TH_SYN) {
> @@ -1118,40 +1214,6 @@
> }
>
> /*
> - * If the RST bit is set examine the state:
> - * SYN_RECEIVED STATE:
> - * If passive open, return to LISTEN state.
> - * If active open, inform user that connection was refused.
> - * ESTABLISHED, FIN_WAIT_1, FIN_WAIT2, CLOSE_WAIT STATES:
> - * Inform user that connection was reset, and close tcb.
> - * CLOSING, LAST_ACK, TIME_WAIT STATES
> - * Close the tcb.
> - */
> - if (tiflags&TH_RST) switch (tp->t_state) {
> -
> - case TCPS_SYN_RECEIVED:
> - so->so_error = ECONNREFUSED;
> - goto close;
> -
> - case TCPS_ESTABLISHED:
> - case TCPS_FIN_WAIT_1:
> - case TCPS_FIN_WAIT_2:
> - case TCPS_CLOSE_WAIT:
> - so->so_error = ECONNRESET;
> - close:
> - tp->t_state = TCPS_CLOSED;
> - tcpstat.tcps_drops++;
> - tp = tcp_close(tp);
> - goto drop;
> -
> - case TCPS_CLOSING:
> - case TCPS_LAST_ACK:
> - case TCPS_TIME_WAIT:
> - tp = tcp_close(tp);
> - goto drop;
> - }
> -
> - /*
> * If a SYN is in the window, then this is an
> * error and we send an RST and drop the connection.
> */
> @@ -1660,9 +1722,22 @@
> /*
> * Generate an ACK dropping incoming segment if it occupies
> * sequence space, where the ACK reflects our state.
> - */
> - if (tiflags & TH_RST)
> - goto drop;
> + *
> + * We can now skip the test for the RST flag since all
> + * paths to this code happen after packets containing
> + * RST have been dropped.
> + *
> + * In the SYN-RECEIVED state, don't send an ACK unless the
> + * segment we received passes the SYN-RECEIVED ACK test.
> + * If it fails send a RST. This breaks the loop in the
> + * "LAND" DoS attack, and also prevents an ACK storm
> + * between two listening ports that have been sent forged
> + * SYN segments, each with the source address of the other.
> + */
> + if (tp->t_state == TCPS_SYN_RECEIVED && (tiflags & TH_ACK) &&
> + (SEQ_GT(tp->snd_una, ti->ti_ack) ||
> + SEQ_GT(ti->ti_ack, tp->snd_max)) )
> + goto dropwithreset;
> #ifdef TCPDEBUG
> if (so->so_options & SO_DEBUG)
> tcp_trace(TA_DROP, ostate, tp, &tcp_saveti, 0);
> ----------------- Cut Here --------------------------
>
> To Unsubscribe: send mail to majordomo@FreeBSD.org
> with "unsubscribe freebsd-security" in the body of the message
>
___________________________________________________________________________
Archie Cobbs * Whistle Communications, Inc. * http://www.whistle.com
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-hackers" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199808311800.LAA06751>