interfaces: mediate netlink sockets via seccomp #2969

Merged
merged 50 commits into from May 3, 2017

Conversation

Projects
None yet
7 participants
Contributor

jdstrand commented Mar 1, 2017

AppArmor only has coarse-grained mediation for the type for netlink (man 7 netlink) sockets but it is useful to be able to mediate by the netlink_family as well. This branch achieves that by introducing support for socket AF_NETLINK - NETLINK_* seccomp rules by:

  • adding all the missing AF_* (and PF_*) mappings (see include/linux/socket.h from the kernel)
  • adding NETLINK_* family mappings
  • adding socket AF_... for everything except AF_NETLINK in the default template. This allows us to continue to rely on AppArmor for mediation of non-netlink sockets
  • adjusting account-control to use NETLINK_AUDIT (from libaudit1)
  • adjusting bluez to use NETLINK_KOBJECT_UEVENT for udev (also fix a bug in the apparmor policy to allow 'network netlink raw')
  • adjusting firewall-control to use NETLINK_ROUTE and NETLINK_* families related to firewalling
  • adjusting fwupd to use NETLINK_KOBJECT_UEVENT for udev
  • adjusting mir to use NETLINK_KOBJECT_UEVENT for udev
  • adjusting modem-manager to use NETLINK_KOBJECT_UEVENT for udev (also fix a bug in the apparmor policy to allow 'network netlink raw')
  • adjusting network and network-bind to use NETWORK_ROUTE (for monitoring address changes, etc)
  • adjusting network-control to use NETLINK_* families related to networking
  • adjusting network-manager to use all netlink families (which is what is allowed by the current apparmor policy. Remove redundant netlink rules from apparmor policy)
  • adjusting network-observe to use NETWORK_ROUTE (for arp) and NETWORK_INET_DIAG (for ss) and add 'ss' binary to policy
  • adjusting ofono to use NETLINK_ROUTE and NETLINK_KOBJECT_UEVENT for udev
  • adjusting pulseaudio to use NETLINK_KOBJECT_UEVENT for udev
  • adjusting udisks2 to use NETLINK_KOBJECT_UEVENT for udev
  • adjusting upower-observe to use NETLINK_KOBJECT_UEVENT for udev
  • adjusting docker, lxd, hardware-observe and network-observe to use NETLINK_GENERIC based on codesearch
  • adding the netlink-audit interface to support NETLINK_AUDIT
  • adding the netlink-connector interface to support NETLINK_CONNECTOR

We omit PF_* in the policy since those are synonymous with AF_*. We do not use 'socket !AF_NETLINK' in the default template because that doesn't work well with an interface that uses 'socket AF_NETLINK').

Importantly, network netlink AppArmor rules were not in the default policy or most of the interfaces and the changes to policy outside of the template are for permanent slot policy for slot implementations for those that had 'network netlink' rules (and modem-manager and bluez that were mistakenly missing them). We do allow NETLINK_ROUTE in network-bind but continue to disallow netlink rules there to avoid KILLing java servers which attempt it but otherwise fail gracefully.

Since there are applications in codesearch.debian.net known to use NETLINK_AUDIT and NETLINK_CONNECTOR, add two new interfaces for them.

Testing performed:

  • account-control: useradd (NETLINK_AUDIT) - DONE
  • bluez: bluez (NETLINK_KOBJECT_UEVENT) - DONE (it starts)
  • firewall-control: iptables: DONE (only for NETLINK_ROUTE)
  • fwupd: uefi-fw-tools - DONE (it starts)
  • mir: mir-libs --edge and mir-kiosk --edge - DONE (it starts)
  • network: wuzz - DONE
  • network-bind: minecraft-server-jdstrand - DONE
  • network-control: arp (NETLINK_ROUTE), ping - DONE
  • network-manager: nmcli - DONE
  • network-observe: arp (NETLINK_ROUTE), ping, ss (NETLINK_INET_DIAG) - DONE
  • ofono: ofono --edge - DONE (it starts)
  • pulseaudio: pulseaudio - sudo pulseaudio.pactl stat - DONE
  • udisks2: udisks2 - sudo udisks2.udisksctl info -b /dev/vda - DONE
  • upower-observe: upower --edge - DONE (it starts)
  • verified applications run with the new policy for all updated interfaces

While this PR is useful in and of itself because we can finely-mediate netlink sockets for our slot implementations, account-control, network-control and network-observe, this functionality is most important for adding NETLINK_KOBJECT_UEVENT for udev to x11/unity7 to support QtSystem applications on X. Since there is some conditional logic that is going to be introduced there, it will be in a separate PR.

jdstrand added some commits Feb 17, 2017

@jdstrand jdstrand closed this Mar 1, 2017

@jdstrand jdstrand changed the title from Netlink mediation to interfaces: mediate netlink sockets via seccomp Mar 1, 2017

Contributor

jdstrand commented Mar 2, 2017

Creating a PR for others to see (but closing it since it needs iterating).

Contributor

jdstrand commented Mar 2, 2017

@morphis - can you also take a look at this?

@jdstrand jdstrand reopened this Mar 2, 2017

jdstrand added some commits Mar 2, 2017

Contributor

jdstrand commented Mar 2, 2017

@tyhicks - can you also take a look at this? I'm wondering if I should create additional interfaces or even a 'netlink-socket' interface with 'socket AF_NETLINK' as an escape hatch since I observed that some uses of AF_NETLINK don't go through the LSM. Eg, java server running as root triggered a NETLINK_ROUTE seccomp denial but not an AppArmor 'network netlink' denial. I then saw net/netlink/af_netlink.c and thought this might be because the LSM hooks aren't triggered under certain circumstances (eg, netlink_sendmsg() if not root), but didn't chase down NETLINK_ROUTE.

I then went codesearched the various NETLINK_* families and sprinkled them in various places (eg, cadvisor is a docker tool and needs NETLINK_GENERIC, ibacm is an infiniband tool and needs NETLINK_RDMA, etc). Results:

  • NETLINK_AUDIT - used by libaudit1. in account-control
  • NETLINK_CONNECTOR - https://github.com/torvalds/linux/blob/master/Documentation/connector/connector.txt
  • NETLINK_CRYPTO - referenced but not used. Omitted from policy
  • NETLINK_DNRTMSG - in network-control
  • NETLINK_ECRYPTFS - referenced but not used. Omitted from policy
  • NETLINK_FIB_LOOKUP - in network-control
  • NETLINK_FIREWALL - in firewall-control
  • NETLINK_GENERIC - in docker, hardware-observe, lxd, and network-observe
  • NETLINK_INET_DIAG - in network-control and network-observe
  • NETLINK_IP6_FW - in firewall-control
  • NETLINK_ISCSI - network-control
  • NETLINK_KOBJECT_UEVENT - in slot implementations that use libudev/libgudev
  • NETLINK_NETFILTER - in firewall-control
  • NETLINK_NFLOG - in firewall-control
  • NETLINK_RDMA - network-control
  • NETLINK_ROUTE - in firewall-control, network, network-bind, network-observe, ofono
  • NETLINK_SCSITRANSPORT - referenced but not used. Omitted from policy
  • NETLINK_SELINUX - only used by libselinux. Omitted from policy
  • NETLINK_SOCK_DIAG - synonymous with NETLINK_INET_DIAG (see above)
  • NETLINK_USERSOCK - referenced but not used. Omitted from policy
  • NETLINK_XFRM - network-control

Looking at this, I think I should add 'netlink-audit' and 'netlink-connector' interfaces that allow socket AF_NETLINK - NETLINK_AUDIT and socket AF_NETLINK - NETLINK_CONNECTOR respectively.

Contributor

jdstrand commented Mar 2, 2017

The travis-ci issues are real. AF_MPLS, PF_MLPS, AF_IB and PF_IB don't exist on trusty.

jdstrand added some commits Mar 2, 2017

