wireguard: fix crash by not sending arp via wg interface
Type: fix Currently, neighbor adjacencies on a wg interface are converted into a midchain only if one of the peers has a matching allowed prefix configured. If create a route that goes through a wg interface but the next-hop address does not match any allowed prefixes, an ARP/ND request will try to be sent via the wg interface to resolve the next-hop address when matching traffic occurs. And sending an ARP request will cause VPP to crash while copying hardware address of the wg interface which is NULL. Sending an ND message will not cause VPP to crash but the error logged will be unclear (no source address). With this fix, convert all neighbor adjacencies on a wg interface into a midchain and update tests to cover the case. If there is no matching allowed prefix configured, traffic going such routes will be dropped because of "Peer error". No changes if there is matching allowed prefix configured. Also, fix getting peer by adjacency index. Signed-off-by: Alexander Chernavin <achernavin@netgate.com> Change-Id: I15bc1e1f83de719e97edf3f7210a5359a35bddbd
This commit is contained in:

committed by
Fan Zhang

parent
db8dd260d5
commit
1477c7262a
@ -153,6 +153,14 @@ wg_if_update_adj (vnet_main_t * vnm, u32 sw_if_index, adj_index_t ai)
|
||||
{
|
||||
index_t wgii;
|
||||
|
||||
/* Convert any neighbour adjacency that has a next-hop reachable through
|
||||
* the wg interface into a midchain. This is to avoid sending ARP/ND to
|
||||
* resolve the next-hop address via the wg interface. Then, if one of the
|
||||
* peers has matching prefix among allowed prefixes, the midchain will be
|
||||
* updated to the corresponding one.
|
||||
*/
|
||||
adj_nbr_midchain_update_rewrite (ai, NULL, NULL, ADJ_FLAG_NONE, NULL);
|
||||
|
||||
wgii = wg_if_find_by_sw_if_index (sw_if_index);
|
||||
wg_if_peer_walk (wg_if_get (wgii), wg_peer_if_adj_change, &ai);
|
||||
}
|
||||
|
@ -166,7 +166,7 @@ wg_peer_get (index_t peeri)
|
||||
static inline index_t
|
||||
wg_peer_get_by_adj_index (index_t ai)
|
||||
{
|
||||
if (ai > vec_len (wg_peer_by_adj_index))
|
||||
if (ai >= vec_len (wg_peer_by_adj_index))
|
||||
return INDEX_INVALID;
|
||||
return (wg_peer_by_adj_index[ai]);
|
||||
}
|
||||
|
@ -392,10 +392,12 @@ class TestWg(VppTestCase):
|
||||
wg6_input_node_name = "/err/wg6-input/"
|
||||
kp4_error = wg4_output_node_name + "Keypair error"
|
||||
mac4_error = wg4_input_node_name + "Invalid MAC handshake"
|
||||
peer4_error = wg4_input_node_name + "Peer error"
|
||||
peer4_in_err = wg4_input_node_name + "Peer error"
|
||||
peer4_out_err = wg4_output_node_name + "Peer error"
|
||||
kp6_error = wg6_output_node_name + "Keypair error"
|
||||
mac6_error = wg6_input_node_name + "Invalid MAC handshake"
|
||||
peer6_error = wg6_input_node_name + "Peer error"
|
||||
peer6_in_err = wg6_input_node_name + "Peer error"
|
||||
peer6_out_err = wg6_output_node_name + "Peer error"
|
||||
|
||||
@classmethod
|
||||
def setUpClass(cls):
|
||||
@ -421,10 +423,12 @@ class TestWg(VppTestCase):
|
||||
super(VppTestCase, self).setUp()
|
||||
self.base_kp4_err = self.statistics.get_err_counter(self.kp4_error)
|
||||
self.base_mac4_err = self.statistics.get_err_counter(self.mac4_error)
|
||||
self.base_peer4_err = self.statistics.get_err_counter(self.peer4_error)
|
||||
self.base_peer4_in_err = self.statistics.get_err_counter(self.peer4_in_err)
|
||||
self.base_peer4_out_err = self.statistics.get_err_counter(self.peer4_out_err)
|
||||
self.base_kp6_err = self.statistics.get_err_counter(self.kp6_error)
|
||||
self.base_mac6_err = self.statistics.get_err_counter(self.mac6_error)
|
||||
self.base_peer6_err = self.statistics.get_err_counter(self.peer6_error)
|
||||
self.base_peer6_in_err = self.statistics.get_err_counter(self.peer6_in_err)
|
||||
self.base_peer6_out_err = self.statistics.get_err_counter(self.peer6_out_err)
|
||||
|
||||
def test_wg_interface(self):
|
||||
"""Simple interface creation"""
|
||||
@ -577,6 +581,9 @@ class TestWg(VppTestCase):
|
||||
r1 = VppIpRoute(
|
||||
self, "10.11.3.0", 24, [VppRoutePath("10.11.3.1", wg0.sw_if_index)]
|
||||
).add_vpp_config()
|
||||
r2 = VppIpRoute(
|
||||
self, "20.22.3.0", 24, [VppRoutePath("20.22.3.1", wg0.sw_if_index)]
|
||||
).add_vpp_config()
|
||||
|
||||
# route a packet into the wg interface
|
||||
# use the allowed-ip prefix
|
||||
@ -592,6 +599,21 @@ class TestWg(VppTestCase):
|
||||
self.base_kp4_err + 1, self.statistics.get_err_counter(self.kp4_error)
|
||||
)
|
||||
|
||||
# route a packet into the wg interface
|
||||
# use a not allowed-ip prefix
|
||||
# this is dropped because there is no matching peer
|
||||
p = (
|
||||
Ether(dst=self.pg0.local_mac, src=self.pg0.remote_mac)
|
||||
/ IP(src=self.pg0.remote_ip4, dst="20.22.3.2")
|
||||
/ UDP(sport=555, dport=556)
|
||||
/ Raw()
|
||||
)
|
||||
self.send_and_assert_no_replies(self.pg0, [p])
|
||||
self.assertEqual(
|
||||
self.base_peer4_out_err + 1,
|
||||
self.statistics.get_err_counter(self.peer4_out_err),
|
||||
)
|
||||
|
||||
# send a handsake from the peer with an invalid MAC
|
||||
p = peer_1.mk_handshake(self.pg1)
|
||||
p[WireguardInitiation].mac1 = b"foobar"
|
||||
@ -606,7 +628,8 @@ class TestWg(VppTestCase):
|
||||
)
|
||||
self.send_and_assert_no_replies(self.pg1, [p])
|
||||
self.assertEqual(
|
||||
self.base_peer4_err + 1, self.statistics.get_err_counter(self.peer4_error)
|
||||
self.base_peer4_in_err + 1,
|
||||
self.statistics.get_err_counter(self.peer4_in_err),
|
||||
)
|
||||
|
||||
# send a valid handsake init for which we expect a response
|
||||
@ -694,6 +717,7 @@ class TestWg(VppTestCase):
|
||||
self.assertEqual(rx[IP].ttl, 19)
|
||||
|
||||
r1.remove_vpp_config()
|
||||
r2.remove_vpp_config()
|
||||
peer_1.remove_vpp_config()
|
||||
wg0.remove_vpp_config()
|
||||
|
||||
@ -715,6 +739,9 @@ class TestWg(VppTestCase):
|
||||
r1 = VppIpRoute(
|
||||
self, "1::3:0", 112, [VppRoutePath("1::3:1", wg0.sw_if_index)]
|
||||
).add_vpp_config()
|
||||
r2 = VppIpRoute(
|
||||
self, "22::3:0", 112, [VppRoutePath("22::3:1", wg0.sw_if_index)]
|
||||
).add_vpp_config()
|
||||
|
||||
# route a packet into the wg interface
|
||||
# use the allowed-ip prefix
|
||||
@ -732,6 +759,21 @@ class TestWg(VppTestCase):
|
||||
self.base_kp6_err + 1, self.statistics.get_err_counter(self.kp6_error)
|
||||
)
|
||||
|
||||
# route a packet into the wg interface
|
||||
# use a not allowed-ip prefix
|
||||
# this is dropped because there is no matching peer
|
||||
p = (
|
||||
Ether(dst=self.pg0.local_mac, src=self.pg0.remote_mac)
|
||||
/ IPv6(src=self.pg0.remote_ip6, dst="22::3:2")
|
||||
/ UDP(sport=555, dport=556)
|
||||
/ Raw()
|
||||
)
|
||||
self.send_and_assert_no_replies(self.pg0, [p])
|
||||
self.assertEqual(
|
||||
self.base_peer6_out_err + 1,
|
||||
self.statistics.get_err_counter(self.peer6_out_err),
|
||||
)
|
||||
|
||||
# send a handsake from the peer with an invalid MAC
|
||||
p = peer_1.mk_handshake(self.pg1, True)
|
||||
p[WireguardInitiation].mac1 = b"foobar"
|
||||
@ -747,7 +789,8 @@ class TestWg(VppTestCase):
|
||||
)
|
||||
self.send_and_assert_no_replies(self.pg1, [p])
|
||||
self.assertEqual(
|
||||
self.base_peer6_err + 1, self.statistics.get_err_counter(self.peer6_error)
|
||||
self.base_peer6_in_err + 1,
|
||||
self.statistics.get_err_counter(self.peer6_in_err),
|
||||
)
|
||||
|
||||
# send a valid handsake init for which we expect a response
|
||||
@ -835,6 +878,7 @@ class TestWg(VppTestCase):
|
||||
self.assertEqual(rx[IPv6].hlim, 19)
|
||||
|
||||
r1.remove_vpp_config()
|
||||
r2.remove_vpp_config()
|
||||
peer_1.remove_vpp_config()
|
||||
wg0.remove_vpp_config()
|
||||
|
||||
@ -886,7 +930,8 @@ class TestWg(VppTestCase):
|
||||
)
|
||||
self.send_and_assert_no_replies(self.pg1, [p])
|
||||
self.assertEqual(
|
||||
self.base_peer4_err + 1, self.statistics.get_err_counter(self.peer4_error)
|
||||
self.base_peer4_in_err + 1,
|
||||
self.statistics.get_err_counter(self.peer4_in_err),
|
||||
)
|
||||
|
||||
# send a valid handsake init for which we expect a response
|
||||
@ -1024,7 +1069,8 @@ class TestWg(VppTestCase):
|
||||
)
|
||||
self.send_and_assert_no_replies(self.pg1, [p])
|
||||
self.assertEqual(
|
||||
self.base_peer6_err + 1, self.statistics.get_err_counter(self.peer6_error)
|
||||
self.base_peer6_in_err + 1,
|
||||
self.statistics.get_err_counter(self.peer6_in_err),
|
||||
)
|
||||
|
||||
# send a valid handsake init for which we expect a response
|
||||
|
Reference in New Issue
Block a user