Re: [manet] AD Review of draft-ietf-manet-dlep-lid-extension-04

[Rick Taylor <[e-mail]>]

Hi Alvaro,

Comments inline, but a little forward:

We have done a rewrite of most of the descriptive text, focussing on
the use-cases, and attempting to clearly define terminology to aid the
reader.

On Wed, 2018-11-21 at 09:00 -0800, Alvaro Retana wrote:
> Dear authors:
> 
> I just finished reading this document -- thanks for the work!
> 
> I have some significant issues with the document, not the
> extension/functionality itself, but the text.  Please also take a
> look at my comments in the text.
> 
> (1) What problem are you trying to solve?  After reading the
> Abstract/Introduction it was not completely clear to me what is the
> problem -- I looked back at the meeting slides and the mailing list
> discussion to get a better picture.  I think I now have an
> understanding -- in general, I think the document could benefit form
> a little more content explaining the use cases.  [See more below.]

Hopefully covered in the rewritten text.

> 
> (2) Specificity of the specification.  Vague descriptions are used
> throughout the text, even associated to Normative language!  Examples
> include: "the last reachable node", it "might be the address", "some
> kind of backbone infrastructure", "some kind of sleight-of-hand"... 
> This is a Standards Track document, please be specific and clear.

Again hopefully fixed in the rewritten verbiage.

