From 3690de9398011383efdb0a5ac462ef6689b75c22 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Fri, 20 Oct 2023 15:26:42 +0100 Subject: [PATCH 01/23] Revert "Disable the Containers interface" This reverts commit 9d60676ae08b5ff1153ca5cb2e42bb4d5ec563cf. --- CMakeLists.txt | 1 + README.cmake | 3 +++ bus/containers.c | 2 -- bus/driver.c | 4 ---- bus/session.conf.in | 6 ++++++ bus/system.conf.in | 4 ++++ cmake/config.h.cmake | 1 + dbus/dbus-shared.h | 2 ++ doc/dbus-daemon.1.xml.in | 8 ++++++++ test/containers.c | 2 -- test/data/valid-config-files/limit-containers.conf.in | 5 +++++ test/data/valid-config-files/tmp-session.conf.in | 7 +++++++ test/dbus-daemon.c | 4 ++++ 13 files changed, 41 insertions(+), 8 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index d3ec71be..82c15ce0 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -124,6 +124,7 @@ endif() option(DBUS_DISABLE_ASSERT "Disable assertion checking" OFF) option(DBUS_ENABLE_STATS "enable bus daemon usage statistics" OFF) +option(DBUS_ENABLE_CONTAINERS "enable restricted servers for app-containers" OFF) option(ENABLE_TRADITIONAL_ACTIVATION "Enable traditional activation (without using systemd)" ON) find_package(PkgConfig) diff --git a/README.cmake b/README.cmake index 03e5f27b..2cc4be93 100644 --- a/README.cmake +++ b/README.cmake @@ -149,6 +149,9 @@ ENABLE_QT_HELP:STRING=AUTO // enable bus daemon usage statistics DBUS_ENABLE_STATS:BOOL=OFF +// enable restricted servers for app containers +DBUS_ENABLE_CONTAINERS:BOOL=OFF + // build with systemd at_console support ENABLE_SYSTEMD:STRING=AUTO diff --git a/bus/containers.c b/bus/containers.c index 816f3e1b..aa3116e4 100644 --- a/bus/containers.c +++ b/bus/containers.c @@ -29,8 +29,6 @@ #ifdef DBUS_ENABLE_CONTAINERS -#error This feature is not ready for production use - #ifndef DBUS_UNIX # error DBUS_ENABLE_CONTAINERS requires DBUS_UNIX #endif diff --git a/bus/driver.c b/bus/driver.c index ebd98015..c6bff89d 100644 --- a/bus/driver.c +++ b/bus/driver.c @@ -1970,9 +1970,7 @@ bus_driver_fill_connection_credentials (DBusCredentials *credentials, dbus_pid_t pid = DBUS_PID_UNSET; const char *windows_sid = NULL; const char *linux_security_label = NULL; -#ifdef DBUS_ENABLE_CONTAINERS const char *path; -#endif #ifdef HAVE_UNIX_FD_PASSING int pid_fd = -1; /* owned by credentials */ #endif @@ -2033,7 +2031,6 @@ bus_driver_fill_connection_credentials (DBusCredentials *credentials, return FALSE; } -#ifdef DBUS_ENABLE_CONTAINERS /* This has to come from the connection, not the credentials */ if (peer_conn != NULL && bus_containers_connection_is_contained (peer_conn, &path, NULL, NULL)) @@ -2043,7 +2040,6 @@ bus_driver_fill_connection_credentials (DBusCredentials *credentials, path)) return FALSE; } -#endif #ifdef HAVE_UNIX_FD_PASSING if (caller_conn != NULL && pid_fd >= 0 && diff --git a/bus/session.conf.in b/bus/session.conf.in index affa7f1d..ace073c9 100644 --- a/bus/session.conf.in +++ b/bus/session.conf.in @@ -76,5 +76,11 @@ 50000 50000 50000 + 10000 + 10000 + 1000000000 + + 16 diff --git a/bus/system.conf.in b/bus/system.conf.in index d2f3244b..7c79a1ae 100644 --- a/bus/system.conf.in +++ b/bus/system.conf.in @@ -126,6 +126,10 @@ + + + + diff --git a/cmake/config.h.cmake b/cmake/config.h.cmake index 1cf57286..0575fc75 100644 --- a/cmake/config.h.cmake +++ b/cmake/config.h.cmake @@ -41,6 +41,7 @@ #cmakedefine DBUS_RUNSTATEDIR "@DBUS_RUNSTATEDIR@" #cmakedefine DBUS_ENABLE_STATS +#cmakedefine DBUS_ENABLE_CONTAINERS #cmakedefine ENABLE_TRADITIONAL_ACTIVATION #define TEST_LISTEN "@TEST_LISTEN@" diff --git a/dbus/dbus-shared.h b/dbus/dbus-shared.h index 87c0bd84..59b33e94 100644 --- a/dbus/dbus-shared.h +++ b/dbus/dbus-shared.h @@ -88,6 +88,8 @@ typedef enum */ /** The interface exported by the object with #DBUS_SERVICE_DBUS and #DBUS_PATH_DBUS */ #define DBUS_INTERFACE_DBUS "org.freedesktop.DBus" +/** The restricted container interface exported by the dbus-daemon */ +#define DBUS_INTERFACE_CONTAINERS1 "org.freedesktop.DBus.Containers1" /** The monitoring interface exported by the dbus-daemon */ #define DBUS_INTERFACE_MONITORING "org.freedesktop.DBus.Monitoring" diff --git a/doc/dbus-daemon.1.xml.in b/doc/dbus-daemon.1.xml.in index ae9b5aa3..e3ce49b0 100644 --- a/doc/dbus-daemon.1.xml.in +++ b/doc/dbus-daemon.1.xml.in @@ -840,6 +840,14 @@ Available limit names are: (number of calls-in-progress) "reply_timeout" : milliseconds (thousandths) until a method call times out + "max_containers" : max number of restricted servers for use + in app-containers, in total + "max_containers_per_user" : max number of app-containers per Unix uid + "max_container_metadata_bytes": max number of bytes of metadata to store + for each app-container + "max_connections_per_container": max number of (authenticated or + unauthenticated) connections to each + app-container diff --git a/test/containers.c b/test/containers.c index 79636460..f5bf5407 100644 --- a/test/containers.c +++ b/test/containers.c @@ -47,8 +47,6 @@ #include "test-utils-glib.h" -#define DBUS_INTERFACE_CONTAINERS1 "org.freedesktop.DBus.Containers1" - typedef struct { TestMainContext *ctx; gboolean skip; diff --git a/test/data/valid-config-files/limit-containers.conf.in b/test/data/valid-config-files/limit-containers.conf.in index 44dd3979..3cc3bb05 100644 --- a/test/data/valid-config-files/limit-containers.conf.in +++ b/test/data/valid-config-files/limit-containers.conf.in @@ -13,4 +13,9 @@ + + 5 + 3 + 4096 + 3 diff --git a/test/data/valid-config-files/tmp-session.conf.in b/test/data/valid-config-files/tmp-session.conf.in index d1effae1..502619dd 100644 --- a/test/data/valid-config-files/tmp-session.conf.in +++ b/test/data/valid-config-files/tmp-session.conf.in @@ -57,4 +57,11 @@ 50000 50000 50000 + 10000 + 10000 + 1000000000 + + 16 + diff --git a/test/dbus-daemon.c b/test/dbus-daemon.c index 1288fe0c..772fac5f 100644 --- a/test/dbus-daemon.c +++ b/test/dbus-daemon.c @@ -637,6 +637,10 @@ test_creds (Fixture *f, g_assert_not_reached (); #endif } + else if (g_str_has_prefix (name, DBUS_INTERFACE_CONTAINERS1 ".")) + { + g_assert_not_reached (); + } dbus_message_iter_next (&arr_iter); } From 5e6580ca19a754e5bcca0dd53170301ed224603f Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Tue, 1 Oct 2024 17:51:13 +0100 Subject: [PATCH 02/23] Reapply "spec: Document the initial Containers1 interface" This reverts commit f8a2a03ca0f50a84578419de2a16a8e84cbeca50. --- doc/dbus-specification.xml | 556 +++++++++++++++++++++++++++++++++++++ 1 file changed, 556 insertions(+) diff --git a/doc/dbus-specification.xml b/doc/dbus-specification.xml index f278c1e4..b606d85d 100644 --- a/doc/dbus-specification.xml +++ b/doc/dbus-specification.xml @@ -7023,6 +7023,44 @@ + + org.freedesktop.DBus.Containers1.Instance + OBJECT_PATH + + The container instance object path of the server through + which this connection is connected, as returned by + AddServer. Omitted from the + dictionary if this connection is not via a container's + server. + + + + + org.freedesktop.DBus.Containers1.Type + STRING + + The container technology that created the container + instance, as passed to AddServer. + Omitted from the dictionary if this connection is not + via a container's server. For example, a typical value + for a Flatpak application would be + org.flatpak. + + + + + org.freedesktop.DBus.Containers1.Name + STRING + + The contained app name for the container instance, + as passed to AddServer. Omitted + from the dictionary if this connection is not via a + container's server. For example, a typical value for + a Flatpak application would be + org.gnome.Weather. + + + @@ -7431,6 +7469,524 @@ + + + + <literal>org.freedesktop.DBus.Containers1.AddServer</literal> + + As a method: + + AddServer (in STRING container_type, + in STRING container_name, + in DICT<STRING,VARIANT> metadata, + in DICT<STRING,VARIANT> named_arguments, + out OBJECT_PATH container_instance, + out ARRAY<BYTE> socket_path, + out STRING connectable_address) + + Message arguments: + + + + + Argument + Type + Description + + + + + 0 + STRING + + Reversed domain name identifying a container manager + or container technology, such as + org.flatpak or + io.snapcraft. It must follow the + same syntactic rules as a + D-Bus + interface name. + + + + 1 + STRING + + Some unique identifier for an application or + container, whose meaning is defined by the + maintainers of the container type. If the container + type does not have a concept of identifying or + naming its applications or containers by a string, + using the empty string here is suggested. + + + + 2 + DICT<STRING,VARIANT> + + Metadata describing the application or container, with the + keys and values defined by the maintainers of the container + type. + + + + 3 + DICT<STRING,VARIANT> + + Additional arguments that extend this method. + The only named arguments that are allowed are the ones + listed in the + org.freedesktop.DBus.Containers1.SupportedArguments + property. All other named arguments are an error. + + + + 4 + OBJECT_PATH + + An opaque object path identifying the new container + instance. + + + + 5 + ARRAY<BYTE> + + The absolute filesystem path of the resulting + AF_UNIX socket, followed by a single + zero (nul) byte, for example + /run/user/1000/dbus-12345vwxyz\x00 + where \x00 represents the terminating zero byte. + + + + 6 + STRING + + A connectable D-Bus address, for example + unix:path=/run/user/1000/dbus-12345vwxyz. + + + + + + + + + Set up a new server socket for use in an application container. + Clients can connect to that socket, and the result will be as + though they had connected to the message bus, with a few differences. + The intention is that a container manager will call this method + to get a new server socket, bind-mount the server socket + into a location that is accessible by the confined (sandboxed) + application, and ensure that the normal message bus socket is + not accessible by the confined application. + + + + Each call to this method creates a new + container instance, identified by an object + path. Even if the specified container type and name are the + same as for a pre-existing container instance, each call + creates a new server with a new unique object path, because + the new container instance might represent a different + version of the confined application with different + characteristics and restrictions. The message bus may provide + an object at the given object path, but is not required to + do so. + + + + Metadata from the first three arguments is stored by the message + bus, but not interpreted; software that interacts with the + container manager can use this metadata. + + The method call may fail with the error + org.freedesktop.DBus.Error.LimitsExceeded + if the caller provides more metadata than the message bus + is willing to store. + + + + The last argument (the named arguments) + will be used in future versions of this specification to modify + the behaviour of this method call; all keys in this dictionary not + containing . are reserved by this specification + for this purpose. It can also contain implementation-specific + arguments for a particular message bus implementation, which + should start with a reversed domain name in the same way as + interface names. For example, GLib's gdbus-daemon might use + org.gtk.GDBus.SomeArgument if it used this + extension point. + + + + In the most basic form of a call to this method, the new server + socket is an AF_UNIX socket at a path chosen + by the message bus and returned from the method. Future versions + of this specification are likely to provide a way for the + message bus implementation to receive a socket from the caller + and arrange for it to listen for connections, using the named + arguments dictionary. + + + + Connections to the new server socket are said to be + confined. The message bus may prevent + confined connections from calling certain security-sensitive methods, + and may apply lower limits to these connections. However, container + managers and services must not rely on confined connections having + any particular filtering applied to them by the message bus. + + + + In the most basic form of a call to this method, the server + socket will cease to listen for new connections (in the same + way as if StopListening had been called + successfully) when the caller of this method disconnects from + the bus. Any confined connections that are already active are + unaffected. Future versions of this specification are likely + to provide a way for the caller to alter this behaviour by + specifying named arguments. + + + + The container instance object path remains valid for as + long as one or more confined connection via the same server + socket remain open, or there is a way for the server socket + to produce new connections in future (in other words, + either it is preparing to listen for new connections, or + it is currently listening for new connections). When the + container instance has ceased to listen for new connections + and no longer has any confined connections, the object path + becomes invalid, and API calls that specify it will fail with + the org.freedesktop.DBus.Error.NotContainer + error. + + + + + <literal>org.freedesktop.DBus.Containers1.StopInstance</literal> + + As a method: + + StopInstance (in OBJECT_PATH container_instance) + + Message arguments: + + + + + Argument + Type + Description + + + + + 0 + OBJECT_PATH + + The opaque object path that was returned from the + AddServer method, identifying a + container instance. + + + + + + + + + Terminate the container instance. The server will stop + listening for new connections, and any existing connections to + that server will be disconnected. When all connections have + been disconnected, the instance will cease to exist. + + + If the given object path does not represent a valid container + instance (see + ), the + org.freedesktop.DBus.Error.NotContainer + error is returned. In particular, if this method is called + twice, the second call will fail in this way. + + + If the given container instance exists but the caller of this + method is not allowed to stop it (for example because the + caller is in a container instance or because its user ID does + not match the user ID of the creator of the container + instance), + the org.freedesktop.DBus.Error.AccessDenied + error is returned and nothing is stopped. + + + + + <literal>org.freedesktop.DBus.Containers1.StopListening</literal> + + As a method: + + StopListening (in OBJECT_PATH container_instance) + + Message arguments: + + + + + Argument + Type + Description + + + + + 0 + OBJECT_PATH + + The opaque object path that was returned from the + AddServer method, identifying a + container instance. + + + + + + + + + Stop listening for new connections to the server in the given + container instance, but do not disconnect any existing + connections. + + + If this method is called more than once, while the container + instance still exists because connections to it are still + open, the second and subsequent calls will be successful but + will have no practical effect. + + + If the given object path does not represent a valid container + instance (see + ), the + org.freedesktop.DBus.Error.NotContainer + error is returned. In particular, this will happen if the + container instance already stopped listening, and all + connections to it (if any) were already closed. + + + If the given container instance exists but the caller of this + method is not allowed to stop it (for example because the + caller is in a container instance or because its user ID does + not match the user ID of the creator of the container + instance), + the org.freedesktop.DBus.Error.AccessDenied + error is returned and nothing is stopped. + + + + + <literal>org.freedesktop.DBus.Containers1.GetConnectionInstance</literal> + + As a method: + + GetConnectionInstance (in STRING bus_name, + out OBJECT_PATH container_instance, + out STRING container_type, + out STRING container_name, + out DICT<STRING,VARIANT> metadata) + + Message arguments: + + + + + Argument + Type + Description + + + + + 0 + STRING + + Unique or well-known bus name of the connection to + query, such as :12.34 or + com.example.tea. + + + + 1 + OBJECT_PATH + + The opaque object path that was returned from the + AddServer method, identifying a + container instance. + + + + 2 + STRING + + Reversed domain name identifying a container + manager or container technology, as passed to the + AddServer method, such as + org.flatpak or + io.snapcraft. + + + + 3 + STRING + + Some unique identifier for an application or container, + whose meaning is defined by the maintainers of the + container type. + + + + 4 + DICT<STRING,VARIANT> + + Metadata describing the application or container, with the + keys and values defined by the maintainers of the container + type. + + + + + + + + + If the given unique or well-known bus name is a connection to a + container server, return information about that container instance. + If the bus name exists but is not confined (in other words, if it + is a direct connection to the main message bus socket), the + org.freedesktop.DBus.Error.NotContainer error + is returned instead. If the given bus name has no owner at all, the + org.freedesktop.DBus.Error.NameHasNoOwner error + is returned instead. + + + + + <literal>org.freedesktop.DBus.Containers1.GetInstanceInfo</literal> + + As a method: + + GetInstanceInfo (in OBJECT_PATH container_instance, + out STRING container_type, + out STRING container_name, + out DICT<STRING,VARIANT> metadata) + + Message arguments: + + + + + Argument + Type + Description + + + + + 0 + OBJECT_PATH + + The opaque object path that was returned from the + AddServer method, identifying a + container instance. + + + + 1 + STRING + + Reversed domain name identifying a container + manager or container technology, as passed to the + AddServer method, such as + org.flatpak or + io.snapcraft. + + + + 2 + STRING + + Some unique identifier for an application or container, + whose meaning is defined by the maintainers of the + container type. + + + + 3 + DICT<STRING,VARIANT> + + Metadata describing the application or container, with the + keys and values defined by the maintainers of the container + type. + + + + + + + + + If the given object path represents a valid container instance, + (see ), + return information about it. Otherwise, the + org.freedesktop.DBus.Error.NotContainer error + is returned. + + + + + <literal>org.freedesktop.DBus.Containers1.InstanceRemoved</literal> + + As a signal emitted by the message bus: + + InstanceRemoved (OBJECT_PATH container_instance) + + Message arguments: + + + + + Argument + Type + Description + + + + + 0 + OBJECT_PATH + + The opaque object path that was returned from the + AddServer method, identifying a + container instance. + + + + + + + + + Emitted when a container instance ceases to exist. A container + instance continues to exist as long as it is listening for + new connections, or as long as connections to the instance + are open, whichever is longer. + + + + + Monitoring Interface: <literal>org.freedesktop.DBus.Monitoring</literal> From bc4a8d89851f3c5a5971f441be7aa7af29aeec5e Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Tue, 1 Oct 2024 17:51:49 +0100 Subject: [PATCH 03/23] spec: Add an introduction to the Containers1 interface Signed-off-by: Simon McVittie --- doc/dbus-specification.xml | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/doc/dbus-specification.xml b/doc/dbus-specification.xml index b606d85d..0bc40a26 100644 --- a/doc/dbus-specification.xml +++ b/doc/dbus-specification.xml @@ -7470,6 +7470,38 @@ + Containers Interface v1: <literal>org.freedesktop.DBus.Containers1</literal> + + The special message bus name org.freedesktop.DBus + may optionally implement the + org.freedesktop.DBus.Containers1 interface on + the object path /org/freedesktop/DBus. + + + + This interface allows container managers and similar sandboxing + mechanisms to ask the message bus to create a special socket + for each sandboxed application, + which uniquely identifies the application to other message bus + clients without introducing race conditions. + For this mechanism to be useful, the sandboxed application must be + prevented from connecting to the message bus's usual socket. + This interface is a D-Bus equivalent of the + Wayland security-context extension. + + + + As currently implemented, this interface does not apply any + special filtering to the D-Bus messages sent and received by a + sandboxed application. + To limit what a sandboxed application can do on D-Bus, it is + likely to be necessary to impose restrictions, + perhaps by using a Linux security module such as AppArmor or a + filtering proxy such as + xdg-dbus-proxy. + A future version of this specification might add a mechanism for + the creator of a confined socket to specify filtering rules. + <literal>org.freedesktop.DBus.Containers1.AddServer</literal> From af407343d19ca3fa32ba6b6741d9d8a496721aa7 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Fri, 20 Oct 2023 16:30:06 +0100 Subject: [PATCH 04/23] specification: Document container info as containing the creator This was added to the implementation after Containers was removed from the spec. Signed-off-by: Simon McVittie --- doc/dbus-specification.xml | 32 ++++++++++++++++++++++++++++---- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/doc/dbus-specification.xml b/doc/dbus-specification.xml index 0bc40a26..a61218cd 100644 --- a/doc/dbus-specification.xml +++ b/doc/dbus-specification.xml @@ -7825,6 +7825,7 @@ GetConnectionInstance (in STRING bus_name, out OBJECT_PATH container_instance, + out DICT<STRING,VARIANT> creator, out STRING container_type, out STRING container_name, out DICT<STRING,VARIANT> metadata) @@ -7860,6 +7861,17 @@ 2 + DICT<STRING,VARIANT> + + The connection that created this container instance. + The keys and values are the same as those that are + documented for + GetConnectionCredentials. + + + + 3 STRING Reversed domain name identifying a container @@ -7870,7 +7882,7 @@ - 3 + 4 STRING Some unique identifier for an application or container, @@ -7879,7 +7891,7 @@ - 4 + 5 DICT<STRING,VARIANT> Metadata describing the application or container, with the @@ -7910,6 +7922,7 @@ As a method: GetInstanceInfo (in OBJECT_PATH container_instance, + out DICT<STRING,VARIANT> creator, out STRING container_type, out STRING container_name, out DICT<STRING,VARIANT> metadata) @@ -7936,6 +7949,17 @@ 1 + DICT<STRING,VARIANT> + + The connection that created this container instance. + The keys and values are the same as those that are + documented for + GetConnectionCredentials. + + + + 2 STRING Reversed domain name identifying a container @@ -7946,7 +7970,7 @@ - 2 + 3 STRING Some unique identifier for an application or container, @@ -7955,7 +7979,7 @@ - 3 + 4 DICT<STRING,VARIANT> Metadata describing the application or container, with the From 9428df3740e668ab822cdb33d391d06ac9a0dd30 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Fri, 20 Oct 2023 16:30:26 +0100 Subject: [PATCH 05/23] specification: Describe the trust model for container info Signed-off-by: Simon McVittie --- doc/dbus-specification.xml | 34 ++++++++++++++++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) diff --git a/doc/dbus-specification.xml b/doc/dbus-specification.xml index a61218cd..c20a6bbe 100644 --- a/doc/dbus-specification.xml +++ b/doc/dbus-specification.xml @@ -7632,8 +7632,6 @@ Metadata from the first three arguments is stored by the message bus, but not interpreted; software that interacts with the container manager can use this metadata. - The method call may fail with the error org.freedesktop.DBus.Error.LimitsExceeded if the caller provides more metadata than the message bus @@ -7857,6 +7855,7 @@ The opaque object path that was returned from the AddServer method, identifying a container instance. + This output parameter is produced by the message bus. @@ -7868,6 +7867,8 @@ documented for GetConnectionCredentials. + This output parameter is produced by the message bus + and is appropriate to use in trust decisions. @@ -7879,6 +7880,8 @@ AddServer method, such as org.flatpak or io.snapcraft. + This output parameter is controlled by the creator + of the container instance. @@ -7888,6 +7891,8 @@ Some unique identifier for an application or container, whose meaning is defined by the maintainers of the container type. + This output parameter is controlled by the creator + of the container instance. @@ -7897,6 +7902,8 @@ Metadata describing the application or container, with the keys and values defined by the maintainers of the container type. + This output parameter is controlled by the creator + of the container instance. @@ -7914,6 +7921,15 @@ org.freedesktop.DBus.Error.NameHasNoOwner error is returned instead. + + Several of the output parameters are controlled by the creator of + the container instance. + On a bus that accepts connections from multiple clients at + different privilege levels, it is not appropriate to trust those + output parameters or use them in trust decisions unless the + process identified by the creator parameter + is trusted. + @@ -7956,6 +7972,8 @@ documented for GetConnectionCredentials. + This output parameter is produced by the message bus + and is appropriate to use in trust decisions. @@ -7967,6 +7985,8 @@ AddServer method, such as org.flatpak or io.snapcraft. + This output parameter is controlled by the creator + of the container instance. @@ -7976,6 +7996,8 @@ Some unique identifier for an application or container, whose meaning is defined by the maintainers of the container type. + This output parameter is controlled by the creator + of the container instance. @@ -7985,6 +8007,8 @@ Metadata describing the application or container, with the keys and values defined by the maintainers of the container type. + This output parameter is controlled by the creator + of the container instance. @@ -7999,6 +8023,12 @@ org.freedesktop.DBus.Error.NotContainer error is returned. + + Several of the output parameters are controlled by the creator of + the container instance. + See + for more details. + From 0196ed354322d34de2bd347a0ec1e2c2282aa3b5 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Fri, 27 Oct 2023 18:50:41 +0100 Subject: [PATCH 06/23] spec: GetConnectionCredentials doesn't include container context metadata In early prototypes we put the Type and Name here, but that leads to some difficult questions about whether they can be trusted, and the answer is unfortunately "it depends". Signed-off-by: Simon McVittie --- doc/dbus-specification.xml | 27 --------------------------- 1 file changed, 27 deletions(-) diff --git a/doc/dbus-specification.xml b/doc/dbus-specification.xml index c20a6bbe..f163e623 100644 --- a/doc/dbus-specification.xml +++ b/doc/dbus-specification.xml @@ -7034,33 +7034,6 @@ server. - - - org.freedesktop.DBus.Containers1.Type - STRING - - The container technology that created the container - instance, as passed to AddServer. - Omitted from the dictionary if this connection is not - via a container's server. For example, a typical value - for a Flatpak application would be - org.flatpak. - - - - - org.freedesktop.DBus.Containers1.Name - STRING - - The contained app name for the container instance, - as passed to AddServer. Omitted - from the dictionary if this connection is not via a - container's server. For example, a typical value for - a Flatpak application would be - org.gnome.Weather. - - - From 2c884a5e5f18bcbc17b5d1a33d364f8dd9e063ef Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Fri, 20 Oct 2023 16:32:02 +0100 Subject: [PATCH 07/23] CI: Enable Containers1 in debug builds, explicitly disable in reduced build This ensures that both "enabled" and "disabled" are tested, regardless of whether it is enabled by default (currently it is not). Create /var/local/run/dbus/containers so that we can run the tests. Signed-off-by: Simon McVittie --- tools/ci-build.sh | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tools/ci-build.sh b/tools/ci-build.sh index c971415c..b0cdade7 100755 --- a/tools/ci-build.sh +++ b/tools/ci-build.sh @@ -369,6 +369,7 @@ case "$ci_buildsys" in fi set -- -Db_pie=true "$@" + set -- -Dcontainers=true "$@" set -- -Duser_session=true "$@" ;; esac @@ -393,6 +394,7 @@ case "$ci_buildsys" in set "$@" -Dlibaudit=disabled -Dvalgrind=disabled # Disable optional features, some of which are on by # default + set "$@" -Dcontainers=false set "$@" -Dstats=false set "$@" -Duser_session=false shift @@ -454,6 +456,12 @@ case "$ci_buildsys" in $meson_setup "$@" "$srcdir" meson compile -v + if [ "$(id -u)" = 0 ]; then + # In production, this would be /run/dbus/containers and would + # have been set up by the init script or tmpfiles.d + mkdir -p /var/local/run/dbus/containers + fi + # This is too slow and verbose to keep enabled at the moment export DBUS_TEST_MALLOC_FAILURES=0 From 70c9402fa84ab4173ff2f85fcb24a459eb6b193e Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Fri, 27 Oct 2023 18:51:23 +0100 Subject: [PATCH 08/23] containers: Rename "container instance" to "container server" Flatpak has the concept of an "instance ID" for a running app, which we should expose in Containers1, similar to the analogous Wayland specification security-context-v1[1]. If we use the word "instance" for both the Flatpak (or other container manager) side and the D-Bus side, the resulting API will be really confusing. [1] https://gitlab.freedesktop.org/wayland/wayland-protocols/-/tree/main/staging/security-context Signed-off-by: Simon McVittie --- bus/connection.c | 6 +- bus/connection.h | 2 +- bus/containers.c | 372 ++++++++++++++--------------- bus/containers.h | 12 +- bus/dispatch.c | 2 +- bus/driver.c | 14 +- dbus/dbus-marshal-header.c | 4 +- dbus/dbus-message.c | 40 +++- dbus/dbus-message.h | 9 +- dbus/dbus-protocol.h | 10 +- doc/dbus-specification.xml | 142 +++++------ test/containers.c | 138 +++++------ test/internals/dbus-message-util.c | 4 +- 13 files changed, 398 insertions(+), 357 deletions(-) diff --git a/bus/connection.c b/bus/connection.c index f0177c6f..de00ceee 100644 --- a/bus/connection.c +++ b/bus/connection.c @@ -2478,8 +2478,8 @@ bus_transaction_send (BusTransaction *transaction, * until we know we have enough memory for the entire transaction, * and that doesn't happen until we know all the recipients. * So this is about the last possible time we could edit the header. */ - if ((d->want_headers & BUS_EXTRA_HEADERS_CONTAINER_INSTANCE) && - dbus_message_get_container_instance (message) == NULL) + if ((d->want_headers & BUS_EXTRA_HEADERS_CONTAINER_PATH) && + dbus_message_get_container_path (message) == NULL) { const char *path; @@ -2488,7 +2488,7 @@ bus_transaction_send (BusTransaction *transaction, NULL, NULL)) path = "/"; - if (!dbus_message_set_container_instance (message, path)) + if (!dbus_message_set_container_path (message, path)) return FALSE; } diff --git a/bus/connection.h b/bus/connection.h index 0ce60d11..f3eca5d0 100644 --- a/bus/connection.h +++ b/bus/connection.h @@ -32,7 +32,7 @@ typedef enum { - BUS_EXTRA_HEADERS_CONTAINER_INSTANCE = (1 << 0), + BUS_EXTRA_HEADERS_CONTAINER_PATH = (1 << 0), BUS_EXTRA_HEADERS_NONE = 0 } BusExtraHeaders; diff --git a/bus/containers.c b/bus/containers.c index aa3116e4..08c32e13 100644 --- a/bus/containers.c +++ b/bus/containers.c @@ -1,6 +1,6 @@ /* containers.c - restricted bus servers for containers * - * Copyright © 2017 Collabora Ltd. + * Copyright © 2017-2023 Collabora Ltd. * * SPDX-License-Identifier: AFL-2.1 OR GPL-2.0-or-later * @@ -45,7 +45,7 @@ #include "utils.h" /* - * A container instance groups together a per-app-container server with + * A container server groups together a per-app-container server with * all the connections for which it is responsible. */ typedef struct @@ -64,14 +64,14 @@ typedef struct DBusList *connections; unsigned long uid; unsigned announced:1; -} BusContainerInstance; +} BusContainerServer; -/* Data attached to a DBusConnection that has created container instances. */ +/* Data attached to a DBusConnection that has created container servers. */ typedef struct { - /* List of instances created by this connection; unowned. - * The BusContainerInstance removes itself from here on destruction. */ - DBusList *instances; + /* List of servers created by this connection; unowned. + * The BusContainerServer removes itself from here on destruction. */ + DBusList *servers; } BusContainerCreatorData; /* Data slot on DBusConnection, holding BusContainerCreatorData */ @@ -84,16 +84,16 @@ static dbus_int32_t container_creator_data_slot = -1; struct BusContainers { int refcount; - /* path borrowed from BusContainerInstance => unowned BusContainerInstance - * The BusContainerInstance removes itself from here on destruction. */ - DBusHashTable *instances_by_path; + /* path borrowed from BusContainerServer => unowned BusContainerServer + * The BusContainerServer removes itself from here on destruction. */ + DBusHashTable *servers_by_path; /* uid => (void *) (uintptr_t) number of containers */ DBusHashTable *n_containers_by_user; DBusString address_template; dbus_uint64_t next_container_id; }; -/* Data slot on DBusConnection, holding BusContainerInstance */ +/* Data slot on DBusConnection, holding BusContainerServer */ static dbus_int32_t contained_data_slot = -1; BusContainers * @@ -120,7 +120,7 @@ bus_containers_new (void) goto oom; self->refcount = 1; - self->instances_by_path = NULL; + self->servers_by_path = NULL; self->next_container_id = DBUS_UINT64_CONSTANT (0); self->address_template = invalid; @@ -193,7 +193,7 @@ bus_containers_unref (BusContainers *self) if (--self->refcount == 0) { - _dbus_clear_hash_table (&self->instances_by_path); + _dbus_clear_hash_table (&self->servers_by_path); _dbus_clear_hash_table (&self->n_containers_by_user); _dbus_string_free (&self->address_template); dbus_free (self); @@ -206,8 +206,8 @@ bus_containers_unref (BusContainers *self) } } -static BusContainerInstance * -bus_container_instance_ref (BusContainerInstance *self) +static BusContainerServer * +bus_container_server_ref (BusContainerServer *self) { _dbus_assert (self->refcount > 0); _dbus_assert (self->refcount < _DBUS_INT_MAX); @@ -217,7 +217,7 @@ bus_container_instance_ref (BusContainerInstance *self) } static dbus_bool_t -bus_container_instance_emit_removed (BusContainerInstance *self) +bus_container_server_emit_removed (BusContainerServer *self) { BusTransaction *transaction = NULL; DBusMessage *message = NULL; @@ -230,7 +230,7 @@ bus_container_instance_emit_removed (BusContainerInstance *self) message = dbus_message_new_signal (DBUS_PATH_DBUS, DBUS_INTERFACE_CONTAINERS1, - "InstanceRemoved"); + "ServerRemoved"); if (message == NULL || !dbus_message_set_sender (message, DBUS_SERVICE_DBUS) || @@ -251,7 +251,7 @@ bus_container_instance_emit_removed (BusContainerInstance *self) * somehow does happen, we don't want to stay in the OOM-retry loop, * because waiting for more memory will not help; so continue to * execute the transaction anyway. */ - _dbus_warn ("Failed to send InstanceRemoved for a reason " + _dbus_warn ("Failed to send ServerRemoved for a reason " "other than OOM: %s: %s", error.name, error.message); dbus_error_free (&error); } @@ -272,7 +272,7 @@ oom: } static void -bus_container_instance_unref (BusContainerInstance *self) +bus_container_server_unref (BusContainerServer *self) { _dbus_assert (self->refcount > 0); @@ -280,10 +280,10 @@ bus_container_instance_unref (BusContainerInstance *self) { BusContainerCreatorData *creator_data; - /* If we announced the container instance in a reply from + /* If we announced the container server in a reply from * AddServer() (which is also the time at which it becomes * available for the querying methods), then we have to emit - * InstanceRemoved for it. + * ServerRemoved for it. * * Similar to bus/connection.c dropping well-known name ownership, * this isn't really a situation where we can "fail", because @@ -291,29 +291,29 @@ bus_container_instance_unref (BusContainerInstance *self) * connection disconnecting; so we use a retry loop on OOM. */ for (; self->announced; _dbus_wait_for_memory ()) { - if (bus_container_instance_emit_removed (self)) + if (bus_container_server_emit_removed (self)) self->announced = FALSE; } - /* As long as the server is listening, the BusContainerInstance can't + /* As long as the server is listening, the BusContainerServer can't * be freed, because the DBusServer holds a reference to the - * BusContainerInstance */ + * BusContainerServer */ _dbus_assert (self->server == NULL); - /* Similarly, as long as there are connections, the BusContainerInstance + /* Similarly, as long as there are connections, the BusContainerServer * can't be freed, because each connection holds a reference to the - * BusContainerInstance */ + * BusContainerServer */ _dbus_assert (self->connections == NULL); creator_data = dbus_connection_get_data (self->creator, container_creator_data_slot); _dbus_assert (creator_data != NULL); - _dbus_list_remove (&creator_data->instances, self); + _dbus_list_remove (&creator_data->servers, self); - /* It's OK to do this even if we were never added to instances_by_path, + /* It's OK to do this even if we were never added to servers_by_path, * because the paths are globally unique. */ - if (self->path != NULL && self->containers->instances_by_path != NULL && - _dbus_hash_table_remove_string (self->containers->instances_by_path, + if (self->path != NULL && self->containers->servers_by_path != NULL && + _dbus_hash_table_remove_string (self->containers->servers_by_path, self->path)) { DBusHashIter entry; @@ -323,7 +323,7 @@ bus_container_instance_unref (BusContainerInstance *self) (void *) (uintptr_t) self->uid, FALSE, &entry)) _dbus_assert_not_reached ("Container should not be placed in " - "instances_by_path until its " + "servers_by_path until its " "n_containers_by_user entry has " "been allocated"); @@ -345,17 +345,17 @@ bus_container_instance_unref (BusContainerInstance *self) } static inline void -bus_clear_container_instance (BusContainerInstance **instance_p) +bus_clear_container_server (BusContainerServer **server_p) { - _dbus_clear_pointer_impl (BusContainerInstance, instance_p, - bus_container_instance_unref); + _dbus_clear_pointer_impl (BusContainerServer, server_p, + bus_container_server_unref); } static void -bus_container_instance_stop_listening (BusContainerInstance *self) +bus_container_server_stop_listening (BusContainerServer *self) { /* In case the DBusServer holds the last reference to self */ - bus_container_instance_ref (self); + bus_container_server_ref (self); if (self->server != NULL) { @@ -364,16 +364,16 @@ bus_container_instance_stop_listening (BusContainerInstance *self) dbus_clear_server (&self->server); } - bus_container_instance_unref (self); + bus_container_server_unref (self); } -static BusContainerInstance * -bus_container_instance_new (BusContext *context, - BusContainers *containers, - DBusConnection *creator, - DBusError *error) +static BusContainerServer * +bus_container_server_new (BusContext *context, + BusContainers *containers, + DBusConnection *creator, + DBusError *error) { - BusContainerInstance *self = NULL; + BusContainerServer *self = NULL; DBusString path = _DBUS_STRING_INIT_INVALID; _dbus_assert (context != NULL); @@ -387,7 +387,7 @@ bus_container_instance_new (BusContext *context, goto fail; } - self = dbus_new0 (BusContainerInstance, 1); + self = dbus_new0 (BusContainerServer, 1); if (self == NULL) { @@ -432,7 +432,7 @@ fail: _dbus_string_free (&path); if (self != NULL) - bus_container_instance_unref (self); + bus_container_server_unref (self); return NULL; } @@ -440,9 +440,9 @@ fail: static void bus_container_creator_data_free (BusContainerCreatorData *self) { - /* Each instance holds a ref to the creator, so there should be + /* Each server holds a ref to the creator, so there should be * nothing here */ - _dbus_assert (self->instances == NULL); + _dbus_assert (self->servers == NULL); dbus_free (self); } @@ -472,74 +472,74 @@ allow_same_uid_only (DBusConnection *connection, } static void -bus_container_instance_lost_connection (BusContainerInstance *instance, - DBusConnection *connection) +bus_container_server_lost_connection (BusContainerServer *server, + DBusConnection *connection) { - bus_container_instance_ref (instance); + bus_container_server_ref (server); dbus_connection_ref (connection); /* This is O(n), but we don't expect to have many connections per - * container instance. */ - if (_dbus_list_remove (&instance->connections, connection)) + * container server. */ + if (_dbus_list_remove (&server->connections, connection)) dbus_connection_unref (connection); /* We don't set connection's contained_data_slot to NULL, to make sure * that once we have marked a connection as belonging to a container, * there is no going back: even if we somehow keep a reference to it * around, it will never be treated as uncontained. The connection's - * reference to the instance will be cleaned up on last-unref, and - * the list removal above ensures that the instance does not hold a + * reference to the server will be cleaned up on last-unref, and + * the list removal above ensures that the server does not hold a * circular ref to the connection, so the last-unref will happen. */ dbus_connection_unref (connection); - bus_container_instance_unref (instance); + bus_container_server_unref (server); } static void -new_connection_cb (DBusServer *server, +new_connection_cb (DBusServer *lower_level_server, DBusConnection *new_connection, void *data) { - BusContainerInstance *instance = data; - int limit = bus_context_get_max_connections_per_container (instance->context); + BusContainerServer *server = data; + int limit = bus_context_get_max_connections_per_container (server->context); /* This is O(n), but we assume n is small in practice. */ - if (_dbus_list_get_length (&instance->connections) >= limit) + if (_dbus_list_get_length (&server->connections) >= limit) { /* We can't send this error to the new connection, so just log it */ - bus_context_log (instance->context, DBUS_SYSTEM_LOG_WARNING, + bus_context_log (server->context, DBUS_SYSTEM_LOG_WARNING, "Closing connection to container server " "%s (%s \"%s\") because it would exceed resource limit " "(max_connections_per_container=%d)", - instance->path, instance->type, instance->name, limit); + server->path, server->type, server->name, limit); return; } if (!dbus_connection_set_data (new_connection, contained_data_slot, - bus_container_instance_ref (instance), - (DBusFreeFunction) bus_container_instance_unref)) + bus_container_server_ref (server), + (DBusFreeFunction) bus_container_server_unref)) { - bus_container_instance_unref (instance); - bus_container_instance_lost_connection (instance, new_connection); + bus_container_server_unref (server); + bus_container_server_lost_connection (server, new_connection); return; } - if (_dbus_list_append (&instance->connections, new_connection)) + if (_dbus_list_append (&server->connections, new_connection)) { dbus_connection_ref (new_connection); } else { - bus_container_instance_lost_connection (instance, new_connection); + bus_container_server_lost_connection (server, new_connection); return; } /* If this fails it logs a warning, so we don't need to do that. * We don't know how to undo this, so do it last (apart from things that * cannot fail) */ - if (!bus_context_add_incoming_connection (instance->context, new_connection)) + if (!bus_context_add_incoming_connection (server->context, new_connection)) { - bus_container_instance_lost_connection (instance, new_connection); + bus_container_server_lost_connection (server, new_connection); return; } @@ -557,7 +557,7 @@ new_connection_cb (DBusServer *server, * allow_same_uid_only ensures that * this cast does not lose * information */ - (void *) (uintptr_t) instance->uid, + (void *) (uintptr_t) server->uid, NULL); } @@ -635,8 +635,8 @@ out: } static dbus_bool_t -bus_container_instance_listen (BusContainerInstance *self, - DBusError *error) +bus_container_server_listen (BusContainerServer *self, + DBusError *error) { BusContainers *containers = bus_context_get_containers (self->context); const char *address; @@ -663,8 +663,8 @@ bus_container_instance_listen (BusContainerInstance *self, /* Cannot fail because the memory it uses was already allocated */ dbus_server_set_new_connection_function (self->server, new_connection_cb, - bus_container_instance_ref (self), - (DBusFreeFunction) bus_container_instance_unref); + bus_container_server_ref (self), + (DBusFreeFunction) bus_container_server_unref); return TRUE; } @@ -682,7 +682,7 @@ bus_containers_handle_add_server (DBusConnection *connection, const char *type; const char *name; const char *path; - BusContainerInstance *instance = NULL; + BusContainerServer *server = NULL; BusContext *context; BusContainers *containers; char *address = NULL; @@ -707,7 +707,7 @@ bus_containers_handle_add_server (DBusConnection *connection, if (creator_data == NULL) goto oom; - creator_data->instances = NULL; + creator_data->servers = NULL; if (!dbus_connection_set_data (connection, container_creator_data_slot, creator_data, @@ -718,13 +718,13 @@ bus_containers_handle_add_server (DBusConnection *connection, } } - instance = bus_container_instance_new (context, containers, connection, - error); + server = bus_container_server_new (context, containers, connection, + error); - if (instance == NULL) + if (server == NULL) goto fail; - if (!dbus_connection_get_unix_user (connection, &instance->uid)) + if (!dbus_connection_get_unix_user (connection, &server->uid)) { dbus_set_error (error, DBUS_ERROR_FAILED, "Unable to determine user ID of caller"); @@ -740,9 +740,9 @@ bus_containers_handle_add_server (DBusConnection *connection, _dbus_assert (dbus_message_iter_get_arg_type (&iter) == DBUS_TYPE_STRING); dbus_message_iter_get_basic (&iter, &type); - instance->type = _dbus_strdup (type); + server->type = _dbus_strdup (type); - if (instance->type == NULL) + if (server->type == NULL) goto oom; if (!dbus_validate_interface (type, NULL)) @@ -759,9 +759,9 @@ bus_containers_handle_add_server (DBusConnection *connection, _dbus_assert (dbus_message_iter_get_arg_type (&iter) == DBUS_TYPE_STRING); dbus_message_iter_get_basic (&iter, &name); - instance->name = _dbus_strdup (name); + server->name = _dbus_strdup (name); - if (instance->name == NULL) + if (server->name == NULL) goto oom; /* Argument 2: Metadata as defined by container manager */ @@ -769,17 +769,17 @@ bus_containers_handle_add_server (DBusConnection *connection, _dbus_assert_not_reached ("Message type was already checked"); _dbus_assert (dbus_message_iter_get_arg_type (&iter) == DBUS_TYPE_ARRAY); - instance->metadata = _dbus_variant_read (&iter); - _dbus_assert (strcmp (_dbus_variant_get_signature (instance->metadata), + server->metadata = _dbus_variant_read (&iter); + _dbus_assert (strcmp (_dbus_variant_get_signature (server->metadata), "a{sv}") == 0); - /* For simplicity we don't count the size of the BusContainerInstance + /* For simplicity we don't count the size of the BusContainerServer * itself, the object path, lengths, the non-payload parts of the DBusString, * NUL terminators and so on. That overhead is O(1) and relatively small. * This cannot overflow because all parts came from a message, and messages * are constrained to be orders of magnitude smaller than the maximum * int value. */ - metadata_size = _dbus_variant_get_length (instance->metadata) + + metadata_size = _dbus_variant_get_length (server->metadata) + (int) strlen (type) + (int) strlen (name); limit = bus_context_get_max_container_metadata_bytes (context); @@ -831,12 +831,12 @@ bus_containers_handle_add_server (DBusConnection *connection, /* End of arguments */ _dbus_assert (!dbus_message_iter_has_next (&iter)); - if (containers->instances_by_path == NULL) + if (containers->servers_by_path == NULL) { - containers->instances_by_path = _dbus_hash_table_new (DBUS_HASH_STRING, - NULL, NULL); + containers->servers_by_path = _dbus_hash_table_new (DBUS_HASH_STRING, + NULL, NULL); - if (containers->instances_by_path == NULL) + if (containers->servers_by_path == NULL) goto oom; } @@ -851,7 +851,7 @@ bus_containers_handle_add_server (DBusConnection *connection, limit = bus_context_get_max_containers (context); - if (_dbus_hash_table_get_n_entries (containers->instances_by_path) >= limit) + if (_dbus_hash_table_get_n_entries (containers->servers_by_path) >= limit) { DBusError local_error = DBUS_ERROR_INIT; @@ -870,7 +870,7 @@ bus_containers_handle_add_server (DBusConnection *connection, if (!_dbus_hash_iter_lookup (containers->n_containers_by_user, /* We statically assert that a uid fits in a * uintptr_t, so this can't lose information */ - (void *) (uintptr_t) instance->uid, TRUE, + (void *) (uintptr_t) server->uid, TRUE, &n_containers_by_user_entry)) goto oom; @@ -889,34 +889,34 @@ bus_containers_handle_add_server (DBusConnection *connection, "(max_containers_per_user=%d)", bus_connection_get_name (connection), bus_connection_get_loginfo (connection), - instance->uid, limit); + server->uid, limit); bus_context_log_literal (context, DBUS_SYSTEM_LOG_WARNING, local_error.message); dbus_move_error (&local_error, error); goto fail; } - if (!_dbus_hash_table_insert_string (containers->instances_by_path, - instance->path, instance)) + if (!_dbus_hash_table_insert_string (containers->servers_by_path, + server->path, server)) goto oom; /* This cannot fail (we already allocated the memory) so we can do it after - * we already succeeded in adding it to instances_by_path. The matching - * decrement is done whenever we remove it from instances_by_path. */ + * we already succeeded in adding it to servers_by_path. The matching + * decrement is done whenever we remove it from servers_by_path. */ this_user_containers += 1; _dbus_hash_iter_set_value (&n_containers_by_user_entry, (void *) this_user_containers); - if (!_dbus_list_append (&creator_data->instances, instance)) + if (!_dbus_list_append (&creator_data->servers, server)) goto oom; /* This part is separated out because we eventually want to be able to * accept a fd-passed server socket in the named parameters, instead of * creating our own server, and defer listening on it until later */ - if (!bus_container_instance_listen (instance, error)) + if (!bus_container_server_listen (server, error)) goto fail; - address = dbus_server_get_address (instance->server); + address = dbus_server_get_address (server->server); if (!dbus_parse_address (address, &entries, &n_entries, error)) _dbus_assert_not_reached ("listening on unix:dir= should yield a valid address"); @@ -930,7 +930,7 @@ bus_containers_handle_add_server (DBusConnection *connection, reply = dbus_message_new_method_return (message); if (!dbus_message_append_args (reply, - DBUS_TYPE_OBJECT_PATH, &instance->path, + DBUS_TYPE_OBJECT_PATH, &server->path, DBUS_TYPE_INVALID)) goto oom; @@ -961,9 +961,9 @@ bus_containers_handle_add_server (DBusConnection *connection, if (! bus_transaction_send_from_driver (transaction, connection, reply)) goto oom; - instance->announced = TRUE; + server->announced = TRUE; dbus_message_unref (reply); - bus_container_instance_unref (instance); + bus_container_server_unref (server); dbus_address_entries_free (entries); dbus_free (address); return TRUE; @@ -972,18 +972,18 @@ oom: BUS_SET_OOM (error); /* fall through */ fail: - if (instance != NULL) - bus_container_instance_stop_listening (instance); + if (server != NULL) + bus_container_server_stop_listening (server); dbus_clear_message (&reply); dbus_clear_address_entries (&entries); - bus_clear_container_instance (&instance); + bus_clear_container_server (&server); dbus_free (address); return FALSE; } dbus_bool_t -bus_containers_supported_arguments_getter (BusContext *context, +bus_containers_supported_arguments_getter (BusContext *server, DBusMessageIter *var_iter) { DBusMessageIter arr_iter; @@ -996,14 +996,14 @@ bus_containers_supported_arguments_getter (BusContext *context, } dbus_bool_t -bus_containers_handle_stop_instance (DBusConnection *connection, - BusTransaction *transaction, - DBusMessage *message, - DBusError *error) +bus_containers_handle_stop_server (DBusConnection *connection, + BusTransaction *transaction, + DBusMessage *message, + DBusError *error) { BusContext *context; BusContainers *containers; - BusContainerInstance *instance = NULL; + BusContainerServer *server = NULL; DBusList *iter; const char *path; unsigned long uid; @@ -1016,13 +1016,13 @@ bus_containers_handle_stop_instance (DBusConnection *connection, context = bus_transaction_get_context (transaction); containers = bus_context_get_containers (context); - if (containers->instances_by_path != NULL) + if (containers->servers_by_path != NULL) { - instance = _dbus_hash_table_lookup_string (containers->instances_by_path, - path); + server = _dbus_hash_table_lookup_string (containers->servers_by_path, + path); } - if (instance == NULL) + if (server == NULL) { dbus_set_error (error, DBUS_ERROR_NOT_CONTAINER, "There is no container with path '%s'", path); @@ -1036,23 +1036,23 @@ bus_containers_handle_stop_instance (DBusConnection *connection, goto failed; } - if (uid != instance->uid) + if (uid != server->uid) { dbus_set_error (error, DBUS_ERROR_ACCESS_DENIED, "User %lu cannot stop a container server started by " - "user %lu", uid, instance->uid); + "user %lu", uid, server->uid); goto failed; } - bus_container_instance_ref (instance); - bus_container_instance_stop_listening (instance); + bus_container_server_ref (server); + bus_container_server_stop_listening (server); - for (iter = _dbus_list_get_first_link (&instance->connections); + for (iter = _dbus_list_get_first_link (&server->connections); iter != NULL; - iter = _dbus_list_get_next_link (&instance->connections, iter)) + iter = _dbus_list_get_next_link (&server->connections, iter)) dbus_connection_close (iter->data); - bus_container_instance_unref (instance); + bus_container_server_unref (server); if (!bus_driver_send_ack_reply (connection, transaction, message, error)) goto failed; @@ -1072,7 +1072,7 @@ bus_containers_handle_stop_listening (DBusConnection *connection, { BusContext *context; BusContainers *containers; - BusContainerInstance *instance = NULL; + BusContainerServer *server = NULL; const char *path; unsigned long uid; @@ -1084,13 +1084,13 @@ bus_containers_handle_stop_listening (DBusConnection *connection, context = bus_transaction_get_context (transaction); containers = bus_context_get_containers (context); - if (containers->instances_by_path != NULL) + if (containers->servers_by_path != NULL) { - instance = _dbus_hash_table_lookup_string (containers->instances_by_path, - path); + server = _dbus_hash_table_lookup_string (containers->servers_by_path, + path); } - if (instance == NULL) + if (server == NULL) { dbus_set_error (error, DBUS_ERROR_NOT_CONTAINER, "There is no container with path '%s'", path); @@ -1104,17 +1104,17 @@ bus_containers_handle_stop_listening (DBusConnection *connection, goto failed; } - if (uid != instance->uid) + if (uid != server->uid) { dbus_set_error (error, DBUS_ERROR_ACCESS_DENIED, "User %lu cannot stop a container server started by " - "user %lu", uid, instance->uid); + "user %lu", uid, server->uid); goto failed; } - bus_container_instance_ref (instance); - bus_container_instance_stop_listening (instance); - bus_container_instance_unref (instance); + bus_container_server_ref (server); + bus_container_server_stop_listening (server); + bus_container_server_unref (server); if (!bus_driver_send_ack_reply (connection, transaction, message, error)) goto failed; @@ -1131,8 +1131,8 @@ failed: * whether to allow sending or receiving a message, which might involve * the dbus-daemon itself as a message sender or recipient. */ -static BusContainerInstance * -connection_get_instance (DBusConnection *connection) +static BusContainerServer * +connection_get_server (DBusConnection *connection) { if (connection == NULL) return NULL; @@ -1144,12 +1144,12 @@ connection_get_instance (DBusConnection *connection) } dbus_bool_t -bus_containers_handle_get_connection_instance (DBusConnection *caller, - BusTransaction *transaction, - DBusMessage *message, - DBusError *error) +bus_containers_handle_get_connection_info (DBusConnection *caller, + BusTransaction *transaction, + DBusMessage *message, + DBusError *error) { - BusContainerInstance *instance; + BusContainerServer *server; BusDriverFound found; DBusConnection *subject; DBusMessage *reply = NULL; @@ -1159,7 +1159,7 @@ bus_containers_handle_get_connection_instance (DBusConnection *caller, _DBUS_ASSERT_ERROR_IS_CLEAR (error); - found = bus_driver_get_conn_helper (caller, message, "container instance", + found = bus_driver_get_conn_helper (caller, message, "container information", &bus_name, &subject, error); switch (found) @@ -1178,9 +1178,9 @@ bus_containers_handle_get_connection_instance (DBusConnection *caller, goto failed; } - instance = connection_get_instance (subject); + server = connection_get_server (subject); - if (instance == NULL) + if (server == NULL) { dbus_set_error (error, DBUS_ERROR_NOT_CONTAINER, "Connection '%s' is not in a container", bus_name); @@ -1193,7 +1193,7 @@ bus_containers_handle_get_connection_instance (DBusConnection *caller, goto oom; if (!dbus_message_append_args (reply, - DBUS_TYPE_OBJECT_PATH, &instance->path, + DBUS_TYPE_OBJECT_PATH, &server->path, DBUS_TYPE_INVALID)) goto oom; @@ -1203,7 +1203,7 @@ bus_containers_handle_get_connection_instance (DBusConnection *caller, &arr_writer)) goto oom; - if (!bus_driver_fill_connection_credentials (NULL, instance->creator, + if (!bus_driver_fill_connection_credentials (NULL, server->creator, caller, &arr_writer)) { @@ -1215,14 +1215,14 @@ bus_containers_handle_get_connection_instance (DBusConnection *caller, goto oom; if (!dbus_message_append_args (reply, - DBUS_TYPE_STRING, &instance->type, - DBUS_TYPE_STRING, &instance->name, + DBUS_TYPE_STRING, &server->type, + DBUS_TYPE_STRING, &server->name, DBUS_TYPE_INVALID)) goto oom; dbus_message_iter_init_append (reply, &writer); - if (!_dbus_variant_write (instance->metadata, &writer)) + if (!_dbus_variant_write (server->metadata, &writer)) goto oom; if (!bus_transaction_send_from_driver (transaction, caller, reply)) @@ -1242,14 +1242,14 @@ failed: } dbus_bool_t -bus_containers_handle_get_instance_info (DBusConnection *connection, - BusTransaction *transaction, - DBusMessage *message, - DBusError *error) +bus_containers_handle_get_server_info (DBusConnection *connection, + BusTransaction *transaction, + DBusMessage *message, + DBusError *error) { BusContext *context; BusContainers *containers; - BusContainerInstance *instance = NULL; + BusContainerServer *server = NULL; DBusMessage *reply = NULL; DBusMessageIter writer; DBusMessageIter arr_writer; @@ -1263,13 +1263,13 @@ bus_containers_handle_get_instance_info (DBusConnection *connection, context = bus_transaction_get_context (transaction); containers = bus_context_get_containers (context); - if (containers->instances_by_path != NULL) + if (containers->servers_by_path != NULL) { - instance = _dbus_hash_table_lookup_string (containers->instances_by_path, - path); + server = _dbus_hash_table_lookup_string (containers->servers_by_path, + path); } - if (instance == NULL) + if (server == NULL) { dbus_set_error (error, DBUS_ERROR_NOT_CONTAINER, "There is no container with path '%s'", path); @@ -1287,7 +1287,7 @@ bus_containers_handle_get_instance_info (DBusConnection *connection, &arr_writer)) goto oom; - if (!bus_driver_fill_connection_credentials (NULL, instance->creator, + if (!bus_driver_fill_connection_credentials (NULL, server->creator, connection, &arr_writer)) { @@ -1299,14 +1299,14 @@ bus_containers_handle_get_instance_info (DBusConnection *connection, goto oom; if (!dbus_message_append_args (reply, - DBUS_TYPE_STRING, &instance->type, - DBUS_TYPE_STRING, &instance->name, + DBUS_TYPE_STRING, &server->type, + DBUS_TYPE_STRING, &server->name, DBUS_TYPE_INVALID)) goto oom; dbus_message_iter_init_append (reply, &writer); - if (!_dbus_variant_write (instance->metadata, &writer)) + if (!_dbus_variant_write (server->metadata, &writer)) goto oom; if (!bus_transaction_send_from_driver (transaction, connection, reply)) @@ -1346,7 +1346,7 @@ bus_containers_handle_request_header (DBusConnection *caller, } bus_connection_request_headers (caller, - BUS_EXTRA_HEADERS_CONTAINER_INSTANCE); + BUS_EXTRA_HEADERS_CONTAINER_PATH); ret = TRUE; out: @@ -1357,17 +1357,17 @@ out: void bus_containers_stop_listening (BusContainers *self) { - if (self->instances_by_path != NULL) + if (self->servers_by_path != NULL) { DBusHashIter iter; - _dbus_hash_iter_init (self->instances_by_path, &iter); + _dbus_hash_iter_init (self->servers_by_path, &iter); while (_dbus_hash_iter_next (&iter)) { - BusContainerInstance *instance = _dbus_hash_iter_get_value (&iter); + BusContainerServer *server = _dbus_hash_iter_get_value (&iter); - bus_container_instance_stop_listening (instance); + bus_container_server_stop_listening (server); } } } @@ -1410,7 +1410,7 @@ bus_containers_remove_connection (BusContainers *self, { #ifdef DBUS_ENABLE_CONTAINERS BusContainerCreatorData *creator_data; - BusContainerInstance *instance; + BusContainerServer *server; dbus_connection_ref (connection); creator_data = dbus_connection_get_data (connection, @@ -1421,28 +1421,28 @@ bus_containers_remove_connection (BusContainers *self, DBusList *iter; DBusList *next; - for (iter = _dbus_list_get_first_link (&creator_data->instances); + for (iter = _dbus_list_get_first_link (&creator_data->servers); iter != NULL; iter = next) { - instance = iter->data; + server = iter->data; /* Remember where we got to before we do something that might free - * iter and instance */ - next = _dbus_list_get_next_link (&creator_data->instances, iter); + * iter and server */ + next = _dbus_list_get_next_link (&creator_data->servers, iter); - _dbus_assert (instance->creator == connection); + _dbus_assert (server->creator == connection); - /* This will invalidate iter and instance if there are no open - * connections to this instance */ - bus_container_instance_stop_listening (instance); + /* This will invalidate iter and server if there are no open + * connections to this server */ + bus_container_server_stop_listening (server); } } - instance = connection_get_instance (connection); + server = connection_get_server (connection); - if (instance != NULL) - bus_container_instance_lost_connection (instance, connection); + if (server != NULL) + bus_container_server_lost_connection (server, connection); dbus_connection_unref (connection); #endif /* DBUS_ENABLE_CONTAINERS */ @@ -1455,20 +1455,20 @@ bus_containers_connection_is_contained (DBusConnection *connection, const char **name) { #ifdef DBUS_ENABLE_CONTAINERS - BusContainerInstance *instance; + BusContainerServer *server; - instance = connection_get_instance (connection); + server = connection_get_server (connection); - if (instance != NULL) + if (server != NULL) { if (path != NULL) - *path = instance->path; + *path = server->path; if (type != NULL) - *type = instance->type; + *type = server->type; if (name != NULL) - *name = instance->name; + *name = server->name; return TRUE; } diff --git a/bus/containers.h b/bus/containers.h index 842a43f4..c5027f8d 100644 --- a/bus/containers.h +++ b/bus/containers.h @@ -38,7 +38,7 @@ dbus_bool_t bus_containers_handle_add_server (DBusConnection *connecti BusTransaction *transaction, DBusMessage *message, DBusError *error); -dbus_bool_t bus_containers_handle_stop_instance (DBusConnection *connection, +dbus_bool_t bus_containers_handle_stop_server (DBusConnection *connection, BusTransaction *transaction, DBusMessage *message, DBusError *error); @@ -46,14 +46,14 @@ dbus_bool_t bus_containers_handle_stop_listening (DBusConnection *connecti BusTransaction *transaction, DBusMessage *message, DBusError *error); -dbus_bool_t bus_containers_handle_get_instance_info (DBusConnection *connection, +dbus_bool_t bus_containers_handle_get_server_info (DBusConnection *connection, BusTransaction *transaction, DBusMessage *message, DBusError *error); -dbus_bool_t bus_containers_handle_get_connection_instance (DBusConnection *connection, - BusTransaction *transaction, - DBusMessage *message, - DBusError *error); +dbus_bool_t bus_containers_handle_get_connection_info (DBusConnection *connection, + BusTransaction *transaction, + DBusMessage *message, + DBusError *error); dbus_bool_t bus_containers_handle_request_header (DBusConnection *connection, BusTransaction *transaction, DBusMessage *message, diff --git a/bus/dispatch.c b/bus/dispatch.c index 9bfdf795..b61d9917 100644 --- a/bus/dispatch.c +++ b/bus/dispatch.c @@ -293,7 +293,7 @@ bus_dispatch (DBusConnection *connection, * don't understand (or validate), so that we can add header fields * in future and clients can assume that we have checked them. */ if (!_dbus_message_remove_unknown_fields (message) || - !dbus_message_set_container_instance (message, NULL)) + !dbus_message_set_container_path (message, NULL)) { BUS_SET_OOM (&error); goto out; diff --git a/bus/driver.c b/bus/driver.c index c6bff89d..7b0bba96 100644 --- a/bus/driver.c +++ b/bus/driver.c @@ -2036,7 +2036,7 @@ bus_driver_fill_connection_credentials (DBusCredentials *credentials, bus_containers_connection_is_contained (peer_conn, &path, NULL, NULL)) { if (!_dbus_asv_add_object_path (asv_iter, - DBUS_INTERFACE_CONTAINERS1 ".Instance", + DBUS_INTERFACE_CONTAINERS1 ".Path", path)) return FALSE; } @@ -2496,7 +2496,7 @@ typedef enum * containers are never privileged. */ METHOD_FLAG_PRIVILEGED = (1 << 1), - /* If set, callers must not be associated with a container instance. */ + /* If set, callers must not be associated with a container. */ METHOD_FLAG_NO_CONTAINERS = (1 << 2), METHOD_FLAG_NONE = 0 @@ -2649,14 +2649,14 @@ static const MessageHandler introspectable_message_handlers[] = { static const MessageHandler containers_message_handlers[] = { { "AddServer", "ssa{sv}a{sv}", "oays", bus_containers_handle_add_server, METHOD_FLAG_NO_CONTAINERS }, - { "StopInstance", "o", "", bus_containers_handle_stop_instance, + { "StopServer", "o", "", bus_containers_handle_stop_server, METHOD_FLAG_NO_CONTAINERS }, { "StopListening", "o", "", bus_containers_handle_stop_listening, METHOD_FLAG_NO_CONTAINERS }, - { "GetConnectionInstance", "s", "oa{sv}ssa{sv}", - bus_containers_handle_get_connection_instance, + { "GetConnectionInfo", "s", "oa{sv}ssa{sv}", + bus_containers_handle_get_connection_info, METHOD_FLAG_NONE }, - { "GetInstanceInfo", "o", "a{sv}ssa{sv}", bus_containers_handle_get_instance_info, + { "GetServerInfo", "o", "a{sv}ssa{sv}", bus_containers_handle_get_server_info, METHOD_FLAG_NONE }, { "RequestHeader", "", "", bus_containers_handle_request_header, METHOD_FLAG_NONE }, @@ -2776,7 +2776,7 @@ static InterfaceHandler interface_handlers[] = { #endif #ifdef DBUS_ENABLE_CONTAINERS { DBUS_INTERFACE_CONTAINERS1, containers_message_handlers, - " \n" + " \n" " \n" " \n", INTERFACE_FLAG_NONE, containers_property_handlers }, diff --git a/dbus/dbus-marshal-header.c b/dbus/dbus-marshal-header.c index 30636d79..c4824318 100644 --- a/dbus/dbus-marshal-header.c +++ b/dbus/dbus-marshal-header.c @@ -86,7 +86,7 @@ _dbus_header_field_types[DBUS_HEADER_FIELD_LAST+1] = { { DBUS_HEADER_FIELD_SENDER, DBUS_TYPE_STRING }, { DBUS_HEADER_FIELD_SIGNATURE, DBUS_TYPE_SIGNATURE }, { DBUS_HEADER_FIELD_UNIX_FDS, DBUS_TYPE_UINT32 }, - { DBUS_HEADER_FIELD_CONTAINER_INSTANCE, DBUS_TYPE_OBJECT_PATH } + { DBUS_HEADER_FIELD_CONTAINER_PATH, DBUS_TYPE_OBJECT_PATH } }; /** Macro to look up the correct type for a field */ @@ -922,7 +922,7 @@ load_and_validate_field (DBusHeader *header, string_validation_func = NULL; break; - case DBUS_HEADER_FIELD_CONTAINER_INSTANCE: + case DBUS_HEADER_FIELD_CONTAINER_PATH: /* OBJECT_PATH was validated generically due to its type */ string_validation_func = NULL; break; diff --git a/dbus/dbus-message.c b/dbus/dbus-message.c index b47a8638..edca49fa 100644 --- a/dbus/dbus-message.c +++ b/dbus/dbus-message.c @@ -4097,7 +4097,7 @@ dbus_message_contains_unix_fds(DBusMessage *message) } /** - * Sets the container instance this message was sent from. + * Sets the container context this message was sent from. * * The path must contain only valid characters for an object path * as defined in the D-Bus specification. @@ -4107,8 +4107,8 @@ dbus_message_contains_unix_fds(DBusMessage *message) * @returns #FALSE if not enough memory */ dbus_bool_t -dbus_message_set_container_instance (DBusMessage *message, - const char *object_path) +dbus_message_set_container_path (DBusMessage *message, + const char *object_path) { _dbus_return_val_if_fail (message != NULL, FALSE); _dbus_return_val_if_fail (!message->locked, FALSE); @@ -4117,13 +4117,27 @@ dbus_message_set_container_instance (DBusMessage *message, FALSE); return set_or_delete_string_field (message, - DBUS_HEADER_FIELD_CONTAINER_INSTANCE, + DBUS_HEADER_FIELD_CONTAINER_PATH, DBUS_TYPE_OBJECT_PATH, object_path); } /** - * Gets the container instance this message was sent from, or #NULL + * Deprecated alias for dbus_message_set_container_path(). + * + * @param message the message + * @param object_path the path or #NULL to unset + * @returns #FALSE if not enough memory + */ +dbus_bool_t +dbus_message_set_container_instance (DBusMessage *message, + const char *object_path) +{ + return dbus_message_set_container_path (message, object_path); +} + +/** + * Gets the container context this message was sent from, or #NULL * if none. * * The returned string becomes invalid if the message is @@ -4133,7 +4147,7 @@ dbus_message_set_container_instance (DBusMessage *message, * @returns the path (should not be freed) or #NULL */ const char * -dbus_message_get_container_instance (DBusMessage *message) +dbus_message_get_container_path (DBusMessage *message) { const char *v; @@ -4141,12 +4155,24 @@ dbus_message_get_container_instance (DBusMessage *message) v = NULL; /* in case field doesn't exist */ _dbus_header_get_field_basic (&message->header, - DBUS_HEADER_FIELD_CONTAINER_INSTANCE, + DBUS_HEADER_FIELD_CONTAINER_PATH, DBUS_TYPE_OBJECT_PATH, (void *) &v); return v; } +/** + * Deprecated alias for dbus_message_set_container_instance(). + * + * @param message the message + * @returns the path (should not be freed) or #NULL + */ +const char * +dbus_message_get_container_instance (DBusMessage *message) +{ + return dbus_message_get_container_path (message); +} + /** @} */ /** diff --git a/dbus/dbus-message.h b/dbus/dbus-message.h index 0bbad3d1..3b9e2ded 100644 --- a/dbus/dbus-message.h +++ b/dbus/dbus-message.h @@ -236,10 +236,17 @@ dbus_bool_t dbus_message_get_path_decomposed (DBusMessage *message, char ***path); DBUS_EXPORT -const char *dbus_message_get_container_instance (DBusMessage *message); +const char *dbus_message_get_container_path (DBusMessage *message); DBUS_EXPORT +dbus_bool_t dbus_message_set_container_path (DBusMessage *message, + const char *object_path); +#ifndef DBUS_DISABLE_DEPRECATED +DBUS_EXPORT DBUS_DEPRECATED +const char *dbus_message_get_container_instance (DBusMessage *message); +DBUS_EXPORT DBUS_DEPRECATED dbus_bool_t dbus_message_set_container_instance (DBusMessage *message, const char *object_path); +#endif DBUS_EXPORT dbus_bool_t dbus_message_append_args (DBusMessage *message, diff --git a/dbus/dbus-protocol.h b/dbus/dbus-protocol.h index 33e39df3..8bcd289a 100644 --- a/dbus/dbus-protocol.h +++ b/dbus/dbus-protocol.h @@ -303,9 +303,15 @@ extern "C" { */ #define DBUS_HEADER_FIELD_UNIX_FDS 9 /** - * Header field code for the container instance that sent this message. + * Header field code for the container context that sent this message. */ +#define DBUS_HEADER_FIELD_CONTAINER_PATH 10 +/** + * Deprecated alias for #DBUS_HEADER_FIELD_CONTAINER_PATH + */ +#ifndef DBUS_DISABLE_DEPRECATED #define DBUS_HEADER_FIELD_CONTAINER_INSTANCE 10 +#endif /** @@ -314,7 +320,7 @@ extern "C" { * that unknown codes must be ignored, so check for that before * indexing the array. */ -#define DBUS_HEADER_FIELD_LAST DBUS_HEADER_FIELD_CONTAINER_INSTANCE +#define DBUS_HEADER_FIELD_LAST DBUS_HEADER_FIELD_CONTAINER_PATH /** Header format is defined as a signature: * byte byte order diff --git a/doc/dbus-specification.xml b/doc/dbus-specification.xml index f163e623..83001963 100644 --- a/doc/dbus-specification.xml +++ b/doc/dbus-specification.xml @@ -7024,16 +7024,18 @@ - org.freedesktop.DBus.Containers1.Instance + org.freedesktop.DBus.Containers1.Path OBJECT_PATH - The container instance object path of the server through - which this connection is connected, as returned by - AddServer. Omitted from the + The object path of the container-specific server through + which this connection is connected, as returned by the + AddServer method. Omitted from the dictionary if this connection is not via a container's server. + @@ -7485,7 +7487,7 @@ in STRING container_name, in DICT<STRING,VARIANT> metadata, in DICT<STRING,VARIANT> named_arguments, - out OBJECT_PATH container_instance, + out OBJECT_PATH server_path, out ARRAY<BYTE> socket_path, out STRING connectable_address) @@ -7550,7 +7552,7 @@ OBJECT_PATH An opaque object path identifying the new container - instance. + context. @@ -7590,11 +7592,11 @@ Each call to this method creates a new - container instance, identified by an object + container server, identified by an object path. Even if the specified container type and name are the - same as for a pre-existing container instance, each call + same as for a pre-existing container server, each call creates a new server with a new unique object path, because - the new container instance might represent a different + the new container server might represent a different version of the confined application with different characteristics and restrictions. The message bus may provide an object at the given object path, but is not required to @@ -7655,13 +7657,13 @@ - The container instance object path remains valid for as + The container server object path remains valid for as long as one or more confined connection via the same server socket remain open, or there is a way for the server socket to produce new connections in future (in other words, either it is preparing to listen for new connections, or it is currently listening for new connections). When the - container instance has ceased to listen for new connections + container server has ceased to listen for new connections and no longer has any confined connections, the object path becomes invalid, and API calls that specify it will fail with the org.freedesktop.DBus.Error.NotContainer @@ -7669,12 +7671,12 @@ - - <literal>org.freedesktop.DBus.Containers1.StopInstance</literal> + + <literal>org.freedesktop.DBus.Containers1.StopServer</literal> As a method: - StopInstance (in OBJECT_PATH container_instance) + StopServer (in OBJECT_PATH server_path) Message arguments: @@ -7693,7 +7695,7 @@ The opaque object path that was returned from the AddServer method, identifying a - container instance. + per-container server. @@ -7702,25 +7704,25 @@ - Terminate the container instance. The server will stop + Terminate the container server. The server will stop listening for new connections, and any existing connections to that server will be disconnected. When all connections have - been disconnected, the instance will cease to exist. + been disconnected, the context will cease to exist. If the given object path does not represent a valid container - instance (see + context (see ), the org.freedesktop.DBus.Error.NotContainer error is returned. In particular, if this method is called twice, the second call will fail in this way. - If the given container instance exists but the caller of this + If the given container server exists but the caller of this method is not allowed to stop it (for example because the - caller is in a container instance or because its user ID does + caller is in a container server or because its user ID does not match the user ID of the creator of the container - instance), + context), the org.freedesktop.DBus.Error.AccessDenied error is returned and nothing is stopped. @@ -7731,7 +7733,7 @@ As a method: - StopListening (in OBJECT_PATH container_instance) + StopListening (in OBJECT_PATH server_path) Message arguments: @@ -7750,7 +7752,7 @@ The opaque object path that was returned from the AddServer method, identifying a - container instance. + container server. @@ -7759,47 +7761,45 @@ - Stop listening for new connections to the server in the given - container instance, but do not disconnect any existing - connections. + Stop listening for new connections to the given per-container + server, but do not disconnect any existing connections. If this method is called more than once, while the container - instance still exists because connections to it are still + context still exists because connections to it are still open, the second and subsequent calls will be successful but will have no practical effect. If the given object path does not represent a valid container - instance (see + context (see ), the org.freedesktop.DBus.Error.NotContainer error is returned. In particular, this will happen if the - container instance already stopped listening, and all + per-container server already stopped listening, and all connections to it (if any) were already closed. - If the given container instance exists but the caller of this + If the given per-container server exists but the caller of this method is not allowed to stop it (for example because the - caller is in a container instance or because its user ID does - not match the user ID of the creator of the container - instance), + caller is itself in a container, or because its user ID does + not match the user ID of the creator of the container server), the org.freedesktop.DBus.Error.AccessDenied error is returned and nothing is stopped. - - <literal>org.freedesktop.DBus.Containers1.GetConnectionInstance</literal> + + <literal>org.freedesktop.DBus.Containers1.GetConnectionInfo</literal> As a method: - GetConnectionInstance (in STRING bus_name, - out OBJECT_PATH container_instance, - out DICT<STRING,VARIANT> creator, - out STRING container_type, - out STRING container_name, - out DICT<STRING,VARIANT> metadata) + GetConnectionInfo (in STRING bus_name, + out OBJECT_PATH server_path, + out DICT<STRING,VARIANT> creator, + out STRING container_type, + out STRING container_name, + out DICT<STRING,VARIANT> metadata) Message arguments: @@ -7827,7 +7827,7 @@ The opaque object path that was returned from the AddServer method, identifying a - container instance. + per-container server. This output parameter is produced by the message bus. @@ -7835,7 +7835,8 @@ 2 DICT<STRING,VARIANT> - The connection that created this container instance. + Credentials of the connection that created this + per-container server. The keys and values are the same as those that are documented for org.flatpak or io.snapcraft. This output parameter is controlled by the creator - of the container instance. + of the per-container server. @@ -7865,7 +7866,7 @@ whose meaning is defined by the maintainers of the container type. This output parameter is controlled by the creator - of the container instance. + of the per-container server. @@ -7876,7 +7877,7 @@ keys and values defined by the maintainers of the container type. This output parameter is controlled by the creator - of the container instance. + of the per-container server. @@ -7886,7 +7887,7 @@ If the given unique or well-known bus name is a connection to a - container server, return information about that container instance. + container server, return information about that per-container server. If the bus name exists but is not confined (in other words, if it is a direct connection to the main message bus socket), the org.freedesktop.DBus.Error.NotContainer error @@ -7896,7 +7897,7 @@ Several of the output parameters are controlled by the creator of - the container instance. + the per-container server. On a bus that accepts connections from multiple clients at different privilege levels, it is not appropriate to trust those output parameters or use them in trust decisions unless the @@ -7905,16 +7906,16 @@ - - <literal>org.freedesktop.DBus.Containers1.GetInstanceInfo</literal> + + <literal>org.freedesktop.DBus.Containers1.GetServerInfo</literal> As a method: - GetInstanceInfo (in OBJECT_PATH container_instance, - out DICT<STRING,VARIANT> creator, - out STRING container_type, - out STRING container_name, - out DICT<STRING,VARIANT> metadata) + GetServerInfo (in OBJECT_PATH server_path, + out DICT<STRING,VARIANT> creator, + out STRING container_type, + out STRING container_name, + out DICT<STRING,VARIANT> metadata) Message arguments: @@ -7933,14 +7934,15 @@ The opaque object path that was returned from the AddServer method, identifying a - container instance. + per-container server. 1 DICT<STRING,VARIANT> - The connection that created this container instance. + Credentials of the connection that created this + per-container server. The keys and values are the same as those that are documented for org.flatpak or io.snapcraft. This output parameter is controlled by the creator - of the container instance. + of the per-container server. @@ -7970,7 +7972,7 @@ whose meaning is defined by the maintainers of the container type. This output parameter is controlled by the creator - of the container instance. + of the per-container server. @@ -7981,7 +7983,7 @@ keys and values defined by the maintainers of the container type. This output parameter is controlled by the creator - of the container instance. + of the per-container server. @@ -7990,7 +7992,7 @@ - If the given object path represents a valid container instance, + If the given object path represents a valid per-container server (see ), return information about it. Otherwise, the org.freedesktop.DBus.Error.NotContainer error @@ -7998,18 +8000,18 @@ Several of the output parameters are controlled by the creator of - the container instance. - See + the per-container server. + See for more details. - - <literal>org.freedesktop.DBus.Containers1.InstanceRemoved</literal> + + <literal>org.freedesktop.DBus.Containers1.ServerRemoved</literal> As a signal emitted by the message bus: - InstanceRemoved (OBJECT_PATH container_instance) + ServerRemoved (OBJECT_PATH server_path) Message arguments: @@ -8028,7 +8030,7 @@ The opaque object path that was returned from the AddServer method, identifying a - container instance. + per-container server. @@ -8037,9 +8039,9 @@ - Emitted when a container instance ceases to exist. A container - instance continues to exist as long as it is listening for - new connections, or as long as connections to the instance + Emitted when a per-container server ceases to exist. A per-container + server continues to exist as long as it is listening for + new connections, or as long as connections to the server are open, whichever is longer. diff --git a/test/containers.c b/test/containers.c index f5bf5407..3af9bfb7 100644 --- a/test/containers.c +++ b/test/containers.c @@ -56,7 +56,7 @@ typedef struct { GDBusProxy *proxy; - gchar *instance_path; + gchar *server_path; gchar *socket_path; gchar *socket_dbus_address; GDBusConnection *unconfined_conn; @@ -65,7 +65,7 @@ typedef struct { GDBusConnection *observer_conn; GDBusProxy *observer_proxy; - GHashTable *containers_removed; + GHashTable *servers_removed; guint removed_sub; DBusConnection *libdbus_observer; DBusMessage *latest_shout; @@ -136,13 +136,13 @@ observe_shouting_cb (DBusConnection *observer, } static void -instance_removed_cb (GDBusConnection *observer, - const gchar *sender, - const gchar *path, - const gchar *iface, - const gchar *member, - GVariant *parameters, - gpointer user_data) +server_removed_cb (GDBusConnection *observer, + const gchar *sender, + const gchar *path, + const gchar *iface, + const gchar *member, + GVariant *parameters, + gpointer user_data) { Fixture *f = user_data; const gchar *container; @@ -150,11 +150,11 @@ instance_removed_cb (GDBusConnection *observer, g_assert_cmpstr (sender, ==, DBUS_SERVICE_DBUS); g_assert_cmpstr (path, ==, DBUS_PATH_DBUS); g_assert_cmpstr (iface, ==, DBUS_INTERFACE_CONTAINERS1); - g_assert_cmpstr (member, ==, "InstanceRemoved"); + g_assert_cmpstr (member, ==, "ServerRemoved"); g_assert_cmpstr (g_variant_get_type_string (parameters), ==, "(o)"); g_variant_get (parameters, "(&o)", &container); - g_assert (!g_hash_table_contains (f->containers_removed, container)); - g_hash_table_add (f->containers_removed, g_strdup (container)); + g_assert (!g_hash_table_contains (f->servers_removed, container)); + g_hash_table_add (f->servers_removed, g_strdup (container)); } static void @@ -231,15 +231,15 @@ setup (Fixture *f, G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_CLIENT), NULL, NULL, &f->error); g_assert_no_error (f->error); - f->containers_removed = g_hash_table_new_full (g_str_hash, g_str_equal, - g_free, NULL); + f->servers_removed = g_hash_table_new_full (g_str_hash, g_str_equal, + g_free, NULL); f->removed_sub = g_dbus_connection_signal_subscribe (f->observer_conn, DBUS_SERVICE_DBUS, DBUS_INTERFACE_CONTAINERS1, - "InstanceRemoved", + "ServerRemoved", DBUS_PATH_DBUS, NULL, G_DBUS_SIGNAL_FLAGS_NONE, - instance_removed_cb, + server_removed_cb, f, NULL); /* We have to use libdbus for new header fields, because GDBus doesn't @@ -340,15 +340,15 @@ add_container_server (Fixture *f, g_assert_nonnull (tuple); g_assert_cmpstr (g_variant_get_type_string (tuple), ==, "(oays)"); - g_variant_get (tuple, "(o^ays)", &f->instance_path, &f->socket_path, + g_variant_get (tuple, "(o^ays)", &f->server_path, &f->socket_path, &f->socket_dbus_address); g_assert_true (g_str_has_prefix (f->socket_dbus_address, "unix:")); g_assert_null (strchr (f->socket_dbus_address, ';')); g_assert_null (strchr (f->socket_dbus_address + strlen ("unix:"), ':')); g_clear_pointer (&tuple, g_variant_unref); - g_assert_nonnull (f->instance_path); - g_assert_true (g_variant_is_object_path (f->instance_path)); + g_assert_nonnull (f->server_path); + g_assert_true (g_variant_is_object_path (f->server_path)); g_assert_nonnull (f->socket_path); g_assert_true (g_path_is_absolute (f->socket_path)); g_assert_nonnull (f->socket_dbus_address); @@ -443,7 +443,7 @@ test_basic (Fixture *f, while (f->latest_shout == NULL) iterate_both_main_loops (f->ctx); - g_assert_cmpstr (dbus_message_get_container_instance (f->latest_shout), ==, + g_assert_cmpstr (dbus_message_get_container_path (f->latest_shout), ==, NULL); dbus_clear_message (&f->latest_shout); @@ -454,7 +454,7 @@ test_basic (Fixture *f, while (f->latest_shout == NULL) iterate_both_main_loops (f->ctx); - g_assert_cmpstr (dbus_message_get_container_instance (f->latest_shout), ==, + g_assert_cmpstr (dbus_message_get_container_path (f->latest_shout), ==, NULL); dbus_clear_message (&f->latest_shout); @@ -482,8 +482,8 @@ test_basic (Fixture *f, while (f->latest_shout == NULL) iterate_both_main_loops (f->ctx); - g_assert_cmpstr (dbus_message_get_container_instance (f->latest_shout), ==, - f->instance_path); + g_assert_cmpstr (dbus_message_get_container_path (f->latest_shout), ==, + f->server_path); dbus_clear_message (&f->latest_shout); g_dbus_connection_emit_signal (f->unconfined_conn, NULL, "/", @@ -493,7 +493,7 @@ test_basic (Fixture *f, while (f->latest_shout == NULL) iterate_both_main_loops (f->ctx); - g_assert_cmpstr (dbus_message_get_container_instance (f->latest_shout), ==, + g_assert_cmpstr (dbus_message_get_container_path (f->latest_shout), ==, "/"); dbus_clear_message (&f->latest_shout); @@ -512,7 +512,7 @@ test_basic (Fixture *f, g_test_message ("Inspecting connection container info"); confined_unique_name = g_dbus_connection_get_unique_name (f->confined_conn); - tuple = g_dbus_proxy_call_sync (f->proxy, "GetConnectionInstance", + tuple = g_dbus_proxy_call_sync (f->proxy, "GetConnectionInfo", g_variant_new ("(s)", confined_unique_name), G_DBUS_CALL_FLAGS_NONE, -1, NULL, &f->error); g_assert_no_error (f->error); @@ -520,7 +520,7 @@ test_basic (Fixture *f, g_assert_cmpstr (g_variant_get_type_string (tuple), ==, "(oa{sv}ssa{sv})"); g_variant_get (tuple, "(&o@a{sv}&s&s@a{sv})", &path_from_query, &creator, &type, &name, &asv); - g_assert_cmpstr (path_from_query, ==, f->instance_path); + g_assert_cmpstr (path_from_query, ==, f->server_path); g_variant_dict_init (&dict, creator); g_assert_true (g_variant_dict_lookup (&dict, "UnixUserID", "u", &uid)); g_assert_cmpuint (uid, ==, _dbus_getuid ()); @@ -533,9 +533,9 @@ test_basic (Fixture *f, g_clear_pointer (&creator, g_variant_unref); g_clear_pointer (&tuple, g_variant_unref); - g_test_message ("Inspecting container instance info"); - tuple = g_dbus_proxy_call_sync (f->proxy, "GetInstanceInfo", - g_variant_new ("(o)", f->instance_path), + g_test_message ("Inspecting container server info"); + tuple = g_dbus_proxy_call_sync (f->proxy, "GetServerInfo", + g_variant_new ("(o)", f->server_path), G_DBUS_CALL_FLAGS_NONE, -1, NULL, &f->error); g_assert_no_error (f->error); g_assert_nonnull (tuple); @@ -675,15 +675,15 @@ test_metadata (Fixture *f, asv = g_variant_get_child_value (tuple, 0); g_variant_dict_init (&dict, asv); g_assert_true (g_variant_dict_lookup (&dict, - DBUS_INTERFACE_CONTAINERS1 ".Instance", + DBUS_INTERFACE_CONTAINERS1 ".Path", "&o", &path_from_query)); - g_assert_cmpstr (path_from_query, ==, f->instance_path); + g_assert_cmpstr (path_from_query, ==, f->server_path); g_variant_dict_clear (&dict); g_clear_pointer (&asv, g_variant_unref); g_clear_pointer (&tuple, g_variant_unref); g_test_message ("Inspecting connection container info"); - tuple = g_dbus_proxy_call_sync (f->proxy, "GetConnectionInstance", + tuple = g_dbus_proxy_call_sync (f->proxy, "GetConnectionInfo", g_variant_new ("(s)", confined_unique_name), G_DBUS_CALL_FLAGS_NONE, -1, NULL, &f->error); g_assert_no_error (f->error); @@ -691,7 +691,7 @@ test_metadata (Fixture *f, g_assert_cmpstr (g_variant_get_type_string (tuple), ==, "(oa{sv}ssa{sv})"); g_variant_get (tuple, "(&o@a{sv}&s&s@a{sv})", &path_from_query, &creator, &type, &name, &asv); - g_assert_cmpstr (path_from_query, ==, f->instance_path); + g_assert_cmpstr (path_from_query, ==, f->server_path); g_variant_dict_init (&dict, creator); g_assert_true (g_variant_dict_lookup (&dict, "UnixUserID", "u", &uid)); g_assert_cmpuint (uid, ==, _dbus_getuid ()); @@ -711,9 +711,9 @@ test_metadata (Fixture *f, g_clear_pointer (&creator, g_variant_unref); g_clear_pointer (&tuple, g_variant_unref); - g_test_message ("Inspecting container instance info"); - tuple = g_dbus_proxy_call_sync (f->proxy, "GetInstanceInfo", - g_variant_new ("(o)", f->instance_path), + g_test_message ("Inspecting container server info"); + tuple = g_dbus_proxy_call_sync (f->proxy, "GetServerInfo", + g_variant_new ("(o)", f->server_path), G_DBUS_CALL_FLAGS_NONE, -1, NULL, &f->error); g_assert_no_error (f->error); g_assert_nonnull (tuple); @@ -753,7 +753,7 @@ test_metadata (Fixture *f, * Test StopListening(), which just closes the listening socket. * * With config->stop_server == STOP_SERVER_FORCE: - * Test StopInstance(), which closes the listening socket and + * Test StopServer(), which closes the listening socket and * disconnects all its clients. */ static void @@ -834,7 +834,7 @@ test_stop_server (Fixture *f, } /* If we are able to switch uid (i.e. we are root), check that a local - * attacker with a different uid cannot close our container instances. */ + * attacker with a different uid cannot close our container servers. */ attacker = test_try_connect_gdbus_as_user (f->bus_address, TEST_USER_OTHER, &f->error); @@ -849,15 +849,15 @@ test_stop_server (Fixture *f, g_assert_no_error (f->error); tuple = g_dbus_proxy_call_sync (attacker_proxy, "StopListening", - g_variant_new ("(o)", f->instance_path), + g_variant_new ("(o)", f->server_path), G_DBUS_CALL_FLAGS_NONE, -1, NULL, &f->error); g_assert_error (f->error, G_DBUS_ERROR, G_DBUS_ERROR_ACCESS_DENIED); g_assert_null (tuple); g_clear_error (&f->error); - tuple = g_dbus_proxy_call_sync (attacker_proxy, "StopInstance", - g_variant_new ("(o)", f->instance_path), + tuple = g_dbus_proxy_call_sync (attacker_proxy, "StopServer", + g_variant_new ("(o)", f->server_path), G_DBUS_CALL_FLAGS_NONE, -1, NULL, &f->error); g_assert_error (f->error, G_DBUS_ERROR, G_DBUS_ERROR_ACCESS_DENIED); @@ -878,8 +878,8 @@ test_stop_server (Fixture *f, g_clear_error (&f->error); } - g_assert_false (g_hash_table_contains (f->containers_removed, - f->instance_path)); + g_assert_false (g_hash_table_contains (f->servers_removed, + f->server_path)); switch (config->stop_server) { @@ -906,18 +906,18 @@ test_stop_server (Fixture *f, case STOP_SERVER_EXPLICITLY: g_test_message ("Stopping server (but not confined connection)..."); tuple = g_dbus_proxy_call_sync (f->proxy, "StopListening", - g_variant_new ("(o)", f->instance_path), + g_variant_new ("(o)", f->server_path), G_DBUS_CALL_FLAGS_NONE, -1, NULL, &f->error); g_assert_no_error (f->error); g_variant_unref (tuple); - /* The container instance remains open, because the connection has + /* The container server remains open, because the connection has * not gone away yet. Do another method call: if we were going to * get the signal, it would arrive before the reply to this second * method call. Any method will do here, even one that doesn't * exist. */ - g_test_message ("Checking we do not get InstanceRemoved..."); + g_test_message ("Checking we do not get ServerRemoved..."); tuple = g_dbus_proxy_call_sync (f->proxy, "NoSuchMethod", NULL, G_DBUS_CALL_FLAGS_NONE, -1, NULL, &f->error); @@ -930,29 +930,29 @@ test_stop_server (Fixture *f, case STOP_SERVER_NEVER_CONNECTED: g_test_message ("Stopping server (with no confined connections)..."); tuple = g_dbus_proxy_call_sync (f->proxy, "StopListening", - g_variant_new ("(o)", f->instance_path), + g_variant_new ("(o)", f->server_path), G_DBUS_CALL_FLAGS_NONE, -1, NULL, &f->error); g_assert_no_error (f->error); g_variant_unref (tuple); - g_test_message ("Waiting for InstanceRemoved..."); - while (!g_hash_table_contains (f->containers_removed, f->instance_path)) + g_test_message ("Waiting for ServerRemoved..."); + while (!g_hash_table_contains (f->servers_removed, f->server_path)) g_main_context_iteration (NULL, TRUE); break; case STOP_SERVER_FORCE: g_test_message ("Stopping server and all confined connections..."); - tuple = g_dbus_proxy_call_sync (f->proxy, "StopInstance", - g_variant_new ("(o)", f->instance_path), + tuple = g_dbus_proxy_call_sync (f->proxy, "StopServer", + g_variant_new ("(o)", f->server_path), G_DBUS_CALL_FLAGS_NONE, -1, NULL, &f->error); g_assert_no_error (f->error); g_variant_unref (tuple); - g_test_message ("Waiting for InstanceRemoved..."); - while (!g_hash_table_contains (f->containers_removed, f->instance_path)) + g_test_message ("Waiting for ServerRemoved..."); + while (!g_hash_table_contains (f->servers_removed, f->server_path)) g_main_context_iteration (NULL, TRUE); break; @@ -1051,10 +1051,10 @@ test_stop_server (Fixture *f, g_assert_cmpstr (name_owner, ==, DBUS_SERVICE_DBUS); g_clear_pointer (&tuple, g_variant_unref); - /* The container instance will not disappear from the bus + /* The container server will not disappear from the bus * until the confined connection goes away */ - tuple = g_dbus_proxy_call_sync (f->observer_proxy, "GetInstanceInfo", - g_variant_new ("(o)", f->instance_path), + tuple = g_dbus_proxy_call_sync (f->observer_proxy, "GetServerInfo", + g_variant_new ("(o)", f->server_path), G_DBUS_CALL_FLAGS_NONE, -1, NULL, &f->error); g_assert_no_error (f->error); @@ -1062,7 +1062,7 @@ test_stop_server (Fixture *f, g_clear_pointer (&tuple, g_variant_unref); /* Now disconnect the last confined connection, which will make the - * container instance go away */ + * container server go away */ g_test_message ("Closing confined connection..."); g_dbus_connection_close_sync (f->confined_conn, NULL, &f->error); g_assert_no_error (f->error); @@ -1074,12 +1074,12 @@ test_stop_server (Fixture *f, /* Whatever happened above, by now it has gone away */ - g_test_message ("Waiting for InstanceRemoved..."); - while (!g_hash_table_contains (f->containers_removed, f->instance_path)) + g_test_message ("Waiting for ServerRemoved..."); + while (!g_hash_table_contains (f->servers_removed, f->server_path)) g_main_context_iteration (NULL, TRUE); - tuple = g_dbus_proxy_call_sync (f->observer_proxy, "GetInstanceInfo", - g_variant_new ("(o)", f->instance_path), + tuple = g_dbus_proxy_call_sync (f->observer_proxy, "GetServerInfo", + g_variant_new ("(o)", f->server_path), G_DBUS_CALL_FLAGS_NONE, -1, NULL, &f->error); g_assert_nonnull (f->error); @@ -1097,7 +1097,7 @@ test_stop_server (Fixture *f, /* * Assert that we cannot get the container metadata for a path that - * isn't a container instance, or a bus name that isn't in a container + * isn't a container server, or a bus name that isn't in a container * or doesn't exist at all. */ static void @@ -1117,7 +1117,7 @@ test_invalid_metadata_getters (Fixture *f, g_test_message ("Inspecting unconfined connection"); unique_name = g_dbus_connection_get_unique_name (f->unconfined_conn); - tuple = g_dbus_proxy_call_sync (f->proxy, "GetConnectionInstance", + tuple = g_dbus_proxy_call_sync (f->proxy, "GetConnectionInfo", g_variant_new ("(s)", unique_name), G_DBUS_CALL_FLAGS_NONE, -1, NULL, &f->error); g_assert_nonnull (f->error); @@ -1133,7 +1133,7 @@ test_invalid_metadata_getters (Fixture *f, g_clear_error (&f->error); g_test_message ("Inspecting dbus-daemon"); - tuple = g_dbus_proxy_call_sync (f->proxy, "GetConnectionInstance", + tuple = g_dbus_proxy_call_sync (f->proxy, "GetConnectionInfo", g_variant_new ("(s)", DBUS_SERVICE_DBUS), G_DBUS_CALL_FLAGS_NONE, -1, NULL, &f->error); g_assert_nonnull (f->error); @@ -1150,7 +1150,7 @@ test_invalid_metadata_getters (Fixture *f, g_test_message ("Inspecting a non-connection"); unique_name = g_dbus_connection_get_unique_name (f->unconfined_conn); - tuple = g_dbus_proxy_call_sync (f->proxy, "GetConnectionInstance", + tuple = g_dbus_proxy_call_sync (f->proxy, "GetConnectionInfo", g_variant_new ("(s)", "com.example.Nope"), G_DBUS_CALL_FLAGS_NONE, -1, NULL, &f->error); g_assert_nonnull (f->error); @@ -1166,8 +1166,8 @@ test_invalid_metadata_getters (Fixture *f, g_clear_error (&f->error); - g_test_message ("Inspecting container instance info"); - tuple = g_dbus_proxy_call_sync (f->proxy, "GetInstanceInfo", + g_test_message ("Inspecting container server info"); + tuple = g_dbus_proxy_call_sync (f->proxy, "GetServerInfo", g_variant_new ("(o)", "/nope"), G_DBUS_CALL_FLAGS_NONE, -1, NULL, &f->error); g_assert_nonnull (f->error); @@ -1643,7 +1643,7 @@ teardown (Fixture *f, g_clear_object (&f->proxy); fixture_disconnect_observer (f); - g_clear_pointer (&f->containers_removed, g_hash_table_unref); + g_clear_pointer (&f->servers_removed, g_hash_table_unref); if (f->libdbus_observer != NULL) { @@ -1679,7 +1679,7 @@ teardown (Fixture *f, } dbus_clear_message (&f->latest_shout); - g_free (f->instance_path); + g_free (f->server_path); g_free (f->socket_path); g_free (f->socket_dbus_address); g_free (f->bus_address); diff --git a/test/internals/dbus-message-util.c b/test/internals/dbus-message-util.c index 310822d6..cd8259c7 100644 --- a/test/internals/dbus-message-util.c +++ b/test/internals/dbus-message-util.c @@ -1170,9 +1170,9 @@ _dbus_message_test (const char *test_data_dir _DBUS_GNUC_UNUSED) _dbus_assert (strcmp (dbus_message_get_path (message), "/foo") == 0); - if (!dbus_message_set_container_instance (message, "/org/freedesktop/DBus/Containers1/c42")) + if (!dbus_message_set_container_path (message, "/org/freedesktop/DBus/Containers1/c42")) _dbus_test_fatal ("out of memory"); - _dbus_assert (strcmp (dbus_message_get_container_instance (message), + _dbus_assert (strcmp (dbus_message_get_container_path (message), "/org/freedesktop/DBus/Containers1/c42") == 0); if (!dbus_message_set_interface (message, "org.Foo")) From 59ebc4e62a0dfc6cbb3d71bdc04663e68f29cd9d Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Fri, 27 Oct 2023 19:25:56 +0100 Subject: [PATCH 09/23] spec: Reduce repetition in Containers1 All outputs from GetServerInfo are the same as GetConnectionInfo, so only give a very short summary in GetServerInfo. Signed-off-by: Simon McVittie --- doc/dbus-specification.xml | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/doc/dbus-specification.xml b/doc/dbus-specification.xml index 83001963..ce6cf04c 100644 --- a/doc/dbus-specification.xml +++ b/doc/dbus-specification.xml @@ -7826,7 +7826,8 @@ OBJECT_PATH The opaque object path that was returned from the - AddServer method, identifying a + AddServer method, identifying a per-container server. This output parameter is produced by the message bus. @@ -7956,10 +7957,10 @@ STRING Reversed domain name identifying a container - manager or container technology, as passed to the - AddServer method, such as - org.flatpak or - io.snapcraft. + manager or container technology, + the same as the corresponding parameter in + GetConnectionInfo. This output parameter is controlled by the creator of the per-container server. @@ -7969,8 +7970,9 @@ STRING Some unique identifier for an application or container, - whose meaning is defined by the maintainers of the - container type. + the same as the corresponding parameter in + GetConnectionInfo. This output parameter is controlled by the creator of the per-container server. @@ -7979,9 +7981,10 @@ 4 DICT<STRING,VARIANT> - Metadata describing the application or container, with the - keys and values defined by the maintainers of the container - type. + Metadata describing the application or container, + the same as the corresponding parameter in + GetConnectionInfo. This output parameter is controlled by the creator of the per-container server. From 3d5d9152aa1dda9b2494f70c876decb8e50a178b Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Fri, 27 Oct 2023 19:45:41 +0100 Subject: [PATCH 10/23] Containers: Replace "name" with the app ID and instance ID This aligns it with the analogous Wayland specification security-context-v1, and in particular allows Flatpak-aware applications to look up the instance's sandboxing parameters and other metadata. Helps: https://gitlab.freedesktop.org/dbus/dbus/-/issues/479 Signed-off-by: Simon McVittie --- bus/connection.c | 13 ++++--- bus/containers.c | 58 +++++++++++++++++++--------- bus/containers.h | 3 +- bus/driver.c | 10 ++--- doc/dbus-specification.xml | 77 +++++++++++++++++++++++++++++++------ test/containers.c | 79 ++++++++++++++++++++++++-------------- 6 files changed, 171 insertions(+), 69 deletions(-) diff --git a/bus/connection.c b/bus/connection.c index de00ceee..dccb0d61 100644 --- a/bus/connection.c +++ b/bus/connection.c @@ -595,7 +595,8 @@ cache_peer_loginfo_string (BusConnectionData *d, dbus_bool_t prev_added; const char *container = NULL; const char *container_type = NULL; - const char *container_name = NULL; + const char *app_id = NULL; + const char *instance_id = NULL; DBusCredentials *credentials; if (!_dbus_string_init (&loginfo_buf)) @@ -679,7 +680,8 @@ cache_peer_loginfo_string (BusConnectionData *d, /* This does have to come from the connection, not the credentials */ if (bus_containers_connection_is_contained (connection, &container, &container_type, - &container_name)) + &app_id, + &instance_id)) { dbus_bool_t did_append; @@ -690,10 +692,11 @@ cache_peer_loginfo_string (BusConnectionData *d, } did_append = _dbus_string_append_printf (&loginfo_buf, - "container=%s %s=\"%s\")", + "container=%s %s=\"%s\" inst=\"%s\")", container, container_type, - container_name); + app_id, + instance_id); if (!did_append) goto oom; else @@ -2485,7 +2488,7 @@ bus_transaction_send (BusTransaction *transaction, if (sender == NULL || !bus_containers_connection_is_contained (sender, &path, - NULL, NULL)) + NULL, NULL, NULL)) path = "/"; if (!dbus_message_set_container_path (message, path)) diff --git a/bus/containers.c b/bus/containers.c index 08c32e13..a65f3daf 100644 --- a/bus/containers.c +++ b/bus/containers.c @@ -53,7 +53,8 @@ typedef struct int refcount; char *path; char *type; - char *name; + char *app_id; + char *instance_id; DBusVariant *metadata; BusContext *context; BusContainers *containers; @@ -339,7 +340,8 @@ bus_container_server_unref (BusContainerServer *self) dbus_connection_unref (self->creator); dbus_free (self->path); dbus_free (self->type); - dbus_free (self->name); + dbus_free (self->app_id); + dbus_free (self->instance_id); dbus_free (self); } } @@ -397,7 +399,8 @@ bus_container_server_new (BusContext *context, self->refcount = 1; self->type = NULL; - self->name = NULL; + self->app_id = NULL; + self->instance_id = NULL; self->metadata = NULL; self->context = bus_context_ref (context); self->containers = bus_containers_ref (containers); @@ -511,7 +514,7 @@ new_connection_cb (DBusServer *lower_level_server, "Closing connection to container server " "%s (%s \"%s\") because it would exceed resource limit " "(max_connections_per_container=%d)", - server->path, server->type, server->name, limit); + server->path, server->type, server->app_id, limit); return; } @@ -680,7 +683,8 @@ bus_containers_handle_add_server (DBusConnection *connection, DBusMessageIter writer; DBusMessageIter array_writer; const char *type; - const char *name; + const char *app_id; + const char *instance_id; const char *path; BusContainerServer *server = NULL; BusContext *context; @@ -732,7 +736,7 @@ bus_containers_handle_add_server (DBusConnection *connection, } /* We already checked this in bus_driver_handle_message() */ - _dbus_assert (dbus_message_has_signature (message, "ssa{sv}a{sv}")); + _dbus_assert (dbus_message_has_signature (message, "sssa{sv}a{sv}")); /* Argument 0: Container type */ if (!dbus_message_iter_init (message, &iter)) @@ -753,18 +757,29 @@ bus_containers_handle_add_server (DBusConnection *connection, goto fail; } - /* Argument 1: Name as defined by container manager */ + /* Argument 1: app-ID as defined by container manager */ if (!dbus_message_iter_next (&iter)) _dbus_assert_not_reached ("Message type was already checked"); _dbus_assert (dbus_message_iter_get_arg_type (&iter) == DBUS_TYPE_STRING); - dbus_message_iter_get_basic (&iter, &name); - server->name = _dbus_strdup (name); + dbus_message_iter_get_basic (&iter, &app_id); + server->app_id = _dbus_strdup (app_id); - if (server->name == NULL) + if (server->app_id == NULL) goto oom; - /* Argument 2: Metadata as defined by container manager */ + /* Argument 2: instance-ID as defined by container manager */ + if (!dbus_message_iter_next (&iter)) + _dbus_assert_not_reached ("Message type was already checked"); + + _dbus_assert (dbus_message_iter_get_arg_type (&iter) == DBUS_TYPE_STRING); + dbus_message_iter_get_basic (&iter, &instance_id); + server->instance_id = _dbus_strdup (instance_id); + + if (server->instance_id == NULL) + goto oom; + + /* Argument 3: Metadata as defined by container manager */ if (!dbus_message_iter_next (&iter)) _dbus_assert_not_reached ("Message type was already checked"); @@ -781,7 +796,8 @@ bus_containers_handle_add_server (DBusConnection *connection, * int value. */ metadata_size = _dbus_variant_get_length (server->metadata) + (int) strlen (type) + - (int) strlen (name); + (int) strlen (app_id) + + (int) strlen (instance_id); limit = bus_context_get_max_container_metadata_bytes (context); if (metadata_size > limit) @@ -801,7 +817,7 @@ bus_containers_handle_add_server (DBusConnection *connection, goto fail; } - /* Argument 3: Named parameters */ + /* Argument 4: Named parameters */ if (!dbus_message_iter_next (&iter)) _dbus_assert_not_reached ("Message type was already checked"); @@ -1216,7 +1232,8 @@ bus_containers_handle_get_connection_info (DBusConnection *caller, if (!dbus_message_append_args (reply, DBUS_TYPE_STRING, &server->type, - DBUS_TYPE_STRING, &server->name, + DBUS_TYPE_STRING, &server->app_id, + DBUS_TYPE_STRING, &server->instance_id, DBUS_TYPE_INVALID)) goto oom; @@ -1300,7 +1317,8 @@ bus_containers_handle_get_server_info (DBusConnection *connection, if (!dbus_message_append_args (reply, DBUS_TYPE_STRING, &server->type, - DBUS_TYPE_STRING, &server->name, + DBUS_TYPE_STRING, &server->app_id, + DBUS_TYPE_STRING, &server->instance_id, DBUS_TYPE_INVALID)) goto oom; @@ -1452,7 +1470,8 @@ dbus_bool_t bus_containers_connection_is_contained (DBusConnection *connection, const char **path, const char **type, - const char **name) + const char **app_id, + const char **instance_id) { #ifdef DBUS_ENABLE_CONTAINERS BusContainerServer *server; @@ -1467,8 +1486,11 @@ bus_containers_connection_is_contained (DBusConnection *connection, if (type != NULL) *type = server->type; - if (name != NULL) - *name = server->name; + if (app_id != NULL) + *app_id = server->app_id; + + if (instance_id != NULL) + *instance_id = server->instance_id; return TRUE; } diff --git a/bus/containers.h b/bus/containers.h index c5027f8d..4e3af6a7 100644 --- a/bus/containers.h +++ b/bus/containers.h @@ -66,7 +66,8 @@ void bus_containers_remove_connection (BusContainers *self, dbus_bool_t bus_containers_connection_is_contained (DBusConnection *connection, const char **path, const char **type, - const char **name); + const char **app_id, + const char **instance_id); static inline void bus_clear_containers (BusContainers **containers_p) diff --git a/bus/driver.c b/bus/driver.c index 7b0bba96..0ef07e92 100644 --- a/bus/driver.c +++ b/bus/driver.c @@ -118,7 +118,7 @@ bus_driver_check_caller_is_not_container (DBusConnection *connection, DBusMessage *message, DBusError *error) { - if (bus_containers_connection_is_contained (connection, NULL, NULL, NULL)) + if (bus_containers_connection_is_contained (connection, NULL, NULL, NULL, NULL)) { const char *method = dbus_message_get_member (message); @@ -2033,7 +2033,7 @@ bus_driver_fill_connection_credentials (DBusCredentials *credentials, /* This has to come from the connection, not the credentials */ if (peer_conn != NULL && - bus_containers_connection_is_contained (peer_conn, &path, NULL, NULL)) + bus_containers_connection_is_contained (peer_conn, &path, NULL, NULL, NULL)) { if (!_dbus_asv_add_object_path (asv_iter, DBUS_INTERFACE_CONTAINERS1 ".Path", @@ -2647,16 +2647,16 @@ static const MessageHandler introspectable_message_handlers[] = { #ifdef DBUS_ENABLE_CONTAINERS static const MessageHandler containers_message_handlers[] = { - { "AddServer", "ssa{sv}a{sv}", "oays", bus_containers_handle_add_server, + { "AddServer", "sssa{sv}a{sv}", "oays", bus_containers_handle_add_server, METHOD_FLAG_NO_CONTAINERS }, { "StopServer", "o", "", bus_containers_handle_stop_server, METHOD_FLAG_NO_CONTAINERS }, { "StopListening", "o", "", bus_containers_handle_stop_listening, METHOD_FLAG_NO_CONTAINERS }, - { "GetConnectionInfo", "s", "oa{sv}ssa{sv}", + { "GetConnectionInfo", "s", "oa{sv}sssa{sv}", bus_containers_handle_get_connection_info, METHOD_FLAG_NONE }, - { "GetServerInfo", "o", "a{sv}ssa{sv}", bus_containers_handle_get_server_info, + { "GetServerInfo", "o", "a{sv}sssa{sv}", bus_containers_handle_get_server_info, METHOD_FLAG_NONE }, { "RequestHeader", "", "", bus_containers_handle_request_header, METHOD_FLAG_NONE }, diff --git a/doc/dbus-specification.xml b/doc/dbus-specification.xml index ce6cf04c..d3da33d1 100644 --- a/doc/dbus-specification.xml +++ b/doc/dbus-specification.xml @@ -7484,7 +7484,8 @@ As a method: AddServer (in STRING container_type, - in STRING container_name, + in STRING app_id, + in STRING instance_id, in DICT<STRING,VARIANT> metadata, in DICT<STRING,VARIANT> named_arguments, out OBJECT_PATH server_path, @@ -7529,6 +7530,18 @@ 2 + STRING + + Some unique identifier for a particular instance of an + application or container, whose meaning is defined by the + maintainers of the container type. If the container + type does not have a concept of identifying or + naming instances by a string, + using the empty string here is suggested. + + + + 3 DICT<STRING,VARIANT> Metadata describing the application or container, with the @@ -7537,7 +7550,7 @@ - 3 + 4 DICT<STRING,VARIANT> Additional arguments that extend this method. @@ -7548,7 +7561,7 @@ - 4 + 5 OBJECT_PATH An opaque object path identifying the new container @@ -7556,7 +7569,7 @@ - 5 + 6 ARRAY<BYTE> The absolute filesystem path of the resulting @@ -7567,7 +7580,7 @@ - 6 + 7 STRING A connectable D-Bus address, for example @@ -7593,12 +7606,14 @@ Each call to this method creates a new container server, identified by an object - path. Even if the specified container type and name are the - same as for a pre-existing container server, each call + path. Even if the specified container type, app ID and instance ID + are the same as for a pre-existing container server, each call creates a new server with a new unique object path, because the new container server might represent a different version of the confined application with different - characteristics and restrictions. The message bus may provide + characteristics and restrictions (although in practice it is + expected that container managers are most likely to create one + server per instance ID). The message bus may provide an object at the given object path, but is not required to do so. @@ -7798,7 +7813,8 @@ out OBJECT_PATH server_path, out DICT<STRING,VARIANT> creator, out STRING container_type, - out STRING container_name, + out STRING app_id, + out STRING instance_id, out DICT<STRING,VARIANT> metadata) Message arguments: @@ -7863,15 +7879,40 @@ 4 STRING - Some unique identifier for an application or container, + A unique identifier for an application or container, whose meaning is defined by the maintainers of the container type. + For example, Flatpak would use a freedesktop.org app ID + such as org.mozilla.firefox, but + Snap might use a Snap app ID such as + firefox. This output parameter is controlled by the creator of the per-container server. + Depending on the container type, it might be possible + to look up further metadata for the application in a + container-type-specific way. 5 + STRING + + A unique identifier for a particular instance of an + application or container, whose meaning is defined by + the maintainers of the container type. + For example, Flatpak would use the same numeric + instance ID that is shown by flatpak ps. + This output parameter is controlled by the creator + of the per-container server. + Depending on the container type, it might be possible + to look up further metadata for the instance in a + container-type-specific way, for example by reading + $XDG_RUNTIME_DIR/.flatpak/$INSTANCE/info + for a Flatpak app. + + + + 6 DICT<STRING,VARIANT> Metadata describing the application or container, with the @@ -7915,7 +7956,8 @@ GetServerInfo (in OBJECT_PATH server_path, out DICT<STRING,VARIANT> creator, out STRING container_type, - out STRING container_name, + out STRING app_id, + out STRING instance_id, out DICT<STRING,VARIANT> metadata) Message arguments: @@ -7979,6 +8021,19 @@ 4 + STRING + + Some unique identifier for a particular instance of an + application or container, + the same as the corresponding parameter in + GetConnectionInfo. + This output parameter is controlled by the creator + of the per-container server. + + + + 5 DICT<STRING,VARIANT> Metadata describing the application or container, diff --git a/test/containers.c b/test/containers.c index 3af9bfb7..e613e0f4 100644 --- a/test/containers.c +++ b/test/containers.c @@ -378,7 +378,8 @@ test_basic (Fixture *f, GVariantDict dict; const gchar *confined_unique_name; const gchar *path_from_query; - const gchar *name; + const gchar *app_id; + const gchar *instance_id; const gchar *name_owner; const gchar *type; guint32 uid; @@ -391,9 +392,10 @@ test_basic (Fixture *f, if (f->skip) return; - parameters = g_variant_new ("(ssa{sv}a{sv})", + parameters = g_variant_new ("(sssa{sv}a{sv})", "com.example.NotFlatpak", "sample-app", + "sample-instance", NULL, /* no metadata */ NULL); /* no named arguments */ if (!add_container_server (f, g_steal_pointer (¶meters))) @@ -517,16 +519,17 @@ test_basic (Fixture *f, G_DBUS_CALL_FLAGS_NONE, -1, NULL, &f->error); g_assert_no_error (f->error); g_assert_nonnull (tuple); - g_assert_cmpstr (g_variant_get_type_string (tuple), ==, "(oa{sv}ssa{sv})"); - g_variant_get (tuple, "(&o@a{sv}&s&s@a{sv})", - &path_from_query, &creator, &type, &name, &asv); + g_assert_cmpstr (g_variant_get_type_string (tuple), ==, "(oa{sv}sssa{sv})"); + g_variant_get (tuple, "(&o@a{sv}&s&s&s@a{sv})", + &path_from_query, &creator, &type, &app_id, &instance_id, &asv); g_assert_cmpstr (path_from_query, ==, f->server_path); g_variant_dict_init (&dict, creator); g_assert_true (g_variant_dict_lookup (&dict, "UnixUserID", "u", &uid)); g_assert_cmpuint (uid, ==, _dbus_getuid ()); g_variant_dict_clear (&dict); g_assert_cmpstr (type, ==, "com.example.NotFlatpak"); - g_assert_cmpstr (name, ==, "sample-app"); + g_assert_cmpstr (app_id, ==, "sample-app"); + g_assert_cmpstr (instance_id, ==, "sample-instance"); /* Trivial case: the metadata a{sv} is empty */ g_assert_cmpuint (g_variant_n_children (asv), ==, 0); g_clear_pointer (&asv, g_variant_unref); @@ -539,14 +542,16 @@ test_basic (Fixture *f, G_DBUS_CALL_FLAGS_NONE, -1, NULL, &f->error); g_assert_no_error (f->error); g_assert_nonnull (tuple); - g_assert_cmpstr (g_variant_get_type_string (tuple), ==, "(a{sv}ssa{sv})"); - g_variant_get (tuple, "(@a{sv}&s&s@a{sv})", &creator, &type, &name, &asv); + g_assert_cmpstr (g_variant_get_type_string (tuple), ==, "(a{sv}sssa{sv})"); + g_variant_get (tuple, "(@a{sv}&s&s&s@a{sv})", + &creator, &type, &app_id, &instance_id, &asv); g_variant_dict_init (&dict, creator); g_assert_true (g_variant_dict_lookup (&dict, "UnixUserID", "u", &uid)); g_assert_cmpuint (uid, ==, _dbus_getuid ()); g_variant_dict_clear (&dict); g_assert_cmpstr (type, ==, "com.example.NotFlatpak"); - g_assert_cmpstr (name, ==, "sample-app"); + g_assert_cmpstr (app_id, ==, "sample-app"); + g_assert_cmpstr (instance_id, ==, "sample-instance"); /* Trivial case: the metadata a{sv} is empty */ g_assert_cmpuint (g_variant_n_children (asv), ==, 0); g_clear_pointer (&asv, g_variant_unref); @@ -582,9 +587,10 @@ test_wrong_uid (Fixture *f, if (f->skip) return; - parameters = g_variant_new ("(ssa{sv}a{sv})", + parameters = g_variant_new ("(sssa{sv}a{sv})", "com.example.NotFlatpak", "sample-app", + "instance1", NULL, /* no metadata */ NULL); /* no named arguments */ if (!add_container_server (f, g_steal_pointer (¶meters))) @@ -613,7 +619,8 @@ test_wrong_uid (Fixture *f, /* * Test for non-trivial metadata: assert that the metadata a{sv} is - * carried through correctly, and that the app name is allowed to be empty. + * carried through correctly, and that the app/instance IDs are + * allowed to be empty. */ static void test_metadata (Fixture *f, @@ -627,7 +634,8 @@ test_metadata (Fixture *f, GVariantDict dict; const gchar *confined_unique_name; const gchar *path_from_query; - const gchar *name; + const gchar *app_id; + const gchar *instance_id; const gchar *type; guint32 uid; guint u; @@ -642,9 +650,11 @@ test_metadata (Fixture *f, g_variant_dict_insert (&dict, "IsCrepuscular", "b", TRUE); g_variant_dict_insert (&dict, "NChildren", "u", 2); - parameters = g_variant_new ("(ss@a{sv}a{sv})", + parameters = g_variant_new ("(sss@a{sv}a{sv})", "org.example.Springwatch", - /* Verify that empty app names are OK */ + /* Verify that empty app IDs are OK */ + "", + /* Verify that empty instance IDs are OK */ "", g_variant_dict_end (&dict), NULL); /* no named arguments */ @@ -688,16 +698,17 @@ test_metadata (Fixture *f, G_DBUS_CALL_FLAGS_NONE, -1, NULL, &f->error); g_assert_no_error (f->error); g_assert_nonnull (tuple); - g_assert_cmpstr (g_variant_get_type_string (tuple), ==, "(oa{sv}ssa{sv})"); - g_variant_get (tuple, "(&o@a{sv}&s&s@a{sv})", - &path_from_query, &creator, &type, &name, &asv); + g_assert_cmpstr (g_variant_get_type_string (tuple), ==, "(oa{sv}sssa{sv})"); + g_variant_get (tuple, "(&o@a{sv}&s&s&s@a{sv})", + &path_from_query, &creator, &type, &app_id, &instance_id, &asv); g_assert_cmpstr (path_from_query, ==, f->server_path); g_variant_dict_init (&dict, creator); g_assert_true (g_variant_dict_lookup (&dict, "UnixUserID", "u", &uid)); g_assert_cmpuint (uid, ==, _dbus_getuid ()); g_variant_dict_clear (&dict); g_assert_cmpstr (type, ==, "org.example.Springwatch"); - g_assert_cmpstr (name, ==, ""); + g_assert_cmpstr (app_id, ==, ""); + g_assert_cmpstr (instance_id, ==, ""); g_variant_dict_init (&dict, asv); g_assert_true (g_variant_dict_lookup (&dict, "NChildren", "u", &u)); g_assert_cmpuint (u, ==, 2); @@ -717,14 +728,16 @@ test_metadata (Fixture *f, G_DBUS_CALL_FLAGS_NONE, -1, NULL, &f->error); g_assert_no_error (f->error); g_assert_nonnull (tuple); - g_assert_cmpstr (g_variant_get_type_string (tuple), ==, "(a{sv}ssa{sv})"); - g_variant_get (tuple, "(@a{sv}&s&s@a{sv})", &creator, &type, &name, &asv); + g_assert_cmpstr (g_variant_get_type_string (tuple), ==, "(a{sv}sssa{sv})"); + g_variant_get (tuple, "(@a{sv}&s&s&s@a{sv})", + &creator, &type, &app_id, &instance_id, &asv); g_variant_dict_init (&dict, creator); g_assert_true (g_variant_dict_lookup (&dict, "UnixUserID", "u", &uid)); g_assert_cmpuint (uid, ==, _dbus_getuid ()); g_variant_dict_clear (&dict); g_assert_cmpstr (type, ==, "org.example.Springwatch"); - g_assert_cmpstr (name, ==, ""); + g_assert_cmpstr (app_id, ==, ""); + g_assert_cmpstr (instance_id, ==, ""); g_variant_dict_init (&dict, asv); g_assert_true (g_variant_dict_lookup (&dict, "NChildren", "u", &u)); g_assert_cmpuint (u, ==, 2); @@ -789,9 +802,10 @@ test_stop_server (Fixture *f, &f->error); g_assert_no_error (f->error); - parameters = g_variant_new ("(ssa{sv}a{sv})", + parameters = g_variant_new ("(sssa{sv}a{sv})", "com.example.NotFlatpak", "sample-app", + "", NULL, /* no metadata */ NULL); /* no named arguments */ if (!add_container_server (f, g_steal_pointer (¶meters))) @@ -1211,9 +1225,10 @@ test_unsupported_parameter (Fixture *f, "ThisArgumentIsntImplemented", "b", FALSE); - parameters = g_variant_new ("(ssa{sv}@a{sv})", + parameters = g_variant_new ("(sssa{sv}@a{sv})", "com.example.NotFlatpak", "sample-app", + "", NULL, /* no metadata */ g_variant_dict_end (&named_argument_builder)); tuple = g_dbus_proxy_call_sync (f->proxy, "AddServer", @@ -1251,9 +1266,10 @@ test_invalid_type_name (Fixture *f, NULL, &f->error); g_assert_no_error (f->error); - parameters = g_variant_new ("(ssa{sv}a{sv})", + parameters = g_variant_new ("(sssa{sv}a{sv})", "this is not a valid container type name", "sample-app", + "", NULL, /* no metadata */ NULL); /* no named arguments */ tuple = g_dbus_proxy_call_sync (f->proxy, "AddServer", @@ -1286,9 +1302,10 @@ test_invalid_nesting (Fixture *f, if (f->skip) return; - parameters = g_variant_new ("(ssa{sv}a{sv})", + parameters = g_variant_new ("(sssa{sv}a{sv})", "com.example.NotFlatpak", "sample-app", + "instance0", NULL, /* no metadata */ NULL); /* no named arguments */ if (!add_container_server (f, g_steal_pointer (¶meters))) @@ -1310,9 +1327,10 @@ test_invalid_nesting (Fixture *f, &f->error); g_assert_no_error (f->error); - parameters = g_variant_new ("(ssa{sv}a{sv})", + parameters = g_variant_new ("(sssa{sv}a{sv})", "com.example.NotFlatpak", "inner-app", + "instance1", NULL, /* no metadata */ NULL); /* no named arguments */ tuple = g_dbus_proxy_call_sync (nested_proxy, "AddServer", @@ -1359,9 +1377,10 @@ test_max_containers (Fixture *f, NULL, &f->error); g_assert_no_error (f->error); - parameters = g_variant_new ("(ssa{sv}a{sv})", + parameters = g_variant_new ("(sssa{sv}a{sv})", "com.example.NotFlatpak", "sample-app", + "instance1", NULL, /* no metadata */ NULL); /* no named arguments */ /* We will reuse this variant several times, so don't use floating refs */ @@ -1469,9 +1488,10 @@ test_max_connections_per_container (Fixture *f, NULL, &f->error); g_assert_no_error (f->error); - parameters = g_variant_new ("(ssa{sv}a{sv})", + parameters = g_variant_new ("(sssa{sv}a{sv})", "com.example.NotFlatpak", "sample-app", + "", NULL, /* no metadata */ NULL); /* no named arguments */ /* We will reuse this variant several times, so don't use floating refs */ @@ -1619,9 +1639,10 @@ test_max_container_metadata_bytes (Fixture *f, 1)); /* Floating reference, call_..._sync takes ownership */ - parameters = g_variant_new ("(ss@a{sv}a{sv})", + parameters = g_variant_new ("(sss@a{sv}a{sv})", "com.wasteheadquarters", "Packt Like Sardines in a Crushd Tin Box", + "", g_variant_dict_end (&dict), NULL); /* no named arguments */ From 70526a33810b0ffcd2b3fa09a379ef8af5aca6fd Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Fri, 27 Oct 2023 19:51:41 +0100 Subject: [PATCH 11/23] Containers: Reserve all metadata fields for future standardization Now that we have the instance ID, Flatpak-aware apps can look up per-instance metadata in a Flatpak-specific way (by reading the file that Flatpak provides), and similarly any other container framework can provide its own mechanism to get extensible metadata; so the value of providing container-manager-defined metadata is perhaps limited. However, it seems valuable to have somewhere to put standardized metadata: for example, we could have a shared specification between Wayland and D-Bus to define a name for keys that could be common to multiple sandbox frameworks. For example, it could include a string that is a freedesktop.org app ID, or a string that is an icon name, or a boolean that is true if networking is permitted. This takes dbus/dbus#479 off the critical path for getting this feature merged. Helps: https://gitlab.freedesktop.org/dbus/dbus/-/issues/479 Signed-off-by: Simon McVittie --- doc/dbus-specification.xml | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/doc/dbus-specification.xml b/doc/dbus-specification.xml index d3da33d1..b4380dfb 100644 --- a/doc/dbus-specification.xml +++ b/doc/dbus-specification.xml @@ -7544,9 +7544,10 @@ 3 DICT<STRING,VARIANT> - Metadata describing the application or container, with the - keys and values defined by the maintainers of the container - type. + Metadata describing the application or container. + All keys and values are reserved for future standardization, + either in this specification or in a separate + freedesktop.org specification referenced by this one. @@ -7915,9 +7916,10 @@ 6 DICT<STRING,VARIANT> - Metadata describing the application or container, with the - keys and values defined by the maintainers of the container - type. + Metadata describing the application or container. + All keys and values are reserved for future standardization, + either in this specification or in a separate + freedesktop.org specification referenced by this one. This output parameter is controlled by the creator of the per-container server. From 76380efdbd44df6b17119323ec7727e04736d500 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Thu, 30 Nov 2023 16:56:09 +0000 Subject: [PATCH 12/23] containers: Convert out parameters into an a{sv} This allows for potential future mechanisms where the caller, rather than the message bus, is responsible for creating the socket, without needing to have a "null-like" representation for the absence of a path and the absence of an address (in practice the empty string). I've left the per-container server object path as a top-level thing rather than moving it into the a{sv}, because I don't see any reason why we would want to crate a per-container server without having a way to talk about it in future API calls. Requested-by: Sebastian Wick Signed-off-by: Simon McVittie --- bus/containers.c | 31 +++++++-------- bus/driver.c | 2 +- doc/dbus-specification.xml | 81 ++++++++++++++++++++++++++++---------- test/containers.c | 43 ++++++++++++++------ 4 files changed, 107 insertions(+), 50 deletions(-) diff --git a/bus/containers.c b/bus/containers.c index a65f3daf..24fe3de9 100644 --- a/bus/containers.c +++ b/bus/containers.c @@ -35,6 +35,7 @@ #include +#include "dbus/dbus-asv-util.h" #include "dbus/dbus-hash.h" #include "dbus/dbus-message-internal.h" #include "dbus/dbus-sysdeps-unix.h" @@ -680,8 +681,8 @@ bus_containers_handle_add_server (DBusConnection *connection, BusContainerCreatorData *creator_data; DBusMessageIter iter; DBusMessageIter dict_iter; - DBusMessageIter writer; - DBusMessageIter array_writer; + DBusMessageIter writer = DBUS_MESSAGE_ITER_INIT_CLOSED; + DBusMessageIter asv_writer = DBUS_MESSAGE_ITER_INIT_CLOSED; const char *type; const char *app_id; const char *instance_id; @@ -952,27 +953,21 @@ bus_containers_handle_add_server (DBusConnection *connection, dbus_message_iter_init_append (reply, &writer); - if (!dbus_message_iter_open_container (&writer, DBUS_TYPE_ARRAY, - DBUS_TYPE_BYTE_AS_STRING, - &array_writer)) + if (!dbus_message_iter_open_container (&writer, DBUS_TYPE_ARRAY, "{sv}", + &asv_writer)) goto oom; - if (!dbus_message_iter_append_fixed_array (&array_writer, DBUS_TYPE_BYTE, - &path, strlen (path) + 1)) - { - dbus_message_iter_abandon_container (&writer, &array_writer); - goto oom; - } - - if (!dbus_message_iter_close_container (&writer, &array_writer)) + if (!_dbus_asv_add_string (&asv_writer, "Address", address)) goto oom; - if (!dbus_message_append_args (reply, - DBUS_TYPE_STRING, &address, - DBUS_TYPE_INVALID)) + if (!_dbus_asv_add_byte_array (&asv_writer, "SocketPath", + path, strlen (path) + 1)) goto oom; - _dbus_assert (dbus_message_has_signature (reply, "oays")); + if (!dbus_message_iter_close_container (&writer, &asv_writer)) + goto oom; + + _dbus_assert (dbus_message_has_signature (reply, "oa{sv}")); if (! bus_transaction_send_from_driver (transaction, connection, reply)) goto oom; @@ -988,6 +983,8 @@ oom: BUS_SET_OOM (error); /* fall through */ fail: + dbus_message_iter_abandon_container_if_open (&writer, &asv_writer); + if (server != NULL) bus_container_server_stop_listening (server); diff --git a/bus/driver.c b/bus/driver.c index 0ef07e92..f0063247 100644 --- a/bus/driver.c +++ b/bus/driver.c @@ -2647,7 +2647,7 @@ static const MessageHandler introspectable_message_handlers[] = { #ifdef DBUS_ENABLE_CONTAINERS static const MessageHandler containers_message_handlers[] = { - { "AddServer", "sssa{sv}a{sv}", "oays", bus_containers_handle_add_server, + { "AddServer", "sssa{sv}a{sv}", "oa{sv}", bus_containers_handle_add_server, METHOD_FLAG_NO_CONTAINERS }, { "StopServer", "o", "", bus_containers_handle_stop_server, METHOD_FLAG_NO_CONTAINERS }, diff --git a/doc/dbus-specification.xml b/doc/dbus-specification.xml index b4380dfb..a7663f5e 100644 --- a/doc/dbus-specification.xml +++ b/doc/dbus-specification.xml @@ -7489,8 +7489,7 @@ in DICT<STRING,VARIANT> metadata, in DICT<STRING,VARIANT> named_arguments, out OBJECT_PATH server_path, - out ARRAY<BYTE> socket_path, - out STRING connectable_address) + out DICT<STRING,VARIANT> results) Message arguments: @@ -7571,21 +7570,10 @@ 6 - ARRAY<BYTE> + DICT<STRING,VARIANT> - The absolute filesystem path of the resulting - AF_UNIX socket, followed by a single - zero (nul) byte, for example - /run/user/1000/dbus-12345vwxyz\x00 - where \x00 represents the terminating zero byte. - - - - 7 - STRING - - A connectable D-Bus address, for example - unix:path=/run/user/1000/dbus-12345vwxyz. + Details of the container-specific server that was + created (see below). @@ -7642,14 +7630,65 @@ extension point. + + Keys in the returned result dictionary not containing "." are defined + by this specification. Bus daemon implementors wishing to emit + information not mentioned in this document should use keys + containing "." and starting with a reversed domain name. + Additional keys may be added to the result at any time, and + callers must accept and ignore unknown keys in this dictionary. + + + + The defined keys for the result dictionary are: + + + + + Key + Value type + Value + + + + + Address + STRING + + A connectable D-Bus address, for example + unix:path=/run/user/1000/dbus-12345vwxyz. + + + + SocketPath + ARRAY of BYTE + + The absolute filesystem path of the resulting + AF_UNIX socket, followed by a single + zero (nul) byte, for example + /run/user/1000/dbus-12345vwxyz\x00 + where \x00 represents the terminating zero byte. + + + + + + + In the most basic form of a call to this method, the new server socket is an AF_UNIX socket at a path chosen - by the message bus and returned from the method. Future versions - of this specification are likely to provide a way for the - message bus implementation to receive a socket from the caller - and arrange for it to listen for connections, using the named - arguments dictionary. + by the message bus and returned from the method. + When operating in this way, each successful call will return a + result dictionary containing both Address and + SocketPath. + Callers may use whichever of those members is more convenient. + If a future version of this specification adds a mechanism to + request that the server listens in some way that makes these two + members inapplicable, for example receiving a socket from the + caller in the named arguments dictionary via file descriptor passing + and arranging for it to listen for connections, then those members + will be omitted from the result dictionary. diff --git a/test/containers.c b/test/containers.c index e613e0f4..d7df72f7 100644 --- a/test/containers.c +++ b/test/containers.c @@ -309,7 +309,9 @@ add_container_server (Fixture *f, GVariant *parameters) { GVariant *tuple; + GVariant *named_results; GStatBuf stat_buf; + gboolean found; f->proxy = g_dbus_proxy_new_sync (f->unconfined_conn, G_DBUS_PROXY_FLAGS_DO_NOT_LOAD_PROPERTIES, @@ -339,22 +341,30 @@ add_container_server (Fixture *f, g_assert_no_error (f->error); g_assert_nonnull (tuple); - g_assert_cmpstr (g_variant_get_type_string (tuple), ==, "(oays)"); - g_variant_get (tuple, "(o^ays)", &f->server_path, &f->socket_path, - &f->socket_dbus_address); + g_assert_cmpstr (g_variant_get_type_string (tuple), ==, "(oa{sv})"); + g_variant_get (tuple, "(o@a{sv})", &f->server_path, &named_results); + g_assert_nonnull (f->server_path); + g_assert_true (g_variant_is_object_path (f->server_path)); + + found = g_variant_lookup (named_results, "Address", + "s", &f->socket_dbus_address); + g_assert_true (found); + g_assert_nonnull (f->socket_dbus_address); g_assert_true (g_str_has_prefix (f->socket_dbus_address, "unix:")); g_assert_null (strchr (f->socket_dbus_address, ';')); g_assert_null (strchr (f->socket_dbus_address + strlen ("unix:"), ':')); - g_clear_pointer (&tuple, g_variant_unref); - g_assert_nonnull (f->server_path); - g_assert_true (g_variant_is_object_path (f->server_path)); + found = g_variant_lookup (named_results, "SocketPath", + "^ay", &f->socket_path); + g_assert_true (found); g_assert_nonnull (f->socket_path); g_assert_true (g_path_is_absolute (f->socket_path)); - g_assert_nonnull (f->socket_dbus_address); g_assert_cmpstr (g_stat (f->socket_path, &stat_buf) == 0 ? NULL : g_strerror (errno), ==, NULL); g_assert_cmpuint ((stat_buf.st_mode & S_IFMT), ==, S_IFSOCK); + + g_clear_pointer (&named_results, g_variant_unref); + g_clear_pointer (&tuple, g_variant_unref); return TRUE; } #endif @@ -1394,7 +1404,7 @@ test_max_containers (Fixture *f, NULL, &f->error); g_assert_no_error (f->error); g_assert_nonnull (tuple); - g_variant_get (tuple, "(o^ays)", &placeholders[i], NULL, NULL); + g_variant_get (tuple, "(o@a{sv})", &placeholders[i], NULL); g_variant_unref (tuple); g_test_message ("Placeholder server at %s", placeholders[i]); } @@ -1475,7 +1485,6 @@ test_max_connections_per_container (Fixture *f, * limit-containers.conf */ GSocket *placeholders[G_N_ELEMENTS (socket_paths) * 3] = { NULL }; GVariant *parameters; - GVariant *tuple; guint i; if (f->skip) @@ -1499,13 +1508,25 @@ test_max_connections_per_container (Fixture *f, for (i = 0; i < G_N_ELEMENTS (socket_paths); i++) { + GVariant *tuple; + GVariant *named_results; + gboolean found; + tuple = g_dbus_proxy_call_sync (f->proxy, "AddServer", parameters, G_DBUS_CALL_FLAGS_NONE, -1, NULL, &f->error); g_assert_no_error (f->error); g_assert_nonnull (tuple); - g_variant_get (tuple, "(o^ays)", NULL, &socket_paths[i], - &dbus_addresses[i]); + g_variant_get (tuple, "(o@a{sv})", NULL, &named_results); + + found = g_variant_lookup (named_results, "Address", + "s", &dbus_addresses[i]); + g_assert_true (found); + found = g_variant_lookup (named_results, "SocketPath", + "^ay", &socket_paths[i]); + g_assert_true (found); + + g_variant_unref (named_results); g_variant_unref (tuple); socket_addresses[i] = g_unix_socket_address_new (socket_paths[i]); g_test_message ("Server #%u at %s", i, socket_paths[i]); From 256b9dbe1e26b8fc074cc1e4a18af29a517443a6 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Thu, 30 Nov 2023 14:56:32 +0000 Subject: [PATCH 13/23] spawn-unix: Move _dbus_unix_make_pipe() to common code I want to use this in a test-case. Note that this changes the error code used if pipe() fails (which in practice should not happen) from DBUS_ERROR_SPAWN_FAILED to some reasonable interpretation of errno. Signed-off-by: Simon McVittie --- dbus/dbus-spawn-unix.c | 44 +--------------------------------------- dbus/dbus-sysdeps-unix.c | 42 ++++++++++++++++++++++++++++++++++++++ dbus/dbus-sysdeps-unix.h | 4 ++++ 3 files changed, 47 insertions(+), 43 deletions(-) diff --git a/dbus/dbus-spawn-unix.c b/dbus/dbus-spawn-unix.c index 0bf476bd..1966a41d 100644 --- a/dbus/dbus-spawn-unix.c +++ b/dbus/dbus-spawn-unix.c @@ -922,48 +922,6 @@ close_and_invalidate (int *fd) return ret; } -static dbus_bool_t -make_pipe (int p[2], - DBusError *error) -{ - int retval; - -#ifdef HAVE_PIPE2 - dbus_bool_t cloexec_done; - - retval = pipe2 (p, O_CLOEXEC); - cloexec_done = retval >= 0; - - /* Check if kernel seems to be too old to know pipe2(). We assume - that if pipe2 is available, O_CLOEXEC is too. */ - if (retval < 0 && errno == ENOSYS) -#endif - { - retval = pipe(p); - } - - _DBUS_ASSERT_ERROR_IS_CLEAR (error); - - if (retval < 0) - { - dbus_set_error (error, - DBUS_ERROR_SPAWN_FAILED, - "Failed to create pipe for communicating with child process (%s)", - _dbus_strerror (errno)); - return FALSE; - } - -#ifdef HAVE_PIPE2 - if (!cloexec_done) -#endif - { - _dbus_fd_set_close_on_exec (p[0]); - _dbus_fd_set_close_on_exec (p[1]); - } - - return TRUE; -} - static void do_write (int fd, const void *buf, size_t count) { @@ -1317,7 +1275,7 @@ _dbus_spawn_async_with_babysitter (DBusBabysitter **sitter_p, goto cleanup_and_fail; } - if (!make_pipe (child_err_report_pipe, error)) + if (!_dbus_unix_make_pipe (child_err_report_pipe, error)) goto cleanup_and_fail; if (!_dbus_socketpair (&babysitter_pipe[0], &babysitter_pipe[1], TRUE, error)) diff --git a/dbus/dbus-sysdeps-unix.c b/dbus/dbus-sysdeps-unix.c index 59d4afcf..af346a4b 100644 --- a/dbus/dbus-sysdeps-unix.c +++ b/dbus/dbus-sysdeps-unix.c @@ -5273,4 +5273,46 @@ _dbus_get_low_level_socket_errno (void) return errno; } +dbus_bool_t +_dbus_unix_make_pipe (int p[2], + DBusError *error) +{ + int retval; + +#ifdef HAVE_PIPE2 + dbus_bool_t cloexec_done; + + retval = pipe2 (p, O_CLOEXEC); + cloexec_done = retval >= 0; + + /* Check if kernel seems to be too old to know pipe2(). We assume + that if pipe2 is available, O_CLOEXEC is too. */ + if (retval < 0 && errno == ENOSYS) +#endif + { + retval = pipe(p); + } + + _DBUS_ASSERT_ERROR_IS_CLEAR (error); + + if (retval < 0) + { + dbus_set_error (error, + _dbus_error_from_errno (errno), + "Failed to create pipe (%s)", + _dbus_strerror (errno)); + return FALSE; + } + +#ifdef HAVE_PIPE2 + if (!cloexec_done) +#endif + { + _dbus_fd_set_close_on_exec (p[0]); + _dbus_fd_set_close_on_exec (p[1]); + } + + return TRUE; +} + /* tests in dbus-sysdeps-util.c */ diff --git a/dbus/dbus-sysdeps-unix.h b/dbus/dbus-sysdeps-unix.h index b400cf86..8c4addcc 100644 --- a/dbus/dbus-sysdeps-unix.h +++ b/dbus/dbus-sysdeps-unix.h @@ -162,6 +162,10 @@ void _dbus_set_signal_handler (int sig, dbus_bool_t _dbus_reset_oom_score_adj (const char **error_str_p); +DBUS_PRIVATE_EXPORT +dbus_bool_t _dbus_unix_make_pipe (int p[2], + DBusError *error); + /** @} */ DBUS_END_DECLS From 52317e15fa7111862b278f5ecb01c5fea2c0f4e8 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Thu, 30 Nov 2023 18:30:21 +0000 Subject: [PATCH 14/23] containers test: Show the AddServer call and response in the log Signed-off-by: Simon McVittie --- test/containers.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/test/containers.c b/test/containers.c index d7df72f7..a47a2af2 100644 --- a/test/containers.c +++ b/test/containers.c @@ -312,6 +312,7 @@ add_container_server (Fixture *f, GVariant *named_results; GStatBuf stat_buf; gboolean found; + gchar *stringified_args; f->proxy = g_dbus_proxy_new_sync (f->unconfined_conn, G_DBUS_PROXY_FLAGS_DO_NOT_LOAD_PROPERTIES, @@ -320,7 +321,10 @@ add_container_server (Fixture *f, NULL, &f->error); g_assert_no_error (f->error); - g_test_message ("Calling AddServer..."); + stringified_args = g_variant_print (parameters, TRUE); + g_test_message ("Calling AddServer%s...", stringified_args); + g_free (stringified_args); + tuple = g_dbus_proxy_call_sync (f->proxy, "AddServer", parameters, G_DBUS_CALL_FLAGS_NONE, -1, NULL, &f->error); @@ -341,6 +345,10 @@ add_container_server (Fixture *f, g_assert_no_error (f->error); g_assert_nonnull (tuple); + stringified_args = g_variant_print (tuple, TRUE); + g_test_message ("-> %s", stringified_args); + g_free (stringified_args); + g_assert_cmpstr (g_variant_get_type_string (tuple), ==, "(oa{sv})"); g_variant_get (tuple, "(o@a{sv})", &f->server_path, &named_results); g_assert_nonnull (f->server_path); From ed65ef6708e35fee828b2e3c31e11a72d49861a5 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Thu, 30 Nov 2023 18:34:05 +0000 Subject: [PATCH 15/23] containers test: Add infrastructure for sending fds to AddServer Signed-off-by: Simon McVittie --- test/containers.c | 48 +++++++++++++++++++++++++++++++++++++---------- 1 file changed, 38 insertions(+), 10 deletions(-) diff --git a/test/containers.c b/test/containers.c index a47a2af2..e203e921 100644 --- a/test/containers.c +++ b/test/containers.c @@ -306,7 +306,9 @@ test_get_supported_arguments (Fixture *f, */ static gboolean add_container_server (Fixture *f, - GVariant *parameters) + GVariant *parameters, + GUnixFDList *fds, + GUnixFDList **fds_out) { GVariant *tuple; GVariant *named_results; @@ -325,8 +327,29 @@ add_container_server (Fixture *f, g_test_message ("Calling AddServer%s...", stringified_args); g_free (stringified_args); - tuple = g_dbus_proxy_call_sync (f->proxy, "AddServer", parameters, - G_DBUS_CALL_FLAGS_NONE, -1, NULL, &f->error); + if (fds != NULL || fds_out != NULL) + { + if (fds != NULL) + g_test_message ("Sending %d fds", g_unix_fd_list_get_length (fds)); + + if (fds_out != NULL) + g_test_message ("Expecting to receive fds in result"); + + tuple = g_dbus_proxy_call_with_unix_fd_list_sync (f->proxy, + "AddServer", + parameters, + G_DBUS_CALL_FLAGS_NONE, + -1, + fds, + fds_out, + NULL, + &f->error); + } + else + { + tuple = g_dbus_proxy_call_sync (f->proxy, "AddServer", parameters, + G_DBUS_CALL_FLAGS_NONE, -1, NULL, &f->error); + } /* For root, the sockets go in /run/dbus/containers, which we rely on * system infrastructure to create; so it's OK for AddServer to fail @@ -416,7 +439,7 @@ test_basic (Fixture *f, "sample-instance", NULL, /* no metadata */ NULL); /* no named arguments */ - if (!add_container_server (f, g_steal_pointer (¶meters))) + if (!add_container_server (f, g_steal_pointer (¶meters), NULL, NULL)) return; g_test_message ("Connecting to %s...", f->socket_dbus_address); @@ -611,7 +634,7 @@ test_wrong_uid (Fixture *f, "instance1", NULL, /* no metadata */ NULL); /* no named arguments */ - if (!add_container_server (f, g_steal_pointer (¶meters))) + if (!add_container_server (f, g_steal_pointer (¶meters), NULL, NULL)) return; g_test_message ("Connecting to %s...", f->socket_dbus_address); @@ -676,7 +699,7 @@ test_metadata (Fixture *f, "", g_variant_dict_end (&dict), NULL); /* no named arguments */ - if (!add_container_server (f, g_steal_pointer (¶meters))) + if (!add_container_server (f, g_steal_pointer (¶meters), NULL, NULL)) return; g_test_message ("Connecting to %s...", f->socket_dbus_address); @@ -798,8 +821,10 @@ test_stop_server (Fixture *f, GDBusProxy *attacker_proxy; GSocket *client_socket; GSocketAddress *socket_address; + GUnixFDList *fds = NULL; GVariant *tuple; GVariant *parameters; + GVariantDict named_argument_builder; gchar *error_name; const gchar *confined_unique_name; const gchar *name_owner; @@ -820,13 +845,16 @@ test_stop_server (Fixture *f, &f->error); g_assert_no_error (f->error); - parameters = g_variant_new ("(sssa{sv}a{sv})", + g_variant_dict_init (&named_argument_builder, NULL); + fds = g_unix_fd_list_new (); + + parameters = g_variant_new ("(sssa{sv}@a{sv})", "com.example.NotFlatpak", "sample-app", "", NULL, /* no metadata */ - NULL); /* no named arguments */ - if (!add_container_server (f, g_steal_pointer (¶meters))) + g_variant_dict_end (&named_argument_builder)); + if (!add_container_server (f, g_steal_pointer (¶meters), fds, NULL)) return; socket_address = g_unix_socket_address_new (f->socket_path); @@ -1326,7 +1354,7 @@ test_invalid_nesting (Fixture *f, "instance0", NULL, /* no metadata */ NULL); /* no named arguments */ - if (!add_container_server (f, g_steal_pointer (¶meters))) + if (!add_container_server (f, g_steal_pointer (¶meters), NULL, NULL)) return; g_test_message ("Connecting to %s...", f->socket_dbus_address); From 9a8d7fb90f45b3466a2c32814afd57ee513ad469 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Thu, 30 Nov 2023 18:35:09 +0000 Subject: [PATCH 16/23] containers test: Factor out waiting for the unconfined connection to close We'll want to do this from more than one place in future. Signed-off-by: Simon McVittie --- test/containers.c | 52 ++++++++++++++++++++++++++++++----------------- 1 file changed, 33 insertions(+), 19 deletions(-) diff --git a/test/containers.c b/test/containers.c index e203e921..6b90ac51 100644 --- a/test/containers.c +++ b/test/containers.c @@ -175,6 +175,33 @@ fixture_disconnect_unconfined (Fixture *f) g_clear_object (&f->unconfined_conn); } +#ifdef DBUS_ENABLE_CONTAINERS +static void +fixture_disconnect_unconfined_and_wait (Fixture *f) +{ + guint name_watch; + gboolean gone = FALSE; + + /* Close the unconfined connection (the container manager) and wait + * for it to go away */ + g_test_message ("Closing container manager..."); + name_watch = g_bus_watch_name_on_connection (f->confined_conn, + f->unconfined_unique_name, + G_BUS_NAME_WATCHER_FLAGS_NONE, + NULL, + name_gone_set_boolean_cb, + &gone, NULL); + fixture_disconnect_unconfined (f); + + g_test_message ("Waiting for container manager bus name to disappear..."); + + while (!gone) + g_main_context_iteration (NULL, TRUE); + + g_bus_unwatch_name (name_watch); +} +#endif + static void fixture_disconnect_observer (Fixture *f) { @@ -828,8 +855,6 @@ test_stop_server (Fixture *f, gchar *error_name; const gchar *confined_unique_name; const gchar *name_owner; - gboolean gone = FALSE; - guint name_watch; guint i; g_assert_nonnull (config); @@ -871,6 +896,9 @@ test_stop_server (Fixture *f, if (config->stop_server == STOP_SERVER_DISCONNECT_FIRST) { + guint name_watch; + gboolean gone = FALSE; + g_test_message ("Disconnecting confined connection..."); gone = FALSE; confined_unique_name = g_dbus_connection_get_unique_name ( @@ -944,23 +972,9 @@ test_stop_server (Fixture *f, switch (config->stop_server) { case STOP_SERVER_WITH_MANAGER: - /* Close the unconfined connection (the container manager) and wait - * for it to go away */ - g_test_message ("Closing container manager..."); - name_watch = g_bus_watch_name_on_connection (f->confined_conn, - f->unconfined_unique_name, - G_BUS_NAME_WATCHER_FLAGS_NONE, - NULL, - name_gone_set_boolean_cb, - &gone, NULL); - fixture_disconnect_unconfined (f); - - g_test_message ("Waiting for container manager bus name to disappear..."); - - while (!gone) - g_main_context_iteration (NULL, TRUE); - - g_bus_unwatch_name (name_watch); + g_test_message ("Stopping server (but not confined connection) by " + "disconnecting container manager from bus"); + fixture_disconnect_unconfined_and_wait (f); break; case STOP_SERVER_EXPLICITLY: From 5db4c852fac5c888e17c8dfdf0379d79f3842c05 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Thu, 30 Nov 2023 18:38:11 +0000 Subject: [PATCH 17/23] containers: Add named arguments to alter how the server is stopped This adds the initial named arguments that we anticipate we will need for Flatpak and Snap: Flatpak will want to hold the server open until the xdg-dbus-proxy exits, while Snap developers want to clean up the per-container servers explicitly and have their existence stored as part of the persistent state of the restartable snapd service. Resolves: https://gitlab.freedesktop.org/dbus/dbus/-/issues/186 Signed-off-by: Simon McVittie --- bus/containers.c | 184 ++++++++++++++++++++++++++++++++++--- doc/dbus-specification.xml | 67 +++++++++++--- test/containers.c | 8 +- 3 files changed, 231 insertions(+), 28 deletions(-) diff --git a/bus/containers.c b/bus/containers.c index 24fe3de9..9f8c638f 100644 --- a/bus/containers.c +++ b/bus/containers.c @@ -34,11 +34,13 @@ #endif #include +#include #include "dbus/dbus-asv-util.h" #include "dbus/dbus-hash.h" #include "dbus/dbus-message-internal.h" #include "dbus/dbus-sysdeps-unix.h" +#include "dbus/dbus-watch.h" #include "connection.h" #include "dispatch.h" @@ -64,8 +66,11 @@ typedef struct /* List of owned DBusConnection, removed when the DBusConnection is * removed from the bus */ DBusList *connections; + DBusWatch *stop_on_notify_watch; unsigned long uid; + int stop_on_notify_fd; unsigned announced:1; + unsigned stop_on_disconnect:1; } BusContainerServer; /* Data attached to a DBusConnection that has created container servers. */ @@ -273,6 +278,29 @@ oom: return FALSE; } +static void +bus_container_server_remove_notify (BusContainerServer *self) +{ + DBusWatch *watch; + + /* Transfer the reference to this function */ + watch = self->stop_on_notify_watch; + self->stop_on_notify_watch = NULL; + + if (watch != NULL) + { + _dbus_loop_remove_watch (bus_context_get_loop (self->context), watch); + _dbus_watch_invalidate (watch); + _dbus_watch_unref (watch); + } + + if (self->stop_on_notify_fd >= 0) + { + close (self->stop_on_notify_fd); + self->stop_on_notify_fd = -1; + } +} + static void bus_container_server_unref (BusContainerServer *self) { @@ -297,6 +325,8 @@ bus_container_server_unref (BusContainerServer *self) self->announced = FALSE; } + bus_container_server_remove_notify (self); + /* As long as the server is listening, the BusContainerServer can't * be freed, because the DBusServer holds a reference to the * BusContainerServer */ @@ -360,6 +390,8 @@ bus_container_server_stop_listening (BusContainerServer *self) /* In case the DBusServer holds the last reference to self */ bus_container_server_ref (self); + bus_container_server_remove_notify (self); + if (self->server != NULL) { dbus_server_set_new_connection_function (self->server, NULL, NULL, NULL); @@ -407,6 +439,8 @@ bus_container_server_new (BusContext *context, self->containers = bus_containers_ref (containers); self->server = NULL; self->creator = dbus_connection_ref (creator); + self->stop_on_notify_fd = -1; + self->stop_on_disconnect = TRUE; if (containers->next_container_id >= DBUS_UINT64_CONSTANT (0xFFFFFFFFFFFFFFFF)) @@ -672,6 +706,38 @@ bus_container_server_listen (BusContainerServer *self, return TRUE; } +static dbus_bool_t +check_named_parameter_type (DBusMessageIter *variant_iter, + const char *param_name, + int expected_type, + DBusError *error) +{ + if (dbus_message_iter_get_arg_type (variant_iter) != expected_type) + { + dbus_set_error (error, DBUS_ERROR_INVALID_ARGS, + "Named parameter %s should have type '%c', not '%c'", + param_name, expected_type, + dbus_message_iter_get_arg_type (variant_iter)); + return FALSE; + } + + return TRUE; +} + +static dbus_bool_t +bus_containers_handle_stop_notify (DBusWatch *watch, + unsigned int flags, + void *data) +{ + BusContainerServer *server = data; + + _dbus_assert (watch == server->stop_on_notify_watch); + _dbus_verbose ("Stopping %s because notify fd became readable", + server->path); + bus_container_server_stop_listening (server); + return TRUE; +} + dbus_bool_t bus_containers_handle_add_server (DBusConnection *connection, BusTransaction *transaction, @@ -825,9 +891,12 @@ bus_containers_handle_add_server (DBusConnection *connection, _dbus_assert (dbus_message_iter_get_arg_type (&iter) == DBUS_TYPE_ARRAY); dbus_message_iter_recurse (&iter, &dict_iter); - while (dbus_message_iter_get_arg_type (&dict_iter) != DBUS_TYPE_INVALID) + for (; + dbus_message_iter_get_arg_type (&dict_iter) != DBUS_TYPE_INVALID; + dbus_message_iter_next (&dict_iter)) { DBusMessageIter pair_iter; + DBusMessageIter variant_iter; const char *param_name; _dbus_assert (dbus_message_iter_get_arg_type (&dict_iter) == @@ -838,11 +907,64 @@ bus_containers_handle_add_server (DBusConnection *connection, DBUS_TYPE_STRING); dbus_message_iter_get_basic (&pair_iter, ¶m_name); - /* If we supported any named parameters, we'd copy them into the data - * structure here; but we don't, so fail instead. */ - dbus_set_error (error, DBUS_ERROR_INVALID_ARGS, - "Named parameter %s is not understood", param_name); - goto fail; + if (!dbus_message_iter_next (&pair_iter)) + _dbus_assert_not_reached ("dict entry with less than 2 items"); + + _dbus_assert (dbus_message_iter_get_arg_type (&pair_iter) == + DBUS_TYPE_VARIANT); + dbus_message_iter_recurse (&pair_iter, &variant_iter); + + if (strcmp (param_name, "StopOnDisconnect") == 0) + { + dbus_bool_t value; + + if (!check_named_parameter_type (&variant_iter, param_name, + DBUS_TYPE_BOOLEAN, error)) + goto fail; + + dbus_message_iter_get_basic (&variant_iter, &value); + server->stop_on_disconnect = !!value; + } + else if (strcmp (param_name, "StopOnNotify") == 0) + { + if (!check_named_parameter_type (&variant_iter, param_name, + DBUS_TYPE_UNIX_FD, error)) + goto fail; + + dbus_message_iter_get_basic (&variant_iter, + &server->stop_on_notify_fd); + + if (server->stop_on_notify_fd < 0) + { + dbus_set_error (error, DBUS_ERROR_INVALID_ARGS, + "Unable to retrieve StopOnNotify fd"); + goto fail; + } + + server->stop_on_notify_watch = _dbus_watch_new (server->stop_on_notify_fd, + DBUS_WATCH_READABLE, + TRUE, /* enabled */ + bus_containers_handle_stop_notify, + server, + NULL); + + if (server->stop_on_notify_watch == NULL || + !_dbus_loop_add_watch (bus_context_get_loop (context), + server->stop_on_notify_watch)) + { + BUS_SET_OOM (error); + goto fail; + } + } + else + { + dbus_set_error (error, DBUS_ERROR_INVALID_ARGS, + "Named parameter %s is not understood", param_name); + goto fail; + } + + if (dbus_message_iter_next (&pair_iter)) + _dbus_assert_not_reached ("dict entry with more than 2 items"); } /* End of arguments */ @@ -999,13 +1121,32 @@ dbus_bool_t bus_containers_supported_arguments_getter (BusContext *server, DBusMessageIter *var_iter) { + /* Please keep these sorted in strcmp (LC_ALL=C sort) order */ + static const char * const supported[] = + { + "StopOnDisconnect", + "StopOnNotify", + }; DBusMessageIter arr_iter; + size_t i; - /* There are none so far */ - return dbus_message_iter_open_container (var_iter, DBUS_TYPE_ARRAY, - DBUS_TYPE_STRING_AS_STRING, - &arr_iter) && - dbus_message_iter_close_container (var_iter, &arr_iter); + if (!dbus_message_iter_open_container (var_iter, DBUS_TYPE_ARRAY, + DBUS_TYPE_STRING_AS_STRING, + &arr_iter)) + return FALSE; + + for (i = 0; i < _DBUS_N_ELEMENTS (supported); i++) + { + if (!dbus_message_iter_append_basic (&arr_iter, + DBUS_TYPE_STRING, + &supported[i])) + { + dbus_message_iter_abandon_container (var_iter, &arr_iter); + return FALSE; + } + } + + return dbus_message_iter_close_container (var_iter, &arr_iter); } dbus_bool_t @@ -1436,6 +1577,11 @@ bus_containers_remove_connection (BusContainers *self, DBusList *iter; DBusList *next; + _dbus_verbose ("Closing connection %s (%s) which has owned at least " + "one server", + bus_connection_get_name (connection), + bus_connection_get_loginfo (connection)); + for (iter = _dbus_list_get_first_link (&creator_data->servers); iter != NULL; iter = next) @@ -1448,9 +1594,19 @@ bus_containers_remove_connection (BusContainers *self, _dbus_assert (server->creator == connection); - /* This will invalidate iter and server if there are no open - * connections to this server */ - bus_container_server_stop_listening (server); + if (server->stop_on_disconnect) + { + _dbus_verbose ("Stopping %s", server->path); + /* This will invalidate iter and server if there are no open + * connections to this server */ + bus_container_server_stop_listening (server); + } + else + { + _dbus_verbose ("Not stopping %s because it was created with " + "StopOnDisconnect=FALSE", + server->path); + } } } diff --git a/doc/dbus-specification.xml b/doc/dbus-specification.xml index a7663f5e..3099a46c 100644 --- a/doc/dbus-specification.xml +++ b/doc/dbus-specification.xml @@ -7619,7 +7619,7 @@ The last argument (the named arguments) - will be used in future versions of this specification to modify + can be used to modify the behaviour of this method call; all keys in this dictionary not containing . are reserved by this specification for this purpose. It can also contain implementation-specific @@ -7630,6 +7630,60 @@ extension point. + + The only named arguments that are allowed are the ones + listed in the + org.freedesktop.DBus.Containers1.SupportedArguments + property. All other named arguments are an error. + The possible keys currently defined for the named arguments are: + + + + + Key + Value type + Default value + Meaning + + + + + StopOnDisconnect + BOOLEAN + true + + If true or omitted, the server socket will cease to + listen for new connections (in the same way as if + StopListening had been called + successfully) when the caller of this method + disconnects from the bus. Any confined connections + that are already active are unaffected. + If false, the server socket continues to listen after + the caller disconnects, until there is some other reason + for it to stop. + + + + StopOnNotify + UNIX_FD + (none) + + A file descriptor which will become readable (either + data available or end-of-file) when it is time to stop + listening for new connections. In practice, this is + expected to be the read end of a Unix pipe, for which + the corresponding write end will be closed (either + explicitly or when its owning process exits) when the + container manager detects that the application has + stopped running. This will often be combined with + { "StopOnDisconnect": false }. + + + + + + + Keys in the returned result dictionary not containing "." are defined by this specification. Bus daemon implementors wishing to emit @@ -7700,17 +7754,6 @@ any particular filtering applied to them by the message bus. - - In the most basic form of a call to this method, the server - socket will cease to listen for new connections (in the same - way as if StopListening had been called - successfully) when the caller of this method disconnects from - the bus. Any confined connections that are already active are - unaffected. Future versions of this specification are likely - to provide a way for the caller to alter this behaviour by - specifying named arguments. - - The container server object path remains valid for as long as one or more confined connection via the same server diff --git a/test/containers.c b/test/containers.c index 6b90ac51..80f2df2c 100644 --- a/test/containers.c +++ b/test/containers.c @@ -290,6 +290,7 @@ test_get_supported_arguments (Fixture *f, GVariant *v; #ifdef DBUS_ENABLE_CONTAINERS const gchar **args; + gsize i; gsize len; #endif @@ -311,8 +312,11 @@ test_get_supported_arguments (Fixture *f, g_assert_cmpstr (g_variant_get_type_string (v), ==, "as"); args = g_variant_get_strv (v, &len); - /* No arguments are defined yet */ - g_assert_cmpuint (len, ==, 0); + g_assert_cmpuint (len, ==, 2); + i = 0; + g_assert_cmpstr (args[i++], ==, "StopOnDisconnect"); + g_assert_cmpstr (args[i++], ==, "StopOnNotify"); + g_assert_cmpstr (args[i++], ==, NULL); g_free (args); g_variant_unref (v); From 168a8edad0eab8a35ddd3e2ba3354be1ef3c514c Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Thu, 30 Nov 2023 18:39:47 +0000 Subject: [PATCH 18/23] containers test: Exercise StopOnDisconnect and StopOnNotify In the STOP_SERVER_VIA_NOTIFY test, the server would stop whenever either the mock container manager disconnects from the bus or the notify fd is closed, whichever comes first. In the STOP_SERVER_VIA_NOTIFY_NOT_MANAGER test, we assert that the server does not stop when the mock container manager disconnects, only when the notify fd is closed. Signed-off-by: Simon McVittie --- test/containers.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/test/containers.c b/test/containers.c index 80f2df2c..d307a143 100644 --- a/test/containers.c +++ b/test/containers.c @@ -69,6 +69,7 @@ typedef struct { guint removed_sub; DBusConnection *libdbus_observer; DBusMessage *latest_shout; + int stop_on_notify_pipe_write_end; } Fixture; typedef struct @@ -80,6 +81,8 @@ typedef struct STOP_SERVER_DISCONNECT_FIRST, STOP_SERVER_NEVER_CONNECTED, STOP_SERVER_FORCE, + STOP_SERVER_VIA_NOTIFY, + STOP_SERVER_VIA_NOTIFY_NOT_MANAGER, STOP_SERVER_WITH_MANAGER } stop_server; @@ -278,6 +281,8 @@ setup (Fixture *f, if (!dbus_connection_add_filter (f->libdbus_observer, observe_shouting_cb, f, NULL)) g_error ("OOM"); + + f->stop_on_notify_pipe_write_end = -1; } /* @@ -877,6 +882,33 @@ test_stop_server (Fixture *f, g_variant_dict_init (&named_argument_builder, NULL); fds = g_unix_fd_list_new (); + if (config->stop_server == STOP_SERVER_VIA_NOTIFY_NOT_MANAGER) + g_variant_dict_insert (&named_argument_builder, "StopOnDisconnect", + "b", FALSE); + + if (config->stop_server == STOP_SERVER_VIA_NOTIFY || + config->stop_server == STOP_SERVER_VIA_NOTIFY_NOT_MANAGER) + { + DBusError error = DBUS_ERROR_INIT; + int notify_pipe[2]; + int fd_index; + + _dbus_unix_make_pipe (notify_pipe, &error); + test_assert_no_error (&error); + + /* transfer ownership */ + f->stop_on_notify_pipe_write_end = notify_pipe[1]; + + /* duplicate into the fd list and close the original */ + fd_index = g_unix_fd_list_append (fds, notify_pipe[0], &f->error); + g_assert_no_error (f->error); + g_assert_cmpint (fd_index, >=, 0); + close (notify_pipe[0]); + + g_variant_dict_insert (&named_argument_builder, "StopOnNotify", + "h", fd_index); + } + parameters = g_variant_new ("(sssa{sv}@a{sv})", "com.example.NotFlatpak", "sample-app", @@ -1004,6 +1036,22 @@ test_stop_server (Fixture *f, g_clear_error (&f->error); break; + case STOP_SERVER_VIA_NOTIFY_NOT_MANAGER: + fixture_disconnect_unconfined_and_wait (f); + + /* Assert that the server is still functional at this point, + * because we configured it with StopOnDisconnect=FALSE */ + g_assert_true (g_file_test (f->socket_path, G_FILE_TEST_EXISTS)); + /* fall through */ + + case STOP_SERVER_VIA_NOTIFY: + g_test_message ("Stopping server (but not confined connection) by " + "closing pipe fd"); + g_close (f->stop_on_notify_pipe_write_end, &f->error); + g_assert_no_error (f->error); + f->stop_on_notify_pipe_write_end = -1; + break; + case STOP_SERVER_DISCONNECT_FIRST: case STOP_SERVER_NEVER_CONNECTED: g_test_message ("Stopping server (with no confined connections)..."); @@ -1110,6 +1158,8 @@ test_stop_server (Fixture *f, break; case STOP_SERVER_EXPLICITLY: + case STOP_SERVER_VIA_NOTIFY: + case STOP_SERVER_VIA_NOTIFY_NOT_MANAGER: case STOP_SERVER_WITH_MANAGER: g_test_message ("Checking that the confined app still works..."); tuple = g_dbus_connection_call_sync (f->confined_conn, @@ -1774,6 +1824,15 @@ teardown (Fixture *f, f->daemon_pid = 0; } + if (f->stop_on_notify_pipe_write_end >= 0) + { + GError *error = NULL; + + g_close (f->stop_on_notify_pipe_write_end, &error); + g_assert_no_error (error); + f->stop_on_notify_pipe_write_end = -1; + } + dbus_clear_message (&f->latest_shout); g_free (f->server_path); g_free (f->socket_path); @@ -1804,6 +1863,16 @@ static const Config stop_server_force = "valid-config-files/multi-user.conf", STOP_SERVER_FORCE }; +static const Config stop_server_via_notify = +{ + "valid-config-files/multi-user.conf", + STOP_SERVER_VIA_NOTIFY +}; +static const Config stop_server_not_with_manager = +{ + "valid-config-files/multi-user.conf", + STOP_SERVER_VIA_NOTIFY_NOT_MANAGER +}; static const Config stop_server_with_manager = { "valid-config-files/multi-user.conf", @@ -1863,6 +1932,10 @@ main (int argc, &stop_server_never_connected, setup, test_stop_server, teardown); g_test_add ("/containers/stop-server/force", Fixture, &stop_server_force, setup, test_stop_server, teardown); + g_test_add ("/containers/stop-server/via-notify", Fixture, + &stop_server_via_notify, setup, test_stop_server, teardown); + g_test_add ("/containers/stop-server/not-with-manager", Fixture, + &stop_server_not_with_manager, setup, test_stop_server, teardown); g_test_add ("/containers/stop-server/with-manager", Fixture, &stop_server_with_manager, setup, test_stop_server, teardown); g_test_add ("/containers/metadata", Fixture, &limit_containers, From 70930672d4b0b62d17990e6a13ccc1ed3eb634b7 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Thu, 30 Nov 2023 20:10:51 +0000 Subject: [PATCH 19/23] containers test: Show server/connection info in test log Signed-off-by: Simon McVittie --- test/containers.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/test/containers.c b/test/containers.c index d307a143..96ec9239 100644 --- a/test/containers.c +++ b/test/containers.c @@ -465,6 +465,7 @@ test_basic (Fixture *f, DBusMessage *libdbus_message = NULL; DBusMessage *libdbus_reply = NULL; DBusError libdbus_error = DBUS_ERROR_INIT; + gchar *stringified_result; if (f->skip) return; @@ -596,6 +597,9 @@ test_basic (Fixture *f, G_DBUS_CALL_FLAGS_NONE, -1, NULL, &f->error); g_assert_no_error (f->error); g_assert_nonnull (tuple); + stringified_result = g_variant_print (tuple, TRUE); + g_test_message ("GetConnectionInfo() -> %s", stringified_result); + g_free (stringified_result); g_assert_cmpstr (g_variant_get_type_string (tuple), ==, "(oa{sv}sssa{sv})"); g_variant_get (tuple, "(&o@a{sv}&s&s&s@a{sv})", &path_from_query, &creator, &type, &app_id, &instance_id, &asv); @@ -619,6 +623,9 @@ test_basic (Fixture *f, G_DBUS_CALL_FLAGS_NONE, -1, NULL, &f->error); g_assert_no_error (f->error); g_assert_nonnull (tuple); + stringified_result = g_variant_print (tuple, TRUE); + g_test_message ("GetServerInfo() -> %s", stringified_result); + g_free (stringified_result); g_assert_cmpstr (g_variant_get_type_string (tuple), ==, "(a{sv}sssa{sv})"); g_variant_get (tuple, "(@a{sv}&s&s&s@a{sv})", &creator, &type, &app_id, &instance_id, &asv); @@ -862,6 +869,7 @@ test_stop_server (Fixture *f, GVariant *parameters; GVariantDict named_argument_builder; gchar *error_name; + gchar *stringified_result; const gchar *confined_unique_name; const gchar *name_owner; guint i; @@ -1187,6 +1195,9 @@ test_stop_server (Fixture *f, &f->error); g_assert_no_error (f->error); g_assert_nonnull (tuple); + stringified_result = g_variant_print (tuple, TRUE); + g_test_message ("GetServerInfo() -> %s", stringified_result); + g_free (stringified_result); g_clear_pointer (&tuple, g_variant_unref); /* Now disconnect the last confined connection, which will make the From 093f31744401522a832e9fa4685e410e291eddc9 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Thu, 30 Nov 2023 20:13:29 +0000 Subject: [PATCH 20/23] containers test: Assert metadata still works after creator disconnected In some of the tests involving test_stop_server(), the creator of the server (the mock container engine) already disconnected from the bus, so it seems worthwhile to check that its credentials can still be retrieved via GetServerInfo() even though the connection itself is no longer there. Signed-off-by: Simon McVittie --- test/containers.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/test/containers.c b/test/containers.c index 96ec9239..a80db915 100644 --- a/test/containers.c +++ b/test/containers.c @@ -866,13 +866,20 @@ test_stop_server (Fixture *f, GSocketAddress *socket_address; GUnixFDList *fds = NULL; GVariant *tuple; + GVariant *creator; GVariant *parameters; + GVariant *asv; + GVariantDict dict; GVariantDict named_argument_builder; gchar *error_name; gchar *stringified_result; const gchar *confined_unique_name; const gchar *name_owner; + const char *type; + const char *app_id; + const char *instance_id; guint i; + guint32 uid; g_assert_nonnull (config); @@ -1198,6 +1205,19 @@ test_stop_server (Fixture *f, stringified_result = g_variant_print (tuple, TRUE); g_test_message ("GetServerInfo() -> %s", stringified_result); g_free (stringified_result); + g_assert_cmpstr (g_variant_get_type_string (tuple), ==, "(a{sv}sssa{sv})"); + g_variant_get (tuple, "(@a{sv}&s&s&s@a{sv})", + &creator, &type, &app_id, &instance_id, &asv); + g_variant_dict_init (&dict, creator); + g_assert_true (g_variant_dict_lookup (&dict, "UnixUserID", "u", &uid)); + g_assert_cmpuint (uid, ==, _dbus_getuid ()); + g_variant_dict_clear (&dict); + g_assert_cmpstr (type, ==, "com.example.NotFlatpak"); + g_assert_cmpstr (app_id, ==, "sample-app"); + g_assert_cmpstr (instance_id, ==, ""); + g_assert_cmpuint (g_variant_n_children (asv), ==, 0); + g_clear_pointer (&asv, g_variant_unref); + g_clear_pointer (&creator, g_variant_unref); g_clear_pointer (&tuple, g_variant_unref); /* Now disconnect the last confined connection, which will make the From 12f66ac590f121a2481a9e021528743a0a2768ec Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Tue, 1 Oct 2024 18:00:39 +0100 Subject: [PATCH 21/23] spec: Make the Containers spec consistent with other interfaces Follow-up after dbus!472. Signed-off-by: Simon McVittie --- doc/dbus-specification.xml | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/doc/dbus-specification.xml b/doc/dbus-specification.xml index 3099a46c..486d2e53 100644 --- a/doc/dbus-specification.xml +++ b/doc/dbus-specification.xml @@ -7479,7 +7479,7 @@ - <literal>org.freedesktop.DBus.Containers1.AddServer</literal> + Method: <literal>org.freedesktop.DBus.Containers1.AddServer</literal> As a method: @@ -7648,7 +7648,7 @@ - StopOnDisconnect + StopOnDisconnect BOOLEAN true @@ -7664,7 +7664,7 @@ - StopOnNotify + StopOnNotify UNIX_FD (none) @@ -7706,7 +7706,7 @@ - Address + Address STRING A connectable D-Bus address, for example @@ -7714,7 +7714,7 @@ - SocketPath + SocketPath ARRAY of BYTE The absolute filesystem path of the resulting @@ -7770,7 +7770,7 @@ - <literal>org.freedesktop.DBus.Containers1.StopServer</literal> + Method: <literal>org.freedesktop.DBus.Containers1.StopServer</literal> As a method: @@ -7827,7 +7827,7 @@ - <literal>org.freedesktop.DBus.Containers1.StopListening</literal> + Method: <literal>org.freedesktop.DBus.Containers1.StopListening</literal> As a method: @@ -7888,7 +7888,7 @@ - <literal>org.freedesktop.DBus.Containers1.GetConnectionInfo</literal> + Method: <literal>org.freedesktop.DBus.Containers1.GetConnectionInfo</literal> As a method: @@ -8033,7 +8033,7 @@ - <literal>org.freedesktop.DBus.Containers1.GetServerInfo</literal> + Method: <literal>org.freedesktop.DBus.Containers1.GetServerInfo</literal> As a method: @@ -8149,7 +8149,7 @@ - <literal>org.freedesktop.DBus.Containers1.ServerRemoved</literal> + Signal: <literal>org.freedesktop.DBus.Containers1.ServerRemoved</literal> As a signal emitted by the message bus: From 4012eb96dbb0ffb1d179ba6e0c3897e2a83c5f27 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Tue, 1 Oct 2024 18:05:56 +0100 Subject: [PATCH 22/23] spec: Recommend using the same IDs for Containers1 and security-context These two specifications have been made intentionally similar. Signed-off-by: Simon McVittie --- doc/dbus-specification.xml | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/doc/dbus-specification.xml b/doc/dbus-specification.xml index 486d2e53..5523693d 100644 --- a/doc/dbus-specification.xml +++ b/doc/dbus-specification.xml @@ -7513,6 +7513,10 @@ same syntactic rules as a D-Bus interface name. + Container managers should use the same identifier here + that they use for the + set_sandbox_engine request in the + Wayland security-context extension. @@ -7525,6 +7529,10 @@ type does not have a concept of identifying or naming its applications or containers by a string, using the empty string here is suggested. + Container managers should use the same identifier here + that they use for the + set_app_id request in the + Wayland security-context extension. @@ -7537,6 +7545,10 @@ type does not have a concept of identifying or naming instances by a string, using the empty string here is suggested. + Container managers should use the same identifier here + that they use for the + set_instance_id request in the + Wayland security-context extension. From 69cd3eb652c116f1cee152c2f9e3fa51e21515c1 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Tue, 1 Oct 2024 18:15:37 +0100 Subject: [PATCH 23/23] spec: Mention that Containers1 confined connections can't BecomeMonitor Signed-off-by: Simon McVittie --- doc/dbus-specification.xml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/doc/dbus-specification.xml b/doc/dbus-specification.xml index 5523693d..7c6a7e71 100644 --- a/doc/dbus-specification.xml +++ b/doc/dbus-specification.xml @@ -8252,7 +8252,12 @@ monitor connections In the reference implementation, - the default configuration is that each user (identified by + the default configuration is that connections to a + container server managed by + the + Containers1 interface + are not privileged and cannot become a monitor; + otherwise, each user (identified by numeric user ID) may monitor their own session bus, and the root user (user ID zero) may monitor the system bus.