Draft: draft-nystrom-eap-potp-05.txt Reviewer: Black_David@emc.com Review Date: Friday 6/30/2006 8:02 PM CST IETF LC Date: 7/03/2006 Summary: This draft is on the right track but has open issues, described in the review. The draft is generally well written, and reasonably clear. Everything I found is minor, but a couple of the concerns rise to the level of open issues that deserve attention. ---------- Issues ---------- (A) Page 31: the definition of "msg_hash" in running text is dense, informal and mixes the algorithm for computing the hash input with a description of the design rationale for the algorithm. The result may be error-prone for an implementer. This should be separated to explicitly specify the algorithm step by step, and then explain why the algorithm is the way it is. At a high level, the algorithm has two basic steps: 1) Concatenate specified messages excluding retransmissions. 2) Remove a list of fields and TLVs from the concatenation, shortening it. The explanation of the algorithm should explain why step 2) is a "remove" step instead of a "set contents to zero" step. The description of "msg_hash'" on p.33 needs to point back to this algorithm, as do the descriptions of "msg_hash2" and "msg_hash2'" on p.40. The same is probably the case for the description of "msg_hash" on p.45, but I'm not sure about that one - it's an example of an ambiguity that could cause interoperability problems. It may also be useful to look at Section 3.3.3 of RFC 4302 for an example of a well-specified similar calculation (the AH ICV). This may look like a nit, but this sort of sloppiness leads to interoperability problems caused by subtle differences in the hash inputs across implementations, hence it's tagged as an open issue. (B) Section 4.10.7 requires that Vendor-Specific TLVs always be Mandatory (M bit is always 1). That's a formula for lack of interoperability where vendor A's implementations always issue a mandatory vendor-specific TLV that only vendor A's implementations understand, thereby preventing interoperation with vendor B's implementations that don't understand that TLV. This is deja vu for me, as I've recently been through a long discussion of this issue in the context of another draft. In the hope of avoiding a repeat of that discussion, here are the conclusions, as expressed in extracts from two email messages authored by IETF Area Directors: ----- The problem we're trying to avoid is Company C shipping a box that sends a vendor private extension that requires implementations that do not understand the extension to error, while company J doesn't implement the extension. If C's products always send this extension, they will not interoperate with J's products, even though both comply with the spec. Thus, the spec does not provide sufficient requirements to guarantee interoperability. [Hence] ... there must be a configuration in which no mandatory vendor private extensions are sent ---- It seems to me that there are two ways to deal with vendor private extensions, and still allow interoperability: - One way is to allow use of the vendor private extension to be turned off via configuration. - Another way is to ensure that if an system sends a vendor-private extension, and the other system doesn't understand it, then it responds in a well-defined way (which I believe that the spec already does), and the first system is REQUIRED to continue in a way that makes sense (ie, follows the defined standard and stops using the vendor-private extension, or at least stops relying on it). ---- Please select one or both of the approaches in the second message and add text to the draft to require it/them. (C) If this were a standards-track RFC, the identifiers for the hash, encryption and MAC algorithms defined in Section 4.10.16 would go into an IANA registry. Instead the document says that any addition of algorithms requires a new RFC. That's probably ok, but I would advise double-checking this with Jari Arkko and/or Russ Housley. ---------- Nits ---------- (1) Page 30: - For dial-up, "auth_id" SHALL either be the empty string or the phone number called by the peer. The phone number SHALL be specified in the form of a URL conformant with RFC 2806 ([8]), e.g. "tel:+1234567890". Processing of received phone numbers SHALL be conformant with RFC 2806. That's not a good example phone number, try +16175550101 - see Section 3, subsection 6 of http://www.ietf.org/ID-Checklist.html (2) Page 30: - For IP-based EAP, "auth_id" SHALL either be the empty string or the IPv4 or IPv6 address of the authenticator as seen by the peer and in binary format (4 respectively 16 octets). As an example, the IPv4 address "10.129.13.15" would be represented as (in hex) 0A 81 0D 0F, whereas the IPv6 address "0A0A:0B0B:0C0C:0D0D:0E0E: 0F0F:1010:1111" would be represented as (in hex) 0A 0A 0B 0B 0C 0C 0D 0D 0E 0E 0F 0F 10 10 11 11. Similar problem - bad example IP addresses. 192.0.2.5 and 2001:DB8::101 would be ok. See same reference as phone number issue above. The same issue occurs on p.31 at: auth_id = "10.129.13.1", iteration_count = 2000, and Unfortunately, this affects the example calculation used, so its results will need to be recalculated. (3) p.32 Note: To save on storage space, each EAP entity may hash messages as they are sent and received. This reduces the amount of state needed for this purpose to the state required for the negotiated hash algorithm. "hash" is not the right verb, but it's close. Read literally, the result over 3 messages could be H(msg3 | H(msg2 | H (msg1))) which is not equivalent to the correct H(msg1 | msg2 | msg3). The text should be rewritten to talk about partial evaluation of the hash as messages are sent and received so that the internal state of the hash function is stored instead of the contents of all previous messages (and the word "internal" is important). Thanks, --David p.s. And please note that Magnus didn't get a free pass, even though my employer (EMC) just announced that it's buying his employer (RSA Security) ;-).