[PATCH weston] introduces a setting to give permission to any client to do screenshots

[Sebastian Wick]

Am 2013-12-10 00:20, schrieb Bryce W. Harrington:
> On Wed, Dec 04, 2013 at 05:38:23PM +0100, Sebastian Wick wrote:
>> This patch adds a screenshooter section with the "restrict-access"
>> setting which is on by default and is the current behavior of weston.
>> When turning it off, all clients can use the screenshooter protocol.
>> This makes screen capturing for clients easier because they don't
>> have to be started by weston.
>> ---
>>  man/weston.ini.man  | 6 ++++++
>>  src/screenshooter.c | 8 +++++++-
>>  weston.ini.in       | 3 +++
>>  3 files changed, 16 insertions(+), 1 deletion(-)
>> 
>> diff --git a/man/weston.ini.man b/man/weston.ini.man
>> index 6be90bf..b94ac3d 100644
>> --- a/man/weston.ini.man
>> +++ b/man/weston.ini.man
>> @@ -408,6 +408,12 @@ The terminal shell (string). Sets the $TERM 
>> variable.
>>  sets the path to the xserver to run (string).
>>  .RE
>>  .RE
>> +.SH "SCREENSHOOTER SECTION"
>> +.TP 7
>> +.BI "restrict-access=" true
>> +only allow authenticated clients to take screenshots (boolean).
>> +.RE
>> +.RE
>>  .SH "SEE ALSO"
>>  .BR weston (1),
>>  .BR weston-launch (1),
>> diff --git a/src/screenshooter.c b/src/screenshooter.c
>> index 0c657bc..65b6c09 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 restrict_access;
>> 
>>  	resource = wl_resource_create(client,
>>  				      &screenshooter_interface, 1, id);
>> 
>> -	if (client != shooter->client) {
>> +	section = weston_config_get_section(shooter->ec->config, 
>> "screenshooter", NULL, NULL);
>> +	weston_config_section_get_bool(section,
>> +				"restrict-access", &restrict_access, 1);
> 
> Could also check the return value of weston_config_section_get_bool;
> it'll set errno and return -1 if the config value was typo'd or
> omitted.

It will save the default value, true in this case, if it's missing so it
should be fine.

> But does this have security implications?  I assume it is restricted by
> default in order to prevent clients from snooping.  Could you add a bit
> more detail about the specific problem(s) being solved with this?  
> Maybe
> there's a way to solve the problem without fully dropping the
> restriction?

I wrote a GStreamer wayland source element which needs to receive the 
data
somehow and it uses the screenshooter protocol to do so.

The screenshooter protocol however is restricted to clients which got
started by weston itself (only weston-screenshooter so far) to make sure
the client has not been manipulated. You would have to start every
application which might use the GStreamer wayland source element by 
weston.
It would make it drastically harder to use it and on a average linux PC
the current mechanism doesn't give you more security so it's pretty save
to turn the restriction off.

If you have a system where every client is sandboxed and you don't want 
any
client to see what the others have rendered, you should not turn the
restriction off.

>> +
>> +	if (restrict_access && 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..bc32567 100644
>> --- a/weston.ini.in
>> +++ b/weston.ini.in
>> @@ -65,3 +65,6 @@ path=@libexecdir@/weston-keyboard
>>  #constant_accel_factor = 50
>>  #min_accel_factor = 0.16
>>  #max_accel_factor = 1.0
>> +
>> +#[screenshooter]
>> +#restrict-access=false
> 
> 
> Bryce

Sebastian