Subject: Re: [Lsr] AD Review of draft-ietf-isis-te-app-09

From: Alvaro Retana <[email-masked]>
Date: Thu, 5 Mar 2020 16:53:50 +0000

On February 27, 2020 at 11:00:19 PM, Les Ginsberg wrote:

Les:

Hi!  How are you?

Some of the responses, yours and mine, feel like we're not
communicating well, like we are talking past each other.  I am
probably not clearly asking the right questions...  I think it would
be a good idea to talk live.  Let me know a time that works for you
and we can set up a short call.  Maybe the Shepherd can help us sort
through things.

Nonetheless, I responded to some of your comments.

Thanks!

Alvaro.

...
> > > > 177 3. Legacy Advertisements
> > ...
> > > > [minor] Please add references to where all the values in this section
> > > > are defined...if the same as the references above.
> > > >
> > > [Les:] Done.
> >
> > Sorry, I meant the RFCs where the sub-TLVs are defined, not the registries.
>
> [Les:] Well, the IANA registry identifies the Reference RFCs - and has the
> advantage that it gets updated should that ever change. So I would suggest
> providing a link to the registry is actually better than identifying the RFC
> for each code point. :-)
> ??

[nit] I disagree because the RFC gives you the detail not just the
value, which is what you originally get from the registry.
Sure...it's just an indirection.

...
> > > > [major] In a mixed network, where some routers support this
> > > > specification, but other don't, it seems that setting the L-flag would
> > > > be useful as all applications would "fall back" to the legacy
> > > > advertisements. However, not setting the L-flag seems to have the
> > > > potential for the routers to use different information resulting in
> > > > (at best) inconsistent decisions if the values are not the same.
> > > >
> > > [Les:] The revised Deployment considerations section now addresses this.
> >
> > [nit] In the new §6.3.3 (Interoperability with Legacy Routers) it
> > might be good to mention the setting of the L-flag. For example,
> > extend Step 1, "Receiving routers continue to use legacy
> > advertisements" to include a mention of the L-flag being set.
> >
> [Les:] This isn't a nit - it is a misunderstanding.
> We aren't using L-bit here at all.
> We are accommodating the coexistence of legacy/non-legacy routers.
>
> Legacy routers only send/receive legacy advertisements. They don't understand
> the new advertisements - let alone the L-bit.
>
> To migrate to all new routers - at which point legacy advertisements can be
> deprecated - you must follow the three steps described. The transition
> between steps is controlled by some implementation specific knob.
>
> Please reread and let me know if it is clear.

What I think I missed (even in the initial review) is this text (now
in §6.1 - Use of Legacy Advertisements) which talks about the knob you
mention:

   Under the conditions defined above, implementations which support the
   extensions defined in this document have the choice of using legacy
   advertisements or application specific advertisements in support of
   SRTE and/or LFA.  This will require implementations to provide
   controls specifying which type of advertisements are to be sent/
   processed on receive for these applications.  Further discussion of
   the associated issues can be found in Section 6.3.

The steps in §6.3.3 are:

   1)Send application specific advertisements while continuing to
   advertise using legacy (all advertisements are then duplicated).
   Receiving routers continue to use legacy advertisements.

   2)Enable the use of the application specific advertisements on all
   routers

   3)Remove legacy advertisements

I agree that this transition can be achieved with a knob.  But it can
also be achieved if the L-flag is set (during steps 1 and 2), right?
Sure, the legacy routers won't know what the L-flag is, but the new
ones will -- and will then use the legacy advertisements.

The difference that I see is that the L-flag affects *all* the
applications in the bit mask while the knob may be more specific.  It
would be very nice if some text was added about this to avoid more
people misunderstanding.

...
> > > > [major] "Undefined bits MUST be transmitted as 0 and MUST be ignored
> > > > on receipt." What if the sender and the receiver support a different
> > > > set of applications? For example, suppose that the sender supports a
> > > > new application identified by the N-bit, and sets only that bit; the
> > > > receiver treats the N-bit as undefined and ignores it. The result is
> > > > that, for the receiver, there seems to be no indication of an
> > > > application. Is "no application" a valid indication, or should at
> > > > least one bit always be set?
> > > >
> > > [Les:] Just like unknown TLVs, unknown bits MUST be ignored.
> > > I have added text.
> >
> > Yes, that is ok.
> >
> > [major] But you didn't answer my question: Is "no application" a valid
> > indication, or should at least one bit always be set? I guess that
> > the receiver wouldn't see anything it understands... Would this be
> > equivalent to zero-length masks?
> >
> [Les:] There are three cases:
>
> 1)You have application specific advertisements - which you send with the
> appropriate bit(s) set in the mask.
>
> 2)You have application specific advertisements but want all applications to
> use them. You send with zero-length bit mask.
>
> 3)You have no advertisements.

