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

Greg Mirsky <gregimirsky@gmail.com> Wed, 16 October 2019 23:44 UTC

Return-Path: <gregimirsky@gmail.com>
X-Original-To: ippm@ietfa.amsl.com
Delivered-To: ippm@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 4D6AB120914; Wed, 16 Oct 2019 16:44:49 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -0.596
X-Spam-Level:
X-Spam-Status: No, score=-0.596 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, FREEMAIL_FROM=0.001, HTML_COMMENT_SAVED_URL=1.391, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_HTML_ATTACH=0.01, URIBL_BLOCKED=0.001] autolearn=no autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=gmail.com
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id BV5NMMvCXUSE; Wed, 16 Oct 2019 16:44:39 -0700 (PDT)
Received: from mail-lf1-x12f.google.com (mail-lf1-x12f.google.com [IPv6:2a00:1450:4864:20::12f]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 72E10120086; Wed, 16 Oct 2019 16:44:38 -0700 (PDT)
Received: by mail-lf1-x12f.google.com with SMTP id t8so288019lfc.13; Wed, 16 Oct 2019 16:44:38 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=Li/CiIKz84rZ9Isn6HZDUGTDyXD3nbgLwkl8FGasqrg=; b=Wed3I/vgm7Xdgw/VoIjXpUBhqEnRlTwzn0GbvltmbhZGg1IeNkHtjlK92CZbh5uDcN /WcD8HgGElmSHCUO5hUTXVx0W63xIFgIXeKDaPGzN6C54CPyMtQuvDgUpt1PxrPDTmc/ QaN3QZOPAOZ01bv9ui3ryXExH5ECSiQAiOgSJXV8eshudoQj4w6lZV84CRoyns3OE12S rQLG7povnj0f0rjM+qbAEFec191TwRtxI8AUZPkFm8iq78wh6NX1TY0C1DGun5ZtDo+R u4Q5KmD0riS5Fc2U6rDIgKGMNxmDzpOF9aIsYtAvgyOjG9iN0feH+KnJKYeQ/QkgzDb3 ykHw==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=Li/CiIKz84rZ9Isn6HZDUGTDyXD3nbgLwkl8FGasqrg=; b=RAAm9G2tNoyfYlbneeFDHXWJKrd91kkhhZh+BdvVD5k1fXPHluG6wj4M/a3w6VjF0E 8SnTjactAv0IInv2OUB3W1Tnu0UyToxdAEf8kMbdO+I0nNsvNGhNldJaI5T93q27csnq BC2zBYHaNTuBmphQfNNjD9iD3WVzA3fpm9FiXrUPquUi4vvY0E9Mbr8vZUIr1I1rnsYe Ch6kTMFw3UcQaBpt6NvZ51WSL7OWRgtJqbi2iFnkqGARPltjp4aBhkdXoGUXC0dHhbm3 nXTc55EMfOti2ZCMoktgsJH4OKO4nkaTeWl5g1tijc66fdH3SyMcSV0ChzRPm8++hL4T WB2Q==
X-Gm-Message-State: APjAAAVbmF7/eNx12zTVm4kZYSMaCTA8ZEnftgCBP2OdZNHaCcQlvYkJ aQH0mcJyiAZa3ITNWRrLOu0etB32lSouYj7PI0Q=
X-Google-Smtp-Source: APXvYqzm3vscZjMT39nwMrRBPxmf9xsY1EVOYCPKG3iWeeoRnomo0lg64Bcm0YuWZ28KSjKu4IwtkejZVjG0T0nHAAE=
X-Received: by 2002:ac2:46e3:: with SMTP id q3mr192285lfo.147.1571269476304; Wed, 16 Oct 2019 16:44:36 -0700 (PDT)
MIME-Version: 1.0
References: <156764462100.22846.16937322291769285829.idtracker@ietfa.amsl.com> <CA+RyBmWQ9VgPe27gdrF0_7sdhWMwDTAMtYk6EUYiO9tQBKv4_w@mail.gmail.com> <20191015155618.GL61805@kduck.mit.edu>
In-Reply-To: <20191015155618.GL61805@kduck.mit.edu>
From: Greg Mirsky <gregimirsky@gmail.com>
Date: Wed, 16 Oct 2019 16:44:25 -0700
Message-ID: <CA+RyBmUFKCb7=AyBNFQTyhNhu+CtLuzEwjqPknLR6_A-BSap5A@mail.gmail.com>
To: Benjamin Kaduk <kaduk@mit.edu>
Cc: The IESG <iesg@ietf.org>, draft-ietf-ippm-stamp@ietf.org, Tal Mizrahi <tal.mizrahi.phd@gmail.com>, IPPM Chairs <ippm-chairs@ietf.org>, IETF IPPM WG <ippm@ietf.org>
Content-Type: multipart/mixed; boundary="000000000000fb667b05950faeb1"
Archived-At: <https://mailarchive.ietf.org/arch/msg/ippm/9BUj0kpbosNsUhnB0Gvp6LZZxAA>
X-Mailman-Approved-At: Mon, 21 Oct 2019 09:46:03 -0700
Subject: Re: [ippm] Benjamin Kaduk's Discuss on draft-ietf-ippm-stamp-07: (with DISCUSS and COMMENT)
X-BeenThere: ippm@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: IETF IP Performance Metrics Working Group <ippm.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/ippm>, <mailto:ippm-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/ippm/>
List-Post: <mailto:ippm@ietf.org>
List-Help: <mailto:ippm-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/ippm>, <mailto:ippm-request@ietf.org?subject=subscribe>
X-List-Received-Date: Wed, 16 Oct 2019 23:44:50 -0000

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 <kaduk@mit.edu> 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 <
> > noreply@ietf.org> 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).
>
>
> > >
> > > 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].


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

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


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


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

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.

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