openvpn-devel — OpenVPN developers list

[Openvpn-devel] [S] Change in openvpn[master]: t_server_null: print error when server startup fails

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

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

Change subject: t_server_null: print error when server startup fails
......................................................................

t_server_null: print error when server startup fails

Use "&" to background so that the exit code and all output can be
obtained in all failure cases.

Change-Id: I39dc6b08952a06dae7901e468f9487c8541d83c3
Signed-off-by: Samuli Seppänen <sam...@gm...>
Acked-by: Gert Doering <ge...@gr...>
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg31932.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M tests/t_server_null_default.rc
M tests/t_server_null_server.sh
2 files changed, 13 insertions(+), 15 deletions(-)




diff --git a/tests/t_server_null_default.rc b/tests/t_server_null_default.rc
index 365b5a8..41ec591 100755
--- a/tests/t_server_null_default.rc
+++ b/tests/t_server_null_default.rc
@@ -38,7 +38,7 @@
 MAX_CLIENTS="10"
 CLIENT_MATCH="Test-Client"
 SERVER_EXEC="${top_builddir}/src/openvpn/openvpn"
-SERVER_BASE_OPTS="--daemon --local 127.0.0.1 --dev tun --topology subnet --max-clients $MAX_CLIENTS --persist-tun --verb 3 --duplicate-cn"
+SERVER_BASE_OPTS="--local 127.0.0.1 --dev tun --topology subnet --max-clients $MAX_CLIENTS --persist-tun --verb 3 --duplicate-cn"
 SERVER_CIPHER_OPTS=""
 SERVER_CERT_OPTS="--ca ${CA} --cert ${SERVER_CERT} --key ${SERVER_KEY} --tls-auth ${TA} 0"
 SERVER_CONF_BASE="${SERVER_BASE_OPTS} ${SERVER_CIPHER_OPTS} ${SERVER_CERT_OPTS}"
diff --git a/tests/t_server_null_server.sh b/tests/t_server_null_server.sh
index acf8479..65b7d56 100755
--- a/tests/t_server_null_server.sh
+++ b/tests/t_server_null_server.sh
@@ -11,20 +11,18 @@
     # Allow reading this file even umask values are strict
     touch "$log"
 
-    if [ -z "${RUN_SUDO}" ]; then
-        "${server_exec}" \
-         $server_conf \
-         --status "${status}" 1 \
-         --log "${log}" \
-         --writepid "${pid}" \
-         --explicit-exit-notify 3
-    else
-        $RUN_SUDO "${server_exec}" \
-                   $server_conf \
-                   --status "${status}" 1 \
-                   --log "${log}" \
-                   --writepid "${pid}" \
-                   --explicit-exit-notify 3
+    # Try to launch the server
+    $RUN_SUDO "${server_exec}" \
+               $server_conf \
+               --status "${status}" 1 \
+               --writepid "${pid}" \
+               --explicit-exit-notify 3 > "$log" 2>&1 &
+
+    sleep 1
+
+    if ! [ -r "$pid" ] || [ -z "$pid" ]; then
+        echo "ERROR: failed to start server $server_name"
+        tail -n 20 "$log"
     fi
 }
 

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1024?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: I39dc6b08952a06dae7901e468f9487c8541d83c3
Gerrit-Change-Number: 1024
Gerrit-PatchSet: 4
Gerrit-Owner: mattock <sa...@pr...>
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]: t_server_null: print error when server startup fails

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

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

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


Change subject: t_server_null: print error when server startup fails
......................................................................

t_server_null: print error when server startup fails

Use "&" to background so that the exit code and all output can be
obtained in all failure cases.

Change-Id: I39dc6b08952a06dae7901e468f9487c8541d83c3
Signed-off-by: Samuli Seppänen <sam...@gm...>
Acked-by: Gert Doering <ge...@gr...>
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg31932.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M tests/t_server_null_default.rc
M tests/t_server_null_server.sh
2 files changed, 13 insertions(+), 15 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/24/1024/4

diff --git a/tests/t_server_null_default.rc b/tests/t_server_null_default.rc
index 365b5a8..41ec591 100755
--- a/tests/t_server_null_default.rc
+++ b/tests/t_server_null_default.rc
@@ -38,7 +38,7 @@
 MAX_CLIENTS="10"
 CLIENT_MATCH="Test-Client"
 SERVER_EXEC="${top_builddir}/src/openvpn/openvpn"
-SERVER_BASE_OPTS="--daemon --local 127.0.0.1 --dev tun --topology subnet --max-clients $MAX_CLIENTS --persist-tun --verb 3 --duplicate-cn"
+SERVER_BASE_OPTS="--local 127.0.0.1 --dev tun --topology subnet --max-clients $MAX_CLIENTS --persist-tun --verb 3 --duplicate-cn"
 SERVER_CIPHER_OPTS=""
 SERVER_CERT_OPTS="--ca ${CA} --cert ${SERVER_CERT} --key ${SERVER_KEY} --tls-auth ${TA} 0"
 SERVER_CONF_BASE="${SERVER_BASE_OPTS} ${SERVER_CIPHER_OPTS} ${SERVER_CERT_OPTS}"
diff --git a/tests/t_server_null_server.sh b/tests/t_server_null_server.sh
index acf8479..65b7d56 100755
--- a/tests/t_server_null_server.sh
+++ b/tests/t_server_null_server.sh
@@ -11,20 +11,18 @@
     # Allow reading this file even umask values are strict
     touch "$log"
 
-    if [ -z "${RUN_SUDO}" ]; then
-        "${server_exec}" \
-         $server_conf \
-         --status "${status}" 1 \
-         --log "${log}" \
-         --writepid "${pid}" \
-         --explicit-exit-notify 3
-    else
-        $RUN_SUDO "${server_exec}" \
-                   $server_conf \
-                   --status "${status}" 1 \
-                   --log "${log}" \
-                   --writepid "${pid}" \
-                   --explicit-exit-notify 3
+    # Try to launch the server
+    $RUN_SUDO "${server_exec}" \
+               $server_conf \
+               --status "${status}" 1 \
+               --writepid "${pid}" \
+               --explicit-exit-notify 3 > "$log" 2>&1 &
+
+    sleep 1
+
+    if ! [ -r "$pid" ] || [ -z "$pid" ]; then
+        echo "ERROR: failed to start server $server_name"
+        tail -n 20 "$log"
     fi
 }
 

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1024?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: I39dc6b08952a06dae7901e468f9487c8541d83c3
Gerrit-Change-Number: 1024
Gerrit-PatchSet: 4
Gerrit-Owner: mattock <sa...@pr...>
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: t_server_null: print error when server startup fails

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

v3 looks good and does the job :-) - it needed a bit of massaging
because the context has changed (commit 4d104a385) and because it
really wants to go on top of #915 - but that's the wrong order, the
"we want to see the failures?!" patch needs to go in before the
one that triggers new failures...

So, adjustments to t_server_null_default.rc to adapt to this.

Your patch has been applied to the master branch.

commit 378c2e2f2c155d92989f988b6fffb0e8980fa83d
Author: Samuli Seppänen
Date:   Wed Jun 18 16:13:21 2025 +0200

     t_server_null: print error when server startup fails

     Signed-off-by: Samuli Seppänen <sam...@gm...>
     Acked-by: Gert Doering <ge...@gr...>
     Message-Id: <202...@gr...>
     URL: https://www.mail-archive.com/ope...@li.../msg31932.html
     Signed-off-by: Gert Doering <ge...@gr...>


--
kind regards,

Gert Doering

[Openvpn-devel] [PATCH v3] t_server_null: print error when server startup fails

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

From: Samuli Seppänen <sam...@gm...>

Use "&" to background so that the exit code and all output can be
obtained in all failure cases.

Change-Id: I39dc6b08952a06dae7901e468f9487c8541d83c3
Signed-off-by: Samuli Seppänen <sam...@gm...>
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/+/1024
This mail reflects revision 3 of this Change.

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

        
diff --git a/tests/t_server_null_default.rc b/tests/t_server_null_default.rc
index 5b74761..1f2fa2c 100755
--- a/tests/t_server_null_default.rc
+++ b/tests/t_server_null_default.rc
@@ -38,7 +38,7 @@
 MAX_CLIENTS="10"
 CLIENT_MATCH="Test-Client"
 SERVER_EXEC="${top_builddir}/src/openvpn/openvpn"
-SERVER_BASE_OPTS="--daemon --dev tun --topology subnet --max-clients $MAX_CLIENTS --persist-tun --verb 3 --duplicate-cn"
+SERVER_BASE_OPTS="--dev tun --topology subnet --max-clients $MAX_CLIENTS --persist-tun --verb 3 --duplicate-cn"
 SERVER_BIND_OPTS="--local 127.0.0.1"
 SERVER_CIPHER_OPTS=""
 SERVER_CERT_OPTS="--ca ${CA} --dh ${DH} --cert ${SERVER_CERT} --key ${SERVER_KEY} --tls-auth ${TA} 0"
diff --git a/tests/t_server_null_server.sh b/tests/t_server_null_server.sh
index acf8479..65b7d56 100755
--- a/tests/t_server_null_server.sh
+++ b/tests/t_server_null_server.sh
@@ -11,20 +11,18 @@
     # Allow reading this file even umask values are strict
     touch "$log"
 
-    if [ -z "${RUN_SUDO}" ]; then
-        "${server_exec}" \
-         $server_conf \
-         --status "${status}" 1 \
-         --log "${log}" \
-         --writepid "${pid}" \
-         --explicit-exit-notify 3
-    else
-        $RUN_SUDO "${server_exec}" \
-                   $server_conf \
-                   --status "${status}" 1 \
-                   --log "${log}" \
-                   --writepid "${pid}" \
-                   --explicit-exit-notify 3
+    # Try to launch the server
+    $RUN_SUDO "${server_exec}" \
+               $server_conf \
+               --status "${status}" 1 \
+               --writepid "${pid}" \
+               --explicit-exit-notify 3 > "$log" 2>&1 &
+
+    sleep 1
+
+    if ! [ -r "$pid" ] || [ -z "$pid" ]; then
+        echo "ERROR: failed to start server $server_name"
+        tail -n 20 "$log"
     fi
 }
 

[Openvpn-devel] [S] Change in openvpn[master]: t_server_null: print error when server startup fails

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

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

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

Change subject: t_server_null: print error when server startup fails
......................................................................


Patch Set 3: Code-Review+2


-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1024?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: I39dc6b08952a06dae7901e468f9487c8541d83c3
Gerrit-Change-Number: 1024
Gerrit-PatchSet: 3
Gerrit-Owner: mattock <sa...@pr...>
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: mattock <sa...@pr...>
Gerrit-Comment-Date: Wed, 18 Jun 2025 14:11:24 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

[Openvpn-devel] [S] Change in openvpn[master]: Multi-socket: local_list clean-up

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

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

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


Change subject: Multi-socket: local_list clean-up
......................................................................

Multi-socket: local_list clean-up

Optimize the current local_list implementation
by replacing the static array with a resizable
one, as the static allocation serves no real
purpose, particularly on the client side.

Github: OpenVPN/openvpn#682

Change-Id: I32effed9e273fbe8986d1f4e8da4a4d0ac216463
Signed-off-by: Gianmarco De Gregori <gia...@ma...>
Acked-by: Gert Doering <ge...@gr...>
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg31927.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M src/openvpn/options.c
M src/openvpn/options.h
2 files changed, 14 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/39/1039/2

diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index d758a67..3cf8c2a 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -2067,12 +2067,20 @@
     struct local_list *l = alloc_local_list_if_undef(ce, gc);
     struct local_entry *e;
 
-    if (l->len >= CONNECTION_LIST_SIZE)
+    if (l->len >= l->capacity)
     {
-        msg(msglevel, "Maximum number of 'local' options (%d) exceeded",
-            CONNECTION_LIST_SIZE);
+        const int new_cap = l->capacity + 1;
+        const size_t elem_size = sizeof(*l->array);
 
-        return NULL;
+        struct local_entry **new_array = gc_realloc(l->array, new_cap * elem_size, gc);
+        if (!new_array)
+        {
+            msg(msglevel, "Unable to process more local options: out of memory. Number of entries = %d", l->len);
+            return NULL;
+        }
+
+        l->array = new_array;
+        l->capacity = new_cap;
     }
 
     ALLOC_OBJ_CLEAR_GC(e, struct local_entry, gc);
diff --git a/src/openvpn/options.h b/src/openvpn/options.h
index b28ad58..46ec32b 100644
--- a/src/openvpn/options.h
+++ b/src/openvpn/options.h
@@ -188,8 +188,9 @@
 
 struct local_list
 {
+    int capacity;
     int len;
-    struct local_entry *array[CONNECTION_LIST_SIZE];
+    struct local_entry **array;
 };
 
 struct connection_list

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1039?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: I32effed9e273fbe8986d1f4e8da4a4d0ac216463
Gerrit-Change-Number: 1039
Gerrit-PatchSet: 2
Gerrit-Owner: its_Giaan <gia...@ma...>
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] [S] Change in openvpn[master]: Multi-socket: local_list clean-up

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

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

Change subject: Multi-socket: local_list clean-up
......................................................................

Multi-socket: local_list clean-up

Optimize the current local_list implementation
by replacing the static array with a resizable
one, as the static allocation serves no real
purpose, particularly on the client side.

Github: OpenVPN/openvpn#682

Change-Id: I32effed9e273fbe8986d1f4e8da4a4d0ac216463
Signed-off-by: Gianmarco De Gregori <gia...@ma...>
Acked-by: Gert Doering <ge...@gr...>
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg31927.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M src/openvpn/options.c
M src/openvpn/options.h
2 files changed, 14 insertions(+), 5 deletions(-)




diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index d758a67..3cf8c2a 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -2067,12 +2067,20 @@
     struct local_list *l = alloc_local_list_if_undef(ce, gc);
     struct local_entry *e;
 
-    if (l->len >= CONNECTION_LIST_SIZE)
+    if (l->len >= l->capacity)
     {
-        msg(msglevel, "Maximum number of 'local' options (%d) exceeded",
-            CONNECTION_LIST_SIZE);
+        const int new_cap = l->capacity + 1;
+        const size_t elem_size = sizeof(*l->array);
 
-        return NULL;
+        struct local_entry **new_array = gc_realloc(l->array, new_cap * elem_size, gc);
+        if (!new_array)
+        {
+            msg(msglevel, "Unable to process more local options: out of memory. Number of entries = %d", l->len);
+            return NULL;
+        }
+
+        l->array = new_array;
+        l->capacity = new_cap;
     }
 
     ALLOC_OBJ_CLEAR_GC(e, struct local_entry, gc);
diff --git a/src/openvpn/options.h b/src/openvpn/options.h
index b28ad58..46ec32b 100644
--- a/src/openvpn/options.h
+++ b/src/openvpn/options.h
@@ -188,8 +188,9 @@
 
 struct local_list
 {
+    int capacity;
     int len;
-    struct local_entry *array[CONNECTION_LIST_SIZE];
+    struct local_entry **array;
 };
 
 struct connection_list

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1039?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: I32effed9e273fbe8986d1f4e8da4a4d0ac216463
Gerrit-Change-Number: 1039
Gerrit-PatchSet: 2
Gerrit-Owner: its_Giaan <gia...@ma...>
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] [PATCH applied] Re: Multi-socket: local_list clean-up

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

Stared-at-code, looks good.  Tested on the server instance with lots of
--local lines (+ some msg() calls to see l->len and l_capacity), and it
does what it promises.  Thanks :-)

Your patch has been applied to the master branch.

commit 9bb02bc34f5ecc85364fa7ab64e52b6c5c918055
Author: Gianmarco De Gregori
Date:   Wed Jun 18 16:00:09 2025 +0200

     Multi-socket: local_list clean-up

     Signed-off-by: Gianmarco De Gregori <gia...@ma...>
     Acked-by: Gert Doering <ge...@gr...>
     Message-Id: <202...@gr...>
     URL: https://www.mail-archive.com/ope...@li.../msg31927.html
     Signed-off-by: Gert Doering <ge...@gr...>


