Last Call Review of draft-mrw-nat66-
review-mrw-nat66-secdir-lc-barnes-2011-02-16-00

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.

This document defines a mechanism for stateless translation of IPv6 addresses
in packets from one prefix to another, which affects only the IP layer (neither
transport-layer ports nor checksums are affected).  Overall, the document is
well and clearly written, and does a good job of discussing the trade-offs
involved in adding NAT to a network.

From a security perspective, the document correctly documents most major
security impacts.  They note that running with NAT66 provides no more
protection than running an un-NATted network, and that while limiting NAT to
the IP layer interacts well with most security protocols, it will break things
that protect the IP header (like AH).

The one major security comment I have is that it would also be helpful for this
document to discuss how NAT66 interacts with IP-address based access control
lists, in particular, the SPD and PAD in IPsec.

Other general, non-security comments follow.

Major: The description of the mapping algorithm is a little bit unclear.  For
example, I didn't realize that the algorithm leaves the low-order bits of the
address unchanged.  In addition, the current algorithm is only applicable for
prefix lengths that are a multiple of 16.  I would suggest the following
modified algorithm: "
 o The NAT is configured with an interior and exterior prefix, each one
   the same length, n_pre bits.
    o The NAT computes the number of "residual bits" to the next 16-bit
      boundary: n_res = 16 - (n_pre % 16)
    o The NAT computes the checksum of the two prefixes, padded with 0
      to a multiple of 16 bits.
    o The NAT swaps the low-order n_pre bits of the checksum with the
      remaining bits to compute the "patch bytes"
       - hi_patch = sum >> n_res;
       - lo_patch = sum ^ (hi_patch << n_res);
       - patch = (lo_patch << 16-n_res) ^ hi_patch;
 o The NAT computes a "difference address" in the following steps
    o The base difference address is the XOR of the two prefixes
    o The patch bytes are appended to the end of the difference address
    o The difference address is padded with 0 to 128 bits
    o In summary: diff = preA ^ preB ^ (patch << (addr_len - n_pre - 16))
 o When the NAT receives a packet with an address matching either prefix,
   it maps the address to the other prefix by XORing the address with
   the difference address
"
Because I'm writing this on a long flight with nothing much else to do, I went
ahead and wrote an sample implementation of this algorithm.  C code is pasted
below.

Major: What is the point of Section 9?  Why are you concerned about protecting
0xFFFF values in the IID?  If you just want to prevent translation of anycast
identifiers, it seems like it would be more effective to just special-case them.

