RE: FW: external mobike-protocol-02 review (technical) (issue 42)

<[email protected]>
Newsgroups gmane.ietf.mobike
Message-ID <[email protected]>
Pekka Savola wrote:
> 
> (Note: I'm not subscribed)
> 
> Jari asked me to review the MOBIKE protocol.  Beware, I'm not
> really familiar with IKE, so there are probably things I have
> wrong or have missed.

Thanks for your thorough review! (I've filed it as issue #43).

> In any case, I thought the document was in a good shape and
> written in a manner where it should be easy to get
> interoperable implementations.  It's also short -- good.  What
> troubled me a bit was a significant number of forward
> references to other places in the spec, but this is
> understandable in the interest of avoiding text duplication.
> 
> 
> meta-issues
> -----------
>    It is assumed that issues such as transport mode (updating
>    traffic selectors), PFKEY extensions, and tunnel overhead
>    reduction will be handled in separate documents.
> 
> ==> is transport mode even in the scope of the WG?  I thought 
> not, but the charter doesn't seem conclusive either way..

Well, this sentence in the draft was not intended as an opinion
either way; only as a note that regardless of whether it's in the
WG scope or not, it's certainly not within the scope of this
document :-)

> substantial
> -----------
> 
>    Note that if some of the initiator's interfaces are behind a
>    NAT (from the responder's point of view), the addresses
>    received by the responder will be incorrect.  This means the
>    procedure for changing responder addresses described in
>    Section 2.4 does not fully work when the initiator is behind
>    a NAT. [...]
> 
> ==> this is probably something that needs more air time.  What
> does "does not fully work" mean?  This also probably applies to
> the responder, though in different ways.  This may also have
> security implications (exposing internal topology), and it may
> affect address selection (if for example smaller scope
> addresses are preferable, but actually belong to different
> sites).  This probably needs a bunch of more analysis...

Yes, this is something that probably deserves a slightly longer
explanation (but I don't think we should spend too much time on
it). How about this?

1.3  MOBIKE and Network Address Translation (NAT)

   In some MOBIKE scenarios the network may contain NATs or stateful
   packet filters (for brevity, the rest of this document talks simply
   about NATs).  The NAT Traversal feature specified in [IKEv2] allows
   IKEv2 to work through NATs in many cases, and MOBIKE can leverage
   this functionality: when the addresses used for IPsec SAs are
   changed, MOBIKE can enable or disable IKEv2 NAT Traversal as needed.

   Nevertheless, there are some limitations.  NATs usually introduce an
   asymmetry in the network: packets reaching the NAT from the "inside"
   are treated differently from packets coming from the "outside".  This
   asymmetry leads to restrictions on what MOBIKE can do.  To give a
   concrete example, consider a situation where both peers have only a
   single address, and the initiator is behind a NAT.  If the
   responder's address now changes, it needs to send a packet to the
   initiator using its new address.  However, if the NAT is, for
   instance, of the "restricted cone" type (see [STUN] for one
   description of different NAT types), this is not possible: the NAT
   will drop packets sent from the new address (unless the initiator has
   previously sent a packet to that address -- which it cannot do until
   it knows the address).

   For simplicity, MOBIKE does not attempt to handle all possible NAT-
   related scenarios.  Instead, MOBIKE assumes that if NATs are present,
   the initiator is the party "behind" the NAT, and does not fully
   support the case where the responder's addresses change.

   "Does not fully support" means that no special effort is made to
   support this functionality.  However, if the alternative is losing
   connectivity completely, the responder can still attempt to proceed
   with the change, and depending on, e.g., the exact type of NAT, it
   may succeed.  However, analyzing the exact circumstances when this
   will or will not work is not done in this document.

>    This specification requires the use of return routability
>    tests (under certain conditions) to limit the duration of
>    any "third party bombing" attacks by off-path (relative to
>    the victim) attackers.
> 
> ==> this is not true.  The spec only says RR _SHOULD_ be done
> before updating the SAs.  There is no "MUST" for RR (whether
> before or after), but a "MAY skip" under certain conditions.
> The normativeness requirements need to be reviewed.  

Yes, the security considerations text needs to be fixed.

> Further, the procedures for handling the payloads (sections 2.3
> and 2.4) should include a bullet point on RR tests and a
> pointer to 2.5.

Section 2.3 already has this for the responder; I'll add this
for the initiator as well..

>   When the initiator is behind a NAT, it SHOULD include these 
>   payloads in DPD messages, and compare the received ...
> 
>   When MOBIKE is in use, the host not behind a NAT SHOULD NOT 
>   use the dynamic updates specified ...
> 
> ==> How does the MOBIKE code know whether you're behind a NAT?
> Is this done automatically as part of IKEv2 NAT detection?
> Note that the spec doesn't seem to require running NAT presence
> detection so with current spec this can't be done.

Yes, this is done by IKEv2 NAT Traversal already (so saying
that it must be done would be redundant).

>    To give the initiator enough time to detect the error, the 
>    responder SHOULD use relatively long timeout intervals when, 
>    for instance, retransmitting IKEv2 requests or deciding whether 
>    to initiate dead peer detection.
> 
> ==> "relatively long timeout intervals" is pretty ambiguous.
> While this may not affect the interoperability, shouldn't we be
> a bit more precise what kind of order of magnitude we're
> talking about here?  Seconds?  Ten seconds?  30 sec?  Minutes?
> 10 minutes? Hours?

This is to some degree also a matter of local policy... but I
guess it would be useful to at least give a reasonable example.

Tero, do you have an opinion about what would a good
recommendation be? Maybe something like

   "While no specific timeout lengths are required, it is
   suggested that responders continue retransmitting IKEv2
   requests for at least five minutes before giving up."

>    The data associated with this notification is the SHA-1 
>    hash [FIPS180-2] of the following data:
> 
> ==> this spec is not algorithm-agile. (I can't find the ref
> right now where this is encouraged.)  In this case, maybe it
> doesn't need to be considering the impact (I don't take a
> stance on that) -- but this probably deserves at least brief
> discussion in the security considerations section.

This part is basically copied from IKEv2 NAT detection payloads,
which also use SHA-1 (and IKEv2 spec didn't include any
discussion about this either)....

However, here the situation might actually be slightly different,
since the security implications of NAT_DETECTION_*_IP payloads
and NO_NATS_ALLOWED are different. The simplest solution might
be just to place the IP addresses in this payload (and mandate
comparing them with the IP header)...

Any comments from other on this?

> semi-editorial
> --------------
> 
> ==> there seem to be a number of assumptions scattered
> throughout the document (e.g., relating to the responder not
> being behind a NAT) -- maybe it would be useful to put these as
> a new subsection of the Introduction?

This particular assumption is actually mentioned in the
Introduction section (and gets better treatment in the proposed
NAT text above). Were there any others you had in mind..?

> ==> I would also consider splitting out 2.3 in subsections for
> readability and more exact references.  For example,
> initiator's first steps could be 2.3.1, Responder's reply
> 2.3.2, initiator's reply 2.3.3, and the exception 2.3.4.

Hmm... as it's currently organized, I'm not sure if splitting 
it would actually improve the readability (it might even be
confusing if the subsection suddenly changes in the middle of 
the description).

> ==> It might make sense to spell out, probably in section 1.2,
> that apparently (AFAICS) the initiator never tells the
> responder which addresses it takes to use.  It just sends
> UPDATE_SA_ADDRESSES with the updated addresses (inmplicit
> update).

It does tell the responder, by sending UPDATE_SA_ADDRESSES.
I'm not sure how it could be more explicit than this...?

(Or are you referring to the detail that the addresses are not
contained inside the IKEv2 payload, but in the IP header?)

>      HDR, SK { IDi, [CERT], [IDr], AUTH,
>                 [CP(CFG_REQUEST)]
>                 SAi2, TSi, TSr,
>                 N(MOBIKE_SUPPORTED),
>                 [N(ADDITIONAL_*_ADDRESS)+]  -->
> 
> ==> this raises a minor issue of the normativeness of this spec
> with regard to the non-MOBIKE -related IKE payloads.  The above
> exchange is just an example; some other payloads could be
> included as well, or some listed above could be omitted.  Also
> the optional/mandatory status might change.  Perhaps the
> document could give a short disclaimer about the IKE exchange
> examples.

Right... How about adding this:

   In some cases, the diagrams also show what payloads defined in
   [IKEv2] would be typically included in, for instance, the
   IKE_AUTH exchange. These payloads are shown for illustrative
   purposes only; see [IKEv2] for an authoritative description.

to section 1.3, after the first paragraph?

>    o  The initiator receives a NAT_DETECTION_DESTINATION_IP 
>       payload that does not match the previous UPDATE_SA_ADDRESSES 
>       response (see Section 2.6 for a more detailed description).
> 
> ==> In some cases, you use NAT_DETECTION_*_IP and in some
> NAT_DETECTION_DESTINATION_IP.  I guess this is intentional, 
> but I'd still review the cases carefully..

I went through all the occurrences of these terms, and it seems
they were all correct.

>    o  Determines whether it has already received a newer
>       UPDATE_SA_ADDRESSES request than this one (if the 
>       responder uses a window size greater than one, it is 
>       possible that requests are received out of order).  
>       If it has, a response message is sent, but no other 
>       action is taken.
> 
> ==> at first, it was not clear to me what "a response message"
> referred to. An unspecified response?  Probably you meant the
> normal response specified a few bullets down but this was not
> unambiguous.

Ok, I'll clarify this.

>    There is one additional issue that must be taken into account.
>    If the INFORMATIONAL request has been sent to several different
>    addresses (i.e., the destination address in the IKE_SA has been
>    updated after the request was first sent), ...
> 
> ==> s/If the/If the same/ (?)

Yes, that's right.

> ==> this is actually a good issue in more general.  Is it worth
> the effort to do so in the first place? That is, couldn't we
> just specify that the requests be sent with different cookies
> for each destination. I don't think a hash is computationally
> all that expensive.  That would simplify the spec and eliminate
> an extra roundtrip in some cases.

This would require everyone to support large "window sizes", and
based on feedback we've received from implementors, most of them
don't plan to support anything else than 1.

(MOBIKE in general would be _much_ simpler if the designers of
IKEv2 had done the reliability/windowing parts slightly 
differently...:-)

>    IKEv2 requires that if an IPsec endpoint discovers a NAT
>    between it and its correspondent, it MUST send all
>    subsequent traffic to and from port 4500.  To simplify
>    things, implementations that support both this specification
>    and NAT Traversal MUST change to port 4500 if the
>    correspondent also supports both, even if no NAT was
>    detected between them (this way, there is no need to change
>    the ports later).
> 
> ==> While this seems to be a logical place to talk about this,
> I'm troubled. The title is "NAT Prohibition".  What about
> MOBIKE implementations which don't implement NAT prohibition (I
> guess that's OK).  The above requirement is more general though
> -- not really anything to with NAT prohibition.  These kind of
> more generic requirements should be placed in a much more
> prominent place in the spec.

Hmm... yes, you're right, this should be somewhere else. 
Logically it would also belong earlier in the text; I'll
try to find a new place for it.

>    NO_NATS_ALLOWED payloads can also be included when changing
>    the addresses of IPsec SAs (see Section 2.3) and updating
>    the additional addresses (see Section 2.4).  An initiator
>    using this "NAT prohibition" feature includes a
>    NO_NATS_ALLOWED payload in all address update messages.
> 
> ==> s/includes/MUST include/ or..?

I think "includes" is correct, since it's describing a logical
necessity, not a choice (where we could discuss whether it 
should be a MAY, SHOULD or MUST).
 
> 3.1  MOBIKE_SUPPORTED Notification Payload
> 
> ==> note that the IKEv2 spec refers these as "Notify Payloads"
> (at least in some places of the spec; "Notification" is also
> mentioned though)..?

The IKEv2 spec is not very consistent in this (for instance,
Section 3.10 titled "Notify Payload" later on says "The payload
type for the Notification Payload is...").

But I'll try to make the use of "notification", "Notify Payload", 
and "payload" more consistent in the next version.

>    The Notify Message Type for MOBIKE_SUPPORTED is TBD-BY-
>    IANA(16396..40959).
> 
> ==> per http://www.ietf.org/internet-drafts/draft-narten-iana-
> considerations-rfc2434bis-02.txt,
> 
> thse types should be named TBD-BY-IANA1, ...., TBD-BY-IANAN (or
> something like that), so we know IANA has assigned a separate
> value for each and it can be easily search-n-replaced.
>
> I'd also suggest dropping the assignment range from here, and 
> giving that guidance in IANA Considerations.

OK, I'll do that... (I'll also add a table in the IANA
considerations section summarizing all the new notifications).

>   of the stack, such as properly dealing with ICMP errors
>   [ICMPAttacks]
>
> ==> some folks may argue that what ICMPattacks proposes may not
> be "proper". So, maybe find a different word for "proper", or
> use something like "as validating ICMP errors" which takes no
> stance on whether it's useful or not.

Ok.

>   [IPsecArch]
>              Kent, S. and K. Seo, "Security Architecture for the
>              Internet Protocol", draft-ietf-ipsec-rfc2401bis-06 
>              (work in progress), March 2005.
>
> ==> this should probably be under normative refs?

Yes.

Best regards,
Pasi
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.