interfaces: misc updates for network-control, firewall-control, unity7 and default policy

[jdstrand]

• interfaces/network-control,firewall-control: allow reading module parameters (LP: #1662662)
• interfaces/unity7: allow registering interest in media keys (LP: #1663565)
• interfaces: allow /run/user/..., not /dev/user/...

[zyga]

Question and LGTM. Is opening netlink safe? I assume you know what you are doing and that the kernel does smart things (e.g. provides some access control) but can a snap using netlink and running as root do some "damage" (or just be nasty in some way?)

[jdstrand]

@zyga - this was covered in the comment:

Unfortunately 'network netlink raw' isn't finely mediated, but since this only
happens on X, adding this rule to the transitional unity7 and x11 interfaces
should be fine. Justification: DAC offers some protections for netlink raw on
the system, most applications using X/unity7 run as non-root, X is insecure so
blocking this particular rule needed by Qt on X is arguably specious, and
breaking out 'netlink raw' into an interface isn't very interesting because it
is entirely too general.

I don't like this rule at all but it is difficult to see an alternative. Mir doesn't need it, and it is required by all ubuntu-ui-toolkit apps to find the keyboard and mouse on systems with X. We could say 'no' to this rule, but that would break app development where people want to have their app work on unity7/x11 and unity8/mir. I found it hard to justify saying 'no' to this rule when X/unity7 are auto-connected already and have gaping security holes.

The only alternative I see is to tell the SDK team to change the toolkit to not talk to udev directly and instead talk to a trusted DBus service that can inform the app where the keyboard and mouse are. Then unity7 is adjusted to allow talking to that service. @tsdgeos - can you comment?

@tyhicks - thoughts?

UPDATE: to answer your question, 'yes, allowing root to use netlink raw would allow root to do bad things'. I forgot to mention that because specifying unity7 or x11 with 'daemon' isn't expected to work anyway, I was going to add a check to the review tools that would flag for human review when doing that. This should mitigate this somewhat.

It did occur to me that I could investigate how Qt is calling socket and use seccomp arg filtering in the x11 and unity7 interfaces to have:

socket AF_NETLINK SOCK_RAW NETLINK_<something for just this access>

I'll look into that.

[tyhicks]

@jdstrand until we know what netlink family (see the netlink(7) man page) is required, it is hard to understand what kind of attack surface full netlink access provides. I think the proposed seccomp rule can greatly narrow the scope and is something that we should pursue.

Once we know the netlink family used, we can trace the kernel code for that family to get a better idea about how dangerous this is.

[tsdgeos]

FWIW as I commented in https://bugs.launchpad.net/snapd/+bug/1663221 ubuntu-ui-toolkit is not talking to udev directly but via QInputInfoManager (in QtSystems).

About if ubuntu-ui-toolkit would be able to stop using QInputInfoManager and using some other dbus service, i don't know, i'm not really part of the ubuntu-ui-toolkit developers (just do some minor stuff), you'd have to ask them, but given that dbus service doesn't exist (as far as i know) seems a theoretical discussion and not an actual fix for this problem.

About how dangeroues this is, maybe it can be restricted to allow only protocol NETLINK_KOBJECT_UEVENT ? Not sure if that helps at all or not.

[jdstrand]

I removed the netlink raw rule from this PR as the merge window is closing and if implemented, we'll need seccomp arg filtering to implement it and that is blocked on #2810.

[zyga]

LGTM

[jdstrand]

@mvo5 - can you consider this for 2.23?

[mvo5]

❤️

[jdstrand]

Thanks for the review!