From e9b43a695112a292e947208ad64d95f2996de468 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Mon, 16 Apr 2018 19:12:23 +0100 Subject: [PATCH 02/39] activation: Refuse to load activatable services with invalid names We aren't going to allow activating them anyway, so there's no point in having them in memory. (A secondary justification is that this makes my Containers1 work more straightforward, by making sure I don't have to handle the case where unique names, malformed names or org.freedesktop.DBus can be included in the entries hash table.) Signed-off-by: Simon McVittie --- bus/activation.c | 61 +++++++++++++++++++ bus/dispatch.c | 31 ++++++++++ bus/test.c | 7 +++ bus/test.h | 1 + cmake/test/CMakeLists.txt | 1 + test/Makefile.am | 4 ++ ...esktop.DBus.TestSuiteHasUniqueName.service | 3 + ...desktop.DBus.TestSuiteNotValidName.service | 3 + .../org.freedesktop.DBus.service | 3 + .../debug-allow-all-fail.conf.in | 15 +++++ 10 files changed, 129 insertions(+) create mode 100644 test/data/invalid-service-files/org.freedesktop.DBus.TestSuiteHasUniqueName.service create mode 100644 test/data/invalid-service-files/org.freedesktop.DBus.TestSuiteNotValidName.service create mode 100644 test/data/invalid-service-files/org.freedesktop.DBus.service create mode 100644 test/data/valid-config-files/debug-allow-all-fail.conf.in diff --git a/bus/activation.c b/bus/activation.c index 2a427d9c..519fff18 100644 --- a/bus/activation.c +++ b/bus/activation.c @@ -309,6 +309,25 @@ update_desktop_file_entry (BusActivation *activation, error)) goto out; + if (!dbus_validate_bus_name (name, error)) + goto out; + + if (name[0] == ':') + { + dbus_set_error (error, DBUS_ERROR_INVALID_ARGS, + "Bus name \"%s\" cannot be activatable because it " + "starts with ':'", name); + goto out; + } + + if (strcmp (name, DBUS_SERVICE_DBUS) == 0) + { + dbus_set_error (error, DBUS_ERROR_INVALID_ARGS, + "Bus name \"%s\" cannot be activatable because it " + "is reserved for the message bus", name); + goto out; + } + if (!bus_desktop_file_get_string (desktop_file, DBUS_SERVICE_SECTION, DBUS_SERVICE_EXEC, @@ -2771,4 +2790,46 @@ out: return ret; } +void +bus_activation_check_services (BusActivation *self) +{ + DBusHashIter iter; + + _dbus_hash_iter_init (self->entries, &iter); + + while (_dbus_hash_iter_next (&iter)) + { + const char *k; + BusActivationEntry *v; + + k = _dbus_hash_iter_get_string_key (&iter); + v = _dbus_hash_iter_get_value (&iter); + + if (k[0] == '\0') + _dbus_test_fatal ("Empty service name found"); + + if (k[0] == ':') + _dbus_test_fatal ("Service name \"%s\" is a unique name", k); + + if (strcmp (k, DBUS_SERVICE_DBUS) == 0) + _dbus_test_fatal ("dbus-daemon's service name found"); + + if (strcmp (k, v->name) != 0) + _dbus_test_fatal ("Mismatched service name found"); + + if (!dbus_validate_bus_name (k, NULL)) + _dbus_test_fatal ("Service name \"%s\" is invalid", k); + } + + _dbus_assert ( + _dbus_hash_table_lookup_string (self->entries, ":2.1") == NULL); + _dbus_assert ( + _dbus_hash_table_lookup_string (self->entries, "?!") == NULL); + _dbus_assert ( + _dbus_hash_table_lookup_string (self->entries, "") == NULL); + _dbus_assert ( + _dbus_hash_table_lookup_string (self->entries, + DBUS_SERVICE_DBUS) == NULL); +} + #endif /* DBUS_ENABLE_EMBEDDED_TESTS */ diff --git a/bus/dispatch.c b/bus/dispatch.c index c3914a86..3c4c031b 100644 --- a/bus/dispatch.c +++ b/bus/dispatch.c @@ -5017,6 +5017,32 @@ bus_dispatch_test_conf (const DBusString *test_data_dir, return TRUE; } +static dbus_bool_t +bus_dispatch_test_bad_services (const DBusString *test_data_dir, + const char *filename) +{ + BusContext *context; + + _dbus_test_diag ("%s:%s...", _DBUS_FUNCTION_NAME, filename); + + context = bus_context_new_test (test_data_dir, filename); + + if (context == NULL) + { + _dbus_test_not_ok ("%s:%s - bus_context_new_test() failed", + _DBUS_FUNCTION_NAME, filename); + return FALSE; + } + + /* All the real testing for these services is done when + * bus_context_new_test() calls bus_activation_check_services () */ + + bus_context_unref (context); + + _dbus_test_ok ("%s:%s", _DBUS_FUNCTION_NAME, filename); + return TRUE; +} + #ifndef DBUS_WIN static dbus_bool_t bus_dispatch_test_conf_fail (const DBusString *test_data_dir, @@ -5092,6 +5118,11 @@ bus_dispatch_test (const DBusString *test_data_dir) "valid-config-files/debug-allow-all.conf", FALSE)) return FALSE; + /* run select launch-helper activation tests on broken service files */ + if (!bus_dispatch_test_bad_services (test_data_dir, + "valid-config-files/debug-allow-all-fail.conf")) + return FALSE; + #ifndef DBUS_WIN /* run launch-helper activation tests */ _dbus_verbose ("Launch helper activation tests\n"); diff --git a/bus/test.c b/bus/test.c index 76960a30..dec17bd8 100644 --- a/bus/test.c +++ b/bus/test.c @@ -24,6 +24,7 @@ #include #ifdef DBUS_ENABLE_EMBEDDED_TESTS +#include "activation.h" #include "test.h" #include #include @@ -267,6 +268,7 @@ bus_context_new_test (const DBusString *test_data_dir, DBusString config_file; DBusString relative; BusContext *context; + BusActivation *activation; if (!_dbus_string_init (&config_file)) { @@ -309,6 +311,11 @@ bus_context_new_test (const DBusString *test_data_dir, _dbus_string_free (&config_file); + /* Self-test: Check that all services in the BusActivation are + * reasonable */ + activation = bus_context_get_activation (context); + bus_activation_check_services (activation); + return context; } diff --git a/bus/test.h b/bus/test.h index 38b74e89..bc1726cc 100644 --- a/bus/test.h +++ b/bus/test.h @@ -47,6 +47,7 @@ void bus_test_run_clients_loop (dbus_bool_t block); void bus_test_run_everything (BusContext *context); BusContext* bus_context_new_test (const DBusString *test_data_dir, const char *filename); +void bus_activation_check_services (BusActivation *self); #ifdef HAVE_UNIX_FD_PASSING dbus_bool_t bus_unix_fds_passing_test (const DBusString *test_data_dir); diff --git a/cmake/test/CMakeLists.txt b/cmake/test/CMakeLists.txt index c9ac41f4..baae4092 100644 --- a/cmake/test/CMakeLists.txt +++ b/cmake/test/CMakeLists.txt @@ -130,6 +130,7 @@ set (TESTDIRS test/data/invalid-config-files test/data/invalid-config-files-system test/data/invalid-messages + test/data/invalid-service-files test/data/invalid-service-files-system test/data/equiv-config-files test/data/equiv-config-files/basic diff --git a/test/Makefile.am b/test/Makefile.am index 5153ab6c..41575ffb 100644 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -442,6 +442,7 @@ in_data = \ data/valid-config-files-system/debug-allow-all-pass.conf.in \ data/valid-config-files/as-another-user.conf.in \ data/valid-config-files/count-fds.conf.in \ + data/valid-config-files/debug-allow-all-fail.conf.in \ data/valid-config-files/debug-allow-all-sha1.conf.in \ data/valid-config-files/debug-allow-all.conf.in \ data/valid-config-files/finite-timeout.conf.in \ @@ -509,6 +510,9 @@ static_data = \ data/invalid-config-files/truncated-file.conf \ data/invalid-config-files/send-and-receive.conf \ data/invalid-messages/boolean-has-no-value.message-raw \ + data/invalid-service-files/org.freedesktop.DBus.service \ + data/invalid-service-files/org.freedesktop.DBus.TestSuiteHasUniqueName.service \ + data/invalid-service-files/org.freedesktop.DBus.TestSuiteNotValidName.service \ data/sha-1/Readme.txt \ data/sha-1/bit-hashes.sha1 \ data/sha-1/bit-messages.sha1 \ diff --git a/test/data/invalid-service-files/org.freedesktop.DBus.TestSuiteHasUniqueName.service b/test/data/invalid-service-files/org.freedesktop.DBus.TestSuiteHasUniqueName.service new file mode 100644 index 00000000..62da306f --- /dev/null +++ b/test/data/invalid-service-files/org.freedesktop.DBus.TestSuiteHasUniqueName.service @@ -0,0 +1,3 @@ +[D-BUS Service] +Name=:2.1 +Exec=/bin/true :2.1 diff --git a/test/data/invalid-service-files/org.freedesktop.DBus.TestSuiteNotValidName.service b/test/data/invalid-service-files/org.freedesktop.DBus.TestSuiteNotValidName.service new file mode 100644 index 00000000..c530facb --- /dev/null +++ b/test/data/invalid-service-files/org.freedesktop.DBus.TestSuiteNotValidName.service @@ -0,0 +1,3 @@ +[D-BUS Service] +Name=?! +Exec=/bin/true ?! diff --git a/test/data/invalid-service-files/org.freedesktop.DBus.service b/test/data/invalid-service-files/org.freedesktop.DBus.service new file mode 100644 index 00000000..4d37f4c5 --- /dev/null +++ b/test/data/invalid-service-files/org.freedesktop.DBus.service @@ -0,0 +1,3 @@ +[D-BUS Service] +Name=org.freedesktop.DBus +Exec=/bin/true org.freedesktop.DBus diff --git a/test/data/valid-config-files/debug-allow-all-fail.conf.in b/test/data/valid-config-files/debug-allow-all-fail.conf.in new file mode 100644 index 00000000..396b180b --- /dev/null +++ b/test/data/valid-config-files/debug-allow-all-fail.conf.in @@ -0,0 +1,15 @@ + + + + + debug-pipe:name=test-server + @TEST_LISTEN@ + @DBUS_TEST_DATA@/invalid-service-files + + + + + + + -- 2.17.0