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

From: Benjamin Kaduk <[email-masked]>
Date: Mon, 18 Jun 2018 09:18:14 -0700

Benjamin Kaduk has entered the following ballot position for
draft-ietf-ippm-twamp-yang-11: No Objection

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-ippm-twamp-yang/

----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

Perhaps I am confused and/or misreading things, but the descriptions
of the control-client and session-reflector include discussion of
'sid' session identifiers as if they were always used, but the mode
bitmap includes a separate bit for negotiation of
'individual-session-control' for session identifier usage.  Is there
some conflict between this mandatory/negotiable distinction, or are
they actually talking about different things?

Comments below in document order, but please pay special note to the
(potential) need for global uniqueness of key-ids, the PBKDF2
iteration count, and the list of sensitive nodes to call out in the
security considerations.

Section 3.1

   o  Authentication and encryption attributes such as KeyID, Token and
      the Client Initialization Vector (Client-IV); see also the last
      paragraph of Section 6 in OWAMP [RFC4656] and Randomness
      Requirements for Security [RFC4086].

I'm confused about what the RFC4656 reference is intended to call
out -- the reliance on AES to be resistant to chosen plaintext, or
the randomly generated challenge from the server, or the existential
forgeries?

   o  Information pertaining to the test packet stream, such as the test
      starting time, which performance metric is to be used Registry for
      Performance Metrics [I-D.ietf-ippm-metric-registry], or whether
      the test should be repeated.

Is there something missing before or around "Registry for
Performance Metrics"?  The current text is hard to read.

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.

   Read-only access to other data model parameters, such as the Sender
   IP address is foreseen.  Each test session can be uniquely identified
   by the 4-tuple mentioned in Section 3.2.

Nit: comma after "Sender IP address".

Section 4.1

   [...] Specifically, mode-preference-chain lists the
   mode and its corresponding priority, expressed as a 16-bit unsigned
   integer, where zero is the highest priority and subsequent integers
   increase by one.

I thought I remembered some discussion about this text being unclear
and removing "and subsequent integers increase by one" being
proposed.  But I don't see that discussion in an obvious place, so
maybe it was on a different document.

   Note that the list of preferred Modes may set bit position
   combinations when necessary, such as when referring to the extended
   [...]

Maybe "may set multiple bits independently" would be more clear?
But it seems that some bit combinations don't make any sense, like
unauthenticated+authenticated -- is there need for more expository
text here?

   [...] The secret-key is the shared secret, a sequence
   of octets of arbitrary length whose interpretation is unspecified.
   The key-id and secret-key encoding SHOULD follow Section 9.4 of YANG
   [RFC7950]. [...]

Section 9.4 of YANG is for (printable) strings, but the secret-key
is binary -- should this get a Section 9.8 reference as well?
I'm also not sure that leaving it as "arbitrary length"
is great -- if we're using it to derive 16-byte AES keys and 32-byte
HMAC-SHA1 keys, we could at least say "SHOULD contain at least 128
bits of entropy".

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

Is there any privacy concern about exposing the parent-connection
4-tuple?

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?

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.

Appendix A

The <secret-key> elements appear to be using base64-encoded values.
Where is it specified that such encoding is used for the binary
values?  (I assume this is just my ignorance of a generic standard,
so please enlighten me!)

Am I reading it right that the <count>30</count> means 2^30 (one
billion) PBKDF2 iterations?  Has this actually been run in practice?
It seems like it would be painfully slow.

---

From: "MORTON, ALFRED C (AL)" <[email-masked]>
Date: Tue, 19 Jun 2018 20:06:55 +0000

Hi Ben,

[acm] 

Yes, there is a small amount of confusion, easily 
rectified I think.

All test sessions are assigned Session IDs (SIDs).
Each SID is assigned during the session setup phase of operation.

All test sessions START and STOP with the those commands,
*unless* the individual-session-control feature has been negotiated:

   [RFC5938] ... describes an OPTIONAL feature for TWAMP.  [RFC5357] TWAMP
   (and OWAMP) start all previously requested and accepted test sessions
   at once.  This feature allows the Control-Client to control
   individual test sessions on the basis of their Session Identifier
   (SID).  This feature permits a short-duration TWAMP test to start
   (and/or stop) during a longer test.  This feature permits a specific
   diagnostic test to begin if intermediate results indicate that the
   test is warranted, for example.

Let us know if there is specific text you would like to modify,
to avoid any confusion...

thanks,
Al

---

From: Benjamin Kaduk <[email-masked]>
Date: Wed, 20 Jun 2018 14:19:25 -0500

Hi Al,

> to avoid any confusion...

Thanks for the explanation.  I don't see any need to modify the text;
someone who actually pulls the references has a clear picture of what's
going on as-is.

-Benjamin

---

From: Mahesh Jethanandani <[email-masked]>
Date: Wed, 20 Jun 2018 18:55:23 -0700

Hi Ben,

> On Jun 18, 2018, at 9:18 AM, Benjamin Kaduk <[email-masked]> wrote:
> 

I believe Al has responded on this issue already.

> the randomly generated challenge from the server, or the existential

The reference should have been to Section 3.1 of RFC4656.

> Performance Metrics"?  The current text is hard to read.

Agree. How about this:

OLD
      Information pertaining to the test packet stream, such as the test
      starting time, which performance metric is to be used Registry for
      Performance Metrics [I-D.ietf-ippm-metric-registry <https://tools.ietf.org/html/draft-ietf-ippm-twamp-yang-11#ref-I-D.ietf-ippm-metric-registry>], or whether
      the test should be repeated.

NEW
      Information pertaining to the test packet stream, such as the test
      starting time, which performance metric is to be used, as defined in Registry for
      Performance Metrics [I-D.ietf-ippm-metric-registry <https://tools.ietf.org/html/draft-ietf-ippm-twamp-yang-11#ref-I-D.ietf-ippm-metric-registry>], or whether
      the test should be repeated.

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

> Nit: comma after "Sender IP address”.

Ok.

> maybe it was on a different document.

You do have a good memory. It was for this document. See the discussion on the other thread with Pete, where this has come up and edits proposed.

> unauthenticated+authenticated -- is there need for more expository

How about:

OLD:
   Note that the list of preferred Modes may set bit position
   combinations when necessary, such as when referring to the extended
   TWAMP features in [RFC5618 <https://tools.ietf.org/html/rfc5618>], [RFC5938 <https://tools.ietf.org/html/rfc5938>], [RFC6038 <https://tools.ietf.org/html/rfc6038>], and [RFC7717 <https://tools.ietf.org/html/rfc7717>].  If
   the Control-Client cannot determine an acceptable Mode, it MUST
   respond with zero Mode bits set in the Set-up Response message,
   indicating it will not continue with the control connection.

NEW:
   Note that the list of preferred Modes may set multiple bit position
   independently, such as when referring to the extended
   TWAMP features in [RFC5618 <https://tools.ietf.org/html/rfc5618>], [RFC5938 <https://tools.ietf.org/html/rfc5938>], [RFC6038 <https://tools.ietf.org/html/rfc6038>], and [RFC7717 <https://tools.ietf.org/html/rfc7717>].  If
   the Control-Client cannot determine an acceptable Mode, or when the 
   bit combinations do not make sense, e.g. both authenticated and 
   unauthenticated bit set, it MUST respond with zero Mode bits set 
   in the Set-up Response message, indicating it will not continue with 
   the control connection.

> bits of entropy”.

This was my bad. I changed the text for the secret-key from string to binary in the YANG model, at the behest of Tom Petch, but forgot to update this text. I can update the reference to Section 9.8 of RFC7950. Will add text around the 128 bits of entropy.

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

>                            | ctrl-connection-name {ro} |

Ok. 

> Is there any privacy concern about exposing the parent-connection

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.

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

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

> values?  (I assume this is just my ignorance of a generic standard,

RFC 7950 says in section 9.8.2 that:

   Binary values are encoded with the base64 encoding scheme (see
   Section 4 in [RFC4648] <https://tools.ietf.org/html/rfc4648#section-4>).

> It seems like it would be painfully slow.

It was setup as an example to show how a test session could be configured. We can reduce the value of count to 15.

Thanks.

Mahesh Jethanandani
[email-masked]

---

From: "t.petch" <[email-masked]>
Date: Thu, 21 Jun 2018 12:53:52 +0100

Benjamin

Top posting on one minor comment

You say
> values?  (I assume this is just my ignorance of a generic standard,

RFC7950 YANG 1.1

9.8.2.  Lexical Representation

   Section 4 in [RFC4648]).

Tom Petch

----- Original Message -----
From: "Benjamin Kaduk" <[email-masked]>
To: "The IESG" <[email-masked]>
Cc: <[email-masked]>; <[email-masked]>;
<[email-masked]>
Sent: Monday, June 18, 2018 5:18 PM

> Please refer to
https://www.ietf.org/iesg/statement/discuss-criteria.html
> https://datatracker.ietf.org/doc/draft-ietf-ippm-twamp-yang/

RFC7950 YANG 1.1

9.8.2.  Lexical Representation

   Section 4 in [RFC4648]).

> _______________________________________________
> ippm mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/ippm

---

From: Benjamin Kaduk <[email-masked]>
Date: Fri, 22 Jun 2018 10:15:01 -0500

> The reference should have been to Section 3.1 of RFC4656.

To be clear, to all of Section 3.1, not just the last paragraph?

>       the test should be repeated.

Much better!

>    received (TWAMP [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.

> You do have a good memory. It was for this document. See the discussion on the other thread with Pete, where this has come up and edits proposed.

I think I remember those going by and looking good.

>    in the Set-up Response message, indicating it will not continue with 

That looks good.  (But put a comma after "e.g." as well as before.)

> This was my bad. I changed the text for the secret-key from string to binary in the YANG model, at the behest of Tom Petch, but forgot to update this text. I can update the reference to Section 9.8…

Ah, okay.  (I had originally put in a lot more text about this before I
noticed that he rest of the document had the key as binary, and it was why
I was looking so closely at the base64 encoding later on.)

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

> We can bump the default for ‘count to be 15, and the default for ‘max-count-exponent’ to 20.

Thanks!

> 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 ‘coun…

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.

> It was setup as an example to show how a test session could be configured. We can reduce the value of count to 15.

Thanks!

-Benjamin

---

From: Mahesh Jethanandani <[email-masked]>
Date: Fri, 22 Jun 2018 10:46:25 -0700

Hi Ben,

> On Jun 22, 2018, at 8:15 AM, Benjamin Kaduk <[email-masked]> wrote:
> 

That is correct.

>>      Performance Metrics [I-D.ietf-ippm-metric-registry <https://tools.ietf.org/html/draft-ietf-ippm-twamp-yang-11#ref-I-D.ietf-ippm-metric-registry <https://tools.ietf.org/html/draft-ietf-ippm-twamp-yang-11#ref-I-D.ietf-ippm-metric-registry>>], or whether
>>      Performance Metrics [I-D.ietf-ippm-metric-registry <https://tools.ietf.org/html/draft-ietf-ippm-twamp-yang-11#ref-I-D.ietf-ippm-metric-registry <https://tools.ietf.org/html/draft-ietf-ippm-twamp-yang-11#ref-I-D.ietf-ippm-metric-registry>>], or whether
>>   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
>>   received (TWAMP [RFC5357], Section 4.2 <https://tools.ietf.org/html/rfc5357#section-4.2 <https://tools.ietf.org/html/rfc5357#section-4.2>>).
> 

Ok. How about?

OLD:

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.

>>   TWAMP features in [RFC5618 <https://tools.ietf.org/html/rfc5618 <https://tools.ietf.org/html/rfc5618>>], [RFC5938 <https://tools.ietf.org/html/rfc5938 <https://tools.ietf.org/html/rfc5938>>], [RFC6038 <https://tools.ietf.org/html/rfc6038 <https://tools.ietf.org/html/rfc6038>>], and [RFC7717 <https://tools.ietf.org/html/rfc7717 <https://tools.ietf.org/html/rfc7717>>].  If
>>   TWAMP features in [RFC5618 <https://tools.ietf.org/html/rfc5618 <https://tools.ietf.org/html/rfc5618>>], [RFC5938 <https://tools.ietf.org/html/rfc5938 <https://tools.ietf.org/html/rfc5938>>], [RFC6038 <https://tools.ietf.org/html/rfc6038 <https://tools.ietf.org/html/rfc6038>>], and [RFC7717 <https://tools.ietf.org/html/rfc7717 <https://tools.ietf.org/html/rfc7717>>].  If

Will do.

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

> unauthenticated (and MITM-able) test traffic instead of authenticated

Ok. Will add text around it.

Thanks.

>>   Section 4 in [RFC4648] <https://tools.ietf.org/html/rfc4648#section-4 <https://tools.ietf.org/html/rfc4648#section-4>>).
>> 
>> 
>>> 

Mahesh Jethanandani
[email-masked]

---

From: Benjamin Kaduk <[email-masked]>
Date: Fri, 22 Jun 2018 13:05:27 -0500

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

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

Thank you!

> Ok. Will add text around it.

Great; thanks!

-Benjamin