--
kind regards,

Gert Doering

[Openvpn-devel] [PATCH v1] Multi-socket: local_list clean-up

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

From: Gianmarco De Gregori <gia...@ma...>

Optimize the current local_list implementation
by replacing the static array with a resizable
one, as the static allocation serves no real
purpose, particularly on the client side.

Github: #682

Change-Id: I32effed9e273fbe8986d1f4e8da4a4d0ac216463
Signed-off-by: Gianmarco De Gregori <gia...@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/+/1039
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 6ea01d4..70337b1 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -2212,12 +2212,20 @@
     struct local_list *l = alloc_local_list_if_undef(ce, gc);
     struct local_entry *e;
 
-    if (l->len >= CONNECTION_LIST_SIZE)
+    if (l->len >= l->capacity)
     {
-        msg(msglevel, "Maximum number of 'local' options (%d) exceeded",
-            CONNECTION_LIST_SIZE);
+        const int new_cap = l->capacity + 1;
+        const size_t elem_size = sizeof(*l->array);
 
-        return NULL;
+        struct local_entry **new_array = gc_realloc(l->array, new_cap * elem_size, gc);
+        if (!new_array)
+        {
+            msg(msglevel, "Unable to process more local options: out of memory. Number of entries = %d", l->len);
+            return NULL;
+        }
+
+        l->array = new_array;
+        l->capacity = new_cap;
     }
 
     ALLOC_OBJ_CLEAR_GC(e, struct local_entry, gc);
diff --git a/src/openvpn/options.h b/src/openvpn/options.h
index b28ad58..46ec32b 100644
--- a/src/openvpn/options.h
+++ b/src/openvpn/options.h
@@ -188,8 +188,9 @@
 
 struct local_list
 {
+    int capacity;
     int len;
-    struct local_entry *array[CONNECTION_LIST_SIZE];
+    struct local_entry **array;
 };
 
 struct connection_list

[Openvpn-devel] [S] Change in openvpn[master]: Multi-socket: local_list clean-up

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

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

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

Change subject: Multi-socket: local_list clean-up
......................................................................


Patch Set 1: Code-Review+2


-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1039?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: I32effed9e273fbe8986d1f4e8da4a4d0ac216463
Gerrit-Change-Number: 1039
Gerrit-PatchSet: 1
Gerrit-Owner: its_Giaan <gia...@ma...>
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: its_Giaan <gia...@ma...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Comment-Date: Wed, 18 Jun 2025 13:59:57 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

[Openvpn-devel] [L] Change in openvpn[master]: dns: deal with --dhcp-options when --dns is active

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

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

Change subject: dns: deal with --dhcp-options when --dns is active
......................................................................

dns: deal with --dhcp-options when --dns is active

Since --dns settings overrule DNS related --dhcp-options,
remove the latter when values were defined via --dns.

To stay as backward compatible as possible, we add foreign_options to
the script hook environment from the --dns values when a --up script
is defined. In that case the default --dns-updown is not run, even
when --dns values are present, to prevent double DNS configuration.
This way an existing --up script that deals with DNS can run, without
the immediate need to change after an openvpn upgrade and a server
pushing --dns options.

If you specify a custom --dns-updown, or force running the default
dns-updown that comes with openvpn, those compat env vars are not set
for --up scripts and the dns-updown command is run, even when there's
an --up script present.

Since Android uses the DNS values from tuntap_options, we always
override those with --dns stuff unconditionally. Also on Windows when
--ip-win32 is dynamic or adaptive, since DHCP relies on these as well.

Change-Id: I635c4018fb43b5976a39b6a90cb2e9cb2570cd6a
Signed-off-by: Heiko Hund <he...@is...>
Acked-by: Gert Doering <ge...@gr...>
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg31922.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M src/openvpn/dns.c
M src/openvpn/dns.h
M src/openvpn/options.c
3 files changed, 323 insertions(+), 184 deletions(-)




diff --git a/src/openvpn/dns.c b/src/openvpn/dns.c
index 7cf1b63..939ae09 100644
--- a/src/openvpn/dns.c
+++ b/src/openvpn/dns.c
@@ -691,7 +691,8 @@
 static void
 run_up_down_command(bool up, struct options *o, const struct tuntap *tt, struct dns_updown_runner_info *updown_runner)
 {
-    if (!o->dns_options.updown)
+    struct dns_options *dns = &o->dns_options;
+    if (!dns->updown || (o->up_script && !dns->user_set_updown))
     {
         return;
     }
@@ -701,7 +702,7 @@
     if (!updown_runner->required)
     {
         /* Run dns updown directly */
-        status = do_run_up_down_command(up, NULL, &o->dns_options, tt);
+        status = do_run_up_down_command(up, NULL, dns, tt);
     }
     else
     {
@@ -852,6 +853,14 @@
     {
         return;
     }
+#ifdef _WIN32
+    /* Don't use iservice in DHCP mode */
+    struct tuntap_options *tto = &o->tuntap_options;
+    if (tto->ip_win32_type == IPW32_SET_DHCP_MASQ || tto->ip_win32_type == IPW32_SET_ADAPTIVE)
+    {
+        return;
+    }
+#endif
 
     /* Warn about adding servers of unsupported AF */
     const struct dns_server *s = o->dns_options.servers;
diff --git a/src/openvpn/dns.h b/src/openvpn/dns.h
index 60f5471..688daa7 100644
--- a/src/openvpn/dns.h
+++ b/src/openvpn/dns.h
@@ -76,7 +76,28 @@
 #endif
 };
 
+#ifndef N_DHCP_ADDR
+#define N_DHCP_ADDR 4
+#endif
+
+#ifndef N_SEARCH_LIST_LEN
+#define N_SEARCH_LIST_LEN 10
+#endif
+
+struct dhcp_options {
+    in_addr_t dns[N_DHCP_ADDR];
+    int dns_len;
+
+    struct in6_addr dns6[N_DHCP_ADDR];
+    int dns6_len;
+
+    const char *domain;
+    const char *domain_search_list[N_SEARCH_LIST_LEN];
+    int domain_search_list_len;
+};
+
 struct dns_options {
+    struct dhcp_options from_dhcp;
     struct dns_domain *search_domains;
     struct dns_server *servers_prepull;
     struct dns_server *servers;
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 6ea01d4..d758a67 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -1328,7 +1328,6 @@
 #endif /* ifndef ENABLE_SMALL */
 #endif /* ifdef _WIN32 */
 
-#if defined(_WIN32) || defined(TARGET_ANDROID)
 static void
 dhcp_option_dns6_parse(const char *parm, struct in6_addr *dns6_list, int *len, int msglevel)
 {
@@ -1371,150 +1370,6 @@
     }
 }
 
-/*
- * If DNS options are set use these for TUN/TAP options as well.
- * Applies to DNS, DNS6 and DOMAIN-SEARCH.
- * Existing options will be discarded.
- */
-static void
-tuntap_options_copy_dns(struct options *o)
-{
-    struct tuntap_options *tt = &o->tuntap_options;
-    struct dns_options *dns = &o->dns_options;
-
-    if (dns->search_domains)
-    {
-        tt->domain_search_list_len = 0;
-        const struct dns_domain *domain = dns->search_domains;
-        while (domain && tt->domain_search_list_len < N_SEARCH_LIST_LEN)
-        {
-            tt->domain_search_list[tt->domain_search_list_len++] = domain->name;
-            domain = domain->next;
-        }
-        if (domain)
-        {
-            msg(M_WARN, "WARNING: couldn't copy all --dns search-domains to --dhcp-option");
-        }
-        tt->dhcp_options |= DHCP_OPTIONS_DHCP_REQUIRED;
-    }
-
-    if (dns->servers)
-    {
-        tt->dns_len = 0;
-        tt->dns6_len = 0;
-        bool overflow = false;
-        const struct dns_server *server = dns->servers;
-        while (server)
-        {
-            for (int i = 0; i < server->addr_count; ++i)
-            {
-                if (server->addr[i].family == AF_INET)
-                {
-                    if (tt->dns_len >= N_DHCP_ADDR)
-                    {
-                        overflow = true;
-                        continue;
-                    }
-                    tt->dns[tt->dns_len++] = ntohl(server->addr[i].in.a4.s_addr);
-                }
-                else
-                {
-                    if (tt->dns6_len >= N_DHCP_ADDR)
-                    {
-                        overflow = true;
-                        continue;
-                    }
-                    tt->dns6[tt->dns6_len++] = server->addr[i].in.a6;
-                }
-            }
-            server = server->next;
-        }
-        if (overflow)
-        {
-            msg(M_WARN, "WARNING: couldn't copy all --dns server addresses to --dhcp-option");
-        }
-        tt->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL;
-    }
-}
-#else /* if defined(_WIN32) || defined(TARGET_ANDROID) */
-static void
-foreign_options_copy_dns(struct options *o, struct env_set *es)
-{
-    const struct dns_domain *domain = o->dns_options.search_domains;
-    const struct dns_server *server = o->dns_options.servers;
-    if (!domain && !server)
-    {
-        return;
-    }
-
-    /* reset the index since we're starting all over again */
-    int opt_max = o->foreign_option_index;
-    o->foreign_option_index = 0;
-
-    for (int i = 1; i <= opt_max; ++i)
-    {
-        char name[32];
-        snprintf(name, sizeof(name), "foreign_option_%d", i);
-
-        const char *env_str = env_set_get(es, name);
-        const char *value = strchr(env_str, '=') + 1;
-        if ((domain && strstr(value, "dhcp-option DOMAIN-SEARCH") == value)
-            || (server && strstr(value, "dhcp-option DNS") == value))
-        {
-            setenv_del(es, name);
-        }
-        else
-        {
-            setenv_foreign_option(o, &value, 1, es);
-        }
-    }
-
-    struct gc_arena gc = gc_new();
-
-    while (server)
-    {
-        for (size_t i = 0; i < server->addr_count; ++i)
-        {
-            if (server->addr[i].family == AF_INET)
-            {
-                const char *argv[] = {
-                    "dhcp-option",
-                    "DNS",
-                    print_in_addr_t(server->addr[i].in.a4.s_addr, 0, &gc)
-                };
-                setenv_foreign_option(o, argv, 3, es);
-            }
-            else
-            {
-                const char *argv[] = {
-                    "dhcp-option",
-                    "DNS6",
-                    print_in6_addr(server->addr[i].in.a6, 0, &gc)
-                };
-                setenv_foreign_option(o, argv, 3, es);
-            }
-        }
-        server = server->next;
-    }
-    while (domain)
-    {
-        const char *argv[] = { "dhcp-option", "DOMAIN-SEARCH", domain->name };
-        setenv_foreign_option(o, argv, 3, es);
-        domain = domain->next;
-    }
-
-    gc_free(&gc);
-
-    /* remove old leftover entries */
-    while (o->foreign_option_index < opt_max)
-    {
-        char name[32];
-        snprintf(name, sizeof(name), "foreign_option_%d", opt_max--);
-        setenv_del(es, name);
-    }
-}
-#endif /* if defined(_WIN32) || defined(TARGET_ANDROID) */
-
 #ifndef ENABLE_SMALL
 static const char *
 print_vlan_accept(enum vlan_acceptable_frames mode)
@@ -3603,6 +3458,260 @@
     }
 }
 
+#if defined(_WIN32) || defined(TARGET_ANDROID)
+/**
+ * @brief Postprocess DNS related settings
+ *
+ * Set TUN/TAP DNS options with values from either --dns
+ * or --dhcp-option.
+ *
+ * @param o     pointer to the options struct
+ */
+static void
+tuntap_options_postprocess_dns(struct options *o)
+{
+    struct dns_options *dns = &o->dns_options;
+    struct tuntap_options *tt = &o->tuntap_options;
+    if (!dns->servers)
+    {
+        /* Copy --dhcp-options to tuntap_options */
+        struct dhcp_options *dhcp = &dns->from_dhcp;
+        assert(sizeof(dhcp->dns) == sizeof(tt->dns));
+        assert(sizeof(dhcp->dns6) == sizeof(tt->dns6));
+        assert(sizeof(dhcp->domain_search_list) == sizeof(tt->domain_search_list));
+
+        tt->domain = dhcp->domain;
+        tt->dns_len = dhcp->dns_len;
+        tt->dns6_len = dhcp->dns6_len;
+
+        memcpy(tt->dns, dhcp->dns, sizeof(tt->dns));
+        memcpy(tt->dns6, dhcp->dns6, sizeof(tt->dns6));
+
+        tt->domain_search_list_len = dhcp->domain_search_list_len;
+        for (size_t i = 0; i < SIZE(tt->domain_search_list); ++i)
+        {
+            tt->domain_search_list[i] = dhcp->domain_search_list[i];
+        }
+
+        return;
+    }
+
+#if defined(_WIN32)
+    if (tt->ip_win32_type != IPW32_SET_DHCP_MASQ && tt->ip_win32_type != IPW32_SET_ADAPTIVE)
+    {
+        return;     /* Not in DHCP mode */
+    }
+#endif /* if defined(_WIN32) */
+
+    /* Copy --dns options to tuntap_options */
+    const struct dns_domain *d = dns->search_domains;
+    while (d && tt->domain_search_list_len + 1 < N_SEARCH_LIST_LEN)
+    {
+        tt->domain_search_list[tt->domain_search_list_len++] = d->name;
+        d = d->next;
+    }
+    if (d)
+    {
+        msg(M_WARN, "WARNING: couldn't copy all --dns search-domains to TUN/TAP");
+    }
+
+    const struct dns_server *s = dns->servers;
+    while (s)
+    {
+        bool non_standard_server_port = false;
+        for (int i = 0; i < s->addr_count; ++i)
+        {
+            if (s->addr[i].port && s->addr[i].port != 53)
+            {
+                non_standard_server_port = true;
+                break;
+            }
+        }
+        if ((s->transport && s->transport != DNS_TRANSPORT_PLAIN)
+            || (s->dnssec && s->dnssec != DNS_SECURITY_NO)
+            || non_standard_server_port)
+        {
+            /* Skip servers requiring unsupported config to be set */
+            s = s->next;
+        }
+        else
+        {
+            bool overflow = false;
+            for (int i = 0; i < s->addr_count; ++i)
+            {
+                if (s->addr[i].family == AF_INET && tt->dns_len + 1 < N_DHCP_ADDR)
+                {
+                    tt->dns[tt->dns_len++] = s->addr[i].in.a4.s_addr;
+                }
+                else if (tt->dns6_len + 1 < N_DHCP_ADDR)
+                {
+                    tt->dns6[tt->dns6_len++] = s->addr[i].in.a6;
+                }
+                else
+                {
+                    overflow = true;
+                }
+            }
+            if (overflow)
+            {
+                msg(M_WARN, "WARNING: couldn't copy all --dns server addresses to TUN/TAP");
+            }
+            return;
+        }
+    }
+}
+
+#else /* if defined(_WIN32) || defined(TARGET_ANDROID) */
+
+/**
+ * @brief Postprocess DNS related settings
+ *
+ * Discard existing --dhcp-options from the env if needed and possibly
+ * replace them with values from --dns. If no --dns servers are set copy
+ * the --dhcp-option values over for --dns-updown runs.
+ *
+ * @param o     pointer to the options struct
+ * @param es    env set to modify potentially
+ */
+static void
+dhcp_options_postprocess_dns(struct options *o, struct env_set *es)
+{
+    struct gc_arena gc = gc_new();
+    struct dns_options *dns = &o->dns_options;
+
+    if (dns->servers || dns->user_set_updown)
+    {
+        /* Clean up env from --dhcp-option DNS config */
+        struct buffer name = alloc_buf_gc(OPTION_PARM_SIZE, &gc);
+        struct buffer value = alloc_buf_gc(OPTION_PARM_SIZE, &gc);
+
+        const int fo_count = o->foreign_option_index;
+        o->foreign_option_index = 0;
+
+        for (int i = 1; i <= fo_count; ++i)
+        {
+            buf_clear(&name);
+            buf_printf(&name, "foreign_option_%d", i);
+            const char *env_str = env_set_get(es, BSTR(&name));
+            const char *item_val = strchr(env_str, '=') + 1;
+            buf_clear(&value);
+            buf_printf(&value, "%s", item_val);
+
+            /* Remove foreign option item from env set */
+            env_set_del(es, BSTR(&name));
+
+            item_val = BSTR(&value);
+            if (strncmp(item_val, "dhcp-option ", 12) != 0
+                || (strncmp(item_val + 12, "ADAPTER-DOMAIN-SUFFIX ", 22) != 0
+                    && strncmp(item_val + 12, "DOMAIN-SEARCH ", 14) != 0
+                    && strncmp(item_val + 12, "DOMAIN ", 7) != 0
+                    && strncmp(item_val + 12, "DNS6 ", 5) != 0
+                    && strncmp(item_val + 12, "DNS ", 4) != 0))
+            {
+                /* Re-set the item with potentially updated name */
+                buf_clear(&name);
+                buf_printf(&name, "foreign_option_%d", ++o->foreign_option_index);
+                setenv_str(es, BSTR(&name), BSTR(&value));
+            }
+        }
+    }
+
+    if (!dns->servers)
+    {
+        /* Copy --dhcp-options to dns_options */
+        struct dhcp_options *dhcp = &dns->from_dhcp;
+
+        if (dhcp->dns_len || dhcp->dns6_len)
+        {
+            struct dns_domain **entry = &dns->search_domains;
+            ALLOC_OBJ_CLEAR_GC(*entry, struct dns_domain, &dns->gc);
+            struct dns_domain *new = *entry;
+            new->name = dhcp->domain;
+            entry = &new->next;
+
+            for (size_t i = 0; i < dhcp->domain_search_list_len; ++i)
+            {
+                ALLOC_OBJ_CLEAR_GC(*entry, struct dns_domain, &dns->gc);
+                struct dns_domain *new = *entry;
+                new->name = dhcp->domain_search_list[i];
+                entry = &new->next;
+            }
+
+            struct dns_server *server = dns_server_get(&dns->servers, 0, &dns->gc);
+            const size_t max_addrs = SIZE(server->addr);
+            for (size_t i = 0; i < dhcp->dns_len && server->addr_count < max_addrs; ++i)
+            {
+                server->addr[server->addr_count].in.a4.s_addr = htonl(dhcp->dns[i]);
+                server->addr[server->addr_count].family = AF_INET;
+                server->addr_count += 1;
+            }
+            for (size_t i = 0; i < dhcp->dns6_len && server->addr_count < max_addrs; ++i)
+            {
+                server->addr[server->addr_count].in.a6 = dhcp->dns6[i];
+                server->addr[server->addr_count].family = AF_INET6;
+                server->addr_count += 1;
+            }
+        }
+    }
+    else if (o->up_script && !dns->user_set_updown)
+    {
+        /* Set foreign option env vars from --dns config */
+        const char *p[] = { "dhcp-option", NULL, NULL };
+        size_t p_len = sizeof(p) / sizeof(p[0]);
+
+        p[1] = "DOMAIN";
+        const struct dns_domain *d = dns->search_domains;
+        while (d)
+        {
+            p[2] = d->name;
+            setenv_foreign_option(o, (const char **)p, p_len, es);
+            d = d->next;
+        }
+
+        const struct dns_server *s = dns->servers;
+        while (s)
+        {
+            bool non_standard_server_port = false;
+            for (int i = 0; i < s->addr_count; ++i)
+            {
+                if (s->addr[i].port && s->addr[i].port != 53)
+                {
+                    non_standard_server_port = true;
+                    break;
+                }
+            }
+            if ((s->transport && s->transport != DNS_TRANSPORT_PLAIN)
+                || (s->dnssec && s->dnssec != DNS_SECURITY_NO)
+                || non_standard_server_port)
+            {
+                /* Skip servers requiring unsupported config to be set */
+                s = s->next;
+            }
+            else
+            {
+                for (int i = 0; i < s->addr_count; ++i)
+                {
+                    if (s->addr[i].family == AF_INET)
+                    {
+                        p[1] = "DNS";
+                        p[2] = print_in_addr_t(s->addr[i].in.a4.s_addr, IA_NET_ORDER, &gc);
+                    }
+                    else
+                    {
+                        p[1] = "DNS6";
+                        p[2] = print_in6_addr(s->addr[i].in.a6, 0, &gc);
+                    }
+                    setenv_foreign_option(o, (const char **)p, p_len, es);
+                }
+                break;
+            }
+        }
+    }
+
+    gc_free(&gc);
+}
+#endif /* if defined(_WIN32) || defined(TARGET_ANDROID) */
+
 static void
 options_postprocess_mutate(struct options *o, struct env_set *es)
 {
@@ -3786,9 +3895,9 @@
     else
     {
 #if defined(_WIN32) || defined(TARGET_ANDROID)
-        tuntap_options_copy_dns(o);
+        tuntap_options_postprocess_dns(o);
 #else
-        foreign_options_copy_dns(o, es);
+        dhcp_options_postprocess_dns(o, es);
 #endif
     }
     if (o->auth_token_generate && !o->auth_token_renewal)
@@ -4171,9 +4280,9 @@
     {
         dns_options_postprocess_pull(&o->dns_options);
 #if defined(_WIN32) || defined(TARGET_ANDROID)
-        tuntap_options_copy_dns(o);
+        tuntap_options_postprocess_dns(o);
 #else
-        foreign_options_copy_dns(o, es);
+        dhcp_options_postprocess_dns(o, es);
 #endif
     }
     return success;
@@ -8162,18 +8271,43 @@
             goto err;
         }
     }
-#if defined(_WIN32) || defined(TARGET_ANDROID)
     else if (streq(p[0], "dhcp-option") && p[1])
     {
+        struct dhcp_options *dhcp = &options->dns_options.from_dhcp;
+#if defined(_WIN32) || defined(TARGET_ANDROID)
         struct tuntap_options *o = &options->tuntap_options;
+#endif
         VERIFY_PERMISSION(OPT_P_DHCPDNS);
 
-        if ((streq(p[1], "DOMAIN") || streq(p[1], "ADAPTER_DOMAIN_SUFFIX"))
-            && p[2] && !p[3])
+        if ((streq(p[1], "DOMAIN") || streq(p[1], "ADAPTER_DOMAIN_SUFFIX")) && p[2] && !p[3])
         {
-            o->domain = p[2];
-            o->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL;
+            dhcp->domain = p[2];
         }
+        else if (streq(p[1], "DOMAIN-SEARCH") && p[2] && !p[3])
+        {
+            if (dhcp->domain_search_list_len < N_SEARCH_LIST_LEN)
+            {
+                dhcp->domain_search_list[dhcp->domain_search_list_len++] = p[2];
+            }
+            else
+            {
+                msg(msglevel, "--dhcp-option %s: maximum of %d search entries can be specified",
+                    p[1], N_SEARCH_LIST_LEN);
+            }
+        }
+        else if ((streq(p[1], "DNS") || streq(p[1], "DNS6")) && p[2] && !p[3]
+                 && (!strstr(p[2], ":") || ipv6_addr_safe(p[2])))
+        {
+            if (strstr(p[2], ":"))
+            {
+                dhcp_option_dns6_parse(p[2], dhcp->dns6, &dhcp->dns6_len, msglevel);
+            }
+            else
+            {
+                dhcp_option_address_parse("DNS", p[2], dhcp->dns, &dhcp->dns_len, msglevel);
+            }
+        }
+#if defined(_WIN32) || defined(TARGET_ANDROID)
         else if (streq(p[1], "NBS") && p[2] && !p[3])
         {
             o->netbios_scope = p[2];
@@ -8191,23 +8325,9 @@
             o->netbios_node_type = t;
             o->dhcp_options |= DHCP_OPTIONS_DHCP_REQUIRED;
         }
-        else if ((streq(p[1], "DNS") || streq(p[1], "DNS6")) && p[2] && !p[3]
-                 && (!strstr(p[2], ":") || ipv6_addr_safe(p[2])))
-        {
-            if (strstr(p[2], ":"))
-            {
-                dhcp_option_dns6_parse(p[2], o->dns6, &o->dns6_len, msglevel);
-            }
-            else
-            {
-                dhcp_option_address_parse("DNS", p[2], o->dns, &o->dns_len, msglevel);
-                o->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL;
-            }
-        }
         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])
         {
@@ -8219,19 +8339,6 @@
             dhcp_option_address_parse("NBDD", p[2], o->nbdd, &o->nbdd_len, msglevel);
             o->dhcp_options |= DHCP_OPTIONS_DHCP_REQUIRED;
         }
-        else if (streq(p[1], "DOMAIN-SEARCH") && p[2] && !p[3])
-        {
-            if (o->domain_search_list_len < N_SEARCH_LIST_LEN)
-            {
-                o->domain_search_list[o->domain_search_list_len++] = p[2];
-            }
-            else
-            {
-                msg(msglevel, "--dhcp-option %s: maximum of %d search entries can be specified",
-                    p[1], N_SEARCH_LIST_LEN);
-            }
-            o->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL;
-        }
         else if (streq(p[1], "DISABLE-NBT") && !p[2])
         {
             o->disable_nbt = 1;
@@ -8249,8 +8356,10 @@
             msg(msglevel, "--dhcp-option: unknown option type '%s' or missing or unknown parameter", p[1]);
             goto err;
         }
-    }
+#else /* if defined(_WIN32) || defined(TARGET_ANDROID) */
+        setenv_foreign_option(options, (const char **)p, 3, es);
 #endif /* if defined(_WIN32) || defined(TARGET_ANDROID) */
+    }
 #ifdef _WIN32
     else if (streq(p[0], "show-adapters") && !p[1])
     {

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/904?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: I635c4018fb43b5976a39b6a90cb2e9cb2570cd6a
Gerrit-Change-Number: 904
Gerrit-PatchSet: 25
Gerrit-Owner: d12fk <he...@op...>
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]: dns: deal with --dhcp-options when --dns is active

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

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

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


Change subject: dns: deal with --dhcp-options when --dns is active
......................................................................

dns: deal with --dhcp-options when --dns is active

Since --dns settings overrule DNS related --dhcp-options,
remove the latter when values were defined via --dns.

To stay as backward compatible as possible, we add foreign_options to
the script hook environment from the --dns values when a --up script
is defined. In that case the default --dns-updown is not run, even
when --dns values are present, to prevent double DNS configuration.
This way an existing --up script that deals with DNS can run, without
the immediate need to change after an openvpn upgrade and a server
pushing --dns options.

If you specify a custom --dns-updown, or force running the default
dns-updown that comes with openvpn, those compat env vars are not set
for --up scripts and the dns-updown command is run, even when there's
an --up script present.

Since Android uses the DNS values from tuntap_options, we always
override those with --dns stuff unconditionally. Also on Windows when
--ip-win32 is dynamic or adaptive, since DHCP relies on these as well.

Change-Id: I635c4018fb43b5976a39b6a90cb2e9cb2570cd6a
Signed-off-by: Heiko Hund <he...@is...>
Acked-by: Gert Doering <ge...@gr...>
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg31922.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M src/openvpn/dns.c
M src/openvpn/dns.h
M src/openvpn/options.c
3 files changed, 323 insertions(+), 184 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/04/904/25

diff --git a/src/openvpn/dns.c b/src/openvpn/dns.c
index 7cf1b63..939ae09 100644
--- a/src/openvpn/dns.c
+++ b/src/openvpn/dns.c
@@ -691,7 +691,8 @@
 static void
 run_up_down_command(bool up, struct options *o, const struct tuntap *tt, struct dns_updown_runner_info *updown_runner)
 {
-    if (!o->dns_options.updown)
+    struct dns_options *dns = &o->dns_options;
+    if (!dns->updown || (o->up_script && !dns->user_set_updown))
     {
         return;
     }
@@ -701,7 +702,7 @@
     if (!updown_runner->required)
     {
         /* Run dns updown directly */
-        status = do_run_up_down_command(up, NULL, &o->dns_options, tt);
+        status = do_run_up_down_command(up, NULL, dns, tt);
     }
     else
     {
@@ -852,6 +853,14 @@
     {
         return;
     }
+#ifdef _WIN32
+    /* Don't use iservice in DHCP mode */
+    struct tuntap_options *tto = &o->tuntap_options;
+    if (tto->ip_win32_type == IPW32_SET_DHCP_MASQ || tto->ip_win32_type == IPW32_SET_ADAPTIVE)
+    {
+        return;
+    }
+#endif
 
     /* Warn about adding servers of unsupported AF */
     const struct dns_server *s = o->dns_options.servers;
diff --git a/src/openvpn/dns.h b/src/openvpn/dns.h
index 60f5471..688daa7 100644
--- a/src/openvpn/dns.h
+++ b/src/openvpn/dns.h
@@ -76,7 +76,28 @@
 #endif
 };
 
+#ifndef N_DHCP_ADDR
+#define N_DHCP_ADDR 4
+#endif
+
+#ifndef N_SEARCH_LIST_LEN
+#define N_SEARCH_LIST_LEN 10
+#endif
+
+struct dhcp_options {
+    in_addr_t dns[N_DHCP_ADDR];
+    int dns_len;
+
+    struct in6_addr dns6[N_DHCP_ADDR];
+    int dns6_len;
+
+    const char *domain;
+    const char *domain_search_list[N_SEARCH_LIST_LEN];
+    int domain_search_list_len;
+};
+
 struct dns_options {
+    struct dhcp_options from_dhcp;
     struct dns_domain *search_domains;
     struct dns_server *servers_prepull;
     struct dns_server *servers;
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 6ea01d4..d758a67 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -1328,7 +1328,6 @@
 #endif /* ifndef ENABLE_SMALL */
 #endif /* ifdef _WIN32 */
 
-#if defined(_WIN32) || defined(TARGET_ANDROID)
 static void
 dhcp_option_dns6_parse(const char *parm, struct in6_addr *dns6_list, int *len, int msglevel)
 {
@@ -1371,150 +1370,6 @@
     }
 }
 
-/*
- * If DNS options are set use these for TUN/TAP options as well.
- * Applies to DNS, DNS6 and DOMAIN-SEARCH.
- * Existing options will be discarded.
- */
-static void
-tuntap_options_copy_dns(struct options *o)
-{
-    struct tuntap_options *tt = &o->tuntap_options;
-    struct dns_options *dns = &o->dns_options;
-
-    if (dns->search_domains)
-    {
-        tt->domain_search_list_len = 0;
-        const struct dns_domain *domain = dns->search_domains;
-        while (domain && tt->domain_search_list_len < N_SEARCH_LIST_LEN)
-        {
-            tt->domain_search_list[tt->domain_search_list_len++] = domain->name;
-            domain = domain->next;
-        }
-        if (domain)
-        {
-            msg(M_WARN, "WARNING: couldn't copy all --dns search-domains to --dhcp-option");
-        }
-        tt->dhcp_options |= DHCP_OPTIONS_DHCP_REQUIRED;
-    }
-
-    if (dns->servers)
-    {
-        tt->dns_len = 0;
-        tt->dns6_len = 0;
-        bool overflow = false;
-        const struct dns_server *server = dns->servers;
-        while (server)
-        {
-            for (int i = 0; i < server->addr_count; ++i)
-            {
-                if (server->addr[i].family == AF_INET)
-                {
-                    if (tt->dns_len >= N_DHCP_ADDR)
-                    {
-                        overflow = true;
-                        continue;
-                    }
-                    tt->dns[tt->dns_len++] = ntohl(server->addr[i].in.a4.s_addr);
-                }
-                else
-                {
-                    if (tt->dns6_len >= N_DHCP_ADDR)
-                    {
-                        overflow = true;
-                        continue;
-                    }
-                    tt->dns6[tt->dns6_len++] = server->addr[i].in.a6;
-                }
-            }
-            server = server->next;
-        }
-        if (overflow)
-        {
-            msg(M_WARN, "WARNING: couldn't copy all --dns server addresses to --dhcp-option");
-        }
-        tt->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL;
-    }
-}
-#else /* if defined(_WIN32) || defined(TARGET_ANDROID) */
-static void
-foreign_options_copy_dns(struct options *o, struct env_set *es)
-{
-    const struct dns_domain *domain = o->dns_options.search_domains;
-    const struct dns_server *server = o->dns_options.servers;
-    if (!domain && !server)
-    {
-        return;
-    }
-
-    /* reset the index since we're starting all over again */
-    int opt_max = o->foreign_option_index;
-    o->foreign_option_index = 0;
-
-    for (int i = 1; i <= opt_max; ++i)
-    {
-        char name[32];
-        snprintf(name, sizeof(name), "foreign_option_%d", i);
-
-        const char *env_str = env_set_get(es, name);
-        const char *value = strchr(env_str, '=') + 1;
-        if ((domain && strstr(value, "dhcp-option DOMAIN-SEARCH") == value)
-            || (server && strstr(value, "dhcp-option DNS") == value))
-        {
-            setenv_del(es, name);
-        }
-        else
-        {
-            setenv_foreign_option(o, &value, 1, es);
-        }
-    }
-
-    struct gc_arena gc = gc_new();
-
-    while (server)
-    {
-        for (size_t i = 0; i < server->addr_count; ++i)
-        {
-            if (server->addr[i].family == AF_INET)
-            {
-                const char *argv[] = {
-                    "dhcp-option",
-                    "DNS",
-                    print_in_addr_t(server->addr[i].in.a4.s_addr, 0, &gc)
-                };
-                setenv_foreign_option(o, argv, 3, es);
-            }
-            else
-            {
-                const char *argv[] = {
-                    "dhcp-option",
-                    "DNS6",
-                    print_in6_addr(server->addr[i].in.a6, 0, &gc)
-                };
-                setenv_foreign_option(o, argv, 3, es);
-            }
-        }
-        server = server->next;
-    }
-    while (domain)
-    {
-        const char *argv[] = { "dhcp-option", "DOMAIN-SEARCH", domain->name };
-        setenv_foreign_option(o, argv, 3, es);
-        domain = domain->next;
-    }
-
-    gc_free(&gc);
-
-    /* remove old leftover entries */
-    while (o->foreign_option_index < opt_max)
-    {
-        char name[32];
-        snprintf(name, sizeof(name), "foreign_option_%d", opt_max--);
-        setenv_del(es, name);
-    }
-}
-#endif /* if defined(_WIN32) || defined(TARGET_ANDROID) */
-
 #ifndef ENABLE_SMALL
 static const char *
 print_vlan_accept(enum vlan_acceptable_frames mode)
@@ -3603,6 +3458,260 @@
     }
 }
 
+#if defined(_WIN32) || defined(TARGET_ANDROID)
+/**
+ * @brief Postprocess DNS related settings
+ *
+ * Set TUN/TAP DNS options with values from either --dns
+ * or --dhcp-option.
+ *
+ * @param o     pointer to the options struct
+ */
+static void
+tuntap_options_postprocess_dns(struct options *o)
+{
+    struct dns_options *dns = &o->dns_options;
+    struct tuntap_options *tt = &o->tuntap_options;
+    if (!dns->servers)
+    {
+        /* Copy --dhcp-options to tuntap_options */
+        struct dhcp_options *dhcp = &dns->from_dhcp;
+        assert(sizeof(dhcp->dns) == sizeof(tt->dns));
+        assert(sizeof(dhcp->dns6) == sizeof(tt->dns6));
+        assert(sizeof(dhcp->domain_search_list) == sizeof(tt->domain_search_list));
+
+        tt->domain = dhcp->domain;
+        tt->dns_len = dhcp->dns_len;
+        tt->dns6_len = dhcp->dns6_len;
+
+        memcpy(tt->dns, dhcp->dns, sizeof(tt->dns));
+        memcpy(tt->dns6, dhcp->dns6, sizeof(tt->dns6));
+
+        tt->domain_search_list_len = dhcp->domain_search_list_len;
+        for (size_t i = 0; i < SIZE(tt->domain_search_list); ++i)
+        {
+            tt->domain_search_list[i] = dhcp->domain_search_list[i];
+        }
+
+        return;
+    }
+
+#if defined(_WIN32)
+    if (tt->ip_win32_type != IPW32_SET_DHCP_MASQ && tt->ip_win32_type != IPW32_SET_ADAPTIVE)
+    {
+        return;     /* Not in DHCP mode */
+    }
+#endif /* if defined(_WIN32) */
+
+    /* Copy --dns options to tuntap_options */
+    const struct dns_domain *d = dns->search_domains;
+    while (d && tt->domain_search_list_len + 1 < N_SEARCH_LIST_LEN)
+    {
+        tt->domain_search_list[tt->domain_search_list_len++] = d->name;
+        d = d->next;
+    }
+    if (d)
+    {
+        msg(M_WARN, "WARNING: couldn't copy all --dns search-domains to TUN/TAP");
+    }
+
+    const struct dns_server *s = dns->servers;
+    while (s)
+    {
+        bool non_standard_server_port = false;
+        for (int i = 0; i < s->addr_count; ++i)
+        {
+            if (s->addr[i].port && s->addr[i].port != 53)
+            {
+                non_standard_server_port = true;
+                break;
+            }
+        }
+        if ((s->transport && s->transport != DNS_TRANSPORT_PLAIN)
+            || (s->dnssec && s->dnssec != DNS_SECURITY_NO)
+            || non_standard_server_port)
+        {
+            /* Skip servers requiring unsupported config to be set */
+            s = s->next;
+        }
+        else
+        {
+            bool overflow = false;
+            for (int i = 0; i < s->addr_count; ++i)
+            {
+                if (s->addr[i].family == AF_INET && tt->dns_len + 1 < N_DHCP_ADDR)
+                {
+                    tt->dns[tt->dns_len++] = s->addr[i].in.a4.s_addr;
+                }
+                else if (tt->dns6_len + 1 < N_DHCP_ADDR)
+                {
+                    tt->dns6[tt->dns6_len++] = s->addr[i].in.a6;
+                }
+                else
+                {
+                    overflow = true;
+                }
+            }
+            if (overflow)
+            {
+                msg(M_WARN, "WARNING: couldn't copy all --dns server addresses to TUN/TAP");
+            }
+            return;
+        }
+    }
+}
+
+#else /* if defined(_WIN32) || defined(TARGET_ANDROID) */
+
+/**
+ * @brief Postprocess DNS related settings
+ *
+ * Discard existing --dhcp-options from the env if needed and possibly
+ * replace them with values from --dns. If no --dns servers are set copy
+ * the --dhcp-option values over for --dns-updown runs.
+ *
+ * @param o     pointer to the options struct
+ * @param es    env set to modify potentially
+ */
+static void
+dhcp_options_postprocess_dns(struct options *o, struct env_set *es)
+{
+    struct gc_arena gc = gc_new();
+    struct dns_options *dns = &o->dns_options;
+
+    if (dns->servers || dns->user_set_updown)
+    {
+        /* Clean up env from --dhcp-option DNS config */
+        struct buffer name = alloc_buf_gc(OPTION_PARM_SIZE, &gc);
+        struct buffer value = alloc_buf_gc(OPTION_PARM_SIZE, &gc);
+
+        const int fo_count = o->foreign_option_index;
+        o->foreign_option_index = 0;
+
+        for (int i = 1; i <= fo_count; ++i)
+        {
+            buf_clear(&name);
+            buf_printf(&name, "foreign_option_%d", i);
+            const char *env_str = env_set_get(es, BSTR(&name));
+            const char *item_val = strchr(env_str, '=') + 1;
+            buf_clear(&value);
+            buf_printf(&value, "%s", item_val);
+
+            /* Remove foreign option item from env set */
+            env_set_del(es, BSTR(&name));
+
+            item_val = BSTR(&value);
+            if (strncmp(item_val, "dhcp-option ", 12) != 0
+                || (strncmp(item_val + 12, "ADAPTER-DOMAIN-SUFFIX ", 22) != 0
+                    && strncmp(item_val + 12, "DOMAIN-SEARCH ", 14) != 0
+                    && strncmp(item_val + 12, "DOMAIN ", 7) != 0
+                    && strncmp(item_val + 12, "DNS6 ", 5) != 0
+                    && strncmp(item_val + 12, "DNS ", 4) != 0))
+            {
+                /* Re-set the item with potentially updated name */
+                buf_clear(&name);
+                buf_printf(&name, "foreign_option_%d", ++o->foreign_option_index);
+                setenv_str(es, BSTR(&name), BSTR(&value));
+            }
+        }
+    }
+
+    if (!dns->servers)
+    {
+        /* Copy --dhcp-options to dns_options */
+        struct dhcp_options *dhcp = &dns->from_dhcp;
+
+        if (dhcp->dns_len || dhcp->dns6_len)
+        {
+            struct dns_domain **entry = &dns->search_domains;
+            ALLOC_OBJ_CLEAR_GC(*entry, struct dns_domain, &dns->gc);
+            struct dns_domain *new = *entry;
+            new->name = dhcp->domain;
+            entry = &new->next;
+
+            for (size_t i = 0; i < dhcp->domain_search_list_len; ++i)
+            {
+                ALLOC_OBJ_CLEAR_GC(*entry, struct dns_domain, &dns->gc);
+                struct dns_domain *new = *entry;
+                new->name = dhcp->domain_search_list[i];
+                entry = &new->next;
+            }
+
+            struct dns_server *server = dns_server_get(&dns->servers, 0, &dns->gc);
+            const size_t max_addrs = SIZE(server->addr);
+            for (size_t i = 0; i < dhcp->dns_len && server->addr_count < max_addrs; ++i)
+            {
+                server->addr[server->addr_count].in.a4.s_addr = htonl(dhcp->dns[i]);
+                server->addr[server->addr_count].family = AF_INET;
+                server->addr_count += 1;
+            }
+            for (size_t i = 0; i < dhcp->dns6_len && server->addr_count < max_addrs; ++i)
+            {
+                server->addr[server->addr_count].in.a6 = dhcp->dns6[i];
+                server->addr[server->addr_count].family = AF_INET6;
+                server->addr_count += 1;
+            }
+        }
+    }
+    else if (o->up_script && !dns->user_set_updown)
+    {
+        /* Set foreign option env vars from --dns config */
+        const char *p[] = { "dhcp-option", NULL, NULL };
+        size_t p_len = sizeof(p) / sizeof(p[0]);
+
+        p[1] = "DOMAIN";
+        const struct dns_domain *d = dns->search_domains;
+        while (d)
+        {
+            p[2] = d->name;
+            setenv_foreign_option(o, (const char **)p, p_len, es);
+            d = d->next;
+        }
+
+        const struct dns_server *s = dns->servers;
+        while (s)
+        {
+            bool non_standard_server_port = false;
+            for (int i = 0; i < s->addr_count; ++i)
+            {
+                if (s->addr[i].port && s->addr[i].port != 53)
+                {
+                    non_standard_server_port = true;
+                    break;
+                }
+            }
+            if ((s->transport && s->transport != DNS_TRANSPORT_PLAIN)
+                || (s->dnssec && s->dnssec != DNS_SECURITY_NO)
+                || non_standard_server_port)
+            {
+                /* Skip servers requiring unsupported config to be set */
+                s = s->next;
+            }
+            else
+            {
+                for (int i = 0; i < s->addr_count; ++i)
+                {
+                    if (s->addr[i].family == AF_INET)
+                    {
+                        p[1] = "DNS";
+                        p[2] = print_in_addr_t(s->addr[i].in.a4.s_addr, IA_NET_ORDER, &gc);
+                    }
+                    else
+                    {
+                        p[1] = "DNS6";
+                        p[2] = print_in6_addr(s->addr[i].in.a6, 0, &gc);
+                    }
+                    setenv_foreign_option(o, (const char **)p, p_len, es);
+                }
+                break;
+            }
+        }
+    }
+
+    gc_free(&gc);
+}
+#endif /* if defined(_WIN32) || defined(TARGET_ANDROID) */
+
 static void
 options_postprocess_mutate(struct options *o, struct env_set *es)
 {
@@ -3786,9 +3895,9 @@
     else
     {
 #if defined(_WIN32) || defined(TARGET_ANDROID)
-        tuntap_options_copy_dns(o);
+        tuntap_options_postprocess_dns(o);
 #else
-        foreign_options_copy_dns(o, es);
+        dhcp_options_postprocess_dns(o, es);
 #endif
     }
     if (o->auth_token_generate && !o->auth_token_renewal)
@@ -4171,9 +4280,9 @@
     {
         dns_options_postprocess_pull(&o->dns_options);
 #if defined(_WIN32) || defined(TARGET_ANDROID)
-        tuntap_options_copy_dns(o);
+        tuntap_options_postprocess_dns(o);
 #else
-        foreign_options_copy_dns(o, es);
+        dhcp_options_postprocess_dns(o, es);
 #endif
     }
     return success;
@@ -8162,18 +8271,43 @@
             goto err;
         }
     }
-#if defined(_WIN32) || defined(TARGET_ANDROID)
     else if (streq(p[0], "dhcp-option") && p[1])
     {
+        struct dhcp_options *dhcp = &options->dns_options.from_dhcp;
+#if defined(_WIN32) || defined(TARGET_ANDROID)
         struct tuntap_options *o = &options->tuntap_options;
+#endif
         VERIFY_PERMISSION(OPT_P_DHCPDNS);
 
-        if ((streq(p[1], "DOMAIN") || streq(p[1], "ADAPTER_DOMAIN_SUFFIX"))
-            && p[2] && !p[3])
+        if ((streq(p[1], "DOMAIN") || streq(p[1], "ADAPTER_DOMAIN_SUFFIX")) && p[2] && !p[3])
         {
-            o->domain = p[2];
-            o->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL;
+            dhcp->domain = p[2];
         }
+        else if (streq(p[1], "DOMAIN-SEARCH") && p[2] && !p[3])
+        {
+            if (dhcp->domain_search_list_len < N_SEARCH_LIST_LEN)
+            {
+                dhcp->domain_search_list[dhcp->domain_search_list_len++] = p[2];
+            }
+            else
+            {
+                msg(msglevel, "--dhcp-option %s: maximum of %d search entries can be specified",
+                    p[1], N_SEARCH_LIST_LEN);
+            }
+        }
+        else if ((streq(p[1], "DNS") || streq(p[1], "DNS6")) && p[2] && !p[3]
+                 && (!strstr(p[2], ":") || ipv6_addr_safe(p[2])))
+        {
+            if (strstr(p[2], ":"))
+            {
+                dhcp_option_dns6_parse(p[2], dhcp->dns6, &dhcp->dns6_len, msglevel);
+            }
+            else
+            {
+                dhcp_option_address_parse("DNS", p[2], dhcp->dns, &dhcp->dns_len, msglevel);
+            }
+        }
+#if defined(_WIN32) || defined(TARGET_ANDROID)
         else if (streq(p[1], "NBS") && p[2] && !p[3])
         {
             o->netbios_scope = p[2];
@@ -8191,23 +8325,9 @@
             o->netbios_node_type = t;
             o->dhcp_options |= DHCP_OPTIONS_DHCP_REQUIRED;
         }
-        else if ((streq(p[1], "DNS") || streq(p[1], "DNS6")) && p[2] && !p[3]
-                 && (!strstr(p[2], ":") || ipv6_addr_safe(p[2])))
-        {
-            if (strstr(p[2], ":"))
-            {
-                dhcp_option_dns6_parse(p[2], o->dns6, &o->dns6_len, msglevel);
-            }
-            else
-            {
-                dhcp_option_address_parse("DNS", p[2], o->dns, &o->dns_len, msglevel);
-                o->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL;
-            }
-        }
         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])
         {
@@ -8219,19 +8339,6 @@
             dhcp_option_address_parse("NBDD", p[2], o->nbdd, &o->nbdd_len, msglevel);
             o->dhcp_options |= DHCP_OPTIONS_DHCP_REQUIRED;
         }
