Last Call Review of draft-bao-v6ops-rfc6145bis-05
review-bao-v6ops-rfc6145bis-05-secdir-lc-nir-2016-03-10-00

Request Review of draft-bao-v6ops-rfc6145bis
Requested rev. no specific revision (document currently at 07)
Type Last Call Review
Team Security Area Directorate (secdir)
Deadline 2016-03-09
Requested 2016-02-25
Authors Congxiao Bao, Xing Li, Fred Baker, Tore Anderson, Fernando Gont
Draft last updated 2016-03-10
Completed reviews Genart Last Call review of -05 by Jouni Korhonen (diff)
Secdir Last Call review of -05 by Yoav Nir (diff)
Opsdir Last Call review of -05 by Qin Wu (diff)
Assignment Reviewer Yoav Nir
State Completed
Review review-bao-v6ops-rfc6145bis-05-secdir-lc-nir-2016-03-10
Reviewed rev. 05 (document currently at 07)
Review result Has Nits
Review completed: 2016-03-10

Review
review-bao-v6ops-rfc6145bis-05-secdir-lc-nir-2016-03-10

I have reviewed this document as part of the security directorate's  ongoing effort to review all IETF documents being processed by the IESG.  These comments were written primarily for the benefit of the security area directors.  Document editors and WG chairs should treat  these comments just like any other last call comments.

Summary: Ready with nits.

The nits are not about security. they are mostly around readability. We have statements in one section that are only explained several sections later:

Section 1.2 says this:

   Fragmented IPv4 UDP packets that do not contain a UDP checksum (i.e.,
   the UDP checksum field is zero) are not of significant use in the
   Internet, and in general will not be translated by the IP/ICMP
   translator.  

It's not clear why this is true. What do UDP checksums have to do with fragmentation? The mystery is solved in section 4.5 (You have to calculate the UDP checksum for IPv6, and you can’t recalculate it if it’s zero). It’s fine to explain later, but you should have a forward reference in section 1.2.

And speaking of the explanation in section 4.5:
       For a stateful translator, the handling of fragmented UDP IPv4
       packets with a zero checksum is discussed in [RFC6146]), Section 
       3.1.
There is an extra closing bracket at the reference, and the section is 3.4, not 3.1.

First paragraph in section 1.3 mentions a lot of terms that are not defined anywhere in the draft (masking addresses, hairpinning, contiguous routing connectivity). These terms are not explained in RFC 6144 either.

Section 4 talks about "IPv4 datagram addressed to a destination towards the IPv6 domain". This term is not clear, because IPv4 packets tend to have destination addresses that are IPv4. I take this to mean that some portion of the IPv4 space was allocated by DNS64 to "represent" IPv6 addresses, but the text should say this. Later, section 4.6 actually says this, but I think this needs to be mentioned before use.

The security considerations draft is fine and addresses all the issues I can think of. There is one nit, though:
   As with network address translation of IPv4 to IPv4, packets with
   tunnel mode Encapsulating Security Payload (ESP) can be translated
   since tunnel mode ESP does not depend on header fields prior to the
   ESP header.  
...
        the IPsec ESP endpoints will normally detect the presence of
   the translator and encapsulate ESP in UDP packets [RFC3948].

If ESP packets could be translated with no problems, we wouldn’t need RFC 3948 (UDP encapsulation of IPsec ESP packets). 

Yoav