openvpn-devel — OpenVPN developers list

[Openvpn-devel] [XS] Change in openvpn[master]: Add ASSERT to afunix code that dev_node is always set up the way we e...

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

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

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


Change subject: Add ASSERT to afunix code that dev_node is always set up the way we expect
......................................................................

Add ASSERT to afunix code that dev_node is always set up the way we expect

The calling code only calls tun_afunix_exec_child if is_tun_afunix is true,
which checks that the path is having unix: as prefix.

But since adding an ASSERT here to ensure that it is really the case
does not cost us anything, just add the ASSERT.

Reported-By: Joshua Rogers <co...@jo...>
Found-By: Zeropath

Change-Id: Idbb7bf279eb467fc1d56ab75a50b5eb2c8d0a57e
Signed-off-by: Arne Schwabe <arn...@rf...>
Acked-by: Gert Doering <ge...@gr...>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1320
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg33934.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M src/openvpn/tun_afunix.c
1 file changed, 2 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/20/1320/3

diff --git a/src/openvpn/tun_afunix.c b/src/openvpn/tun_afunix.c
index 124db6d..42bcd0d 100644
--- a/src/openvpn/tun_afunix.c
+++ b/src/openvpn/tun_afunix.c
@@ -53,6 +53,8 @@
     const char *msgprefix = "ERROR: failure executing process for tun:";
     struct argv argv = argv_new();
 
+    /* we should always called with a proper unix: dev node string */
+    ASSERT(dev_node && strncmp(dev_node, "unix:", strlen("unix:")) == 0);
     /* since we know that dev-node starts with unix: we can just skip that
      * to get the program name */
     const char *program = dev_node + strlen("unix:");

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

Gerrit-MessageType: newpatchset
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Idbb7bf279eb467fc1d56ab75a50b5eb2c8d0a57e
Gerrit-Change-Number: 1320
Gerrit-PatchSet: 3
Gerrit-Owner: plaisthos <arn...@rf...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-CC: openvpn-devel <ope...@li...>

[Openvpn-devel] [XS] Change in openvpn[master]: Add ASSERT to afunix code that dev_node is always set up the way we e...

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

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

Change subject: Add ASSERT to afunix code that dev_node is always set up the way we expect
......................................................................

Add ASSERT to afunix code that dev_node is always set up the way we expect

The calling code only calls tun_afunix_exec_child if is_tun_afunix is true,
which checks that the path is having unix: as prefix.

But since adding an ASSERT here to ensure that it is really the case
does not cost us anything, just add the ASSERT.

Reported-By: Joshua Rogers <co...@jo...>
Found-By: Zeropath

Change-Id: Idbb7bf279eb467fc1d56ab75a50b5eb2c8d0a57e
Signed-off-by: Arne Schwabe <arn...@rf...>
Acked-by: Gert Doering <ge...@gr...>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1320
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg33934.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M src/openvpn/tun_afunix.c
1 file changed, 2 insertions(+), 0 deletions(-)




diff --git a/src/openvpn/tun_afunix.c b/src/openvpn/tun_afunix.c
index 124db6d..42bcd0d 100644
--- a/src/openvpn/tun_afunix.c
+++ b/src/openvpn/tun_afunix.c
@@ -53,6 +53,8 @@
     const char *msgprefix = "ERROR: failure executing process for tun:";
     struct argv argv = argv_new();
 
+    /* we should always called with a proper unix: dev node string */
+    ASSERT(dev_node && strncmp(dev_node, "unix:", strlen("unix:")) == 0);
     /* since we know that dev-node starts with unix: we can just skip that
      * to get the program name */
     const char *program = dev_node + strlen("unix:");

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

Gerrit-MessageType: merged
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Idbb7bf279eb467fc1d56ab75a50b5eb2c8d0a57e
Gerrit-Change-Number: 1320
Gerrit-PatchSet: 3
Gerrit-Owner: plaisthos <arn...@rf...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-CC: openvpn-devel <ope...@li...>

[Openvpn-devel] [PATCH applied] Re: Add ASSERT to afunix code that dev_node is always set up the way we expect

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

Looks reasonable, BB is happy, and the unit tests excercising tun_afunix
(t_server_null) still pass.  Also, I have a t_client test excercising
this, which still works :-)

I have updated the commit message a bit (language, Reported-By:), and
for one decided to try doing this in gerrit - it works, but is not the
best way to do it (it then does a "v2" of the patch, which hides the
test result of the actual patch, and creates extra noise on the list).

Your patch has been applied to the master branch.

commit 5bc0eae87ccf1abd6c400cb27d8e51819feb2036
Author: Arne Schwabe
Date:   Tue Oct 28 12:59:47 2025 +0100

     Add ASSERT to afunix code that dev_node is always set up the way we expect

     Signed-off-by: Arne Schwabe <arn...@rf...>
     Acked-by: Gert Doering <ge...@gr...>
     Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1320
     Message-Id: <202...@gr...>
     URL: https://www.mail-archive.com/ope...@li.../msg33934.html
     Signed-off-by: Gert Doering <ge...@gr...>


--
kind regards,

Gert Doering

[Openvpn-devel] [PATCH v2] Add ASSERT to afunix code that dev_node is always set up the way we expect

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

