Re: [tcpm] TCP SEQ validation (Fwd: New Version Notification for draft-gont-tcpm-tcp-seq-validation-02.txt)

["Scharf, Michael (Michael)" <[e-mail]>]

David, Fernando,

Thanks a lot for this useful information. Similar like Yoshifumi, I really think it is relevant what implementations actually do.

Is my understanding correct that this patch has been used in BSD for a while, but it has been replaced by other (more complex) sequence number checks in more recent BSD versions?

In my point of view, the statement "The aforementioned issues have affected a number of popular TCP implementations" is a bit vague and IMHO a more explicit analysis is needed for an update of RFC 793.

For what it is worth, I quickly scanned through the kernel sources of Linux and FreeBSD. It seems to me that natively implementing the sequence number checks as documented in RFC 793 is not really common today, also because of optional enhancements such as RFC 5961 (which does not update RFC 793, BTW).

For instance, according to my short research, in Linux 3.19 the non-SYN check seems to be (e.g., http://lxr.free-electrons.com/source/net/ipv4/tcp_input.c):

---snip---
/* Check segment sequence number for validity.
 *
 * Segment controls are considered valid, if the segment
 * fits to the window after truncation to the window. Acceptability
 * of data (and SYN, FIN, of course) is checked separately.
 * See tcp_data_queue(), for example.
 *
 * Also, controls (RST is main one) are accepted using RCV.WUP instead
 * of RCV.NXT. Peer still did not advance his SND.UNA when we
 * delayed ACK, so that hisSND.UNA<=ourRCV.WUP.
 * (borrowed from freebsd)
 */

static inline bool tcp_sequence(const struct tcp_sock *tp, u32 seq, u32 end_seq)
{
  return !before(end_seq, tp->rcv_wup) &&
         !after(seq, tp->rcv_nxt + tcp_receive_window(tp));
}
---snip---

At first sight, in FreeBSD the SYN chase is checked by this code (https://github.com/freebsd/freebsd/blob/master/sys/netinet/tcp_input.c), which considers the 1-byte offset:

---snip---
todrop = tp->rcv_nxt - th->th_seq;
if (todrop > 0) {
  if (thflags & TH_SYN) {
    thflags &= ~TH_SYN;
    th->th_seq++;
    if (th->th_urp > 1)
      th->th_urp--;
    else
      thflags &= ~TH_URG;
    todrop--;
  }
---snip---

I've not comprehensively analyzed other cases and TCP implementations.

In order to move forward, my thinking is as follows:

* First, the document has to more clearly state whether the proposed fix is only one out of several possibilities, or whether it is the only solution. My current assumption is that it is the most straightforward solution that only relies on the state variables defined in RFC 793, but I might be wrong.
 
* Second, Section 4.2 of draft-gont-tcpm-tcp-seq-validation-02 only mentions Linux, which seems a bit narrow-focused to me (also, the Linux kernel changes, i.e., any statement only applies to a given version). I'd really prefer a section (or an appendix) that more comprehensively discusses how the issues presented in draft-gont-tcpm-tcp-seq-validation-02 are solved by modern TCP stacks. For instance, a section could explicitly acknowledge that any sequence number check would work as long as the scenarios explained in this document are correctly handled. 
 
* Third, I wonder if the security considerations section is really complete. A lot has happened since publication of RFC 793. For instance, would it make sense to refer to RFC 5961 (even if it addresses a slightly different problem)?

As individual contributor to TCPM, I'd prefer to discuss WG adoption for a updated document that better addresses such questions - we are not really in a hurry, as far as I can tell.

Any thoughts?

Michael


> -----Original Message-----
> From: tcpm [mailto:[e-mail]] On Behalf Of David Borman
> Sent: Saturday, March 28, 2015 3:00 AM
> To: Yoshifumi Nishida
> Cc: [e-mail]; Fernando Gont
> Subject: Re: [tcpm] TCP SEQ validation (Fwd: New Version Notification
> for draft-gont-tcpm-tcp-seq-validation-02.txt)
> 
> Accepting the ACK in a packet one to the left of the window is what was
> implemented years (decades?) ago.  It’s what I put into BSD/OS, the
> comment at that time was:
>                         /*
>                          * If segment is just one to the left of the
> window,
>                          * check three special cases:
>                          * 1. Don't toss RST in response to 4.2-style
> keepalive.
>                          * 2. If the only thing to drop is a FIN, we
> can drop
>                          *    it, but check the ACK or we will get into
> FIN
>                          *    wars if our FINs crossed (both CLOSING).
>                          * 3. If we have sent a window probe, it may or
> may not
>                          *    have been accepted.  If window probes
> crossed,
>                          *    we must accept ACK on segments one to the
> left
>                          *    of the window, or we can get ACK wars
> after
>                          *    exchanging probes.  (After sending a
> probe,
>                          *    ACK-only packets are sent with the pre-
> probe
>                          *    sequence number.)
>                          * In any of these cases, send ACK to
> resynchronize,
>                          * but keep on processing for RST or ACK.
>                          */
> So mainly, the “accept the ACK if it is one to the left of the window”
> was the original implementation.
> 
> The only question is if there are any TCPs out there that actually do a
> window probe of more than 1 byte; in that case you’d want to accept an
> ACK if it is within the maximum window probe size to the left of the
> window.  But only the implementations that generate a window probe of
> more than one byte would need to worry about that, in case they had
> crossing probes with another TCP that also generated probes more than
> one byte in length.  If you have a one-byte probe crossing with a
> multi-byte probe, the one-byte probe check is sufficient to break the
> ACK war.
> 
> So maybe a comment needs to be inserted that if an implementation can
> generate a multi-byte probe, then they have to also implement accepting
> ACKs in packets within the maximum window probe size that they might
> generate.  That guarantees that whichever side sends the larger probe,
> it’ll accept the ACK from the smaller probe, and the ACK war will be
> broken.
> 
> 			-David Borman
> 
> > On Mar 27, 2015, at 12:54 PM, Yoshifumi Nishida
> <[e-mail]> wrote:
> >
> > Hi Richard,
> >
> > On Fri, Mar 27, 2015 at 4:41 AM, Scheffenegger, Richard
> <[e-mail]> wrote:
> > I would think PS is the proper way – there is no experimenting with a
> buggy spec, and Fernando pointed out, that most stacks had fixed that
> particular bug in 793 for quite some while, right?
> >
> >
> > Yes.  I believe no one would argue the bug in 793 pointed out in the
> draft. Also, I believe the solution presented in the draft is "make
> sense".
> > What I am wondering is this make sense solution can be the "once for
> all" solution and we can happily update 793.
> > I guess some implementations have been using a bit different approach
> than this, which gives me some concerns to update 793.
> > It will be very useful info if some implementations use this approach
> or have a plan to use this approach.
> >
> > Thanks,
> > --
> > Yoshi
> >
> >
> >
> > From: tcpm [mailto:[e-mail]] On Behalf Of Yoshifumi
> Nishida
> > Sent: Donnerstag, 26. März 2015 18:23
> > To: Fernando Gont
> > Cc: [e-mail]
> > Subject: Re: [tcpm] TCP SEQ validation (Fwd: New Version Notification
> for draft-gont-tcpm-tcp-seq-validation-02.txt)
> >
> >
> >
> > Hi,
> >
> > I would like to check people who are willing to support it also agree
> that this draft is published as a PS and it updates 793, or prefer
> other ways.
> >
> > --
> >
> > Yoshi
> >
> >
> >
> > On Thu, Mar 26, 2015 at 12:03 PM, Fernando Gont
> <[e-mail]> wrote:
> >
> > Folks,
> >
> > Thanks to some folks' push over time, and the fact that both co-
> authors
> > happened to find themselves at IETF 92, we've reposted our latests
> > version of this I-D (since the previous one had expired), in the
> hopes
> > of making progress.
> >
> > Any comments on this rev will be appreciated. Additionally, I'll
> > repost/fwd Karen's latest comments on this I-D, which will be the
> basis
> > for the changes in the next rev.
> >
> > Thanks!
> >
> > Best regards,
> > Fernando
> >
> >
> >
> >
> > -------- Forwarded Message --------
> > Subject: New Version Notification for
> > draft-gont-tcpm-tcp-seq-validation-02.txt
> > Date: Thu, 26 Mar 2015 11:55:46 -0700
> > From: [e-mail]
> > To: Fernando Gont <[e-mail]>, David Borman
> > <[e-mail]>, Fernando Gont <[e-mail]>,
> David
> > Borman <[e-mail]>
> >
> >
> > A new version of I-D, draft-gont-tcpm-tcp-seq-validation-02.txt
> > has been successfully submitted by Fernando Gont and posted to the
> > IETF repository.
> >
> > Name:           draft-gont-tcpm-tcp-seq-validation
> > Revision:       02
> > Title:          On the Validation of TCP Sequence Numbers
> > Document date:  2015-03-26
> > Group:          Individual Submission
> > Pages:          16
> > URL:
> > http://www.ietf.org/internet-drafts/draft-gont-tcpm-tcp-seq-
> validation-02.txt
> > Status:
> > https://datatracker.ietf.org/doc/draft-gont-tcpm-tcp-seq-validation/
> > Htmlized:
> > http://tools.ietf.org/html/draft-gont-tcpm-tcp-seq-validation-02
> > Diff:
> > http://www.ietf.org/rfcdiff?url2=draft-gont-tcpm-tcp-seq-validation-
> 02
> >
> > Abstract:
> >    When TCP receives packets that lie outside of the receive window,
> the
> >    corresponding packets are dropped and either an ACK, RST or no
> >    response is generated due to the out-of-window packet, with no
> >    further processing of the packet.  Most of the time, this works
> just
> >    fine and TCP remains stable, especially when a TCP connection has
> >    unidirectional data flow.  However, there are three scenarios in
> >    which packets that are outside of the receive window should still
> >    have their ACK field processed, or else a packet war will take
> place.
> >    The aforementioned issues have affected a number of popular TCP
> >    implementations, typically leading to connection failures, system
> >    crashes, or other undesirable behaviors.  This document describes
> the
> >    three scenarios in which the aforementioned issues might arise,
> and
> >    formally updates RFC 793 such that these potential problems are
> >    mitigated.
> >
> >
> >
> >
> >
> > Please note that it may take a couple of minutes from the time of
> submission
> > until the htmlized version and diff are available at tools.ietf.org.
> >
> > The IETF Secretariat
> >
> >
> >
> >
> > _______________________________________________
> > tcpm mailing list
> > [e-mail]
> > https://www.ietf.org/mailman/listinfo/tcpm
> >
> >
> >
> >
> > _______________________________________________
> > tcpm mailing list
> > [e-mail]
> > https://www.ietf.org/mailman/listinfo/tcpm
> 
> _______________________________________________
> tcpm mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/tcpm