interfaces: support network namespaces via 'ip netns' in network-control (LP: #1624675) #2450

Merged
merged 21 commits into from Dec 14, 2016

Conversation

Projects
None yet
4 participants
Contributor

jdstrand commented Dec 10, 2016

This interface allows snaps to use the 'ip netns' command to create and use persistent network namespaces that can by entered via setns(), nsenter --net=/run/netns/... and ip netns exec by other snaps or unsnapped processes.

This PR:

  • adds apparmor and seccomp policy necessary to use 'ip netns' with the network-control interface
  • updates snap-confine for CLONE_NEW* and updates the testsuite
  • updates snap-confine to create /run/netns and make it a bidirectional mount and adds a spread test
  • fixes an errant commit in upstream that broke the syntax check in error.h

With the recent move of snap-confine into snapd, snap-confine's spread.yaml was removed and its spread tests cannot be run. I did install an updated snapd and snap-confine in a VM and ran the spread test's commands successfully.

I'm on the fence regarding having network-namespace-control in its own interface as opposed to just being added to network-control. On the one hand, not all snaps that use network-control will need or want to setup persistent network interfaces via 'ip netns', but probably every snap that plugs network-namespace-control will then need network-control to do anything interesting with the network namespace. In addition, having only a subset of the ip command supported in each of network-control and network-namespace-control feels a little odd.

Interestingly, developing this PR resulted in discovering two apparmor bugs and highlighting a seccomp missing feature in snappy kernels:

  1. LP: #1648245 requires the use of a too-broad mount rule. Once the fix is made available, we can remove this workaround rule
  2. LP: #1648903 requires a hack in the spread test (we could also create a snap that plugs: [ network-namespace-control ] and avoid the hack)
  3. ip netns identify <pid> and ip netns pids <namespace> require ptracing other processes. Up until the 4.8 kernels allowing ptrace would allow for a seccomp sandbox escape. As a result, I added explicit deny rules with audit to block and document the attack but serve as a reminder to get it fixed. Once this patch is backported, made available as part of our snappy kernel porting guide and in our supported kernels, we can allow ptrace (trace) peer=snap.@{SNAP_NAME}.*, in the default template then add ptrace (trace), to the process-control interface or perhaps a separate ptrace interface.

UPDATE: @niemeyer and I decided this would be better in network-control so I've updated the PR accordingly. Note that references to 'network-namespace-control' in the comments refer to the original PR that implemented this as a separate interface.

jdstrand added some commits Dec 8, 2016

add apparmor and seccomp policy for network-namespace-control (LP: #1…
…624675)

Description: Can configure network namespaces via the standard 'ip netns'
command (man ip-netns(8)). In order to create network namespaces that
persist outside of the process and be entered (eg, via 'ip netns exec ...')
the ip command uses mount namespaces such that applications can open the
/run/netns/NAME object and use it with setns(2). For 'ip netns exec' it will
also create a mount namespace and bind mount network configuration files into
/etc in that namespace. See man ip-netns(8) for details.
Contributor

jdstrand commented Dec 10, 2016

@javacruft - I tried to test neutron with this using 'snap-deploy' but it wouldn't install neutron. Note, you need to connect the firewall-control interface for nova, but something else is wrong (tried on 16.04).

This looks good apart from the spread test as outlined above. The first thing I'd like to do next week is to land the packaging branch so that snapd spread tests can run against the locally built snap-confine. After this is done and after the spread test is adjusted we can land this knowing the feature is fully supported and works OK

@@ -0,0 +1,34 @@
+summary: The /run/netns directory propagates events outwards
@zyga

zyga Dec 10, 2016

Contributor

This looks good but those spread tests don't run (yet). Could you please add this test under tests/main instead?

@jdstrand

jdstrand Dec 12, 2016

Contributor

@zyga Since you just said that the spread tests aren't yet running against the locally built snap-confine, moving the test will result in a spread test failure since the spread-tested snap-confine won't have the changes required. That's why I put the spread test where I did.

@zyga

zyga Dec 12, 2016

Contributor

This makes sense (obviously, sorry for not realizing this earlier).

We can now run spread tests as snap-confine is merged at the packaging level in master so we can revisit this.

jdstrand added some commits Dec 12, 2016

Contributor

jdstrand commented Dec 12, 2016

@zyga: thanks. I moved the spread test to tests/main/interfaces-network-namespace-control

Contributor

jdstrand commented Dec 12, 2016

FYI, the trusty failure is unrelated:
blame: https://github.com/snapcore/snapd.git#refs/pull/2450/head
badpkg: Test dependencies are unsatisfiable. A common reason is that your testbed is out of date with respect to the archive, and you need to use a current testbed or run apt-get update or use -U.
autopkgtest [21:26:49]: ERROR: erroneous package: Test dependencies are unsatisfiable. A common reason is that your testbed is out of date with respect to the archive, and you need to use a current testbed or run apt-get update or use -U.

Contributor

jdstrand commented Dec 12, 2016

I'm puzzled what happened with the travis failure-- afaics it just didn't finish, so I 'restarted the build'.

Contributor

jdstrand commented Dec 13, 2016

linode:ubuntu-core-16-64:tests/main/interfaces-network-control fails with:

hsearch_r failed: No such process

hsearch_r sets errno to ESRCH ("No such process") if it can't find the value. This is caused by this seccomp rule:

setns - CLONE_NEWNET

The linode:ubuntu-core-16-64:tests/main/interfaces-network-namespace-control failed with errors that I would expect with an older snap-confine. It's almost as if the linode machine didn't get the new snap-confine code.

If I run spread with -debug, I see:

# /usr/lib/snapd/snap-confine --version
snap-confine 2.19+ppa127.8970b0e4-1

but I don't know how to map that to a snap-confine or snapd commit.

Note that these spread tests pass on autopkgtest for xenial, yakkety and zesty (trusty is a different issue, see my previous comment) so it seems like there is something awry on the linode system.

Contributor

jdstrand commented Dec 13, 2016

@mvo5 and I discussed this a bit on IRC. What is happening is that in Travis, the test harness is not pulling in the changes to snap-confine in this PR. This is demonstrated by:

  • spread with -debug gives a snap-confine version of 2.19+ppa127.8970b0e4-1
  • git show 8970b0e in a checkout of this PR results in an error
  • git show in upstream/master has 8970b0e

As such, Travis pulled upstream/master, not this PR, which explains why both interfaces-network-control and interfaces-network-namespace-control failed (Travis got the new spread test, but not the code to work with it).

Because the autopkgtests did get the updated code and the tests passed there, I believe that the Travis spread failures should not impact this PR's acceptance, since once this PR is committed, upstream/master will have all the bits needed for the spread test to pass.

Contributor

jdstrand commented Dec 13, 2016

@mvo5 asked me to update this PR with upstream/master so keep in mind that will remove the specific evidence for the Travis failure.

Collaborator

mvo5 commented Dec 13, 2016

Fwiw, snap-confine 2.19+ppa127.8970b0e4-1 is exactly the version in the ppa:snappy-dev/edge. So something pulled that in instead of the one that got build from the branch.

Contributor

jdstrand commented Dec 13, 2016

FYI, with the new push these are the only failures:
- linode:ubuntu-core-16-64:tests/main/install-remove-multi
- linode:ubuntu-core-16-64:tests/main/interfaces-network-control
- linode:ubuntu-core-16-64:tests/main/interfaces-network-namespace-control

I don't know what the first is. @mvo5 - could the last two be because it is an all snaps image and not getting the updated snap-confine?

Contributor

jdstrand commented Dec 13, 2016

install-remove-multi failure is unrelated:

+ echo 'Install multiple snaps from the store'
Install multiple snaps from the store
+ snap install test-snapd-tools test-snapd-control-consumer
error: cannot install ["test-snapd-tools" "test-snapd-control-consumer"]: cannot get nonce from store: Post https://myapps.developer.ubuntu.com/identity/api/v1/nonces: net/http: request canceled while waiting for connection (Client.Timeout exceeded while awaiting headers)

@mvo5 mvo5 added this to the 2.20 milestone Dec 13, 2016

jdstrand added some commits Dec 13, 2016

@jdstrand jdstrand changed the title from interfaces: add network-namespace-control (LP: #1624675) to interfaces: support network namespaces via 'ip netns' in network-control (LP: #1624675) Dec 13, 2016

Contributor

jdstrand commented Dec 14, 2016

With the new commits into network-control, only linode:ubuntu-core-16-64:tests/main/interfaces-network-control-ip-netns fails for the same reasons as before.

On zesty, these unrelated failures:

  • autopkgtest:ubuntu-16.04-64:tests/main/interfaces-network-bind
  • autopkgtest:ubuntu-16.04-64:tests/main/server-snap:goServer
  • autopkgtest:ubuntu-16.04-64:tests/main/server-snap:pythonServer

Note, these same to be failing in other PRs. Eg #1613 (comment)

Contributor

jdstrand commented Dec 14, 2016

@mvo5 - I incorporated the feedback from Gustavo and retested locally and see that it is passing in non-all-snaps systems. This got a +1 from Zygmunt. I believe this is ready to merge pending how you want to handle the test infrastructure.

Collaborator

mvo5 commented Dec 14, 2016

Thanks, I merged master and now spread is happy.

@mvo5 mvo5 merged commit 1bb919f into snapcore:master Dec 14, 2016

4 of 6 checks passed

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

jdstrand commented Dec 14, 2016

Thanks!

@jdstrand jdstrand deleted the jdstrand:network-namespace-control branch Jan 9, 2017

croepha commented Jan 12, 2017

I feel like I'm late to party here, and also I might be missing something, as I'm not very familiar with the internals of snappy as you all are, but as I understand it the goal of these changes is to allow snaps to set their network namespace. While I think that Is a fine and acceptable goal, wouldn't a more powerful feature be a way for snappy-confine to set the appropriate net namespace for the snap? With the approach that is enabled by this PR you are trusting the snap to set its namespace properly, but if you did it with confine then you wouldn't have to trust the snap... The snap would just run in that net namespace and if there were appropriate ipsec rules then that snap wouldn't be allowed to do anything on/from the root ip and not able to access the systems root loopback interface...

Contributor

jdstrand commented Jan 12, 2017

@croepha - Hi! We intentionally have snaps run in the global network namespace so they are fully integrated into the system. Snappy is a bit different from container technologies like docker and lxd in that while we leverage various container technologies to implement the sandbox, snaps are meant to integrate into the system and feel like installed applications that can interact with each other and the system in controlled ways. A mental model one can use is: snaps are for applications that integrate into the system, docker can be used for 'process containers' where the application process is in its own namespace for everything and lxd can be thought of as full machine as a container (eg, think 'migrating a VM to a container' where everything in that container shares the same container-specific namespace for everything).

With that in mind, this interface allows snaps that 'plugs: [ network-control ]' (a very privileged interface that allows configuring all kings of networking on the device) to also be able to use 'ip netns' to setup network namespaces for the system or other snaps to then use and enter. This is useful in and of itself for management snaps. I agree it would be useful to limit what snaps can do with networking (eg, loopback, talking to other snaps on the system via network IPC) and this work is planned. To preserve the properties of integrating into the system, this will be implemented using AppArmor fine-grained network mediation which will (rather easily once fine-grained network mediation is finished) allow things like interface connections for daemon network listeners just like we do with unix sockets.

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