Subject: [tcpm] review of 1323bis-15

From: Mark Allman <[email-masked]>
Date: Mon, 04 Nov 2013 12:29:10 -0500

Hi folks!

Sorry to be so late with this review of 1323bis-15.  I think it is in
general in a lot better shape than the last time I read it.  The
following are a set of minor-to-modest comments.  No show stoppers,
IMHO.

  - I'd remove the mention of RTTM in the abstract.  It is downplayed in
    the text.

  - Sec 1: "has resulted" --> "have resulted"

  - Sec 1: In the last paragraph before 1.1:

    - should "recommended" be "RECOMMENDED"?  (I am not sure)

    - perhaps a mention here that the changes from 1323 are partially
      due to our changed understanding of how things work (i.e., this is
      not just a set of bug fixes)

  - Sec 1.1: I found this a bit strange.  The two "fundamental problems"
    seem right to me.  But, this document will go on to develop
    PAWS---which addresses neither problem directly.  You might develop
    the notion that making the windows bigger causes *another*
    fundamental problem and that is too quickly wrapping.  And,
    therefore, that problem must be tackled and this document does so.

  - Sec 1.2, point (1): This is not entirely correct.  You need both a
    high transfer rate *and a whole bunch of data* to have a sequence
    wrapping issue.  This point could be a lot better by noting that it
    takes 4GB of data to wrap.

  - Sec 1.2, the paragraph starting "Duplicates from earlier ....": You
    might also throw in there that randomizing ephemeral ports can also
    help probabilistically reduce the chances of duplicates from
    previous connections muddying the waters.

  - Sec 1.3: "be crashed by" --> "crash on"

  - Sec 1.3 (and other places): I'd like to see the TS option be
    discussed as the 10 byte option that it is.  It is fine to note that
    often it consumes 12 bytes because passing is needed.  However,
    there is a disconnect in the document between the actual
    specification (10 bytes) and the discussion (12 bytes) that could
    lead to confusion.

  - Sec 1.3: I'd remove the word "optimal".  I don't think anyone ever
    claimed that any RTO scheme---TS based or non-TS based---ever
    produced an "optimal" RTO.  The statement should be precise and say
    that additional RTT samples per RTT is shown to have little effect
    on the ultimate RTO when compared to one sample per RTT.

  - Sec 2.2: "to do both" --> "to both"

  - Sec 2.2: "is never scaled." --> "MUST NOT be scaled."

  - Sec 2.3: The list at the beginning of the section should be
    converted to standards language (MUST, SHOULD, etc.).

  - Sec 2.3: I find this notion in the first bullet that the window
    should be stored as 32-bit quantities to be weird.  The maximum size
    for advertised windows is 30 bits.  So, why specify the use of 32
    bit memory locations in the control block?  I mean, that is the
    natural thing to do, but it just adds a little confusion to things
    without really helping the implementer at all.

  - Sec 2.3: "This is safe as a sender can always choose to only
    partially use any signaled receive window."  That is correct, but it
    takes a leap from the previous discussion to figure out what you
    mean.  I'd flesh this out a bit more so the reader doesn't have to
    scratch their head.  I.e., if the receiver is scaling by X>14 and
    the receiver is only scaling by 14 then the window will appear
    smaller than it is in reality.

  - Sec 3.1: The third paragraph doesn't make much sense, or at least
    isn't precise.  You are trying to make a clear distinction with
    imprecise words. :-)

    I'd say you draw a distinction between the timestamp option that
    *conveys* timestamp information and the *use* of that information
    (e.g., by PAWS or by RTTM or by Eifel or whatnot).  In particular,
    not all the mechanisms that use *timestamps* use "the RTT signal as
    input".  E.g., PAWS does not.  So, you are trying to push a
    distinction that doesn't make sense.

  - Sec 3.1: "option is useful" --> "option may be useful" (i.e., it
    depends on the balancing of costs and benefits and so I don't think
    an absolute statement is warranted)

  - Sec 3.1: "Eifel ([cites]) and others" --> I'd add cites for "and
    others".  Or, drop it.  This is a way to try to make an argument
    that there are scads of these things and it isn't worth listing them
    all, when in reality there are not.

  - Sec 3.2: The first paragraph feels out of place.  Before you even
    define timestamps you are telling us how they are used.  I'd move
    this to later in the section.

    Sec 3.2: This whole discussion of valid and invalid TSecr is
    confusing.  You note that when the value is invalid it should be set
    to zero.  But, a TSecr of zero does not mean the TS is invalid.  So,
    how should we tell?

  - Sec 3.2:

      "A TCP MAY send the Timestamps option (TSopt) in an initial <SYN>
      segment (i.e., segment containing a SYN bit and no ACK bit), and
      MAY send a TSopt in other segments only if it received a TSopt in
      the initial <SYN> or <SYN,ACK> segment for the connection."

    That second MAY strikes me as totally wrong.  Shouldn't it be a
    MUST?  Didn't the WG decide that once timestamps are turned on (in
    the 3WHS) that they must always be used?  The next paragraph seems
    to back up this notion of using a MUST here.  In fact, I am not
    entirely sure why there are two paragraphs here.  They seem to at
    least largely overlap.

  - Likewise there is a notion that segments without timestamps "SHOULD"
    be dropped.  But, if we're going to enforce things maybe that should
    also be a "MUST"?

  - Sec 3.3: It'd be nice to see references to this middlebox behavior
    you sketch.  Even if just to a mailing list discussion it'd be more
    concrete than the nebulous language in there now.

  - I would suggest splitting 3.3 & 3.4 off into their own section.  You
    say you are splitting the TS mechanism from its use, but then you
    don't in the document.  I think a new section would make this
    distinction more clear.  I.e., the new section could start with "One
    use for the timestamp option is ...".  And, the PAWS section could
    similarly start with "Another use for the timestamp option is ...".

  - Sec 3.3: Since you have a section on updating the RTO you should
    move all the discussion of updating the RTO there.  I.e., you should
    not have it scattered.

  - Sec 3.4: Swap the first two paragraphs.

  - Sec 3.4: You say one way to deal with the problem of the gains is
    addressed in the appendix.  Fine.  But, it might be useful to stake
    out a principle here.  E.g., something like "in the face of multiple
    RTT samples per RTT an implementation SHOULD try to adhere to the
    spirit of the history specified in [RFC6298]".  Something like
    that.  I.e., throw out at least a high level goal.

  - Sec 4.2: "PAWS uses the same TCP Timestamps option as the RTTM
    mechanism" --> PAWS uses the TCP Timestamps option"

  - Sec 4.2: "In both the PAWS and the RTTM mechanism," --> "In the PAWS
    mechanism,"

  - Sec 4.2: The first few paragraphs are just a mess.  Get rid of the
    references to RTTM.  PAWS does not and never did depend on RTTM.
    Change this to the correct and more accurate "timestamp option" sort
    of language.
 
  - Sec 4.2: What is 'snd.ts.ok'???  This is seemingly important, but
    not defined.  (It is defined in an appendix, but that is not good
    enough for a normative discussion.)

  - Sec 4.2:

      When an <RST> segment is received, it MUST NOT be subjected to
      PAWS checks, and information from the Timestamps option MUST NOT
      be used to update connection state information.  SEG.TSecr MAY be
      used to provide stricter <RST> acceptance checks.

    Don't these two sentences contradict each other?  They are at least
    not clear.

  - Sec 4.4: "to measure RTTs" .... PAWS does not measure RTTs.  Get rid
    of this.

  - Sec 4.4: "internet" --> "Internet"