Minor: The document says that ALGs may be necessary for applications to work
through NAT66.  The consensus in the RAI/APP community actually seems to be
that host-based NAT traversal mechanisms are more effective than ALGs.  It
would be helpful to have an informative references to ICE and/or ICE-TCP (I
apologize, I don't have the RFC numbers on hand).

Minor: It could be helpful to add a note about how traffic gets to the NAT66 in
the first place.  In the case where the NAT66 is a gateway to the Internet,
this is pretty obvious, but the network-to-network case is less clear.

Minor: The document says that parallel NATs can be use different prefixes; it
might be helpful to comment briefly what the impact of this change would be --
in particular, it would pin routing of certain packets through a particular
NAT, possibly leading to inefficient routing.

Nit: In Section 8, it's not clear what the reference "as described above"
refers to.

-----BEGIN nat66.c-----
#include <stdint.h>
#include <stdlib.h>
#include <stdio.h>
#include <assert.h>

#define ADDR_LEN_32 4

struct inaddr6 {
    uint32_t bytes[ADDR_LEN_32];
};

/**
 * Compute the IP checksum over an address structure
 */
uint16_t inaddr6_checksum(struct inaddr6 *addr) {
    uint16_t sum = 0x0000;
    size_t i;
    for (i=0; i<ADDR_LEN_32; ++i) {
        sum ^= (addr->bytes[i]) % (1<<16);
        sum ^= (addr->bytes[i]) >> 16;
    }
    return sum;
}

/**
 * Compute the XOR of two addresses
 */
struct inaddr6 inaddr6_xor(struct inaddr6 *addr1, struct inaddr6 *addr2) {
    struct inaddr6 xor;
    size_t i;
    for (i=0; i<ADDR_LEN_32; ++i) {
        xor.bytes[i] = addr1->bytes[i] ^ addr2->bytes[i];
    }
    return xor;
}

/**
 * Place a uint16_t somewhere within an address (overwriting the
 * current value at that position).  Positions start at 0.
 */
void inaddr6_place(struct inaddr6 *addr, uint16_t val, size_t bitpos) {
    assert(bitpos <= 112);
    size_t uint_pos = bitpos >> 5;
    size_t in_uint_pos = bitpos % (1<<5);
    uint32_t bigval, mask;
    if (in_uint_pos <= 16) {
        bigval = (uint32_t) val;
        mask = (0xFFFFFFFF >> (32-in_uint_pos)) << (32-in_uint_pos);
        addr->bytes[uint_pos] &= mask;
        addr->bytes[uint_pos] ^= (bigval << (16-in_uint_pos));
    } else {
        bigval = ((uint32_t) val) >> (in_uint_pos - 16);
        mask = (0xFFFFFFFF >> (in_uint_pos - 16)) << (in_uint_pos - 16);
        addr->bytes[uint_pos] &= mask;
        addr->bytes[uint_pos] ^= bigval;
        bigval = ((uint32_t) val) % (1<<(in_uint_pos-16)) << (48 - in_uint_pos);
        mask = (0xFFFFFFFF >> (32 - in_uint_pos));
        addr->bytes[uint_pos+1] &= mask;
        addr->bytes[uint_pos+1] ^= bigval;
    }
}

/**
 * Pretty-print an IPv6 address
 */
char *inaddr6_string(struct inaddr6 *addr) {
    char *saddr;
    asprintf( &saddr, "%04x:%04x:%04x:%04x:%04x:%04x:%04x:%04x",
        addr->bytes[0] >> 16,
        addr->bytes[0] % (1<<16),
        addr->bytes[1] >> 16,
        addr->bytes[1] % (1<<16),
        addr->bytes[2] >> 16,
        addr->bytes[2] % (1<<16),
        addr->bytes[3] >> 16,
        addr->bytes[3] % (1<<16)
        );
    return saddr;
}

/**
 * Compute the patch necessary to correct for a given checksum,
 * with a given number of "residual bits".  The number of residual
 * bits is the distance from the end of the prefix to the next
 * 16-bit boundary.  Basically this function swaps the part of the
 * checksum that overlaps the prefix with that which doesn't.
 */
uint16_t nat66_patch(uint16_t sum, uint16_t n_res) {
    uint16_t hi_patch = sum >> n_res;
    uint16_t lo_patch = sum ^ (hi_patch << n_res);
    uint16_t patch    = (lo_patch << (16-n_res)) ^ hi_patch;
    return patch;
}

int main(int ac, char **av) {
    // Inputs: 2001:db8:0100::/40, fd01:0203:0400::/40
    struct inaddr6 preA = { 0x20010db8, 0x01000000, 0x00000000, 0x00000000 };
    struct inaddr6 preB = { 0xfd010203, 0x04000000, 0x00000000, 0x00000000 };
    size_t n_pre = 40;

    // Prepare the NAT delta
    size_t n_res = 16 - (n_pre % 16);
    struct inaddr6 rawdiff = inaddr6_xor(&preA, &preB);
    uint16_t patch = nat66_patch( inaddr6_checksum(&rawdiff), n_res );
    struct inaddr6 diff = rawdiff;
    inaddr6_place( &diff, patch, n_pre );

    // Test on a sample address: 2001:db8:0100::1234
    struct inaddr6 addrA = { 0x20010db8, 0x01000000, 0x00000000, 0x00001234 };
    struct inaddr6 addrB = inaddr6_xor( &addrA, &rawdiff );
    struct inaddr6 addrC = inaddr6_xor( &addrA, &diff );

    // Display results
    printf("Original address:  %s [%04x]\n", inaddr6_string(&addrA),
    inaddr6_checksum(&addrA)); printf("Unpatched address: %s [%04x]\n",
    inaddr6_string(&addrB), inaddr6_checksum(&addrB)); printf("Patched address:
      %s [%04x]\n", inaddr6_string(&addrC), inaddr6_checksum(&addrC));
}
-----END nat66.c-----