openvpn-devel — OpenVPN developers list

[Openvpn-devel] [XS] Change in openvpn[master]: Fix --dns options for TAP adapter

[stipa (C. Review) <ge...@op...>]

Attention is currently required from: flichtenheld, plaisthos.

Hello plaisthos, flichtenheld,

I'd like you to do a code review.
Please visit

    http://gerrit.openvpn.net/c/openvpn/+/1120?usp=email

to review the following change.


Change subject: Fix --dns options for TAP adapter
......................................................................

Fix --dns options for TAP adapter

Commit

    2dfc4f ("dns: deal with --dhcp-options when --dns is active")

has accidentally removed setting of the DHCP_OPTIONS_DHCP_OPTIONAL
flag when copying --dns options. This flag is required to apply options
via DHCP string, which we do for TAP adapter. As a result, --dns options
stopped working for TAP.

Fix by setting this flag when copying --dns options to tuntap_options.

Change-Id: Id95cd14095a03afb3140a03ae96e9f5679e4fe89
Signed-off-by: Lev Stipakov <le...@op...>
---
M src/openvpn/options.c
1 file changed, 3 insertions(+), 0 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/20/1120/1

diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 53be6f5..fde3e73 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -3579,6 +3579,9 @@
             {
                 msg(M_WARN, "WARNING: couldn't copy all --dns server addresses to TUN/TAP");
             }
+#if defined(_WIN32)
+            tt->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL;
+#endif
             return;
         }
     }

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1120?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Id95cd14095a03afb3140a03ae96e9f5679e4fe89
Gerrit-Change-Number: 1120
Gerrit-PatchSet: 1
Gerrit-Owner: stipa <lst...@gm...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-MessageType: newchange

[Openvpn-devel] IRC community meeting summary

[Johan D. <jo...@op...>]

Meeting summary for 30 July 2025:

  *

    *New: build failures*
    Changes in Gerrit do not automatically pick up fixes from master, so
    sometimes we see a lot of build failures even though those are fixed
    in master.
    Some options were discussed, seems like the one preferred is to
    reject pushes that are behind. djpig will look into it.

  *

    *New: Reference manuals OpenVPN 2.x*
    There was a mistake made on the main website which for a few hours?
    or days? made the reference manual 404.
    This has been restored. However the URL will slightly change in the
    future, but a redirect on the old URL will be in place.
    mattock raised the question where the latest OpenVPN man page should
    be hosted.
    When discussing this we agreed that djpig will contact the technical
    writer for openvpn.net and see if the ingestion/updating process can
    be smoother.
    Independently we can also investigate getting the manuals on the
    wiki somewhere, perhaps automated.

  *

    *Updated: push_update / live route updates*
    Client-side support for push_update is now merged.
    For server-side support, company did QA on it with openvpn2 but
    found some missing features that are being added now.
    At least client-side support will probably make it into alpha3.

  *

    *Updated: Release 2.7*
    A 2.7 alpha3 release is planned for 31 July.
    This includes a fix for a couple of rare crashes on Windows DCO
    driver. We'll backport these fixes to the Windows installer for 2.6.
    The FreeBSD patch for float is now in review.
    For the DCO related changes, DCO+TCP is improving. Epoch data keys
    still needs to be done, mssfix not planned yet.

As always you're welcome to join at #openvpn-meeting on Libera IRC 
network every Wednesday at 14:00 Central European Time.

Kind regards,
Johan Draaisma

[Openvpn-devel] [S] Change in openvpn[master]: Fix broken DHCP options

[cron2 (C. Review) <ge...@op...>]

cron2 has submitted this change. ( http://gerrit.openvpn.net/c/openvpn/+/1090?usp=email )

Change subject: Fix broken DHCP options
......................................................................

Fix broken DHCP options

Commit

  2dfc4f ("dns: deal with --dhcp-options when --dns is active")

broke support for --dhcp-options. It removed the setting of the
DHCP_OPTIONS_DHCP_OPTIONAL flag for some DHCP options. This flag is
required for those options to be applied correctly, as it is used
when building the DHCP options string that is passed to the TAP driver.

This commit fixes the issue by restoring the setting of this flag.

GitHub: fixes OpenVPN/openvpn#791

Change-Id: I0d75efcceb826d06e74abd003d5377468ff9fe3b
Signed-off-by: Lev Stipakov <le...@op...>
Acked-by: Gert Doering <ge...@gr...>
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg32427.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M src/openvpn/options.c
1 file changed, 13 insertions(+), 0 deletions(-)




diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index b7328d7..53be6f5 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -8712,9 +8712,12 @@
 #endif
         VERIFY_PERMISSION(OPT_P_DHCPDNS);
 
+        bool dhcp_optional = false;
+
         if ((streq(p[1], "DOMAIN") || streq(p[1], "ADAPTER_DOMAIN_SUFFIX")) && p[2] && !p[3])
         {
             dhcp->domain = p[2];
+            dhcp_optional = true;
         }
         else if (streq(p[1], "DOMAIN-SEARCH") && p[2] && !p[3])
         {
@@ -8727,6 +8730,7 @@
                 msg(msglevel, "--dhcp-option %s: maximum of %d search entries can be specified",
                     p[1], N_SEARCH_LIST_LEN);
             }
+            dhcp_optional = true;
         }
         else if ((streq(p[1], "DNS") || streq(p[1], "DNS6")) && p[2] && !p[3]
                  && (!strstr(p[2], ":") || ipv6_addr_safe(p[2])))
@@ -8738,6 +8742,7 @@
             else
             {
                 dhcp_option_address_parse("DNS", p[2], dhcp->dns, &dhcp->dns_len, msglevel);
+                dhcp_optional = true;
             }
         }
 #if defined(_WIN32) || defined(TARGET_ANDROID)
@@ -8761,6 +8766,7 @@
         else if (streq(p[1], "WINS") && p[2] && !p[3])
         {
             dhcp_option_address_parse("WINS", p[2], o->wins, &o->wins_len, msglevel);
+            o->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL;
         }
         else if (streq(p[1], "NTP") && p[2] && !p[3])
         {
@@ -8792,6 +8798,13 @@
 #else /* if defined(_WIN32) || defined(TARGET_ANDROID) */
         setenv_foreign_option(options, p[1], p[2], es);
 #endif /* if defined(_WIN32) || defined(TARGET_ANDROID) */
+
+        if (dhcp_optional)
+        {
+#if defined(_WIN32) || defined(TARGET_ANDROID)
+            o->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL;
+#endif
+        }
     }
 #ifdef _WIN32
     else if (streq(p[0], "show-adapters") && !p[1])

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1090?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I0d75efcceb826d06e74abd003d5377468ff9fe3b
Gerrit-Change-Number: 1090
Gerrit-PatchSet: 2
Gerrit-Owner: stipa <lst...@gm...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-MessageType: merged

[Openvpn-devel] [S] Change in openvpn[master]: Fix broken DHCP options

[cron2 (C. Review) <ge...@op...>]

cron2 has uploaded a new patch set (#2) to the change originally created by stipa. ( http://gerrit.openvpn.net/c/openvpn/+/1090?usp=email )

The following approvals got outdated and were removed:
Code-Review+2 by cron2


Change subject: Fix broken DHCP options
......................................................................

Fix broken DHCP options

Commit

  2dfc4f ("dns: deal with --dhcp-options when --dns is active")

broke support for --dhcp-options. It removed the setting of the
DHCP_OPTIONS_DHCP_OPTIONAL flag for some DHCP options. This flag is
required for those options to be applied correctly, as it is used
when building the DHCP options string that is passed to the TAP driver.

This commit fixes the issue by restoring the setting of this flag.

GitHub: fixes OpenVPN/openvpn#791

Change-Id: I0d75efcceb826d06e74abd003d5377468ff9fe3b
Signed-off-by: Lev Stipakov <le...@op...>
Acked-by: Gert Doering <ge...@gr...>
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg32427.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M src/openvpn/options.c
1 file changed, 13 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/90/1090/2

diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index b7328d7..53be6f5 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -8712,9 +8712,12 @@
 #endif
         VERIFY_PERMISSION(OPT_P_DHCPDNS);
 
+        bool dhcp_optional = false;
+
         if ((streq(p[1], "DOMAIN") || streq(p[1], "ADAPTER_DOMAIN_SUFFIX")) && p[2] && !p[3])
         {
             dhcp->domain = p[2];
+            dhcp_optional = true;
         }
         else if (streq(p[1], "DOMAIN-SEARCH") && p[2] && !p[3])
         {
@@ -8727,6 +8730,7 @@
                 msg(msglevel, "--dhcp-option %s: maximum of %d search entries can be specified",
                     p[1], N_SEARCH_LIST_LEN);
             }
+            dhcp_optional = true;
         }
         else if ((streq(p[1], "DNS") || streq(p[1], "DNS6")) && p[2] && !p[3]
                  && (!strstr(p[2], ":") || ipv6_addr_safe(p[2])))
@@ -8738,6 +8742,7 @@
             else
             {
                 dhcp_option_address_parse("DNS", p[2], dhcp->dns, &dhcp->dns_len, msglevel);
+                dhcp_optional = true;
             }
         }
 #if defined(_WIN32) || defined(TARGET_ANDROID)
@@ -8761,6 +8766,7 @@
         else if (streq(p[1], "WINS") && p[2] && !p[3])
         {
             dhcp_option_address_parse("WINS", p[2], o->wins, &o->wins_len, msglevel);
+            o->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL;
         }
         else if (streq(p[1], "NTP") && p[2] && !p[3])
         {
@@ -8792,6 +8798,13 @@
 #else /* if defined(_WIN32) || defined(TARGET_ANDROID) */
         setenv_foreign_option(options, p[1], p[2], es);
 #endif /* if defined(_WIN32) || defined(TARGET_ANDROID) */
+
+        if (dhcp_optional)
+        {
+#if defined(_WIN32) || defined(TARGET_ANDROID)
+            o->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL;
+#endif
+        }
     }
 #ifdef _WIN32
     else if (streq(p[0], "show-adapters") && !p[1])

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1090?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I0d75efcceb826d06e74abd003d5377468ff9fe3b
Gerrit-Change-Number: 1090
Gerrit-PatchSet: 2
Gerrit-Owner: stipa <lst...@gm...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-MessageType: newpatchset

[Openvpn-devel] [PATCH applied] Re: Fix broken DHCP options

[Gert D. <ge...@gr...>]

Thanks.  I've ran into this myself with 2.7_alpha2 and a setup that 
ended up doing DHCP on TAP, so thanks for the fix :-)

I have not tested it beyond "does it compile", but looking at the code
flow and finding tt->options.dhcp_options in tun.c, this is "obviously
correct".  The dance with the helper variable is unavoidable due to
the platform-dependent nature of the options.c code here... ("or you
get 3 extra #ifdefs").

Your patch has been applied to the master branch.

commit 040e0d1c308a587ae9a26295eeaa070561befa0d
Author: Lev Stipakov
Date:   Wed Jul 30 20:04:26 2025 +0200

     Fix broken DHCP options

     Signed-off-by: Lev Stipakov <le...@op...>
     Acked-by: Gert Doering <ge...@gr...>
     Message-Id: <202...@gr...>
     URL: https://www.mail-archive.com/ope...@li.../msg32427.html
     Signed-off-by: Gert Doering <ge...@gr...>


--
kind regards,

Gert Doering

[Openvpn-devel] [PATCH v1] Fix broken DHCP options

[Gert D. <ge...@gr...>]

From: Lev Stipakov <le...@op...>

Commit

  2dfc4f ("dns: deal with --dhcp-options when --dns is active")

broke support for --dhcp-options. It removed the setting of the
DHCP_OPTIONS_DHCP_OPTIONAL flag for some DHCP options. This flag is
required for those options to be applied correctly, as it is used
when building the DHCP options string that is passed to the TAP driver.

This commit fixes the issue by restoring the setting of this flag.

GitHub: fixes OpenVPN/openvpn#791

Change-Id: I0d75efcceb826d06e74abd003d5377468ff9fe3b
Signed-off-by: Lev Stipakov <le...@op...>
Acked-by: Gert Doering <ge...@gr...>
---

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/+/1090
This mail reflects revision 1 of this Change.

Acked-by according to Gerrit (reflected above):
Gert Doering <ge...@gr...>

        
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 0662b49..4ba9243 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -8310,9 +8310,12 @@
 #endif
         VERIFY_PERMISSION(OPT_P_DHCPDNS);
 
+        bool dhcp_optional = false;
+
         if ((streq(p[1], "DOMAIN") || streq(p[1], "ADAPTER_DOMAIN_SUFFIX")) && p[2] && !p[3])
         {
             dhcp->domain = p[2];
+            dhcp_optional = true;
         }
         else if (streq(p[1], "DOMAIN-SEARCH") && p[2] && !p[3])
         {
@@ -8325,6 +8328,7 @@
                 msg(msglevel, "--dhcp-option %s: maximum of %d search entries can be specified",
                     p[1], N_SEARCH_LIST_LEN);
             }
+            dhcp_optional = true;
         }
         else if ((streq(p[1], "DNS") || streq(p[1], "DNS6")) && p[2] && !p[3]
                  && (!strstr(p[2], ":") || ipv6_addr_safe(p[2])))
@@ -8336,6 +8340,7 @@
             else
             {
                 dhcp_option_address_parse("DNS", p[2], dhcp->dns, &dhcp->dns_len, msglevel);
+                dhcp_optional = true;
             }
         }
 #if defined(_WIN32) || defined(TARGET_ANDROID)
@@ -8359,6 +8364,7 @@
         else if (streq(p[1], "WINS") && p[2] && !p[3])
         {
             dhcp_option_address_parse("WINS", p[2], o->wins, &o->wins_len, msglevel);
+            o->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL;
         }
         else if (streq(p[1], "NTP") && p[2] && !p[3])
         {
@@ -8390,6 +8396,13 @@
 #else /* if defined(_WIN32) || defined(TARGET_ANDROID) */
         setenv_foreign_option(options, (const char **)p, 3, es);
 #endif /* if defined(_WIN32) || defined(TARGET_ANDROID) */
+
+        if (dhcp_optional)
+        {
+#if defined(_WIN32) || defined(TARGET_ANDROID)
+            o->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL;
+#endif
+        }
     }
 #ifdef _WIN32
     else if (streq(p[0], "show-adapters") && !p[1])

[Openvpn-devel] [S] Change in openvpn[master]: Fix broken DHCP options

[cron2 (C. Review) <ge...@op...>]

Attention is currently required from: flichtenheld, plaisthos, stipa.

cron2 has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/1090?usp=email )

Change subject: Fix broken DHCP options
......................................................................


Patch Set 1: Code-Review+2

(1 comment)

Patchset:

PS1: 
This plays together with the code in tun.c that checks whether or not the TAP driver needs to be informed about DHCP options

```
    /* user-supplied DHCP options capability */
    if (tt->options.dhcp_options)
    {
..
            if (!DeviceIoControl(tt->hand, TAP_WIN_IOCTL_CONFIG_DHCP_SET_OPT,
..
```

... so if we no longer flag these "it can be done with DHCP or not" options as "well, if we have DHCP, we should set them!" (DHCP_OPTIONS_DHCP_OPTIONAL), and we have no "this can only be done with DHCP!" options (DHCP_OPTIONS_DHCP_REQUIRED), then tun.c just assumes "well, nothing required, nothing to set up" and DNS will not work.

I experienced the issue myself with 2.7_alpha2 and a setup that ended up using TAP and DHCP, and the explanation makes sense.



-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1090?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I0d75efcceb826d06e74abd003d5377468ff9fe3b
Gerrit-Change-Number: 1090
Gerrit-PatchSet: 1
Gerrit-Owner: stipa <lst...@gm...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Attention: stipa <lst...@gm...>
Gerrit-Comment-Date: Wed, 30 Jul 2025 18:04:15 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Re: [Openvpn-devel] [PATCH] Update systemd service name param to match command

[Timo R. <ti...@ro...>]

On 29/07/2025 20:00, Terrance via Openvpn-devel wrote:
> From: Terrance <gi...@te...>
> 
> The service name displays `%I` which invokes systemd's path mangling
> (notably, converting `-` to `/`), suggesting you need to provide an
> encoded parameter (via e.g. `systemd-escape`), but the start command
> itself uses `%i` which doesn't do the conversion.
> 
> This updates the service name to match the start command.

This makes me wonder if it isn't the one passed to the start command 
that's actually not what it was intended to be.

Cause using %I there would enable to put configs in subdirs. With %i 
that's impossible, since it can't contain slashes.

Re: [Openvpn-devel] [PATCH] Update systemd service name param to match command

[Frank L. <fr...@li...>]

On Tue, Jul 29, 2025 at 07:00:05PM +0100, Terrance via Openvpn-devel wrote:
> From: Terrance <gi...@te...>
> 
> The service name displays `%I` which invokes systemd's path mangling
> (notably, converting `-` to `/`), suggesting you need to provide an
> encoded parameter (via e.g. `systemd-escape`), but the start command
> itself uses `%i` which doesn't do the conversion.
> 
> This updates the service name to match the start command.

Change makes sense to me. I haven't specifically tested it but it only
changes the description so I don't see how it could break anything.

Acked-by: Frank Lichtenheld <fr...@li...>

Regards,
-- 
  Frank Lichtenheld

[Openvpn-devel] [PATCH] Update systemd service name param to match command

[Terrance <sou...@te...>]

From: Terrance <gi...@te...>

The service name displays `%I` which invokes systemd's path mangling
(notably, converting `-` to `/`), suggesting you need to provide an
encoded parameter (via e.g. `systemd-escape`), but the start command
itself uses `%i` which doesn't do the conversion.

This updates the service name to match the start command.
---
Additional discussion: https://github.com/OpenVPN/openvpn/pull/802

 distro/systemd/openvpn-client@.service.in | 2 +-
 distro/systemd/openvpn-server@.service.in | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/distro/systemd/openvpn-client@.service.in b/distro/systemd/openvpn-client@.service.in
index ae62e8c7..c32bb333 100644
--- a/distro/systemd/openvpn-client@.service.in
+++ b/distro/systemd/openvpn-client@.service.in
@@ -1,4 +1,4 @@
 [Unit]
-Description=OpenVPN tunnel for %I
+Description=OpenVPN tunnel for %i
 After=network-online.target
 Wants=network-online.target
diff --git a/distro/systemd/openvpn-server@.service.in b/distro/systemd/openvpn-server@.service.in
index 5123e072..547f5f54 100644
--- a/distro/systemd/openvpn-server@.service.in
+++ b/distro/systemd/openvpn-server@.service.in
@@ -1,4 +1,4 @@
 [Unit]
-Description=OpenVPN service for %I
+Description=OpenVPN service for %i
 After=network-online.target
 Wants=network-online.target
--
2.50.1

[Openvpn-devel] [PATCH applied] Re: dco-freebsd: pass address scope to the kernel

[Gert D. <ge...@gr...>]

I have tested that this does not break on kernels that do not have scope
support (as discussed on the list).  Good :-) - float notifications also
still work fine (unsurprisingly).

Need to setup a testbed with client and server on the same LAN, and
then test DCO with link-local addresses, scope, and floating...

Your patch has been applied to the master branch.

commit b5e0032bf09afa928d9728051599a5e738ff4d10
Author: Kristof Provost
Date:   Tue Jul 29 11:38:57 2025 +0200

     dco-freebsd: pass address scope to the kernel

     Signed-off-by: Kristof Provost <kpr...@ne...>
     Acked-by: Gert Doering <ge...@gr...>
     Message-Id: <202...@ne...>
     URL: https://www.mail-archive.com/ope...@li.../msg32401.html
     Signed-off-by: Gert Doering <ge...@gr...>


--
kind regards,

Gert Doering

[Openvpn-devel] [PATCH applied] Re: PUSH_UPDATE: Added update_option() function.

[Gert D. <ge...@gr...>]

.. and the last one in the series, with similar text leading to it...
(so just look at 808+809).

Tested on the t_client/t_server framework, which exercises many of those
options where the handling even in the "normal path" is now changed (due
to moving the syntax check into smaller helper functions).

I'm still not convinced that we really need to do all these syntax checks
for "remove an option", but that message was not really heard... also, I'm
not really sure the current behaviour is that useful outside fairly limited
OpenVPN Corp scenarios ("why would a PUSH_UPDATE route statement remove
all routes I have in my config file, not only those sent by PUSH_REPLY?"),
but we all agreed that this is the beginning of a journey, also expecting
incremental improvements ("not reopen the tun device if there is no reason
for it", for example, because the server just pushes extra routes).

Your patch has been applied to the master branch.

commit dd9c6978ec851213deece9229e413d44d0289cad
Author: Marco Baffo
Date:   Tue Jul 29 12:41:01 2025 +0200

     PUSH_UPDATE: Added update_option() function.

     Signed-off-by: Marco Baffo <ma...@ma...>
     Acked-by: Gert Doering <ge...@gr...>
     Message-Id: <202...@gr...>
     URL: https://www.mail-archive.com/ope...@li.../msg32408.html
     Signed-off-by: Gert Doering <ge...@gr...>


--
kind regards,

Gert Doering

[Openvpn-devel] [L] Change in openvpn[master]: PUSH_UPDATE: Added update_option() function.

[cron2 (C. Review) <ge...@op...>]

cron2 has uploaded a new patch set (#24) to the change originally created by mrbff. ( http://gerrit.openvpn.net/c/openvpn/+/810?usp=email )

The following approvals got outdated and were removed:
Code-Review+2 by cron2


Change subject: PUSH_UPDATE: Added update_option() function.
......................................................................

PUSH_UPDATE: Added update_option() function.

When the function receives an option to update, it first checks whether it has
already received an option of the same type within the same update message.
If it has already received it, it simply calls add_option(), otherwise it
deletes all the values already present for that option first.

Change-Id: Ia45c99e6df7b3ad24020c10b8a9b3577984ecdc2
Signed-off-by: Marco Baffo <ma...@ma...>
Acked-by: Gert Doering <ge...@gr...>
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg32408.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M src/openvpn/options.c
1 file changed, 331 insertions(+), 124 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/10/810/24

diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 469360d..b7328d7 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -5360,6 +5360,20 @@
               struct env_set *es);
 
 static void
+update_option(struct context *c,
+              struct options *options,
+              char *p[],
+              bool is_inline,
+              const char *file,
+              int line,
+              const int level,
+              const int msglevel,
+              const unsigned int permission_mask,
+              unsigned int *option_types_found,
+              struct env_set *es,
+              unsigned int *update_options_found);
+
+static void
 read_config_file(struct options *options,
                  const char *file,
                  int level,
@@ -5545,6 +5559,7 @@
     int line_num = 0;
     const char *file = "[PUSH-OPTIONS]";
     const int msglevel = D_PUSH_ERRORS|M_OPTERR;
+    unsigned int update_options_found = 0;
 
     while (buf_parse(buf, ',', line, sizeof(line)))
     {
@@ -5590,6 +5605,11 @@
                 remove_option(c, options, p, false, file, line_num, msglevel,
                               permission_mask, option_types_found, es);
             }
+            else
+            {
+                update_option(c, options, p, false, file, line_num, 0, msglevel,
+                              permission_mask, option_types_found, es, &update_options_found);
+            }
         }
     }
     return true;
@@ -5728,6 +5748,13 @@
     return options->forward_compatible ? M_WARN : msglevel;
 }
 
+#define RESET_OPTION_ROUTES(option_ptr, field) \
+    if (option_ptr)                        \
+    {                                      \
+        option_ptr->field = NULL;          \
+        option_ptr->flags = 0;             \
+    }
+
 /**
  * @brief Resets options found in the PUSH_UPDATE message that are preceded by the `-` flag.
  *        This function is used in push-updates to reset specified options.
@@ -5782,11 +5809,7 @@
             delete_routes_v4(c->c1.route_list, c->c1.tuntap,
                              ROUTE_OPTION_FLAGS(&c->options),
                              es, &c->net_ctx);
-            if (options->routes)
-            {
-                options->routes->routes = NULL;
-                options->routes->flags = 0;
-            }
+            RESET_OPTION_ROUTES(options->routes, routes);
         }
     }
     else if (streq(p[0], "route-ipv6") && !p[1])
@@ -5797,11 +5820,7 @@
             delete_routes_v6(c->c1.route_ipv6_list, c->c1.tuntap,
                              ROUTE_OPTION_FLAGS(&c->options),
                              es, &c->net_ctx);
-            if (options->routes_ipv6)
-            {
-                options->routes_ipv6->routes_ipv6 = NULL;
-                options->routes_ipv6->flags = 0;
-            }
+            RESET_OPTION_ROUTES(options->routes_ipv6, routes_ipv6);
         }
     }
     else if (streq(p[0], "route-gateway") && !p[1])
@@ -5921,6 +5940,303 @@
     msg(msglevel, "Error occurred trying to remove %s option", p[0]);
 }
 
+
+static bool
+check_route_option(struct options *options, char *p[], const int msglevel, bool pull_mode)
+{
+    rol_check_alloc(options);
+    if (pull_mode)
+    {
+        if (!ip_or_dns_addr_safe(p[1], options->allow_pull_fqdn) && !is_special_addr(p[1])) /* FQDN -- may be DNS name */
+        {
+            msg(msglevel, "route parameter network/IP '%s' must be a valid address", p[1]);
+            return false;
+        }
+        if (p[2] && !ip_addr_dotted_quad_safe(p[2])) /* FQDN -- must be IP address */
+        {
+            msg(msglevel, "route parameter netmask '%s' must be an IP address", p[2]);
+            return false;
+        }
+        if (p[3] && !ip_or_dns_addr_safe(p[3], options->allow_pull_fqdn) && !is_special_addr(p[3])) /* FQDN -- may be DNS name */
+        {
+            msg(msglevel, "route parameter gateway '%s' must be a valid address", p[3]);
+            return false;
+        }
+    }
+    return true;
+}
+
+
+static bool
+check_route6_option(struct options *options, char *p[], const int msglevel, bool pull_mode)
+{
+    rol6_check_alloc(options);
+    if (pull_mode)
+    {
+        if (!ipv6_addr_safe_hexplusbits(p[1]))
+        {
+            msg(msglevel, "route-ipv6 parameter network/IP '%s' must be a valid address", p[1]);
+            return false;
+        }
+        if (p[2] && !ipv6_addr_safe(p[2]))
+        {
+            msg(msglevel, "route-ipv6 parameter gateway '%s' must be a valid address", p[2]);
+            return false;
+        }
+        /* p[3] is metric, if present */
+    }
+    return true;
+}
+
+static bool
+check_dns_option(struct options *options, char *p[], const int msglevel, bool pull_mode)
+{
+    if (streq(p[1], "search-domains") && p[2])
+    {
+        dns_domain_list_append(&options->dns_options.search_domains, &p[2], &options->dns_options.gc);
+    }
+    else if (streq(p[1], "server") && p[2] && p[3] && p[4])
+    {
+        long priority;
+        if (!dns_server_priority_parse(&priority, p[2], pull_mode))
+        {
+            msg(msglevel, "--dns server: invalid priority value '%s'", p[2]);
+            return false;
+        }
+
+        struct dns_server *server = dns_server_get(&options->dns_options.servers, priority, &options->dns_options.gc);
+
+        if (streq(p[3], "address") && p[4])
+        {
+            for (int i = 4; p[i]; ++i)
+            {
+                if (!dns_server_addr_parse(server, p[i]))
+                {
+                    msg(msglevel, "--dns server %ld: malformed address or maximum exceeded '%s'", priority, p[i]);
+                    return false;
+                }
+            }
+        }
+        else if (streq(p[3], "resolve-domains"))
+        {
+            dns_domain_list_append(&server->domains, &p[4], &options->dns_options.gc);
+        }
+        else if (streq(p[3], "dnssec") && !p[5])
+        {
+            if (streq(p[4], "yes"))
+            {
+                server->dnssec = DNS_SECURITY_YES;
+            }
+            else if (streq(p[4], "no"))
+            {
+                server->dnssec = DNS_SECURITY_NO;
+            }
+            else if (streq(p[4], "optional"))
+            {
+                server->dnssec = DNS_SECURITY_OPTIONAL;
+            }
+            else
+            {
+                msg(msglevel, "--dns server %ld: malformed dnssec value '%s'", priority, p[4]);
+                return false;
+            }
+        }
+        else if (streq(p[3], "transport") && !p[5])
+        {
+            if (streq(p[4], "plain"))
+            {
+                server->transport = DNS_TRANSPORT_PLAIN;
+            }
+            else if (streq(p[4], "DoH"))
+            {
+                server->transport = DNS_TRANSPORT_HTTPS;
+            }
+            else if (streq(p[4], "DoT"))
+            {
+                server->transport = DNS_TRANSPORT_TLS;
+            }
+            else
+            {
+                msg(msglevel, "--dns server %ld: malformed transport value '%s'", priority, p[4]);
+                return false;
+            }
+        }
+        else if (streq(p[3], "sni") && !p[5])
+        {
+            server->sni = p[4];
+        }
+        else
+        {
+            msg(msglevel, "--dns server %ld: unknown option type '%s' or missing or unknown parameter", priority, p[3]);
+            return false;
+        }
+    }
+    else
+    {
+        msg(msglevel, "--dns: unknown option type '%s' or missing or unknown parameter", p[1]);
+        return false;
+    }
+    return true;
+}
+
+/**
+ * @brief Processes an option to update. It first checks whether it has already
+ *        received an option of the same type within the same update message.
+ *        If the option has already been received, it calls add_option().
+ *        Otherwise, it deletes all existing values related to that option before calling add_option().
+ *
+ * @param c The context structure.
+ * @param options A pointer to the options structure.
+ * @param p An array of strings containing the options and their parameters.
+ * @param is_inline A boolean indicating if the option is inline.
+ * @param file The file where the function is called.
+ * @param line The line number where the function is called.
+ * @param level The level of the option.
+ * @param msglevel The message level for logging.
+ * @param permission_mask The permission mask used by VERIFY_PERMISSION().
+ * @param option_types_found A pointer to the variable where the flags corresponding to the options found are stored.
+ * @param es The environment set structure.
+ * @param update_options_found A pointer to the variable where the flags corresponding to the update options found are stored,
+ *                             used to check if an option of the same type has already been processed by update_option() within the same push-update message.
+ */
+static void
+update_option(struct context *c,
+              struct options *options,
+              char *p[],
+              bool is_inline,
+              const char *file,
+              int line,
+              const int level,
+              const int msglevel,
+              const unsigned int permission_mask,
+              unsigned int *option_types_found,
+              struct env_set *es,
+              unsigned int *update_options_found)
+{
+    const bool pull_mode = BOOL_CAST(permission_mask & OPT_P_PULL_MODE);
+    ASSERT(MAX_PARMS >= 7);
+
+    if (streq(p[0], "route") && p[1] && !p[5])
+    {
+        if (!(*update_options_found & OPT_P_U_ROUTE))
+        {
+            VERIFY_PERMISSION(OPT_P_ROUTE);
+            if (!check_route_option(options, p, msglevel, pull_mode))
+            {
+                goto err;
+            }
+            if (c->c1.route_list)
+            {
+                delete_routes_v4(c->c1.route_list, c->c1.tuntap,
+                                 ROUTE_OPTION_FLAGS(&c->options),
+                                 es, &c->net_ctx);
+                RESET_OPTION_ROUTES(options->routes, routes);
+            }
+            *update_options_found |= OPT_P_U_ROUTE;
+        }
+    }
+    else if (streq(p[0], "route-ipv6") && p[1] && !p[4])
+    {
+        if (!(*update_options_found & OPT_P_U_ROUTE6))
+        {
+            VERIFY_PERMISSION(OPT_P_ROUTE);
+            if (!check_route6_option(options, p, msglevel, pull_mode))
+            {
+                goto err;
+            }
+            if (c->c1.route_ipv6_list)
+            {
+                delete_routes_v6(c->c1.route_ipv6_list, c->c1.tuntap,
+                                 ROUTE_OPTION_FLAGS(&c->options),
+                                 es, &c->net_ctx);
+                RESET_OPTION_ROUTES(options->routes_ipv6, routes_ipv6);
+            }
+            *update_options_found |= OPT_P_U_ROUTE6;
+        }
+    }
+    else if (streq(p[0], "redirect-gateway") || streq(p[0], "redirect-private"))
+    {
+        if (!(*update_options_found & OPT_P_U_REDIR_GATEWAY))
+        {
+            VERIFY_PERMISSION(OPT_P_ROUTE);
+            if (options->routes)
+            {
+                options->routes->flags = 0;
+            }
+            if (options->routes_ipv6)
+            {
+                options->routes_ipv6->flags = 0;
+            }
+            *update_options_found |= OPT_P_U_REDIR_GATEWAY;
+        }
+    }
+    else if (streq(p[0], "dns") && p[1])
+    {
+        if (!(*update_options_found & OPT_P_U_DNS))
+        {
+            VERIFY_PERMISSION(OPT_P_DHCPDNS);
+            if (!check_dns_option(options, p, msglevel, pull_mode))
+            {
+                goto err;
+            }
+            gc_free(&options->dns_options.gc);
+            CLEAR(options->dns_options);
+            *update_options_found |= OPT_P_U_DNS;
+        }
+    }
+#if defined(_WIN32) || defined(TARGET_ANDROID)
+    else if (streq(p[0], "dhcp-option") && p[1] && !p[3])
+    {
+        if (!(*update_options_found & OPT_P_U_DHCP))
+        {
+            struct tuntap_options *o = &options->tuntap_options;
+            VERIFY_PERMISSION(OPT_P_DHCPDNS);
+
+            o->domain = NULL;
+            o->netbios_scope = NULL;
+            o->netbios_node_type = 0;
+            o->dns6_len = 0;
+            CLEAR(o->dns6);
+            o->dns_len = 0;
+            CLEAR(o->dns);
+            o->wins_len = 0;
+            CLEAR(o->wins);
+            o->ntp_len = 0;
+            CLEAR(o->ntp);
+            o->nbdd_len = 0;
+            CLEAR(o->nbdd);
+            while (o->domain_search_list_len-- > 0)
+            {
+                o->domain_search_list[o->domain_search_list_len] = NULL;
+            }
+            o->disable_nbt = 0;
+            o->dhcp_options = 0;
+#if defined(TARGET_ANDROID)
+            o->http_proxy_port = 0;
+            o->http_proxy = NULL;
+#endif
+            *update_options_found |= OPT_P_U_DHCP;
+        }
+    }
+#else /* if defined(_WIN32) || defined(TARGET_ANDROID) */
+    else if (streq(p[0], "dhcp-option") && p[1] && !p[3])
+    {
+        if (!(*update_options_found & OPT_P_U_DHCP))
+        {
+            VERIFY_PERMISSION(OPT_P_DHCPDNS);
+            delete_all_dhcp_fo(options, &es->list);
+            *update_options_found |= OPT_P_U_DHCP;
+        }
+    }
+#endif /* if defined(_WIN32) || defined(TARGET_ANDROID) */
+    add_option(options, p, is_inline, file, line,
+               level, msglevel, permission_mask,
+               option_types_found, es);
+    return;
+err:
+    msg(msglevel, "Error occurred trying to update %s option", p[0]);
+}
+
 static void
 set_user_script(struct options *options,
                 const char **script,
@@ -7284,45 +7600,18 @@
     else if (streq(p[0], "route") && p[1] && !p[5])
     {
         VERIFY_PERMISSION(OPT_P_ROUTE);
-        rol_check_alloc(options);
-        if (pull_mode)
+        if (!check_route_option(options, p, msglevel, pull_mode))
         {
-            if (!ip_or_dns_addr_safe(p[1], options->allow_pull_fqdn) && !is_special_addr(p[1])) /* FQDN -- may be DNS name */
-            {
-                msg(msglevel, "route parameter network/IP '%s' must be a valid address", p[1]);
-                goto err;
-            }
-            if (p[2] && !ip_addr_dotted_quad_safe(p[2])) /* FQDN -- must be IP address */
-            {
-                msg(msglevel, "route parameter netmask '%s' must be an IP address", p[2]);
-                goto err;
-            }
-            if (p[3] && !ip_or_dns_addr_safe(p[3], options->allow_pull_fqdn) && !is_special_addr(p[3])) /* FQDN -- may be DNS name */
-            {
-                msg(msglevel, "route parameter gateway '%s' must be a valid address", p[3]);
-                goto err;
-            }
-            /* p[4] is metric, if specified */
+            goto err;
         }
         add_route_to_option_list(options->routes, p[1], p[2], p[3], p[4], options->route_default_table_id);
     }
     else if (streq(p[0], "route-ipv6") && p[1] && !p[4])
     {
         VERIFY_PERMISSION(OPT_P_ROUTE);
-        rol6_check_alloc(options);
-        if (pull_mode)
+        if (!check_route6_option(options, p, msglevel, pull_mode))
         {
-            if (!ipv6_addr_safe_hexplusbits(p[1]))
-            {
-                msg(msglevel, "route-ipv6 parameter network/IP '%s' must be a valid address", p[1]);
-                goto err;
-            }
-            if (p[2] && !ipv6_addr_safe(p[2]))
-            {
-                msg(msglevel, "route-ipv6 parameter gateway '%s' must be a valid address", p[2]);
-                goto err;
-            }
-            /* p[3] is metric, if specified */
+            goto err;
         }
         add_route_ipv6_to_option_list(options->routes_ipv6, p[1], p[2], p[3], options->route_default_table_id);
     }
@@ -8410,90 +8699,8 @@
     else if (streq(p[0], "dns") && p[1])
     {
         VERIFY_PERMISSION(OPT_P_DHCPDNS);
-
-        if (streq(p[1], "search-domains") && p[2])
+        if (!check_dns_option(options, p, msglevel, pull_mode))
         {
-            dns_domain_list_append(&options->dns_options.search_domains, &p[2], &options->dns_options.gc);
-        }
-        else if (streq(p[1], "server") && p[2] && p[3] && p[4])
-        {
-            long priority;
-            if (!dns_server_priority_parse(&priority, p[2], pull_mode))
-            {
-                msg(msglevel, "--dns server: invalid priority value '%s'", p[2]);
-                goto err;
-            }
-
-            struct dns_server *server = dns_server_get(&options->dns_options.servers, priority, &options->dns_options.gc);
-
-            if (streq(p[3], "address") && p[4])
-            {
-                for (int i = 4; p[i]; ++i)
-                {
-                    if (!dns_server_addr_parse(server, p[i]))
-                    {
-                        msg(msglevel, "--dns server %ld: malformed address or maximum exceeded '%s'", priority, p[i]);
-                        goto err;
-                    }
-                }
-            }
-            else if (streq(p[3], "resolve-domains"))
-            {
-                dns_domain_list_append(&server->domains, &p[4], &options->dns_options.gc);
-            }
-            else if (streq(p[3], "dnssec") && !p[5])
-            {
-                if (streq(p[4], "yes"))
-                {
-                    server->dnssec = DNS_SECURITY_YES;
-                }
-                else if (streq(p[4], "no"))
-                {
-                    server->dnssec = DNS_SECURITY_NO;
-                }
-                else if (streq(p[4], "optional"))
-                {
-                    server->dnssec = DNS_SECURITY_OPTIONAL;
-                }
-                else
-                {
-                    msg(msglevel, "--dns server %ld: malformed dnssec value '%s'", priority, p[4]);
-                    goto err;
-                }
-            }
-            else if (streq(p[3], "transport") && !p[5])
-            {
-                if (streq(p[4], "plain"))
-                {
-                    server->transport = DNS_TRANSPORT_PLAIN;
-                }
-                else if (streq(p[4], "DoH"))
-                {
-                    server->transport = DNS_TRANSPORT_HTTPS;
-                }
-                else if (streq(p[4], "DoT"))
-                {
-                    server->transport = DNS_TRANSPORT_TLS;
-                }
-                else
-                {
-                    msg(msglevel, "--dns server %ld: malformed transport value '%s'", priority, p[4]);
-                    goto err;
-                }
-            }
-            else if (streq(p[3], "sni") && !p[5])
-            {
-                server->sni = p[4];
-            }
-            else
-            {
-                msg(msglevel, "--dns server %ld: unknown option type '%s' or missing or unknown parameter", priority, p[3]);
-                goto err;
-            }
-        }
-        else
-        {
-            msg(msglevel, "--dns: unknown option type '%s' or missing or unknown parameter", p[1]);
             goto err;
         }
     }

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/810?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Ia45c99e6df7b3ad24020c10b8a9b3577984ecdc2
Gerrit-Change-Number: 810
Gerrit-PatchSet: 24
Gerrit-Owner: mrbff <ma...@ma...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-Reviewer: stipa <lst...@gm...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-MessageType: newpatchset

[Openvpn-devel] [L] Change in openvpn[master]: PUSH_UPDATE: Added update_option() function.

[cron2 (C. Review) <ge...@op...>]

cron2 has submitted this change. ( http://gerrit.openvpn.net/c/openvpn/+/810?usp=email )

Change subject: PUSH_UPDATE: Added update_option() function.
......................................................................

PUSH_UPDATE: Added update_option() function.

When the function receives an option to update, it first checks whether it has
already received an option of the same type within the same update message.
If it has already received it, it simply calls add_option(), otherwise it
deletes all the values already present for that option first.

Change-Id: Ia45c99e6df7b3ad24020c10b8a9b3577984ecdc2
Signed-off-by: Marco Baffo <ma...@ma...>
Acked-by: Gert Doering <ge...@gr...>
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg32408.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M src/openvpn/options.c
1 file changed, 331 insertions(+), 124 deletions(-)




diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 469360d..b7328d7 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -5360,6 +5360,20 @@
               struct env_set *es);
 
 static void
+update_option(struct context *c,
+              struct options *options,
+              char *p[],
+              bool is_inline,
+              const char *file,
+              int line,
+              const int level,
+              const int msglevel,
+              const unsigned int permission_mask,
+              unsigned int *option_types_found,
+              struct env_set *es,
+              unsigned int *update_options_found);
+
+static void
 read_config_file(struct options *options,
                  const char *file,
                  int level,
@@ -5545,6 +5559,7 @@
     int line_num = 0;
     const char *file = "[PUSH-OPTIONS]";
     const int msglevel = D_PUSH_ERRORS|M_OPTERR;
+    unsigned int update_options_found = 0;
 
     while (buf_parse(buf, ',', line, sizeof(line)))
     {
@@ -5590,6 +5605,11 @@
                 remove_option(c, options, p, false, file, line_num, msglevel,
                               permission_mask, option_types_found, es);
             }
+            else
+            {
+                update_option(c, options, p, false, file, line_num, 0, msglevel,
+                              permission_mask, option_types_found, es, &update_options_found);
+            }
         }
     }
     return true;
@@ -5728,6 +5748,13 @@
     return options->forward_compatible ? M_WARN : msglevel;
 }
 