-        else if (streq(p[1], "DOMAIN-SEARCH") && p[2] && !p[3])
-        {
-            if (o->domain_search_list_len < N_SEARCH_LIST_LEN)
-            {
-                o->domain_search_list[o->domain_search_list_len++] = p[2];
-            }
-            else
-            {
-                msg(msglevel, "--dhcp-option %s: maximum of %d search entries can be specified",
-                    p[1], N_SEARCH_LIST_LEN);
-            }
-            o->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL;
-        }
         else if (streq(p[1], "DISABLE-NBT") && !p[2])
         {
             o->disable_nbt = 1;
@@ -8249,8 +8356,10 @@
             msg(msglevel, "--dhcp-option: unknown option type '%s' or missing or unknown parameter", p[1]);
             goto err;
         }
-    }
+#else /* if defined(_WIN32) || defined(TARGET_ANDROID) */
+        setenv_foreign_option(options, (const char **)p, 3, es);
 #endif /* if defined(_WIN32) || defined(TARGET_ANDROID) */
+    }
 #ifdef _WIN32
     else if (streq(p[0], "show-adapters") && !p[1])
     {

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/904?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: I635c4018fb43b5976a39b6a90cb2e9cb2570cd6a
Gerrit-Change-Number: 904
Gerrit-PatchSet: 25
Gerrit-Owner: d12fk <he...@op...>
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: dns: deal with --dhcp-options when --dns is active

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

So - finally the Grand DNS Rewrite is reaching the final commits - thanks
Heiko for all the work.

This patch brings working --dns-updown for configs where the server still
pushes "dhcp-options DNS" - and vice versa, compat "foreign_option..."
settings in --up for configs where the server pushes "--dns ..."

Windows will see DNS via iservice "but not if TAP + DHCP is active"
(compat again).

Lev has tested the various windows variants, I have tested all the
"incoming" variants (reference server can push this or that, depending
on client requests) and verified that --dns-updown applies whatever
makes sense, and alternatively "--up printenv.sh" will show the correct
DNS options.

There might be bugs lurking, but overall this does what it aims for, and
the code looks good (I stared at it quite a bit over the last months ;-) ).

NOTE: Android has not explicitely been tested, but given that it shares
the "tuntap" option handling with Windows, I expect nothing to break there.

Your patch has been applied to the master branch.

commit 2dfc4f82792b197dfa47d2bfe32c2af66b329611
Author: Heiko Hund
Date:   Wed Jun 18 14:48:29 2025 +0200

     dns: deal with --dhcp-options when --dns is active

     Signed-off-by: Heiko Hund <he...@is...>
     Acked-by: Gert Doering <ge...@gr...>
     Message-Id: <202...@gr...>
     URL: https://www.mail-archive.com/ope...@li.../msg31922.html
     Signed-off-by: Gert Doering <ge...@gr...>


--
kind regards,

Gert Doering

[Openvpn-devel] [L] Change in openvpn[master]: dns: deal with --dhcp-options when --dns is active

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

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

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

Change subject: dns: deal with --dhcp-options when --dns is active
......................................................................


Patch Set 24:

(2 comments)

Patchset:

PS24: 
Tested various combinations of incoming --dns/--dhcp-option DNS and outgoing --up and --dns-updown, and with the missing ++, it now looks really nice.


File src/openvpn/options.c:

http://gerrit.openvpn.net/c/openvpn/+/904/comment/a940614d_7efe3270 :
PS15, Line 4299:         tuntap_options_postprocess_dns(o);
> Think all this is covered in the latest push, please check.
Done



-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/904?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: I635c4018fb43b5976a39b6a90cb2e9cb2570cd6a
Gerrit-Change-Number: 904
Gerrit-PatchSet: 24
Gerrit-Owner: d12fk <he...@op...>
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-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Attention: d12fk <he...@op...>
Gerrit-Attention: stipa <lst...@gm...>
Gerrit-Comment-Date: Wed, 18 Jun 2025 12:49:50 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: cron2 <ge...@gr...>
Comment-In-Reply-To: d12fk <he...@op...>
Comment-In-Reply-To: stipa <lst...@gm...>
Gerrit-MessageType: comment

[Openvpn-devel] [PATCH v24] dns: deal with --dhcp-options when --dns is active

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

From: Heiko Hund <he...@is...>

Since --dns settings overrule DNS related --dhcp-options,
remove the latter when values were defined via --dns.

To stay as backward compatible as possible, we add foreign_options to
the script hook environment from the --dns values when a --up script
is defined. In that case the default --dns-updown is not run, even
when --dns values are present, to prevent double DNS configuration.
This way an existing --up script that deals with DNS can run, without
the immediate need to change after an openvpn upgrade and a server
pushing --dns options.

If you specify a custom --dns-updown, or force running the default
dns-updown that comes with openvpn, those compat env vars are not set
for --up scripts and the dns-updown command is run, even when there's
an --up script present.

Since Android uses the DNS values from tuntap_options, we always
override those with --dns stuff unconditionally. Also on Windows when
--ip-win32 is dynamic or adaptive, since DHCP relies on these as well.

Change-Id: I635c4018fb43b5976a39b6a90cb2e9cb2570cd6a
Signed-off-by: Heiko Hund <he...@is...>
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/+/904
This mail reflects revision 24 of this Change.

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

        
diff --git a/src/openvpn/dns.c b/src/openvpn/dns.c
index 7cf1b63..939ae09 100644
--- a/src/openvpn/dns.c
+++ b/src/openvpn/dns.c
@@ -691,7 +691,8 @@
 static void
 run_up_down_command(bool up, struct options *o, const struct tuntap *tt, struct dns_updown_runner_info *updown_runner)
 {
-    if (!o->dns_options.updown)
+    struct dns_options *dns = &o->dns_options;
+    if (!dns->updown || (o->up_script && !dns->user_set_updown))
     {
         return;
     }
@@ -701,7 +702,7 @@
     if (!updown_runner->required)
     {
         /* Run dns updown directly */
-        status = do_run_up_down_command(up, NULL, &o->dns_options, tt);
+        status = do_run_up_down_command(up, NULL, dns, tt);
     }
     else
     {
@@ -852,6 +853,14 @@
     {
         return;
     }
+#ifdef _WIN32
+    /* Don't use iservice in DHCP mode */
+    struct tuntap_options *tto = &o->tuntap_options;
+    if (tto->ip_win32_type == IPW32_SET_DHCP_MASQ || tto->ip_win32_type == IPW32_SET_ADAPTIVE)
+    {
+        return;
+    }
+#endif
 
     /* Warn about adding servers of unsupported AF */
     const struct dns_server *s = o->dns_options.servers;
diff --git a/src/openvpn/dns.h b/src/openvpn/dns.h
index 60f5471..688daa7 100644
--- a/src/openvpn/dns.h
+++ b/src/openvpn/dns.h
@@ -76,7 +76,28 @@
 #endif
 };
 
+#ifndef N_DHCP_ADDR
+#define N_DHCP_ADDR 4
+#endif
+
+#ifndef N_SEARCH_LIST_LEN
+#define N_SEARCH_LIST_LEN 10
+#endif
+
+struct dhcp_options {
+    in_addr_t dns[N_DHCP_ADDR];
+    int dns_len;
+
+    struct in6_addr dns6[N_DHCP_ADDR];
+    int dns6_len;
+
+    const char *domain;
+    const char *domain_search_list[N_SEARCH_LIST_LEN];
+    int domain_search_list_len;
+};
+
 struct dns_options {
+    struct dhcp_options from_dhcp;
     struct dns_domain *search_domains;
     struct dns_server *servers_prepull;
     struct dns_server *servers;
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 6ea01d4..d758a67 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -1328,7 +1328,6 @@
 #endif /* ifndef ENABLE_SMALL */
 #endif /* ifdef _WIN32 */
 
-#if defined(_WIN32) || defined(TARGET_ANDROID)
 static void
 dhcp_option_dns6_parse(const char *parm, struct in6_addr *dns6_list, int *len, int msglevel)
 {
@@ -1371,150 +1370,6 @@
     }
 }
 
-/*
- * If DNS options are set use these for TUN/TAP options as well.
- * Applies to DNS, DNS6 and DOMAIN-SEARCH.
- * Existing options will be discarded.
- */
-static void
-tuntap_options_copy_dns(struct options *o)
-{
-    struct tuntap_options *tt = &o->tuntap_options;
-    struct dns_options *dns = &o->dns_options;
-
-    if (dns->search_domains)
-    {
-        tt->domain_search_list_len = 0;
-        const struct dns_domain *domain = dns->search_domains;
-        while (domain && tt->domain_search_list_len < N_SEARCH_LIST_LEN)
-        {
-            tt->domain_search_list[tt->domain_search_list_len++] = domain->name;
-            domain = domain->next;
-        }
-        if (domain)
-        {
-            msg(M_WARN, "WARNING: couldn't copy all --dns search-domains to --dhcp-option");
-        }
-        tt->dhcp_options |= DHCP_OPTIONS_DHCP_REQUIRED;
-    }
-
-    if (dns->servers)
-    {
-        tt->dns_len = 0;
-        tt->dns6_len = 0;
-        bool overflow = false;
-        const struct dns_server *server = dns->servers;
-        while (server)
-        {
-            for (int i = 0; i < server->addr_count; ++i)
-            {
-                if (server->addr[i].family == AF_INET)
-                {
-                    if (tt->dns_len >= N_DHCP_ADDR)
-                    {
-                        overflow = true;
-                        continue;
-                    }
-                    tt->dns[tt->dns_len++] = ntohl(server->addr[i].in.a4.s_addr);
-                }
-                else
-                {
-                    if (tt->dns6_len >= N_DHCP_ADDR)
-                    {
-                        overflow = true;
-                        continue;
-                    }
-                    tt->dns6[tt->dns6_len++] = server->addr[i].in.a6;
-                }
-            }
-            server = server->next;
-        }
-        if (overflow)
-        {
-            msg(M_WARN, "WARNING: couldn't copy all --dns server addresses to --dhcp-option");
-        }
-        tt->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL;
-    }
-}
-#else /* if defined(_WIN32) || defined(TARGET_ANDROID) */
-static void
-foreign_options_copy_dns(struct options *o, struct env_set *es)
-{
-    const struct dns_domain *domain = o->dns_options.search_domains;
-    const struct dns_server *server = o->dns_options.servers;
-    if (!domain && !server)
-    {
-        return;
-    }
-
-    /* reset the index since we're starting all over again */
-    int opt_max = o->foreign_option_index;
-    o->foreign_option_index = 0;
-
-    for (int i = 1; i <= opt_max; ++i)
-    {
-        char name[32];
-        snprintf(name, sizeof(name), "foreign_option_%d", i);
-
-        const char *env_str = env_set_get(es, name);
-        const char *value = strchr(env_str, '=') + 1;
-        if ((domain && strstr(value, "dhcp-option DOMAIN-SEARCH") == value)
-            || (server && strstr(value, "dhcp-option DNS") == value))
-        {
-            setenv_del(es, name);
-        }
-        else
-        {
-            setenv_foreign_option(o, &value, 1, es);
-        }
-    }
-
-    struct gc_arena gc = gc_new();
-
-    while (server)
-    {
-        for (size_t i = 0; i < server->addr_count; ++i)
-        {
-            if (server->addr[i].family == AF_INET)
-            {
-                const char *argv[] = {
-                    "dhcp-option",
-                    "DNS",
-                    print_in_addr_t(server->addr[i].in.a4.s_addr, 0, &gc)
-                };
-                setenv_foreign_option(o, argv, 3, es);
-            }
-            else
-            {
-                const char *argv[] = {
-                    "dhcp-option",
-                    "DNS6",
-                    print_in6_addr(server->addr[i].in.a6, 0, &gc)
-                };
-                setenv_foreign_option(o, argv, 3, es);
-            }
-        }
-        server = server->next;
-    }
-    while (domain)
-    {
-        const char *argv[] = { "dhcp-option", "DOMAIN-SEARCH", domain->name };
-        setenv_foreign_option(o, argv, 3, es);
-        domain = domain->next;
-    }
-
-    gc_free(&gc);
-
-    /* remove old leftover entries */
-    while (o->foreign_option_index < opt_max)
-    {
-        char name[32];
-        snprintf(name, sizeof(name), "foreign_option_%d", opt_max--);
-        setenv_del(es, name);
-    }
-}
-#endif /* if defined(_WIN32) || defined(TARGET_ANDROID) */
-
 #ifndef ENABLE_SMALL
 static const char *
 print_vlan_accept(enum vlan_acceptable_frames mode)
@@ -3603,6 +3458,260 @@
     }
 }
 
