[v3,wayland-protocols] virtual-keyboard: Add new virtual keyboard protocol

Submitted by Dorota Czaplejewicz on June 22, 2018, 3:20 p.m.

Details

Message ID 20180622152045.10563-1-dorota.czaplejewicz@puri.sm
State Superseded
Headers show
Series "virtual-keyboard: Add new virtual keyboard protocol" ( rev: 3 ) in Wayland

Not browsing as part of any series.

Commit Message

Dorota Czaplejewicz June 22, 2018, 3:20 p.m.
Provides the ability to emulate keyboards by applications. Complementary to input-method protocol.

The interface is a mirror copy of wl_keyboard, with removed serials, and added seat binding.
---
Hi,

this patch is another improvement to the previously sent virtual keyboard protocol. Changes compared to v2:

- readded enum names
- removed suggestion to listen to modifiers
- clarified untrusted client behaviour
- added destructor on virtual_keyboard_manager
- fixed text wrapping

First off, the new version of wayland-scanner looks up enums found in other xml files, so the textual reference are gone and actual enum values are used.

Secondly, there was a suggestion that if the client wants to know global modifiers, it should listen to wl_keyboard.modifiers. In reality, this can only be done when the client has keyboard focus. I decided to remove this suggestion.

Clarified that create_virtual_keyboard must error out with unauthorized when an untrusted client connects. That doesn't preclude making the whole virtual_keyboard_manager interface unavailable when the client is determined as untrusted ahead of time.

Lastly, added a missing destructor as per Simon Ser's suggestion.

I hope that we're getting closer to perfect with this revision! As usual, feedback is welcome.

Cheers,
Dorota Czaplejewicz

 Makefile.am                                        |   1 +
 unstable/virtual-keyboard/README                   |   2 +
 .../virtual-keyboard-unstable-v1.xml               | 116 +++++++++++++++++++++
 3 files changed, 119 insertions(+)
 create mode 100644 unstable/virtual-keyboard/README
 create mode 100644 unstable/virtual-keyboard/virtual-keyboard-unstable-v1.xml

Patch hide | download patch | download mbox

diff --git a/Makefile.am b/Makefile.am
index 1aa13cf..17cedc1 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -19,6 +19,7 @@  unstable_protocols =								\
 	unstable/keyboard-shortcuts-inhibit/keyboard-shortcuts-inhibit-unstable-v1.xml \
 	unstable/xdg-output/xdg-output-unstable-v1.xml				\
 	unstable/input-timestamps/input-timestamps-unstable-v1.xml	\
+	unstable/virtual-keyboard/virtual-keyboard-unstable-v1.xml	\
 	$(NULL)
 
 stable_protocols =								\
diff --git a/unstable/virtual-keyboard/README b/unstable/virtual-keyboard/README
new file mode 100644
index 0000000..a2c646d
--- /dev/null
+++ b/unstable/virtual-keyboard/README
@@ -0,0 +1,2 @@ 
+Virtual keyboard protocol
+
diff --git a/unstable/virtual-keyboard/virtual-keyboard-unstable-v1.xml b/unstable/virtual-keyboard/virtual-keyboard-unstable-v1.xml
new file mode 100644
index 0000000..bde55e8
--- /dev/null
+++ b/unstable/virtual-keyboard/virtual-keyboard-unstable-v1.xml
@@ -0,0 +1,116 @@ 
+<?xml version="1.0" encoding="UTF-8"?>
+<protocol name="virtual_keyboard_unstable_v1">
+  <copyright>
+    Copyright © 2008-2011  Kristian Høgsberg
+    Copyright © 2010-2013  Intel Corporation
+    Copyright © 2012-2013  Collabora, Ltd.
+    Copyright © 2018       Purism SPC
+
+    Permission is hereby granted, free of charge, to any person obtaining a
+    copy of this software and associated documentation files (the "Software"),
+    to deal in the Software without restriction, including without limitation
+    the rights to use, copy, modify, merge, publish, distribute, sublicense,
+    and/or sell copies of the Software, and to permit persons to whom the
+    Software is furnished to do so, subject to the following conditions:
+
+    The above copyright notice and this permission notice (including the next
+    paragraph) shall be included in all copies or substantial portions of the
+    Software.
+
+    THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+    IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+    FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+    THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+    LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+    FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+    DEALINGS IN THE SOFTWARE.
+  </copyright>
+
+  <interface name="zwp_virtual_keyboard_v1" version="1">
+    <description summary="virtual keyboard">
+      The virtual keyboard provides an application with requests which emulate
+      the behaviour of a physical keyboard.
+
+      This interface can be used by clients on its own to provide raw input
+      events, or it can accompany the input method protocol.
+    </description>
+
+    <request name="keymap">
+      <description summary="keyboard mapping">
+        Provide a file descriptor to the compositor which can be memory-mapped
+        to provide a keyboard mapping description.
+      </description>
+      <arg name="format" type="uint" enum="keymap_format"
+        summary="keymap format"/>
+      <arg name="fd" type="fd" summary="keymap file descriptor"/>
+      <arg name="size" type="uint" summary="keymap size, in bytes"/>
+    </request>
+    
+    <enum name="error">
+      <entry name="no_keymap" value="0" summary="No keymap was set"/>
+    </enum>
+
+    <request name="key">
+      <description summary="key event">
+        A key was pressed or released.
+        The time argument is a timestamp with millisecond granularity, with an
+        undefined base. All requests regarding a single object must share the
+        same clock.
+
+        Keymap must be set before issuing this request.
+      </description>
+      <arg name="time" type="uint"
+        summary="timestamp with millisecond granularity"/>
+      <arg name="key" type="uint" summary="key that produced the event"/>
+      <arg name="state" type="uint" enum="key_state"
+        summary="physical state of the key"/>
+    </request>
+
+    <request name="modifiers">
+      <description summary="modifier and group state">
+        Notifies the compositor that the modifier and/or group state has
+        changed, and it should update state.
+
+        Keymap must be set before issuing this request.
+      </description>
+      <arg name="mods_depressed" type="uint" summary="depressed modifiers"/>
+      <arg name="mods_latched" type="uint" summary="latched modifiers"/>
+      <arg name="mods_locked" type="uint" summary="locked modifiers"/>
+      <arg name="group" type="uint" summary="keyboard layout"/>
+    </request>
+
+    <request name="destroy" type="destructor">
+      <description summary="destroy the virtual keyboard object"/>
+    </request>
+  </interface>
+
+  <interface name="zwp_virtual_keyboard_manager_v1" version="1">
+    <description summary="virtual keyboard manager">
+      A virtual keyboard manager allows an application to provide keyboard
+      input events as if they came from a physical keyboard.
+
+      If the compositor enables a keyboard to perform arbitrary actions, it
+      should prevent untrusted clients from using this interface.
+    </description>
+
+    <enum name="error">
+      <entry name="unauthorized" value="0"
+        summary="client not authorized to use the interface"/>
+    </enum>
+
+    <request name="create_virtual_keyboard">
+      <description summary="Create a new virtual keyboard">
+        Creates a new virtual keyboard associated to a seat.
+
+        If an untrusted client issues this request, it should receive the
+        unauthorized error in return.
+      </description>
+      <arg name="seat" type="object" interface="wl_seat"/>
+      <arg name="id" type="new_id" interface="zwp_virtual_keyboard_v1"/>
+    </request>
+
+    <request name="destroy" type="destructor">
+      <description summary="destroy the virtual keyboard manager object"/>
+    </request>
+  </interface>
+</protocol>

