Re: [secdir] Secdir review of draft-ietf-ippm-stamp-option-tlv-06

[Roman Danyliw <[e-mail]>]

Hi Charlie, thank you for your review.

Hi Greg, thank you for your response to the SECDIR review.

I’ve entered a discuss ballot to cover privacy consideration and to clarify the keying of the two HMACs.

Regards,
Roman

From: secdir <[e-mail]> On Behalf Of Greg Mirsky
Sent: Thursday, July 2, 2020 3:30 PM
To: Charlie Kaufman <[e-mail]>
Cc: [e-mail]; [e-mail]; [e-mail]
Subject: Re: [secdir] Secdir review of draft-ietf-ippm-stamp-option-tlv-06

Hi Charlie,
you might be interested to know that another case of using ICMP in our document was pointed out in LC comments. That was used to report a failure of HMAC authentication verification. The authors discussed it and have agreed to use a new STAMP TLV Flag. You can find updates (also addressing GenART IETF LC comments by Dan Romascanu) in the attached copies of the working version of the draft and the diff to -06.
I greatly appreciate your consideration and feedback.

Regards,
Greg

On Mon, Jun 29, 2020 at 1:19 PM Greg Mirsky <[e-mail]<mailto:[e-mail]>> wrote:
Hi Charlie,
thank you for your thorough review, detailed questions, and thoughtful suggestions. We've discussed your comments and propose changes that are described below. Also, you can find the changes highlighted in the attached diff between the -06 and the new working version of the document. Please find my answers, notes in-line below under the GIM>> tag.

Regards,
Greg

On Fri, Jun 26, 2020 at 10:34 PM Charlie Kaufman <[e-mail]<mailto:[e-mail]>> wrote:
I have reviewed this document as part of the security directorate's ongoing effort to review all IETF documents being processed by the IESG.  These comments were written primarily for the benefit of the security area directors.  Document editors and WG chairs should treat these comments just like any other last call comments.

This I-D specifies extensions to the STAMP protocol defined in RFC 8762. The STAMP protocol runs over UDP and is similar to an "echo" protocol intended to be used to test network speed and bandwidth. This I-D specifies a series of TLV encoded extensions that can be appended to the end of the UDP packets of the STAMP protocol (as well as defining the TLV mechanism itself).

I could not find anything objectionable from a security standpoint in this extension mechanism. It is a bit odd that when extensions are added to an HMAC protected packet, instead of adding the extensions before the HMAC and having the HMAC protect the entire contents, they keep the existing HMAC that protects the base packet and add a second HMAC at the end of the extensions that protects only the extensions. This would potentially allow an attacker to "mix and match" the extensions from one packet with the base content of a different packet, but it's not clear what an attacker might be able to gain by doing that.
GIM>> I think that the STAMP YANG data model could be used to minimize the possible attack vector you've described. We will certainly keep that as one of the objectives when updating draft-ietf-ippm-stamp-yang<https://datatracker.ietf.org/doc/draft-ietf-ippm-stamp-yang/>. Also, HMAC TLV might be required even in unauthenticated STAMP mode to protect the integrity on a certain TLV(s). Do you see this as a useful scenario or unnecessary feature?

One of the pieces of information an extension can request is the 6 byte MAC address of the incoming packet as seen by the peer (which will normally by the MAC address of the last router on the path). That information is not normally available to someone on a distant network, but it's not obvious what harm could come from their knowing it.
GIM>> As mentioned above, HMAC TLV may follow some STAMP TLVs. Do you think that the document should recommend using HMAC TLV with the Location TLV?

The protocol can also request the source and destination IP addresses as seen by the peer (presumably to detect NAT gateways), but the protocol assumes the originator will know whether the peer will see IPv4 or IPv6 addresses, so dealing with 6-to-4 gateways could be tricky.

On page 19 table 2, this document asks IANA to assign extension type codes. In all other documents of this sort that I have seen, the document assigns the codes and asks IANA to track subsequent additions.
GIM>> Martin Duke, the Responsible AD for IPPM WG, asked this question in his review. He has agreed to leave the text as-is since no one asked for a particular number.

On some kinds of errors, the protocol specifies that the error is reported via and ICMP message. For others, it specifies that a field be returned containing all zeros. It's probably better to report errors "in band" because so many routers swallow ICMP messages. The spec does not - and should - specify what to do if the length of the TLV coded extensions does not match the length of the UDP packet containing them (or at least I couldn't find where it was specified).
GIM>> Thank you for pointing out this awkward handling. We had continued discussion with Martin Duke and Rakesh Gandhi on improving the mechanism of reporting TLV errors. The discussion is in the updated Section 4 of the new working version and is copied below:
4.  TLV Extensions to STAMP

   Type-Length-Value (TLV) encoding scheme provides a flexible extension
   mechanism for optional informational elements.  TLV is an optional
   field in the STAMP test packet.  Multiple TLVs MAY be placed in the
   STAMP test packet.  A TLV MAY be enclosed in a TLV.  TLVs have the
   two octets long Type field, two octets long Length field that is
   equal to the length of the Value field in octets.  If a Type value
   for TLV or sub-TLV is in the range for Vendor Private Use, the Length
   MUST be at least 4, and the first four octets MUST be that vendor's
   the Structure of Management Information (SMI) Private Enterprise
   Codes, as recorded in IANA's SMI Private Enterprise Codes sub-
   registry, in network octet order.  The rest of the Value field is
   private to the vendor.  The following sections describe the use of
   TLVs for STAMP that extends STAMP capability beyond its base
   specification.

       0                   1                   2                   3
       0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |U|L|R|R|R|R|R|R|     Type      |           Length              |
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      ~                            Value                              ~
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

              Figure 5: TLV Format in a STAMP Extended Packet

   where fields are defined as the following:

   o  U - a one-bit flag, located as specified in Section 5.2.  A
      Session-Sender MUST set the U flag to 0 before transmitting an
      extended STAMP test packet.  A Session-Reflector MUST set the U
      flag to 1 if the Session-Reflector has not understood the TLV.

   o  L - a one-bit flag, located as specified in Section 5.2.  A
      Session-Sender MUST set the L flag to 0 before transmitting an
      extended STAMP test packet.  A Session-Reflector MUST set the L
      flag to 1 if the Session-Reflector determined the TLV is
      malformed, i.e., the Length field value of the fixed-size TLV is
      not equal to the value defined for the particular type, or the
      remaining length of the extended STAMP packet is less than the
      size of the TLV.

   o  R - reserved flags for future use.  These flags MUST be zeroed on
      transmit and ignored on receipt.

   o  Type - one-octet long field that characterizes the interpretation
      of the Value field.  It is allocated by IANA, as specified in
      Section 5.1

   o  Length - two-octets long field equals length on the Value field in
      octets.

   o  Value - a variable-length field.  Its interpretation and encoding
      determined by the value of the Type field.

   A STAMP node, whether Session-Sender or Session-Reflector, receiving
   a test packet MUST determine whether the packet is a base STAMP
   packet or includes one or more TLVs.  The node MUST compare the value
   in the Length field of the UDP header and the length of the base
   STAMP test packet in the mode, unauthenticated or authenticated based
   on the configuration of the particular STAMP test session.  If the
   difference between the two values is larger than the length of UDP
   header, then the test packet includes one or more STAMP TLVs that
   immediately follow the base STAMP test packet.

   A system 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.  The implementation MUST try to process the next TLV if
      present in the extended STAMP packet.

      if the L flag is set, the STAMP system MUST stop processing the
      remainder of the extended STAMP packet.

      if an implementation of a Session-Reflector does not recognize the
      Type field value, it MUST include the copy of the TLV into the
      reflected STAMP packet.  The Session-Reflector MUST set the U flag
      to 1.  The Session-Reflector MUST try to process the next TLV in
      the extended STAMP packet;

      if a TLV is malformed, the processing of extension TLVs MUST be
      stopped.  The Session-Reflector MUST copy the remainder of the
      received extended STAMP packet into the reflected STAMP packet.
      The Session-Reflector MUST set the L flag to 1.

   Detected error events MUST be logged.  Note that rate of logging MUST
   be controlled.

Please note, that IANA is also requested to create a STAMP TLV Flags sub-registry.
We hope this is an acceptable solution. Always welcome your questions, comments, and suggestions.


--Charlie