Change log ID from a path to UUID + path.

A UUID is now stored for each I/O log that sudo_logsrvd creates.
The log ID in a restart message must now contain a log ID that
matches both the I/O log path and the UUID it contains.
For journaled I/O logs, only the UUID is used.

Author: millert

logsrvd/iolog_writer.c

@@ -151,6 +151,13 @@ evlog_new(const TimeSpec *submit_time, InfoMessage * const *info_msgs,
        goto bad;
     }
 
+    /* Create a UUID to store in the event log. */
+    sudo_uuid_create(uuid);
+    if (sudo_uuid_to_string(uuid, evlog->uuid_str, sizeof(evlog->uuid_str)) == NULL) {
+       sudo_warnx("%s", U_("unable to generate UUID"));
+       goto bad;
+    }
+
     /* Client/peer IP address. */
     if ((evlog->peeraddr = strdup(closure->ipaddr)) == NULL) {
 	sudo_warnx(U_("%s: %s"), __func__, U_("unable to allocate memory"));
@@ -687,6 +694,46 @@ iolog_flush_all(struct connection_closure *closure)
     debug_return_bool(ret);
 }
 
+static bool
+iolog_store_uuid(int dfd, struct connection_closure *closure)
+{
+    char uuid_str[37];
+    int fd = -1;
+    debug_decl(iolog_create_uuid, SUDO_DEBUG_UTIL);
+
+    /* Create a UUID to store in the I/O log. */
+    sudo_uuid_create(closure->uuid);
+    if (sudo_uuid_to_string(closure->uuid, uuid_str, sizeof(uuid_str)) == NULL) {
+       sudo_warnx("%s", U_("unable to generate UUID"));
+       goto bad;
+    }
+
+    /* Write UUID in string form to the I/O log directory. */
+    fd = iolog_openat(dfd, "uuid", O_CREAT|O_TRUNC|O_WRONLY);
+    if (fd == -1) {
+	sudo_warn(U_("unable to open %s/%s"), closure->evlog->iolog_path,
+	    "uuid");
+        goto bad;
+    }
+    if (fchown(fd, iolog_get_uid(), iolog_get_gid()) != 0) {
+        sudo_debug_printf(SUDO_DEBUG_ERROR|SUDO_DEBUG_ERRNO,
+            "%s: unable to fchown %d:%d %s/uuid", __func__,
+            (int)iolog_get_uid(), (int)iolog_get_gid(),
+	    closure->evlog->iolog_path);
+    }
+    if (write(fd, uuid_str, sizeof(uuid_str) - 1) != ssizeof(uuid_str) - 1) {
+	sudo_warn(U_("unable to write %s/%s"), closure->evlog->iolog_path,
+	    "uuid");
+        goto bad;
+    }
+
+    debug_return_bool(true);
+bad:
+    if (fd != -1)
+	close(fd);
+    debug_return_bool(false);
+}
+
 bool
 iolog_init(const AcceptMessage *msg, struct connection_closure *closure)
 {
@@ -697,6 +744,10 @@ iolog_init(const AcceptMessage *msg, struct connection_closure *closure)
     if (!create_iolog_path(closure))
 	debug_return_bool(false);
 
+    /* Create and store I/O log UUID */
+    if (!iolog_store_uuid(closure->iolog_dir_fd, closure))
+	debug_return_bool(false);
+
     /* Write sudo I/O log info file */
     if (!iolog_write_info_file(closure->iolog_dir_fd, evlog))
 	debug_return_bool(false);

logsrvd/logsrvd.c

@@ -322,7 +322,7 @@ get_free_buf(size_t len, struct connection_closure *closure)
     debug_return_ptr(NULL);
 }
 
-static bool
+bool
 fmt_server_message(struct connection_closure *closure, ServerMessage *msg)
 {
     struct connection_buffer *buf = NULL;
@@ -376,16 +376,58 @@ fmt_hello_message(struct connection_closure *closure)
     debug_return_bool(fmt_server_message(closure, &msg));
 }
 
