[secdir] Re: Secdir last call review of draft-ietf-opsawg-ipfix-tcpo-v6eh-11

[[e-mail]]

Hi Tero, 

Thank you for the review. 

Please see inline. 

Cheers,
Med

> -----Message d'origine-----
> De : Tero Kivinen via Datatracker <[e-mail]>
> Envoyé : jeudi 9 mai 2024 17:35
> À : [e-mail]
> Cc : [e-mail]; last-
> [e-mail]; [e-mail]
> Objet : Secdir last call review of draft-ietf-opsawg-ipfix-tcpo-
> v6eh-11
> 
> Reviewer: Tero Kivinen
> Review result: Has Issues
> 
> 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.

[Med] Please note that we have this sentence right before the one you quoted to call out new threats: 

ipv6ExtensionHeadersChainLength and ipv6ExtensionHeadersLimit IEs can be exploited by an unauthorized observer as a means to deduce the processing capabilities of nodes. Section 8 of [RFC7012] discusses the required measures to guarantee the integrity and confidentiality of the exported information.

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

[Med] I think the text I quoted above is an example of what is sensitive. Unless there is a specific item to specifically list, the guards in the base specs are sufficient.

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

[Med] No. The nominal mode won't involve a DE because mirroring will be sufficient to reflect (new, modification, deprecate, etc.). "otherwise" is a catchup to cover cases where modifications are local to the registry, not the parent one. For example, we do have two entries for fragments, while only one protocol number is used for fragments in the parent EH registry. Thanks.

 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?

[Med] made this change to make the behavior explicit: 

OLD:
If an implementation determines that an observed packet of a Flow includes an extension header that it does not support, then the exact observed code of that extension header MUST be echoed in the ipv6ExtensionHeaderTypeCountList IE (Section 3.4).

NEW:
If an implementation determines that an observed packet of a Flow includes an extension header (including an extension header that it does not support), then the exact observed code of that extension header MUST be echoed in the ipv6ExtensionHeaderTypeCountList IE (Section 3.4).


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

[Med] This is not required for ipv6ExtensionHeaderTypeCountList IE. 

> 
> -----------------------------------------------------------------
> -----
> 
> 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]).
> 

[Med] I will keep the OLD.

> --
> 
> 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]".

[Med] Done.

> 
> 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)").

[Med] OK. 

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

[Med] Deal.

> Similar text is also in end of section 1.2.

[Med] ACK.

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

[Med] OK

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

[Med] OK.

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

[Med] This is a matter of editing taste. I'm not fun of expanding the ref title.

> 
> Section 9.1 [IANA-EH] url is wrong, it points to the "Next Header
> Types" registry, not to the "IPv6 Extension Header Types"
> registry.

[Med] This is on purpose because the RFC Editor does not cite the specific URLs, only the URL of the registry group.

> Correct url is
> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2
> Fwww.iana.org%2Fassignments%2Fipv6-parameters%2Fipv6-
> parameters.xhtml%23extension-
> header&data=05%7C02%7Cmohamed.boucadair%40orange.com%7Ce04d445565
> be47dc729308dc703d9ca0%7C90c7a20af34b40bfbc48b9253b6f5d20%7C0%7C0
> %7C638508657119400747%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDA
> iLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdat
> a=sAElkvoUqJFN0K5eFgX%2F73LEbbqAWqerHq0IDzQsy%2Bo%3D&reserved=0
> 
> 
> 

____________________________________________________________________________________________________________
Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.

This message and its attachments may contain confidential or privileged information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
Thank you.