Last Call Review of draft-trammell-ipfix-tcpcontrolbits-revision-04
review-trammell-ipfix-tcpcontrolbits-revision-04-secdir-lc-josefsson-2013-11-05-00

Hi.

I have reviewed this document as part of the security directorate's 
ongoing effort to review all IETF documents being processed by the 
IESG.  These comments were written primarily for the benefit of the 
security area directors.  Document editors and WG chairs should treat 
these comments just like any other last call comments.

Summary: This is an informational document fixing what appears as a bug
in the IPFIX IANA registry that was introduced in RFC 5102.  The error
was that RFC 5102 used the wrong size for TCP header field, and did not
include three TCP header flags defined after RFC793 (but long before
RFC 5102).  It also changes the semantics to be future proof in case
new TCP header flags are used.

From a security review point of view, I see no significant issues with
this document.

While reading the document, I noticed a couple of things.

MAJOR:

* The document says that processes that can't observe the ECN Nonce
  Sum (NS) bit or Future Use bits "should" export the 8-byte value as
  the old 8-bit value.  I believe RFC 2119 terms is warranted here, as
  it appears to influence bytes-on-the-wire.  Is this SHOULD or MUST?

MINOR:

* If the above SHOULD/MUST is honored, some information will be
  potentially be lost.  Consider a process that can observe the CWR/ECE
  bits but not the NS or Future Use bits.  That process would export
  the tcpControlBits as a 8-bit value.  However, earlier specifications
  said that CWR/ECE bits must be zero even if observed. This means that
  instead of being able to trust the CWR/ECE bit values, those fields
  are unreliable.  If instead a 16-bit value is always sent, the CWR/ECE
  bits will be reliable.

  However, it may be that this aspect is entirely theoretical, and that
  all implementations that support CWR/ECE also support NS or Future Use
  bits.

* The document says "Octets 12 and 13" throughout, however if I'm
  counting the TCP header octets properly, it should be "Octets 13 and
  14".  I call the first octet octet 1 rather than 0, maybe that was
  the origin of the +-1 issue.  Whenever you start counting on 0 rather
  than 1, I think that should be spelled out; it might make things more
  clear to spell that out regardless.

* The acronym IE is not spelled out.

  OLD:
  length), the corresponding bits in this IE must be exported as
  NEW:
  length), the corresponding bits in this Information Element (IE) must
  be exported as

* The Security Considerations section could mention that this document
  changes the size of the tcpControlBits IE from unsigned8 to
  unsigned16, which is probably about the only thing you would want to
  consider when studying the security aspect of this document.

* In section 2 there is one sentence:

      The values of each bit are shown below, per the definition of the
      bits in the TCP header [RFC0793]:

  That isn't really correct, since what is shown below the text above is
  not consistent with RFC793 alone.  I would instead say:

      The values of each bit are shown below, per the definition of the
      bits in the TCP header [RFC0793][RFC3168][RFC3540]:

* The IANA-IPFIX reference looks bad in section 6.2.

/Simon