summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSimon McVittie <smcv@debian.org>2017-07-21 10:46:39 +0100
committerSimon McVittie <smcv@debian.org>2017-07-28 11:17:04 +0100
commit8d8b5eb8a409031e224d088c1e3385bc2874c873 (patch)
treee5d65c5a9012ec0f6b09c4d0ff62f564ed18e0a7
parent962bfdd9929918e298c7640c4434f4697ed5bbea (diff)
downloaddbus-8d8b5eb8a409031e224d088c1e3385bc2874c873.tar.gz
config-loader-expat: Tell Expat not to defend against hash collisions
By default, Expat uses cryptographic-quality random numbers as a salt for its hash algorithm, and since 2.2.1 it gets them from the getrandom syscall on Linux. That syscall refuses to return any entropy until the kernel's CSPRNG (random pool) has been initialized. Unfortunately, this can take as long as 40 seconds on embedded devices with few entropy sources, which is too long: if the system dbus-daemon blocks for that length of time, important D-Bus clients like systemd and systemd-logind time out and fail to connect to it. We're parsing small configuration files here, and we trust them completely, so we don't need to defend against hash collisions: nobody is going to be crafting them to cause pathological performance. Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101858 Tested-by: Christopher Hewitt <hewitt@ieee.org> [smcv: Adjust build-system changes for 1.11.x] Signed-off-by: Simon McVittie <smcv@debian.org> Reviewed-by: Philip Withnall <withnall@endlessm.com>
-rw-r--r--bus/config-loader-expat.c14
-rw-r--r--configure.ac8
2 files changed, 22 insertions, 0 deletions
diff --git a/bus/config-loader-expat.c b/bus/config-loader-expat.c
index f59942e3..6cbd37eb 100644
--- a/bus/config-loader-expat.c
+++ b/bus/config-loader-expat.c
@@ -203,6 +203,20 @@ bus_config_load (const DBusString *file,
goto failed;
}
+ /* We do not need protection against hash collisions (CVE-2012-0876)
+ * because we are only parsing trusted XML; and if we let Expat block
+ * waiting for the CSPRNG to be initialized, as it does by default to
+ * defeat CVE-2012-0876, it can cause timeouts during early boot on
+ * entropy-starved embedded devices.
+ *
+ * TODO: When Expat gets a more explicit API for this than
+ * XML_SetHashSalt, check for that too, and use it preferentially.
+ * https://github.com/libexpat/libexpat/issues/91 */
+#if defined(HAVE_XML_SETHASHSALT)
+ /* Any nonzero number will do. https://xkcd.com/221/ */
+ XML_SetHashSalt (expat, 4);
+#endif
+
if (!_dbus_string_get_dirname (file, &dirname))
{
dbus_set_error (error, DBUS_ERROR_NO_MEMORY, NULL);
diff --git a/configure.ac b/configure.ac
index 9797035f..05182d1f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -935,6 +935,14 @@ AC_SUBST(DBUS_PATH_OR_ABSTRACT)
PKG_CHECK_MODULES([EXPAT], [expat])
+save_cflags="$CFLAGS"
+save_libs="$LIBS"
+CFLAGS="$CFLAGS $EXPAT_CFLAGS"
+LIBS="$LIBS $EXPAT_LIBS"
+AC_CHECK_FUNCS([XML_SetHashSalt])
+CFLAGS="$save_cflags"
+LIBS="$save_libs"
+
# Thread lib detection
AC_ARG_VAR([THREAD_LIBS])
save_libs="$LIBS"