Re: [Openvpn-devel] [PATCH 1/1] do not race on RuntimeDirectory

2016-12-26 Thread Christian Hesse
David Sommerseth  on Mon, 2016/12/26 17:45:
> On 26/12/16 17:12, Christian Hesse wrote:
> > debbie10t  on Sat, 2016/12/24 11:10:  
> >> On 16/12/16 22:00, Christian Hesse wrote:  
> >>> From: Christian Hesse 
> >>>
> >>> 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 
> >>> ---
> >>>  distro/systemd/openvpn-client@.service | 2 --
> >>>  distro/systemd/openvpn-server@.service | 2 --
> >>>  distro/systemd/openvpn.conf| 2 ++
> >>>  3 files changed, 2 insertions(+), 4 deletions(-)
> >>>  create mode 100644 distro/systemd/openvpn.conf
> >>>
> >>> diff --git a/distro/systemd/openvpn-client@.service
> >>> b/distro/systemd/openvpn-client@.service index 5618af3..1187ee8 100644
> >>> --- a/distro/systemd/openvpn-client@.service
> >>> +++ b/distro/systemd/openvpn-client@.service
> >>> @@ -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 diff --git
> >>> a/distro/systemd/openvpn-server@.service
> >>> b/distro/systemd/openvpn-server@.service index b9b4dba..25a6bb7 100644
> >>> --- a/distro/systemd/openvpn-server@.service +++
> >>> b/distro/systemd/openvpn-server@.service @@ -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 diff
> >>> --git a/distro/systemd/openvpn.conf b/distro/systemd/openvpn.conf new
> >>> file mode 100644 index 000..bb79671 --- /dev/null
> >>> +++ b/distro/systemd/openvpn.conf
> >>> @@ -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  
> > 
> > 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?  
> 
> 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

> 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.

> 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.
-- 
main(a){char*c=/*Schoene Gruesse */"B?IJj;MEH"
"CX:;",b;for(a/*Best regards my address:*/=0;b=c[a++];)
putchar(b-1/(/*Chriscc -ox -xc - && ./x*/b/42*2-3)*42);}


pgpRz_4bwUVVV.pgp
Description: OpenPGP digital signature
--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Or

Re: [Openvpn-devel] [PATCH 1/1] do not race on RuntimeDirectory

2016-12-26 Thread David Sommerseth
On 26/12/16 17:12, Christian Hesse wrote:
> debbie10t  on Sat, 2016/12/24 11:10:
>> On 16/12/16 22:00, Christian Hesse wrote:
>>> From: Christian Hesse 
>>>
>>> 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 
>>> ---
>>>  distro/systemd/openvpn-client@.service | 2 --
>>>  distro/systemd/openvpn-server@.service | 2 --
>>>  distro/systemd/openvpn.conf| 2 ++
>>>  3 files changed, 2 insertions(+), 4 deletions(-)
>>>  create mode 100644 distro/systemd/openvpn.conf
>>>
>>> diff --git a/distro/systemd/openvpn-client@.service
>>> b/distro/systemd/openvpn-client@.service index 5618af3..1187ee8 100644
>>> --- a/distro/systemd/openvpn-client@.service
>>> +++ b/distro/systemd/openvpn-client@.service
>>> @@ -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 diff --git
>>> a/distro/systemd/openvpn-server@.service
>>> b/distro/systemd/openvpn-server@.service index b9b4dba..25a6bb7 100644
>>> --- a/distro/systemd/openvpn-server@.service +++
>>> b/distro/systemd/openvpn-server@.service @@ -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 diff
>>> --git a/distro/systemd/openvpn.conf b/distro/systemd/openvpn.conf new
>>> file mode 100644 index 000..bb79671 --- /dev/null
>>> +++ b/distro/systemd/openvpn.conf
>>> @@ -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
> 
> 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?

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.


-- 
kind regards,

David Sommerseth
OpenVPN Technologies, Inc




signature.asc
Description: OpenPGP digital signature
--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/intel___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH 1/1] do not race on RuntimeDirectory