Also, one thing to think about ... Might it be useful to recommend a
random offset to the TS per connection?  People have shown it is
possible to get signal from these things and perhaps that is not
useful.  I know OpenBSD uses a random offset.  Perhaps just as a MAY or
something.  I.e., there is no harm in it and so let implementers chew on
it.

I hope that is useful!

allman

---

From: Yoshifumi Nishida <[email-masked]>
Date: Mon, 11 Nov 2013 17:56:58 -0800

Hi Mark,

> something.  I.e., there is no harm in it and so let implementers chew on

I am inclined to this as it is more secure approach, but I have one concern
here.
If we start using random offset per connection, I think it will be
difficult to tell which one is older when we see two TCP connections with
the same tuples. This won't be good thing for PAWS as it is expected to
provide protections against duplicates from previous connections.
I guess we might need something more than purely random value for this.

Thanks,
--
Yoshifumi

---

From: Joe Touch <[email-masked]>
Date: Mon, 11 Nov 2013 20:12:30 -0800

On 11/11/2013 5:56 PM, Yoshifumi Nishida wrote:
>
> Hi Mark,
>
> On Mon, Nov 4, 2013 at 9:29 AM, Mark Allman <[email-masked]
> <mailto:[email-masked]>> wrote:
>
>
> I am inclined to this as it is more secure approach,

