diff mbox

[ovs-dev] bridge: allow OVS to connect to Unix Domain Sockets outside its run directory

Message ID 1464922053-30254-1-git-send-email-aatteka@ovn.org
State Not Applicable
Headers show

Commit Message

Ansis Atteka June 3, 2016, 2:47 a.m. UTC
Before this patch OVS refused to connect to a local controller that
had its Unix Domain Socket outside Open vSwitch run directory (e.g.
outside '/var/run/openvswitch/').

After this patch this restriction imposed by Open vSwitch itself is
abandoned and OVS should be able to connect to controller's Unix Domain
Sockets anywhere under filesystem.

Note, that, if process is running under 'root' user and is acting
as client, then it is effectively bypassing all access control
restrictions imposed by Unix Discretionary Access Control (because
root does not care who owns UNIX domain socket).  The security precautions
that should be taken into account after this patch are that directory under
which controller will create its server socket and OVS will be told
to connect should not be:
1. writable by "everyone" (i.e. o+w); OR
2. writable by "group" (g+w) to which untrusted user belongs; OR
3. owned by untrusted "user".
Otherwise, a malicious process could create its Unix Domain Socket and
trick Open vSwitch to connect to it.  Nevertheless, this should not be
a big concern, because the same issue is already present in TCP
mode (e.g. tcp:127.0.0.1:6632) that would not obey any Unix Discretionary
Access restrictions anyway.

VMware-BZ: #1525857
---
 lib/vconn-active.man |  3 +++
 vswitchd/bridge.c    | 61 ++++++++++++++++------------------------------------
 2 files changed, 22 insertions(+), 42 deletions(-)

Comments

Ben Pfaff June 8, 2016, 9:02 p.m. UTC | #1
On Thu, Jun 02, 2016 at 07:47:33PM -0700, Ansis Atteka wrote:
> Before this patch OVS refused to connect to a local controller that
> had its Unix Domain Socket outside Open vSwitch run directory (e.g.
> outside '/var/run/openvswitch/').
> 
> After this patch this restriction imposed by Open vSwitch itself is
> abandoned and OVS should be able to connect to controller's Unix Domain
> Sockets anywhere under filesystem.

When I run "netstat -lnx" on my laptop, I see a bunch of listening Unix
domain sockets.

Some of these listening sockets are security sensitive, such as SSH
agents, so it wouldn't be good to have a remote manager be able to point
OVS to them: what if a clever person could figure out how to send
arbitrary data to them (maybe in a packet-in message somehow?) via
OpenFlow.  Other examples are dbus and udev sockets.

That's my main worry here.
Ansis June 8, 2016, 11:45 p.m. UTC | #2
On 8 June 2016 at 14:02, Ben Pfaff <blp@ovn.org> wrote:

> On Thu, Jun 02, 2016 at 07:47:33PM -0700, Ansis Atteka wrote:
> > Before this patch OVS refused to connect to a local controller that
> > had its Unix Domain Socket outside Open vSwitch run directory (e.g.
> > outside '/var/run/openvswitch/').
> >
> > After this patch this restriction imposed by Open vSwitch itself is
> > abandoned and OVS should be able to connect to controller's Unix Domain
> > Sockets anywhere under filesystem.
>
> When I run "netstat -lnx" on my laptop, I see a bunch of listening Unix
> domain sockets.
>

> Some of these listening sockets are security sensitive, such as SSH
> agents, so it wouldn't be good to have a remote manager be able to point
> OVS to them: what if a clever person could figure out how to send
> arbitrary data to them (maybe in a packet-in message somehow?) via
> OpenFlow.  Other examples are dbus and udev sockets.
>
> That's my main worry here.
>

Ok, I see your concern now. At least with SELinux enabled such attacks
would fail on default Fedora, RHEL and CentOS:

[root@localhost ovs]# tail /var/log/openvswitch/ovs-vswitchd.log
2016-06-08T22:51:48.215Z|00033|connmgr|INFO|brz: added service controller
"punix://var/run/openvswitch/brz.mgmt"
2016-06-08T22:51:48.215Z|00034|bridge|INFO|bridge brs: using datapath ID
0000e6d0bb7b3047
2016-06-08T22:51:48.215Z|00035|connmgr|INFO|brs: added service controller
"punix://var/run/openvswitch/brs.mgmt"
2016-06-08T22:51:48.220Z|00036|bridge|INFO|ovs-vswitchd (Open vSwitch)
2.5.90
2016-06-08T22:51:48.665Z|00037|rconn|INFO|brX<->unix:/run/udev/control:
connecting...
2016-06-08T22:51:48.665Z|00038|rconn|WARN|brX<->unix:/run/udev/control:
connection failed (Permission denied)
2016-06-08T22:51:48.665Z|00039|rconn|INFO|brX<->unix:/run/udev/control:
waiting 2 seconds before reconnect
2016-06-08T22:51:50.666Z|00040|rconn|INFO|brX<->unix:/run/udev/control:
connecting...
2016-06-08T22:51:50.666Z|00041|rconn|WARN|brX<->unix:/run/udev/control:
connection failed (Permission denied)

because Open vSwitch runs under openvswitch_t domain:

system_u:system_r:openvswitch_t:s0 root  16567 16566  0 66914 36460   0
15:51 ?        00:00:00 ovs-vswitchd unix:/var/run/openvswitch/db.sock
-vconsole:emer -vsyslog:err -vfile:info --mlockall --no-chdir
--log-file=/var/log/openvswitch/ovs-vswitchd.log
--pidfile=/var/run/openvswitch/ovs-vswitchd.pid --detach --monitor

and OVS is unable to connect sockets other than those that have type
openvswitch*_t type:

srwxr-x---. 1 root root system_u:object_r:openvswitch_var_run_t:s0 0 Jun  8
15:51 /var/run/openvswitch/brT.mgmt

For example, udev control socket and others use udev_var_run_t type:

srw-------. 1 root root system_u:object_r:udev_var_run_t:s0 0 Jun  2 15:43
/run/udev/control


So the problem my patch is trying to solve is that there could be over
confined processes trying

Here are the options I see:
1. abandon my patch and tell everyone to figure out on their own on how to
connect to Open vSwitch sockets under /var/run/openvswitch/ directory (e.g.
by changing group of /var/run/openvswitch or by opening socket while their
processes are running under root user and only after that they change the
user).
2. abandon my patch, but implement something similar to Aaron's DPDK socket
chown()/chmod() patch and tell others to use it.
3. abandon my patch, but implement some kind of access control inside
OVSDB. For example, the roles could be "local admin" and "remote
controller". "remote controller" should have limited access to certain
database files. This is non-trivial, but may be something to consider
long-term because OVSDB currently contains a lot of things to which access
is granted on all-or-nothing basis.
4. move forward with my patch, but allow to dynamically specify whitelist.
Obviously this whitelist must not be configurable through OVSDB because
then it loses its purpose.
5. move forward with my patch, but use Mandatory Access Control to specify
whitelist of paths to which OVS can connect to (this makes assumption that
MAC - SELinux or AppArmor - is up and running).

Unfortunately there is no silver bullet for this problem.
Ansis June 9, 2016, 12:17 a.m. UTC | #3
On 8 June 2016 at 16:45, Ansis Atteka <ansisatteka@gmail.com> wrote:

