Subject: Re: [ippm] Comments on draft-cmzrjp-ippm-twamp-yang

From: Kostas Pentikousis <[email-masked]>
Date: Wed, 15 Jul 2015 19:03:08 +0200

Dear Rob,

Many thanks for the detailed and thorough comments and suggestions! They really helped us improve the document.

| I have some comments/questions on this draft. I've organised them by the
| section they best apply to, although in some cases they may have effects on
| other sections as well.
| 
| Section 4.1:
| 
|     keys for different Servers. keyId is a UTF-8 string, up to 80 octets
|     in length (if the string is shorter, it is padded with zero octets),
|     that tells the Server which shared secret the Control-Client wishes
|     to use to authenticate or encrypt.  The secretKey is the shared
|     secret, an octet string of arbitrary length whose interpretation as a
|     text string is unspecified.  In the interest of interoperability,
|     however, the UTF-8 text encoding MUST be used for secretKey.
| 
| There's no need to specify the encoding of keyId or secretKey because section
| 9.4 of RFC6020 (YANG) specifies the encoding of the string (presumably along
| with the top-level XML encoding attribute). There's also no need to specifying
| how the padding works because YANG strings are variable-sized.

We stand corrected. Updated in -01.

| 
| Section 4.1:
| 
|     dkLen   Intended length in octets of the derived key, a positive
|             integer, at most (2^32 - 1) * hLen.
| 
| Changing the derived key length from value of 128 bits inferred from section
| 6.10 of RFC4656 will result in a client that cannot authenticate with a server
| unless the derived key length in use by the server matches and the value is
| known to the user/client. Neither RFC4656 nor RFC5357 suggest that this is a
| parameter that should be configurable. Is there an RFC/draft that suggests
| this and extends the server greeting to advertise the derived key length?

Good catch, we have no intention to introduce non-standard values. We have removed the attribute.

| Section 4.1:
| 
|     pmIndex Numerical index value of a Registered Metric in the
|             Performance Metric Registry [I-D.ietf-ippm-metric-registry].
|             Output Statistics will be specified in the Registry entry.
| 
| Perhaps I'm missing something fundamental, or some background information, or
| this is work in progress, but the purpose of I-D.ietf-ippm-metric-registry is
| stated to be:
| 
|     This document defines the IANA Registry for Performance Metrics.
|     This document also gives a set of guidelines for Registered
|     Performance Metric requesters and reviewers.
| 
| Whilst I can see the usefulness of abstracting out the definition of a
| performance metric, how does the index of the registered metric help in
| retrieval of the output statistics?

In the registry, each registered metric specifies output statistics. For an example please see https://tools.ietf.org/html/draft-mornuley-ippm-initial-registry-01. In the present registry design, each registered metric produces exactly ONE well-defined output statistic, so in common use it appears to incorporate multiple output statistics (note the introduction of pm-reg-list in draft-cmzrjp-ippm-twamp-yang-01. The output stats are not part of TWAMP or OWAMP. A separate effort is needed to model the communication/collection of output.

| Section 4.1:
| 
| What triggers the Control-Client to send the Start-Sessions message?

In general, it could be triggered programmatically over NETCONF/RESTCONF or using a CLI. We consider this as an operational command, external to the configuration of the system, and out of scope of the data model per se. See also Appendix B.

| Section 4.1:
| 
| Should twampSessionRequest expose a property to allow the DSCP to be specified
| via the Type-P field of the Request-TW-Session message?

Updated accordingly. 

| Section 4.1:
| 
| Should the behaviour be specified if any of the settable properties of
| twampSessionRequest aren't set?

We have updated the draft accordingly as well. In particular:

| - senderIp

..."If not configured, the device SHALL choose its own source IP address."

| - senderUdpPort

Sender could choose one, socket API would choose it at random

| - reflectorIp

This one is mandatory.

| - reflectorUdpPort

" If this value is not set, the	device SHALL use the same port number as defined in the server-tcp-port parameter of this twamp-session-request's parent twamp-client-ctrl-connection."

| - timeout

"The default value is 2 seconds"

| - paddingLength

