From patchwork Sat Aug 20 04:01:24 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Gert Doering X-Patchwork-Id: 2707 Return-Path: Delivered-To: patchwork@openvpn.net Delivered-To: patchwork@openvpn.net Received: from director12.mail.ord1d.rsapps.net ([172.28.255.1]) by backend30.mail.ord1d.rsapps.net with LMTP id 8FJ/H2zpAGOAeQAAIUCqbw (envelope-from ) for ; Sat, 20 Aug 2022 10:02:20 -0400 Received: from proxy9.mail.ord1c.rsapps.net ([172.28.255.1]) by director12.mail.ord1d.rsapps.net with LMTP id +HxcH2zpAGMqcwAAIasKDg (envelope-from ) for ; Sat, 20 Aug 2022 10:02:20 -0400 Received: from smtp27.gate.ord1c ([172.28.255.1]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by proxy9.mail.ord1c.rsapps.net with LMTPS id 6B0SH2zpAGMHaQAAgxtkuw (envelope-from ) for ; Sat, 20 Aug 2022 10:02:20 -0400 X-Spam-Threshold: 95 X-Spam-Score: 0 X-Spam-Flag: NO X-Virus-Scanned: OK X-Orig-To: openvpnslackdevel@openvpn.net X-Originating-Ip: [216.105.38.7] Authentication-Results: smtp27.gate.ord1c.rsapps.net; iprev=pass policy.iprev="216.105.38.7"; spf=pass smtp.mailfrom="openvpn-devel-bounces@lists.sourceforge.net" smtp.helo="lists.sourceforge.net"; dkim=fail (signature verification failed) header.d=sourceforge.net; dkim=fail (signature verification failed) header.d=sf.net; dmarc=none (p=nil; dis=none) header.from=greenie.muc.de X-Suspicious-Flag: YES X-Classification-ID: b4993118-2090-11ed-af2d-b8ca3a655ab8-1-1 Received: from [216.105.38.7] ([216.105.38.7:41408] helo=lists.sourceforge.net) by smtp27.gate.ord1c.rsapps.net (envelope-from ) (ecelerity 4.2.38.62370 r(:)) with ESMTPS (cipher=DHE-RSA-AES256-GCM-SHA384) id 7C/66-06467-B69E0036; Sat, 20 Aug 2022 10:02:20 -0400 Received: from [127.0.0.1] (helo=sfs-ml-2.v29.lw.sourceforge.com) by sfs-ml-2.v29.lw.sourceforge.com with esmtp (Exim 4.95) (envelope-from ) id 1oPP2r-0001Ua-0f; Sat, 20 Aug 2022 14:01:49 +0000 Received: from [172.30.20.202] (helo=mx.sourceforge.net) by sfs-ml-2.v29.lw.sourceforge.com with esmtps (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1oPP2f-0001UI-15 for openvpn-devel@lists.sourceforge.net; Sat, 20 Aug 2022 14:01:42 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=Content-Transfer-Encoding:MIME-Version:Message-Id: Date:Subject:To:From:Sender:Reply-To:Cc:Content-Type:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:In-Reply-To:References:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=+ddCH/Q33ngXgo3OwRAFFFlQdSVGJAzcDVMjpDppkIc=; b=mJ9bxkDuNUmQiEPh3e1sTMOWdE 2YiftfLu5rnoayk3RHFLtAmwcZfyhW8gNr6ekv0XqVeEunrYLMbS8vMIgCsWqBvFpQX2GuZEIr1vK 92DwI4M+5p+Ne4T9XtZiwRoT+DdDHmIgADRt+FicjfQDkMZJUUJ82VW/YHqQAr1Hho9s=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=Content-Transfer-Encoding:MIME-Version:Message-Id:Date:Subject:To:From: Sender:Reply-To:Cc:Content-Type:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To: References:List-Id:List-Help:List-Unsubscribe:List-Subscribe:List-Post: List-Owner:List-Archive; bh=+ddCH/Q33ngXgo3OwRAFFFlQdSVGJAzcDVMjpDppkIc=; b=l 9WP0fqga8W7br9iY+Nm+5WuPQft6NgXAJY7+4pZQum3UnDVzKE4iIkaCoUz2uGwAfDw4Uzp0tk8AQ kRpASmFw5bVVO+KZC6WV2IkwrHmfhrdvEl/HdOOVhjKV+A5mJWvzGeDmC1fXPwS/0Dgmtd/TzZvLQ XyVh8sXtrk2DARbA=; Received: from vmail1.greenie.net ([195.30.8.66]) by sfi-mx-1.v28.lw.sourceforge.com with esmtps (TLS1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.95) id 1oPP2a-00Aih3-8a for openvpn-devel@lists.sourceforge.net; Sat, 20 Aug 2022 14:01:36 +0000 Received: from fbsd14.ov.greenie.net (fbsd14.ov.greenie.net [IPv6:2001:608:0:814:0:0:fb00:14]) by vmail1.greenie.net (8.17.1/8.16.1) with ESMTPS id 27KE1OB8086078 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=FAIL) for ; Sat, 20 Aug 2022 16:01:24 +0200 (CEST) Received: from fbsd14.ov.greenie.net (localhost [127.0.0.1]) by fbsd14.ov.greenie.net (8.16.1/8.16.1) with ESMTPS id 27KE1O5I011334 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NO) for ; Sat, 20 Aug 2022 16:01:24 +0200 (CEST) (envelope-from gert@fbsd14.ov.greenie.net) Received: (from gert@localhost) by fbsd14.ov.greenie.net (8.16.1/8.16.1/Submit) id 27KE1OdK011333 for openvpn-devel@lists.sourceforge.net; Sat, 20 Aug 2022 16:01:24 +0200 (CEST) (envelope-from gert) From: Gert Doering To: openvpn-devel@lists.sourceforge.net Date: Sat, 20 Aug 2022 16:01:24 +0200 Message-Id: <20220820140124.11325-1-gert@greenie.muc.de> X-Mailer: git-send-email 2.37.1 MIME-Version: 1.0 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.6.4 (vmail1.greenie.net [IPv6:2001:608:1:995a:20c:29ff:feb8:10eb]); Sat, 20 Aug 2022 16:01:24 +0200 (CEST) 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: The existing DCO code had extra logic for "if this is not MR_WITH_NETBITS, set 32/128 as address length", but only for iroute addition. For iroute deletion, this was missing, and subsequently iroute d [...] Content analysis details: (-2.0 points, 6.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- -2.3 RCVD_IN_DNSWL_MED RBL: Sender listed at https://www.dnswl.org/, medium trust [195.30.8.66 listed in list.dnswl.org] 0.2 HEADER_FROM_DIFFERENT_DOMAINS From and EnvelopeFrom 2nd level mail domains are different 0.0 SPF_NONE SPF: sender does not publish an SPF Record 0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record X-Headers-End: 1oPP2a-00Aih3-8a Subject: [Openvpn-devel] [PATCH] DCO: require valid netbits setting for non-primary iroutes. 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: , Errors-To: openvpn-devel-bounces@lists.sourceforge.net X-getmail-retrieved-from-mailbox: Inbox The existing DCO code had extra logic for "if this is not MR_WITH_NETBITS, set 32/128 as address length", but only for iroute addition. For iroute deletion, this was missing, and subsequently iroute deletion for IPv4 host routes failed on FreeBSD DCO (commit 3433577a99). Iroute handling differenciates between "primary" iroutes (coming from anm IP pool or ccd/ifconfig-push), and "non-primary" iroutes, coming from --iroute and --iroute-ipv6 statements in per-client config. "Primary" iroutes always use "-1" for their netbits, but since these are not installed via DCO, this is of no concern here. Whether these can and should be changed needs further study on internal route learning and cleanup. Refactor options.c and multi.c to ensure that netbits is always set for non-primary iroutes - and ASSERT() on this in the DCO path, so we can find out if there might be other code violating this. Change options.c::option_iroute() to always set netbits=32 for IPv4 host routes (options_iroute_ipv6() never differenciated). Since netmask_to_netbits() also insists on "-1" for host routes, change to netmask_to_netbits2(). Remove all the extra MR_WITH_NETBITS logic from dco.c, where it should have never appeared. Signed-off-by: Gert Doering Acked-by: Kristof Provost Acked-by: Antonio Quartulli --- src/openvpn/dco.c | 16 ++-------------- src/openvpn/multi.c | 2 ++ src/openvpn/options.c | 6 ++++-- 3 files changed, 8 insertions(+), 16 deletions(-) diff --git a/src/openvpn/dco.c b/src/openvpn/dco.c index 99b544a5..cf20334c 100644 --- a/src/openvpn/dco.c +++ b/src/openvpn/dco.c @@ -603,26 +603,14 @@ dco_install_iroute(struct multi_context *m, struct multi_instance *mi, if (addrtype == MR_ADDR_IPV6) { - int netbits = 128; - if (addr->type & MR_WITH_NETBITS) - { - netbits = addr->netbits; - } - - net_route_v6_add(&m->top.net_ctx, &addr->v6.addr, netbits, + net_route_v6_add(&m->top.net_ctx, &addr->v6.addr, addr->netbits, &mi->context.c2.push_ifconfig_ipv6_local, dev, 0, DCO_IROUTE_METRIC); } else if (addrtype == MR_ADDR_IPV4) { - int netbits = 32; - if (addr->type & MR_WITH_NETBITS) - { - netbits = addr->netbits; - } - in_addr_t dest = htonl(addr->v4.addr); - net_route_v4_add(&m->top.net_ctx, &dest, netbits, + net_route_v4_add(&m->top.net_ctx, &dest, addr->netbits, &mi->context.c2.push_ifconfig_local, dev, 0, DCO_IROUTE_METRIC); } diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c index 95414429..1f9d0201 100644 --- a/src/openvpn/multi.c +++ b/src/openvpn/multi.c @@ -1241,6 +1241,7 @@ multi_learn_in_addr_t(struct multi_context *m, /* "primary" is the VPN ifconfig address of the peer and already * known to DCO, so only install "extra" iroutes (primary = false) */ + ASSERT(netbits>=0); /* DCO requires populated netbits */ dco_install_iroute(m, mi, &addr); } @@ -1280,6 +1281,7 @@ multi_learn_in6_addr(struct multi_context *m, /* "primary" is the VPN ifconfig address of the peer and already * known to DCO, so only install "extra" iroutes (primary = false) */ + ASSERT(netbits>=0); /* DCO requires populated netbits */ dco_install_iroute(m, mi, &addr); } diff --git a/src/openvpn/options.c b/src/openvpn/options.c index 2b0bb20c..21e2f69b 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -1572,12 +1572,14 @@ option_iroute(struct options *o, ALLOC_OBJ_GC(ir, struct iroute, &o->gc); ir->network = getaddr(GETADDR_HOST_ORDER, network_str, 0, NULL, NULL); - ir->netbits = -1; + ir->netbits = 32; /* host route if no netmask given */ if (netmask_str) { const in_addr_t netmask = getaddr(GETADDR_HOST_ORDER, netmask_str, 0, NULL, NULL); - if (!netmask_to_netbits(ir->network, netmask, &ir->netbits)) + ir->netbits = netmask_to_netbits2(netmask); + + if (ir->netbits<0) { msg(msglevel, "in --iroute %s %s : Bad network/subnet specification", network_str,