Re: [Idr] Benjamin Kaduk's No Objection on draft-ietf-idr-bgp-ls-segment-routing-ext-15: (with COMMENT)

["Ketan Talaulikar (ketant)" <[e-mail]>]

Hi Benjamin,

Thanks for your review. I've updated the draft to address your comments and please check inline below for responses.

The version 16 of the draft has been posted with changes to address your and other comments during the IESG review:
https://tools.ietf.org/html/draft-ietf-idr-bgp-ls-segment-routing-ext-16 

Thanks,
Ketan

-----Original Message-----
From: Benjamin Kaduk via Datatracker <[e-mail]> 
Sent: 13 June 2019 00:25
To: The IESG <[e-mail]>
Cc: [e-mail]; Susan Hares <[e-mail]>; [e-mail]; [e-mail]; [e-mail]; [e-mail]
Subject: Benjamin Kaduk's No Objection on draft-ietf-idr-bgp-ls-segment-routing-ext-15: (with COMMENT)

Benjamin Kaduk has entered the following ballot position for
draft-ietf-idr-bgp-ls-segment-routing-ext-15: No Objection

When responding, please keep the subject line intact and reply to all email addresses included in the To and CC lines. (Feel free to cut this introductory paragraph, however.)


Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.


The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-idr-bgp-ls-segment-routing-ext/



----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

-15

I agree with Mirja that mentioning the scope earlier in the document would be helpful (but will follow the discussion in her ballot thread).
Basically, even though the SR Architecture is clear about the scope being limited to a trusted administrative domain, it's still helpful to remind the reader briefly at the start of the document that there is some expectation of trust to all participants receiving this information, and that it is not expected to leave into the broader Internet or generic BGP peers.

[KT] Ack. Please check the updated text and let know your views.

Section 1

   When Segment Routing is enabled in an IGP domain, segments are
   advertised in the form of Segment Identifiers (SIDs).  The IGP link-
   state routing protocols have been extended to advertise SIDs and
   other SR-related information.  IGP extensions are described in: IS-IS
   [I-D.ietf-isis-segment-routing-extensions], OSPFv2
   [I-D.ietf-ospf-segment-routing-extensions] and OSPFv3
   [I-D.ietf-ospf-ospfv3-segment-routing-extensions].  [...]

nit: I think "described for" is more appropriate than "described in", which might suggest to the reader that these are part of the core protocols.
[KT] Ack - fixed.

Section 2.1.1

      Length: Variable.  Either 3 or 4 depending whether the value is
      encoded as a label or as an index/SID.

nit: I think the secdir reviewer's concern might be alleviated by spelling these constants as 0x0003 and 0x0004.
[KT] I am not sure how specifying the value in decimal or hexadecimal changes security concerns for something that is not a bit/flag field.

Section 2.1.2

Are the "SID/Label sub-TLV N" fields actually variable length?  The description seems to be saying that we can only use the "label" variant encoding, which makes the overall length of the sub-TLV 7 bytes, always.
[KT] Agree and I've removed the "variable" from the figure for the TLVs in section 2.1.2 and 2.1.4 where this holds true.

      Flags: 1 octet of flags as defined in
      [I-D.ietf-isis-segment-routing-extensions] for IS-IS.  The flags
      are not currently defined for OSPFv2 and OSPFv3 and SHOULD be set
      to 0 and MUST be ignored on receipt.

I agree with the other reviewers that the SHOULD here is surprising.
Perhaps "Until the specification of flag value usage for OSPFv2 and/or
OSPFv3 is defined, the flags MUST be set to zero and ignored on receipt"?

The case for Reserved seems even more clear to mandate setting to zero.

[KT] Fixed for both the above.

Section 2.1.3

Do the algorithm values need to appear in any specific order?
[KT] No. 

The note about the maximum length being 256 seems to imply that duplicates are not expected, but I do not see any specific text forbidding them.
[KT] The values for these fields are picked from their corresponding IGP specifications. E.g. refer to https://tools.ietf.org/html/draft-ietf-isis-segment-routing-extensions-25#section-3.2. There is no processing that is needed when picking the value from underlying IGP and propagating them in BGP-LS.

Section 2.1.4

[Same comments as Section 2.1.2 about variable length sub-TLVs, and SHOULD/MUST]
[KT] Fixed.

Section 2.2.1

   The Adjacency SID TLV is used in order to advertise information
   related to an Adjacency SID.  This information is derived from Adj-
   SID sub-TLV of IS-IS [I-D.ietf-isis-segment-routing-extensions],
   OSPFv2 [I-D.ietf-ospf-segment-routing-extensions] and OSPFv3
   [I-D.ietf-ospf-ospfv3-segment-routing-extensions].

nit: I think this should be "Adj-SID sub-TLVs" plural.
[KT] We are talking about a specific TLV name here.

      Weight: 1 octet carrying the weight used for load-balancing
      purposes.

Maybe reference RFC 8402 for the weight semantics?  (Otherwise we have to guess whether a larger value means to send more traffic vs. less
traffic.)
[KT] Agreed. Fixed for similar comment also from Roman.

[Same comment about SHOULD/MUST for Reserved]
[KT] Fixed.

Section 2.2.2

[same comment about Weight and SHOULD/MUST for Reserved]
[KT] Fixed.

Do we want some IANA considerations and/or regisitry modifications to provide for any future BGP-LS Attribute TLVs that might be defined and be appropriate to include as sub-TLVs of the L2 Bundle Member Attribute TLV?
[KT] Not required since we don't maintain sub-TLV space registries for BGP-LS TLVs and they are all from a flat space.

Section 2.3.1

[same comment about SHOULD/MUST for Reserved]
[KT] Fixed.

Section 2.3.3

The figure says "4 or 6 octet Router-ID"; should that bt "4 or 16" to match the body text?
[KT] Fixed.

Section 2.3.4

[same comment about SHOULD/MUST for Reserved]
[KT] Fixed.

It's a little weird to blandly cite the OSPF document for the Range Size definition and expect that to plainly transfer over to the IS-IS case as well.  (Also, the linked document has three different usages for "Range Size"; I assume we mean the one in Section 4, "OSPF Extended  Prefix Range TLV", since it's the right width, but it might be worth stating
explicitly.)
[KT] Fixed with updated reference for each TLV section to be specific. Removed the specific reference to Range Size field since we've already identified the mapping TLVs for all 3 IGPs.

I didn't take the time to try to convince myself that the sub-TLV usage for prefix-to-SID mappings matches up with the statement that the length is "11 or 12 depending on Label or Index encoding of the SID", but I think the TLV overhead would push the length larger than 12.
[KT] I am not sure of what you mean by TLV overhead since there is no padding requirement in BGP for the TLVs/sub-TLVs.

Section 5

The links for "required security and authentication mechanisms" are a little surprising, as those documents are the segment-routing-extensions documents for IS-IS and OSPF, respectively, and do not define the mechamisms themselves, rather referring to other document for  the details of the security mechanisms.  Furthermore, there seem to only be SHOULD-level requirements for authentication mechanisms, and only in certain cases (and only in the OSPFv2 document).  So to say "the required mechanisms" here seems misleading, in that the set of *required* mechanisms seems to be the empty set.
[KT] This BGP-LS extension (on the same lines as base RFC7752) picks up information from underlying IGPs and propagating it via BGP-LS. The point of reference to the IGP specs for security considerations was to discuss the need to "secure" the source of the information via authentication and other security mechanisms of those underlying IGPs.

I always have a hard time accepting statements about "present no additional risk"; we are going from presenting "generic"
link-state/prefix/node information across the BGP-LS domain to additionally presenting information about the SR topology and segment location/identifiers.  So, one might concoct a scenario in which an attacker can learn about the internal SR configuration/topology based on the information conveyed by the mechanisms in  this document, which is in some sense an "additional risk".  That said, it is probably a minor one, given the expected scope of distribution.
[KT] In the next paragraph, we do discuss the "additional" aspects and so I've reworded this text to clarify this aspect.