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

From: <[email-masked]>
Date: Tue, 19 Feb 2008 14:15:22 +0200

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: [email-masked]'; 'ext Tim Polk'; [email-masked]'
> Cc: [email-masked]'; [email-masked]'; 'ext Lakshminath 
> Dondeti'; [email-masked]
> 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 [email-masked]"; 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 mailing list
[email-masked]
http://www.ietf.org/mailman/listinfo/ietf

---

From: Lakshminath Dondeti <[email-masked]>
Date: Tue, 19 Feb 2008 09:18:00 -0800

> can tell); and encoding 64 bits as hexadecimal doesn't take

Oops, I meant 128 bits.  Is my math still wrong?  I thought I added a 
reference.  I will make sure.

thanks,
Lakshminath

---

From: Lakshminath Dondeti <[email-masked]>
Date: Wed, 20 Feb 2008 00:39:31 -0800

Hi Pasi, all

Following up on this, I think ERX-11 addresses all of Pasi's comments. 
I have previously added (in v10) clarifying text to address the 
lock-step issue based on Bernard's suggestion in his detailed review.  I 
have now added a section on lower layer considerations.  The channel 
binding parameter processing is clarified in v11, primarily in its own 
section and also elsewhere.  I have incorporated Pasi's suggestions on 
IANA considerations.  Pasi's comment 3 is addressed by the 
EMSK-hierarchy document.  Anyway, I went over Pasi's list a few times, 
and think that all of them are now addressed :).

Based on Dan's comments, I have added some more text to the security 
considerations section (talked about implications of key compromise, 
implications of hop-by-hop security and compromise of proxies, etc.).  I 
have made the key naming changes in v10 already.

I worked with Bernard's detailed comments in preparing v10; additional 
clarifications were added in v11 as well.

I have also incorporated Joe's suggestions in v10 and v11.  (Which 
implies one of Alan's comments is not addressed.  Joe wants less 
repetition between the EMSK and the ERX specs and Alan wants some 
duplication so the ERX document is more independent.)  Joe's suggestions 
helped clean up the AAA related stuff and also the multitude of options 
in identifying the rIK used.  The text is much more clear now and there 
is only one way to identify the key.  Earlier options existed for 
identity privacy reasons, but the working group consensus was that we 
don't care about that requirement at the moment.  It is simpler to 
delete those now and add them in a future revision if identify privacy 
is deemed necessary then.

Thanks again to all the reviewers.  Thank you for your patience while I 
worked through all the comments and prepared the revised version(s).

best regards,
Lakshminath