Re: FreeBSD's RST validation

From: Don Lewis (Don.Lewisat_private)
Date: Mon Aug 31 1998 - 06:06:36 PDT

  • Next message: M.C.Mar: "Re: Buffer overflows in Minicom 1.80.1"

    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 --------------------------
    



    This archive was generated by hypermail 2b30 : Fri Apr 13 2001 - 14:14:20 PDT