Last Call Review of draft-ietf-ippm-stamp-option-tlv-06
review-ietf-ippm-stamp-option-tlv-06-secdir-lc-kaufman-2020-07-02-00

Request Review of draft-ietf-ippm-stamp-option-tlv
Requested rev. no specific revision (document currently at 10)
Type Last Call Review
Team Security Area Directorate (secdir)
Deadline 2020-07-06
Requested 2020-06-22
Authors Greg Mirsky, Xiao Min, Henrik Nydell, Richard Foote, Adi Masputra, Ernesto Ruffini
Draft last updated 2020-07-02
Completed reviews Secdir Last Call review of -06 by Charlie Kaufman (diff)
Genart Last Call review of -06 by Dan Romascanu (diff)
Assignment Reviewer Charlie Kaufman 
State Completed
Review review-ietf-ippm-stamp-option-tlv-06-secdir-lc-kaufman-2020-07-02
Posted at https://mailarchive.ietf.org/arch/msg/secdir/sAuSpk9QCe7BW1Gmac40B-aLLSQ/
Reviewed rev. 06 (document currently at 10)
Review result Has Issues
Review completed: 2020-06-26

Review
review-ietf-ippm-stamp-option-tlv-06-secdir-lc-kaufman-2020-07-02

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.

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.

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.

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

--Charlie