As per [RFC4656], Section 3.5.

| - startTime

Default is 0.

| - repeat

Deafult is false, i.e. the session will not be repeated. 

| - pmIndex

Required and updated in -01.

| 
| Section 4.2:
| 
|     maxCount
|             If an attacking system sets the maximum value in Count
|             (2**32), then the system under attack would stall for a
|             significant period of time while it attempts to generate
|             keys.  Therefore, TWAMP-compliant systems SHOULD have a
|             configuration control to limit the maximum Count value.  The
|             default maximum Count value SHOULD be 32768.
| 
| 
| Was this intended to be a server property?

Yes, it was intended to be a Server property - the Server sends this count in the Server Greeting.

| Section 6 of RFC5357 continues to say:
| 
|     If a
|     Control-Client receives a Server-Greeting message with Count greater
|     that its maximum configured value, it SHOULD close the control
|     connection.
| 
| I.e. this applies to clients. The client never suggests a Count to the server;
| instead it's the client that needs to validate the count the server has
| provided.

We have added max-count at the client side so it can make this evaluation.

| 
| Section 4.2:
| 
|     dscp
|             The DSCP value used in the header of the TCP control packets
|             sent by the Server for this control connection.  This will
|             usually be the same value as is configured for
|             twampServer:dscp under the twampServer.  However, in the
|             event that the user re-configures twampServer:dscp after this
|             control connection is already in progress, this read-only
|             value will show the actual dscp value in use by this control
|             connection.
| 
| I support the inclusion of this property despite section 3.1 of RFC5357
| saying:
| 
|     The Server SHOULD use the DSCP of the Control-
|     Client's TCP SYN in ALL subsequent packets on that connection
|     (avoiding any ambiguity in case of re-marking).
| 
| The reasoning being that for practical purposes TWAMP will typically be
| implemented by using a general purpose TCP stack provided by the underlying
| operating system. This statement imposes a requirement on the behaviour of the
| TCP stack which AFAICS isn't specified in standards that directly relate to
| TCP or DSCP. This means that it's not practical to implement this behaviour in
| a OS-portable implementation of TWAMP.
| 
| In addition, there are DoS concerns with allowing a client to control DSCP
| values on the return path from a server.
| 
| Configuring the server use a DSCP for control packets it sends mitigates both
| issues.

We agree. 

| 
| Section 4.2:
| 
|     dkLen
|             The dkLen value that is in use by this control connection.
|             This will usually be the same value as is configured under
|             twampServer.  However, in the event that the user re-
|             configured twampServer:dkLen after this control connection is
|             already in progress, this read-only value will show the
|             actual dkLen that is in use for this control connection.
| 
| The description here refers to twampServer:dkLen but there is no item named
| dkLen under twampServer. Such an item under twampServer is required if the
| client is to have such an option (see above).

Excellent catch, mnay thanks. Updated accordingly.

| 
| Section 4.2:
| 
|     The twampServer container items are as follows:
| 
| ...
|     The following parameters are read-only:
| 
|     salt    A parameter used in deriving a key from a shared secret as
|             described in Section 3.1 of [RFC4656].  Salt MUST be
|             generated pseudo-randomly (independently of anything else in
|             the RFC) and is communicated to the Control-Client as part of
|             the Server greeting message.
| 
|     serverIv
|             The Server Initialization Vector (IV) is generated randomly
|             by the server.
| 
|     challenge
|             Challenge is a random sequence of octets generated by the
|             server.  As described in Section 4.1 challenge is used by the
|             Control-Client to prove possession of a shared secret.
| 
| These are all per-connection properties and so should be properties of the
| twampServerCtrlConnection container.
|

Done.
 
| Section 4.3:
| 
|     dscp    The DSCP value to be placed in the UDP header of TWAMP-
|             Control packets generated by the Session-Sender.
| 
| This should say "... UDP header of TWAMP-Test packets ...".

Correct. Updated :)

| 
| Section 4.3:
| 
| Allowing properties to be changed at will whilst test sessions are in progress
| may present implementation challenges (i.e. synchronisation with the sending
| of packets). Do we think this is a valid use case? If not, could text be added
| to guide implementors as to the expectations?

