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(-)

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);
-- 
2.7.4

_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to