From: Arne Schwabe <ar...@rf...>

The calling code only calls tun_afunix_exec_child if is_tun_afunix is true,
which checks that the path is having unix: as prefix.

But since adding an ASSERT here to ensure that it is really the case
does not cost us anything, just add the ASSERT.

Reported-By: Joshua Rogers <co...@jo...>
Found-By: Zeropath

Change-Id: Idbb7bf279eb467fc1d56ab75a50b5eb2c8d0a57e
Signed-off-by: Arne Schwabe <arn...@rf...>
Acked-by: Gert Doering <ge...@gr...>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1320
---

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

Signed-off-by line for the author was added as per our policy.

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

        
diff --git a/src/openvpn/tun_afunix.c b/src/openvpn/tun_afunix.c
index 4d48a31..e6f2be1 100644
--- a/src/openvpn/tun_afunix.c
+++ b/src/openvpn/tun_afunix.c
@@ -53,6 +53,8 @@
     const char *msgprefix = "ERROR: failure executing process for tun:";
     struct argv argv = argv_new();
 
+    /* we should always called with a proper unix: dev node string */
+    ASSERT(dev_node && strncmp(dev_node, "unix:", strlen("unix:")) == 0);
     /* since we know that dev-node starts with unix: we can just skip that
      * to get the program name */
     const char *program = dev_node + strlen("unix:");

[Openvpn-devel] [XS] Change in openvpn[master]: Add ASSERT to afunix code that dev_node is always set up the way we e...

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

Attention is currently required from: flichtenheld, plaisthos.

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

Change subject: Add ASSERT to afunix code that dev_node is always set up the way we expect
......................................................................


Patch Set 2: Code-Review+2


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

Gerrit-MessageType: comment
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Idbb7bf279eb467fc1d56ab75a50b5eb2c8d0a57e
Gerrit-Change-Number: 1320
Gerrit-PatchSet: 2
Gerrit-Owner: plaisthos <arn...@rf...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Comment-Date: Tue, 28 Oct 2025 11:59:38 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes

[Openvpn-devel] [XS] Change in openvpn[master]: Add ASSERT to afunix code that dev_node is always set up the way we e...

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

Attention is currently required from: flichtenheld.

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


Change subject: Add ASSERT to afunix code that dev_node is always set up the way we expect
......................................................................

Add ASSERT to afunix code that dev_node is always set up the way we expect

The calling code only calls tun_afunix_exec_child if is_tun_afunix is true,
which checks that the path is having unix: as prefix.

But since adding an ASSERT here to ensure that it is really the case
does not cost us anything, just add the ASSERT.

Reported-By: Joshua Rogers <co...@jo...>
Found-By: Zeropath

Change-Id: Idbb7bf279eb467fc1d56ab75a50b5eb2c8d0a57e
---
M src/openvpn/tun_afunix.c
1 file changed, 2 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/20/1320/2

diff --git a/src/openvpn/tun_afunix.c b/src/openvpn/tun_afunix.c
index 4d48a31..e6f2be1 100644
--- a/src/openvpn/tun_afunix.c
+++ b/src/openvpn/tun_afunix.c
@@ -53,6 +53,8 @@
     const char *msgprefix = "ERROR: failure executing process for tun:";
     struct argv argv = argv_new();
 
+    /* we should always called with a proper unix: dev node string */
+    ASSERT(dev_node && strncmp(dev_node, "unix:", strlen("unix:")) == 0);
     /* since we know that dev-node starts with unix: we can just skip that
      * to get the program name */
     const char *program = dev_node + strlen("unix:");

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

Gerrit-MessageType: newpatchset
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Idbb7bf279eb467fc1d56ab75a50b5eb2c8d0a57e
Gerrit-Change-Number: 1320
Gerrit-PatchSet: 2
Gerrit-Owner: plaisthos <arn...@rf...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: flichtenheld <fr...@li...>

[Openvpn-devel] [S] Change in openvpn[master]: Zeroize tls-crypt-v2 client keys

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

Attention is currently required from: flichtenheld, plaisthos.

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


Change subject: Zeroize tls-crypt-v2 client keys
......................................................................

Zeroize tls-crypt-v2 client keys

Joshua Rogers sent in a bug report generated with ZeroPath that the
tls-crypt-v2 client key is loaded before running the verify script. If
the verify script fails, the key is not zeroized.

While investigating this report, I found that free_tls_pre_decrypt_state
never zeroizes tls_wrap_tmp.original_wrap_keydata. So also when the
check is successful, key data will remain in memory when it is no longer
needed.

This commit moves the tls-crypt-v2-verify check before loading the key.
If it fails, original_wrap_keydata is zeroized. Also, in
free_tls_pre_decrypt_state, if a key has been loaded,
original_wrap_keydata is zeroized.

Reported-By: Joshua Rogers <co...@jo...>
Found-By: Zeropath

Change-Id: Icfcbf8ee20c1c0016eb98b570f24b9325b157c5c
Signed-off-by: Max Fillinger <ma...@ma...>
---
M src/openvpn/ssl_pkt.c
M src/openvpn/tls_crypt.c
2 files changed, 7 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/15/1315/2

