Skip to main content

Last Call Review of draft-ietf-opsawg-ipfix-tcpo-v6eh-11
review-ietf-opsawg-ipfix-tcpo-v6eh-11-secdir-lc-kivinen-2024-05-09-00

Request Review of draft-ietf-opsawg-ipfix-tcpo-v6eh
Requested revision No specific revision (document currently at 13)
Type Last Call Review
Team Security Area Directorate (secdir)
Deadline 2024-05-10
Requested 2024-04-26
Authors Mohamed Boucadair , Benoît Claise
I-D last updated 2024-05-09
Completed reviews Genart Last Call review of -11 by Joel M. Halpern (diff)
Secdir Last Call review of -11 by Tero Kivinen (diff)
Tsvart Last Call review of -11 by Wesley Eddy (diff)
Intdir Last Call review of -11 by Dirk Von Hugo (diff)
Tsvart Early review of -05 by Wesley Eddy (diff)
Opsdir Early review of -05 by Yingzhen Qu (diff)
Intdir Early review of -05 by Dirk Von Hugo (diff)
Assignment Reviewer Tero Kivinen
State Completed
Request Last Call review on draft-ietf-opsawg-ipfix-tcpo-v6eh by Security Area Directorate Assigned
Posted at https://mailarchive.ietf.org/arch/msg/secdir/zc61zACJc6yXtRFvKJh6q6Equy4
Reviewed revision 11 (document currently at 13)
Result Has issues
Completed 2024-05-09
review-ietf-opsawg-ipfix-tcpo-v6eh-11-secdir-lc-kivinen-2024-05-09-00
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.

This document redefines the IPFIX IEs for IPv6 extension headers and
TCP options to allow more data to be exported.

Issues:

In section 7 the text claims that "This document does not add new
security considerations for exporting IES.", but as this document
allows more information to be exported, there is also possibility that
more data that was not available, thus there might be new security
considerations.

For example this allows seeing multiple same extension headers, etc,
thus there should be additional considerations.

Perhaps adding text saying that as this document allows more data to
be available and some of that data might be sensitive, the
implementations needs to take this into account when exporting data".

--

In section 8.4 if the IANA is automatically allocating next bit for
each new IPv6 Extension Header, is there still separate expert review
done for this automatic allocation or not? What happens if the experts
in new registry do not allow registration for the extension header
that was added to the IPv6 Extension Headers registry?

If implementation then sees that new IPv6 extension header that was
allocated by IANA, but which is not in the IPFIX subregistry, how does
it fill the bitfield?

Also when new IPv6 extension headers are added, all implementations of
IPFIX needs to be updated to map the protocol number to the bit, thus
they can't add new extension headers until they are updated with the
mapping.

----------------------------------------------------------------------

Nits:

--
In section 1.1

Write out the [RFC8200] in first bullet, i.e., change to:

   *  Cover the full extension headers' range (Section 4 of IPv6
      Specification [RFC8200]).

--

In section 1.1

   *  Specify how to automatically update the IANA IPFIX registry
      ([IANA-IPFIX]) when a new value is assigned in [IANA-EH].  Only a

I think the [IANA-EH] here should be properly spelled out, i.e.,
change the text to "when a new value is assigned in the IANA IPv6
extension header types registry [IANA-EH]".

Also the text:

				For example, the ipv6ExtensionHeaders IE
      can't report some IPv6 EHs, specifically 139, 140, 253, and 254.


should not use numbers, but instead of names of those extension
headers, i.e., it should say "specifically extension headers for Host
Identity and Shim6 Protocol, or extension headers for experimentation
and testing.".

If numbers are needed they can be added in parenthesis after the
protocol name (i.e., "Shim6 Protocol (140)").

--

In section 1.1

Write out 8883:

      (e.g., Section 1.1 of ICMPv6 Errors for Discarding Packets Due
      to Processing Limits [RFC8883]).

--

In section 1.1

The text "Also, ipv6ExtensionHeaders IPFIX IE is deprecated in favor
of the new IEs defined in this document." makes it feel like the
deprecation is an aftertought. I think the whole idea of this document
is to deprecate the old IE and create new. Perhaps say "This
specification will deprecate ipv6ExtensionHeaders IPFIX IE in favor of
the new IEs defined in this document".

Similar text is also in end of section 1.2.

--

In section 1.2 write out 6994 properly, i.e., change:

   *  Allow reporting the observed Experimental Identifiers (ExIDs) that
      are carried in shared TCP options (Kind=253 or 254) [RFC6994].

to

   *  Allow reporting the observed Experimental Identifiers (ExIDs)
      (Kind=253 or 254) that are carried in shared experimental TCP
      options [RFC6994].

--

In section 2 do use proper name instead of just reference. When
someone decideds that all references are changed to [1], [2], [3] etc,
the text should still be readable. Also requiring readers to keep
track of mapping from RFC numbers to actual specification name is bad
idea, and makes it harder for reader to understand what the document
is trying to change.

Change


   This document uses the IPFIX-specific terminology (Information
   Element, Template Record, Flow, etc.) defined in Section 2 of
   [RFC7011].  As in [RFC7011], these IPFIX-specific terms have the
   first letter of a word capitalized.


to

   This document uses the IPFIX-specific terminology (Information
   Element, Template Record, Flow, etc.) defined in Section 2 of IPFIX
   specification [RFC7011]. As in that document, these IPFIX-specific
   terms have the first letter of a word capitalized.

--

In section 2

If would be nice to know what those documents 8200 and 9239 are so
changing the text "Also, the document uses the terms defined in
[RFC8200] and [RFC9293]." to

    This document uses the terms defined in IPv6 [RFC8200], and TCP
    [RFC9293] specifications.

--

In section 3 there is several cases where it says "Section xxx of
[RFC8200]", change that to "Section 4 of IPv6 specication [RFC8200]".

--

In section 3.6 change "As discussed in Section 1.2 of [RFC8883]," to
"As discussed in ICMPv6 Errors for Discarding Packets Due to
Processing Limits [RFC8883],"

--

In section 5.1 change "Section 6.2 of [RFC7011]." to "Section 6.2 of
IPFIX specification [RFC7011].".

--

In section 5.1 change "Section 2.2 of [RFC8883]" to "Section 2.2 of
ICMPv6 Errors for Discarding Packets Due to Processing
Limits [RFC8883]".

--

In section 5.2 change "Section 6.2 of [RFC7011]." to "Section 6.2 of
IPFIX specification [RFC7011]."

--

In section 6.1 it would be good to have example where more than one
octet is needed, so it would clarify the byteorder of the data.

--

In section 7 change "Section 11 of [RFC7011]." to "Section 11 of IPFIX
specification[RFC7011]."

--

In section 7 change "Section 8 of [RFC7012]" to "Section 8 of
Information Model for IPFIX [RFC7012]".

--

In section 8.3 change

					This type MUST be encoded per
   Section 6.1.1 of [RFC7011].  Reduced-Size encoding (Section 6.2 of
   [RFC7011]) applies to this data type.

to

					This type MUST be encoded per
   Section 6.1.1 of IPFIX specification [RFC7011]. Reduced-Size
   encoding (Section 6.2 of IPFIX specification [RFC7011]) applies to
   this data type.

--

Section 9.1 [IANA-EH] url is wrong, it points to the "Next Header
Types" registry, not to the "IPv6 Extension Header Types" registry.
Correct url is
https://www.iana.org/assignments/ipv6-parameters/ipv6-parameters.xhtml#extension-header