1. 17 Apr, 2019 1 commit
  2. 02 Aug, 2018 1 commit
    • Simon McVittie's avatar
      sysdeps: Reassure gcc 8 that we are not overflowing struct sockaddr_un · 4937a36c
      Simon McVittie authored
      Using strncpy (buffer, str, strlen (str)) is a "code smell" that
      might indicate a serious bug (it effectively turns strncpy into
      strcpy), and gcc 8 now warns about it. In fact we avoided the bug
      here, but it wasn't at all obvious.
      
      We already checked that path_len is less than or equal to
      _DBUS_MAX_SUN_PATH_LENGTH, which is 99, chosen to be strictly less
      than the POSIX minimum sizeof(sun_path) >= 100, so we couldn't
      actually be overflowing the available buffer.
      
      The new static assertion in this commit matches a comment above the
      definition of _DBUS_MAX_SUN_PATH_LENGTH: we define
      _DBUS_MAX_SUN_PATH_LENGTH to 99, because POSIX says struct
      sockaddr_un's sun_path member is at least 100 bytes (including space
      for a \0 terminator). dbus will now fail to compile on
      platforms that are non-POSIX-compliant in this way, except for Windows.
      
      We zeroed the struct sockaddr_un before writing into it, so stopping
      one byte short of the end of sun_path ensures that we get \0
      termination.
      Signed-off-by: Simon McVittie's avatarSimon McVittie <smcv@collabora.com>
      Bug: https://bugs.freedesktop.org/show_bug.cgi?id=107350Reviewed-by: Thiago Macieira's avatarThiago Macieira <thiago@kde.org>
      Reviewed-by: Philip Withnall's avatarPhilip Withnall <withnall@endlessm.com>
      (cherry picked from commit f4296313)
      4937a36c
  3. 04 Jun, 2018 1 commit
  4. 24 Nov, 2017 1 commit
  5. 29 Sep, 2017 1 commit
  6. 25 Sep, 2017 1 commit
  7. 15 Aug, 2017 2 commits
  8. 28 Jul, 2017 1 commit
    • Simon McVittie's avatar
      userdb: Respect $HOME for the home directory of our own uid · 3f377c51
      Simon McVittie authored
      This lets cooperating processes with the same value of $HOME
      interoperate for DBUS_COOKIE_SHA1 by reading and writing $HOME, even
      if their $HOME differs from the uid's "official" home directory
      according to getpwuid(). Out of paranoia, we only do this if the uid
      and the euid are equal, since if they were unequal the correct thing
      to do would be ambiguous.
      
      In particular, Debian autobuilders run as a user whose "official"
      home directory in /etc/passwd is "/nonexistent", as a mechanism to
      detect non-deterministic build processes that rely on the contents of
      the home directory. Until now, this meant we couldn't run dbus'
      build-time tests, because every test that used DBUS_COOKIE_SHA1 would
      fail in this environment.
      
      In the tests, set HOME as well as DBUS_TEST_HOMEDIR. We keep
      DBUS_TEST_HOMEDIR too, because Windows doesn't use HOME, only HOMEDRIVE
      and HOMEPATH.
      
      Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101960
      Bug-Debian: https://bugs.debian.org/630152Signed-off-by: default avatarSimon McVittie <smcv@debian.org>
      Reviewed-by: Philip Withnall's avatarPhilip Withnall <withnall@endlessm.com>
      3f377c51
  9. 05 Jul, 2017 1 commit
  10. 08 Jun, 2017 2 commits
    • Simon McVittie's avatar
      Make _dbus_get_local_machine_uuid_encoded() properly failable · 6f751caf
      Simon McVittie authored
      This function already raised an error, and all callers handled that
      error as gracefully as they could (because _dbus_generate_uuid() is
      failable, since 2015). Given that, it seems unnecessarily hostile
      to do a _dbus_warn_check_failed() unless we have no better alternative:
      yes, it indicates that dbus has not been installed correctly, but
      during build-time tests it's entirely reasonable that dbus has not
      yet been installed.
      
      Callers are:
      
      * DBusConnection, to implement Peer.GetMachineId()
      * The bus driver, to implement Peer.GetMachineId()
      * X11 autolaunching
      * dbus_get_local_machine_id()
      
      Of those, only the last one is not in a position to return an error
      gracefully, so move the _dbus_warn_check_failed() to there.
      
      Migrate the text about the D-Bus library being incorrectly set up
      into the error emitted by the Unix implementation, and to make it
      less misleading, include separate error messages for both the
      files we try to read:
      
      $ bwrap --ro-bind / / --dev /dev --tmpfs /etc --tmpfs /var \
        ./tools/dbus-uuidgen --get
      D-Bus library appears to be incorrectly set up: see the manual
      page for dbus-uuidgen to correct this issue. (Failed to open
      "/var/lib/dbus/machine-id": No such file or directory; Failed to open
      "/etc/machine-id": No such file or directory)
      Signed-off-by: Simon McVittie's avatarSimon McVittie <smcv@collabora.com>
      Reviewed-by: Philip Withnall's avatarPhilip Withnall <withnall@endlessm.com>
      Bug: https://bugs.freedesktop.org/show_bug.cgi?id=13194
      6f751caf
    • Simon McVittie's avatar
      Unix sysdeps: Only copy /etc/machine-id to ${sysconfdir} in "ensure" mode · bde40c89
      Simon McVittie authored
      System integration scripts use dbus-uuidgen --ensure, so they are
      unaffected by this, and in particular the solution to Bug #77941
      is still valid.
      
      The shared library is typically loaded by unprivileged users, so
      trying to write out the machine-id file is not going to work anyway.
      However, if we *can* write to our ${sysconfdir} - notably during
      `make distcheck` - then it is unexpected that merely reading the
      machine ID has the side-effect of writing to ${sysconfdir},
      and in particular it will make the check for a complete uninstall
      fail. We definitely must not delete the machine ID during
      `make uninstall`.
      Signed-off-by: Simon McVittie's avatarSimon McVittie <smcv@collabora.com>
      Reviewed-by: Philip Withnall's avatarPhilip Withnall <withnall@endlessm.com>
      Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101257
      bde40c89
  11. 07 Apr, 2017 1 commit
  12. 16 Feb, 2017 1 commit
    • Simon McVittie's avatar
      Change _dbus_create_directory to fail for existing directories · be51bfe9
      Simon McVittie authored
      If we don't trap EEXIST and its Windows equivalent, we are unable to
      detect the situation where we create an ostensibly unique
      subdirectory in a shared /tmp, but an attacker has already created it.
      This affects dbus-nonce (the nonce-tcp transport) and the activation
      reload test.
      
      Add a new _dbus_ensure_directory() for the one case where we want it to
      succeed even on EEXIST: the DBUS_COOKIE_SHA1 keyring, which we know
      we are creating in our own trusted "official" $HOME. In the new
      transient service support on Bug #99825, ensure_owned_directory()
      would need the same treatment.
      
      We are not treating this as a serious security problem, because the
      nonce-tcp transport is rarely enabled on Unix and there are multiple
      mitigations.
      
      The nonce-tcp transport creates a new unique file with O_EXCL and 0600
      (private to user) permissions, then overwrites the requested filename
      via atomic-overwrite, so the worst that could happen there is that an
      attacker could place a symbolic link matching the name of a directory
      we are going to create, causing a dbus-daemon configured for nonce-tcp
      to traverse the symlink and atomically overwrite a file named "nonce"
      in a directory of the attacker's choice, with new random contents that
      are not known to the attacker. This seems unlikely to be exploitable
      for anything worse than denial of service in practice. In mainline
      Linux since 3.6, this attack is also defeated by the
      fs.protected_symlinks sysctl, which many distributions enable by default.
      
      The activation reload test suffers from a classic symlink attack
      due to time-of-check/time-of-use errors in its implementation, but as
      part of the developer-only "embedded tests" that are only intended
      to be run on a trusted machine, it is not treated as security-sensitive.
      That code path will be fixed in a subsequent commit.
      
      Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99828Signed-off-by: default avatarSimon McVittie <simon.mcvittie@collabora.co.uk>
      Reviewed-by: Philip Withnall's avatarPhilip Withnall <withnall@endlessm.com>
      be51bfe9
  13. 13 Oct, 2016 3 commits
  14. 30 Sep, 2016 2 commits
    • Simon McVittie's avatar
      Remove trailing newlines from _dbus_warn, _dbus_warn_check_failed · f1cd229f
      Simon McVittie authored
      They used to be needed, but are not needed any more, and we were
      never completely consistent about including them in any case.
      Signed-off-by: default avatarSimon McVittie <smcv@debian.org>
      f1cd229f
    • Simon McVittie's avatar
      _dbus_logv: configurably log to syslog and/or stderr · 92bd5ef2
      Simon McVittie authored
      This changes the behaviour of _dbus_logv() if _dbus_init_system_log() was
      not called. Previously, _dbus_logv() would always log to syslog;
      additionally, it would log to stderr, unless the process is dbus-daemon
      and it was started by systemd. Now, it will log to stderr only,
      unless _dbus_init_system_log() was called first.
      
      This is the desired behaviour because when we hook up
      _dbus_warn_check_failed() to _dbus_logv() in the next commit, we don't
      want typical users of libdbus to start logging their check failures to
      syslog - we only want the dbus-daemon to do that.
      
      In practice this is not usually a behaviour change, because there was
      only one situation in which we called _dbus_logv() without first calling
      _dbus_init_system_log(), namely an error while parsing configuration
      files. Initialize the system log "just in time" in that situation
      to preserve existing behaviour.
      Signed-off-by: default avatarSimon McVittie <smcv@debian.org>
      92bd5ef2
  15. 12 Aug, 2016 1 commit
  16. 11 Aug, 2016 3 commits
  17. 25 Jul, 2016 2 commits
  18. 09 May, 2016 1 commit
    • Lennart Poettering's avatar
      sysdeps: increase listen() backlog of AF_UNIX sockets to SOMAXCONN · 12bd6e89
      Lennart Poettering authored
      Previously, the listen() backlog was set to an arbitrary 30. This means
      that if dbus-daemon is overloaded only 30 more connections may be queued
      by the kernel, before connect() fails with EAGAIN. (Note that EAGAIN !=
      EINPROGRESS -- the latter is what is returned if a connection is queued
      and being processed for asynchronous sockets; EAGAIN in this case is
      really an error, that cannot be recovered from).
      
      Most software simply sets SOMAXCONN as backlog for AF_UNIX sockets, to
      allow queuing of as many connections as the kernel allows. SOMAXCONN is
      128 on Linux, which is not particularly high, but at least higher than
      30.
      
      This patch changes dbus-daemon to do the same.
      
      I noticed this when flooding dbus-daemon with a lot of connections,
      where it pretty quickly ceased to respond, much earlier than it really
      should.
      
      Note that the backlog has nothing to do with the number of concurrent
      connections allowed, it simply controls how many queued, but not
      accept()ed connections there may be on the listening socket.
      
      https://bugs.freedesktop.org/show_bug.cgi?id=95264
      12bd6e89
  19. 12 Feb, 2016 2 commits
  20. 11 Feb, 2016 3 commits
  21. 06 Aug, 2015 1 commit
  22. 14 May, 2015 2 commits
  23. 12 May, 2015 5 commits
  24. 24 Mar, 2015 1 commit