[MIR] dbus-broker

Bug #2015538 reported by Sebastien Bacher
This bug affects 1 person
Affects Status Importance Assigned to Milestone
dbus-broker (Ubuntu)

Bug Description

The package dbus-broker is already in Ubuntu universe.
The package dbus-broker build for the architectures it is designed to work on.
It currently builds and works for architetcures: amd64, arm64, armhf, i386, ppc64el, riscv64, s390x
Link to package https://launchpad.net/ubuntu/+source/dbus-broker

- The package dbus-broker is required in Ubuntu main to replace dbus-daemon.
- The package dbus-broker will generally from server to desktop.
- Package dbus-broker covers the same use case as dbus-daemon but is a better alternative for the reason described in https://dvdhrm.github.io/rethinking-the-dbus-message-bus/. Other distributions are using it for years, Fedora for example, https://fedoraproject.org/wiki/Changes/DbusBrokerAsTheDefaultDbusImplementation
- There is no other/better way to solve this that is already in main or
  should go universe->main instead of this.

- The package dbus-broker is required in Ubuntu main no later than august due to FF, ideally we would like land it earlier in the cycle

- Had 2 security issues in the past


Those reports seem to have been fixed in timelined fashion upstream. The issues are resolved in Ubuntu in series > Kinetic

- no `suid` or `sgid` binaries
- no executables in `/sbin` and `/usr/sbin`
- Package does install services, timers or recurring jobs

  The system unit use the following systemd security features

- Packages does not open privileged ports (ports < 1024)
- Packages does not contain extensions to security-sensitive software

[Quality assurance - function/usage]
- The package works well right after install

[Quality assurance - maintenance]
- The package does not deal with exotic hardware we cannot support

[Quality assurance - testing]
- The package runs a test suite on build time, if it fails
  it makes the build fail


Ok: 46
Expected Fail: 0
Fail: 0
Unexpected Pass: 0
Skipped: 0
Timeout: 0

- The package runs an autopkgtest, and is currently passing on
  amd64, arm64, armhf, i386, ppc64el, riscv64, s390x

- The package does have not failing autopkgtests right now
- The autopkgtest is the running the upstream testsuite so is not trivial

[Quality assurance - packaging]
- debian/watch is present and works

- debian/control defines a correct Maintainer since it's in sync from Debian

- The package has no lintian warnings
# lintian --pedantic

- Please link to a recent build log of the package

  `lintian --pedantic` as an extra post to this bug.

- Lintian overrides are present
# dbus-broker only supports systemd
dbus-broker: maintainer-script-calls-systemctl
dbus-broker: package-supports-alternative-init-but-no-init.d-script [lib/systemd/system/dbus-broker.service]
# need to override dh_installsystemd
dbus-broker: maintainer-script-empty [prerm]
dbus-broker: maintainer-script-ignores-errors [prerm]
# matches dbus-daemon package, activated by socket
dbus-broker: systemd-service-file-missing-install-key [lib/systemd/system/dbus-broker.service]

Those have to do with the fact that package is set to work only with systemd, that's not an issue in Ubuntu since we don't support alternative init systems anyway
Also the service shouldn't be stopped on package removal to avoid seeing the user session close

- This package does not rely on obsolete or about to be demoted packages.
- This package has no python2 or GTK2 dependencies

- The package will be installed by default, but does not ask debconf questions

- Packaging and build is easy, https://salsa.debian.org/utopia-team/dbus-broker/-/blob/debian/sid/debian/rules

[UI standards]
- Application is not end-user facing (does not need translation)

- No further depends or recommends dependencies that are not yet in main

[Standards compliance]
- This package correctly follows FHS and Debian Policy

- Owning Teams will be foundations and desktop
- desktop-packages is already subscribed to the package, we will get foundations added

- This does not use static builds
- This does not use vendored code
- This package is not rust based

- The package successfully built during the most recent test rebuild

[Background information]
The Package description explains the package well
Upstream Name is dbus-broker
Link to upstream project https://github.com/bus1/dbus-broker