Comments

On June 22, 2018 4:20 PM, Dorota Czaplejewicz <dorota.czaplejewicz@puri.sm> wrote:
> Provides the ability to emulate keyboards by applications. Complementary to input-method protocol.
>
> The interface is a mirror copy of wl_keyboard, with removed serials, and added seat binding.
> ---
> Hi,
>
> this patch is another improvement to the previously sent virtual keyboard protocol. Changes compared to v2:
>
> - readded enum names
> - removed suggestion to listen to modifiers
> - clarified untrusted client behaviour
> - added destructor on virtual_keyboard_manager
> - fixed text wrapping
>
> First off, the new version of wayland-scanner looks up enums found in other xml files, so the textual reference are gone and actual enum values are used.
>
> Secondly, there was a suggestion that if the client wants to know global modifiers, it should listen to wl_keyboard.modifiers. In reality, this can only be done when the client has keyboard focus. I decided to remove this suggestion.
>
> Clarified that create_virtual_keyboard must error out with unauthorized when an untrusted client connects. That doesn't preclude making the whole virtual_keyboard_manager interface unavailable when the client is determined as untrusted ahead of time.
>
> Lastly, added a missing destructor as per Simon Ser's suggestion.
>
> I hope that we're getting closer to perfect with this revision! As usual, feedback is welcome.
>
> Cheers,
> Dorota Czaplejewicz

Hi,

Thanks for this updated version. A few comments below.

