Re: [Lsvr] AD Review of draft-ietf-lsvr-bgp-spf-11.txt

[Alvaro Retana <[e-mail]>]

On February 15, 2021 at 10:55:53 AM, Acee Lindem wrote:


Hi!

This message covers the initial set of up-front comments/questions.

Thanks!

Alvaro.



> [**]
>
> > It would be ideal for the document to include a "Summary of Operation"
> > section, similar to ??3/rfc4271. This would also serve as a place to
> > point back at what applies and what doesn't.
>
> We've added a "Document Overview" in section 1.3. This should suffice in
> giving the reader a preview of what is coming.

241	1.3.  Document Overview

243	   The document begins with sections defining the precise relationship
244	   that BGP SPF has with both the base BGP protocol [RFC4271]
245	   (Section 2) and the BGP Link-State (BGP-LS) extensions [RFC7752]
246	   (Section 3).  This is required to dispel the notion that BGP SPF is
247	   an independent protocol.  The BGP peering models, as well as the
248	   their respective trade-offs are then discussed in Section 4.  The
249	   remaining sections, which make up the bulk of the document, define
250	   the protocol enhancements necessary to support BGP SPF.  The BGP-LS
251	   extensions to support BGP SPF are defined in Section 5.  The
252	   replacement of the base BGP decision process with the SPF computation
253	   is specified in Section 6.  Finally, BGP SPF error handling is
254	   defined in Section 7


[major] Before I forget, please update the rfc7752 reference to
draft-ietf-idr-rfc7752bis.  That document is already in front of the
IESG; I expect to approve it by March.


[minor] "This is required to dispel the notion that BGP SPF is an
independent protocol."

Not needed.


[nit] The last couple of sentences are a little repetitive, and
there's a period missing at the end.

Suggestion>
  The document begins with sections defining the precise relationship
  that BGP SPF has with both the base BGP protocol [RFC4271]
  (Section 2) and the BGP Link-State (BGP-LS) extensions [RFC7752bis]
  (Section 3). The BGP peering models, as well as the their respective
  trade-offs are then discussed in Section 4. The remaining sections
  define the protocol enhancements necessary to support BGP SPF
  including BGP-LS extensions (Section 5), the replacement decision
  process with the SPF computation (Section 6), and error handling
  (Section 7).



> > (1) Use of the BGP Base (rfc4271)
> >
> > - The well-known RIB structure from BGP (Adj-RIB-In, Loc-RIB, Adj-RIB-Out)
> > didn't quite make it into BGP SPF. It looks like the LSNDB may correspond
> > to the Adj-RIB-In, and there is a Local RIB. The non-existence of an
> > Adj-RIB-Out implies changes. [Note that it is obviously ok to not follow
> > the same structure -- what needs to be explained are the differences.]
>
> Since we have completely replaced the base BGP decision process, we don't use
> these terms.

[major] You did change the "Local RIB" for "LOC-RIB", but the
definition is not the same as Loc-RIB in rfc4271.  Even if the case is
different, it would be better to change the terminology (maybe back to
"Local RIB") to eliminate any confusion.



...
> > - About the attributes... In ??5.1 you mention that attributes that "would
> > influence the Decision process...are ignored".
> >
> > What about other attributes, the ones that don't influence the Decision
> > Process? If all the information is encoded in the NLRI or carried in the
> > BGP-LS Attribute, does BGP SPF need any other attribute? Maybe requiring
> > that other attributes not to be included (especially the mandatory ones) is
> > too much, but can it be recommended? Given that BGP (and rfc7606) require
> > specific actions if the attributes are malformed, it would be bad if we
> > apply treat-as-withdraw because of an error in an attribute that is not
> > relevant in BGP SPF. [Note: we need to add this risk to the Security
> > Considerations.]
...
> We clarified the usage of the attributes in section 2. Error handling is
> covered in section 7.

§2:

273	   Due to the changes to the decision process, there are mechanisms and
274	   encodings that are no longer applicable.  While not necessarily
275	   required for computation, the ORIGIN, AS_PATH, MULTI_EXIT_DISC,
276	   LOCAL_PREF, and NEXT_HOP path attributes are mandatory and will be
277	   validated.  The ATOMIC_AGGEGATE, and AGGREGATOR are not applicable
278	   within the context of BGP SPF and SHOULD NOT be advertised.  However,
279	   if they are advertised, they will be accepted, validated, and
280	   propagated consistent with the BGP protocol.

[minor] s/are mandatory/are mandatory [RFC4271]

To clarify that they are not mandatory in this specification.


[major] What about other attributes [1]?  I know that most are not
mandatory but they could still be included in an UPDATE.

Is it possible to generalize the statement above?  Here's a suggestion:

s/
   The ATOMIC_AGGEGATE, and AGGREGATOR are not applicable within
   the context of BGP SPF and SHOULD NOT be advertised.  However,
   if they are advertised, they will be accepted, validated, and
   propagated consistent with the BGP protocol.
/
   Unless explicitly specified in the context of BGP SPF, all other
   attributes SHOULD NOT be advertised.  However, if they are
   advertised, they will be accepted, validated, and propagated
   consistent with the BGP protocol.

[1] https://www.iana.org/assignments/bgp-parameters/bgp-parameters.xhtml#bgp-parameters-2


[major] Processing non-applicable attributes represents an increase in
the attack surface.  A rogue node can advertise a non-applicable
attribute and affect the routing decisions; for example a malformed
Community results in "treat-as-withdraw".

This is not a new threat.  The difference is that for other BGP
AFI/SAFIs alll the attributes are "applicable".  In the case of BGP
SPF, this document explicitly says that the nodes will accept,
validate, and propagate information that is not relevant.

One option to reduce the attack surface is to ignore/drop
non-applicable attributes, or even mandate that they must not be
included.  I understand that implementation-wise that is not the best
option.

Please include a couple of sentences in the Security Considerations to
show that the threat was considered -- that it is an existing threat
-- ...


...
> > - Using a Confederation ID is mentioned, which seems ok. I don't see an
> > advantage/reason for having Confederations in a BGP SPF network. Is there
> > one?
>
> Confederations are no longer applicable.

>From §5.2:

   The BGP Confederation Member (TLV 517) [RFC7752] is not appliable
   and SHOULD NOT be included.  If TLV 517 is included, it will be
   ignored.

[nit] s/appliable/applicable

[?] s/will be ignored/MUST be ignored