>
>
> On 8 June 2016 at 14:02, Ben Pfaff <blp@ovn.org> wrote:
>
>> On Thu, Jun 02, 2016 at 07:47:33PM -0700, Ansis Atteka wrote:
>> > Before this patch OVS refused to connect to a local controller that
>> > had its Unix Domain Socket outside Open vSwitch run directory (e.g.
>> > outside '/var/run/openvswitch/').
>> >
>> > After this patch this restriction imposed by Open vSwitch itself is
>> > abandoned and OVS should be able to connect to controller's Unix Domain
>> > Sockets anywhere under filesystem.
>>
>> When I run "netstat -lnx" on my laptop, I see a bunch of listening Unix
>> domain sockets.
>>
>
>> Some of these listening sockets are security sensitive, such as SSH
>> agents, so it wouldn't be good to have a remote manager be able to point
>> OVS to them: what if a clever person could figure out how to send
>> arbitrary data to them (maybe in a packet-in message somehow?) via
>> OpenFlow.  Other examples are dbus and udev sockets.
>>
>> That's my main worry here.
>>
>
> Ok, I see your concern now. At least with SELinux enabled such attacks
> would fail on default Fedora, RHEL and CentOS:
>
> [root@localhost ovs]# tail /var/log/openvswitch/ovs-vswitchd.log
> 2016-06-08T22:51:48.215Z|00033|connmgr|INFO|brz: added service controller
> "punix://var/run/openvswitch/brz.mgmt"
> 2016-06-08T22:51:48.215Z|00034|bridge|INFO|bridge brs: using datapath ID
> 0000e6d0bb7b3047
> 2016-06-08T22:51:48.215Z|00035|connmgr|INFO|brs: added service controller
> "punix://var/run/openvswitch/brs.mgmt"
> 2016-06-08T22:51:48.220Z|00036|bridge|INFO|ovs-vswitchd (Open vSwitch)
> 2.5.90
> 2016-06-08T22:51:48.665Z|00037|rconn|INFO|brX<->unix:/run/udev/control:
> connecting...
> 2016-06-08T22:51:48.665Z|00038|rconn|WARN|brX<->unix:/run/udev/control:
> connection failed (Permission denied)
> 2016-06-08T22:51:48.665Z|00039|rconn|INFO|brX<->unix:/run/udev/control:
> waiting 2 seconds before reconnect
> 2016-06-08T22:51:50.666Z|00040|rconn|INFO|brX<->unix:/run/udev/control:
> connecting...
> 2016-06-08T22:51:50.666Z|00041|rconn|WARN|brX<->unix:/run/udev/control:
> connection failed (Permission denied)
>
> because Open vSwitch runs under openvswitch_t domain:
>
> system_u:system_r:openvswitch_t:s0 root  16567 16566  0 66914 36460   0
> 15:51 ?        00:00:00 ovs-vswitchd unix:/var/run/openvswitch/db.sock
> -vconsole:emer -vsyslog:err -vfile:info --mlockall --no-chdir
> --log-file=/var/log/openvswitch/ovs-vswitchd.log
> --pidfile=/var/run/openvswitch/ovs-vswitchd.pid --detach --monitor
>
> and OVS is unable to connect sockets other than those that have type
> openvswitch*_t type:
>
> srwxr-x---. 1 root root system_u:object_r:openvswitch_var_run_t:s0 0 Jun
>  8 15:51 /var/run/openvswitch/brT.mgmt
>
> For example, udev control socket and others use udev_var_run_t type:
>
> srw-------. 1 root root system_u:object_r:udev_var_run_t:s0 0 Jun  2 15:43
> /run/udev/control
>
>
> So the problem my patch is trying to solve is that there could be over
> confined processes trying
>
Just noticed that my sentence was abruptly cut:

"So the problem my patch is trying to solve is that there could be
over-confined processes trying to connect to OVS but they can't do that
because /var/run/openvswitch and its contents are owned by root:root."


>
> Here are the options I see:
> 1. abandon my patch and tell everyone to figure out on their own on how to
> connect to Open vSwitch sockets under /var/run/openvswitch/ directory (e.g.
> by changing group of /var/run/openvswitch or by opening socket while their
> processes are running under root user and only after that they change the
> user).
> 2. abandon my patch, but implement something similar to Aaron's DPDK
> socket chown()/chmod() patch and tell others to use it.
> 3. abandon my patch, but implement some kind of access control inside
> OVSDB. For example, the roles could be "local admin" and "remote
> controller". "remote controller" should have limited access to certain
> database files. This is non-
>
s/database files/database tables or columns/

> trivial, but may be something to consider long-term because OVSDB
> currently contains a lot of things to which access is granted on
> all-or-nothing basis.
> 4. move forward with my patch, but allow to dynamically specify whitelist.
> Obviously this whitelist must not be configurable through OVSDB because
> then it loses its purpose.
> 5. move forward with my patch, but use Mandatory Access Control to specify
> whitelist of paths to which OVS can connect to (this makes assumption that
> MAC - SELinux or AppArmor - is up and running).
>
> Unfortunately there is no silver bullet for this problem.
>
> Actually, there may be a 6th solution:
6. move forward with my patch and hope that OpenFlow handshake would make
the other side to close the socket early. This is still not that good as
proper access control.
Ansis June 20, 2016, 9:28 p.m. UTC | #4
On 8 June 2016 at 17:17, Ansis Atteka <ansisatteka@gmail.com> wrote:

>
>
> On 8 June 2016 at 16:45, Ansis Atteka <ansisatteka@gmail.com> wrote:
>
>>
>>
>> On 8 June 2016 at 14:02, Ben Pfaff <blp@ovn.org> wrote:
>>
>>> On Thu, Jun 02, 2016 at 07:47:33PM -0700, Ansis Atteka wrote:
>>> > Before this patch OVS refused to connect to a local controller that
>>> > had its Unix Domain Socket outside Open vSwitch run directory (e.g.
>>> > outside '/var/run/openvswitch/').
>>> >
>>> > After this patch this restriction imposed by Open vSwitch itself is
>>> > abandoned and OVS should be able to connect to controller's Unix Domain
>>> > Sockets anywhere under filesystem.
>>>
>>> When I run "netstat -lnx" on my laptop, I see a bunch of listening Unix
>>> domain sockets.
>>>
>>
>>> Some of these listening sockets are security sensitive, such as SSH
>>> agents, so it wouldn't be good to have a remote manager be able to point
>>> OVS to them: what if a clever person could figure out how to send
>>> arbitrary data to them (maybe in a packet-in message somehow?) via
>>> OpenFlow.  Other examples are dbus and udev sockets.
>>>
>>> That's my main worry here.
>>>
>>
>> Ok, I see your concern now. At least with SELinux enabled such attacks
>> would fail on default Fedora, RHEL and CentOS:
>>
>> [root@localhost ovs]# tail /var/log/openvswitch/ovs-vswitchd.log
>> 2016-06-08T22:51:48.215Z|00033|connmgr|INFO|brz: added service controller
>> "punix://var/run/openvswitch/brz.mgmt"
>> 2016-06-08T22:51:48.215Z|00034|bridge|INFO|bridge brs: using datapath ID
>> 0000e6d0bb7b3047
>> 2016-06-08T22:51:48.215Z|00035|connmgr|INFO|brs: added service controller
>> "punix://var/run/openvswitch/brs.mgmt"
>> 2016-06-08T22:51:48.220Z|00036|bridge|INFO|ovs-vswitchd (Open vSwitch)
>> 2.5.90
>> 2016-06-08T22:51:48.665Z|00037|rconn|INFO|brX<->unix:/run/udev/control:
>> connecting...
>> 2016-06-08T22:51:48.665Z|00038|rconn|WARN|brX<->unix:/run/udev/control:
>> connection failed (Permission denied)
>> 2016-06-08T22:51:48.665Z|00039|rconn|INFO|brX<->unix:/run/udev/control:
>> waiting 2 seconds before reconnect
>> 2016-06-08T22:51:50.666Z|00040|rconn|INFO|brX<->unix:/run/udev/control:
>> connecting...
>> 2016-06-08T22:51:50.666Z|00041|rconn|WARN|brX<->unix:/run/udev/control:
>> connection failed (Permission denied)
>>
>> because Open vSwitch runs under openvswitch_t domain:
>>
>> system_u:system_r:openvswitch_t:s0 root  16567 16566  0 66914 36460   0
>> 15:51 ?        00:00:00 ovs-vswitchd unix:/var/run/openvswitch/db.sock
>> -vconsole:emer -vsyslog:err -vfile:info --mlockall --no-chdir
>> --log-file=/var/log/openvswitch/ovs-vswitchd.log
>> --pidfile=/var/run/openvswitch/ovs-vswitchd.pid --detach --monitor
>>
>> and OVS is unable to connect sockets other than those that have type
>> openvswitch*_t type:
>>
>> srwxr-x---. 1 root root system_u:object_r:openvswitch_var_run_t:s0 0 Jun
>>  8 15:51 /var/run/openvswitch/brT.mgmt
>>
>> For example, udev control socket and others use udev_var_run_t type:
>>
>> srw-------. 1 root root system_u:object_r:udev_var_run_t:s0 0 Jun  2
>> 15:43 /run/udev/control
>>
>>
>> So the problem my patch is trying to solve is that there could be over
>> confined processes trying
>>
> Just noticed that my sentence was abruptly cut:
>
> "So the problem my patch is trying to solve is that there could be
> over-confined processes trying to connect to OVS but they can't do that
> because /var/run/openvswitch and its contents are owned by root:root."
>
>
>>
>> Here are the options I see:
>> 1. abandon my patch and tell everyone to figure out on their own on how
>> to connect to Open vSwitch sockets under /var/run/openvswitch/ directory
>> (e.g. by changing group of /var/run/openvswitch or by opening socket while
>> their processes are running under root user and only after that they change
>> the user).
>> 2. abandon my patch, but implement something similar to Aaron's DPDK
>> socket chown()/chmod() patch and tell others to use it.
>> 3. abandon my patch, but implement some kind of access control inside
>> OVSDB. For example, the roles could be "local admin" and "remote
>> controller". "remote controller" should have limited access to certain
>> database files. This is non-
>>
> s/database files/database tables or columns/
>
>> trivial, but may be something to consider long-term because OVSDB
>> currently contains a lot of things to which access is granted on
>> all-or-nothing basis.
>> 4. move forward with my patch, but allow to dynamically specify
>> whitelist. Obviously this whitelist must not be configurable through OVSDB
>> because then it loses its purpose.
>> 5. move forward with my patch, but use Mandatory Access Control to
>> specify whitelist of paths to which OVS can connect to (this makes
>> assumption that MAC - SELinux or AppArmor - is up and running).
>>
>> Unfortunately there is no silver bullet for this problem.
>>
>> Actually, there may be a 6th solution:
> 6. move forward with my patch and hope that OpenFlow handshake would make
> the other side to close the socket early. This is still not that good as
> proper access control.
>


This patch can be abandoned. I sent a new one with slightly different
approach: http://openvswitch.org/pipermail/dev/2016-June/073211.html

Basically it leaves the default behavior unchanged, but if one is running
under SELinux or Apparmor, then he can disable the Self-confinement feature
in OVS through /etc/default/openvswitch file.
diff mbox

Patch

diff --git a/lib/vconn-active.man b/lib/vconn-active.man
index 252438d..6b5fce9 100644
--- a/lib/vconn-active.man
+++ b/lib/vconn-active.man
@@ -10,5 +10,8 @@  If \fIport\fR is not specified, it defaults to 6653.
 .TP
 \fBunix:\fIfile\fR
 On POSIX, a Unix domain server socket named \fIfile\fR.
+For best security practices ensure that directory under
+which \fIfile\fR resides is accessible only for trusted
+users (as minimum o+w should not be set).
 .IP
 On Windows, a localhost TCP port written in \fIfile\fR.
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 41ec4ba..72947b3 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -3558,49 +3558,26 @@  bridge_configure_remotes(struct bridge *br,
     for (i = 0; i < n_controllers; i++) {
         struct ovsrec_controller *c = controllers[i];
 
-        if (!strncmp(c->target, "punix:", 6)
-            || !strncmp(c->target, "unix:", 5)) {
+        if (!strncmp(c->target, "punix:", 6)) {
             static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
-            char *whitelist;
-
-            if (!strncmp(c->target, "unix:", 5)) {
-                /* Connect to a listening socket */
-                whitelist = xasprintf("unix:%s/", ovs_rundir());
-                if (strchr(c->target, '/') &&
-                   !equal_pathnames(c->target, whitelist,
-                     strlen(whitelist))) {
-                    /* Absolute path specified, but not in ovs_rundir */
-                    VLOG_ERR_RL(&rl, "bridge %s: Not connecting to socket "
-                                  "controller \"%s\" due to possibility for "
-                                  "remote exploit.  Instead, specify socket "
-                                  "in whitelisted \"%s\" or connect to "
-                                  "\"unix:%s/%s.mgmt\" (which is always "
-                                  "available without special configuration).",
-                                  br->name, c->target, whitelist,
-                                  ovs_rundir(), br->name);
-                    free(whitelist);
-                    continue;
-                }
-            } else {
-               whitelist = xasprintf("punix:%s/%s.",
-                                     ovs_rundir(), br->name);
-               if (!equal_pathnames(c->target, whitelist, strlen(whitelist))
-                   || strchr(c->target + strlen(whitelist), '/')) {
-                   /* Prevent remote ovsdb-server users from accessing
-                    * arbitrary Unix domain sockets and overwriting arbitrary
-                    * local files. */
-                   VLOG_ERR_RL(&rl, "bridge %s: Not adding Unix domain socket "
-                                  "controller \"%s\" due to possibility of "
-                                  "overwriting local files. Instead, specify "
-                                  "path in whitelisted format \"%s*\" or "
-                                  "connect to \"unix:%s/%s.mgmt\" (which is "
-                                  "always available without special "
-                                  "configuration).",
-                                  br->name, c->target, whitelist,
-                                  ovs_rundir(), br->name);
-                   free(whitelist);
-                   continue;
-               }
+            char *whitelist = xasprintf("punix:%s/%s.", ovs_rundir(),
+                                        br->name);
+            if (!equal_pathnames(c->target, whitelist, strlen(whitelist))
+                || strchr(c->target + strlen(whitelist), '/')) {
+                /* Prevent remote ovsdb-server users from accessing
+                 * arbitrary Unix domain sockets and overwriting arbitrary
+                 * local files. */
+                VLOG_ERR_RL(&rl, "bridge %s: Not adding Unix domain socket "
+                            "controller \"%s\" due to possibility of "
+                            "overwriting local files. Instead, specify "
+                            "path in whitelisted format \"%s*\" or "
+                            "connect to \"unix:%s/%s.mgmt\" (which is "
+                            "always available without special "
+                            "configuration).",
+                            br->name, c->target, whitelist,
+                            ovs_rundir(), br->name);
+                free(whitelist);
+                continue;
             }
 
             free(whitelist);