[ovs-dev] [PATCH V2] service controller: Allow change to punix socket file group ownership.

Alex Wang ee07b291 at gmail.com
Sun Aug 30 06:11:24 UTC 2015


This commit adds a new key-value pair, 'punix_file_group=<user group>',
to the 'other_config' column in the 'Controller' table.  This new config
allows user to change the punix socket file's group ownership, so that
non-root process can also connect to ovs bridge.

Signed-off-by: Alex Wang <ee07b291 at gmail.com>
---
PATCH->V2:
- drop sysconf(_SC_GETGR_R_SIZE_MAX) and use 256 as buffer size.
- do not rely on errno = 0 when getgrnam_r() cannot find the group.
- do not assume there is a 'root' group, use gid 0 as default.
- fix the vlog format.
---
 lib/socket-util-unix.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++
 lib/socket-util.h      |  7 +++++
 vswitchd/bridge.c      | 17 ++++++++++++
 vswitchd/vswitch.xml   | 16 +++++++++++
 4 files changed, 112 insertions(+)

diff --git a/lib/socket-util-unix.c b/lib/socket-util-unix.c
index afab195..072558e 100644
--- a/lib/socket-util-unix.c
+++ b/lib/socket-util-unix.c
@@ -19,6 +19,7 @@
 #include <errno.h>
 #include <fcntl.h>
 #include <net/if.h>
+#include <grp.h>
 #include <sys/ioctl.h>
 #include <sys/types.h>
 #include <sys/stat.h>
@@ -49,6 +50,77 @@ VLOG_DEFINE_THIS_MODULE(socket_util_unix);
  * space for a null terminator. */
 #define MAX_UN_LEN (sizeof(((struct sockaddr_un *) 0)->sun_path) - 1)
 
