2016-08-08 11:07 GMT+02:00 Olivier Fourdan <ofour...@redhat.com>: > Hi Giulio, > > On 8 August 2016 at 10:53, Giulio Camuffo <giuliocamu...@gmail.com> wrote: >> Hi, >> >> this looks like a nice way to implement restricted interfaces... i >> just wonder, would we need to revoke allowed interfaces? How would >> this approach allow that? > > Not sure what you mean, but if an interface was allowed, revoking it is just > a matter of returning false in the filter for that given interface. > > But I don't think we should revoke interfaces that were previously allowed > (unless there are good reasons for that, of course) > > Take the weston shell for example, that one was not allowed but visible, a > client can still "see" it and might try to bind to this interface (but would > get an error WL_DISPLAY_ERROR_INVALID_OBJECT in bind_desktop_shell() when > trying), we can now completely hide this interface using the filter.
I mean revoking it in the client lifetime, after it bound it, so we'd need a way to remove a global only for some clients. I'm not really sure if we need it but it was a question when i sent my proposal for an interface to allow negotiating the access to restricted interfaces. But now that i think about it, it probably doesn't matter since i don't think we can use this approach to negotiate access to interface, we need a different solution. > >> Also, this still doesn't prevent a client from binding an interface if >> it knows the name and id. Do we care? > > registry_bind() will raise an WL_DISPLAY_ERROR_INVALID_OBJECT if a client try > to bind an interface which is filtered out, so this should be covered, or am > I missing something? Sorry, you're right. So, r-b by me, all three. > > Cheers, > Olivier > >> 2016-08-08 10:10 GMT+02:00 Olivier Fourdan <ofour...@redhat.com>: >> > Add a new API to let compositor decide whether or not a wl_global >> > should be advertised to the clients via wl_registry_bind() or >> > display_get_registry() >> > >> > By using its own filter, the compositor can decide which wl_global would >> > be listed to clients. >> > >> > Compositors can use this mechanism to hide their own private interfaces >> > that regular clients should not use. >> > >> > Signed-off-by: Olivier Fourdan <ofour...@redhat.com> >> > --- >> > v2: Follow-up on Jonas' comments on v1: >> > Add global's data as user data in callback filter function >> > Pass wl_global instead of wl_interface in callback filter function >> > Add wl_global_get_interface() to retrieve the wl_interface from the >> > given wl_global >> > Post an error if a client tries to bind a global which is filtered >> > out. >> > v3: Follow-up on Jonas' comments on v2: >> > Separate from the other wl_global and test additions >> > Add its own user data to the global filter >> > Rephrase the API doc >> > Other few small changes >> > >> > src/wayland-server-core.h | 10 ++++++++ >> > src/wayland-server.c | 62 >> > +++++++++++++++++++++++++++++++++++++++++++---- >> > 2 files changed, 67 insertions(+), 5 deletions(-) >> > >> > diff --git a/src/wayland-server-core.h b/src/wayland-server-core.h >> > index ad1292f..b70d85f 100644 >> > --- a/src/wayland-server-core.h >> > +++ b/src/wayland-server-core.h >> > @@ -28,6 +28,7 @@ >> > >> > #include <sys/types.h> >> > #include <stdint.h> >> > +#include <stdbool.h> >> > #include "wayland-util.h" >> > #include "wayland-version.h" >> > >> > @@ -164,6 +165,15 @@ wl_global_create(struct wl_display *display, >> > void >> > wl_global_destroy(struct wl_global *global); >> > >> > +typedef bool (*wl_display_filter_global_func_t)(const struct wl_client >> > *client, >> > + const struct wl_global >> > *global, >> > + void *data); >> > + >> > +void >> > +wl_display_set_filter_global(struct wl_display *display, >> > + wl_display_filter_global_func_t filter, >> > + void *data); >> > + >> > struct wl_client * >> > wl_client_create(struct wl_display *display, int fd); >> > >> > diff --git a/src/wayland-server.c b/src/wayland-server.c >> > index 19aa2e8..480af23 100644 >> > --- a/src/wayland-server.c >> > +++ b/src/wayland-server.c >> > @@ -98,6 +98,9 @@ struct wl_display { >> > struct wl_signal destroy_signal; >> > >> > struct wl_array additional_shm_formats; >> > + >> > + wl_display_filter_global_func_t global_filter; >> > + void *global_filter_data; >> > }; >> > >> > struct wl_global { >> > @@ -714,6 +717,16 @@ wl_client_destroy(struct wl_client *client) >> > free(client); >> > } >> > >> > +static bool >> > +filter_global(const struct wl_client *client, >> > + const struct wl_global *global) >> > +{ >> > + struct wl_display *display = client->display; >> > + >> > + return (display->global_filter == NULL || >> > + display->global_filter(client, global, >> > display->global_filter_data)); >> > +} >> > + >> > static void >> > registry_bind(struct wl_client *client, >> > struct wl_resource *resource, uint32_t name, >> > @@ -740,6 +753,10 @@ registry_bind(struct wl_client *client, >> > WL_DISPLAY_ERROR_INVALID_OBJECT, >> > "invalid version for global %s (%d): >> > have %d, wanted %d", >> > interface, name, global->version, >> > version); >> > + else if (!filter_global(client, global)) >> > + wl_resource_post_error(resource, >> > + WL_DISPLAY_ERROR_INVALID_OBJECT, >> > + "invalid global %s (%d)", interface, >> > name); >> > else >> > global->bind(client, global->data, version, id); >> > } >> > @@ -795,11 +812,12 @@ display_get_registry(struct wl_client *client, >> > ®istry_resource->link); >> > >> > wl_list_for_each(global, &display->global_list, link) >> > - wl_resource_post_event(registry_resource, >> > - WL_REGISTRY_GLOBAL, >> > - global->name, >> > - global->interface->name, >> > - global->version); >> > + if (filter_global(client, global)) >> > + wl_resource_post_event(registry_resource, >> > + WL_REGISTRY_GLOBAL, >> > + global->name, >> > + global->interface->name, >> > + global->version); >> > } >> > >> > static const struct wl_display_interface display_interface = { >> > @@ -868,6 +886,9 @@ wl_display_create(void) >> > display->id = 1; >> > display->serial = 0; >> > >> > + display->global_filter = NULL; >> > + display->global_filter_data = NULL; >> > + >> > wl_array_init(&display->additional_shm_formats); >> > >> > return display; >> > @@ -940,6 +961,37 @@ wl_display_destroy(struct wl_display *display) >> > free(display); >> > } >> > >> > +/** Set a filter function for global objects >> > + * >> > + * \param display The Wayland display object. >> > + * \param filter The global filter funtion. >> > + * \param data User data to be associated with the global filter. >> > + * \return None. >> > + * >> > + * Set a filter for the wl_display to advertise or hide global objects >> > + * to clients. >> > + * The set filter will be used during wl_global advertisment to >> > + * determine whether a global object should be advertised to a >> > + * given client, and during wl_global binding to determine whether >> > + * a given client should be allowed to bind to a global. >> > + * >> > + * Clients that try to bind to a global that was filtered out will >> > + * have an error raised. >> > + * >> > + * Setting the filter NULL will result in all globals being >> > + * advertised to all clients. The default is no filter. >> > + * >> > + * \memberof wl_display >> > + */ >> > +WL_EXPORT void >> > +wl_display_set_filter_global(struct wl_display *display, >> > + wl_display_filter_global_func_t filter, >> > + void *data) >> > +{ >> > + display->global_filter = filter; >> > + display->global_filter_data = data; >> > +} >> > + >> > WL_EXPORT struct wl_global * >> > wl_global_create(struct wl_display *display, >> > const struct wl_interface *interface, int version, >> > -- >> > 2.7.4 >> > >> > _______________________________________________ >> > wayland-devel mailing list >> > wayland-devel@lists.freedesktop.org >> > https://lists.freedesktop.org/mailman/listinfo/wayland-devel >> _______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel