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

From: Greg Mirsky <[email-masked]>
Date: Wed, 19 Aug 2020 11:49:57 -0700

Hi Ben,
I hope this note finds you well.
I much appreciate the suggestions you've kindly shared in the course of our
discussion. All the updates have been applied and the new version
<https://datatracker.ietf.org/doc/draft-ietf-ippm-stamp-option-tlv/> of the
document has been published. Could you please review the changes and let me
know whether your DISCUSSes have been addressed?

Regards,
Greg

On Wed, Jul 22, 2020 at 3:24 PM Greg Mirsky <[email-masked]> wrote:

> Hi Ben,
> thank you for the clarifications and suggested text that improves the
> document. Please find my answers and notes below tagged GIM2>>.
> Attached is the new working version and its diff to -07 (including the
> new Location TLV section).
>
> Regards,
> Greg
>
> PS. I've clipped the COMMENTS as these now have the discussion thread
> of their own.
>
> On Thu, Jul 16, 2020 at 7:48 PM Benjamin Kaduk <[email-masked]> wrote:
> >
> > Hi Greg,
> >
> > On Thu, Jul 16, 2020 at 05:38:08PM -0700, Greg Mirsky wrote:
> > > Hi Benjamin,
> > > thank you for your thorough review and insightful comments (and
> DISCUSSes,
> > > of course).
> > > I hope it will be okay if I start with addressing DISCUSSes and split
> > > comments resolution into a separate thread(s).
> >
> > Of course!
> >
> > > Hence, I've added my answers and notes in-line under tag GIM>>.
> > >
> > > Regards,
> > > Greg
> > >
> > > On Tue, Jul 14, 2020 at 7:30 PM Benjamin Kaduk via Datatracker <
> > > [email-masked]> wrote:
> > >
> > > > Benjamin Kaduk has entered the following ballot position for
> > > > draft-ietf-ippm-stamp-option-tlv-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-option-tlv/
> > > >
> > > >
> > > >
> > > >
> ----------------------------------------------------------------------
> > > > DISCUSS:
> > > >
> ----------------------------------------------------------------------
> > > >
> > > > I support Roman's discusses and am happy to see the ongoing
> discussion
> > > > thereof.
> > > >
> > > > (1) I think there's a conflict between this document and RFC 8762
> with
> > > > respect to the behavior of pure RFC 8762 implementations that receive
> > > > packets longer than the base packet for the given operational mode.
> > > >
> > > > RFC 8762 says (Section 4.3):
> > > >
> > > > % The Session-Reflector receives the STAMP-Test packet and verifies
> it. If
> > > > % the base STAMP-Test packet is 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).
> > > >
> > > > But Section 4 of this document says:
> > > >
> > > >                                                    A
> Session-Reflector
> > > >    that does not support STAMP extensions is not expected to compare
> the
> > > >    value in the Length field of the UDP header and the length of the
> > > >    STAMP base packet.  Hence the Session-Reflector will transmit the
> > > >    base STAMP packet.  [...]
> > > >
> > > > Does "will transmit the base STAMP packet" mean something other than
> > > > "with the exact length of the base packet [for the given operational
> > > > mode]"?
> > > >
> > > GIM>> Thank you for pointing out the text in RFC 8672. I agree that
> text in
> > > the draft requires re-work. Would the following update make it
> consistent
> > > with RFC 8762 (I will accept your suggestion to change the use of the U
> > > flag so that the Session-Sender sets it to 1 on the transmission. Thus
> if
> > > the Session-Sender receives a TLV flag set to 1 in the reflected
> packet it
> > > MUST skip the TLV):
> > > OLD TEXT:
> > >    A Session-Reflector
> > >    that does not support STAMP extensions is not expected to compare
> the
> > >    value in the Length field of the UDP header and the length of the
> > >    STAMP base packet.  Hence the Session-Reflector will transmit the
> > >    base STAMP packet.  It is the local policy on the Session-Sender
> > >    (similar to the handling of SSID == 0 scenario described in
> > >    Section 3) that will control the sender's behavior.
> > > NEW TEXT:
> > >    A Session-Reflector
> > >    that does not support STAMP extensions will not process but copy
> them
> > >    into the reflected packet, as defined in Section 4.3 [RFC8762].  The
> > >    Session-Sender receives unprocessed TLV indicated by the U flag
> being
> > >    set to 1.
> >
> > This should make us consistent with RFC 8762, yes.  I think it might be
> > worth reminding the reader that the Session-Sender will know if it is
> > talking to a Session-Reflector that does not support STAMP extensions,
> > because such Session-Reflectors will send a zeroed SSID, though I guess
> > that behavior is already mentioned a couple sentences later, so maybe it
> > would be redundant here.
> >
> > I'd suggest rewording the last sentence (of the new text) to someething
> > like "a Session-Sender that supports TLVs will indicate specific TLVs
> that
> > it did not process by setting the U flag to 1 in those TLVs", to help
> > reinforce that the U flag will only be set by a Session-Reflector that
> > supports extensions/this document at all.
>
> GIM2>> Thank you for the suggested text Accepted and it reads as:
> NEW TEXT:
>    A Session-Reflector
>    that does not support STAMP extensions will not process but copy them
>    into the reflected packet, as defined in Section 4.3 [RFC8762].  A
>    Session-Reflector that supports TLVs will indicate specific TLVs
>    that it did not process by setting the U flag to 1 in those TLVs.
> >
> > > >
> > > > (2) As I remarked on (then-) draft-ietf-ippm-stamp, 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, and specifically, that the HMAC TLV must be
> > > > used, since the base authenticated STAMP packet's HMAC does not cover
> > > > the options.
> > > >
> > > GIM>> The use of the HMAC TLV is required in the authenticated mode
> STAMP:
> > >    The keyed Hashed
> > >    Message Authentication Code (HMAC) TLV MUST be included in a STAMP
> > >    test packet in the authenticated mode, excluding the case where the
> > >    only TLV present is the Extra Padding TLV.
> > > We can require that in unauthenticated STAMP mode some TLVs be
> accompanied
> > > by the HMAC TLV. Would mitigate the exposure of the Session-Reflector?
> > > Which TLVs, in your opinion, require such a requirement?
> GIM2>> I have some reservations to say that a particular TLV MUST be
> accompanied by the HMAC TLV. If a domain secured, an operator may use
> a TLV, e.g., CoS TLV, without authentication. I think that the current
> text:
>    The HMAC TLV MAY be used to protect the integrity of STAMP extensions
>    in STAMP unauthenticated mode.
> can be expanded to require that an implementation can enable
> authentication of extensions via the management plane. It might be as:
> NEW TEXT:
>    The HMAC TLV MAY be used to protect the integrity of STAMP extensions
>    in STAMP unauthenticated mode.  An implementation of STAMP extensions
>    MUST provide controls to enable the integrity protection of STAMP
>    extensions in STAMP unauthenticated mode.
> Would the updated text be acceptable?
>
>
> >
> > I think the idea was to require the HMAC TLV to accompany those TLVs that
> > represent "control information", yes.  My initial read of the document
> > (reflected in my longer comments) says that the Location and Timestamp
> > Information TLVs would be considered to contain such "control
> information",
> > though on reread perhaps the Timestamp Information is not always
> considered
> > sensitive.  I'm on the fence about Location, and I think that Class of
> > Service is pretty clearly on the "control information" side of things
> > (since it affects the headers of the containing IP packet and
> consequently
> > the behavior of the network).
>
> GIM2>> I agree that CoS is very close to what we consider "control
> information" as it includes information that affects how the reflected
> test packet is transmitted. But as in RFC 8762 itself, HMAC TLV is
> defined as optional in STAMP unauthenticated mode and, as I think of
> it, that leaves the choice to an operator whether to protect the
> integrity of the CoS TLV or not. Do you think that such arrangement is
> acceptable?
>
> >
> > > >
> > > > (3) The secdir reviewer's question about dealing with 6-to-4 gateways
> > > > seems to have not gotten a response.  Specifically, the requirement
> that
> > > > "[t]he Session-Reflector MUST validate the Length value against the
> > > > address family of the transport encapsulating the STAMP test packet"
> > > > seems to require the protocol to fail when sender and reflector use
> > > > different address families, or perhaps to require the sender to use
> > > > trial and error to determine which address family is used by the
> > > > reflector.  Some clarification on the intended operation in such
> > > > scenarios seems appropriate.
> > > >
> > > GIM>> I've misunderstood the question and thank you for clarifying the
> > > scenario. I now see that IP addresses in the Location TLV must allow
> > > explicit signaling of the address family while providing space for
> IPv6.
> > > So, we introduce sub-TLVs and a new sub-registry for IANA. Does that
> sound
> > > reasonable? I'll work on details if you agree with the proposed
> approach.
> >
> > That sounds like it would work.
> >
> > > >
> > > > (4) The ability for a Session-Sender to (MUST-level!) control the
> DSCP
> > > > codepoint used by packets generated by a Session-Reflector feels
> like it
> > > > opens up significant risk in site-local (security-relevant) policy.
> That
> > > > is, the interpretation of the DSCP codepoints is to large extent
> > > > site-specific, and allowing a nominally external system to set
> any/all
> > > > possible values, without a chance for site policy to be applied and
> > > > block the use of potentially disruptive DSCP values.  So I think we
> need
> > > > to modify the "MUST set", perhaps requiring that either the requested
> > > > DSCP value is used or the entire TLV/packet/whatever is rejected.
> > > >
> > > GIM>> Perhaps we can add a note in the security section that an
> > > implementation must control the support of each TLV. In other words, a
> > > particular TLV can be disabled for the given test session.
> Alternatively,
> > > the CoS TLV might require the use of the HMAC TLV. I think that the
> first
> > > option is sufficient. What do you think? Or would you suggest an
> entirely
> > > different approach?
> >
> > I think my previous replies here combine to say that use of the CoS TLV
> > should require ues of the HMAC TLV, but I think it's also good to let an
> > implementation control the support for each TLV as you propose.  My
> > intuition is still saying that a separate local policy filter should be
> > available, though -- just because the peer is authenticated does not
> > inherently grant them privilege to set arbitrary DSCP values.  In some
> > sense this is the classic distinction between authentication and
> > authorization.  In some networks, setting some DSCP values might require
> > more privilege than setting other such values, and the simple
> > "authenticated or not" knob provided by the HMAC TLV doesn't allow us to
> > express that distinction.  (I'm thinking about scenarios where setting a
> > particular DSCP value privileges the enclosed packet to such a degree
> that
> > other traffic on the network suffers significantly as a result -- a "drop
> > everything and move this packet!" bit, if you will.)
> GIM2>> Yes, I agree that the CoS TLV might become another loaded gun
> ith which an operator shoots himself/herself in the foot. Well, one
> more in a large arsenal we've provided them over the years. I envision
> that the CoS TLV might be very useful during the service activation
> testing and then exercising that "drop everything and move this
> packet!" marking seems reasonable. On the other hand, using the same
> marking as part of in-service testing requires analysis and
> consideration. Do you think that additional text to explain the
> possible impact of CoS TLV on the production network is needed?
> >
> > > >
> > > > (5) If we're not going to remedy the severability of authenticated
> > > > options from authenticated base packets (which would be my preferred
> > > > resolution), we need to document that weakness in the security
> > > > considerations.
> > > >
> > > GIM>> I think that the document does require HMAC TLV is used in the
> > > authenticated mode.  Please see my comments to DISCUSS #2. Would you
> agree?
> >
> > I think my point didn't make it through properly, sorry.
> >
> > The issue is not that we need to authenticate both the "base part" and
> the
> > "extensions part", but rather that we need a way to say that "base part
> of
> > packet 1" and "extensions part of packet 1" go together.  Right now, with
> > two separate HMACs, the Session-Reflector would happily process a packet
> > that has "base part of packet 1" and "extensions part of packet 2" and
> not
> > even notice that anything was wrong.  (A similar thing could be done to
> > responses processed at the Session-Sender, of course.)  That is, I can
> > freely "mix and match" authenticated base parts and authenticated
> > extensions parts (as an on-path attacker).  Including as input to one of
> > the HMAC calls a unique value from the other part of the packet will be
> > enough to tie the two together, and let the recipient detect the
> > modification.  The most promising candidates for such a "unique
> per-packet
> > value" would be the HMAC output itself and the packet sequence number.
> > Using the HMAC value itself is a more robust cryptographic construction
> (in
> > that it authenticates the entire other part by reference), but I suspect
> > that we'll have to use the sequence number due to implementation
> > (performance) concerns -- needing to wait for the first HMAC before
> > extensions can be finalized seems like it would introduce unwanted delay
> in
> > the timestamping.  Assuming that implementations actually check the
> > sequence number for repeats, that should still provide enough protection.
> >
> > Thanks,
> >
> > Ben
> >
> > > >
> > > >
>