global: cleanup IP header checking

This way is more correct and ensures we're within the skb head.

7 files changed, 46 insertions, 77 deletions

diff --git a/src/cookie.c b/src/cookie.c
index 0e9c211..33beea8 100644
--- a/src/cookie.c
+++ b/src/cookie.c
@@ -85,9 +85,9 @@ static void make_cookie(u8 cookie[COOKIE_LEN], struct sk_buff *skb, struct cooki
 	down_read(&checker->secret_lock);
 
 	blake2s_init_key(&state, COOKIE_LEN, checker->secret, NOISE_HASH_LEN);
-	if (ip_hdr(skb)->version == 4)
+	if (skb->protocol == htons(ETH_P_IP))
 		blake2s_update(&state, (u8 *)&ip_hdr(skb)->saddr, sizeof(struct in_addr));
-	else if (ip_hdr(skb)->version == 6)
+	else if (skb->protocol == htons(ETH_P_IPV6))
 		blake2s_update(&state, (u8 *)&ipv6_hdr(skb)->saddr, sizeof(struct in6_addr));
 	blake2s_update(&state, (u8 *)&udp_hdr(skb)->source, sizeof(__be16));
 	blake2s_final(&state, cookie, COOKIE_LEN);
diff --git a/src/device.c b/src/device.c
index 22632e5..c299d19 100644
--- a/src/device.c
+++ b/src/device.c
@@ -119,6 +119,12 @@ static netdev_tx_t xmit(struct sk_buff *skb, struct net_device *dev)
 		goto err;
 	}
 
