diff mbox series

[Openvpn-devel,v2] port-share: do not write journal file through symlinks or hardlinks

Message ID 20261102093834.29593-1-gert@greenie.muc.de
State New
Headers
Series [Openvpn-devel,v2] port-share: do not write journal file through symlinks or hardlinks |

Commit Message

Gert Doering Nov. 2, 2026, 9:38 a.m. UTC 
  From: Naveed Khan <dxbnaveed.k@gmail.com>

journal_add() creates <journal dir>/<local ip:port> with
O_CREAT | O_TRUNC and writes the remote client's address into it.
The name is predictable (the proxy connection's ephemeral local port)
and the write is triggered by any TCP client sending non-OpenVPN data
to the shared port. Anyone who can create entries in the journal
directory can therefore plant symlinks or hardlinks and have the
port-share process, which runs as root unless --user is used, truncate
and overwrite an arbitrary file with the client address.

Create the journal file with O_EXCL | O_NOFOLLOW instead. Entries are
removed as soon as the proxied connection is closed, so an existing
entry is either a leftover from a proxy process that exited without
cleaning up, or something planted. Unlink it and retry the exclusive
create once, which keeps the old behavior of replacing stale entries.

Also document that the journal directory must not be writable by other
users.

Github: closes OpenVPN/openvpn#1124
Change-Id: I70ebdadd660e41b40ed53f4238be6885f0a7bb8b
Signed-off-by: Naveed Khan <dxbnaveed.k@gmail.com>
Acked-by: Razvan Cojocaru <razvanc@mailbox.org>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/2074
---

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

Acked-by according to Gerrit (reflected above):
Razvan Cojocaru <razvanc@mailbox.org>
diff mbox series

Patch

diff --git a/doc/man-sections/server-options.rst b/doc/man-sections/server-options.rst
index 9dfa6bd..7a09809 100644
--- a/doc/man-sections/server-options.rst
+++ b/doc/man-sections/server-options.rst
@@ -489,6 +489,10 @@ 
   the origin of the connection. Each generated file will be automatically
   deleted when the proxied connection is torn down.
 
+  The files in ``dir`` are created and removed with the privileges of the
+  OpenVPN process, so ``dir`` must not be writable by any user other than
+  the one OpenVPN runs as.
+
   Not implemented on Windows.
 
 --push option
diff --git a/src/openvpn/ps.c b/src/openvpn/ps.c
index 03c6f48..f8cfe1c5 100644
--- a/src/openvpn/ps.c
+++ b/src/openvpn/ps.c
@@ -341,7 +341,15 @@ 
         check_malloc_return(jfn);
         snprintf(jfn, fnlen, "%s/%s", journal_dir, t);
         dmsg(D_PS_PROXY_DEBUG, "PORT SHARE PROXY: client origin %s -> %s", jfn, f);
-        int fd = platform_open(jfn, O_CREAT | O_TRUNC | O_WRONLY, S_IRUSR | S_IWUSR | S_IRGRP);
+        /* never write through a pre-existing symlink or hardlink */
+        const int flags = O_CREAT | O_EXCL | O_WRONLY | O_NOFOLLOW;
+        int fd = platform_open(jfn, flags, S_IRUSR | S_IWUSR | S_IRGRP);
+        if (fd == -1 && errno == EEXIST)
+        {
+            /* stale entry, e.g. left behind by a killed proxy process */
+            unlink(jfn);
+            fd = platform_open(jfn, flags, S_IRUSR | S_IWUSR | S_IRGRP);
+        }
         if (fd != -1)
         {
             ssize_t write_len = strlen(f);