Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make accessibility work in the sandbox #79

Closed
alexlarsson opened this issue Jun 3, 2016 · 15 comments
Closed

Make accessibility work in the sandbox #79

alexlarsson opened this issue Jun 3, 2016 · 15 comments

Comments

@alexlarsson
Copy link
Member

Accessibility works by having its own dbus bus, which currently doesn't work at all in the sandbox. However, just granting access to it is extremely unsafe as it can basically control anything. This needs serious research into how its going to work.

@matthiasclasen
Copy link
Collaborator

We talked a bit about this at the GTK+ hackfest. I believe the current plan is to a) patch at-spi-atk to look for the a11y bus socked in XDG_RUNTIME_DIR, b) expose the socked the same way we do for the session bus and c) give apps limited access to the bus that is sufficient to make orca work

@alexlarsson
Copy link
Member Author

It turns out however that apps need to be able to send messages to some services on the a11y bus, which is complicated due to security reasons.

@webczat
Copy link

webczat commented Oct 12, 2016

many developers do not care about accessibility, so it would be useful to grant such access by default, as there is a chance most of them just will ignore that

@hadess
Copy link
Contributor

hadess commented Oct 12, 2016

The problem isn't simply about granting access, but about the technical problems of having a 2nd, separate, D-Bus daemon running, and the fact that all sandboxed applications would share it.

@miksuh
Copy link

miksuh commented Jul 14, 2017

It is extremely important that BOTH Orca AND brltty will be made to work with flatpak. I am blind and it is important for me that Orca screen reader, speech synthesizers and brltty do work in the future too. brltty is important too because it is needed if you want to use braille display. Orca uses brltty.

@alexlarsson alexlarsson added this to the 1.0 Blockers milestone Aug 14, 2017
alexlarsson added a commit to alexlarsson/flatpak that referenced this issue Sep 1, 2017
This creates a dbus proxy for the a11y bus and sets AT_SPI_BUS_ADDRESS in the
environment to the filtered bus.

The app is only allowed to talk to org.a11y.atspi.Registry on the bus.

This requires a patch to at-spi2-core to read the address from
AT_SPI_BUS_ADDRESS:
 https://github.com/flatpak/freedesktop-sdk-images/blob/1.6/at-spi2-core-address-env-var.patch

This is work-in-progress, see discussion in flatpak#79
@alexlarsson
Copy link
Member Author

Ok, I started looking into this. Here is how a11y works from high level perspective:

There is a a11y bus, which is separate from the session bus, but you can get the address to it from the session bus, or from a X11 root-window property.

Each a11y client connects to this bus, as does the at-spi registry daemon. Normally, an a11y client talks directly only to to the registry daemon, and additionally it broadcasts signals when its properties change. Additionally it exposes on the bus a lot of objects that anyone can call, which do things like expose all the widgets and let you see and modify their state. As a special case, for socket/plug embedding it seems like the a11y client might call directly to the parent/child.

A11y services, like orca, connect to the same bus, and then asks the registry about what is happening, and then calls directly into the clients to read and modify them.

I created a brach with some start of a11 support here: https://github.com/alexlarsson/flatpak/tree/wip/a11y

It works by creating a dbus proxy for the a11y bus, and exporting the address via the AT_SPI_BUS_ADDRESS env var (requires a patch to at-spi2-core to pick up the env var). On this bus the app is only allowed to talk to the registry, send broadcast signals and reply to messages sent directly to it. It can't directly talk to others, or receive signals broadcast from them.This seems to be enough to make at least orca work.

However, there are at least a few issues that prevent this from being truly safe:

  • The "real" a11y bus is an abstract socket, rather than a regular socket. This means any sandbox that has network access can talk to it. This is easily fixed by just changing the address type though.
  • There are lots of interfaces in the registry that are not normally used by standard clients, but instead by a11y services (such as orca). I'm in no way an expert on this, but it seems that there are some clearly dangerous calls such as DeviceEventController.RegisterKeystrokeListener and DeviceEventController.GenerateKeyboardEvent, which if allowed by a sandboxed client could be used to escape the sandbox and attack other clients. There could also be other operations that are dangerous, we need some feedback from the people who really know how at-spi works.

I can think of two ways to make this safe. Either modify the registry itself to only allow certain operations if the client is sandboxed. Alternatively, we could make the dbus proxy filter more complicated (although this risks slowing it down, and could cause issues when we later try to move the dbus filtering from the proxy into dbus itself).

@mgorse, @joanmarie could you help us figure out what subset of the registry would be "safe" for a sandboxed client to use?

@smcv what are the possible extensions of the new dbus container rules we could make that would let us filter things with a higher degree of accuracy? For instance, could we add filters on interface name? method name? object path?

@smcv
Copy link
Collaborator

smcv commented Sep 1, 2017

The "real" a11y bus is an abstract socket, rather than a regular socket. This means any sandbox that has network access can talk to it. This is easily fixed by just changing the address type though.

This needs fixing anyway. The easiest way would be for AT-SPI to detect whether XDG_RUNTIME_DIR is set to /run/user/$uid, and if so, use unix:path=/run/user/$uid/at-spi-bus-X$display or some such, where $uid is the numeric uid and $display is the numeric X display (so typically 0 for DISPLAY=:0). Or something along those lines.

Alternatively, dbus 1.11.x has a new listenable address family unix:dir=/tmp, which is just like unix:tmpdir=/tmp, but always yields a filesystem socket (unix:path=/tmp/dbus-$random), never a Linux abstract socket (unix:abstract=/tmp/dbus-$random).

@smcv
Copy link
Collaborator

smcv commented Sep 1, 2017

what are the possible extensions of the new dbus container rules we could make that would let us filter things with a higher degree of accuracy? For instance, could we add filters on interface name? method name? object path?

https://bugs.freedesktop.org/show_bug.cgi?id=100344 is the metabug with the general design, and https://bugs.freedesktop.org/show_bug.cgi?id=101902 is the feature request for filtering.

Interface name is fine, we probably want that anyway.

Object path is fine, desrt wants that anyway for dconf. Object path subtrees (ask for /foo and it matches /foo, /foo/bar, /foo/bar/baz but not /foolish) are also fine, we probably want those already.

I would prefer not to support filtering by member (method, signal) name if we can help it, because interfaces and object paths seem a much more reasonable granularity to work at.

@smcv
Copy link
Collaborator

smcv commented Sep 1, 2017

There is a a11y bus, which is separate from the session bus, but you can get the address to it from the session bus, or from a X11 root-window property.

a11y people: Please clarify the multiplicity of this bus?

If uid 1000 logs in on seat0 with X11 display :1 and on seat1 with X11 display :2, how many a11y buses do they have? (I believe the answer is: two, one per X11 display)

If uid 1001 on X11 display :3 uses pkexec or sux or something to run a GUI application on :3 as uid 0 (I know this is inadvisable and deprecated, but it happens), how many a11y buses are there? (I believe the answer is: there is one a11y bus, and both uid 1001 and uid 0 are connected to it)

@alexlarsson
Copy link
Member Author

@smcv You find the a11y normally via either a X11 root property, or via a service on the session bus. So, there is one per display if you're using X11, and one per dbus session if you're using wayland i guess...

@alexlarsson
Copy link
Member Author

@smcv So, there are a unfortunately a bunch of methods we'd like to call intermixed with ones that are dangerous. For instance, we need to call org.a11y.atspi.DeviceEventController.GetKeystrokeListeners() to know if there is anyone listening for these, and if so, we should send them. But we don't want the sandboxed apps to call e.g. org.a11y.atspi.DeviceEventController.GenerateKeyboardEvent.

In the proxy i'm probably going to be filtering this by method. But, long term i think a better approach is to filter by interface only, and splitting out the safe methods into a separate interface.

Another problem is that atk calls the above methods, which give us list of unique names on the bus. And atk then listens for NameOwnerChanges on these so that we can track the lifetime of them. This is so we can stop sending signals when there is nobody listening to some specific type of event anymore.

But, we currently filter out all the NameOwnerChanges you can't see, which makes this not work.

@alexlarsson
Copy link
Member Author

New version has support in the dbus-proxy for per-iface+method+path filtering, and a very limited set of allowed calls:

--filter=org.a11y.atspi.Registry=org.a11y.atspi.Socket.Embed@/org/a11y/atspi/accessible/root
--filter=org.a11y.atspi.Registry=org.a11y.atspi.Socket.Unembed@/org/a11y/atspi/accessible/root
--filter=org.a11y.atspi.Registry=org.a11y.atspi.Registry.GetRegisteredEvents@/org/a11y/atspi/registry
--filter=org.a11y.atspi.Registry=org.a11y.atspi.DeviceEventController.GetKeystrokeListeners@/org/a11y/atspi/registry/deviceeventcontroller
--filter=org.a11y.atspi.Registry=org.a11y.atspi.DeviceEventController.GetDeviceEventListeners@/org/a11y/atspi/registry/deviceeventcontroller
--filter=org.a11y.atspi.Registry=org.a11y.atspi.DeviceEventController.NotifyListenersSync@/org/a11y/atspi/registry/deviceeventcontroller
--filter=org.a11y.atspi.Registry=org.a11y.atspi.DeviceEventController.NotifyListenersAsync@/org/a11y/atspi/registry/deviceeventcontroller

Additionally it runs the a11y bus in a "sloppy names" mode where all clients are allowed to see NameOwnerChanged events for all unique names, so that they can track lifetimes of a11y services.

I believe these are safe, and enough. A trivial test of orca against gtk3-demo seems to work. But, it would be nice to have some review from people involved in a11y.

@smcv
Copy link
Collaborator

smcv commented Sep 1, 2017

New version has support in the dbus-proxy for per-iface+method+path filtering

I suppose in dbus we could have a flag INTERFACE_IS_REALLY_MEMBER and set the "interface" in the data structure to be inter.face.member instead of inter.face. Then we wouldn't need a separate field for the member name. It's a bit ugly, but if we need it, we need it.

all clients are allowed to see NameOwnerChanged events for all unique names, so that they can track lifetimes of a11y services

This seems like a viable flag to add.

@miksuh
Copy link

miksuh commented Sep 1, 2017 via email

gnomesysadmins pushed a commit to GNOME/at-spi2-core that referenced this issue Sep 4, 2017
This will be used for flatpak to set a custom bus (which is really
the bus proxy). It can be used for testing purposes too.

For more details, see flatpak/flatpak#79

https://bugzilla.gnome.org/show_bug.cgi?id=787126
@hadess
Copy link
Contributor

hadess commented Sep 6, 2017

  1. Use Orca screen reader which speaks the content on screen

Orca, and brltty should both work as they run outside the sandbox, in the "OS" part of the system. If brltty works in Wayland, then there's no reason for it not to work once the application is sandboxed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants