On 2/19/2008 4:15 AM, Pasi.Eronen@xxxxxxxxx wrote: > I have just scanned through version -10 of this draft, posted > couple of hours ago. > > 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. Oops, I meant 128 bits. Is my math still wrong? I thought I added a reference. I will make sure. thanks, Lakshminath > > Best regards, > Pasi > >> -----Original Message----- >> From: Eronen Pasi (Nokia-NRC/Helsinki) >> Sent: 05 February, 2008 14:30 >> To: 'gen-art@xxxxxxxx'; 'ext Tim Polk'; 'clancy@xxxxxxxxxx' >> Cc: 'hokey@xxxxxxxx'; 'ietf@xxxxxxxx'; 'ext Lakshminath >> Dondeti'; vidyan@xxxxxxxxxxxx >> Subject: Gen-ART review of draft-ietf-hokey-erx-09 >> >> >> I have been selected as the General Area Review Team (Gen-ART) >> reviewer for this draft (for background on Gen-ART, please see >> http://www.alvestrand.no/ietf/gen/art/gen-art-FAQ.html). >> >> Please resolve these comments along with any other Last Call >> comments you may receive. >> >> >> Document: draft-ietf-hokey-erx-09 >> Reviewer: Pasi Eronen >> Review Date: 2008-02-05 >> IETF LC End Date: 2008-02-07 >> IESG Telechat date: 2008-02-07 >> >> Summary: This draft is on the right track but has open issues, >> described in the review. >> >> Comments: >> >> 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@xxxxxxxx"; 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. >> >> Best regards, >> Pasi >> > _______________________________________________ Ietf@xxxxxxxx http://www.ietf.org/mailman/listinfo/ietf