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

[Simon Ser]

On June 22, 2018 7:00 PM, Dorota Czaplejewicz <dorota.czaplejewicz at puri.sm> wrote:
> On Fri, 22 Jun 2018 12:36:16 -0400
> Simon Ser <contact at emersion.fr> wrote:
>
> > On June 22, 2018 4:20 PM, Dorota Czaplejewicz <dorota.czaplejewicz at 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