Early Review of draft-ietf-ippm-capacity-protocol-04
review-ietf-ippm-capacity-protocol-04-secdir-early-weis-2023-03-04-00
Request | Review of | draft-ietf-ippm-capacity-protocol-04 |
---|---|---|
Requested revision | 04 (document currently at 10) | |
Type | Early Review | |
Team | Security Area Directorate (secdir) | |
Deadline | 2023-02-10 | |
Requested | 2023-01-10 | |
Requested by | Tommy Pauly | |
Authors | Len Ciavattone , Ruediger Geib | |
I-D last updated | 2023-03-04 | |
Completed reviews |
Secdir Early review of -01
by Brian Weis
(diff)
Secdir Early review of -04 by Brian Weis (diff) |
|
Comments |
The authors have updated the document based on the previous SECDIR review, and the chairs would like to get another security review on the updates. Thank you! |
|
Assignment | Reviewer | Brian Weis |
State | Completed | |
Request | Early review on draft-ietf-ippm-capacity-protocol by Security Area Directorate Assigned | |
Posted at | https://mailarchive.ietf.org/arch/msg/secdir/3UzSasB6OPUUlrJ_nrhJzCOWg8Y | |
Reviewed revision | 04 (document currently at 10) | |
Result | Has issues | |
Completed | 2023-03-04 |
review-ietf-ippm-capacity-protocol-04-secdir-early-weis-2023-03-04-00
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. General comments (1) The guidance of what bytes are included in the SHA-256 calculation and result placed in the authDigest field might need some enhancement. — Some places say that the authDigest field is zeroed (e.g., Section 4.2.2 ) and some says “The SHA-256 calculation SHALL cover all the preceding fields.”, which seems to omit the auhDigest field. I guess these aren't mutually exclusive statements, but it would be cleaner to have all of the description in one place so it is clear. — Section 5.1.1 doesn’t say the network addresses and headers are included in the digest. It’s generally good practice to include more context into the digest calculation so that a valid message (e.g., Setup Request) can’t be extracted and placed in a different measurement flow that is not intended by the original sender. This is a substitution attack. — If network addresses and headers aren’t included in the digest, then it will be important to describe why the substitution attack isn’t a threat. I understand that if NAT on the client that including it's network address is not possible, but there's still an issue. — Note that the inclusion of authUnixTime with the receiver checking “acceptable immediacy” is only a partial mitigation for a substitution attack, since the receiver’s notion of “acceptable immediacy” is apparently quite long from an attacker's perspective “(+/- 2.5 minutes)” and any other message including this PDU sent fron an attacker (e.g., with different network headers) during this period will be acceptable to the receiver. Reducing the window size won’t completely mitigate this attack either. (2) There doesn’t seem to be a definition for the MBZ octets and remainder bits, or a description of their purpose. (I’m guessing it’s “must be zero” but even that should be defined for the reader.) Specific comments Section 4.2.4. — The use of AES-GCM with a long-lived symmetric key (such as one on an RFC 7210 key chain) is not safe. The IV used by GCM is not a random IV such as used by some other modes of which you might be aware (e.g., CBC), but rather it is a counter that must NEVER repeat with that key. Ensuring no repeats is not operationally feasible for a key kept on a key chain. I.e., The counter must keep incrementing between packets, reliably stored over a reboot so that it isn't used again, and when the counter reaches the maximum value the key is not longer usable. I would recommend specifying CBC mode and a randomly chosen IV. — Also, there are some additional parameters that need to determined to guarantee interoperability. For example, which octets are encrypted, if a partial block needs to be encrypted how is it padded out before encryption to make a full block, and requirements on IV generation (e.g., that is must be from a good quality random number generator). — Unauthenticated encryption is generally seen as risky, so when encryption is included the the authDigest should also be used as it is in the other modes. Figure 2 should have both authDigest AND IV fields. Section 4.2.5. This section mentions that DTLS is not useful because “The replay protection would remove duplicated packets and prevent transparent measurement of this impairment.”. IPsec will also transparently remove duplicate packets, and since TLS is based on TCP, which has duplicate segment protection, I would expect that the application would also never see duplicate measurement PDUs from TLS either. This is generally seen as a feature for network encryption methods. Section 10. “Client-server authentication and integrity protection for feedback messages conveying measurements is REQUIRED.” Since one of the methods allows “unauthenticated mode for all messages”, I think this is meant to start with “Support for client-server authentication ….”. That is, the REQUIRED language means it must be implemented, but not used. Nits — Section 4.2 says “There are four security modes of operation”, which is also stated elsewhere, but there seems to actually be five. I understand that the fifth one is actually an external security mode that can be combined with one of the four modes (e.g., “unauthenticated mode for all messages”) in the PDU. It might be clearer to take the fifth one out of the list and just make it a paragraph. — Section 4.2.1 s/miss-match/mismatch/