+#if defined(_WIN32) || defined(TARGET_ANDROID)
+/**
+ * @brief Postprocess DNS related settings
+ *
+ * Set TUN/TAP DNS options with values from either --dns
+ * or --dhcp-option.
+ *
+ * @param o     pointer to the options struct
+ */
+static void
+tuntap_options_postprocess_dns(struct options *o)
+{
+    struct dns_options *dns = &o->dns_options;
+    struct tuntap_options *tt = &o->tuntap_options;
+    if (!dns->servers)
+    {
+        /* Copy --dhcp-options to tuntap_options */
+        struct dhcp_options *dhcp = &dns->from_dhcp;
+        assert(sizeof(dhcp->dns) == sizeof(tt->dns));
+        assert(sizeof(dhcp->dns6) == sizeof(tt->dns6));
+        assert(sizeof(dhcp->domain_search_list) == sizeof(tt->domain_search_list));
+
+        tt->domain = dhcp->domain;
+        tt->dns_len = dhcp->dns_len;
+        tt->dns6_len = dhcp->dns6_len;
+
+        memcpy(tt->dns, dhcp->dns, sizeof(tt->dns));
+        memcpy(tt->dns6, dhcp->dns6, sizeof(tt->dns6));
+
+        tt->domain_search_list_len = dhcp->domain_search_list_len;
+        for (size_t i = 0; i < SIZE(tt->domain_search_list); ++i)
+        {
+            tt->domain_search_list[i] = dhcp->domain_search_list[i];
+        }
+
+        return;
+    }
+
+#if defined(_WIN32)
+    if (tt->ip_win32_type != IPW32_SET_DHCP_MASQ && tt->ip_win32_type != IPW32_SET_ADAPTIVE)
+    {
+        return;     /* Not in DHCP mode */
+    }
+#endif /* if defined(_WIN32) */
+
+    /* Copy --dns options to tuntap_options */
+    const struct dns_domain *d = dns->search_domains;
+    while (d && tt->domain_search_list_len + 1 < N_SEARCH_LIST_LEN)
+    {
+        tt->domain_search_list[tt->domain_search_list_len++] = d->name;
+        d = d->next;
+    }
+    if (d)
+    {
+        msg(M_WARN, "WARNING: couldn't copy all --dns search-domains to TUN/TAP");
+    }
+
+    const struct dns_server *s = dns->servers;
+    while (s)
+    {
+        bool non_standard_server_port = false;
+        for (int i = 0; i < s->addr_count; ++i)
+        {
+            if (s->addr[i].port && s->addr[i].port != 53)
+            {
+                non_standard_server_port = true;
+                break;
+            }
+        }
+        if ((s->transport && s->transport != DNS_TRANSPORT_PLAIN)
+            || (s->dnssec && s->dnssec != DNS_SECURITY_NO)
+            || non_standard_server_port)
+        {
+            /* Skip servers requiring unsupported config to be set */
+            s = s->next;
+        }
+        else
+        {
+            bool overflow = false;
+            for (int i = 0; i < s->addr_count; ++i)
+            {
+                if (s->addr[i].family == AF_INET && tt->dns_len + 1 < N_DHCP_ADDR)
+                {
+                    tt->dns[tt->dns_len++] = s->addr[i].in.a4.s_addr;
+                }
+                else if (tt->dns6_len + 1 < N_DHCP_ADDR)
+                {
+                    tt->dns6[tt->dns6_len++] = s->addr[i].in.a6;
+                }
+                else
+                {
+                    overflow = true;
+                }
+            }
+            if (overflow)
+            {
+                msg(M_WARN, "WARNING: couldn't copy all --dns server addresses to TUN/TAP");
+            }
+            return;
+        }
+    }
+}
+
+#else /* if defined(_WIN32) || defined(TARGET_ANDROID) */
+
+/**
+ * @brief Postprocess DNS related settings
+ *
+ * Discard existing --dhcp-options from the env if needed and possibly
+ * replace them with values from --dns. If no --dns servers are set copy
+ * the --dhcp-option values over for --dns-updown runs.
+ *
+ * @param o     pointer to the options struct
+ * @param es    env set to modify potentially
+ */
+static void
+dhcp_options_postprocess_dns(struct options *o, struct env_set *es)
+{
+    struct gc_arena gc = gc_new();
+    struct dns_options *dns = &o->dns_options;
+
+    if (dns->servers || dns->user_set_updown)
+    {
+        /* Clean up env from --dhcp-option DNS config */
+        struct buffer name = alloc_buf_gc(OPTION_PARM_SIZE, &gc);
+        struct buffer value = alloc_buf_gc(OPTION_PARM_SIZE, &gc);
+
+        const int fo_count = o->foreign_option_index;
+        o->foreign_option_index = 0;
+
+        for (int i = 1; i <= fo_count; ++i)
+        {
+            buf_clear(&name);
+            buf_printf(&name, "foreign_option_%d", i);
+            const char *env_str = env_set_get(es, BSTR(&name));
+            const char *item_val = strchr(env_str, '=') + 1;
+            buf_clear(&value);
+            buf_printf(&value, "%s", item_val);
+
+            /* Remove foreign option item from env set */
+            env_set_del(es, BSTR(&name));
+
+            item_val = BSTR(&value);
+            if (strncmp(item_val, "dhcp-option ", 12) != 0
+                || (strncmp(item_val + 12, "ADAPTER-DOMAIN-SUFFIX ", 22) != 0
+                    && strncmp(item_val + 12, "DOMAIN-SEARCH ", 14) != 0
+                    && strncmp(item_val + 12, "DOMAIN ", 7) != 0
+                    && strncmp(item_val + 12, "DNS6 ", 5) != 0
+                    && strncmp(item_val + 12, "DNS ", 4) != 0))
+            {
+                /* Re-set the item with potentially updated name */
+                buf_clear(&name);
+                buf_printf(&name, "foreign_option_%d", ++o->foreign_option_index);
+                setenv_str(es, BSTR(&name), BSTR(&value));
+            }
+        }
+    }
+
+    if (!dns->servers)
+    {
+        /* Copy --dhcp-options to dns_options */
+        struct dhcp_options *dhcp = &dns->from_dhcp;
+
+        if (dhcp->dns_len || dhcp->dns6_len)
+        {
+            struct dns_domain **entry = &dns->search_domains;
+            ALLOC_OBJ_CLEAR_GC(*entry, struct dns_domain, &dns->gc);
+            struct dns_domain *new = *entry;
+            new->name = dhcp->domain;
+            entry = &new->next;
+
+            for (size_t i = 0; i < dhcp->domain_search_list_len; ++i)
+            {
+                ALLOC_OBJ_CLEAR_GC(*entry, struct dns_domain, &dns->gc);
+                struct dns_domain *new = *entry;
+                new->name = dhcp->domain_search_list[i];
+                entry = &new->next;
+            }
+
+            struct dns_server *server = dns_server_get(&dns->servers, 0, &dns->gc);
+            const size_t max_addrs = SIZE(server->addr);
+            for (size_t i = 0; i < dhcp->dns_len && server->addr_count < max_addrs; ++i)
+            {
+                server->addr[server->addr_count].in.a4.s_addr = htonl(dhcp->dns[i]);
+                server->addr[server->addr_count].family = AF_INET;
+                server->addr_count += 1;
+            }
+            for (size_t i = 0; i < dhcp->dns6_len && server->addr_count < max_addrs; ++i)
+            {
+                server->addr[server->addr_count].in.a6 = dhcp->dns6[i];
+                server->addr[server->addr_count].family = AF_INET6;
+                server->addr_count += 1;
+            }
+        }
+    }
+    else if (o->up_script && !dns->user_set_updown)
+    {
+        /* Set foreign option env vars from --dns config */
+        const char *p[] = { "dhcp-option", NULL, NULL };
+        size_t p_len = sizeof(p) / sizeof(p[0]);
+
+        p[1] = "DOMAIN";
+        const struct dns_domain *d = dns->search_domains;
+        while (d)
+        {
+            p[2] = d->name;
+            setenv_foreign_option(o, (const char **)p, p_len, es);
+            d = d->next;
+        }
+
+        const struct dns_server *s = dns->servers;
+        while (s)
+        {
+            bool non_standard_server_port = false;
+            for (int i = 0; i < s->addr_count; ++i)
+            {
+                if (s->addr[i].port && s->addr[i].port != 53)
+                {
+                    non_standard_server_port = true;
+                    break;
+                }
+            }
+            if ((s->transport && s->transport != DNS_TRANSPORT_PLAIN)
+                || (s->dnssec && s->dnssec != DNS_SECURITY_NO)
+                || non_standard_server_port)
+            {
+                /* Skip servers requiring unsupported config to be set */
+                s = s->next;
+            }
+            else
+            {
+                for (int i = 0; i < s->addr_count; ++i)
+                {
+                    if (s->addr[i].family == AF_INET)
+                    {
+                        p[1] = "DNS";
+                        p[2] = print_in_addr_t(s->addr[i].in.a4.s_addr, IA_NET_ORDER, &gc);
+                    }
+                    else
+                    {
+                        p[1] = "DNS6";
+                        p[2] = print_in6_addr(s->addr[i].in.a6, 0, &gc);
+                    }
+                    setenv_foreign_option(o, (const char **)p, p_len, es);
+                }
+                break;
+            }
+        }
+    }
+
+    gc_free(&gc);
+}
+#endif /* if defined(_WIN32) || defined(TARGET_ANDROID) */
+
 static void
 options_postprocess_mutate(struct options *o, struct env_set *es)
 {
@@ -3786,9 +3895,9 @@
     else
     {
 #if defined(_WIN32) || defined(TARGET_ANDROID)
-        tuntap_options_copy_dns(o);
+        tuntap_options_postprocess_dns(o);
 #else
-        foreign_options_copy_dns(o, es);
+        dhcp_options_postprocess_dns(o, es);
 #endif
     }
     if (o->auth_token_generate && !o->auth_token_renewal)
@@ -4171,9 +4280,9 @@
     {
         dns_options_postprocess_pull(&o->dns_options);
 #if defined(_WIN32) || defined(TARGET_ANDROID)
-        tuntap_options_copy_dns(o);
+        tuntap_options_postprocess_dns(o);
 #else
-        foreign_options_copy_dns(o, es);
+        dhcp_options_postprocess_dns(o, es);
 #endif
     }
     return success;
@@ -8162,18 +8271,43 @@
             goto err;
         }
     }
-#if defined(_WIN32) || defined(TARGET_ANDROID)
     else if (streq(p[0], "dhcp-option") && p[1])
     {
+        struct dhcp_options *dhcp = &options->dns_options.from_dhcp;
+#if defined(_WIN32) || defined(TARGET_ANDROID)
         struct tuntap_options *o = &options->tuntap_options;
+#endif
         VERIFY_PERMISSION(OPT_P_DHCPDNS);
 
-        if ((streq(p[1], "DOMAIN") || streq(p[1], "ADAPTER_DOMAIN_SUFFIX"))
-            && p[2] && !p[3])
+        if ((streq(p[1], "DOMAIN") || streq(p[1], "ADAPTER_DOMAIN_SUFFIX")) && p[2] && !p[3])
         {
-            o->domain = p[2];
-            o->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL;
+            dhcp->domain = p[2];
         }
+        else if (streq(p[1], "DOMAIN-SEARCH") && p[2] && !p[3])
+        {
+            if (dhcp->domain_search_list_len < N_SEARCH_LIST_LEN)
+            {
+                dhcp->domain_search_list[dhcp->domain_search_list_len++] = p[2];
+            }
+            else
+            {
+                msg(msglevel, "--dhcp-option %s: maximum of %d search entries can be specified",
+                    p[1], N_SEARCH_LIST_LEN);
+            }
+        }
+        else if ((streq(p[1], "DNS") || streq(p[1], "DNS6")) && p[2] && !p[3]
+                 && (!strstr(p[2], ":") || ipv6_addr_safe(p[2])))
+        {
+            if (strstr(p[2], ":"))
+            {
+                dhcp_option_dns6_parse(p[2], dhcp->dns6, &dhcp->dns6_len, msglevel);
+            }
+            else
+            {
+                dhcp_option_address_parse("DNS", p[2], dhcp->dns, &dhcp->dns_len, msglevel);
+            }
+        }
+#if defined(_WIN32) || defined(TARGET_ANDROID)
         else if (streq(p[1], "NBS") && p[2] && !p[3])
         {
             o->netbios_scope = p[2];
@@ -8191,23 +8325,9 @@
             o->netbios_node_type = t;
             o->dhcp_options |= DHCP_OPTIONS_DHCP_REQUIRED;
         }
-        else if ((streq(p[1], "DNS") || streq(p[1], "DNS6")) && p[2] && !p[3]
-                 && (!strstr(p[2], ":") || ipv6_addr_safe(p[2])))
-        {
-            if (strstr(p[2], ":"))
-            {
-                dhcp_option_dns6_parse(p[2], o->dns6, &o->dns6_len, msglevel);
-            }
-            else
-            {
-                dhcp_option_address_parse("DNS", p[2], o->dns, &o->dns_len, msglevel);
-                o->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL;
-            }
-        }
         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])
         {
@@ -8219,19 +8339,6 @@
             dhcp_option_address_parse("NBDD", p[2], o->nbdd, &o->nbdd_len, msglevel);
             o->dhcp_options |= DHCP_OPTIONS_DHCP_REQUIRED;
         }
-        else if (streq(p[1], "DOMAIN-SEARCH") && p[2] && !p[3])
-        {
-            if (o->domain_search_list_len < N_SEARCH_LIST_LEN)
-            {
-                o->domain_search_list[o->domain_search_list_len++] = p[2];
-            }
-            else
-            {
-                msg(msglevel, "--dhcp-option %s: maximum of %d search entries can be specified",
-                    p[1], N_SEARCH_LIST_LEN);
-            }
-            o->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL;
-        }
         else if (streq(p[1], "DISABLE-NBT") && !p[2])
         {
             o->disable_nbt = 1;
@@ -8249,8 +8356,10 @@
             msg(msglevel, "--dhcp-option: unknown option type '%s' or missing or unknown parameter", p[1]);
             goto err;
         }
-    }
+#else /* if defined(_WIN32) || defined(TARGET_ANDROID) */
+        setenv_foreign_option(options, (const char **)p, 3, es);
 #endif /* if defined(_WIN32) || defined(TARGET_ANDROID) */
+    }
 #ifdef _WIN32
     else if (streq(p[0], "show-adapters") && !p[1])
     {

[Openvpn-devel] [L] Change in openvpn[master]: dns: deal with --dhcp-options when --dns is active

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

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

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

Change subject: dns: deal with --dhcp-options when --dns is active
......................................................................


Patch Set 24: Code-Review+2


-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/904?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: I635c4018fb43b5976a39b6a90cb2e9cb2570cd6a
Gerrit-Change-Number: 904
Gerrit-PatchSet: 24
Gerrit-Owner: d12fk <he...@op...>
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-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Attention: d12fk <he...@op...>
Gerrit-Attention: stipa <lst...@gm...>
Gerrit-Comment-Date: Wed, 18 Jun 2025 12:48:16 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

[Openvpn-devel] IRC community meeting summary

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

Meeting summary for 18 June 2025:

  *

    *Updated, closed: (embargo) security issue*
    CVE ID CVE-2025-50054 reserved for this security issue.
    For OpenVPN 2.6 the Windows installer will be updated.
    For OpenVPN 2.7 a new alpha2 release will be made.
    Estimated delivery date 19 june 2025.

  *

    *Updated: Release 2.7*
    OpenVPN 2.7 alpha2 expected 19 june 2025 (includes embargoed
    security issue fix).
    For the DNS related changes, the macOS DNS script and the
    compatibility code is yet to be merged.
    For the DCO related changes, Windows server support is done, epoch
    data keys being worked on in both Linux and Windows.
    For the live route updates changes, it is likely that this will not
    make it into 2.7.

  *

    *Updated: push_update / live route updates*
    For client-side support, company did QA on it against the current
    only server implementation (cloudconnexa) and it works as expected.
    Server-side support patch is up and requires review and is a bit
    more involved, company QA will test it.

  *

    *Updated: OpenVPN community meetup 2025*
    https://community.openvpn.net/openvpn/wiki/CommunityMeetup2025
    When: weekend of 25 and 26 october.
    Where: Napoli, Italy.
    Meeting room: giaan found a first location - too expensive. will
    look for a more normal option next.
    Hotel: giaan will take a look into this.
    Beer: yes.
    T-shirts: yes.

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] [L] Change in openvpn[master]: dns: deal with --dhcp-options when --dns is active

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

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

Hello flichtenheld, plaisthos, stipa, 

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#24).


Change subject: dns: deal with --dhcp-options when --dns is active
......................................................................

dns: deal with --dhcp-options when --dns is active

Since --dns settings overrule DNS related --dhcp-options,
remove the latter when values were defined via --dns.

To stay as backward compatible as possible, we add foreign_options to
the script hook environment from the --dns values when a --up script
is defined. In that case the default --dns-updown is not run, even
when --dns values are present, to prevent double DNS configuration.
This way an existing --up script that deals with DNS can run, without
the immediate need to change after an openvpn upgrade and a server
pushing --dns options.

If you specify a custom --dns-updown, or force running the default
dns-updown that comes with openvpn, those compat env vars are not set
for --up scripts and the dns-updown command is run, even when there's
an --up script present.

Since Android uses the DNS values from tuntap_options, we always
override those with --dns stuff unconditionally. Also on Windows when
--ip-win32 is dynamic or adaptive, since DHCP relies on these as well.

Change-Id: I635c4018fb43b5976a39b6a90cb2e9cb2570cd6a
Signed-off-by: Heiko Hund <he...@is...>
---
M src/openvpn/dns.c
M src/openvpn/dns.h
M src/openvpn/options.c
3 files changed, 323 insertions(+), 184 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/04/904/24

diff --git a/src/openvpn/dns.c b/src/openvpn/dns.c
index 7cf1b63..939ae09 100644
--- a/src/openvpn/dns.c
+++ b/src/openvpn/dns.c
@@ -691,7 +691,8 @@
 static void
 run_up_down_command(bool up, struct options *o, const struct tuntap *tt, struct dns_updown_runner_info *updown_runner)
 {
-    if (!o->dns_options.updown)
+    struct dns_options *dns = &o->dns_options;
+    if (!dns->updown || (o->up_script && !dns->user_set_updown))
     {
         return;
     }
@@ -701,7 +702,7 @@
     if (!updown_runner->required)
     {
         /* Run dns updown directly */
-        status = do_run_up_down_command(up, NULL, &o->dns_options, tt);
+        status = do_run_up_down_command(up, NULL, dns, tt);
     }
     else
     {
@@ -852,6 +853,14 @@
     {
         return;
     }
+#ifdef _WIN32
+    /* Don't use iservice in DHCP mode */
+    struct tuntap_options *tto = &o->tuntap_options;
+    if (tto->ip_win32_type == IPW32_SET_DHCP_MASQ || tto->ip_win32_type == IPW32_SET_ADAPTIVE)
+    {
+        return;
+    }
+#endif
 
     /* Warn about adding servers of unsupported AF */
     const struct dns_server *s = o->dns_options.servers;
diff --git a/src/openvpn/dns.h b/src/openvpn/dns.h
index 60f5471..688daa7 100644
--- a/src/openvpn/dns.h
+++ b/src/openvpn/dns.h
@@ -76,7 +76,28 @@
 #endif
 };
 
+#ifndef N_DHCP_ADDR
+#define N_DHCP_ADDR 4
+#endif
+
+#ifndef N_SEARCH_LIST_LEN
+#define N_SEARCH_LIST_LEN 10
+#endif
+
+struct dhcp_options {
+    in_addr_t dns[N_DHCP_ADDR];
+    int dns_len;
+
+    struct in6_addr dns6[N_DHCP_ADDR];
+    int dns6_len;
+
+    const char *domain;
+    const char *domain_search_list[N_SEARCH_LIST_LEN];
+    int domain_search_list_len;
+};
+
 struct dns_options {
+    struct dhcp_options from_dhcp;
     struct dns_domain *search_domains;
     struct dns_server *servers_prepull;
     struct dns_server *servers;
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 6ea01d4..d758a67 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -1328,7 +1328,6 @@
 #endif /* ifndef ENABLE_SMALL */
 #endif /* ifdef _WIN32 */
 
-#if defined(_WIN32) || defined(TARGET_ANDROID)
 static void
 dhcp_option_dns6_parse(const char *parm, struct in6_addr *dns6_list, int *len, int msglevel)
 {
@@ -1371,150 +1370,6 @@
     }
 }
 
-/*
- * If DNS options are set use these for TUN/TAP options as well.
- * Applies to DNS, DNS6 and DOMAIN-SEARCH.
- * Existing options will be discarded.
- */
-static void
-tuntap_options_copy_dns(struct options *o)
-{
-    struct tuntap_options *tt = &o->tuntap_options;
-    struct dns_options *dns = &o->dns_options;
-
-    if (dns->search_domains)
-    {
-        tt->domain_search_list_len = 0;
-        const struct dns_domain *domain = dns->search_domains;
-        while (domain && tt->domain_search_list_len < N_SEARCH_LIST_LEN)
-        {
-            tt->domain_search_list[tt->domain_search_list_len++] = domain->name;
-            domain = domain->next;
-        }
-        if (domain)
-        {
-            msg(M_WARN, "WARNING: couldn't copy all --dns search-domains to --dhcp-option");
-        }
-        tt->dhcp_options |= DHCP_OPTIONS_DHCP_REQUIRED;
-    }
-
-    if (dns->servers)
-    {
-        tt->dns_len = 0;
-        tt->dns6_len = 0;
-        bool overflow = false;
-        const struct dns_server *server = dns->servers;
-        while (server)
-        {
-            for (int i = 0; i < server->addr_count; ++i)
-            {
-                if (server->addr[i].family == AF_INET)
-                {
-                    if (tt->dns_len >= N_DHCP_ADDR)
-                    {
-                        overflow = true;
-                        continue;
-                    }
-                    tt->dns[tt->dns_len++] = ntohl(server->addr[i].in.a4.s_addr);
-                }
-                else
-                {
-                    if (tt->dns6_len >= N_DHCP_ADDR)
-                    {
-                        overflow = true;
-                        continue;
-                    }
-                    tt->dns6[tt->dns6_len++] = server->addr[i].in.a6;
-                }
-            }
-            server = server->next;
-        }
-        if (overflow)
-        {
-            msg(M_WARN, "WARNING: couldn't copy all --dns server addresses to --dhcp-option");
-        }
-        tt->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL;
-    }
-}
-#else /* if defined(_WIN32) || defined(TARGET_ANDROID) */
-static void
-foreign_options_copy_dns(struct options *o, struct env_set *es)
-{
-    const struct dns_domain *domain = o->dns_options.search_domains;
-    const struct dns_server *server = o->dns_options.servers;
-    if (!domain && !server)
-    {
-        return;
-    }
-
-    /* reset the index since we're starting all over again */
-    int opt_max = o->foreign_option_index;
-    o->foreign_option_index = 0;
-
-    for (int i = 1; i <= opt_max; ++i)
-    {
-        char name[32];
-        snprintf(name, sizeof(name), "foreign_option_%d", i);
-
-        const char *env_str = env_set_get(es, name);
-        const char *value = strchr(env_str, '=') + 1;
-        if ((domain && strstr(value, "dhcp-option DOMAIN-SEARCH") == value)
-            || (server && strstr(value, "dhcp-option DNS") == value))
-        {
-            setenv_del(es, name);
-        }
-        else
-        {
-            setenv_foreign_option(o, &value, 1, es);
-        }
-    }
-
-    struct gc_arena gc = gc_new();
-
-    while (server)
-    {
-        for (size_t i = 0; i < server->addr_count; ++i)
-        {
-            if (server->addr[i].family == AF_INET)
-            {
-                const char *argv[] = {
-                    "dhcp-option",
-                    "DNS",
-                    print_in_addr_t(server->addr[i].in.a4.s_addr, 0, &gc)
-                };
-                setenv_foreign_option(o, argv, 3, es);
-            }
-            else
-            {
-                const char *argv[] = {
-                    "dhcp-option",
-                    "DNS6",
-                    print_in6_addr(server->addr[i].in.a6, 0, &gc)
-                };
-                setenv_foreign_option(o, argv, 3, es);
-            }
-        }
-        server = server->next;
-    }
-    while (domain)
-    {
-        const char *argv[] = { "dhcp-option", "DOMAIN-SEARCH", domain->name };
-        setenv_foreign_option(o, argv, 3, es);
-        domain = domain->next;
-    }
-
-    gc_free(&gc);
-
-    /* remove old leftover entries */
-    while (o->foreign_option_index < opt_max)
-    {
-        char name[32];
-        snprintf(name, sizeof(name), "foreign_option_%d", opt_max--);
-        setenv_del(es, name);
-    }
-}
-#endif /* if defined(_WIN32) || defined(TARGET_ANDROID) */
-
 #ifndef ENABLE_SMALL
 static const char *
 print_vlan_accept(enum vlan_acceptable_frames mode)
@@ -3603,6 +3458,260 @@
     }
 }
 
+#if defined(_WIN32) || defined(TARGET_ANDROID)
+/**
+ * @brief Postprocess DNS related settings
+ *
+ * Set TUN/TAP DNS options with values from either --dns
+ * or --dhcp-option.
+ *
+ * @param o     pointer to the options struct
+ */
+static void
+tuntap_options_postprocess_dns(struct options *o)
+{
+    struct dns_options *dns = &o->dns_options;
+    struct tuntap_options *tt = &o->tuntap_options;
+    if (!dns->servers)
+    {
+        /* Copy --dhcp-options to tuntap_options */
+        struct dhcp_options *dhcp = &dns->from_dhcp;
+        assert(sizeof(dhcp->dns) == sizeof(tt->dns));
+        assert(sizeof(dhcp->dns6) == sizeof(tt->dns6));
+        assert(sizeof(dhcp->domain_search_list) == sizeof(tt->domain_search_list));
+
+        tt->domain = dhcp->domain;
+        tt->dns_len = dhcp->dns_len;
+        tt->dns6_len = dhcp->dns6_len;
+
+        memcpy(tt->dns, dhcp->dns, sizeof(tt->dns));
+        memcpy(tt->dns6, dhcp->dns6, sizeof(tt->dns6));
+
+        tt->domain_search_list_len = dhcp->domain_search_list_len;
+        for (size_t i = 0; i < SIZE(tt->domain_search_list); ++i)
+        {
+            tt->domain_search_list[i] = dhcp->domain_search_list[i];
+        }
+
+        return;
+    }
+
+#if defined(_WIN32)
+    if (tt->ip_win32_type != IPW32_SET_DHCP_MASQ && tt->ip_win32_type != IPW32_SET_ADAPTIVE)
+    {
+        return;     /* Not in DHCP mode */
+    }
+#endif /* if defined(_WIN32) */
+
+    /* Copy --dns options to tuntap_options */
+    const struct dns_domain *d = dns->search_domains;
+    while (d && tt->domain_search_list_len + 1 < N_SEARCH_LIST_LEN)
+    {
+        tt->domain_search_list[tt->domain_search_list_len++] = d->name;
+        d = d->next;
+    }
+    if (d)
+    {
+        msg(M_WARN, "WARNING: couldn't copy all --dns search-domains to TUN/TAP");
+    }
+
+    const struct dns_server *s = dns->servers;
+    while (s)
+    {
+        bool non_standard_server_port = false;
+        for (int i = 0; i < s->addr_count; ++i)
+        {
+            if (s->addr[i].port && s->addr[i].port != 53)
+            {
+                non_standard_server_port = true;
+                break;
+            }
+        }
+        if ((s->transport && s->transport != DNS_TRANSPORT_PLAIN)
+            || (s->dnssec && s->dnssec != DNS_SECURITY_NO)
+            || non_standard_server_port)
+        {
+            /* Skip servers requiring unsupported config to be set */
+            s = s->next;
+        }
+        else
+        {
+            bool overflow = false;
+            for (int i = 0; i < s->addr_count; ++i)
+            {
+                if (s->addr[i].family == AF_INET && tt->dns_len + 1 < N_DHCP_ADDR)
+                {
+                    tt->dns[tt->dns_len++] = s->addr[i].in.a4.s_addr;
+                }
+                else if (tt->dns6_len + 1 < N_DHCP_ADDR)
+                {
+                    tt->dns6[tt->dns6_len++] = s->addr[i].in.a6;
+                }
+                else
+                {
+                    overflow = true;
+                }
+            }
+            if (overflow)
+            {
+                msg(M_WARN, "WARNING: couldn't copy all --dns server addresses to TUN/TAP");
+            }
+            return;
+        }
+    }
+}
+
+#else /* if defined(_WIN32) || defined(TARGET_ANDROID) */
+
+/**
+ * @brief Postprocess DNS related settings
+ *
+ * Discard existing --dhcp-options from the env if needed and possibly
+ * replace them with values from --dns. If no --dns servers are set copy
+ * the --dhcp-option values over for --dns-updown runs.
+ *
+ * @param o     pointer to the options struct
+ * @param es    env set to modify potentially
+ */
+static void
+dhcp_options_postprocess_dns(struct options *o, struct env_set *es)
+{
+    struct gc_arena gc = gc_new();
+    struct dns_options *dns = &o->dns_options;
+
+    if (dns->servers || dns->user_set_updown)
+    {
+        /* Clean up env from --dhcp-option DNS config */
+        struct buffer name = alloc_buf_gc(OPTION_PARM_SIZE, &gc);
+        struct buffer value = alloc_buf_gc(OPTION_PARM_SIZE, &gc);
+
+        const int fo_count = o->foreign_option_index;
+        o->foreign_option_index = 0;
+
+        for (int i = 1; i <= fo_count; ++i)
+        {
+            buf_clear(&name);
+            buf_printf(&name, "foreign_option_%d", i);
+            const char *env_str = env_set_get(es, BSTR(&name));
+            const char *item_val = strchr(env_str, '=') + 1;
+            buf_clear(&value);
+            buf_printf(&value, "%s", item_val);
+
+            /* Remove foreign option item from env set */
+            env_set_del(es, BSTR(&name));
+
+            item_val = BSTR(&value);
+            if (strncmp(item_val, "dhcp-option ", 12) != 0
+                || (strncmp(item_val + 12, "ADAPTER-DOMAIN-SUFFIX ", 22) != 0
+                    && strncmp(item_val + 12, "DOMAIN-SEARCH ", 14) != 0
+                    && strncmp(item_val + 12, "DOMAIN ", 7) != 0
+                    && strncmp(item_val + 12, "DNS6 ", 5) != 0
+                    && strncmp(item_val + 12, "DNS ", 4) != 0))
+            {
+                /* Re-set the item with potentially updated name */
+                buf_clear(&name);
+                buf_printf(&name, "foreign_option_%d", ++o->foreign_option_index);
+                setenv_str(es, BSTR(&name), BSTR(&value));
+            }
+        }
+    }
+
+    if (!dns->servers)
+    {
+        /* Copy --dhcp-options to dns_options */
+        struct dhcp_options *dhcp = &dns->from_dhcp;
+
+        if (dhcp->dns_len || dhcp->dns6_len)
+        {
+            struct dns_domain **entry = &dns->search_domains;
+            ALLOC_OBJ_CLEAR_GC(*entry, struct dns_domain, &dns->gc);
+            struct dns_domain *new = *entry;
+            new->name = dhcp->domain;
+            entry = &new->next;
+
+            for (size_t i = 0; i < dhcp->domain_search_list_len; ++i)
+            {
+                ALLOC_OBJ_CLEAR_GC(*entry, struct dns_domain, &dns->gc);
+                struct dns_domain *new = *entry;
+                new->name = dhcp->domain_search_list[i];
+                entry = &new->next;
+            }
+
+            struct dns_server *server = dns_server_get(&dns->servers, 0, &dns->gc);
+            const size_t max_addrs = SIZE(server->addr);
+            for (size_t i = 0; i < dhcp->dns_len && server->addr_count < max_addrs; ++i)
+            {
+                server->addr[server->addr_count].in.a4.s_addr = htonl(dhcp->dns[i]);
+                server->addr[server->addr_count].family = AF_INET;
+                server->addr_count += 1;
+            }
+            for (size_t i = 0; i < dhcp->dns6_len && server->addr_count < max_addrs; ++i)
+            {
+                server->addr[server->addr_count].in.a6 = dhcp->dns6[i];
+                server->addr[server->addr_count].family = AF_INET6;
+                server->addr_count += 1;
+            }
+        }
+    }
+    else if (o->up_script && !dns->user_set_updown)
+    {
+        /* Set foreign option env vars from --dns config */
+        const char *p[] = { "dhcp-option", NULL, NULL };
+        size_t p_len = sizeof(p) / sizeof(p[0]);
+
+        p[1] = "DOMAIN";
+        const struct dns_domain *d = dns->search_domains;
+        while (d)
+        {
+            p[2] = d->name;
+            setenv_foreign_option(o, (const char **)p, p_len, es);
+            d = d->next;
+        }
+
+        const struct dns_server *s = dns->servers;
+        while (s)
+        {
+            bool non_standard_server_port = false;
+            for (int i = 0; i < s->addr_count; ++i)
+            {
+                if (s->addr[i].port && s->addr[i].port != 53)
+                {
+                    non_standard_server_port = true;
+                    break;
+                }
+            }
+            if ((s->transport && s->transport != DNS_TRANSPORT_PLAIN)
+                || (s->dnssec && s->dnssec != DNS_SECURITY_NO)
+                || non_standard_server_port)
+            {
+                /* Skip servers requiring unsupported config to be set */
+                s = s->next;
+            }
+            else
+            {
+                for (int i = 0; i < s->addr_count; ++i)
+                {
+                    if (s->addr[i].family == AF_INET)
+                    {
+                        p[1] = "DNS";
+                        p[2] = print_in_addr_t(s->addr[i].in.a4.s_addr, IA_NET_ORDER, &gc);
+                    }
+                    else
+                    {
+                        p[1] = "DNS6";
+                        p[2] = print_in6_addr(s->addr[i].in.a6, 0, &gc);
+                    }
+                    setenv_foreign_option(o, (const char **)p, p_len, es);
+                }
+                break;
+            }
+        }
+    }
+
+    gc_free(&gc);
+}
+#endif /* if defined(_WIN32) || defined(TARGET_ANDROID) */
+
 static void
 options_postprocess_mutate(struct options *o, struct env_set *es)
 {
@@ -3786,9 +3895,9 @@
     else
     {
 #if defined(_WIN32) || defined(TARGET_ANDROID)
-        tuntap_options_copy_dns(o);
+        tuntap_options_postprocess_dns(o);
 #else
-        foreign_options_copy_dns(o, es);
+        dhcp_options_postprocess_dns(o, es);
 #endif
     }
     if (o->auth_token_generate && !o->auth_token_renewal)
@@ -4171,9 +4280,9 @@
     {
         dns_options_postprocess_pull(&o->dns_options);
 #if defined(_WIN32) || defined(TARGET_ANDROID)
-        tuntap_options_copy_dns(o);
+        tuntap_options_postprocess_dns(o);
 #else
-        foreign_options_copy_dns(o, es);
+        dhcp_options_postprocess_dns(o, es);
 #endif
     }
     return success;
@@ -8162,18 +8271,43 @@
             goto err;
         }
     }
