Re: [Lsr] Secdir last call review of draft-ietf-isis-segment-routing-msd-16

[Benjamin Kaduk <[e-mail]>]

On Wed, Sep 26, 2018 at 08:45:03PM +0000, Les Ginsberg (ginsberg) wrote:
> David -
> 
> Thanx for the review.
> A new version of the draft (17) has been published to address your comments - subject to my responses below.

Just in time for me to see the updated version for my IESG review; thanks.

> > -----Original Message-----
> > From: David Waltermire <[e-mail]>
> > Sent: Tuesday, September 25, 2018 12:14 PM
> > To: [e-mail]
> > Cc: [e-mail]; [e-mail]; draft-ietf-isis-segment-routing-
> > [e-mail]
> > Subject: Secdir last call review of draft-ietf-isis-segment-routing-msd-16
> > 
> > Reviewer: David Waltermire
> > Review result: Has Issues
> > 
> > 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 (minor) issues
> > 
> > My apologies for the late review on this draft. Overall I found this document
> > to be well-written, and concise.
> > 
> > General Comments:
> > 
> > This document uses a mix of case around RFC2119 language (e.g., MAY may).
> > You should use text from RFC8174 to indicate that lowercase versions of the
> > keywords are not normative, or adjust the case of the lowercase words to
> > ensure there is no confusion.
> > 
> [Les:] Section 1.2 does include the standard boilerplate for RFC 2119/RFC8174.
> 
> I checked all the lower case uses of "may" and they are intentional.
> There was one instance of "should" that I changed to uppercase.
> 
> > Minor nit: There is some inconsistency in the use of "MSD-Type" (the value)
> > and "MSD type" (the concept). Suggest cleaning this up.
> > 
> [Les:] Done
> 
> > Specific comments:
> > 
> > Section 1:
> > 
> > Para 1: s/to insure/to ensure/
> 
> [Les:] Done.
> 
> > 
> > Section 4:
> > 
> > The last paragraph establishes a requirement on the registration of an MSD
> > Type to define what the absence of a given MSD Type means. This is an
> > important requirement that must be addressed during registration of new
> > MSD Types. IMHO, this requirement should be echoed in the registration
> > information in section 6 to make sure it is not overlooked.
> >
> [Les:] I disagree. Section 6 is defining exactly what should go into the new IANA registry.
> The definition of "absence" is something that will have to be provided in the documents which define new MSD-types, but that will NOT be captured in the registry so including this in Section 6 isn’t appropriate.

I think a good way to think about this is as giving guidance to the
Experts, that they should not approve registration requests that fail to
provide this information along with the request.  Guidance for the Experts
is appropriate in the IANA Considerations section.
(Also, my understanding is that IANA prefers to have a more explicit
template for new registrations to follow, though I should not try to speak
for them.)

>  
> > Section 6:
> > 
> > The "Base MPLS Imposition MSD" should reference section 5 of this
> > document.
> 
> [Les:] Again - Section 6 is defining what will go into the registry. The registry will reference the document - not a specific section of the document.

The contents of the "Reference" column for Value 1 can refer to a specific
section of a document, surely?

> > 
> > The registration for "Experimental" should be marked as "Reserved for
> > Experimental Use" or just "Experimental Use" to align with RFC8126. RFC8126
> > states that "it is not appropriate for documents to select explicit values from
> > registries or ranges with this policy". It might be good to add a note alongside
> > the one on "Designated Experts" indicating that values from this range are
> > not assignable.
> > 
> [Les:] I have changed the text to "Experimental Use".
> I think the rest of your comment is addressed by RFC 8126 - which is referenced.
> 
> > The "Interior Gateway Protocol (IGP) Parameters" registry has the
> > "Standards Action" policy assigned. The new "IGP MSD Types" sub-registry
> > does not have the "Standards Action" policy. Was this intentional? If so, this
> > should be explained. This is also confusing since the guidance for expert
> > reviewers in
> > RFC7370 implies that registrations are based on the "RFC Required" or
> > "Standards Action" policies.
> >
> [Les:] IS-IS registries are typically Expert Review. This derives from considerations related to the liason with ISO JTC 1/SC6 (RFC 3563).
> OSPF registries are typically Standards Action.
> 
> As IGP Parameters was defined by draft-ietf-ospf-segment-routing-extensions, it is Standards Action.
> But as MSD-Types is being defined in an IS-IS draft...
> 
> Please learn to live with this.
> It isn’t a significant issue in my view.
> 
> 
>  
> > Section 7:
> > 
> > The security considerations in RFC7981 ask that security considerations
> > around the disclosure and modification of this type of information is
> > described in extensions. This has been done, but RFC7981 also asks that an
> > integrity mechanism be provided if there is a high risk resulting from
> > modification of capability information. There is no discussion in the
> > document's security consideration about the nature of risk in this case and
> > why an integrity mechanism is not needed. It seems like false information
> > can be used to cause a denial of service regarding computed paths. This
> > sounds like having this happen could be bad. I am not an expert on routing
> > protocols, so I am not sure if this is an issue. How bad and likely is such a risk?
> > 
> 
> [Les:] The integrity mechanism is (as you point out) discussed in the Security section of RFC 7981 - which is referenced in the Security Section of this document.
> The introduction of a new TLV does not alter the integrity mechanism requirements.

My read of 7981 is that "there are these existing integrity protection
mechanisms; when the consequences of modification are bad, use them".
So there is not necessarily a need for each TLV to provide its own internal
integrity scheme, as Les notes.

-Benjamin