From a426e8b6adb729fce85a6a7bf058e35fee8abc99 Mon Sep 17 00:00:00 2001 From: joshua stein Date: Fri, 10 Jul 2020 17:14:17 -0500 Subject: [PATCH] Overhaul IPC mechanism to use a Unix socket for increased security The Atom-based IPC inherited from Ratpoison is overly complex and insecure. An unprivileged application that only has an X11 connection (say a sandboxed Firefox child process) could set the RP_COMMAND_REQUEST atom property on its window with a value of "0exec something" to cause us to read it and execute that shell command. Switching to a Unix socket in our ~/.config/sdorfehs directory ensures the requesting process is running as our uid, has the ability to make socket connections, and can write to that path. --- communications.c | 232 +++++++++++++++++++++++++++-------------------- communications.h | 5 +- data.h | 4 + events.c | 159 +++----------------------------- globals.c | 3 - globals.h | 6 +- sdorfehs.1 | 4 + sdorfehs.c | 12 +-- 8 files changed, 163 insertions(+), 262 deletions(-) diff --git a/communications.c b/communications.c index 60340e8c..bb4f8523 100644 --- a/communications.c +++ b/communications.c @@ -23,125 +23,157 @@ #include #include +#include +#include +#include #include +#include +#include +#include #include "sdorfehs.h" -/* Sending commands to us from another process */ -static int -receive_command_result(Window w) +void +init_control_socket_path(void) { - int query; - int return_status = RET_FAILURE; - Atom type_ret; - int format_ret; - unsigned long nitems; - unsigned long bytes_after; - unsigned char *result = NULL; - - /* First, find out how big the property is. */ - query = XGetWindowProperty(dpy, w, rp_command_result, - 0, 0, False, xa_string, - &type_ret, &format_ret, &nitems, &bytes_after, - &result); - - /* Failed to retrieve property. */ - if (query != Success || result == NULL) { - PRINT_DEBUG(("failed to get command result length\n")); - return return_status; - } - /* - * XGetWindowProperty always allocates one extra byte even if the - * property is zero length. - */ - XFree(result); - - /* - * Now that we have the length of the message, we can get the whole - * message. - */ - query = XGetWindowProperty(dpy, w, rp_command_result, - 0, (bytes_after / 4) + (bytes_after % 4 ? 1 : 0), - True, xa_string, &type_ret, &format_ret, &nitems, - &bytes_after, &result); - - /* Failed to retrieve property. */ - if (query != Success || result == NULL) { - PRINT_DEBUG(("failed to get command result\n")); - return return_status; - } - /* - * We can receive: - * - an empty string, indicating a success but no output - * - a string starting with '1', indicating a success and an output - * - a string starting with '0', indicating a failure and an optional - * output - */ - switch (result[0]) { - case '\0': - /* Command succeeded but no string to print */ - return_status = RET_SUCCESS; - break; - case '0': - /* Command failed, don't print an empty line if no explanation - * was given */ - if (result[1] != '\0') - fprintf(stderr, "%s\n", &result[1]); - return_status = RET_FAILURE; - break; - case '1': - /* Command succeeded, print the output */ - printf("%s\n", &result[1]); - return_status = RET_SUCCESS; - break; - default: - /* We probably got junk, so ignore it */ - return_status = RET_FAILURE; - } + char *config_dir; - /* Free the result. */ - XFree(result); - - return return_status; + config_dir = get_config_dir(); + rp_glob_screen.control_socket_path = xsprintf("%s/control", config_dir); + free(config_dir); } -int -send_command(unsigned char interactive, unsigned char *cmd) +void +listen_for_commands(void) { - Window w, root; - int done = 0, return_status = RET_FAILURE; - struct sbuf *s; + struct sockaddr_un sun; + + if ((rp_glob_screen.control_socket_fd = socket(AF_UNIX, + SOCK_STREAM | SOCK_NONBLOCK, 0)) == -1) + err(1, "socket"); - s = sbuf_new(0); - sbuf_printf(s, "%c%s", interactive, cmd); + sun.sun_family = AF_UNIX; + if (strlcpy(sun.sun_path, rp_glob_screen.control_socket_path, + sizeof(sun.sun_path)) >= sizeof(sun.sun_path)) + err(1, "control socket path too long: %s", + rp_glob_screen.control_socket_path); + + if (unlink(rp_glob_screen.control_socket_path) == -1 && + errno != ENOENT) + err(1, "unlink %s",rp_glob_screen.control_socket_path); + + if (bind(rp_glob_screen.control_socket_fd, (struct sockaddr *)&sun, + sizeof(sun)) == -1) + err(1, "bind %s", rp_glob_screen.control_socket_path); + + if (chmod(rp_glob_screen.control_socket_path, 0600) == -1) + err(1, "chmod %s", rp_glob_screen.control_socket_path); + + if (listen(rp_glob_screen.control_socket_fd, 2) == -1) + err(1, "listen %s", rp_glob_screen.control_socket_path); + + PRINT_DEBUG(("listening for commands at %s\n", + rp_glob_screen.control_socket_path)); +} - root = RootWindow(dpy, DefaultScreen(dpy)); - w = XCreateSimpleWindow(dpy, root, 0, 0, 1, 1, 0, 0, 0); +int +send_command(int interactive, unsigned char *cmd) +{ + struct sockaddr_un sun; + char *wcmd; + char ret[1024]; + size_t len; + int fd; + + len = 1 + strlen(cmd) + 2; + wcmd = malloc(len); + if (snprintf(wcmd, len, "%c%s\n", interactive, cmd) != (len - 1)) + errx(1, "snprintf"); + + if ((fd = socket(AF_UNIX, SOCK_STREAM, 0)) == -1) + err(1, "socket"); + + sun.sun_family = AF_UNIX; + if (strlcpy(sun.sun_path, rp_glob_screen.control_socket_path, + sizeof(sun.sun_path)) >= sizeof(sun.sun_path)) + err(1, "control socket path too long: %s", + rp_glob_screen.control_socket_path); + + if (connect(fd, (struct sockaddr *)&sun, sizeof(sun)) == -1) + err(1, "failed to connect to control socket at %s", + rp_glob_screen.control_socket_path); + + if (write(fd, wcmd, len) != len) + err(1, "short write to control socket"); + + free(wcmd); + + len = read(fd, &ret, sizeof(ret) - 1); + if (len > 2) { + ret[len - 1] = '\0'; + fprintf(stderr, "%s\n", &ret[1]); + } - /* Select first to avoid race condition */ - XSelectInput(dpy, w, PropertyChangeMask); + return ret[0]; +} - XChangeProperty(dpy, w, rp_command, xa_string, 8, PropModeReplace, - (unsigned char *)sbuf_get(s), strlen((char *) cmd) + 2); +void +receive_command(void) +{ + cmdret *cmd_ret; + char cmd[1024] = { 0 }, c; + char *result, *rcmd; + int cl, len = 0, interactive = 0; - XChangeProperty(dpy, root, rp_command_request, XA_WINDOW, - 8, PropModeAppend, (unsigned char *) &w, sizeof(Window)); + PRINT_DEBUG(("have connection waiting on command socket\n")); - sbuf_free(s); + if ((cl = accept(rp_glob_screen.control_socket_fd, NULL, NULL)) == -1) { + warn("accept"); + return; + } - while (!done) { - XEvent ev; + while (len <= sizeof(cmd)) { + if (len == sizeof(cmd)) { + warn("%s: bogus command length", __func__); + close(cl); + return; + } - XMaskEvent(dpy, PropertyChangeMask, &ev); - if (ev.xproperty.atom == rp_command_result - && ev.xproperty.state == PropertyNewValue) { - return_status = - receive_command_result(ev.xproperty.window); - done = 1; + if (read(cl, &c, 1) == 1) { + if (c == '\n') { + cmd[len++] = '\0'; + break; + } + cmd[len++] = c; + } else if (errno != EAGAIN) { + PRINT_DEBUG(("bad read result on control socket: %s\n", + strerror(errno))); + break; } } - XDestroyWindow(dpy, w); + interactive = cmd[0]; + rcmd = cmd + 1; + + PRINT_DEBUG(("read %d byte(s) on command socket: %s\n", len, rcmd)); + + cmd_ret = command(interactive, rcmd); + + /* notify the client of any text that was returned by the command */ + len = 2; + if (cmd_ret->output) { + result = xsprintf("%c%s\n", cmd_ret->success ? 1 : 0, + cmd_ret->output); + len = 1 + strlen(cmd_ret->output) + 1; + } else if (cmd_ret->success) + result = xsprintf("%c\n", 1); + else + result = xsprintf("%c\n", 0); + + cmdret_free(cmd_ret); + + PRINT_DEBUG(("writing back %d to command client: %s", len, result + 1)); - return return_status; + write(cl, result, len); + close(cl); } diff --git a/communications.h b/communications.h index 3a87c298..9c7a9d0c 100644 --- a/communications.h +++ b/communications.h @@ -20,6 +20,9 @@ #ifndef _SDORFEHS_COMMUNICATIONS_H #define _SDORFEHS_COMMUNICATIONS_H 1 -int send_command(unsigned char interactive, unsigned char *cmd); +void init_control_socket_path(void); +void listen_for_commands(void); +int send_command(int interactive, unsigned char *cmd); +void receive_command(void); #endif /* ! _SDORFEHS_COMMUNICATIONS_H */ diff --git a/data.h b/data.h index e7dd52a6..2fea80d8 100644 --- a/data.h +++ b/data.h @@ -168,6 +168,10 @@ struct rp_global_screen { /* This numset is responsible for giving out numbers for each screen */ struct numset *numset; + /* The path to and open fd of our control socket */ + char *control_socket_path; + int control_socket_fd; + /* The path to and open fd of our bar FIFO */ char *bar_fifo_path; int bar_fifo_fd; diff --git a/events.c b/events.c index 7cf0b9b0..5b4ed374 100644 --- a/events.c +++ b/events.c @@ -527,141 +527,6 @@ button_press(XEvent *ev) bar_handle_click(s, xbe); } -/* - * Read a command off the window and execute it. Some commands return text. - * This text is passed back using the RP_COMMAND_RESULT Atom. The client will - * wait for this property change so something must be returned. - */ -static cmdret * -execute_remote_command(Window w) -{ - int status; - cmdret *ret; - Atom type_ret; - int format_ret; - unsigned long nitems; - unsigned long bytes_after; - unsigned char *req; - - status = XGetWindowProperty(dpy, w, rp_command, - 0, 0, False, xa_string, - &type_ret, &format_ret, &nitems, &bytes_after, - &req); - - if (status != Success || req == NULL) { - return cmdret_new(RET_FAILURE, - "Couldn't get RP_COMMAND Property"); - } - /* - * XGetWindowProperty always allocates one extra byte even if the - * property is zero length. - */ - XFree(req); - - status = XGetWindowProperty(dpy, w, rp_command, - 0, (bytes_after / 4) + (bytes_after % 4 ? 1 : 0), - True, xa_string, &type_ret, &format_ret, &nitems, - &bytes_after, &req); - - if (status != Success || req == NULL) { - return cmdret_new(RET_FAILURE, - "Couldn't get RP_COMMAND Property"); - } - PRINT_DEBUG(("command: %s\n", req)); - ret = command(req[0], (char *) &req[1]); - XFree(req); - - return ret; -} - -/* - * Command requests are posted as a property change using the - * RP_COMMAND_REQUEST Atom on the root window. A Command request is a Window - * that holds the actual command as a property using the RP_COMMAND Atom. - * receive_command reads the list of Windows and executes their associated - * command. - */ -static void -receive_command(Window root) -{ - cmdret *cmd_ret; - char *result; - Atom type_ret; - int format_ret; - unsigned long nitems; - unsigned long bytes_after; - unsigned char *prop_return; - int offset; - - /* - * Init offset to 0. In the case where there is more than one window in - * the property, a partial read does not delete the property and we - * need to grab the next window by incementing offset to the offset of - * the next window. - */ - offset = 0; - do { - int ret; - int length; - Window w; - - length = sizeof(Window) / 4 + (sizeof(Window) % 4 ? 1 : 0); - ret = XGetWindowProperty(dpy, root, - rp_command_request, - offset, length, - True, XA_WINDOW, &type_ret, &format_ret, - &nitems, - &bytes_after, &prop_return); - - /* - * Update the offset to point to the next window (if there is - * another one). - */ - offset += length; - - if (ret != Success) { - warnx("XGetWindowProperty Failed"); - if (prop_return) - XFree(prop_return); - break; - } - /* If there was no window, then we're done. */ - if (prop_return == NULL) { - PRINT_DEBUG(("No property to read\n")); - break; - } - /* - * We grabbed a window, so now read the command stored in this - * window and execute it. - */ - w = *(Window *) prop_return; - XFree(prop_return); - cmd_ret = execute_remote_command(w); - - /* - * notify the client of any text that was returned by the - * command. see communications.c:receive_command_result() - */ - if (cmd_ret->output) - result = xsprintf("%c%s", cmd_ret->success ? '1' : '0', - cmd_ret->output); - else if (!cmd_ret->success) - result = xstrdup("0"); - else - result = NULL; - - if (result) - XChangeProperty(dpy, w, rp_command_result, xa_string, - 8, PropModeReplace, (unsigned char *)result, - strlen(result)); - else - XChangeProperty(dpy, w, rp_command_result, xa_string, - 8, PropModeReplace, NULL, 0); - free(result); - cmdret_free(cmd_ret); - } while (bytes_after > 0); -} - static void property_notify(XEvent *ev) { @@ -670,13 +535,6 @@ property_notify(XEvent *ev) PRINT_DEBUG(("atom: %ld (%s)\n", ev->xproperty.atom, XGetAtomName(dpy, ev->xproperty.atom))); - if (ev->xproperty.atom == rp_command_request && - is_a_root_window(ev->xproperty.window) && - ev->xproperty.state == PropertyNewValue) { - PRINT_DEBUG((PROGNAME " command\n")); - receive_command(ev->xproperty.window); - } - win = find_window(ev->xproperty.window); if (!win) return; @@ -1032,14 +890,16 @@ handle_signals(void) void listen_for_events(void) { - struct pollfd pfd[2]; + struct pollfd pfd[3]; int pollfifo = 1; memset(&pfd, 0, sizeof(pfd)); pfd[0].fd = ConnectionNumber(dpy); pfd[0].events = POLLIN; - pfd[1].fd = rp_glob_screen.bar_fifo_fd; + pfd[1].fd = rp_glob_screen.control_socket_fd; pfd[1].events = POLLIN; + pfd[2].fd = rp_glob_screen.bar_fifo_fd; + pfd[2].events = POLLIN; /* Loop forever. */ for (;;) { @@ -1049,19 +909,22 @@ listen_for_events(void) if (pollfifo && rp_glob_screen.bar_fifo_fd == -1) pollfifo = 0; else if (pollfifo) - pfd[1].fd = rp_glob_screen.bar_fifo_fd; + pfd[2].fd = rp_glob_screen.bar_fifo_fd; - poll(pfd, pollfifo ? 2 : 1, -1); + poll(pfd, pollfifo ? 3 : 2, -1); - if (pollfifo && (pfd[1].revents & (POLLERR|POLLNVAL))) { + if (pollfifo && (pfd[2].revents & (POLLERR|POLLNVAL))) { warnx("error polling on FIFO"); pollfifo = 0; continue; } - if (pollfifo && (pfd[1].revents & (POLLHUP|POLLIN))) + if (pollfifo && (pfd[2].revents & (POLLHUP|POLLIN))) bar_read_fifo(); + if (pfd[1].revents & (POLLHUP|POLLIN)) + receive_command(); + if (!XPending(dpy)) continue; } diff --git a/globals.c b/globals.c index 7935cc68..d1fa3fb3 100644 --- a/globals.c +++ b/globals.c @@ -58,9 +58,6 @@ Atom wm_delete; Atom wm_take_focus; Atom wm_colormaps; -Atom rp_command; -Atom rp_command_request; -Atom rp_command_result; Atom rp_selection; /* TEXT atoms */ diff --git a/globals.h b/globals.h index 788acfe1..2401d9b6 100644 --- a/globals.h +++ b/globals.h @@ -102,12 +102,10 @@ extern struct list_head rp_screens; extern rp_screen *rp_current_screen; extern rp_global_screen rp_glob_screen; +extern Display *dpy; + extern XEvent rp_current_event; -extern Display *dpy; -extern Atom rp_command; -extern Atom rp_command_request; -extern Atom rp_command_result; extern Atom rp_selection; extern Atom wm_name; diff --git a/sdorfehs.1 b/sdorfehs.1 index 0773b1e4..c6a18959 100644 --- a/sdorfehs.1 +++ b/sdorfehs.1 @@ -1198,6 +1198,10 @@ Default is .It Pa ~/.config/sdorfehs/config Configuration file read at startup time, if present. .Pp +.It Pa ~/.config/sdorfehs/control +Unix Socket which accepts remote control commands sent by +.Nm Fl c . +.Pp .It Pa ~/.config/sdorfehs/bar FIFO/named pipe which accepts input to show on the sticky bar when .Ic barsticky diff --git a/sdorfehs.c b/sdorfehs.c index d0e87fb7..02925137 100644 --- a/sdorfehs.c +++ b/sdorfehs.c @@ -234,7 +234,7 @@ main(int argc, char *argv[]) char **cmd = NULL; int cmd_count = 0; char *display = NULL; - unsigned char interactive = 0; + int interactive = 0; char *alt_rcfile = NULL; setlocale(LC_CTYPE, ""); @@ -284,9 +284,6 @@ main(int argc, char *argv[]) set_close_on_exec(ConnectionNumber(dpy)); /* Set our own specific Atoms. */ - rp_command = XInternAtom(dpy, "RP_COMMAND", False); - rp_command_request = XInternAtom(dpy, "RP_COMMAND_REQUEST", False); - rp_command_result = XInternAtom(dpy, "RP_COMMAND_RESULT", False); rp_selection = XInternAtom(dpy, "RP_SELECTION", False); /* TEXT atoms */ @@ -294,11 +291,13 @@ main(int argc, char *argv[]) xa_compound_text = XInternAtom(dpy, "COMPOUND_TEXT", False); xa_utf8_string = XInternAtom(dpy, "UTF8_STRING", False); + init_control_socket_path(); + if (cmd_count > 0) { int j, exit_status = 0; for (j = 0; j < cmd_count; j++) { - if (!send_command(interactive, (unsigned char *) cmd[j])) + if (!send_command(interactive, (unsigned char *)cmd[j])) exit_status = 1; free(cmd[j]); } @@ -371,7 +370,7 @@ main(int argc, char *argv[]) c = read_startup_files(alt_rcfile); if (c == -1) return 1; - else if (c == 0) { + if (c == 0) { /* No config file, just do something basic. */ cmdret *result; if ((result = command(0, "hsplit"))) @@ -391,6 +390,7 @@ main(int argc, char *argv[]) pledge("stdio rpath cpath wpath fattr unix proc exec", NULL); #endif + listen_for_commands(); listen_for_events(); return 0;