2016-12-26 Thread Christian Hesse
debbie10t  on Sat, 2016/12/24 11:10:
> On 16/12/16 22:00, Christian Hesse wrote:
> > From: Christian Hesse 
> >
> > 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 
> > ---
> >  distro/systemd/openvpn-client@.service | 2 --
> >  distro/systemd/openvpn-server@.service | 2 --
> >  distro/systemd/openvpn.conf| 2 ++
> >  3 files changed, 2 insertions(+), 4 deletions(-)
> >  create mode 100644 distro/systemd/openvpn.conf
> >
> > diff --git a/distro/systemd/openvpn-client@.service
> > b/distro/systemd/openvpn-client@.service index 5618af3..1187ee8 100644
> > --- a/distro/systemd/openvpn-client@.service
> > +++ b/distro/systemd/openvpn-client@.service
> > @@ -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 diff --git
> > a/distro/systemd/openvpn-server@.service
> > b/distro/systemd/openvpn-server@.service index b9b4dba..25a6bb7 100644
> > --- a/distro/systemd/openvpn-server@.service +++
> > b/distro/systemd/openvpn-server@.service @@ -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 diff
> > --git a/distro/systemd/openvpn.conf b/distro/systemd/openvpn.conf new
> > file mode 100644 index 000..bb79671 --- /dev/null
> > +++ b/distro/systemd/openvpn.conf
> > @@ -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

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?
-- 
main(a){char*c=/*Schoene Gruesse */"B?IJj;MEH"
"CX:;",b;for(a/*Best regards my address:*/=0;b=c[a++];)
putchar(b-1/(/*Chriscc -ox -xc - && ./x*/b/42*2-3)*42);}


pgpVpD5pqwYEy.pgp
Description: OpenPGP digital signature
--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/intel___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH 1/1] do not race on RuntimeDirectory

2016-12-24 Thread debbie10t


On 16/12/16 22:00, Christian Hesse wrote:
> From: Christian Hesse 
>
> 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 
> ---
>  distro/systemd/openvpn-client@.service | 2 --
>  distro/systemd/openvpn-server@.service | 2 --
>  distro/systemd/openvpn.conf| 2 ++
>  3 files changed, 2 insertions(+), 4 deletions(-)
>  create mode 100644 distro/systemd/openvpn.conf
>
> diff --git a/distro/systemd/openvpn-client@.service 
> b/distro/systemd/openvpn-client@.service
> index 5618af3..1187ee8 100644
> --- a/distro/systemd/openvpn-client@.service
> +++ b/distro/systemd/openvpn-client@.service
> @@ -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
> diff --git a/distro/systemd/openvpn-server@.service 
> b/distro/systemd/openvpn-server@.service
> index b9b4dba..25a6bb7 100644
> --- a/distro/systemd/openvpn-server@.service
> +++ b/distro/systemd/openvpn-server@.service
> @@ -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
> diff --git a/distro/systemd/openvpn.conf b/distro/systemd/openvpn.conf
> new file mode 100644
> index 000..bb79671
> --- /dev/null
> +++ b/distro/systemd/openvpn.conf
> @@ -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

-- 

--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/intel
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH 1/1] do not race on RuntimeDirectory

2016-12-21 Thread Christian Hesse
debbie10t  on Tue, 2016/12/20 00:32:
> On 16/12/16 15:57, Christian Hesse wrote:
> > From: Christian Hesse 
> >
> > 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 
> > ---
> >  distro/systemd/openvpn-client@.service | 2 +-
> >  distro/systemd/openvpn-server@.service | 4 ++--
> >  2 files changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/distro/systemd/openvpn-client@.service
> > b/distro/systemd/openvpn-client@.service index 5618af3..fcb5302 100644
> > --- a/distro/systemd/openvpn-client@.service
> > +++ b/distro/systemd/openvpn-client@.service
> > @@ -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 diff --git a/distro/systemd/openvpn-server@.service
> > b/distro/systemd/openvpn-server@.service index b9b4dba..8b240cf 100644
> > --- a/distro/systemd/openvpn-server@.service
> > +++ b/distro/systemd/openvpn-server@.service
> > @@ -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 
> 
> 
> 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

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?
-- 
main(a){char*c=/*Schoene Gruesse */"B?IJj;MEH"
"CX:;",b;for(a/*Best regards my address:*/=0;b=c[a++];)
putchar(b-1/(/*Chriscc -ox -xc - && ./x*/b/42*2-3)*42);}


pgpY35vqpn_LT.pgp
Description: OpenPGP digital signature
--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/intel___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH 1/1] do not race on RuntimeDirectory

2016-12-19 Thread debbie10t


On 16/12/16 15:57, Christian Hesse wrote:
> From: Christian Hesse 
>
> 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 
> ---
>  distro/systemd/openvpn-client@.service | 2 +-
>  distro/systemd/openvpn-server@.service | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/distro/systemd/openvpn-client@.service 
> b/distro/systemd/openvpn-client@.service
> index 5618af3..fcb5302 100644
> --- a/distro/systemd/openvpn-client@.service
> +++ b/distro/systemd/openvpn-client@.service
> @@ -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
> diff --git a/distro/systemd/openvpn-server@.service 
> b/distro/systemd/openvpn-server@.service
> index b9b4dba..8b240cf 100644
> --- a/distro/systemd/openvpn-server@.service
> +++ b/distro/systemd/openvpn-server@.service
> @@ -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
>


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)


--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/intel
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH 1/1] do not race on RuntimeDirectory

2016-12-16 Thread debbie10t


On 16/12/16 18:14, David Sommerseth wrote:
> On 16/12/16 16:57, Christian Hesse wrote:
>> From: Christian Hesse 
>>
>> 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 
>> ---
>>  distro/systemd/openvpn-client@.service | 2 +-
>>  distro/systemd/openvpn-server@.service | 4 ++--
>>  2 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/distro/systemd/openvpn-client@.service 
>> b/distro/systemd/openvpn-client@.service
>> index 5618af3..fcb5302 100644
>> --- a/distro/systemd/openvpn-client@.service
>> +++ b/distro/systemd/openvpn-client@.service
>> @@ -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.

This was spotted but I did not take sufficient ownership ..

I will happily test @Christian 's final patch.

regards

--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [PATCH 1/1] do not race on RuntimeDirectory

2016-12-16 Thread Christian Hesse
From: Christian Hesse 

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 
---
 distro/systemd/openvpn-client@.service | 2 --
 distro/systemd/openvpn-server@.service | 2 --
 distro/systemd/openvpn.conf| 2 ++
 3 files changed, 2 insertions(+), 4 deletions(-)
 create mode 100644 distro/systemd/openvpn.conf

diff --git a/distro/systemd/openvpn-client@.service 
b/distro/systemd/openvpn-client@.service
index 5618af3..1187ee8 100644
--- a/distro/systemd/openvpn-client@.service
+++ b/distro/systemd/openvpn-client@.service
@@ -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
diff --git a/distro/systemd/openvpn-server@.service 
b/distro/systemd/openvpn-server@.service
index b9b4dba..25a6bb7 100644
--- a/distro/systemd/openvpn-server@.service
+++ b/distro/systemd/openvpn-server@.service
@@ -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
diff --git a/distro/systemd/openvpn.conf b/distro/systemd/openvpn.conf
new file mode 100644
index 000..bb79671
--- /dev/null
+++ b/distro/systemd/openvpn.conf
@@ -0,0 +1,2 @@
+d /run/openvpn-client 0710 root root -
+d /run/openvpn-server 0710 root root -
-- 
2.11.0


--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH 1/1] do not race on RuntimeDirectory

2016-12-16 Thread Christian Hesse
David Sommerseth  on Fri, 2016/12/16 22:15:
> On 16/12/16 20:09, Christian Hesse wrote:
> > David Sommerseth  on Fri, 2016/12/16
> > 19:14:  
> >> On 16/12/16 16:57, Christian Hesse wrote:  
> >>> From: Christian Hesse 
> >>>
> >>> 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 
> >>> ---
> >>>  distro/systemd/openvpn-client@.service | 2 +-
> >>>  distro/systemd/openvpn-server@.service | 4 ++--
> >>>  2 files changed, 3 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/distro/systemd/openvpn-client@.service
> >>> b/distro/systemd/openvpn-client@.service index 5618af3..fcb5302 100644
> >>> --- a/distro/systemd/openvpn-client@.service
> >>> +++ b/distro/systemd/openvpn-client@.service
> >>> @@ -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.  
> > 
> > Running systemd 232 here and everything works as expected. This is with
> > unit files from current master:
> > 
> > # systemctl start openvpn-server@test1.service
> > # systemctl start openvpn-server@test2.service
> > # systemctl stop openvpn-server@test2.service
> > # 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/openvpn-server@test1/status.log 
> > -rw--- 1 root root 8419 Dec 16
> > 20:03 /run/openvpn-server@test1/status.log
> > 
> > From systemd.exec(5) about RuntimeDirectory=:  
> >> 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'

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.
-- 
main(a){char*c=/*Schoene Gruesse */"B?IJj;MEH"
"CX:;",b;for(a/*Best regards my address:*/=0;b=c[a++];)
putchar(b-1/(/*Chriscc -ox -xc - && ./x*/b/42*2-3)*42);}


pgpzEsTVbc2Yn.pgp
Description: OpenPGP digital signature
--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH 1/1] do not race on RuntimeDirectory