>  Makefile.am                                        |   1 +
>  unstable/virtual-keyboard/README                   |   2 +
>  .../virtual-keyboard-unstable-v1.xml               | 116 +++++++++++++++++++++
>  3 files changed, 119 insertions(+)
>  create mode 100644 unstable/virtual-keyboard/README
>  create mode 100644 unstable/virtual-keyboard/virtual-keyboard-unstable-v1.xml
>
> diff --git a/Makefile.am b/Makefile.am
> index 1aa13cf..17cedc1 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -19,6 +19,7 @@ unstable_protocols =								\
>  	unstable/keyboard-shortcuts-inhibit/keyboard-shortcuts-inhibit-unstable-v1.xml \
>  	unstable/xdg-output/xdg-output-unstable-v1.xml				\
>  	unstable/input-timestamps/input-timestamps-unstable-v1.xml	\
> +	unstable/virtual-keyboard/virtual-keyboard-unstable-v1.xml	\
>  	$(NULL)
>
>  stable_protocols =								\
> diff --git a/unstable/virtual-keyboard/README b/unstable/virtual-keyboard/README
> new file mode 100644
> index 0000000..a2c646d
> --- /dev/null
> +++ b/unstable/virtual-keyboard/README
> @@ -0,0 +1,2 @@
> +Virtual keyboard protocol
> +
> diff --git a/unstable/virtual-keyboard/virtual-keyboard-unstable-v1.xml b/unstable/virtual-keyboard/virtual-keyboard-unstable-v1.xml
> new file mode 100644
> index 0000000..bde55e8
> --- /dev/null
> +++ b/unstable/virtual-keyboard/virtual-keyboard-unstable-v1.xml
> @@ -0,0 +1,116 @@
> +<?xml version="1.0" encoding="UTF-8"?>
> +<protocol name="virtual_keyboard_unstable_v1">
> +  <copyright>
> +    Copyright © 2008-2011  Kristian Høgsberg
> +    Copyright © 2010-2013  Intel Corporation
> +    Copyright © 2012-2013  Collabora, Ltd.
> +    Copyright © 2018       Purism SPC
> +
> +    Permission is hereby granted, free of charge, to any person obtaining a
> +    copy of this software and associated documentation files (the "Software"),
> +    to deal in the Software without restriction, including without limitation
> +    the rights to use, copy, modify, merge, publish, distribute, sublicense,
> +    and/or sell copies of the Software, and to permit persons to whom the
> +    Software is furnished to do so, subject to the following conditions:
> +
> +    The above copyright notice and this permission notice (including the next
> +    paragraph) shall be included in all copies or substantial portions of the
> +    Software.
> +
> +    THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> +    IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> +    FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> +    THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> +    LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> +    FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> +    DEALINGS IN THE SOFTWARE.
> +  </copyright>
> +
> +  <interface name="zwp_virtual_keyboard_v1" version="1">
> +    <description summary="virtual keyboard">
> +      The virtual keyboard provides an application with requests which emulate
> +      the behaviour of a physical keyboard.
> +
> +      This interface can be used by clients on its own to provide raw input
> +      events, or it can accompany the input method protocol.
> +    </description>
> +
> +    <request name="keymap">
> +      <description summary="keyboard mapping">
> +        Provide a file descriptor to the compositor which can be memory-mapped
> +        to provide a keyboard mapping description.
> +      </description>
> +      <arg name="format" type="uint" enum="keymap_format"
> +        summary="keymap format"/>
> +      <arg name="fd" type="fd" summary="keymap file descriptor"/>
> +      <arg name="size" type="uint" summary="keymap size, in bytes"/>
> +    </request>
> +
> +    <enum name="error">
> +      <entry name="no_keymap" value="0" summary="No keymap was set"/>
> +    </enum>
> +
> +    <request name="key">
> +      <description summary="key event">
> +        A key was pressed or released.
> +        The time argument is a timestamp with millisecond granularity, with an
> +        undefined base. All requests regarding a single object must share the
> +        same clock.
> +
> +        Keymap must be set before issuing this request.
> +      </description>
> +      <arg name="time" type="uint"
> +        summary="timestamp with millisecond granularity"/>
> +      <arg name="key" type="uint" summary="key that produced the event"/>
> +      <arg name="state" type="uint" enum="key_state"
> +        summary="physical state of the key"/>
> +    </request>
> +
> +    <request name="modifiers">
> +      <description summary="modifier and group state">
> +        Notifies the compositor that the modifier and/or group state has
> +        changed, and it should update state.
> +
> +        Keymap must be set before issuing this request.
> +      </description>
> +      <arg name="mods_depressed" type="uint" summary="depressed modifiers"/>
> +      <arg name="mods_latched" type="uint" summary="latched modifiers"/>
> +      <arg name="mods_locked" type="uint" summary="locked modifiers"/>
> +      <arg name="group" type="uint" summary="keyboard layout"/>
> +    </request>
> +
> +    <request name="destroy" type="destructor">
> +      <description summary="destroy the virtual keyboard object"/>
> +    </request>
> +  </interface>
> +
> +  <interface name="zwp_virtual_keyboard_manager_v1" version="1">
> +    <description summary="virtual keyboard manager">
> +      A virtual keyboard manager allows an application to provide keyboard
> +      input events as if they came from a physical keyboard.
> +
> +      If the compositor enables a keyboard to perform arbitrary actions, it
> +      should prevent untrusted clients from using this interface.
> +    </description>
> +
> +    <enum name="error">
> +      <entry name="unauthorized" value="0"
> +        summary="client not authorized to use the interface"/>
> +    </enum>

This is more of a general metaphorical question than anything else: I wonder how
we should handle unauthorized clients, and if adding an error for them is a good
idea. Generally errors are meant for protocol violations: it's clear from the
protocol spec that e.g. sending a request with a negative value will trigger a
protocol error. Also, errors are unrecoverable, they abort the whole client
(though this is being worked on).

So here we use an error, and the conditions in which the error is sent are
implementation-defined. I wonder if there's a better way to handle this
situation?

> +    <request name="create_virtual_keyboard">
> +      <description summary="Create a new virtual keyboard">
> +        Creates a new virtual keyboard associated to a seat.
> +
> +        If an untrusted client issues this request, it should receive the
> +        unauthorized error in return.
> +      </description>
> +      <arg name="seat" type="object" interface="wl_seat"/>
> +      <arg name="id" type="new_id" interface="zwp_virtual_keyboard_v1"/>
> +    </request>
> +
> +    <request name="destroy" type="destructor">
> +      <description summary="destroy the virtual keyboard manager object"/>

This description should probably specify what happens to keyboards created with
the manager when the manager is destroyed. A common practice is not to destroy
objects when the manager is destroyed, so that one can easily create the
manager, use the factory request, and immediately destroy the manager (see e.g.
wl_subcompositor).

> +    </request>
> +  </interface>
> +</protocol>
> --
> 2.14.4
>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
On Fri, 22 Jun 2018 12:36:16 -0400
Simon Ser <contact@emersion.fr> wrote:

> On June 22, 2018 4:20 PM, Dorota Czaplejewicz <dorota.czaplejewicz@puri.sm> wrote:
> > Provides the ability to emulate keyboards by applications. Complementary to input-method protocol.
> >
> > The interface is a mirror copy of wl_keyboard, with removed serials, and added seat binding.
> > ---
> > Hi,
> >
> > this patch is another improvement to the previously sent virtual keyboard protocol. Changes compared to v2:
> >
> > - readded enum names
> > - removed suggestion to listen to modifiers
> > - clarified untrusted client behaviour
> > - added destructor on virtual_keyboard_manager
> > - fixed text wrapping
> >
> > First off, the new version of wayland-scanner looks up enums found in other xml files, so the textual reference are gone and actual enum values are used.
> >
> > Secondly, there was a suggestion that if the client wants to know global modifiers, it should listen to wl_keyboard.modifiers. In reality, this can only be done when the client has keyboard focus. I decided to remove this suggestion.
> >
> > Clarified that create_virtual_keyboard must error out with unauthorized when an untrusted client connects. That doesn't preclude making the whole virtual_keyboard_manager interface unavailable when the client is determined as untrusted ahead of time.
> >
> > Lastly, added a missing destructor as per Simon Ser's suggestion.
> >
> > I hope that we're getting closer to perfect with this revision! As usual, feedback is welcome.
> >
> > Cheers,
> > Dorota Czaplejewicz  
> 
> Hi,
> 
> Thanks for this updated version. A few comments below.
> 
---- snip ----
> > +
> > +  <interface name="zwp_virtual_keyboard_manager_v1" version="1">
> > +    <description summary="virtual keyboard manager">
> > +      A virtual keyboard manager allows an application to provide keyboard
> > +      input events as if they came from a physical keyboard.
> > +
> > +      If the compositor enables a keyboard to perform arbitrary actions, it
> > +      should prevent untrusted clients from using this interface.
> > +    </description>
> > +
> > +    <enum name="error">
> > +      <entry name="unauthorized" value="0"
> > +        summary="client not authorized to use the interface"/>
> > +    </enum>  
> 
> This is more of a general metaphorical question than anything else: I wonder how
> we should handle unauthorized clients, and if adding an error for them is a good
> idea. Generally errors are meant for protocol violations: it's clear from the
> protocol spec that e.g. sending a request with a negative value will trigger a
> protocol error. Also, errors are unrecoverable, they abort the whole client
> (though this is being worked on).
> 
> So here we use an error, and the conditions in which the error is sent are
> implementation-defined. I wonder if there's a better way to handle this
> situation?
> 
Speaking from a position of someone without a lot of Wayland experience: are Wayland errors meant to be recoverable by a client? It's obvious that if an application doing its primary task and then trying to use a restricted protocol as a secondary action crashes, that's undesireable.

As a more concrete example, an automation application may use e.g. DBus API to automate tasks, and display a window to control it. It may request a virtual keyboard to extend its possibilities, but it shouldn't suddenly stop working if it's refused by the compositor.

That brings me to the question: should applications using restricted protocols create additional connections which may be broken and recovered from individually or should they rather use one connection? If the latter is required for some use cases, then authorization and probing/graceful rejection should be specified inside protocols. Unfortunately, I'm not sure where to look for examples. Perhaps chat applications where screen sharing may be decided ad hoc check the marks?

> > +    <request name="create_virtual_keyboard">
> > +      <description summary="Create a new virtual keyboard">
> > +        Creates a new virtual keyboard associated to a seat.
> > +
> > +        If an untrusted client issues this request, it should receive the
> > +        unauthorized error in return.
> > +      </description>
> > +      <arg name="seat" type="object" interface="wl_seat"/>
> > +      <arg name="id" type="new_id" interface="zwp_virtual_keyboard_v1"/>
> > +    </request>
> > +
> > +    <request name="destroy" type="destructor">
> > +      <description summary="destroy the virtual keyboard manager object"/>  
> 
> This description should probably specify what happens to keyboards created with
> the manager when the manager is destroyed. A common practice is not to destroy
> objects when the manager is destroyed, so that one can easily create the
> manager, use the factory request, and immediately destroy the manager (see e.g.
> wl_subcompositor).

This is an oversight and will be fixed.
> 
> > +    </request>
> > +  </interface>
> > +</protocol>
> > --
> > 2.14.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
On 2018/6月/22 08:00, Dorota Czaplejewicz wrote:
> On Fri, 22 Jun 2018 12:36:16 -0400
> Simon Ser <contact@emersion.fr> wrote:
> 
> > On June 22, 2018 4:20 PM, Dorota Czaplejewicz <dorota.czaplejewicz@puri.sm> wrote:
> > > Provides the ability to emulate keyboards by applications. Complementary to input-method protocol.
> > >
> > > The interface is a mirror copy of wl_keyboard, with removed serials, and added seat binding.
> > > ---
> > > Hi,
> > >
> > > this patch is another improvement to the previously sent virtual keyboard protocol. Changes compared to v2:
> > >
> > > - readded enum names
> > > - removed suggestion to listen to modifiers
> > > - clarified untrusted client behaviour
> > > - added destructor on virtual_keyboard_manager
> > > - fixed text wrapping
> > >
> > > First off, the new version of wayland-scanner looks up enums found in other xml files, so the textual reference are gone and actual enum values are used.
> > >
> > > Secondly, there was a suggestion that if the client wants to know global modifiers, it should listen to wl_keyboard.modifiers. In reality, this can only be done when the client has keyboard focus. I decided to remove this suggestion.
> > >
> > > Clarified that create_virtual_keyboard must error out with unauthorized when an untrusted client connects. That doesn't preclude making the whole virtual_keyboard_manager interface unavailable when the client is determined as untrusted ahead of time.
> > >
> > > Lastly, added a missing destructor as per Simon Ser's suggestion.
> > >
> > > I hope that we're getting closer to perfect with this revision! As usual, feedback is welcome.
> > >
> > > Cheers,
> > > Dorota Czaplejewicz  
> > 
> > Hi,
> > 
> > Thanks for this updated version. A few comments below.
> > 
> ---- snip ----
> > > +
> > > +  <interface name="zwp_virtual_keyboard_manager_v1" version="1">
> > > +    <description summary="virtual keyboard manager">
> > > +      A virtual keyboard manager allows an application to provide keyboard
> > > +      input events as if they came from a physical keyboard.
> > > +
> > > +      If the compositor enables a keyboard to perform arbitrary actions, it
> > > +      should prevent untrusted clients from using this interface.
> > > +    </description>
> > > +
> > > +    <enum name="error">
> > > +      <entry name="unauthorized" value="0"
> > > +        summary="client not authorized to use the interface"/>
> > > +    </enum>  
> > 
> > This is more of a general metaphorical question than anything else: I wonder how
> > we should handle unauthorized clients, and if adding an error for them is a good
> > idea. Generally errors are meant for protocol violations: it's clear from the
> > protocol spec that e.g. sending a request with a negative value will trigger a
> > protocol error. Also, errors are unrecoverable, they abort the whole client
> > (though this is being worked on).
> > 
> > So here we use an error, and the conditions in which the error is sent are
> > implementation-defined. I wonder if there's a better way to handle this
> > situation?
> > 
> Speaking from a position of someone without a lot of Wayland experience: are Wayland errors meant to be recoverable by a client? It's obvious that if an application doing its primary task and then trying to use a restricted protocol as a secondary action crashes, that's undesireable.
> 
> As a more concrete example, an automation application may use e.g. DBus API to automate tasks, and display a window to control it. It may request a virtual keyboard to extend its possibilities, but it shouldn't suddenly stop working if it's refused by the compositor.
Currently the errors will result in an abort() by libwayland. There is a patch 
series that changes this, but it will still be fatal on the wayland connection 
as far as I'm aware. Which isn't quite as bad, but can still be a problem, if 
an application uses the same wayland connection for more than one feature.
> 
> That brings me to the question: should applications using restricted protocols create additional connections which may be broken and recovered from individually or should they rather use one connection?
I'd personally prefer single fat connection. Mostly because it makes some 
bookkeeping easier, and some of the things I want to try out from the 
compositor side work better this way. But it's more robust (or rather will be) 
to have multiple, especially if one feature is expected to result in errors 
for some reason (the only expected errors should be design mistakes IMO, but 
this might still happen)

> If the latter is required for some use cases, then authorization and probing/graceful rejection should be specified inside protocols.  Unfortunately, I'm not sure where to look for examples. Perhaps chat applications where screen sharing may be decided ad hoc check the marks?
This is a good usecase for defaulting to query the user IMO. The client would 
try to access the interface at that time (either bind late, or have the first 
real request) at which point the compositor shows the user a query window that 
asks for permission to be granted for the client in question.
IFF the client uses the fat connection, it could display the window of the 
client in the query/highlight it on screen.
In the split case, this isn't quite as easy.

If the application is trusted with screen contents in general, it can be 
started by the compositor with a pre-prepared wl_client (with WAYLAND_SOCKET 
and socketpair()) which is associated with permissions. Sadly this fails hard 
with the multi-connection approach, since only the first client will be 
recognised.

Another approach I want to try out (but haven't gotten around to yet, due to 
lack of immediate usecase) would be to use dedicated WAYLAND_DISPLAY sockets 
for some applications (protected by container mountpoints) and have
permissions bound to all clients accepted over that socket.