Contributor

jdstrand commented Mar 2, 2017

Per discussions with @tyhicks, added netlink-audit and netlink-connector.

Contributor

tyhicks commented Mar 2, 2017

Contributor

jdstrand commented Mar 3, 2017

"since I observed that some uses of AF_NETLINK don't go through the LSM. Eg, java server running as root triggered a NETLINK_ROUTE seccomp denial but not an AppArmor 'network netlink' denial"

@tyhicks and I looked at this more and it seems that what I observed was related to kernel rate limiting (even though I had it disabled). I think it is fine to keep the NETLINK_ROUTE in network-bind for the moment though. In fact, we have a note about intentionally not allowing 'network netlink dgram' in network-bind for java servers because they work fine without it. If we leave out the NETLINK_ROUTE seccomp rule, this previously known harmless denial will turn into a seccomp KILL. Once seccomp errno with logging is finished, we can remove this rule.

jdstrand added some commits Mar 3, 2017

Contributor

zyga commented Mar 4, 2017

Just FYI, this cannot land unless we run snap-confine from the core snap.

@niemeyer niemeyer added the Blocked label Mar 13, 2017

jdstrand and others added some commits Mar 29, 2017

Contributor

zyga commented Apr 4, 2017

This failed on networking/ssl:

# cd .; git clone https://go.googlesource.com/crypto /tmp/go/.cache/govendor/golang.org/x/crypto
Cloning into '/tmp/go/.cache/govendor/golang.org/x/crypto'...
error: RPC failed; curl 56 GnuTLS recv error (-110): The TLS connection was non-properly terminated.

@zyga zyga removed the Blocked label Apr 14, 2017

Contributor

zyga commented Apr 14, 2017

This is no longer blocked, we should re-introduce all the new features to snap-confine, that were removed when reexec was a problem.

Contributor

jdstrand commented Apr 17, 2017

It is my understanding that this is now unblocked, so removing that tag, but closing until I have a chance to review and retest everything.

@jdstrand jdstrand closed this Apr 17, 2017

Contributor

jdstrand commented Apr 18, 2017

Since I'm told that snap-confine now re-exec's as needed, reopening.

@jdstrand jdstrand reopened this Apr 18, 2017

jdstrand and others added some commits Apr 18, 2017

jdstrand added some commits May 3, 2017

zyga approved these changes May 3, 2017

Looks good, builds on Ubuntu, Debian and Fedora.

+1

+ sc_map_add(AF_NFC);
+ sc_map_add(PF_NFC);
+ sc_map_add(AF_VSOCK);
+ sc_map_add(PF_VSOCK);
@zyga

zyga May 3, 2017

Contributor

Wow, that's quite the list. Reading about them is pretty interesting in itself!

+ sc_map_add(NETLINK_ECRYPTFS);
+ sc_map_add(NETLINK_RDMA);
+ sc_map_add(NETLINK_CRYPTO);
+ sc_map_add(NETLINK_INET_DIAG); // synonymous with NETLINK_SOCK_DIAG
@zyga

zyga May 3, 2017

Contributor

This looks good! (cross checked with the header)

@chipaca chipaca merged commit 1abdf4b into snapcore:master May 3, 2017

7 checks passed

artful-amd64 autopkgtest finished (success)
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
xenial-amd64 autopkgtest finished (success)
Details
xenial-i386 autopkgtest finished (success)
Details
xenial-ppc64el autopkgtest finished (success)
Details
yakkety-amd64 autopkgtest finished (success)
Details
zesty-amd64 autopkgtest finished (success)
Details
Contributor

morphis commented May 3, 2017

Code looks fine and I am fine with landing this. We run a full weekly CI cycle against beta/edge core snaps so merging this early enough before the next release should give us confidence if this breaks something or not.

Contributor

jdstrand commented May 3, 2017

Thanks for all the reviews, comments and merge! :)

@jdstrand jdstrand deleted the jdstrand:netlink-mediation branch May 3, 2017

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