The apparmor integration patch in review upstream on https://github.com/bus1/dbus-broker/pull/286 has got a +1 from our security team, we will include the change either by distro patching or through a newer upstream version since that's required for our confinement story, especially in snaps.

CVE References

description: updated
Changed in dbus-broker (Ubuntu):
assignee: nobody → Lukas Märdian (slyon)
Revision history for this message
Lukas Märdian (slyon) wrote :
Download full text (5.0 KiB)

Review for Package: src:dbus-broker

dbus-broker can be considered a drop-in replacement for dbus-daemon, the
reference implementation of the DBus protocol. It is supposed a more modern,
faster and safer alternative.

MIR team ACK

This does need a security review, so I'll assign ubuntu-security

List of specific binary packages to be promoted to main: dbus-broker
Specific binary packages built, but NOT to be promoted to main: None

- This needs security review because of data-structure parsing, out-of-tree
  patch (see below), usage of setuid & embedded sources (which can be considered
  part of the project, though).

Required TODOs:
- None

Recommended TODOs:
- consider dropping privileges via systemd unit configuration instead of setuid
- dbus-broker is supposed to be a drop in replacement; try copying/migrating
  some autopkgtests from src:dbus (especially the apparmor one, as we will
  carry some related modifications)

There is no other package in main providing the same functionality, BUT src:dbus
which we're trying replace.
Other (deprecated) alternatives include kdbus and bus1 kernel-space
implementations of the broker daemon or gdbus-daemon.

- no other Dependencies to MIR due to this
  - SRCPKG checked with `check-mir`
  - all dependencies can be found in `seeded-in-ubuntu` (already in main)
  - none of the (potentially auto-generated) dependencies (Depends
    and Recommends) that are present after build are not in main
- no -dev/-debug/-doc packages that need exclusion
- No dependencies in main that are only superficially tested requiring
  more tests now.

Problems: None

[Embedded sources and static linking]
- no static linking (but embedded subprojects, see below)
- does not have unexpected Built-Using entries
- not a go package, no extra constraints to consider in that regard
- not a rust package, no extra constraints to consider in that regard
- Includes vendored code, the package has documented how to refresh this
  code at: https://mesonbuild.com/Subprojects.html

- embedded source present (several tiny C utils, which are not otherwise
  part of the Ubuntu archive): https://github.com/c-util
$ tree -d subprojects/ | grep "[libc|subprojects]"
├── libcdvar-1
│   ├── src
│   └── subprojects
├── libcini-1
│   ├── src
│   └── subprojects
├── libclist-3
│   └── src
├── libcrbtree-3
│   ├── src
│   │   └── docs
│   └── subprojects
├── libcshquote-1
│   ├── src
│   └── subprojects
├── libcstdaux-1
│   └── src
│   └── docs
└── libcutf8-1
    ├── src
    └── subprojects
21 directories

But, quoting ./NEWS.md (CHANGES WITH v30):
          All subprojects are still statically linked and considered part of
          dbus-broker. Any critical update to any subproject will cause a new
          release of dbus-broker, as it always did. Distributions are not
          required to monitor the subprojects manually."

- history of CVEs does not look concerning
- does not run a daemon as root (it uses the "messagebus" user)
- does not use webkit1,2
- does not use lib*v8 directly
- does not process arbitrary web content
- does not use centralized...


Changed in dbus-broker (Ubuntu):
assignee: Lukas Märdian (slyon) → Ubuntu Security Team (ubuntu-security)
tags: added: sec-1969
Revision history for this message
Sebastien Bacher (seb128) wrote :

While starting to work on preparing more for that transition we found an issue than gdm currently depends on dbus-daemon because it requires dbus-run-session to start greeter sessions

there are discussion on the dbus-broker project about that gap

which also points to similar requirements in other components and desktops environment. It's unclear if we will be able to demote dbus-daemon / provide an equivalent solution in dbus-broker this cycle. Would be an acceptable option to maybe split dbus-run-session in its own binary built from dbus-daemon and keep installing that utility?

