Merge pull request #25 from hamishcoleman/main

Improve management socket permissions controls

apps/n3n-supernode.c

@@ -596,11 +596,20 @@ int main (int argc, char * argv[]) {
  char unixsock[1024];
  snprintf(unixsock, sizeof(unixsock), "%s/mgmt", sss_node.conf.sessiondir);
 
- if(slots_listen_unix(sss_node.mgmt_slots, unixsock)!=0) {
+ int e = slots_listen_unix(
+ sss_node.mgmt_slots,
+ unixsock,
+ sss_node.conf.mgmt_sock_perms,
+ sss_node.conf.userid,
+ sss_node.conf.groupid
+ );
+ // TODO:
+ // - do we actually want to tie the user/group to the running pid?
+
+ if(e !=0) {
  perror("slots_listen_tcp");
  exit(1);
  }
- chown(unixsock, sss_node.conf.userid, sss_node.conf.groupid);
 #endif
 
  // Add our freshly opened socket to any edges added by federation

include/n2n_typedefs.h

@@ -466,6 +466,7 @@ typedef struct n2n_edge_conf {
  struct sockaddr *bind_address; /**< The address to bind to if provided */
  n2n_sock_t preferred_sock; /**< propagated local sock for better p2p in LAN (-e) */
  uint32_t mgmt_port; // TODO: ports are actually uint16_t
+ uint32_t mgmt_sock_perms;
  bool enable_debug_pages;
  uint32_t metric; /**< Network interface metric (Windows only). */
  n2n_auth_t auth;

libs/connslot/connslot.c

@@ -19,6 +19,7 @@
 #include <string.h>
 #ifndef _WIN32
 #include <sys/socket.h>
+#include <sys/stat.h>
 #include <sys/types.h>
 #include <sys/uio.h>
 #include <sys/un.h>
@@ -437,7 +438,7 @@ int slots_listen_tcp(slots_t *slots, int port, bool allow_remote) {
 }
 
 #ifndef _WIN32
-int slots_listen_unix(slots_t *slots, char *path) {
+int slots_listen_unix(slots_t *slots, char *path, int mode, int uid, int gid) {
  int listen_nr = _slots_listen_find_empty(slots);
  if (listen_nr <0) {
  return -2;
@@ -466,13 +467,29 @@ int slots_listen_unix(slots_t *slots, char *path) {
  return -1;
  }
 
+ // For both the chmod and chown, we would be happy to ignore the result:
+ // either it worked or not. But -Wunused-result will not let us do that
+ //
+ // TODO:
+ // - mark it so the compiler doesnt complain
+
+ int result = 0;
+
+ if(mode > 0) {
+ result += fchmod(server, mode);
+ }
+
+ if(uid != -1 && gid != -1) {
+ result += chown(path, uid, gid);
+ }
+
  // backlog of 1 - low, but sheds load quickly when we run out of slots
  if (listen(server, 1) < 0) {
  return -1;
  }
 
  slots->listen[listen_nr] = server;
- return 0;
+ return result;
 }
 #endif
 

libs/connslot/connslot.h

@@ -61,7 +61,7 @@ void conn_close(conn_t *);
 void slots_free(slots_t *slots);
 slots_t *slots_malloc(int nr_slots, size_t, size_t);
 int slots_listen_tcp(slots_t *, int, bool);
-int slots_listen_unix(slots_t *, char *);
+int slots_listen_unix(slots_t *, char *, int, int, int);
 void slots_listen_close(slots_t *);
 int slots_fdset(slots_t *, fd_set *, fd_set *);
 int slots_accept(slots_t *, int);

libs/connslot/httpd-test.c

@@ -73,7 +73,7 @@ void httpd_test(int port) {
  }
 
 #ifndef _WIN32
- if (slots_listen_unix(slots, "test.sock")!=0) {
+ if (slots_listen_unix(slots, "test.sock", -1, -1, -1)!=0) {
  perror("slots_listen_tcp");
  exit(1);
  }

src/conffile_defs.c

@@ -302,6 +302,13 @@ static struct n3n_conf_option section_management[] = {
  "Use this if you wish to use the TCP API.",
 
  },
+ {
+ .name = "unix_sock_perms",
+ .type = n3n_conf_uint32,
+ .offset = offsetof(n2n_edge_conf_t, mgmt_sock_perms),
+ .desc = "Unix management socket permissions",
+ .help = "Sets the permissions for the management interface socket.",
+ },
  {.name = NULL},
 };
 

src/edge_utils.c

@@ -3241,13 +3241,20 @@ static int edge_init_sockets (struct n3n_runtime_data *eee) {
  char unixsock[1024];
  snprintf(unixsock, sizeof(unixsock), "%s/mgmt", eee->conf.sessiondir);
 
- if(slots_listen_unix(eee->mgmt_slots, unixsock)!=0) {
+ int e = slots_listen_unix(
+ eee->mgmt_slots,
+ unixsock,
+ eee->conf.mgmt_sock_perms,
+ eee->conf.userid,
+ eee->conf.groupid
+ );
+ // TODO:
+ // - do we actually want to tie the user/group to the running pid?
+
+ if(e!=0) {
  perror("slots_listen_tcp");
  exit(1);
  }
- chown(unixsock, eee->conf.userid, eee->conf.groupid);
- // Deliberately ignore the result - either it worked or not
- // TODO: mark it so the compiler doesnt complain
 #endif
 
 #ifndef SKIP_MULTICAST_PEERS_DISCOVERY

src/management.c

@@ -30,6 +30,10 @@
 #include <sys/socket.h> // for sendto, sockaddr
 #endif
 
+static struct metrics {
+ uint32_t event_write_error;
+} metrics;
+
 static void generate_http_headers (conn_t *conn, const char *type, int code) {
  strbuf_t **pp = &conn->reply_header;
  sb_reprintf(pp, "HTTP/1.1 %i result\r\n", code);
@@ -248,13 +252,15 @@ static void event_subscribe (struct n3n_runtime_data *eee, conn_t *conn) {
  // will usefully show you the raw streaming data if we use the wrong
  // content type
  char *msg1 = "HTTP/1.1 200 event\r\nContent-Type: application/json\r\n\r\n";
- write(mgmt_event_subscribers[topicid], msg1, strlen(msg1));
- // Ignore the result
- // (the message is leaving here fine, the problem must be at your end)
+ if(write(mgmt_event_subscribers[topicid], msg1, strlen(msg1))<1) {
+  metrics.event_write_error++;
+ }
 
  if(replacing) {
  char *msg2 = "\x1e\"replacing\"\n";
- write(mgmt_event_subscribers[topicid], msg2, strlen(msg2));
+ if(write(mgmt_event_subscribers[topicid], msg2, strlen(msg2))<1) {
+ metrics.event_write_error++;
+ }
  }
 }
 

tests/test_builtin_edge.sh.expected

@@ -35,6 +35,7 @@ verbose=2
 [management]
 enable_debug_pages=false
 port=0
+unix_sock_perms=0
 
 [supernode]
 auto_ip_max=0.0.0.0/0