[RFC PATCH wayland] wayland-server: Add API to control globals visibility

[Jonas Ådahl]

On Wed, Jul 06, 2016 at 03:45:00PM +0200, Olivier Fourdan wrote:
> 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.

FWIW, I think this is a reasonable idea, especially when compositors use
private shell protocols that shouldn't be used by random clients, 
thus there is no reason to expose them. Right now we simply rely on
disconnecting any client that tries to bind a private protocol. Some
comments on the patch itself follow.

> 
> Signed-off-by: Olivier Fourdan <ofourdan at redhat.com>
> ---
>  src/wayland-server-core.h |  8 +++++++
>  src/wayland-server.c      | 38 ++++++++++++++++++++++++++-----
>  tests/display-test.c      | 57 +++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 97 insertions(+), 6 deletions(-)
> 
> diff --git a/src/wayland-server-core.h b/src/wayland-server-core.h
> index ad1292f..b02ebcd 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"
>  
> @@ -141,6 +142,9 @@ struct wl_client;
>  typedef void (*wl_global_bind_func_t)(struct wl_client *client, void *data,
>  				      uint32_t version, uint32_t id);
>  
> +typedef bool (*wl_display_filter_global_func_t)(const struct wl_client *client,
> +						const struct wl_interface *interface);

Probably want a void *user_data here. I also wonder whether we should
just pass the wl_global instead of the interface, and maybe add a
wl_global_get_interface if we need to. If we just always pass just a
client and an interface, but for some reason want to add more details
about what global, we'll be in a difficult situation.

> +
>  uint32_t
>  wl_display_get_serial(struct wl_display *display);
>  
> @@ -155,6 +159,10 @@ struct wl_listener *
>  wl_display_get_destroy_listener(struct wl_display *display,
>  				wl_notify_func_t notify);
>  
> +void
> +wl_display_set_filter_global(struct wl_display *display,
> +			     wl_display_filter_global_func_t filter);
> +
>  struct wl_global *
>  wl_global_create(struct wl_display *display,
>  		 const struct wl_interface *interface,
> diff --git a/src/wayland-server.c b/src/wayland-server.c
> index 19aa2e8..55e532e 100644
> --- a/src/wayland-server.c
> +++ b/src/wayland-server.c
> @@ -98,6 +98,7 @@ struct wl_display {
>  	struct wl_signal destroy_signal;
>  
>  	struct wl_array additional_shm_formats;
> +	wl_display_filter_global_func_t wl_display_global_filter;
>  };
>  
>  struct wl_global {
> @@ -740,7 +741,8 @@ 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
> +	else if (display->wl_display_global_filter == NULL ||
> +		 display->wl_display_global_filter (client, global->interface))

Could probably put these two lines in a filter_global (client, global)
helper, which can be reused in display_get_registry().

Also, in the case that a client tries to bind a filtered global, we
should probably just disconnect it.


Jonas

>  		global->bind(client, global->data, version, id);
>  }
>  
> @@ -795,11 +797,13 @@ display_get_registry(struct wl_client *client,
>  		       &registry_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 (display->wl_display_global_filter == NULL ||
> +		    display->wl_display_global_filter (client, global->interface))
> +			wl_resource_post_event(registry_resource,
> +					       WL_REGISTRY_GLOBAL,
> +					       global->name,
> +					       global->interface->name,
> +					       global->version);
>  }
>  
>  static const struct wl_display_interface display_interface = {
> @@ -867,6 +871,7 @@ wl_display_create(void)
>  
>  	display->id = 1;
>  	display->serial = 0;
> +	display->wl_display_global_filter = NULL;
>  
>  	wl_array_init(&display->additional_shm_formats);
>  
> @@ -940,6 +945,27 @@ wl_display_destroy(struct wl_display *display)
>  	free(display);
>  }
>  
> +/** Set-up a filter function for global objects
> + *
> + * \param display The Wayland display object.
> + * \param filter  The global filter funtion.
> + * \return None.
> + *
> + * Setup a filter for the wl_display to advertise or hide global objects
> + * to clients.
> + * Using such a filter that will be called from wl_registry_bind()
> + * handler and display_get_registry(), the compositor can control which
> + * wl_global objects are advertised to any given client.
> + *
> + * \memberof wl_display
> + */
> +WL_EXPORT void
> +wl_display_set_filter_global(struct wl_display *display,
> +			     wl_display_filter_global_func_t filter)
> +{
> +	display->wl_display_global_filter = filter;
> +}
> +
>  WL_EXPORT struct wl_global *
>  wl_global_create(struct wl_display *display,
>  		 const struct wl_interface *interface, int version,
> diff --git a/tests/display-test.c b/tests/display-test.c
> index 17956db..1548c59 100644
> --- a/tests/display-test.c
> +++ b/tests/display-test.c
> @@ -125,6 +125,7 @@ struct handler_info {
>  	struct wl_seat *seat;
>  	uint32_t bind_version;
>  	bool use_unversioned;
> +	bool has_data_offer;
>  };
>  
>  static void
> @@ -145,6 +146,8 @@ registry_handle_globals(void *data, struct wl_registry *registry,
>  			hi->seat = wl_registry_bind(registry, id,
>  						    &wl_seat_interface, ver);
>  		assert(hi->seat);
> +	} else if (strcmp(intf, "wl_data_offer") == 0) {
> +		hi->has_data_offer = true;
>  	}
>  }
>  
> @@ -926,3 +929,57 @@ TEST(error_on_destroyed_object)
>  	display_resume(d);
>  	display_destroy(d);
>  }
> +
> +static bool
> +display_global_filter(const struct wl_client *client,
> +		      const struct wl_interface *interface)
> +{
> +	/* Hide the wl_data_offer interface */
> +	if (interface == &wl_data_offer_interface)
> +		return false;
> +
> +	/* Show all the others */
> +	return true;
> +}
> +
> +static void
> +bind_data_offer(struct wl_client *client, void *data,
> +		uint32_t vers, uint32_t id)
> +{
> +	struct display *d = data;
> +	struct client_info *ci;
> +	struct wl_resource *res;
> +
> +	ci = find_client_info(d, client);
> +	assert(ci);
> +
> +	res = wl_resource_create(client, &wl_data_offer_interface, vers, id);
> +	assert(res);
> +}
> +
> +TEST(filter)
> +{
> +	struct handler_info hi;
> +	struct display *d;
> +	struct wl_global *g1, *g2;
> +
> +	d = display_create();
> +	wl_display_set_filter_global(d->wl_display, display_global_filter);
> +
> +	g1 = wl_global_create(d->wl_display, &wl_seat_interface,
> +			      1, d, bind_seat);
> +	g2 = wl_global_create(d->wl_display, &wl_data_offer_interface,
> +			      1, d, bind_data_offer);
> +
> +	hi.has_data_offer = false;
> +	client_create(d, seat_version, &hi);
> +	assert(hi.seat != NULL);
> +	assert(hi.has_data_offer != true);
> +
> +	display_run(d);
> +
> +	wl_global_destroy(g1);
> +	wl_global_destroy(g2);
> +
> +	display_destroy(d);
> +}
> -- 
> 2.7.4
>