From c952c6ece28b6c0f774f823c917f458fe3424993 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 18 Jun 2010 23:12:48 +0200 Subject: [PATCH 1/1] service: add minimal access control logic for notifcation socket --- fixme | 2 -- src/load-fragment.c | 6 ++++ src/manager.c | 17 +++-------- src/manager.h | 2 ++ src/service.c | 73 +++++++++++++++++++++++++++++++++++++++++---- src/service.h | 13 ++++++++ src/unit.h | 2 +- 7 files changed, 93 insertions(+), 22 deletions(-) diff --git a/fixme b/fixme index 792647b93..29a3c11bc 100644 --- a/fixme +++ b/fixme @@ -63,8 +63,6 @@ * abstract namespace dbus socket -* discuss NOTIFY_SOCKET, make it configurable? security implications? - Regularly: * look for close() vs. close_nointr() vs. close_nointr_nofail() diff --git a/src/load-fragment.c b/src/load-fragment.c index 7a51bf81c..88fedceea 100644 --- a/src/load-fragment.c +++ b/src/load-fragment.c @@ -1204,6 +1204,8 @@ finish: return r; } +DEFINE_CONFIG_PARSE_ENUM(config_parse_notify_access, notify_access, NotifyAccess, "Failed to parse notify access specifier"); + #define FOLLOW_MAX 8 static int open_follow(char **filename, FILE **_f, Set *names, char **_final) { @@ -1360,6 +1362,9 @@ static void dump_items(FILE *f, const ConfigItem *items) { { config_parse_description, "DESCRIPTION" }, { config_parse_timer, "TIMER" }, { config_parse_timer_unit, "NAME" }, + { config_parse_path_spec, "PATH" }, + { config_parse_path_unit, "UNIT" }, + { config_parse_notify_access, "ACCESS" } }; assert(f); @@ -1491,6 +1496,7 @@ static int load_from_path(Unit *u, const char *path) { { "KillMode", config_parse_kill_mode, &u->service.kill_mode, "Service" }, { "NonBlocking", config_parse_bool, &u->service.exec_context.non_blocking, "Service" }, { "BusName", config_parse_string, &u->service.bus_name, "Service" }, + { "NotifyAccess", config_parse_notify_access, &u->service.notify_access, "Service" }, EXEC_CONTEXT_CONFIG_ITEMS(u->service.exec_context, "Service"), { "ListenStream", config_parse_listen, &u->socket, "Socket" }, diff --git a/src/manager.c b/src/manager.c index 5e627ba9c..c2d5e5f0e 100644 --- a/src/manager.c +++ b/src/manager.c @@ -70,7 +70,6 @@ static int manager_setup_notify(Manager *m) { struct sockaddr_un un; } sa; struct epoll_event ev; - char *ne[2], **t; int one = 1; assert(m); @@ -106,19 +105,9 @@ static int manager_setup_notify(Manager *m) { if (epoll_ctl(m->epoll_fd, EPOLL_CTL_ADD, m->notify_watch.fd, &ev) < 0) return -errno; - if (asprintf(&ne[0], "NOTIFY_SOCKET=@%s", sa.un.sun_path+1) < 0) + if (!(m->notify_socket = strdup(sa.un.sun_path+1))) return -ENOMEM; - ne[1] = NULL; - t = strv_env_merge(2, m->environment, ne); - free(ne[0]); - - if (!t) - return -ENOMEM; - - strv_free(m->environment); - m->environment = t; - return 0; } @@ -451,6 +440,8 @@ void manager_free(Manager *m) { if (m->notify_watch.fd >= 0) close_nointr_nofail(m->notify_watch.fd); + free(m->notify_socket); + lookup_paths_free(&m->lookup_paths); strv_free(m->environment); @@ -1672,7 +1663,7 @@ static int manager_process_notify_fd(Manager *m) { log_debug("Got notification message for unit %s", u->meta.id); if (UNIT_VTABLE(u)->notify_message) - UNIT_VTABLE(u)->notify_message(u, tags); + UNIT_VTABLE(u)->notify_message(u, ucred->pid, tags); strv_free(tags); } diff --git a/src/manager.h b/src/manager.h index 762a891ca..6c3434e5a 100644 --- a/src/manager.h +++ b/src/manager.h @@ -125,6 +125,8 @@ struct Manager { Hashmap *watch_pids; /* pid => Unit object n:1 */ + char *notify_socket; + Watch notify_watch; Watch signal_watch; diff --git a/src/service.c b/src/service.c index 905eadb98..abd2a6d7c 100644 --- a/src/service.c +++ b/src/service.c @@ -850,6 +850,9 @@ static int service_load(Unit *u) { if ((r = unit_watch_bus_name(u, s->bus_name)) < 0) return r; } + + if (s->type == SERVICE_NOTIFY && s->notify_access == NOTIFY_NONE) + s->notify_access = NOTIFY_MAIN; } return service_verify(s); @@ -873,13 +876,15 @@ static void service_dump(Unit *u, FILE *f, const char *prefix) { "%sRootDirectoryStartOnly: %s\n" "%sValidNoProcess: %s\n" "%sKillMode: %s\n" - "%sType: %s\n", + "%sType: %s\n" + "%sNotifyAccess: %s\n", prefix, service_state_to_string(s->state), prefix, yes_no(s->permissions_start_only), prefix, yes_no(s->root_directory_start_only), prefix, yes_no(s->valid_no_process), prefix, kill_mode_to_string(s->kill_mode), - prefix, service_type_to_string(s->type)); + prefix, service_type_to_string(s->type), + prefix, notify_access_to_string(s->notify_access)); if (s->control_pid > 0) fprintf(f, @@ -1245,13 +1250,14 @@ static int service_spawn( bool pass_fds, bool apply_permissions, bool apply_chroot, + bool set_notify_socket, pid_t *_pid) { pid_t pid; int r; int *fds = NULL; unsigned n_fds = 0; - char **argv; + char **argv = NULL, **env = NULL; assert(s); assert(c); @@ -1276,11 +1282,29 @@ static int service_spawn( goto fail; } + if (set_notify_socket) { + char *t; + + if (asprintf(&t, "NOTIFY_SOCKET=@%s", s->meta.manager->notify_socket) < 0) { + r = -ENOMEM; + goto fail; + } + + env = strv_env_set(s->meta.manager->environment, t); + free(t); + + if (!env) { + r = -ENOMEM; + goto fail; + } + } else + env = s->meta.manager->environment; + r = exec_spawn(c, argv, &s->exec_context, fds, n_fds, - s->meta.manager->environment, + env, apply_permissions, apply_chroot, UNIT(s)->meta.manager->confirm_spawn, @@ -1288,6 +1312,12 @@ static int service_spawn( &pid); strv_free(argv); + argv = NULL; + + if (set_notify_socket) + strv_free(env); + env = NULL; + if (r < 0) goto fail; @@ -1309,6 +1339,11 @@ static int service_spawn( fail: free(fds); + strv_free(argv); + + if (set_notify_socket) + strv_free(env); + if (timeout) unit_unwatch_timer(UNIT(s), &s->timer_watch); @@ -1395,6 +1430,7 @@ static void service_enter_stop_post(Service *s, bool success) { false, !s->permissions_start_only, !s->root_directory_start_only, + false, &s->control_pid)) < 0) goto fail; @@ -1493,6 +1529,7 @@ static void service_enter_stop(Service *s, bool success) { false, !s->permissions_start_only, !s->root_directory_start_only, + false, &s->control_pid)) < 0) goto fail; @@ -1537,6 +1574,7 @@ static void service_enter_start_post(Service *s) { false, !s->permissions_start_only, !s->root_directory_start_only, + false, &s->control_pid)) < 0) goto fail; @@ -1571,6 +1609,7 @@ static void service_enter_start(Service *s) { true, true, true, + s->notify_access != NOTIFY_NONE, &pid)) < 0) goto fail; @@ -1630,6 +1669,7 @@ static void service_enter_start_pre(Service *s) { false, !s->permissions_start_only, !s->root_directory_start_only, + false, &s->control_pid)) < 0) goto fail; @@ -1677,6 +1717,7 @@ static void service_enter_reload(Service *s) { false, !s->permissions_start_only, !s->root_directory_start_only, + false, &s->control_pid)) < 0) goto fail; @@ -1711,6 +1752,7 @@ static void service_run_next(Service *s, bool success) { false, !s->permissions_start_only, !s->root_directory_start_only, + false, &s->control_pid)) < 0) goto fail; @@ -2218,12 +2260,24 @@ static void service_cgroup_notify_event(Unit *u) { } } -static void service_notify_message(Unit *u, char **tags) { +static void service_notify_message(Unit *u, pid_t pid, char **tags) { Service *s = SERVICE(u); const char *e; assert(u); + if (s->notify_access == NOTIFY_NONE) { + log_warning("%s: Got notification message from PID %lu, but reception is disabled.", + u->meta.id, (unsigned long) pid); + return; + } + + if (s->notify_access == NOTIFY_MAIN && pid != s->main_pid) { + log_warning("%s: Got notification message from PID %lu, but reception only permitted for PID %lu", + u->meta.id, (unsigned long) pid, (unsigned long) s->main_pid); + return; + } + log_debug("%s: Got message", u->meta.id); /* Interpret MAINPID= */ @@ -2232,7 +2286,6 @@ static void service_notify_message(Unit *u, char **tags) { s->state == SERVICE_START_POST || s->state == SERVICE_RUNNING || s->state == SERVICE_RELOAD)) { - pid_t pid; if (parse_pid(e + 8, &pid) < 0) log_warning("Failed to parse %s", e); @@ -2545,6 +2598,14 @@ static const char* const service_exec_command_table[_SERVICE_EXEC_COMMAND_MAX] = DEFINE_STRING_TABLE_LOOKUP(service_exec_command, ServiceExecCommand); +static const char* const notify_access_table[_NOTIFY_ACCESS_MAX] = { + [NOTIFY_NONE] = "none", + [NOTIFY_MAIN] = "main", + [NOTIFY_ALL] = "all" +}; + +DEFINE_STRING_TABLE_LOOKUP(notify_access, NotifyAccess); + const UnitVTable service_vtable = { .suffix = ".service", diff --git a/src/service.h b/src/service.h index 521baaa1f..7b85771a3 100644 --- a/src/service.h +++ b/src/service.h @@ -76,12 +76,22 @@ typedef enum ServiceExecCommand { _SERVICE_EXEC_COMMAND_INVALID = -1 } ServiceExecCommand; +typedef enum NotifyAccess { + NOTIFY_NONE, + NOTIFY_ALL, + NOTIFY_MAIN, + _NOTIFY_ACCESS_MAX, + _NOTIFY_ACCESS_INVALID = -1 +} NotifyAccess; + struct Service { Meta meta; ServiceType type; ServiceRestart restart; + NotifyAccess notify_access; + /* If set we'll read the main daemon PID from this file */ char *pid_file; @@ -147,4 +157,7 @@ ServiceType service_type_from_string(const char *s); const char* service_exec_command_to_string(ServiceExecCommand i); ServiceExecCommand service_exec_command_from_string(const char *s); +const char* notify_access_to_string(NotifyAccess i); +NotifyAccess notify_access_from_string(const char *s); + #endif diff --git a/src/unit.h b/src/unit.h index 1f8874f4c..3397d472c 100644 --- a/src/unit.h +++ b/src/unit.h @@ -286,7 +286,7 @@ struct UnitVTable { void (*cgroup_notify_empty)(Unit *u); /* Called whenever a process of this unit sends us a message */ - void (*notify_message)(Unit *u, char **tags); + void (*notify_message)(Unit *u, pid_t pid, char **tags); /* Called whenever a name thus Unit registered for comes or * goes away. */ -- 2.30.2