Re: Gen-ART review of draft-ietf-hokey-erx-09 (-10)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

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

[Index of Archives]     [IETF Annoucements]     [IETF]     [IP Storage]     [Yosemite News]     [Linux SCTP]     [Linux Newbies]     [Fedora Users]