Re: [ippm] Benjamin Kaduk's No Objection on draft-ietf-ippm-twamp-yang-11: (with COMMENT)

[Benjamin Kaduk <[e-mail]>]

On Fri, Jun 22, 2018 at 10:46:25AM -0700, Mahesh Jethanandani wrote:
> Hi Ben,
> 
> > On Jun 22, 2018, at 8:15 AM, Benjamin Kaduk <[e-mail]> wrote:
> > 
> > On Wed, Jun 20, 2018 at 06:55:23PM -0700, Mahesh Jethanandani wrote:
> >> Hi Ben,
> >> 
> >>> On Jun 18, 2018, at 9:18 AM, Benjamin Kaduk <[e-mail]> wrote:
> >>> 
> >>> Section 3.4
> >>> 
> >>>  Each Session-Reflector is associated with zero or more TWAMP-Test
> >>>  sessions.  For each test session, the REFWAIT timeout parameter which
> >>>  determines whether to discontinue the session if no packets have been
> >>>  received (TWAMP [RFC5357], Section 4.2) can be configured.
> >>> 
> >>> nit: I think this would be easier to read if "which
> >>> determines...received" was offset by commas or parentheses.
> >> 
> >> How about this:
> >> 
> >> OLD
> >>   For each test session, the REFWAIT timeout parameter which
> >>   determines whether to discontinue the session if no packets have been
> >>   received (TWAMP [RFC5357], Section 4.2 <https://tools.ietf.org/html/rfc5357#section-4.2 <https://tools.ietf.org/html/rfc5357#section-4.2>>) can be configured.
> >> 
> >> 
> >> NEW
> >>   For each test session, the REFWAIT timeout parameter can be configured, which
> >>   determines whether to discontinue the session, if no packets have been
> >>   received (TWAMP [RFC5357], Section 4.2 <https://tools.ietf.org/html/rfc5357#section-4.2 <https://tools.ietf.org/html/rfc5357#section-4.2>>).
> > 
> > I'm not sure this is quite right -- the clause offset by commas starting
> > with "which" is supposed to be a "parenthetical", a clause adding some
> > additional information that is not necessary to understand the sentence.
> > That is, the sentence is still supposed to read fine if the entire clause
> > (and commas) are removed.  But "the REFWAIT timeout parameter can be
> > configured if no packets have been received" seems like a different meaning
> > than is intended.
> 
> Ok. How about?
> 
> OLD:
>    For each test session, the REFWAIT timeout parameter which
>    determines whether to discontinue the session if no packets have been
>    received (TWAMP [RFC5357], Section 4.2 <https://tools.ietf.org/html/rfc5357#section-4.2>) can be configured.
> 
> 
> NEW:
>    For each test session, the REFWAIT timeout parameter, which
>    determines whether to discontinue the session if no packets have been
>    received (TWAMP [RFC5357], Section 4.2 <https://tools.ietf.org/html/rfc5357#section-4.2>), can be configured.

That works for me.  (I bet the RFC Editor will think there are too many
commas, but will let them decide that for themselves.)

> 
> > 
> >> 
> > 
> >>> 
> >>> Section 4.2
> >>> 
> >>>  [...] The Server, being prepared to conduct
> >>>  sessions with more than one Control-Client, uses KeyIDs to choose the
> >>>  appropriate secret-key; a Control-Client would typically have
> >>>  different secret keys for different Servers. key-id tells the Server
> >>>  which shared-secret the Control-Client wishes to use for
> >>>  authentication or encryption.
> >>> 
> >>> Does this imply a global uniqueness requirement for key-ids?  If so,
> >>> that should be called out more clearly.
> >> 
> >> Section 4.1 says. Do you think this is clear enough?
> >> 
> >>   Both the Server and
> >>   the Control-Client use the same mappings from KeyIDs to shared
> >>   secrets (key-id and secret-key in Figure 3, respectively)
> > 
> > I'd prefer to have it also say explicitly that they must be unique, as in
> > something like
> > 
> > NEW:
> > 
> >    Both the Server and
> >    the Control-Client use the same mappings from KeyIDs to shared
> >    secrets (key-id and secret-key in Figure 3, respectively); in order
> >    for this to work properly, KeyIds must be unique across all systems in
> >    the administrative domain.
> > 
> 
> Ok. I have made some minor tweaks to the above suggested changes, mainly to consolidate all references of “KeyIDs" to "key-id" and “shared secret” and “shared-secret” to “secret-key” so they are consistent with the UML diagram and the box “key-chain”. It now appears as:
> 
> NEW: (In Section 4.1)
> Both the Server and the Control-Client use the same mappings from key-id to secret-key (in Figure 3); in order for this to work properly, key-id must be unique across all systems in the administrative domain.
> 
> NEW: (In Section 4.2)
> As mentioned in Section 4.1, both the Server and the Control-Client use the same mapping from key-id to shared secret-key; in order for this to work properly, key-id must be unique across all the systems in the administrative domain.
> 

Thank you!

> > 
> >>> 
> >>> Section 4.3
> >>> 
> >>>                           | name                      |
> >>>                           | ctrl-connection-name {ro} |
> >>>                           | fill-mode                 |
> >>>                           | number-of-packets         |
> >>>                           | state {ro}                |
> >>>                           | sent-packets         {ro} |
> >>>                           | rcv-packets          {ro} |
> >>>                           | last-sent-seq        {ro} |
> >>>                           | last-rcv-seq         {ro} |
> >>>                           +---------------------------+
> >>> 
> >>> nit: should the "{ro}" on "state" be right-aligned with the others?
> >> 
> >> Ok. 
> >> 
> >>> 
> >>> Is there any privacy concern about exposing the parent-connection
> >>> 4-tuple?
> >> 
> >> As discussed in the other thread, the test is run between the four logical entities within the same provider network. That combined with the security of the session, and use of access control (NACM) can limit how much is exposed and what they can do with that information.
> >> 
> >>> 
> >>> Section 5.2
> >>> 
> >>> In the 'count' leaf, a default value of 10 (corresponding to an
> >>> iteration count of 2^10 == 1024 for PBKDF2) is described.  This
> >>> seems quite low for a PBKDF2 iteration count, by modern standards.
> >>> In "normal" cryptographic protocols we would generally be using a
> >>> default closer to 32768 == 2^15 (which I see is the default *max*
> >>> count value, and there is additional discussion of the issue in the
> >>> leaf description for that leaf).  Perhaps one could make an argument
> >>> that this is just for test setups and the keys and data exchanged
> >>> are "not very valuable", but there is always risk of key sharing
> >>> across protocols, and my preference is to present the strong
> >>> defaults and give users the option to reduce where appropriate.
> >>> What are the authors' thoughts here?
> >> 
> >> We can bump the default for ‘count to be 15, and the default for ‘max-count-exponent’ to 20.
> > 
> > Thanks!
> > 
> >>> 
> >>> Section 7
> >>> 
> >>> There are probably more nodes that can get called out as
> >>> particularly vulnerable, such as the count and max-count nodes that
> >>> can cause a long time to be spent on PBKDF2 iterations, the dscp
> >>> markings, the mode bitmask, etc.
> >> 
> >> The question of more nodes being identified from a vulnerability perspective has been called out on the other thread. We can use the above discussion, and the point you make below to call out ‘count’ and ‘max-count-exponent’. What do you see as issue with different DSCP markings or with mode bit mask?
> > 
> > If I understand correctly, different DCSP markings could affect the
> > processing of the test traffic on the network (skewing the results), and
> > changing the mode mask could result in big changes, like using
> > unauthenticated (and MITM-able) test traffic instead of authenticated
> > traffic.
> 
> Ok. Will add text around it.

Great; thanks!

-Benjamin