Re: [ippm] Benjamin Kaduk's Discuss on draft-ietf-ippm-stamp-07: (with DISCUSS and COMMENT)

[Mirja Kuehlewind <[e-mail]>]

Hi Ben, hi Greg, 

Ben thanks a lot for your thorough review and good feedback! Also thanks, Greg, for the updates and good work. This really improved the spec quite a bit and I think we are now ready to go!

Ben, are you ready to clear your discuss?

Thanks!
Mirja


> On 18. Oct 2019, at 22:33, Benjamin Kaduk <[e-mail]> wrote:
> 
> On Wed, Oct 16, 2019 at 04:44:25PM -0700, Greg Mirsky wrote:
>> Hi Benjamin,
>> thank you for the additional details. Please find my answers below under
>> GIM2>> tag. Also, the copy of the working version and its diff to -07 are
>> attached. I greatly appreciate your feedback.
>> 
>> Regards,
>> Greg
>> 
>> On Tue, Oct 15, 2019 at 8:56 AM Benjamin Kaduk <[e-mail]> wrote:
>> 
>>> Hi Greg,
>>> 
>>> Sorry for the delayed response -- I was travelling last week.
>>> 
>>> A couple notes on the -08 before I get into the inline replies:
>>> 
>>> Thanks for continuing the dialogue with the gen-art reviewer; I'm happy to
>>> see those refinements made.
>>> 
>>> In Section 4.1.1 we are now talking about both the "Z flag" and "Z field";
>>> it's probably best to just pick one.
>>> 
>> GIM2>> Changed to "Z field" as in RFC 8186.
>> 
>>> 
>>> On Wed, Oct 09, 2019 at 08:37:26PM -0700, Greg Mirsky wrote:
>>>> Hi Benjamin,
>>>> thank you for your thorough review and detailed comments. Please find
>>>> answers, notes, and the proposed updates below in-line tagged GIM>>.
>>>> I much appreciate your feedback, suggestions to address your concerns.
>>>> 
>>>> Regards,
>>>> Greg
>>>> 
>>>> On Wed, Sep 4, 2019 at 5:50 PM Benjamin Kaduk via Datatracker <
>>>> [e-mail]> wrote:
>>>> 
>>>>> Benjamin Kaduk has entered the following ballot position for
>>>>> draft-ietf-ippm-stamp-07: 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-ippm-stamp/
>>>>> 
>>>>> 
>>>>> 
>>>>> ----------------------------------------------------------------------
>>>>> DISCUSS:
>>>>> ----------------------------------------------------------------------
>>>>> 
>>>>> We don't ever clearly state that the protocol allows for packet sizes
>>>>> other than the listed 44- and 112-octet variants, that content larger
>>>>> than that is to be treated as padding unless directed otherwise by
>>>>> configuration, that the reflected packet must be the same size as the
>>>>> incoming packet, and how a Session-Reflector should set any such
>>> padding
>>>>> that it needs to add in order to produce a same-sized packet.
>>>>> 
>>>> GIM>> We had discussed this and the current working version of the draft
>>> in
>>>> Section 4.2 refers to the STAMP Optional Extensions
>>>> <https://datatracker.ietf.org/doc/draft-ietf-ippm-stamp-option-tlv/>
>>> draft:
>>>>   STAMP supports symmetrical test packets.  The base STAMP Session-
>>>>   Sender packet has a minimum size of 44 octets in unauthenticated
>>>>   mode, see Figure 2, and 112 octets in the authenticated mode, see
>>>>   Figure 4.  The variable length of a test packet in STAMP is supported
>>>>   by using Extra Padding TLV defined in
>>>>   [I-D.ietf-ippm-stamp-option-tlv].
>>>> As discussed in Section 4.6 Interoperability with TWAMP Light, TWAMP
>>> Light
>>>> Session-Reflector will treat STAMP optional extensions as Padding and, if
>>>> configured to symmetrical packet size mode, will respond with Padding as
>>>> per RFC 6038. This draft defines the use of only base STAMP packets and
>>> the
>>>> discussion of all extensions is in the draft-ietf-ippm-stamp-option-tlv.
>>> 
>>> I understand that this document only defines base STAMP packets, but it
>>> also needs to cover the "protocol invariants" for STAMP, even when both
>>> endpoints are STAMP and no TWAMP-Light is involved.  So, adding the
>>> sentence about variable length being supported by the padding TLV is good,
>>> but I still think we should have some discussion about, e.g., what a
>>> receiver should do when it receives a packet larger than the base size
>>> which does not parse properly as having trailing TLV(s), and what bytes are
>>> used to fill a reflected packet when it is larger than the base test
>>> packet.  I'm also still unclear on whether we always require the reflected
>>> packet to be the same size as the test packet -- Section 4 has a brief not
>>> that "[b]y default, STAMP uses symmetrical packets" but I did not find any
>>> discussion of when or how it would work otherwise.
>>> 
>> GIM2>> I see your point and agree that that needs clarification in the
>> spec. I think that Section 4.3 Session-Reflector Behavior and Packet Format
>> is the right place. Below is the updated paragraph:
>>   The Session-Reflector receives the STAMP test packet and verifies it.
>>   If the base STAMP test packet validated, the Session-Reflector, that
>>   supports this specification, prepares and transmits the reflected
>>   test packet symmetric to the packet received from the Session-Sender
>>   copying the content beyond the size of the base STAMP packet (see
>>   Section 4.2).
> 
> That sounds good; thanks!
> 
>>> 
>>>>> 
>>>>> This document hardcodes the truncated HMAC-SHA-256 algorithm.  Per BCP
>>>>> 201, what is the procedure for cryptographic algorithm agility?
>>>>> 
>>>> GIM>> Support of other cryptographic algorithms is important but the WG
>>>> agreed that in this specification only the use of HMAC-SHA-256 is
>>> defined.
>>>> Future specifications may define the use of other, more advanced
>>>> cryptographic algorithms, possibly providing an update to the STAMP YANG
>>>> data model <https://datatracker.ietf.org/doc/draft-ietf-ippm-stamp-yang/
>>>> .
>>> 
>>> That's a reasonable approach for agility; I'd suggest adding a note to the
>>> document to indicate that this is the plan.
>>> 
>> GIM2>> Would add the last sentence to Section 4.4 Integrity Protection in
>> STAMP:
>>   Future specifications may define the use of other, more advanced
>>   cryptographic algorithms, possibly providing an update to the STAMP
>>   YANG data model [I-D.ietf-ippm-stamp-yang].
> 
> Sounds good.
> 
>> 
>>>>> 
>>>>> Please also consider the discussion in BCP 107 about key lifecycles and
>>>>> key management, including whether it is appropriate to use a
>>>>> key-derivation function to produce short-term (e.g., per flow) keys
>>> from
>>>>> a long-lived key (e.g., one fixed in static configuration).
>>>>> 
>>>> GIM>> In the course of the discussion, we've clarified in the section
>>>> Integrity Protection in STAMP that:
>>>>   HMAC uses its own key, and the definition of the
>>>>   mechanism to distribute the HMAC key is outside the scope of this
>>>>   specification.  One example is to use an orchestrator to configure
>>>>   HMAC key based on STAMP YANG data model [I-D.ietf-ippm-stamp-yang].
>>> 
>>> Hmm, I'm not sure I was a part of the discussion in question, since this
>>> text looks unchanged from what I balloted on for the -07.  I'd suggest to
>>> clarify further "HMAC uses its own key" with respect to the scope of the
>>> key -- is it a unique key per test session?
>>> 
>> GIM2>> HMAC key may be unique for each STAMP test session. Update to the
>> sentence:
>> OLD TEXT:
>>  HMAC uses its own key, and the definition of the
>>  mechanism to distribute the HMAC key is outside the scope of this
>>  specification
>> NEW TEXT:
>>   HMAC uses its own key that may be unique for
>>   each STAMP test session; key management and the mechanisms to
>>   distribute the HMAC key is outside the scope of this specification.
> 
> Okay.
> 
>>> 
>>>>> 
>>>>> What is the input plaintext to the HMAC computation?  In the case of
>>>>> future extensions, does the HMAC field remain at its current fixed
>>>>> offset in the packet or move to always be the last 16 octets?  Is any
>>>>> additional padding/TLV content protected by the HMAC?
>>> 
>>> I see in the editor's copy that this is clarified to have the HMAC cover
>>> the first 96 bytes; okay.
>>> 
>>>>> What error does the error estimate ... estimate?
>>>>> Clock skew between sender and receiver?
>>>>> 
>>>> GIM>> The Error Estimate field has been originally defined in RFC 4656
>>>> One-Way Active Measurement Protocol. One flag (S) indicates whether the
>>>> originator of the timestamp has clock synchronized to UTC (GPS, NTP or
>>>> PTP). Other fields can be used to express the error estimate of the
>>>> timestamping process.
>>> 
>>> I looked at the linked section of RFC 4656 in my initial review, and was
>>> only able to find the interpretation of the 'scale' and 'multiplier' fields
>>> to form a combined "error estimate" in seconds (with sub-second precision).
>>> What I didn't find was a discussion of its abstract semantics -- what is
>>> the reference value and the measured value whose error is being estimated
>>> with respect to the reference?  A timestamp of some form, given the units
>>> (seconds), but which one?
>>> 
>> GIM2>> In my experience with OWAMP/TWAMP implementations, the value
>> produced by the Error Estimate (Scale and Multiplier) was hard-coded and
>> not reflective of how a timestamp obtained. That was the reason we've
>> introduced the Timestamp Information TLV in draft-ietf-ippm-stamp-option-tlv
>> <https://datatracker.ietf.org/doc/draft-ietf-ippm-stamp-option-tlv/>.
> 
> That's kind of unfortunate (hard-coding), but the text here does make more
> sense with that context!  I'll let this one go.
> 
>> 
>>>>> 
>>>>> I think we need to require some level of cryptographic protection
>>>>> whenever control information is included in a Session-Sender's test
>>>>> packet.  That is, that a Session-Reflector MUST NOT act on control
>>>>> information received in unauthenticated packets.  (That said, this
>>>>> document itself does not describe a way to include control information,
>>>>> so perhaps the note about "optional control information communicated in
>>>>> the Session-Sender's test packet" in Section 4 is misplaced.
>>>>> 
>>>> GIM>> Thank you for catching this. Clearly, it must be removed:
>>>> OLD TEXT:
>>>>   STAMP Session-Reflector receives Session-Sender's packet and acts
>>>>   according to the configuration and optional control information
>>>>   communicated in the Session-Sender's test packet.
>>>> NEW TEXT:
>>>>   STAMP Session-Reflector receives Session-
>>>>   Sender's packet and acts according to the configuration.
>>>> 
>>>> In Section 4.2.1:
>>>>> 
>>>>>   o  Timestamp and Receiver Timestamp fields are each eight octets
>>>>>      long.  The format of these fields, NTP or PTPv2, indicated by the
>>>>>      Z flag of the Error Estimate field as described in Section 4.1.
>>>>> 
>>>>> I think you need to explicitly say that "Timestamp" is echoed from the
>>>>> received packet and "Receiver Timestamp" is determined locally as close
>>>>> to (reciept? transmission?) as possible.
>>>>> 
>>>> GIM>> You've helped find a typo that makes the name of the field
>>> confusing.
>>>> The field is tagged correctly in Figure 5 - Receive Timestamp. In fact,
>>> the
>>>> Receive Timestamp is also the local to the Session-Reflector. It is the
>>>> time value the Reflector received the STAMP test packet. The value in the
>>>> Timestamp field is taken at the transmission of the reflected packet. The
>>>> Sender Timestamp field is a copy of the Timestamp field in the
>>>> Session-Sender's test packet. I propose the update as follows:
>>>> OLD TEXT:
>>>>   o  Timestamp and Receiver Timestamp fields are each eight octets
>>>>      long.  The format of these fields, NTP or PTPv2, indicated by the
>>>>      Z flag of the Error Estimate field as described in Section 4.1.
>>>> NEW TEXT:
>>>>   o  Timestamp and Receive Timestamp fields are each eight octets long.
>>>>      The format of these fields, NTP or PTPv2, indicated by the Z flag
>>>>      of the Error Estimate field as described in Section 4.2.  Receive
>>>>      Timestamp is the time the test packet was received by the Session-
>>>>      Reflector.  Timestamp - the time taken by the Session-Reflector at
>>>>      the start of transmitting the test packet.
>>> 
>>> Thanks!
>>> 
>>>>> 
>>>>> I think we need greater clarity on whether the normative statements in
>>>>> Section 4.4 apply only to STAMP peers that are aware they are
>>>>> interacting with TWAMP Light, or apply to all STAMP peers (see Comment
>>>>> for further discussion on why the current text seems internally
>>>>> inconsistent).
>>> 
>>> [It looks like discussion of this is down in the Comment section]
>>> 
>>>>> 
>>>>> In Section 4.1.1:
>>>>> 
>>>>>   o  Timestamp is eight octets long field.  STAMP node MUST support
>>>>>      Network Time Protocol (NTP) version 4 64-bit timestamp format
>>>>>      [RFC5905], the format used in [RFC5357].  STAMP node MAY support
>>>>>      IEEE 1588v2 Precision Time Protocol truncated 64-bit timestamp
>>>>>      format [IEEE.1588.2008], the format used in [RFC8186].
>>>>> 
>>>>> I think a note that which one is in use will be configured by the
>>>>> configuration/management function is in order.  Except that the Z bit
>>>>> below confuses things terribly...
>>>>> 
>>>>>      The STAMP Session-Sender and Session-Reflector MAY use, not use,
>>>>>      or set value of the Z field in accordance with the timestamp
>>>>>      format in use.  This optional field is to enhance operations, but
>>>>>      local configuration or defaults could be used in its place.
>>>>> 
>>>>> ... since, as noted by the secdir reviewer, this line just confuses
>>>>> everything.  Either keep the "must be zero" semantics of 4656 or the
>>>>> "MUST match reality" semantics of 8186, but this middle case is
>>> actively
>>>>> harmful.
>>>>> 
>>>> GIM>> As result of the discussion, this text is changed to:
>>>> NEW TEXT:
>>>>      The STAMP Session-Sender and Session-Reflector MUST use the NTP 64
>>>>      bit format of a timestamp (Z field value of 0).  as the default.
>>>>      A configuration/management function MAY configure STAMP Session-
>>>>      Sender and Session-Reflector to using the PTPv2 truncated format
>>>>      of a timestamp (Z field value of 1).
>>>> Hope it is clearer now.
>>> 
>>> Yes, that language addresses my concerns.
>>> 
>>>>> 
>>>>> (I also support Barry and Magnus' Discusses.)
>>>>> 
>>>> GIM>> It took some time to address them.
>>>> 
>>>>> 
>>>>> 
>>>>> ----------------------------------------------------------------------
>>>>> COMMENT:
>>>>> ----------------------------------------------------------------------
>>>>> 
>>>>> Section 1
>>>>> 
>>>>> I'll note several grammar nits, inline, though perhaps some of them
>>> will
>>>>> not apply after the rewrite in response to the secdir review:
>>>>> 
>>>>>   Development and deployment of Two-Way Active Measurement Protocol
>>>>> 
>>>>> "the Two-Way"
>>>>> 
>>>> GIM>> Applied, thank you.
>>>> 
>>>>> 
>>>>>   (TWAMP) [RFC5357] and its extensions, e.g., [RFC6038] that defined
>>>>>   features such as Reflect Octets and Symmetrical Size for TWAMP
>>>>> 
>>>>> comma after TWAMP
>>>>> 
>>>> GIM>> Done.
>>>> 
>>>>> 
>>>>>   provided invaluable experience.  Several independent implementations
>>>>>   exist, have been deployed and provide important operational
>>>>>   performance measurements.  At the same time, there has been
>>>>>   noticeable interest in using a more straightforward mechanism for
>>>>>   active performance monitoring that can provide deterministic
>>> behavior
>>>>>   and inherit separation of control (vendor-specific configuration or
>>>>> 
>>>>> "inherit" from what?
>>>>> 
>>>> GIM>> Right, should have been "inherent". Now in the working version.
>>> 
>>> Ah, that makes much more sense now :)
>>> 
>>>>> 
>>>>>   orchestration) and test functions.  One of such is Performance
>>>>> 
>>>>> "One such mechanism is"
>>>>> 
>>>> GIM>> This passage updated to:
>>>>   Recent work on IP Edge to Customer Equipment using TWAMP Light from
>>>>   Broadband Forum [BBF.TR-390] demonstrated that interoperability among
>>>>   implementations of TWAMP Light is challenged because the composition
>>>>   and operation of TWAMP Light were not sufficiently specified in
>>>>   [RFC5357].
>>>> 
>>>>> 
>>>>>   Measurement from IP Edge to Customer Equipment using TWAMP Light
>>> from
>>>>>   Broadband Forum [BBF.TR-390] used as the reference TWAMP Light that,
>>>>> 
>>>>> I'm not sure what the intent here is, but maybe ", which is used as the
>>>>> reference TWAMP Light".
>>>>> 
>>>> GIM>> Replaced by the sentence I've copied above.
>>>> 
>>>>> 
>>>>>   according to [RFC8545], includes sub-set of TWAMP-Test functions in
>>>>> 
>>>>> I'd also suggest starting a new sentence for "According to [RFC8545]"
>>>>> (and adding the then-needed "this" and "a" for "this includes a").
>>>>> 
>>>> GIM>> Re-worded as follows:
>>>>   According to [RFC8545], TWAMP Light includes sub-set of
>>>>   TWAMP-Test functions to provide comprehensive solution requires
>>>>   support by other applications that provide, for example, control and
>>>>   security.
>>>> 
>>>> 
>>>>> 
>>>>>   combination with other applications that provide, for example,
>>>>>   control and security.  This document defines an active performance
>>>>>   measurement test protocol, Simple Two-way Active Measurement
>>> Protocol
>>>>>   (STAMP), that enables measurement of both one-way and round-trip
>>>>>   performance metrics like delay, delay variation, and packet loss.
>>>>> 
>>>>> I agree with the secdir reviewer that the relationship between STAMP
>>> and
>>>>> TWAMP Light could be much more clear.
>>>>> 
>>>> GIM>> The new paragraph at the closing of the Introduction section:
>>>>   This document defines an active performance measurement test
>>>>   protocol, Simple Two-way Active Measurement Protocol (STAMP), that
>>>>   enables measurement of both one-way and round-trip performance
>>>>   metrics like delay, delay variation, and packet loss.  Some TWAMP
>>>>   extensions, e.g., [RFC7750] are supported by the extensions to STAMP
>>>>   base specification in [I-D.ietf-ippm-stamp-option-tlv].
>>>> 
>>>>> 
>>>>> Section 2.1
>>>>> 
>>>>>   MBZ May be Zero
>>>>> 
>>>>> I commonly see this expand to "Must be zero"; requiring the sender to
>>>>> not set any bits seems more likely to preserve the ability to use the
>>>>> field for future extensibility, since a recipient that sees a nonzero
>>>>> bit knows it was consciously set (i.e., with intent to use the
>>>>> extension) rather than inadvertently set by someone expecting it to be
>>>>> ignored.
>>>>> (Also, if the bits are covered under the HMAC, then the recipient can't
>>>>> actually ignore them, since they have to be used to verify the HMAC.)
>>>>> 
>>>> GIM>> Changed MBZ full form to the Must-be-zero. Named padding fields in
>>>> unauthenticated mode - Reserved. Would that be acceptable?
>>> 
>>> That's probably fine.  I still wish we could do something to alleviate the
>>> dissonance between "ignored on receipt" and (presumably) needing to use the
>>> fields as input to HMAC validation.
>>> 
>> GIM2>> This specification follows the language used in RFC 4656 OWAMP and
>> RFC 5357 TWAMP to describe the authenticated mode for test components of
>> the respective protocols. I agree, in the authenticated mode MBZ is not
>> "ignored on receipt". I propose a note in the description of MBZ fields in
>> the authenticated mode. Below is the updated text of the Session-Sender's
>> format:
>>   The field definitions are the same as the unauthenticated mode,
>>   listed in Section 4.2.1.  Also, Must-Be-Zero (MBZ) fields are used to
>>   to make the packet length a multiple of 16 octets.  The value of the
>>   field MUST be zeroed on transmission and MUST be ignored on receipt.
>>   Note, that the MBZ field is used to calculate a key-hashed message
>>   authentication code (HMAC) ([RFC2104]) hash.  Also, the packet
>>   includes HMAC hash at the end of the PDU.  The detailed use of the
>>   HMAC field is described in Section 4.4.
>> And the updated text for the Session-Reflector's packet:
>>   The field definitions are the same as the unauthenticated mode,
>>   listed in Section 4.3.1.  Additionally, the MBZ field is used to to
>>   make the packet length a multiple of 16 octets.  The value of the
>>   field MAY be zeroed on transmission and MUST be ignored on receipt.
>>   Note, that the MBZ field is used to calculate HMAC hash value.  Also,
>>   STAMP Session-Reflector test packet format in authenticated mode
>>   includes HMAC ([RFC2104]) hash at the end of the PDU.  The detailed
>>   use of the HMAC field is in Section 4.4.
> 
> I don't have any better alternatives, so thanks for this.
> 
>> 
>>>>> 
>>>>> Section 3
>>>>> 
>>>>>   be achieved through various means.  Command Line Interface, OSS/BSS
>>>>>   (operations support system/business support system as a combination
>>>>>   of two systems used to support a range of telecommunication
>>> services)
>>>>>   using SNMP or controllers in Software-Defined Networking using
>>>>>   Netconf/YANG are but a few examples.
>>>>> 
>>>>> nit: if "using SNMP or controllers[...]" is supposed to be separate
>>> from
>>>>> "OSS/BSS", then some additional punctuation/conjunction is needed.
>>>>> 
>>>> GIM>> Also re-worded as:
>>>>   The configuration and management of the STAMP Session-
>>>>   Sender, Session-Reflector, and management of the STAMP sessions are
>>>>   outside the scope of this document and can be achieved through
>>>>   various means.  A few examples are:  Command Line Interface,
>>>>   telecommunication services' OSS/BSS systems, SNMP, and Netconf/YANG-
>>>>   based SDN controllers.
>>> 
>>> Looks great!
>>> 
>>>>> 
>>>>> Section 4
>>>>> 
>>>>>   number.  A STAMP implementation of Session-Sender MUST be able to
>>> use
>>>>>   UDP port numbers from User, a.k.a.  Registered, Ports and Dynamic,
>>>>>   a.k.a.  Private or Ephemeral, Ports ranges defined in [RFC6335].
>>>>> 
>>>>> Able to use as source, destination, or both?  (We just talked about
>>>>> destination but not source in the previous sentence.)
>>>>> 
>>>> GIM>> The text is now in Section 4.1. Will clarify that it applies to the
>>>> destination port:
>>>>   A STAMP implementation of Session-Sender MUST be able to use as the
>>>>   destination UDP port numbers from User, a.k.a.  Registered, Ports and
>>>>   Dynamic, a.k.a.  Private or Ephemeral, Ports ranges defined in
>>>>   [RFC6335].
>>>> 
>>>>> 
>>>>> Section 4.1
>>>>> 
>>>>>   Because STAMP supports symmetrical test packets, STAMP
>>> Session-Sender
>>>>>   packet has a minimum size of 44 octets in unauthenticated mode, see
>>>>>   Figure 2, and 112 octets in the authenticated mode, see Figure 4.
>>>>> 
>>>>> nit: I don't see how merely "support"ing (as opposed to "require"ing or
>>>>> "use"ing) symmetrical packets implies these minimum packet sizes.
>>> (That
>>>>> is, I find the word "because" unjustified absent some statement that
>>>>> requires the Session-Reflector packets to be that size and a
>>> requirement
>>>>> for the symmetry is present.)
>>>>> 
>>>> GIM>> The use of the symmetrical test packets is the default behavior:
>>>> NEW TEXT:
>>>>   A STAMP Session-Reflector supports symmetrical size of test packets
>>>>   [RFC6038] as the default behavior.  Because of that, the base STAMP
>>>>   Session-Sender packet has a minimum size of 44 octets in
>>>>   unauthenticated mode, see Figure 2, and 112 octets in the
>>>>   authenticated mode, see Figure 4.  The variable length of a test
>>>>   packet in STAMP is supported by using Extra Padding TLV defined in
>>>>   [I-D.ietf-ippm-stamp-option-tlv].
>>> 
>>> Sorry for being dense, but I'm still not seeing the logical chain of
>>> deductions that makes "because" applicable.  It seems like the minimum size
>>> of a base packet is a decision that can be made independently of whether to
>>> use symmetrical test packets (and, furthermore, just because something is a
>>> default behavior does not mean that it can be used to justify any
>>> authoritative statements about the whole system absent some discussion of
>>> permitted deviations from the default).
>>> 
>> GIM2>> Here's an update to that text:
>> NEW TEXT:
>>   A STAMP Session-Reflector supports the symmetrical size of test
>>   packets [RFC6038] as the default behavior.  A reflected test packet
>>   includes more information and thus is larger.  Because of that, the
>>   base STAMP Session-Sender packet is padded to match the size of a
>>   reflected STAMP test packet.  Hence, the base STAMP Session-Sender
>>   packet has a minimum size of 44 octets in unauthenticated mode, see
>>   Figure 2, and 112 octets in the authenticated mode, see Figure 4.
>>   The variable length of a test packet in STAMP is supported by using
>>   Extra Padding TLV defined in [I-D.ietf-ippm-stamp-option-tlv].
> 
> Thank you!  I understand what is going on here, now.
> 
>> I agree that we'll discuss the control of the test packet length in more
>> detail in draft-ietf-ippm-stamp-option-tlv.
>> 
>>> 
>>>>> 
>>>>> Section 4.2
>>>>> 
>>>>>      That implies that the STAMP Session-Reflector MUST keep a state
>>>>>      for each accepted STAMP-test session, uniquely identifying STAMP-
>>>>>      test packets to one such session instance, and enabling adding a
>>>>>      sequence number in the test reply that is individually
>>> incremented
>>>>>      on a per-session basis.
>>>>> 
>>>>> How does it "accept a STAMP-test session"?
>>>>> 
>>>> GIM>> Would s/accepted/configured/ work?
>>> 
>>> That would be great.
>>> 
>>>>> 
>>>>> Section 4.2.1
>>>>> 
>>>>>      *  in the stateful mode the Session-Reflector counts the received
>>>>>         STAMP test packets in each test session and uses that counter
>>>>>         to set the value of the Sequence Number field.
>>>>> 
>>>>> Should we say anything about whether the initial sequence number
>>> (having
>>>>> received one packet from the Session-Sender) is zero or one?
>>>>> 
>>>> GIM>> In the description of the format of the Session-Sender
>>>> unauthenticated test packet stated:
>>>>   o  Sequence Number is four octets long field.  For each new session
>>>>      its value starts at zero and is incremented with each transmitted
>>>>      packet.
>>>> Will add similar note for the Session-Reflector:
>>>> OLD TEXT:
>>>>      *  in the stateful mode the Session-Reflector counts the received
>>>>         STAMP test packets in each test session and uses that counter
>>>>         to set the value of the Sequence Number field.
>>>> NEW TEXT:
>>>>      *  in the stateful mode, the Session-Reflector counts the
>>>>         transmitted STAMP test packets.  It starts with zero and is
>>>>         incremented by one for each subsequent packet for each test
>>>>         session.  The Session-Reflector uses that counter to set the
>>>>         value of the Sequence Number field.
>>> 
>>> Thanks!
>>> 
>>>>> 
>>>>> Section 4.2.2
>>>>> 
>>>>>                                                              Also,
>>>>>   STAMP Session-Reflector test packet format in authenticated mode
>>>>>   includes a key (HMAC) ([RFC2104]) hash at the end of the PDU.  The
>>>>>   detailed use of the HMAC field is in Section 4.3.
>>>>> 
>>>>> nit: "keyed"
>>>>> 
>>>> GIM>> Done, thank you
>>>> 
>>>>> 
>>>>> Section 4.3
>>>>> 
>>>>> I think we should have a statement about HMAC key (non-)reuse across
>>>>> separate measurement sessions.
>>>>> 
>>>>> I agree with the secdir reviewer that the confidentiality protection
>>>>> seems like something that would be done at a "lower" level, not a
>>>>> "higher" level.
>>>>> 
>>>> GIM>> Resulting from our discussion with SecDir, the following text is
>>> now
>>>> in the Integrity Protection in STAMP section:
>>>> HMAC uses its own key; key management and the
>>>>   mechanisms to distribute the HMAC key is outside the scope of this
>>>>   specification.  One example is to use an orchestrator to configure
>>>>   HMAC key based on STAMP YANG data model [I-D.ietf-ippm-stamp-yang].
>>>> Would you suggest additional text or an update?
>>> 
>>> This text is fine with respect to the "lower" vs. "higher" question; as I
>>> mentioned above I'd still like to see a bit more about whether the key is
>>> expected to be unique across sessions.
>>> 
>> GIM2>> I've updated this text to state that the key may be unique per test
>> session (see above).
>> 
>>> 
>>>>> 
>>>>> Section 4.4
>>>>> 
>>>>>   In the former case, the Session-Sender MAY not be aware that its
>>>>> 
>>>>> It's unclear that this "MAY" is normative as opposed to descriptive.
>>>>> 
>>>> GIM>> Yes, it should be in descriptive form s/MAY/might/
>>> 
>>> It looks like this didn't make it into the -08?  Ah, because the editor's
>>> copy was attached and hasn't been pushed to the datatracker yet.
>>> 
>>>>> 
>>>>>   Session-Reflector does not support STAMP.  For example, a TWAMP
>>> Light
>>>>>   Session-Reflector may not support the use of UDP port 862 as defined
>>>>>   in [RFC8545].  Thus STAMP Session-Sender MAY use port numbers as
>>>>>   defined in Section 4.  If any of STAMP extensions are used, the
>>> TWAMP
>>>>>   Light Session-Reflector will view them as Packet Padding field.  The
>>>>>   Session-Sender SHOULD use the default format for its timestamps -
>>>>>   NTP.  And it MAY use PTPv2 timestamp format.
>>>>> 
>>>>> Given the above note about not knowing that the peer is TWAMP Light vs.
>>>>> STAMP, it seems that this SHOULD/MAY apply to all STAMP
>>> implementations,
>>>>> not just ones that are interacting with TWAMP Light.  Which in turn
>>> might
>>>>> suggest that the normative statements are best made in a different
>>>>> section.
>>>>> (Also (nit), where do we say that NTP is the default format?)
>>>>> 
>>>> GIM>> We've clarified the default format for timestamp when addressing
>>>> other review comments. Now the draft states in Section 4.2.1:
>>>>      The STAMP Session-Sender and Session-Reflector MUST use the NTP 64
>>>>      bit format of a timestamp (Z field value of 0).  as the default.
>>>> And, as I've mentioned in response to the question above, the draft
>>>> clarifies for PTPv2 format:
>>>>      A configuration/management function MAY configure STAMP Session-
>>>>      Sender and Session-Reflector to using the PTPv2 truncated format
>>>>      of a timestamp (Z field value of 1).
>>>> I hope it is not seen as duplication and the message is consistent.
>>> 
>>> Going from -07 to -08 reduced duplication and improved clarity, so I'm not
>>> too worried about this aspect.
>>> 
>>>> 
>>>>> 
>>>>>   In the latter scenario, if a TWAMP Light Session-Sender does not
>>>>>   support the use of UDP port 862, the test management system MUST set
>>>>>   STAMP Session-Reflector to use UDP port number as defined in
>>>>>   Section 4.  If the TWAMP Light Session-Sender includes Packet
>>> Padding
>>>>>   field in its transmitted packet, the STAMP Session-Reflector will
>>>>>   return the reflected packet of the symmetrical size if the size of
>>>>>   the received test packet is larger than the size of the STAMP base
>>>>>   packet.  The Session-Reflector MUST be set to use the default format
>>>>>   for its timestamps, NTP.
>>>>> 
>>>>> On the other hand, if we take the same approach here, and assume that
>>>>> the Session-Reflector may not know that the Session-Sender is TWAMP
>>>>> Light vs. STAMP, then this MUST would seem to always apply, and thus
>>>>> prevent the Session-Reflector from ever using the PTPv2 timestamp
>>>>> format, in which case the text related to its doing so is "dead code"
>>>>> and should be removed to avoid confusion.
>>>>> 
>>>> GIM>> When we say in the draft that a Session-Sender or Session-Reflector
>>>> "know" something, we imly that that is known to an operator, the one who
>>>> configures, manages the test session. If both entities support STAMP,
>>> then
>>>> the test session may be instantiated using Netconf/YANG and use PTPv2
>>>> format. If only one entity is STAMP-based, then operator may assume that
>>>> the remote node only supprots STAMP and set its system to use NTP format.
>>>> Do you see that reasonable?
>>> 
>>> That's a perfectly reasonable approach to session configuration/management;
>>> my only concern is that the document's text gives a clear and accurate
>>> description thereof.  So perhaps it's better to reword the text(s) about
>>> Session-{Sender,Reflector} being aware of things with a view to the
>>> operator's knowledge as manifested in configuration rather than purely
>>> local knowledge.
>>> 
>> GIM2>> Thank you for your clarification. Below is the update to Section
>> 4.2.1:
>> OLD TEXT:
>>      The STAMP Session-Sender and Session-Reflector MAY use, not use,
>>      or set value of the Z field in accordance with the timestamp
>>      format in use.  This optional field is to enhance operations, but
>>      local configuration or defaults could be used in its place.
>> NEW TEXT:
>>      The STAMP Session-Sender and Session-Reflector MUST use the NTP 64
>>      bits format of a timestamp (Z field value of 0), as the default.
>>      An operator, using configuration/management function, MAY
>>      configure STAMP Session-Sender and Session-Reflector to using the
>>      PTPv2 truncated format of a timestamp (Z field value of 1).  Note,
>>      that an implementation of a Session-Sender that supports this
>>      specification MAY be configured to use PTPv2 format of a timestamp
>>      even though the Session-Reflector is configured to use NTP format.
> 
> That works for me, thanks.  I think some of my IESG colleagues dislike
> constructions of the form "MUST [...] except for $condition", though, so
> perhaps "The default behavior of the STAMP Session-Sender and
> Session-Reflector is to use the NTP 64-bit timestamp format (Z field value
> of 0)" is safer.
> 
> -Ben
> 
>>> 
>>>>> 
>>>>> Section 8.2
>>>>> 
>>>>> RFC 2104 needs to be a normative reference.  The truncation of the HMAC
>>>>> is simple enough that we probably don't need to consider RFC 4868
>>>>> normative just for it, though.
>>>>> 
>>>> GIM>> Agreed and moved to the Normative list though it causes Downref:
>>>> ** Downref: Normative reference to an Informational RFC: RFC 2104
>>> 
>>> RFC 2104 is already listed at https://datatracker.ietf.org/doc/downref/ so
>>> there's no issue with the downref.
>>> 
>>> Thanks,
>>> 
>>> Ben
>>> 
> 
>