Re: [HOKEY] AD review of draft-ietf-hokey-erp-aak

[Stephen Farrell <[e-mail]>]

Hiya,

A few notes below but mostly those changes look like they
should be fine.

Any idea when the new I-D will pop out? Be good to get this
moving along...

On 01/11/2012 02:59 AM, Zhen Cao wrote:
> Hi, Stephen:
>
> Sorry for late reply. I have skipped the comments that have already
> been addressed on the list. Please see my replies inline.
> #1, p4 - Figure 1 - should that be "CAP authorized"?
>
> [CZ]: Yes.CAP needs to be authorized rather than authenticated.
> Will fix this in the new version.
>
> #2, p4 - I can see why you might not want to make figure 1 longer,
> but it seems to stop short - shouldn't it end with the Peer and CAP
> doing stuff together? Maybe add another figure that shows the
> latter exchange?
>
> [CZ]: That’s a good suggestion. We will remove interaction between CAP
> and the server in the figure1 and add another figure to show such
> process.
>
> #3, p5 - if you could tie the various actions described back to
> figure 1, that'd help the reader I think. (I'm not insisting on
> that, just asking:-)
>
> [CZ]: Reasonable.
>
> #4, p5 - Would this entire description benefit from more 2119
> language?  (As above:-)
>
> [CZ]: Okay.
>
> #5, p5 - "If either the peer or the SAP does not support ERP/AAK,
> it should fall back to full EAP authentication." Is that right?  I
> guess I wondered if the first fallback might be ERP without AAK and
> then "full" EAP but maybe I'm confused.
>
> [CZ]: No, you can not fall back to ERP since you as the peer are still
> not leaving
> the previous attachment point. ERP should only be used when you are
> connecting to the new attachment point.
>
> #6, p5, 2nd para - s/The SAP may/The SAP MAY/ or s/may/can/? And in
> that same para s/it is silently/it MUST be silently/ (if that's a
> new MUST I guess?)
>
> [CZ]: Okay.
>
> #9, p5 - Does the SAP need to verify the integrity of the message
> at step 2? You don't say. If it doesn't then bad things might be
> enabled...
>
> [CZ]: You are right, will add this in the update.
>
> #10, p5/6 - the key hierarchy text is dense (like me:-) It'd have
> helped me to know which keys are new and which KDFs are used. How
> come rIK isn't mentioned? What does "can be used to derive" mean?
> That's probably all just my ignorance of ERP though but there's not
> enough here to understand what's going on. I suspect best will be
> to fix whatever else needs fixing and then return to this at the
> end.
>
> [CZ]: The key hierarchy is quite similar to one defined in the ERP
> (i.e.,RFC5296)What is new in the key hierarchy is pRK, pIK, pMSK. They
> have different usage from rRK, rIK,rMSK.
> You are right, KDF needs to specified for pRK, pIK, pMSK rather than
> just say “can be used to derive”.

I think if you added more text to explain the hierarchy that'd
be good for the reader. But if you say that's known to folks who
are familiar with ERP I can live with that. Good to add the
KDFs all right.

>
> #11, p7 - I assume that the 'E' flag is set (value 1) to indicate
> early-authentication and unser (valule 0) to indicate that
> early-authentication is not in use. That may be a general EAP or
> ERP convention or it may be worth saying, I dunno. (There are a few
> occurrences of this.)
>
> [CZ]: Good suggestion, will add them in the update.
>
> #12, p7 - In 5.2 you say reserved bits MUST be set to zero and
> ignored on reception, but in 5.1 you don't say "ignored on
> reception." Is there a real difference or is that just an
> editorial glitch?
>
> [CZ]: That’s one nits we need to fix. Thank for catching this.
>
> #13, p7 - Are there any I18N considerations applying to the
> CAP-Identifier or keyName-NAI TLVs? I seem to recall hearing recent
> discussion about that somewhere else but can't recall if that'd
> apply here or not. (And so I don't know if something ought be said
> about I18N here.)
>
> [CZ]: I18N consider may apply to KeyName-NAI since KeyName-NAI has realm part.
> We can add a note to say realm part of KeyName-NAI follows the use of
> internationalized domain names defined in the RFC5890, is that what
> you say?
>
> #14, p7 - In 5.2 is the behavior for handling the SEQ field well
> defined? If that's done elsewhere, a reference to that would seem
> needed or else someone might do something different here.
>
> [CZ]: Yes, SEQ is defined in ERP. We can add a reference to RFC5296.
>
> #17, p8 - Is the sequence number TLV called "Sequence"? If not,
> then are you allowed spaces in TLV names?
>
> [CZ]: In this draft, the sequence number is defined as TV format
> rather than TLV format.
> In the sequence number TV format, 8 bit type for sequence number will
> be followed by 16 bit
> Sequence number value.

