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

interfaces: allow access to new at-spi socket location in desktop-legacy #11208

Merged
merged 2 commits into from Jan 14, 2022

Conversation

jhenstridge
Copy link
Contributor

In at-spi 2.40, the socket location for the accessibility bus was changed from an abstract namespace socket to a regular unix socket, with the aim of making it easier to sandbox: without the help of a kernel MAC system like AppArmor, you've really only got a choice between allowing access to all abstract sockets or denying access to all sockets. In contrast, access to regular unix sockets can be controlled via mount namespace manipulation or file permissions.

Unfortunately that release changed to using a randomised socket name under /tmp, which could not reasonably be supported with snapd's private /tmp. That issue has been fixed upstream in at-spi, presumably to be included in 2.44:

https://gitlab.gnome.org/GNOME/at-spi2-core/-/issues/43

Now the socket will be named something matching $XDG_RUNTIME_DIR/at-spi/bus* (where the exact name will depend on the value of $DISPLAY). This PR updates the desktop-legacy AppArmor rules to allow access to this socket.

@codecov-commenter
Copy link

Codecov Report

Merging #11208 (23c9232) into master (7fcec7b) will increase coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #11208   +/-   ##
=======================================
  Coverage   78.36%   78.36%           
=======================================
  Files         922      922           
  Lines      105128   105128           
=======================================
+ Hits        82383    82384    +1     
+ Misses      17613    17612    -1     
  Partials     5132     5132           
Flag Coverage Δ
unittests 78.36% <ø> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
interfaces/builtin/desktop_legacy.go 100.00% <ø> (ø)
cmd/snap/cmd_debug_state.go 70.64% <0.00%> (+0.45%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7fcec7b...23c9232. Read the comment docs.

Copy link
Contributor

@mvo5 mvo5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, this looks fine

@mvo5 mvo5 added the Needs security review Can only be merged once security gave a :+1: label Jan 11, 2022
@mvo5 mvo5 added this to the 2.54 milestone Jan 11, 2022
Copy link
Contributor

@mardy mardy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Copy link
Collaborator

@alexmurray alexmurray left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - thanks.

@alexmurray alexmurray removed the Needs security review Can only be merged once security gave a :+1: label Jan 12, 2022
@jhenstridge jhenstridge merged commit 66806ed into snapcore:master Jan 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants