diff options
| -rw-r--r-- | pkg/sentry/socket/netfilter/owner_matcher.go | 81 | ||||
| -rw-r--r-- | test/iptables/filter_output.go | 169 | ||||
| -rw-r--r-- | test/iptables/iptables_test.go | 20 |
3 files changed, 230 insertions, 40 deletions
diff --git a/pkg/sentry/socket/netfilter/owner_matcher.go b/pkg/sentry/socket/netfilter/owner_matcher.go index 5949a7c29..3863293c7 100644 --- a/pkg/sentry/socket/netfilter/owner_matcher.go +++ b/pkg/sentry/socket/netfilter/owner_matcher.go @@ -45,14 +45,18 @@ func (ownerMarshaler) marshal(mr stack.Matcher) []byte { GID: matcher.gid, } - // Support for UID match. - // TODO(gvisor.dev/issue/170): Need to support gid match. + // Support for UID and GID match. if matcher.matchUID { iptOwnerInfo.Match = linux.XT_OWNER_UID - } else if matcher.matchGID { - panic("GID match is not supported.") - } else { - panic("UID match is not set.") + if matcher.invertUID { + iptOwnerInfo.Invert = linux.XT_OWNER_UID + } + } + if matcher.matchGID { + iptOwnerInfo.Match |= linux.XT_OWNER_GID + if matcher.invertGID { + iptOwnerInfo.Invert |= linux.XT_OWNER_GID + } } buf := make([]byte, 0, linux.SizeOfIPTOwnerInfo) @@ -71,31 +75,34 @@ func (ownerMarshaler) unmarshal(buf []byte, filter stack.IPHeaderFilter) (stack. binary.Unmarshal(buf[:linux.SizeOfIPTOwnerInfo], usermem.ByteOrder, &matchData) nflog("parseMatchers: parsed IPTOwnerInfo: %+v", matchData) - if matchData.Invert != 0 { - return nil, fmt.Errorf("invert flag is not supported for owner match") - } - - // Support for UID match. - // TODO(gvisor.dev/issue/170): Need to support gid match. - if matchData.Match&linux.XT_OWNER_UID != linux.XT_OWNER_UID { - return nil, fmt.Errorf("owner match is only supported for uid") - } - - // Check Flags. var owner OwnerMatcher owner.uid = matchData.UID owner.gid = matchData.GID - owner.matchUID = true + + // Check flags. + if matchData.Match&linux.XT_OWNER_UID != 0 { + owner.matchUID = true + if matchData.Invert&linux.XT_OWNER_UID != 0 { + owner.invertUID = true + } + } + if matchData.Match&linux.XT_OWNER_GID != 0 { + owner.matchGID = true + if matchData.Invert&linux.XT_OWNER_GID != 0 { + owner.invertGID = true + } + } return &owner, nil } type OwnerMatcher struct { - uid uint32 - gid uint32 - matchUID bool - matchGID bool - invert uint8 + uid uint32 + gid uint32 + matchUID bool + matchGID bool + invertUID bool + invertGID bool } // Name implements Matcher.Name. @@ -112,16 +119,30 @@ func (om *OwnerMatcher) Match(hook stack.Hook, pkt stack.PacketBuffer, interface } // If the packet owner is not set, drop the packet. - // Support for uid match. - // TODO(gvisor.dev/issue/170): Need to support gid match. - if pkt.Owner == nil || !om.matchUID { + if pkt.Owner == nil { return false, true } - // TODO(gvisor.dev/issue/170): Need to add tests to verify - // drop rule when packet UID does not match owner matcher UID. - if pkt.Owner.UID() != om.uid { - return false, false + var matches bool + // Check for UID match. + if om.matchUID { + if pkt.Owner.UID() == om.uid { + matches = true + } + if matches == om.invertUID { + return false, false + } + } + + // Check for GID match. + if om.matchGID { + matches = false + if pkt.Owner.GID() == om.gid { + matches = true + } + if matches == om.invertGID { + return false, false + } } return true, false diff --git a/test/iptables/filter_output.go b/test/iptables/filter_output.go index c145bd1e9..ba0d6fc29 100644 --- a/test/iptables/filter_output.go +++ b/test/iptables/filter_output.go @@ -29,6 +29,11 @@ func init() { RegisterTestCase(FilterOutputAcceptUDPOwner{}) RegisterTestCase(FilterOutputDropUDPOwner{}) RegisterTestCase(FilterOutputOwnerFail{}) + RegisterTestCase(FilterOutputAcceptGIDOwner{}) + RegisterTestCase(FilterOutputDropGIDOwner{}) + RegisterTestCase(FilterOutputInvertGIDOwner{}) + RegisterTestCase(FilterOutputInvertUIDOwner{}) + RegisterTestCase(FilterOutputInvertUIDAndGIDOwner{}) RegisterTestCase(FilterOutputInterfaceAccept{}) RegisterTestCase(FilterOutputInterfaceDrop{}) RegisterTestCase(FilterOutputInterface{}) @@ -116,20 +121,12 @@ func (FilterOutputAcceptTCPOwner) ContainerAction(ip net.IP) error { } // Listen for TCP packets on accept port. - if err := listenTCP(acceptPort, sendloopDuration); err != nil { - return fmt.Errorf("connection on port %d should be accepted, but got dropped", acceptPort) - } - - return nil + return listenTCP(acceptPort, sendloopDuration) } // LocalAction implements TestCase.LocalAction. func (FilterOutputAcceptTCPOwner) LocalAction(ip net.IP) error { - if err := connectTCP(ip, acceptPort, sendloopDuration); err != nil { - return fmt.Errorf("connection destined to port %d should be accepted, but got dropped", acceptPort) - } - - return nil + return connectTCP(ip, acceptPort, sendloopDuration) } // FilterOutputDropTCPOwner tests that TCP connections from uid owner are dropped. @@ -239,6 +236,158 @@ func (FilterOutputOwnerFail) LocalAction(ip net.IP) error { return nil } +// FilterOutputAcceptGIDOwner tests that TCP connections from gid owner are accepted. +type FilterOutputAcceptGIDOwner struct{} + +// Name implements TestCase.Name. +func (FilterOutputAcceptGIDOwner) Name() string { + return "FilterOutputAcceptGIDOwner" +} + +// ContainerAction implements TestCase.ContainerAction. +func (FilterOutputAcceptGIDOwner) ContainerAction(ip net.IP) error { + if err := filterTable("-A", "OUTPUT", "-p", "tcp", "-m", "owner", "--gid-owner", "root", "-j", "ACCEPT"); err != nil { + return err + } + + // Listen for TCP packets on accept port. + return listenTCP(acceptPort, sendloopDuration) +} + +// LocalAction implements TestCase.LocalAction. +func (FilterOutputAcceptGIDOwner) LocalAction(ip net.IP) error { + return connectTCP(ip, acceptPort, sendloopDuration) +} + +// FilterOutputDropGIDOwner tests that TCP connections from gid owner are dropped. +type FilterOutputDropGIDOwner struct{} + +// Name implements TestCase.Name. +func (FilterOutputDropGIDOwner) Name() string { + return "FilterOutputDropGIDOwner" +} + +// ContainerAction implements TestCase.ContainerAction. +func (FilterOutputDropGIDOwner) ContainerAction(ip net.IP) error { + if err := filterTable("-A", "OUTPUT", "-p", "tcp", "-m", "owner", "--gid-owner", "root", "-j", "DROP"); err != nil { + return err + } + + // Listen for TCP packets on accept port. + if err := listenTCP(acceptPort, sendloopDuration); err == nil { + return fmt.Errorf("connection on port %d should not be accepted, but got accepted", acceptPort) + } + + return nil +} + +// LocalAction implements TestCase.LocalAction. +func (FilterOutputDropGIDOwner) LocalAction(ip net.IP) error { + if err := connectTCP(ip, acceptPort, sendloopDuration); err == nil { + return fmt.Errorf("connection destined to port %d should not be accepted, but got accepted", acceptPort) + } + + return nil +} + +// FilterOutputInvertGIDOwner tests that TCP connections from gid owner are dropped. +type FilterOutputInvertGIDOwner struct{} + +// Name implements TestCase.Name. +func (FilterOutputInvertGIDOwner) Name() string { + return "FilterOutputInvertGIDOwner" +} + +// ContainerAction implements TestCase.ContainerAction. +func (FilterOutputInvertGIDOwner) ContainerAction(ip net.IP) error { + rules := [][]string{ + {"-A", "OUTPUT", "-p", "tcp", "-m", "owner", "!", "--gid-owner", "root", "-j", "ACCEPT"}, + {"-A", "OUTPUT", "-p", "tcp", "-j", "DROP"}, + } + if err := filterTableRules(rules); err != nil { + return err + } + + // Listen for TCP packets on accept port. + if err := listenTCP(acceptPort, sendloopDuration); err == nil { + return fmt.Errorf("connection on port %d should not be accepted, but got accepted", acceptPort) + } + + return nil +} + +// LocalAction implements TestCase.LocalAction. +func (FilterOutputInvertGIDOwner) LocalAction(ip net.IP) error { + if err := connectTCP(ip, acceptPort, sendloopDuration); err == nil { + return fmt.Errorf("connection destined to port %d should not be accepted, but got accepted", acceptPort) + } + + return nil +} + +// FilterOutputInvertUIDOwner tests that TCP connections from gid owner are dropped. +type FilterOutputInvertUIDOwner struct{} + +// Name implements TestCase.Name. +func (FilterOutputInvertUIDOwner) Name() string { + return "FilterOutputInvertUIDOwner" +} + +// ContainerAction implements TestCase.ContainerAction. +func (FilterOutputInvertUIDOwner) ContainerAction(ip net.IP) error { + rules := [][]string{ + {"-A", "OUTPUT", "-p", "tcp", "-m", "owner", "!", "--uid-owner", "root", "-j", "DROP"}, + {"-A", "OUTPUT", "-p", "tcp", "-j", "ACCEPT"}, + } + if err := filterTableRules(rules); err != nil { + return err + } + + // Listen for TCP packets on accept port. + return listenTCP(acceptPort, sendloopDuration) +} + +// LocalAction implements TestCase.LocalAction. +func (FilterOutputInvertUIDOwner) LocalAction(ip net.IP) error { + return connectTCP(ip, acceptPort, sendloopDuration) +} + +// FilterOutputInvertUIDAndGIDOwner tests that TCP connections from uid and gid +// owner are dropped. +type FilterOutputInvertUIDAndGIDOwner struct{} + +// Name implements TestCase.Name. +func (FilterOutputInvertUIDAndGIDOwner) Name() string { + return "FilterOutputInvertUIDAndGIDOwner" +} + +// ContainerAction implements TestCase.ContainerAction. +func (FilterOutputInvertUIDAndGIDOwner) ContainerAction(ip net.IP) error { + rules := [][]string{ + {"-A", "OUTPUT", "-p", "tcp", "-m", "owner", "!", "--uid-owner", "root", "!", "--gid-owner", "root", "-j", "ACCEPT"}, + {"-A", "OUTPUT", "-p", "tcp", "-j", "DROP"}, + } + if err := filterTableRules(rules); err != nil { + return err + } + + // Listen for TCP packets on accept port. + if err := listenTCP(acceptPort, sendloopDuration); err == nil { + return fmt.Errorf("connection on port %d should not be accepted, but got accepted", acceptPort) + } + + return nil +} + +// LocalAction implements TestCase.LocalAction. +func (FilterOutputInvertUIDAndGIDOwner) LocalAction(ip net.IP) error { + if err := connectTCP(ip, acceptPort, sendloopDuration); err == nil { + return fmt.Errorf("connection destined to port %d should not be accepted, but got accepted", acceptPort) + } + + return nil +} + // FilterOutputDestination tests that we can selectively allow packets to // certain destinations. type FilterOutputDestination struct{} diff --git a/test/iptables/iptables_test.go b/test/iptables/iptables_test.go index 84eb75a40..4fd2cb46a 100644 --- a/test/iptables/iptables_test.go +++ b/test/iptables/iptables_test.go @@ -167,6 +167,26 @@ func TestFilterOutputOwnerFail(t *testing.T) { singleTest(t, FilterOutputOwnerFail{}) } +func TestFilterOutputAcceptGIDOwner(t *testing.T) { + singleTest(t, FilterOutputAcceptGIDOwner{}) +} + +func TestFilterOutputDropGIDOwner(t *testing.T) { + singleTest(t, FilterOutputDropGIDOwner{}) +} + +func TestFilterOutputInvertGIDOwner(t *testing.T) { + singleTest(t, FilterOutputInvertGIDOwner{}) +} + +func TestFilterOutputInvertUIDOwner(t *testing.T) { + singleTest(t, FilterOutputInvertUIDOwner{}) +} + +func TestFilterOutputInvertUIDAndGIDOwner(t *testing.T) { + singleTest(t, FilterOutputInvertUIDAndGIDOwner{}) +} + func TestFilterOutputInterfaceAccept(t *testing.T) { singleTest(t, FilterOutputInterfaceAccept{}) } |