Document: draft-ietf-hokey-erx-10 Reviewer: Pasi Eronen Review Date: 2008-02-19 IETF LC End Date: 2008-02-07 IESG Telechat date: 2008-02-21 Summary: This draft is on the right track but has open issues, described in the review. Comments: ========== This version addresses my comments 5 and 6; and comments 4 and 10 are obsolete since the text I commented has been removed. The remaining comments are still valid. One additional comment for version -10: 16) Section 5.3.2, "EMSKname is in the username part of the NAI and is encoded in hexadecimal values. The EMSKname is 64-bits in length and so the username portion takes up 128 octets." EMSKname is not defined in this document (or any of its references, as far as I can tell); and encoding 64 bits as hexadecimal doesn't take 128 octets anyway. Comments from -09: =================== Most serious comments first: 1) The document should contain explicit text about the relationship between ERP and the lower layers; for example, what would need to be changed in lower layers that use EAP to add support for ERP. (E.g. with parallel EAP-Request/Identity+EAP-Initiate/Reauth-Start the protocol is no longer lock-step; the authenticator is no longer responsible for all the retransmissions, etc.) 2) The document specifies message fields for so-called "channel binding" information, but contains basically no text about what to put in the field, or how to process them. Note that just saying "RADIUS Calling-Station-Id" is not very helpful, since peers don't usually implement RADIUS. The spec either needs to describe what the field should contain, or should tell where that is described. Also, the semantics are highly unclear: the spec says these attributes can be included in EAP-Initiate/Re-Auth and EAP-Finish/Re-Auth messages -- but how do the peers know when to include them? Or what to do with them when received? E.g. if the EAP-Initiate/Re-Auth contains some of these attributes, should the EAP-Finish/Re-Auth also contain them? With same values? (Answers to some of these questions may be "obvious" to people who participated in the channel binding discussions 3..5 years ago; but they're not in the current specification. And if there's any difficulty in writing text about them, it IMHO suggests they are not that obvious.) 3) The document uses terms EAP Peer-ID and EAP Session-ID which are not part of RFC 3748; they are defined in draft-ietf-eap-keying, which needs to be (normatively) referenced. 4) Section 4.1.1 defines "NameDerivationKey = EAP Session-ID, when K used in rRK derivation is the EMSK"; however, existing EAP methods are not required to export a Session-Id. This document needs to specify what is done when no Session-ID is exported, or explicitly say that it works only with EAP methods that export a session id. 5) Section 5.1, "In this case, the lower layer may already have derived the TSKs based on the MSK received earlier. The lower layer may then choose to ignore the rMSK that was received with the ER bootstrapping exchange. Alternatively, the lower layer may choose to generate a TSK from the rMSK." Who/what coordinates this; that is, ensures that both peer and authenticator use the same key (MSK or rMSK)? The following comments are basically nits that should be easy to fix: 6) Section 4.1.1 specifies rRK derivation seed as "S = rRK Label + "\0" + NULL + length". It's not clear what "NULL" means here; IMHO one obvious interpretation would be a single zero octet (same as "\0"), but then again, perhaps an empty (zero-length) string is intended, since a different notation was used? 7) Section 4.1.1 specifies the rRK label as "EAP Re-(newline)(white space)authentication Root Key@ietf.org"; this is a rather unfortunate place to break a line, as the hyphen could be interpreted in two different ways. 8) Inconsistent IANA considerations: slightly different USRK label string is used in Sections 4.1.1 and 8. 9) Section 4.1.5 says "the most significant k octets of the output are used"; the term "most significant" makes sense when talking about integers. When talking about octet strings, I'd find "first" or "last" less ambiguous. 10) Sections 5.2 and 5.3.2.2, "If rIKname-NAI is present, the authenticator MUST use that NAI to route the message. If the rIKname-NAI is not present, the authenticator MUST use the NAI in the Peer-ID to forward the message via AAA. If neither are available, the authenticator MUST forward the ERP messages to the local ER server. If none of these rules apply, the authenticator MUST drop the packets silently." Let's see; it seems the logic is as follows: if (rIKname-NAI is present) { use rIKname-NAI } else if (rIKname-NAI not present && Peer-ID present) { use Peer-ID NAI } else if (rIKname-NAI not present && Peer-ID not present) { use local } else { drop silently; } I can't quite figure out when the "If none of these rules apply" text would be used. Perhaps the intent was to drop the packet if it can't be parsed at all? If so, this should be described more clearly. 11) Sections 5.3.1, 5.3.2, and 5.3.3 do describe TVs/TLVs which can be included in the messages, but don't clearly specify which of them are always present, and which may be omitted in some circumstances. 12) Section 5.4, "The server expects a sequence number of zero or higher. When the server receives an EAP- Initiate/Re-auth message, it uses the same sequence number in the EAP-Finish Re-auth message. It increments the expected sequence number by 1." The last sentence is obviously wrong; the text probably intended to say that the server sets the expected sequence number to the received sequence number plus 1. 13) Section 6, "Transport of ERP messages is specified in [10] and [11]"; this presumably means "Transport of ERP messages between the ER Authenticator and ER Server", as neither draft seems to cover transport between ER Peer and ER Authenticator. 14) Section 7, "..is indicated in the EAP re-authentication Response message" Does this mean EAP-Initiate/Re-auth message, or something else? 15) Section 8 (IANA considerations) could be clearer about where IANA is expected to assign values from existing registry, and where creating a new registry is required. This section should also provide names for the new registries.