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

Alvaro Retana <aretana.ietf@gmail.com> Fri, 16 November 2018 21:59 UTC

Return-Path: <aretana.ietf@gmail.com>
X-Original-To: manet@ietfa.amsl.com
Delivered-To: manet@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 7CC4412D4EE; Fri, 16 Nov 2018 13:59:46 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: 3.002
X-Spam-Level: ***
X-Spam-Status: No, score=3.002 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, FREEMAIL_FROM=0.001, GB_SUMOF=5, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_NONE=-0.0001, SPF_PASS=-0.001, UNPARSEABLE_RELAY=0.001] autolearn=no autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=gmail.com
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id zknWFuArVAl8; Fri, 16 Nov 2018 13:59:44 -0800 (PST)
Received: from mail-oi1-x22f.google.com (mail-oi1-x22f.google.com [IPv6:2607:f8b0:4864:20::22f]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id EE630128D09; Fri, 16 Nov 2018 13:59:40 -0800 (PST)
Received: by mail-oi1-x22f.google.com with SMTP id r127-v6so20879605oie.3; Fri, 16 Nov 2018 13:59:40 -0800 (PST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:mime-version:date:message-id:subject:to:cc; bh=HUTXKRMljCQGzjnnqBMtM4kVB+kG7oMdET1xiMLHzPo=; b=ijfvqkq805L1L52JLrLHL1kPK2E6gTeRWg3tV5X5RTtyRkc+dwR0x4WG8iYeEM/qeM rSnRXVEdAE24zhnne6HoiealYVunOTrP2DaYbpgkyZjjUsHviXhkrTRSmbmraWEnondN e0OQFgz81CZoUL3ANQR1QySAS88utl3T4dxg4GBR2ALzqQysTjSCktm1XAlwHMske9Hk 1nOXG7AepzgN0r4SO0sSezolqmypDIaIn6Zt/EIRKzcuIYmVgbeBPixv/F8X3nhXPuob EMoDXjTI+1ep+az/iHByZQ9vRp4gY6lUSVrKOrIdzkyJOO2WIVTQWTqXZgNztJhJbn41 jcQg==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:mime-version:date:message-id:subject:to:cc; bh=HUTXKRMljCQGzjnnqBMtM4kVB+kG7oMdET1xiMLHzPo=; b=W3ruB4F1Z448ELHrbH/PqKPYvksHlN804FgM1owRssG4IMFeU2kFTTCFzWm2QFtjvT xMcTAy8LImcEwIxYsLLHh+gnKvRoJNyHpalhOerbGNfk4uegAcSNCINd7FrRfP0mf5KG Wv8CtAvK0ZcKC20no3k11XcH2rmPwYZo6YJbvWIPcn33xO3HNrlRoptUAr31KSBIfgcv QiOiXBXJsF2gQUpeG0HIN5W6B/GLTDacJv9vNneCc9dEPDCU4JoSaRrFDXNWFSB7Ge6c X9XRLXWhXsuDQTeO9r5xJdAONhfbT4Vp6rywmpzg1S0suCQczE7gqmnVtcpFJxNzxiJC vwYA==
X-Gm-Message-State: AGRZ1gIGXwIeIWRffsFfsluQ6uC437L1qkJuY6U6Nra/5G+GNQyptTwP P9ep3MKISqFE0Bybs8aUuI0N0cI1oBgOYGg8cYK9qDVw
X-Google-Smtp-Source: AJdET5c7YfMS4p8XRSG5rXFFCTm9BDt8PncTVLS/ASP/6mZyY3RybVSG2YdiaR/z/th+TKCrjJEguQ1zYXzE+MakhTs=
X-Received: by 2002:aca:a881:: with SMTP id r123mr1945742oie.207.1542405579892; Fri, 16 Nov 2018 13:59:39 -0800 (PST)
Received: from 1058052472880 named unknown by gmailapi.google.com with HTTPREST; Fri, 16 Nov 2018 16:59:39 -0500
From: Alvaro Retana <aretana.ietf@gmail.com>
X-Mailer: Airmail (528)
MIME-Version: 1.0
Date: Fri, 16 Nov 2018 16:59:39 -0500
Message-ID: <CAMMESszBwRNbn2_gu6of-fKCToyHKafpnXUqPekOKqKnEz1Jbw@mail.gmail.com>
To: draft-ietf-manet-dlep-pause-extension@ietf.org
Cc: "Ratliff, Stanley" <sratliff@idirect.net>, Mobile Ad-hoc Networks Working Group <manet-chairs@ietf.org>, manet@ietf.org
Content-Type: multipart/alternative; boundary="000000000000b06d9c057acf48a9"
Archived-At: <https://mailarchive.ietf.org/arch/msg/manet/YMwOuhXt-D9-6eJA2B9B-3YkfZQ>
Subject: [manet] AD Review of draft-ietf-manet-dlep-pause-extension-04
X-BeenThere: manet@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Mobile Ad-hoc Networks <manet.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/manet>, <mailto:manet-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/manet/>
List-Post: <mailto:manet@ietf.org>
List-Help: <mailto:manet-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/manet>, <mailto:manet-request@ietf.org?subject=subscribe>
X-List-Received-Date: Fri, 16 Nov 2018 21:59:47 -0000

Dear authors:

I just finished reviewing this document.  I have several comments (see
below) and a couple of significant issues, which I'm mentioning in the next
2 bullets.  I will wait for these to be addressed before starting the IETF
LC.

(1) This is a comment for the WG (not just the authors):

This is the first draft that specifies a Sub Data Item.  Given that rfc8175
says that the Value in a Data Item "contains data specific to a particular
Data Item", I don't think it is necessary to define a generic DLEP Sub Data
Item.  However, not all future extensions may follow the same model.  Is
there a need/interest in normatively specifying the characteristics of a
Sub Data Item?

(2) Security Considerations:  The extension in itself doesn't change the
security characteristics of DLEP, I agree with that.  However, I think that
the functionality does -- please see specific comments below.

Thanks!

Alvaro.


[Line numbers from idnits.]

...
70 1.  Introduction
...
78   The base DLEP specification does not include any data plane flow
79   control capability.  Various flow control methods are possible, e.g.,
80   see [I-D.ietf-manet-credit-window].  The extension defined in this
...

[minor] A reference to something other than I-D.ietf-manet-credit-window
(expired, etc.) may be more appropriate.


...
103 2.  Extension Usage and Identification

105   The use of the Control Plane Based Pause Extension SHOULD be
106   configurable.  To indicate that the Control Plane Based Pause
107   Extension is to be used, an implementation MUST include the Control
108   Plane Based Pause Extension Type Value in the Extensions Supported
109   Data Item.  The Extensions Supported Data Item is sent and processed
110   according to [RFC8175].

[minor] "The use of the Control Plane Based Pause Extension SHOULD be
configurable."  Is there a default recommended configuration?


...
201 3.1.1.  Queue Parameter Sub Data Item
...
234   Sub Data Item Type:

236      A 16-bit unsigned integer that indicates the type and
237      corresponding format of the Sub Data Item's Value field.  Sub Data
238      Item Types are scoped within the Data Item in which they are
239      carried, i.e., the Sub Data Item Type field MUST be used together
240      with the Data Item Type to identify the format of the Sub Data
241      Item.  This field MUST be set to one (1) for the Queue Parameter
242      Sub Data Item.

[minor] "...the Sub Data Item Type field MUST be used together with the
Data Item Type to identify the format of the Sub Data Item."  It sounds as
if this text wants to specify the behavior for *any* Sub Data Item Type.
Please reword to be specific in to this document.  [Also, see my note at
the top.]

[major] Please define a registry for the Sub Data Item Types to be used
with the Queue Parameters Data Item.

244   Length:  Variable

246      Copying [RFC8175], Length is the number of octets in the sub data
247      item, excluding the Type and Length fields.

[minor] Copying?  As far as I can tell, rfc8175 doesn't define any sub data
items.

249   Queue Index:

251      An 8-bit field indicating the queue index of the queue parameter
252      represented in the sub data item.  Only the first instance a a
253      particular Queue Index value is meaningful.  Subsequent sub data
254      items containing the same Queue Index values, if present, MAY be
255      logged via a management interface and MUST otherwise be ignored.

[nit] s/a a/of a

[minor] Pause and Restart reserve the Queue Index of 255 to indicate "all
traffic", but that value is not reserved here.  Should it?


...
269   DS Field Qn:

271      The data item contains a sequence of 8 bit DS Fields.  The
272      position in the sequence identifies the associated queue index.
273      The number of DS Fields present should equal the sum of all Num
274      DSCPs field values.

[major] "should equal", or "MUST equal"?  If the number of DS Fields
doesn't add up, should the sub data item be considered invalid?  I assume
that would invalidate the whole data item.


...
286 3.2.  Pause
...
293   A modem may indicate that traffic is to be suppressed on a device
294   wide or destination specific basis.  An example of when a modem might
295   use device wide indications is when output queues are shared across
296   all destinations, and destination specific might be used when per
297   destination queuing is used.  To indicate that suppression applies to
298   all destinations, a modem MAY send the Pause Data Item in a Session
299   Update Message.  To indicate that suppression applies to a particular
300   destination a modem MAY send the Pause Data Item in a Destination
301   Update Message.

[major] I think that the two MAYs above are out of place.  I understand
that sending the Pause Data Item in the corresponding Update Message is
optional, but the sentences say that "To indicate that suppression
applies...", which means that if the Pause Data Item is not sent, then
there is no indication...so sending it is not optional.


...
312   A router which receives the Pause Data Item MUST cease sending the
313   identified traffic to the modem.  This may of course translate into
314   the router's queues exceeding their own thresholds.  If a received
315   Pause Data Item contains a Queue Index value other than 0, 255, or a
316   queue index established by a Session Initialization or Session Update
317   Message, the router MUST terminate the session with a Status Data
318   Item indicating Invalid Data.

[major] Terminating the session seems drastic to me given that the wrong
index relates to a single destination and the action has an impact over all.


...
347 3.3.  Restart
...
354   The sending of this data item parallels the Pause Data Item, see the
355   previous section, and follows the same rules.  This includes that to
356   indicate that transmission can resume to all destinations, a modem
357   MAY send the Restart Data Item in a Session Update Message.  It also
358   includes that to indicate that transmission can resume to a
359   particular destination a modem MAY send the Pause Restart Item in a
360   Destination Update Message.  Finally, the same rules apply to queue
361   indexes.

[major] Same comment as above about the MAYs.

363   A router which receives the Restart Data Item SHOULD resume
364   transmission of the identified traffic to the modem.

[minor] Should the Restart always follow a Pause?  I think that is the
intent.  The question is whether it MUST happen that way?  As is, the text
leaves the possibility open for a modem to send a Restart without having
sent a Pause.  I think that is ok, just checking...


...
384 4.  Security Considerations

386   The extension introduces a new mechanism for flow control between a
387   router and modem using the DLEP protocol.  The extension does not
388   inherently introduce any additional threats above those documented in
389   [RFC8175].  The approach taken to Security in that document applies
390   equally when running the extension defined in this document.

[major] The extension gives the modem the ability to stop the traffic sent
by a router.  A rogue or compromised modem could stop traffic sent by the
attached router.  Protecting the modem is out of scope, but I think the
threat should at least be pointed out.

rfc8175 already mentions the risk that "an attacker might pretend to be a
DLEP participant...by injection of DLEP Messages once a session has been
established", which is related to this case.  However, I think that the
point still needs to be called out specifically because of the new
functionality introduced.


...
466 Appendix A.  Acknowledgments

468   The sub data item format was inspired by Rick Taylor's "Data Item
469   Containers".

[nit] Is this a draft?  Reference?