PUSH_UPDATE: fix option reset logic in continuation messages #925
+131
−26
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
cron2
pushed a commit
to cron2/openvpn
that referenced
this pull request
Dec 10, 2025
OpenVPN/openvpn#925 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. Acked-by: Marco Baffo <marco@mandelbit.com> Message-Id: <CAM8w-qEE6vHj=yUOpTFbM7DqPKzUV0NupvEG4rUefY=kNB2DxQ@mail.gmail.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg34814.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
cron2
pushed a commit
that referenced
this pull request
Dec 10, 2025
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. Github: #925 Signed-off-by: Moritz Fain <moritz-openvpn@fain.io> Acked-by: Marco Baffo <marco@mandelbit.com> Message-Id: <CAM8w-qEE6vHj=yUOpTFbM7DqPKzUV0NupvEG4rUefY=kNB2DxQ@mail.gmail.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg34814.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
cron2
added a commit
that referenced
this pull request
Dec 17, 2025
version.m4, ChangeLog, Changes.rst
Changes.rst has not received an "2.7_rc4" section - it has the
"highlevel" overview of what is new in 2.7, but for alpha/beta/rc*
releases it's better to look at git log to see what has been added/fixed.
Notable changes rc3 -> rc4 are:
- Windows interactive service: do not configure adapter DNS if
there are no search-domains but there are resolve-domains (which
get resolved via NRPT rules) - GH: #473
- improve documentation and error messages for a number of deprecated
options
- improve documentation for not-really-deprecated-yet ``--ns-cert-type``
- Windows IPv4 configuration with netsh.exe: ensure addresses are added
with "store=active" (ensure proper cleanup) - GH: #915
- Windows: set UTF8 code page in openvpn.exe manifest, to make cert/key
loading work again for files with non-ASCII characters in their file
name (GH: #920)
- tun.c: unify read_tun()/write_tun() functions for all BSD platforms
- more type conversion related cleanups
- add NULL check before freeaddrinfo() call, which might lead to a
crash on OpenBSD (GH: #930)
- add NULL check to mbedtls handling of external and inline certificates
- add check for auth none / cipher none on FreeBSD DCO
- add CAP_SYS_NICE to positive list in Linux systemd unit files
(GH: #834)
- drop mbedtls 2.x support (which is end of life, and work on mbedtls 4
is much simplified by not having to take care of 2.x compat as well)
- PUSH_UPDATE: bugfix for the client side where split/continued messages
(due to large number of "route" statements) would not correctly handle
the full set of routes. Add unit test. (GH: #925)
- new unit test module for mbuf handling
- deprecate --fast-io option (it got partially broken by the multisocket
implementation, and the benefits of the existing implementation did
not outweigh the extra code complexity to make it work again)
- change the ssl_ctx in struct tls_options to be a pointer - this is
a shared data structure between various contexts, but previously it
was shallow-copied, leading to needless CRL reloading - and when
working on implementing the new OpenSSL CRL API, to segfaults
(the existing code works, as these new APIs are not used yet).
Signed-off-by: Gert Doering <gert@greenie.muc.de>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously, the logic for resetting push options (like 'route') was based on
update_options_foundwhich was local toapply_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 optionsaspush_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_accumulationto verify the fix.