From 5e845e7f48febae6de0641ea123eee4f92bfbccc Mon Sep 17 00:00:00 2001 From: Alberto Leiva Popper Date: Wed, 22 May 2013 13:17:09 -0500 Subject: [PATCH] Issues #38 and #40. - The NAT64 now validates inner packets. - The NAT64 now drops ICMP inside ICMP. This will be fixed in future iterations. --- abbreviations.txt | 8 ++- include/nat64/comm/constants.h | 2 +- include/nat64/mod/translate_packet.h | 10 --- mod/determine_incoming_tuple.c | 14 ++++- mod/packet.c | 4 ++ mod/translate_packet.c | 92 ++++++++++++++++++++++++++-- mod/translate_packet_4to6.c | 10 --- mod/translate_packet_6to4.c | 12 ---- 8 files changed, 111 insertions(+), 41 deletions(-) diff --git a/abbreviations.txt b/abbreviations.txt index 6bf30de7f..2069f644a 100644 --- a/abbreviations.txt +++ b/abbreviations.txt @@ -23,5 +23,11 @@ comm = common mod = module usr = user[space] nl = netlink -cb = callback +cb = callback (synonym for "fn") pool = IPv pool (pool4 = IPv4 pool) + + +BTW: Except from line width, tab size and "kernel-doc" (100, 4 and doxygen, respectively), we're +making an effort to follow the kernel's code conventions: +https://www.kernel.org/doc/Documentation/CodingStyle +So please don't freak out when you catch glimpse of those gotos :). diff --git a/include/nat64/comm/constants.h b/include/nat64/comm/constants.h index 0b45d8a4e..0a22f82c8 100644 --- a/include/nat64/comm/constants.h +++ b/include/nat64/comm/constants.h @@ -14,7 +14,7 @@ /** * Step the module will be injected in within Netfilter's prerouting hook. - *(After defragmentation, before Conntrack). + * (After defragmentation, before Conntrack). */ #define NF_PRI_NAT64 (NF_IP_PRI_CONNTRACK_DEFRAG + NF_IP_PRI_RAW) / 2 diff --git a/include/nat64/mod/translate_packet.h b/include/nat64/mod/translate_packet.h index 2f2900dd1..c7f45e4b4 100644 --- a/include/nat64/mod/translate_packet.h +++ b/include/nat64/mod/translate_packet.h @@ -56,16 +56,6 @@ struct packet_in { * protocols behave like transport protocols here. */ __u16 l3_hdr_len; - /** - * "l3_hdr"'s length, stripped of options and extension headers. - * So it's just a sizeof(struct iphdr) or a sizeof(struct ipv6hdr). - */ - __u16 l3_hdr_basic_len; - /** - * A helper function, that can help you get a packet's l3_hdr_len, assuming its l3_hdr_type is - * the same as this packet_in's. - */ - __u16 (*compute_l3_hdr_len)(void *l3_hdr); /** * skb_transport_header(packet_in.packet)'s type. diff --git a/mod/determine_incoming_tuple.c b/mod/determine_incoming_tuple.c index 12a9b074c..bbd795909 100644 --- a/mod/determine_incoming_tuple.c +++ b/mod/determine_incoming_tuple.c @@ -53,7 +53,7 @@ static bool ipv4_icmp_err(struct iphdr *hdr_ipv4, struct icmphdr *hdr_icmp, stru struct iphdr *inner_ipv4 = (struct iphdr *) (hdr_icmp + 1); struct udphdr *inner_udp; struct tcphdr *inner_tcp; - struct icmphdr *inner_icmp; + /* struct icmphdr *inner_icmp; */ tuple->src.addr.ipv4.s_addr = inner_ipv4->daddr; tuple->dst.addr.ipv4.s_addr = inner_ipv4->saddr; @@ -72,6 +72,10 @@ static bool ipv4_icmp_err(struct iphdr *hdr_ipv4, struct icmphdr *hdr_icmp, stru break; case IPPROTO_ICMP: + /* We're not supporting ICMP inside ICMP yet. */ + return false; + + /* inner_icmp = ipv4_extract_l4_hdr(inner_ipv4); if (is_icmp4_error(inner_icmp->type)) @@ -80,6 +84,7 @@ static bool ipv4_icmp_err(struct iphdr *hdr_ipv4, struct icmphdr *hdr_icmp, stru tuple->src.l4_id = be16_to_cpu(inner_icmp->un.echo.id); tuple->dst.l4_id = tuple->src.l4_id; break; + */ default: log_warning("Packet's inner packet is not UDP, TCP or ICMP (%d)", inner_ipv4->protocol); @@ -131,7 +136,7 @@ static bool ipv6_icmp_err(struct ipv6hdr *hdr_ipv6, struct icmp6hdr *hdr_icmp, s struct hdr_iterator iterator = HDR_ITERATOR_INIT(inner_ipv6); struct udphdr *inner_udp; struct tcphdr *inner_tcp; - struct icmp6hdr *inner_icmp; + /* struct icmp6hdr *inner_icmp; */ tuple->src.addr.ipv6 = inner_ipv6->daddr; tuple->dst.addr.ipv6 = inner_ipv6->saddr; @@ -151,6 +156,10 @@ static bool ipv6_icmp_err(struct ipv6hdr *hdr_ipv6, struct icmp6hdr *hdr_icmp, s break; case IPPROTO_ICMPV6: + /* We're not supporting ICMP inside ICMP yet. */ + return false; + + /* inner_icmp = iterator.data; if (is_icmp6_error(inner_icmp->icmp6_type)) @@ -159,6 +168,7 @@ static bool ipv6_icmp_err(struct ipv6hdr *hdr_ipv6, struct icmp6hdr *hdr_icmp, s tuple->src.l4_id = be16_to_cpu(inner_icmp->icmp6_dataun.u_echo.identifier); tuple->dst.l4_id = tuple->src.l4_id; break; + */ default: log_warning("Packet's inner packet is not UDP, TCP or ICMPv6 (%d).", iterator.hdr_type); diff --git a/mod/packet.c b/mod/packet.c index f021465be..f42459180 100644 --- a/mod/packet.c +++ b/mod/packet.c @@ -267,6 +267,10 @@ enum verdict validate_skb_ipv4(struct sk_buff *skb) return VER_DROP; } */ + if (ip4_hdr->ihl < 5) { + log_debug("Packet's IHL field is too small."); + return VER_DROP; + } if (ip_fast_csum((u8 *) ip4_hdr, ip4_hdr->ihl)) { log_debug("Packet's IPv4 checksum is incorrect."); return VER_DROP; diff --git a/mod/translate_packet.c b/mod/translate_packet.c index b45a4767f..56360345d 100644 --- a/mod/translate_packet.c +++ b/mod/translate_packet.c @@ -232,6 +232,90 @@ static bool copy_l4_hdr_and_payload(struct packet_in *in, struct packet_out *out return true; } +/* + * TODO (later) eventually merge this with packet.c when we support fragmentation. + */ +static bool validate_inner_packet(int l3_hdr_type, void *inner_l3_hdr, int pkt_size, + unsigned int *l3_hdr_len_result) +{ + struct iphdr *hdr4; + struct ipv6hdr *hdr6; + struct hdr_iterator iterator; + enum hdr_iterator_result result; + int l3_hdr_len; + + switch (l3_hdr_type) { + case PF_INET: + if (pkt_size < sizeof(*hdr4)) { + log_warning("The packet is too small to contain a inner IPv4 header."); + return false; + } + + hdr4 = inner_l3_hdr; + if (hdr4->ihl < 5) { + log_warning("The inner packet's IHL field is too small."); + return false; + } + + l3_hdr_len = 4 * hdr4->ihl; + /* RFC 792: "Internet Header + 64 bits of Original Data Datagram" */ + if (l3_hdr_len + 8 > pkt_size) { + log_warning("The IPv4 packet is too small to contain transport-layer IDs."); + return false; + } + break; + + case PF_INET6: + if (pkt_size < sizeof(struct ipv6hdr)) { + log_warning("The packet is too small to contain a inner IPv6 header."); + return false; + } + + hdr6 = inner_l3_hdr; + hdr_iterator_init(&iterator, hdr6); + + if (sizeof(struct ipv6hdr) + be16_to_cpu(hdr6->payload_len) > pkt_size) + /* The packet is truncated; do not trust its payload_len field. */ + iterator.limit = inner_l3_hdr + pkt_size; + + result = hdr_iterator_last(&iterator); + switch (result) { + case HDR_ITERATOR_SUCCESS: + log_crit(ERR_INVALID_ITERATOR, "Iterator reports there are headers beyond the payload."); + return false; + case HDR_ITERATOR_END: + l3_hdr_len = iterator.data - inner_l3_hdr; + /* + * I'm going to inherit this 8 from ICMPv4 in the meantime. When we support ICMP inside + * ICMP, this might not be appropriate. + */ + if (l3_hdr_len + 8 > pkt_size) { + log_warning("The IPv6 packet is too small to contain transport-layer IDs."); + return false; + } + break; + case HDR_ITERATOR_UNSUPPORTED: + log_info("Packet contains an Authentication or ESP header, which I do not support."); + return false; + case HDR_ITERATOR_OVERFLOW: + log_warning("The packet is too small to contain a inner transport header."); + return false; + default: + log_crit(ERR_INVALID_ITERATOR, "Unknown header iterator result code: %d.", result); + return false; + } + + break; + + default: + log_warning("Unsupported network protocol: %d.", l3_hdr_type); + return false; + } + + *l3_hdr_len_result = l3_hdr_len; + return true; +} + bool translate_inner_packet(struct packet_in *in_outer, struct packet_out *out_outer, bool (*l3_function)(struct packet_in *, struct packet_out *)) { @@ -275,14 +359,12 @@ bool translate_inner_packet(struct packet_in *in_outer, struct packet_out *out_o log_debug("Translating the inner packet..."); - if (in_outer->payload_len < in_outer->l3_hdr_basic_len) { - log_warning("Packet is too small to contain a packet."); - goto failure; - } /* Get references to the original data. */ in_inner.hdr.src = in_outer->payload; - in_inner.hdr.len = in_outer->compute_l3_hdr_len(in_inner.hdr.src); + if (!validate_inner_packet(in_outer->l3_hdr_type, in_outer->payload, in_outer->payload_len, + &in_inner.hdr.len)) + goto failure; in_inner.payload.src = in_inner.hdr.src + in_inner.hdr.len; in_inner.payload.len = in_outer->payload_len - in_inner.hdr.len; diff --git a/mod/translate_packet_4to6.c b/mod/translate_packet_4to6.c index f20bfdca0..d8add7be3 100644 --- a/mod/translate_packet_4to6.c +++ b/mod/translate_packet_4to6.c @@ -4,14 +4,6 @@ * Would normally be part of translate_packet.c; the constant scrolling was killing me. */ -/** - * Assumes that "l3_hdr" points to a iphdr, and returns its size, options included. - */ -static __u16 compute_ipv4_hdr_len(void *l3_hdr) -{ - return 4 * ((struct iphdr *) l3_hdr)->ihl; -} - /** * Initializes "in" using the data from "tuple", "skb_in", and the assumption that we're translating * from 4 to 6. @@ -27,8 +19,6 @@ static bool init_packet_in_4to6(struct tuple *tuple, struct sk_buff *skb_in, in->l3_hdr = ip4_hdr; in->l3_hdr_type = PF_INET; in->l3_hdr_len = skb_transport_header(skb_in) - skb_network_header(skb_in); - in->l3_hdr_basic_len = sizeof(*ip4_hdr); - in->compute_l3_hdr_len = compute_ipv4_hdr_len; in->l4_hdr_type = ip4_hdr->protocol; switch (in->l4_hdr_type) { diff --git a/mod/translate_packet_6to4.c b/mod/translate_packet_6to4.c index 897a3abbc..f7ea16149 100644 --- a/mod/translate_packet_6to4.c +++ b/mod/translate_packet_6to4.c @@ -4,16 +4,6 @@ * Would normally be part of translate_packet.c; the constant scrolling was killing me. */ -/** - * Assumes that "l3_hdr" points to a ipv6hdr, and returns its size, extension headers included. - */ -static __u16 compute_ipv6_hdr_len(void *l3_hdr) -{ - struct hdr_iterator iterator = HDR_ITERATOR_INIT((struct ipv6hdr *) l3_hdr); - hdr_iterator_last(&iterator); - return iterator.data - l3_hdr; -} - /** * Initializes "in" using the data from "tuple", "skb_in", and the assumption that we're translating * from 6 to 4. @@ -30,8 +20,6 @@ static bool init_packet_in_6to4(struct tuple *tuple, struct sk_buff *skb_in, in->l3_hdr = ip6_hdr; in->l3_hdr_type = PF_INET6; in->l3_hdr_len = skb_transport_header(skb_in) - skb_network_header(skb_in); - in->l3_hdr_basic_len = sizeof(*ip6_hdr); - in->compute_l3_hdr_len = compute_ipv6_hdr_len; hdr_iterator_last(&iterator); in->l4_hdr_type = iterator.hdr_type;