Good point. Perhaps we could discuss further next week in Prague?

| 
| Section 4.4:
| 
|     twampServerCtrlConnection object.  This four4-tuple allows the user
| 
| The "four" should be removed.

Done.

| 
| Section 5.2:
| 
|           leaf serverIp {
|             config "true";
|             type inet:ip-address;
|           }
| 
| Nit: the parent list defaults to config being true, so the config statement is
| unnecessary and leads to questions in the readers' mind as to why other
| apparently configurable properties in the same list don't have this statement.

Good catch. In now reads: mandatory "true"

| 
| Section 5.2, list twampClientCtrlConnection:
| 
|           leaf token {
|             config "false";
|             type string {
|               length "1..64";
|             }
|             description "64 octets, containing the concatenation of a
|               16-octet challenge, a 16-octet AES Session-key used
|               for encryption, and a 32-octet HMAC-SHA1 Session-key
|               used for authentication";
|           }
|           leaf clientIv{
|             config "false";
|             type string {
|               length "1..16";
|             }
|             description "16 octets, Client-IV is generated randomly
|               by the Control-Client.";
|           }
| 
| Should these be binary types of fixed length instead?

Updated to binary. 

| 
| Section 5.2, list twampClientCtrlConnection:
| 
|           leaf serverStartTime {
|             config "false";
|             type uint64;
|           }
| 
| Should this be exposed in a form more convenient for the operator, such as
| ietf-yang-types:date-and-time?

Actually, Start-Time is a timestamp representing the time when the current instantiation of the Server started operating. Timestamp format is described in Section 4.1.2 of RFC 4656.

| 
| Section 5.2, container twampServer:
| 
|         leaf salt{
|           config "false";
|           type string {
|             length "1..16";
|           }
|             description "Salt MUST be generated pseudo-randomly";
|         }
|         leaf serverIv {
|           config "false";
|           type string {
|             length "1..16";
|           }
|           description "16 octets, Server-IV is generated randomly
|             by the Control-Client.";
|         }
|         leaf challenge {
|           config "false";
|           type string {
|             length "1..16";
|           }
|           description "Challenge is a random sequence of octets
|             generated by the Server";
|         }
| 
| Should these be binary types of fixed length instead?

Updated in -01. Many thanks!

| 
| Section 7:
| 
| Is there a use case in mind behind the exposing of values that can be used to
| derive the session key on both clients and servers?
| 
| The reason for this question is that such information will need to be
| restricted to privileged operators and if the implementation of the management
| interface doesn't support a fine enough grained access control mechanism then
| it could prevent much of the data in this model from being accessible by
| ordinary operators.

Are you are referring to this text from Appendix A.1 in -00?

         <keychain>
            <keyid>KeyClient1ToRouterA</keyid>
            <secretKey>secret1</secretKey>
         </keychain>
         <keychain>
            <keyid>KeyForRouterB</keyid>
            <secretKey>secret2</secretKey>
         </keychain>

Best regards,

Kostas (as editor, on behalf of the authors)

---

From: Robert Shearman <[email-masked]>
Date: Fri, 24 Jul 2015 14:05:34 +0100

Hi Kostas,

> Many thanks for the detailed and thorough comments and suggestions! They really helped us improve the document.

You're most welcome. Thanks for taking note of them when drafting -01.

> In the registry, each registered metric specifies output statistics. For an example please see https://tools.ietf.org/html/draft-mornuley-ippm-initial-registry-01. In the present registry design, e…

Ok, thanks. I look forward to seeing the results of that effort.

> In general, it could be triggered programmatically over NETCONF/RESTCONF or using a CLI. We consider this as an operational command, external to the configuration of the system, and out of scope of…

I understand your reasoning, but it would be a shame to not have the 
mechanism of at least starting TWAMP test sessions standardised, given 
that it is essential to the ability to use the client at all.

> Yes, it was intended to be a Server property - the Server sends this count in the Server Greeting.