+#define RESET_OPTION_ROUTES(option_ptr, field) \
+    if (option_ptr)                        \
+    {                                      \
+        option_ptr->field = NULL;          \
+        option_ptr->flags = 0;             \
+    }
+
 /**
  * @brief Resets options found in the PUSH_UPDATE message that are preceded by the `-` flag.
  *        This function is used in push-updates to reset specified options.
@@ -5782,11 +5809,7 @@
             delete_routes_v4(c->c1.route_list, c->c1.tuntap,
                              ROUTE_OPTION_FLAGS(&c->options),
                              es, &c->net_ctx);
-            if (options->routes)
-            {
-                options->routes->routes = NULL;
-                options->routes->flags = 0;
-            }
+            RESET_OPTION_ROUTES(options->routes, routes);
         }
     }
     else if (streq(p[0], "route-ipv6") && !p[1])
@@ -5797,11 +5820,7 @@
             delete_routes_v6(c->c1.route_ipv6_list, c->c1.tuntap,
                              ROUTE_OPTION_FLAGS(&c->options),
                              es, &c->net_ctx);
-            if (options->routes_ipv6)
-            {
-                options->routes_ipv6->routes_ipv6 = NULL;
-                options->routes_ipv6->flags = 0;
-            }
+            RESET_OPTION_ROUTES(options->routes_ipv6, routes_ipv6);
         }
     }
     else if (streq(p[0], "route-gateway") && !p[1])
@@ -5921,6 +5940,303 @@
     msg(msglevel, "Error occurred trying to remove %s option", p[0]);
 }
 
+
+static bool
+check_route_option(struct options *options, char *p[], const int msglevel, bool pull_mode)
+{
+    rol_check_alloc(options);
+    if (pull_mode)
+    {
+        if (!ip_or_dns_addr_safe(p[1], options->allow_pull_fqdn) && !is_special_addr(p[1])) /* FQDN -- may be DNS name */
+        {
+            msg(msglevel, "route parameter network/IP '%s' must be a valid address", p[1]);
+            return false;
+        }
+        if (p[2] && !ip_addr_dotted_quad_safe(p[2])) /* FQDN -- must be IP address */
+        {
+            msg(msglevel, "route parameter netmask '%s' must be an IP address", p[2]);
+            return false;
+        }
+        if (p[3] && !ip_or_dns_addr_safe(p[3], options->allow_pull_fqdn) && !is_special_addr(p[3])) /* FQDN -- may be DNS name */
+        {
+            msg(msglevel, "route parameter gateway '%s' must be a valid address", p[3]);
+            return false;
+        }
+    }
+    return true;
+}
+
+
+static bool
+check_route6_option(struct options *options, char *p[], const int msglevel, bool pull_mode)
+{
+    rol6_check_alloc(options);
+    if (pull_mode)
+    {
+        if (!ipv6_addr_safe_hexplusbits(p[1]))
+        {
+            msg(msglevel, "route-ipv6 parameter network/IP '%s' must be a valid address", p[1]);
+            return false;
+        }
+        if (p[2] && !ipv6_addr_safe(p[2]))
+        {
+            msg(msglevel, "route-ipv6 parameter gateway '%s' must be a valid address", p[2]);
+            return false;
+        }
+        /* p[3] is metric, if present */
+    }
+    return true;
+}
+
+static bool
+check_dns_option(struct options *options, char *p[], const int msglevel, bool pull_mode)
+{
+    if (streq(p[1], "search-domains") && p[2])
+    {
+        dns_domain_list_append(&options->dns_options.search_domains, &p[2], &options->dns_options.gc);
+    }
+    else if (streq(p[1], "server") && p[2] && p[3] && p[4])
+    {
+        long priority;
+        if (!dns_server_priority_parse(&priority, p[2], pull_mode))
+        {
+            msg(msglevel, "--dns server: invalid priority value '%s'", p[2]);
+            return false;
+        }
+
+        struct dns_server *server = dns_server_get(&options->dns_options.servers, priority, &options->dns_options.gc);
+
+        if (streq(p[3], "address") && p[4])
+        {
+            for (int i = 4; p[i]; ++i)
+            {
+                if (!dns_server_addr_parse(server, p[i]))
+                {
+                    msg(msglevel, "--dns server %ld: malformed address or maximum exceeded '%s'", priority, p[i]);
+                    return false;
+                }
+            }
+        }
+        else if (streq(p[3], "resolve-domains"))
+        {
+            dns_domain_list_append(&server->domains, &p[4], &options->dns_options.gc);
+        }
+        else if (streq(p[3], "dnssec") && !p[5])
+        {
+            if (streq(p[4], "yes"))
+            {
+                server->dnssec = DNS_SECURITY_YES;
+            }
+            else if (streq(p[4], "no"))
+            {
+                server->dnssec = DNS_SECURITY_NO;
+            }
+            else if (streq(p[4], "optional"))
+            {
+                server->dnssec = DNS_SECURITY_OPTIONAL;
+            }
+            else
+            {
+                msg(msglevel, "--dns server %ld: malformed dnssec value '%s'", priority, p[4]);
+                return false;
+            }
+        }
+        else if (streq(p[3], "transport") && !p[5])
+        {
+            if (streq(p[4], "plain"))
+            {
+                server->transport = DNS_TRANSPORT_PLAIN;
+            }
+            else if (streq(p[4], "DoH"))
+            {
+                server->transport = DNS_TRANSPORT_HTTPS;
+            }
+            else if (streq(p[4], "DoT"))
+            {
+                server->transport = DNS_TRANSPORT_TLS;
+            }
+            else
+            {
+                msg(msglevel, "--dns server %ld: malformed transport value '%s'", priority, p[4]);
+                return false;
+            }
+        }
+        else if (streq(p[3], "sni") && !p[5])
+        {
+            server->sni = p[4];
+        }
+        else
+        {
+            msg(msglevel, "--dns server %ld: unknown option type '%s' or missing or unknown parameter", priority, p[3]);
+            return false;
+        }
+    }
+    else
+    {
+        msg(msglevel, "--dns: unknown option type '%s' or missing or unknown parameter", p[1]);
+        return false;
+    }
+    return true;
+}
+
+/**
+ * @brief Processes an option to update. It first checks whether it has already
+ *        received an option of the same type within the same update message.
+ *        If the option has already been received, it calls add_option().
+ *        Otherwise, it deletes all existing values related to that option before calling add_option().
+ *
+ * @param c The context structure.
+ * @param options A pointer to the options structure.
+ * @param p An array of strings containing the options and their parameters.
+ * @param is_inline A boolean indicating if the option is inline.
+ * @param file The file where the function is called.
+ * @param line The line number where the function is called.
+ * @param level The level of the option.
+ * @param msglevel The message level for logging.
+ * @param permission_mask The permission mask used by VERIFY_PERMISSION().
+ * @param option_types_found A pointer to the variable where the flags corresponding to the options found are stored.
+ * @param es The environment set structure.
+ * @param update_options_found A pointer to the variable where the flags corresponding to the update options found are stored,
+ *                             used to check if an option of the same type has already been processed by update_option() within the same push-update message.
+ */
+static void
+update_option(struct context *c,
+              struct options *options,
+              char *p[],
+              bool is_inline,
+              const char *file,
+              int line,
+              const int level,
+              const int msglevel,
+              const unsigned int permission_mask,
+              unsigned int *option_types_found,
+              struct env_set *es,
+              unsigned int *update_options_found)
+{
+    const bool pull_mode = BOOL_CAST(permission_mask & OPT_P_PULL_MODE);
+    ASSERT(MAX_PARMS >= 7);
+
+    if (streq(p[0], "route") && p[1] && !p[5])
+    {
+        if (!(*update_options_found & OPT_P_U_ROUTE))
+        {
+            VERIFY_PERMISSION(OPT_P_ROUTE);
+            if (!check_route_option(options, p, msglevel, pull_mode))
+            {
+                goto err;
+            }
+            if (c->c1.route_list)
+            {
+                delete_routes_v4(c->c1.route_list, c->c1.tuntap,
+                                 ROUTE_OPTION_FLAGS(&c->options),
+                                 es, &c->net_ctx);
+                RESET_OPTION_ROUTES(options->routes, routes);
+            }
+            *update_options_found |= OPT_P_U_ROUTE;
+        }
+    }
+    else if (streq(p[0], "route-ipv6") && p[1] && !p[4])
+    {
+        if (!(*update_options_found & OPT_P_U_ROUTE6))
+        {
+            VERIFY_PERMISSION(OPT_P_ROUTE);
+            if (!check_route6_option(options, p, msglevel, pull_mode))
+            {
+                goto err;
+            }
+            if (c->c1.route_ipv6_list)
+            {
+                delete_routes_v6(c->c1.route_ipv6_list, c->c1.tuntap,
+                                 ROUTE_OPTION_FLAGS(&c->options),
+                                 es, &c->net_ctx);
+                RESET_OPTION_ROUTES(options->routes_ipv6, routes_ipv6);
+            }
+            *update_options_found |= OPT_P_U_ROUTE6;
+        }
+    }
+    else if (streq(p[0], "redirect-gateway") || streq(p[0], "redirect-private"))
+    {
+        if (!(*update_options_found & OPT_P_U_REDIR_GATEWAY))
+        {
+            VERIFY_PERMISSION(OPT_P_ROUTE);
+            if (options->routes)
+            {
+                options->routes->flags = 0;
+            }
+            if (options->routes_ipv6)
+            {
+                options->routes_ipv6->flags = 0;
+            }
+            *update_options_found |= OPT_P_U_REDIR_GATEWAY;
+        }
+    }
+    else if (streq(p[0], "dns") && p[1])
+    {
+        if (!(*update_options_found & OPT_P_U_DNS))
+        {
+            VERIFY_PERMISSION(OPT_P_DHCPDNS);
+            if (!check_dns_option(options, p, msglevel, pull_mode))
+            {
+                goto err;
+            }
+            gc_free(&options->dns_options.gc);
+            CLEAR(options->dns_options);
+            *update_options_found |= OPT_P_U_DNS;
+        }
+    }
+#if defined(_WIN32) || defined(TARGET_ANDROID)
+    else if (streq(p[0], "dhcp-option") && p[1] && !p[3])
+    {
+        if (!(*update_options_found & OPT_P_U_DHCP))
+        {
+            struct tuntap_options *o = &options->tuntap_options;
+            VERIFY_PERMISSION(OPT_P_DHCPDNS);
+
+            o->domain = NULL;
+            o->netbios_scope = NULL;
+            o->netbios_node_type = 0;
+            o->dns6_len = 0;
+            CLEAR(o->dns6);
+            o->dns_len = 0;
+            CLEAR(o->dns);
+            o->wins_len = 0;
+            CLEAR(o->wins);
+            o->ntp_len = 0;
+            CLEAR(o->ntp);
+            o->nbdd_len = 0;
+            CLEAR(o->nbdd);
+            while (o->domain_search_list_len-- > 0)
+            {
+                o->domain_search_list[o->domain_search_list_len] = NULL;
+            }
+            o->disable_nbt = 0;
+            o->dhcp_options = 0;
+#if defined(TARGET_ANDROID)
+            o->http_proxy_port = 0;
+            o->http_proxy = NULL;
+#endif
+            *update_options_found |= OPT_P_U_DHCP;
+        }
+    }
+#else /* if defined(_WIN32) || defined(TARGET_ANDROID) */
+    else if (streq(p[0], "dhcp-option") && p[1] && !p[3])
+    {
+        if (!(*update_options_found & OPT_P_U_DHCP))
+        {
+            VERIFY_PERMISSION(OPT_P_DHCPDNS);
+            delete_all_dhcp_fo(options, &es->list);
+            *update_options_found |= OPT_P_U_DHCP;
+        }
+    }
+#endif /* if defined(_WIN32) || defined(TARGET_ANDROID) */
+    add_option(options, p, is_inline, file, line,
+               level, msglevel, permission_mask,
+               option_types_found, es);
+    return;
+err:
+    msg(msglevel, "Error occurred trying to update %s option", p[0]);
+}
+
 static void
 set_user_script(struct options *options,
                 const char **script,
@@ -7284,45 +7600,18 @@
     else if (streq(p[0], "route") && p[1] && !p[5])
     {
         VERIFY_PERMISSION(OPT_P_ROUTE);
-        rol_check_alloc(options);
-        if (pull_mode)
+        if (!check_route_option(options, p, msglevel, pull_mode))
         {
-            if (!ip_or_dns_addr_safe(p[1], options->allow_pull_fqdn) && !is_special_addr(p[1])) /* FQDN -- may be DNS name */
-            {
-                msg(msglevel, "route parameter network/IP '%s' must be a valid address", p[1]);
-                goto err;
-            }
-            if (p[2] && !ip_addr_dotted_quad_safe(p[2])) /* FQDN -- must be IP address */
-            {
-                msg(msglevel, "route parameter netmask '%s' must be an IP address", p[2]);
-                goto err;
-            }
-            if (p[3] && !ip_or_dns_addr_safe(p[3], options->allow_pull_fqdn) && !is_special_addr(p[3])) /* FQDN -- may be DNS name */
-            {
-                msg(msglevel, "route parameter gateway '%s' must be a valid address", p[3]);
-                goto err;
-            }
-            /* p[4] is metric, if specified */
+            goto err;
         }
         add_route_to_option_list(options->routes, p[1], p[2], p[3], p[4], options->route_default_table_id);
     }
     else if (streq(p[0], "route-ipv6") && p[1] && !p[4])
     {
         VERIFY_PERMISSION(OPT_P_ROUTE);
-        rol6_check_alloc(options);
-        if (pull_mode)
+        if (!check_route6_option(options, p, msglevel, pull_mode))
         {
-            if (!ipv6_addr_safe_hexplusbits(p[1]))
-            {
-                msg(msglevel, "route-ipv6 parameter network/IP '%s' must be a valid address", p[1]);
-                goto err;
-            }
-            if (p[2] && !ipv6_addr_safe(p[2]))
-            {
-                msg(msglevel, "route-ipv6 parameter gateway '%s' must be a valid address", p[2]);
-                goto err;
-            }
-            /* p[3] is metric, if specified */
+            goto err;
         }
         add_route_ipv6_to_option_list(options->routes_ipv6, p[1], p[2], p[3], options->route_default_table_id);
     }
@@ -8410,90 +8699,8 @@
     else if (streq(p[0], "dns") && p[1])
     {
         VERIFY_PERMISSION(OPT_P_DHCPDNS);
-
-        if (streq(p[1], "search-domains") && p[2])
+        if (!check_dns_option(options, p, msglevel, pull_mode))
         {
-            dns_domain_list_append(&options->dns_options.search_domains, &p[2], &options->dns_options.gc);
-        }
-        else if (streq(p[1], "server") && p[2] && p[3] && p[4])
-        {
-            long priority;
-            if (!dns_server_priority_parse(&priority, p[2], pull_mode))
-            {
-                msg(msglevel, "--dns server: invalid priority value '%s'", p[2]);
-                goto err;
-            }
-
-            struct dns_server *server = dns_server_get(&options->dns_options.servers, priority, &options->dns_options.gc);
-
-            if (streq(p[3], "address") && p[4])
-            {
-                for (int i = 4; p[i]; ++i)
-                {
-                    if (!dns_server_addr_parse(server, p[i]))
-                    {
-                        msg(msglevel, "--dns server %ld: malformed address or maximum exceeded '%s'", priority, p[i]);
-                        goto err;
-                    }
-                }
-            }
-            else if (streq(p[3], "resolve-domains"))
-            {
-                dns_domain_list_append(&server->domains, &p[4], &options->dns_options.gc);
-            }
-            else if (streq(p[3], "dnssec") && !p[5])
-            {
-                if (streq(p[4], "yes"))
-                {
-                    server->dnssec = DNS_SECURITY_YES;
-                }
-                else if (streq(p[4], "no"))
-                {
-                    server->dnssec = DNS_SECURITY_NO;
-                }
-                else if (streq(p[4], "optional"))
-                {
-                    server->dnssec = DNS_SECURITY_OPTIONAL;
-                }
-                else
-                {
-                    msg(msglevel, "--dns server %ld: malformed dnssec value '%s'", priority, p[4]);
-                    goto err;
-                }
-            }
-            else if (streq(p[3], "transport") && !p[5])
-            {
-                if (streq(p[4], "plain"))
-                {
-                    server->transport = DNS_TRANSPORT_PLAIN;
-                }
-                else if (streq(p[4], "DoH"))
-                {
-                    server->transport = DNS_TRANSPORT_HTTPS;
-                }
-                else if (streq(p[4], "DoT"))
-                {
-                    server->transport = DNS_TRANSPORT_TLS;
-                }
-                else
-                {
-                    msg(msglevel, "--dns server %ld: malformed transport value '%s'", priority, p[4]);
-                    goto err;
-                }
-            }
-            else if (streq(p[3], "sni") && !p[5])
-            {
-                server->sni = p[4];
-            }
-            else
-            {
-                msg(msglevel, "--dns server %ld: unknown option type '%s' or missing or unknown parameter", priority, p[3]);
-                goto err;
-            }
-        }
-        else
-        {
-            msg(msglevel, "--dns: unknown option type '%s' or missing or unknown parameter", p[1]);
             goto err;
         }
     }

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/810?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Ia45c99e6df7b3ad24020c10b8a9b3577984ecdc2
Gerrit-Change-Number: 810
Gerrit-PatchSet: 24
Gerrit-Owner: mrbff <ma...@ma...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-Reviewer: stipa <lst...@gm...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-MessageType: merged

[Openvpn-devel] [L] Change in openvpn[master]: PUSH_UPDATE: Added remove_option() and do_update().

[cron2 (C. Review) <ge...@op...>]

cron2 has submitted this change. ( http://gerrit.openvpn.net/c/openvpn/+/809?usp=email )

Change subject: PUSH_UPDATE: Added remove_option() and do_update().
......................................................................

PUSH_UPDATE: Added remove_option() and do_update().

* Added remove_option() function and some utility functions to remove options at
  runtime following the push-update logic.
* Added do_update() function to close and reopen the tun and apply option updates.

Change-Id: I507180d7397b6959844a30908010132bc3411067
Signed-off-by: Marco Baffo <ma...@ma...>
Acked-by: Gert Doering <ge...@gr...>
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg32407.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M src/openvpn/init.c
M src/openvpn/init.h
M src/openvpn/multi.c
M src/openvpn/options.c
M src/openvpn/push.c
M src/openvpn/route.c
M src/openvpn/route.h
7 files changed, 350 insertions(+), 11 deletions(-)




diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index ba1dda4..3254cc6 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -2470,7 +2470,7 @@
 
         if (pulled_options)
         {
-            if (!do_deferred_options(c, option_types_found))
+            if (!do_deferred_options(c, option_types_found, false))
             {
                 msg(D_PUSH_ERRORS, "ERROR: Failed to apply push options");
                 return false;
@@ -2594,6 +2594,55 @@
     return true;
 }
 
+bool
+do_update(struct context *c, unsigned int option_types_found)
+{
+    /* Not necessary since to receive the update the openvpn
+     * instance must be up and running but just in case
+     */
+    if (!c->c2.do_up_ran)
+    {
+        return false;
+    }
+
+    bool tt_dco_win = tuntap_is_dco_win(c->c1.tuntap);
+    if (tt_dco_win)
+    {
+        msg(M_NONFATAL, "dco-win doesn't yet support reopening TUN device");
+        return false;
+    }
+
+    if (!do_deferred_options(c, option_types_found, true))
+    {
+        msg(D_PUSH_ERRORS, "ERROR: Failed to apply push options");
+        return false;
+    }
+
+    do_close_tun(c, true);
+
+    management_sleep(1);
+    int error_flags = 0;
+    c->c2.did_open_tun = do_open_tun(c, &error_flags);
+    update_time();
+
+    if (c->c2.did_open_tun)
+    {
+        /* if --route-delay was specified, start timer */
+        if ((route_order(c->c1.tuntap) == ROUTE_AFTER_TUN) && c->options.route_delay_defined)
+        {
+            event_timeout_init(&c->c2.route_wakeup, c->options.route_delay, now);
+            event_timeout_init(&c->c2.route_wakeup_expire, c->options.route_delay + c->options.route_delay_window, now);
+            tun_standby_init(c->c1.tuntap);
+        }
+
+        initialization_sequence_completed(c, error_flags);
+    }
+
+    CLEAR(c->c1.pulled_options_digest_save);
+
+    return true;
+}
+
 /*
  * These are the option categories which will be accepted by pull.
  */
@@ -2672,11 +2721,8 @@
     return true;
 }
 
-/*
- * Handle non-tun-related pulled options.
- */
 bool
-do_deferred_options(struct context *c, const unsigned int found)
+do_deferred_options(struct context *c, const unsigned int found, const bool is_update)
 {
     if (found & OPT_P_MESSAGES)
     {
@@ -2784,7 +2830,10 @@
     /* process (potentially) pushed options */
     if (c->options.pull)
     {
-        if (!check_pull_client_ncp(c, found))
+        /* On PUSH_UPDATE, NCP related flags are never updated, and so the code
+         * would assume "no cipher pushed = NCP failed" - so, don't call it on
+         * updates */
+        if (!is_update && !check_pull_client_ncp(c, found))
         {
             return false;
         }
diff --git a/src/openvpn/init.h b/src/openvpn/init.h
index 5c6b9c1..25078a6 100644
--- a/src/openvpn/init.h
+++ b/src/openvpn/init.h
@@ -86,13 +86,29 @@
            bool pulled_options,
            unsigned int option_types_found);
 
+/**
+ * @brief A simplified version of the do_up() function. This function is called
+ *        after receiving a successful PUSH_UPDATE message. It closes and reopens
+ *        the TUN device to apply the updated options.
+ *
+ * @param c The context structure.
+ * @param option_types_found The options found in the PUSH_UPDATE message.
+ * @return true on success.
+ * @return false on error.
+ */
+bool do_update(struct context *c, unsigned int option_types_found);
+
 unsigned int pull_permission_mask(const struct context *c);
 
 const char *format_common_name(struct context *c, struct gc_arena *gc);
 
 void reset_coarse_timers(struct context *c);
 
-bool do_deferred_options(struct context *c, const unsigned int found);
+/*
+ * Handle non-tun-related pulled options.
+ * Set `is_update` param to true to skip NCP check.
+ */
+bool do_deferred_options(struct context *c, const unsigned int found, const bool is_update);
 
 void inherit_context_child(struct context *dest,
                            const struct context *src,
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index b2d2b6c..68b4da6 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -2424,7 +2424,7 @@
     /*
      * Process sourced options.
      */
-    do_deferred_options(&mi->context, option_types_found);
+    do_deferred_options(&mi->context, option_types_found, false);
 
     /*
      * make sure we got ifconfig settings from somewhere
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index eb53a60..469360d 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -1077,6 +1077,40 @@
     }
     gc_free(&gc);
 }
+
+static void
+delete_all_dhcp_fo(struct options *o, struct env_item **list)
+{
+    struct env_item *current, *prev;
+
+    ASSERT(list);
+
+    for (current = *list, prev = NULL; current != NULL; current = current->next)
+    {
+        char *tmp_value = NULL;
+        if (!strncmp(current->string, "foreign_option_", sizeof("foreign_option_")-1))
+        {
+            tmp_value = strchr(current->string, '=');
+            if (tmp_value && ++tmp_value)
+            {
+                if (!strncmp(tmp_value, "dhcp-option ", sizeof("dhcp-option ")-1))
+                {
+                    if (prev)
+                    {
+                        prev->next = current->next;
+                    }
+                    else
+                    {
+                        *list = current->next;
+                    }
+                    o->foreign_option_index--;
+                }
+            }
+        }
+        prev = current;
+    }
+}
+
 #endif /* ifndef _WIN32 */
 
 static in_addr_t
@@ -3082,7 +3116,7 @@
         opt->routes->flags |= RG_DEF1;
     }
 }
-#endif
+#endif /* ifdef _WIN32 */
 
 /*
  * Save/Restore certain option defaults before --pull is applied.
@@ -5314,6 +5348,18 @@
            struct env_set *es);
 
 static void
+remove_option(struct context *c,
+              struct options *options,
+              char *p[],
+              bool is_inline,
+              const char *file,
+              int line,
+              const int msglevel,
+              const unsigned int permission_mask,
+              unsigned int *option_types_found,
+              struct env_set *es);
+
+static void
 read_config_file(struct options *options,
                  const char *file,
                  int level,
@@ -5539,6 +5585,11 @@
                 add_option(options, p, false, file, line_num, 0, msglevel,
                            permission_mask, option_types_found, es);
             }
+            else if (push_update_option_flags & PUSH_OPT_TO_REMOVE)
+            {
+                remove_option(c, options, p, false, file, line_num, msglevel,
+                              permission_mask, option_types_found, es);
+            }
         }
     }
     return true;
@@ -5677,6 +5728,199 @@
     return options->forward_compatible ? M_WARN : msglevel;
 }
 
+/**
+ * @brief Resets options found in the PUSH_UPDATE message that are preceded by the `-` flag.
+ *        This function is used in push-updates to reset specified options.
+ *        The number of parameters `p` must always be 1. If the permission is verified,
+ *        all related options are erased or reset to their default values.
+ *        Upon successful permission verification (by VERIFY_PERMISSION()),
+ *        `option_types_found` is filled with the flag corresponding to the option.
+ *
+ * @param c The context structure.
+ * @param options A pointer to the options structure.
+ * @param p An array of strings containing the options and their parameters.
+ * @param is_inline A boolean indicating if the option is inline.
+ * @param file The file where the function is called.
+ * @param line The line number where the function is called.
+ * @param msglevel The message level.
+ * @param permission_mask The permission mask used by VERIFY_PERMISSION().
+ * @param option_types_found A pointer to the variable where the flags corresponding to the options found are stored.
+ * @param es The environment set structure.
+ */
+static void
+remove_option(struct context *c,
+              struct options *options,
+              char *p[],
+              bool is_inline,
+              const char *file,
+              int line,
+              const int msglevel,
+              const unsigned int permission_mask,
+              unsigned int *option_types_found,
+              struct env_set *es)
+{
+    int msglevel_fc = msglevel_forward_compatible(options, msglevel);
+
+    if (streq(p[0], "ifconfig") && !p[1])
+    {
+        VERIFY_PERMISSION(OPT_P_UP);
+        options->ifconfig_local = NULL;
+        options->ifconfig_remote_netmask = NULL;
+    }
+    else if (streq(p[0], "ifconfig-ipv6") && !p[1])
+    {
+        VERIFY_PERMISSION(OPT_P_UP);
+        options->ifconfig_ipv6_local = NULL;
+        options->ifconfig_ipv6_netbits = 0;
+        options->ifconfig_ipv6_remote = NULL;
+    }
+    else if (streq(p[0], "route") && !p[1])
+    {
+        VERIFY_PERMISSION(OPT_P_ROUTE);
+        if (c->c1.route_list)
+        {
+            delete_routes_v4(c->c1.route_list, c->c1.tuntap,
+                             ROUTE_OPTION_FLAGS(&c->options),
+                             es, &c->net_ctx);
+            if (options->routes)
+            {
+                options->routes->routes = NULL;
+                options->routes->flags = 0;
+            }
+        }
+    }
+    else if (streq(p[0], "route-ipv6") && !p[1])
+    {
+        VERIFY_PERMISSION(OPT_P_ROUTE);
+        if (c->c1.route_ipv6_list)
+        {
+            delete_routes_v6(c->c1.route_ipv6_list, c->c1.tuntap,
+                             ROUTE_OPTION_FLAGS(&c->options),
+                             es, &c->net_ctx);
+            if (options->routes_ipv6)
+            {
+                options->routes_ipv6->routes_ipv6 = NULL;
+                options->routes_ipv6->flags = 0;
+            }
+        }
+    }
+    else if (streq(p[0], "route-gateway") && !p[1])
+    {
+        VERIFY_PERMISSION(OPT_P_ROUTE_EXTRAS);
+        options->route_gateway_via_dhcp = false;
+        options->route_default_gateway = NULL;
+    }
+    else if (streq(p[0], "route-metric") && !p[1])
+    {
+        VERIFY_PERMISSION(OPT_P_ROUTE);
+        options->route_default_metric = 0;
+    }
+    else if (streq(p[0], "push-continuation") && !p[1])
+    {
+        VERIFY_PERMISSION(OPT_P_PULL_MODE);
+        options->push_continuation = 0;
+    }
+    else if ((streq(p[0], "redirect-gateway") || streq(p[0], "redirect-private")) && !p[1])
+    {
+        VERIFY_PERMISSION(OPT_P_ROUTE);
+        if (options->routes)
+        {
+            options->routes->flags = 0;
+        }
+        if (options->routes_ipv6)
+        {
+            options->routes_ipv6->flags = 0;
+        }
+    }
+    else if (streq(p[0], "dns") && !p[1])
+    {
+        VERIFY_PERMISSION(OPT_P_DHCPDNS);
+        gc_free(&options->dns_options.gc);
+        CLEAR(options->dns_options);
+    }
+    else if (streq(p[0], "topology") && !p[1])
+    {
+        VERIFY_PERMISSION(OPT_P_UP);
+        options->topology = TOP_UNDEF;
+        helper_setdefault_topology(options);
+    }
+    else if (streq(p[0], "tun-mtu") && !p[1])
+    {
+        VERIFY_PERMISSION(OPT_P_PUSH_MTU|OPT_P_CONNECTION);
+        options->ce.tun_mtu = TUN_MTU_DEFAULT;
+        options->ce.tun_mtu_defined = false;
+        options->ce.occ_mtu = 0;
+    }
+    else if (streq(p[0], "block-ipv6") && !p[1])
+    {
+        VERIFY_PERMISSION(OPT_P_ROUTE);
+        options->block_ipv6 = false;
+    }
+#if defined(_WIN32) || defined(TARGET_ANDROID)
+    else if (streq(p[0], "dhcp-option") && !p[1])
+    {
+        struct tuntap_options *o = &options->tuntap_options;
+        VERIFY_PERMISSION(OPT_P_DHCPDNS);
+
+        o->domain = NULL;
+        o->netbios_scope = NULL;
+        o->netbios_node_type = 0;
+        o->dns6_len = 0;
+        memset(o->dns6, 0, sizeof(o->dns6));
+        o->dns_len = 0;
+        memset(o->dns, 0, sizeof(o->dns));
+        o->wins_len = 0;
+        memset(o->wins, 0, sizeof(o->wins));
+        o->ntp_len = 0;
+        memset(o->ntp, 0, sizeof(o->ntp));
+        o->nbdd_len = 0;
+        memset(o->nbdd, 0, sizeof(o->nbdd));
+        while (o->domain_search_list_len-- > 0)
+        {
+            o->domain_search_list[o->domain_search_list_len] = NULL;
+        }
+        o->disable_nbt = 0;
+        o->dhcp_options = 0;
+#if defined(TARGET_ANDROID)
+        o->http_proxy_port = 0;
+        o->http_proxy = NULL;
+#endif
+    }
+#endif /* if defined(_WIN32) || defined(TARGET_ANDROID) */
+#ifdef _WIN32
+    else if (streq(p[0], "block-outside-dns") && !p[1])
+    {
+        VERIFY_PERMISSION(OPT_P_DHCPDNS);
+        options->block_outside_dns = false;
+    }
+#else /* ifdef _WIN32 */
+    else if (streq(p[0], "dhcp-option") && !p[1])
+    {
+        VERIFY_PERMISSION(OPT_P_DHCPDNS);
+        delete_all_dhcp_fo(options, &es->list);
+    }
+#endif
+    else
+    {
+        int i;
+        int msglevel_unknown = msglevel_fc;
+        /* Check if an option is in --ignore-unknown-option and
+         * set warning level to non fatal */
+        for (i = 0; options->ignore_unknown_option && options->ignore_unknown_option[i]; i++)
+        {
+            if (streq(p[0], options->ignore_unknown_option[i]))
+            {
+                msglevel_unknown = M_WARN;
+                break;
+            }
+        }
+        msg(msglevel_unknown, "Unrecognized option or missing or extra parameter(s) in %s:%d: -%s (%s)", file, line, p[0], PACKAGE_VERSION);
+    }
+    return;
+err:
+    msg(msglevel, "Error occurred trying to remove %s option", p[0]);
+}
+
 static void
 set_user_script(struct options *options,
                 const char **script,
diff --git a/src/openvpn/push.c b/src/openvpn/push.c
index 858b821..22082a9 100644
--- a/src/openvpn/push.c
+++ b/src/openvpn/push.c
@@ -542,6 +542,11 @@
                 {
                     msg(M_WARN, "No updatable options found in incoming PUSH_UPDATE message");
                 }
+                else if (!do_update(c, option_types_found))
+                {
+                    msg(D_PUSH_ERRORS, "Failed to update options");
+                    goto error;
+                }
             }
         }
         event_timeout_clear(&c->c2.push_request_interval);
diff --git a/src/openvpn/route.c b/src/openvpn/route.c
index 156262a..89ebaee 100644
--- a/src/openvpn/route.c
+++ b/src/openvpn/route.c
@@ -1265,7 +1265,16 @@
               const struct tuntap *tt, unsigned int flags,
               const struct env_set *es, openvpn_net_ctx_t *ctx)
 {
-    if (rl && rl->iflags & RL_ROUTES_ADDED)
+    delete_routes_v4(rl, tt, flags, es, ctx);
+    delete_routes_v6(rl6, tt, flags, es, ctx);
+}
+
+void
+delete_routes_v4(struct route_list *rl, const struct tuntap *tt,
+                 unsigned int flags, const struct env_set *es,
+                 openvpn_net_ctx_t *ctx)
+{
+    if (rl && (rl->iflags & RL_ROUTES_ADDED))
     {
         struct route_ipv4 *r;
         for (r = rl->routes; r; r = r->next)
@@ -1281,8 +1290,14 @@
     {
         clear_route_list(rl);
     }
+}
 
-    if (rl6 && (rl6->iflags & RL_ROUTES_ADDED) )
+void
+delete_routes_v6(struct route_ipv6_list *rl6, const struct tuntap *tt,
+                 unsigned int flags, const struct env_set *es,
+                 openvpn_net_ctx_t *ctx)
+{
+    if (rl6 && (rl6->iflags & RL_ROUTES_ADDED))
     {
         struct route_ipv6 *r6;
         for (r6 = rl6->routes_ipv6; r6; r6 = r6->next)
diff --git a/src/openvpn/route.h b/src/openvpn/route.h
index 237375c..b89ec9f 100644
--- a/src/openvpn/route.h
+++ b/src/openvpn/route.h
@@ -335,6 +335,16 @@
                    const struct env_set *es,
                    openvpn_net_ctx_t *ctx);
 
+void
+delete_routes_v4(struct route_list *rl, const struct tuntap *tt,
+                 unsigned int flags, const struct env_set *es,
+                 openvpn_net_ctx_t *ctx);
+
+void
+delete_routes_v6(struct route_ipv6_list *rl6, const struct tuntap *tt,
+                 unsigned int flags, const struct env_set *es,
+                 openvpn_net_ctx_t *ctx);
+
 void setenv_routes(struct env_set *es, const struct route_list *rl);
 
 void setenv_routes_ipv6(struct env_set *es, const struct route_ipv6_list *rl6);

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/809?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I507180d7397b6959844a30908010132bc3411067
Gerrit-Change-Number: 809
Gerrit-PatchSet: 24
Gerrit-Owner: mrbff <ma...@ma...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-Reviewer: stipa <lst...@gm...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-MessageType: merged

[Openvpn-devel] [L] Change in openvpn[master]: PUSH_UPDATE: Added remove_option() and do_update().

[cron2 (C. Review) <ge...@op...>]

cron2 has uploaded a new patch set (#24) to the change originally created by mrbff. ( http://gerrit.openvpn.net/c/openvpn/+/809?usp=email )

The following approvals got outdated and were removed:
Code-Review+2 by cron2


Change subject: PUSH_UPDATE: Added remove_option() and do_update().
......................................................................

PUSH_UPDATE: Added remove_option() and do_update().

* Added remove_option() function and some utility functions to remove options at
  runtime following the push-update logic.
* Added do_update() function to close and reopen the tun and apply option updates.

Change-Id: I507180d7397b6959844a30908010132bc3411067
Signed-off-by: Marco Baffo <ma...@ma...>
Acked-by: Gert Doering <ge...@gr...>
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg32407.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M src/openvpn/init.c
M src/openvpn/init.h
M src/openvpn/multi.c
M src/openvpn/options.c
M src/openvpn/push.c
M src/openvpn/route.c
M src/openvpn/route.h
7 files changed, 350 insertions(+), 11 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/09/809/24

diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index ba1dda4..3254cc6 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -2470,7 +2470,7 @@
 
         if (pulled_options)
         {
-            if (!do_deferred_options(c, option_types_found))
+            if (!do_deferred_options(c, option_types_found, false))
             {
                 msg(D_PUSH_ERRORS, "ERROR: Failed to apply push options");
                 return false;
@@ -2594,6 +2594,55 @@
     return true;
 }
 
+bool
+do_update(struct context *c, unsigned int option_types_found)
+{
+    /* Not necessary since to receive the update the openvpn
+     * instance must be up and running but just in case
+     */
+    if (!c->c2.do_up_ran)
+    {
+        return false;
+    }
+
+    bool tt_dco_win = tuntap_is_dco_win(c->c1.tuntap);
+    if (tt_dco_win)
+    {
+        msg(M_NONFATAL, "dco-win doesn't yet support reopening TUN device");
+        return false;
+    }
+
+    if (!do_deferred_options(c, option_types_found, true))
+    {
+        msg(D_PUSH_ERRORS, "ERROR: Failed to apply push options");
+        return false;
+    }
+
+    do_close_tun(c, true);
+
+    management_sleep(1);
+    int error_flags = 0;
+    c->c2.did_open_tun = do_open_tun(c, &error_flags);
+    update_time();
+
+    if (c->c2.did_open_tun)
+    {
+        /* if --route-delay was specified, start timer */
+        if ((route_order(c->c1.tuntap) == ROUTE_AFTER_TUN) && c->options.route_delay_defined)
+        {
+            event_timeout_init(&c->c2.route_wakeup, c->options.route_delay, now);
+            event_timeout_init(&c->c2.route_wakeup_expire, c->options.route_delay + c->options.route_delay_window, now);
+            tun_standby_init(c->c1.tuntap);
+        }
+
+        initialization_sequence_completed(c, error_flags);
+    }
+
+    CLEAR(c->c1.pulled_options_digest_save);
+
+    return true;
+}
+
 /*
  * These are the option categories which will be accepted by pull.
  */
@@ -2672,11 +2721,8 @@
     return true;
 }
 
-/*
- * Handle non-tun-related pulled options.
- */
 bool
-do_deferred_options(struct context *c, const unsigned int found)
+do_deferred_options(struct context *c, const unsigned int found, const bool is_update)
 {
     if (found & OPT_P_MESSAGES)
     {
@@ -2784,7 +2830,10 @@
     /* process (potentially) pushed options */
     if (c->options.pull)
     {
-        if (!check_pull_client_ncp(c, found))
+        /* On PUSH_UPDATE, NCP related flags are never updated, and so the code
+         * would assume "no cipher pushed = NCP failed" - so, don't call it on
+         * updates */
+        if (!is_update && !check_pull_client_ncp(c, found))
         {
             return false;
         }
diff --git a/src/openvpn/init.h b/src/openvpn/init.h
index 5c6b9c1..25078a6 100644
--- a/src/openvpn/init.h
+++ b/src/openvpn/init.h
@@ -86,13 +86,29 @@
            bool pulled_options,
            unsigned int option_types_found);
 
+/**
+ * @brief A simplified version of the do_up() function. This function is called
+ *        after receiving a successful PUSH_UPDATE message. It closes and reopens
+ *        the TUN device to apply the updated options.
+ *
+ * @param c The context structure.
+ * @param option_types_found The options found in the PUSH_UPDATE message.
+ * @return true on success.
+ * @return false on error.
+ */
+bool do_update(struct context *c, unsigned int option_types_found);
+
 unsigned int pull_permission_mask(const struct context *c);
 
 const char *format_common_name(struct context *c, struct gc_arena *gc);
 
 void reset_coarse_timers(struct context *c);
 
-bool do_deferred_options(struct context *c, const unsigned int found);
+/*
+ * Handle non-tun-related pulled options.
+ * Set `is_update` param to true to skip NCP check.
+ */
+bool do_deferred_options(struct context *c, const unsigned int found, const bool is_update);
 
 void inherit_context_child(struct context *dest,
                            const struct context *src,
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index b2d2b6c..68b4da6 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -2424,7 +2424,7 @@
     /*
      * Process sourced options.
      */
-    do_deferred_options(&mi->context, option_types_found);
+    do_deferred_options(&mi->context, option_types_found, false);
 
     /*
      * make sure we got ifconfig settings from somewhere
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index eb53a60..469360d 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -1077,6 +1077,40 @@
     }
     gc_free(&gc);
 }
+
+static void
+delete_all_dhcp_fo(struct options *o, struct env_item **list)
+{
+    struct env_item *current, *prev;
+
+    ASSERT(list);
+
+    for (current = *list, prev = NULL; current != NULL; current = current->next)
+    {
+        char *tmp_value = NULL;
+        if (!strncmp(current->string, "foreign_option_", sizeof("foreign_option_")-1))
+        {
+            tmp_value = strchr(current->string, '=');
+            if (tmp_value && ++tmp_value)
+            {
+                if (!strncmp(tmp_value, "dhcp-option ", sizeof("dhcp-option ")-1))
+                {
+                    if (prev)
+                    {
+                        prev->next = current->next;
+                    }
+                    else
+                    {
+                        *list = current->next;
+                    }
+                    o->foreign_option_index--;
+                }
+            }
+        }
+        prev = current;
+    }
+}
+
 #endif /* ifndef _WIN32 */
 
 static in_addr_t
@@ -3082,7 +3116,7 @@
         opt->routes->flags |= RG_DEF1;
     }
 }
-#endif
+#endif /* ifdef _WIN32 */
 
 /*
  * Save/Restore certain option defaults before --pull is applied.
@@ -5314,6 +5348,18 @@
            struct env_set *es);
 
 static void
+remove_option(struct context *c,
+              struct options *options,
+              char *p[],
+              bool is_inline,
+              const char *file,
+              int line,
+              const int msglevel,
+              const unsigned int permission_mask,
+              unsigned int *option_types_found,
+              struct env_set *es);
+
+static void
 read_config_file(struct options *options,
                  const char *file,
                  int level,
@@ -5539,6 +5585,11 @@
                 add_option(options, p, false, file, line_num, 0, msglevel,
                            permission_mask, option_types_found, es);
             }
+            else if (push_update_option_flags & PUSH_OPT_TO_REMOVE)
+            {
+                remove_option(c, options, p, false, file, line_num, msglevel,
+                              permission_mask, option_types_found, es);
+            }
         }
     }
     return true;
@@ -5677,6 +5728,199 @@
     return options->forward_compatible ? M_WARN : msglevel;
 }
 
+/**
+ * @brief Resets options found in the PUSH_UPDATE message that are preceded by the `-` flag.
+ *        This function is used in push-updates to reset specified options.
+ *        The number of parameters `p` must always be 1. If the permission is verified,
+ *        all related options are erased or reset to their default values.
+ *        Upon successful permission verification (by VERIFY_PERMISSION()),
+ *        `option_types_found` is filled with the flag corresponding to the option.
+ *
+ * @param c The context structure.
+ * @param options A pointer to the options structure.
+ * @param p An array of strings containing the options and their parameters.
+ * @param is_inline A boolean indicating if the option is inline.
+ * @param file The file where the function is called.
+ * @param line The line number where the function is called.
+ * @param msglevel The message level.
+ * @param permission_mask The permission mask used by VERIFY_PERMISSION().
+ * @param option_types_found A pointer to the variable where the flags corresponding to the options found are stored.
+ * @param es The environment set structure.
+ */
+static void
+remove_option(struct context *c,
+              struct options *options,
+              char *p[],
+              bool is_inline,
+              const char *file,
+              int line,
+              const int msglevel,
+              const unsigned int permission_mask,
+              unsigned int *option_types_found,
+              struct env_set *es)
+{
+    int msglevel_fc = msglevel_forward_compatible(options, msglevel);
+
+    if (streq(p[0], "ifconfig") && !p[1])
+    {
+        VERIFY_PERMISSION(OPT_P_UP);
+        options->ifconfig_local = NULL;
+        options->ifconfig_remote_netmask = NULL;
+    }
+    else if (streq(p[0], "ifconfig-ipv6") && !p[1])
+    {
+        VERIFY_PERMISSION(OPT_P_UP);
+        options->ifconfig_ipv6_local = NULL;
+        options->ifconfig_ipv6_netbits = 0;
+        options->ifconfig_ipv6_remote = NULL;
+    }
+    else if (streq(p[0], "route") && !p[1])
+    {
+        VERIFY_PERMISSION(OPT_P_ROUTE);
+        if (c->c1.route_list)
+        {
+            delete_routes_v4(c->c1.route_list, c->c1.tuntap,
+                             ROUTE_OPTION_FLAGS(&c->options),
+                             es, &c->net_ctx);
+            if (options->routes)
+            {
+                options->routes->routes = NULL;
+                options->routes->flags = 0;
+            }
+        }
+    }
+    else if (streq(p[0], "route-ipv6") && !p[1])
+    {
+        VERIFY_PERMISSION(OPT_P_ROUTE);
+        if (c->c1.route_ipv6_list)
+        {
+            delete_routes_v6(c->c1.route_ipv6_list, c->c1.tuntap,
+                             ROUTE_OPTION_FLAGS(&c->options),
+                             es, &c->net_ctx);
+            if (options->routes_ipv6)
+            {
+                options->routes_ipv6->routes_ipv6 = NULL;
+                options->routes_ipv6->flags = 0;
+            }
+        }
+    }
+    else if (streq(p[0], "route-gateway") && !p[1])
+    {
+        VERIFY_PERMISSION(OPT_P_ROUTE_EXTRAS);
+        options->route_gateway_via_dhcp = false;
+        options->route_default_gateway = NULL;
+    }
+    else if (streq(p[0], "route-metric") && !p[1])
+    {
+        VERIFY_PERMISSION(OPT_P_ROUTE);
+        options->route_default_metric = 0;
+    }
+    else if (streq(p[0], "push-continuation") && !p[1])
+    {
+        VERIFY_PERMISSION(OPT_P_PULL_MODE);
+        options->push_continuation = 0;
+    }
+    else if ((streq(p[0], "redirect-gateway") || streq(p[0], "redirect-private")) && !p[1])
+    {
+        VERIFY_PERMISSION(OPT_P_ROUTE);
+        if (options->routes)
+        {
+            options->routes->flags = 0;
+        }
+        if (options->routes_ipv6)
+        {
+            options->routes_ipv6->flags = 0;
+        }
+    }
+    else if (streq(p[0], "dns") && !p[1])
+    {
+        VERIFY_PERMISSION(OPT_P_DHCPDNS);
+        gc_free(&options->dns_options.gc);
+        CLEAR(options->dns_options);
+    }
+    else if (streq(p[0], "topology") && !p[1])
+    {
+        VERIFY_PERMISSION(OPT_P_UP);
+        options->topology = TOP_UNDEF;
+        helper_setdefault_topology(options);
+    }
+    else if (streq(p[0], "tun-mtu") && !p[1])
+    {
+        VERIFY_PERMISSION(OPT_P_PUSH_MTU|OPT_P_CONNECTION);
+        options->ce.tun_mtu = TUN_MTU_DEFAULT;
+        options->ce.tun_mtu_defined = false;
+        options->ce.occ_mtu = 0;
+    }
+    else if (streq(p[0], "block-ipv6") && !p[1])
+    {
+        VERIFY_PERMISSION(OPT_P_ROUTE);
+        options->block_ipv6 = false;
+    }
+#if defined(_WIN32) || defined(TARGET_ANDROID)
+    else if (streq(p[0], "dhcp-option") && !p[1])
+    {
+        struct tuntap_options *o = &options->tuntap_options;
+        VERIFY_PERMISSION(OPT_P_DHCPDNS);
+
+        o->domain = NULL;
+        o->netbios_scope = NULL;
+        o->netbios_node_type = 0;
+        o->dns6_len = 0;
+        memset(o->dns6, 0, sizeof(o->dns6));
+        o->dns_len = 0;
+        memset(o->dns, 0, sizeof(o->dns));
+        o->wins_len = 0;
+        memset(o->wins, 0, sizeof(o->wins));
+        o->ntp_len = 0;
+        memset(o->ntp, 0, sizeof(o->ntp));
+        o->nbdd_len = 0;
+        memset(o->nbdd, 0, sizeof(o->nbdd));
+        while (o->domain_search_list_len-- > 0)
+        {
+            o->domain_search_list[o->domain_search_list_len] = NULL;
+        }
+        o->disable_nbt = 0;
+        o->dhcp_options = 0;
+#if defined(TARGET_ANDROID)
+        o->http_proxy_port = 0;
+        o->http_proxy = NULL;
+#endif
+    }
+#endif /* if defined(_WIN32) || defined(TARGET_ANDROID) */
+#ifdef _WIN32
+    else if (streq(p[0], "block-outside-dns") && !p[1])
+    {
+        VERIFY_PERMISSION(OPT_P_DHCPDNS);
+        options->block_outside_dns = false;
+    }
+#else /* ifdef _WIN32 */
+    else if (streq(p[0], "dhcp-option") && !p[1])
+    {
+        VERIFY_PERMISSION(OPT_P_DHCPDNS);
+        delete_all_dhcp_fo(options, &es->list);
+    }
+#endif
+    else
+    {
+        int i;
+        int msglevel_unknown = msglevel_fc;
+        /* Check if an option is in --ignore-unknown-option and
+         * set warning level to non fatal */
+        for (i = 0; options->ignore_unknown_option && options->ignore_unknown_option[i]; i++)
+        {
+            if (streq(p[0], options->ignore_unknown_option[i]))
+            {
+                msglevel_unknown = M_WARN;
+                break;
+            }
+        }
+        msg(msglevel_unknown, "Unrecognized option or missing or extra parameter(s) in %s:%d: -%s (%s)", file, line, p[0], PACKAGE_VERSION);
+    }
+    return;
+err:
+    msg(msglevel, "Error occurred trying to remove %s option", p[0]);
+}
+
 static void
 set_user_script(struct options *options,
                 const char **script,
diff --git a/src/openvpn/push.c b/src/openvpn/push.c
index 858b821..22082a9 100644
--- a/src/openvpn/push.c
+++ b/src/openvpn/push.c
@@ -542,6 +542,11 @@
                 {
                     msg(M_WARN, "No updatable options found in incoming PUSH_UPDATE message");
                 }
+                else if (!do_update(c, option_types_found))
+                {
+                    msg(D_PUSH_ERRORS, "Failed to update options");
+                    goto error;
+                }
             }
         }
         event_timeout_clear(&c->c2.push_request_interval);
diff --git a/src/openvpn/route.c b/src/openvpn/route.c
index 156262a..89ebaee 100644
--- a/src/openvpn/route.c
+++ b/src/openvpn/route.c
@@ -1265,7 +1265,16 @@
               const struct tuntap *tt, unsigned int flags,
               const struct env_set *es, openvpn_net_ctx_t *ctx)
 {
-    if (rl && rl->iflags & RL_ROUTES_ADDED)
+    delete_routes_v4(rl, tt, flags, es, ctx);
+    delete_routes_v6(rl6, tt, flags, es, ctx);
+}
+
+void
+delete_routes_v4(struct route_list *rl, const struct tuntap *tt,
+                 unsigned int flags, const struct env_set *es,
+                 openvpn_net_ctx_t *ctx)
+{
+    if (rl && (rl->iflags & RL_ROUTES_ADDED))
     {
         struct route_ipv4 *r;
         for (r = rl->routes; r; r = r->next)
@@ -1281,8 +1290,14 @@
     {
         clear_route_list(rl);
     }
+}
 
-    if (rl6 && (rl6->iflags & RL_ROUTES_ADDED) )
+void
+delete_routes_v6(struct route_ipv6_list *rl6, const struct tuntap *tt,
+                 unsigned int flags, const struct env_set *es,
+                 openvpn_net_ctx_t *ctx)
+{
+    if (rl6 && (rl6->iflags & RL_ROUTES_ADDED))
     {
         struct route_ipv6 *r6;
         for (r6 = rl6->routes_ipv6; r6; r6 = r6->next)
diff --git a/src/openvpn/route.h b/src/openvpn/route.h
index 237375c..b89ec9f 100644
--- a/src/openvpn/route.h
+++ b/src/openvpn/route.h
@@ -335,6 +335,16 @@
                    const struct env_set *es,
                    openvpn_net_ctx_t *ctx);
 
+void
+delete_routes_v4(struct route_list *rl, const struct tuntap *tt,
+                 unsigned int flags, const struct env_set *es,
+                 openvpn_net_ctx_t *ctx);
+
+void
+delete_routes_v6(struct route_ipv6_list *rl6, const struct tuntap *tt,
+                 unsigned int flags, const struct env_set *es,
+                 openvpn_net_ctx_t *ctx);
+
 void setenv_routes(struct env_set *es, const struct route_list *rl);
 
 void setenv_routes_ipv6(struct env_set *es, const struct route_ipv6_list *rl6);

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/809?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I507180d7397b6959844a30908010132bc3411067
Gerrit-Change-Number: 809
Gerrit-PatchSet: 24
Gerrit-Owner: mrbff <ma...@ma...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-Reviewer: stipa <lst...@gm...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-MessageType: newpatchset

[Openvpn-devel] [PATCH applied] Re: PUSH_UPDATE: Added remove_option() and do_update().

[Gert D. <ge...@gr...>]

As for the previous patch, this had quite a journey...

+2 from Lev on v12, then rebases up to v21, stylistic discussions with
me (and "please add comments so we can remember in two months why this
extra check needs to be done" ;-) ), to be integrated into v22, +2'ed
again, and v23 another rebase... so recording my +2 as "Acked-by:".

Lev has tested this and confirms the new stuff (808+809+810) works.

I have stared long and hard at the code, and run the client/server side
testbeds on it, and nothing breaks.  I have not tested the PUSH_UPDATE
machinery yet  (the t_client tests do excercise the change to split
delete_routes(), but that's trivial enough)


Your patch has been applied to the master branch.

commit 73f3247e89bd66fa0b5142e3e1773951f6c3cba0
Author: Marco Baffo
Date:   Tue Jul 29 12:40:50 2025 +0200

     PUSH_UPDATE: Added remove_option() and do_update().

     Signed-off-by: Marco Baffo <ma...@ma...>
     Acked-by: Gert Doering <ge...@gr...>
     Message-Id: <202...@gr...>
     URL: https://www.mail-archive.com/ope...@li.../msg32407.html
     Signed-off-by: Gert Doering <ge...@gr...>


--
kind regards,

Gert Doering

[Openvpn-devel] [M] Change in openvpn[master]: list: Make types of hash elements consistent

[flichtenheld (C. Review) <ge...@op...>]

Attention is currently required from: plaisthos.

Hello plaisthos,

I'd like you to do a code review.
Please visit

    http://gerrit.openvpn.net/c/openvpn/+/1119?usp=email

to review the following change.


Change subject: list: Make types of hash elements consistent
......................................................................

list: Make types of hash elements consistent

Really no use in having the indices and limits in int.

Change-Id: I3334465738fb1fbf508dfd719b6a238b500cc0ae
Signed-off-by: Frank Lichtenheld <fr...@li...>
---
M src/openvpn/integer.h
M src/openvpn/list.c
M src/openvpn/list.h
M src/openvpn/multi.c
M src/openvpn/multi.h
M src/openvpn/options.c
M src/openvpn/options.h
M tests/unit_tests/openvpn/test_misc.c
8 files changed, 63 insertions(+), 50 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/19/1119/1

diff --git a/src/openvpn/integer.h b/src/openvpn/integer.h
index c568036..e6106c9 100644
--- a/src/openvpn/integer.h
+++ b/src/openvpn/integer.h
@@ -132,6 +132,27 @@
     }
 }
 
+static inline unsigned int
+constrain_uint(unsigned int x, unsigned int min, unsigned int max)
+{
+    if (min > max)
+    {
+        return min;
+    }
+    if (x < min)
+    {
+        return min;
+    }
+    else if (x > max)
+    {
+        return max;
+    }
+    else
+    {
+        return x;
+    }
+}
+
 /*
  * Functions used for circular buffer index arithmetic.
  */
diff --git a/src/openvpn/list.c b/src/openvpn/list.c
index 1f33b5f..3971d62 100644
--- a/src/openvpn/list.c
+++ b/src/openvpn/list.c
@@ -35,23 +35,22 @@
 #include "memdbg.h"
 
 struct hash *
-hash_init(const int n_buckets,
+hash_init(const uint32_t n_buckets,
           const uint32_t iv,
           uint32_t (*hash_function)(const void *key, uint32_t iv),
           bool (*compare_function)(const void *key1, const void *key2))
 {
     struct hash *h;
-    int i;
 
     ASSERT(n_buckets > 0);
     ALLOC_OBJ_CLEAR(h, struct hash);
-    h->n_buckets = (int) adjust_power_of_2(n_buckets);
+    h->n_buckets = (uint32_t)adjust_power_of_2(n_buckets);
     h->mask = h->n_buckets - 1;
     h->hash_function = hash_function;
     h->compare_function = compare_function;
     h->iv = iv;
     ALLOC_ARRAY(h->buckets, struct hash_bucket, h->n_buckets);
-    for (i = 0; i < h->n_buckets; ++i)
+    for (uint32_t i = 0; i < h->n_buckets; ++i)
     {
         struct hash_bucket *b = &h->buckets[i];
         b->list = NULL;
@@ -62,8 +61,7 @@
 void
 hash_free(struct hash *hash)
 {
-    int i;
-    for (i = 0; i < hash->n_buckets; ++i)
+    for (uint32_t i = 0; i < hash->n_buckets; ++i)
     {
         struct hash_bucket *b = &hash->buckets[i];
         struct hash_element *he = b->list;
@@ -222,15 +220,15 @@
 void
 hash_iterator_init_range(struct hash *hash,
                          struct hash_iterator *hi,
-                         int start_bucket,
-                         int end_bucket)
+                         uint32_t start_bucket,
+                         uint32_t end_bucket)
 {
     if (end_bucket > hash->n_buckets)
     {
         end_bucket = hash->n_buckets;
     }
 
-    ASSERT(start_bucket >= 0 && start_bucket <= end_bucket);
+    ASSERT(start_bucket <= end_bucket);
 
     hi->hash = hash;
     hi->elem = NULL;
@@ -336,6 +334,9 @@
  * the return value.  Every 1-bit and 2-bit delta achieves avalanche.
  * About 36+6len instructions.
  *
+ * #define hashsize(n) ((uint32_t)1<<(n))
+ * #define hashmask(n) (hashsize(n)-1)
+ *
  * The best hash table sizes are powers of 2.  There is no need to do
  * mod a prime (mod is sooo slow!).  If you need less than 32 bits,
  * use a bitmask.  For example, if you need only 10 bits, do
diff --git a/src/openvpn/list.h b/src/openvpn/list.h
index 783570f..d2c5373 100644
--- a/src/openvpn/list.h
+++ b/src/openvpn/list.h
@@ -37,14 +37,11 @@
 #include "basic.h"
 #include "buffer.h"
 
-#define hashsize(n) ((uint32_t)1<<(n))
-#define hashmask(n) (hashsize(n)-1)
-
 struct hash_element
 {
     void *value;
     const void *key;
-    unsigned int hash_value;
+    uint32_t hash_value;
     struct hash_element *next;
 };
 
@@ -55,16 +52,16 @@
 
 struct hash
 {
-    int n_buckets;
-    int n_elements;
-    int mask;
+    uint32_t n_buckets;
+    uint32_t n_elements;
+    uint32_t mask;
     uint32_t iv;
     uint32_t (*hash_function)(const void *key, uint32_t iv);
     bool (*compare_function)(const void *key1, const void *key2); /* return true if equal */
     struct hash_bucket *buckets;
 };
 
-struct hash *hash_init(const int n_buckets,
+struct hash *hash_init(const uint32_t n_buckets,
                        const uint32_t iv,
                        uint32_t (*hash_function)(const void *key, uint32_t iv),
                        bool (*compare_function)(const void *key1, const void *key2));
@@ -88,19 +85,19 @@
 struct hash_iterator
 {
     struct hash *hash;
-    int bucket_index;
+    uint32_t bucket_index;
     struct hash_bucket *bucket;
     struct hash_element *elem;
     struct hash_element *last;
     bool bucket_marked;
-    int bucket_index_start;
-    int bucket_index_end;
+    uint32_t bucket_index_start;
+    uint32_t bucket_index_end;
 };
 
 void hash_iterator_init_range(struct hash *hash,
                               struct hash_iterator *hi,
-                              int start_bucket,
-                              int end_bucket);
+                              uint32_t start_bucket,
+                              uint32_t end_bucket);
 
 void hash_iterator_init(struct hash *hash, struct hash_iterator *iter);
 
@@ -118,13 +115,13 @@
     return (*hash->hash_function)(key, hash->iv);
 }
 
-static inline int
+static inline uint32_t
 hash_n_elements(const struct hash *hash)
 {
     return hash->n_elements;
 }
 
-static inline int
+static inline uint32_t
 hash_n_buckets(const struct hash *hash)
 {
     return hash->n_buckets;
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index c0e8f9c..02157ac 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -175,19 +175,13 @@
 
 static void
 multi_reap_range(const struct multi_context *m,
-                 int start_bucket,
-                 int end_bucket)
+                 uint32_t start_bucket,
+                 uint32_t end_bucket)
 {
     struct gc_arena gc = gc_new();
     struct hash_iterator hi;
     struct hash_element *he;
 
-    if (start_bucket < 0)
-    {
-        start_bucket = 0;
-        end_bucket = hash_n_buckets(m->vhash);
-    }
-
     dmsg(D_MULTI_DEBUG, "MULTI: REAP range %d -> %d", start_bucket, end_bucket);
     hash_iterator_init_range(m->vhash, &hi, start_bucket, end_bucket);
     while ((he = hash_iterator_next(&hi)) != NULL)
@@ -209,11 +203,11 @@
 static void
 multi_reap_all(const struct multi_context *m)
 {
-    multi_reap_range(m, -1, 0);
+    multi_reap_range(m, 0, hash_n_buckets(m->vhash));
 }
 
 static struct multi_reap *
-multi_reap_new(int buckets_per_pass)
+multi_reap_new(uint32_t buckets_per_pass)
 {
     struct multi_reap *mr;
     ALLOC_OBJ(mr, struct multi_reap);
@@ -245,10 +239,10 @@
 /*
  * How many buckets in vhash to reap per pass.
  */
-static int
-reap_buckets_per_pass(int n_buckets)
+static uint32_t
+reap_buckets_per_pass(uint32_t n_buckets)
 {
-    return constrain_int(n_buckets / REAP_DIVISOR, REAP_MIN, REAP_MAX);
+    return constrain_uint(n_buckets / REAP_DIVISOR, REAP_MIN, REAP_MAX);
 }
 
 #ifdef ENABLE_MANAGEMENT
diff --git a/src/openvpn/multi.h b/src/openvpn/multi.h
index 3c821d7..a622e8c 100644
--- a/src/openvpn/multi.h
+++ b/src/openvpn/multi.h
@@ -52,8 +52,8 @@
  */
 struct multi_reap
 {
-    int bucket_base;
-    int buckets_per_pass;
+    uint32_t bucket_base;
+    uint32_t buckets_per_pass;
     time_t last_call;
 };
 
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index d3ca340..9ca2c4a 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -7588,11 +7588,11 @@
     }
     else if (streq(p[0], "hash-size") && p[1] && p[2] && !p[3])
     {
-        int real, virtual;
+        uint32_t real, virtual;
 
         VERIFY_PERMISSION(OPT_P_GENERAL);
-        real = atoi_warn(p[1], msglevel);
-        virtual = atoi_warn(p[2], msglevel);
+        real = positive_atoi(p[1], msglevel);
+        virtual = positive_atoi(p[2], msglevel);
         if (real < 1 || virtual < 1)
         {
             msg(msglevel, "--hash-size sizes must be >= 1 (preferably a power of 2)");
diff --git a/src/openvpn/options.h b/src/openvpn/options.h
index 78b0d7c..9542d93 100644
--- a/src/openvpn/options.h
+++ b/src/openvpn/options.h
@@ -498,8 +498,8 @@
     struct in6_addr ifconfig_ipv6_pool_base;            /* IPv6 */
     int ifconfig_ipv6_pool_netbits;                     /* IPv6 */
 
-    int real_hash_size;
-    int virtual_hash_size;
+    uint32_t real_hash_size;
+    uint32_t virtual_hash_size;
     const char *client_connect_script;
     const char *client_disconnect_script;
     const char *learn_address_script;
diff --git a/tests/unit_tests/openvpn/test_misc.c b/tests/unit_tests/openvpn/test_misc.c
index 3272f6b..b1e0bef 100644
--- a/tests/unit_tests/openvpn/test_misc.c
+++ b/tests/unit_tests/openvpn/test_misc.c
@@ -123,7 +123,7 @@
 word_hash_function(const void *key, uint32_t iv)
 {
     const char *str = (const char *) key;
-    const int len = strlen(str);
+    const uint32_t len = (uint32_t)strlen(str);
     return hash_func((const uint8_t *)str, len, iv);
 }
 
@@ -133,11 +133,11 @@
     return strcmp((const char *)key1, (const char *)key2) == 0;
 }
 
-static unsigned long
+static uint32_t
 get_random(void)
 {
     /* rand() is not very random, but it's C99 and this is just for testing */
-    return rand();
+    return (uint32_t)rand();
 }
 
 static struct hash_element *
@@ -172,7 +172,7 @@
     struct hash *hash = hash_init(10000, get_random(), word_hash_function, word_compare_function);
     struct hash *nhash = hash_init(256, get_random(), word_hash_function, word_compare_function);
 
-    printf("hash_init n_buckets=%d mask=0x%08x\n", hash->n_buckets, hash->mask);
+    printf("hash_init n_buckets=%u mask=0x%08x\n", hash->n_buckets, hash->mask);
 
     char wordfile[PATH_MAX] = { 0 };
     openvpn_test_get_srcdir_dir(wordfile, PATH_MAX, "/../../../COPYRIGHT.GPL" );
@@ -250,10 +250,10 @@
 
     /* output contents of hash table */
     {
-        ptr_type inc = 0;
+        uint32_t inc = 0;
         int count = 0;
 
-        for (ptr_type base = 0; base < hash_n_buckets(hash); base += inc)
+        for (uint32_t base = 0; base < hash_n_buckets(hash); base += inc)
         {
             struct hash_iterator hi;
             struct hash_element *he;

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1119?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I3334465738fb1fbf508dfd719b6a238b500cc0ae
Gerrit-Change-Number: 1119
Gerrit-PatchSet: 1
Gerrit-Owner: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-MessageType: newchange

[Openvpn-devel] [L] Change in openvpn[master]: PUSH_UPDATE: Allow OpenVPN in client mode to receive and handle PUSH ...

[cron2 (C. Review) <ge...@op...>]

cron2 has submitted this change. ( http://gerrit.openvpn.net/c/openvpn/+/808?usp=email )

Change subject: PUSH_UPDATE: Allow OpenVPN in client mode to receive and handle PUSH UPDATE control messages to allow options updating at runtime.
......................................................................

PUSH_UPDATE: Allow OpenVPN in client mode to receive and handle PUSH UPDATE control messages to allow options updating at runtime.

* Added IV_PROTO_PUSH_UPDATE flag bit to support push-updates.
* Added process_incoming_push_update(), in a separate file to create tests more easily.
* Modified incoming_push_message(), process_incoming_push_msg(), apply_push_options(),
  apply_pull_filter() to process also push-update messages.
* Added the check_push_update_option_flags() function used in apply_pull_filter() to
  check options formatting inside push-update messages, if the options are updatables
  and to check for '?' and '-' flags that may be present in front of the options.
  The '-' flag is used to indicate that the option in question should be removed,
  while the '?' indicates that the option is optional and to do not generate
  errors if the client cannot update that option.
  For more info you can read the RFC at https://github.com/OpenVPN/openvpn-rfc .
* Created some unit tests for the push-update message handling in test_push_update_msg.c.

Change-Id: I6ecd4cb47571cc8c20e46de8595c742aeec6064a
Signed-off-by: Marco Baffo <ma...@ma...>
Acked-by: Gert Doering <ge...@gr...>
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg32406.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M CMakeLists.txt
M src/openvpn/Makefile.am
M src/openvpn/forward.c
M src/openvpn/init.c
M src/openvpn/options.c
M src/openvpn/options.h
M src/openvpn/options_util.c
M src/openvpn/options_util.h
M src/openvpn/push.c
M src/openvpn/push.h
A src/openvpn/push_util.c
M src/openvpn/ssl.c
M src/openvpn/ssl.h
M tests/unit_tests/openvpn/Makefile.am
A tests/unit_tests/openvpn/test_push_update_msg.c
15 files changed, 625 insertions(+), 88 deletions(-)




diff --git a/CMakeLists.txt b/CMakeLists.txt
index efb2d2d..3866e21 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -534,6 +534,7 @@
     src/openvpn/ps.c
     src/openvpn/ps.h
     src/openvpn/push.c
+    src/openvpn/push_util.c
     src/openvpn/push.h
     src/openvpn/pushlist.h
     src/openvpn/reflect_filter.c
@@ -652,6 +653,7 @@
         "test_provider"
         "test_ssl"
         "test_user_pass"
+        "test_push_update_msg"
         )
 
     if (WIN32)
@@ -854,6 +856,14 @@
         src/openvpn/run_command.c
         )
 
+    target_sources(test_push_update_msg PRIVATE
+        tests/unit_tests/openvpn/mock_msg.c
+        tests/unit_tests/openvpn/mock_get_random.c
+	    src/openvpn/push_util.c
+	    src/openvpn/options_util.c
+	    src/openvpn/otime.c
+        )
+
     if (TARGET test_argv)
         target_link_options(test_argv PRIVATE -Wl,--wrap=parse_line)
         target_sources(test_argv PRIVATE
diff --git a/src/openvpn/Makefile.am b/src/openvpn/Makefile.am
index a5d4cdc..1e17c3f 100644
--- a/src/openvpn/Makefile.am
+++ b/src/openvpn/Makefile.am
@@ -117,7 +117,7 @@
 	proto.c proto.h \
 	proxy.c proxy.h \
 	ps.c ps.h \
-	push.c push.h \
+	push.c push_util.c push.h \
 	pushlist.h \
 	reflect_filter.c reflect_filter.h \
 	reliable.c reliable.h \
diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c
index dfc6708..475d142 100644
--- a/src/openvpn/forward.c
+++ b/src/openvpn/forward.c
@@ -2064,7 +2064,7 @@
     }
 
     /* check for incoming control messages on the control channel like
-     * push request/reply, or authentication failure and 2FA messages */
+     * push request/reply/update, or authentication failure and 2FA messages */
     if (tls_test_payload_len(c->c2.tls_multi) > 0)
     {
         check_incoming_control_channel(c);
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index aac8a6a..ba1dda4 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -2051,8 +2051,8 @@
         /* possibly add routes */
         if ((route_order(c->c1.tuntap) == ROUTE_AFTER_TUN) && (!c->options.route_delay_defined))
         {
-            int status = do_route(&c->options, c->c1.route_list, c->c1.route_ipv6_list,
-                                  c->c1.tuntap, c->plugins, c->c2.es, &c->net_ctx);
+            bool status = do_route(&c->options, c->c1.route_list, c->c1.route_ipv6_list,
+                                   c->c1.tuntap, c->plugins, c->c2.es, &c->net_ctx);
             *error_flags |= (status ? 0 : ISC_ROUTE_ERRORS);
         }
 
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index d44c102..eb53a60 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -66,6 +66,7 @@
 #include <ctype.h>
 
 #include "memdbg.h"
+#include "options_util.h"
 
 const char title_string[] =
     PACKAGE_STRING
@@ -948,24 +949,6 @@
     }
 }
 
-struct pull_filter
-{
-#define PUF_TYPE_UNDEF  0    /**< undefined filter type */
-#define PUF_TYPE_ACCEPT 1    /**< filter type to accept a matching option */
-#define PUF_TYPE_IGNORE 2    /**< filter type to ignore a matching option */
-#define PUF_TYPE_REJECT 3    /**< filter type to reject and trigger SIGUSR1 */
-    int type;
-    int size;
-    char *pattern;
-    struct pull_filter *next;
-};
-
-struct pull_filter_list
-{
-    struct pull_filter *head;
-    struct pull_filter *tail;
-};
-
 #ifndef ENABLE_SMALL
 
 static const char *
@@ -5503,60 +5486,14 @@
     }
 }
 
-/**
- * Filter an option line by all pull filters.
- *
- * If a match is found, the line is modified depending on
- * the filter type, and returns true. If the filter type is
- * reject, SIGUSR1 is triggered and the return value is false.
- * In that case the caller must end the push processing.
- */
-static bool
-apply_pull_filter(const struct options *o, char *line)
-{
-    struct pull_filter *f;
-
-    if (!o->pull_filter_list)
-    {
-        return true;
-    }
-
-    /* skip leading spaces matching the behaviour of parse_line */
-    while (isspace(*line))
-    {
-        line++;
-    }
-
-    for (f = o->pull_filter_list->head; f; f = f->next)
-    {
-        if (f->type == PUF_TYPE_ACCEPT && strncmp(line, f->pattern, f->size) == 0)
-        {
-            msg(D_LOW, "Pushed option accepted by filter: '%s'", line);
-            return true;
-        }
-        else if (f->type == PUF_TYPE_IGNORE && strncmp(line, f->pattern, f->size) == 0)
-        {
-            msg(D_PUSH, "Pushed option removed by filter: '%s'", line);
-            *line = '\0';
-            return true;
-        }
-        else if (f->type == PUF_TYPE_REJECT && strncmp(line, f->pattern, f->size) == 0)
-        {
-            msg(M_WARN, "Pushed option rejected by filter: '%s'. Restarting.", line);
-            *line = '\0';
-            throw_signal_soft(SIGUSR1, "Offending option received from server");
-            return false;
-        }
-    }
-    return true;
-}
-
 bool
-apply_push_options(struct options *options,
+apply_push_options(struct context *c,
+                   struct options *options,
                    struct buffer *buf,
                    unsigned int permission_mask,
                    unsigned int *option_types_found,
-                   struct env_set *es)
+                   struct env_set *es,
+                   bool is_update)
 {
     char line[OPTION_PARM_SIZE];
     int line_num = 0;
@@ -5568,14 +5505,40 @@
         char *p[MAX_PARMS+1];
         CLEAR(p);
         ++line_num;
-        if (!apply_pull_filter(options, line))
+        unsigned int push_update_option_flags = 0;
+        int i = 0;
+
+        /* skip leading spaces matching the behaviour of parse_line */
+        while (isspace(line[i]))
         {
+            i++;
+        }
+
+        /* If we are not in a 'PUSH_UPDATE' we just check `apply_pull_filter()`
+         * otherwise we must call `check_push_update_option_flags()` first
+         */
+        if ((is_update && !check_push_update_option_flags(line, &i, &push_update_option_flags))
+            || !apply_pull_filter(options, &line[i]))
+        {
+            /* In case we are in a `PUSH_UPDATE` and `check_push_update_option_flags()`
+             * or `apply_pull_filter()` fail but the option is flagged by `PUSH_OPT_OPTIONAL`,
+             * instead of restarting, we just ignore the option and we process the next one
+             */
+            if (push_update_option_flags & PUSH_OPT_OPTIONAL)
+            {
+                continue; /* Ignoring this option */
+            }
+            throw_signal_soft(SIGUSR1, "Offending option received from server");
             return false; /* Cause push/pull error and stop push processing */
         }
-        if (parse_line(line, p, SIZE(p)-1, file, line_num, msglevel, &options->gc))
+
+        if (parse_line(&line[i], p, SIZE(p)-1, file, line_num, msglevel, &options->gc))
         {
-            add_option(options, p, false, file, line_num, 0, msglevel,
-                       permission_mask, option_types_found, es);
+            if (!is_update)
+            {
+                add_option(options, p, false, file, line_num, 0, msglevel,
+                           permission_mask, option_types_found, es);
+            }
         }
     }
     return true;
diff --git a/src/openvpn/options.h b/src/openvpn/options.h
index 56e85d7..0544ca9 100644
--- a/src/openvpn/options.h
+++ b/src/openvpn/options.h
@@ -794,6 +794,33 @@
 #define MAN_CLIENT_AUTH_ENABLED(opt) (false)
 #endif
 
+/*
+ * some PUSH_UPDATE options
+ */
+#define OPT_P_U_ROUTE           (1<<0)
+#define OPT_P_U_ROUTE6          (1<<1)
+#define OPT_P_U_DNS             (1<<2)
+#define OPT_P_U_DHCP            (1<<3)
+#define OPT_P_U_REDIR_GATEWAY   (1<<4)
+
+struct pull_filter
+{
+#define PUF_TYPE_UNDEF  0    /**< undefined filter type */
+#define PUF_TYPE_ACCEPT 1    /**< filter type to accept a matching option */
+#define PUF_TYPE_IGNORE 2    /**< filter type to ignore a matching option */
+#define PUF_TYPE_REJECT 3    /**< filter type to reject and trigger SIGUSR1 */
+    int type;
+    int size;
+    char *pattern;
+    struct pull_filter *next;
+};
+
+struct pull_filter_list
+{
+    struct pull_filter *head;
+    struct pull_filter *tail;
+};
+
 void parse_argv(struct options *options,
                 const int argc,
                 char *argv[],
@@ -862,11 +889,13 @@
 
 void pre_connect_restore(struct options *o, struct gc_arena *gc);
 
-bool apply_push_options(struct options *options,
+bool apply_push_options(struct context *c,
+                        struct options *options,
                         struct buffer *buf,
                         unsigned int permission_mask,
                         unsigned int *option_types_found,
-                        struct env_set *es);
+                        struct env_set *es,
+                        bool is_update);
 
 void options_detach(struct options *o);
 
diff --git a/src/openvpn/options_util.c b/src/openvpn/options_util.c
index 80d0c08..d317c1a 100644
--- a/src/openvpn/options_util.c
+++ b/src/openvpn/options_util.c
@@ -30,6 +30,8 @@
 
 #include "options_util.h"
 
+#include "push.h"
+
 const char *
 parse_auth_failed_temp(struct options *o, const char *reason)
 {
@@ -145,3 +147,116 @@
 
     return (int) i;
 }
+
+static const char *updatable_options[] = {
+    "block-ipv6",
+    "block-outside-dns",
+    "dhcp-option",
+    "dns",
+    "ifconfig",
+    "ifconfig-ipv6",
+    "push-continuation",
+    "redirect-gateway",
+    "redirect-private",
+    "route",
+    "route-gateway",
+    "route-ipv6",
+    "route-metric",
+    "topology",
+    "tun-mtu",
+    "keepalive"
+};
+
+bool
+check_push_update_option_flags(char *line, int *i, unsigned int *flags)
+{
+    *flags = 0;
+    bool opt_is_updatable = false;
+    char c = line[*i];
+
+    /* We check for '?' and '-' and
+     * if they are present we skip them.
+     */
+    if (c == '-')
+    {
+        if (!(line)[*i + 1])
+        {
+            return false;
+        }
+        *flags |= PUSH_OPT_TO_REMOVE;
+        c = (line)[++(*i)];
+    }
+    if (c == '?')
+    {
+        if (!(line)[*i + 1] || (line)[*i + 1] == '-')
+        {
+            return false;
+        }
+        *flags |= PUSH_OPT_OPTIONAL;
+        c = (line)[++(*i)];
+    }
+
+    size_t len = strlen(&line[*i]);
+    int count = sizeof(updatable_options)/sizeof(char *);
+    for (int j = 0; j < count; ++j)
+    {
+        size_t opt_len = strlen(updatable_options[j]);
+        if (len < opt_len)
+        {
+            continue;
+        }
+        if (!strncmp(&line[*i], updatable_options[j], opt_len)
+            && (!line[*i + opt_len] || line[*i + opt_len] == ' '))
+        {
+            opt_is_updatable = true;
+            break;
+        }
+    }
+
+    if (!opt_is_updatable)
+    {
+        if (*flags & PUSH_OPT_OPTIONAL)
+        {
+            msg(D_PUSH, "Pushed option is not updatable: '%s'. Ignoring.", line);
+        }
+        else
+        {
+            msg(M_WARN, "Pushed option is not updatable: '%s'. Restarting.", line);
+            return false;
+        }
+    }
+
+    return true;
+}
+
+bool
+apply_pull_filter(const struct options *o, char *line)
+{
+    if (!o->pull_filter_list)
+    {
+        return true;
+    }
+
+    struct pull_filter *f;
+
+    for (f = o->pull_filter_list->head; f; f = f->next)
+    {
+        if (f->type == PUF_TYPE_ACCEPT && strncmp(line, f->pattern, f->size) == 0)
+        {
+            msg(D_LOW, "Pushed option accepted by filter: '%s'", line);
+            return true;
+        }
+        else if (f->type == PUF_TYPE_IGNORE && strncmp(line, f->pattern, f->size) == 0)
+        {
+            msg(D_PUSH, "Pushed option removed by filter: '%s'", line);
+            *line = '\0';
+            return true;
+        }
+        else if (f->type == PUF_TYPE_REJECT && strncmp(line, f->pattern, f->size) == 0)
+        {
+            msg(M_WARN, "Pushed option rejected by filter: '%s'.", line);
+            return false;
+        }
+    }
+    return true;
+}
diff --git a/src/openvpn/options_util.h b/src/openvpn/options_util.h
index 2474e7f..71d51d6 100644
--- a/src/openvpn/options_util.h
+++ b/src/openvpn/options_util.h
@@ -50,4 +50,47 @@
 int
 atoi_warn(const char *str, int msglevel);
 
+/**
+ * Filter an option line by all pull filters.
+ *
+ * If a match is found, the line is modified depending on
+ * the filter type, and returns true. If the filter type is
+ * reject, SIGUSR1 is triggered and the return value is false.
+ * In that case the caller must end the push processing.
+ */
+bool
+apply_pull_filter(const struct options *o,
+                  char *line);
+
+/**
+ * @brief Checks the formatting and validity of options inside push-update messages.
+ *
+ * This function is used to validate and process options
+ * in push-update messages. It performs the following checks:
+ * - Determines if the options are updatable.
+ * - Checks for the presence of the `-` flag, which indicates that the option
+ *   should be removed.
+ * - Checks for the `?` flag, which marks the option as optional and suppresses
+ *   errors if the client cannot update it.
+ * - Increase the value pointed by 'i' when we encounter the `'-'` and `'?'` flags
+ *   after validating them and updating the appropriate flags in the `flags` variable.
+ * - `-?option`, `-option`, `?option` are valid formats, `?-option` is not a valid format.
+ * - If the flags and the option are not consecutive, the option is invalid:
+ *   `- ?option`, `-? option`, `- option` are invalid formats.
+ *
+ * @param line A pointer to an option string. This string is the option being validated.
+ * @param i A pointer to an integer that represents the current index in the `line` string.
+ * @param flags A pointer where flags will be stored:
+ *              - `PUSH_OPT_TO_REMOVE`: Set if the `-` flag is present.
+ *              - `PUSH_OPT_OPTIONAL`: Set if the `?` flag is present.
+ *
+ * @return true if the flags and option combination are valid.
+ * @return false if:
+ *         - The `-` and `?` flags are not formatted correctly.
+ *         - The `line` parameter is empty or `NULL`.
+ *         - The `?` flag is absent and the option is not updatable.
+ */
+bool
+check_push_update_option_flags(char *line, int *i, unsigned int *flags);
+
 #endif /* ifndef OPTIONS_UTIL_H_ */
diff --git a/src/openvpn/push.c b/src/openvpn/push.c
index ad8fa3d7..858b821 100644
--- a/src/openvpn/push.c
+++ b/src/openvpn/push.c
@@ -39,8 +39,6 @@
 #include "ssl_util.h"
 #include "options_util.h"
 
-static char push_reply_cmd[] = "PUSH_REPLY";
-
 /*
  * Auth username/password
  *
@@ -519,21 +517,31 @@
     {
         msg(D_PUSH_ERRORS, "WARNING: Received bad push/pull message: %s", sanitize_control_message(BSTR(buffer), &gc));
     }
-    else if (status == PUSH_MSG_REPLY || status == PUSH_MSG_CONTINUATION)
+    else if (status == PUSH_MSG_REPLY || status == PUSH_MSG_UPDATE || status == PUSH_MSG_CONTINUATION)
     {
         c->options.push_option_types_found |= option_types_found;
 
         /* delay bringing tun/tap up until --push parms received from remote */
-        if (status == PUSH_MSG_REPLY)
+        if (status == PUSH_MSG_REPLY || status == PUSH_MSG_UPDATE)
         {
             if (!options_postprocess_pull(&c->options, c->c2.es))
             {
                 goto error;
             }
-            if (!do_up(c, true, c->options.push_option_types_found))
+            if (status == PUSH_MSG_REPLY)
             {
-                msg(D_PUSH_ERRORS, "Failed to open tun/tap interface");
-                goto error;
+                if (!do_up(c, true, c->options.push_option_types_found))
+                {
+                    msg(D_PUSH_ERRORS, "Failed to open tun/tap interface");
+                    goto error;
+                }
+            }
+            else
+            {
+                if (!option_types_found)
+                {
+                    msg(M_WARN, "No updatable options found in incoming PUSH_UPDATE message");
+                }
             }
         }
         event_timeout_clear(&c->c2.push_request_interval);
@@ -1056,11 +1064,13 @@
             md_ctx_init(c->c2.pulled_options_state, "SHA256");
             c->c2.pulled_options_digest_init_done = true;
         }
-        if (apply_push_options(&c->options,
+        if (apply_push_options(c,
+                               &c->options,
                                buf,
                                permission_mask,
                                option_types_found,
-                               c->c2.es))
+                               c->c2.es,
+                               false))
         {
             push_update_digest(c->c2.pulled_options_state, &buf_orig,
                                &c->options);
@@ -1111,6 +1121,12 @@
         return process_incoming_push_reply(c, permission_mask,
                                            option_types_found, &buf);
     }
+    else if (honor_received_options
+             && buf_string_compare_advance(&buf, push_update_cmd))
+    {
+        return process_incoming_push_update(c, permission_mask,
+                                            option_types_found, &buf);
+    }
     else
     {
         return PUSH_MSG_ERROR;
diff --git a/src/openvpn/push.h b/src/openvpn/push.h
index 048b4c4..18dfcd8 100644
--- a/src/openvpn/push.h
+++ b/src/openvpn/push.h
@@ -33,9 +33,42 @@
 #define PUSH_MSG_AUTH_FAILURE     4
 #define PUSH_MSG_CONTINUATION     5
 #define PUSH_MSG_ALREADY_REPLIED  6
+#define PUSH_MSG_UPDATE           7
+
+#define push_reply_cmd "PUSH_REPLY"
+#define push_update_cmd "PUSH_UPDATE"
+
+/* Push-update options flags */
+#define PUSH_OPT_TO_REMOVE (1<<0)
+#define PUSH_OPT_OPTIONAL (1<<1)
 
 int process_incoming_push_request(struct context *c);
 
+/**
+ * @brief Handles the receiving of a push-update message and applies updates to the specified options.
+ *
+ * This function processes a push-update message, validating its content and applying updates
+ * to the options specified in the message. It also handles split messages if the complete
+ * message has not yet been received.
+ *
+ * @param c The context for the operation.
+ * @param permission_mask The permission mask specifying which options are allowed to be pulled.
+ * @param option_types_found A pointer to a variable that will be filled with the types of options
+ *                           found in the message.
+ * @param buf A buffer containing the received message.
+ *
+ * @return
+ * - `PUSH_MSG_UPDATE`: The message was processed successfully, and the updates were applied.
+ * - `PUSH_MSG_CONTINUATION`: The message is a fragment of a larger message, and the program is
+ *                            waiting for the final part.
+ * - `PUSH_MSG_ERROR`: An error occurred during message processing, or the message is invalid.
+ */
+
+int process_incoming_push_update(struct context *c,
+                                 unsigned int permission_mask,
+                                 unsigned int *option_types_found,
+                                 struct buffer *buf);
+
 int process_incoming_push_msg(struct context *c,
                               const struct buffer *buffer,
                               bool honor_received_options,
diff --git a/src/openvpn/push_util.c b/src/openvpn/push_util.c
new file mode 100644
index 0000000..b4d1e8b
--- /dev/null
+++ b/src/openvpn/push_util.c
@@ -0,0 +1,44 @@
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
+#include "push.h"
+
+int
+process_incoming_push_update(struct context *c,
+                             unsigned int permission_mask,
+                             unsigned int *option_types_found,
+                             struct buffer *buf)
+{
+    int ret = PUSH_MSG_ERROR;
+    const uint8_t ch = buf_read_u8(buf);
+    if (ch == ',')
+    {
+        if (apply_push_options(c,
+                               &c->options,
+                               buf,
+                               permission_mask,
+                               option_types_found,
+                               c->c2.es,
+                               true))
+        {
+            switch (c->options.push_continuation)
+            {
+                case 0:
+                case 1:
+                    ret = PUSH_MSG_UPDATE;
+                    break;
+
+                case 2:
+                    ret = PUSH_MSG_CONTINUATION;
+                    break;
+            }
+        }
+    }
+    else if (ch == '\0')
+    {
+        ret = PUSH_MSG_UPDATE;
+    }
+
+    return ret;
+}
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 9c6616a..a7658dd 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -1993,6 +1993,9 @@
         /* support for exit notify via control channel */
         iv_proto |= IV_PROTO_CC_EXIT_NOTIFY;
 
+        /* support push-updates */
+        iv_proto |= IV_PROTO_PUSH_UPDATE;
+
         if (session->opt->pull)
         {
             /* support for receiving push_reply before sending
diff --git a/src/openvpn/ssl.h b/src/openvpn/ssl.h
index 000992d..0a43ea1 100644
--- a/src/openvpn/ssl.h
+++ b/src/openvpn/ssl.h
@@ -114,6 +114,9 @@
 /** Supports the --dns option after all the incompatible changes */
 #define IV_PROTO_DNS_OPTION_V2   (1<<11)
 
+/** Supports push-update */
+#define IV_PROTO_PUSH_UPDATE     (1<<12)
+
 /* Default field in X509 to be username */
 #define X509_USERNAME_FIELD_DEFAULT "CN"
 
diff --git a/tests/unit_tests/openvpn/Makefile.am b/tests/unit_tests/openvpn/Makefile.am
index b47b495..b24e03c 100644
--- a/tests/unit_tests/openvpn/Makefile.am
+++ b/tests/unit_tests/openvpn/Makefile.am
@@ -11,7 +11,7 @@
 endif
 
 test_binaries += crypto_testdriver packet_id_testdriver auth_token_testdriver ncp_testdriver misc_testdriver \
-	pkt_testdriver ssl_testdriver user_pass_testdriver
+	pkt_testdriver ssl_testdriver user_pass_testdriver push_update_msg_testdriver
 
 if HAVE_LD_WRAP_SUPPORT
 if !WIN32
@@ -326,3 +326,21 @@
 	$(top_srcdir)/src/openvpn/win32-util.c \
 	$(top_srcdir)/src/openvpn/platform.c \
 	$(top_srcdir)/src/openvpn/list.c
+
+push_update_msg_testdriver_CFLAGS = -I$(top_srcdir)/src/openvpn \
+	-I$(top_srcdir)/src/compat \
+	-I$(top_srcdir)/tests/unit_tests/openvpn \
+	@TEST_CFLAGS@
+
+push_update_msg_testdriver_LDFLAGS = \
+	@TEST_LDFLAGS@ \
+	-L$(top_srcdir)/src/openvpn
+
+push_update_msg_testdriver_SOURCES = test_push_update_msg.c \
+	mock_msg.c \
+	mock_get_random.c \
+	$(top_srcdir)/src/openvpn/buffer.c \
+	$(top_srcdir)/src/openvpn/platform.c \
+	$(top_srcdir)/src/openvpn/push_util.c \
+	$(top_srcdir)/src/openvpn/options_util.c \
+	$(top_srcdir)/src/openvpn/otime.c
\ No newline at end of file
diff --git a/tests/unit_tests/openvpn/test_push_update_msg.c b/tests/unit_tests/openvpn/test_push_update_msg.c
new file mode 100644
index 0000000..d47286a
--- /dev/null
+++ b/tests/unit_tests/openvpn/test_push_update_msg.c
@@ -0,0 +1,260 @@
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
+#include <stdlib.h>
+#include <stdarg.h>
+#include <setjmp.h>
+#include <cmocka.h>
+#include "push.h"
+#include "options_util.h"
+
+/* mocks */
+
+unsigned int
+pull_permission_mask(const struct context *c)
+{
+    unsigned int flags =
+        OPT_P_UP
+        | OPT_P_ROUTE_EXTRAS
+        | OPT_P_SOCKBUF
+        | OPT_P_SOCKFLAGS
+        | OPT_P_SETENV
+        | OPT_P_SHAPER
+        | OPT_P_TIMER
+        | OPT_P_COMP
+        | OPT_P_PERSIST
+        | OPT_P_MESSAGES
+        | OPT_P_EXPLICIT_NOTIFY
+        | OPT_P_ECHO
+        | OPT_P_PULL_MODE
+        | OPT_P_PEER_ID
+        | OPT_P_NCP
+        | OPT_P_PUSH_MTU
+        | OPT_P_ROUTE
+        | OPT_P_DHCPDNS;
+    return flags;
+}
+
+bool
+apply_push_options(struct context *c,
+                   struct options *options,
+                   struct buffer *buf,
+                   unsigned int permission_mask,
+                   unsigned int *option_types_found,
+                   struct env_set *es,
+                   bool is_update)
+{
+    char line[OPTION_PARM_SIZE];
+
+    while (buf_parse(buf, ',', line, sizeof(line)))
+    {
+        unsigned int push_update_option_flags = 0;
+        int i = 0;
+
+        if (is_update || options->pull_filter_list)
+        {
+            /* skip leading spaces matching the behaviour of parse_line */
+            while (isspace(line[i]))
+            {
+                i++;
+            }
+
+            if ((is_update && !check_push_update_option_flags(line, &i, &push_update_option_flags))
+                || (options->pull_filter_list && !apply_pull_filter(options, &line[i])))
+            {
+                if (push_update_option_flags & PUSH_OPT_OPTIONAL)
+                {
+                    continue; /* Ignoring this option */
+                }
+                msg(M_WARN, "Offending option received from server");
+                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)
+         */
+    }
+    return true;
+}
+
+int
+process_incoming_push_msg(struct context *c,
+                          const struct buffer *buffer,
+                          bool honor_received_options,
+                          unsigned int permission_mask,
+                          unsigned int *option_types_found)
+{
+    struct buffer buf = *buffer;
+
+    if (buf_string_compare_advance(&buf, "PUSH_REQUEST"))
+    {
+        return PUSH_MSG_REQUEST;
+    }
+    else if (honor_received_options
+             && buf_string_compare_advance(&buf, push_reply_cmd))
+    {
+        return PUSH_MSG_REPLY;
+    }
+    else if (honor_received_options
+             && buf_string_compare_advance(&buf, push_update_cmd))
+    {
+        return process_incoming_push_update(c, permission_mask,
+                                            option_types_found, &buf);
+    }
+    else
+    {
+        return PUSH_MSG_ERROR;
+    }
+}
+
+/* tests */
+
+static void
+test_incoming_push_message_basic(void **state)
+{
+    struct context *c = *state;
+    struct buffer buf = alloc_buf(256);
+    const char *update_msg = "PUSH_UPDATE,dhcp-option DNS 8.8.8.8, route 0.0.0.0 0.0.0.0 10.10.10.1";
+    buf_write(&buf, update_msg, strlen(update_msg));
+    unsigned int option_types_found = 0;
+
+    assert_int_equal(process_incoming_push_msg(c, &buf, c->options.pull, pull_permission_mask(c), &option_types_found), PUSH_MSG_UPDATE);
+
+    free_buf(&buf);
+}
+
+static void
+test_incoming_push_message_error1(void **state)
+{
+    struct context *c = *state;
+    struct buffer buf = alloc_buf(256);
+    const char *update_msg = "PUSH_UPDATEerr,dhcp-option DNS 8.8.8.8";
+    buf_write(&buf, update_msg, strlen(update_msg));
+    unsigned int option_types_found = 0;
+
+    assert_int_equal(process_incoming_push_msg(c, &buf, c->options.pull, pull_permission_mask(c), &option_types_found), PUSH_MSG_ERROR);
+
+    free_buf(&buf);
+}
+
+
+static void
+test_incoming_push_message_error2(void **state)
+{
+    struct context *c = *state;
+    struct buffer buf = alloc_buf(256);
+    const char *update_msg = "PUSH_UPDATE ,dhcp-option DNS 8.8.8.8";
+    buf_write(&buf, update_msg, strlen(update_msg));
+    unsigned int option_types_found = 0;
+
+    assert_int_equal(process_incoming_push_msg(c, &buf, c->options.pull, pull_permission_mask(c), &option_types_found), PUSH_MSG_ERROR);
+
+    free_buf(&buf);
+}
+
+static void
+test_incoming_push_message_1(void **state)
+{
+    struct context *c = *state;
+    struct buffer buf = alloc_buf(256);
+    const char *update_msg = "PUSH_UPDATE, -?dns, route something, ?dhcp-option DNS 8.8.8.8";
+    buf_write(&buf, update_msg, strlen(update_msg));
+    unsigned int option_types_found = 0;
+
+    assert_int_equal(process_incoming_push_msg(c, &buf, c->options.pull, pull_permission_mask(c), &option_types_found), PUSH_MSG_UPDATE);
+
+    free_buf(&buf);
+}
+
+static void
+test_incoming_push_message_bad_format(void **state)
+{
+    struct context *c = *state;
+    struct buffer buf = alloc_buf(256);
+    const char *update_msg = "PUSH_UPDATE, -dhcp-option, ?-dns";
+    buf_write(&buf, update_msg, strlen(update_msg));
+    unsigned int option_types_found = 0;
+
+    assert_int_equal(process_incoming_push_msg(c, &buf, c->options.pull, pull_permission_mask(c), &option_types_found), PUSH_MSG_ERROR);
+
+    free_buf(&buf);
+}
+
+static void
+test_incoming_push_message_not_updatable_option(void **state)
+{
+    struct context *c = *state;
+    struct buffer buf = alloc_buf(256);
+    const char *update_msg = "PUSH_UPDATE, dev tun";
+    buf_write(&buf, update_msg, strlen(update_msg));
+    unsigned int option_types_found = 0;
+
+    assert_int_equal(process_incoming_push_msg(c, &buf, c->options.pull, pull_permission_mask(c), &option_types_found), PUSH_MSG_ERROR);
+
+    free_buf(&buf);
+}
+
+static void
+test_incoming_push_message_mix(void **state)
+{
+    struct context *c = *state;
+    struct buffer buf = alloc_buf(256);
+    const char *update_msg = "PUSH_UPDATE,-dhcp-option, route 10.10.10.0, dhcp-option DNS 1.1.1.1, route 10.11.12.0, dhcp-option DOMAIN corp.local, keepalive 10 60";
+    buf_write(&buf, update_msg, strlen(update_msg));
+    unsigned int option_types_found = 0;
+
+    assert_int_equal(process_incoming_push_msg(c, &buf, c->options.pull, pull_permission_mask(c), &option_types_found), PUSH_MSG_UPDATE);
+
+    free_buf(&buf);
+}
+
+static void
+test_incoming_push_message_mix2(void **state)
+{
+    struct context *c = *state;
+    struct buffer buf = alloc_buf(256);
+    const char *update_msg = "PUSH_UPDATE,-dhcp-option,dhcp-option DNS 8.8.8.8,redirect-gateway local,route 192.168.1.0 255.255.255.0";
+    buf_write(&buf, update_msg, strlen(update_msg));
+    unsigned int option_types_found = 0;
+
+    assert_int_equal(process_incoming_push_msg(c, &buf, c->options.pull, pull_permission_mask(c), &option_types_found), PUSH_MSG_UPDATE);
+
+    free_buf(&buf);
+}
+
+static int
+setup(void **state)
+{
+    struct context *c = calloc(1, sizeof(struct context));
+    c->options.pull = true;
+    c->options.route_nopull = false;
+    *state = c;
+    return 0;
+}
+
+static int
+teardown(void **state)
+{
+    struct context *c = *state;
+    free(c);
+    return 0;
+}
+
+int
+main(void)
+{
+    const struct CMUnitTest tests[] = {
+        cmocka_unit_test_setup_teardown(test_incoming_push_message_basic, setup, teardown),
+        cmocka_unit_test_setup_teardown(test_incoming_push_message_error1, setup, teardown),
+        cmocka_unit_test_setup_teardown(test_incoming_push_message_error2, setup, teardown),
+        cmocka_unit_test_setup_teardown(test_incoming_push_message_not_updatable_option, setup, teardown),
+        cmocka_unit_test_setup_teardown(test_incoming_push_message_1, setup, teardown),
+        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)
+    };
+
+    return cmocka_run_group_tests(tests, NULL, NULL);
+}

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/808?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I6ecd4cb47571cc8c20e46de8595c742aeec6064a
Gerrit-Change-Number: 808
Gerrit-PatchSet: 23
Gerrit-Owner: mrbff <ma...@ma...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-Reviewer: stipa <lst...@gm...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-MessageType: merged

[Openvpn-devel] [L] Change in openvpn[master]: PUSH_UPDATE: Allow OpenVPN in client mode to receive and handle PUSH ...

[cron2 (C. Review) <ge...@op...>]

cron2 has uploaded a new patch set (#23) to the change originally created by mrbff. ( http://gerrit.openvpn.net/c/openvpn/+/808?usp=email )

The following approvals got outdated and were removed:
Code-Review+2 by cron2


Change subject: PUSH_UPDATE: Allow OpenVPN in client mode to receive and handle PUSH UPDATE control messages to allow options updating at runtime.
......................................................................

PUSH_UPDATE: Allow OpenVPN in client mode to receive and handle PUSH UPDATE control messages to allow options updating at runtime.

* Added IV_PROTO_PUSH_UPDATE flag bit to support push-updates.
* Added process_incoming_push_update(), in a separate file to create tests more easily.
* Modified incoming_push_message(), process_incoming_push_msg(), apply_push_options(),
  apply_pull_filter() to process also push-update messages.
* Added the check_push_update_option_flags() function used in apply_pull_filter() to
  check options formatting inside push-update messages, if the options are updatables
  and to check for '?' and '-' flags that may be present in front of the options.
  The '-' flag is used to indicate that the option in question should be removed,
  while the '?' indicates that the option is optional and to do not generate
  errors if the client cannot update that option.
  For more info you can read the RFC at https://github.com/OpenVPN/openvpn-rfc .
* Created some unit tests for the push-update message handling in test_push_update_msg.c.

Change-Id: I6ecd4cb47571cc8c20e46de8595c742aeec6064a
Signed-off-by: Marco Baffo <ma...@ma...>
Acked-by: Gert Doering <ge...@gr...>
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg32406.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M CMakeLists.txt
M src/openvpn/Makefile.am
M src/openvpn/forward.c
M src/openvpn/init.c
M src/openvpn/options.c
M src/openvpn/options.h
M src/openvpn/options_util.c
M src/openvpn/options_util.h
M src/openvpn/push.c
M src/openvpn/push.h
A src/openvpn/push_util.c
M src/openvpn/ssl.c
M src/openvpn/ssl.h
M tests/unit_tests/openvpn/Makefile.am
A tests/unit_tests/openvpn/test_push_update_msg.c
15 files changed, 625 insertions(+), 88 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/08/808/23

diff --git a/CMakeLists.txt b/CMakeLists.txt
index efb2d2d..3866e21 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -534,6 +534,7 @@
     src/openvpn/ps.c
     src/openvpn/ps.h
     src/openvpn/push.c
+    src/openvpn/push_util.c
     src/openvpn/push.h
     src/openvpn/pushlist.h
     src/openvpn/reflect_filter.c
@@ -652,6 +653,7 @@
         "test_provider"
         "test_ssl"
         "test_user_pass"
+        "test_push_update_msg"
         )
 
     if (WIN32)
@@ -854,6 +856,14 @@
         src/openvpn/run_command.c
         )
 
+    target_sources(test_push_update_msg PRIVATE
+        tests/unit_tests/openvpn/mock_msg.c
+        tests/unit_tests/openvpn/mock_get_random.c
+	    src/openvpn/push_util.c
+	    src/openvpn/options_util.c
+	    src/openvpn/otime.c
+        )
+
     if (TARGET test_argv)
         target_link_options(test_argv PRIVATE -Wl,--wrap=parse_line)
         target_sources(test_argv PRIVATE
diff --git a/src/openvpn/Makefile.am b/src/openvpn/Makefile.am
index a5d4cdc..1e17c3f 100644
--- a/src/openvpn/Makefile.am
+++ b/src/openvpn/Makefile.am
@@ -117,7 +117,7 @@
 	proto.c proto.h \
 	proxy.c proxy.h \
 	ps.c ps.h \
-	push.c push.h \
+	push.c push_util.c push.h \
 	pushlist.h \
 	reflect_filter.c reflect_filter.h \
 	reliable.c reliable.h \
diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c
index dfc6708..475d142 100644
--- a/src/openvpn/forward.c
+++ b/src/openvpn/forward.c
@@ -2064,7 +2064,7 @@
     }
 
     /* check for incoming control messages on the control channel like
-     * push request/reply, or authentication failure and 2FA messages */
+     * push request/reply/update, or authentication failure and 2FA messages */
     if (tls_test_payload_len(c->c2.tls_multi) > 0)
     {
         check_incoming_control_channel(c);
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index aac8a6a..ba1dda4 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -2051,8 +2051,8 @@
         /* possibly add routes */
         if ((route_order(c->c1.tuntap) == ROUTE_AFTER_TUN) && (!c->options.route_delay_defined))
         {
-            int status = do_route(&c->options, c->c1.route_list, c->c1.route_ipv6_list,
-                                  c->c1.tuntap, c->plugins, c->c2.es, &c->net_ctx);
+            bool status = do_route(&c->options, c->c1.route_list, c->c1.route_ipv6_list,
+                                   c->c1.tuntap, c->plugins, c->c2.es, &c->net_ctx);
             *error_flags |= (status ? 0 : ISC_ROUTE_ERRORS);
         }
 
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index d44c102..eb53a60 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -66,6 +66,7 @@
 #include <ctype.h>
 
 #include "memdbg.h"
+#include "options_util.h"
 
 const char title_string[] =
     PACKAGE_STRING
@@ -948,24 +949,6 @@
     }
 }
 
-struct pull_filter
-{
-#define PUF_TYPE_UNDEF  0    /**< undefined filter type */
-#define PUF_TYPE_ACCEPT 1    /**< filter type to accept a matching option */
-#define PUF_TYPE_IGNORE 2    /**< filter type to ignore a matching option */
-#define PUF_TYPE_REJECT 3    /**< filter type to reject and trigger SIGUSR1 */
-    int type;
-    int size;
-    char *pattern;
-    struct pull_filter *next;
-};
-
-struct pull_filter_list
-{
-    struct pull_filter *head;
-    struct pull_filter *tail;
-};
-
 #ifndef ENABLE_SMALL
 
 static const char *
@@ -5503,60 +5486,14 @@
     }
 }
 
-/**
- * Filter an option line by all pull filters.
- *
- * If a match is found, the line is modified depending on
- * the filter type, and returns true. If the filter type is
- * reject, SIGUSR1 is triggered and the return value is false.
- * In that case the caller must end the push processing.
- */
-static bool
-apply_pull_filter(const struct options *o, char *line)
-{
-    struct pull_filter *f;
-
-    if (!o->pull_filter_list)
-    {
-        return true;
-    }
-
-    /* skip leading spaces matching the behaviour of parse_line */
-    while (isspace(*line))
-    {
-        line++;
-    }
-
-    for (f = o->pull_filter_list->head; f; f = f->next)
-    {
-        if (f->type == PUF_TYPE_ACCEPT && strncmp(line, f->pattern, f->size) == 0)
-        {
-            msg(D_LOW, "Pushed option accepted by filter: '%s'", line);
-            return true;
-        }
-        else if (f->type == PUF_TYPE_IGNORE && strncmp(line, f->pattern, f->size) == 0)
-        {
-            msg(D_PUSH, "Pushed option removed by filter: '%s'", line);
-            *line = '\0';
-            return true;
-        }
-        else if (f->type == PUF_TYPE_REJECT && strncmp(line, f->pattern, f->size) == 0)
-        {
-            msg(M_WARN, "Pushed option rejected by filter: '%s'. Restarting.", line);
-            *line = '\0';
-            throw_signal_soft(SIGUSR1, "Offending option received from server");
-            return false;
-        }
-    }
-    return true;
-}
-
 bool
-apply_push_options(struct options *options,
+apply_push_options(struct context *c,
+                   struct options *options,
                    struct buffer *buf,
                    unsigned int permission_mask,
                    unsigned int *option_types_found,
-                   struct env_set *es)
+                   struct env_set *es,
+                   bool is_update)
 {
     char line[OPTION_PARM_SIZE];
     int line_num = 0;
@@ -5568,14 +5505,40 @@
         char *p[MAX_PARMS+1];
         CLEAR(p);
         ++line_num;
-        if (!apply_pull_filter(options, line))
+        unsigned int push_update_option_flags = 0;
+        int i = 0;
+
+        /* skip leading spaces matching the behaviour of parse_line */
+        while (isspace(line[i]))
         {
+            i++;
+        }
+
+        /* If we are not in a 'PUSH_UPDATE' we just check `apply_pull_filter()`
+         * otherwise we must call `check_push_update_option_flags()` first
+         */
+        if ((is_update && !check_push_update_option_flags(line, &i, &push_update_option_flags))
+            || !apply_pull_filter(options, &line[i]))
+        {
+            /* In case we are in a `PUSH_UPDATE` and `check_push_update_option_flags()`
+             * or `apply_pull_filter()` fail but the option is flagged by `PUSH_OPT_OPTIONAL`,
+             * instead of restarting, we just ignore the option and we process the next one
+             */
+            if (push_update_option_flags & PUSH_OPT_OPTIONAL)
+            {
+                continue; /* Ignoring this option */
+            }
+            throw_signal_soft(SIGUSR1, "Offending option received from server");
             return false; /* Cause push/pull error and stop push processing */
         }
-        if (parse_line(line, p, SIZE(p)-1, file, line_num, msglevel, &options->gc))
+
+        if (parse_line(&line[i], p, SIZE(p)-1, file, line_num, msglevel, &options->gc))
         {
-            add_option(options, p, false, file, line_num, 0, msglevel,
-                       permission_mask, option_types_found, es);
+            if (!is_update)
+            {
+                add_option(options, p, false, file, line_num, 0, msglevel,
+                           permission_mask, option_types_found, es);
+            }
         }
     }
     return true;
diff --git a/src/openvpn/options.h b/src/openvpn/options.h
index 56e85d7..0544ca9 100644
--- a/src/openvpn/options.h
+++ b/src/openvpn/options.h
@@ -794,6 +794,33 @@
 #define MAN_CLIENT_AUTH_ENABLED(opt) (false)
 #endif
 
+/*
+ * some PUSH_UPDATE options
+ */
+#define OPT_P_U_ROUTE           (1<<0)
+#define OPT_P_U_ROUTE6          (1<<1)
+#define OPT_P_U_DNS             (1<<2)
+#define OPT_P_U_DHCP            (1<<3)
+#define OPT_P_U_REDIR_GATEWAY   (1<<4)
+
+struct pull_filter
+{
+#define PUF_TYPE_UNDEF  0    /**< undefined filter type */
+#define PUF_TYPE_ACCEPT 1    /**< filter type to accept a matching option */
+#define PUF_TYPE_IGNORE 2    /**< filter type to ignore a matching option */
+#define PUF_TYPE_REJECT 3    /**< filter type to reject and trigger SIGUSR1 */
+    int type;
+    int size;
+    char *pattern;
+    struct pull_filter *next;
+};
+
+struct pull_filter_list
+{
+    struct pull_filter *head;
+    struct pull_filter *tail;
+};
+
 void parse_argv(struct options *options,
                 const int argc,
                 char *argv[],
@@ -862,11 +889,13 @@
 
 void pre_connect_restore(struct options *o, struct gc_arena *gc);
 
-bool apply_push_options(struct options *options,
+bool apply_push_options(struct context *c,
+                        struct options *options,
                         struct buffer *buf,
                         unsigned int permission_mask,
                         unsigned int *option_types_found,
-                        struct env_set *es);
+                        struct env_set *es,
+                        bool is_update);
 
 void options_detach(struct options *o);
 
diff --git a/src/openvpn/options_util.c b/src/openvpn/options_util.c
index 80d0c08..d317c1a 100644
--- a/src/openvpn/options_util.c
+++ b/src/openvpn/options_util.c
@@ -30,6 +30,8 @@
 
 #include "options_util.h"
 
+#include "push.h"
+
 const char *
 parse_auth_failed_temp(struct options *o, const char *reason)
 {
@@ -145,3 +147,116 @@
 
     return (int) i;
 }
+
+static const char *updatable_options[] = {
+    "block-ipv6",
+    "block-outside-dns",
+    "dhcp-option",
+    "dns",
+    "ifconfig",
+    "ifconfig-ipv6",
+    "push-continuation",
+    "redirect-gateway",
+    "redirect-private",
+    "route",
+    "route-gateway",
+    "route-ipv6",
+    "route-metric",
+    "topology",
+    "tun-mtu",
+    "keepalive"
+};
+
+bool
+check_push_update_option_flags(char *line, int *i, unsigned int *flags)
+{
+    *flags = 0;
+    bool opt_is_updatable = false;
+    char c = line[*i];
+
+    /* We check for '?' and '-' and
+     * if they are present we skip them.
+     */
+    if (c == '-')
+    {
+        if (!(line)[*i + 1])
+        {
+            return false;
+        }
+        *flags |= PUSH_OPT_TO_REMOVE;
+        c = (line)[++(*i)];
+    }
+    if (c == '?')
+    {
+        if (!(line)[*i + 1] || (line)[*i + 1] == '-')
+        {
+            return false;
+        }
+        *flags |= PUSH_OPT_OPTIONAL;
+        c = (line)[++(*i)];
+    }
+
+    size_t len = strlen(&line[*i]);
+    int count = sizeof(updatable_options)/sizeof(char *);
+    for (int j = 0; j < count; ++j)
+    {
+        size_t opt_len = strlen(updatable_options[j]);
+        if (len < opt_len)
+        {
+            continue;
+        }
+        if (!strncmp(&line[*i], updatable_options[j], opt_len)
+            && (!line[*i + opt_len] || line[*i + opt_len] == ' '))
+        {
+            opt_is_updatable = true;
+            break;
+        }
+    }
+
+    if (!opt_is_updatable)
+    {
+        if (*flags & PUSH_OPT_OPTIONAL)
+        {
+            msg(D_PUSH, "Pushed option is not updatable: '%s'. Ignoring.", line);
+        }
+        else
+        {
+            msg(M_WARN, "Pushed option is not updatable: '%s'. Restarting.", line);
+            return false;
+        }
+    }
+
+    return true;
+}
+
+bool
+apply_pull_filter(const struct options *o, char *line)
+{
+    if (!o->pull_filter_list)
+    {
+        return true;
+    }
+
+    struct pull_filter *f;
+
+    for (f = o->pull_filter_list->head; f; f = f->next)
+    {
+        if (f->type == PUF_TYPE_ACCEPT && strncmp(line, f->pattern, f->size) == 0)
+        {
+            msg(D_LOW, "Pushed option accepted by filter: '%s'", line);
+            return true;
+        }
+        else if (f->type == PUF_TYPE_IGNORE && strncmp(line, f->pattern, f->size) == 0)
+        {
+            msg(D_PUSH, "Pushed option removed by filter: '%s'", line);
+            *line = '\0';
+            return true;
+        }
+        else if (f->type == PUF_TYPE_REJECT && strncmp(line, f->pattern, f->size) == 0)
+        {
+            msg(M_WARN, "Pushed option rejected by filter: '%s'.", line);
+            return false;
+        }
+    }
+    return true;
+}
diff --git a/src/openvpn/options_util.h b/src/openvpn/options_util.h
index 2474e7f..71d51d6 100644
--- a/src/openvpn/options_util.h
+++ b/src/openvpn/options_util.h
@@ -50,4 +50,47 @@
 int
 atoi_warn(const char *str, int msglevel);
 
+/**
+ * Filter an option line by all pull filters.
+ *
+ * If a match is found, the line is modified depending on
+ * the filter type, and returns true. If the filter type is
+ * reject, SIGUSR1 is triggered and the return value is false.
+ * In that case the caller must end the push processing.
+ */
+bool
+apply_pull_filter(const struct options *o,
+                  char *line);
+
+/**
+ * @brief Checks the formatting and validity of options inside push-update messages.
+ *
+ * This function is used to validate and process options
+ * in push-update messages. It performs the following checks:
+ * - Determines if the options are updatable.
+ * - Checks for the presence of the `-` flag, which indicates that the option
+ *   should be removed.
+ * - Checks for the `?` flag, which marks the option as optional and suppresses
+ *   errors if the client cannot update it.
+ * - Increase the value pointed by 'i' when we encounter the `'-'` and `'?'` flags
+ *   after validating them and updating the appropriate flags in the `flags` variable.
+ * - `-?option`, `-option`, `?option` are valid formats, `?-option` is not a valid format.
+ * - If the flags and the option are not consecutive, the option is invalid:
+ *   `- ?option`, `-? option`, `- option` are invalid formats.
+ *
+ * @param line A pointer to an option string. This string is the option being validated.
+ * @param i A pointer to an integer that represents the current index in the `line` string.
+ * @param flags A pointer where flags will be stored:
+ *              - `PUSH_OPT_TO_REMOVE`: Set if the `-` flag is present.
+ *              - `PUSH_OPT_OPTIONAL`: Set if the `?` flag is present.
+ *
+ * @return true if the flags and option combination are valid.
+ * @return false if:
+ *         - The `-` and `?` flags are not formatted correctly.
+ *         - The `line` parameter is empty or `NULL`.
+ *         - The `?` flag is absent and the option is not updatable.
+ */
+bool
+check_push_update_option_flags(char *line, int *i, unsigned int *flags);
+
 #endif /* ifndef OPTIONS_UTIL_H_ */
diff --git a/src/openvpn/push.c b/src/openvpn/push.c
index ad8fa3d7..858b821 100644
--- a/src/openvpn/push.c
+++ b/src/openvpn/push.c
@@ -39,8 +39,6 @@
 #include "ssl_util.h"
 #include "options_util.h"
 
-static char push_reply_cmd[] = "PUSH_REPLY";
-
 /*
  * Auth username/password
  *
@@ -519,21 +517,31 @@
     {
         msg(D_PUSH_ERRORS, "WARNING: Received bad push/pull message: %s", sanitize_control_message(BSTR(buffer), &gc));
     }
-    else if (status == PUSH_MSG_REPLY || status == PUSH_MSG_CONTINUATION)
+    else if (status == PUSH_MSG_REPLY || status == PUSH_MSG_UPDATE || status == PUSH_MSG_CONTINUATION)
     {
         c->options.push_option_types_found |= option_types_found;
 
         /* delay bringing tun/tap up until --push parms received from remote */
-        if (status == PUSH_MSG_REPLY)
+        if (status == PUSH_MSG_REPLY || status == PUSH_MSG_UPDATE)
         {
             if (!options_postprocess_pull(&c->options, c->c2.es))
             {
                 goto error;
             }
-            if (!do_up(c, true, c->options.push_option_types_found))
+            if (status == PUSH_MSG_REPLY)
             {
-                msg(D_PUSH_ERRORS, "Failed to open tun/tap interface");
-                goto error;
+                if (!do_up(c, true, c->options.push_option_types_found))
+                {
+                    msg(D_PUSH_ERRORS, "Failed to open tun/tap interface");
+                    goto error;
+                }
+            }
+            else
+            {
+                if (!option_types_found)
+                {
+                    msg(M_WARN, "No updatable options found in incoming PUSH_UPDATE message");
+                }
             }
         }
         event_timeout_clear(&c->c2.push_request_interval);
@@ -1056,11 +1064,13 @@
             md_ctx_init(c->c2.pulled_options_state, "SHA256");
             c->c2.pulled_options_digest_init_done = true;
         }
-        if (apply_push_options(&c->options,
+        if (apply_push_options(c,
+                               &c->options,
                                buf,
                                permission_mask,
                                option_types_found,
-                               c->c2.es))
+                               c->c2.es,
+                               false))
         {
             push_update_digest(c->c2.pulled_options_state, &buf_orig,
                                &c->options);
@@ -1111,6 +1121,12 @@
         return process_incoming_push_reply(c, permission_mask,
                                            option_types_found, &buf);
     }
+    else if (honor_received_options
+             && buf_string_compare_advance(&buf, push_update_cmd))
+    {
+        return process_incoming_push_update(c, permission_mask,
+                                            option_types_found, &buf);
+    }
     else
     {
         return PUSH_MSG_ERROR;
diff --git a/src/openvpn/push.h b/src/openvpn/push.h
index 048b4c4..18dfcd8 100644
--- a/src/openvpn/push.h
+++ b/src/openvpn/push.h
@@ -33,9 +33,42 @@
 #define PUSH_MSG_AUTH_FAILURE     4
 #define PUSH_MSG_CONTINUATION     5
 #define PUSH_MSG_ALREADY_REPLIED  6
+#define PUSH_MSG_UPDATE           7
+
+#define push_reply_cmd "PUSH_REPLY"
+#define push_update_cmd "PUSH_UPDATE"
+
+/* Push-update options flags */
+#define PUSH_OPT_TO_REMOVE (1<<0)
+#define PUSH_OPT_OPTIONAL (1<<1)
 
 int process_incoming_push_request(struct context *c);
 
+/**
+ * @brief Handles the receiving of a push-update message and applies updates to the specified options.
+ *
+ * This function processes a push-update message, validating its content and applying updates
+ * to the options specified in the message. It also handles split messages if the complete
+ * message has not yet been received.
+ *
+ * @param c The context for the operation.
+ * @param permission_mask The permission mask specifying which options are allowed to be pulled.
+ * @param option_types_found A pointer to a variable that will be filled with the types of options
+ *                           found in the message.
+ * @param buf A buffer containing the received message.
+ *
+ * @return
+ * - `PUSH_MSG_UPDATE`: The message was processed successfully, and the updates were applied.
+ * - `PUSH_MSG_CONTINUATION`: The message is a fragment of a larger message, and the program is
+ *                            waiting for the final part.
+ * - `PUSH_MSG_ERROR`: An error occurred during message processing, or the message is invalid.
+ */
+
+int process_incoming_push_update(struct context *c,
+                                 unsigned int permission_mask,
+                                 unsigned int *option_types_found,
+                                 struct buffer *buf);
+
 int process_incoming_push_msg(struct context *c,
                               const struct buffer *buffer,
                               bool honor_received_options,
diff --git a/src/openvpn/push_util.c b/src/openvpn/push_util.c
new file mode 100644
index 0000000..b4d1e8b
--- /dev/null
+++ b/src/openvpn/push_util.c
@@ -0,0 +1,44 @@
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
+#include "push.h"
+
+int
+process_incoming_push_update(struct context *c,
+                             unsigned int permission_mask,
+                             unsigned int *option_types_found,
+                             struct buffer *buf)
+{
+    int ret = PUSH_MSG_ERROR;
+    const uint8_t ch = buf_read_u8(buf);
+    if (ch == ',')
+    {
+        if (apply_push_options(c,
+                               &c->options,
+                               buf,
+                               permission_mask,
+                               option_types_found,
+                               c->c2.es,
+                               true))
+        {
+            switch (c->options.push_continuation)
+            {
+                case 0:
+                case 1:
+                    ret = PUSH_MSG_UPDATE;
+                    break;
+
+                case 2:
+                    ret = PUSH_MSG_CONTINUATION;
+                    break;
+            }
+        }
+    }
+    else if (ch == '\0')
+    {
+        ret = PUSH_MSG_UPDATE;
+    }
+
+    return ret;
+}
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 9c6616a..a7658dd 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -1993,6 +1993,9 @@
         /* support for exit notify via control channel */
         iv_proto |= IV_PROTO_CC_EXIT_NOTIFY;
 
+        /* support push-updates */
+        iv_proto |= IV_PROTO_PUSH_UPDATE;
+
         if (session->opt->pull)
         {
             /* support for receiving push_reply before sending
diff --git a/src/openvpn/ssl.h b/src/openvpn/ssl.h
index 000992d..0a43ea1 100644
--- a/src/openvpn/ssl.h
+++ b/src/openvpn/ssl.h
@@ -114,6 +114,9 @@
 /** Supports the --dns option after all the incompatible changes */
 #define IV_PROTO_DNS_OPTION_V2   (1<<11)
 
+/** Supports push-update */
+#define IV_PROTO_PUSH_UPDATE     (1<<12)
+
 /* Default field in X509 to be username */
 #define X509_USERNAME_FIELD_DEFAULT "CN"
 
diff --git a/tests/unit_tests/openvpn/Makefile.am b/tests/unit_tests/openvpn/Makefile.am
index b47b495..b24e03c 100644
--- a/tests/unit_tests/openvpn/Makefile.am
+++ b/tests/unit_tests/openvpn/Makefile.am
@@ -11,7 +11,7 @@
 endif
 
 test_binaries += crypto_testdriver packet_id_testdriver auth_token_testdriver ncp_testdriver misc_testdriver \
-	pkt_testdriver ssl_testdriver user_pass_testdriver
+	pkt_testdriver ssl_testdriver user_pass_testdriver push_update_msg_testdriver
 
 if HAVE_LD_WRAP_SUPPORT
 if !WIN32
@@ -326,3 +326,21 @@
 	$(top_srcdir)/src/openvpn/win32-util.c \
 	$(top_srcdir)/src/openvpn/platform.c \
 	$(top_srcdir)/src/openvpn/list.c
+
+push_update_msg_testdriver_CFLAGS = -I$(top_srcdir)/src/openvpn \
+	-I$(top_srcdir)/src/compat \
+	-I$(top_srcdir)/tests/unit_tests/openvpn \
+	@TEST_CFLAGS@
+
+push_update_msg_testdriver_LDFLAGS = \
+	@TEST_LDFLAGS@ \
+	-L$(top_srcdir)/src/openvpn
+
+push_update_msg_testdriver_SOURCES = test_push_update_msg.c \
+	mock_msg.c \
+	mock_get_random.c \
+	$(top_srcdir)/src/openvpn/buffer.c \
+	$(top_srcdir)/src/openvpn/platform.c \
+	$(top_srcdir)/src/openvpn/push_util.c \
+	$(top_srcdir)/src/openvpn/options_util.c \
+	$(top_srcdir)/src/openvpn/otime.c
\ No newline at end of file
diff --git a/tests/unit_tests/openvpn/test_push_update_msg.c b/tests/unit_tests/openvpn/test_push_update_msg.c
new file mode 100644
index 0000000..d47286a
--- /dev/null
+++ b/tests/unit_tests/openvpn/test_push_update_msg.c
@@ -0,0 +1,260 @@
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
+#include <stdlib.h>
+#include <stdarg.h>
+#include <setjmp.h>
+#include <cmocka.h>
+#include "push.h"
+#include "options_util.h"
+
+/* mocks */
+
+unsigned int
+pull_permission_mask(const struct context *c)
+{
+    unsigned int flags =
+        OPT_P_UP
+        | OPT_P_ROUTE_EXTRAS
+        | OPT_P_SOCKBUF
+        | OPT_P_SOCKFLAGS
+        | OPT_P_SETENV
+        | OPT_P_SHAPER
+        | OPT_P_TIMER
+        | OPT_P_COMP
+        | OPT_P_PERSIST
+        | OPT_P_MESSAGES
+        | OPT_P_EXPLICIT_NOTIFY
+        | OPT_P_ECHO
+        | OPT_P_PULL_MODE
+        | OPT_P_PEER_ID
+        | OPT_P_NCP
+        | OPT_P_PUSH_MTU
+        | OPT_P_ROUTE
+        | OPT_P_DHCPDNS;
+    return flags;
+}
+
+bool
+apply_push_options(struct context *c,
+                   struct options *options,
+                   struct buffer *buf,
+                   unsigned int permission_mask,
+                   unsigned int *option_types_found,
+                   struct env_set *es,
+                   bool is_update)
+{
+    char line[OPTION_PARM_SIZE];
+
+    while (buf_parse(buf, ',', line, sizeof(line)))
+    {
+        unsigned int push_update_option_flags = 0;
+        int i = 0;
+
+        if (is_update || options->pull_filter_list)
+        {
+            /* skip leading spaces matching the behaviour of parse_line */
+            while (isspace(line[i]))
+            {
+                i++;
+            }
+
+            if ((is_update && !check_push_update_option_flags(line, &i, &push_update_option_flags))
+                || (options->pull_filter_list && !apply_pull_filter(options, &line[i])))
+            {
+                if (push_update_option_flags & PUSH_OPT_OPTIONAL)
+                {
+                    continue; /* Ignoring this option */
+                }
+                msg(M_WARN, "Offending option received from server");
+                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)
+         */
+    }
+    return true;
+}
+
+int
+process_incoming_push_msg(struct context *c,
+                          const struct buffer *buffer,
+                          bool honor_received_options,
+                          unsigned int permission_mask,
+                          unsigned int *option_types_found)
+{
+    struct buffer buf = *buffer;
+
+    if (buf_string_compare_advance(&buf, "PUSH_REQUEST"))
+    {
+        return PUSH_MSG_REQUEST;
+    }
+    else if (honor_received_options
+             && buf_string_compare_advance(&buf, push_reply_cmd))
+    {
+        return PUSH_MSG_REPLY;
+    }
+    else if (honor_received_options
+             && buf_string_compare_advance(&buf, push_update_cmd))
+    {
+        return process_incoming_push_update(c, permission_mask,
+                                            option_types_found, &buf);
+    }
+    else
+    {
+        return PUSH_MSG_ERROR;
+    }
+}
+
+/* tests */
+
+static void
+test_incoming_push_message_basic(void **state)
+{
+    struct context *c = *state;
+    struct buffer buf = alloc_buf(256);
+    const char *update_msg = "PUSH_UPDATE,dhcp-option DNS 8.8.8.8, route 0.0.0.0 0.0.0.0 10.10.10.1";
+    buf_write(&buf, update_msg, strlen(update_msg));
+    unsigned int option_types_found = 0;
+
+    assert_int_equal(process_incoming_push_msg(c, &buf, c->options.pull, pull_permission_mask(c), &option_types_found), PUSH_MSG_UPDATE);
+
+    free_buf(&buf);
+}
+
+static void
+test_incoming_push_message_error1(void **state)
+{
+    struct context *c = *state;
+    struct buffer buf = alloc_buf(256);
+    const char *update_msg = "PUSH_UPDATEerr,dhcp-option DNS 8.8.8.8";
+    buf_write(&buf, update_msg, strlen(update_msg));
+    unsigned int option_types_found = 0;
+
+    assert_int_equal(process_incoming_push_msg(c, &buf, c->options.pull, pull_permission_mask(c), &option_types_found), PUSH_MSG_ERROR);
+
+    free_buf(&buf);
+}
+
+
+static void
+test_incoming_push_message_error2(void **state)
+{
+    struct context *c = *state;
+    struct buffer buf = alloc_buf(256);
+    const char *update_msg = "PUSH_UPDATE ,dhcp-option DNS 8.8.8.8";
+    buf_write(&buf, update_msg, strlen(update_msg));
+    unsigned int option_types_found = 0;
+
+    assert_int_equal(process_incoming_push_msg(c, &buf, c->options.pull, pull_permission_mask(c), &option_types_found), PUSH_MSG_ERROR);
+
+    free_buf(&buf);
+}
+
+static void
+test_incoming_push_message_1(void **state)
+{
+    struct context *c = *state;
+    struct buffer buf = alloc_buf(256);
+    const char *update_msg = "PUSH_UPDATE, -?dns, route something, ?dhcp-option DNS 8.8.8.8";
+    buf_write(&buf, update_msg, strlen(update_msg));
+    unsigned int option_types_found = 0;
+
+    assert_int_equal(process_incoming_push_msg(c, &buf, c->options.pull, pull_permission_mask(c), &option_types_found), PUSH_MSG_UPDATE);
+
+    free_buf(&buf);
+}
+
+static void
+test_incoming_push_message_bad_format(void **state)
+{
+    struct context *c = *state;
+    struct buffer buf = alloc_buf(256);
+    const char *update_msg = "PUSH_UPDATE, -dhcp-option, ?-dns";
+    buf_write(&buf, update_msg, strlen(update_msg));
+    unsigned int option_types_found = 0;
+
+    assert_int_equal(process_incoming_push_msg(c, &buf, c->options.pull, pull_permission_mask(c), &option_types_found), PUSH_MSG_ERROR);
+
+    free_buf(&buf);
+}
+
+static void
+test_incoming_push_message_not_updatable_option(void **state)
+{
+    struct context *c = *state;
+    struct buffer buf = alloc_buf(256);
+    const char *update_msg = "PUSH_UPDATE, dev tun";
+    buf_write(&buf, update_msg, strlen(update_msg));
+    unsigned int option_types_found = 0;
+
+    assert_int_equal(process_incoming_push_msg(c, &buf, c->options.pull, pull_permission_mask(c), &option_types_found), PUSH_MSG_ERROR);
+
+    free_buf(&buf);
+}
+
+static void
+test_incoming_push_message_mix(void **state)
+{
+    struct context *c = *state;
+    struct buffer buf = alloc_buf(256);
+    const char *update_msg = "PUSH_UPDATE,-dhcp-option, route 10.10.10.0, dhcp-option DNS 1.1.1.1, route 10.11.12.0, dhcp-option DOMAIN corp.local, keepalive 10 60";
+    buf_write(&buf, update_msg, strlen(update_msg));
+    unsigned int option_types_found = 0;
+
+    assert_int_equal(process_incoming_push_msg(c, &buf, c->options.pull, pull_permission_mask(c), &option_types_found), PUSH_MSG_UPDATE);
+
+    free_buf(&buf);
+}
+
+static void
+test_incoming_push_message_mix2(void **state)
+{
+    struct context *c = *state;
+    struct buffer buf = alloc_buf(256);
+    const char *update_msg = "PUSH_UPDATE,-dhcp-option,dhcp-option DNS 8.8.8.8,redirect-gateway local,route 192.168.1.0 255.255.255.0";
+    buf_write(&buf, update_msg, strlen(update_msg));
+    unsigned int option_types_found = 0;
+
+    assert_int_equal(process_incoming_push_msg(c, &buf, c->options.pull, pull_permission_mask(c), &option_types_found), PUSH_MSG_UPDATE);
+
+    free_buf(&buf);
+}
+
+static int
+setup(void **state)
+{
+    struct context *c = calloc(1, sizeof(struct context));
+    c->options.pull = true;
+    c->options.route_nopull = false;
+    *state = c;
+    return 0;
+}
+
+static int
+teardown(void **state)
+{
+    struct context *c = *state;
+    free(c);
+    return 0;
+}
+
+int
+main(void)
+{
+    const struct CMUnitTest tests[] = {
+        cmocka_unit_test_setup_teardown(test_incoming_push_message_basic, setup, teardown),
+        cmocka_unit_test_setup_teardown(test_incoming_push_message_error1, setup, teardown),
+        cmocka_unit_test_setup_teardown(test_incoming_push_message_error2, setup, teardown),
+        cmocka_unit_test_setup_teardown(test_incoming_push_message_not_updatable_option, setup, teardown),
+        cmocka_unit_test_setup_teardown(test_incoming_push_message_1, setup, teardown),
+        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)
+    };
+
+    return cmocka_run_group_tests(tests, NULL, NULL);
+}

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/808?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I6ecd4cb47571cc8c20e46de8595c742aeec6064a
Gerrit-Change-Number: 808
Gerrit-PatchSet: 23
Gerrit-Owner: mrbff <ma...@ma...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-Reviewer: stipa <lst...@gm...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-MessageType: newpatchset

[Openvpn-devel] [PATCH applied] Re: PUSH_UPDATE: Allow OpenVPN in client mode to receive and handle PUSH UPDATE control messages to allow options updating at runtime.

[Gert D. <ge...@gr...>]

So, this has been quite a journey... (and thanks, Marco, for keeping your
patience).  Lev has tested and +2'ed v11, which was then lost in the
"rebase / avoid conflict" for v12.  Up to v20, only reviews, and then I
asked for lots of stylistic / code organizational changes which then
brought this up to v22...

I have stared a lot at the code, and I have run the full "normal client
and server side test" bed on it.  I have not actually tested incoming
PUSH_UPDATE messages, so at this point I have good faith that this does
what it is supposed to do - and I'm pretty sure that it does not break
anything.  Lev has given 808+809+810 another test run on the "modified
community server" (thanks!), and Corp QA has been asked to test against
OpenVPN Inc commercial server infrastructure (so we'll hear if it does
not work).

(Staring again, I *did* find one thing we should fix, namely the 
amount of options passed to apply_push_options() now, but this can
be done in a new patch)


My plan for future testing is to integrate this into the t_client/t_server
testbeds, setting up one instance that will do PUSH_UPDATE to send over
the routes that are needed for ping success - but this needs #869 and
quite a bit of new "drive management interface" infrastructure to get
done.  Soon.  *cough*.

Your patch has been applied to the master branch.

commit 80b62545881cbe39e813c85c23984e62aaa76860
Author: Marco Baffo
Date:   Tue Jul 29 12:40:39 2025 +0200

     PUSH_UPDATE: Allow OpenVPN in client mode to receive and handle PUSH UPDATE control messages to allow options updating at runtime.

     Signed-off-by: Marco Baffo <ma...@ma...>
     Acked-by: Gert Doering <ge...@gr...>
     Message-Id: <202...@gr...>
     URL: https://www.mail-archive.com/ope...@li.../msg32406.html
     Signed-off-by: Gert Doering <ge...@gr...>


--
kind regards,

Gert Doering

Re: [Openvpn-devel] [PATCH 1/2] dco-freebsd: pass address scope to the kernel

[Kristof P. <kpr...@ne...>]

On 29 Jul 2025, at 13:07, Gert Doering wrote:
> On Tue, Jul 29, 2025 at 12:55:29PM +0200, Kristof Provost via 
> Openvpn-devel wrote:
>> This requires kernel side support: 
>> [https://reviews.freebsd.org/D51596](https://reviews.freebsd.org/D51596)
>
> I did not yet test it, but I assume that including scope-id in the
> message to an older kernel will not break anything for non-LLA peers?
>
Correct. That’s why we’re using nvlists in the ioctl path. We can 
add fields without confusing either kernel or userspace. They’ll just 
be ignored.

—
Kristof

Re: [Openvpn-devel] [PATCH 1/2] dco-freebsd: pass address scope to the kernel

[Gert D. <ge...@gr...>]

Hi,

On Tue, Jul 29, 2025 at 12:55:29PM +0200, Kristof Provost via Openvpn-devel wrote:
> This requires kernel side support: [https://reviews.freebsd.org/D51596](https://reviews.freebsd.org/D51596)

I did not yet test it, but I assume that including scope-id in the
message to an older kernel will not break anything for non-LLA peers?

(For LLA%scope peers, it will not work today, and won't work without
kernel-side support - this much is clear ;-) ).

gert
-- 
"If was one thing all people took for granted, was conviction that if you 
 feed honest figures into a computer, honest figures come out. Never doubted 
 it myself till I met a computer with a sense of humor."
                             Robert A. Heinlein, The Moon is a Harsh Mistress

Gert Doering - Munich, Germany                             ge...@gr...

Re: [Openvpn-devel] [PATCH 1/2] dco-freebsd: pass address scope to the kernel

[Kristof P. <kpr...@ne...>]

This requires kernel side support: [https://reviews.freebsd.org/D51596](https://reviews.freebsd.org/D51596)


—
Kristof

On 29 Jul 2025, at 11:38, Kristof Provost wrote:
> From: Kristof Provost <kp...@Fr...>
>
> To support link-local (IPv6) addresses we must pass the scope to the kernel as
> well. We should also extract it from the kernel notification for float events.
>
> Signed-off-by: Kristof Provost <kpr...@ne...>
> ---
>  src/openvpn/dco_freebsd.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/src/openvpn/dco_freebsd.c b/src/openvpn/dco_freebsd.c
> index 8add41af..306ecd31 100644
> --- a/src/openvpn/dco_freebsd.c
> +++ b/src/openvpn/dco_freebsd.c
> @@ -62,6 +62,7 @@ sockaddr_to_nvlist(const struct sockaddr *sa)
>              const struct sockaddr_in6 *in6 = (const struct sockaddr_in6 *)sa;
>              nvlist_add_binary(nvl, "address", &in6->sin6_addr, sizeof(in6->sin6_addr));
>              nvlist_add_number(nvl, "port", in6->sin6_port);
> +            nvlist_add_number(nvl, "scopeid", in6->sin6_scope_id);
>              break;
>          }
>
> @@ -118,6 +119,11 @@ nvlist_to_sockaddr(const nvlist_t *nvl, struct sockaddr_storage *ss)
>              assert(len == sizeof(in6->sin6_addr));
>              memcpy(&in6->sin6_addr, data, sizeof(in6->sin6_addr));
>              in6->sin6_port = nvlist_get_number(nvl, "port");
> +
> +            if (nvlist_exists_number(nvl, "scopeid"))
> +            {
> +                in6->sin6_scope_id = nvlist_get_number(nvl, "scopeid");
> +            }
>              break;
>          }
>
> -- 
> 2.50.1

[Openvpn-devel] [PATCH v23] PUSH_UPDATE: Added update_option() function.

[Gert D. <ge...@gr...>]

From: Marco Baffo <ma...@ma...>

When the function receives an option to update, it first checks whether it has
already received an option of the same type within the same update message.
If it has already received it, it simply calls add_option(), otherwise it
deletes all the values ​​already present regarding that option.

Change-Id: Ia45c99e6df7b3ad24020c10b8a9b3577984ecdc2
Signed-off-by: Marco Baffo <ma...@ma...>
Acked-by: Gert Doering <ge...@gr...>
---

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/+/810
This mail reflects revision 23 of this Change.

Acked-by according to Gerrit (reflected above):
Gert Doering <ge...@gr...>

        
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 3a8ce86..f056263 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -5372,6 +5372,20 @@
               struct env_set *es);
 
 static void
+update_option(struct context *c,
+              struct options *options,
+              char *p[],
+              bool is_inline,
+              const char *file,
+              int line,
+              const int level,
+              const int msglevel,
+              const unsigned int permission_mask,
+              unsigned int *option_types_found,
+              struct env_set *es,
+              unsigned int *update_options_found);
+
+static void
 read_config_file(struct options *options,
                  const char *file,
                  int level,
@@ -5557,6 +5571,7 @@
     int line_num = 0;
     const char *file = "[PUSH-OPTIONS]";
     const int msglevel = D_PUSH_ERRORS|M_OPTERR;
+    unsigned int update_options_found = 0;
 
     while (buf_parse(buf, ',', line, sizeof(line)))
     {
@@ -5602,6 +5617,11 @@
                 remove_option(c, options, p, false, file, line_num, msglevel,
                               permission_mask, option_types_found, es);
             }
+            else
+            {
+                update_option(c, options, p, false, file, line_num, 0, msglevel,
+                              permission_mask, option_types_found, es, &update_options_found);
+            }
         }
     }
     return true;
@@ -5740,6 +5760,13 @@
     return options->forward_compatible ? M_WARN : msglevel;
 }
 
+#define RESET_OPTION_ROUTES(option_ptr, field) \
+    if (option_ptr)                        \
+    {                                      \
+        option_ptr->field = NULL;          \
+        option_ptr->flags = 0;             \
+    }
+
 /**
  * @brief Resets options found in the PUSH_UPDATE message that are preceded by the `-` flag.
  *        This function is used in push-updates to reset specified options.
@@ -5794,11 +5821,7 @@
             delete_routes_v4(c->c1.route_list, c->c1.tuntap,
                              ROUTE_OPTION_FLAGS(&c->options),
                              es, &c->net_ctx);
-            if (options->routes)
-            {
-                options->routes->routes = NULL;
-                options->routes->flags = 0;
-            }
+            RESET_OPTION_ROUTES(options->routes, routes);
         }
     }
     else if (streq(p[0], "route-ipv6") && !p[1])
@@ -5809,11 +5832,7 @@
             delete_routes_v6(c->c1.route_ipv6_list, c->c1.tuntap,
                              ROUTE_OPTION_FLAGS(&c->options),
                              es, &c->net_ctx);
-            if (options->routes_ipv6)
-            {
-                options->routes_ipv6->routes_ipv6 = NULL;
-                options->routes_ipv6->flags = 0;
-            }
+            RESET_OPTION_ROUTES(options->routes_ipv6, routes_ipv6);
         }
     }
     else if (streq(p[0], "route-gateway") && !p[1])
@@ -5933,6 +5952,303 @@
     msg(msglevel, "Error occurred trying to remove %s option", p[0]);
 }
 
+
+static bool
+check_route_option(struct options *options, char *p[], const int msglevel, bool pull_mode)
+{
+    rol_check_alloc(options);
+    if (pull_mode)
+    {
+        if (!ip_or_dns_addr_safe(p[1], options->allow_pull_fqdn) && !is_special_addr(p[1])) /* FQDN -- may be DNS name */
+        {
+            msg(msglevel, "route parameter network/IP '%s' must be a valid address", p[1]);
+            return false;
+        }
+        if (p[2] && !ip_addr_dotted_quad_safe(p[2])) /* FQDN -- must be IP address */
+        {
+            msg(msglevel, "route parameter netmask '%s' must be an IP address", p[2]);
+            return false;
+        }
+        if (p[3] && !ip_or_dns_addr_safe(p[3], options->allow_pull_fqdn) && !is_special_addr(p[3])) /* FQDN -- may be DNS name */
+        {
+            msg(msglevel, "route parameter gateway '%s' must be a valid address", p[3]);
+            return false;
+        }
+    }
+    return true;
+}
+
+
+static bool
+check_route6_option(struct options *options, char *p[], const int msglevel, bool pull_mode)
+{
+    rol6_check_alloc(options);
+    if (pull_mode)
+    {
+        if (!ipv6_addr_safe_hexplusbits(p[1]))
+        {
+            msg(msglevel, "route-ipv6 parameter network/IP '%s' must be a valid address", p[1]);
+            return false;
+        }
+        if (p[2] && !ipv6_addr_safe(p[2]))
+        {
+            msg(msglevel, "route-ipv6 parameter gateway '%s' must be a valid address", p[2]);
+            return false;
+        }
+        /* p[3] is metric, if present */
+    }
+    return true;
+}
+
+static bool
+check_dns_option(struct options *options, char *p[], const int msglevel, bool pull_mode)
+{
+    if (streq(p[1], "search-domains") && p[2])
+    {
+        dns_domain_list_append(&options->dns_options.search_domains, &p[2], &options->dns_options.gc);
+    }
+    else if (streq(p[1], "server") && p[2] && p[3] && p[4])
+    {
+        long priority;
+        if (!dns_server_priority_parse(&priority, p[2], pull_mode))
+        {
+            msg(msglevel, "--dns server: invalid priority value '%s'", p[2]);
+            return false;
+        }
+
+        struct dns_server *server = dns_server_get(&options->dns_options.servers, priority, &options->dns_options.gc);
+
+        if (streq(p[3], "address") && p[4])
+        {
+            for (int i = 4; p[i]; ++i)
+            {
+                if (!dns_server_addr_parse(server, p[i]))
+                {
+                    msg(msglevel, "--dns server %ld: malformed address or maximum exceeded '%s'", priority, p[i]);
+                    return false;
+                }
+            }
+        }
+        else if (streq(p[3], "resolve-domains"))
+        {
+            dns_domain_list_append(&server->domains, &p[4], &options->dns_options.gc);
+        }
+        else if (streq(p[3], "dnssec") && !p[5])
+        {
+            if (streq(p[4], "yes"))
+            {
+                server->dnssec = DNS_SECURITY_YES;
+            }
+            else if (streq(p[4], "no"))
+            {
+                server->dnssec = DNS_SECURITY_NO;
+            }
+            else if (streq(p[4], "optional"))
+            {
+                server->dnssec = DNS_SECURITY_OPTIONAL;
+            }
+            else
+            {
+                msg(msglevel, "--dns server %ld: malformed dnssec value '%s'", priority, p[4]);
+                return false;
+            }
+        }
+        else if (streq(p[3], "transport") && !p[5])
+        {
+            if (streq(p[4], "plain"))
+            {
+                server->transport = DNS_TRANSPORT_PLAIN;
+            }
+            else if (streq(p[4], "DoH"))
+            {
+                server->transport = DNS_TRANSPORT_HTTPS;
+            }
+            else if (streq(p[4], "DoT"))
+            {
+                server->transport = DNS_TRANSPORT_TLS;
+            }
+            else
+            {
+                msg(msglevel, "--dns server %ld: malformed transport value '%s'", priority, p[4]);
+                return false;
+            }
+        }
+        else if (streq(p[3], "sni") && !p[5])
+        {
+            server->sni = p[4];
+        }
+        else
+        {
+            msg(msglevel, "--dns server %ld: unknown option type '%s' or missing or unknown parameter", priority, p[3]);
+            return false;
+        }
+    }
+    else
+    {
+        msg(msglevel, "--dns: unknown option type '%s' or missing or unknown parameter", p[1]);
+        return false;
+    }
+    return true;
+}
+
+/**
+ * @brief Processes an option to update. It first checks whether it has already
+ *        received an option of the same type within the same update message.
+ *        If the option has already been received, it calls add_option().
+ *        Otherwise, it deletes all existing values related to that option before calling add_option().
+ *
+ * @param c The context structure.
+ * @param options A pointer to the options structure.
+ * @param p An array of strings containing the options and their parameters.
+ * @param is_inline A boolean indicating if the option is inline.
+ * @param file The file where the function is called.
+ * @param line The line number where the function is called.
+ * @param level The level of the option.
+ * @param msglevel The message level for logging.
+ * @param permission_mask The permission mask used by VERIFY_PERMISSION().
+ * @param option_types_found A pointer to the variable where the flags corresponding to the options found are stored.
+ * @param es The environment set structure.
+ * @param update_options_found A pointer to the variable where the flags corresponding to the update options found are stored,
+ *                             used to check if an option of the same type has already been processed by update_option() within the same push-update message.
+ */
+static void
+update_option(struct context *c,
+              struct options *options,
+              char *p[],
+              bool is_inline,
+              const char *file,
+              int line,
+              const int level,
+              const int msglevel,
+              const unsigned int permission_mask,
+              unsigned int *option_types_found,
+              struct env_set *es,
+              unsigned int *update_options_found)
+{
+    const bool pull_mode = BOOL_CAST(permission_mask & OPT_P_PULL_MODE);
+    ASSERT(MAX_PARMS >= 7);
+
+    if (streq(p[0], "route") && p[1] && !p[5])
+    {
+        if (!(*update_options_found & OPT_P_U_ROUTE))
+        {
+            VERIFY_PERMISSION(OPT_P_ROUTE);
+            if (!check_route_option(options, p, msglevel, pull_mode))
+            {
+                goto err;
+            }
+            if (c->c1.route_list)
+            {
+                delete_routes_v4(c->c1.route_list, c->c1.tuntap,
+                                 ROUTE_OPTION_FLAGS(&c->options),
+                                 es, &c->net_ctx);
+                RESET_OPTION_ROUTES(options->routes, routes);
+            }
+            *update_options_found |= OPT_P_U_ROUTE;
+        }
+    }
+    else if (streq(p[0], "route-ipv6") && p[1] && !p[4])
+    {
+        if (!(*update_options_found & OPT_P_U_ROUTE6))
+        {
+            VERIFY_PERMISSION(OPT_P_ROUTE);
+            if (!check_route6_option(options, p, msglevel, pull_mode))
+            {
+                goto err;
+            }
+            if (c->c1.route_ipv6_list)
+            {
+                delete_routes_v6(c->c1.route_ipv6_list, c->c1.tuntap,
+                                 ROUTE_OPTION_FLAGS(&c->options),
+                                 es, &c->net_ctx);
+                RESET_OPTION_ROUTES(options->routes_ipv6, routes_ipv6);
+            }
+            *update_options_found |= OPT_P_U_ROUTE6;
+        }
+    }
+    else if (streq(p[0], "redirect-gateway") || streq(p[0], "redirect-private"))
+    {
+        if (!(*update_options_found & OPT_P_U_REDIR_GATEWAY))
+        {
+            VERIFY_PERMISSION(OPT_P_ROUTE);
+            if (options->routes)
+            {
+                options->routes->flags = 0;
+            }
+            if (options->routes_ipv6)
+            {
+                options->routes_ipv6->flags = 0;
+            }
+            *update_options_found |= OPT_P_U_REDIR_GATEWAY;
+        }
+    }
+    else if (streq(p[0], "dns") && p[1])
+    {
+        if (!(*update_options_found & OPT_P_U_DNS))
+        {
+            VERIFY_PERMISSION(OPT_P_DHCPDNS);
+            if (!check_dns_option(options, p, msglevel, pull_mode))
+            {
+                goto err;
+            }
+            gc_free(&options->dns_options.gc);
+            CLEAR(options->dns_options);
+            *update_options_found |= OPT_P_U_DNS;
+        }
+    }
+#if defined(_WIN32) || defined(TARGET_ANDROID)
+    else if (streq(p[0], "dhcp-option") && p[1] && !p[3])
+    {
+        if (!(*update_options_found & OPT_P_U_DHCP))
+        {
+            struct tuntap_options *o = &options->tuntap_options;
+            VERIFY_PERMISSION(OPT_P_DHCPDNS);
+
+            o->domain = NULL;
+            o->netbios_scope = NULL;
+            o->netbios_node_type = 0;
+            o->dns6_len = 0;
+            CLEAR(o->dns6);
+            o->dns_len = 0;
+            CLEAR(o->dns);
+            o->wins_len = 0;
+            CLEAR(o->wins);
+            o->ntp_len = 0;
+            CLEAR(o->ntp);
+            o->nbdd_len = 0;
+            CLEAR(o->nbdd);
+            while (o->domain_search_list_len-- > 0)
+            {
+                o->domain_search_list[o->domain_search_list_len] = NULL;
+            }
+            o->disable_nbt = 0;
+            o->dhcp_options = 0;
+#if defined(TARGET_ANDROID)
+            o->http_proxy_port = 0;
+            o->http_proxy = NULL;
+#endif
+            *update_options_found |= OPT_P_U_DHCP;
+        }
+    }
+#else /* if defined(_WIN32) || defined(TARGET_ANDROID) */
+    else if (streq(p[0], "dhcp-option") && p[1] && !p[3])
+    {
+        if (!(*update_options_found & OPT_P_U_DHCP))
+        {
+            VERIFY_PERMISSION(OPT_P_DHCPDNS);
+            delete_all_dhcp_fo(options, &es->list);
+            *update_options_found |= OPT_P_U_DHCP;
+        }
+    }
+#endif /* if defined(_WIN32) || defined(TARGET_ANDROID) */
+    add_option(options, p, is_inline, file, line,
+               level, msglevel, permission_mask,
+               option_types_found, es);
+    return;
+err:
+    msg(msglevel, "Error occurred trying to update %s option", p[0]);
+}
+
 static void
 set_user_script(struct options *options,
                 const char **script,
@@ -7296,45 +7612,18 @@
     else if (streq(p[0], "route") && p[1] && !p[5])
     {
         VERIFY_PERMISSION(OPT_P_ROUTE);
-        rol_check_alloc(options);
-        if (pull_mode)
+        if (!check_route_option(options, p, msglevel, pull_mode))
         {
-            if (!ip_or_dns_addr_safe(p[1], options->allow_pull_fqdn) && !is_special_addr(p[1])) /* FQDN -- may be DNS name */
-            {
-                msg(msglevel, "route parameter network/IP '%s' must be a valid address", p[1]);
-                goto err;
-            }
-            if (p[2] && !ip_addr_dotted_quad_safe(p[2])) /* FQDN -- must be IP address */
-            {
-                msg(msglevel, "route parameter netmask '%s' must be an IP address", p[2]);
-                goto err;
-            }
-            if (p[3] && !ip_or_dns_addr_safe(p[3], options->allow_pull_fqdn) && !is_special_addr(p[3])) /* FQDN -- may be DNS name */
-            {
-                msg(msglevel, "route parameter gateway '%s' must be a valid address", p[3]);
-                goto err;
-            }
-            /* p[4] is metric, if specified */
+            goto err;
         }
         add_route_to_option_list(options->routes, p[1], p[2], p[3], p[4], options->route_default_table_id);
     }
     else if (streq(p[0], "route-ipv6") && p[1] && !p[4])
     {
         VERIFY_PERMISSION(OPT_P_ROUTE);
-        rol6_check_alloc(options);
-        if (pull_mode)
+        if (!check_route6_option(options, p, msglevel, pull_mode))
         {
-            if (!ipv6_addr_safe_hexplusbits(p[1]))
-            {
-                msg(msglevel, "route-ipv6 parameter network/IP '%s' must be a valid address", p[1]);
-                goto err;
-            }
-            if (p[2] && !ipv6_addr_safe(p[2]))
-            {
-                msg(msglevel, "route-ipv6 parameter gateway '%s' must be a valid address", p[2]);
-                goto err;
-            }
-            /* p[3] is metric, if specified */
+            goto err;
         }
         add_route_ipv6_to_option_list(options->routes_ipv6, p[1], p[2], p[3], options->route_default_table_id);
     }
@@ -8422,90 +8711,8 @@
     else if (streq(p[0], "dns") && p[1])
     {
         VERIFY_PERMISSION(OPT_P_DHCPDNS);
-
-        if (streq(p[1], "search-domains") && p[2])
+        if (!check_dns_option(options, p, msglevel, pull_mode))
         {
-            dns_domain_list_append(&options->dns_options.search_domains, &p[2], &options->dns_options.gc);
-        }
-        else if (streq(p[1], "server") && p[2] && p[3] && p[4])
-        {
-            long priority;
-            if (!dns_server_priority_parse(&priority, p[2], pull_mode))
-            {
-                msg(msglevel, "--dns server: invalid priority value '%s'", p[2]);
-                goto err;
-            }
-
-            struct dns_server *server = dns_server_get(&options->dns_options.servers, priority, &options->dns_options.gc);
-
-            if (streq(p[3], "address") && p[4])
-            {
-                for (int i = 4; p[i]; ++i)
-                {
-                    if (!dns_server_addr_parse(server, p[i]))
-                    {
-                        msg(msglevel, "--dns server %ld: malformed address or maximum exceeded '%s'", priority, p[i]);
-                        goto err;
-                    }
-                }
-            }
-            else if (streq(p[3], "resolve-domains"))
-            {
-                dns_domain_list_append(&server->domains, &p[4], &options->dns_options.gc);
-            }
-            else if (streq(p[3], "dnssec") && !p[5])
-            {
-                if (streq(p[4], "yes"))
-                {
-                    server->dnssec = DNS_SECURITY_YES;
-                }
-                else if (streq(p[4], "no"))
-                {
-                    server->dnssec = DNS_SECURITY_NO;
-                }
-                else if (streq(p[4], "optional"))
-                {
-                    server->dnssec = DNS_SECURITY_OPTIONAL;
-                }
-                else
-                {
-                    msg(msglevel, "--dns server %ld: malformed dnssec value '%s'", priority, p[4]);
-                    goto err;
-                }
-            }
-            else if (streq(p[3], "transport") && !p[5])
-            {
-                if (streq(p[4], "plain"))
-                {
-                    server->transport = DNS_TRANSPORT_PLAIN;
-                }
-                else if (streq(p[4], "DoH"))
-                {
-                    server->transport = DNS_TRANSPORT_HTTPS;
-                }
-                else if (streq(p[4], "DoT"))
-                {
-                    server->transport = DNS_TRANSPORT_TLS;
-                }
-                else
-                {
-                    msg(msglevel, "--dns server %ld: malformed transport value '%s'", priority, p[4]);
-                    goto err;
-                }
-            }
-            else if (streq(p[3], "sni") && !p[5])
-            {
-                server->sni = p[4];
-            }
-            else
-            {
-                msg(msglevel, "--dns server %ld: unknown option type '%s' or missing or unknown parameter", priority, p[3]);
-                goto err;
-            }
-        }
-        else
-        {
-            msg(msglevel, "--dns: unknown option type '%s' or missing or unknown parameter", p[1]);
             goto err;
         }
     }