Skip to content

Commit c85244f

Browse files
shailend-ggvisor-bot
authored andcommitted
netstack: Fix cap check around IFLA_NET_NS_FD
When bridging, or when moving a veth, we must check if we have CAP_NET_ADMIN in the target netns. PiperOrigin-RevId: 889518624
1 parent e2588d7 commit c85244f

2 files changed

Lines changed: 28 additions & 2 deletions

File tree

‎pkg/sentry/socket/netstack/stack.go‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"gvisor.dev/gvisor/pkg/log"
2424
"gvisor.dev/gvisor/pkg/refs"
2525
"gvisor.dev/gvisor/pkg/sentry/inet"
26+
"gvisor.dev/gvisor/pkg/sentry/kernel/auth"
2627
"gvisor.dev/gvisor/pkg/sentry/socket/netlink/nlmsg"
2728
"gvisor.dev/gvisor/pkg/syserr"
2829
"gvisor.dev/gvisor/pkg/tcpip"
@@ -253,7 +254,10 @@ func (s *Stack) SetInterface(ctx context.Context, msg *nlmsg.Message) *syserr.Er
253254
//
254255
// If instead the linkAttrs map does not contain IFLA_NET_NS_FD, or if it
255256
// points to the same netns as the source stack, only locks the source stack
256-
// and returns nil. And if the netns fd is invalid, returns an error.
257+
// and returns nil.
258+
//
259+
// If the netns fd is invalid, or the calling context lacks CAP_NET_ADMIN,
260+
// returns an error.
257261
func (s *Stack) lockSrcAndDst(ctx context.Context, linkAttrs map[uint16]nlmsg.BytesView) (*inet.Namespace, *syserr.Error) {
258262
if linkAttrs == nil {
259263
s.linkMu.Lock()
@@ -273,17 +277,22 @@ func (s *Stack) lockSrcAndDst(ctx context.Context, linkAttrs map[uint16]nlmsg.By
273277
if f == nil {
274278
return nil, syserr.ErrInvalidArgument
275279
}
276-
ns, err := f(int32(fd)) // ns.DecRef() is called in unlockSrcAndDst().
280+
ns, err := f(int32(fd))
277281
if err != nil {
278282
return nil, syserr.FromError(err)
279283
}
284+
if !auth.CredentialsFromContext(ctx).HasCapabilityIn(linux.CAP_NET_ADMIN, ns.UserNamespace()) {
285+
ns.DecRef(ctx)
286+
return nil, syserr.ErrNotPermittedNet
287+
}
280288

281289
dst := ns.Stack().(*Stack)
282290
if s == dst {
283291
ns.DecRef(ctx)
284292
s.linkMu.Lock()
285293
return nil, nil
286294
}
295+
// No failures from this point on, ns.DecRef() is called in unlockSrcAndDst().
287296

288297
if s.id < dst.id {
289298
s.linkMu.Lock()

‎test/rtnetlink/linux/veth_test.sh‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,3 +52,20 @@ if ! wait_for ! ip netns exec test ip link show test_veth02; then
5252
fail "test_veth02 hasn't been destroyed"
5353
fi
5454
ip netns del test
55+
56+
# Test for the fact that we need CAP_NET_ADMIN in the target namespace to move a veth.
57+
ip netns add priv_target_ns
58+
unshare_status=0
59+
unshare -U -r -n bash -c '
60+
ip link add veth_src type veth peer name veth_dst || exit 2
61+
if ip link set veth_dst netns /var/run/netns/priv_target_ns 2>/dev/null; then
62+
exit 1
63+
fi
64+
exit 0
65+
' || unshare_status=$?
66+
ip netns del priv_target_ns
67+
if [[ "${unshare_status}" -eq 1 ]]; then
68+
fail "Move succeeded without CAP_NET_ADMIN in target"
69+
elif [[ "${unshare_status}" -eq 2 ]]; then
70+
fail "Unexpected failure to create veth pair"
71+
fi

0 commit comments

Comments
 (0)