do not race on RuntimeDirectory
PATCH 1/1 do not race on RuntimeDirectory
From: Christian Hesse <[email protected]...>
Different unit instances create and destroy the same RuntimeDirectory. This leads to running instances where the status file (and possibly more runtime data) is no longer accessible.
So create a RuntimeDirectory per instance.
--- distro/systemd/[email protected]... | 2 +- distro/systemd/[email protected]... | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)
distro/systemd/[email protected]... +1 -1
@@ -9,7 +9,7 @@ Documentation=https://community.openvpn.net/openvpn/wiki/HOWTO [Service] Type=notify PrivateTmp=true -RuntimeDirectory=openvpn-client +RuntimeDirectory=openvpn-client@%i RuntimeDirectoryMode=0710 WorkingDirectory=/etc/openvpn/client ExecStart=/usr/sbin/openvpn --suppress-timestamps --nobind --config %i.conf
distro/systemd/[email protected]... +2 -2
@@ -9,10 +9,10 @@ Documentation=https://community.openvpn.net/openvpn/wiki/HOWTO [Service] Type=notify PrivateTmp=true -RuntimeDirectory=openvpn-server +RuntimeDirectory=openvpn-server@%i RuntimeDirectoryMode=0710 WorkingDirectory=/etc/openvpn/server -ExecStart=/usr/sbin/openvpn --status %t/openvpn-server/status-%i.log --status-version 2 --suppress-timestamps --config %i.conf +ExecStart=/usr/sbin/openvpn --status %t/openvpn-server@%i/status.log --status-version 2 --suppress-timestamps --config %i.conf CapabilityBoundingSet=CAP_IPC_LOCK CAP_NET_ADMIN CAP_NET_BIND_SERVICE CAP_NET_RAW CAP_SETGID CAP_SETUID CAP_SYS_CHROOT CAP_DAC_OVERRIDE LimitNPROC=10 DeviceAllow=/dev/null rw
Signature · 1 line
2.11.0
Quoted 19 lines · Christian Hesse
From: Christian Hesse <[email protected]...>
Different unit instances create and destroy the same RuntimeDirectory. This leads to running instances where the status file (and possibly more runtime data) is no longer accessible.
So create a RuntimeDirectory per instance.
Signed-off-by Christian Hesse <[email protected]...>--- distro/systemd/[email protected]... | 2 +- distro/systemd/[email protected]... | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)distro/systemd/[email protected]... +0
-0
distro/systemd/[email protected]... Documentation=https://community.openvpn.net/openvpn/wiki/HOWTO · line 12
Quoted 5 lines
distro/systemd/[email protected]... +1
-1@@ -9,7 +9,7 @@ Documentation=https://community.openvpn.net/openvpn/wiki/HOWTO [Service] Type=notify PrivateTmp=true -RuntimeDirectory=openvpn-client +RuntimeDirectory=openvpn-client@%i
I am quite sure we tested this, without any luck. We ended up with a directory named openvpn-client@%i ... well, we used openvpn-client-%i, but the principle should be the same.
This *might* have been fixed in a newer systemd releases, but as we have RHEL7 as the oldest release we need to ensure this works properly there too.
For the server side, we used status-%i.log to avoid clashing on the status log.
debbie10t did plenty of testing with various configurations, so he can probably fill out more details in the various test cases he used. We should probably document those tests in our wiki though.
Signature · 4 lines
kind regards, David Sommerseth OpenVPN Technologies, Inc
David Sommerseth <[email protected]...> on Fri, 2016/12/16 19:14:
Quoted 33 lines · Christian Hesse
Quoted 22 lines
From: Christian Hesse <[email protected]...>
Different unit instances create and destroy the same RuntimeDirectory. This leads to running instances where the status file (and possibly more runtime data) is no longer accessible.
So create a RuntimeDirectory per instance.
Signed-off-by Christian Hesse <[email protected]...>--- distro/systemd/[email protected]... | 2 +- distro/systemd/[email protected]... | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)(quoted diff) +0
-0b/distro/systemd/[email protected]... index 5618af3..fcb5302 100644
distro/systemd/[email protected]... +0
-0@@ -9,7 +9,7 @@Documentation=https://community.openvpn.net/openvpn/wiki/HOWTO [Service]
Type=notify
PrivateTmp=truedistro/systemd/[email protected]... Documentation=https://community.openvpn.net/openvpn/wiki/HOWTO · line 12
Quoted 2 lines
distro/systemd/[email protected]... +1
-1-RuntimeDirectory=openvpn-client +RuntimeDirectory=openvpn-client@%iI am quite sure we tested this, without any luck. We ended up with a directory named openvpn-client@%i ... well, we used openvpn-client-%i, but the principle should be the same.
This *might* have been fixed in a newer systemd releases, but as we have RHEL7 as the oldest release we need to ensure this works properly there too.
Quoted 6 lines
For the server side, we used status-%i.log to avoid clashing on the status log.
debbie10t did plenty of testing with various configurations, so he can probably fill out more details in the various test cases he used. We should probably document those tests in our wiki though.
Running systemd 232 here and everything works as expected. This is with unit files from current master:
# systemctl start [email protected]... # systemctl start [email protected]... # systemctl stop [email protected]... # ls -l /run/openvpn-server/ ls: cannot access '/run/openvpn-server/': No such file or directory
And the same with my patch applied:
# ls -l /run/[email protected].../status.log -rw------- 1 root root 8419 Dec 16 20:03 /run/[email protected].../status.log
Quoted 55 lines · Christian Hesse
David Sommerseth <[email protected]...> on Fri, 2016/12/16 19:14:
Quoted 33 lines · Christian Hesse
Quoted 22 lines
From: Christian Hesse <[email protected]...>
Different unit instances create and destroy the same RuntimeDirectory. This leads to running instances where the status file (and possibly more runtime data) is no longer accessible.
So create a RuntimeDirectory per instance.
Signed-off-by Christian Hesse <[email protected]...>--- distro/systemd/[email protected]... | 2 +- distro/systemd/[email protected]... | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)(quoted diff) +0
-0b/distro/systemd/[email protected]... index 5618af3..fcb5302 100644
distro/systemd/[email protected]... +0
-0@@ -9,7 +9,7 @@Documentation=https://community.openvpn.net/openvpn/wiki/HOWTO [Service]
Type=notify
PrivateTmp=truedistro/systemd/[email protected]... Documentation=https://community.openvpn.net/openvpn/wiki/HOWTO · line 12
Quoted 2 lines
distro/systemd/[email protected]... +1
-1-RuntimeDirectory=openvpn-client +RuntimeDirectory=openvpn-client@%iI am quite sure we tested this, without any luck. We ended up with a directory named openvpn-client@%i ... well, we used openvpn-client-%i, but the principle should be the same.
This *might* have been fixed in a newer systemd releases, but as we have RHEL7 as the oldest release we need to ensure this works properly there too.
Quoted 6 lines
For the server side, we used status-%i.log to avoid clashing on the status log.
debbie10t did plenty of testing with various configurations, so he can probably fill out more details in the various test cases he used. We should probably document those tests in our wiki though.
Running systemd 232 here and everything works as expected. This is with unit files from current master:
# systemctl start [email protected]... # systemctl start [email protected]... # systemctl stop [email protected]... # ls -l /run/openvpn-server/ ls: cannot access '/run/openvpn-server/': No such file or directoryAnd the same with my patch applied:
# ls -l /run/[email protected].../status.log -rw------- 1 root root 8419 Dec 16 20:03 /run/[email protected].../status.log
Quoted 8 lines
From
systemd.exec(5)about RuntimeDirectory=:Quoted 3 lines
Takes a list of directory names. If set, one or more directories by the specified names will be created below /run (for system services) [...] when the unit is started, and removed when the unit is stopped.
So this works as long the no service is stopped or fails.
I do not have RHEL7 around... What exactly fails there?
The directory is literally named 'openvpn-server@%i'
RHEL7.3 (latest common release) ships with:
$ rpm -q systemd systemd-219-30.el7_3.6.x86_64 $ cat /etc/redhat-release Red Hat Enterprise Linux Server release 7.3 (Maipo)
We don't have any particular policy that we should support anything else than the latest major version (so 7.0 to 7.2 would not normally be supported).
Regarding access to RHEL: There is a developer subscription available for free if you sign-up with the Red Hat developer program (also no cost sign-up) ... which is valid for one physical box and unlimited virtual guests for non-production environments. It also includes access to the vast majority of RHEL add-on modules as well.
<https://developers.redhat.com/products/rhel/get-started/>
Otherwise, CentOS or Scientific Linux is very close to RHEL, just lagging some weeks/months after each minor point release (7.2 -> 7.3).
Signature · 4 lines
kind regards, David Sommerseth OpenVPN Technologies, Inc
David Sommerseth <[email protected]...> on Fri, 2016/12/16 22:15:
Quoted 64 lines · Christian Hesse
Quoted 63 lines
David Sommerseth <[email protected]...> on Fri, 2016/12/16 19:14:
Quoted 34 lines · Christian Hesse
Quoted 22 lines
From: Christian Hesse <[email protected]...>
Different unit instances create and destroy the same RuntimeDirectory. This leads to running instances where the status file (and possibly more runtime data) is no longer accessible.
So create a RuntimeDirectory per instance.
Signed-off-by Christian Hesse <[email protected]...>--- distro/systemd/[email protected]... | 2 +- distro/systemd/[email protected]... | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)(quoted diff) +0
-0b/distro/systemd/[email protected]... index 5618af3..fcb5302 100644
distro/systemd/[email protected]... +0
-0@@ -9,7 +9,7 @@Documentation=https://community.openvpn.net/openvpn/wiki/HOWTO [Service]
Type=notify
PrivateTmp=truedistro/systemd/[email protected]... Documentation=https://community.openvpn.net/openvpn/wiki/HOWTO · line 12
Quoted 2 lines
distro/systemd/[email protected]... +1
-1-RuntimeDirectory=openvpn-client +RuntimeDirectory=openvpn-client@%iI am quite sure we tested this, without any luck. We ended up with a directory named openvpn-client@%i ... well, we used openvpn-client-%i, but the principle should be the same.
This *might* have been fixed in a newer systemd releases, but as we have RHEL7 as the oldest release we need to ensure this works properly there too.
Quoted 6 lines
For the server side, we used status-%i.log to avoid clashing on the status log.
debbie10t did plenty of testing with various configurations, so he can probably fill out more details in the various test cases he used. We should probably document those tests in our wiki though.
Running systemd 232 here and everything works as expected. This is with unit files from current master:
# systemctl start [email protected]... # systemctl start [email protected]... # systemctl stop [email protected]... # ls -l /run/openvpn-server/ ls: cannot access '/run/openvpn-server/': No such file or directoryAnd the same with my patch applied:
# ls -l /run/[email protected].../status.log -rw------- 1 root root 8419 Dec 16 20:03 /run/[email protected].../status.logFrom
systemd.exec(5)about RuntimeDirectory=:Quoted 3 lines
Takes a list of directory names. If set, one or more directories by the specified names will be created below /run (for system services) [...] when the unit is started, and removed when the unit is stopped.
Quoted 5 lines
Quoted 3 lines
So this works as long the no service is stopped or fails.
I do not have RHEL7 around... What exactly fails there?
The directory is literally named 'openvpn-server@%i'
Ok, that's bad. So another idea... Let's remove RuntimeDirectory= and RuntimeDirectoryMode= from unit files. Instead we provide a file openvpn.conf to be installed to /usr/lib/tmpfiles.d/ containing:
d /run/openvpn-client 0710 root root -
d /run/openvpn-server 0710 root root -
systemd will create the directories for us and nobody will (unintentionally) delete them. As /run is volatile we do not have to care.
Signature · 3 lines
main(a){char*c=/* Schoene Gruesse */"B?IJj;MEH"
"CX:;",b;for(a/* Best regards my address: */=0;b=c[a++];)
putchar(b-1/(/* Chris cc -ox -xc - && ./x */b/42*2-3)*42);}PATCH 1/1 do not race on RuntimeDirectory
From: Christian Hesse <[email protected]...>
Different unit instances create and destroy the same RuntimeDirectory. This leads to running instances where the status file (and possibly more runtime data) is no longer accessible.
So do not handle this in unit files but provide a tmpfiles.d configuration and let systemd-tmpfiles do the work.
Nobody will (unintentionally) delete the directories and its content. As /run is volatile we do not have to care about cleanup.
--- distro/systemd/[email protected]... | 2 -- distro/systemd/[email protected]... | 2 -- distro/systemd/openvpn.conf | 2 ++ 3 files changed, 2 insertions(+), 4 deletions(-) create mode 100644 distro/systemd/openvpn.conf
distro/systemd/[email protected]... +0 -2
@@ -9,8 +9,6 @@ Documentation=https://community.openvpn.net/openvpn/wiki/HOWTO [Service] Type=notify PrivateTmp=true -RuntimeDirectory=openvpn-client -RuntimeDirectoryMode=0710 WorkingDirectory=/etc/openvpn/client ExecStart=/usr/sbin/openvpn --suppress-timestamps --nobind --config %i.conf CapabilityBoundingSet=CAP_IPC_LOCK CAP_NET_ADMIN CAP_NET_RAW CAP_SETGID CAP_SETUID CAP_SYS_CHROOT CAP_DAC_OVERRIDE
distro/systemd/[email protected]... +0 -2
@@ -9,8 +9,6 @@ Documentation=https://community.openvpn.net/openvpn/wiki/HOWTO [Service] Type=notify PrivateTmp=true -RuntimeDirectory=openvpn-server -RuntimeDirectoryMode=0710 WorkingDirectory=/etc/openvpn/server ExecStart=/usr/sbin/openvpn --status %t/openvpn-server/status-%i.log --status-version 2 --suppress-timestamps --config %i.conf CapabilityBoundingSet=CAP_IPC_LOCK CAP_NET_ADMIN CAP_NET_BIND_SERVICE CAP_NET_RAW CAP_SETGID CAP_SETUID CAP_SYS_CHROOT CAP_DAC_OVERRIDE
distro/systemd/openvpn.conf new file +2 -0
@@ -0,0 +1,2 @@ +d /run/openvpn-client 0710 root root - +d /run/openvpn-server 0710 root root -
Signature · 1 line
2.11.0
Quoted 52 lines · Christian Hesse
From: Christian Hesse <[email protected]...>
Different unit instances create and destroy the same RuntimeDirectory. This leads to running instances where the status file (and possibly more runtime data) is no longer accessible.
So do not handle this in unit files but provide a tmpfiles.d configuration and let systemd-tmpfiles do the work.
Nobody will (unintentionally) delete the directories and its content. As /run is volatile we do not have to care about cleanup.Signed-off-by Christian Hesse <[email protected]...>--- distro/systemd/[email protected]... | 2 -- distro/systemd/[email protected]... | 2 -- distro/systemd/openvpn.conf | 2 ++ 3 files changed, 2 insertions(+), 4 deletions(-) create mode 100644 distro/systemd/openvpn.confdistro/systemd/[email protected]... +0
-2@@ -9,8 +9,6 @@ Documentation=https://community.openvpn.net/openvpn/wiki/HOWTO [Service] Type=notify PrivateTmp=true -RuntimeDirectory=openvpn-client -RuntimeDirectoryMode=0710 WorkingDirectory=/etc/openvpn/client ExecStart=/usr/sbin/openvpn --suppress-timestamps --nobind --config %i.conf CapabilityBoundingSet=CAP_IPC_LOCK CAP_NET_ADMIN CAP_NET_RAW CAP_SETGID CAP_SETUID CAP_SYS_CHROOT CAP_DAC_OVERRIDEdistro/systemd/[email protected]... +0
-2@@ -9,8 +9,6 @@ Documentation=https://community.openvpn.net/openvpn/wiki/HOWTO [Service] Type=notify PrivateTmp=true -RuntimeDirectory=openvpn-server -RuntimeDirectoryMode=0710 WorkingDirectory=/etc/openvpn/server ExecStart=/usr/sbin/openvpn --status %t/openvpn-server/status-%i.log --status-version 2 --suppress-timestamps --config %i.conf CapabilityBoundingSet=CAP_IPC_LOCK CAP_NET_ADMIN CAP_NET_BIND_SERVICE CAP_NET_RAW CAP_SETGID CAP_SETUID CAP_SYS_CHROOT CAP_DAC_OVERRIDEdistro/systemd/openvpn.conf new file +0
-0
distro/systemd/openvpn.conf line 2
Quoted 2 lines
distro/systemd/openvpn.conf new file +2
-0@@ -0,0 +1,2 @@ +d /run/openvpn-client 0710 root root - +d /run/openvpn-server 0710 root root -
ACK
This works as expected from debian8/systemd 215 to arch/systemd 232
debbie10t <[email protected]...> on Sat, 2016/12/24 11:10:
Quoted 58 lines · Christian Hesse
Quoted 52 lines
From: Christian Hesse <[email protected]...>
Different unit instances create and destroy the same RuntimeDirectory. This leads to running instances where the status file (and possibly more runtime data) is no longer accessible.
So do not handle this in unit files but provide a tmpfiles.d configuration and let systemd-tmpfiles do the work.
Nobody will (unintentionally) delete the directories and its content. As /run is volatile we do not have to care about cleanup.Signed-off-by Christian Hesse <[email protected]...>--- distro/systemd/[email protected]... | 2 -- distro/systemd/[email protected]... | 2 -- distro/systemd/openvpn.conf | 2 ++ 3 files changed, 2 insertions(+), 4 deletions(-) create mode 100644 distro/systemd/openvpn.conf(quoted diff) +0
-0b/distro/systemd/[email protected]... index 5618af3..1187ee8 100644
distro/systemd/[email protected]... +0
-0@@ -9,8 +9,6 @@Documentation=https://community.openvpn.net/openvpn/wiki/HOWTO [Service]
Type=notify
PrivateTmp=truedistro/systemd/[email protected]... +0
-2-RuntimeDirectory=openvpn-client -RuntimeDirectoryMode=0710 WorkingDirectory=/etc/openvpn/client ExecStart=/usr/sbin/openvpn --suppress-timestamps --nobind --config%i.conf CapabilityBoundingSet=CAP_IPC_LOCK
CAP_NET_ADMINCAP_NET_RAWCAP_SETGIDCAP_SETUIDCAP_SYS_CHROOTCAP_DAC_OVERRIDEdiff --git a/distro/systemd/[email protected]... b/distro/systemd/[email protected]... index b9b4dba..25a6bb7 100644 --- a/distro/systemd/[email protected]... +++ b/distro/systemd/[email protected]... @@ -9,8 +9,6 @@ Documentation=https://community.openvpn.net/openvpn/wiki/HOWTO [Service]
Type=notify
PrivateTmp=truedistro/systemd/[email protected]... +0
-3-RuntimeDirectory=openvpn-server -RuntimeDirectoryMode=0710 WorkingDirectory=/etc/openvpn/server ExecStart=/usr/sbin/openvpn --status %t/openvpn-server/status-%i.log --status-version 2 --suppress-timestamps --config %i.confCapabilityBoundingSet=CAP_IPC_LOCK
CAP_NET_ADMINCAP_NET_BIND_SERVICECAP_NET_RAWCAP_SETGIDCAP_SETUIDCAP_SYS_CHROOTCAP_DAC_OVERRIDEdiff --gita/distro/systemd/openvpn.confb/distro/systemd/openvpn.confnew file mode 100644 index 0000000..bb79671 ---/dev/null
+++b/distro/systemd/openvpn.confdistro/systemd/[email protected]... line 2
Quoted 2 lines
distro/systemd/[email protected]... +2
-0@@ -0,0 +1,2 @@ +d /run/openvpn-client 0710 root root - +d /run/openvpn-server 0710 root root -ACK
Quoted 1 line
This works as expected from debian8/systemd 215 to
arch/systemd232
Great! Thanks for testing!
But I think this will not make its way into 2.4.0? Will we see this in release/2.4 for a bugfix release?
Signature · 3 lines
main(a){char*c=/* Schoene Gruesse */"B?IJj;MEH"
"CX:;",b;for(a/* Best regards my address: */=0;b=c[a++];)
putchar(b-1/(/* Chris cc -ox -xc - && ./x */b/42*2-3)*42);}Quoted 59 lines · Christian Hesse
debbie10t <[email protected]...> on Sat, 2016/12/24 11:10:
Quoted 58 lines · Christian Hesse
Quoted 52 lines
From: Christian Hesse <[email protected]...>
Different unit instances create and destroy the same RuntimeDirectory. This leads to running instances where the status file (and possibly more runtime data) is no longer accessible.
So do not handle this in unit files but provide a tmpfiles.d configuration and let systemd-tmpfiles do the work.
Nobody will (unintentionally) delete the directories and its content. As /run is volatile we do not have to care about cleanup.Signed-off-by Christian Hesse <[email protected]...>--- distro/systemd/[email protected]... | 2 -- distro/systemd/[email protected]... | 2 -- distro/systemd/openvpn.conf | 2 ++ 3 files changed, 2 insertions(+), 4 deletions(-) create mode 100644 distro/systemd/openvpn.conf(quoted diff) +0
-0b/distro/systemd/[email protected]... index 5618af3..1187ee8 100644
distro/systemd/[email protected]... +0
-0@@ -9,8 +9,6 @@Documentation=https://community.openvpn.net/openvpn/wiki/HOWTO [Service]
Type=notify
PrivateTmp=truedistro/systemd/[email protected]... +0
-2-RuntimeDirectory=openvpn-client -RuntimeDirectoryMode=0710 WorkingDirectory=/etc/openvpn/client ExecStart=/usr/sbin/openvpn --suppress-timestamps --nobind --config%i.conf CapabilityBoundingSet=CAP_IPC_LOCK
CAP_NET_ADMINCAP_NET_RAWCAP_SETGIDCAP_SETUIDCAP_SYS_CHROOTCAP_DAC_OVERRIDEdiff --git a/distro/systemd/[email protected]... b/distro/systemd/[email protected]... index b9b4dba..25a6bb7 100644 --- a/distro/systemd/[email protected]... +++ b/distro/systemd/[email protected]... @@ -9,8 +9,6 @@ Documentation=https://community.openvpn.net/openvpn/wiki/HOWTO [Service]
Type=notify
PrivateTmp=truedistro/systemd/[email protected]... +0
-3-RuntimeDirectory=openvpn-server -RuntimeDirectoryMode=0710 WorkingDirectory=/etc/openvpn/server ExecStart=/usr/sbin/openvpn --status %t/openvpn-server/status-%i.log --status-version 2 --suppress-timestamps --config %i.confCapabilityBoundingSet=CAP_IPC_LOCK
CAP_NET_ADMINCAP_NET_BIND_SERVICECAP_NET_RAWCAP_SETGIDCAP_SETUIDCAP_SYS_CHROOTCAP_DAC_OVERRIDEdiff --gita/distro/systemd/openvpn.confb/distro/systemd/openvpn.confnew file mode 100644 index 0000000..bb79671 ---/dev/null
+++b/distro/systemd/openvpn.confdistro/systemd/[email protected]... line 2
Quoted 2 lines
distro/systemd/[email protected]... +2
-0@@ -0,0 +1,2 @@ +d /run/openvpn-client 0710 root root - +d /run/openvpn-server 0710 root root -ACK
Quoted 6 lines
Quoted 1 line
This works as expected from debian8/systemd 215 to
arch/systemd232Great! Thanks for testing!
But I think this will not make its way into 2.4.0? Will we see this in release/2.4 for a bugfix release?
That is correct. What is in release/2.4 currently is what will be the v2.4.0 release unless something of a real blocker appears before I'll tag and push out the release commit some time tomorrow. I don't dare to add anything which is not absolutely strictly needed for the comming release. I'm just waiting for a the final Windows test results from Samuli before pushing out the final release. Then Samuli will publish all source tarballs, Windows installers and do the announcement.
It's good that this is tested, but I'd like to have a more thorough review of this patch as well in addition to look at the automake rules so that we can install unit files and the tmpfiles.d config to the proper place as well during 'make install'. This will simplify the job of the packagers as well.
One thing I'm pondering on though, is how clever it is to call the tmpfiles.d config file 'openvpn.conf' ... but that's something I can fix at commit time.
Signature · 4 lines
kind regards, David Sommerseth OpenVPN Technologies, Inc
David Sommerseth <[email protected]...> on Mon, 2016/12/26 17:45:
Quoted 67 lines · Christian Hesse
Quoted 59 lines
debbie10t <[email protected]...> on Sat, 2016/12/24 11:10:
Quoted 58 lines · Christian Hesse
Quoted 52 lines
From: Christian Hesse <[email protected]...>
Different unit instances create and destroy the same RuntimeDirectory. This leads to running instances where the status file (and possibly more runtime data) is no longer accessible.
So do not handle this in unit files but provide a tmpfiles.d configuration and let systemd-tmpfiles do the work.
Nobody will (unintentionally) delete the directories and its content. As /run is volatile we do not have to care about cleanup.Signed-off-by Christian Hesse <[email protected]...>--- distro/systemd/[email protected]... | 2 -- distro/systemd/[email protected]... | 2 -- distro/systemd/openvpn.conf | 2 ++ 3 files changed, 2 insertions(+), 4 deletions(-) create mode 100644 distro/systemd/openvpn.conf(quoted diff) +0
-0b/distro/systemd/[email protected]... index 5618af3..1187ee8 100644
distro/systemd/[email protected]... +0
-0@@ -9,8 +9,6 @@Documentation=https://community.openvpn.net/openvpn/wiki/HOWTO [Service]
Type=notify
PrivateTmp=truedistro/systemd/[email protected]... +0
-2-RuntimeDirectory=openvpn-client -RuntimeDirectoryMode=0710 WorkingDirectory=/etc/openvpn/client ExecStart=/usr/sbin/openvpn --suppress-timestamps --nobind --config%i.conf CapabilityBoundingSet=CAP_IPC_LOCK
CAP_NET_ADMINCAP_NET_RAWCAP_SETGIDCAP_SETUIDCAP_SYS_CHROOTCAP_DAC_OVERRIDEdiff --git a/distro/systemd/[email protected]... b/distro/systemd/[email protected]... index b9b4dba..25a6bb7 100644 --- a/distro/systemd/[email protected]... +++ b/distro/systemd/[email protected]... @@ -9,8 +9,6 @@ Documentation=https://community.openvpn.net/openvpn/wiki/HOWTO [Service]
Type=notify
PrivateTmp=truedistro/systemd/[email protected]... +0
-3-RuntimeDirectory=openvpn-server -RuntimeDirectoryMode=0710 WorkingDirectory=/etc/openvpn/server ExecStart=/usr/sbin/openvpn --status %t/openvpn-server/status-%i.log --status-version 2 --suppress-timestamps --config %i.confCapabilityBoundingSet=CAP_IPC_LOCK
CAP_NET_ADMINCAP_NET_BIND_SERVICECAP_NET_RAWCAP_SETGIDCAP_SETUIDCAP_SYS_CHROOTCAP_DAC_OVERRIDEdiff --gita/distro/systemd/openvpn.confb/distro/systemd/openvpn.confnew file mode 100644 index 0000000..bb79671 ---/dev/null
+++b/distro/systemd/openvpn.confdistro/systemd/[email protected]... line 2
Quoted 2 lines
distro/systemd/[email protected]... +2
-0@@ -0,0 +1,2 @@ +d /run/openvpn-client 0710 root root - +d /run/openvpn-server 0710 root root -ACK
Quoted 6 lines
Quoted 1 line
This works as expected from debian8/systemd 215 to
arch/systemd232Great! Thanks for testing!
But I think this will not make its way into 2.4.0? Will we see this in release/2.4 for a bugfix release?
Quoted 7 lines
That is correct. What is in release/2.4 currently is what will be the v2.4.0 release unless something of a real blocker appears before I'll tag and push out the release commit some time tomorrow. I don't dare to add anything which is not absolutely strictly needed for the comming release. I'm just waiting for a the final Windows test results from Samuli before pushing out the final release. Then Samuli will publish all source tarballs, Windows installers and do the announcement.
Waiting for that to happen. :-p
Quoted 5 lines
It's good that this is tested, but I'd like to have a more thorough review of this patch as well in addition to look at the automake rules so that we can install unit files and the tmpfiles.d config to the proper place as well during 'make install'. This will simplify the job of the packagers as well.
Ok, let's discuss this on irc any time soon. I will have some last working days this year - and hopefully some spare time.
Quoted 3 lines
One thing I'm pondering on though, is how clever it is to call the tmpfiles.d config file 'openvpn.conf' ... but that's something I can fix at commit time.
Well, files in /usr/lib/tmpfiles.d/ need to end in '.conf' and it makes sense to name them like their package - so 'openvpn.conf'.
We can have a different file name in repository / tarball, but the build system should install it there.
Signature · 3 lines
main(a){char*c=/* Schoene Gruesse */"B?IJj;MEH"
"CX:;",b;for(a/* Best regards my address: */=0;b=c[a++];)
putchar(b-1/(/* Chris cc -ox -xc - && ./x */b/42*2-3)*42);}Quoted 33 lines · David Sommerseth
Quoted 19 lines · Christian Hesse
From: Christian Hesse <[email protected]...>
Different unit instances create and destroy the same RuntimeDirectory. This leads to running instances where the status file (and possibly more runtime data) is no longer accessible.
So create a RuntimeDirectory per instance.
Signed-off-by Christian Hesse <[email protected]...>--- distro/systemd/[email protected]... | 2 +- distro/systemd/[email protected]... | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)distro/systemd/[email protected]... +0
-0distro/systemd/[email protected]... Documentation=https://community.openvpn.net/openvpn/wiki/HOWTO · line 12
Quoted 5 lines
distro/systemd/[email protected]... +1
-1@@ -9,7 +9,7 @@ Documentation=https://community.openvpn.net/openvpn/wiki/HOWTO [Service] Type=notify PrivateTmp=true -RuntimeDirectory=openvpn-client +RuntimeDirectory=openvpn-client@%iI am quite sure we tested this, without any luck. We ended up with a directory named openvpn-client@%i ... well, we used openvpn-client-%i, but the principle should be the same.
This *might* have been fixed in a newer systemd releases, but as we have RHEL7 as the oldest release we need to ensure this works properly there too.
Quoted 6 lines
For the server side, we used status-%i.log to avoid clashing on the status log.
debbie10t did plenty of testing with various configurations, so he can probably fill out more details in the various test cases he used. We should probably document those tests in our wiki though.
This was spotted but I did not take sufficient ownership ..
I will happily test @Christian 's final patch.
regards
Quoted 35 lines · Christian Hesse
From: Christian Hesse <[email protected]...>
Different unit instances create and destroy the same RuntimeDirectory. This leads to running instances where the status file (and possibly more runtime data) is no longer accessible.
So create a RuntimeDirectory per instance.
Signed-off-by Christian Hesse <[email protected]...>--- distro/systemd/[email protected]... | 2 +- distro/systemd/[email protected]... | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)distro/systemd/[email protected]... +1
-1@@ -9,7 +9,7 @@ Documentation=https://community.openvpn.net/openvpn/wiki/HOWTO [Service] Type=notify PrivateTmp=true -RuntimeDirectory=openvpn-client +RuntimeDirectory=openvpn-client@%i RuntimeDirectoryMode=0710 WorkingDirectory=/etc/openvpn/client ExecStart=/usr/sbin/openvpn --suppress-timestamps --nobind --config %i.confdistro/systemd/[email protected]... +0
-0@@ -9,10 +9,10 @@ Documentation=https://community.openvpn.net/openvpn/wiki/HOWTO [Service] Type=notify
distro/systemd/[email protected]... Documentation=https://community.openvpn.net/openvpn/wiki/HOWTO · line 18
Quoted 10 lines
distro/systemd/[email protected]... +2
-2PrivateTmp=true -RuntimeDirectory=openvpn-server +RuntimeDirectory=openvpn-server@%i RuntimeDirectoryMode=0710 WorkingDirectory=/etc/openvpn/server -ExecStart=/usr/sbin/openvpn --status %t/openvpn-server/status-%i.log --status-version 2 --suppress-timestamps --config %i.conf +ExecStart=/usr/sbin/openvpn --status %t/openvpn-server@%i/status.log --status-version 2 --suppress-timestamps --config %i.conf CapabilityBoundingSet=CAP_IPC_LOCK CAP_NET_ADMIN CAP_NET_BIND_SERVICE CAP_NET_RAW CAP_SETGID CAP_SETUID CAP_SYS_CHROOT CAP_DAC_OVERRIDE LimitNPROC=10 DeviceAllow=/dev/null rw
CentOS 7 (after full update) still only uses systemd version 219
This functionality is not introduced until systemd 227
https://github.com/systemd/systemd/blob/master/NEWS
line 1380 ..
(pfft)
debbie10t <[email protected]...> on Tue, 2016/12/20 00:32:
Quoted 51 lines · Christian Hesse
Quoted 43 lines
From: Christian Hesse <[email protected]...>
Different unit instances create and destroy the same RuntimeDirectory. This leads to running instances where the status file (and possibly more runtime data) is no longer accessible.
So create a RuntimeDirectory per instance.
Signed-off-by Christian Hesse <[email protected]...>--- distro/systemd/[email protected]... | 2 +- distro/systemd/[email protected]... | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)(quoted diff) +0
-0b/distro/systemd/[email protected]... index 5618af3..fcb5302 100644
distro/systemd/[email protected]... +0
-0@@ -9,7 +9,7 @@Documentation=https://community.openvpn.net/openvpn/wiki/HOWTO [Service]
Type=notify
PrivateTmp=truedistro/systemd/[email protected]... +1
-1-RuntimeDirectory=openvpn-client +RuntimeDirectory=openvpn-client@%i RuntimeDirectoryMode=0710 WorkingDirectory=/etc/openvpn/client ExecStart=/usr/sbin/openvpn --suppress-timestamps --nobind --config%i.conf diff --git a/distro/systemd/[email protected]... b/distro/systemd/[email protected]... index b9b4dba..8b240cf 100644
distro/systemd/[email protected]... +0
-0@@ -9,10 +9,10 @@Documentation=https://community.openvpn.net/openvpn/wiki/HOWTO [Service]
Type=notify
PrivateTmp=truedistro/systemd/[email protected]... +2
-4-RuntimeDirectory=openvpn-server +RuntimeDirectory=openvpn-server@%i RuntimeDirectoryMode=0710 WorkingDirectory=/etc/openvpn/server -ExecStart=/usr/sbin/openvpn --status %t/openvpn-server/status-%i.log --status-version 2 --suppress-timestamps --config %i.conf +ExecStart=/usr/sbin/openvpn --status %t/openvpn-server@%i/status.log --status-version 2 --suppress-timestamps --config %i.confQuoted 3 lines
CapabilityBoundingSet=CAP_IPC_LOCK
CAP_NET_ADMINCAP_NET_BIND_SERVICECAP_NET_RAWCAP_SETGIDCAP_SETUIDCAP_SYS_CHROOTCAP_DAC_OVERRIDELimitNPROC=10 DeviceAllow=/dev/null rwCentOS 7 (after full update) still only uses systemd version 219
Quoted 3 lines
This functionality is not introduced until systemd 227
Ah, good to know when this was implemented. :-D
But we already know that this does not work for version we want to support. Thus I had sent a patch with new approach on Dec 16th where RuntimeDirectory is removed from unit files and directories are handled via tmpfiles.d. Can you take a look at that one?
Signature · 3 lines
main(a){char*c=/* Schoene Gruesse */"B?IJj;MEH"
"CX:;",b;for(a/* Best regards my address: */=0;b=c[a++];)
putchar(b-1/(/* Chris cc -ox -xc - && ./x */b/42*2-3)*42);}