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

[Alvaro Retana <[e-mail]>]

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.]

(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.

(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.

...
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.


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

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.

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.

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.

(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.

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.

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

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.

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?

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)?

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.

[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.]

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...

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?

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

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?).


[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.

[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...

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


...
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."

[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...


...
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.


...
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.