Isn't that already covered by the count parameter?

    count   Parameter used in deriving a key from a shared secret as
            described in Section 3.1 of [RFC4656], and are communicated
            to the Control-Client as part of the Server Greeting message.
            count MUST be a power of 2. count MUST be at least 1024.
            count SHOULD be increased as more computing power becomes
            common.

In other words, I believe the max-count (as now called in -01) property 
should be removed from the twamp-server container.

> Good point. Perhaps we could discuss further next week in Prague?

As I'm sure you've already concluded, this won't be possible :-)

However, I'm happy to discuss this further. Feel free to unicast me to 
discuss a suitable medium if required.

> Actually, Start-Time is a timestamp representing the time when the current instantiation of the Server started operating. Timestamp format is described in Section 4.1.2 of RFC 4656.

I understand, but what I was getting at is that whilst a uint64 is very 
convenient for an implementor of the module (no conversion necessary), 
it might not be the most convenient for the user of the module.

>              <secretKey>secret2</secretKey>

I'm referring to the following twamp-client-ctrl-connection properties:
- token
- client-iv

These are used to derive the session key for the client.

And the following twamp-server-ctrl-connection properties:
- server-iv
- challenge

These are used to derive the session key for the server.

What I was attempting to say is that since keeping these private is 
essential to keeping the control and/or test session secure then care 
needs to be taken in exposing them. Naturally, the easiest way of 
keeping them secure is to not expose them at all.

In conclusion:
- if there is no good reason to expose them, I'd suggest removing the 
twamp-client-ctrl-connection:token, 
twamp-client-ctrl-connection:client-iv, 
twamp-server-ctrl-connection:challenge and 
twamp-server-ctrl-connection:server-iv properties
- otherwise, I'd suggest adding a note to the security considerations 
that read access to these properties should be restricted to avoid the 
security of TWAMP sessions being compromised.

Thanks,
Rob

---

From: Robert Shearman <[email-masked]>
Date: Mon, 27 Jul 2015 14:50:40 +0100

Hi Al,

On 24/07/15 14:41, MORTON, ALFRED C (AL) wrote:
> Hi Robert,
>>> Kostas replied:
>>> In general, it could be triggered programmatically over NETCONF/RESTCONF
>> or using a CLI. We consider this as an operational command, external to
>> the configuration of the system, and out of scope of the data model per se.
>> See also Appendix B.
>>
> There is a proposal to add time-triggered configuration to
> NETCONF, which would seem to meet your need:
> https://tools.ietf.org/html/draft-mm-netconf-time-capability-05
>
> This draft is currently in IETF-Wide Last Call, so it is worthwhile
> to express you opinion on [email-masked], if you come up with one:
> https://datatracker.ietf.org/doc/draft-mm-netconf-time-capability/history/

The twamp-session-request:start-time property would appear to do the 
same job, namely setting a lower-bound on the time that a session is 
started?

While it doesn't provide the time at which the configuration was 
actually applied, this isn't actually useful because the configuration 
results in messages sent to the Control-Server and the protocol doesn't 
allow the actual time of execution of the message to be returned. Not 
that I think it would be useful to do anyway.

So I don't think this proposal is required in order for 
draft-cmzrjp-ippm-twamp-yang to specify a mechanism for TWAMP test 
sessions to be started.

Thanks,
Rob

---

From: Kostas Pentikousis <[email-masked]>
Date: Mon, 27 Jul 2015 16:03:30 +0200

Hi Rob,

<snip>

Many thanks for the comments, which we will process with due care. This is just a short note regarding the following

<snip>

| > In general, it could be triggered programmatically over NETCONF/RESTCONF or
| using a CLI. We consider this as an operational command, external to the
| configuration of the system, and out of scope of the data model per se. See
| also Appendix B.
| 
| I understand your reasoning, but it would be a shame to not have the mechanism
| of at least starting TWAMP test sessions standardised, given that it is
| essential to the ability to use the client at all.

(no hat on) I couldn't agree more. I think that in combination with the data model, both this and the collection of stats you mentioned would provide the essentials for a programmable TWAMP deployment. I would be happy to contribute to such an effort. Perhaps ask for volunteers to form an (informal) design team?

Best regards,

Kostas