Re: [Openvpn-devel] [PATCH 1/1] add more security features for systemd units
On 10/12/16 12:57, Christian Hesse wrote:
> SviMik on Sat, 2016/12/10 06:06:
>>> You can break this with something like:
>>>
>>> status /etc/openvpn/client/status.log
>>>
>>> in your configuration. Writing a status file
>>> to /run/openvpn-{client,server}/status.log works, though. So the default
>>> setups should be fine. Do we have any more cases where openvpn wants write
>>> access for whatever?
>>
>> From my configuration:
>> 1) status
>
> That is fine if it is written to /run/openvpn-{client,server}/. It breaks
> with the status file in /etc/openvpn/{client,server}/ or example.
FWIW, the default SELinux policies actually denies any openvpn_exec_t
process to write to /etc ... I believe that is independent of Linux
distros, as long as SELinux have been enabled and the system is Enforced
mode.
>> 2) ifconfig-pool-persist
>
> That is a problem... As the name suggests this should be persistent. :-/
Same SELinux issue here too. IIRC, these files needs to be located
under /var/lib/openvpn or /var/run/openvpn. But I do see there is one
exception ... /etc/openvpn/ipp\.txt will be labelled openvpn_etc_rw_t,
which is OpenVPN is allowed to write to.
>> 3) tmp-dir (for storing openvpn_pf_*.tmp files)
>
> Never used this. What is it for?
> Anyway, I think this is not persistent stuff? Writing to /tmp/
> or /run/openvpn-{client,server}/ should be fine.
The openvpn_pf_*.tmp files are just one thing. If you use
--auth-user-pass-verify script hooks or perhaps even --plugin for
authentication, other temp files are generated in the default tmp-dir.
See commit 4e1cc5f6dda22e9ff12 for more info.
>> 4) client-connect script may want to write something
>
> My scripts do some configuration and dbus-stuff, but do not write anything...
> Writing to read-only path would fail, of course.
Again, SELinux can again block this already ... unless you write in the
properly labelled directories for OpenVPN.
>> 5) a plugin may want to write something
>
> Same here... /run/ and /tmp/ is fine, other paths fail.
The same as 4)
>> For me even the read-only option will break nearly *everything*. And for
>> user it will be completely not obvious why his scripts doesn't work, why
>> his status file is not updated, and what's wrong with ifconfig-pool-persist.
>
> Well, the error message should include something like: "cannot open file
> 'file': Read-only file system".
>
> But this is more problematic than I thought initially.
If we have some directories which complies with the SELinux policies in
regards to read/write privileges, we should be fine. And restricting
which directories OpenVPN can write to is quite sane. All those plenty
of blogs putting runtime status files into /etc/openvpn have
misunderstood quite some of the concept of the Unix file system layout.
--
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/xeonphi___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH 1/1] bind mount systemd notification socket into chroot
On 10/12/16 13:08, Christian Hesse wrote:
> David Sommerseth on Sat, 2016/12/10 01:03:
>> On 10/12/16 00:19, Christian Hesse wrote:
>>> From: Christian Hesse
>>>
>>> sd_notify() uses a socket to communicate with systemd. Communication
>>> fails if the socket is not available within the chroot. So bind mount
>>> the socket into the chroot when startet from systemd.
>>>
>>> Unsharing namespace and mounting requires extra capability
>>> CAP_SYS_ADMIN.
>>
>> I will pick up this one after 2.4.0 has been released. This is a very
>> promising approach. However, I'm not too happy about CAP_SYS_ADMIN
>> though, that grants quite some privileges. Can we look at dropping this
>> capability once we know we won't need it any more? Perhaps when we send
>> READY=1?
>
> Never tried to drop capabilities... Have to look into that.
> We do no longer need CAP_SYS_ADMIN after the bind mount. (Or not at all
> without chrooting.)
Sounds reasonable. Since we need to take CAP_SYS_ADMIN out of the unit
file regardless, we need to drop the CAP_SYS_ADMIN capability inside
OpenVPN anyhow. Just need to figure out the best place for it.
As a pointer for managing capabilities, have a look at the capsetp(3)
man page.
>>> + char * chroot_notify = NULL;
>>> +
>>> + if (sd_notify(0, "READY=0") > 0)
>>> +{
>>> + asprintf(&chroot_notify, "%s/notify",
>>> c->options.chroot_dir);
>>
>> Here we should use the buffer/string functions, based on the gc_arena
>> implementation. Unfortunately we do not have a direct equivalent to
>> asprintf(). A starting point would be to for example look at the string
>> handling in print_sockaddr_ex() [socket.c:2386] or x_msg_va()
>> [error.c:251] ... there might be better examples too, I'm just not able
>> to remember them now :) buffer.[ch] keeps most of these functions.
>>
>> The reason for this is basically to use the same well tested
>> infrastructure. And with gc_arena, only a single gc_free() is required,
>> regardless of how many buffers you allocate to that arena.
>
> I do not like this myself. The patch is just a proof of concept... So this
> should be polished before committing. ;)
>
> Thanks for the hints, I will have a look.
It's a great PoC! And good to see that we might find a solution for
this. Will look forward to dig deeper into this in the coming weeks too.
--
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/xeonphi___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH 1/1] bind mount systemd notification socket into chroot
On 10/12/16 13:29, Gert Doering wrote:
> Hi,
>
> On Sat, Dec 10, 2016 at 12:19:07AM +0100, Christian Hesse wrote:
>> + int fd;
>> + char * chroot_notify = NULL;
>> +
>> + if (sd_notify(0, "READY=0") > 0)
>> +{
>> + asprintf(&chroot_notify, "%s/notify",
>> c->options.chroot_dir);
>> +
>> + if (unshare(CLONE_NEWNS) != 0)
>> +msg (M_ERR, "unshare failed");
>> + if ((fd = open(chroot_notify, O_WRONLY | O_CREAT |
>> O_TRUNC, 0644)) < 0)
>> +msg (M_ERR, "touch failed");
>> + close(fd);
>> + if (mount(getenv("NOTIFY_SOCKET"), chroot_notify, NULL,
>> MS_BIND, NULL) != 0)
>> +msg (M_ERR, "bind mounting notification socket failed");
>> +
>
> This is WAY over the top of what should go into OpenVPN code. Really.
>
> NAK on approach, NAK on code.
>
>
> Is there a way make sd_notify() behave like syslog()? That is, you call
> something like "openlog()" which will acquire the necessary file descriptor
> and then you can afterwards chroot() to your heart's content and do not
> need access to the actual socket file anymore (because openlog() will
> keep it around for syslog() to use).
First of all, bind mounts are fairly common in Linux these days -
especially when you start to try to chroot/containerize (including
docker)/"jail" and otherwise isolate applications. Chroots in general
are considered a bit too easy to escape from, unless privileges are
dropped at the right point. Namespaces and other container techniques
are often somewhat better, but still does not yet provide the full
isolation many expects. In this context bind mounts are often
considered simpler and to some degree safer than a full mount of /dev,
/proc, /sys, /run and so on.
Unfortunately, there exists no "openlog()" equivalent to my knowledge
for sd_notify() and other functions depending on the /run/systemd
directory. But it's a good idea, and in fact such a "prepare for
chroot" in libsystemd could probably do such a bind mount for the
caller. I have a good contact with one of the upstream systemd
developers and will discuss this with him ... so with time, we might
have something better.
Well, there is one feature systemd guys might throw at us instantly when
we open this topic ... PrivateNetwork=true in the unit file. But that
is useless for OpenVPN as then we will only have access to the lo
network device, all other network devices are unavailable.
> If sd_notify() cannot be taught to do that, either do what David proposed
> (disable chroot if running under systemd), or at least move that code
> out of init.c into something like platform.c.
That's a fair argument. As long as thelibsystemd shipped in RHEL7 does
not, to our knowledge, carry such a feature which we need; implementing
this functionality inside platform.c is reasonable.
What I am wondering then is if we should implement platform_chroot()
which is called from init.c. Where it will just call chroot() on
non-systemd and non-Windows systems. When systemd is enabled and
detected detected it will prepare the bind mount, drop capabilities and
then call chroot().
Is that a reasonable approach? Or would you prefer to have just the
bind mount stuff isolated into platform.c?
--
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/xeonphi___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH 1/1] bind mount systemd notification socket into chroot
Hi,
On Sat, Dec 10, 2016 at 12:19:07AM +0100, Christian Hesse wrote:
> + int fd;
> + char * chroot_notify = NULL;
> +
> + if (sd_notify(0, "READY=0") > 0)
> +{
> + asprintf(&chroot_notify, "%s/notify",
> c->options.chroot_dir);
> +
> + if (unshare(CLONE_NEWNS) != 0)
> +msg (M_ERR, "unshare failed");
> + if ((fd = open(chroot_notify, O_WRONLY | O_CREAT |
> O_TRUNC, 0644)) < 0)
> +msg (M_ERR, "touch failed");
> + close(fd);
> + if (mount(getenv("NOTIFY_SOCKET"), chroot_notify, NULL,
> MS_BIND, NULL) != 0)
> +msg (M_ERR, "bind mounting notification socket failed");
> +
This is WAY over the top of what should go into OpenVPN code. Really.
NAK on approach, NAK on code.
Is there a way make sd_notify() behave like syslog()? That is, you call
something like "openlog()" which will acquire the necessary file descriptor
and then you can afterwards chroot() to your heart's content and do not
need access to the actual socket file anymore (because openlog() will
keep it around for syslog() to use).
If sd_notify() cannot be taught to do that, either do what David proposed
(disable chroot if running under systemd), or at least move that code
out of init.c into something like platform.c.
gert
--
USENET is *not* the non-clickable part of WWW!
//www.muc.de/~gert/
Gert Doering - Munich, Germany g...@greenie.muc.de
fax: +49-89-35655025g...@net.informatik.tu-muenchen.de
signature.asc
Description: PGP 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/xeonphi___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH 1/1] bind mount systemd notification socket into chroot
David Sommerseth on Sat, 2016/12/10 01:03:
> On 10/12/16 00:19, Christian Hesse wrote:
> > From: Christian Hesse
> >
> > sd_notify() uses a socket to communicate with systemd. Communication
> > fails if the socket is not available within the chroot. So bind mount
> > the socket into the chroot when startet from systemd.
> >
> > Unsharing namespace and mounting requires extra capability
> > CAP_SYS_ADMIN.
>
> I will pick up this one after 2.4.0 has been released. This is a very
> promising approach. However, I'm not too happy about CAP_SYS_ADMIN
> though, that grants quite some privileges. Can we look at dropping this
> capability once we know we won't need it any more? Perhaps when we send
> READY=1?
Never tried to drop capabilities... Have to look into that.
We do no longer need CAP_SYS_ADMIN after the bind mount. (Or not at all
without chrooting.)
> > + char * chroot_notify = NULL;
> > +
> > + if (sd_notify(0, "READY=0") > 0)
> > +{
> > + asprintf(&chroot_notify, "%s/notify",
> > c->options.chroot_dir);
>
> Here we should use the buffer/string functions, based on the gc_arena
> implementation. Unfortunately we do not have a direct equivalent to
> asprintf(). A starting point would be to for example look at the string
> handling in print_sockaddr_ex() [socket.c:2386] or x_msg_va()
> [error.c:251] ... there might be better examples too, I'm just not able
> to remember them now :) buffer.[ch] keeps most of these functions.
>
> The reason for this is basically to use the same well tested
> infrastructure. And with gc_arena, only a single gc_free() is required,
> regardless of how many buffers you allocate to that arena.
I do not like this myself. The patch is just a proof of concept... So this
should be polished before committing. ;)
Thanks for the hints, I will have a look.
--
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);}
pgpdFAvSbXJm8.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/xeonphi___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH 1/1] add more security features for systemd units
SviMik on Sat, 2016/12/10 06:06:
> > You can break this with something like:
> >
> > status /etc/openvpn/client/status.log
> >
> > in your configuration. Writing a status file
> > to /run/openvpn-{client,server}/status.log works, though. So the default
> > setups should be fine. Do we have any more cases where openvpn wants write
> > access for whatever?
>
> From my configuration:
> 1) status
That is fine if it is written to /run/openvpn-{client,server}/. It breaks
with the status file in /etc/openvpn/{client,server}/ or example.
> 2) ifconfig-pool-persist
That is a problem... As the name suggests this should be persistent. :-/
> 3) tmp-dir (for storing openvpn_pf_*.tmp files)
Never used this. What is it for?
Anyway, I think this is not persistent stuff? Writing to /tmp/
or /run/openvpn-{client,server}/ should be fine.
> 4) client-connect script may want to write something
My scripts do some configuration and dbus-stuff, but do not write anything...
Writing to read-only path would fail, of course.
> 5) a plugin may want to write something
Same here... /run/ and /tmp/ is fine, other paths fail.
> For me even the read-only option will break nearly *everything*. And for
> user it will be completely not obvious why his scripts doesn't work, why
> his status file is not updated, and what's wrong with ifconfig-pool-persist.
Well, the error message should include something like: "cannot open file
'file': Read-only file system".
But this is more problematic than I thought initially.
--
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);}
pgpkdE7knZNap.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/xeonphi___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
