[mpls] Ben's Comments on draft-ietf-mpls-sr-over-ip

["Adrian Farrel" <[e-mail]>]

Hi Ben,

Thanks for your very detailed comments on this draft. Quite a lot to chew on.

> Thanks for this clearly-written document!  I do have several
> suggestions for improvements, and note many places where IS-IS and OSPF
> seem to be described as normative references would, in the vein of
> Mirja's DISCUSS.  I think it is possible to adjust the document to keep
> those as informative references, but the changes might be pretty
> intrusive.

I'm going to come back to Mirja's Discuss (and part of Alvaro's Discuss) when I have some more clarity about how the LSR working group is advancing the relevant drafts.

> I'm a little confused about what exactly the new protocol specified here
> is -- there seems to be a lot of "use these existing technologies
> together in the following way" going on, that may be screening the
> actual new bits from prominence.  The only thing that comes to mind is
> the requirement to use the explicit null label when the last label has
> PHP and is going over the tunnel, which is admittedly important behavior
> to have!  (To be clear, the implication is that a document consisting
> solely of "glue these existing pieces together" could well be published
> as Informational, though does not strictly speaking need to be.)

I think the same way. It *could* have been Informational, but Standards Track felt better in this case as it is telling people how make an operational system out of the existing building blocks.

> It might also be helpful to have a reference to RFC 4023 for the plain
> MPLS-in-IP encapsulation, even just to contrast it to the MPLS-in-UDP
> (RFC 7510) that we do use.  (This reader did not discern why the RFC
> 4023 encapsulation was unpreferred.)

Oh, right.
The short answer is "for the same reasons that 7510 unpreffered MPLS-in-IP" which was largely load balancing through an entropy indicator.
Will add a short note.

> Section-by-section comments follow.
>
> Abstract
>
>                                                               SR-MPLS
>   could be leveraged to realize a source routing mechanism across MPLS,
>   IPv4, and IPv6 data planes by using an MPLS label stack as a source
>   routing instruction set while preserving backward compatibility with
>   SR-MPLS.
>
> Is this really saying "SR-MPLS could be leveraged ... while preserving
> compatibility with SR-MPLS"?

Fixed per Alvaro's comments.

> Section 1 (Introduction)
> 
> Similarly to the above:

Ditto

> Section 2
>
>   o  If encoding of entropy ([RFC6790] is desired, IP tunneling
>      mechanisms that allow encoding of entropy, such as MPLS-in-UDP
>      encapsulation [RFC7510] where the source port of the UDP header is
>
> I think it was already noted, but this language reads a little oddly,
> and it might be better as "If injection of additional entropy into the
> flow is needed".

Just buffed up that text a bit.

> Section 3.1
>
>   Consider router A that receives a labeled packet with top label L(E)
>   that corresponds to the prefix-SID SID(E) of prefix P(E) advertised
>   by router E.  Suppose the i-th next-hop router (termed NHi) along the
>   shortest path from router A toward SID(E) is not SR-MPLS capable
>   while both routers A and E are SR-MPLS capable.  [...]
>
> Do we ever actually use the fact that it is specifically the i-th router
> that is SR-MPLS-incapable, or just need to know that at least one router
> on the path is deficient in that way?

It is just a name for that router.
It is mentioned again on the last line of section 3.1

>   o  The Segment Routing Global Block (SRGB) is defined in [RFC8402].
>      Router E is SR-MPLS capable, so it advertises an SRGB as described
>      in [I-D.ietf-ospf-segment-routing-extensions] and
>      [I-D.ietf-isis-segment-routing-extensions].
>
> I see the first sentence was added in discussion with another reviewer;
> I wonder if the text would flow better if the order of the sentences was
> swapped (particularly since this is the very first "processing step",
> and that declarative sentence does not describe any processing
> actions).

I'll work something out. The "as described" had confused a previous reader because it seemed to suggest that the SRGB was defined in those drafts.

>   o  When Router E advertises the prefix-SID SID(E) of prefix P(E) it
>     MUST also advertise the encapsulation endpoint and the tunnel type
>      of any tunnel used to reach E.  It does this using the mechanisms
>      described in [I-D.ietf-isis-encapsulation-cap] or
>      [I-D.ietf-ospf-encapsulation-cap].
>
> This "it does this" still is implying that IS-IS and OSPF are the only
> ways this can be done, which (AIUI) is not the intention.  (Similarly
> for the next bullet point's sub-bullets.)

s/it does this/it can do this/

>      *  If router E is running ISIS and advertises the ISIS
>         capabilities TLV (TLV 242) [RFC7981], it MUST set the "router-
>
> nit: 7981 seems to call this the "ISIS Router CAPABILITY TLV"; I don't
> know if the usage here has since become conventional.

s/ies/y/

>   o  Router A programs the FIB entry for prefix P(E) corresponding to
>      the SID(E) as follows:
>
> Keeping with the theme, this is a declarative statement of procedure,
> and the following sub-bullets only cover OSPF and ISIS.  It would need
> to be a statement of requirements for behavior/functionality in order to
> keep them from being normative references.
>
>   Once constructed, the FIB can be used to tell a router how to process
>   packets.  It encapsulates the packets according to the encapsulation
>   advertised in [I-D.ietf-isis-encapsulation-cap] or
>   [I-D.ietf-ospf-encapsulation-cap].  Then it sends the packets towards
>   the next hop NHi.
>
> Oh, finally, "NHi" appears again.  But are we actually forwarding
> towards the SR-MPLS-incapable router specifically, or just generically
> to the immediate next hop (which may or may not be NHi)?

I told you so 😉
It really means NHi. The previous hop does the encapsulation and then forwards *towards* (i.e., out of the interface pointing at NHi) but not with the DA set to NHi.

> Also, I'll note that we introduced this list with "the following
> processing steps apply [for A wanting to send a packet towards E]", but
> most of the procedures therein involve things that E has to do to
> advertise the information that A will need in order to make the
> encapsulation, which feels like somewhat mismatched language.  In short,
> this list of points doesn't tell me much about what specifically *A*
> does with this packet -- the last point handles what to do with the top
> label, but I guess we defer to the encapsulation scheme for the gory
> details (which is probably fine, but maybe the language could be tidied
> up to mention something about "using the appropriate mechanism for the
> selected tunnel type (e.g., IP)").

I'm going to call upon your "which is probably fine".

>  packets.  It encapsulates the packets according to the encapsulation
>   advertised in [I-D.ietf-isis-encapsulation-cap] or
>   [I-D.ietf-ospf-encapsulation-cap].  Then it sends the packets towards
>
> nit: advertised *using the mechanisms in*

Ack

> Section 3.2
>
>   [RFC7510] specifies an IP-based encapsulation for MPLS, i.e., MPLS-
>   in-UDP.  This approach is applicable where IP-based encapsulation for
>   MPLS is required and further fine-grained load balancing of MPLS
>   packets over IP networks over Equal-Cost Multipath (ECMP) and/or Link
>   Aggregation Groups (LAGs) is also required.  This section provides
>   details about the forwarding procedure when when UDP encapsulation is
>   adopted for SR-MPLS over IP.
>
> (side note) Presumably this is just my lack of background showing, but
> this text seems to assume that the UDP encapsulation is the only way to
> do SR-MPLS over IP, with no discussion of RFC 4023.

This section is certainly limited to the UDP choice.
Actually, we can add "Other encapsulation and tunnelling mechanisms can be applied using similar techniques, but for clarity this section uses UDP encapsulation as the exemplar."

>  Nodes that are SR-MPLS capable can process SR-MPLS packets.  Not all
>  of the nodes in an SR-MPLS domain are SR-MPLS capable.  Some nodes
>  may be "legacy routers" that cannot handle SR-MPLS packets but can
>  forward IP packets.  An SR-MPLS-capable node MAY advertise its
>  capabilities using the IGP as described in Section 3.  There are six
>  types of node in an SR-MPLS domain:
>
> (side note) Similarly, we only talk about SR-MPLS and IP routers here;
> is there some intermediate "MPLS but not SR-MPLS" category or similar?

Ah, a good question.
MPLS routers are automatically able to forward SR-MPLS packets. So, if they lack SR capabilities they still participate seamlessly.
And, in cast, if you had a sea of MPLS routers, you wouldn't need any encapsulation at all.

> Also, the top-level Section 3 is just introductory text and doesn't
> really describe much of anything, and if we are considering this to
> refer to Section 3 and subsections, it then becomes self-referential.

I think it applies to 3.2.

> Section 3.2.1
>
>                                           Routers A, E, G, and H
>   advertise their Segment Routing related information via IS-IS or
>   OSPF.
>
> [still declarative]

Still fixing 😊

>   To handle this, when a router (here it is router G) pops the final
>   SR-MPLS label, it inserts an explicit null label [RFC3032] before
>   encapsulating the packet in an MPLS-over-UDP tunnel toward router H
>   and sending it out.  That is, router G pops the top label, discovers
>   it has reached the bottom of stack, pushes an explicit null label,
>   and then encapsulates the MPLS packet in a UDP tunnel to router H.
>
> This seems like a critical functional behavior that is not specific to
> the usage of IS-IS or OSPF.

Agreed. Covered by previous fix.

> Section 3.2.3
>
>      For instance, in the example as described in Figure 4, assume F is
>      now an SR-MPLS-capable transit node while all the other
>      assumptions keep unchanged, since F is not identified by a SID in
>      the stack and an MPLS-over-UDP tunnel is preferred to an MPLS LSP
>      according to local policies, router E would replace the current
>      top label with an MPLS-over-UDP tunnel toward router G and send it
>      out.
>
> (nit: There's a comma splice before "since F".)

Ack

> This description is a little sparse, and seems to assume the reader
> knows that, all else being equal, with E, F, and G all being
> MPLS-capable, the segment from E to G would transit native MPLS, absent
> the indicated local policy.  Since this is explicitly the thing we are
> contrasting against, it's probably good style to mention the behavior
> explicitly, too.

OK. "Good style" is our watchword.

>   IP Header Fields:  When encapsulating an MPLS packet in UDP, the
>      resulting packet is further encapsulated in IP for transmission.
>      IPv4 or IPv6 may be used according to the capabilities of the
>      network.  The address fields are set as described in Section 2.
>
> (Does Section 2 say how to set the source address?)

Sure does.
      The tunnel selected MUST have its
      remote end point (destination) address equal to the address of the
      next SR-MPLS capable node along the path (i.e., the egress of the
      active node segment).

>   Entropy and ECMP:  When encapsulating an MPLS packet with an IP
>      tunnel header that is capable of encoding entropy (such as
>      [RFC7510]), the corresponding entropy field (the source port in
>      case UDP tunnel) MAY be filled with an entropy value that is
>
> nit: "in the case of a UDP tunnel"

Ack

> Section 5
>
> [I'm going to comment on basically every line here; sorry to be so
> nitpicky.]

I'm going to cross-check. If you have left a line uncommented I will demand a refund.
If only we had written more lines.

>   The security consideration of [RFC8354] and [RFC7510] apply.  DTLS
>
> RFC 8354's security considerations are, for practical purposes, empty.
> Perhaps a direct reference to 5095 would be more appropriate.
> (The RFC 7510 security considerations are of course quite relevant and
> helpful.)

Yes, let's have...
The security consideration of [RFC8354] (which redirects the reader to [RFC5095]) and [RFC7510] apply
...since 8354 remains the more "relevant" reference.

>  [RFC6347] SHOULD be used where security is needed on an MPLS-SR-over-
>  UDP segment.
>
> I think you need to give some indication of when that might be the case,
> e.g., if the IP segment crosses the public Internet or some other
> untrusted environment.

I guess that is a good example. I'm worried that it may limit the imagination of the reader.

I'm going to use "including when" in place of "e.g., if"

>   It is difficult for an attacker to pass a raw MPLS encoded packet
>   into a network and operators have considerable experience at
>   excluding such packets at the network boundaries.
>
> This is describing an expected/desired state (hopefully also the current
> state!) but not why that should be or what the consequences of failing
> to achieve that state are.  I'm happy to contribute some text, but this
> seems like a generic MPLS consideration and I hope there is an
> already-published reference that could be used instead.

Well, I'll add a reference to 5920, but I don't think that completely covers it.
This is the equivalent of an ACL to prevent IP-in-IP intrusion, but essentially says don't let MPLS emerge from an IP tunnel in the network. Everyone does it, not sure it is documented anywhere.

That gives us...

      It is difficult for an attacker to pass a raw MPLS encoded packet
      into a network and operators have considerable experience at excluding
      such packets at the network boundaries, for example by excluding all
      MPLS packets that are revealed as payload of IP tunnels. Further discussion
      of MPLS security is found in [RFC5920].

>   It is easy for an ingress node to detect any attempt to smuggle an IP
>   packet into the network since it would see that the UDP destination
>   port was set to MPLS.  SR packets not having a destination address
>
> Easy, sure, if you know to look for it.  Where is that requirement
> mandated?

We don't mandate against self-harm.
But I add...
       , and such filtering SHOULD be applied

>   terminating in the network would be transparently carried and would
>   pose no security risk to the network under consideration.
>
> I'm not sure I see how the second sentence relates to the first; is this
> just trying to say that SR-MPLS is transparent to traffic flowing over
> such a network?  There would still be generic capacity risks, of course,
> though maybe we don't need to be so specific -- "pose no different
> security risk than any other traffic" would be plenty.

OK

>  Where control plane techniques are used (as described in Section 3),
>  it is important that these protocols are adequately secured for the
>  environment in which they are run.
>
> What does "adequately secured" mean?  Is there a best-practices
> document, or security considerations for the control protocols in
> question that cover this?  Is the needed property that "any packets
> interpreted as MPLS must be originated from authorized network devices
> within the administrative domain (or group of such domains), with no
> ability for attackers to inject such traffic inside the domain and
> blocking at the boundary"?  When different administrative domains are
> joined together, what are the counterparty risks?

OK, these are not MPLS packets, they are control plane packets.
The injection of MPLS packets was where we were a few comments further up.

"Adequately secured" is indeed a wide scope. I think 5920 is our base reference for all MPLS control plane security issues.

But perhaps 6862 as well.

Best,
Adrian