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

[Benjamin Kaduk <[e-mail]>]

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