+static char *
+gen_log_id(const unsigned char *uuid, const char *path,
+    struct connection_closure *closure)
+{
+    size_t b64_size, id_len, pathlen = strlen(path);
+    char *b64_id = NULL;
+    unsigned char *id;
+    debug_decl(gen_log_id, SUDO_DEBUG_UTIL);
+
+    /* Log ID is 16-byte UUID + path. */
+    id_len = 16 + pathlen;
+    if ((id = malloc(id_len)) == NULL) {
+	sudo_warnx(U_("%s: %s"), __func__, U_("unable to allocate memory"));
+	goto done;
+    }
+    memcpy(id, uuid, 16);
+    memcpy(id + 16, path, pathlen);
+
+    /* Base64 encode log ID */
+    b64_size = ((id_len + 2) / 3 * 4) + 1;
+    if ((b64_id = malloc(b64_size)) == NULL) {
+	sudo_warnx(U_("%s: %s"), __func__, U_("unable to allocate memory"));
+	goto done;
+    }
+    if (sudo_base64_encode(id, id_len, b64_id, b64_size) == (size_t)-1) {
+	sudo_warnx("%s", U_("unable to base64 log ID"));
+	free(b64_id);
+	b64_id = NULL;
+    }
+
+done:
+    free(id);
+    debug_return_str(b64_id);
+}
+
 bool
-fmt_log_id_message(const char *id, struct connection_closure *closure)
+fmt_log_id_message(const unsigned char uuid[restrict static 16],
+    const char *path, struct connection_closure *closure)
 {
     ServerMessage msg = SERVER_MESSAGE__INIT;
+    bool ret = false;
     debug_decl(fmt_log_id_message, SUDO_DEBUG_UTIL);
 
-    msg.u.log_id = (char *)id;
+    /* Generate log_id from path and closure's UUID */
     msg.type_case = SERVER_MESSAGE__TYPE_LOG_ID;
+    msg.u.log_id = gen_log_id(uuid, path, closure);
+    if (msg.u.log_id != NULL) {
+	ret = fmt_server_message(closure, &msg);
+	free(msg.u.log_id);
+    }
 
-    debug_return_bool(fmt_server_message(closure, &msg));
+    debug_return_bool(ret);
 }
 
 static bool
@@ -592,14 +634,6 @@ handle_restart(const RestartMessage *msg, const uint8_t *buf, size_t len,
 	"%s: received RestartMessage for %s from %s", __func__, msg->log_id,
 	source);
 
-    /* The log_id is used to create a path name, prevent path traversal. */
-    if (contains_dot_dot(msg->log_id)) {
-	sudo_warnx(U_("%s: %s"), source,
-	    U_("RestartMessage log_id path traversal attack"));
-	closure->errstr = _("invalid RestartMessage");
-	debug_return_bool(false);
-    }
-
     /* Only I/O logs are restartable. */
     closure->log_io = true;
 

logsrvd/logsrvd.h

@@ -118,6 +118,7 @@ struct connection_closure {
     bool read_instead_of_write;
     bool write_instead_of_read;
     bool temporary_write_event;
+    unsigned char uuid[16];
 #ifdef HAVE_STRUCT_IN6_ADDR
     char ipaddr[INET6_ADDRSTRLEN];
 #else
@@ -200,7 +201,8 @@ extern struct client_message_switch cms_local;
 bool start_protocol(struct connection_closure *closure);
 void connection_close(struct connection_closure *closure);
 bool schedule_commit_point(const TimeSpec *commit_point, struct connection_closure *closure);
-bool fmt_log_id_message(const char *id, struct connection_closure *closure);
+bool fmt_server_message(struct connection_closure *closure, ServerMessage *msg);
+bool fmt_log_id_message(const unsigned char uuid[restrict static 16], const char *path, struct connection_closure *closure);
 bool schedule_error_message(const char *errstr, struct connection_closure *closure);
 struct connection_buffer *get_free_buf(size_t, struct connection_closure *closure);
 struct connection_closure *connection_closure_alloc(int fd, bool tls, bool relay_only, struct sudo_event_base *base);

logsrvd/logsrvd_journal.c

@@ -85,12 +85,13 @@ journal_fdopen(int fd, const char *journal_path,
 }
 
 static int
-journal_mkstemp(const char *parent_dir, char *pathbuf, size_t pathsize)
+journal_mkuuid(const char *parent_dir, char *pathbuf, size_t pathsize)
 {
     int len, dfd = -1, fd = -1;
     mode_t dirmode, oldmask;
-    char *template;
-    debug_decl(journal_mkstemp, SUDO_DEBUG_UTIL);
+    unsigned char uuid[16];
+    char uuid_str[37];
+    debug_decl(journal_mkuuid, SUDO_DEBUG_UTIL);
 
     /* umask must not be more restrictive than the file modes. */
     dirmode = logsrvd_conf_iolog_mode() | S_IXUSR;
@@ -100,24 +101,35 @@ journal_mkstemp(const char *parent_dir, char *pathbuf, size_t pathsize)
         dirmode |= S_IXOTH;
     oldmask = umask(ACCESSPERMS & ~dirmode);
 
-    len = snprintf(pathbuf, pathsize, "%s/%s/%s",
-	logsrvd_conf_relay_dir(), parent_dir, RELAY_TEMPLATE);
-    if ((size_t)len >= pathsize) {
-	errno = ENAMETOOLONG;
-	sudo_warn("%s/%s/%s", logsrvd_conf_relay_dir(), parent_dir,
-	    RELAY_TEMPLATE);
-	goto done;
-    }
-    dfd = sudo_open_parent_dir(pathbuf, logsrvd_conf_iolog_uid(),
-	logsrvd_conf_iolog_gid(), S_IRWXU|S_IXGRP|S_IXOTH, false);
-    if (dfd == -1) {
-	sudo_debug_printf(SUDO_DEBUG_ERROR|SUDO_DEBUG_LINENO|SUDO_DEBUG_ERRNO,
-	    "unable to create parent dir for %s", pathbuf);
-	goto done;
-    }
-    template = &pathbuf[(size_t)len - (sizeof(RELAY_TEMPLATE) - 1)];
-    if ((fd = mkostempsat(dfd, template, 0, 0)) == -1) {
-	sudo_warn(U_("%s: %s"), "mkstemp", pathbuf);
+    do {
+	sudo_uuid_create(uuid);
+	if (sudo_uuid_to_string(uuid, uuid_str, sizeof(uuid_str)) == NULL) {
+	    sudo_warnx("%s", U_("unable to generate UUID"));
+	    goto done;
+	}
+
+	len = snprintf(pathbuf, pathsize, "%s/%s/%s",
+	    logsrvd_conf_relay_dir(), parent_dir, uuid_str);
+	if ((size_t)len >= pathsize) {
+	    errno = ENAMETOOLONG;
+	    sudo_warn("%s/%s/%s",
+		logsrvd_conf_relay_dir(), parent_dir, uuid_str);
+	    goto done;
+	}
+	if (dfd == -1) {
+	    dfd = sudo_open_parent_dir(pathbuf, logsrvd_conf_iolog_uid(),
+		logsrvd_conf_iolog_gid(), S_IRWXU|S_IXGRP|S_IXOTH, false);
+	    if (dfd == -1) {
+		sudo_debug_printf(
+		    SUDO_DEBUG_ERROR|SUDO_DEBUG_LINENO|SUDO_DEBUG_ERRNO,
+		    "unable to create parent dir for %s", pathbuf);
+		goto done;
+	    }
+	}
+	fd = openat(dfd, uuid_str, O_CREAT|O_EXCL|O_RDWR, S_IRUSR|S_IWUSR);
+    } while (fd == -1 && errno == EEXIST);
+    if (fd == -1) {
+	sudo_warn(U_("%s: %s"), "openat", pathbuf);
 	goto done;
     }
 
@@ -139,7 +151,7 @@ journal_create(struct connection_closure *closure)
     int fd;
     debug_decl(journal_create, SUDO_DEBUG_UTIL);
 
-    fd = journal_mkstemp("incoming", journal_path, sizeof(journal_path));
+    fd = journal_mkuuid("incoming", journal_path, sizeof(journal_path));
     if (fd == -1) {
 	closure->errstr = _("unable to create journal file");
 	debug_return_bool(false);
@@ -183,7 +195,7 @@ journal_finish(struct connection_closure *closure)
     rewind(closure->journal);
 
     /* Move journal to the outgoing directory. */
-    fd = journal_mkstemp("outgoing", outgoing_path, sizeof(outgoing_path));
+    fd = journal_mkuuid("outgoing", outgoing_path, sizeof(outgoing_path));
     if (fd == -1) {
 	closure->errstr = _("unable to rename journal file");
 	debug_return_bool(false);
@@ -405,23 +417,30 @@ static bool
 journal_restart(const RestartMessage *msg, const uint8_t *buf, size_t buflen,
     struct connection_closure *closure)
 {
+    char uuid_str[37], journal_path[PATH_MAX];
+    unsigned char uuid[16];
     struct timespec target;
     int fd, len;
-    char *cp, journal_path[PATH_MAX];
     debug_decl(journal_restart, SUDO_DEBUG_UTIL);
 
-    /* Strip off leading hostname from log_id. */
-    if ((cp = strchr(msg->log_id, '/')) != NULL) {
-        if (cp != msg->log_id)
-            cp++;
-    } else {
-    	cp = msg->log_id;
+    /* A journal log_id must be a base64-encoded UUID. */
+    if (strlen(msg->log_id) != ((16 + 2) / 3 * 4))
+	goto parse_err;
+    if (sudo_base64_decode(msg->log_id, uuid, sizeof(uuid)) != sizeof(uuid))
+	goto parse_err;
+    if (sudo_uuid_to_string(uuid, uuid_str, sizeof(uuid_str)) == NULL) {
+	parse_err:
+	sudo_warnx(U_("%s: %s"), __func__, U_("unable to parse log_id"));
+	closure->errstr = _("unable to open journal file");
+	debug_return_bool(false);
     }
+
+    /* Journal files are stored by UUID. */
     len = snprintf(journal_path, sizeof(journal_path), "%s/incoming/%s",
-	logsrvd_conf_relay_dir(), cp);
+	logsrvd_conf_relay_dir(), uuid_str);
     if (len >= ssizeof(journal_path)) {
 	errno = ENAMETOOLONG;
-	sudo_warn("%s/incoming/%s", logsrvd_conf_relay_dir(), cp);
+	sudo_warn("%s/incoming/%s", logsrvd_conf_relay_dir(), uuid_str);
 	closure->errstr = _("unable to open journal file");
 	debug_return_bool(false);
     }
@@ -498,7 +517,19 @@ journal_accept(const AcceptMessage *msg, const uint8_t *buf, size_t len,
 
     if (msg->expect_iobufs) {
 	/* Send log ID to client for restarting connections. */
-	if (!fmt_log_id_message(closure->journal_path, closure))
+	unsigned char uuid[16];
+	const char *uuid_str;
+
+	/* Journaled I/O log files are files stored by UUID. */
+	uuid_str = strrchr(closure->journal_path, '/');
+	if (uuid_str == NULL)
+	    debug_return_bool(false);
+	uuid_str++;
+
+	/* Parse UUID and format as a log ID (base64-encoded). */
+	if (sudo_uuid_from_string(uuid_str, uuid) != 0)
+	    debug_return_bool(false);
+	if (!fmt_log_id_message(uuid, "", closure))
 	    debug_return_bool(false);
 	if (sudo_ev_add(closure->evbase, closure->write_ev,
 		logsrvd_conf_server_timeout(), false) == -1) {