2016-12-16 Thread David Sommerseth
On 16/12/16 20:09, Christian Hesse wrote:
> David Sommerseth  on Fri, 2016/12/16 19:14:
>> On 16/12/16 16:57, Christian Hesse wrote:
>>> From: Christian Hesse 
>>>
>>> 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 
>>> ---
>>>  distro/systemd/openvpn-client@.service | 2 +-
>>>  distro/systemd/openvpn-server@.service | 4 ++--
>>>  2 files changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/distro/systemd/openvpn-client@.service
>>> b/distro/systemd/openvpn-client@.service index 5618af3..fcb5302 100644
>>> --- a/distro/systemd/openvpn-client@.service
>>> +++ b/distro/systemd/openvpn-client@.service
>>> @@ -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.
> 
> Running systemd 232 here and everything works as expected. This is with unit
> files from current master:
> 
> # systemctl start openvpn-server@test1.service
> # systemctl start openvpn-server@test2.service
> # systemctl stop openvpn-server@test2.service
> # 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/openvpn-server@test1/status.log 
> -rw--- 1 root root 8419 Dec 16 20:03 /run/openvpn-server@test1/status.log
> 
> From systemd.exec(5) about RuntimeDirectory=:
>> 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.



Otherwise, CentOS or Scientific Linux is very close to RHEL, just
lagging some weeks/months after each minor point release (7.2 -> 7.3).


-- 
kind regards,

David Sommerseth
OpenVPN Technologies, Inc




signature.asc
Description: OpenPGP digital signature
--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH 1/1] do not race on RuntimeDirectory

2016-12-16 Thread Christian Hesse
David Sommerseth  on Fri, 2016/12/16 19:14:
> On 16/12/16 16:57, Christian Hesse wrote:
> > From: Christian Hesse 
> > 
> > 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 
> > ---
> >  distro/systemd/openvpn-client@.service | 2 +-
> >  distro/systemd/openvpn-server@.service | 4 ++--
> >  2 files changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/distro/systemd/openvpn-client@.service
> > b/distro/systemd/openvpn-client@.service index 5618af3..fcb5302 100644
> > --- a/distro/systemd/openvpn-client@.service
> > +++ b/distro/systemd/openvpn-client@.service
> > @@ -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.

Running systemd 232 here and everything works as expected. This is with unit
files from current master:

# systemctl start openvpn-server@test1.service
# systemctl start openvpn-server@test2.service
# systemctl stop openvpn-server@test2.service
# 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/openvpn-server@test1/status.log 
-rw--- 1 root root 8419 Dec 16 20:03 /run/openvpn-server@test1/status.log

From systemd.exec(5) about RuntimeDirectory=:
> 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?
-- 
main(a){char*c=/*Schoene Gruesse */"B?IJj;MEH"
"CX:;",b;for(a/*Best regards my address:*/=0;b=c[a++];)
putchar(b-1/(/*Chriscc -ox -xc - && ./x*/b/42*2-3)*42);}


pgp96ZXOb55lq.pgp
Description: OpenPGP digital signature
--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH 1/1] do not race on RuntimeDirectory

2016-12-16 Thread David Sommerseth
On 16/12/16 16:57, Christian Hesse wrote:
> From: Christian Hesse 
> 
> 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 
> ---
>  distro/systemd/openvpn-client@.service | 2 +-
>  distro/systemd/openvpn-server@.service | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/distro/systemd/openvpn-client@.service 
> b/distro/systemd/openvpn-client@.service
> index 5618af3..fcb5302 100644
> --- a/distro/systemd/openvpn-client@.service
> +++ b/distro/systemd/openvpn-client@.service
> @@ -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.


-- 
kind regards,

David Sommerseth
OpenVPN Technologies, Inc




signature.asc
Description: OpenPGP digital signature
--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [PATCH 1/1] do not race on RuntimeDirectory

2016-12-16 Thread Christian Hesse
From: Christian Hesse 

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 
---
 distro/systemd/openvpn-client@.service | 2 +-
 distro/systemd/openvpn-server@.service | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/distro/systemd/openvpn-client@.service 
b/distro/systemd/openvpn-client@.service
index 5618af3..fcb5302 100644
--- a/distro/systemd/openvpn-client@.service
+++ b/distro/systemd/openvpn-client@.service
@@ -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
diff --git a/distro/systemd/openvpn-server@.service 
b/distro/systemd/openvpn-server@.service
index b9b4dba..8b240cf 100644
--- a/distro/systemd/openvpn-server@.service
+++ b/distro/systemd/openvpn-server@.service
@@ -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
-- 
2.11.0


--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel