Re: [Openvpn-devel] [PATCH v2] Retain CAP_NET_ADMIN when dropping privileges

2022-03-31 Thread Gert Doering
Hi,

On Thu, Mar 31, 2022 at 04:38:06PM +0200, David Sommerseth wrote:
> We could "fix" --down now, but I will not recommend it at all.  We could 
> add the CAP_DAC_OVERRIDE capability.  But that's a massive sledge 
> hammer, giving read/write access to any file on the system. Only 
> security modules like SELinux, AppArmor and such can block access with 
> this capability enabled.  So this is definitely not the right capability 
> to have in the main OpenVPN process now.

I agree.  

This is not what I was suggesting (not at all), just pointing out that 
the combination of --up, --user and --down is not with its own set 
of surprises ;-)

gert

-- 
"If was one thing all people took for granted, was conviction that if you 
 feed honest figures into a computer, honest figures come out. Never doubted 
 it myself till I met a computer with a sense of humor."
 Robert A. Heinlein, The Moon is a Harsh Mistress

Gert Doering - Munich, Germany g...@greenie.muc.de


signature.asc
Description: PGP signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH v2] Retain CAP_NET_ADMIN when dropping privileges

2022-03-31 Thread David Sommerseth

On 31/03/2022 15:26, Gert Doering wrote:

Hi,

On Thu, Mar 31, 2022 at 03:20:59PM +0200, David Sommerseth wrote:

I've also run a few tests using an --up script which modified
/etc/resolv.conf, which also worked as expected with capabilities enabled.


This is actually an interesting corner case.  As far as I understand,
--up runs before setuid, so that should always succeed - but if you do
that, cleaning up resolv.conf in --down won't succeed.


That is actually correct, and to be honest I didn't think about the 
order of when running as client.


We could "fix" --down now, but I will not recommend it at all.  We could 
add the CAP_DAC_OVERRIDE capability.  But that's a massive sledge 
hammer, giving read/write access to any file on the system. Only 
security modules like SELinux, AppArmor and such can block access with 
this capability enabled.  So this is definitely not the right capability 
to have in the main OpenVPN process now.



--
kind regards,

David Sommerseth
OpenVPN Inc



OpenPGP_signature
Description: OpenPGP digital signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH v2] Retain CAP_NET_ADMIN when dropping privileges

2022-03-31 Thread Gert Doering
Hi,

On Thu, Mar 31, 2022 at 03:20:59PM +0200, David Sommerseth wrote:
> I've also run a few tests using an --up script which modified 
> /etc/resolv.conf, which also worked as expected with capabilities enabled.

This is actually an interesting corner case.  As far as I understand,
--up runs before setuid, so that should always succeed - but if you do
that, cleaning up resolv.conf in --down won't succeed.

(But this is a totally independent problem of "network things without
root" that this patch addresses)

[..]
> SELinux on Fedora and RHEL (which Rocky Linux inherits) denies the 
> OpenVPN process when run via systemd to use the SET_PCAP capability.  In 
> addition, the SELinux reference policy also denies all interactions with 
> the Generic Netlink interfaces used by ovpn-dco.  I will follow up this 
> with the upstream SELinux reference policy maintainers.

This is a good find.  Thanks :-)

gert
-- 
"If was one thing all people took for granted, was conviction that if you 
 feed honest figures into a computer, honest figures come out. Never doubted 
 it myself till I met a computer with a sense of humor."
 Robert A. Heinlein, The Moon is a Harsh Mistress

Gert Doering - Munich, Germany g...@greenie.muc.de


signature.asc
Description: PGP signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH v2] Retain CAP_NET_ADMIN when dropping privileges

2022-03-31 Thread David Sommerseth

On 30/03/2022 22:55, Timo Rothenpieler wrote:

---
Using libcap-ng now


  configure.ac  | 19 +
  distro/systemd/openvpn-cli...@.service.in |  2 +-
  distro/systemd/openvpn-ser...@.service.in |  2 +-
  src/openvpn/init.c| 25 ++-
  src/openvpn/platform.c| 91 +++
  src/openvpn/platform.h|  5 ++
  6 files changed, 140 insertions(+), 4 deletions(-)

Since I worked closely with Timo on this patch version, I don't feel I 
should give it an ACK verdict alone.  But I believe this is the right 
patch to include.


I will just suggest a commit message:

--
platform: Retain CAP_NET_ADMIN when dropping privileges

On Linux, when dropping privileges, interaction with the network 
configuration, such as tearing down routes or ovpn-dco interfaces

will fail when --user/--group are used.

This patch set sets the CAP_NET_ADMIN capability, which grants the 
needed privileges during the lifetime of the OpenVPN process when 
dropping root privileges.


Signed-off-by: Timo Rothenpieler 
Reviewed-By: David Sommerseth 
--


I have otherwise tested this patch on a Rocky Linux 8 distribution.
Client test cases I ran when testing this was:

  * from the command line, with and without DCO
  * via systemd, with and without DCO

With these 4 test cases, each of them were run with combinations of

  * no --user/--group
  * only --user
  * only --group
  * both --user and --group

I've also run a few tests using an --up script which modified 
/etc/resolv.conf, which also worked as expected with capabilities enabled.


There were no unexpected behavior with this final patch set, with one 
special exception which is outside the scope of this patch - SELinux.


SELinux on Fedora and RHEL (which Rocky Linux inherits) denies the 
OpenVPN process when run via systemd to use the SET_PCAP capability.  In 
addition, the SELinux reference policy also denies all interactions with 
the Generic Netlink interfaces used by ovpn-dco.  I will follow up this 
with the upstream SELinux reference policy maintainers.


Package maintainers needing SELinux can in the mean time, until an 
updated SELinux policy is available, provide an additional SELinux 
module which grants the needed privileges to openvpn_t labelled processes.



--
kind regards,

David Sommerseth
OpenVPN Inc



OpenPGP_signature
Description: OpenPGP digital signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH v2] Retain CAP_NET_ADMIN when dropping privileges

2022-03-31 Thread David Sommerseth

On 31/03/2022 13:34, Gert Doering wrote:

Hi,

On Thu, Mar 31, 2022 at 01:29:28PM +0200, Timo Rothenpieler wrote:

That's exactly what the patch does.


Which I very much like :-)  (I said that on IRC already, repeating here
for the list archive)


Only difference is that for sitnl, to avoid breaking existing setups, it
will fall back to the old approach of switching user if the capability
retaining approach failed.


I'm a bit undecided if this is really something to worry about... but
then, in an existing and working systemd environment with reduced
capabilities it might break setups going 2.5 -> 2.6, so maybe "being
careful about things" is the better way :-)


Yeah, I agree with this.  For v2.6, the time is too short to be dare too 
much potential breakage now.  But we can consider further steps with v2.7.



--
kind regards,

David Sommerseth
OpenVPN Inc



OpenPGP_signature
Description: OpenPGP digital signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH v2] Retain CAP_NET_ADMIN when dropping privileges

2022-03-31 Thread Gert Doering
Hi,

On Thu, Mar 31, 2022 at 01:29:28PM +0200, Timo Rothenpieler wrote:
> That's exactly what the patch does.

Which I very much like :-)  (I said that on IRC already, repeating here
for the list archive)

> Only difference is that for sitnl, to avoid breaking existing setups, it 
> will fall back to the old approach of switching user if the capability 
> retaining approach failed.

I'm a bit undecided if this is really something to worry about... but
then, in an existing and working systemd environment with reduced
capabilities it might break setups going 2.5 -> 2.6, so maybe "being
careful about things" is the better way :-)

gert
-- 
"If was one thing all people took for granted, was conviction that if you 
 feed honest figures into a computer, honest figures come out. Never doubted 
 it myself till I met a computer with a sense of humor."
 Robert A. Heinlein, The Moon is a Harsh Mistress

Gert Doering - Munich, Germany g...@greenie.muc.de


signature.asc
Description: PGP signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH v2] Retain CAP_NET_ADMIN when dropping privileges

2022-03-31 Thread Timo Rothenpieler

On 31.03.2022 13:02, Gert Doering wrote:

Hi,

On Thu, Mar 31, 2022 at 12:06:06PM +0200, David Sommerseth wrote:

There is however another related challenge in OpenVPN 2.x, which became
even clearer than be fore with the sitnl implementation we switched over
to on Linux by default with v2.5.  When using --user/--group without
--persist-tun, a reconnect would tear down the interface but could not
recover again and the connection dies.  Using --persist-tun, it could
work a bit better *unless* it needs to change the IP address of the tun
interface.  I'm not sure how well, OpenVPN 2.x works if new routes are
being pushed (OpenVPN 3 supports that as well).  This challenge is also
resolved by granting the process CAP_NET_ADMIN capabilities.


For most non-trivial stuff, OpenVPN with --user will run into problems,
be it route teardown, installing of new routes at renegotiation time,
...

So most people today just run 2.x as root, not getting any security
benefits.


For now, my opinion is that it is currently acceptable to have
CAP_NET_ADMIN available when running with ovpn-dco; to have a smooth
user experience.  OpenVPN is after all a network related process.


I'd even go for "keep CAP_NET_ADMIN for DCO and sitnl" - because it
means "all the route/interface manipulation *and cleanup* stuff can
be done properly, without having to carry root privileges".


That's exactly what the patch does.
Only difference is that for sitnl, to avoid breaking existing setups, it 
will fall back to the old approach of switching user if the capability 
retaining approach failed.



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH v2] Retain CAP_NET_ADMIN when dropping privileges

2022-03-31 Thread Gert Doering
Hi,

On Thu, Mar 31, 2022 at 12:06:06PM +0200, David Sommerseth wrote:
> There is however another related challenge in OpenVPN 2.x, which became 
> even clearer than be fore with the sitnl implementation we switched over 
> to on Linux by default with v2.5.  When using --user/--group without 
> --persist-tun, a reconnect would tear down the interface but could not 
> recover again and the connection dies.  Using --persist-tun, it could 
> work a bit better *unless* it needs to change the IP address of the tun 
> interface.  I'm not sure how well, OpenVPN 2.x works if new routes are 
> being pushed (OpenVPN 3 supports that as well).  This challenge is also 
> resolved by granting the process CAP_NET_ADMIN capabilities.

For most non-trivial stuff, OpenVPN with --user will run into problems,
be it route teardown, installing of new routes at renegotiation time,
...

So most people today just run 2.x as root, not getting any security 
benefits.

> For now, my opinion is that it is currently acceptable to have 
> CAP_NET_ADMIN available when running with ovpn-dco; to have a smooth 
> user experience.  OpenVPN is after all a network related process.

I'd even go for "keep CAP_NET_ADMIN for DCO and sitnl" - because it
means "all the route/interface manipulation *and cleanup* stuff can
be done properly, without having to carry root privileges".

> As a way forward after this, the aspect of how much to trust, 
> capabilities and privileges you put into a single process needs to be 
> better defined.  OpenVPN 2.x has a monolithic design, and the 
> architecture of privilege separation is lacking at best.

You might be surprised at what we have in 2.x :-) - with the service
pipe, we can run OpenVPN fully unprivileged, and do so on Windows.  

We just never had anyone bother to implement a backend for this for
"Unixy" platforms...

The benefit of that, securitywise, wouldn't be very large anyway,
compared to "CAP_NET_ADMIN + --user nobody" - the service is still
able to mess up routing and interface config, and that's about what
privileges remain in that combo... - so, dubious benefits, lots of
work.

gert
-- 
"If was one thing all people took for granted, was conviction that if you 
 feed honest figures into a computer, honest figures come out. Never doubted 
 it myself till I met a computer with a sense of humor."
 Robert A. Heinlein, The Moon is a Harsh Mistress

Gert Doering - Munich, Germany g...@greenie.muc.de


signature.asc
Description: PGP signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH v2] Retain CAP_NET_ADMIN when dropping privileges

2022-03-31 Thread Arne Schwabe


I am willing to work on making the netcfg service even less "OpenVPN 3 
centric", and it has a potential to grow towards a generic VPN API on 
Linux.  The current D-Bus interface it uses is highly inspired by the 
Android VPN API.  But this won't happen in a short time and not in time 
for the OpenVPN 2.6 release.  This is probably something which is more 
realistic for OpenVPN 2.8.  But this needs to be discussed more 
thoroughly (next hackathon?).


The current interface and interface design is ill suited for a server 
and I would even go so far as saying that extending the interface to 
support the full server functionality will go against the goal of the 
interface design.


A limited client mode might work but even that is something were you 
loose features that OpenVPN support in client mode. On platforms like 
ANdroid/iOS were you need to use an API like that you can hand wave that 
away as limitation of the platform but on Linux that is harder.


Also it is a very linux specific solution that will not work on macOS or 
Windows. So we might consider approaches as well.


Arne


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH v2] Retain CAP_NET_ADMIN when dropping privileges

2022-03-31 Thread David Sommerseth

On 31/03/2022 08:53, Jan Just Keijser wrote:

Hi,

On 30/03/22 22:55, Timo Rothenpieler wrote:

---
Using libcap-ng now
sorry to butt in late, but I've got a nasty feeling about this... the 
whole purpose of using

   --user
is, according to the man page
    --user user
   Change the user ID of the OpenVPN process to user after 
initialization,  dropping  privileges  in  the
   process.  This  option is useful to protect the system in 
the event that some hostile party was able to
   gain control of an OpenVPN session. Though OpenVPN's 
security features make this unlikely, it  is  pro‐

   vided as a second line of defense.

   By  setting  user  to  nobody or somebody similarly 
unprivileged, the hostile party would be limited in
   what damage they could cause. Of course once you take 
away privileges, you cannot  return  them  to an
   OpenVPN  session.  This  means, for example, that if you 
want to reset an OpenVPN daemon with a SIGUSR1
   signal (for example in response to a DHCP reset), you 
should make use of one or more of  the  --persist
   options  to  ensure  that OpenVPN doesn't need to execute 
any privileged operations in order to restart
   (such as re-reading key files or running ifconfig on the 
TUN device).


yet with this patch, the openvpn process remains capable of

    CAP_NET_ADMIN
   Perform various network-related operations:
   * interface configuration;
   * administration of IP firewall, masquerading, and
     accounting;
   * modify routing tables;
   * bind to any address for transparent proxying;
   * set type-of-service (TOS);
   * clear driver statistics;
   * set promiscuous mode;
   * enabling multicasting;
   * use setsockopt(2) to set the following socket options:
     SO_DEBUG, SO_MARK, SO_PRIORITY (for a priority outside
     the range 0 to 6), SO_RCVBUFFORCE, and SO_SNDBUFFORCE.

so this "second line of defense" it getting *VERY* leaky in my opinion 
(and warrants a manpage update, at the very least).


The proper solution would be to have openvpn fork on itself, keep a 
"barebones" process running as root, but with the actual control and 
data channels running in the forked process using truly minimal privileges.


Hi,

You have some valid points about locking down OpenVPN.  However, without 
this change and having OpenVPN run completely without privileges it will 
not work well in certain situations with ovpn-dco.  Because it will not 
have the capabilities needed to interact with the kernel module. 
CAP_NET_ADMIN does give this possibility.


In regards to man-page update, I agree.  And we should do that as a 
additional patch.


And when it comes to this patch, currently it only use the capabilities 
feature when DCO is available on the system and not disabled in the 
configuration.  This in options.c which calls 
dco_check_option_conflict(), which is implemented in dco.c.


There is however another related challenge in OpenVPN 2.x, which became 
even clearer than be fore with the sitnl implementation we switched over 
to on Linux by default with v2.5.  When using --user/--group without 
--persist-tun, a reconnect would tear down the interface but could not 
recover again and the connection dies.  Using --persist-tun, it could 
work a bit better *unless* it needs to change the IP address of the tun 
interface.  I'm not sure how well, OpenVPN 2.x works if new routes are 
being pushed (OpenVPN 3 supports that as well).  This challenge is also 
resolved by granting the process CAP_NET_ADMIN capabilities.


For now, my opinion is that it is currently acceptable to have 
CAP_NET_ADMIN available when running with ovpn-dco; to have a smooth 
user experience.  OpenVPN is after all a network related process.




As a way forward after this, the aspect of how much to trust, 
capabilities and privileges you put into a single process needs to be 
better defined.  OpenVPN 2.x has a monolithic design, and the 
architecture of privilege separation is lacking at best.


We have not done any attempts improving this, as this will not be a 
trivial refactoring.  With ovpn-dco, the data-channel handling is 
already handled outside of the master OpenVPN process; so here there is 
some improvements indirectly.  Now the control-channel handling + 
network configuration (with CAP_NET_ADMIN) runs in the same process scope.


Arne and I discussed a while back to look into if the Network 
Configuration service (openvpn3-service-netcfg) from OpenVPN 3 Linux can 
be implemented in a way where it can be used by OpenVPN 2.x as well. 
Theoretically, this is possible but not trivial.


In OpenVPN 3 Linux, privilege separation is part of the design.  The 
client process (openvpn3-serivce-client) runs completely 

Re: [Openvpn-devel] [PATCH v2] Retain CAP_NET_ADMIN when dropping privileges

2022-03-31 Thread Jan Just Keijser

Hi,

On 30/03/22 22:55, Timo Rothenpieler wrote:

---
Using libcap-ng now
sorry to butt in late, but I've got a nasty feeling about this... the 
whole purpose of using

  --user
is, according to the man page
   --user user
  Change the user ID of the OpenVPN process to user after  
