From ea1935803984398f6b5aeda23c75e1eb472fd543 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Wed, 24 Feb 2021 11:36:34 +0000 Subject: [PATCH 1/6] gdbusprivate: Simplify some variable initialisations Signed-off-by: Philip Withnall --- gio/gdbusprivate.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/gio/gdbusprivate.c b/gio/gdbusprivate.c index 4e42c1a4d..70c098582 100644 --- a/gio/gdbusprivate.c +++ b/gio/gdbusprivate.c @@ -2470,11 +2470,10 @@ _g_dbus_get_machine_id (GError **error) return res; #else - gchar *ret; - GError *first_error; + gchar *ret = NULL; + GError *first_error = NULL; + /* TODO: use PACKAGE_LOCALSTATEDIR ? */ - ret = NULL; - first_error = NULL; if (!g_file_get_contents ("/var/lib/dbus/machine-id", &ret, NULL, From daa62a35e15a794623167b860505b9b28b37dddc Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Wed, 24 Feb 2021 11:49:47 +0000 Subject: [PATCH 2/6] gdbusprivate: Validate machine ID after loading it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It’s unlikely that the machine ID will be invalid (it’s system configuration), but it would be helpful to not propagate invalid IDs further, since a lot of things rely on it. It’s not easy to test this (it requires factoring out the code so it can be used from a test program, or allowing it to load a machine ID from a custom path), so I haven’t added unit tests. I’ve tested manually by overriding the loaded machine ID. Coverity CID: #1430944 Signed-off-by: Philip Withnall --- gio/gdbusprivate.c | 40 +++++++++++++++++++++++++++++++++------- 1 file changed, 33 insertions(+), 7 deletions(-) diff --git a/gio/gdbusprivate.c b/gio/gdbusprivate.c index 70c098582..8bb52227a 100644 --- a/gio/gdbusprivate.c +++ b/gio/gdbusprivate.c @@ -2472,6 +2472,8 @@ _g_dbus_get_machine_id (GError **error) #else gchar *ret = NULL; GError *first_error = NULL; + gsize i; + gboolean non_zero = FALSE; /* TODO: use PACKAGE_LOCALSTATEDIR ? */ if (!g_file_get_contents ("/var/lib/dbus/machine-id", @@ -2483,17 +2485,41 @@ _g_dbus_get_machine_id (GError **error) NULL, NULL)) { - g_propagate_prefixed_error (error, first_error, + g_propagate_prefixed_error (error, g_steal_pointer (&first_error), _("Unable to load /var/lib/dbus/machine-id or /etc/machine-id: ")); + return NULL; } - else + + /* ignore the error from the first try, if any */ + g_clear_error (&first_error); + + /* Validate the machine ID. From `man 5 machine-id`: + * > The machine ID is a single newline-terminated, hexadecimal, 32-character, + * > lowercase ID. When decoded from hexadecimal, this corresponds to a + * > 16-byte/128-bit value. This ID may not be all zeros. + */ + for (i = 0; ret[i] != '\0' && ret[i] != '\n'; i++) { - /* ignore the error from the first try, if any */ - g_clear_error (&first_error); - /* TODO: validate value */ - g_strstrip (ret); + /* Break early if it’s invalid. */ + if (!g_ascii_isxdigit (ret[i]) || g_ascii_isupper (ret[i])) + break; + + if (ret[i] != '0') + non_zero = TRUE; } - return ret; + + if (i != 32 || ret[i] != '\n' || ret[i + 1] != '\0' || !non_zero) + { + g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_FAILED, + "Invalid machine ID in /var/lib/dbus/machine-id or /etc/machine-id"); + g_free (ret); + return NULL; + } + + /* Strip trailing newline. */ + ret[32] = '\0'; + + return g_steal_pointer (&ret); #endif } From 05ff2f877ca301096106991c4f455519b5695cad Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Wed, 24 Feb 2021 11:58:49 +0000 Subject: [PATCH 3/6] gdbusprivate: Stop hard-coding path to /var/lib This will require distributions to ensure they pass `--localstatedir=/var` correctly to Meson, but they should be doing that already. See https://mesonbuild.com/Builtin-options.html#directories for details about how Meson treats `localstatedir` differently from most other `dir` variables. Signed-off-by: Philip Withnall --- gio/gdbusprivate.c | 19 +++++++++++++------ gio/meson.build | 1 + meson.build | 6 ++++++ 3 files changed, 20 insertions(+), 6 deletions(-) diff --git a/gio/gdbusprivate.c b/gio/gdbusprivate.c index 8bb52227a..282678f3b 100644 --- a/gio/gdbusprivate.c +++ b/gio/gdbusprivate.c @@ -2475,18 +2475,24 @@ _g_dbus_get_machine_id (GError **error) gsize i; gboolean non_zero = FALSE; - /* TODO: use PACKAGE_LOCALSTATEDIR ? */ - if (!g_file_get_contents ("/var/lib/dbus/machine-id", + /* Copy what dbus.git does: allow the /var/lib path to be configurable at + * build time, but hard-code the system-wide machine ID path in /etc. */ + const gchar *var_lib_path = LOCALSTATEDIR "/lib/dbus/machine-id"; + const gchar *etc_path = "/etc/machine-id"; + + if (!g_file_get_contents (var_lib_path, &ret, NULL, &first_error) && - !g_file_get_contents ("/etc/machine-id", + !g_file_get_contents (etc_path, &ret, NULL, NULL)) { g_propagate_prefixed_error (error, g_steal_pointer (&first_error), - _("Unable to load /var/lib/dbus/machine-id or /etc/machine-id: ")); + /* Translators: Both placeholders are file paths */ + _("Unable to load %s or %s: "), + var_lib_path, etc_path); return NULL; } @@ -2510,8 +2516,9 @@ _g_dbus_get_machine_id (GError **error) if (i != 32 || ret[i] != '\n' || ret[i + 1] != '\0' || !non_zero) { - g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_FAILED, - "Invalid machine ID in /var/lib/dbus/machine-id or /etc/machine-id"); + g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, + "Invalid machine ID in %s or %s", + var_lib_path, etc_path); g_free (ret); return NULL; } diff --git a/gio/meson.build b/gio/meson.build index 2bf1e1973..397882f75 100644 --- a/gio/meson.build +++ b/gio/meson.build @@ -2,6 +2,7 @@ gio_c_args = [ '-DG_LOG_DOMAIN="GLib-GIO"', '-DGIO_COMPILATION', '-DGIO_MODULE_DIR="@0@"'.format(glib_giomodulesdir), + '-DLOCALSTATEDIR="@0@"'.format(glib_localstatedir), ] gio_c_args += glib_hidden_visibility_args diff --git a/meson.build b/meson.build index 2e3acbef6..816ff21c0 100644 --- a/meson.build +++ b/meson.build @@ -87,6 +87,12 @@ else glib_charsetaliasdir = glib_libdir endif +glib_localstatedir = get_option('localstatedir') +if not glib_localstatedir.startswith('/') + # See https://mesonbuild.com/Builtin-options.html#directories + glib_localstatedir = join_paths(glib_prefix, glib_localstatedir) +endif + installed_tests_metadir = join_paths(glib_datadir, 'installed-tests', meson.project_name()) installed_tests_execdir = join_paths(glib_libexecdir, 'installed-tests', meson.project_name()) installed_tests_enabled = get_option('installed_tests') From 62cc3158e907fc051c37d808e407d1a5dc6611d3 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Wed, 24 Feb 2021 13:16:57 +0000 Subject: [PATCH 4/6] ci: Set localstatedir to the system directory on CI machines MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit So the tests can access `/var/lib/dbus/machine-id`. This is not a behaviour change relative to older behaviour on CI. In future, it might make more sense to revert this commit and change the CI scripts so they symlink `/home/user/glib-installed/var/lib/dbus/machine-id` to the system machine ID; or ensure that `/etc/machine-id` exists on all the CI machines. That’s too complicated to do right now though. Signed-off-by: Philip Withnall --- .gitlab-ci.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index e79a66767..cc61edf1e 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -74,6 +74,7 @@ fedora-x86_64: --werror --default-library=both --prefix=$HOME/glib-installed + --localstatedir=/var --libdir=lib -Dsystemtap=true -Ddtrace=true @@ -117,6 +118,7 @@ debian-stable-x86_64: --werror --default-library=both --prefix=$HOME/glib-installed + --localstatedir=/var --libdir=lib -Dsystemtap=true -Ddtrace=true @@ -461,6 +463,7 @@ scan-build: --werror --default-library=both --prefix=$HOME/glib-installed + --localstatedir=/var --libdir=lib -Dsystemtap=true -Ddtrace=true @@ -487,6 +490,7 @@ coverity: --werror --default-library=both --prefix=$HOME/glib-installed + --localstatedir=/var --libdir=lib -Dsystemtap=true -Ddtrace=true From da50de9b30d73e73ac4d6a62f671d24d9471c944 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Mon, 1 Mar 2021 13:54:09 +0000 Subject: [PATCH 5/6] ci: Include details of machine ID in CI output MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Include the size of the `machine-id` file, but not the value itself as that is sensitive for non-throwaway machines. What’s most useful for debugging CI problems is knowing whether, and where, the `machine-id` is set. Signed-off-by: Philip Withnall --- .gitlab-ci/show-execution-environment.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.gitlab-ci/show-execution-environment.sh b/.gitlab-ci/show-execution-environment.sh index 0c2265161..5075f9730 100755 --- a/.gitlab-ci/show-execution-environment.sh +++ b/.gitlab-ci/show-execution-environment.sh @@ -9,3 +9,5 @@ setpriv --dump || : ulimit -a || : cat /proc/self/status || : cat /proc/self/mountinfo || : +stat /etc/machine-id || : +stat /var/lib/dbus/machine-id || : From ef41cc28b465a620663e4e0034bbbf50e1fc63d6 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Mon, 1 Mar 2021 13:55:03 +0000 Subject: [PATCH 6/6] ci: Ensure the machine-id is set on the Fedora CI image MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Run `systemd-machine-id-setup` when creating the image, so that `/etc/machine-id` is created with a valid ID. Since systemd isn’t started when running the CI image with podman/Docker, it’s not created otherwise. This causes some tests to fail. Signed-off-by: Philip Withnall --- .gitlab-ci.yml | 2 +- .gitlab-ci/fedora.Dockerfile | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index cc61edf1e..e6b29dd04 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -10,7 +10,7 @@ cache: - _ccache/ variables: - FEDORA_IMAGE: "registry.gitlab.gnome.org/gnome/glib/fedora:v9" + FEDORA_IMAGE: "registry.gitlab.gnome.org/gnome/glib/fedora:v10" COVERITY_IMAGE: "registry.gitlab.gnome.org/gnome/glib/coverity:v1" DEBIAN_IMAGE: "registry.gitlab.gnome.org/gnome/glib/debian-stable:v7" ANDROID_IMAGE: "registry.gitlab.gnome.org/gnome/glib/android-ndk:v3" diff --git a/.gitlab-ci/fedora.Dockerfile b/.gitlab-ci/fedora.Dockerfile index 39553e280..139d87fb8 100644 --- a/.gitlab-ci/fedora.Dockerfile +++ b/.gitlab-ci/fedora.Dockerfile @@ -1,5 +1,8 @@ FROM fedora:31 +# Set /etc/machine-id as it’s needed for some D-Bus tests +RUN systemd-machine-id-setup + RUN dnf -y update \ && dnf -y install \ bindfs \