isatap-20 comments

[Pekka Savola <pekkas@netcore.fi>]

[as the applicability of different solutions is being evaluated, it's 
probably OK to use the WG list for discussions as long as they don't 
too out of hand.. :)]

Some comments on the latest ISATAP spec.

substantial
-----------

1) there seems to be a lot of stuff, which doesn't appear to be fundamental
to ISATAP itself, like:

 - "Considerations for anycast", 
 - "must implement basic and advanced API"
 - ICMP code handling in section 8.6, point 5. 
 - that pref lifetime mustn't be greater than valid lifetime (sect 9.2.1)
 - the ICMP code updates are duplicated from the ICMP spec revision (sect
11, appendix D)
 - IPv6 minimum MTU is kind of obvious, in appendix B (but it's in appendix
so it's not as big an issue)

maybe these should be removed if their relevance to ISATAP in
particular is not made clearer.

2) the specification (sect 7.4) appears to default to forward between
interfaces -- was this intentional ??

3) the specification appears to incorporate a lot separate ideas about
topics which really seem to have very little to do with ISATAP, such as:

 - new IPv4 packet reassembly algorithms (section 8.2, first point) 
 - advanced MTU handling mechanisms
 - some address rewriting stuff (sect 8.6, point 4, 2nd paragraph)
 - how advertised MTU doesn't affect link MTU (9.2.2.3), but is used between
hosts
 - (some MANET derivative work?)

These should probably be out of scope for the base spec, and pursued
separately.  Beware of the feature creep....

4) you mention authenticated RA, but don't specify how:

  6.  If the packet is "INCOMPLETE" (see section 8.2) prepare an
       authenticated, unsolicited Router Advertisement message
[...]

5) as mentioned before, security considerations should list implications of
administrators not properly maintaining the PRL lists, and what actions they
must take, precisely, to prevent anything bad from happening.

Also, now that the mechanism includes NAT traversal to cross site boundaries
(well, the boundaries can be crossed otherwise as well, but you stated this
in the draft), such considerations should be called out in the security
considerations.

6) Very important pieces of the spec appear to be more or less "obfuscated"
under the management/configuration requirements section, section 7 -- while
almost none of the issues there are really such requirements.   (I'd also
specify the MIB objects etc. in another document, as appears to be the
custom.)

7) As the ISATAP model has changed quite a bit during the drafts, I think
section 4 or the like should probably include a bit more verbose "overview
of the protocol operation" in the most common cases.

semi-editorial
--------------


   The following example diagram depicts the ISATAP conceptual model:

==> well, at least I had trouble figuring out what the figure wanted to
represent...

    +-------+-------+-------+-------+-------+-------+-------+--------+
    | Type  |Length |   0   |   0   |        IPv4 Address            |
    +-------+-------+-------+-------+-------+-------+-------+--------+

==> might make sense to represent similarly as SLLA/TLLA options in ND spec,
or at least add "0", "31" and "63" bits at appropriate places..

   Native IPv6 and/or ip-protocol-41 encapsulation provides sufficient
   functionality to support communications between peers that reside
   within the same site (i.e., the same enterprise network). When the
   remote peer is in a different site, NAT traversal via UDP/IPv4
   encapsulation MAY be necessary.

==> unless you're excluding to enterprise network (no objections from me
:-), "i.e." should be "e.g.".

==> s/MAY/may/ ?

8.1.2 Multicast

   ISATAP interfaces encapsulate packets with IPv6 multicast destination
   addresses using a mapped Organization-Local Scope IPv4 multicast
   address ([RFC2529], section 6) as the destination address in the
   encapsulating IPv4 header.

==> might not hurt to spell this assumption about v4 multicast better.

      For packets received on an ISATAP interface, the IPv4 source
       address is correct if:
[...]
      -  the IPv6 source address is the address of an IPv6 neighbor on
          an ISATAP interface associated with the locator that matched
          the packet (see: section 7.2.3), or:

==> I didn't understand what this implied, precisely.  Section 7.2 was
difficult to understand anyway.

   4.  Perform IPv4 ingress filtering (optional; disabled by default)

==> it might not hurt to specify what exactly you mean with ingress
filtering.. it's used in many different ways, so folks may have different
assumptions about what it entails..

   6.  If the packet is "INCOMPLETE" (see section 8.2) prepare an
       authenticated, unsolicited Router Advertisement message
       ([RFC2461], section 6.2.4) with an MTU option that encodes the  
       maximum of "ACTUAL_BYTES" and (68 bytes minus the size of
       encapsulating headers.)

==> IPv6 MTUs lower than 1280 are ignored so 68 bytes minus anything is a
no-op.  This might fail otherwise as well.

   FQDNs are established via manual configuration or an unspecified
   alternate method.

==> might be pretty important for interop purposes.


Normative References

==> there is a very high number of references, even rather obvious ones
(like "UDP" or "IP").  I'm not sure if this is a big issue, but IMHO at
least the normative references section could be a bit shorter.  Maybe some
of these could be removed, or moved to informational references.

editorial 
---------
(very good editorial quality -- thanks!)


      an IPv4 address-to-interface mapping, i.e., a node's IPv4 address 
      and the index for it's associated interface.

==> s/it's/its/

   See: Appendix C for additional non-normative details.

==> s/See:/See/


-- 
Pekka Savola                 "You each name yourselves king, yet the
Netcore Oy                    kingdom bleeds."
Systems. Networks. Security. -- George R.R. Martin: A Clash of Kings