From patchwork Thu Nov 5 17:15:52 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Gert Doering X-Patchwork-Id: 5607 Return-Path: Delivered-To: patchwork@openvpn.net Received: by 2002:a05:6022:30a9:b0:118:2077:975e with SMTP id f41csp2192813laa; Thu, 5 Nov 2026 09:16:09 -0800 (PST) X-Forwarded-Encrypted: i=2; AKwUvBxaK+x7ADRbtEKHHL7F8+5yOXhU4flsD5oQeDaICs3LggJfy+KLrOLKz1H7f+1gN30jvVmevLIb2A0=@openvpn.net X-Received: by 2002:a05:7022:f409:b0:151:2e62:fe6d with SMTP id a92af1059eb24-1ac5bda5860mr38983530c88.22.1793898969050; Thu, 05 Nov 2026 09:16:09 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1793898969; cv=none; d=google.com; s=arc-20260327; b=nZoAQmnGHWhOs79puQG/1KlVhXHwnGnWim9TSTprZoPyWxTbUTt1FX8PFtLL4LvGoO PyXHWijkFEMGlqPDoCOuZ7zz3PKvxejmcpalkznXpfmx2VrKeen75YnUavSXLMnDJ1Ju X8Ln9/Ew/kiLl9PX/4GdNMUOe/R3HnkLu7MOP/NbFpm4aUH8H36OQxtO1DXaoLTMreqC TMOtEbwBLW7IFJ/w86VJ9jr7pmdRdmCxcDMUN1k8D8yxskcF4z91Nc1Jxed9RFVgRnhP KD0uWBiSsi8uLw/q7Z6vK8zIDS++RW861voSBApQP2q/r4PORSkdnraVj4TPOK1SoXE/ rvNg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20260327; h=errors-to:content-transfer-encoding:list-subscribe:list-help :list-post:list-archive:list-unsubscribe:list-id:precedence:subject :mime-version:references:in-reply-to:message-id:date:to:from :dkim-signature:dkim-signature:dkim-signature; bh=KhHG4e1WBzNhUgi4OeTJnaFh9OE7pkdh1XBahm6NBfw=; fh=4NbAC/LsuMLI0S0hprUlLSLCiHwg6SCAifhH718Jh0Q=; b=dAZxegXFthCgjWHOun1aHcrn+u9uMHTCzn4dk/ti6ZoktHUDdyNn5UmbsVQ4rjiDaV p16yYLc3bdqHgvrAzfimuJdiC45axHt+RORZzxK24xT79EIWZb8UfjG2PMPwNONv0Qb4 Zmo2nj4MQO+kVJgDnaQf3Ev5uIm1uQgETEP1Oj5v0zH3Z+KpAk779/kPGcmdO1/a64Pn TzHu9rp2KeAvEfZn/MtOb0imHym3HzkHS1U4cmylSsHKBrMLNrQW97NaDOw5NcqDwmyD GFcnmE78mv4UmxtaQMuB0cRpRObBBXhEwu2MELBZhXO69xmy4eEAvLMN7RilFC3FjRwN jILA==; dara=google.com ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@lists.sourceforge.net header.s=beta header.b=Efay2IoL; dkim=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b=DqPUPMWe; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=KEKUJqfW; 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=muc.de Received: from lists.sourceforge.net (lists.sourceforge.net. [216.105.38.7]) by mx.google.com with ESMTPS id a92af1059eb24-1b2588e9a64si1310608c88.81.2026.11.05.09.16.08 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Thu, 05 Nov 2026 09:16:09 -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=pass header.i=@lists.sourceforge.net header.s=beta header.b=Efay2IoL; dkim=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b=DqPUPMWe; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=KEKUJqfW; 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=muc.de DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.sourceforge.net; s=beta; h=Content-Transfer-Encoding:Content-Type: List-Subscribe:List-Help:List-Post:List-Archive:List-Unsubscribe:List-Id: Subject:MIME-Version:References:In-Reply-To:Message-ID:Date:To:From:Sender: Reply-To:Cc:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Unsubscribe-Post: List-Owner; bh=KhHG4e1WBzNhUgi4OeTJnaFh9OE7pkdh1XBahm6NBfw=; b=Efay2IoL7OcBVs GPEGCwpRmqm2JT1Oz+CiZ0EIfTpPY95dvHOh5F/1iCqvmiQXL1sYhAAezYYeKpUjlYsk8HsTdU79F VMbshGa6uQL47zGoEfkf8alfMpv6N4M2AFR/WsVXp/NO1LTdr8E/mDiKXrVlbFLKbTi3nviaIvYka ffU=; Received: from [127.0.0.1] (helo=sfs-ml-4.v29.lw.sourceforge.com) by sfs-ml-4.v29.lw.sourceforge.com with esmtp (Exim 4.100.1) (envelope-from ) id 1xP14I-000000005sD-3vmF; Thu, 05 Nov 2026 17:16:06 +0000 Received: from [172.30.29.66] (helo=mx.sourceforge.net) by sfs-ml-4.v29.lw.sourceforge.com with esmtps (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.100.1) (envelope-from ) id 1xP14H-000000005s5-3Isa for openvpn-devel@lists.sourceforge.net; Thu, 05 Nov 2026 17:16:05 +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:References: In-Reply-To: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:List-Id:List-Help:List-Unsubscribe: List-Unsubscribe-Post:List-Subscribe:List-Post:List-Owner:List-Archive; bh=GGqXfqP7GVLJJ/mvGHzl2z9WcdNFCO35p6fY/fdkAhM=; b=DqPUPMWe2bSTDXx+TZPCLxLA1a obmcaurfgv6lSSiuYbu5R+x3IiHoJohVgT4779RHzZVs0D2vuOZjHgCcAC+sNxho12V9Edw2FIhXF 3db14rZbIF87N03QE2a+mL27YFyXPOqUU0qNNJAayd/V+hnm29+cCuYVZiNUSzAR5GrI=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=Content-Transfer-Encoding:MIME-Version:References:In-Reply-To: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:List-Id:List-Help:List-Unsubscribe:List-Unsubscribe-Post: List-Subscribe:List-Post:List-Owner:List-Archive; bh=GGqXfqP7GVLJJ/mvGHzl2z9WcdNFCO35p6fY/fdkAhM=; b=KEKUJqfWee2iHuCyk9L3G5fqPj 0bFHSZ0fZjPICm30GBo1lcRMVEnKpnZJelC1LNdj5fYFSzaM0ZMboVoLX0QotE/M9aZKKUpsRwuzd 10uUkLPXNVftsYOZXVdNngCjPSzX8noR0OQHLH7grKBUy8bUOZIiTmLafEbIE5+0jP5g=; Received: from [193.149.48.129] (helo=blue.greenie.muc.de) by sfi-mx-1.v28.lw.sourceforge.com with esmtps (TLS1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.100.1) id 1xP14G-000000007qz-0JR1 for openvpn-devel@lists.sourceforge.net; Thu, 05 Nov 2026 17:16:05 +0000 Received: from blue.greenie.muc.de (localhost [127.0.0.1]) by blue.greenie.muc.de (8.18.1/8.18.1) with ESMTP id 6A5HFvOn028237 for ; Thu, 5 Nov 2026 18:15:57 +0100 Received: (from gert@localhost) by blue.greenie.muc.de (8.18.2/8.18.1/Submit) id 6A5HFvLN028234 for openvpn-devel@lists.sourceforge.net; Thu, 5 Nov 2026 18:15:57 +0100 From: Gert Doering To: openvpn-devel@lists.sourceforge.net Date: Thu, 5 Nov 2026 18:15:52 +0100 Message-ID: <20261105171557.28206-1-gert@greenie.muc.de> X-Mailer: git-send-email 2.55.0 In-Reply-To: References: MIME-Version: 1.0 X-Spam-Score: 1.3 (+) X-Spam-Report: Spam detection software, running on the system "sfi-spamd-2.hosts.colo.sdot.me", 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: From: Marco Baffo Use the current virtual addresses as parser defaults so omitted options remain unchanged and NULL consistently means address removal. Move removal into update_vaddr(), skip unchanged addresses, and se [...] Content analysis details: (1.3 points, 5.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- 1.3 RDNS_NONE Delivered to internal network by a host with no rDNS X-Headers-End: 1xP14G-000000007qz-0JR1 Subject: [Openvpn-devel] [PATCH v2] PUSH_UPDATE: centralize virtual address changes in update_vaddr 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 X-GMAIL-THRID: 1881039405234458394 X-GMAIL-MSGID: 1881039405234458394 From: Marco Baffo Use the current virtual addresses as parser defaults so omitted options remain unchanged and NULL consistently means address removal. Move removal into update_vaddr(), skip unchanged addresses, and set the new address before learning its route. If learning either new address fails, restart the client since the PUSH_UPDATE is already queued. Report the scheduled exit as a failed update so management does not count the client as successfully updated. Change-Id: I75ba77fcbb11790c7cc86cc436fc75ff99bd1c9e Signed-off-by: Marco Baffo Acked-by: Razvan Cojocaru Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/2060 --- This change was reviewed on Gerrit and approved by at least one developer. I request to merge it to master. Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/2060 This mail reflects revision 2 of this Change. Acked-by according to Gerrit (reflected above): Razvan Cojocaru diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c index af8b1c7..0027550 100644 --- a/src/openvpn/multi.c +++ b/src/openvpn/multi.c @@ -4477,7 +4477,7 @@ } /* Function to unlearn previous ifconfig of a client in the server multi_context after a PUSH_UPDATE */ -void +static void unlearn_ifconfig(struct multi_context *m, struct multi_instance *mi) { in_addr_t old_addr = 0; @@ -4489,7 +4489,7 @@ } /* Function to unlearn previous ifconfig-ipv6 of a client in the server multi_context after a PUSH_UPDATE */ -void +static void unlearn_ifconfig_ipv6(struct multi_context *m, struct multi_instance *mi) { struct in6_addr old_addr6; @@ -4502,59 +4502,102 @@ } /** - * Update the vhash with new IP/IPv6 addresses in the multi_context when a - * push-update message containing ifconfig/ifconfig-ipv6 options is sent - * from the server. + * Update the vhash when a push-update message containing + * ifconfig/ifconfig-ipv6 options is sent from the server. * * @param m The multi_context * @param mi The multi_instance of the client we are updating - * @param new_ip The new IPv4 address or NULL if no change - * @param new_ipv6 The new IPv6 address or NULL if no change + * @param new_ip The new IPv4 address, NULL to remove, current address to keep + * @param new_ipv6 The new IPv6 address, NULL to remove, current address to keep + * @return false if the update failed, true otherwise. On failure the + * client is restarted */ -void -update_vhash(struct multi_context *m, struct multi_instance *mi, const char *new_ip, const char *new_ipv6) +bool +update_vaddr(struct multi_context *m, struct multi_instance *mi, const char *new_ip, const char *new_ipv6) { + /* -ifconfig */ + if (!new_ip && mi->context.c2.push_ifconfig_defined) + { + unlearn_ifconfig(m, mi); + } + /* -ifconfig-ipv6 */ + if (!new_ipv6 && mi->context.c2.push_ifconfig_ipv6_defined) + { + unlearn_ifconfig_ipv6(m, mi); + } + if (new_ip) { - /* Remove old IP */ - if (mi->context.c2.push_ifconfig_defined) - { - unlearn_ifconfig(m, mi); - } - - /* Add new IP */ struct in_addr new_addr; - CLEAR(new_addr); if (inet_pton(AF_INET, new_ip, &new_addr) == 1 - && multi_learn_in_addr_t(m, mi, ntohl(new_addr.s_addr), -1, true)) + && (!mi->context.c2.push_ifconfig_defined + || ntohl(new_addr.s_addr) != mi->context.c2.push_ifconfig_local)) { + /* Remove old IP */ + if (mi->context.c2.push_ifconfig_defined) + { + unlearn_ifconfig(m, mi); + } + + /* DCO needs the new address while installing its on-link route. */ mi->context.c2.push_ifconfig_defined = true; mi->context.c2.push_ifconfig_local = ntohl(new_addr.s_addr); - /* set our client's VPN endpoint for status reporting purposes */ - mi->reporting_addr = mi->context.c2.push_ifconfig_local; + if (multi_learn_in_addr_t(m, mi, mi->context.c2.push_ifconfig_local, -1, true) == mi) + { + /* set our client's VPN endpoint for status reporting purposes */ + mi->reporting_addr = mi->context.c2.push_ifconfig_local; + } + else + { + mi->context.c2.push_ifconfig_defined = false; + mi->context.c2.push_ifconfig_local = 0; + + msg(M_WARN, "MULTI: failed to learn PUSH_UPDATE IPv4 address %s, restarting client", + new_ip); + /* PUSH_UPDATE is already queued, so reconnect to restore consistent state. */ + send_restart(&mi->context, NULL); + multi_schedule_context_wakeup(m, mi); + return false; + } } } if (new_ipv6) { - /* Remove old IPv6 */ - if (mi->context.c2.push_ifconfig_ipv6_defined) - { - unlearn_ifconfig_ipv6(m, mi); - } - - /* Add new IPv6 */ struct in6_addr new_addr6; - CLEAR(new_addr6); if (inet_pton(AF_INET6, new_ipv6, &new_addr6) == 1 - && multi_learn_in6_addr(m, mi, new_addr6, -1, true)) + && (!mi->context.c2.push_ifconfig_ipv6_defined + || memcmp(&new_addr6, &mi->context.c2.push_ifconfig_ipv6_local, sizeof(new_addr6)))) { + /* Remove old IPv6 */ + if (mi->context.c2.push_ifconfig_ipv6_defined) + { + unlearn_ifconfig_ipv6(m, mi); + } + + /* DCO needs the new address while installing its on-link route. */ mi->context.c2.push_ifconfig_ipv6_defined = true; mi->context.c2.push_ifconfig_ipv6_local = new_addr6; - /* set our client's VPN endpoint for status reporting purposes */ - mi->reporting_addr_ipv6 = mi->context.c2.push_ifconfig_ipv6_local; + if (multi_learn_in6_addr(m, mi, new_addr6, -1, true) == mi) + { + /* set our client's VPN endpoint for status reporting purposes */ + mi->reporting_addr_ipv6 = mi->context.c2.push_ifconfig_ipv6_local; + } + else + { + mi->context.c2.push_ifconfig_ipv6_defined = false; + CLEAR(mi->context.c2.push_ifconfig_ipv6_local); + + msg(M_WARN, "MULTI: failed to learn PUSH_UPDATE IPv6 address %s, restarting client", + new_ipv6); + /* PUSH_UPDATE is already queued, so reconnect to restore consistent state. */ + send_restart(&mi->context, NULL); + multi_schedule_context_wakeup(m, mi); + return false; + } } } + return true; } bool diff --git a/src/openvpn/multi.h b/src/openvpn/multi.h index 3d2e932..f5a1a6f 100644 --- a/src/openvpn/multi.h +++ b/src/openvpn/multi.h @@ -661,9 +661,7 @@ lookup_by_cid(struct multi_context *m, const unsigned long cid); #endif -void -update_vhash(struct multi_context *m, struct multi_instance *mi, const char *new_ip, const char *new_ipv6); -void unlearn_ifconfig(struct multi_context *m, struct multi_instance *mi); -void unlearn_ifconfig_ipv6(struct multi_context *m, struct multi_instance *mi); +bool +update_vaddr(struct multi_context *m, struct multi_instance *mi, const char *new_ip, const char *new_ipv6); #endif /* MULTI_H */ diff --git a/src/openvpn/push_util.c b/src/openvpn/push_util.c index 34cb26c..cd940fb 100644 --- a/src/openvpn/push_util.c +++ b/src/openvpn/push_util.c @@ -149,14 +149,35 @@ struct options o; CLEAR(o); - /* Set canary values to detect ifconfig options in push-update messages. - * These placeholder strings will be overwritten to NULL by the option - * parser if -ifconfig or -ifconfig-ipv6 options are present in the - * push-update. + /* Set canary values to detect -ifconfig and -ifconfig-ipv6 options in + * push-update messages. These placeholder strings will be overwritten + * to NULL by the option parser if -ifconfig or -ifconfig-ipv6 options + * are present in the push-update, to the new value if present, or + * they will remain equal to the current values otherwise. + * If the current values are not defined they will remain NULL. */ - const char *canary = "canary"; - o.ifconfig_local = canary; - o.ifconfig_ipv6_local = canary; + + char ifconfig_local[INET_ADDRSTRLEN]; + if (mi->context.c2.push_ifconfig_defined) + { + struct in_addr addr4 = { .s_addr = htonl(mi->context.c2.push_ifconfig_local) }; + if (!inet_ntop(AF_INET, &addr4, ifconfig_local, sizeof(ifconfig_local))) + { + return false; + } + o.ifconfig_local = ifconfig_local; + } + + char ifconfig_ipv6_local[INET6_ADDRSTRLEN]; + if (mi->context.c2.push_ifconfig_ipv6_defined) + { + if (!inet_ntop(AF_INET6, &mi->context.c2.push_ifconfig_ipv6_local, + ifconfig_ipv6_local, sizeof(ifconfig_ipv6_local))) + { + return false; + } + o.ifconfig_ipv6_local = ifconfig_ipv6_local; + } struct buffer_entry *e = msgs->head; while (e) @@ -192,34 +213,15 @@ e = e->next; } + bool success = true; if (option_types_found & OPT_P_UP) { - /* -ifconfig */ - if (!o.ifconfig_local && mi->context.c2.push_ifconfig_defined) - { - unlearn_ifconfig(m, mi); - } - /* -ifconfig-ipv6 */ - if (!o.ifconfig_ipv6_local && mi->context.c2.push_ifconfig_ipv6_defined) - { - unlearn_ifconfig_ipv6(m, mi); - } - - if (o.ifconfig_local && !strcmp(o.ifconfig_local, canary)) - { - o.ifconfig_local = NULL; - } - if (o.ifconfig_ipv6_local && !strcmp(o.ifconfig_ipv6_local, canary)) - { - o.ifconfig_ipv6_local = NULL; - } - - /* new ifconfig or new ifconfig-ipv6 */ - update_vhash(m, mi, o.ifconfig_local, o.ifconfig_ipv6_local); + /* ifconfig or ifconfig-ipv6 */ + success = update_vaddr(m, mi, o.ifconfig_local, o.ifconfig_ipv6_local); } uninit_options(&o); - return true; + return success; } /* Return true if the client supports push-update */ diff --git a/tests/unit_tests/openvpn/mock_push_dependencies.c b/tests/unit_tests/openvpn/mock_push_dependencies.c index f083f89..041a5e6 100644 --- a/tests/unit_tests/openvpn/mock_push_dependencies.c +++ b/tests/unit_tests/openvpn/mock_push_dependencies.c @@ -48,19 +48,10 @@ return flags; } -void -unlearn_ifconfig(struct multi_context *m, struct multi_instance *mi) +bool +update_vaddr(struct multi_context *m, struct multi_instance *mi, const char *new_ip, const char *new_ipv6) { -} - -void -unlearn_ifconfig_ipv6(struct multi_context *m, struct multi_instance *mi) -{ -} - -void -update_vhash(struct multi_context *m, struct multi_instance *mi, const char *new_ip, const char *new_ipv6) -{ + return true; } void