[RFC weston] make client isolation optional

[Pekka Paalanen]

Hi Sebastian

On Wed,  4 Dec 2013 01:09:55 +0100
Sebastian Wick <sebastian at sebastianwick.net> wrote:

> This patch allows wayland clients to use protocols which give away information about other clients without being started by the compositor. The reason to denie access on those protocols is to make sure no information about the clients is leaked to other clients (=security). I think that we don't need to enforce this on the most systems because without complete isolation of all processes it's possible to get the information even without using the compositor. In all other cases you can simply turn it on again.
> ---

Please, cut also the lines in the commit message body.

When I read the title and commit message I thought "this change will
somehow make all protocol objects global and referenciable by any
client, or at least surfaces" and thought how on Earth can it do that.
Of course, my first impression was completely off.

>  man/weston.ini.man  | 3 +++
>  src/screenshooter.c | 8 +++++++-
>  weston.ini.in       | 1 +
>  3 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/man/weston.ini.man b/man/weston.ini.man
> index cc88ca8..6d41653 100644
> --- a/man/weston.ini.man
> +++ b/man/weston.ini.man
> @@ -128,6 +128,9 @@ sets the GBM format used for the framebuffer for the GBM backend. Can be
>  .B xrgb2101010,
>  .B rgb565.
>  By default, xrgb8888 is used.
> +.TP 7
> +.BI "client-isolation="false
> +completely isolate clients (boolean).

This is not really "client isolation", this is only about
screen capturing. Client isolation is much much more in Wayland.

I would suggest to call and document this just like it is: it gives
permission to *any* client to do screenshots.

If there will be other features like screenshooting, I think it would
make sense to have them unrestricted per feature, and not with a single
big switch.

>  .RS
>  .PP
>  
> diff --git a/src/screenshooter.c b/src/screenshooter.c
> index 0c657bc..fa3dbb8 100644
> --- a/src/screenshooter.c
> +++ b/src/screenshooter.c
> @@ -224,11 +224,17 @@ bind_shooter(struct wl_client *client,
>  {
>  	struct screenshooter *shooter = data;
>  	struct wl_resource *resource;
> +	struct weston_config_section *section;
> +	int client_isolation;
>  
>  	resource = wl_resource_create(client,
>  				      &screenshooter_interface, 1, id);
>  
> -	if (client != shooter->client) {
> +	section = weston_config_get_section(shooter->ec->config, "core", NULL, NULL);
> +	weston_config_section_get_bool(section,
> +				"client-isolation", &client_isolation, 0);

I'd really like screenshooting to be restricted by default, instead of
free-for-all.

If we start really splitting hairs, there is no such thing as complete
isolation for a multi-user, network-connected computer. A system is as
secure as its weakest link, therefore it makes sense to plug at least
the obvious holes. I do not buy the argument "no need to plug this
hole, because there are other holes."

> +
> +	if (client_isolation && client != shooter->client) {
>  		wl_resource_post_error(resource, WL_DISPLAY_ERROR_INVALID_OBJECT,
>  				       "screenshooter failed: permission denied");
>  		wl_resource_destroy(resource);
> diff --git a/weston.ini.in b/weston.ini.in
> index 5181a9e..1261788 100644
> --- a/weston.ini.in
> +++ b/weston.ini.in
> @@ -2,6 +2,7 @@
>  #modules=xwayland.so,cms-colord.so
>  #shell=desktop-shell.so
>  #gbm-format=xrgb2101010
> +#client-isolation=true
>  
>  [shell]
>  background-image=/usr/share/backgrounds/gnome/Aqua.jpg

I'd presume a real DE would have a configuration dialog with a tick box
"allow all programs to take screenshots" that you can toggle at runtime,
but I think a weston.ini setting is fine for weston.


Thanks,
pq