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: Tue, 1 Sep 2020 12:25:02 -0700

Hi Ben,
I hope you're well.
All the changes we've discussed have been included in the new version of
the draft
<https://datatracker.ietf.org/doc/draft-ietf-ippm-stamp-option-tlv/>. Could
you please let me know whether all your DISCUSSes have been satisfactory
addressed?

Best regards,
Greg

On Wed, Aug 19, 2020 at 11:49 AM Greg Mirsky <[email-masked]> wrote:

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

---

From: Benjamin Kaduk <[email-masked]>
Date: Tue, 1 Sep 2020 20:53:05 -0700

Hi Greg,

Thanks for the updates;

> > On Thu, Jul 16, 2020 at 05:38:08PM -0700, Greg Mirsky wrote:
[...]
> Would the updated text be acceptable?

In light of the long delay (entirely my fault) in responding, I've prepared
a new ballot position to cover this topic and the other notes I made from
my re-review.  But in short: I can accept the weaker form of mandatory
behavior that you propose here, but we should still clearly document the
considerations that come into play for unauthenticated operation.

Thanks, and sorry again for the slow response,

Ben

> 
> >
> > I think the idea was to require the HMAC TLV to accompany those TLVs that
> > Information TLVs would be considered to contain such "control information",
> > though on reread perhaps the Timestamp Information is not always considered
> > (since it affects the headers of the containing IP packet and consequently