+/* Given 'group_name', returns true if the corresponding 'group'
+ * is found and saves the group id into 'gid'.  Returns false if
+ * cannot find 'group'. */
+static bool
+query_group_gid(const char *group_name, gid_t *gid)
+{
+    struct group group;
+    struct group *result;
+    size_t buflen;
+    char *buf;
+    int ret;
+
+    if (!group_name) {
+        return false;
+    }
+
+    /* Uses 256 as buffer size for now, seems large enough. */
+    buflen = 256;
+    buf = xmalloc(buflen * sizeof *buf);
+    ret = getgrnam_r(group_name, &group, buf, buflen, &result);
+
+    if (!result) {
+        VLOG_WARN("cannot retrieve group info for specified group "
+                  "(%s): %s",
+                  group_name,
+                  ret ? ovs_strerror(errno) : "group not found");
+    } else {
+        *gid = result->gr_gid;
+    }
+    free(buf);
+
+    return result;
+}
+
+/* Sets the unix domain socket file's group ownership to 'group'. */
+void
+unix_socket_set_file_group(const char *path, const char *group)
+{
+    struct stat info;
+    gid_t gid = 0;
+
+    if (stat(path, &info)) {
+        VLOG_WARN("set (%s) group fails: cannot find file (%s)",
+                  path, path);
+        return;
+    }
+
+    if (group && !query_group_gid(group, &gid)) {
+        VLOG_WARN("set (%s) group fails: cannot find group (%s)",
+                  path, group);
+        return;
+    }
+
+    /* Returns if current group id is same as desired group id. */
+    if (gid == info.st_gid) {
+        return;
+    } else {
+        /* Changes group ownership to 'gid'. */
+        if (chown(path, -1, gid)) {
+            VLOG_WARN("chown (%s) group to (%s) fails: %s",
+                      path, group ? group : "root", ovs_strerror(errno));
+            return;
+        }
+        if (chmod(path, 0770)) {
+            VLOG_WARN("chmod (%s) permission to 0770 fails: %s",
+                      path, ovs_strerror(errno));
+            return;
+        }
+    }
+}
+
 void
 xpipe(int fds[2])
 {
diff --git a/lib/socket-util.h b/lib/socket-util.h
index 8015c7f..77c251c 100644
--- a/lib/socket-util.h
+++ b/lib/socket-util.h
@@ -81,6 +81,7 @@ void xpipe_nonblocking(int fds[2]);
 
 int drain_rcvbuf(int fd);
 
+void unix_socket_set_file_group(const char *path, const char *group);
 int make_unix_socket(int style, bool nonblock,
                      const char *bind_path, const char *connect_path);
 int get_unix_name_len(socklen_t sun_len);
@@ -95,6 +96,12 @@ int af_inet_ifreq_ioctl(const char *name, struct ifreq *,
 #endif
 
 #ifdef _WIN32
+static inline void unix_socket_set_file_group(const char *path,
+                                              const char *group)
+{
+    return;
+}
+
 static inline int make_unix_socket(int style, bool nonblock,
                                    const char *bind_path,
                                    const char *connect_path)
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 232a334..984fada 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -3509,6 +3509,7 @@ bridge_configure_remotes(struct bridge *br,
 
     enum ofproto_fail_mode fail_mode;
 
+    const char **punix_file_groups;
     struct ofproto_controller *ocs;
     size_t n_ocs;
     size_t i;
@@ -3530,9 +3531,12 @@ bridge_configure_remotes(struct bridge *br,
 
     n_controllers = bridge_get_controllers(br, &controllers);
 
+    punix_file_groups =
+        xmalloc((n_controllers + 1) * sizeof *punix_file_groups);
     ocs = xmalloc((n_controllers + 1) * sizeof *ocs);
     n_ocs = 0;
 
+    punix_file_groups[n_ocs] = NULL;
     bridge_ofproto_controller_for_mgmt(br, &ocs[n_ocs++]);
     for (i = 0; i < n_controllers; i++) {
         struct ovsrec_controller *c = controllers[i];
@@ -3587,6 +3591,8 @@ bridge_configure_remotes(struct bridge *br,
 
         bridge_configure_local_iface_netdev(br, c);
         bridge_ofproto_controller_from_ovsrec(c, &ocs[n_ocs]);
+        punix_file_groups[n_ocs] = smap_get(&c->other_config,
+                                            "punix_file_group");
         if (disable_in_band) {
             ocs[n_ocs].band = OFPROTO_OUT_OF_BAND;
         }
@@ -3595,6 +3601,17 @@ bridge_configure_remotes(struct bridge *br,
 
     ofproto_set_controllers(br->ofproto, ocs, n_ocs,
                             bridge_get_allowed_versions(br));
+
+    for (i = 0; i < n_ocs; i++) {
+        if (!strncmp(ocs[i].target, "punix:", 6)) {
+            const char *path = ocs[i].target + 6;
+            const char *group = punix_file_groups[i];
+
+            unix_socket_set_file_group(path, group);
+        }
+    }
+
+    free(punix_file_groups);
     free(ocs[0].target); /* From bridge_ofproto_controller_for_mgmt(). */
     free(ocs);
 
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index 457f34a..3a1fa94 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -3989,6 +3989,22 @@
         a default value of 48 is chosen.  Valid DSCP values must be in the
         range 0 to 63.
       </column>
+
+      <column name="other_config" key="punix_file_group"
+                type='{"type": "string"}'>
+        <p>
+          When connection method in <ref column="target"/> is
+          <code>punix</code>, this config specifies the user group to which
+          the group ownership for 'punix' (unix domain socket) file created
+          by ovs will be applied.  Also, the file's access permission will be
+          changed to '0770'.
+        </p>
+        <p>
+          By default, the 'punix' file is associated with the 'root' group
+          (or equivalent super user group) and have access permission '0700'.
+          If this config is cleared or not specified, the default is used.
+        </p>
+      </column>
     </group>
 
 
-- 
1.9.1




More information about the dev mailing list