Browse code

Make recursive routing check more fine-grained

The existing recursive routing check drops TUN packets
if their address matches the remote. While this works in
most cases, a more fine-grained check is preferable for
complex routing rules.

Since we only need to drop traffic originating from OpenVPN,
all of the following values must match between the packet
and the link:

- IP protocol
- Transport protocol (TCP/UDP)
- Destination address
- Destination port

GitHub: #699

Change-Id: I6841e2f2a85275254a04e2d8ae3defe4420db8f6
Signed-off-by: Lev Stipakov <lev@openvpn.net>
Acked-by: Gert Doering <gert@greenie.muc.de>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/903
Message-Id: <20251011114448.14501-1-gert@greenie.muc.de>
URL: https://sourceforge.net/p/openvpn/mailman/message/59245301/
Signed-off-by: Gert Doering <gert@greenie.muc.de>

Lev Stipakov authored on 2025/10/11 20:44:42
Showing 1 changed files
... ...
@@ -1391,73 +1391,99 @@ read_incoming_tun(struct context *c)
1391 1391
 static void
1392 1392
 drop_if_recursive_routing(struct context *c, struct buffer *buf)
1393 1393
 {
1394
-    bool drop = false;
1395
-    struct openvpn_sockaddr tun_sa;
1396
-    int ip_hdr_offset = 0;
1397
-
1398 1394
     if (c->c2.to_link_addr == NULL) /* no remote addr known */
1399 1395
     {
1400 1396
         return;
1401 1397
     }
1402 1398
 
1403
-    tun_sa = c->c2.to_link_addr->dest;
1399
+    struct openvpn_sockaddr *link_addr = &c->c2.to_link_addr->dest;
1400
+    struct link_socket_info *lsi = get_link_socket_info(c);
1401
+    uint16_t link_port = atoi(c->c2.link_sockets[0]->remote_port);
1404 1402
 
1405
-    int proto_ver = get_tun_ip_ver(TUNNEL_TYPE(c->c1.tuntap), &c->c2.buf, &ip_hdr_offset);
1403
+    int ip_hdr_offset = 0;
1404
+    int tun_ip_ver = get_tun_ip_ver(TUNNEL_TYPE(c->c1.tuntap), &c->c2.buf, &ip_hdr_offset);
1406 1405
 
1407
-    if (proto_ver == 4)
1406
+    if (tun_ip_ver == 4)
1408 1407
     {
1409
-        /* make sure we got whole IP header */
1410
-        if (BLEN(buf) < ((int)sizeof(struct openvpn_iphdr) + ip_hdr_offset))
1408
+        /* make sure we got whole IP header and TCP/UDP src/dst ports */
1409
+        if (BLEN(buf) < ((int)sizeof(struct openvpn_iphdr) + ip_hdr_offset + sizeof(uint16_t) * 2))
1411 1410
         {
1412 1411
             return;
1413 1412
         }
1414 1413
 
1415 1414
         /* skip ipv4 packets for ipv6 tun */
1416
-        if (tun_sa.addr.sa.sa_family != AF_INET)
1415
+        if (link_addr->addr.sa.sa_family != AF_INET)
1417 1416
         {
1418 1417
             return;
1419 1418
         }
1420 1419
 
1421 1420
         struct openvpn_iphdr *pip = (struct openvpn_iphdr *)(BPTR(buf) + ip_hdr_offset);
1422 1421
 
1423
-        /* drop packets with same dest addr as gateway */
1424
-        if (memcmp(&tun_sa.addr.in4.sin_addr.s_addr, &pip->daddr, sizeof(pip->daddr)) == 0)
1422
+        /* skip if tun protocol doesn't match link protocol */
1423
+        if ((lsi->proto == PROTO_TCP && pip->protocol != OPENVPN_IPPROTO_TCP)
1424
+            || (lsi->proto == PROTO_UDP && pip->protocol != OPENVPN_IPPROTO_UDP))
1425
+        {
1426
+            return;
1427
+        }
1428
+
1429
+
1430
+        /* drop packets with same dest addr and port as remote */
1431
+        uint8_t *l4_hdr = (uint8_t *)pip + sizeof(struct openvpn_iphdr);
1432
+
1433
+        /* TCP and UDP ports are at the same place in the header, and other protocols
1434
+         * can not happen here due to the lsi->proto check above */
1435
+        uint16_t src_port = ntohs(*(uint16_t *)l4_hdr);
1436
+        uint16_t dst_port = ntohs(*(uint16_t *)(l4_hdr + sizeof(uint16_t)));
1437
+        if ((memcmp(&link_addr->addr.in4.sin_addr.s_addr, &pip->daddr, sizeof(pip->daddr)) == 0) && (link_port == dst_port))
1425 1438
         {
1426
-            drop = true;
1439
+            c->c2.buf.len = 0;
1440
+
1441
+            struct gc_arena gc = gc_new();
1442
+            msg(D_LOW, "Recursive routing detected, packet dropped %s:%" PRIu16 " -> %s",
1443
+                print_in_addr_t(pip->saddr, IA_NET_ORDER, &gc),
1444
+                src_port,
1445
+                print_link_socket_actual(c->c2.to_link_addr, &gc));
1446
+            gc_free(&gc);
1427 1447
         }
1428 1448
     }
1429
-    else if (proto_ver == 6)
1449
+    else if (tun_ip_ver == 6)
1430 1450
     {
1431
-        /* make sure we got whole IPv6 header */
1432
-        if (BLEN(buf) < ((int)sizeof(struct openvpn_ipv6hdr) + ip_hdr_offset))
1451
+        /* make sure we got whole IPv6 header and TCP/UDP src/dst ports */
1452
+        if (BLEN(buf) < ((int)sizeof(struct openvpn_ipv6hdr) + ip_hdr_offset + sizeof(uint16_t) * 2))
1433 1453
         {
1434 1454
             return;
1435 1455
         }
1436 1456
 
1437 1457
         /* skip ipv6 packets for ipv4 tun */
1438
-        if (tun_sa.addr.sa.sa_family != AF_INET6)
1458
+        if (link_addr->addr.sa.sa_family != AF_INET6)
1439 1459
         {
1440 1460
             return;
1441 1461
         }
1442 1462
 
1443 1463
         struct openvpn_ipv6hdr *pip6 = (struct openvpn_ipv6hdr *)(BPTR(buf) + ip_hdr_offset);
1444 1464
 
1445
-        /* drop packets with same dest addr as gateway */
1446
-        if (OPENVPN_IN6_ARE_ADDR_EQUAL(&tun_sa.addr.in6.sin6_addr, &pip6->daddr))
1465
+        /* skip if tun protocol doesn't match link protocol */
1466
+        if ((lsi->proto == PROTO_TCP && pip6->nexthdr != OPENVPN_IPPROTO_TCP)
1467
+            || (lsi->proto == PROTO_UDP && pip6->nexthdr != OPENVPN_IPPROTO_UDP))
1447 1468
         {
1448
-            drop = true;
1469
+            return;
1449 1470
         }
1450
-    }
1451
-
1452
-    if (drop)
1453
-    {
1454
-        struct gc_arena gc = gc_new();
1455 1471
 
1456
-        c->c2.buf.len = 0;
1472
+        /* drop packets with same dest addr and port as remote */
1473
+        uint8_t *l4_hdr = (uint8_t *)pip6 + sizeof(struct openvpn_ipv6hdr);
1474
+        uint16_t src_port = ntohs(*(uint16_t *)l4_hdr);
1475
+        uint16_t dst_port = ntohs(*(uint16_t *)(l4_hdr + sizeof(uint16_t)));
1476
+        if ((OPENVPN_IN6_ARE_ADDR_EQUAL(&link_addr->addr.in6.sin6_addr, &pip6->daddr)) && (link_port == dst_port))
1477
+        {
1478
+            c->c2.buf.len = 0;
1457 1479
 
1458
-        msg(D_LOW, "Recursive routing detected, drop tun packet to %s",
1459
-            print_link_socket_actual(c->c2.to_link_addr, &gc));
1460
-        gc_free(&gc);
1480
+            struct gc_arena gc = gc_new();
1481
+            msg(D_LOW, "Recursive routing detected, packet dropped %s:%" PRIu16 " -> %s",
1482
+                print_in6_addr(pip6->saddr, IA_NET_ORDER, &gc),
1483
+                src_port,
1484
+                print_link_socket_actual(c->c2.to_link_addr, &gc));
1485
+            gc_free(&gc);
1486
+        }
1461 1487
     }
1462 1488
 }
1463 1489