> 
> (3) Terminate-resulting Errors and Security.  Because of how the
> operation is specified (for example, requiring "the Link Identifier
> Data Items referring to a new link [to] first appear in a DLEP
> Destination Up Message from the modem to the router"), there seem to
> be several opportunities for a rogue/compromised modem/router to
> terminate the DLEP session.  Please call these cases out in the
> Security Considerations section as potential risks.  The Shepherd's
> writeup mentions one implementation, so it is probably too late to
> change the operation to minimize the risk.  [I made some comments
> bellow pointing at items that I think should be mentioned as a risk.]
> 
> I will start the IETF Last Call when these issues have been
> addressed.
> 
> Thanks!
> 
> Alvaro.
> 
> 
> [Line numbers come from idnits.]
> 
> 
> ...
> 11	Abstract
> 
> 13	   There exists a class of modems that would benefit from
> supporting the
> 14	   Dynamic Link Exchange Protocol (DLEP) [RFC8175] but do not
> present a
> 15	   single Layer 2 network domain as required by DLEP.  Such
> devices may
> 16	   be:
> 
> [nit] Don't include references in the Abstract.

I don't know how to avoid this - can you suggest some text please?

> 
> ...
> 34	   Note:
> 
> 36	   o  This document is intended as an extension to the core
> DLEP
> 37	      specification, and readers are expected to be fully
> conversant
> 38	      with the operation of core DLEP.
> 
> [minor] This note is not needed (specially in the Abstract) -- making
> DLEP a Normative Reference is enough.

Gone.

> 
> 
> ...
> 89	1.  Introduction
> ...
> [nit] Some of the sentences are very long...a couple of commas here
> and there would not hurt.

Commas liberally sprinkled.


> 
> 108	   A Layer 3 destination may be an attached DLEP router, in the
> case of
> 109	   a modem that provides Layer 3 wide area network connectivity
> between
> 110	   devices, or a logical destination that describes a set of
> attached
> 111	   subnets, when referring to some upstream backbone network
> 112	   infrastructure.
> 
> [minor] To be honest, it took me several reads of the
> Abstract/Introduction for it to make sense to me -- I looked at the
> slides and read the mailing list as well.  I'm not sure that it will
> be clear to other readers (e.g. directorate reviewers, IESG). 
> Consider expanding on the use cases.
> 
> I'm missing how the reference to "some upstream backbone network
> infrastructure" comes into play here.  It sounds like you want to
> advertise non-directly-attached destination information, but it is
> not clear to me if the modem has a DLEP session with those remote
> nodes or not.  Among other things, understanding this is important
> because of the Security Considerations..
> 
> I think I now have a good mental picture, but the document should
> clearly explain as well.

Hopefully we have addressed this by introducing a terminology section,
and removing cases of "some kind of..." and attempting to be much more
explicit.

> 
> 114	1.1.  Requirements
> 
> 116	   The key words "MUST", "MUST NOT", "REQUIRED", "SHALL",
> "SHALL NOT",
> 117	   "SHOULD", "SHOULD NOT", "RECOMMENDED", "MAY", and "OPTIONAL"
> in this
> 118	   document are to be interpreted as described in BCP 14, RFC
> 2119.
> 
> [major] Please use the template from rfc8174.

Done

> 
> 120	2.  Operation
> 
> 122	   To refer to a Layer 3 DLEP Destination, the DLEP session
> participant
> 123	   adds a Link Identifier Data Item (Section 3.2) to the
> relevant
> 124	   Destination Message, and (as usual) includes a MAC Address
> Data Item.
> 125	   When paired with a Link Identifier Data Item, the MAC
> Address Data
> 126	   Item MUST contain the MAC address of the last reachable node
> in the
> 127	   Layer 2 domain beyond which the Layer 3 DLEP Destination
> resides.
> 128	   For example, if the over-the-air network is not a single
> Layer 2
> 129	   domain, the MAC Address Data Item might be the address of
> the LAN-
> 130	   side interface of the local modem.  Alternatively, when used
> with
> 131	   some kind of backbone infrastructure, the MAC Address Data
> Item would
> 132	   be the address of the last device reachable on the local
> Layer 2
> 133	   domain.  However, how such remote destinations are
> discovered is
> 134	   beyond the scope of this specification.
> 
> [major] I think that the specification has to be more specific:
> 
> (1) "the last reachable node" -- the first example seems clear, even
> though the text points at a MAC address that "*might*" be it.  Not a
> warm a fuzzy feeling.

Now called the "Gateway Node" because it's a gateway, and (hopefully)
more tightly defined.

> 
> (2) "some kind of backbone infrastructure, the MAC Address Data Item
> would be the address of the last device reachable on the local Layer
> 2 domain" -- "*some kind*" is not clear.  Even though the discovery
> of the "last reachable node" is out of scope, it is important to know
> which node we're talking about!!  It has to be crystal clear because
> of the MUST above.

This text has gone.

> 
> 136	   As only modems are initially aware of Layer 3 DLEP
> Destinations, Link
> 137	   Identifier Data Items referring to a new link MUST first
> appear in a
> 138	   DLEP Destination Up Message from the modem to the router. 
> Once a
> 139	   link has been identified in this way, Link Identifier Data
> Items MAY
> 140	   be used by either DLEP participant during the lifetime of a
> DLEP
> 141	   session.  Because of this, a router MUST NOT send a DLEP
> Destination
> 142	   Announce Message containing a Link Identifier Data Item
> referring to
> 143	   a link that has not been mentioned in a prior DLEP
> Destination Up
> 144	   Message.
> 
> [major] What if the Link Identifier Data Items referring to a new
> link don't first appear in a DLEP Destination Up Message from the
> modem to the router?  It seems to me that this case should result in
> an "Invalid Data" status, right?  If so, then I think it is important
> to call out as a risk.

We now call out the exact Termination behaviour.

> 
> [major] s/MAY/may  It is not specifying anything...just pointing out
> a fact.

Fixed.

> 
> 146	   Because the MAC Address associated with any DLEP Destination
> Message
> 147	   containing a Link Identifier Data Item is not the Layer 2
> address of
> 148	   the destination, all DLEP Destination Up Messages MUST
> contain Layer
> 149	   3 information.  In the case of modems that provide Layer 3
> wide area
> 150	   network connectivity between devices, this means one or more
> IPv4 or
> 151	   IPv6 Address Data Items providing the Layer 3 address of the
> 152	   destination.  When referring to some upstream backbone
> network
> 153	   infrastructure, this means one or more IPv4 or IPv6 Attached
> Subnet
> 154	   Data Items, for example: '0.0.0.0/0' or '::/0'.  This allows
> the DLEP
> 155	   peer router to understand the properties of the link to
> those routes.
> 
> 157	   When the DLEP peer router wishes to forward packets to the
> Layer 3
> 158	   destination or subnet, the MAC address associated with the
> link MUST
> 159	   be used as the Layer 2 destination of the packet if it
> wishes to use
> 160	   the modem network to forward the packet.
> 
> [minor] Is this MAC address the same as the one in the MAC Address
> Data Item from "the last reachable node"?  If so, then this seems to
> be a much better explanation than what was included above.

The text has been moved/reused earlier to improve clarity.

> 
> 162	   As most mainstream routers expect to populate their routing
> 163	   information base with the IP address of the next router
> towards a
> 164	   destination, implementations supporting this extension
> SHOULD
> 165	   announce one or more valid IPv4 or IPv6 addresses of the
> last
> 166	   reachable Layer 2 device, i.e. the device with the
> corresponding MAC
> 167	   Address.
> 
> [major] Why use SHOULD and not MUST?  What is the
> advantage/disadvantage of advertising more than one address?

It's still a SHOULD, because one can get round not having it, however,
we have explicitly called out the advantage of including it.

> 
> 169	   If the last reachable Layer 2 device is not the DLEP peer
> modem, then
> 170	   the modem SHOULD announce a DLEP Destination with the
> required MAC
> 171	   Address without including a Link Identifier Data Item.
> 
> [major] Isn't that what is already included in the MAC Address Data
> Item at the beginning of this section (but advertised *with* the Link
> Identifier Data Item)?

Yes, this paragraph was confusing and didn't add anything of value, so
it's gone.


> 
> 173	2.1.  Identifier Restrictions
> 
> 175	   A Link Identifier is by default 4 octets in length.  If a
> modem
> 176	   wishes to use a Link Identifier of a different length, it
> MUST be
> 177	   announced using the Link Identifier Length Data Item
> (Section 3.1)
> 178	   contained in the DLEP Session Initialization Response
> message sent by
> 179	   the modem to the router.
> 
> 181	   During the lifetime of a DLEP session, the length of Link
> Identifiers
> 182	   MUST remain constant, i.e. the Length field of the Link
> Identifier
> 183	   Data Item MUST NOT differ between destinations.
> 
> [major] This is another case where the session could be terminated if
> the wrong length is used...  Call out as a risk.

Damn.. I missed this one.  I shall have to uprev with specific
Termination rules.

> 
> [minor] It seems to me that the intended Link Identifier Length could
> have also been derived form the first Link Identifier Data Item
> advertised by the modem.  Why is the extra Data Item required?  [It
> may be too late to change anything, I'm mostly wondering why the
> extra moving parts.]

Aaah, because some members of the WG wanted it (they want to
preallocate and/or assume 4 unless explicitly told otherwise).

> 
> 185	   The method for generating Link Identifiers is a modem
> implementation
> 186	   matter and out of scope of this document.  Routers MUST NOT
> make any
> 187	   assumptions about the meaning of Link Identifiers, or how
> Link
> 188	   Identifiers are generated.
> 
> [major] s/MUST NOT/must not   There's no specification there...

Fixed

> 
> 190	   Within a single DLEP session, all Link Identifiers MUST be
> unique per
> 191	   MAC Address.  This means that a Layer 3 DLEP Destination is
> uniquely
> 192	   identified by the pair: {MAC Address,Link Identifier}.
> 
> 194	   Link Identifiers MUST NOT be reused, i.e. a {MAC
> Address,Link
> 195	   Identifier} pair that has been used to refer to one DLEP
> Destination
> 196	   MUST NOT be recycled to refer to a different destination
> within the
> 197	   lifetime of a single DLEP session.
> 
> [minor] Aren't these last 2 paragraphs redundant?

Possibly, but I have been asked to clarify these points, so I'd prefer
to leave them in.

> 
> 199	2.2.  Negotiation
> 
> 201	   To use this extension, as with all DLEP extensions, the
> extension
> 202	   MUST be announced during DLEP session initialization.  A
> router
> 203	   advertises support by including the value 'Link Identifiers'
> (TBD1),
> 204	   Section 5, in the Extension Data Item within the Session
> 205	   Initialization Message.  A modem advertises support by
> including the
> 206	   value 'Link Identifiers' (TBD1) in the Extension Data Item
> within the
> 207	   Session Initialization Response Message.  If both DLEP peers
> 208	   advertise support for this extension then the Link
> Identifier Data
> 209	   Item MAY be used.
> 
> [major] s/MAY/may

Fixed to 'can be included'

> 
> 211	   If a modem requires support for this extension in order to
> describe
> 212	   destinations, and the router does not advertise support,
> then the
> 213	   modem MUST NOT include a Link Identifier Data Item in any
> DLEP
> 214	   Message.  However, the modem SHOULD NOT immediately
> terminate the
> 215	   DLEP session, rather it SHOULD use session-wide DLEP Data
> Items to
> 216	   announce general information about all reachable
> destinations via the
> 217	   modem.  By doing this, a modem allows a router not
> supporting this
> 218	   extension to at least make a best guess at the state of any
> reachable
> 219	   network.  A modem MUST NOT attempt to re-use the MAC Address
> Data
> 220	   Item to perform some kind of sleight-of-hand, assuming that
> the
> 221	   router will notice the DLEP Peer Type of the modem is
> special in some
> 222	   way.
> 
> [major] "SHOULD NOT immediately terminate"   But it may do it later? 
> Are there cases where it would/should?  Why not MUST NOT instead of
> SHOULD NOT?
> 
> There seems to be no reason for the session to be terminated -- yes,
> if the modem can't communicate what it need to, then there's no point
> in having the session...but that is not a reason to reset (or is
> it?).

You're right, MUST works fine here, but we were trying to recommend
good citizenship rather than enforce something that breaks the
protocol.

> 
> 
> [minor] What do you mean by "session-wide DLEP Data Items"?  From
> rfc8175 it looks like you mean Data Items in a Session Update
> Message.  Please be more specific.  In fact, it would be very nice if
> you expanded in how to do it.

We have expanded to be more specific.

> 
> [minor] "make a best guess"  It seems to me that the difference
> between using the new procedure defined in this document, and, simply
> using the Session Update Message is that the new functionality
> explicitly indicates that the destination is remote (vs giving the
> appearance that the destinations are attached to the router), any
> maybe being able to use different metrics.  Is that a correct
> interpretation?  IOW, it is not really be a guess...

The "best guess" text has been reworded.

> 
> [major] "MUST NOT...perform some kind of sleight-of-hand"  This is
> the first time that I see magic normatively prohibited.  :-(

Magic has been purged (the whole paragraph covering stopping people 
doing dumb things via a backdoor is unenforceable anyway)

> 
> 
> ...
> 232	3.1.  Link Identifier Length Data Item
> 
> 234	   The Link Identifier Length Data Item is used by a DLEP modem
> 235	   implementation to specify the length of Link Identifier Data
> Items.
> 236	   It MUST be used if the specified length is not the default
> value of 4
> 237	   octets.
> 
> 239	   The Link Identifier Length Data Item MAY be used during
> Session
> 240	   Initialization, contained in a Session Initialization
> Response
> 241	   Message.
> 
> [minor] Perhaps reword to avoid an apparent Normative contradiction
> (MUST vs MAY)...for example: "It MUST be used during Session
> Initialization, contained in a Session Initialization Response
> Message, if the specified length is not the default value of 4
> octets."

Thank you for the text!

> 
> [major] If this Data Item is used (during Session Initialization,
> contained in a Session Initialization Response Message), but it
> indicates a Link Identifier Length of 4...what should happen?  Should
> it be considered Invalid Data or maybe an Unexpected Message, or ?? 
> Specifying that it "MUST be used if the specified length is not the
> default value of 4 octets" seems to indicate that it should't be used
> otherwise... Maybe another risk...

There is now a clarifying paragraph stating that announcing a 4 octet
length is valid.

> 
> 
> ...
> 281	4.  Security Considerations
> 
> 283	   As an extension to the core DLEP protocol, the security
> 284	   considerations of that protocol apply to this extension. 
> This
> 285	   extension adds no additional security mechanisms or
> features.
> 
> 287	   None of the features introduced by this extension require
> extra
> 288	   consideration by an implementation.
> 
> [major] I think that the functionality in this extension may result
> in Invalid Data (and a terminated session) -- see the comments in
> §2/2.1 above.  While this case may only be the result of a rogue
> modem/router, and rfc8175 already says something general about that,
> it is important to point it out here because the
> functionality/operation is new.
> 

Having called out the additional reasons for termination, I don't see
that any of them cause any additional security considerations beyond
normal DLEP.  The consideration is: deviate from the protocol and your
session will terminate - that's covered by DLEP.



> 
> ...
> 318	6.2.  Informative References
> 
> 320	   [RFC5226]  Narten, T. and H. Alvestrand, "Guidelines for
> Writing an
> 321	              IANA Considerations Section in RFCs", RFC 5226,
> 322	              DOI 10.17487/RFC5226, May 2008,
> 323	              <https://www.rfc-editor.org/info/rfc5226>;.
> 
> [minor] There's no reference to rfc5226 in the text.

Fixed!

Hope that helps?

Rick