-#if defined(_WIN32) || defined(TARGET_ANDROID)
     else if (streq(p[0], "dhcp-option") && p[1])
     {
+        struct dhcp_options *dhcp = &options->dns_options.from_dhcp;
+#if defined(_WIN32) || defined(TARGET_ANDROID)
         struct tuntap_options *o = &options->tuntap_options;
+#endif
         VERIFY_PERMISSION(OPT_P_DHCPDNS);
 
-        if ((streq(p[1], "DOMAIN") || streq(p[1], "ADAPTER_DOMAIN_SUFFIX"))
-            && p[2] && !p[3])
+        if ((streq(p[1], "DOMAIN") || streq(p[1], "ADAPTER_DOMAIN_SUFFIX")) && p[2] && !p[3])
         {
-            o->domain = p[2];
-            o->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL;
+            dhcp->domain = p[2];
         }
+        else if (streq(p[1], "DOMAIN-SEARCH") && p[2] && !p[3])
+        {
+            if (dhcp->domain_search_list_len < N_SEARCH_LIST_LEN)
+            {
+                dhcp->domain_search_list[dhcp->domain_search_list_len++] = p[2];
+            }
+            else
+            {
+                msg(msglevel, "--dhcp-option %s: maximum of %d search entries can be specified",
+                    p[1], N_SEARCH_LIST_LEN);
+            }
+        }
+        else if ((streq(p[1], "DNS") || streq(p[1], "DNS6")) && p[2] && !p[3]
+                 && (!strstr(p[2], ":") || ipv6_addr_safe(p[2])))
+        {
+            if (strstr(p[2], ":"))
+            {
+                dhcp_option_dns6_parse(p[2], dhcp->dns6, &dhcp->dns6_len, msglevel);
+            }
+            else
+            {
+                dhcp_option_address_parse("DNS", p[2], dhcp->dns, &dhcp->dns_len, msglevel);
+            }
+        }
+#if defined(_WIN32) || defined(TARGET_ANDROID)
         else if (streq(p[1], "NBS") && p[2] && !p[3])
         {
             o->netbios_scope = p[2];
@@ -8191,23 +8325,9 @@
             o->netbios_node_type = t;
             o->dhcp_options |= DHCP_OPTIONS_DHCP_REQUIRED;
         }
-        else if ((streq(p[1], "DNS") || streq(p[1], "DNS6")) && p[2] && !p[3]
-                 && (!strstr(p[2], ":") || ipv6_addr_safe(p[2])))
-        {
-            if (strstr(p[2], ":"))
-            {
-                dhcp_option_dns6_parse(p[2], o->dns6, &o->dns6_len, msglevel);
-            }
-            else
-            {
-                dhcp_option_address_parse("DNS", p[2], o->dns, &o->dns_len, msglevel);
-                o->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL;
-            }
-        }
         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])
         {
@@ -8219,19 +8339,6 @@
             dhcp_option_address_parse("NBDD", p[2], o->nbdd, &o->nbdd_len, msglevel);
             o->dhcp_options |= DHCP_OPTIONS_DHCP_REQUIRED;
         }
-        else if (streq(p[1], "DOMAIN-SEARCH") && p[2] && !p[3])
-        {
-            if (o->domain_search_list_len < N_SEARCH_LIST_LEN)
-            {
-                o->domain_search_list[o->domain_search_list_len++] = p[2];
-            }
-            else
-            {
-                msg(msglevel, "--dhcp-option %s: maximum of %d search entries can be specified",
-                    p[1], N_SEARCH_LIST_LEN);
-            }
-            o->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL;
-        }
         else if (streq(p[1], "DISABLE-NBT") && !p[2])
         {
             o->disable_nbt = 1;
@@ -8249,8 +8356,10 @@
             msg(msglevel, "--dhcp-option: unknown option type '%s' or missing or unknown parameter", p[1]);
             goto err;
         }
-    }
+#else /* if defined(_WIN32) || defined(TARGET_ANDROID) */
+        setenv_foreign_option(options, (const char **)p, 3, es);
 #endif /* if defined(_WIN32) || defined(TARGET_ANDROID) */
+    }
 #ifdef _WIN32
     else if (streq(p[0], "show-adapters") && !p[1])
     {

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/904?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: I635c4018fb43b5976a39b6a90cb2e9cb2570cd6a
Gerrit-Change-Number: 904
Gerrit-PatchSet: 24
Gerrit-Owner: d12fk <he...@op...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-Reviewer: stipa <lst...@gm...>
Gerrit-CC: cron2 <ge...@gr...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: cron2 <ge...@gr...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Attention: stipa <lst...@gm...>
Gerrit-MessageType: newpatchset

[Openvpn-devel] [M] Change in openvpn[master]: Added PQE to WolfSSL

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

Attention is currently required from: comododragon, flichtenheld.

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

Change subject: Added PQE to WolfSSL
......................................................................


Patch Set 1:

(1 comment)

Patchset:

PS1: 
You don't need to close/reopen to update your patch, you can just push the change again. But here is the same comment that I had on the original version of the patch:

I feel this is the wrong approach. We are trying to avoid the NID dance. And it seems that wolfSSL already has a function 

int wolfSSL_CTX_set1_groups_list(WOLFSSL_CTX *ctx, const char *list)

that has the same (better) functionality of its SSL_CTX_set1_groups_list OpenSSL equivalent. So rather than doing this NID dance, I would prefer that we use the proper function instead. 

Maybe wolfSSL upstream can also be convinced to map this to SSL_CTX_set1_groups_list as well.



-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1046?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: Ie0529c2074964b3be034f01e0ef53090a6edbd35
Gerrit-Change-Number: 1046
Gerrit-PatchSet: 1
Gerrit-Owner: comododragon <rei...@fo...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Attention: comododragon <rei...@fo...>
Gerrit-Comment-Date: Wed, 18 Jun 2025 09:12:08 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

[Openvpn-devel] [XS] Change in openvpn[master]: fix typo in haikuos dns-updown script

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

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

Change subject: fix typo in haikuos dns-updown script
......................................................................

fix typo in haikuos dns-updown script

Change-Id: I48cec222d46c67e6620281cd8b2346323b546fcd
Signed-off-by: Heiko Hund <he...@is...>
Acked-by: Gert Doering <ge...@gr...>
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg31911.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M distro/dns-scripts/haikuos_file-dns-updown.sh
1 file changed, 1 insertion(+), 1 deletion(-)




diff --git a/distro/dns-scripts/haikuos_file-dns-updown.sh b/distro/dns-scripts/haikuos_file-dns-updown.sh
index 777b72d..6da0954 100644
--- a/distro/dns-scripts/haikuos_file-dns-updown.sh
+++ b/distro/dns-scripts/haikuos_file-dns-updown.sh
@@ -28,7 +28,7 @@
 
 set -e +u
 
-conly_standard_server_ports() {
+only_standard_server_ports() {
     i=1
     while true; do
         eval addr=\"\$dns_server_${n}_address_${i}\"

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1061?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: I48cec222d46c67e6620281cd8b2346323b546fcd
Gerrit-Change-Number: 1061
Gerrit-PatchSet: 3
Gerrit-Owner: d12fk <he...@op...>
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] [XS] Change in openvpn[master]: fix typo in haikuos dns-updown script

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

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

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


Change subject: fix typo in haikuos dns-updown script
......................................................................

fix typo in haikuos dns-updown script

Change-Id: I48cec222d46c67e6620281cd8b2346323b546fcd
Signed-off-by: Heiko Hund <he...@is...>
Acked-by: Gert Doering <ge...@gr...>
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg31911.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M distro/dns-scripts/haikuos_file-dns-updown.sh
1 file changed, 1 insertion(+), 1 deletion(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/61/1061/3

diff --git a/distro/dns-scripts/haikuos_file-dns-updown.sh b/distro/dns-scripts/haikuos_file-dns-updown.sh
index 777b72d..6da0954 100644
--- a/distro/dns-scripts/haikuos_file-dns-updown.sh
+++ b/distro/dns-scripts/haikuos_file-dns-updown.sh
@@ -28,7 +28,7 @@
 
 set -e +u
 
-conly_standard_server_ports() {
+only_standard_server_ports() {
     i=1
     while true; do
         eval addr=\"\$dns_server_${n}_address_${i}\"

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1061?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: I48cec222d46c67e6620281cd8b2346323b546fcd
Gerrit-Change-Number: 1061
Gerrit-PatchSet: 3
Gerrit-Owner: d12fk <he...@op...>
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 typo in haikuos dns-updown script

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

It would certainly help to actually test things on the right target
platform...  but this typo fix is obviously-correct.

Your patch has been applied to the master branch.

commit 51b7dc5c1093b6df82aefd464873acb9ffc93882
Author: Heiko Hund
Date:   Wed Jun 18 08:48:53 2025 +0200

     fix typo in haikuos dns-updown script

     Signed-off-by: Heiko Hund <he...@is...>
     Acked-by: Gert Doering <ge...@gr...>
     Message-Id: <202...@gr...>
     URL: https://www.mail-archive.com/ope...@li.../msg31911.html
     Signed-off-by: Gert Doering <ge...@gr...>


--
kind regards,

Gert Doering

[Openvpn-devel] [M] Change in openvpn[master]: Added PQE to WolfSSL

[comododragon (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/+/1046?usp=email

to review the following change.


Change subject: Added PQE to WolfSSL
......................................................................

Added PQE to WolfSSL

Change-Id: Ie0529c2074964b3be034f01e0ef53090a6edbd35
---
M README.wolfssl
M src/openvpn/ssl_openssl.c
A src/openvpn/wolfssl_compat.h
3 files changed, 92 insertions(+), 0 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/46/1046/1

diff --git a/README.wolfssl b/README.wolfssl
index a5dfe31..27b9087 100644
--- a/README.wolfssl
+++ b/README.wolfssl
@@ -28,3 +28,40 @@
  * blowfish support (BF-CBC), you must use something like
    cipher AES-128-CBC to avoid trying to use BF-CBC
  * Windows CryptoAPI support
+
+*************************************************************************
+To build WolfSSL with these PQE, you'll want to configure it like this:
+
+```bash
+./configure --enable-openvpn --enable-kyber --enable-mlkem --enable-curve25519
+```
+
+WolfSSL supports the following Quantum Safe algorithms by specifying them using the `tls-groups` 
+option in a config.
+
+```
+ML_KEM_512
+ML_KEM_768
+ML_KEM_1024
+P256_ML_KEM_512
+P384_ML_KEM_768
+P256_ML_KEM_768
+P521_ML_KEM_1024
+P384_ML_KEM_1024
+X25519_ML_KEM_512
+X448_ML_KEM_768
+X25519_ML_KEM_768
+
+KYBER_LEVEL1
+KYBER_LEVEL3
+KYBER_LEVEL5
+
+P256_KYBER_LEVEL1
+P384_KYBER_LEVEL3
+P256_KYBER_LEVEL3
+P521_KYBER_LEVEL5
+
+X25519_KYBER_LEVEL1
+X448_KYBER_LEVEL3
+X25519_KYBER_LEVEL3
+```
diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index 2fc77d8..29aa502 100644
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -76,6 +76,8 @@
 #include <openssl/applink.c>
 #endif
 
+#include "wolfssl_compat.h"
+
 OSSL_LIB_CTX *tls_libctx; /* Global */
 
 static void unload_xkey_provider(void);
@@ -588,7 +590,12 @@
         {
             token = "prime256v1";
         }
+
+#ifdef ENABLE_CRYPTO_WOLFSSL
+        int nid = wolfssl_OBJ_sn2nid(token);
+#else
         int nid = OBJ_sn2nid(token);
+#endif
 
         if (nid == 0)
         {
diff --git a/src/openvpn/wolfssl_compat.h b/src/openvpn/wolfssl_compat.h
new file mode 100644
index 0000000..a25e5c3
--- /dev/null
+++ b/src/openvpn/wolfssl_compat.h
@@ -0,0 +1,48 @@
+#pragma once
+
+/*
+ *  OpenVPN -- An application to securely tunnel IP networks
+ *             over a single TCP/UDP port, with support for SSL/TLS-based
+ *             session authentication and key exchange,
+ *             packet encryption, packet authentication, and
+ *             packet compression.
+ *
+ *  Copyright (C) 2023 Fox Crypto B.V. <op...@fo...>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2
+ *  as published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License along
+ *  with this program; if not, write to the Free Software Foundation, Inc.,
+ *  51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+/**
+ * @file
+ * wolfssl compatibility stub.
+ * This file provide compatibility stubs for the wolfssl libraries
+ */
+
+#ifdef ENABLE_CRYPTO_WOLFSSL
+
+int
+wolfssl_OBJ_sn2nid(const char *sn)
+{
+
+	int nid = OBJ_sn2nid(sn);
+
+	if (nid == WC_NID_undef)
+	{
+		return wolfSSL_EC_curve_nist2nid(sn);
+	}
+
+	return WC_NID_undef;
+}
+
+#endif

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1046?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: Ie0529c2074964b3be034f01e0ef53090a6edbd35
Gerrit-Change-Number: 1046
Gerrit-PatchSet: 1
Gerrit-Owner: comododragon <rei...@fo...>
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] [S] Change in openvpn[master]: Removed trailing whitespaces

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

comododragon has abandoned this change. ( http://gerrit.openvpn.net/c/openvpn/+/1045?usp=email )

Change subject: Removed trailing whitespaces
......................................................................


Abandoned
-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1045?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: I8eb1ded62a415a0734de11a4503cffafcc23d9f3
Gerrit-Change-Number: 1045
Gerrit-PatchSet: 1
Gerrit-Owner: comododragon <rei...@fo...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-MessageType: abandon

[Openvpn-devel] [S] Change in openvpn[master]: Removed trailing whitespaces

[comododragon (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/+/1045?usp=email

to review the following change.


Change subject: Removed trailing whitespaces
......................................................................

Removed trailing whitespaces

Change-Id: I8eb1ded62a415a0734de11a4503cffafcc23d9f3
---
M src/openvpn/ssl_openssl.c
M src/openvpn/wolfssl_compat.h
2 files changed, 9 insertions(+), 9 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/45/1045/1

diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index 5dbacf0..29aa502 100644
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -593,7 +593,7 @@
 
 #ifdef ENABLE_CRYPTO_WOLFSSL
         int nid = wolfssl_OBJ_sn2nid(token);
-#else 
+#else
         int nid = OBJ_sn2nid(token);
 #endif
 
diff --git a/src/openvpn/wolfssl_compat.h b/src/openvpn/wolfssl_compat.h
index c16d918..a25e5c3 100644
--- a/src/openvpn/wolfssl_compat.h
+++ b/src/openvpn/wolfssl_compat.h
@@ -31,18 +31,18 @@
 
 #ifdef ENABLE_CRYPTO_WOLFSSL
 
-int 
-wolfssl_OBJ_sn2nid(const char *sn) 
+int
+wolfssl_OBJ_sn2nid(const char *sn)
 {
 
-    int nid = OBJ_sn2nid(sn);
+	int nid = OBJ_sn2nid(sn);
 
-    if (nid == WC_NID_undef) 
-    {
-        return wolfSSL_EC_curve_nist2nid(sn);
-    }
+	if (nid == WC_NID_undef)
+	{
+		return wolfSSL_EC_curve_nist2nid(sn);
+	}
 
-    return WC_NID_undef;
+	return WC_NID_undef;
 }
 
 #endif

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1045?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: I8eb1ded62a415a0734de11a4503cffafcc23d9f3
Gerrit-Change-Number: 1045
Gerrit-PatchSet: 1
Gerrit-Owner: comododragon <rei...@fo...>
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