Revision history for this message
Lukas Märdian (slyon) wrote :

IMO the suggested solution is OK (for Mantic, as long as we're not doing it in an LTS). Though, the plan should still stay in place to get it fully transitioned to dbus-broker (demoting dbus-daemon) by the next LTS, so we don't have to maintain two DBus codebases for a long time.

The desktop team mentioned that they won't have capacity to re-implement 'dbus-run-session' as part of the dbus-broker codebase. So we need a decision between Foundations and Security if there's any capacity to get this done in Mantic, or alternatively maintain both 'dbus-daemon' and 'dbus-broker' packages for a longer time in parallel ...

If not the switch to dbus-broker might need to be dropped/postponed for the time being.

tags: added: rls-mm-incoming
Revision history for this message
Marc Deslauriers (mdeslaur) wrote (last edit ):

AFAIK, the dbus-run-session tool spawns dbus-daemon, so it wouldn't be possible to split it into its own binary package.

tags: added: rls-nn-incoming
removed: rls-mm-incoming
Revision history for this message
Mark Esler (eslerm) wrote (last edit ):
Download full text (5.1 KiB)

I partially reviewed dbus-broker 33-1 as checked into lunar. This shouldn't be considered a full audit but rather a quick gauge of maintainability.

> The dbus-broker project is an implementation of a message bus as defined by the D-Bus specification. Its aim is to provide high performance and reliability, while keeping compatibility to the D-Bus reference implementation. It is exclusively written for Linux systems, and makes use of many modern features provided by recent linux kernel releases.

- CVE History:
  - previous CVEs (CVE-2022-31212 and CVE-2022-31213) based on using untrusted input
    - https://sec-consult.com/vulnerability-lab/advisory/memory-corruption-vulnerabilities-dbus-broker/
    - upstream responded quickly to report
  - some security relevant commits
    - private disclosure is bad for downstream security maintenance
    - e.g., b08cc9dab51eab5a4b10c147ecd4cbd8efc238d6
 - upstream is using static analyzers and fuzzers with github actions \o/
 - upstream does not have a Security Policy
- Build-Depends?
  - debhelper-compat
  - libapparmor-dev
  - libaudit-dev
  - libcap-ng-dev
  - libdbus-1-dev
  - libexpat1-dev
  - libselinux1-dev
  - libsystemd-dev
  - linux-libc-dev
  - meson
  - pkg-config
  - python3-docutils
  - systemd
    - dbus-broker itself is not dependent on systemd, but dbus-broker-launch is
- pre/post inst/rm scripts?
  - adds or removes systemd services, reloads services, and requests reboots when needed
- init scripts?
  - none
- systemd units?
  - systemd service for main dbus-broker
  - owning team: check syslog for deprecation warnings that need to be fixed outside fo dbus-broker !
- dbus services?
  - minimizing global state and other design principals makes the security of dbus-broker more robust than dbus-daemon
    - see developers post https://dvdhrm.github.io/rethinking-the-dbus-message-bus/
- setuid binaries?
  - none
- binaries in PATH?
  - ./usr/bin/dbus-broker
  - ./usr/bin/dbus-broker-launch
- sudo fragments?
  - none
- polkit files?
  - none
- udev rules?
  - none
- unit tests / autopkgtests?
  - includes build tests
  - autopkgtests seem sparse
    - dbus programs like gnome-shell don't have autopkgtests
- cron jobs?
  - none
- Build logs:
  - looks okay

- Processes spawned?
  - only in launcher
    - for process and systemd
- Memory management?
  - **very** heavy use
  - cursory view looks o-k
- File IO?
  - use appears safe
- Logging?
  - written with logging mind
- Environment variable usage?
  - only XDG_* for launcher
- Use of privileged functions?
  - setuid/setgid/setgroups used to drop permissions
  - log_stream_send() contains a SIOCOUTQ ioctl for lossy D-Bus messages
- Use of cryptography / random number sources etc?
  - includes SASL wrapper to allow SASL D-Bus
- Use of temp files?
  - none
- Use of networking?
  - heavy socket use
  - heavily uses c-dvar for stream encoding/decoding D-Bus
    - https://github.com/c-util/c-dvar
    - same developer as dbus-broker
- Use of WebKit?
  - none
- Use of PolicyKit?
  - none

- Any significant cppcheck results?
  - nothing Coverity did not catch
- Any significant Coverity results?
  - reported upstream
  - upstream also runs https://sc...


Changed in dbus-broker (Ubuntu):
assignee: Ubuntu Security Team (ubuntu-security) → nobody
status: New → Incomplete
Revision history for this message
Mark Esler (eslerm) wrote (last edit ):

Desktop packaged AppArmor features in the mantic version of dbus-broker \o/

Using a fresh mantic vm with dbus-broker [0], I ran dfuzzer on every dbus service (dfuzzer -l) and collected crash reproducers.

In another fresh mantic vm, running the default dbus-daemon, I reproduced most of these crashes.

An unprivileged user can access ~20% of the dbus services that user 1000 can.

Neither dbus nor dbus-broker have AppArmor profiles (apparmor_status).

apparmor.d is a set of +1400 AppArmor profiles [1][2]. Among its features is confining dbus (i.e., dbus-daemon). apparmor.d mitigated most crashes when a profile was available.

Snap does not currently confine dbus.

Confinement of dbus is a pre-existing issue and does not affect MIR outcome.

[0] https://github.com/bus1/dbus-broker
[1] https://github.com/roddhjav/apparmor.d
[2] https://www.youtube.com/watch?v=OzyalrOzxE8

Revision history for this message
Mark Esler (eslerm) wrote :

I'm curious what the status is for writing a replacement wrapper to dbus-run-session.


Revision history for this message
Sebastien Bacher (seb128) wrote :

There is no upstream traction and I don't think we have the resources to work on that from the Canonical Desktop Team perspective so we are stuck there for now

Revision history for this message
Mark Esler (eslerm) wrote :

Thanks seb128

If you don't mind, I'll reach out to SEG to see if they have spare cycles.

Revision history for this message
Sebastien Bacher (seb128) wrote :

Please do, we would still like to do the switch if possible

Revision history for this message
Julian Andres Klode (juliank) wrote :

I'm inclined to move this to rls-oo-incoming in January.

Lukas Märdian (slyon)
tags: added: rls-oo-incoming
removed: rls-nn-incoming
Revision history for this message
Luca Boccassi (bluca) wrote :

I see that you are backporting the unmerged apparmor patch in a forked package - please feel free to send a MR on Salsa to add it https://salsa.debian.org/utopia-team/dbus-broker instead of forking the package.
That way we can keep the package in sync between Debian and Ubuntu.

Currently Ubuntu Noble is lagging behind, we have v35 in Debian. I'd really like to see v35 in Noble too, as it enables a new D-Bus interface for PIDFD tracking that we need in systemd.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

We were already convinced by the former rationale, just no team found enough time to complete it so far.
But to add the reasoning for the same as recently discussed in arch - https://gitlab.archlinux.org/archlinux/rfcs/-/blob/master/rfcs/0025-dbus-broker-default.rst

And @Luca - thanks for the offer to help, I hope someone gets in touch with you.

Revision history for this message
Lukas Märdian (slyon) wrote :

@seb128 mentioned that there's also some activity on implemeting a `dbus-broker-session` https://github.com/bus1/dbus-broker/pull/321

Revision history for this message
Luca Boccassi (bluca) wrote :

Thanks to Christian the apparmor patch is now in Debian too, would be good to get 35-2 synced to Noble in time for the freeze

Revision history for this message
Mark Esler (eslerm) wrote :

`dbus-run-session` needs to be implemented before `dbus` can be demoted and `dbus-broker` is promoted to main.

Vendored rust is not compatible with Debian, but it is allowed in Ubuntu packaging [0]. Possibly the current PR [1] can be used today. (`dbus-run-session` would need to be an alias for `dbus-broker-session`)

Having this promoted sooner than later is much better for testing, but _could_ be promoted between feature freeze and beta freeze.

[0] https://wiki.ubuntu.com/RustCodeInMain
[1] https://github.com/bus1/dbus-broker/pull/321

Revision history for this message
Luca Boccassi (bluca) wrote :

In Debian we just keep both, dbus-run-session from the src:dbus package works just fine, and it can be installed in parallel without any issues when somebody needs it. So it doesn't seem to me that you need that separate implementation?
The src:dbus package needs to remain in main anyway, as it provides the basic policy/configuration files/dependencies. This is by design.

Revision history for this message
Lukas Märdian (slyon) wrote :

I confirmed the delta in d/p/github_apparmor_support.patch is identical to what we have in Debian now and sync'ed the package.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Thank you Luca,
so a good next step for us would be to split the binary packages (if not already set up that way) to have one for dbus-run-session, one package for the policy/config/deps that you mentions (those two we'd keep in main), and one package (for universe) for the daemon that we want to demote.

Thanks sarnold for the suggestion.

Back to Seb128 I guess to consider this from the Desktop POV?
This looks much closer to being doable now AFAICS.

Revision history for this message
Mark Esler (eslerm) wrote :

@seb128, could you please review the recent discussion?

Revision history for this message
Sebastien Bacher (seb128) wrote :

Sorry for the lack of reply, I've been busy and now I'm out sick probably for a few days, anyway quick notes

The plan suggested sounds reasonable but

- the dbus-broker patch is still under review upstream and got not real world testing which means it's difficult to have confidence it's working to work without issues

- it is adding rust to the package which makes it more difficult to cherrypick/package/MIR

- the timeline doesn't feel realistic for the LTS, that's not a trivial change and not something we can land that late in the cycle especially when the archive is still not in working shape due to the time_t transition (which impacts our capacity to tests feature work that landed and get feedback)

We could do the package split but I don't see the point of rushing doing that work before the LTS if we don't do the switch, I would prefer for us to work with Debian on those changes and bring them back to Ubuntu next cycle

Does it make sense to others?

Revision history for this message
Mark Esler (eslerm) wrote :

Thank you @seb128. I was asked to get your feedback before completing the Security review. Get well soon!

Security team ACK for promoting dbus-broker to main, under the condition that src:dbus' binary packages are split as described by @paelzer in comment #19.

Revision history for this message
Lukas Märdian (slyon) wrote :

@seb128 IIUC comment #17, we do not necessarily need the new Rust implementation, but rather "just" split dbus-run-session out of the "dbus-daemon" binary package. So it can be installed individually and we can demote the (pure) dbus-daemon package to universe and replace it with dbus-broker in main.

Revision history for this message
Luca Boccassi (bluca) wrote :

Errata: I talked with smcv, who explained to me that dbus-run-session is actually a wrapper around dbus-daemon itself, so they are not independent.

With user sessions managed by logind&al, what is the use case for dbus-run-session in production? I am aware it is used for self-contained tests and such things, but using depending on the dbus-daemon package should be fine for that. Anything else?

Revision history for this message
Sebastien Bacher (seb128) wrote :

@Luca, thanks, that confirms what I've been told before and pointed out during the previous MIR meeting, dbus-run-session has an hard depends on dbus-daemon so the split idea isn't possible

About what makes us rely on dbus-run-session is currently gdm (which smcv also mentioned on IRC), basically

It does sound like we are blocked for now for that switch

Revision history for this message
Sebastien Bacher (seb128) wrote :

Copying some extra details from that IRC conversation

> until gdm learns to generate one system user per greeter on multi-seat systems, each with their own `systemd --user`, which I think has been the upstream plan for several years

Revision history for this message
Luca Boccassi (bluca) wrote :

IMHO it would be a worthwhile investment to allocate resources to implement that plan, as it would benefit desktop users for all cases, not just with dbus-broker

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.