I was really asking about whether or not TV or TLV names can be
anything of it there's a rule or convention e.g. not include
spaces in those names. (Now that I look I see you also have some
TV names with a "/" character in them). I guess that's a bit
of a nit though.

But now that you mention it, where does it say that the sequence
number TV value is 16 bits long? It does say that for the SEQ
field but maybe needs repeating.

> #19, p8 - cryptosuite - do you really need all three? Why not just
> make one mandatory to implement?
>
> [CZ]: HMAC-SHA256-128 is mandatory to implement, this has been
> described in the draft.

Fair enough. Maybe s/should be enabled/SHOULD be enabled/
there too.

> #21, p8 - authentication tag, I think you need to say what to do if
> this doesn't verify - just ditch the packet?
>
> [CZ]: You are right. Actually the peer uses authentication tag to
> determine the validity of the EAP-Finish/Re-auth message originates at
> a server. If this doesn’t verify or authentication tag
> is not included in the response message, the message will be seen as
> invalid. The peer may fall back to full EAP authentication.
>
> #24, p9/10 - a bunch of the message field comments from 5.2 apply
> also to 5.3
>
> [CZ]: Okay and will check for consistency.
>
> #25, p9 - Maybe I'm misreading the diameter notation - is it clear
> how many sub-TLVs are allowed to be in the ERP/AAK-Key TLV? Are
> they just presented and read in the order given? Do all four
> sub-TLVs need to be present each time?
>
> [CZ]: In the request (i.e.,EAP-Initiate/Re-auth) message to the
> server, only CAP-Identifier needs to be present
> In the response (i.e.,EAP-Finish/Re-auth) message from the server, all
> the sub TLV including CAP-Identifier and pMSK-lifetime and Cryptosuit
> should be present in the message.

I think that needs to be specified here. So adding that would
be good (unless its there already and I missed it;-)

> #26, p10 - From when is the lifetime of the pMSK calculated? (Same
> for the pRK-lifetime field.) Are these signed or unsigned 32-bit
> values?
>
> [CZ]: Upon receiving EAP-Initiate message from the peer, the server
> begin to calculate pRK,pMSK.
> Upon receiving EAP-Finish message, the peer begin to calculate PMSK,pRK.

Ok. I think you need to say that. And also specify if the 32 bits
are unsigned or signed. (Signed probably makes no sense really but
good to say that.) Note that time_t is signed so there's an easy
bug for coders to trip over here.

I think there's no harm here in large values, right? Since 2^32 seconds
is 136 years there could be a DoS if something were just stored that
long but I believe that's not a problem. Correct me if I'm wrong.

>
> #27, p10 - Use one of "foo-lifetime" or "foo Lifetime"
> consistently.
>
> [CZ]: Okay.
>
> #28, p10 - I'm guessing that the lifetimes of some
> keys are related (maybe pIK and pMSK?) but you need to say so if
> that's the case.
>
> [CZ]: pIK and pMSK are both derived from the same root key(i.e.,pRK), therefore
> rIK and pMSK can share the same lifetime. That is to say, if the child
> keys share the same father,
> the child key can has the same lifetime, but the lifetime for father
> key should be set longer than child key.
>
> #29, p10 - What's an example of when the foo-lifetime sub-TLVs
> don't need to be present (i.e. why is this SHOULD and not MUST)
>
> [CZ]: One example is in the request message to the server, the
> foo-lifetime don’t need to be present.
>
> #30, p10 - Can there be zero elements in the list of cryptosuites?
> Is there a MAX number of elements?
>
> [CZ]: we think at least one element should be included in the
> cryptosuit list. The cryptosuit list is used to negotiate the right
> cryptosuit both the peer and the server use.

Saying that would be good.

>
> #31, p10 - The list of cryptosuites desription says "The server
> SHOULD include this if the...was not acceptable and is being
> rejected." That's pretty ambiguous. What's the exception to the
> SHOULD? Does "is being rejected" mean that this MUST NOT be present
> when things worked ok?
>
> [CZ]: No. You have some misunderstanding to the cryptosuite descrption.
> We define Cryptosuite field in both EAP-initiate message and EAP-Finish message,
> We also allow carry Crytosuite List TLV( Type is 5) in both
> EAP-Initiate message and EAP-Finish message.
> If Cryptosuite included in the Cryptosuite field of EAP-Initiate
> message is rejected by the server, the server can indicate to the peer
> the cryptosuit list the server supports using cryptosuite list TLV.
> Does this clarify?

Yep. Good to say that in the spec (unless its there and I missed
it as usual:-)

>
> #32, p10 - 5.4 talks about the "rRK Lifetime" but earlier you had a
> "pRK Lifetime" (or "pRK-lifetime") - what's up with the change
> there?
> [CZ]: is that your misunderstanding? rRK lifetime TV is not allowed here.

I'll look again at the new I-D. Might be me being mixed up all
right.

> #33, p11 - Section 7 says a new message "should" be defined - who's
> gonna do that where? Seems odd that its not here.
>
> [CZ]: Could be removed if you think odd.

I think it is odd. Is this something that's needed though? If
so, who's doing the work or in what document is the required
message defined? What are the consequences of that not being
defined?

> #34, p11 - section 8, 1st bullet says "*The* algorithm..." I think
> you mean the algorithm for calculating the authentication tag, but
> you need to say that.
>
> [CZ]: Okay.
>
> #35, p11 - How are KDFs named/replaced?
>
> [CZ]: KDF is abbreviation of Key Derivation Function which is
> specified in RFC5295.
>
> #37, p11 - Its not clear to me how the replay detection scheme
> works.
>
> [CZ]: The replay protection is quite similar to ERP, i.e.,
> “
> The peer increments the sequence number by one after it sends an ERP message
> The server sets the expected sequence number to the received sequence number
> plus one after verifying the validity of the received message
> and responds to the message.
> ”
> The different to ERP is the server and the peer may generate multiple
> pMSK for each CAP, therefore they should allocate different Sequence
> number for each PMSK.

Be good to say that somewhere or point at the relevant bit of ERP,

Cheers,
S.

>
>
> On Thu, Nov 24, 2011 at 9:13 PM, Stephen Farrell
> <[e-mail]>  wrote:
>>
>> Hiya,
>>
>> Good stuff. I'd say there's enough there already to push out another
>> revision. Not sure if you want to wait on responses to the
>> other issues or not. You can figure that out with your co-authors
>> and WG chairs I guess.
>>
>> Cheers,
>> S.
>>
>>
>> On 11/24/2011 01:06 PM, Qin Wu wrote:
>>>
>>> Hi,
>>> ----- Original Message -----
>>> From: "Stephen Farrell"<[e-mail]>
>>> To: "Qin Wu"<[e-mail]>
>>> Cc:<[e-mail]>; "Glen Zorn"<[e-mail]>;
>>> "Cao,Zhen"<[e-mail]>
>>> Sent: Thursday, November 24, 2011 7:47 PM
>>> Subject: Re: [HOKEY] AD review of draft-ietf-hokey-erp-aak
>>>
>>>
>>>>
>>>> Hiya,
>>>>
>>>> On 11/24/2011 06:08 AM, Qin Wu wrote:
>>>>>
>>>>> Hi, Stephen:
>>>>> Thank for your valuable review.
>>>>> Here is my answer to the main issues I think and I skip most of
>>>>> editorial issues or nits issues.
>>>>> My coauthors will address the remaining issues.
>>>>
>>>>
>>>> Fine thanks.
>>>>
>>>>> ----------------------------------------
>>>>> #7, p5 - "The rIK is used to protect this message." Is that right?
>>>>> How is rIK used to protect the message when the message contain
>>>>> rIK?  I also wondered what "protect" means exactly - are the flags
>>>>> etc. all protected and how?
>>>>>
>>>>> [Qin] I think rIK should be corrected as pIK. pIK is used to protect the
>>>>> ERP message
>>>>> exchanged between the peer and the EA server. MSK used for normal EAP
>>>>> exchange should be further derived into
>>>>> other child keys. Alternatively, the pMSK can be derived into child
>>>>> keys. These child keys protect the ERP
>>>>> message exchanged between the peer and SAP.
>>>>
>>>>
>>>> Alternatively? Isn't it a bit late in the day (just before
>>>> IETF LC) to be considering such alternatives?
>>>>
>>>> If you do replace rIK with pIK please send the chaged paragraph
>>>> to the list before posting the draft to make sure its ok?
>>>
>>>
>>> [Qin]: I just explain how the message is protected between the peer and
>>> authenticator using
>>> the existing key derived from regular EAP exchange, however this is not
>>> the focus of this document.
>>> We more focus on how message is protected between the peer and the server
>>> using pIK.
>>> protect here means integrity protection or prevent  the message sent over
>>> inscure channel being tampered.
>>>
>>>>
>>>>> #8, p5 - If I'm an authenticated user and I send the message at
>>>>> step 2 to the SAP, then can I get the SAP to forward the message to
>>>>> anything on the Internet? If not, where does it say how that's
>>>>> controlled? I guess the SAP knows based on its config and/or the
>>>>> authentication state of the peer but if so you should probably say
>>>>> that?
>>>>>
>>>>> [Qin]: SAP plays the role of authenticator should encapsulate the ERP
>>>>> message
>>>>> into AAA message and route the AAA message based on Realm part of
>>>>> KeyName-NAI.
>>>>
>>>>
>>>> That's misses the point. The Realm part of the KeyName-NAI is
>>>> supplied by the peer (the user). Is the SAP supposed to check
>>>> that its an ok Realm before encapsulating and sending the AAA
>>>> message? If so, you need to say so. If not, then that might
>>>> allow attack attempts and should be noted.
>>>
>>>
>>> [Qin]: Okay.
>>>
>>>>
>>>>> #15, p8 - You can only have one keyName-NAI in the message and that
>>>>> MAY have either the home domain name or the domain name to which
>>>>> the peer is currently attached for ite realm part.  How does anyone
>>>>> know which to include when? Seems underspecified or missing a
>>>>> reference?
>>>>>
>>>>> [Qin]: This was discussed on the list many times. Based on the
>>>>> discussion,
>>>>> we take the following way:
>>>>> The peer should know where to send the message? If the peer
>>>>> communicate with the home server, the peer should carry home domain name
>>>>> in the keyName-NAI. If the peer communicate with the local server, the
>>>>> peer
>>>>> should carry local domain name in the keyName-NAI.
>>>>> The authenticator or local ER server in the path can know if the
>>>>> KeyName-NAI carry
>>>>> local domain name by comparing the domain name carried in KeyName-NAI
>>>>> with local
>>>>> domain name it has already known.
>>>>
>>>>
>>>> So perhaps its clear in your mind and on the list, but its
>>>> not clear in the document and it needs to be.
>>>
>>>
>>> [Qin]: Okay. we need to revise the document to reflect this.
>>>
>>>>
>>>>> #16, p8 - How are CAP-Identifier and "Sequence number" TLVs
>>>>> "associated"?
>>>>>
>>>>> [Qin]: Suppose multiple CAP-identifiers are carried in the ERP message,
>>>>> the same number of Sequence number TLVs should be carried with
>>>>> associated CAP-identifier.
>>>>> We can rely on the order to associate each other.
>>>>
>>>>
>>>> "rely on the order" is what you need to put in the document
>>>> then and you need to say what's ok and what's not. E.g. the
>>>> message field ordering for 3 of both could be
>>>>
>>>>      capid1, capid2, capid3, seq1, seq2, seq3
>>>> OR
>>>>      capid1, seq1, capid2, seq2, capid3, seq3
>>>>
>>>> Are both ok? What about:
>>>>
>>>>      capid1, capid2, seq1, seq2, capid3, seq3
>>>>
>>>> You need to say what's ok I think.
>>>
>>>
>>> [Qin]: Good point, we need to fix this.
>>>
>>>>
>>>>> #18, p8 - Exactly how is the sequence number used in the
>>>>> calculation of the pMSK for each CAP? Can these be re-used? (Across
>>>>> reboots?) Do they need to be random? That all needs stating I
>>>>> think.
>>>>>
>>>>> [Qin]: This was discussed on the list before. The results is:
>>>>> If we carry three CAP-Identifiers, we should also send three Sequence
>>>>> number TLVs
>>>>> with associated CAP-Identifiers. The Sequence number for each CAP MUST
>>>>> not be reused.
>>>>
>>>>
>>>> MJST NOT be re-used is fine but needs to be stated.
>>>>
>>>> Your response doesn't answer the question though.
>>>
>>>
>>> [Qin]: if there are multiple CAPs, the EA server can not send the same
>>> pMSK to all the CAPs.
>>>     The pMSK is derived as follows.
>>>
>>>     pMSK = KDF (K, S), where
>>>
>>>        K = pRK and
>>>
>>>        S = pMSK label | "\0" | Sequence number | length
>>> If the sequence number is set to the same value for each CAP, all the CAP
>>> will get the same pMSK.
>>> That's not secure.
>>>
>>>
>>>> *How* (exactly) is the sequence number used when calculating
>>>> the pMSK? Maybe that's specified in ERP, but it has to be
>>>> specified somewhere so you need more text or a reference.
>>>
>>>
>>>
>>> [Qin]: Yes, see above.
>>>
>>>>> #20, p8 - authentication tag - where is the input to the HMAC
>>>>> function specified? (Its not here anyway.) I think someone needs to
>>>>> say how this is calculated. That means both the plaintext (message)
>>>>> input (e.g. are any header bits in/excluded?) and the key input
>>>>> (which key?). It could be that this just needs a reference if its
>>>>> done the same as some other RFC. An example would be great to give.
>>>>>
>>>>> [Qin]: We should base on HMAC mechanims specified in RFC2104,
>>>>
>>>>
>>>> I think you want RFC 4868 which defines HMAC with SHA256. But
>>>> that doesn't define a 64 bit output I now notice so I think you
>>>> need to define that or drop it or find a place where it's defined.
>>>> The 128 and 256 bit output variants are defined in 4686.
>>>
>>>
>>>
>>> [Qin]: Good point, we need to add a reference to RFC4868 and RFC2104.
>>> As for 64 bit output, we need to find out or drop it.
>>>
>>>>> Use the integrity algorithm indicated in the Cryptosuite field to
>>>>> calculate authentication tag value. rIK will be used for calculation.
>>>>> The message used to calculate authentication tag should exclude
>>>>> authentication
>>>>> tag field and but not exclude header bits.
>>>>
>>>>
>>>> So you need to say that.
>>>
>>>
>>> [Qin]: Okay.
>>>
>>>>>
>>>>> #22, p8 - You need references for the HMAC functions.
>>>>>
>>>>> [Qin] It should be RFC2104.
>>>>
>>>>
>>>> Nope. See above.
>>>>
>>>>>
>>>>> #23, p8 - Should/are the choices for cryptosuites in some IANA
>>>>> registry?  If not, why not? If so, where?
>>>>>
>>>>> [Qin] RFC5296 has already created registries for'Re-authentication
>>>>>      Cryptosuites'. These crytosuites can be reused for ERP-AAK.
>>>>
>>>>
>>>> So use that then and say you're doing so.
>>>
>>>
>>> [Qin]: Okay.
>>>>
>>>>
>>>>> #36, p11 - Is it ok for "different sequence numbers" to mean "just
>>>>> increment" or not?  Is it ok for sequence numbers to be re-used say
>>>>> after the peer reboots?  I think you need to say.
>>>>>
>>>>> [Qin] See the above answer to #16, #18.
>>>>
>>>>
>>>> I looked. I still don't know the answer.
>>>>
>>>> S
>>>>
>>>>>
>>>>>
>>>>> Regards!
>>>>> -Qin
>>>>> ----- Original Message -----
>>>>> From: "Stephen Farrell"<[e-mail]>
>>>>> To:<[e-mail]>
>>>>> Sent: Tuesday, November 22, 2011 2:39 AM
>>>>> Subject: [HOKEY] AD review of draft-ietf-hokey-erp-aak
>>>>>
>>>>>
>>>>>>
>>>>>> Hi all,
>>>>>>
>>>>>> Here's my review of this. There are a lot of comments,
>>>>>> but quite a few are very nitty or are things where I
>>>>>> probably just need to be told that I don't know enough
>>>>>> about ERP. (Which is true of course:-)
>>>>>>
>>>>>> Some are non trivial however, and there are a lot of nits,
>>>>>> so I've put this into the revised-ID-needed state.
>>>>>>
>>>>>> Probably best is to handle any easy ones via email and
>>>>>> then setup a skype chat for whatever's left. Let me
>>>>>> know...
>>>>>>
>>>>>> Cheers,
>>>>>> S.
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>> --------------------------------------------------------------------------------
>>>>>
>>>>>
>>>>>> _______________________________________________
>>>>>> HOKEY mailing list
>>>>>> [e-mail]
>>>>>> https://www.ietf.org/mailman/listinfo/hokey
>>>>>>
>>>>>
>>>
>> _______________________________________________
>> HOKEY mailing list
>> [e-mail]
>> https://www.ietf.org/mailman/listinfo/hokey
>
>
>