Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A couple of test tweaks #350

Merged
merged 3 commits into from
Mar 13, 2024
Merged

A couple of test tweaks #350

merged 3 commits into from
Mar 13, 2024

Conversation

mrc0mmand
Copy link
Contributor

This gets rid of two meson deprecation warnings and also makes most (*) of the reference tests work with newer dbus-daemon (F39 and Rawhide), that started using path-based sockets instead of abstract ones.

(*)

$ meson test -C build --suite 'dbus-daemon(1)' --print-errorlogs                                                                                                                            
ninja: Entering directory `/home/fsumsal/repos/dbus-broker/build'                                                                                                                           
[7/7] Linking target test/dbus/test-fdstream                                                                                                                                                
1/5 dbus-broker:dbus-daemon(1) / FD Stream Constraints        SKIP            0.01s   exit status 77
2/5 dbus-broker:dbus-daemon(1) / Broker API                   OK              0.01s                                                                                                         
3/5 dbus-broker:dbus-daemon(1) / Client Lifetime              OK              0.02s           
4/5 dbus-broker:dbus-daemon(1) / Signals and Matches          OK              0.03s                                                                                                         
dbus-broker:dbus-daemon(1) / Driver API time out (After 30 seconds)                                                                                                                         
5/5 dbus-broker:dbus-daemon(1) / Driver API                   TIMEOUT        30.01s   killed by signal 6 SIGABRT      

The last test fails in this test case:

{
_c_cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL;
_c_cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL;
util_broker_connect(broker, &bus);
r = sd_bus_call_method(bus, "org.freedesktop.DBus", "/org/freedesktop/DBus", "org.freedesktop.DBus",
"GetConnectionCredentials", NULL, &reply,
"s", "org.freedesktop.DBus");
c_assert(r >= 0);
test_verify_credentials(reply);
}

because there's no UnixGroupIDs sent back. I can reproduce this on latest Fedora Rawhide if I switch from dbus-broker to dbus-daemon and issue the D-Bus call directly:

# rpm -q dbus-daemon
dbus-daemon-1.14.10-3.fc40.x86_64
# busctl call org.freedesktop.DBus /org/freedesktop/DBus org.freedesktop.DBus GetConnectionCredentials s org.freedesktop.DBus
a{sv} 2 "ProcessID" u 582 "UnixUserID" u 81

Doing the same with dbus-broker seems to return "expected" results:

# rpm -q dbus-broker
dbus-broker-35-4.fc40.x86_64
# busctl call org.freedesktop.DBus /org/freedesktop/DBus org.freedesktop.DBus GetConnectionCredentials s org.freedesktop.DBus
a{sv} 5 "UnixUserID" u 0 "ProcessID" u 578 "UnixGroupIDs" au 1 0 "LinuxSecurityLabel" ay 48 115 121 115 116 101 109 95 117 58 115 121 115 116 101 109 95 114 58 115 121 115 116 101 109 95 100 98 117 115 100 95 116 58 115 48 45 115 48 58 99 48 46 99 49 48 50 51 0 "ProcessFD" h 4

I'm not sure if this is a bug or not (or at least I couldn't find anything obvious in dbus-daemon's release notes, apart from maybe this bugfix that's in v1.15.8).

get_pkgconfig_variable() was deprecated in 0.56.0.

test/dbus/meson.build:76: WARNING: Project targets '>=0.60.0' but uses feature deprecated since '0.56.0':
    dependency.get_pkgconfig_variable. use dependency.get_variable(pkgconfig : ...) instead
Newer meson doesn't like colons in test names:

test/dbus/meson.build:78: DEPRECATION: ":" is not allowed in test name "dbus-daemon(1): Connection", it has been replaced with "_"
test/dbus/meson.build:79: DEPRECATION: ":" is not allowed in test name "dbus-daemon(1): Message passing", it has been replaced with "_"
...

Let's instead separate the reference tests/benchmarks into their own
suite. This also makes running just the reference tests easier by using
meson test's --suite= argument:

$ meson test -C build --list --suite 'dbus-daemon(1)'
dbus-broker:dbus-daemon(1) / Broker API
dbus-broker:dbus-daemon(1) / Driver API
dbus-broker:dbus-daemon(1) / FD Stream Constraints
dbus-broker:dbus-daemon(1) / Client Lifetime
dbus-broker:dbus-daemon(1) / Signals and Matches
dbus-daemon v1.15.2 switched from using abstract sockets to path-based
sockets. Let's tweak the reference tests to support both cases.
@teg
Copy link
Contributor

teg commented Mar 13, 2024

Thanks for this @mrc0mmand !

Do you have any idea if the remaining failure is a regression (I assume so, but have not tested this manually in a very long time)?

@smcv do you happen to know if this is expected?

@teg teg merged commit 0cdc6e0 into bus1:main Mar 13, 2024
37 checks passed
@smcv
Copy link

smcv commented Mar 13, 2024

dbus-daemon 1.14.10 should be able to implement UnixGroupIDs if the stars align appropriately: it needs to have been compiled with a new enough glibc and run on a new enough kernel, and the getsockopt needs to not be prevented (for example by SELinux or whatever). It includes the same bug fix for a valid-but-empty supplementary group list as 1.15.8.

The fact that you're also not seeing a LinuxSecurityLabel in your GetConnectionCredentials call suggests that either this is not an apples-to-apples comparison, or something else is going wrong, because LinuxSecurityLabel should have been available on systems with SELinux for a long time.

@mrc0mmand
Copy link
Contributor Author

mrc0mmand commented Mar 13, 2024

Thanks for this @mrc0mmand !

Do you have any idea if the remaining failure is a regression (I assume so, but have not tested this manually in a very long time)?

I did some more testing and can reproduce it even with dbus-daemon-1.15.8-1.fc41.x86_64 (after building it locally, with SELinux in permissive mode):

# busctl call org.freedesktop.DBus /org/freedesktop/DBus org.freedesktop.DBus GetConnectionCredentials s org.freedesktop.DBus
a{sv} 3 "ProcessID" u 601 "UnixUserID" u 81 "ProcessFD" h 4

But the same stuff happens on Arch Linux, again after switching from dbus-broker to dbus-daemon (here the lack of LinuxSecurityLabel is expected, since Arch doesn't do SELinux by default):

# pacman -Q dbus-daemon-units
dbus-daemon-units 1.14.10-2
# busctl call org.freedesktop.DBus /org/freedesktop/DBus org.freedesktop.DBus GetConnectionCredentials s org.freedesktop.DBus
a{sv} 2 "ProcessID" u 370 "UnixUserID" u 81

With dbus-broker I get:

# pacman -Q dbus-broker-units
dbus-broker-units 35-2
# busctl call org.freedesktop.DBus /org/freedesktop/DBus org.freedesktop.DBus GetConnectionCredentials s org.freedesktop.DBus
a{sv} 4 "UnixUserID" u 0 "ProcessID" u 375 "UnixGroupIDs" au 1 0 "ProcessFD" h 4

@mrc0mmand mrc0mmand deleted the a-couple-of-test-tweaks branch March 13, 2024 18:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants