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

[Benjamin Kaduk via Datatracker <[e-mail]>]

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.

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.

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.

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.

      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.

Section 2.1.3

Do the algorithm values need to appear in any specific order?
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.

Section 2.1.4

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

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.

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

[Same comment about SHOULD/MUST for Reserved]

Section 2.2.2

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

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?

Section 2.3.1

[same comment about SHOULD/MUST for Reserved]

Section 2.3.3

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

Section 2.3.4

[same comment about SHOULD/MUST for Reserved]

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

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.

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.

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.