Re: [manet] Benjamin Kaduk's Discuss on draft-ietf-manet-dlep-pause-extension-06: (with DISCUSS and COMMENT)

[Benjamin Kaduk <[e-mail]>]

Hi Lou,

Sorry for the long delay here; I'm in the process of moving from St. Louis
to Seattle and have had to take a bunch of time off in the process.

On Tue, May 07, 2019 at 10:34:57AM -0400, Lou Berger wrote:
> Please see responses below...
> 
> On 5/6/2019 6:40 PM, Benjamin Kaduk wrote:
> > On Sun, May 05, 2019 at 09:22:16PM -0400, Lou Berger wrote:
> >> Hi,
> >> 	My apologies, I thought I responded to this, but can't seem to find the
> >> response (hopefully my responses are consistent if I did!).
> >>
> >> On 4/10/19 9:00 PM, Benjamin Kaduk via Datatracker wrote:
> >>> Benjamin Kaduk has entered the following ballot position for
> >>> draft-ietf-manet-dlep-pause-extension-06: Discuss
> >>>
> >>> When responding, please keep the subject line intact and reply to all
> >>> email addresses included in the To and CC lines. (Feel free to cut this
> >>> introductory paragraph, however.)
> >>>
> >>>
> >>> Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
> >>> for more information about IESG DISCUSS and COMMENT positions.
> >>>
> >>>
> >>> The document, along with other ballot positions, can be found here:
> >>> https://datatracker.ietf.org/doc/draft-ietf-manet-dlep-pause-extension/
> >>>
> >>>
> >>>
> >>> ----------------------------------------------------------------------
> >>> DISCUSS:
> >>> ----------------------------------------------------------------------
> >>>
> >>> Section 3.1 defines 'Scale' as a four-bit unsigned integer field but only the
> >>> values 0-3 are assigned, leaving 4-16k usable.  How will future values be
> >>> assigned?
> >>>
> >> A future revision or update of this document.  We discussed a IANA
> >> registry, but thought it overkill as future additions seem quite unlikely.
> > Okay.  The Discuss was just to make sure it was considered in some fashion;
> > we do have a bunch of documents that explictly note "additional values are
> > to be allocated by updates to this specification" but I do not insist on
> > it.
> >
> >>> In Section 3.1.1:
> >>>
> >>>     DS Field Qn:
> >>>
> >>>        The data item contains a sequence of 8 bit DS Fields.  The number
> >>>        of DS Fields present MUST equal the sum of all Num DSCPs field
> >>>        values.
> >>>
> >>> Perhaps I'm misreading, but I thought the Num DSCPs Qn field was global
> >>> for this queue index, so there would just be one of them and no need to
> >>> take a sum.
> >> It's just saying that more than one DSCP can match to a single queue
> >> index value.
> > Okay, if I'm looking at the figure properly then this is just an editorial
> > issue, then:
> >
> >          0                   1                   2                   3
> >          0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
> >         +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> >         |  Queue Index  |             Queue Size Qn                     |
> >         +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> >         | Num DSCPs Qn  |  DS Field Qn  |              ...              :
> >         +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> >         :                          ...                  |  DS Field Qn  |
> >         +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> >
> > So we have the chunk of stuff for a given queue (index), and the size of
> > that queue, then a separate array/length for the DS values.  The various DS
> > values that can match this queue index are the contents of this array, but
> > this is the only array in the sub data item describing this queue index.
> > If that's correct, then isn't the description of "DS Field Qn" just "The
> > sub data item contains a sequence of 8 bit DS Fields for this queue index.
> > The number of DS Fields present MUST equal this value."?
> 
> Yes - I'm fine with a specific text revision if you have one.

How about:

OLD:

      The data item contains a sequence of 8 bit DS Fields.  The number
      of DS Fields present MUST equal the sum of all Num DSCPs field
      values.

NEW:

      The data item contains a sequence of 8 bit DS Fields.  The number
      of DS Fields present MUST equal the value of the Num DSCPs field.

> 
> >
> >>> In Section 3.3:
> >>>
> >>>     A router which receives the Restart Data Item SHOULD resume
> >>>     transmission of the identified traffic to the modem.
> >>>
> >>> Why is this only a "SHOULD"?
> >> to cover the case where the router is choosing not to send data, e.g.,
> >> due to lack of available data or other policy such as traffic rate shaping.
> > Okay.  Frequently documents will give the reader some hint as to what the
> > exceptional cases might be, but I will not insist on it here.
> >
> >
> >>> Section 4
> >>>
> >>>                                   The extension does not inherently
> >>>     introduce any additional vulnerabilities above those documented in
> >>>     [RFC8175].  [...]
> >>>
> >>> As noted by others, this sentence is just not true.
> >>> I will not duplicate the suggestions for additional considerations that
> >>> need to be documented.
> >> Please see comments / discussion elsewhere on this.
> > Sorry, most of these threads have aged out of my cache; could you give me a
> > hint about whose ballot thread to look in, or a link?
> 
> Perhaps it's just best to point you at the current text and see if 
> you're okay with it -- see
> 
> https://tools.ietf.org/html/draft-ietf-manet-dlep-pause-extension-07#section-4
> 
> and let me/us know if you still have any unresolved comments.

My main issue is with "does not inherently introduce any additional
vunlerabilities" -- there's a new signal that if received by the router,
causes all traffic to stop.  There is some inherent risk of such a signal
being received erroneously, even if an attacker that could introduce such a
signal would need to be highly capable (and thus also capable of doing
damage in other ways), and of course the risk of random accidents is always
present.

So I think the intent here may be to say instead that "this extension does
not introduce any vulnerabilities that are inherently different from those
documented in [RFC8175]", since there are already other ways that an
attacker could cause the router to stop sending traffic to the modem.

> 
> >>> In particular, I agree with Roman that the use
> >>> of TLS needs to be mandatory, especially since this protocol is
> >>> nominally only defined for use with radio links.
> >>
> >> As covered elsewhere, in particular this is really a comment on base
> >> DLEP and I think this extension should be consistent with that document.
> >> (Consider an example from when 8175 was issued, consider the case where
> >> the modem and router use MACsec, 802.1X and 802.1 AE, why is TLS needed
> >> as well.)
> > Perhaps.  I think it's highly dependent on whether DLEP is only conveyed
> > over wired links and the physical scope of those links, though -- my
> > understanding is that most of the IESG was treating this protocol as
> > running over the radio link.
> 
> umm, DLEP is *NEVER* expected to run over the air.  This is covered in 
> RFC8175 and is not impacted by this extension.

I'm sorry that I (and the collective IESG) missed that; you are quite
correct.  (I think I was under severe time pressure reviewing this document
and only did targetted keyword searches in 8175 as I was reading.)

-Ben