It seems the security team was no longer assigned, but we knew this was coming so I performed the security team MIR.
I reviewed xdg-dbus-proxy 0.1.1-1 as checked into eoan. This shouldn't be considered a full audit but rather a quick gauge of maintainability. xdg-dbus-proxy is a "filtering proxy for D-Bus connections". It is invoked via the command line line with arguments that specify the address to proxy and the path for client connections. It will listen on a unix domain socket for clients to connect. For each client connection, it opens a new connection to the proxied address, forwarding data between the two. xdg-dbus-proxy operates in two modes, filtered and unfiltered (ie, forward all messages). With filtering mode, policy is applied to determine which messages to allow and drop. Filtering is applied to signals and method calls from the client and broadcast signals from the proxied address. Replies (errors and method returns) are allowed for the outstanding call, but not otherwise (this is analogous to how dbus-daemon works with LSM policy like AppArmor). Policy is simple, yet flexible and aggregates various common DBus APIs into SEE, TALK and OWN policy levels (listed from lowest to highest, with each higher level implying lower levels) that are specified in rules for use with well-known DBus names (ie, *not* private/unique names). The default policy allows TALKing to the bus itself (org.freedesktop.DBus) and to its own unique ID. Other clients are invisible (this is not unlike the dbus-strict and dbus-session-strict AppArmor abstractions). Policy rules allow a glob syntax for suffixes such that org.foo.* matches org.foo, org.foo.bar, org.foo.bar.baz but not org.norf. While the rule is specified for the well-known name, the rule will also apply such that the policy for the private/unique DBus name of the client is the union of all rules for well-known names that the client owns. where the highest level wins. In addition to these policy levels, it is possible to also specify interface and object paths with method calls/signals on well-known names (similar to fine-grained AppArmor rules). xdg-dbus-proxy is a lowlevel application intended to be driven by a higher level application (eg, bubblewrap, flatpak (which drives bubblewrap, etc). It does not support policy files and typical usage is to proxy, for example, the session bus for an application, with all rules specified on the command line (bwrap typically uses the --args option, which allows passing nul-separated arguments via a file descriptor). Eg (showing full args instead of the normal --args usage): $ xdg-dbus-proxy --fd=26 unix:path=/run/usr/1000/bus /run/usr/1000/.dbus-proxy/session-bus-proxy --filter --own=org.gnome.ghex.* --talk=ca.desrt.dconf --call=org.freedesktop.portal.*=* such that different applications will each have their own proxy. bwrap usage shows sockets in /run/user/<uid>/.dbus-proxy/session-bus-proxy-* where there is a named socket allocated for each application that corresponds to to the (now deleted) --args fd. xdg-dbus-proxy does support proxying the system bus (eg, xdg-dbus-proxy unix:path=/run/dbus/system_bus_socket /tmp/foo). Additionally, with typical bwrap usage, xdg-dbus-proxy isn't going to be invoked as root as it is intended for use in user sessions. This architecture also seems reasonable. The rules language, syntax and xdg-dbus-proxy invocation looks fine from a design POV. Due to time constraints, I did not review the implementation for enforcement correctness. I did some blackbox testing like so: $ sudo apt-get install flatpak gnome-software-plugin-flatpak d-feet $ flatpak remote-add --if-not-exists flathub https://flathub.org/repo/flathub.flatpakrepo $ flatpak install flathub org.gnome.ghex $ flatpak install flathub org.gnome.Logs $ flatpak install flathub org.wireshark.Wireshark $ flatpak run org.gnome.ghex & $ flatpak run org.gnome.Logs & $ flatpak run org.wireshark.Wireshark & $ xdg-dbus-proxy unix:path=/run/dbus/system_bus_socket /tmp/test-system & # unfiltered $ xdg-dbus-proxy unix:path=/run/dbus/system_bus_socket /tmp/test-session & # unfiltered $ d-feet # 'Connect to other bus', choose unix:path=/tmp/test-system and see system bus $ d-feet # 'Connect to other bus', choose unix:path=/tmp/test-session and see session bus $ ls -l /run/user/1000/.dbus-proxy/ total 0 srwxr-xr-x 1 jamie jamie 0 May 10 17:32 a11y-bus-proxy-A5PT1Z srwxr-xr-x 1 jamie jamie 0 May 10 16:35 a11y-bus-proxy-ORKL1Z srwxr-xr-x 1 jamie jamie 0 May 10 16:36 a11y-bus-proxy-PZFD1Z srwxr-xr-x 1 jamie jamie 0 May 10 16:35 session-bus-proxy-9IQL1Z srwxr-xr-x 1 jamie jamie 0 May 10 16:36 session-bus-proxy-EZ9C1Z srwxr-xr-x 1 jamie jamie 0 May 10 17:32 session-bus-proxy-KHRT1Z (interesting that these all end with '1Z', but this would be in bwrap) $ d-feet # 'Connect to other bus', choose unix:path=/run/user/1000 /.dbus-proxy/session-bus-proxy-9IQL1Z In this last example, we see only what ghex is allowed (verified by comparing d-feet with $ flatpak info --show-permissions org.gnome.ghex, eg, it has dconf and a few other things, but not the whole session). Do the same with Logs and wireshark (which doesn't have dconf which is correct per flatpak info). Poked around and indeed other access are denied. - CVE History: - CVE-2018-6560 (when still part of flatpak proper) which was fixed in https://github.com/flatpak/flatpak/commit/52346bf187b5a7f1c0fe9075b328b7ad6abe78f6. This was fixed in a timely manner, though the code changes were a little more than expected because it was updated to match the authentication phase of the dbus-daemon code. The CVE demonstrates the fragility of the proxy wrt dbus-daemon protocols and the proxy curiously failed open rather than closed. - Build-Depends - nothing terribly interesting: libglib2.0-dev, xsltproc, and dbus - pre/post inst/rm scripts - None - init scripts - None - systemd units - None - dbus services - None - setuid binaries - None - binaries in PATH - /usr/bin/xdg-dbus-proxy - sudo fragments - None - udev rules - None - unit tests / autopkgtests - unit: source has unit tests in ./tests and they are enabled during the build. Unit tests are shallow - autopkgtest: packaging provides xdg-dbus-proxy-tests which is callable via gnome-desktop-testing-runner. Ships autopkgtests that call these tests in this manner. This is simply the shallow test-proxy from the unit tests - cron jobs - None - Build logs: - no warnings or errors - there are lintian warnings and errors but they aren't security relevant: W: xdg-dbus-proxy source: debhelper-compat-file-is-missing W: xdg-dbus-proxy source: package-uses-deprecated-debhelper-compat-version 1 E: xdg-dbus-proxy source: package-uses-debhelper-but-lacks-build-depends E: xdg-dbus-proxy source: missing-build-dependency debhelper W: xdg-dbus-proxy source: newer-standards-version 4.3.0 (current is 4.1.4) - Processes spawned - None - Memory management - this isn't exhaustive but a spot check on code quality - uses g_* from glib primarily - it does implement its own buffer_new() convenience function which itself uses g_malloc0 and a g_assert() prior to a memcpy(). There is the magic '16' value in buffer_new() that corresponds to the guchar 'data' field and this magic value is used in several places in flatpak-proxy.c. The handling of this data field in the struct seems a bit clever, but not wrong AFAICS. - another use of memcpy() in message_to_buffer() is fine since it happens after the call to buffer_new() - the final use of memcpy() happens after use of the glib MIN macro with a resizing via glib functions as needed after - File IO - None - Logging - just via standard glib functions - Environment variable usage - None in main code; test-proxy.c uses g_getenv() and correctly checks for NULL - Use of privileged functions - None - Use of cryptography / random number sources etc - None - Use of temp files - Only during build via autotools - Use of networking - no networking - all local unix domain sockets (see flatpak_proxy_start() which uses g_unix_socket_address_new()). Communication with DBus happen via g_dbus_*. - Use of WebKit - None - Use of PolicyKit - None - Any significant cppcheck results - None - Any significant Coverity results - None In terms of code quality, the source is easy to read and coded defensively. The use of glib is nice. Spot-checking revealed nothing shocking. parse_generic_args() in dbus-proxy.c doesn't do range checks with strtol and returns an int rather than a long, but the subsequent checks are sufficient since if LONG_MAX is returned, fd is negative and the program aborts. In general, xdg-dbus-proxy is using glib which guards against a number of common problems. Also see Memory Management, above. The xdg-dbus-proxy has man page with a nice overview of how it operates. Blackbox testing confirms the design though I wouldn't have minded more time to review the policy and enforcement code (but the code quality of what I looked at and the activity from upstream demonstrates it is suitable for main). The CVE gives me some concern since the proxy failed open in the face of unexpected data in the authentication phase. It was updated to match upstream dbus-daemon. dbus-daemon and its implementation of the dbus protocol is very mature, but I'm a bit worried that changes in dbus might introduce other issues when the proxy and the dbus daemon have different ideas on the protocol. The proxy was updated in a timely manner and I don't expect the protocol to change. While I have concern, considering upstream's reaction to the issue, that concern does not outweigh the code quality on the whole or the benefits of using xdg-dbus-proxy via bwrap to harden the massive webkit2gtk code base. Security team ACK for promoting xdg-dbus-proxy to main. It would be nice if the unit and autopkgtests were expanded since there are a lot of testing opportunities, but this is not a requirement for promotion. ** CVE added: https://cve.mitre.org/cgi-bin/cvename.cgi?name=2018-6560 -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/1811824 Title: [MIR] xdg-dbus-proxy To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/xdg-dbus-proxy/+bug/1811824/+subscriptions -- ubuntu-bugs mailing list [email protected] https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
