Re: [secdir] Secdir review of draft-ietf-trill-multilevel-unique-nickname-05

[Roman Danyliw <[e-mail]>]

Hi Donald,

> -----Original Message-----
> From: Donald Eastlake [mailto:[e-mail]]
> Sent: Sunday, March 04, 2018 1:58 PM
> To: Roman Danyliw <[e-mail]>
> Cc: [e-mail]; [e-mail]; draft-ietf-trill-multilevel-unique-
> [e-mail]
> Subject: Re: Secdir review of draft-ietf-trill-multilevel-unique-nickname-05
> 
> Hi Roman,
> 
> Thanks for doing a review. See responses below.
> 
> On Fri, Mar 2, 2018 at 9:00 PM, Roman Danyliw <[e-mail]> wrote:
> > Reviewer: Roman Danyliw
> > Review result: Ready with nits
> >
> > I have reviewed this document as part of the security directorate's ongoing
> effort to review all IETF documents being processed by the IESG.  These
> comments were written primarily for the benefit of the security area
> directors.  Document editors and WG chairs should treat these comments
> just like any other last call comments.
> >
> > The summary of the review is Ready with nits.
> >
> > My feedback is as follows:
> >
> > (1) Section 4.1, Multilevel TRILL Basics, Page 8
> >
> > Thus Level 1 link state
> > information stays within a Level 1 area and Level 2 link state
> > information stays in Level 2 unless there are specific provisions for
> > leaking (copying) information between levels.
> >
> > ** What are these provisions where such leakage of information should
> occur beyond expected routing behavior?
> 
> Typically "link state" information stays within a Level 1 area or within the
> Level 2 routers. Occasionally there is information that it is desirable to  flood
> throughout the domain at both Level 1 and Level 2. IS-IS "link state"
> information is in the form of TLVs. Typically this domain wide flooding is
> accomplished by using two flags in the value portion of the TLV. One flag
> indicates the the TLV is to be flooded domain wide while the other is initially
> zero and is set when the TLV is flooded from Level 2 to Level 1 -- this second
> flag is to stop a TLV from being flooded from Level 2 to Level 1 and then back
> to Level 2 again resulting in TLV looping. See, for example, the IS-IS Router
> Capabilities TLV D and S flags as specified in Section 2 of RFC 7981.  This is all
> standard IS-IS machinery that someone familiar with IS-IS would know --
> there is nothing TRILL specific about it.

Understood.  I was struggled with the wording "... unless there are specific ..." as this didn't imply to me a fixed set of previously understood constraints.  I would have chosen "... beyond the expected provisions ... between levels necessary for protocol function."

[snip]
 
> > (3) Section 6, Security Considerations, Page 12.
> >
> > With TRILL multilevel, flooding of control traffic for link state
> > information of Level 1 and Level 2 is separated. This addresses the
> > TRILL scalability issues as specified in Section 2 of [RFC8243] and
> > also confines the effective scope of possible malicious events.
> >
> > ** Per the sentence "With TRILL ... is separated", I recommend
> > clarifying the language on what and in what way there is separation
> 
> This is basic to multilevel IS-IS and anyone familiar with that would
> understand. We could add a reference to IS-IS.

Agreed.  It makes sense to add the reference.

> > ** Per the follow-up sentence, "... also confines the effective scope of
> possible malicious events", I recommend discussing in more detail how the
> scope of malicious events is reduced with this approach.
> 
> I suggest the following replacement text:
> 
>    Since TRILL multilevel uses the existing IS-IS multilevel facilities [IS-IS],
> flooding of control traffic for link state information is automatically confined
> to a Level 1 area or to Level 2 except (for limited types of information that
> can be specifically flagged for wider flooding). This addresses the TRILL
> scalability issues as specified in Section 2 of [RFC8243] and also, except of the
> wider flooding case, this confines the scope of the effects of malicious
> events that could be communicated through the link state.

The suggested wording works for me.

> > (4) Section 6, Security Considerations, Page 12.
> >
> > However, due to the nature that unique nickname areas share a unique
> > nickname space, border RBridges still have to leak nickname
> > information between levels. For this purpose, border RBridges need to
> > fabricate the nickname announcements as specified in Section 4.3.
> >
> > ** As it is raised as an issue with a mitigation, I recommend articulating the
> implication of leaking nicknames across levels.
> 
> Since nicknames must be unique across the multi-level domain, and
> nicknames in TRILL are auto-allocated, clearly RBridges inside an area need to
> know what nicknames are in use, which is the effect and purpose of leaking
> nickname claim information across levels. I suggest the following wording:
> 
>     However, due to the nature that unique nickname areas share a common
> nickname space, border RBridges still have to leak nickname information
> between levels. Such leaking means that nickname related events in one
> area can affect other areas. For this purpose, border RBridges need to
> fabricate the nickname announcements as specified in Section 4.3.

The suggested wording works for me.

[snip]
> 
> > (6) Section 6, Security Considerations, Page 12.
> >
> > For this reason, RBridges SHOULD be configured to include the IS-IS
> > Authentication TLV (10) in the IS-IS PDUs that contains the
> > NickBlockFlags APPsub-TLV, so that IS-IS security ([RFC5304]
> > [RFC5310]) can be used to secure the network.
> >
> > ** Should a preference be expressed for RFC5310 over RFC5304?  To quote
> RFC5310, "[while at the time of this writing there are no openly published
> attacks on the HMAC-MD5 mechanism, some reports ([Dobb96a],
> [Dobb96b]) create concern about the ultimate strength of the MD5
> cryptographic hash function."
> 
> I would agree that RFC 5310 security is superior to RFC 5304 security.
> Perhaps references to 5304 can be removed.

I'd recommend the more agile RFC5310 too.

> > ** Recommend being more specific with the language "to secure the
> network".  Perhaps "For this reason, RBridges SHOULD authenticate their
> peer by using the IS-IS Authentication TLV (10) in the IS-IS PDUs that contains
> the NickBlockFlags APPsub-TLV."
> 
> Suggest replacing "to secure the network" with "to authenticate those PDUs
> and discard them if they are forged."

The suggested wording works for me.

[snip]

Thanks,
Roman

> Thanks,
> Donald