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

[Simon Ser]

On June 24, 2018 12:24 PM, Simon Ser <contact at emersion.fr> wrote:
> 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.

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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel