Hi James,
Thanks for taking the time to review the revised document.
Please see
some comments inline.
>
> This is my second review of this document. I reviewed
an
> earlier draft of the document. This draft seems to have
> addressed most of the issues I found in the earlier
draft.
>
> High Level Comment:
>
> This protocol strikes me as being somewhat complex.
The protocol itself is a single roundtrip protocol. The
comments you
have included below have not particularly indicated what
strikes you as
complexity (I've addressed your comment on key derivation
below). Could
you please elaborate on what you mean here and provide any
suggestions
on what you think could be simplified?
Perhaps, we can do a better job with the protocol summary
section to
exactly summarize the message exchange. Would that clarify
things
better?
> I did not
> make a concerted effort to dig out possible security
holes,
> but I think it could use some deeper study,
I have received one other review from a secdir member (not
an official
secdir review, but something I had requested). I will
incorporate those
comments along with yours in the next revision as well.
> and, in
> particular, I really think someone should implement it
before
> it is standardized. I know that is not required for
> non-routing protocols, but I think it would be a good
idea.
>
I am not sure why you think implementation before
standardization may be
needed here. Perhaps if we clarfied your complexity comment,
this won't
be your feeling? In this protocol itself, I don't see what
an
implementation can buy - the protocol should be fully
analyzable on
paper. If you feel there are parts of the protocol that
aren't allowing
this, then those must be clarified. I don't think we need
to delay the
standardization process for implementation here.
> Detailed Comments:
>
> pg 7, paragraph 1: The term "PRF algorithm"
is used without
> definition. It should be included in the terminology
section.
>
Ok. Will add this.
> pg 7, paragraph 4: The IP address of the AR is being
used to
> identify the AR, but the text does not say what IP
address.
> If the address is not global, there is a possibility of
> confusion if an AR on another link uses the same link
local address.
>
We'll clarify this.
> pg. 9, Message Type: There is no IANA Handover Key
Exchange
> Mechanism Type registry. If the message type is a
mobility
> header type indicator, then it comes from the mobility
header
> type registry. Since the HReq is send in a Mobility
Header,
> this must be a mobility header type.
>
This seems like a remnant that was missed during the
revision - we'll
clean this up.
> pg 9, again PRF is used without definition.
>
As said earlier, we'll include a definition.
> pg. 11: Same as the last two. Also, it might be best to
make
> the values TBD-IANA1, TBD-IANA2, etc. and let IANA do
the
> allocation, unless you really want to set the number at
'1'.
>
Okay.
> pg. 12: These are a lot of status codes and it is not
clear
> to me that the protocol gets much use out of having so
many.
> More about this below.
>
I agree that we can decrease the status codes - we had a
brief
discussion during the 02 revision on doing this, but somehow
didn't make
it into the draft. We'll revise to simplify this.
> pg. 17: The key derivation algorithm described here
seems to
> be somewhat complex. What security benefit is derived
from
> this complexity? If any, then it should be described in
the document.
>
In general, the key derivation algorithm used is borrowed
from the IKEv2
prf+ with one very minor change (the change was needed to
randomize the
input for the handover integrity key calculation). A prf+
like operation
is needed for two reasons - a) multiple keys are derived
from the same
HMK (an integrity key and a handover key) and two separate
un-coordinated PRFs for doing this is not recommended and b)
we have
built algorithm agility into the protocol, which basically
means that
depending on the algorithm used for the MAC calculation, the
key length
could be different. A static PRF fixes the maximum key
length and this
is undesirable. These are all the reasons why prf+ was
introduced in
IKEv2 - it made sense to re-use this rather than re-invent
anything
here.
If you think it is needed, we can add some text to clarify
this
motivation.
> pg. 20, paragraph 1: This must make it clearer how
> authentication is validated. From my reading of the
text, it
> currently only requires that the MAC mobility option be
> there, not that it be checked. Unless the check is
specified
> somewhere else in the text and I missed it.
>
I'll scrub the text closer to see if this is unclear and
fix it.
> pg. 20, last paragraph: I think it would be easier to
read
> these status codes if they were organized as a bulleted
list.
> I also question whether there is need for so many. For
> example, what value does HKE_NOT_SUPPORTED provide over
a
> simple failure? The MN doesn't know what other
algorithm to
> use. I don't understand why a retransmittal of the
request
> would work for COA_AUTH_FAILED. Why would the MN get
> INVALID_MAC_ALG or INVALID_PRF_ALG?
> Wouldn't the AR just send what it prefers and if the
MN can't
> match it then too bad? There is no provision for
negotiating
> these algorithms, so what is the point of returning an
error code?
>
As I mentioned above, we'll work on reducing the number of
error codes.
I agree that we don't need all of this.
> pg. 23, 6th paragraph: I'm not sure about this. If the
NC
> entry for the CoA expires and the AR removes the key,
then
> the MN is left thinking that the key is valid when it
isn't.
> Why not send an NS to the MN to see if it is still
around and
> refresh the NC entry?
>
Actually, in this protocol, the AR does not remove the key
when the NC
entry for the CoA expires - it only removes the *binding*
between the
key and the CoA. This is because we do allow a new CoA to be
tied to the
same key, if the MN re-appears on that link prior to the
expiry of the
lifetime for that key. Since the protocol itself is
independent of NDP,
this becomes feasible - this ensures that a AAA RT isn't
needed when the
lifetime of a key is still valid.
> I didn't check the Radius and Diameter attribute
definitions
> in the appendix. I'd suggest they be moved to a
separate
> document, since the authors don't intend the
definitions in
> the appendix to be normative.
>
Those appendices do need to become separate documents - they
are present
for informative purposes only here. We can remove the
appendices from
the next revision.
Thanks,
Vidya
> jak
>
>
>
> _______________________________________________
> Mipshop mailing list
> Mipshop ietf.org
> https:
//www1.ietf.org/mailman/listinfo/mipshop
>
_______________________________________________
Mipshop mailing list
Mipshopietf.org
https:
//www1.ietf.org/mailman/listinfo/mipshop
|