Hello wireguard project,

I am currently working on several projects that make use of wireguard as part of a larger networking scheme. Since there are many details about tunneling, network routing, and firewalling that are considered must-know for many of my coworkers I recently had to make a presentation on how packets move through the network stack and, for example, how they end up on the receiving end of a wireguard tunnel. During the presentation a question arose on what happens when an IP packet is routed through a gateway and what wireguard does.


When an IP packet is to be sent over e.g. ethernet, the link layer destination address is usually discovered using ARP. In the case of wireguard, a lookup is performed into a table which maps the entries of allowed-ips to their corresponding wireguard peer. This behavior is relatively straightforward and does what one would expect from a link layer.


However, when an IP packet is to be routed through a gateway the interaction with link layer processes such ARP is usually performed using the gateway address as opposed to the actual destination address of the packet (in Linux this is attached to a given skb as dst info). From what I understand, wireguard completely ignores the presence of such routing information and instead requires the user to manually populate a particular peer with all possible destination addresses. In effect, the concern of packet routing is placed inside the wireguard implementation instead of being left to the routing subsystem.


As for possible motivations for this design choice, I can think of security as one that could be considered motivating enough - A packet cannot travel to a wireguard peer unless its destination address is in the set of allowed-ips. Looking at the implementation it is also evident that wireguard also will not receive packets with source addresses not present in allowed-ips (and complain that there is a "dishonest peer").

However, is this not also a case of the concerns of another subsystem viz. the firewall being placed inside the wireguard implementation? As it stands, what is traditionally considered routing and firewall information has to be shared with wireguard in order to maintain a working tunnel.


Would it not be more reasonable if wireguard acted as a common link layer and respected the boundaries of internet layer routing and firewalling? To this effect I have created and tested a small patch which does two things, namely;

1. Checks for the presence of routing information on outgoing payloads and, if present, uses the specified gateway address as input to the peer lookup.

2. Removes the restriction w.r.t. the source address of incoming payloads.


I'm sure it is not possible at this stage to just fundamentally alter the semantics of allowed-ips, but, if you agree with my observations, perhaps the patch can serve as the foundation of something new which can begin to deprecatee allowed-ips as we know it today?


Regards,

Kim Nilsson


P.S.

Apologies if this has already been discussed before.


Index: linux/drivers/net/wireguard/allowedips.c
===================================================================
--- linux.orig/drivers/net/wireguard/allowedips.c
+++ linux/drivers/net/wireguard/allowedips.c
@@ -5,6 +5,8 @@
 
 #include "allowedips.h"
 #include "peer.h"
+#include "net/dst.h"
+#include "net/route.h"
 
 enum { MAX_ALLOWEDIPS_DEPTH = 129 };
 
@@ -356,6 +358,15 @@ int wg_allowedips_read_node(struct allow
 struct wg_peer *wg_allowedips_lookup_dst(struct allowedips *table,
 					 struct sk_buff *skb)
 {
+	const struct dst_entry *dst = skb_dst(skb);
+	if (dst) {
+		const struct rtable *rt = container_of(dst, struct rtable, dst);
+		if (rt->rt_gw_family == AF_INET)
+			return lookup(table->root4, 32, &rt->rt_gw4);
+		else if (rt->rt_gw_family == AF_INET6)
+			return lookup(table->root6, 128, &rt->rt_gw6);
+	}
+
 	if (skb->protocol == htons(ETH_P_IP))
 		return lookup(table->root4, 32, &ip_hdr(skb)->daddr);
 	else if (skb->protocol == htons(ETH_P_IPV6))
@@ -363,17 +374,6 @@ struct wg_peer *wg_allowedips_lookup_dst
 	return NULL;
 }
 
-/* Returns a strong reference to a peer */
-struct wg_peer *wg_allowedips_lookup_src(struct allowedips *table,
-					 struct sk_buff *skb)
-{
-	if (skb->protocol == htons(ETH_P_IP))
-		return lookup(table->root4, 32, &ip_hdr(skb)->saddr);
-	else if (skb->protocol == htons(ETH_P_IPV6))
-		return lookup(table->root6, 128, &ipv6_hdr(skb)->saddr);
-	return NULL;
-}
-
 int __init wg_allowedips_slab_init(void)
 {
 	node_cache = KMEM_CACHE(allowedips_node, 0);
Index: linux/drivers/net/wireguard/receive.c
===================================================================
--- linux.orig/drivers/net/wireguard/receive.c
+++ linux/drivers/net/wireguard/receive.c
@@ -338,7 +338,6 @@ static void wg_packet_consume_data_done(
 {
 	struct net_device *dev = peer->device->dev;
 	unsigned int len, len_before_trim;
-	struct wg_peer *routed_peer;
 
 	wg_socket_set_peer_endpoint(peer, endpoint);
 
@@ -401,24 +400,10 @@ static void wg_packet_consume_data_done(
 	if (unlikely(pskb_trim(skb, len)))
 		goto packet_processed;
 
-	routed_peer = wg_allowedips_lookup_src(&peer->device->peer_allowedips,
-					       skb);
-	wg_peer_put(routed_peer); /* We don't need the extra reference. */
-
-	if (unlikely(routed_peer != peer))
-		goto dishonest_packet_peer;
-
 	napi_gro_receive(&peer->napi, skb);
 	update_rx_stats(peer, message_data_len(len_before_trim));
 	return;
 
-dishonest_packet_peer:
-	net_dbg_skb_ratelimited("%s: Packet has unallowed src IP (%pISc) from peer %llu (%pISpfsc)\n",
-				dev->name, skb, peer->internal_id,
-				&peer->endpoint.addr);
-	++dev->stats.rx_errors;
-	++dev->stats.rx_frame_errors;
-	goto packet_processed;
 dishonest_packet_type:
 	net_dbg_ratelimited("%s: Packet is neither ipv4 nor ipv6 from peer %llu (%pISpfsc)\n",
 			    dev->name, peer->internal_id, &peer->endpoint.addr);
Index: linux/drivers/net/wireguard/allowedips.h
===================================================================
--- linux.orig/drivers/net/wireguard/allowedips.h
+++ linux/drivers/net/wireguard/allowedips.h
@@ -43,11 +43,9 @@ void wg_allowedips_remove_by_peer(struct
 /* The ip input pointer should be __aligned(__alignof(u64))) */
 int wg_allowedips_read_node(struct allowedips_node *node, u8 ip[16], u8 *cidr);
 
-/* These return a strong reference to a peer: */
+/* Returns a strong reference to a peer: */
 struct wg_peer *wg_allowedips_lookup_dst(struct allowedips *table,
 					 struct sk_buff *skb);
-struct wg_peer *wg_allowedips_lookup_src(struct allowedips *table,
-					 struct sk_buff *skb);
 
 #ifdef DEBUG
 bool wg_allowedips_selftest(void);

Attachment: OpenPGP_signature
Description: OpenPGP digital signature

Reply via email to