The socket/client based approach has the advantage, that we can use the 
wl_display_set_global_filter to prevent clients binding to privileged 
protocols in the first place. On the other hand, sending the query on demand 
is quite a bit more flexible, but can result in odd cases, where a client 
creates an object (which can't fail) but the compositor doesn't give 
permissions to it.

> 
> > > +    <request name="create_virtual_keyboard">
> > > +      <description summary="Create a new virtual keyboard">
> > > +        Creates a new virtual keyboard associated to a seat.
> > > +
> > > +        If an untrusted client issues this request, it should receive the
> > > +        unauthorized error in return.
> > > +      </description>
> > > +      <arg name="seat" type="object" interface="wl_seat"/>
> > > +      <arg name="id" type="new_id" interface="zwp_virtual_keyboard_v1"/>
> > > +    </request>
> > > +
> > > +    <request name="destroy" type="destructor">
> > > +      <description summary="destroy the virtual keyboard manager object"/>  
> > 
> > This description should probably specify what happens to keyboards created with
> > the manager when the manager is destroyed. A common practice is not to destroy
> > objects when the manager is destroyed, so that one can easily create the
> > manager, use the factory request, and immediately destroy the manager (see e.g.
> > wl_subcompositor).
> 
> This is an oversight and will be fixed.
> > 
> > > +    </request>
> > > +  </interface>
> > > +</protocol>
> > > --
> > > 2.14.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
> 



> _______________________________________________
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
On June 22, 2018 7:00 PM, Dorota Czaplejewicz <dorota.czaplejewicz@puri.sm> wrote:
> On Fri, 22 Jun 2018 12:36:16 -0400
> Simon Ser <contact@emersion.fr> wrote:
>
> > On June 22, 2018 4:20 PM, Dorota Czaplejewicz <dorota.czaplejewicz@puri.sm> wrote:
> > > Provides the ability to emulate keyboards by applications. Complementary to input-method protocol.
> > >
> > > The interface is a mirror copy of wl_keyboard, with removed serials, and added seat binding.
> > > ---
> > > Hi,
> > >
> > > this patch is another improvement to the previously sent virtual keyboard protocol. Changes compared to v2:
> > >
> > > - readded enum names
> > > - removed suggestion to listen to modifiers
> > > - clarified untrusted client behaviour
> > > - added destructor on virtual_keyboard_manager
> > > - fixed text wrapping
> > >
> > > First off, the new version of wayland-scanner looks up enums found in other xml files, so the textual reference are gone and actual enum values are used.
> > >
> > > Secondly, there was a suggestion that if the client wants to know global modifiers, it should listen to wl_keyboard.modifiers. In reality, this can only be done when the client has keyboard focus. I decided to remove this suggestion.
> > >
> > > Clarified that create_virtual_keyboard must error out with unauthorized when an untrusted client connects. That doesn't preclude making the whole virtual_keyboard_manager interface unavailable when the client is determined as untrusted ahead of time.
> > >
> > > Lastly, added a missing destructor as per Simon Ser's suggestion.
> > >
> > > I hope that we're getting closer to perfect with this revision! As usual, feedback is welcome.
> > >
> > > Cheers,
> > > Dorota Czaplejewicz
> >
> > Hi,
> >
> > Thanks for this updated version. A few comments below.
> >
> ---- snip ----
> > > +
> > > +  <interface name="zwp_virtual_keyboard_manager_v1" version="1">
> > > +    <description summary="virtual keyboard manager">
> > > +      A virtual keyboard manager allows an application to provide keyboard
> > > +      input events as if they came from a physical keyboard.
> > > +
> > > +      If the compositor enables a keyboard to perform arbitrary actions, it
> > > +      should prevent untrusted clients from using this interface.
> > > +    </description>
> > > +
> > > +    <enum name="error">
> > > +      <entry name="unauthorized" value="0"
> > > +        summary="client not authorized to use the interface"/>
> > > +    </enum>
> >
> > This is more of a general metaphorical question than anything else: I wonder how
> > we should handle unauthorized clients, and if adding an error for them is a good
> > idea. Generally errors are meant for protocol violations: it's clear from the
> > protocol spec that e.g. sending a request with a negative value will trigger a
> > protocol error. Also, errors are unrecoverable, they abort the whole client
> > (though this is being worked on).
> >
> > So here we use an error, and the conditions in which the error is sent are
> > implementation-defined. I wonder if there's a better way to handle this
> > situation?
> >
> Speaking from a position of someone without a lot of Wayland experience: are
> Wayland errors meant to be recoverable by a client? It's obvious that if an
> application doing its primary task and then trying to use a restricted
> protocol as a secondary action crashes, that's undesireable.

Wayland protocol errors are meant to be fatal. They're meant to signal that the
client is doing something really wrong, against the protocol specification. They
indicate a bug in the client. As ongy said, they currently abort() the whole
client.

> As a more concrete example, an automation application may use e.g. DBus API to
> automate tasks, and display a window to control it. It may request a virtual
> keyboard to extend its possibilities, but it shouldn't suddenly stop working
> if it's refused by the compositor.

That's right, and that's a reason why I'd personally prefer not to use protocol
errors for security errors.

> That brings me to the question: should applications using restricted protocols
> create additional connections which may be broken and recovered from
> individually or should they rather use one connection? If the latter is
> required for some use cases, then authorization and probing/graceful rejection
> should be specified inside protocols. Unfortunately, I'm not sure where to
> look for examples. Perhaps chat applications where screen sharing may be
> decided ad hoc check the marks?

Additional connections may not help for now because libwayland just aborts on
protocol errors (though this might change in the future). Also, using multiple
connections makes it very difficult to share objects (e.g. surfaces) between
the privileged connection and the unprivileged one.

Again as ongy said, the compositor can decide to ask the user for permission
either when the client binds or when it sends a particular request. This is
what GNOME does when a client tries to use the pointer-constraints protocol for
instance: the constraint isn't activated until the user accepts. This works
relatively well for protocols allowing the compositor to give feedback to the
client. Another example is our new export-dmabuf protocol [1], it has a
"cancel" event which allows the compositor to say "I don't want you to capture
this output".

This gets more complicated for protocols like yours which don't have such
feedback events. Since binding to an interface or sending a request that creates
a new object can never fail, the client has no way to know if the compositor
refuses his requests or not. If you're asking the user for permission when the
client binds to the global, it gets even more complicated, since you still need
to allow the client to use the global to create objects, but you want these
objects to be inert (you want to ignore all requests on `wp_virtual_keyboard`).
And when the user accepts, you need to iterate over all these objects, and make
them non-inert. What if the objects themselves have requests to create yet other
objects?

However, access to your protocol should be restricted to a few clients. That
makes it viable to require users to update their compositor config to allow
a client to use the interface. To paraphrase ongy again, it's possible to
create a Wayland socket for a particular client and add privileged globals to
this socket only. That way the global is invisible to regular clients but is
visible to your e.g. IME. This would allow a Android-like UI for instance, where
you choose your virtual keyboard in the system settings.

[1]: https://github.com/swaywm/wlr-protocols/blob/master/unstable/wlr-export-dmabuf-unstable-v1.xml
On June 24, 2018 12:24 PM, Simon Ser <contact@emersion.fr> wrote:
> On June 22, 2018 7:00 PM, Dorota Czaplejewicz <dorota.czaplejewicz@puri.sm> wrote:
> > On Fri, 22 Jun 2018 12:36:16 -0400
> > Simon Ser <contact@emersion.fr> wrote:
> >
> > > On June 22, 2018 4:20 PM, Dorota Czaplejewicz <dorota.czaplejewicz@puri.sm> wrote:
> > > > Provides the ability to emulate keyboards by applications. Complementary to input-method protocol.
> > > >
> > > > The interface is a mirror copy of wl_keyboard, with removed serials, and added seat binding.
> > > > ---
> > > > Hi,
> > > >
> > > > this patch is another improvement to the previously sent virtual keyboard protocol. Changes compared to v2:
> > > >
> > > > - readded enum names
> > > > - removed suggestion to listen to modifiers
> > > > - clarified untrusted client behaviour
> > > > - added destructor on virtual_keyboard_manager
> > > > - fixed text wrapping
> > > >
> > > > First off, the new version of wayland-scanner looks up enums found in other xml files, so the textual reference are gone and actual enum values are used.
> > > >
> > > > Secondly, there was a suggestion that if the client wants to know global modifiers, it should listen to wl_keyboard.modifiers. In reality, this can only be done when the client has keyboard focus. I decided to remove this suggestion.
> > > >
> > > > Clarified that create_virtual_keyboard must error out with unauthorized when an untrusted client connects. That doesn't preclude making the whole virtual_keyboard_manager interface unavailable when the client is determined as untrusted ahead of time.
> > > >
> > > > Lastly, added a missing destructor as per Simon Ser's suggestion.
> > > >
> > > > I hope that we're getting closer to perfect with this revision! As usual, feedback is welcome.
> > > >
> > > > Cheers,
> > > > Dorota Czaplejewicz
> > >
> > > Hi,
> > >
> > > Thanks for this updated version. A few comments below.
> > >
> > ---- snip ----
> > > > +
> > > > +  <interface name="zwp_virtual_keyboard_manager_v1" version="1">
> > > > +    <description summary="virtual keyboard manager">
> > > > +      A virtual keyboard manager allows an application to provide keyboard
> > > > +      input events as if they came from a physical keyboard.
> > > > +
> > > > +      If the compositor enables a keyboard to perform arbitrary actions, it
> > > > +      should prevent untrusted clients from using this interface.
> > > > +    </description>
> > > > +
> > > > +    <enum name="error">
> > > > +      <entry name="unauthorized" value="0"
> > > > +        summary="client not authorized to use the interface"/>
> > > > +    </enum>
> > >
> > > This is more of a general metaphorical question than anything else: I wonder how
> > > we should handle unauthorized clients, and if adding an error for them is a good
> > > idea. Generally errors are meant for protocol violations: it's clear from the
> > > protocol spec that e.g. sending a request with a negative value will trigger a
> > > protocol error. Also, errors are unrecoverable, they abort the whole client
> > > (though this is being worked on).
> > >
> > > So here we use an error, and the conditions in which the error is sent are
> > > implementation-defined. I wonder if there's a better way to handle this
> > > situation?
> > >
> > Speaking from a position of someone without a lot of Wayland experience: are
> > Wayland errors meant to be recoverable by a client? It's obvious that if an
> > application doing its primary task and then trying to use a restricted
> > protocol as a secondary action crashes, that's undesireable.
>
> Wayland protocol errors are meant to be fatal. They're meant to signal that the
> client is doing something really wrong, against the protocol specification. They
> indicate a bug in the client. As ongy said, they currently abort() the whole
> client.

It seems we've been misunderstanding this point. They don't abort the client,
they just kill the connection. After that, functions called with the same
wl_display will start returning errors.

There are functions to get details about the protocol error
(wl_display_get_error and wl_display_get_protocol_error) but those are more for
better logging than recovery.

Thanks Pekka for pointing that out!

> > As a more concrete example, an automation application may use e.g. DBus API to
> > automate tasks, and display a window to control it. It may request a virtual
> > keyboard to extend its possibilities, but it shouldn't suddenly stop working
> > if it's refused by the compositor.
>
> That's right, and that's a reason why I'd personally prefer not to use protocol
> errors for security errors.
>
> > That brings me to the question: should applications using restricted protocols
> > create additional connections which may be broken and recovered from
> > individually or should they rather use one connection? If the latter is
> > required for some use cases, then authorization and probing/graceful rejection
> > should be specified inside protocols. Unfortunately, I'm not sure where to
> > look for examples. Perhaps chat applications where screen sharing may be
> > decided ad hoc check the marks?
>
> Additional connections may not help for now because libwayland just aborts on
> protocol errors (though this might change in the future). Also, using multiple
> connections makes it very difficult to share objects (e.g. surfaces) between
> the privileged connection and the unprivileged one.
>
> Again as ongy said, the compositor can decide to ask the user for permission
> either when the client binds or when it sends a particular request. This is
> what GNOME does when a client tries to use the pointer-constraints protocol for
> instance: the constraint isn't activated until the user accepts. This works
> relatively well for protocols allowing the compositor to give feedback to the
> client. Another example is our new export-dmabuf protocol [1], it has a
> "cancel" event which allows the compositor to say "I don't want you to capture
> this output".
>
> This gets more complicated for protocols like yours which don't have such
> feedback events. Since binding to an interface or sending a request that creates
> a new object can never fail, the client has no way to know if the compositor
> refuses his requests or not. If you're asking the user for permission when the
> client binds to the global, it gets even more complicated, since you still need
> to allow the client to use the global to create objects, but you want these
> objects to be inert (you want to ignore all requests on `wp_virtual_keyboard`).
> And when the user accepts, you need to iterate over all these objects, and make
> them non-inert. What if the objects themselves have requests to create yet other
> objects?
>
> However, access to your protocol should be restricted to a few clients. That
> makes it viable to require users to update their compositor config to allow
> a client to use the interface. To paraphrase ongy again, it's possible to
> create a Wayland socket for a particular client and add privileged globals to
> this socket only. That way the global is invisible to regular clients but is
> visible to your e.g. IME. This would allow a Android-like UI for instance, where
> you choose your virtual keyboard in the system settings.
>
> [1]: https://github.com/swaywm/wlr-protocols/blob/master/unstable/wlr-export-dmabuf-unstable-v1.xml
> _______________________________________________
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
On Fri, 22 Jun 2018 20:00:00 +0200
Dorota Czaplejewicz <dorota.czaplejewicz@puri.sm> wrote:

> On Fri, 22 Jun 2018 12:36:16 -0400
> Simon Ser <contact@emersion.fr> wrote:
> 
> > On June 22, 2018 4:20 PM, Dorota Czaplejewicz <dorota.czaplejewicz@puri.sm> wrote:  
> > > Provides the ability to emulate keyboards by applications. Complementary to input-method protocol.
> > >
> > > The interface is a mirror copy of wl_keyboard, with removed serials, and added seat binding.
> > > ---
> > > Hi,
> > >
> > > this patch is another improvement to the previously sent virtual
> > > keyboard protocol. Changes compared to v2:
> > >
> > > - readded enum names
> > > - removed suggestion to listen to modifiers
> > > - clarified untrusted client behaviour
> > > - added destructor on virtual_keyboard_manager
> > > - fixed text wrapping
> > >
> > > First off, the new version of wayland-scanner looks up enums
> > > found in other xml files, so the textual reference are gone and
> > > actual enum values are used.

Hi,

that is not exactly true. Now wayland-scanner just skips checking any
referenced enums that are not defined in the current XML file. It does
not make lookups in other XML files.


> ---- snip ----
> > > +
> > > +  <interface name="zwp_virtual_keyboard_manager_v1" version="1">
> > > +    <description summary="virtual keyboard manager">
> > > +      A virtual keyboard manager allows an application to
> > > provide keyboard
> > > +      input events as if they came from a physical keyboard.
> > > +
> > > +      If the compositor enables a keyboard to perform arbitrary
> > > actions, it
> > > +      should prevent untrusted clients from using this interface.
> > > +    </description>
> > > +
> > > +    <enum name="error">
> > > +      <entry name="unauthorized" value="0"
> > > +        summary="client not authorized to use the interface"/>
> > > +    </enum>    
> > 
> > This is more of a general metaphorical question than anything else:
> > I wonder how we should handle unauthorized clients, and if adding
> > an error for them is a good idea. Generally errors are meant for
> > protocol violations: it's clear from the protocol spec that e.g.
> > sending a request with a negative value will trigger a protocol
> > error. Also, errors are unrecoverable, they abort the whole client
> > (though this is being worked on).

Errors will remain unrecoverable because they will always cause a
disconnection.

What is being worked on is avoiding the abort in the few yet unhandled
loew-level failure cases like implicit flush, so that the app can at
last clean up and save what it was doing, or maybe even reconnect if it
really wants to.

Protocol errors should not cause an abort() already.

> > 
> > So here we use an error, and the conditions in which the error is
> > sent are implementation-defined. I wonder if there's a better way
> > to handle this situation?
> >   
> Speaking from a position of someone without a lot of Wayland
> experience: are Wayland errors meant to be recoverable by a client?

No, protocol errors are always fatal to the Wayland connection, which
means the client gets disconnected and "destroyed". The application
process may continue though and even attemp to reconnect, but that
should not be relied upon in protocol design. So from protocol design
perspective, protocol errors are always unrecoverable.

The idea is that when a protocol error happens, it may invalidate
further requests made by the client and it almost always implies that
the server and the client no longer agree on the protocol state. Hence,
keeping the connection would be an equally good idea to continuing a
process after receiving a SIGSEGV.

> It's obvious that if an application doing its primary task and then
> trying to use a restricted protocol as a secondary action crashes,
> that's undesireable.
> 
> As a more concrete example, an automation application may use e.g.
> DBus API to automate tasks, and display a window to control it. It
> may request a virtual keyboard to extend its possibilities, but it
> shouldn't suddenly stop working if it's refused by the compositor.

Indeed, one cannot use protocol errors for graceful rejection.

> That brings me to the question: should applications using restricted
> protocols create additional connections which may be broken and
> recovered from individually or should they rather use one connection?
> If the latter is required for some use cases, then authorization and
> probing/graceful rejection should be specified inside protocols.
> Unfortunately, I'm not sure where to look for examples. Perhaps chat
> applications where screen sharing may be decided ad hoc check the
> marks?

The intention is for one application to use one connection, but nothing
forbids using multiple connections. Multiple connection are usually
impractical, though. If the client was connected with WAYLAND_SOCKET (as
opposed to WAYLAND_DISPLAY), the client probably won't have the means
to reconnect or open additional connections.

Wayland's isolation between clients means that one cannot refer to a
Wayland object of one connection in another connection. It does not
matter if it's the same process or thread, if it's a separate
connection it is isolated.

There is currently no agreed mechanism to see if an interface is
allowed for the client or if it is even a privileged interface.

Giulio Camuffo at least had a proposal for advertising privileged
interfaces with graceful deny. Authenticating clients OTOH continues to
be an unsolved issue in general, I believe.


Thanks,
pq