[Openvpn-devel] [PATCH 1/2] systemd: extend CapabilityBoundingSet for auth_pam

2018-08-30 Thread David Sommerseth
On 29/08/18 16:27, Christian Ehrhardt wrote:
> Auth_pam will require audit writes or the connection will be rejected
> as the plugin fails to initialize like:
>   openvpn[]: sudo: unable to send audit message
>   openvpn[]: sudo: pam_open_session: System error
>   openvpn[]: sudo: policy plugin failed session initialization
> 
> See links from https://community.openvpn.net/openvpn/ticket/918 for
> more.
> 
> auth_pam is a common use case and capabilties for it should be allowed
> by the .service file.
> 
> Fixes: #918
> 
> Signed-off-by: Christian Ehrhardt 
> ---
>  distro/systemd/openvpn-ser...@.service.in | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/distro/systemd/openvpn-ser...@.service.in 
> b/distro/systemd/openvpn-ser...@.service.in
> index a8366a04..d1cc72cb 100644
> --- a/distro/systemd/openvpn-ser...@.service.in
> +++ b/distro/systemd/openvpn-ser...@.service.in
> @@ -11,7 +11,7 @@ Type=notify
>  PrivateTmp=true
>  WorkingDirectory=/etc/openvpn/server
>  ExecStart=@sbindir@/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
> +CapabilityBoundingSet=CAP_IPC_LOCK CAP_NET_ADMIN CAP_NET_BIND_SERVICE 
> CAP_NET_RAW CAP_SETGID CAP_SETUID CAP_SYS_CHROOT CAP_DAC_OVERRIDE 
> CAP_AUDIT_WRITE

CAP_AUDIT_WRITE sounds safe to add.  But I really need to get a better
understanding *why* this is needed, when OpenVPN itself don't need this. What
is it in the PAM code paths which triggers this requirement and why?

There might be perfect valid reasons, but we can't just blindly jump into
"Yes, we need it" without a good understanding of why.

I have run tests on RHEL-7, Fedora 28 and some older Debian 8-9-ish-sid
release.  I only stumble upon this issue on Debian.  So what is it Debian (and
thus Ubuntu) does which causes this error?

I did a little search the PAM error which occurs (audit_log_acct_message()
failed: Operation not permitted), and I could find a similar error in Fedora 8
(which is from 2007-2008).  But from what I can grasp, this doesn't sound
directly related to this issue we're seeing here.  And this was around PAM
version 0.99.

My Debian test VM uses pam-1.1.8-3.6, RHEL-7 pam-1.1.8-22 and Fedora 28
pam-1.3.1-1.

Since both my Debian VM and my RHEL-7 install uses essentially quite similar
PAM releases  Debian must be doing something different ... but what?  I
even verified that all distros are compiled with libaudit, and they are.

Anyone got a clue?

-- 
kind regards,

David Sommerseth
OpenVPN 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] Fix memory leak after sighup

2018-08-30 Thread Arne Schwabe
Am 29.08.18 um 15:49 schrieb Steffan Karger:
> The c.es env_set is (re)allocated for each "sighup loop iteration", while
> it was free'd only once at process shutdown.  Move the env_set_destroy()
> call to match the same level as the env_set_create() call to fix that.
> 

Acked-by: Arne Schwabe 

This like the right fix and even though the leak is small we should fix
it nevertheless.

Arne

--
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] mbedtls: print warning if random personalisation fails

2018-08-30 Thread Antonio Quartulli


On 29/08/18 20:04, Steffan Karger wrote:
> ... instead of when it doesn't fail.  Looks like 'someone' mixed up the
> mbedtls return style (0 means success) with the openvpn internal return
> style (true means success).
> 
> Signed-off-by: Steffan Karger 

Acked-by: Antonio Quartulli 

I guess at some point we should also convert all these functions
returning int to bool, because the latter is the meaning we are giving
to the return value.


Cheers,

-- 
Antonio Quartulli



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 2/2] systemd: extend CapabilityBoundingSet for learn-address

2018-08-30 Thread Christian Ehrhardt
On Thu, Aug 30, 2018 at 1:38 AM David Sommerseth <
open...@sf.lists.topphemmelig.net> wrote:

> On 29/08/18 21:05, Christian Hesse wrote:
> > Christian Ehrhardt  on Wed, 2018/08/29
> > 16:27:
> >> It seems a not too uncommon case that learn-address needs to recycle
> >> dnsmasq - to do so it would need CAP_KILL.
> >>
> >> This was suggested on https://community.openvpn.net/openvpn/ticket/918
> >>
> >> Signed-off-by: Christian Ehrhardt 
> >> ---
> >>  distro/systemd/openvpn-ser...@.service.in | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/distro/systemd/openvpn-ser...@.service.in
> >> b/distro/systemd/openvpn-ser...@.service.in index d1cc72cb..edace213
> 100644
> >> --- a/distro/systemd/openvpn-ser...@.service.in
> >> +++ b/distro/systemd/openvpn-ser...@.service.in
> >> @@ -11,7 +11,7 @@ Type=notify
> >>  PrivateTmp=true
> >>  WorkingDirectory=/etc/openvpn/server
> >>  ExecStart=@sbindir@/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
> >> CAP_AUDIT_WRITE +CapabilityBoundingSet=CAP_IPC_LOCK CAP_NET_ADMIN
> >> CAP_NET_BIND_SERVICE CAP_NET_RAW CAP_SETGID CAP_SETUID CAP_SYS_CHROOT
> >> CAP_DAC_OVERRIDE CAP_AUDIT_WRITE CAP_KILL LimitNPROC=10
> >> DeviceAllow=/dev/null rw DeviceAllow=/dev/net/tun rw
> >
> > I do not like services being allowed to send signals to other processes.
> As
> > dnsmasq supports a dbus interface... How about using that? For example to
> > clear the dns cache of an instance started from Networkmanager:
> >
> > dbus-send --system --print-reply \
> > --dest=org.freedesktop.NetworkManager.dnsmasq /uk/org/thekelleys/dnsmasq
> \
> > uk.org.thekelleys.ClearCache
>
> +1 ... CAP_KILL privileges can too easily prepare the ground for DoS
> attacks.
>
> The D-Bus approach above seems much saner and safer.  Also because D-Bus
> gives
> a reasonable protection in regards to privilege escalation attacks.  But
> you
> most likely need to prepare a D-Bus policy for dnsmasq though, to allow the
> openvpn user (or whatever user who will execute this script) access to the
> uk.org.thekelleys.ClearCache D-Bus method.
>

I don't mind the KILL signal so much we can keep that off for another
discussion.
I like the suggestion if the dbus signal, clearly worth a try for those
with a matching setup.

After all my own thought of "umm KILL might be too much" is why I have
split it :-)

What bug 918 was originally about and would have to be cleared soon is the
CAP_AUDIT_WRITE.

So while we seem to agree we don't want/like CAP_KILL, could we add
CAP_AUDIT_WRITE as submitted?


> --
> kind regards,
>
> David Sommerseth
> OpenVPN Inc
>
>
>

-- 
Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd
--
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