IMO, it is inappropriate to suggest that such selection is related to 
"security".

 > but I have one
> concern here.
> difficult to tell which one is older when we see two TCP connections
> with the same tuples. This won't be good thing for PAWS as it is
> expected to provide protections against duplicates from previous
> connections.

Indeed.

Joe

---

From: David Borman <[email-masked]>
Date: Tue, 12 Nov 2013 07:37:11 -0600

On Nov 11, 2013, at 7:56 PM, Yoshifumi Nishida <[email-masked]> wrote:

> I am inclined to this as it is more secure approach, but I have one concern here. 
> If we start using random offset per connection, I think it will be difficult to tell which one is older when we see two TCP connections with the same tuples. This won't be good thing for PAWS as it is expected to provide protections against duplicates from previous connections.

The per connection random offset is already there on page 36, Snd.TSoffset, but there is no text in the main body of the document.

Perhaps the end of section 4.4 should have added to it something like:

  o A random offset may be added to the timestamp clock on a
    per connection basis.  See RFC 6528, section 3, on
    randomizing the ISN.  The same function with a different
    secret key can be use to generate the per connection
    timestamp offset.

			-David Borman

> 
> Thanks,
> --
> Yoshifumi
>  
> 
> _______________________________________________
> tcpm mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/tcpm

---

From: Mark Allman <[email-masked]>
Date: Tue, 12 Nov 2013 09:02:43 -0500

> expected to provide protections against duplicates from previous

So, OK, fair enough.

(1) I believe---without going back to the draft right now---that 1323bis
    says PAWS can help protect against old segments from a previous
    incarnation of the connection.  However, this is not part of the
    standard function and requires additional assumptions.

(2) And, further, this is in service of dropping the MSL.  I.e., if we
    don't re-use the tuple within the specified time frame then none of
    this should matter.

(3) Joe, we all understand that this is not some sort of iron clad
    security.  But, just like randomizing ephemeral port choices and
    ISNs and IPIDs and whatnot it seems consistent with our notion of
    making such things unpredictable.  

(4) Some implementations are in fact applying random TS offsets.  If we
    really think this is a bad idea because of PAWS then maybe we should
    say that.

If folks generally feel that random offsets weakens PAWS, I am not going
to fight here.  If Joe wants to loudly shout what we already know about
security, I am not going to fight.  It seems to me like something that
is consistent with SOP and that we could sketch as a MAY and leave it to
implementers, but I am not going to fight about it.

allman

---

From: "Scheffenegger, Richard" <[email-masked]>
Date: Tue, 12 Nov 2013 18:28:13 +0000

Hi Mark,

thanks for your comments.

Richard Scheffenegger

>   - I'd remove the mention of RTTM in the abstract.  It is downplayed in

I replace the "are used" by "can be used", and reversed the order of PAWS and RTTM.

>   - Sec 1: "has resulted" --> "have resulted"

Check

>     - should "recommended" be "RECOMMENDED"?  (I am not sure)

This text is before the definition of RFC2119 style wording; I tend to keep this lower case...

>       not just a set of bug fixes)

Added this to the 2nd before last paragraph of section 1.

>     therefore, that problem must be tackled and this document does so.

Hmm... sec 1.1 is about performance; PAWS is not a performance issue but a integrity (reliability) issue. As 1323bis also defines Window Scale, that appears to address the performance issue described in 1.1 (1); 

But I think you are correctly pointing out that the use WS will make it more likely for wrap around to occur, thus PAWS is necessary. I've added a sentence to point this out.

>     takes 4GB of data to wrap.

Sentence now reads: At a high enough transfer rate of large volume of data (at least 4 GiB in the same session), the 32-bit sequence space may be wrapped within the time that a sement is delayed in queues.

Do you think this is good enough?

>     previous connections muddying the waters.

Check.

>   - Sec 1.3: "be crashed by" --> "crash on"

check

>     specification (10 bytes) and the discussion (12 bytes) that could

Fixed this instance; i couldn't find other text that incorrectly states the option to be 12 bytes in size; only the Appendix A shows the 12-byte layout recommendation (but without much text).

>     on the ultimate RTO when compared to one sample per RTT.

Check.

>   - Sec 2.2: "to do both" --> "to both"

Check

>   - Sec 2.2: "is never scaled." --> "MUST NOT be scaled."

Check

>     converted to standards language (MUST, SHOULD, etc.).

Added mostly MUSTs (only one SHOULD on the receiver, which may choose to limit the window for memory constrains etc).

>     without really helping the implementer at all.

Commented this point out in the xml. (I guess, current OS may want to store this all in 64bit quanta even).

>     smaller than it is in reality.

Check.

>     distinction that doesn't make sense.

Reworded; I'm not sure if the new wording is satisfactory though (more/specific text donations gladly accepted):

        It is necessary to remember that there is a distinction between the 
        Timestamps option conveying timestamp information, and the use of that 
        information. In particular, the Round Trip Time Measurement (RTTM) 
        mechanism must be viewed independently from updating the Retransmission 
        Timeout (RTO) (see <xref target="sec331"/>). In this case, the sample 
        granularity also needs to be taken into account. Other mechanisms, such 
        as PAWS, or Eifel, are not built upon the timestamp information itself, 
        but are based on the intrinsic property of monotonically increasing values.

>     an absolute statement is warranted)

Check

>     all, when in reality there are not.

Added three references that I know offhand, where the timestamps option is used, or at least improves the mechanisms.

>     this to later in the section.

I'm unsure; to me, this paragraph reads like a very short summary of the entire section. It has been carried forward in this form and position from 1323. (also, it provides the rationale why the rfc1072 options were superceded).

>     to zero.  But, a TSecr of zero does not mean the TS is invalid.  So,

This was my attempt at "liberal at what you accept, and conservative in what you send". There are implementations that assume TSecr=0 == invalid, with not strictly defined behavior in that case (e.g. drop that segment and stall until the sender timestamp increases to 1, or end up with strange RTO values...). When TSval is more or less the uptime of the sender, this roll-over hardly matters [which is probably the reason this was not fixed a long time ago in the affected stack], but when TSval is randomized, this corner case is more likely to occur and the implementer should properly treat this case. 

If anyone can think of better wording to express this, please donate text.

>     entirely sure why there are two paragraphs here.  They seem to at

Too much editing in this space since around -06... sorry.

That paragraph now describes just the handshake <SYN>, <SYN,ACK> case, where the receiver is still free to choose not to send TSopt. The next paragraph handles strictly the "established" case.

>     be dropped.  But, if we're going to enforce things maybe that should

We had a discussion around this; the consensus at the time was, to allow existing implementations claim compliance, to have a SHOULD here (most will accept segments, which all of a sudden don't have TSopt any more; in the case of FreeBSD, this was added as a feature - apparently some middleboxes ate TSopt during path changes, leading to consternation...

>     concrete than the nebulous language in there now.

This comment seems to be around paragraph 9, sec 3.2; added a referecence to Andre's comment to that effect, why FreeBSD adheres to a SHOULD (again).

>     similarly start with "Another use for the timestamp option is ...".

Check.

>     move all the discussion of updating the RTO there.  I.e., you should

check

>   - Sec 3.4: Swap the first two paragraphs.

Check. 

>     that.  I.e., throw out at least a high level goal.

Check.

>     mechanism" --> PAWS uses the TCP Timestamps option"

Check

>   - Sec 4.2: "In both the PAWS and the RTTM mechanism," --> "In the PAWS

Check

>     Change this to the correct and more accurate "timestamp option" sort

Tried to fix this.

>     enough for a normative discussion.)

Added this to the Timestamps option section...

>     Don't these two sentences contradict each other?  They are at least

PAWS works on Tsval; added wording to point this out.

>   - Sec 4.4: "to measure RTTs" .... PAWS does not measure RTTs.  Get rid

Updated wording

>   - Sec 4.4: "internet" --> "Internet"

check

> random offset to the TS per connection?  People have shown it is possible
> to get signal from these things and perhaps that is not useful.  I know
> OpenBSD uses a random offset.  Perhaps just as a MAY or something.  I.e.,
> there is no harm in it and so let implementers chew on it.

Used Davids text suggestion at the appropriate section.

> I hope that is useful!

Very!

I'll update the formating (page flow) and post an update shortly.

Best regards,
   Richard Scheffenegger