diff --git a/src/openvpn/ssl_pkt.c b/src/openvpn/ssl_pkt.c
index 825719c..d7f7ac3 100644
--- a/src/openvpn/ssl_pkt.c
+++ b/src/openvpn/ssl_pkt.c
@@ -280,6 +280,7 @@
     if (state->tls_wrap_tmp.cleanup_key_ctx)
     {
         free_key_ctx_bi(&state->tls_wrap_tmp.opt.key_ctx_bi);
+        secure_memzero(&state->tls_wrap_tmp.original_wrap_keydata, sizeof(state->tls_wrap_tmp.original_wrap_keydata));
     }
 }
 
diff --git a/src/openvpn/tls_crypt.c b/src/openvpn/tls_crypt.c
index 51b4eb3..a808de3 100644
--- a/src/openvpn/tls_crypt.c
+++ b/src/openvpn/tls_crypt.c
@@ -642,6 +642,12 @@
         return false;
     }
 
+    if (opt && opt->tls_crypt_v2_verify_script && !tls_crypt_v2_verify_metadata(ctx, opt))
+    {
+        secure_memzero(&ctx->original_wrap_keydata, sizeof(ctx->original_wrap_keydata));
+        return false;
+    }
+
     /* Load the decrypted key */
     ctx->mode = TLS_WRAP_CRYPT;
     ctx->cleanup_key_ctx = true;
@@ -652,11 +658,6 @@
     /* Remove client key from buffer so tls-crypt code can unwrap message */
     ASSERT(buf_inc_len(buf, -(BLEN(&wrapped_client_key))));
 
-    if (opt && opt->tls_crypt_v2_verify_script)
-    {
-        return tls_crypt_v2_verify_metadata(ctx, opt);
-    }
-
     return true;
 }
 

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

Gerrit-MessageType: newpatchset
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Icfcbf8ee20c1c0016eb98b570f24b9325b157c5c
Gerrit-Change-Number: 1315
Gerrit-PatchSet: 2
Gerrit-Owner: MaxF <ma...@ma...>
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...>

[Openvpn-devel] [S] Change in openvpn[master]: dco-freebsd: fix peer stats storage on client instances

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

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

Change subject: dco-freebsd: fix peer stats storage on client instances
......................................................................

dco-freebsd: fix peer stats storage on client instances

Commit bf01a96 introduced a bug in the dco-freebsd path by attempting to
store peer statistics in a structure that only exists on server
instances. This leads to a SIGSEGV on non-server instances due to a NULL
multi_context pointer.

Resolve this by checking what mode the current instance is running in
and storing peer stats accordingly.

Fixes: https://github.com/OpenVPN/openvpn/issues/875
Change-Id: I92b5f3996f2a2180fa5e94719603078c1fc2f7f6
Signed-off-by: Ralf Lici <ra...@ma...>
Acked-by: Gert Doering <ge...@gr...>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1290
Message-Id: <202...@gr...>
Signed-off-by: Gert Doering <ge...@gr...>
---
M src/openvpn/dco_freebsd.c
1 file changed, 11 insertions(+), 1 deletion(-)




diff --git a/src/openvpn/dco_freebsd.c b/src/openvpn/dco_freebsd.c
index e51f8dd..3521fca 100644
--- a/src/openvpn/dco_freebsd.c
+++ b/src/openvpn/dco_freebsd.c
@@ -634,7 +634,17 @@
 
             if (nvlist_exists_nvlist(nvl, "bytes"))
             {
-                dco_update_peer_stat(dco->c->multi, dco->dco_message_peer_id, nvlist_get_nvlist(nvl, "bytes"));
+                const nvlist_t *bytes = nvlist_get_nvlist(nvl, "bytes");
+
+                if (dco->c->mode == CM_TOP)
+                {
+                    dco_update_peer_stat(dco->c->multi, dco->dco_message_peer_id, bytes);
+                }
+                else
+                {
+                    dco->c->c2.dco_read_bytes = nvlist_get_number(bytes, "in");
+                    dco->c->c2.dco_write_bytes = nvlist_get_number(bytes, "out");
+                }
             }
 
             dco->dco_message_type = OVPN_CMD_DEL_PEER;

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

Gerrit-MessageType: merged
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I92b5f3996f2a2180fa5e94719603078c1fc2f7f6
Gerrit-Change-Number: 1290
Gerrit-PatchSet: 2
Gerrit-Owner: ralf_lici <ra...@ma...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: mrbff <ma...@ma...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>

[Openvpn-devel] [S] Change in openvpn[master]: dco-freebsd: fix peer stats storage on client instances

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

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

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


Change subject: dco-freebsd: fix peer stats storage on client instances
......................................................................

dco-freebsd: fix peer stats storage on client instances

Commit bf01a96 introduced a bug in the dco-freebsd path by attempting to
store peer statistics in a structure that only exists on server
instances. This leads to a SIGSEGV on non-server instances due to a NULL
multi_context pointer.

Resolve this by checking what mode the current instance is running in
and storing peer stats accordingly.

Fixes: https://github.com/OpenVPN/openvpn/issues/875
Change-Id: I92b5f3996f2a2180fa5e94719603078c1fc2f7f6
Signed-off-by: Ralf Lici <ra...@ma...>
Acked-by: Gert Doering <ge...@gr...>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1290
Message-Id: <202...@gr...>
Signed-off-by: Gert Doering <ge...@gr...>
---
M src/openvpn/dco_freebsd.c
1 file changed, 11 insertions(+), 1 deletion(-)


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