initialization,  dropping  privileges  in  the
  process.  This  option is useful to protect the system in 
the event that some hostile party was able to
  gain control of an OpenVPN session. Though OpenVPN's 
security features make this unlikely, it  is  pro‐

  vided as a second line of defense.

  By  setting  user  to  nobody or somebody similarly 
unprivileged, the hostile party would be limited in
  what damage they could cause. Of course once you take 
away privileges, you cannot  return  them  to an
  OpenVPN  session.  This  means, for example, that if you 
want to reset an OpenVPN daemon with a SIGUSR1
  signal (for example in response to a DHCP reset), you 
should make use of one or more of  the  --persist
  options  to  ensure  that OpenVPN doesn't need to execute 
any privileged operations in order to restart
  (such as re-reading key files or running ifconfig on the 
TUN device).


yet with this patch, the openvpn process remains capable of

   CAP_NET_ADMIN
  Perform various network-related operations:
  * interface configuration;
  * administration of IP firewall, masquerading, and
    accounting;
  * modify routing tables;
  * bind to any address for transparent proxying;
  * set type-of-service (TOS);
  * clear driver statistics;
  * set promiscuous mode;
  * enabling multicasting;
  * use setsockopt(2) to set the following socket options:
    SO_DEBUG, SO_MARK, SO_PRIORITY (for a priority outside
    the range 0 to 6), SO_RCVBUFFORCE, and SO_SNDBUFFORCE.

so this "second line of defense" it getting *VERY* leaky in my opinion 
(and warrants a manpage update, at the very least).


The proper solution would be to have openvpn fork on itself, keep a 
"barebones" process running as root, but with the actual control and 
data channels running in the forked process using truly minimal privileges.


JM2CW,

JJK




  configure.ac  | 19 +
  distro/systemd/openvpn-cli...@.service.in |  2 +-
  distro/systemd/openvpn-ser...@.service.in |  2 +-
  src/openvpn/init.c| 25 ++-
  src/openvpn/platform.c| 91 +++
  src/openvpn/platform.h|  5 ++
  6 files changed, 140 insertions(+), 4 deletions(-)

diff --git a/configure.ac b/configure.ac
index 7199483a..168360d4 100644
--- a/configure.ac
+++ b/configure.ac
@@ -794,6 +794,25 @@ dnl
esac
  fi
  
+dnl

+dnl Depend on libcap-ng on Linux
+dnl
+case "$host" in
+   *-*-linux*)
+   PKG_CHECK_MODULES([LIBCAPNG],
+ [libcap-ng],
+ [have_libcapng="yes"],
+ [AC_MSG_ERROR([libcap-ng package not found. 
Is the development package and pkg-config installed?])]
+   )
+   AC_CHECK_HEADER([sys/prctl.h],,[AC_MSG_ERROR([sys/prctl.h not 
found!])])
+
+   CFLAGS="${CFLAGS} ${LIBCAPNG_CFALGS}"
+   LIBS="${LIBS} ${LIBCAPNG_LIBS}"
+   AC_DEFINE(HAVE_LIBCAPNG, 1, [Enable libcap-ng support])
+   ;;
+esac
+
+
  if test "${with_crypto_library}" = "openssl"; then
AC_ARG_VAR([OPENSSL_CFLAGS], [C compiler flags for OpenSSL])
AC_ARG_VAR([OPENSSL_LIBS], [linker flags for OpenSSL])
diff --git a/distro/systemd/openvpn-cli...@.service.in 
b/distro/systemd/openvpn-cli...@.service.in
index cbcef653..159fb4dc 100644
--- a/distro/systemd/openvpn-cli...@.service.in
+++ b/distro/systemd/openvpn-cli...@.service.in
@@ -11,7 +11,7 @@ Type=notify
  PrivateTmp=true
  WorkingDirectory=/etc/openvpn/client
  ExecStart=@sbindir@/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
+CapabilityBoundingSet=CAP_IPC_LOCK CAP_NET_ADMIN CAP_NET_RAW CAP_SETGID 
CAP_SETUID CAP_SETPCAP CAP_SYS_CHROOT CAP_DAC_OVERRIDE
  LimitNPROC=10
  DeviceAllow=/dev/null rw
  DeviceAllow=/dev/net/tun rw
diff --git a/distro/systemd/openvpn-ser...@.service.in 
b/distro/systemd/openvpn-ser...@.service.in
index d1cc72cb..6e8e7d94 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