From patchwork Thu Mar 6 14:37:29 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "d12fk (Code Review)" X-Patchwork-Id: 4167 Return-Path: Delivered-To: patchwork@openvpn.net Received: by 2002:a05:7000:b9c6:b0:60a:d70a:d3c7 with SMTP id gh6csp330207mab; Thu, 6 Mar 2025 06:37:49 -0800 (PST) X-Forwarded-Encrypted: i=2; AJvYcCWTqRPKV/s2QYKU395ob/N2JRAtgjn7lRaVhecF8ul4XA+L/aPxdC1rjsNuAyhGxSF0hGwRNqQN8WI=@openvpn.net X-Google-Smtp-Source: AGHT+IG3AEdnQ18YWqZieWBn5Ts7XfSsGywAjTffiE3lFF5GsI/njug3Evj14C6KAHN/TlGhtR95 X-Received: by 2002:a05:6870:2c97:b0:2b8:8d81:4658 with SMTP id 586e51a60fabf-2c23e8b2ba2mr1867523fac.2.1741271869709; Thu, 06 Mar 2025 06:37:49 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1741271869; cv=none; d=google.com; s=arc-20240605; b=LaivlYDcJkk7Mm94v4T9X+mLcAoOoQ9bqXbvTcTt33osZfEhp/wldEcaMelkjQ+S2e MR9veHTNyzz5Oa/0ZtU5AcUfdLUYgubx3q9ZeJofPOLyXaxKTqcjLdt2a9P2rRycFEo1 4OJaREFShTkFr7SkXKqkXYIM0+WdQXuGyCTmkTnuG95GyxsQyxmOzmnbcWxVFmfrH+TA PM6gTjlJa5oqEvqUr0yHnFbdynzGPf293x+uKFyCQYmgT8PGxeg2QI1NYCYE4yMRm+Xz 9LUZ13KQTTMRgS93BO36MCV1fckWi0Klzh8OpM5fGmM+aaskf/Cw1r6lt7Tvwo7/gylV 67CA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20240605; h=errors-to:cc:reply-to:list-subscribe:list-help:list-post :list-archive:list-unsubscribe:list-id:precedence:subject:user-agent :mime-version:message-id:references:auto-submitted:to:date:from :dkim-signature:dkim-signature:dkim-signature; bh=WFcPdnglVtuXAh9GkpKlwRkqyoq59JPH0Mr4I2lFioM=; fh=U7wEyxtwz2o5+UdevFSA47vNeG9knhWH0KV//QhD5a0=; b=Lx2ipP4+Lhn0b0J2Na99o0Zu1Yc9HC4FoxLpcgZXezXNsmgoIT112ff+4x+95dd7ul izvUjTOdTZJ2mx1pVQrXv0gQANSoCP8HSO9UN1jGeAJ9mXCCnjHli0/vUH+yhlcln098 bwN9oAjxlbfYmvQRIDUGfymGfgpnLgSJ5+IY5sBFjm57fbRpbW36sQeRtm2jODIa9LEo /e5iOG6tyHbqjVKmqh2nJxf1sc+QbEBLk2OAWDHQWq1WuGPMHiqE7O5QzM/T3KE6+he5 eM0UDelFTiDVVQHFZTL2dLQQPCSmsC8E28HFr/7i+xHRdk0OCKUIvvcLMWUJKru3QfzP MAEQ==; dara=google.com ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b=FlU9W9Hd; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=DU0EpKte; dkim=neutral (body hash did not verify) header.i=@openvpn.net header.s=google header.b=bA7ZXybn; spf=pass (google.com: domain of openvpn-devel-bounces@lists.sourceforge.net designates 216.105.38.7 as permitted sender) smtp.mailfrom=openvpn-devel-bounces@lists.sourceforge.net; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=openvpn.net; dara=fail header.i=@openvpn.net Received: from lists.sourceforge.net (lists.sourceforge.net. [216.105.38.7]) by mx.google.com with ESMTPS id 586e51a60fabf-2c248d4b4b3si884003fac.223.2025.03.06.06.37.49 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Thu, 06 Mar 2025 06:37:49 -0800 (PST) Received-SPF: pass (google.com: domain of openvpn-devel-bounces@lists.sourceforge.net designates 216.105.38.7 as permitted sender) client-ip=216.105.38.7; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b=FlU9W9Hd; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=DU0EpKte; dkim=neutral (body hash did not verify) header.i=@openvpn.net header.s=google header.b=bA7ZXybn; spf=pass (google.com: domain of openvpn-devel-bounces@lists.sourceforge.net designates 216.105.38.7 as permitted sender) smtp.mailfrom=openvpn-devel-bounces@lists.sourceforge.net; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=openvpn.net; dara=fail header.i=@openvpn.net Received: from [127.0.0.1] (helo=sfs-ml-3.v29.lw.sourceforge.com) by sfs-ml-3.v29.lw.sourceforge.com with esmtp (Exim 4.95) (envelope-from ) id 1tqCM5-0000i4-Uk; Thu, 06 Mar 2025 14:37:45 +0000 Received: from [172.30.29.66] (helo=mx.sourceforge.net) by sfs-ml-3.v29.lw.sourceforge.com with esmtps (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1tqCM4-0000hw-H1 for openvpn-devel@lists.sourceforge.net; Thu, 06 Mar 2025 14:37:43 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=Content-Type:Content-Transfer-Encoding:MIME-Version :Message-ID:Reply-To:References:Subject:List-Unsubscribe:List-Id:Cc:To:Date: From:Sender:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To:List-Help: List-Subscribe:List-Post:List-Owner:List-Archive; bh=LX9xoTDEUnk36+fmOTgzkAVMX76TlYdqL/SnjkxC8vw=; b=FlU9W9HdPeCjR/JeXKlVRgqj4I Sf7HloHx3UGzVs81ld3x+0Ok+Fzz9JA8SX7ZdxpBI2vwKmGbUtozSjJ5+2kkOcLOCylLL1N1INiGp qZRkyX2z6jmDdd5zh4WTrI8//4VQcOeWATZz0D2lm7M7U8O8HW9DllMMM0VlB3H8YzV8=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=Content-Type:Content-Transfer-Encoding:MIME-Version:Message-ID:Reply-To: References:Subject:List-Unsubscribe:List-Id:Cc:To:Date:From:Sender:Content-ID :Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To: Resent-Cc:Resent-Message-ID:In-Reply-To:List-Help:List-Subscribe:List-Post: List-Owner:List-Archive; bh=LX9xoTDEUnk36+fmOTgzkAVMX76TlYdqL/SnjkxC8vw=; b=D U0EpKtempkdDMZ+2W2vQEYXjEdB2VvlSpgX1Y5ly6YXRZHY81UdHBOqXcISfWHoYeAJ56+BrlBaJB 4bF9B0+ZBfKysNY5PrNl6VKEb5MDPpoeUUYvdtenvPsaU++zTkLFeMOPMMreHMI9+qfU/B0L+1i5m sBRhK0RyUWEeufDI=; Received: from mail-wr1-f53.google.com ([209.85.221.53]) by sfi-mx-2.v28.lw.sourceforge.com with esmtps (TLS1.2:ECDHE-RSA-AES128-GCM-SHA256:128) (Exim 4.95) id 1tqCLx-00068H-IE for openvpn-devel@lists.sourceforge.net; Thu, 06 Mar 2025 14:37:43 +0000 Received: by mail-wr1-f53.google.com with SMTP id ffacd0b85a97d-3912c09be7dso475697f8f.1 for ; Thu, 06 Mar 2025 06:37:37 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=openvpn.net; s=google; t=1741271851; x=1741876651; darn=lists.sourceforge.net; h=user-agent:content-disposition:content-transfer-encoding :mime-version:message-id:reply-to:references:subject :list-unsubscribe:list-id:auto-submitted:cc:to:date:from:from:to:cc :subject:date:message-id:reply-to; bh=LX9xoTDEUnk36+fmOTgzkAVMX76TlYdqL/SnjkxC8vw=; b=bA7ZXybnJtfwMXDEmuFNQnowu0FaBPVeIFviqj1P++eqGKciwEQ3yP9nVWhjxsww/L sfIe3lpZZyd6gk35eJvbAibElHxRYW7Blkhr0pPp4HIlwaHylgzUKeeE8kfUAdncLYVa vAENgsFxRDNMRlr7GNonghDyAjVeyQ726GSEqanmBVYS8akAoEpZnzVfcoJfNlK4cLnY VneJ1yVDYNWhRFTI2UN1U8sWkQRzsIVYTNJpndq9HffBgHDAtIGUQeR6YowmiztfMDvW +XRP/u27VKoAKeRBqfWrAWhmQRuiVaIEVTlJLw1icRAUN5+Gk/xlroczql8iGxVCfq+R SChg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1741271851; x=1741876651; h=user-agent:content-disposition:content-transfer-encoding :mime-version:message-id:reply-to:references:subject :list-unsubscribe:list-id:auto-submitted:cc:to:date:from :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=LX9xoTDEUnk36+fmOTgzkAVMX76TlYdqL/SnjkxC8vw=; b=uYrevzoTtHxSI6HfEbqnBUcVmbBZcWdsEcoZ5bmbAatQjvq96IBHp1nhAXRFsfBNCm gIpSsfsueTwpiws/JdDxqqSDQ8d7IeGIT18lEfpsDK64mLE1QR8fenqPJzP6R6b3ioU9 RgRrTCXdMX5b4mCiV+E3jDABvdMHjmHIIkc4Nk9TRhdYmWdK2G7LNSz3B0M5Xu3rkz6p E3ZRKG4r9GZkMuchcsKyWQe4NUCkb4u04UJCPLjhiQlo6BeBDrr7RJKtdpz7ua3t+dWS JupEJs63n38upYeKrpDkSugxNlCmlet/Z52KuqGnMYGqPlEl0kF1DVCit2HnZZROD3iB SvwA== X-Gm-Message-State: AOJu0Yzn9y8YIKyZd4+5tmSfy2fePSMhf+Fif2BIintacOcRi6fZeeJi CYRZjUxv4VKgUU+2m3u8G4h9HoWZCc3XlqvFfdidh5ftxtRgV4VoDgOxIaekR23MkLxU2QM67ZD y X-Gm-Gg: ASbGncuV19UYCHjiRJPiYrjlEGDagVu+OqGChgKpzhdFkUXi4ar/zOVM/7VOeL8TH3l 1Ev3od2Tj73kj9H0G4k1o7eCpIw8Xvaco4ryg3heReSa56jsftyJYTmFbQG+aXp59jEDm7sokR5 2RIbNfbCah0Z2JACWaV5FX5eoUaVQCjuJ3qNegiohl3+boS/XKk9DZXoTT56DXNIwRjN72XbTKI rMf7YWUmhtajZ9m5NYS1vNCHUS/P6GVDNJwpDEsFdDJAGB5eG3neOOOsUcDKnroUJP5Pnf2P4F6 uoj5ISjOtFZCVLYHrsXY+V4xScHGGccWRwE2SKDAHghU1lm+D/JXfSAqszA9FMDCrEHXkagiYIY DMxAzdKO57WWG2Vi/PdgDIcka99wRgMsgoTgT X-Received: by 2002:a05:6000:2a6:b0:38f:2ef1:dd2b with SMTP id ffacd0b85a97d-3911f740b80mr5189494f8f.22.1741271849912; Thu, 06 Mar 2025 06:37:29 -0800 (PST) Received: from gerrit.openvpn.in (ec2-18-159-0-78.eu-central-1.compute.amazonaws.com. [18.159.0.78]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-3912c0e308dsm2273564f8f.67.2025.03.06.06.37.29 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 06 Mar 2025 06:37:29 -0800 (PST) From: "stipa (Code Review)" X-Google-Original-From: "stipa (Code Review)" X-Gerrit-PatchSet: 1 Date: Thu, 6 Mar 2025 14:37:29 +0000 To: plaisthos , flichtenheld Auto-Submitted: auto-generated X-Gerrit-MessageType: newchange X-Gerrit-Change-Id: I6841e2f2a85275254a04e2d8ae3defe4420db8f6 X-Gerrit-Change-Number: 903 X-Gerrit-Project: openvpn X-Gerrit-ChangeURL: X-Gerrit-Commit: 6c2f192b0da338460f7ced6cd19270fcb8f0205e References: Message-ID: <4e1b7d3ab73e978ecb22a56cec438fef03ad3146-HTML@gerrit.openvpn.net> MIME-Version: 1.0 User-Agent: Gerrit/3.8.2 X-Spam-Score: -0.2 (/) X-Spam-Report: Spam detection software, running on the system "util-spamd-1.v13.lw.sourceforge.com", has NOT identified this incoming email as spam. The original message has been attached to this so you can view it or label similar future email. If you have any questions, see the administrator of that system for details. Content preview: Attention is currently required from: flichtenheld, plaisthos. Hello plaisthos, flichtenheld, I'd like you to do a code review. Please visit Content analysis details: (-0.2 points, 6.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- -0.0 RCVD_IN_DNSWL_NONE RBL: Sender listed at https://www.dnswl.org/, no trust [209.85.221.53 listed in list.dnswl.org] 0.0 RCVD_IN_VALIDITY_RPBL_BLOCKED RBL: ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. [209.85.221.53 listed in bl.score.senderscore.com] 0.0 RCVD_IN_VALIDITY_SAFE_BLOCKED RBL: ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. [209.85.221.53 listed in sa-trusted.bondedsender.org] 0.0 RCVD_IN_MSPIKE_H2 RBL: Average reputation (+2) [209.85.221.53 listed in wl.mailspike.net] -0.0 SPF_PASS SPF: sender matches SPF record 0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record 0.0 WEIRD_PORT URI: Uses non-standard port number for HTTP 0.0 HTML_MESSAGE BODY: HTML included in message 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid -0.1 DKIM_VALID_EF Message has a valid DKIM or DK signature from envelope-from domain -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from author's domain 0.0 T_KAM_HTML_FONT_INVALID Test for Invalidly Named or Formatted Colors in HTML X-Headers-End: 1tqCLx-00068H-IE Subject: [Openvpn-devel] [M] Change in openvpn[master]: Make recursive routing check more fine-grained X-BeenThere: openvpn-devel@lists.sourceforge.net X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: lstipakov@gmail.com, arne-openvpn@rfc2549.org, openvpn-devel@lists.sourceforge.net, frank@lichtenheld.com Cc: openvpn-devel Errors-To: openvpn-devel-bounces@lists.sourceforge.net X-getmail-retrieved-from-mailbox: Inbox X-GMAIL-THRID: =?utf-8?q?1825855891908477795?= X-GMAIL-MSGID: =?utf-8?q?1825855891908477795?= X-getmail-filter-classifier: gerrit message type newchange Attention is currently required from: flichtenheld, plaisthos. Hello plaisthos, flichtenheld, I'd like you to do a code review. Please visit http://gerrit.openvpn.net/c/openvpn/+/903?usp=email to review the following change. Change subject: Make recursive routing check more fine-grained ...................................................................... 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 --- M src/openvpn/forward.c 1 file changed, 63 insertions(+), 38 deletions(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/03/903/1 diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c index b025344..7ec3ef2 100644 --- a/src/openvpn/forward.c +++ b/src/openvpn/forward.c @@ -1386,74 +1386,99 @@ static void drop_if_recursive_routing(struct context *c, struct buffer *buf) { - bool drop = false; - struct openvpn_sockaddr tun_sa; - int ip_hdr_offset = 0; + struct gc_arena gc = gc_new(); if (c->c2.to_link_addr == NULL) /* no remote addr known */ { - return; + goto out; } - tun_sa = c->c2.to_link_addr->dest; + struct openvpn_sockaddr *link_addr = &c->c2.to_link_addr->dest; + struct link_socket_info *lsi = get_link_socket_info(c); + uint16_t link_port = atoi(c->c2.link_sockets[0]->remote_port); - int proto_ver = get_tun_ip_ver(TUNNEL_TYPE(c->c1.tuntap), &c->c2.buf, &ip_hdr_offset); + uint16_t src_port = 0, dst_port = 0; - if (proto_ver == 4) + int ip_hdr_offset = 0; + int tun_ip_ver = get_tun_ip_ver(TUNNEL_TYPE(c->c1.tuntap), &c->c2.buf, &ip_hdr_offset); + + if (tun_ip_ver == 4) { - /* make sure we got whole IP header */ - if (BLEN(buf) < ((int) sizeof(struct openvpn_iphdr) + ip_hdr_offset)) + /* make sure we got whole IP header and TCP/UDP src/dst ports */ + if (BLEN(buf) < ((int) sizeof(struct openvpn_iphdr) + ip_hdr_offset + sizeof(uint16_t) * 2)) { - return; - } - - /* skip ipv4 packets for ipv6 tun */ - if (tun_sa.addr.sa.sa_family != AF_INET) - { - return; + goto out; } struct openvpn_iphdr *pip = (struct openvpn_iphdr *) (BPTR(buf) + ip_hdr_offset); - /* drop packets with same dest addr as gateway */ - if (memcmp(&tun_sa.addr.in4.sin_addr.s_addr, &pip->daddr, sizeof(pip->daddr)) == 0) + /* skip if tun protocol doesn't match link protocol */ + if ((lsi->proto == PROTO_TCP && pip->protocol != OPENVPN_IPPROTO_TCP) + || (lsi->proto == PROTO_UDP && pip->protocol != OPENVPN_IPPROTO_UDP)) { - drop = true; + goto out; + } + + /* skip ipv4 packets for ipv6 tun */ + if (link_addr->addr.sa.sa_family != AF_INET) + { + goto out; + } + + /* drop packets with same dest addr and port as remote */ + uint8_t *l4_hdr = (uint8_t *)pip + sizeof(struct openvpn_iphdr); + uint16_t src_port = ntohs(*(uint16_t *)l4_hdr); + uint16_t dst_port = ntohs(*(uint16_t *)(l4_hdr + sizeof(uint16_t))); + if ((memcmp(&link_addr->addr.in4.sin_addr.s_addr, &pip->daddr, sizeof(pip->daddr)) == 0) && (link_port == dst_port)) + { + c->c2.buf.len = 0; + + msg(D_LOW, "Recursive routing detected, packet dropped %s:%" PRIu16 " -> %s", + print_in_addr_t(pip->saddr, IA_NET_ORDER, &gc), + src_port, + print_link_socket_actual(c->c2.to_link_addr, &gc)); } } - else if (proto_ver == 6) + else if (tun_ip_ver == 6) { - /* make sure we got whole IPv6 header */ - if (BLEN(buf) < ((int) sizeof(struct openvpn_ipv6hdr) + ip_hdr_offset)) + /* make sure we got whole IPv6 header and TCP/UDP src/dst ports */ + if (BLEN(buf) < ((int) sizeof(struct openvpn_ipv6hdr) + ip_hdr_offset + sizeof(uint16_t) * 2)) { - return; + goto out; } /* skip ipv6 packets for ipv4 tun */ - if (tun_sa.addr.sa.sa_family != AF_INET6) + if (link_addr->addr.sa.sa_family != AF_INET6) { - return; + goto out; } struct openvpn_ipv6hdr *pip6 = (struct openvpn_ipv6hdr *) (BPTR(buf) + ip_hdr_offset); - /* drop packets with same dest addr as gateway */ - if (OPENVPN_IN6_ARE_ADDR_EQUAL(&tun_sa.addr.in6.sin6_addr, &pip6->daddr)) + /* skip if tun protocol doesn't match link protocol */ + if ((lsi->proto == PROTO_TCP && pip6->nexthdr != OPENVPN_IPPROTO_TCP) + || (lsi->proto == PROTO_UDP && pip6->nexthdr != OPENVPN_IPPROTO_UDP)) { - drop = true; + goto out; + } + + /* drop packets with same dest addr and port as remote */ + uint8_t *l4_hdr = (uint8_t *)pip6 + sizeof(struct openvpn_ipv6hdr); + src_port = ntohs(*(uint16_t *)l4_hdr); + dst_port = ntohs(*(uint16_t *)(l4_hdr + sizeof(uint16_t))); + if ((OPENVPN_IN6_ARE_ADDR_EQUAL(&link_addr->addr.in6.sin6_addr, &pip6->daddr)) && (link_port == dst_port)) + { + c->c2.buf.len = 0; + + msg(D_LOW, "Recursive routing detected, packet dropped %s:%" PRIu16 " -> %s", + print_in6_addr(pip6->saddr, IA_NET_ORDER, &gc), + src_port, + print_link_socket_actual(c->c2.to_link_addr, &gc)); } } - if (drop) - { - struct gc_arena gc = gc_new(); - - c->c2.buf.len = 0; - - msg(D_LOW, "Recursive routing detected, drop tun packet to %s", - print_link_socket_actual(c->c2.to_link_addr, &gc)); - gc_free(&gc); - } +out: + gc_free(&gc); } /*