diff --git a/src/openvpn/dco_freebsd.c b/src/openvpn/dco_freebsd.c
index e51f8dd..3521fca 100644
--- a/src/openvpn/dco_freebsd.c
+++ b/src/openvpn/dco_freebsd.c
@@ -634,7 +634,17 @@
 
             if (nvlist_exists_nvlist(nvl, "bytes"))
             {
-                dco_update_peer_stat(dco->c->multi, dco->dco_message_peer_id, nvlist_get_nvlist(nvl, "bytes"));
+                const nvlist_t *bytes = nvlist_get_nvlist(nvl, "bytes");
+
+                if (dco->c->mode == CM_TOP)
+                {
+                    dco_update_peer_stat(dco->c->multi, dco->dco_message_peer_id, bytes);
+                }
+                else
+                {
+                    dco->c->c2.dco_read_bytes = nvlist_get_number(bytes, "in");
+                    dco->c->c2.dco_write_bytes = nvlist_get_number(bytes, "out");
+                }
             }
 
             dco->dco_message_type = OVPN_CMD_DEL_PEER;

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

Gerrit-MessageType: newpatchset
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I92b5f3996f2a2180fa5e94719603078c1fc2f7f6
Gerrit-Change-Number: 1290
Gerrit-PatchSet: 2
Gerrit-Owner: ralf_lici <ra...@ma...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: mrbff <ma...@ma...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>

[Openvpn-devel] [PATCH applied] Re: dco-freebsd: fix peer stats storage on client instances

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

Marco, thanks for the review, and for finding the problem in the first
place :-)

I have tested this on FreeBSD 14 + DCO, and both the client and the server
side are now well-behaved and put the counter values in a nice place and
do not crash (I might have found a new bug - GH #881 - but that's not 
directly related to the counter stuff).

Your patch has been applied to the master branch.

commit dc6b75788c626add84384ac121e11b65f9e02a6a
Author: Ralf Lici
Date:   Tue Oct 28 12:33:05 2025 +0100

     dco-freebsd: fix peer stats storage on client instances

     Signed-off-by: Ralf Lici <ra...@ma...>
     Acked-by: Gert Doering <ge...@gr...>
     Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1290
     Message-Id: <202...@gr...>
     Signed-off-by: Gert Doering <ge...@gr...>


--
kind regards,

Gert Doering

[Openvpn-devel] [PATCH v1] dco-freebsd: fix peer stats storage on client instances

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

From: Ralf Lici <ra...@ma...>

Commit bf01a96 introduced a bug in the dco-freebsd path by attempting to
store peer statistics in a structure that only exists on server
instances. This leads to a SIGSEGV on non-server instances due to a NULL
multi_context pointer.

Resolve this by checking what mode the current instance is running in
and storing peer stats accordingly.

Fixes: https://github.com/OpenVPN/openvpn/issues/875
Change-Id: I92b5f3996f2a2180fa5e94719603078c1fc2f7f6
Signed-off-by: Ralf Lici <ra...@ma...>
Acked-by: Gert Doering <ge...@gr...>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1290
---

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

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

        
diff --git a/src/openvpn/dco_freebsd.c b/src/openvpn/dco_freebsd.c
index e51f8dd..3521fca 100644
--- a/src/openvpn/dco_freebsd.c
+++ b/src/openvpn/dco_freebsd.c
@@ -634,7 +634,17 @@
 
             if (nvlist_exists_nvlist(nvl, "bytes"))
             {
-                dco_update_peer_stat(dco->c->multi, dco->dco_message_peer_id, nvlist_get_nvlist(nvl, "bytes"));
+                const nvlist_t *bytes = nvlist_get_nvlist(nvl, "bytes");
+
+                if (dco->c->mode == CM_TOP)
+                {
+                    dco_update_peer_stat(dco->c->multi, dco->dco_message_peer_id, bytes);
+                }
+                else
+                {
+                    dco->c->c2.dco_read_bytes = nvlist_get_number(bytes, "in");
+                    dco->c->c2.dco_write_bytes = nvlist_get_number(bytes, "out");
+                }
             }
 
             dco->dco_message_type = OVPN_CMD_DEL_PEER;

[Openvpn-devel] [S] Change in openvpn[master]: dco-freebsd: fix peer stats storage on client instances

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

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

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

Change subject: dco-freebsd: fix peer stats storage on client instances
......................................................................


Patch Set 1: Code-Review+2


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

Gerrit-MessageType: comment
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I92b5f3996f2a2180fa5e94719603078c1fc2f7f6
Gerrit-Change-Number: 1290
Gerrit-PatchSet: 1
Gerrit-Owner: ralf_lici <ra...@ma...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: mrbff <ma...@ma...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Attention: ralf_lici <ra...@ma...>
Gerrit-Comment-Date: Tue, 28 Oct 2025 11:32:55 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes

[Openvpn-devel] [M] Change in openvpn[master]: PUSH_UPDATE server: added new unit tests and improved documentation

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

Attention is currently required from: mrbff, plaisthos.

flichtenheld has posted comments on this change by mrbff. ( http://gerrit.openvpn.net/c/openvpn/+/1316?usp=email )

Change subject: PUSH_UPDATE server: added new unit tests and improved documentation
......................................................................


Patch Set 1: Code-Review-2

(1 comment)

Patchset:

PS1: 
Actually on second thought this whole code should be replaced with "check_expected". This is exactly what it is intended for. See my recent test_options_parse.c for example



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

Gerrit-MessageType: comment
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Idba419681fe3ccc4e6e2f6ce7592332dcff62cd9
Gerrit-Change-Number: 1316
Gerrit-PatchSet: 1
Gerrit-Owner: mrbff <ma...@ma...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: mrbff <ma...@ma...>
Gerrit-Comment-Date: Tue, 28 Oct 2025 10:45:04 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes

[Openvpn-devel] [M] Change in openvpn[master]: PUSH_UPDATE server: added new unit tests and improved documentation

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

Attention is currently required from: mrbff, plaisthos.

flichtenheld has posted comments on this change by mrbff. ( http://gerrit.openvpn.net/c/openvpn/+/1316?usp=email )

Change subject: PUSH_UPDATE server: added new unit tests and improved documentation
......................................................................


Patch Set 1: Code-Review-1

(1 comment)

File tests/unit_tests/openvpn/test_push_update_msg.c:

http://gerrit.openvpn.net/c/openvpn/+/1316/comment/c3c8a28a_a6ce6843?usp=email :
PS1, Line 147:                 printf("\n\nexpected_size: %lu\n  actual_size: %lu", res_len, str_len);
```suggestion
                printf("\n\nexpected_size: %zu\n  actual_size: %zu", res_len, str_len);
```



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

Gerrit-MessageType: comment
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Idba419681fe3ccc4e6e2f6ce7592332dcff62cd9
Gerrit-Change-Number: 1316
Gerrit-PatchSet: 1
Gerrit-Owner: mrbff <ma...@ma...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: mrbff <ma...@ma...>
Gerrit-Comment-Date: Tue, 28 Oct 2025 10:39:06 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes

[Openvpn-devel] [PATCH v1] Canonicalize config_dir before comparing with the config file location

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

From: Selva Nair <sel...@gm...>

Found by ZeroPath

Change-Id: I8e884c00cb94f97a612056e8dca74d821a6d6386
Signed-off-by: Selva Nair <sel...@gm...>
Acked-by: Arne Schwabe <arn...@rf...>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1318
---

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

Acked-by according to Gerrit (reflected above):
Arne Schwabe <arn...@rf...>

        
diff --git a/src/openvpnserv/CMakeLists.txt b/src/openvpnserv/CMakeLists.txt
index 340b904..a92ee08 100644
--- a/src/openvpnserv/CMakeLists.txt
+++ b/src/openvpnserv/CMakeLists.txt
@@ -6,6 +6,11 @@
 
 add_executable(openvpnserv)
 
+include(CheckSymbolExists)
+
+# Some old versions of mingw does not have PATHCCH_OPTIONS enums -- add a check
+check_symbol_exists(PATHCCH_ENSURE_TRAILING_SLASH pathcch.h HAVE_PATHCCH_ENSURE_TRAILING_SLASH)
+
 set(MC_GEN_DIR ${CMAKE_CURRENT_BINARY_DIR}/mc)
 
 target_include_directories(openvpnserv PRIVATE
@@ -31,7 +36,7 @@
     )
 target_link_libraries(openvpnserv
     advapi32.lib userenv.lib iphlpapi.lib fwpuclnt.lib rpcrt4.lib
-    shlwapi.lib netapi32.lib ws2_32.lib ntdll.lib ole32.lib)
+    shlwapi.lib netapi32.lib ws2_32.lib ntdll.lib ole32.lib pathcch.lib)
 if (MINGW)
     target_compile_options(openvpnserv PRIVATE -municode)
     target_link_options(openvpnserv PRIVATE -municode)
diff --git a/src/openvpnserv/validate.c b/src/openvpnserv/validate.c
index 59d5b86..2187fb5 100644
--- a/src/openvpnserv/validate.c
+++ b/src/openvpnserv/validate.c
@@ -25,6 +25,11 @@
 #include <lmaccess.h>
 #include <shlwapi.h>
 #include <lm.h>
+#include <pathcch.h>
+
+#ifndef HAVE_PATHCCH_ENSURE_TRAILING_SLASH
+#define PATHCCH_ENSURE_TRAILING_SLASH 0x20
+#endif
 
 static const WCHAR *white_list[] = {
     L"auth-retry",
@@ -61,7 +66,7 @@
 {
     WCHAR tmp[MAX_PATH];
     const WCHAR *config_file = NULL;
-    const WCHAR *config_dir = NULL;
+    WCHAR config_dir[MAX_PATH];
 
     /* convert fname to full path */
     if (PathIsRelativeW(fname))
@@ -74,9 +79,12 @@
         config_file = fname;
     }
 
-    config_dir = s->config_dir;
+    /* canonicalize config_dir and add trailing slash before comparison */
+    HRESULT res = PathCchCanonicalizeEx(config_dir, _countof(config_dir), s->config_dir,
+                                        PATHCCH_ENSURE_TRAILING_SLASH);
 
-    if (wcsncmp(config_dir, config_file, wcslen(config_dir)) == 0
+    if (res == S_OK
+        && wcsncmp(config_dir, config_file, wcslen(config_dir)) == 0
         && wcsstr(config_file + wcslen(config_dir), L"..") == NULL)
     {
         return TRUE;

[Openvpn-devel] [XS] Change in openvpn[master]: Avoid possible race condition that kill OpenVPN itself

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

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

Change subject: Avoid possible race condition that kill OpenVPN itself
......................................................................

Avoid possible race condition that kill OpenVPN itself

If for whatever reason the child pid is zero, we would kill ourselves
since killing 0 means killing the own process group.

Reported-By: Joshua Rogers <co...@jo...>
Found-By: Zeropath
Change-Id: I7b94de92723f9528b01cb932bb079eedf0f1f272
Signed-off-by: Arne Schwabe <arn...@rf...>
Acked-by: Gert Doering <ge...@gr...>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1319
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg33910.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M src/openvpn/tun_afunix.c
1 file changed, 6 insertions(+), 1 deletion(-)




diff --git a/src/openvpn/tun_afunix.c b/src/openvpn/tun_afunix.c
index 4d48a31..124db6d 100644
--- a/src/openvpn/tun_afunix.c
+++ b/src/openvpn/tun_afunix.c
@@ -128,7 +128,12 @@
         close(tt->fd);
         tt->fd = 0;
     }
-    kill(tt->afunix.childprocess, SIGINT);
+    /* only kill the child process if the PID is not 0 to avoid killing
+     * ourselves by accident */
+    if (tt->afunix.childprocess)
+    {
+        kill(tt->afunix.childprocess, SIGINT);
+    }
 
     free(tt->actual_name);
     free(tt);

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

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

[Openvpn-devel] [XS] Change in openvpn[master]: Avoid possible race condition that kill OpenVPN itself

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

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

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


Change subject: Avoid possible race condition that kill OpenVPN itself
......................................................................

Avoid possible race condition that kill OpenVPN itself

If for whatever reason the child pid is zero, we would kill ourselves
since killing 0 means killing the own process group.

Reported-By: Joshua Rogers <co...@jo...>
Found-By: Zeropath
Change-Id: I7b94de92723f9528b01cb932bb079eedf0f1f272
Signed-off-by: Arne Schwabe <arn...@rf...>
Acked-by: Gert Doering <ge...@gr...>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1319
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg33910.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M src/openvpn/tun_afunix.c
1 file changed, 6 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/19/1319/2

diff --git a/src/openvpn/tun_afunix.c b/src/openvpn/tun_afunix.c
index 4d48a31..124db6d 100644
--- a/src/openvpn/tun_afunix.c
+++ b/src/openvpn/tun_afunix.c
@@ -128,7 +128,12 @@
         close(tt->fd);
         tt->fd = 0;
     }
-    kill(tt->afunix.childprocess, SIGINT);
+    /* only kill the child process if the PID is not 0 to avoid killing
+     * ourselves by accident */
+    if (tt->afunix.childprocess)
+    {
+        kill(tt->afunix.childprocess, SIGINT);
+    }
 
     free(tt->actual_name);
     free(tt);

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

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

[Openvpn-devel] [PATCH applied] Re: Avoid possible race condition that kill OpenVPN itself

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

Amazing find by ZeroPath, and trivially correct fix.

I have completed Joshua's "reported-by:" line (full name) in the
commit message.

Your patch has been applied to the master branch.

commit 18309ff64833523c1ad19e7d56d6f756b53966af
Author: Arne Schwabe
Date:   Mon Oct 27 22:33:02 2025 +0100

     Avoid possible race condition that kill OpenVPN itself

     Signed-off-by: Arne Schwabe <arn...@rf...>
     Acked-by: Gert Doering <ge...@gr...>
     Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1319
     Message-Id: <202...@gr...>
     URL: https://www.mail-archive.com/ope...@li.../msg33910.html
     Signed-off-by: Gert Doering <ge...@gr...>


--
kind regards,

Gert Doering

[Openvpn-devel] [XS] Change in openvpn[master]: pkcs11_management_id_get: Free certificate object after use

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

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

Change subject: pkcs11_management_id_get: Free certificate object after use
......................................................................

pkcs11_management_id_get: Free certificate object after use

Found by ZeroPath

Change-Id: I85320b8f1cfc02dfd561916e5637d9481edac59e
Signed-off-by: Selva Nair <sel...@gm...>
Acked-by: Arne Schwabe <arn...@rf...>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1317
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg33908.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M src/openvpn/pkcs11.c
1 file changed, 3 insertions(+), 3 deletions(-)




diff --git a/src/openvpn/pkcs11.c b/src/openvpn/pkcs11.c
index ce64135..9afb181 100644
--- a/src/openvpn/pkcs11.c
+++ b/src/openvpn/pkcs11.c
@@ -436,9 +436,6 @@
 {
     pkcs11h_certificate_id_list_t id_list = NULL;
     pkcs11h_certificate_id_list_t entry = NULL;
-#if 0 /* certificate_id seems to be unused -- JY */
-    pkcs11h_certificate_id_t certificate_id = NULL;
-#endif
     pkcs11h_certificate_t certificate = NULL;
     CK_RV rv = CKR_OK;
     unsigned char *certificate_blob = NULL;
@@ -548,6 +545,9 @@
     pkcs11h_certificate_freeCertificateIdList(id_list);
     id_list = NULL;
 
+    pkcs11h_certificate_freeCertificate(certificate);
+    certificate = NULL;
+
     free(internal_id);
     internal_id = NULL;
 

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

Gerrit-MessageType: merged
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I85320b8f1cfc02dfd561916e5637d9481edac59e
Gerrit-Change-Number: 1317
Gerrit-PatchSet: 2
Gerrit-Owner: selvanair <sel...@gm...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>

[Openvpn-devel] [XS] Change in openvpn[master]: pkcs11_management_id_get: Free certificate object after use

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

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

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


Change subject: pkcs11_management_id_get: Free certificate object after use
......................................................................

pkcs11_management_id_get: Free certificate object after use

Found by ZeroPath

Change-Id: I85320b8f1cfc02dfd561916e5637d9481edac59e
Signed-off-by: Selva Nair <sel...@gm...>
Acked-by: Arne Schwabe <arn...@rf...>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1317
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg33908.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M src/openvpn/pkcs11.c
1 file changed, 3 insertions(+), 3 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/17/1317/2

diff --git a/src/openvpn/pkcs11.c b/src/openvpn/pkcs11.c
index ce64135..9afb181 100644
--- a/src/openvpn/pkcs11.c
+++ b/src/openvpn/pkcs11.c
@@ -436,9 +436,6 @@
 {
     pkcs11h_certificate_id_list_t id_list = NULL;
     pkcs11h_certificate_id_list_t entry = NULL;
-#if 0 /* certificate_id seems to be unused -- JY */
-    pkcs11h_certificate_id_t certificate_id = NULL;
-#endif
     pkcs11h_certificate_t certificate = NULL;
     CK_RV rv = CKR_OK;
     unsigned char *certificate_blob = NULL;
@@ -548,6 +545,9 @@
     pkcs11h_certificate_freeCertificateIdList(id_list);
     id_list = NULL;
 
+    pkcs11h_certificate_freeCertificate(certificate);
+    certificate = NULL;
+
     free(internal_id);
     internal_id = NULL;
 

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

Gerrit-MessageType: newpatchset
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I85320b8f1cfc02dfd561916e5637d9481edac59e
Gerrit-Change-Number: 1317
Gerrit-PatchSet: 2
Gerrit-Owner: selvanair <sel...@gm...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>

[Openvpn-devel] [PATCH applied] Re: pkcs11_management_id_get: Free certificate object after use

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

I have not reviewed/tested this beyond "BB compiles things, unit tests
(including pkcs11 on some of the instances) work".  Stare-at-code looks
very trivially correct.

Your patch has been applied to the master branch.

commit 87f639c820b8365585fc825ee7ea9cdd86cf88c5
Author: Selva Nair
Date:   Mon Oct 27 22:27:41 2025 +0100

     pkcs11_management_id_get: Free certificate object after use

     Signed-off-by: Selva Nair <sel...@gm...>
     Acked-by: Arne Schwabe <arn...@rf...>
     Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1317
     Message-Id: <202...@gr...>
     URL: https://www.mail-archive.com/ope...@li.../msg33908.html
     Signed-off-by: Gert Doering <ge...@gr...>


--
kind regards,

Gert Doering

[Openvpn-devel] [XS] Change in openvpn[master]: sitnl: set FD_CLOEXEC on socket to prevent abuse

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

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

Hello flichtenheld, plaisthos, 

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

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

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

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


Change subject: sitnl: set FD_CLOEXEC on socket to prevent abuse
......................................................................

sitnl: set FD_CLOEXEC on socket to prevent abuse

Since OpenVPN spawns various child processes, it is important
that sockets are closed after calling exec.

The sitnl socket didn't have the right flag set, resulting
in it surviving in, for example, connect/disconnect scripts
and giving the latter a chance to abuse the socket.

Ensure this doesn't happen by setting FD_CLOEXEC on
this socket right after creation.

Reported-by: ZeroPath (https://zeropath.com/)
Change-Id: I54845bf4dd17d06cfc3b402f188795f74f4b1d3e
Signed-off-by: Antonio Quartulli <an...@ma...>
---
M src/openvpn/networking_sitnl.c
M tests/unit_tests/openvpn/Makefile.am
2 files changed, 5 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/14/1314/3

diff --git a/src/openvpn/networking_sitnl.c b/src/openvpn/networking_sitnl.c
index b3adb16..3e20b70 100644
--- a/src/openvpn/networking_sitnl.c
+++ b/src/openvpn/networking_sitnl.c
@@ -27,6 +27,7 @@
 
 #include "dco.h"
 #include "errlevel.h"
+#include "fdmisc.h"
 #include "buffer.h"
 #include "misc.h"
 #include "networking.h"
@@ -181,6 +182,9 @@
         return fd;
     }
 
+    /* set close on exec to avoid child processes access the socket */
+    set_cloexec(fd);
+
     if (setsockopt(fd, SOL_SOCKET, SO_SNDBUF, &sndbuf, sizeof(sndbuf)) < 0)
     {
         msg(M_WARN | M_ERRNO, "%s: SO_SNDBUF", __func__);
diff --git a/tests/unit_tests/openvpn/Makefile.am b/tests/unit_tests/openvpn/Makefile.am
index 997703a..0f13172 100644
--- a/tests/unit_tests/openvpn/Makefile.am
+++ b/tests/unit_tests/openvpn/Makefile.am
@@ -196,6 +196,7 @@
 	$(top_srcdir)/src/openvpn/crypto_epoch.c \
 	$(top_srcdir)/src/openvpn/crypto_mbedtls.c \
 	$(top_srcdir)/src/openvpn/crypto_openssl.c \
+	$(top_srcdir)/src/openvpn/fdmisc.c \
 	$(top_srcdir)/src/openvpn/otime.c \
 	$(top_srcdir)/src/openvpn/packet_id.c \
 	$(top_srcdir)/src/openvpn/platform.c

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

Gerrit-MessageType: newpatchset
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I54845bf4dd17d06cfc3b402f188795f74f4b1d3e
Gerrit-Change-Number: 1314
Gerrit-PatchSet: 3
Gerrit-Owner: ordex <an...@ma...>
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: ordex <an...@ma...>

[Openvpn-devel] [XS] Change in openvpn[master]: sitnl: set FD_CLOEXEC on socket to prevent abuse

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

Attention is currently required from: flichtenheld, ordex.

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

Change subject: sitnl: set FD_CLOEXEC on socket to prevent abuse
......................................................................


Patch Set 2: Code-Review+2


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

Gerrit-MessageType: comment
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I54845bf4dd17d06cfc3b402f188795f74f4b1d3e
Gerrit-Change-Number: 1314
Gerrit-PatchSet: 2
Gerrit-Owner: ordex <an...@ma...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Attention: ordex <an...@ma...>
Gerrit-Comment-Date: Mon, 27 Oct 2025 22:53:30 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes

[Openvpn-devel] [S] Change in openvpn[master]: Fix logic when pushed cipher triggers tun reopen and ignore more options

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

Attention is currently required from: flichtenheld.

Hello flichtenheld,

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

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

to review the following change.


Change subject: Fix logic when pushed cipher triggers tun reopen and ignore more options
......................................................................

Fix logic when pushed cipher triggers tun reopen and ignore more options

The logic was inverted. Only when link-mtu is used, pushing a cipher can
change the MTU and not the other way round. (found by zeropath)

Also ignore a few more options that should not trigger a reopen of tun
in push message.

Reported-By: co...@jo...
Found-By: Zeropath
Change-Id: I76eb584024610a6054a069340adbac988abf686c
---
M src/openvpn/push.c
1 file changed, 14 insertions(+), 4 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/21/1321/1

diff --git a/src/openvpn/push.c b/src/openvpn/push.c
index 2c717c7..d7063e6 100644
--- a/src/openvpn/push.c
+++ b/src/openvpn/push.c
@@ -1025,15 +1025,25 @@
     char line[OPTION_PARM_SIZE];
     while (buf_parse(buf, ',', line, sizeof(line)))
     {
-        /* peer-id and auth-token might change on restart and this should not trigger reopening tun
+        /* peer-id and auth-token might change on restart and this should not
+         * trigger reopening tun
+         * Also other options that only affect the control channel should
+         * not trigger a reopen of the tun device
          */
-        if (strprefix(line, "peer-id ") || strprefix(line, "auth-token ")
-            || strprefix(line, "auth-token-user "))
+        if (strprefix(line, "peer-id ")
+            || strprefix(line, "auth-token ")
+            || strprefix(line, "auth-token-user")
+            || strprefix(line, "protocol-flags ")
+            || strprefix(line, "key-derivation ")
+            || strprefix(line, "explicit-exit-notify ")
+            || strprefix(line, "ping ")
+            || strprefix(line, "ping-restart ")
+            || strprefix(line, "ping-timer "))
         {
             continue;
         }
         /* tun reopen only needed if cipher change can change tun MTU */
-        if (strprefix(line, "cipher ") && !opt->ce.tun_mtu_defined)
+        if (strprefix(line, "cipher ") && opt->ce.tun_mtu_defined)
         {
             continue;
         }

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

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

[Openvpn-devel] [S] Change in openvpn[master]: Canonicalize config_dir before comparing with the config file location

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

Attention is currently required from: flichtenheld, selvanair.

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

Change subject: Canonicalize config_dir before comparing with the config file location
......................................................................


Patch Set 1: Code-Review+2

(1 comment)

File src/openvpnserv/validate.c:

http://gerrit.openvpn.net/c/openvpn/+/1318/comment/a259ecd3_08086e9f?usp=email :
PS1, Line 32: #endif
> When its defined, its an enum in pathcch.h, not a define. […]
Acknowledged



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

Gerrit-MessageType: comment
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I8e884c00cb94f97a612056e8dca74d821a6d6386
Gerrit-Change-Number: 1318
Gerrit-PatchSet: 1
Gerrit-Owner: selvanair <sel...@gm...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Attention: selvanair <sel...@gm...>
Gerrit-Comment-Date: Mon, 27 Oct 2025 22:11:14 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: plaisthos <arn...@rf...>
Comment-In-Reply-To: selvanair <sel...@gm...>