Maybe I'm not asking the question correctly...

Let's talk about case 1, rtrA sets the appropriate bits...but rtrB
(the receiver) doesn't understand any of them.  Why not?  It may be
that rtrA is signaling a set of new applications and rtrB hasn't been
upgraded.

IOW, rtrA is doing the right thing: sending application specific
advertisements with the appropriate bits set in the mask.  But because
rtrB doesn't understand the bits (and ignores them) then it looks like
no bits are set.

I'm guessing that rtrB would just ignore the complete set of
application specific advertisements: it wouldn't use them at all
because there are no bits set (it ignores the ones that are).  Is that
the correct/expected behavior?

Besides wanting to take guessing out of a specification, I'm asking
all this because the other case where no bits are set is when the
zero-length bit mask is used.  In that case any application can use
the parameters...

> > > > [minor] Following on the previous comment... It seems that a
> > > > requirement is for the routers/nodes in the domain to agree on the
> > > > meaning of the bits: support the same Standard Applications and
> > > > interpret the User Defined bit mask the same way. Please add this
> > > > type of information to the Deployment Considerations Section.
> > > >
> > > [Les:] Standard Applications by definition have bits which are assigned
> > > by IANA. There is therefore no need to say routers have to agree
> > > on their meaning.
> > > User Defined Applications are - by definition - outside the scope of this
> > > specification. They can do whatever they want.
> >
> > Let me try again...
> >
> > What issues can arise from the routers supporting a different set of
> > applications/bits? I can imagine several ways for this to occur, for
> > example: routers that have not been upgraded to support the latest
> > assignments...bad configuration practices there the UDAs are not
> > consistently instantiated in the whole network...
> >
> > IOW, if a new application is implemented in the network, but not all
> > the routers are aware of the corresponding bit (because they're
> > running old software or were not configured correctly, for example),
> > then it is possible that the network will not operate as expected.
> >
> [Les:] How a router indicates that it supports a given application is outside
> the scope of this specification.
> For example,
> https://tools.ietf.org/html/draft-ietf-lsr-flex-algo-06#section-10 describes
> how this is done for flex-algo.

I'm not asking you to make it in scope.  Even if there is a way to
advertise what is supported a router can still send advertisements for
applications that not all the routers support.  Yes, it may be a bug,
it ma be a rogue router, etc..

If that happens, what are the issues?

...
> > > > 337 4.2. Application Specific Link Attributes sub-TLV
> > ...
> > > > 361 Multiple Application Specific Link Attribute sub-TLVs for the same
> > > > 362 link MAY be advertised. When multiple sub-TLVs for the same link are
> > > > 363 advertised, they SHOULD advertise non-conflicting application/
> > > > 364 attribute pairs. A conflict exists when the same application is
> > > > 365 associated with two different values of the same link attribute for a
> > > > 366 given link. In cases where conflicting values for the same
> > > > 367 application/attribute/link are advertised all the conflicting values
> > > > 368 MUST be ignored.
> > > >
> > > > 370 For a given application, the setting of the L-flag MUST be the same
> > > > 371 in all sub-TLVs for a given link. In cases where this constraint is
> > > > 372 violated, the L-flag MUST be considered set for this application.
> > > >
> > > > [major] Tying these two paragraphs together... If one of the sub-TLVs
> > > > has the L-flag set, then all the sub-TLVs that include at least one
> > > > common application MUST be considered as having the L-flag set as
> > > > well. This seems to become an attack vector: a rogue IS can set the
> > > > L-flag (and/or set application bits) in all/some of the
> > > > sub-TLVs...which may result in no attribute information (if the
> > > > sub-sub-TLVs were the only source of information and are ignored) for
> > > > a link...
> > > >
> > > [Les:] The text says:
> > >
> > > "Link attribute sub-sub-TLVs for the corresponding link attributes MUST
> > > NOT be advertised for the set of applications specified in the
> > > Standard/User Application Identifier Bit Masks and all such
> > > advertisements MUST be ignored on receipt."
> > >
> > > There is no reason to send multiple sub-TLVs w L-bit set on a given link
> > > because by definition such a sub-TLV MUST NOT have any sub-sub-TLVs.
> > >
> > > Authentication (as always) is our protection against attacks.
> >
> > [major] No, not against an IS that has been taken over: an attacker
> > has access to it, its configuration, etc., it is rogue.
> > Authentication only proves that the IS is the IS it says it is, not
> > that it is not sending malicious information.
> >
> > To be clear, I'm not asking for the rogue node problem to be solved in
> > this document. Just that the risks are recognized.
> >
> > I'm mentioning this point about rogue nodes because that is the new
> > focus of the SEC area. I really don't want to go forward with
> > Security Considerations consisting of a single line...but if you
> > prefer to discuss with the SecDir/SEC AD, I'm fine with it. :-)
> >
> [Les:] Security discussions always seem to be a bit of a tug-of-war.
> What you seem to be saying is that if an attacker gets "the keys", it
> can fill a TLV with misinformation. I totally agree. But how are the new
> TLVs defined in this document any different in this regard than existing
> TLVs?
>
> An attacker today - given it has the keys - could advertise bogus neighbors
> or bogus metrics or...
>
> ??

True.  However, this documents introduces something new that the
attacker can do: it can signal application specific information.
Before this document, the attacker didn't have the ability to pinpoint
the attack to specific applications...

The case mentioned above (inconsistently setting the L-flag to cause
the advertisements to be ignored) is also not a simple thing to
troubleshoot!

...
> > 634 This document requests a new IANA registry be created to control the
> > 635 assignment of sub-sub-TLV codepoints for the Application Specific
> > 636 Link Attributes sub-TLV. The suggested name of the new registry is
> > 637 "sub-sub-TLV code points for application specific link attributes".
> > 638 The registration procedure is "Expert Review" as defined in
> > 639 [RFC8126]. The following assignments are made by this document:
...
> > [major] If the legacy specifications are extended, is it expected that
> > new sub-sub-TLVs will also be created? If so, there need to be
> > explicit instructions to the DEs in the legacy registries.
> >
> [Les:] Since the draft specifies that new applications MUST NOT use legacy
> sub-TLVs, the case you are concerned with cannot arise.

I was thinking more of the existing applications extending the legacy...

> > ...
> > 663 This document requests a new IANA registry be created, under the
> > 664 category of "Interior Gateway Protocol (IGP) Parameters", to
> > control
> > 665 the assignment of Application Identifier Bits. The suggested name
> > of
> > 666 the new registry is "Link Attribute Applications". The registration
> > 667 policy for this registry is "Standards Action" ([RFC8126] and
> > 668 [RFC7120]). The following assignments are made by this document:
> >
> > [minor] If we're setting up a applications, why not make it generic:
> > "IGP Applications", for example...??
> >
> [Les:] Because this is defining bit positions which only apply to the
> SABM/UDABM fields in the new ASLA sub-TLV.

For now...

> > [minor] s/"Standards Action" ([RFC8126] and [RFC7120])./"Standards
> > Action" [RFC8126]. Standards Action is only defined in rfc8126;
> > the reference to rfc7120 is not needed.
> >
> [Les:] I copied this from RFC8665. Are you saying that document is incorrect?

I am.

...
> > 670 Bit # Name
> > 671 ---------------------------------------------------------
> > 672 0 RSVP-TE (R-bit)
> > 673 1 Segment Routing Traffic Engineering (S-bit)
> > 674 2 Loop Free Alternate (F-bit)
> >
> > [major] Add a line to cover the rest of the range and call it "Unassigned".
> >
> [Les:] Done

The registry can't say "Additional bits are undefined".  The correct
text shold be:

   3-1015   Unassigned

> > 676 This document requests a new IANA registry be created to control the
> > 677 assignment of sub-TLV types for the application specific SRLG TLV.
> > 678 The suggested name of the new registry is "Sub-TLVs for TLV 238".
> > 679 The registration procedure is "Expert Review" as defined in
> > 680 [RFC8126]. The following assignments are made by this document:
> >
> > [major] Indicate where this registry should be created.
> >
> [Les:] Done

That didn't make it in.

...
> > 682 Value Description
> > 683 ---------------------------------------------------------
> > 684 0-3 Unassigned
> > 685 4 Link Local/Remote Identifiers (see [RFC5307])
> > 686 5 Unassigned
> > 687 6 IPv4 interface address (see [RFC5305])
> > 688 7 Unassigned
> > 689 8 IPv4 neighbor address (see [RFC5305])
> > 690 9-11 Unassigned
> > 691 12 IPv6 Interface Address (see [RFC6119])
> > 692 13 IPv6 Neighbor Address (see [RFC6119])
> > 693 14-255 Unassigned
> >
> > [major] Do you want the reference to the assignments to be this
> > document, the RFC where the encoding is defined, or both? I would
> > suggest both. Please add a column to reflect that: e.g.,
> > "[RFC5307] [This Document]"...and take out the "(see ...)" text.
> >
> [Les:] I added another column for "Encoding Reference"

[nit] That's fine, but please add a note to indicate that the
reference (for new values) should be the document where the encoding
is defined and not where the value is assigned (which is what most
people consider the default).

> > 695 9. Security Considerations
...
> > And then there are possible vulnerabilities introduced by this
> > document. I put some comments above about the ability of a rogue
> > router to render the data unusable by setting, for example the L-flag,
> > or sending conflicting values. At best, this action could result in
> > sub-optimal paths (by taking some links out of consideration)...
> > Consider adding text related to that.
>
> [Les:] I have a harder time with this suggestion and so did not incorporate
> it. See my replies above.

See my replies above...

---

From: "Les Ginsberg (ginsberg)" <[email-masked]>
Date: Sat, 21 Mar 2020 17:09:51 +0000

Alvaro -

Following some offline discussion with you, V12 of the draft has been posted to address your additional comments.
Some responses inline. Look for "Les3".

> -----Original Message-----
> From: Alvaro Retana <[email-masked]>
> Sent: Thursday, March 05, 2020 8:54 AM
> To: Les Ginsberg (ginsberg) <[email-masked]>; Acee Lindem (acee)
> <[email-masked]>; [email-masked]
> Cc: [email-masked]; [email-masked]
> Subject: RE: AD Review of draft-ietf-isis-te-app-09
> 

[Les3:] I have decided to leave the reference to the registry.

> would be very nice if some text was added about this to avoid more

[Les3:] The use of the L-flag is of no value in this case. Since legacy routers will never
send the ASLA sub-TLV (w or w/o the L-flag), new routers have to have a knob which says "only process legacy sub-TLVs". They cannot depend on the presence
of ASLA w L-flag since not all routers will send it.

When routers start sending ASLA (Step #1) they might as well keep L-flag clear because the local knob means they will never look at the ASLA sub-TLV anyway.

At Step #2 when routers start using ASLA you do not want L-flag set as that
would simply introduce an extra step in preparation for removing legacy
advertisements i.e., readvertise ASLA w L-flag clear and actual link attribute values present.

Some clarifying text has been added.

> rtrB doesn't understand the bits (and ignores them) then it looks like

[Les3:] It is not correct to say that no bits are set.
In the case of rtrB in your example, it would be correct to say that no bits are set which are supported by rtrB.

> the correct/expected behavior?

[Les3:] Yes - this is the correct/expected behavior.
Standard protocol behavior - whether for a TLV/sub-TLV that is not supported or for an unsupported bit is to ignore it.

> zero-length bit mask is used.  In that case any application can use

[Les3:] If the mask length is 0, then there are no application bits transmitted. It is not correct to say "no bits are set" in this case.

> If that happens, what are the issues?

[Les3:] Text has been added requiring specifications which define new applications to discuss deployment issues in cases where routers which do not support the new application are present in the network.

> the attack to specific applications...

[Les3:] Text has been added to the security section mentioning that the fact that advertisements may be application specific the impact of an attacker may be limited to a specific application.

> I was thinking more of the existing applications extending the legacy...

[Les3:] Existing text says:

" Note to designated experts: If a link attribute can be advertised
   both as a sub-TLV of TLVs 22, 23, 25, 141, 222, and 223 and as a sub-
   sub-TLV of the Application Specific Link Attributes sub-TLV defined
   in this document, then the same numerical code should be assigned to
   the link attribute whenever possible."

I believe this covers the case you mention.

> > SABM/UDABM fields in the new ASLA sub-TLV.

[Les3:] You are speculating that we will find some other use case for SABM?

The problem with changing the name as you suggest is that it would allow
for an application which is not associated with a Link Attribute e.g., some 
sort of node context. In which case that bit couldn't be advertised 
in the existing sub-TLVs. We would then have to specify which standard
bit values are legal to set in the ASLA advertisements and which are not.

I don't find this complication attractive.

> > [Les:] I copied this from RFC8665. Are you saying that document is

[Les:3] Appropriate corrections have been made.

> The registry can't say "Additional bits are undefined".  The correct

[Les3:] In earlier versions SABM Length  is unbounded. But per our discussion, I have introduced an arbitrary upper bound on this length (8 - which translates to a maximum of 64 application bits). This then allows the corrected text you suggested.
As a practical matter, limiting SABM Length makes some sense since it ensures there will be room in the sub-TLV for link attribute information.
I believe 64 applications is well beyond the number that will ever be defined.

> > > [major] Indicate where this registry should be created.

[Les3:] Thanx - corrected.

> people consider the default).

[Les3:] Done

> > it. See my replies above.

[Les3:]  As previously mentioned, some text has been added.

   Les