+	if (unlikely(skb_examine_untrusted_ip_hdr(skb) != skb->protocol)) {
+		ret = -EPROTONOSUPPORT;
+		net_dbg_ratelimited("%s: Invalid IP packet\n", dev->name);
+		goto err;
+	}
+
 	peer = routing_table_lookup_dst(&wg->peer_routing_table, skb);
 	if (unlikely(!peer)) {
 		ret = -ENOKEY;
@@ -130,7 +136,7 @@ static netdev_tx_t xmit(struct sk_buff *skb, struct net_device *dev)
 	ret = peer->endpoint.addr.sa_family != AF_INET && peer->endpoint.addr.sa_family != AF_INET6;
 	read_unlock_bh(&peer->endpoint_lock);
 	if (unlikely(ret)) {
-		ret = -EHOSTUNREACH;
+		ret = -EDESTADDRREQ;
 		net_dbg_ratelimited("%s: No valid endpoint has been configured or discovered for peer %Lu\n", dev->name, peer->internal_id);
 		goto err_peer;
 	}
diff --git a/src/packets.h b/src/packets.h
index 0e909d3..c956c7a 100644
--- a/src/packets.h
+++ b/src/packets.h
@@ -9,6 +9,9 @@
 
 #include <linux/types.h>
 #include <linux/padata.h>
+#include <linux/skbuff.h>
+#include <linux/ip.h>
+#include <linux/ipv6.h>
 
 struct wireguard_device;
 struct wireguard_peer;
@@ -34,11 +37,20 @@ void packet_send_handshake_response(struct wireguard_peer *peer);
 void packet_send_handshake_cookie(struct wireguard_device *wg, struct sk_buff *initiating_skb, __le32 sender_index);
 void packet_create_data_done(struct sk_buff_head *queue, struct wireguard_peer *peer);
 
-
 /* data.c */
 int packet_create_data(struct sk_buff_head *queue, struct wireguard_peer *peer);
 void packet_consume_data(struct sk_buff *skb, struct wireguard_device *wg);
 
+/* Returns either the correct skb->protocol value, or 0 if invalid. */
+static inline __be16 skb_examine_untrusted_ip_hdr(struct sk_buff *skb)
+{
+	if (skb_network_header(skb) >= skb->head && (skb_network_header(skb) + sizeof(struct iphdr)) <= skb_tail_pointer(skb) && ip_hdr(skb)->version == 4)
+		return htons(ETH_P_IP);
+	if (skb_network_header(skb) >= skb->head && (skb_network_header(skb) + sizeof(struct ipv6hdr)) <= skb_tail_pointer(skb) && ipv6_hdr(skb)->version == 6)
+		return htons(ETH_P_IPV6);
+	return 0;
+}
+
 #ifdef CONFIG_WIREGUARD_PARALLEL
 int packet_init_data_caches(void);
 void packet_deinit_data_caches(void);
diff --git a/src/ratelimiter.c b/src/ratelimiter.c
index 2d2e758..b3fdd4c 100644
--- a/src/ratelimiter.c
+++ b/src/ratelimiter.c
@@ -82,12 +82,12 @@ bool ratelimiter_allow(struct sk_buff *skb, struct net *net)
 	struct hlist_head *bucket;
 	struct { u32 net; __be32 ip[3]; } data = { .net = (unsigned long)net & 0xffffffff };
 
-	if (skb->len >= sizeof(struct iphdr) && ip_hdr(skb)->version == 4) {
+	if (skb->protocol == htons(ETH_P_IP)) {
 		data.ip[0] = ip_hdr(skb)->saddr;
 		bucket = &table_v4[hsiphash(&data, sizeof(u32) * 2, &key) & (table_size - 1)];
 	}
 #if IS_ENABLED(CONFIG_IPV6)
-	else if (skb->len >= sizeof(struct ipv6hdr) && ip_hdr(skb)->version == 6) {
+	else if (skb->protocol == htons(ETH_P_IPV6)) {
 		memcpy(data.ip, &ipv6_hdr(skb)->saddr, sizeof(u32) * 3); /* Only 96 bits */
 		bucket = &table_v6[hsiphash(&data, sizeof(u32) * 4, &key) & (table_size - 1)];
 	}
diff --git a/src/receive.c b/src/receive.c
index 95d4bb6..492a62f 100644
--- a/src/receive.c
+++ b/src/receive.c
@@ -35,54 +35,26 @@ static inline int skb_prepare_header(struct sk_buff *skb, struct wireguard_devic
 	struct udphdr *udp;
 	size_t data_offset, data_len;
 	enum message_type message_type;
-
-	if (unlikely(skb->len < sizeof(struct iphdr)))
-		return -EINVAL;
-	if (unlikely(ip_hdr(skb)->version != 4 && ip_hdr(skb)->version != 6))
-		return -EINVAL;
-	if (unlikely(ip_hdr(skb)->version == 6 && skb->len < sizeof(struct ipv6hdr)))
-		return -EINVAL;
-
+	if (unlikely(skb_examine_untrusted_ip_hdr(skb) != skb->protocol || skb_transport_header(skb) < skb->head || (skb_transport_header(skb) + sizeof(struct udphdr)) >= skb_tail_pointer(skb)))
+		return -EINVAL; /* Bogus IP header */
 	udp = udp_hdr(skb);
 	data_offset = (u8 *)udp - skb->data;
-	if (unlikely(data_offset > U16_MAX)) {
-		net_dbg_skb_ratelimited("%s: Packet has offset at impossible location from %pISpfsc\n", netdev_pub(wg)->name, skb);
-		return -EINVAL;
-	}
-	if (unlikely(data_offset + sizeof(struct udphdr) > skb->len)) {
-		net_dbg_skb_ratelimited("%s: Packet isn't big enough to have UDP fields from %pISpfsc\n", netdev_pub(wg)->name, skb);
-		return -EINVAL;
-	}
+	if (unlikely(data_offset > U16_MAX || data_offset + sizeof(struct udphdr) > skb->len))
+		return -EINVAL;  /* Packet has offset at impossible location or isn't big enough to have UDP fields*/
 	data_len = ntohs(udp->len);
-	if (unlikely(data_len < sizeof(struct udphdr))) {
-		net_dbg_skb_ratelimited("%s: UDP packet is reporting too small of a size from %pISpfsc\n", netdev_pub(wg)->name, skb);
-		return -EINVAL;
-	}
-	if (unlikely(data_len > skb->len - data_offset)) {
-		net_dbg_skb_ratelimited("%s: UDP packet is lying about its size from %pISpfsc\n", netdev_pub(wg)->name, skb);
-		return -EINVAL;
-	}
+	if (unlikely(data_len < sizeof(struct udphdr) || data_len > skb->len - data_offset))
+		return -EINVAL;  /* UDP packet is reporting too small of a size or lying about its size */
 	data_len -= sizeof(struct udphdr);
 	data_offset = (u8 *)udp + sizeof(struct udphdr) - skb->data;
-	if (unlikely(!pskb_may_pull(skb, data_offset + sizeof(struct message_header)))) {
-		net_dbg_skb_ratelimited("%s: Could not pull header into data section from %pISpfsc\n", netdev_pub(wg)->name, skb);
+	if (unlikely(!pskb_may_pull(skb, data_offset + sizeof(struct message_header)) || pskb_trim(skb, data_len + data_offset) < 0))
 		return -EINVAL;
-	}
-	if (pskb_trim(skb, data_len + data_offset) < 0) {
-		net_dbg_skb_ratelimited("%s: Could not trim packet from %pISpfsc\n", netdev_pub(wg)->name, skb);
-		return -EINVAL;
-	}
 	skb_pull(skb, data_offset);
-	if (unlikely(skb->len != data_len)) {
-		net_dbg_skb_ratelimited("%s: Final len does not agree with calculated len from %pISpfsc\n", netdev_pub(wg)->name, skb);
-		return -EINVAL;
-	}
+	if (unlikely(skb->len != data_len))
+		return -EINVAL; /* Final len does not agree with calculated len */
 	message_type = message_determine_type(skb);
 	__skb_push(skb, data_offset);
-	if (unlikely(!pskb_may_pull(skb, data_offset + message_header_sizes[message_type]))) {
-		net_dbg_skb_ratelimited("%s: Could not pull full header into data section from %pISpfsc\n", netdev_pub(wg)->name, skb);
+	if (unlikely(!pskb_may_pull(skb, data_offset + message_header_sizes[message_type])))
 		return -EINVAL;
-	}
 	__skb_pull(skb, data_offset);
 	return message_type;
 }
@@ -231,32 +203,26 @@ void packet_consume_data_done(struct sk_buff *skb, struct wireguard_peer *peer,
 		net_dbg_ratelimited("%s: Receiving keepalive packet from peer %Lu (%pISpfsc)\n", netdev_pub(peer->device)->name, peer->internal_id, &peer->endpoint.addr);
 		goto packet_processed;
 	}
-
-	if (!pskb_may_pull(skb, 1 /* For checking the ip version below */)) {
-		++dev->stats.rx_errors;
-		++dev->stats.rx_length_errors;
-		net_dbg_ratelimited("%s: Packet missing IP version from peer %Lu (%pISpfsc)\n", netdev_pub(peer->device)->name, peer->internal_id, &peer->endpoint.addr);
-		goto packet_processed;
-	}
+	if (skb_network_header(skb) < skb->head)
+		goto dishonest_packet_size;
 
 	skb->dev = dev;
 	skb->ip_summed = CHECKSUM_UNNECESSARY;
-	if (skb->len >= sizeof(struct iphdr) && ip_hdr(skb)->version == 4) {
+	skb->protocol = skb_examine_untrusted_ip_hdr(skb);
+	if (skb->protocol == htons(ETH_P_IP)) {
 		len = ntohs(ip_hdr(skb)->tot_len);
 		if (unlikely(len < sizeof(struct iphdr)))
 			goto dishonest_packet_size;
-		skb->protocol = htons(ETH_P_IP);
 		if (INET_ECN_is_ce(PACKET_CB(skb)->ds))
 			IP_ECN_set_ce(ip_hdr(skb));
 
-	} else if (skb->len >= sizeof(struct ipv6hdr) && ip_hdr(skb)->version == 6) {
+	} else if (skb->protocol == htons(ETH_P_IPV6)) {
 		len = ntohs(ipv6_hdr(skb)->payload_len) + sizeof(struct ipv6hdr);
-		skb->protocol = htons(ETH_P_IPV6);
 		if (INET_ECN_is_ce(PACKET_CB(skb)->ds))
 			IP6_ECN_set_ce(skb, ipv6_hdr(skb));
 	} else {
 		++dev->stats.rx_errors;
-		++dev->stats.rx_length_errors;
+		++dev->stats.rx_frame_errors;
 		net_dbg_ratelimited("%s: Packet neither ipv4 nor ipv6 from peer %Lu (%pISpfsc)\n", netdev_pub(peer->device)->name, peer->internal_id, &peer->endpoint.addr);
 		goto packet_processed;
 	}
diff --git a/src/routingtable.c b/src/routingtable.c
index ce94a99..a6abb61 100644
--- a/src/routingtable.c
+++ b/src/routingtable.c
@@ -322,25 +322,12 @@ int routing_table_walk_ips_by_peer_sleepable(struct routing_table *table, void *
 	return ret;
 }
 
-static inline bool has_valid_ip_header(struct sk_buff *skb)
-{
-	if (unlikely(skb->len < sizeof(struct iphdr)))
-		return false;
-	else if (unlikely(skb->len < sizeof(struct ipv6hdr) && ip_hdr(skb)->version == 6))
-		return false;
-	else if (unlikely(ip_hdr(skb)->version != 4 && ip_hdr(skb)->version != 6))
-		return false;
-	return true;
-}
-
 /* Returns a strong reference to a peer */
 struct wireguard_peer *routing_table_lookup_dst(struct routing_table *table, struct sk_buff *skb)
 {
-	if (unlikely(!has_valid_ip_header(skb)))
-		return NULL;
-	if (ip_hdr(skb)->version == 4)
+	if (skb->protocol == htons(ETH_P_IP))
 		return lookup(table->root4, 32, &ip_hdr(skb)->daddr);
-	else if (ip_hdr(skb)->version == 6)
+	else if (skb->protocol == htons(ETH_P_IPV6))
 		return lookup(table->root6, 128, &ipv6_hdr(skb)->daddr);
 	return NULL;
 }
@@ -348,11 +335,9 @@ struct wireguard_peer *routing_table_lookup_dst(struct routing_table *table, str
 /* Returns a strong reference to a peer */
 struct wireguard_peer *routing_table_lookup_src(struct routing_table *table, struct sk_buff *skb)
 {
-	if (unlikely(!has_valid_ip_header(skb)))
-		return NULL;
-	if (ip_hdr(skb)->version == 4)
+	if (skb->protocol == htons(ETH_P_IP))
 		return lookup(table->root4, 32, &ip_hdr(skb)->saddr);
-	else if (ip_hdr(skb)->version == 6)
+	else if (skb->protocol == htons(ETH_P_IPV6))
 		return lookup(table->root6, 128, &ipv6_hdr(skb)->saddr);
 	return NULL;
 }
diff --git a/src/socket.c b/src/socket.c
index 332a682..40f8b8d 100644
--- a/src/socket.c
+++ b/src/socket.c
@@ -207,12 +207,12 @@ int socket_send_buffer_as_reply_to_skb(struct wireguard_device *wg, struct sk_bu
 int socket_endpoint_from_skb(struct endpoint *endpoint, struct sk_buff *skb)
 {
 	memset(endpoint, 0, sizeof(struct endpoint));
-	if (ip_hdr(skb)->version == 4) {
+	if (skb->protocol == htons(ETH_P_IP)) {
 		endpoint->addr4.sin_family = AF_INET;
 		endpoint->addr4.sin_port = udp_hdr(skb)->source;
 		endpoint->addr4.sin_addr.s_addr = ip_hdr(skb)->saddr;
 		endpoint->src4.s_addr = ip_hdr(skb)->daddr;
-	} else if (ip_hdr(skb)->version == 6) {
+	} else if (skb->protocol == htons(ETH_P_IPV6)) {
 		endpoint->addr6.sin6_family = AF_INET6;
 		endpoint->addr6.sin6_port = udp_hdr(skb)->source;
 		endpoint->addr6.sin6_addr = ipv6_hdr(skb)->saddr;