Subject: Re: [BEHAVE] Last Call: draft-ietf-behave-v6v4-xlate-stateful (Stateful NAT64: Network Address and Protocol Translation from IPv6 Clients to IPv4 Servers) to Proposed Standard

From: marcelo bagnulo braun <[email-masked]>
Date: Sat, 10 Jul 2010 15:34:48 +0200

Hi Pekka,

thanks for the review

I think i have addressed most of the comments, but i have some follow ups.
See replies below.

El 03/06/10 11:06, Pekka Savola escribió:
> On Tue, 1 Jun 2010, The IESG wrote:
>> - 'Stateful NAT64: Network Address and Protocol Translation from IPv6
>>   Clients to IPv4 Servers '
>> <draft-ietf-behave-v6v4-xlate-stateful-11.txt> as a Proposed Standard
>
> I've reviewed draft-ietf-behave-v6v4-xlate-stateful-11 for ops-dir.
>
> I believe the document is ready for publication, but there are a couple
> of areas where minor clarifications could improve the document.
>
> As a general comment, spec has good detail (esp S 3.5) on how to 
> handle TCP, UDP and ICMP query traffic, but I did not find similar 
> detail on how ICMP errors sent in response to such traffic would 
> affect packet processing, state tables, etc.

The thing is that NAT64 only translated ICMP errors back and forth when 
it can but does not change its internal state in any way based on the 
errors.
In other words, ICMP errors received do not affect the NAT64 state.
I have made that explicit in the intor of the session handling section.
I have also reworded the other sections to explicitly address the ICMP 
error handling.
>
>
> It would be great if e.g. TCPM WG reviewed Section 3.5.2, especially its
> state table and said it's OK. There could be a number of unthought-of
> cornercases.
>
>> From O&M perspective, the specification appears to be sufficiently 
>> operable
> and configurable in IPv6->IPv4 translation. IPv4->IPv6 translation 
> requires
> manually configured or unspecified bindings. Operationally this is not 
> going
> to be sufficiently as the management interface is unspecified and e.g. 
> MIB
> module or other configuration methods do not exist and are not in 
> Behave WG
> charter.  But this issue should not delay the publication of this 
> document
> and rather should be tackled later, if needed.
>
ok

> The default TCP maximum session lifetime is 2 hours (from RFC5382). I
> personally believe this is too small for long-lived sessions.
>
not sure if there is much we can do about this. We are relying on the 
consensus achieved in RFC5382, not sure how possible is to change that 
at this point.

> some detailed comments:
> -----------------------
>
>    This document may contain material from IETF Documents or IETF
>    Contributions published or made publicly available before November
>    10, 2008. [...]
>
> .. which material is potentially covered by this?  While the non-WG 
> version
> was available prior to Nov 10, 2008, the authors should be able to say 
> so.
> It would be better if this disclaimer could be removed.  License info is
> also an older version (from id-nits checker).
>
ok, i think we can do that

> In S 3.4:
>
>    If the incoming packet is an IPv6 packet that contains a protocol
>    other than TCP, UDP or ICMPv6 in the last Next Header, then the
>    packet SHOULD be discarded and, if the security policy permits, the
>    NAT64 SHOULD send an ICMPv6 Destination Unreachable error message
>    with Code 3 (Destination Unreachable) to the source address of the
>    received packet.  NOTE: This behaviour may be updated by future
>    documents that define how other protocols such as SCTP or DCCP are
>    processed by NAT64.
>
> .. I suppose this assumes the packet is unicast, as NAT64 is an 
> unicast-only
> specification. Nonetheless I'd suggest that somewhere (probably 
> earlier in
> the spec) you'd explicitly say about one sentence on multicast; it's
> currently not mentioned at all.  E.g. "A NAT64 device MUST ignore any
> translation or actions when IPv4 or IPv6 destination address is a 
> multicast
> or the limited broadcast address (255.255.255.255)." (maybe with more
> precise definition what to do instead.)
>
I have added a sentence that states that multicast is out of the scope 
of this specification.
I would rather not to define what to do with multicast packets in this 
spec, so that if a multicast translator is defined, as proposed by stig, 
then we don't need to update this spec, makes sense?

> The same applies but more strongly to the following IPv4 paragraph.  More
> strongly because IPv4 spec does not allow generating ICMPv4's in 
> response to
> multicast packets while in ICMPv6 this is acceptable under certain
> conditions.
right, i have added a sentence in the beginning of the document that 
states that multicast traffic is out of the scope of the spec, so i 
think this would address this as well

