|Summary:||/uid-permissions/uae/root test fails with 1.10.4|
|Product:||dbus||Reporter:||Iain Lane <iain>|
|Component:||core||Assignee:||Simon McVittie <smcv>|
|Status:||RESOLVED FIXED||QA Contact:||D-Bus Maintainers <dbus>|
|i915 platform:||i915 features:|
|Attachments:||uid-permissions test: don't assert that root can UpdateActivationEnvironment|
Description Iain Lane 2015-11-20 10:50:52 UTC
We run Debian's autopkgtests, which contain this test, as part of our upload gating process in Ubuntu. This caught the fact that the test fails. I reproduced it in a clean (enough) unstable chroot against Debian's dbus package. (sid-amd64)root@nightingale:/dbus-1.10.4# DBUS_TEST_DATA=/usr/lib/dbus-1.0/installed-tests/dbus/data /usr/lib/dbus-1.0/installed-tests/dbus/test-uid-permissions /uid-permissions/uae/root: ** ERROR:../../test/uid-permissions.c:129:test_uae: assertion failed (dbus_message_get_type (m) == DBUS_MESSAGE_TYPE_METHOD_RETURN): (3 == 2) I suppose it's related to the UAE change in 1.10.4, but I didn't try reverting the patch to verify.
Comment 1 Simon McVittie 2015-11-20 17:06:24 UTC
Unfortunately I don't routinely run the tests as root for the upstream release process, I don't currently have a pre-upload autopkgtest environment, and ci.debian.net is sufficiently overloaded that dbus only gets tested once a month. Hopefully I'll eventually set up some sort of buildbot that runs autopkgtest on git master and the stable branch, although to do that, the Debian packaging might need some adjustment. Yes, the regression is that the test relies on being able to call UpdateActivation as root or messagebus on a bus where we have an activation helper, which isn't useful (because the activation helper discards its environment anyway) and is no longer allowed since 1.10.4. We'll need to change the test to assert that it fails as nobody, but not assert that it succeeds as root or messagebus. We could use Monitoring.BecomeMonitor() as the function that we assert is allowed for root. The reason that multi-user.conf sets an activation helper (it's /bin/false) is that I wanted to rule out the possibility of arbitrary code execution (via activation) on machines where that test is run. I've considered adding a command line option to disable traditional activation (so all activation must go via systemd, which we can control). That would address that issue.
Comment 2 Simon McVittie 2015-11-20 17:20:32 UTC
Created attachment 119997 [details] [review] uid-permissions test: don't assert that root can UpdateActivationEnvironment Since 1.10.4 this is hard-coded to be disallowed when an activation helper is used. That would be a security flaw waiting to happen, and makes little sense anyway, because the activation helper sanitises its environment. Use BecomeMonitor() instead, as our way to assert that root and messagebus are privileged.
Comment 3 Iain Lane 2015-11-23 13:16:22 UTC
Comment on attachment 119997 [details] [review] uid-permissions test: don't assert that root can UpdateActivationEnvironment Review of attachment 119997 [details] [review]: ----------------------------------------------------------------- It works for me and seems sensible when comparing it to test_uae (although I'm not very familiar with the code). The two tests have a fair bit in common, might be worth factoring some bits out, but not that important. Thanks, I'm going to upload this to Ubuntu.
Comment 4 Simon McVittie 2015-11-24 01:23:54 UTC
Fixed in git for 1.10.6 and 1.11.0, thanks. (In reply to Iain Lane from comment #3) > The two tests have a fair bit in > common, might be worth factoring some bits out, but not that important. I'd be happy to review a patch (also for any other test coverage or simplicity improvements).