Subject: Re: [mpls] AD review of draft-ietf-mpls-ldp-gtsm

From: Carlos Pignataro <[email-masked]>
Date: Mon, 14 May 2012 22:38:07 -0400

Adrian,

Many thanks for your usually thorough review. We are spinning a new I-D after hitting "send", please do ensure all the issues are satisfactorily addressed.

Additionally, please see inline.

On Apr 30, 2012, at 8:49 AM, Adrian Farrel wrote:

> Hello authors of draft-ietf-mpls-ldp-gtsm,
> 
> I have conducted my usual AD review of your draft in response to the
> publication request from the WG. This, of itself, is not reason to
> induce panic :-)
> 
> My review has yielded a number of questions and nits. All of the
> points are open for discussion and I am not mandating changes. But
> I think that there are enough issues here to warrant a new revision.
> 
> I have put the document into "revised I-D" state and look forward to
> a new version or a lively email debate.
> 
> Thanks,
> Adrian
> 
> ---
> 
> I am confused by whether you propose GTSM to be enabled by default. 
> 
> Section 1 says...
>   GTSM specifies that it SHOULD NOT be enabled by default in order to
>   remain backward-compatible with the unmodified protocol
> 
> Section 1.2 says...
>   GTSM and will require (statically or dynamically) disabling GTSM.
>   See Section 3.
> 
> Section 3 says...
> 
>   The reason GTSM is enabled for Basic Discovery by default
> 
> Indeed, 5082 says...
> 
>   3.  Use of GTSM is OPTIONAL, and can be configured on a per-peer
>       (group) basis.
> 
> In fact, I don't think GTSM is enabled by default in your document. I
> think what you have is that the desire to use GTSM (i.e., the
> negotiation to use it) is enabled by default in implementations of your
> spec. But this is subtly different from what the text says.
> 

Correct. Text updated.

> ---
> 
> Can you tidy the English in Section 1. Suggested...
> 
> OLD
>   GTSM specifies that it SHOULD NOT be enabled by default in order to
>   remain backward-compatible with the unmodified protocol; this
>   document specifies having a built-in dynamic GTSM capability
>   negotiation for LDP to suggest the use of GTSM, provided GTSM is not
>   enabled unless both peers can detect each others' support for GTSM
>   procedures and agree on its usage as described in this document.
> NEW
>   GTSM specifies that it should not be enabled by default; this
>   facilitates backward-compatibility with the unmodified protocol. This
>   document specifies a dynamic negotiation for the an LDP peer to 
>   suggest the use of GTSM.  GTSM will be used as specified in this
>   document provided both peers on an LDP session can detect each
>   others' support for GTSM and agree to use it.
> END
> 

I updated the text, slightly different "NEW" but hopefully tidied up.

> Note the change s/SHOULD NOT/should not/. This text in Section 1 is not
> defining any protocol behavior and comes before the citation of RFC 
> 2119. It doesn't need to use upper case.
> 

I agree S1 is not defining protocol. However, the "SHOULD NOT" (capital) is because it is sentence verbatim from RFC 5082 where it is defining protocol. I made that more clear by enclosing in quotes.

> ---
> 
> You need to do some work to expand acronyms on first use if they are not
> listed with an asterisk at:
> 
> http://www.rfc-editor.org/rfc-style-guide/abbrev.expansion.txt
> 
> I found:
> 
> LSR
> 

I thought I had done this. Seems that LSR escaped -- I do not see another one. Fixed.

> ---
> 
> Is it time to define an IANA registry for the common Hello parameters
> bit fields? Had one existed, you would not have needed to update 5036.
> If you create one now, future bit users will not have to update your
> RFC.

OK, let the lively email debate begin here indeed!

We thought it was time indeed. This is why we had taken the time to create an I-D on "LDP IANA Considerations", and specify the Common Hello Parameters bit field IANA registry at http://tools.ietf.org/html/draft-ietf-mpls-ldp-iana-01#section-2.3. We also took the extra time to comb through all of RFC 5036 and add every missing one to this I-D, in hopes of preventing this issue in the future for LDP.

However, Adrian Farrel observed that http://www.ietf.org/mail-archive/web/mpls/current/msg07321.html

