Menu
▾
▴
openvpn-devel
|
From: ordex (C. Review) <ge...@op...> - 2025-10-27 13:55:33
|
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/+/1314?usp=email
to review the following change.
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
1 file changed, 3 insertions(+), 0 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/14/1314/1
diff --git a/src/openvpn/networking_sitnl.c b/src/openvpn/networking_sitnl.c
index b3adb16..a959fa8 100644
--- a/src/openvpn/networking_sitnl.c
+++ b/src/openvpn/networking_sitnl.c
@@ -181,6 +181,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__);
--
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: newchange
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I54845bf4dd17d06cfc3b402f188795f74f4b1d3e
Gerrit-Change-Number: 1314
Gerrit-PatchSet: 1
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...>
|
|
From: ordex (C. Review) <ge...@op...> - 2025-10-27 15:19:09
|
Attention is currently required from: flichtenheld, 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 (#2).
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
1 file changed, 4 insertions(+), 0 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/14/1314/2
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__);
--
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: 2
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...>
|
|
From: plaisthos (C. Review) <ge...@op...> - 2025-10-27 22:53:46
|
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 |
|
From: ordex (C. Review) <ge...@op...> - 2025-10-28 08:46:55
|
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...>
|
|
From: ordex (C. Review) <ge...@op...> - 2025-10-28 14:42:38
|
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 (#4).
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 CMakeLists.txt
M src/openvpn/networking_sitnl.c
M tests/unit_tests/openvpn/Makefile.am
3 files changed, 6 insertions(+), 0 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/14/1314/4
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 5954a6e..bf754f3 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -923,6 +923,7 @@
src/openvpn/crypto_openssl.c
src/openvpn/crypto.c
src/openvpn/crypto_epoch.c
+ src/openvpn/fdmisc.c
src/openvpn/otime.c
src/openvpn/packet_id.c
)
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: 4
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...>
|
|
From: cron2 (C. Review) <ge...@op...> - 2025-10-28 16:27:52
|
Attention is currently required from: flichtenheld, ordex, plaisthos. cron2 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 4: 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: 4 Gerrit-Owner: ordex <an...@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: flichtenheld <fr...@li...> Gerrit-Attention: ordex <an...@ma...> Gerrit-Comment-Date: Tue, 28 Oct 2025 16:27:42 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes |
|
From: Gert D. <ge...@gr...> - 2025-10-28 16:28:57
|
From: Antonio Quartulli <an...@ma...> 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: Joshua Rogers <co...@jo...> Found-by: ZeroPath (https://zeropath.com/) Change-Id: I54845bf4dd17d06cfc3b402f188795f74f4b1d3e Signed-off-by: Antonio Quartulli <an...@ma...> Acked-by: Gert Doering <ge...@gr...> Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1314 --- 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/+/1314 This mail reflects revision 4 of this Change. Acked-by according to Gerrit (reflected above): Gert Doering <ge...@gr...> diff --git a/CMakeLists.txt b/CMakeLists.txt index 5954a6e..bf754f3 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -923,6 +923,7 @@ src/openvpn/crypto_openssl.c src/openvpn/crypto.c src/openvpn/crypto_epoch.c + src/openvpn/fdmisc.c src/openvpn/otime.c src/openvpn/packet_id.c ) 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 |
|
From: Gert D. <ge...@gr...> - 2025-10-28 16:35:17
|
Straightforward enough :-)
Your patch has been applied to the master and release/2.6 branch (bugfix).
commit b9b5470521294209146c7253a97012d399978d72 (master)
commit 12a2e88b9e9fc7b0e6f8fce0125b5272980ec51b (release/2.6)
Author: Antonio Quartulli
Date: Tue Oct 28 17:28:38 2025 +0100
sitnl: set FD_CLOEXEC on socket to prevent abuse
Signed-off-by: Antonio Quartulli <an...@ma...>
Acked-by: Gert Doering <ge...@gr...>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1314
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg33952.html
Signed-off-by: Gert Doering <ge...@gr...>
--
kind regards,
Gert Doering
|
|
From: cron2 (C. Review) <ge...@op...> - 2025-10-28 16:35:54
|
cron2 has uploaded a new patch set (#5) to the change originally created by ordex. ( http://gerrit.openvpn.net/c/openvpn/+/1314?usp=email ) The following approvals got outdated and were removed: Code-Review+2 by cron2 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: Joshua Rogers <co...@jo...> Found-by: ZeroPath (https://zeropath.com/) Change-Id: I54845bf4dd17d06cfc3b402f188795f74f4b1d3e Signed-off-by: Antonio Quartulli <an...@ma...> Acked-by: Gert Doering <ge...@gr...> Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1314 Message-Id: <202...@gr...> URL: https://www.mail-archive.com/ope...@li.../msg33952.html Signed-off-by: Gert Doering <ge...@gr...> --- M CMakeLists.txt M src/openvpn/networking_sitnl.c M tests/unit_tests/openvpn/Makefile.am 3 files changed, 6 insertions(+), 0 deletions(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/14/1314/5 diff --git a/CMakeLists.txt b/CMakeLists.txt index 5954a6e..bf754f3 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -923,6 +923,7 @@ src/openvpn/crypto_openssl.c src/openvpn/crypto.c src/openvpn/crypto_epoch.c + src/openvpn/fdmisc.c src/openvpn/otime.c src/openvpn/packet_id.c ) 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: 5 Gerrit-Owner: ordex <an...@ma...> Gerrit-Reviewer: cron2 <ge...@gr...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> |
|
From: cron2 (C. Review) <ge...@op...> - 2025-10-28 16:35:57
|
cron2 has submitted this change. ( http://gerrit.openvpn.net/c/openvpn/+/1314?usp=email ) 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: Joshua Rogers <co...@jo...> Found-by: ZeroPath (https://zeropath.com/) Change-Id: I54845bf4dd17d06cfc3b402f188795f74f4b1d3e Signed-off-by: Antonio Quartulli <an...@ma...> Acked-by: Gert Doering <ge...@gr...> Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1314 Message-Id: <202...@gr...> URL: https://www.mail-archive.com/ope...@li.../msg33952.html Signed-off-by: Gert Doering <ge...@gr...> --- M CMakeLists.txt M src/openvpn/networking_sitnl.c M tests/unit_tests/openvpn/Makefile.am 3 files changed, 6 insertions(+), 0 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 5954a6e..bf754f3 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -923,6 +923,7 @@ src/openvpn/crypto_openssl.c src/openvpn/crypto.c src/openvpn/crypto_epoch.c + src/openvpn/fdmisc.c src/openvpn/otime.c src/openvpn/packet_id.c ) 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: merged Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: I54845bf4dd17d06cfc3b402f188795f74f4b1d3e Gerrit-Change-Number: 1314 Gerrit-PatchSet: 5 Gerrit-Owner: ordex <an...@ma...> Gerrit-Reviewer: cron2 <ge...@gr...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> |
×
Want the latest updates on software, tech news, and AI?
Get latest updates about software, tech news, and AI from SourceForge directly in your inbox once a month.
Thanks for helping keep SourceForge clean.
X