From patchwork Thu Nov 27 08:50:42 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Moritz Fain X-Patchwork-Id: 4637 Return-Path: Delivered-To: patchwork@openvpn.net Received: by 2002:a05:7000:6c3:b0:7b1:439f:bdf with SMTP id j3csp3936500maw; Thu, 27 Nov 2025 00:51:27 -0800 (PST) X-Forwarded-Encrypted: i=2; AJvYcCXLRoHOhdj2e3agyRBOvOFb2HDKNAtFhrRTLNOzf7CeyuJeCCCtAwqpyvtub6Zb+iLxlB5YqggNTNg=@openvpn.net X-Google-Smtp-Source: AGHT+IHpDaDi2wfuW/bI42+3+y1z0N3QErp66cT0QJS6o/ve7SYozIngbSLbmh+7SxGy/woJ7Gv5 X-Received: by 2002:a05:6870:9488:b0:36c:abe0:83d3 with SMTP id 586e51a60fabf-3ed1fff5989mr4310734fac.39.1764233486969; Thu, 27 Nov 2025 00:51:26 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1764233486; cv=none; d=google.com; s=arc-20240605; b=NN8oFPwLoljGF/eI8m0kmPodQGt0C7Veb9iscQirFaWS8/L8ZxUL0BLMtKw2G2gNXE OLokj5TQ8YZhx6x79RG3drXvZxrYzm41DTwirp0ysziAgyGJNPRVaI+sh2RL6G00ErKn fJEzMIHhrOj90V9wOL0gJp35OUbV4IOBkZgchKikO67nzKYbUL2Ls7nmC+Cfr7MH4jZf lCyVnxhE4oOU5Ulvu2W1ZuqNFq2YmRSKAAVsUFxzKPyvI9pgAbkQQ+vzmvJByjZEhI7t QxQ9X0mNn3c5tVdiHs2BD/mMvHKAPs0lIw0WxqMJDNgvADV0WsMAULIFVsfA4tHcQfx4 xf6A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20240605; h=errors-to:content-transfer-encoding:list-subscribe:list-help :list-post:list-archive:list-unsubscribe:list-id:precedence:subject :to:message-id:date:from:mime-version:dkim-signature:dkim-signature :dkim-signature; bh=8ZgrcI/HlpKDojIJmEcQeZ78vMemQDLfeD3BsXbJDfE=; fh=4NbAC/LsuMLI0S0hprUlLSLCiHwg6SCAifhH718Jh0Q=; b=Tt6yutzCks73+00oWFjmTmpnT957wqJyOONjPKUu7GnXfYVc8mA9sMDxWEkmZHxA9p mBOcpQmJ63k4Nap9f+e55gD77X1m7TG2/eugbwjp/HE/FiNuv3yH91RqOwqThamEzS+7 wUvlChng0ndM71eQIe0/9U6DeVsgudAMKMxH1Jw5wLXhZmVHFkAMkvgzomSxLCOWJGOd OL4BBzaSLazpWuA4qPAQzpqZ2jaaQX2LTy7ZKMNToMHO3b+4NmGjCzxfEdxDTcoh0jQW zak7BdUqh5YM8tNDkmSSjht865B0+1pMtwP0v4YfRg1vHa54oSEcdsEr5JahuAX0xcTX PXNA==; dara=google.com ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@lists.sourceforge.net header.s=beta header.b=mOx0Jvzc; dkim=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b=G80GukAm; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=NJhvS7Xy; 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 Received: from lists.sourceforge.net (lists.sourceforge.net. [216.105.38.7]) by mx.google.com with ESMTPS id 586e51a60fabf-3f0dcff3ea4si106069fac.698.2025.11.27.00.51.26 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Thu, 27 Nov 2025 00:51:26 -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=mOx0Jvzc; dkim=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b=G80GukAm; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=NJhvS7Xy; 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 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:To:Message-ID:Date:From:MIME-Version:Sender:Reply-To:Cc:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:In-Reply-To:References:List-Owner; bh=8ZgrcI/HlpKDojIJmEcQeZ78vMemQDLfeD3BsXbJDfE=; b=mOx0JvzcxV/diyV05e4cp/E3yY z76SOJjmY0qTeWgbMbLIOfPelGF7CNt14bywAJwUZ+vTJz2ZNnsM5cnX4XcuCzfLEaVawo77EhXMt bGM+nyhplfSbKl1zEODkST1T66U3w1nDFHfkZdYBZMoFNZ5ILoiZtDXb7GFko9Et3BXo=; 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 1vOXih-0002To-GQ; Thu, 27 Nov 2025 08:51:19 +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 1vOXif-0002Tg-Q1 for openvpn-devel@lists.sourceforge.net; Thu, 27 Nov 2025 08:51:17 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=Content-Transfer-Encoding:Content-Type:To:Subject: Message-ID:Date:From:MIME-Version:Sender:Reply-To:Cc: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=b+2PCk+7znYbSpMnc68wWAudIGFDb2YOy97ni2zpj2U=; b=G80GukAmuq5nZhKBIJXoQkb7jU pKBqQHGX7FHNf4Z+1D51DztHHSklRlMibFTY+DkYyBDK+m8OvxNFPgD2o9jIY1QBBIDldooABe6/8 5foNJQgkLSnUH8fCgF0olrnuCZvfGVZXRxbzF9vN8pSQbE82HGqg8jy9DCXs6kV7q0rw=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=Content-Transfer-Encoding:Content-Type:To:Subject:Message-ID:Date:From: MIME-Version:Sender:Reply-To:Cc: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=b+2PCk+7znYbSpMnc68wWAudIGFDb2YOy97ni2zpj2U=; b=N JhvS7Xy0OH+RXoTQz8+a9ZsoNqEhNketw5KQO4Yl9s4aQJmfrPMcfwr6XsYnxXnl7nm9Lzo2P06Mz xjm2aXMiU3Pgj/e050gILfAHbW+9acFx8cQUSJ/t9HEyUvvqgxPUJN3CkMfoOdNdHRWq7ghfLM2eT bd5jBcbS0CmAJzd8=; Received: from mail-ua1-f50.google.com ([209.85.222.50]) by sfi-mx-2.v28.lw.sourceforge.com with esmtps (TLS1.2:ECDHE-RSA-AES128-GCM-SHA256:128) (Exim 4.95) id 1vOXid-0005Y1-N1 for openvpn-devel@lists.sourceforge.net; Thu, 27 Nov 2025 08:51:17 +0000 Received: by mail-ua1-f50.google.com with SMTP id a1e0cc1a2514c-935356590ddso174005241.1 for ; Thu, 27 Nov 2025 00:51:15 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1764233469; x=1764838269; h=content-transfer-encoding:to:subject:message-id:date:from :mime-version:x-gm-gg:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=b+2PCk+7znYbSpMnc68wWAudIGFDb2YOy97ni2zpj2U=; b=XN1dusWp2UxXTn0u0z5ShmznZfc5tu3Q3iTknA6e9/1kA7JpHaquWOeRtW6xGKcg1C Fx4yNXxDsB8MDDHWMdBVc6gMmMOFjgAO7S269Clj12TtgvPFpzU/Wp6LeRsHEah2DqXF SeWbGI2Yhr3UpaKGKU0kQdFXy9U1Gp+T/UMZUIcJ6/qXFD66Kpb06jt56JdOVKNy9dNw UrrGh6bDcCNNVTly7FJuLwW7c8tqPKMszphhD4a0inddGM6AXw+HPvQzM2llvVzzQ6J6 Io/DqKGqrj/oD+0qINtyp2e7d1Mcl03rBx1DCIN5QdDpCsoRi533VNMiOckwFVReJCYz LABg== X-Gm-Message-State: AOJu0Yxnwc72/fRdhYfWGi2jhyGGf7ccg+qSy1Nh/PCVxoVnZXTb3ZVR /dXpj9OlKLDZNFFo2yjTp/NsC8Nb1d6qEUu/pcmte9VDolPvvBQqN7bTpdMLCfB4OvL8Nl1y49+ CM7FqdS5py4tO6Y4nObVjyOKUL/0aI2dmQz+lx9EjR8jYNRGoEoVE0g== X-Gm-Gg: ASbGncvVuZcW525YUTR+mUwbP4Xk+L4CMlJxit/l522x8BooY7cxFVrRmHQ4yXrKfAH lEbsWQ70bKu4jQUawLYMe2JiowAW2jCoffPVD3GccHFV0yv2X1EO+W5j52ItYrvBaWDFY0vUOfX rveRuTGRIU1t8fC97fmwo5ojny+IeNncA/1CcHqaSrC3VCalEYs2S5MHi2HmRG+ZQg5TYf/20VH VHYZx4JZoMM15Z0HVRV1RhOnUhl62uZgOMF/bW9GfZsdfH+rLTTv00fb6aYSQ1vCHUkUA== X-Received: by 2002:a05:6102:54a8:b0:5d3:fed4:ac2b with SMTP id ada2fe7eead31-5e22422d27fmr3003750137.1.1764233468959; Thu, 27 Nov 2025 00:51:08 -0800 (PST) MIME-Version: 1.0 From: Moritz Fain Date: Thu, 27 Nov 2025 09:50:42 +0100 X-Gm-Features: AWmQ_bkHMCJYkR5spGsPuP6syO-SdK037z8qWqpiXDlybYImTPVUGnOKegmcir0 Message-ID: To: openvpn-devel@lists.sourceforge.net X-Spam-Score: 0.0 (/) X-Spam-Report: Spam detection software, running on the system "sfi-spamd-1.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: This adds a new management command 'reload-push-options' that allows reloading the push options from the configuration file without restarting the server. This is useful for updating routes or DNS set [...] Content analysis details: (0.0 points, 5.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- 0.0 HEADER_FROM_DIFFERENT_DOMAINS From and EnvelopeFrom 2nd level mail domains are different 0.0 RCVD_IN_MSPIKE_H2 RBL: Average reputation (+2) [209.85.222.50 listed in wl.mailspike.net] X-Headers-End: 1vOXid-0005Y1-N1 Subject: [Openvpn-devel] [PATCH 1/2] Management: add reload-push-options command 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: =?utf-8?q?1849932893009951477?= X-GMAIL-MSGID: =?utf-8?q?1849932893009951477?= This adds a new management command 'reload-push-options' that allows reloading the push options from the configuration file without restarting the server. This is useful for updating routes or DNS settings for new clients without dropping existing connections. The command supports an optional 'sync' argument. When provided, the server will also synchronize the new options to currently connected clients by: 1. Calculating the difference between old and new push options. 2. Sending '-instruction' (e.g. -route) to remove old options. 3. Sending new options via PUSH_UPDATE. This includes a comprehensive integration test suite in tests/reload_push_options. --- .gitignore | 1 + src/openvpn/manage.c | 29 ++ src/openvpn/manage.h | 1 + src/openvpn/multi.c | 280 ++++++++++++++++++ src/openvpn/options.c | 2 + src/openvpn/options.h | 1 + src/openvpn/options_util.c | 7 +- src/openvpn/options_util.h | 3 + tests/reload_push_options/.gitignore | 6 + tests/reload_push_options/Dockerfile | 46 +++ tests/reload_push_options/README.md | 84 ++++++ .../reload_push_options/client-entrypoint.sh | 35 +++ tests/reload_push_options/configs/client.conf | 22 ++ .../configs/server.conf.default | 16 + tests/reload_push_options/docker-compose.yml | 73 +++++ tests/reload_push_options/run.sh | 277 +++++++++++++++++ .../scripts/client-entrypoint.sh | 18 ++ tests/reload_push_options/scripts/gen-keys.sh | 48 +++ .../reload_push_options/scripts/log-routes.sh | 13 + .../scripts/server-entrypoint.sh | 20 ++ 20 files changed, 979 insertions(+), 3 deletions(-) create mode 100644 tests/reload_push_options/.gitignore create mode 100644 tests/reload_push_options/Dockerfile create mode 100644 tests/reload_push_options/README.md create mode 100755 tests/reload_push_options/client-entrypoint.sh create mode 100644 tests/reload_push_options/configs/client.conf create mode 100644 tests/reload_push_options/configs/server.conf.default create mode 100644 tests/reload_push_options/docker-compose.yml create mode 100755 tests/reload_push_options/run.sh create mode 100755 tests/reload_push_options/scripts/client-entrypoint.sh create mode 100755 tests/reload_push_options/scripts/gen-keys.sh create mode 100755 tests/reload_push_options/scripts/log-routes.sh create mode 100755 tests/reload_push_options/scripts/server-entrypoint.sh + diff --git a/.gitignore b/.gitignore index 04523af3..8a712be5 100644 --- a/.gitignore +++ b/.gitignore @@ -15,6 +15,7 @@ out .vs .deps .libs +.cache Makefile Makefile.in aclocal.m4 diff --git a/src/openvpn/manage.c b/src/openvpn/manage.c index 6efa1001..f5dd36a9 100644 --- a/src/openvpn/manage.c +++ b/src/openvpn/manage.c @@ -137,6 +137,8 @@ man_help(void) msg(M_CLIENT, "push-update-broad options : Broadcast a message to update the specified options."); msg(M_CLIENT, " Ex. push-update-broad \"route something, -dns\""); msg(M_CLIENT, "push-update-cid CID options : Send an update message to the client identified by CID."); + msg(M_CLIENT, "reload-push-options [sync] : Reload push options from config file for new clients."); + msg(M_CLIENT, " With 'sync': also sync options to connected clients (add new, remove old)."); msg(M_CLIENT, "END"); } @@ -1723,6 +1725,33 @@ man_dispatch_command(struct management *man, struct status_output *so, const cha man_push_update(man, p, UPT_BY_CID); } } + else if (streq(p[0], "reload-push-options")) + { + if (man->persist.callback.reload_push_options) + { + bool sync = (p[1] && streq(p[1], "sync")); + bool status = (*man->persist.callback.reload_push_options)(man->persist.callback.arg, sync); + if (status) + { + if (sync) + { + msg(M_CLIENT, "SUCCESS: push options reloaded and synced to all clients"); + } + else + { + msg(M_CLIENT, "SUCCESS: push options reloaded from config file"); + } + } + else + { + msg(M_CLIENT, "ERROR: failed to reload push options"); + } + } + else + { + man_command_unsupported("reload-push-options"); + } + } #if 1 else if (streq(p[0], "test")) { diff --git a/src/openvpn/manage.h b/src/openvpn/manage.h index dedcc155..2c4305e7 100644 --- a/src/openvpn/manage.h +++ b/src/openvpn/manage.h @@ -198,6 +198,7 @@ struct management_callback bool (*remote_entry_get)(void *arg, unsigned int index, char **remote); bool (*push_update_broadcast)(void *arg, const char *options); bool (*push_update_by_cid)(void *arg, unsigned long cid, const char *options); + bool (*reload_push_options)(void *arg, bool sync); }; /* diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c index 2b944667..1452bf3d 100644 --- a/src/openvpn/multi.c +++ b/src/openvpn/multi.c @@ -34,6 +34,7 @@ #include "forward.h" #include "multi.h" #include "push.h" +#include "options_util.h" #include "run_command.h" #include "otime.h" #include "gremlin.h" @@ -4100,6 +4101,284 @@ management_get_peer_info(void *arg, const unsigned long cid) return ret; } +/** + * Check if an option string exists in a push_list. + */ +static bool +push_option_exists(const struct push_list *list, const char *option) +{ + const struct push_entry *e = list->head; + while (e) + { + if (e->enable && e->option && strcmp(e->option, option) == 0) + { + return true; + } + e = e->next; + } + return false; +} + +/* + * Helper to append to push list using specific GC. + */ +static void +push_list_add(struct push_list *list, const char *opt, struct gc_arena *gc) +{ + struct push_entry *e; + ALLOC_OBJ_CLEAR_GC(e, struct push_entry, gc); + e->enable = true; + e->option = opt; + + if (list->tail) + { + list->tail->next = e; + list->tail = e; + } + else + { + list->head = e; + list->tail = e; + } +} + +/** + * Find the index of an updatable option type for a given option string. + * @param option The option string to check (e.g., "route 10.0.0.0 255.0.0.0") + * @return Index into updatable_options[] or -1 if not found + */ +static ssize_t +find_updatable_option_index(const char *option) +{ + size_t len = strlen(option); + for (size_t i = 0; i < updatable_options_count; ++i) + { + size_t opt_len = strlen(updatable_options[i]); + if (len >= opt_len + && strncmp(option, updatable_options[i], opt_len) == 0 + && (option[opt_len] == '\0' || option[opt_len] == ' ')) + { + return (ssize_t)i; + } + } + return -1; +} + +/** + * Reload push options from the configuration file. + * This function re-reads the config file and updates the push_list + * that will be sent to new connecting clients. + * + * Thread safety: OpenVPN uses a single-threaded event loop, so this + * function runs sequentially with all other operations. + * + * @param arg Pointer to multi_context + * @param sync If true, sync options to connected clients (add new, remove old) + * @return true on success, false on failure + */ +static bool +management_callback_reload_push_options(void *arg, bool sync) +{ + struct multi_context *m = (struct multi_context *)arg; + struct gc_arena gc = gc_new(); + bool ret = false; + + msg(M_INFO, "MANAGEMENT: Reloading push options from config file"); + + /* Check if we have a config file to reload from */ + if (!m->top.options.config) + { + msg(M_WARN, "MANAGEMENT: Cannot reload push options - no config file specified"); + goto cleanup; + } + + /* Save reference to old push_list for sync comparison */ + struct push_list old_push_list = m->top.options.push_list; + + /* Create a temporary options structure to parse the config */ + struct options new_options; + CLEAR(new_options); + + /* Initialize the gc_arena for the new options */ + new_options.gc = gc_new(); + + /* Set up environment for config parsing */ + struct env_set *es = env_set_create(&gc); + unsigned int option_types_found = 0; + + /* Re-read the configuration file */ + read_config_file(&new_options, m->top.options.config, 0, + m->top.options.config, 0, M_WARN, + OPT_P_DEFAULT, &option_types_found, es); + + /* Validate we got a sensible result - if old list had entries but new is empty, + * this likely indicates a parsing error */ + if (old_push_list.head && !new_options.push_list.head) + { + msg(M_WARN, "MANAGEMENT: Config reload returned empty push list - aborting"); + gc_free(&new_options.gc); + goto cleanup; + } + + /* Create a new GC arena for the new push list */ + struct gc_arena new_push_list_gc = gc_new(); + struct push_list new_push_list = { NULL, NULL }; + + /* Copy each push entry from the parsed config to the new push_list + * using the new dedicated push_list_gc */ + const struct push_entry *e = new_options.push_list.head; + while (e) + { + if (e->enable && e->option) + { + /* Copy the option string to the new dedicated gc_arena */ + const char *opt = string_alloc(e->option, &new_push_list_gc); + push_list_add(&new_push_list, opt, &new_push_list_gc); + } + e = e->next; + } + + /* Free the temporary options gc_arena (parsed config) */ + gc_free(&new_options.gc); + + /* Sync options to connected clients if requested */ + /* We do this BEFORE swapping the lists so we can compare old vs new */ + if (sync) + { + /* Calculate required buffer size: sum of all option lengths + separators */ + size_t opts_size = 0; + const struct push_entry *size_e = new_push_list.head; + while (size_e) + { + if (size_e->enable && size_e->option) + { + opts_size += strlen(size_e->option) + 2; /* option + ", " */ + } + size_e = size_e->next; + } + /* Add space for removal commands: "-type, " for each updatable option type */ + opts_size += updatable_options_count * 32; + /* Minimum size to avoid edge cases */ + if (opts_size < PUSH_BUNDLE_SIZE) + { + opts_size = PUSH_BUNDLE_SIZE; + } + + struct buffer opts = alloc_buf_gc(opts_size, &gc); + bool first = true; + int added = 0, removed = 0; + + /* Set of option types that have been removed/modified */ + bool *type_removed = gc_malloc(updatable_options_count * sizeof(bool), true, &gc); + + /* 1. Detect removed options and mark their types */ + const struct push_entry *old_e = old_push_list.head; + while (old_e) + { + if (old_e->enable && old_e->option) + { + if (!push_option_exists(&new_push_list, old_e->option)) + { + ssize_t type_idx = find_updatable_option_index(old_e->option); + if (type_idx >= 0) + { + type_removed[type_idx] = true; + removed++; + msg(D_PUSH, "MANAGEMENT: Sync removing: %s", old_e->option); + } + else + { + msg(M_WARN, "MANAGEMENT: Cannot sync removal of option '%s' (not updatable)", old_e->option); + } + } + } + old_e = old_e->next; + } + + /* 2. Add removal commands for all marked types */ + for (size_t i = 0; i < updatable_options_count; ++i) + { + if (type_removed[i]) + { + if (!first) + { + buf_printf(&opts, ", "); + } + /* Send -type to remove all options of that type */ + buf_printf(&opts, "-%s", updatable_options[i]); + first = false; + } + } + + /* 3. Add new options AND re-add options belonging to removed types */ + const struct push_entry *new_e = new_push_list.head; + while (new_e) + { + if (new_e->enable && new_e->option) + { + bool should_send = false; + bool is_existing = push_option_exists(&old_push_list, new_e->option); + + /* Check if this option belongs to a type that was reset */ + bool type_was_reset = false; + ssize_t type_idx = find_updatable_option_index(new_e->option); + if (type_idx >= 0 && type_removed[type_idx]) + { + type_was_reset = true; + } + + /* Always send new options */ + if (!is_existing) + { + should_send = true; + added++; + msg(D_PUSH, "MANAGEMENT: Sync adding: %s", new_e->option); + } + /* Also resend options if their type was reset (because we sent -type) */ + else if (type_was_reset) + { + should_send = true; + msg(D_PUSH, "MANAGEMENT: Sync re-adding (type reset): %s", new_e->option); + } + + if (should_send) + { + if (!first) + { + buf_printf(&opts, ", "); + } + buf_printf(&opts, "%s", new_e->option); + first = false; + } + } + new_e = new_e->next; + } + + if (BLEN(&opts) > 0) + { + msg(M_INFO, "MANAGEMENT: Syncing push options to clients (added=%d, removed=%d)", + added, removed); + management_callback_send_push_update_broadcast(m, BSTR(&opts)); + } + else + { + msg(M_INFO, "MANAGEMENT: No changes to sync to clients"); + } + } + + /* Now replace the old push_list with the new one and free old memory */ + gc_free(&m->top.options.push_list_gc); + m->top.options.push_list_gc = new_push_list_gc; + m->top.options.push_list = new_push_list; + + msg(M_INFO, "MANAGEMENT: Push options reloaded successfully"); + ret = true; + +cleanup: + gc_free(&gc); + return ret; +} + #endif /* ifdef ENABLE_MANAGEMENT */ @@ -4125,6 +4404,7 @@ init_management_callback_multi(struct multi_context *m) cb.get_peer_info = management_get_peer_info; cb.push_update_broadcast = management_callback_send_push_update_broadcast; cb.push_update_by_cid = management_callback_send_push_update_by_cid; + cb.reload_push_options = management_callback_reload_push_options; management_set_callback(management, &cb); } #endif /* ifdef ENABLE_MANAGEMENT */ diff --git a/src/openvpn/options.c b/src/openvpn/options.c index 4794315c..8a97374e 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -807,6 +807,7 @@ init_options(struct options *o, const bool init_gc) if (init_gc) { gc_init(&o->gc); + gc_init(&o->push_list_gc); gc_init(&o->dns_options.gc); o->gc_owned = true; } @@ -942,6 +943,7 @@ uninit_options(struct options *o) if (o->gc_owned) { gc_free(&o->gc); + gc_free(&o->push_list_gc); gc_free(&o->dns_options.gc); } } diff --git a/src/openvpn/options.h b/src/openvpn/options.h index 42db9cae..bd013dba 100644 --- a/src/openvpn/options.h +++ b/src/openvpn/options.h @@ -487,6 +487,7 @@ struct options in_addr_t server_bridge_pool_end; struct push_list push_list; + struct gc_arena push_list_gc; bool ifconfig_pool_defined; in_addr_t ifconfig_pool_start; in_addr_t ifconfig_pool_end; diff --git a/src/openvpn/options_util.c b/src/openvpn/options_util.c index eba7d396..5fd35eda 100644 --- a/src/openvpn/options_util.c +++ b/src/openvpn/options_util.c @@ -193,7 +193,7 @@ atoi_constrained(const char *str, int *value, const char *name, int min, int max return true; } -static const char *updatable_options[] = { "block-ipv6", "block-outside-dns", +const char *updatable_options[] = { "block-ipv6", "block-outside-dns", "dhcp-option", "dns", "ifconfig", "ifconfig-ipv6", "push-continuation", "redirect-gateway", @@ -202,6 +202,8 @@ static const char *updatable_options[] = { "block-ipv6", "block-outside-dns", "route-metric", "topology", "tun-mtu", "keepalive" }; +const size_t updatable_options_count = sizeof(updatable_options) / sizeof(char *); + bool check_push_update_option_flags(char *line, int *i, unsigned int *flags) { @@ -232,8 +234,7 @@ check_push_update_option_flags(char *line, int *i, unsigned int *flags) } size_t len = strlen(&line[*i]); - int count = sizeof(updatable_options) / sizeof(char *); - for (int j = 0; j < count; ++j) + for (size_t j = 0; j < updatable_options_count; ++j) { size_t opt_len = strlen(updatable_options[j]); if (len < opt_len) diff --git a/src/openvpn/options_util.h b/src/openvpn/options_util.h index 5bb710a6..50988b1b 100644 --- a/src/openvpn/options_util.h +++ b/src/openvpn/options_util.h @@ -108,4 +108,7 @@ bool apply_pull_filter(const struct options *o, char *line); */ bool check_push_update_option_flags(char *line, int *i, unsigned int *flags); +extern const char *updatable_options[]; +extern const size_t updatable_options_count; + #endif /* ifndef OPTIONS_UTIL_H_ */ diff --git a/tests/reload_push_options/.gitignore b/tests/reload_push_options/.gitignore new file mode 100644 index 00000000..d174b9ac --- /dev/null +++ b/tests/reload_push_options/.gitignore @@ -0,0 +1,6 @@ +# Generated during tests +keys/ +results/ + + + diff --git a/tests/reload_push_options/Dockerfile b/tests/reload_push_options/Dockerfile new file mode 100644 index 00000000..4677aa73 --- /dev/null +++ b/tests/reload_push_options/Dockerfile @@ -0,0 +1,46 @@ +# Build OpenVPN from source +FROM debian:bookworm-slim AS builder + +RUN apt-get update && apt-get install -y \ + build-essential \ + autoconf \ + automake \ + libtool \ + pkg-config \ + libssl-dev \ + liblz4-dev \ + liblzo2-dev \ + libpam0g-dev \ + libcap-ng-dev \ + libnl-genl-3-dev \ + && rm -rf /var/lib/apt/lists/* + +WORKDIR /src +COPY . . + +RUN autoreconf -fvi && \ + ./configure --disable-systemd --disable-plugins && \ + make -j$(nproc) + +# Runtime image +FROM debian:bookworm-slim + +RUN apt-get update && apt-get install -y \ + libssl3 \ + liblz4-1 \ + liblzo2-2 \ + libcap-ng0 \ + libnl-genl-3-200 \ + iproute2 \ + iptables \ + netcat-openbsd \ + gettext-base \ + && rm -rf /var/lib/apt/lists/* + +COPY --from=builder /src/src/openvpn/openvpn /usr/local/sbin/openvpn + +# Copy default server config into the image +COPY tests/reload_push_options/configs/server.conf.default /etc/openvpn/server.conf.default + +RUN mkdir -p /dev/net && \ + mknod /dev/net/tun c 10 200 || true diff --git a/tests/reload_push_options/README.md b/tests/reload_push_options/README.md new file mode 100644 index 00000000..0b35e125 --- /dev/null +++ b/tests/reload_push_options/README.md @@ -0,0 +1,84 @@ +# reload-push-options Test Suite + +Integration tests for the `reload-push-options` management command. + +## Prerequisites + +- Docker & Docker Compose +- OpenSSL (for key generation) + +## Quick Start + +```bash +cd tests/reload_push_options +./run.sh +``` + +## What it tests + +| Test | Description | +|------|-------------| +| 1 | Basic reload without sync - existing clients unchanged | +| 2 | Sync with new route added | +| 3 | Sync with route removed | +| 4 | Sync with all routes removed | +| 5 | Sync with only new routes (complete replacement) | +| 6 | Sync with mixed changes (add + remove) | +| 7 | New client receives updated config | +| 8 | Stress test with 500 routes | + +## Architecture + +``` +┌─────────────────────────────────────────────────┐ +│ Docker Network │ +│ 10.100.0.0/24 │ +│ │ +│ ┌──────────┐ ┌──────────┐ ┌──────────┐ │ +│ │ Server │ │ Client1 │ │ Client2 │ │ +│ │ .0.2 │◄───│ .0.10 │ │ .0.11 │ │ +│ │ │◄───│ │ │ │ │ +│ │ :7505 │ │ │ │ │ │ +│ │ (mgmt) │ │ │ │ │ │ +│ └──────────┘ └──────────┘ └──────────┘ │ +│ │ │ +└───────┼─────────────────────────────────────────┘ + │ + localhost:7505 (management interface) +``` + +## Manual Testing + +```bash +# Start the environment +docker compose up -d + +# Connect to management interface +nc localhost 7505 + +# Commands to try: +help +status +reload-push-options +reload-push-options sync +``` + +## Files + +- `docker-compose.yml` - Container orchestration +- `Dockerfile` - Builds OpenVPN from source +- `configs/server.conf.default` - Default server config (baked into image) +- `configs/client.conf` - Client config +- `keys/` - PKI (auto-generated) +- `scripts/` - Entrypoints and helpers +- `results/` - Test output and logs + +## How Config Updates Work + +1. Default server config (`server.conf.default`) is copied into the Docker image +2. On container start, `server-entrypoint.sh` restores it to `/etc/openvpn/server.conf` +3. During tests, config is updated inside the container via `docker compose exec` +4. This ensures each test run starts from a clean, known state + + + diff --git a/tests/reload_push_options/client-entrypoint.sh b/tests/reload_push_options/client-entrypoint.sh new file mode 100755 index 00000000..638d76d4 --- /dev/null +++ b/tests/reload_push_options/client-entrypoint.sh @@ -0,0 +1,35 @@ +#!/bin/bash +set -e + +CLIENT_NAME=${CLIENT_NAME:-client} +LOG_FILE="/var/log/openvpn/${CLIENT_NAME}.log" + +# Function to log routes +log_routes() { + echo "=== Routes at $(date -Iseconds) ===" >> "$LOG_FILE" + ip route show | grep -E "^(10\.|172\.|192\.168\.)" >> "$LOG_FILE" 2>/dev/null || echo "(no VPN routes)" >> "$LOG_FILE" + echo "" >> "$LOG_FILE" +} + +# Monitor route changes in background +monitor_routes() { + while true; do + ip monitor route 2>/dev/null | while read -r line; do + echo "[$(date -Iseconds)] ROUTE CHANGE: $line" >> "$LOG_FILE" + done + sleep 1 + done +} + +# Start route monitor +monitor_routes & + +# Log initial routes +echo "=== Client $CLIENT_NAME starting ===" > "$LOG_FILE" +log_routes + +# Start OpenVPN +exec /usr/local/sbin/openvpn --config /etc/openvpn/client.conf \ + --log-append "$LOG_FILE" \ + --verb 4 + diff --git a/tests/reload_push_options/configs/client.conf b/tests/reload_push_options/configs/client.conf new file mode 100644 index 00000000..1ce2ea84 --- /dev/null +++ b/tests/reload_push_options/configs/client.conf @@ -0,0 +1,22 @@ +# OpenVPN Client Config for reload-push-options testing +client +dev tun +proto udp +remote 10.100.0.2 1194 + +ca /etc/openvpn/keys/ca.crt +# cert and key are set via command line based on client name + +nobind +persist-key +persist-tun + +# Verbose logging +verb 4 + +# Allow scripts to run +script-security 2 + +# Script to log route changes +route-up /scripts/log-routes.sh +route-pre-down /scripts/log-routes.sh diff --git a/tests/reload_push_options/configs/server.conf.default b/tests/reload_push_options/configs/server.conf.default new file mode 100644 index 00000000..2445b718 --- /dev/null +++ b/tests/reload_push_options/configs/server.conf.default @@ -0,0 +1,16 @@ +dev tun +proto udp +port 1194 +server 10.8.0.0 255.255.255.0 +ca /etc/openvpn/keys/ca.crt +cert /etc/openvpn/keys/server.crt +key /etc/openvpn/keys/server.key +dh /etc/openvpn/keys/dh.pem +keepalive 10 120 +persist-key +persist-tun +management 0.0.0.0 7505 +verb 4 +log /var/log/openvpn.log +${PUSH_OPTIONS} + diff --git a/tests/reload_push_options/docker-compose.yml b/tests/reload_push_options/docker-compose.yml new file mode 100644 index 00000000..69668c88 --- /dev/null +++ b/tests/reload_push_options/docker-compose.yml @@ -0,0 +1,73 @@ +services: + server: + build: + context: ../.. + dockerfile: tests/reload_push_options/Dockerfile + cap_add: + - NET_ADMIN + devices: + - /dev/net/tun:/dev/net/tun + networks: + vpn_net: + ipv4_address: 10.100.0.2 + volumes: + - ./keys:/etc/openvpn/keys:ro + - ./scripts:/scripts:ro + ports: + - "7505:7505" # Management interface + command: ["/scripts/server-entrypoint.sh"] + healthcheck: + test: ["CMD", "sh", "-c", "ip link show tun0 >/dev/null 2>&1"] + interval: 2s + timeout: 2s + retries: 15 + start_period: 5s + + client1: + build: + context: ../.. + dockerfile: tests/reload_push_options/Dockerfile + cap_add: + - NET_ADMIN + devices: + - /dev/net/tun:/dev/net/tun + networks: + vpn_net: + ipv4_address: 10.100.0.10 + volumes: + - ./configs:/etc/openvpn:ro + - ./keys:/etc/openvpn/keys:ro + - ./scripts:/scripts:ro + - ./results:/results + depends_on: + server: + condition: service_healthy + command: ["/scripts/client-entrypoint.sh", "client1"] + + client2: + build: + context: ../.. + dockerfile: tests/reload_push_options/Dockerfile + cap_add: + - NET_ADMIN + devices: + - /dev/net/tun:/dev/net/tun + networks: + vpn_net: + ipv4_address: 10.100.0.11 + volumes: + - ./configs:/etc/openvpn:ro + - ./keys:/etc/openvpn/keys:ro + - ./scripts:/scripts:ro + - ./results:/results + depends_on: + server: + condition: service_healthy + command: ["/scripts/client-entrypoint.sh", "client2"] + +networks: + vpn_net: + driver: bridge + ipam: + config: + - subnet: 10.100.0.0/24 diff --git a/tests/reload_push_options/run.sh b/tests/reload_push_options/run.sh new file mode 100755 index 00000000..21c93b84 --- /dev/null +++ b/tests/reload_push_options/run.sh @@ -0,0 +1,277 @@ +#!/bin/bash +# Test suite for reload-push-options management command +set -e + +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" +cd "$SCRIPT_DIR" + +# Colors +RED='\033[0;31m' +GREEN='\033[0;32m' +YELLOW='\033[1;33m' +NC='\033[0m' + +pass() { echo -e "${GREEN}✓ PASS${NC}: $1"; } +fail() { + echo -e "${RED}✗ FAIL${NC}: $1" + info "Debug: Server logs (last 30 lines):" + docker compose exec -T server tail -30 /var/log/openvpn.log 2>/dev/null || true + info "Debug: Client1 logs (last 30 lines):" + docker compose exec -T client1 tail -30 /results/client1.log 2>/dev/null || true + docker compose down -v 2>/dev/null || true + exit 1 +} +info() { echo -e "${YELLOW}→${NC} $1"; } + +mgmt() { + echo "$1" | nc -q1 localhost 7505 2>/dev/null || echo "$1" | nc -w1 localhost 7505 2>/dev/null +} + +get_client_log_lines() { + local client="$1" + docker compose exec -T "$client" wc -l /results/${client}.log 2>/dev/null | awk '{print $1}' || echo "0" +} + +wait_for_client_ready() { + local client="$1" + local lines_before="$2" + local max_wait=20 + + for i in $(seq 1 $max_wait); do + sleep 1 + if docker compose exec -T "$client" tail -n +$((lines_before + 1)) /results/${client}.log 2>/dev/null | grep -q "Initialization Sequence Completed"; then + return 0 + fi + done + return 1 +} + +get_client_routes() { + local client="$1" + if ! docker compose exec -T "$client" ip link show tun0 &>/dev/null; then + echo "(tun0 not found)" + return + fi + docker compose exec -T "$client" ip route show 2>/dev/null | grep -E "^(192\.168\.|10\.|172\.)" || echo "(no matching routes)" +} + +update_server_config() { + local config_content="$1" + # Generate config from template inside the container using envsubst + docker compose exec -T -e PUSH_OPTIONS="$config_content" server \ + sh -c 'envsubst '"'"'${PUSH_OPTIONS}'"'"' < /etc/openvpn/server.conf.default > /etc/openvpn/server.conf' +} + +wait_for_clients() { + info "Waiting for clients to connect..." + for i in {1..30}; do + local count=$(mgmt "status" | grep -c "^client" || true) + if [ "$count" -ge 2 ]; then + return 0 + fi + sleep 1 + done + fail "Clients did not connect in time" +} + +cleanup() { + info "Cleaning up..." + docker compose down -v 2>/dev/null || true + rm -rf results/* +} + +# Run test: update config, reload (with or without sync), verify routes +# Args: test_name, client, sync (0|1|skip), config_content, must_have_pattern, must_not_have_pattern +# sync=skip: don't send mgmt command, just verify routes (for reconnect tests) +run_test() { + local test_name="$1" + local client="$2" + local sync="$3" + local config_content="$4" + local must_have="$5" + local must_not_have="$6" + + echo "" + echo "--- $test_name ---" + + local routes_before=$(get_client_routes "$client") + local log_lines=$(get_client_log_lines "$client") + + update_server_config "$config_content" + + if [ "$sync" != "skip" ]; then + local cmd="reload-push-options" + [ "$sync" = "1" ] && cmd="reload-push-options sync" + + local result=$(mgmt "$cmd") + echo "Management response: $result" + + if ! echo "$result" | grep -q "SUCCESS"; then + fail "$test_name: command failed" + fi + pass "$test_name: command succeeded" + + if [ "$sync" = "1" ]; then + wait_for_client_ready "$client" "$log_lines" + else + sleep 2 + fi + fi + + local routes=$(get_client_routes "$client") + info "Routes: $routes" + + if [ -n "$must_have" ]; then + if echo "$routes" | grep -qE "$must_have"; then + pass "$test_name: expected routes present" + else + fail "$test_name: expected routes ($must_have) not found" + fi + fi + + if [ -n "$must_not_have" ]; then + if echo "$routes" | grep -qE "$must_not_have"; then + fail "$test_name: removed routes ($must_not_have) still present" + else + pass "$test_name: routes correctly removed" + fi + fi +} + +trap cleanup EXIT + +# Generate keys if needed +if [ ! -f keys/ca.crt ]; then + info "Generating test PKI..." + chmod +x scripts/gen-keys.sh + ./scripts/gen-keys.sh +fi + +chmod +x scripts/*.sh +rm -rf results/* +mkdir -p results + +echo "" +echo "==========================================" +echo " reload-push-options Test Suite" +echo "==========================================" +echo "" + +docker compose down -v 2>/dev/null || true + +# Initial config is now baked into the image (server.conf.default) +# and restored on container start by server-entrypoint.sh + +info "Building and starting containers..." +docker compose build +docker compose up -d --wait +wait_for_clients + +# Test 1: No sync - routes must NOT change (still have initial routes, not the new 192.168.30.0) +run_test "Test 1: No sync" client1 0 \ + 'push "route 192.168.10.0 255.255.255.0" +push "route 192.168.20.0 255.255.255.0" +push "route 192.168.30.0 255.255.255.0" +push "dhcp-option DNS 8.8.8.8"' \ + "192\.168\.10\.0|192\.168\.20\.0" "192\.168\.30\.0" + +# Test 2: Add route +run_test "Test 2: Add route" client1 1 \ + 'push "route 192.168.10.0 255.255.255.0" +push "route 192.168.20.0 255.255.255.0" +push "route 192.168.30.0 255.255.255.0" +push "route 192.168.40.0 255.255.255.0" +push "dhcp-option DNS 8.8.8.8"' \ + "192\.168\.40\.0" "" + +# Test 3: Remove route +run_test "Test 3: Remove route" client1 1 \ + 'push "route 192.168.10.0 255.255.255.0" +push "route 192.168.30.0 255.255.255.0" +push "route 192.168.40.0 255.255.255.0" +push "dhcp-option DNS 8.8.8.8"' \ + "" "192\.168\.20\.0" + +# Test 4: Remove all routes +run_test "Test 4: Remove all routes" client1 1 \ + 'push "dhcp-option DNS 8.8.8.8"' \ + "" "192\.168\." + +# Test 5: Add new routes +run_test "Test 5: New routes" client1 1 \ + 'push "route 172.16.0.0 255.255.0.0" +push "route 172.17.0.0 255.255.0.0" +push "dhcp-option DNS 1.1.1.1"' \ + "172\.(16|17)\.0\.0" "" + +# Test 6: Mixed - remove 172.16, keep 172.17, add 10.10 +run_test "Test 6: Mixed changes" client1 1 \ + 'push "route 172.17.0.0 255.255.0.0" +push "route 10.10.0.0 255.255.0.0" +push "dhcp-option DNS 1.1.1.1"' \ + "172\.17\.0\.0|10\.10\.0\.0" "172\.16\.0\.0" + +# Test 7: Reconnected client gets current config +echo "" +echo "--- Test 7: Reconnect ---" +info "Updating config and restarting client2" + +update_server_config 'push "route 172.17.0.0 255.255.0.0" +push "route 10.10.0.0 255.255.0.0" +push "route 192.168.100.0 255.255.255.0" +push "route 192.168.200.0 255.255.255.0" +push "dhcp-option DNS 1.1.1.1"' + +docker compose restart client2 +sleep 5 + +run_test "Test 7: Reconnect" client2 skip \ + 'push "route 172.17.0.0 255.255.0.0" +push "route 10.10.0.0 255.255.0.0" +push "route 192.168.100.0 255.255.255.0" +push "route 192.168.200.0 255.255.255.0" +push "dhcp-option DNS 1.1.1.1"' \ + "172\.17\.0\.0|10\.10\.0\.0|192\.168\.100\.0|192\.168\.200\.0" "" + +# Test 8: Stress test with 500 routes +echo "" +echo "--- Test 8: 500 routes stress test ---" +info "Generating config with 500 routes..." + +# Generate 500 routes: 10.{1-250}.{0,128}.0/25 +routes_config="" +for i in $(seq 1 250); do + routes_config+="push \"route 10.$i.0.0 255.255.128.0\" +" + routes_config+="push \"route 10.$i.128.0 255.255.128.0\" +" +done +routes_config+='push "dhcp-option DNS 8.8.8.8"' + +update_server_config "$routes_config" + +log_lines=$(get_client_log_lines client1) +result=$(mgmt "reload-push-options sync") +echo "Management response: $result" + +if ! echo "$result" | grep -q "SUCCESS"; then + fail "Test 8: 500 routes - command failed" +fi +pass "Test 8: 500 routes - command succeeded" + +wait_for_client_ready client1 "$log_lines" + +routes=$(get_client_routes client1) +route_count=$(echo "$routes" | grep -c "^10\." || true) +info "Route count: $route_count" + +if [ "$route_count" -ge 450 ]; then + pass "Test 8: 500 routes - received $route_count routes" +else + fail "Test 8: 500 routes - expected ~500 routes, got $route_count" +fi + +echo "" +echo "==========================================" +echo -e "${GREEN}All tests completed!${NC}" +echo "==========================================" diff --git a/tests/reload_push_options/scripts/client-entrypoint.sh b/tests/reload_push_options/scripts/client-entrypoint.sh new file mode 100755 index 00000000..9b65aeaa --- /dev/null +++ b/tests/reload_push_options/scripts/client-entrypoint.sh @@ -0,0 +1,18 @@ +#!/bin/bash +set -e + +CLIENT_NAME="${1:-client1}" +echo "Starting OpenVPN client: $CLIENT_NAME" + +# Wait for server to be ready +sleep 3 + +# Start OpenVPN with client-specific cert/key +exec /usr/local/sbin/openvpn \ + --config /etc/openvpn/client.conf \ + --cert /etc/openvpn/keys/${CLIENT_NAME}.crt \ + --key /etc/openvpn/keys/${CLIENT_NAME}.key \ + --log /results/${CLIENT_NAME}.log + + + diff --git a/tests/reload_push_options/scripts/gen-keys.sh b/tests/reload_push_options/scripts/gen-keys.sh new file mode 100755 index 00000000..a0b8dee4 --- /dev/null +++ b/tests/reload_push_options/scripts/gen-keys.sh @@ -0,0 +1,48 @@ +#!/bin/bash +# Generate test PKI for OpenVPN testing +set -e + +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" +KEYS_DIR="$SCRIPT_DIR/../keys" +mkdir -p "$KEYS_DIR" +cd "$KEYS_DIR" + +# Generate CA +openssl genrsa -out ca.key 2048 +openssl req -new -x509 -days 365 -key ca.key -out ca.crt \ + -subj "/CN=Test CA" + +# Generate server cert +openssl genrsa -out server.key 2048 +openssl req -new -key server.key -out server.csr \ + -subj "/CN=server" +openssl x509 -req -days 365 -in server.csr -CA ca.crt -CAkey ca.key \ + -CAcreateserial -out server.crt + +# Generate client1 cert +openssl genrsa -out client1.key 2048 +openssl req -new -key client1.key -out client1.csr \ + -subj "/CN=client1" +openssl x509 -req -days 365 -in client1.csr -CA ca.crt -CAkey ca.key \ + -CAcreateserial -out client1.crt + +# Generate client2 cert +openssl genrsa -out client2.key 2048 +openssl req -new -key client2.key -out client2.csr \ + -subj "/CN=client2" +openssl x509 -req -days 365 -in client2.csr -CA ca.crt -CAkey ca.key \ + -CAcreateserial -out client2.crt + +# Generate DH params (2048 required by modern OpenSSL) +openssl dhparam -out dh.pem 2048 + +# Generate TLS auth key +openvpn --genkey secret ta.key 2>/dev/null || \ + dd if=/dev/urandom of=ta.key bs=256 count=1 2>/dev/null + +# Cleanup CSRs +rm -f *.csr + +echo "Keys generated in $KEYS_DIR" +ls -la "$KEYS_DIR" + diff --git a/tests/reload_push_options/scripts/log-routes.sh b/tests/reload_push_options/scripts/log-routes.sh new file mode 100755 index 00000000..88d9a229 --- /dev/null +++ b/tests/reload_push_options/scripts/log-routes.sh @@ -0,0 +1,13 @@ +#!/bin/bash +# Log current routes to results file +TIMESTAMP=$(date +%Y%m%d_%H%M%S) +ROUTES_FILE="/results/routes_${common_name:-unknown}_${TIMESTAMP}.txt" + +echo "=== Route event at $TIMESTAMP ===" >> "$ROUTES_FILE" +echo "Script: $script_type" >> "$ROUTES_FILE" +echo "Routes:" >> "$ROUTES_FILE" +ip route show >> "$ROUTES_FILE" +echo "" >> "$ROUTES_FILE" + + + diff --git a/tests/reload_push_options/scripts/server-entrypoint.sh b/tests/reload_push_options/scripts/server-entrypoint.sh new file mode 100755 index 00000000..9f476f99 --- /dev/null +++ b/tests/reload_push_options/scripts/server-entrypoint.sh @@ -0,0 +1,20 @@ +#!/bin/bash +set -e + +echo "Starting OpenVPN server..." + +# Default push options (used on initial start) +export PUSH_OPTIONS='push "route 192.168.10.0 255.255.255.0" +push "route 192.168.20.0 255.255.255.0" +push "dhcp-option DNS 8.8.8.8"' + +# Generate config from template +envsubst '${PUSH_OPTIONS}' < /etc/openvpn/server.conf.default > /etc/openvpn/server.conf +echo "Generated server config with default push options" + +# Enable IP forwarding (ignore error in container) +echo 1 > /proc/sys/net/ipv4/ip_forward 2>/dev/null || true + +# Start OpenVPN +exec /usr/local/sbin/openvpn --config /etc/openvpn/server.conf From patchwork Thu Nov 27 08:51:34 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Moritz Fain X-Patchwork-Id: 4638 Return-Path: Delivered-To: patchwork@openvpn.net Received: by 2002:a05:7000:6c3:b0:7b1:439f:bdf with SMTP id j3csp3936733maw; Thu, 27 Nov 2025 00:52:11 -0800 (PST) X-Forwarded-Encrypted: i=2; AJvYcCVmyzhhoj+UWTJ5a8BN6umkb2rXJPNypRXyK1zlcmh+lyMbgLMaJwLElajj69R5DoNhqlEj8HbLbjg=@openvpn.net X-Google-Smtp-Source: AGHT+IGGI7H6wKvqf/cjQPFkyRntSwFXH2vEm9/KjEPpMtUumCF6YS4VHRZDRl22/LnPLVIdGMxR X-Received: by 2002:a05:6830:679f:b0:7c6:c891:5112 with SMTP id 46e09a7af769-7c798fcd59amr9972058a34.16.1764233531150; Thu, 27 Nov 2025 00:52:11 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1764233531; cv=none; d=google.com; s=arc-20240605; b=IYRqBfXhrhbpLoJ19Mj8MS7/muLMoaFD907URv44tFbuJfDUVfCMCLnWQsHAnQAa/e 4dp3zjJ/DHMGP4N9TjyBrHntUyWlpskm4f3ynMF6J2lk+5AmOoMNH/07WDcig05B6WTx 4XYXmCTE8EpqN+YF7dmERuxSMTLv1XhpFZn48y+2C7YDV5sBRJR5qNIAY8bA6ST/QLRX VQE/JUim5PzKU4fO82TSsVn4j3N0iCF69ixC4S/nHzIUgWocoVAr84drOBsMQdzSVBrP n0K2sy+kSc/o9taOtPQYJMvnXXqAmaE4UfwFxqLz7umDKRnsx2x/8VNFh0WgZBqu+V4R HRfA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20240605; h=errors-to:content-transfer-encoding:list-subscribe:list-help :list-post:list-archive:list-unsubscribe:list-id:precedence:subject :to:message-id:date:from:mime-version:dkim-signature:dkim-signature :dkim-signature; bh=xS7LpnZlN3tNA837ZkjMBAAGEO7KN8j7nR32VwkRfHg=; fh=4NbAC/LsuMLI0S0hprUlLSLCiHwg6SCAifhH718Jh0Q=; b=B4Yoo//sjn6dnqhnTAN6T0Jo/XJ186weKmmcMTsrsIBSxcPw+xiLQilPjD6TxGM44b /COXhAkfIMdg2kDJUSaQiowJJeU1WlfoEOhqUvOMnyFGhKXEXP+1+Jv0pv+FBpMPV42q CI5yuuZMH2ubJAgPjcmfAaSUyr2QGNZWLcxL41iBINXWnyuQTGJnrmLKCfxgBOhU7Xfc 5AvX/XDQJ9R2o/PlFZjJWAWbLvGkQL4rLG4GBt/n1a+CQYIF59l9p754pla+9AWEbNj8 5CndbFGu3yffi0/va1A6UdBelKsawIvFbpy2XIH15UPqHpn2FJ0O1KGe3G86J7EmTg/J RibA==; dara=google.com ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@lists.sourceforge.net header.s=beta header.b=P9PP9roL; dkim=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b=PIk0U0+w; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=IZJW136s; 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 Received: from lists.sourceforge.net (lists.sourceforge.net. [216.105.38.7]) by mx.google.com with ESMTPS id 46e09a7af769-7c90fbea9e5si168259a34.166.2025.11.27.00.52.11 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Thu, 27 Nov 2025 00:52:11 -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=P9PP9roL; dkim=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b=PIk0U0+w; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=IZJW136s; 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 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:To:Message-ID:Date:From:MIME-Version:Sender:Reply-To:Cc:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:In-Reply-To:References:List-Owner; bh=xS7LpnZlN3tNA837ZkjMBAAGEO7KN8j7nR32VwkRfHg=; b=P9PP9roL4NwEJE9A6HHG4U+DfJ cuuj2SOkPti6tzxTsDTmIwGYUvl7uPEv+U7fI41PChD3VxLvhSCvYCG8M2hoYZz/0kxERH1WO6Ty9 v+Wxx/obgNnAuEQQLrdAOxfrwOiwPngHfpXnluDLbsl7IgOBe9Ii1v9dUx16uRNwi6HM=; 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 1vOXjV-0002W5-4x; Thu, 27 Nov 2025 08:52:09 +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 1vOXjT-0002Vw-Kg for openvpn-devel@lists.sourceforge.net; Thu, 27 Nov 2025 08:52:07 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=Content-Type:To:Subject:Message-ID:Date:From: MIME-Version:Sender:Reply-To:Cc:Content-Transfer-Encoding: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=t0Sq7xUuuzJQUU8ODNMDJNPxjoGVSH6LdGcqbLwbx/k=; b=PIk0U0+wM68bhk1RepwhETBkTE Dvd+J+SPH1E+GDe8HzuuAjEgLdSJCzaFOIzru2UAmR31XvVduMYAaZjqwRDJyXZufFUjIOQ0n9AUU FH4VE1dPJYJnQqoBMckKuw+aAFL1nYPEgNbdKEGfDH0bQ5rnPBOXBYolAoayKhvV7J8s=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=Content-Type:To:Subject:Message-ID:Date:From:MIME-Version:Sender:Reply-To :Cc:Content-Transfer-Encoding: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=t0Sq7xUuuzJQUU8ODNMDJNPxjoGVSH6LdGcqbLwbx/k=; b=I ZJW136sXB++PI83iC47jM1S+Go6njyLdN57wpYNPzM+gxYbWnwysGwCyj0kA+Fv3THLM+Nh3YO3ow GZlUXVxH6jSh8juSrGnDqcIS7ifYHgweOKWAuYsCKSoVllGhescKL5+CsFma2GtQ7T/EMqnIiY0fV XDjKQ8tUHc6laI6A=; Received: from mail-ua1-f54.google.com ([209.85.222.54]) by sfi-mx-2.v28.lw.sourceforge.com with esmtps (TLS1.2:ECDHE-RSA-AES128-GCM-SHA256:128) (Exim 4.95) id 1vOXjS-0005ZO-Qk for openvpn-devel@lists.sourceforge.net; Thu, 27 Nov 2025 08:52:07 +0000 Received: by mail-ua1-f54.google.com with SMTP id a1e0cc1a2514c-93518a78d0aso338233241.3 for ; Thu, 27 Nov 2025 00:52:06 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1764233521; x=1764838321; h=to:subject:message-id:date:from:mime-version:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=t0Sq7xUuuzJQUU8ODNMDJNPxjoGVSH6LdGcqbLwbx/k=; b=EUoX22C6A0B7k/njR9S8VpvSLhRyaj7h2QrxwQNXypoox78cF/iLaFYvaidH/vLOtx mfETtWR+p4DrQXQZQP2EhKpJgWkR8hzmMkL4/iNsWckx8Y9S7aJG9Fku3PCQXeDxNZIK Xg3VQPJ9mfk+48G6VEFy1BKLGj2kLwru63596HlfSboKPlzkeRSDA8J6/K6XeMEQ3WJ+ o9FkYHL+mBkwOjSt0tCVSQMfq0TR6JuOR4HLdyC8K+zylAECv+7B6kwaFVy0gkzzKHmN zz4U18i7bc2nAl4rvSmgzeIwpnuwQ4XzAjhUn8EDNd1C/phiFzFn7yqTkOx/U+rSpBdE syqA== X-Gm-Message-State: AOJu0YwX7U8IWJJdkPlyGJKSwhGt5esBNiewgBED6Lx/hNLrrwCcq0C2 zMRrQ95m8suQwCIBBl5Lh4ICPC+NGysSAnGIvuRCnCTBBycmU8zwFl5IciwtzIkTCaf70X5CuHl 6FC91F2nwe88SyuOuxtfpuYnB3nUSbkHQDs6B7nhPWdsqeB9G/sIAUQ== X-Gm-Gg: ASbGncuRB7jh9yr/CW3UanrPPu7CQounShzW+AyXQwdTsfRcLJu2s2z5c+jJTJA20zR loHK3FKRdH44rwURZxoQn62CAS7L/z7n9YrZ3F/WMw6GGPzIzlFPdOdD0LspAKvHiPRENYFiPRI Y0oWbHO4/MCmwMM66I2LTX7cxMF3uipewm/RwxGRd7cQ217wwj/TTYG6CWiNmrboCHqHFZqlJ5B Dpi6ytwc2MSpbShYFGq0zOCzrGvh42VHUBB1NwAIuZa5qRuma58hP50scL/fIFL1moXJg== X-Received: by 2002:a05:6102:3a06:b0:5db:2828:c133 with SMTP id ada2fe7eead31-5e1de1c9734mr7618700137.10.1764233520586; Thu, 27 Nov 2025 00:52:00 -0800 (PST) MIME-Version: 1.0 From: Moritz Fain Date: Thu, 27 Nov 2025 09:51:34 +0100 X-Gm-Features: AWmQ_bkqhsFPub_x4j2o8T3-I3NtJqobikoCNFVzKnVebASYaNUCH3uRdG0Z9GY Message-ID: To: openvpn-devel@lists.sourceforge.net X-Spam-Score: 0.0 (/) 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: Previously, the logic for resetting push options (like 'route') was based on `update_options_found` which was local to `apply_push_options`. This meant that if a PUSH_UPDATE was split across multiple [...] Content analysis details: (0.0 points, 5.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- 0.0 HEADER_FROM_DIFFERENT_DOMAINS From and EnvelopeFrom 2nd level mail domains are different 0.0 RCVD_IN_MSPIKE_H2 RBL: Average reputation (+2) [209.85.222.54 listed in wl.mailspike.net] X-Headers-End: 1vOXjS-0005ZO-Qk Subject: [Openvpn-devel] [PATCH 2/2] Push: fix option reset logic in continuation messages 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: =?utf-8?q?1849932938921208434?= X-GMAIL-MSGID: =?utf-8?q?1849932938921208434?= Previously, the logic for resetting push options (like 'route') was based on `update_options_found` which was local to `apply_push_options`. This meant that if a PUSH_UPDATE was split across multiple continuation messages, the state was lost, causing routes to be reset multiple times (once per message chunk) rather than once per update sequence. This patch moves the state tracking to `struct options` as `push_update_options_found`, allowing it to persist across the entire PUSH_UPDATE sequence. This fixes an issue where large route lists sent via PUSH_UPDATE would result in only the last chunk's routes being applied, or previous routes being continuously deleted and re-added. Added unit test `test_incoming_push_continuation_route_accumulation` to verify the fix. --- src/openvpn/options.c | 1 + src/openvpn/options.h | 1 + src/openvpn/options_parse.c | 9 +- src/openvpn/push.c | 7 +- .../unit_tests/openvpn/test_push_update_msg.c | 101 +++++++++++++++++- 5 files changed, 111 insertions(+), 8 deletions(-) buffer *buf, unsigned int permission_mask, unsigned int *option_types_found, @@ -61,6 +75,12 @@ apply_push_options(struct context *c, struct options *options, struct buffer *bu { char line[OPTION_PARM_SIZE]; + /* + * Use persistent push_update_options_found from options struct to track + * which option types have been reset across continuation messages. + * This is the FIXED behavior - routes are only reset once per PUSH_UPDATE sequence. + */ + while (buf_parse(buf, ',', line, sizeof(line))) { unsigned int push_update_option_flags = 0; @@ -84,10 +104,18 @@ apply_push_options(struct context *c, struct options *options, struct buffer *bu return false; /* Cause push/pull error and stop push processing */ } } - /* - * No need to test also the application part here - * (add_option/remove_option/update_option) - */ + + /* Simulate route handling from update_option() in options.c */ + if (is_update && strncmp(&line[i], "route ", 6) == 0) + { + if (!(options->push_update_options_found & OPT_P_U_ROUTE)) + { + /* First route in entire PUSH_UPDATE sequence - reset routes once */ + route_reset_count++; + options->push_update_options_found |= OPT_P_U_ROUTE; + } + route_add_count++; + } } return true; } @@ -292,6 +320,69 @@ test_incoming_push_message_mix2(void **state) free_buf(&buf); } +/** + * Test that routes accumulate correctly across multiple continuation messages. + * This test exposes a bug where update_options_found is reset to 0 for each + * continuation message, causing routes to be reset on each message instead + * of accumulating. + * + * Expected behavior: routes should only be reset ONCE (when -route is received), + * then all subsequent routes should accumulate. + * + * Current bug: routes are reset on the first route of EACH continuation message. + */ +static void +test_incoming_push_continuation_route_accumulation(void **state) +{ + struct context *c = *state; + unsigned int option_types_found = 0; + + reset_route_counters(); + + /* Message 1: removal + first batch of routes, continuation 2 (more coming) */ + struct buffer buf1 = alloc_buf(512); + const char *msg1 = "PUSH_UPDATE,-route, route 10.1.0.0 255.255.0.0, route 10.2.0.0 255.255.0.0, route 10.3.0.0 255.255.0.0,push-continuation 2"; + buf_write(&buf1, msg1, strlen(msg1)); + + assert_int_equal(process_incoming_push_msg(c, &buf1, c->options.pull, pull_permission_mask(c), + &option_types_found), + PUSH_MSG_CONTINUATION); + free_buf(&buf1); + + /* Message 2: more routes, continuation 2 (more coming) */ + struct buffer buf2 = alloc_buf(512); + const char *msg2 = "PUSH_UPDATE, route 10.4.0.0 255.255.0.0, route 10.5.0.0 255.255.0.0, route 10.6.0.0 255.255.0.0,push-continuation 2"; + buf_write(&buf2, msg2, strlen(msg2)); + + assert_int_equal(process_incoming_push_msg(c, &buf2, c->options.pull, pull_permission_mask(c), + &option_types_found), + PUSH_MSG_CONTINUATION); + free_buf(&buf2); + + /* Message 3: final batch of routes, continuation 1 (last message) */ + struct buffer buf3 = alloc_buf(512); + const char *msg3 = "PUSH_UPDATE, route 10.7.0.0 255.255.0.0, route 10.8.0.0 255.255.0.0, route 10.9.0.0 255.255.0.0,push-continuation 1"; + buf_write(&buf3, msg3, strlen(msg3)); + + assert_int_equal(process_incoming_push_msg(c, &buf3, c->options.pull, pull_permission_mask(c), + &option_types_found), + PUSH_MSG_UPDATE); + free_buf(&buf3); + + /* Verify: all 9 routes should have been added */ + assert_int_equal(route_add_count, 9); + + /* + * BUG CHECK: Routes should only be "reset" once (for the entire update sequence). + * Due to the bug where update_options_found is local to each message, + * routes get reset on the first route of EACH message (3 times instead of 1). + * + * When the bug is fixed, this assertion should pass with route_reset_count == 1. + * Currently it will show route_reset_count == 3, exposing the bug. + */ + assert_int_equal(route_reset_count, 1); +} + #ifdef ENABLE_MANAGEMENT char *r0[] = { "PUSH_UPDATE,redirect-gateway local,route 192.168.1.0 255.255.255.0", @@ -603,6 +694,8 @@ main(void) cmocka_unit_test_setup_teardown(test_incoming_push_message_bad_format, setup, teardown), cmocka_unit_test_setup_teardown(test_incoming_push_message_mix, setup, teardown), cmocka_unit_test_setup_teardown(test_incoming_push_message_mix2, setup, teardown), + cmocka_unit_test_setup_teardown(test_incoming_push_continuation_route_accumulation, setup, + teardown), #ifdef ENABLE_MANAGEMENT cmocka_unit_test_setup_teardown(test_send_push_msg0, setup2, teardown2), diff --git a/src/openvpn/options.c b/src/openvpn/options.c index 8a97374e..fe4f5814 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -3238,6 +3238,7 @@ pre_connect_restore(struct options *o, struct gc_arena *gc) o->push_continuation = 0; o->push_option_types_found = 0; + o->push_update_options_found = 0; o->imported_protocol_flags = 0; } diff --git a/src/openvpn/options.h b/src/openvpn/options.h index bd013dba..ca94b004 100644 --- a/src/openvpn/options.h +++ b/src/openvpn/options.h @@ -559,6 +559,7 @@ struct options bool pull; /* client pull of config options from server */ int push_continuation; unsigned int push_option_types_found; + unsigned int push_update_options_found; /* tracks which option types have been reset in current PUSH_UPDATE sequence */ const char *auth_user_pass_file; bool auth_user_pass_file_inline; struct options_pre_connect *pre_connect; diff --git a/src/openvpn/options_parse.c b/src/openvpn/options_parse.c index bb5b4049..99d096b0 100644 --- a/src/openvpn/options_parse.c +++ b/src/openvpn/options_parse.c @@ -517,7 +517,12 @@ apply_push_options(struct context *c, struct options *options, struct buffer *bu int line_num = 0; const char *file = "[PUSH-OPTIONS]"; const msglvl_t msglevel = D_PUSH_ERRORS | M_OPTERR; - unsigned int update_options_found = 0; + + /* + * Use persistent push_update_options_found from options struct to track + * which option types have been reset across continuation messages. + * This prevents routes from being reset on each continuation message. + */ while (buf_parse(buf, ',', line, sizeof(line))) { @@ -565,7 +570,7 @@ apply_push_options(struct context *c, struct options *options, struct buffer *bu else { update_option(c, options, p, false, file, line_num, 0, msglevel, permission_mask, - option_types_found, es, &update_options_found); + option_types_found, es, &options->push_update_options_found); } } } diff --git a/src/openvpn/push.c b/src/openvpn/push.c index 7852d360..d0601871 100644 --- a/src/openvpn/push.c +++ b/src/openvpn/push.c @@ -540,16 +540,19 @@ incoming_push_message(struct context *c, const struct buffer *buffer) } else { - if (!option_types_found) + /* Use accumulated option_types_found for the entire PUSH_UPDATE sequence */ + if (!c->options.push_option_types_found) { msg(M_WARN, "No updatable options found in incoming PUSH_UPDATE message"); } - else if (!do_update(c, option_types_found)) + else if (!do_update(c, c->options.push_option_types_found)) { msg(D_PUSH_ERRORS, "Failed to update options"); goto error; } } + /* Clear push_update_options_found for next PUSH_UPDATE sequence */ + c->options.push_update_options_found = 0; } event_timeout_clear(&c->c2.push_request_interval); event_timeout_clear(&c->c2.wait_for_connect); diff --git a/tests/unit_tests/openvpn/test_push_update_msg.c b/tests/unit_tests/openvpn/test_push_update_msg.c index 6ef12668..1a8b1542 100644 --- a/tests/unit_tests/openvpn/test_push_update_msg.c +++ b/tests/unit_tests/openvpn/test_push_update_msg.c @@ -54,6 +54,20 @@ options_postprocess_pull(struct options *options, struct env_set *es) return true; } +/* + * Counters to track route accumulation across continuation messages. + * Used to verify the bug where update_options_found resets per message. + */ +static int route_reset_count = 0; +static int route_add_count = 0; + +static void +reset_route_counters(void) +{ + route_reset_count = 0; + route_add_count = 0; +} + bool apply_push_options(struct context *c, struct options *options, struct