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

[Martin Duke <[e-mail]>]

OK, Ben has the token. Thanks.

On Tue, Oct 6, 2020 at 12:32 PM Greg Mirsky <[e-mail]> wrote:

> Hi Martin,
> I've applied the updates suggested by Ben to the working version (attached
> along with the diff). I'll be glad to upload the new version if that helps.
>
> Regards,
> Greg
>
> On Tue, Oct 6, 2020 at 12:04 PM Martin Duke <[e-mail]>
> wrote:
>
>> I believe the current status of this is that Ben is awaiting another
>> version of this draft to resolve his DISCUSS. If anyone has a different
>> understanding, please say so.
>>
>> On Thu, Sep 3, 2020 at 12:44 PM Greg Mirsky <[e-mail]>
>> wrote:
>>
>>> Hi Ben,
>>> thank you for your comments and suggestions, all are much appreciated.
>>> Please find my notes in-lined below tagged by GIM>>. Attached are the diff
>>> and the new working version of the draft.
>>>
>>> Regards,
>>> Greg
>>>
>>> On Tue, Sep 1, 2020 at 8:55 PM Benjamin Kaduk via Datatracker <
>>> [e-mail]> wrote:
>>>
>>>> Benjamin Kaduk has entered the following ballot position for
>>>> draft-ietf-ippm-stamp-option-tlv-09: 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-option-tlv/
>>>>
>>>>
>>>>
>>>> ----------------------------------------------------------------------
>>>> DISCUSS:
>>>> ----------------------------------------------------------------------
>>>>
>>>> Thanks for all the updates; we've made good progress.
>>>>
>>>> I think we're still not converged on the DSCP handling, though.  I have
>>>> a bit more exposition in the COMMENT section, but in short, my
>>>> understanding is that we're setting up a session-reflector to incur
>>>> unbounded levels of risk with hard protocol requirements.  I think we
>>>> need to provide a way to bound that risk, for example by allowing the
>>>> Session-Reflector to selectively choose to treat the CoS TLV as
>>>> unimplemented (set the U flag in its reflected packet) or some other
>>>> mechanism for local policy to filter what DSCP codepoints are set in
>>>> reflected packets (ideally, indicating that the policy made a change).
>>>>
>>> GIM>> Thank you for pointing to the inter-QoS domain scenario. Clearly,
>>> it must be handled with more caution. Using the U flag is a very
>>> interesting approach. But wouldn't the Session-Sender skip the CoS TLV in
>>> the reflected packet? As a result, we may not retrieve information about
>>> the CoS mapping in the forward direction, i.e., towards the
>>> Session-Reflector. And it might be difficult for an operator to recognize
>>> whether the Session-Reflector doesn't support the particular CoS or STAMP
>>> extensions altogether. I would propose a new field being added in the CoS
>>> TLV:
>>>
>>>    -    RP (Reverse Path) - is a two-bit-long field.  A Session-Sender
>>>       MUST set the value of the RP field to 0 on transmission.
>>>
>>> And further in the section another update:
>>>    A STAMP Session-Reflector that receives a test packet with the CoS
>>>    TLV MUST include the CoS TLV in the reflected test packet.  Also, the
>>>    Session-Reflector MUST copy the value of the DSCP and ECN fields of
>>>    the IP header of the received STAMP test packet into the DSCP2 field
>>>    in the reflected test packet.  Finally, the Session-Reflector MUST
>>>    use the local policy to verify whether the CoS corresponding to the
>>>    value of the DSCP1 field is permitted in the domain.  If it is, the
>>>    Session-Reflectorset MUST set the DSCP field's value in the IP header
>>>    of the reflected test packet equal to the value of the DSCP1 field of
>>>    the received test packet.  Otherwise, the Session-Reflector MUST use
>>>    the DSCP value of the received STAMP packet and set the value of the
>>>    RP field to 1.  Upon receiving the reflected packet, if the value of
>>>    the RP field is 0, the Session-Sender will save the DSCP and ECN
>>>    values for analysis of the CoS in the reverse direction.  If the
>>>    value of the RP field in the received reflected packet is 1, only CoS
>>>    in the forward direction can be analyzed.
>>>
>>> Would these updates address your concern?
>>>
>>>>
>>>> Also, there's a bit of fallout from the flags reworking that's left to
>>>> cleanup in Section 4: we now have the Session-Sender set the U flag to
>>>> 1, so this text no longer makes sense:
>>>>
>>>> % A STAMP system, i.e., either a Session-Sender or a Session-Reflector,
>>>> % that has received a STAMP test packet with extension TLVs MUST
>>>> % validate each TLV:
>>>> %
>>>> %    If the U flag is set, the STAMP system MUST skip the processing of
>>>> %    the TLV.
>>>>
>>>> I think it should just apply to the Session-Sender for this case -- the
>>>> Session-Reflector doesn't need to check the received U flag, since the
>>>> Session-Sender will not be generating TLVs it does not understand.
>>>> (Whether or not to keep the behavior for the M and I flags as applying
>>>> to both Session-Sender and Session-Reflector vs. just Session-Sender
>>>> does not immediately seem to be of much consequence.
>>>>
>>> GIM>> Thank you for catching this. Indeed, that brakes STAMP with
>>> extensions. I think that though checking M and I flags of a
>>> Session-Reflector should not do any harm, it doesn't seem to have any
>>> benefits. Hence we may remove the Session-Reflector from the sentence and
>>> the updated introduction to the flag processing reads like below:
>>>    A STAMP Session-Sender that has received a reflected STAMP test
>>>    packet with extension TLVs MUST validate each TLV:
>>>
>>>>
>>>>
>>>> ----------------------------------------------------------------------
>>>> COMMENT:
>>>> ----------------------------------------------------------------------
>>>>
>>>> My most significant remaining comments are on the Security
>>>> Considerations (Section 6):
>>>>
>>>> The security of the HMAC mechanism is not complete, since it is
>>>> susceptible to replay attack.  As such, when HMAC is in use, it is
>>>> important to check that the received sequence numbers are (at least
>>>> mostly) monotonic and to detect replays.  While replayed packets do not
>>>> always indicate an attack (depending on the network technology) they are
>>>> still a noteworthy condition, and we should say something about whether
>>>> we expect to produce a response to each received instance or to suppress
>>>> replies to replayed input.
>>>>
>>> GIM>> Is there a reliable mechanism to distinguish between an
>>> out-of-order duplicated packet and a replayed packet? I've checked RFC 5357
>>> TWAMP for any suggestion and believe that there is no special consideration
>>> by a Session-Reflector for an out-of-order duplicated packet. The TWAMP
>>> Session-Reflector does not monitor whether the sequence numbers of the
>>> received test packets are in the monotonically increasing sequence. STAMP
>>> is designed to be comparable with TWAMP and have some level of
>>> interoperability. Hence, discarding what appears as a replayed packet at a
>>> STAMP Session-Reflector might be less desirable behavior. If we follow
>>> TWAMP's behavior, what else can be done at the Session-Reflector to
>>> strengthen the security?
>>>
>>>>
>>>>    Monitoring and optional control of DSCP do not appear to introduce
>>>>    any additional security threat to hosts that communicate with STAMP
>>>>    as defined in [RFC8762].  As this specification defined the mechanism
>>>>    to test DSCP mapping, this document inherits all the security
>>>>
>>>> I'm afraid I still don't understand the reasoning here.  In my
>>>> understanding, the risk stems from the semantics of the DCSP field being
>>>> (for at least some codepoints) site-local, there not being a guarantee
>>>> that the session-sender and session-reflector are on the same network
>>>> (and thus, using the same DSCP semantics), and the hard requirement for
>>>> the Session-Reflector to set the DCSP value indicated by the
>>>> Session-Sender.  A mechanism for a remote entity to induce generation of
>>>> local packets with unspecified semantics is a risk that cannot be
>>>> qualified at protocol-design time, since the possible outcomes are
>>>> inherently unspecified.  This is analogous to the situation with
>>>> undefined behavior in programming languages like C -- the programmer is
>>>> flat-out required to avoid it, because literally anything could go wrong
>>>> if undefined behavior is triggered.
>>>>
>>> GIM>> I agree that the inter-QoS domain case requires more consideration
>>> in the Security section. I propose the updated text below:
>>>
>>>    As this specification defined the mechanism to test DSCP mapping,
>>>    this document inherits all the security considerations discussed in
>>>    [RFC2474].  Monitoring and optional control of DSCP using the CoS TLV
>>>    may be used across the Internet so that the Session-Sender and the
>>>    Session-Reflector are located in domains that use different CoS
>>>    profiles.  Thus, it is essential that an operator verifies the set of
>>>    CoS values that are used in the Session-Reflector's domain.  Also, an
>>>    implementation of a Session-Reflector SHOULD support a local policy
>>>    to confirm whether the value sent by the Session-Sender can be used
>>>    as the value of the DSCP field.  Section 4.4 defines the use of that
>>>    local policy.
>>>
>>>
>>>
>>>>
>>>> This is especially risky when there is the possibility for the
>>>> Session-Reflector to act on packets that do not have any form of
>>>> authentication (i.e., could be spoofed from off-path).  But we do not
>>>> mention this risk at all, let alone give guidance on its mitigation.
>>>> (Discussion of the security considerations of unauthenticated operation
>>>> would ideally be generalized to all actions/TLVs that have side effects,
>>>> not just the specific case of setting the DSCP codepoint.)
>>>>
>>> GIM>> It seems very challenging for a STAMP system to differentiate
>>> between a duplicate and/or out-of-order test packet and a replayed packet
>>> because one of the metrics STAMP test measures is the network re-ordering
>>> metric per RFC 4737 <https://tools.ietf.org/html/rfc4737>. We may
>>> recommend that the rate-limiting of test packets be selected
>>> conservatively.
>>>
>>>>
>>>> Section 4.2
>>>>
>>>> I'd suggest saying that all fields that are not filled are transmitted
>>>> with all bits set to zero.
>>>>
>>> GIM>> Thank you for the helpful suggestion. Appended Section 4.2:
>>>    Note that all fields not filled by either a Session-Sender or
>>>    Session-Reflector are transmitted with all bits set to zero.
>>>
>>>>
>>>> Section 4.2.1
>>>>
>>>>    o  Source MAC Address sub-TLV - is a 12-octet-long sub-TLV.  The Type
>>>>       value is TBA9.  The value of the Length field MUST equal to 8.
>>>>       The Value field is a 12-octet-long MBZ field that MUST be zeroed
>>>>       on transmission and ignored on receipt.
>>>>
>>>> Value should be 8-octets-long, no?
>>>>
>>> GIM>> Thank you for catching it! You are absolutely correct. Fixed this
>>> and a similar in:
>>>    o  Source EUI-64 Address sub-TLV - is a 12-octet-long sub-TLV that
>>>       includes the EUI-64 source MAC address.  The Type value is TBA11.
>>>       The value of the Length field MUST equal to 8.  The Value field
>>>       consists of an eight-octet-long EUI-64 field.
>>>
>>>
>>>
>>>> Section 4.2.2
>>>>
>>>>    A Session-Sender MAY include the Source MAC Address sub-TLV is the
>>>>    Location TLV.  If the Session-Reflector receives the Location TLV
>>>>
>>>> nit: s/is/in/
>>>>
>>> GIM>> Thank you. Fixed (and three more places in the same sub-section).
>>>
>>>>
>>>> Section 4.3
>>>>
>>>>    MUST NOT fill any information fields except for STAMP TLV Flags,
>>>>    Type, and Length.  All other fields MUST be filled with zeroes The
>>>>
>>>> nit: missing full stop.
>>>>
>>> GIM>> Done.
>>>
>>