So that is not a bit field, really, it is an opaque semanticless reserved space.

> 
> ---
> 
> Section 2.1
> 
>   The G flag is meaingful only if T and R flags are set to 0 (which
>   must be the case for Basic Discovery), otherwise, the value of G flag
>   SHOULD be ignored on receipt.
> 
> This is clear for the T flag: the use of GTSM is not supported for 
> targeted (extended) Hellos.
> 
> Why is this the case for the R flag? The use of the R flag is making a 
> request, not demanding use. It might be the normal case that you would
> not set the R flag unless you were more than one hop away, but it does
> not follow that this is the case, does it? As indicated in RFC 5036, a
> Hello receiver that does not support targeted Hellos ignores the
> request (i.e., the R flag). That would clear it to support the G flag.

Agreed. Removed the "and R" flag.

> 
> ---
> 
> Section 2.1
> 
>   Any LSR not supporting GTSM for LDP, as defined in this document,
>   would continue to ignore the G flag, independent of T and R flags'
>   value, as per Section 3.5.2 of [RFC5036].
> 
> I think you need to cover two cases. As it happens the behavior you are
> asking for is the same, but I think you need to call out the two cases
> separately:
> 
> 1. An LSR that does not recognise the G flag.
> 2. An LSR that does recognise the G flag but does not support GTSM 
>   (either through implementation or configuration).
> 

Sure. Done, please check if the updated text covers this adequately. 

> ---
> 
> Section 2.2
> 
>   Firstly, LSRs using LDP Basic Discovery [RFC5036] send LDP Hello
>   messages to link-level multicast address (224.0.0.2 or "all
>   routers").  Such messages are never forwarded beyond one hop and
>   assumed to have their IP TTL or Hop Count = 1.
> 
> This assumption is, I think, new in your document. I can't find anything
> about this in 5036 (although I may have missed it).
> 
> I suspect you need to change this from an assumption to a RECOMMENDation
> 

OK. Surprisingly (to me) I also did not find it in 5036 or 3036. Initially I thought that was potentially outside this doc, but I agree with you. Changed to "RECOMMENDED"

> ---
> 
> Section 2.2
> 
>   An LSR that is capable of applying GTSM procedures to the subsequent
>   TCP/LDP peering session MUST set the G flag (for GTSM) to 1 in Common
>   Hello Parameter TLV in the LDP Link Hello message [RFC5036].
> 
> "capable" is such an interesting word :-)
> And the text here is interesting. Does this cut into the discussion of
> defaults above? What you are saying is that a device that has been coded
> to support GTSM has to negotiate its use regardless of operator
> preferences. Why?

No, "capable and willing" :-)

> 
> How would the following look?
> 
>   Unless configured otherwise, an LSR that supports GTSM sets the G 
>   flag (for GTSM) to 1 in Common Hello Parameter TLV in the LDP Link
>   Hello message [RFC5036].
> 

OK, I took this as a baseline and slightly updated it.

> ---
> 
> Section 2.2
> 
>   An LSR, upon receiving an LDP Link Hello message, would recognize the
>   presence of G flag (in Common Hello Parameter TLV) only if it
>   supports GTSM for LDP, as specified in this document.
> 
> This is not true :-(
> You are updating 5036 so all new implementations will recognise the G 
> flag whether or not they support GTSM.
> 

Agree. See below.

> ---
> 
> Section 2.2
> 
>   If an LSR
>   recognizes the presence of G flag with the value =1 in the received
>   LDP Link Hello message, then it MUST enforce GTSM for LDP in the
>   subsequent TCP/LDP peering session with the neighbor that sent the
>   Hello message, as specified in Section 2.3 of this document.
> 
> Again, this does not offer the operator any control or choice in the 
> matter. Why is the use of GTSM not configurable?
> 

Agree. Text updated to address the two comments..

> ---
> 
> Section 2.2
> 
>   If an LSR that has sent the LDP Link Hello with G flag = 1, then the
>   LSR MUST set IP TTL or Hop Count = 255 in the forthcoming TCP
>   Transport Connection(s) with that neighbor (e.g., LSR2).  Please see
>   Section 2.3 for more details about the TCP transport connection
>   specifics.
> 
> Why is this MUST? What if the peer has responded with G=0 indicating it
> doesn't support (or want to use) GTSM? Surely, in that case, the local
> LSR can set any TTL.

Yes...

> 
> BTW Why is this paragraph in Section 2.2?

... and in fact this is the first para of S2.3.

> 
> ---
> 
> I think the mentions of "LSR2" in sections 2.2 and 2.3 are a little 
> unnecessary.
> 

I agree. I think they are there for extra clarity, and do not hurt, but can be removed.

> ---
> 
> Are we missing a discussion of what happens if the value of G changes on
> a subsequent Hello?
> 

Perhaps, but that negative testing is also not defined for the T and R bits of RFC 5036 (that I could see). However it is implied from the procedures.

> ---
> 
> Section 3
> 
> Typo
> 
> s/en the third case/In the third case/

Thanks :-)

