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

[Greg Mirsky <[e-mail]>]

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 <[e-mail]> 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 <[e-mail]> 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 <[e-mail]> 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 <
>> > > [e-mail]> 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
>> >
>> > > >
>> > > >
>>
>