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

["Les Ginsberg (ginsberg)" <[e-mail]>]

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 <[e-mail]>
> Sent: Thursday, March 05, 2020 8:54 AM
> To: Les Ginsberg (ginsberg) <[e-mail]>; Acee Lindem (acee)
> <[e-mail]>; [e-mail]
> Cc: [e-mail]; [e-mail]
> Subject: RE: AD Review of draft-ietf-isis-te-app-09
> 
> 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.
> 

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

[Les:3] Appropriate corrections have been made.

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

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

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

[Les3:] Thanx - corrected.

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

[Les3:] Done

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

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

   Les