Browse code

Minor fix to process_ip_header

Removed if-guard checking if any feature is
enabled before performing per-feature check.
It doesn't save us much but instead introduces
uneeded complexity.

While at it, fixed a typo IMCP -> ICMP for defined
PIPV6_ICMP_NOHOST_CLIENT and PIPV6_ICMP_NOHOST_SERVER
macros.

Fixes: Trac https://community.openvpn.net/openvpn/ticket/269
Change-Id: I4b5e8357d872c920efdb64632e9bce72cebee202
Signed-off-by: Gianmarco De Gregori <gianmarco@mandelbit.com>
Acked-by: Arne Schwabe <arne-openvpn@rfc2549.org>
Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20240307124616.16358-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28345.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>

Gianmarco De Gregori authored on 2024/03/07 21:46:16
Showing 3 changed files
... ...
@@ -1460,7 +1460,7 @@ process_incoming_tun(struct context *c)
1460 1460
          * us to examine the IP header (IPv4 or IPv6).
1461 1461
          */
1462 1462
         unsigned int flags = PIPV4_PASSTOS | PIP_MSSFIX | PIPV4_CLIENT_NAT
1463
-                             | PIPV6_IMCP_NOHOST_CLIENT;
1463
+                             | PIPV6_ICMP_NOHOST_CLIENT;
1464 1464
         process_ip_header(c, flags, &c->c2.buf);
1465 1465
 
1466 1466
 #ifdef PACKET_TRUNCATION_CHECK
... ...
@@ -1644,74 +1644,61 @@ process_ip_header(struct context *c, unsigned int flags, struct buffer *buf)
1644 1644
     }
1645 1645
     if (!c->options.block_ipv6)
1646 1646
     {
1647
-        flags &= ~(PIPV6_IMCP_NOHOST_CLIENT | PIPV6_IMCP_NOHOST_SERVER);
1647
+        flags &= ~(PIPV6_ICMP_NOHOST_CLIENT | PIPV6_ICMP_NOHOST_SERVER);
1648 1648
     }
1649 1649
 
1650 1650
     if (buf->len > 0)
1651 1651
     {
1652
-        /*
1653
-         * The --passtos and --mssfix options require
1654
-         * us to examine the IPv4 header.
1655
-         */
1656
-
1657
-        if (flags & (PIP_MSSFIX
1658
-#if PASSTOS_CAPABILITY
1659
-                     | PIPV4_PASSTOS
1660
-#endif
1661
-                     | PIPV4_CLIENT_NAT
1662
-                     ))
1652
+        struct buffer ipbuf = *buf;
1653
+        if (is_ipv4(TUNNEL_TYPE(c->c1.tuntap), &ipbuf))
1663 1654
         {
1664
-            struct buffer ipbuf = *buf;
1665
-            if (is_ipv4(TUNNEL_TYPE(c->c1.tuntap), &ipbuf))
1666
-            {
1667 1655
 #if PASSTOS_CAPABILITY
1668
-                /* extract TOS from IP header */
1669
-                if (flags & PIPV4_PASSTOS)
1670
-                {
1671
-                    link_socket_extract_tos(c->c2.link_socket, &ipbuf);
1672
-                }
1656
+            /* extract TOS from IP header */
1657
+            if (flags & PIPV4_PASSTOS)
1658
+            {
1659
+                link_socket_extract_tos(c->c2.link_socket, &ipbuf);
1660
+            }
1673 1661
 #endif
1674 1662
 
1675
-                /* possibly alter the TCP MSS */
1676
-                if (flags & PIP_MSSFIX)
1677
-                {
1678
-                    mss_fixup_ipv4(&ipbuf, c->c2.frame.mss_fix);
1679
-                }
1663
+            /* possibly alter the TCP MSS */
1664
+            if (flags & PIP_MSSFIX)
1665
+            {
1666
+                mss_fixup_ipv4(&ipbuf, c->c2.frame.mss_fix);
1667
+            }
1680 1668
 
1681
-                /* possibly do NAT on packet */
1682
-                if ((flags & PIPV4_CLIENT_NAT) && c->options.client_nat)
1683
-                {
1684
-                    const int direction = (flags & PIP_OUTGOING) ? CN_INCOMING : CN_OUTGOING;
1685
-                    client_nat_transform(c->options.client_nat, &ipbuf, direction);
1686
-                }
1687
-                /* possibly extract a DHCP router message */
1688
-                if (flags & PIPV4_EXTRACT_DHCP_ROUTER)
1689
-                {
1690
-                    const in_addr_t dhcp_router = dhcp_extract_router_msg(&ipbuf);
1691
-                    if (dhcp_router)
1692
-                    {
1693
-                        route_list_add_vpn_gateway(c->c1.route_list, c->c2.es, dhcp_router);
1694
-                    }
1695
-                }
1669
+            /* possibly do NAT on packet */
1670
+            if ((flags & PIPV4_CLIENT_NAT) && c->options.client_nat)
1671
+            {
1672
+                const int direction = (flags & PIP_OUTGOING) ? CN_INCOMING : CN_OUTGOING;
1673
+                client_nat_transform(c->options.client_nat, &ipbuf, direction);
1696 1674
             }
1697
-            else if (is_ipv6(TUNNEL_TYPE(c->c1.tuntap), &ipbuf))
1675
+            /* possibly extract a DHCP router message */
1676
+            if (flags & PIPV4_EXTRACT_DHCP_ROUTER)
1698 1677
             {
1699
-                /* possibly alter the TCP MSS */
1700
-                if (flags & PIP_MSSFIX)
1701
-                {
1702
-                    mss_fixup_ipv6(&ipbuf, c->c2.frame.mss_fix);
1703
-                }
1704
-                if (!(flags & PIP_OUTGOING) && (flags
1705
-                                                &(PIPV6_IMCP_NOHOST_CLIENT | PIPV6_IMCP_NOHOST_SERVER)))
1678
+                const in_addr_t dhcp_router = dhcp_extract_router_msg(&ipbuf);
1679
+                if (dhcp_router)
1706 1680
                 {
1707
-                    ipv6_send_icmp_unreachable(c, buf,
1708
-                                               (bool)(flags & PIPV6_IMCP_NOHOST_CLIENT));
1709
-                    /* Drop the IPv6 packet */
1710
-                    buf->len = 0;
1681
+                    route_list_add_vpn_gateway(c->c1.route_list, c->c2.es, dhcp_router);
1711 1682
                 }
1712
-
1713 1683
             }
1714 1684
         }
1685
+        else if (is_ipv6(TUNNEL_TYPE(c->c1.tuntap), &ipbuf))
1686
+        {
1687
+            /* possibly alter the TCP MSS */
1688
+            if (flags & PIP_MSSFIX)
1689
+            {
1690
+                mss_fixup_ipv6(&ipbuf, c->c2.frame.mss_fix);
1691
+            }
1692
+            if (!(flags & PIP_OUTGOING) && (flags
1693
+                                            &(PIPV6_ICMP_NOHOST_CLIENT | PIPV6_ICMP_NOHOST_SERVER)))
1694
+            {
1695
+                ipv6_send_icmp_unreachable(c, buf,
1696
+                                           (bool)(flags & PIPV6_ICMP_NOHOST_CLIENT));
1697
+                /* Drop the IPv6 packet */
1698
+                buf->len = 0;
1699
+            }
1700
+
1701
+        }
1715 1702
     }
1716 1703
 }
1717 1704
 
... ...
@@ -297,8 +297,9 @@ void reschedule_multi_process(struct context *c);
297 297
 #define PIP_OUTGOING                    (1<<2)
298 298
 #define PIPV4_EXTRACT_DHCP_ROUTER       (1<<3)
299 299
 #define PIPV4_CLIENT_NAT                (1<<4)
300
-#define PIPV6_IMCP_NOHOST_CLIENT        (1<<5)
301
-#define PIPV6_IMCP_NOHOST_SERVER        (1<<6)
300
+#define PIPV6_ICMP_NOHOST_CLIENT        (1<<5)
301
+#define PIPV6_ICMP_NOHOST_SERVER        (1<<6)
302
+
302 303
 
303 304
 void process_ip_header(struct context *c, unsigned int flags, struct buffer *buf);
304 305
 
... ...
@@ -3645,7 +3645,7 @@ multi_get_queue(struct mbuf_set *ms)
3645 3645
 
3646 3646
     if (mbuf_extract_item(ms, &item)) /* cleartext IP packet */
3647 3647
     {
3648
-        unsigned int pip_flags = PIPV4_PASSTOS | PIPV6_IMCP_NOHOST_SERVER;
3648
+        unsigned int pip_flags = PIPV4_PASSTOS | PIPV6_ICMP_NOHOST_SERVER;
3649 3649
 
3650 3650
         set_prefix(item.instance);
3651 3651
         item.instance->context.c2.buf = item.buffer->buf;