>
> NB: I see that IPv6 destination address is covered under 2nd paragraph 
> of S
> 3.5 but a more explicit mention might be worthwhile.
>
> In S 3.5.1:
>
>       *  The STE destination IPv4 transport is set to (Z(Y'),y), y being
>          the same port as the STE destination IPv6 transport address and
>          Z(Y') being algorithmically generated from the IPv6 destination
>          address (i.e.  Y') using the reverse algorithm as specified in
>          Section 3.5.4.
>
> .. S 3.5.2 can hardly be said to specify any algorithm, but it certainly
> doesn't even mention the reverse algorithm (however trivial it might be).
>

removed the expression "specified in"
In section 3.5.4 it mentions that the algorithm must be reversible and 
that it must be possible to generate the IP4 address out of its IPv6 
representation

> In S 3.5.2:
>
>       V4 SYN RCV: An IPv4 packet containing a TCP SYN was received by
>       the NAT64, implying that a TCP connection is being initiated from
>       the IPv4 side.  The NAT64 is now waiting for a matching IPv6
>       packet containing the TCP SYN in the opposite direction.
> ...
>    A TCP segment with the SYN flag set that is received through the IPv6
>    interface is called a V6 SYN, similarly, V4 SYN, V4 FIN, V6 FIN, V6
>    FIN + V4 FIN, V6 RST and V4 RST.
>
> .. this is somewhat confusing because you appear to be using "V4 SYN 
> RCV" to
> mean "only SYN flag set" (due to "being initiated from ...") while "V4 
> SYN"
> later means "SYN [and possibly ACK] flag set". The same applies to V6 
> of course.
> Maybe reword?  Maybe "being initiated from.." and this distinction is not
> even necessary, because you don't have it on V4 FIN RCV side either? 
> (Though
> when creating sessions, the semantics differ slightly depending on the
> initiator.)
changed the state name to V4 INIT and V6 INIT
>
> V4 FIN RCV, V6 FIN RCV and V4 FIN + V6 FIN are listed in the state 
> diagram
> without "RCV" keyword, and this should be corrected.
>
done

> I'm also a bit hesitant to include 4 min T.O., 2hrs etc. in the 
> diagram as
> these seem to be certain _minimum_ values and the implementation would be
> free to use some other values as well.  More general description would be
> slightly better.
I have changes the numbers by the protocol constants TCP_TRANS and TCP_EST
I also change the 4MIN state name to TRANS

>
> A cornercase from specification language point of view are those TCP 
> flags
> that could at least in theory be applicable alongside "state flags": 
> URG and
> PSH.
>
I am not sure how these would affect the nat64 behaviour...

> In S 3.5.2.2:
>
>          +  The STE transport IPv6 source address is left unspecified
>             and may be populated by other protocols out of the scope of
>             this specification.
>
>
> ... what does leaving source address unspecified mean in the context 
> of TCP
> session table searches, packets on the wire etc.?
>
we had a long discussion about this in the WG. My position, is that in 
this case, i.e. when there is no BIB entry, the V4 SYN shoudl be simply 
dropped.
Some people argued that it may be possible that there is another 
protocol that would be able to populate the BIB off band and that why 
this was left unspecified.
As i said, i don't have any problem with changing this
>          The lifetime of the STE entry is set to TCP_INCOMING_SYN as per
>          [RFC5382] and the packet is stored.  The result is that the
>          NAT64 will not drop the packet based on the filtering, nor
>          create a BIB entry.  Instead, the NAT64 will only create the
>          session table entry and store the packet.  The motivation for
>          this is to support simultaneous open of TCP connections.
>
> ... for how long is the packet supposed to be stored?

during the lifetime of the state
If the state times out, all the state info is discarded including the 
packet. should i clarify that on the draft?
>   Will it ever be sent
> out on the wire?
well, it may be sent inside an ICMP error message in case the state 
times out and the V& SYN has not arrived. At that point the NAT64 will 
send back to the V4 initiator an ICMP error containing the original V4 
SYN, to notify that the TCP connection was not established

> If yes, what will replace the "unspecified" IPv6 source
> address then?
As i mentioned earlier, how the IPv6 source address is populated is out 
of the scope of the spec.

>   If not, what's the point of storing a packet in the first
> place (instead of just creating a state entry and dropping it)?  An 
> explicit
> reference how the packet processing continues could be useful here (it 
> seems
> it does not continue). 
In V4 SYN state processing description it reads:

    If the lifetime expires, an ICMP Port Unreachable error (Type 3, Code
    3) containing the IPv4 SYN packet stored is sent back to the source
    of the v4 SYN, the session table entry is deleted and, the state is
    moved to CLOSED.

This is the only thing that happens with the stored packet.

> The same also applies in the case a TCP BIB entry
> exist (similar description).
>
I don't understand this one.

>    *** ESTABLISHED ***
>
>    If a V4 FIN packet is received, the packet is translated and
>    forwarded.  The state is moved to V4 FIN RCV.
>
>    If a V6 FIN packet is received, the packet is translated and
>    forwarded.  The state is moved to V6 FIN RCV.
>
> .. I suppose you're referring here to _only_ FIN flag set. 
No (but maybe i am missing something)

> Does a V4 FIN
> packet with FIN+ACK flag set more the state to V4 FIN RCV?
>
yes, what would be the problem with that?

> In S 3.5.3,
>
>           If the packet is discarded, then an ICMP error message
>       MAY be sent to the original sender of the packet, unless the
>       discarded packet is itself an ICMP message.
>
> .. there are copy/paste/logical errors here, and this also seems to 
> apply to
> similar sections on UDP (at least).  First of all, under UDP section, 
> isn't
> it already known which kind of packet is in question?
correct, i have removed that

>   Isn't the "itself an ICMP message" condition wrong and should be 
> "itself an ICMP error
> message"?
correct, but i have removed that sentence as per above

>   Is it intentional that the "ICMP packet" condition is different
> for BIB entry search and address-dependent filtering search?
>
You mean that in the address dependent searhc they also include the dest 
address Y? That is intentional.

> S 3.5.3 deals with ICMP Query sessions, so from that perspective, this
> should always result in an ICMP error message generation?
>
I am not sure what do you mean. Processing in section 3.5.3 results in 
translating the ICMP query into an ICMP query when it is possible. That 
shouldn't result in ICMP errors, unless it is not possible to make the 
transaltion.

> In S 3.6.1:
>
>    When translating in the IPv4 --> IPv6 direction, let the incoming
>    source and destination transport addresses in the 5-tuple be (S,s)
>    and (D,d) respectively.  The outgoing source transport address is
>    computed as follows:
>
>       The outgoing source transport address is generated from S using
>       the address transformation algorithm described in Section 3.5.4.
>
>       The BIB table is searched for an entry (X',x) <--> (D,d), and if
>       one is found, the outgoing destination transport address is set to
>       (X',x).
>
> .. you don't mention what happens if an entry is not found in the BIB 
> table.
> (maybe the assumption is that you should never arrive at this step if 
> the BIB
> entry does not exist..) -- the same also applies in 3.6.2.
>
the thing is that either a BIB entry is created during the processing of 
the session described in the earlier section, or the packet is discarded 
in the previou section, so if the processing makes it to this point, 
there must be a BIB entry (if not, the packet has already been discarded)

> 5.3.  Attacks on NAT64
>
> .. it might be worth also mentioning the "packet is stored" case already
> mentioned above in the context of simultaneous opens.
>

added the following sentence

  Similarly, A DoS attack against the NAT64 can be crafted by sending either
     V4 or V6 SYN packets that consume memory in the form of session 
and/or binding table
     entries. In the case of IPv4 SYNs the situation is aggravated by 
thee requirement to also store the data packets
     for a given amount of time, requiring more memory from the NAT64 
device.
> editorial:
> ----------
>
> In S 3.2 also later:
>
> (X',Y',I1) <--> (T,Z,I2)
>
> .. because ICMP identifier corresponds to a port number in some sense, 
> and
> port numbers are designated with lower-case letters, I wonder if in this
> case as well it would be better to use 'i1' and 'i2'.
done
>
> *  The NAT64 MAY require that the UDP, TCP, or ICMP header be
>          completely contained within the fragment that contains OFFSET
>          equal to zero.
>
> s/OFFSET/fragment offset/
done
>
>    o  When the protocol following the IP header is ICMP (Sections 3.4
>       and 4.4) and it is an ICMP error message, the source and
>       destination transport addresses in the embedded packet are set to
>       the destination and source transport addresses from the outgoing
>       5-tuple (note the swap of source and destination).
>
> .. you're referring to S 3.4 & S 4.4 of [I-D.ietf-behave-v6v4-xlate], not
> this document, and this may be confusing.  But reference to S 3.4 and 4.4
> appears to be redundant and could maybe be removed here altogether.
>

done
>    [I-D.ietf-mmusic-ice]
>               Rosenberg, J., "Interactive Connectivity Establishment
>               (ICE): A Protocol for Network Address Translator (NAT)
>               Traversal for Offer/Answer Protocols",
>               draft-ietf-mmusic-ice-19 (work in progress), October 2007.
>
> .. now published as an RFC.
> __
fixed
> _____________________________________________
> Behave mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/behave
>