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

[Lou Berger <[e-mail]>]

Hi,

No worries on the delay.

On 6/6/2019 2:22 PM, Benjamin Kaduk wrote:
> 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 tohttps://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.
yes, this is better.
>>>>> 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.

This works for me too.

I'll push a new version in a few moments.

Thanks,

Lou

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