> 
> ---
> 
> Section 5
> 
> Suppose I touch the setting of the G flag in transit?
> 
> That means that LDP Hello security is more important, right?

In that case you would not be off-link. 

	"This document increases the security for LDP, making it more resilient to off-link attacks."

I think this spec states its scope clearly, and if you touch the G flag on-link, we have worst problems.

Is that oversimplifying or do you see something blatantly missing from the Security Considerations?

> 
> ---
> 
> Section 5
> 
> Shouldn't you make reference to the security section of 5082. In 
> particular, you need to highlight the things GTSM does not protect 
> against.
> 

We could cite the Security Considerations of RFC 5082. However, the Security Considerations of RFC 5082 is about half of RFC 5082. This is partly because it deals with the most generic cases of GTSM including Tunnels, multi-hop, etc. I think that it is a bit implied, but explicit security sounds better than implicit.

> ---
> 
> I believe your document should give stronger hints than the generic
> comments in 5082 about what should be done with an LDP message that
> "fails" a GTSM TTL check. This is appropriate because you are writing
> a protocol-specific application of GTSM.
> 

We basically point to Section 3 of RFC 5082 for this, which says:

         +  Dangerous: these are packets that have been identified as
            belonging to one of the GTSM-enabled sessions, but their TTL
            values are NOT within the expected range, and hence GTSM
            believes there is a risk that these packets have been
            spoofed.
...
         +  MAY drop packets classified as Dangerous.

So we purposely did not want to force a policy to drop. This is something that, during the writing of RFC 5082, was explicitly requested.

Do you feel we should have a more prescriptive guideance (not using the "P" (policy) word)? Which way do you think should hint more strongly?

Adrian, many thanks again for your review, it greatly improves the document!!!

-- Carlos.

> _______________________________________________
> mpls mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/mpls
>

---

From: "Adrian Farrel" <[email-masked]>
Date: Tue, 15 May 2012 09:57:31 +0100

Hi Carlos,

Cutting down to just the IANA issue

> We thought it was time indeed. This is why we had taken the time
> to create an I-D on "LDP IANA Considerations", and specify the
> Common Hello Parameters bit field IANA registry at 
> http://tools.ietf.org/html/draft-ietf-mpls-ldp-iana-01#section-2.3.
> We also took the extra time to comb through all of RFC 5036 and
> add every missing one to this I-D, in hopes of preventing this issue
> in the future for LDP.
>
> However, Adrian Farrel observed that 
> http://www.ietf.org/mail-archive/web/mpls/current/msg07321.html

Who he? You don't want to pay any attention to what he says.

> So that is not a bit field, really, it is an opaque semanticless

You're right.
To be slightly fair, in that email, the mysterious author did offer the option
of updating 5036 to change the reserved fields into bit fields. The main thrust
of the mail was that you cannot use IANA to police a reserved field.

But anyway, I withdraw the comment :-)

And thanks for all your work onthe text. I will issue the IETF last call.

Adrian

---

From: "Carlos Pignataro (cpignata)" <[email-masked]>
Date: Tue, 15 May 2012 06:57:07 -0400

Thank you, Adrian!

Thumb typed by Carlos Pignataro.
Excuze typofraphicak errows

On May 15, 2012, at 4:57 AM, "Adrian Farrel" <[email-masked]> wrote:

>> Common Hello Parameters bit field IANA registry atÂ