Merge pull request #1775 from kevinGC:tcp-matchers-submit

PiperOrigin-RevId: 294340468

pkg/abi/linux/netfilter.go

@@ -348,6 +348,59 @@ func goString(cstring []byte) string {
 	return string(cstring)
 }
 
+// XTTCP holds data for matching TCP packets. It corresponds to struct xt_tcp
+// in include/uapi/linux/netfilter/xt_tcpudp.h.
+type XTTCP struct {
+	// SourcePortStart specifies the inclusive start of the range of source
+	// ports to which the matcher applies.
+	SourcePortStart uint16
+
+	// SourcePortEnd specifies the inclusive end of the range of source ports
+	// to which the matcher applies.
+	SourcePortEnd uint16
+
+	// DestinationPortStart specifies the start of the destination port
+	// range to which the matcher applies.
+	DestinationPortStart uint16
+
+	// DestinationPortEnd specifies the end of the destination port
+	// range to which the matcher applies.
+	DestinationPortEnd uint16
+
+	// Option specifies that a particular TCP option must be set.
+	Option uint8
+
+	// FlagMask masks TCP flags when comparing to the FlagCompare byte. It allows
+	// for specification of which flags are important to the matcher.
+	FlagMask uint8
+
+	// FlagCompare, in combination with FlagMask, is used to match only packets
+	// that have certain flags set.
+	FlagCompare uint8
+
+	// InverseFlags flips the meaning of certain fields. See the
+	// TX_TCP_INV_* flags.
+	InverseFlags uint8
+}
+
+// SizeOfXTTCP is the size of an XTTCP.
+const SizeOfXTTCP = 12
+
+// Flags in XTTCP.InverseFlags. Corresponding constants are in
+// include/uapi/linux/netfilter/xt_tcpudp.h.
+const (
+	// Invert the meaning of SourcePortStart/End.
+	XT_TCP_INV_SRCPT = 0x01
+	// Invert the meaning of DestinationPortStart/End.
+	XT_TCP_INV_DSTPT = 0x02
+	// Invert the meaning of FlagCompare.
+	XT_TCP_INV_FLAGS = 0x04
+	// Invert the meaning of Option.
+	XT_TCP_INV_OPTION = 0x08
+	// Enable all flags.
+	XT_TCP_INV_MASK = 0x0F
+)
+
 // XTUDP holds data for matching UDP packets. It corresponds to struct xt_udp
 // in include/uapi/linux/netfilter/xt_tcpudp.h.
 type XTUDP struct {

pkg/sentry/socket/netfilter/BUILD

@@ -5,7 +5,10 @@ package(licenses = ["notice"])
 go_library(
     name = "netfilter",
     srcs = [
+        "extensions.go",
         "netfilter.go",
+        "tcp_matcher.go",
+        "udp_matcher.go",
     ],
     # This target depends on netstack and should only be used by epsocket,
     # which is allowed to depend on netstack.
@@ -17,6 +20,7 @@ go_library(
         "//pkg/sentry/kernel",
         "//pkg/syserr",
         "//pkg/tcpip",
+        "//pkg/tcpip/header",
         "//pkg/tcpip/iptables",
         "//pkg/tcpip/stack",
         "//pkg/usermem",

pkg/sentry/socket/netfilter/extensions.go

@@ -0,0 +1,100 @@
+// Copyright 2020 The gVisor Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package netfilter
+
+import (
+	"fmt"
+
+	"gvisor.dev/gvisor/pkg/abi/linux"
+	"gvisor.dev/gvisor/pkg/binary"
+	"gvisor.dev/gvisor/pkg/tcpip/iptables"
+	"gvisor.dev/gvisor/pkg/usermem"
+)
+
+// TODO(gvisor.dev/issue/170): The following per-matcher params should be
+// supported:
+// - Table name
+// - Match size
+// - User size
+// - Hooks
+// - Proto
+// - Family
+
+// matchMaker knows how to (un)marshal the matcher named name().
+type matchMaker interface {
+	// name is the matcher name as stored in the xt_entry_match struct.
+	name() string
+
+	// marshal converts from an iptables.Matcher to an ABI struct.
+	marshal(matcher iptables.Matcher) []byte
+
+	// unmarshal converts from the ABI matcher struct to an
+	// iptables.Matcher.
+	unmarshal(buf []byte, filter iptables.IPHeaderFilter) (iptables.Matcher, error)
+}
+
+// matchMakers maps the name of supported matchers to the matchMaker that
+// marshals and unmarshals it. It is immutable after package initialization.
+var matchMakers = map[string]matchMaker{}
+
+// registermatchMaker should be called by match extensions to register them
+// with the netfilter package.
+func registerMatchMaker(mm matchMaker) {
+	if _, ok := matchMakers[mm.name()]; ok {
+		panic(fmt.Sprintf("Multiple matches registered with name %q.", mm.name()))
+	}
+	matchMakers[mm.name()] = mm
+}
+
+func marshalMatcher(matcher iptables.Matcher) []byte {
+	matchMaker, ok := matchMakers[matcher.Name()]
+	if !ok {
+		panic(fmt.Sprintf("Unknown matcher of type %T.", matcher))
+	}
+	return matchMaker.marshal(matcher)
+}
+
+// marshalEntryMatch creates a marshalled XTEntryMatch with the given name and
+// data appended at the end.
+func marshalEntryMatch(name string, data []byte) []byte {
+	nflog("marshaling matcher %q", name)
+
+	// We have to pad this struct size to a multiple of 8 bytes.
+	size := alignUp(linux.SizeOfXTEntryMatch+len(data), 8)
+	matcher := linux.KernelXTEntryMatch{
+		XTEntryMatch: linux.XTEntryMatch{
+			MatchSize: uint16(size),
+		},
+		Data: data,
+	}
+	copy(matcher.Name[:], name)
+
+	buf := make([]byte, 0, size)
+	buf = binary.Marshal(buf, usermem.ByteOrder, matcher)
+	return append(buf, make([]byte, size-len(buf))...)
+}
+
+func unmarshalMatcher(match linux.XTEntryMatch, filter iptables.IPHeaderFilter, buf []byte) (iptables.Matcher, error) {
+	matchMaker, ok := matchMakers[match.Name.String()]
+	if !ok {
+		return nil, fmt.Errorf("unsupported matcher with name %q", match.Name.String())
+	}
+	return matchMaker.unmarshal(buf, filter)
+}
+
+// alignUp rounds a length up to an alignment. align must be a power of 2.
+func alignUp(length int, align uint) int {
+	return (length + int(align) - 1) & ^(int(align) - 1)
+}

pkg/sentry/socket/netfilter/netfilter.go

@@ -17,6 +17,7 @@
 package netfilter
 
 import (
+	"errors"
 	"fmt"
 
 	"gvisor.dev/gvisor/pkg/abi/linux"
@@ -34,10 +35,6 @@ import (
 // shouldn't be reached - an error has occurred if we fall through to one.
 const errorTargetName = "ERROR"
 
-const (
-	matcherNameUDP = "udp"
-)
-
 // Metadata is used to verify that we are correctly serializing and
 // deserializing iptables into structs consumable by the iptables tool. We save
 // a metadata struct when the tables are written, and when they are read out we
@@ -67,7 +64,8 @@ func GetInfo(t *kernel.Task, stack *stack.Stack, outPtr usermem.Addr) (linux.IPT
 	// Find the appropriate table.
 	table, err := findTable(stack, info.Name)
 	if err != nil {
-		return linux.IPTGetinfo{}, err
+		nflog("%v", err)
+		return linux.IPTGetinfo{}, syserr.ErrInvalidArgument
 	}
 
 	// Get the hooks that apply to this table.
@@ -94,39 +92,40 @@ func GetEntries(t *kernel.Task, stack *stack.Stack, outPtr usermem.Addr, outLen
 	// Read in the struct and table name.
 	var userEntries linux.IPTGetEntries
 	if _, err := t.CopyIn(outPtr, &userEntries); err != nil {
-		log.Warningf("netfilter: couldn't copy in entries %q", userEntries.Name)
+		nflog("couldn't copy in entries %q", userEntries.Name)
 		return linux.KernelIPTGetEntries{}, syserr.FromError(err)
 	}
 
 	// Find the appropriate table.
 	table, err := findTable(stack, userEntries.Name)
 	if err != nil {
-		log.Warningf("netfilter: couldn't find table %q", userEntries.Name)
-		return linux.KernelIPTGetEntries{}, err
+		nflog("%v", err)
+		return linux.KernelIPTGetEntries{}, syserr.ErrInvalidArgument
 	}
 
 	// Convert netstack's iptables rules to something that the iptables
 	// tool can understand.
 	entries, meta, err := convertNetstackToBinary(userEntries.Name.String(), table)
 	if err != nil {
-		return linux.KernelIPTGetEntries{}, err
+		nflog("couldn't read entries: %v", err)
+		return linux.KernelIPTGetEntries{}, syserr.ErrInvalidArgument
 	}
 	if meta != table.Metadata().(metadata) {
 		panic(fmt.Sprintf("Table %q metadata changed between writing and reading. Was saved as %+v, but is now %+v", userEntries.Name.String(), table.Metadata().(metadata), meta))
 	}
 	if binary.Size(entries) > uintptr(outLen) {
-		log.Warningf("Insufficient GetEntries output size: %d", uintptr(outLen))
+		nflog("insufficient GetEntries output size: %d", uintptr(outLen))
 		return linux.KernelIPTGetEntries{}, syserr.ErrInvalidArgument
 	}
 
 	return entries, nil
 }
 
-func findTable(stack *stack.Stack, tablename linux.TableName) (iptables.Table, *syserr.Error) {
+func findTable(stack *stack.Stack, tablename linux.TableName) (iptables.Table, error) {
 	ipt := stack.IPTables()
 	table, ok := ipt.Tables[tablename.String()]
 	if !ok {
-		return iptables.Table{}, syserr.ErrInvalidArgument
+		return iptables.Table{}, fmt.Errorf("couldn't find table %q", tablename)
 	}
 	return table, nil
 }
@@ -154,15 +153,14 @@ func FillDefaultIPTables(stack *stack.Stack) {
 // format expected by the iptables tool. Linux stores each table as a binary
 // blob that can only be traversed by parsing a bit, reading some offsets,
 // jumping to those offsets, parsing again, etc.
-func convertNetstackToBinary(tablename string, table iptables.Table) (linux.KernelIPTGetEntries, metadata, *syserr.Error) {
+func convertNetstackToBinary(tablename string, table iptables.Table) (linux.KernelIPTGetEntries, metadata, error) {
 	// Return values.
 	var entries linux.KernelIPTGetEntries
 	var meta metadata
 
 	// The table name has to fit in the struct.
 	if linux.XT_TABLE_MAXNAMELEN < len(tablename) {
-		log.Warningf("Table name %q too long.", tablename)
-		return linux.KernelIPTGetEntries{}, metadata{}, syserr.ErrInvalidArgument
+		return linux.KernelIPTGetEntries{}, metadata{}, fmt.Errorf("table name %q too long.", tablename)
 	}
 	copy(entries.Name[:], tablename)
 
@@ -228,46 +226,6 @@ func convertNetstackToBinary(tablename string, table iptables.Table) (linux.Kern
 	return entries, meta, nil
 }
 
-func marshalMatcher(matcher iptables.Matcher) []byte {
-	switch m := matcher.(type) {
-	case *iptables.UDPMatcher:
-		return marshalUDPMatcher(m)
-	default:
-		// TODO(gvisor.dev/issue/170): Support other matchers.
-		panic(fmt.Errorf("unknown matcher of type %T", matcher))
-	}
-}
-
-func marshalUDPMatcher(matcher *iptables.UDPMatcher) []byte {
-	nflog("convert to binary: marshalling UDP matcher: %+v", matcher)
-
-	// We have to pad this struct size to a multiple of 8 bytes.
-	size := alignUp(linux.SizeOfXTEntryMatch+linux.SizeOfXTUDP, 8)
-
-	linuxMatcher := linux.KernelXTEntryMatch{
-		XTEntryMatch: linux.XTEntryMatch{
-			MatchSize: uint16(size),
-		},
-		Data: make([]byte, 0, linux.SizeOfXTUDP),
-	}
-	copy(linuxMatcher.Name[:], matcherNameUDP)
-
-	xtudp := linux.XTUDP{
-		SourcePortStart:      matcher.Data.SourcePortStart,
-		SourcePortEnd:        matcher.Data.SourcePortEnd,
-		DestinationPortStart: matcher.Data.DestinationPortStart,
-		DestinationPortEnd:   matcher.Data.DestinationPortEnd,
-		InverseFlags:         matcher.Data.InverseFlags,
-	}
-	linuxMatcher.Data = binary.Marshal(linuxMatcher.Data, usermem.ByteOrder, xtudp)
-
-	buf := make([]byte, 0, size)
-	buf = binary.Marshal(buf, usermem.ByteOrder, linuxMatcher)
-	buf = append(buf, make([]byte, size-len(buf))...)
-	nflog("convert to binary: marshalled UDP matcher into %v", buf)
-	return buf[:]
-}
-
 func marshalTarget(target iptables.Target) []byte {
 	switch target.(type) {
 	case iptables.UnconditionalAcceptTarget:
@@ -331,7 +289,7 @@ func translateFromStandardVerdict(verdict iptables.Verdict) int32 {
 
 // translateToStandardVerdict translates from the value in a
 // linux.XTStandardTarget to an iptables.Verdict.
-func translateToStandardVerdict(val int32) (iptables.Verdict, *syserr.Error) {
+func translateToStandardVerdict(val int32) (iptables.Verdict, error) {
 	// TODO(gvisor.dev/issue/170): Support other verdicts.
 	switch val {
 	case -linux.NF_ACCEPT - 1:
@@ -339,13 +297,12 @@ func translateToStandardVerdict(val int32) (iptables.Verdict, *syserr.Error) {
 	case -linux.NF_DROP - 1:
 		return iptables.Drop, nil
 	case -linux.NF_QUEUE - 1:
-		log.Warningf("Unsupported iptables verdict QUEUE.")
+		return iptables.Invalid, errors.New("unsupported iptables verdict QUEUE")
 	case linux.NF_RETURN:
-		log.Warningf("Unsupported iptables verdict RETURN.")
+		return iptables.Invalid, errors.New("unsupported iptables verdict RETURN")
 	default:
-		log.Warningf("Unknown iptables verdict %d.", val)
+		return iptables.Invalid, fmt.Errorf("unknown iptables verdict %d", val)
 	}
-	return iptables.Invalid, syserr.ErrInvalidArgument
 }
 
 // SetEntries sets iptables rules for a single table. See
@@ -353,7 +310,7 @@ func translateToStandardVerdict(val int32) (iptables.Verdict, *syserr.Error) {
 func SetEntries(stack *stack.Stack, optVal []byte) *syserr.Error {
 	// Get the basic rules data (struct ipt_replace).
 	if len(optVal) < linux.SizeOfIPTReplace {
-		log.Warningf("netfilter.SetEntries: optVal has insufficient size for replace %d", len(optVal))
+		nflog("optVal has insufficient size for replace %d", len(optVal))
 		return syserr.ErrInvalidArgument
 	}
 	var replace linux.IPTReplace
@@ -367,7 +324,7 @@ func SetEntries(stack *stack.Stack, optVal []byte) *syserr.Error {
 	case iptables.TablenameFilter:
 		table = iptables.EmptyFilterTable()
 	default:
-		log.Warningf("We don't yet support writing to the %q table (gvisor.dev/issue/170)", replace.Name.String())
+		nflog("we don't yet support writing to the %q table (gvisor.dev/issue/170)", replace.Name.String())
 		return syserr.ErrInvalidArgument
 	}
 
@@ -381,7 +338,7 @@ func SetEntries(stack *stack.Stack, optVal []byte) *syserr.Error {
 
 		// Get the struct ipt_entry.
 		if len(optVal) < linux.SizeOfIPTEntry {
-			log.Warningf("netfilter: optVal has insufficient size for entry %d", len(optVal))
+			nflog("optVal has insufficient size for entry %d", len(optVal))
 			return syserr.ErrInvalidArgument
 		}
 		var entry linux.IPTEntry
@@ -391,7 +348,7 @@ func SetEntries(stack *stack.Stack, optVal []byte) *syserr.Error {
 		optVal = optVal[linux.SizeOfIPTEntry:]
 
 		if entry.TargetOffset < linux.SizeOfIPTEntry {
-			log.Warningf("netfilter: entry has too-small target offset %d", entry.TargetOffset)
+			nflog("entry has too-small target offset %d", entry.TargetOffset)
 			return syserr.ErrInvalidArgument
 		}
 
@@ -399,7 +356,8 @@ func SetEntries(stack *stack.Stack, optVal []byte) *syserr.Error {
 		// filtering fields.
 		filter, err := filterFromIPTIP(entry.IP)
 		if err != nil {
-			return err
+			nflog("bad iptip: %v", err)
+			return syserr.ErrInvalidArgument
 		}
 
 		// TODO(gvisor.dev/issue/170): Matchers and targets can specify
@@ -407,25 +365,26 @@ func SetEntries(stack *stack.Stack, optVal []byte) *syserr.Error {
 		// Get matchers.
 		matchersSize := entry.TargetOffset - linux.SizeOfIPTEntry
 		if len(optVal) < int(matchersSize) {
-			log.Warningf("netfilter: entry doesn't have enough room for its matchers (only %d bytes remain)", len(optVal))
+			nflog("entry doesn't have enough room for its matchers (only %d bytes remain)", len(optVal))
 			return syserr.ErrInvalidArgument
 		}
 		matchers, err := parseMatchers(filter, optVal[:matchersSize])
 		if err != nil {
-			log.Warningf("netfilter: failed to parse matchers: %v", err)
-			return err
+			nflog("failed to parse matchers: %v", err)
+			return syserr.ErrInvalidArgument
 		}
 		optVal = optVal[matchersSize:]
 
 		// Get the target of the rule.
 		targetSize := entry.NextOffset - entry.TargetOffset
 		if len(optVal) < int(targetSize) {
-			log.Warningf("netfilter: entry doesn't have enough room for its target (only %d bytes remain)", len(optVal))
+			nflog("entry doesn't have enough room for its target (only %d bytes remain)", len(optVal))
 			return syserr.ErrInvalidArgument
 		}
 		target, err := parseTarget(optVal[:targetSize])
 		if err != nil {
-			return err
+			nflog("failed to parse target: %v", err)
+			return syserr.ErrInvalidArgument
 		}
 		optVal = optVal[targetSize:]
 
@@ -438,7 +397,7 @@ func SetEntries(stack *stack.Stack, optVal []byte) *syserr.Error {
 		offset += uint32(entry.NextOffset)
 
 		if initialOptValLen-len(optVal) != int(entry.NextOffset) {
-			log.Warningf("netfilter: entry NextOffset is %d, but entry took up %d bytes", entry.NextOffset, initialOptValLen-len(optVal))
+			nflog("entry NextOffset is %d, but entry took up %d bytes", entry.NextOffset, initialOptValLen-len(optVal))
 		}
 	}
 
@@ -456,11 +415,11 @@ func SetEntries(stack *stack.Stack, optVal []byte) *syserr.Error {
 				}
 			}
 			if ruleIdx := table.BuiltinChains[hk]; ruleIdx == iptables.HookUnset {
-				log.Warningf("Hook %v is unset.", hk)
+				nflog("hook %v is unset.", hk)
 				return syserr.ErrInvalidArgument
 			}
 			if ruleIdx := table.Underflows[hk]; ruleIdx == iptables.HookUnset {
-				log.Warningf("Underflow %v is unset.", hk)
+				nflog("underflow %v is unset.", hk)
 				return syserr.ErrInvalidArgument
 			}
 		}
@@ -472,7 +431,7 @@ func SetEntries(stack *stack.Stack, optVal []byte) *syserr.Error {
 	for hook, ruleIdx := range table.BuiltinChains {
 		if hook != iptables.Input {
 			if _, ok := table.Rules[ruleIdx].Target.(iptables.UnconditionalAcceptTarget); !ok {
-				log.Warningf("Hook %d is unsupported.", hook)
+				nflog("hook %d is unsupported.", hook)
 				return syserr.ErrInvalidArgument
 			}
 		}
@@ -498,7 +457,7 @@ func SetEntries(stack *stack.Stack, optVal []byte) *syserr.Error {
 
 // parseMatchers parses 0 or more matchers from optVal. optVal should contain
 // only the matchers.
-func parseMatchers(filter iptables.IPHeaderFilter, optVal []byte) ([]iptables.Matcher, *syserr.Error) {
+func parseMatchers(filter iptables.IPHeaderFilter, optVal []byte) ([]iptables.Matcher, error) {
 	nflog("set entries: parsing matchers of size %d", len(optVal))
 	var matchers []iptables.Matcher
 	for len(optVal) > 0 {
@@ -506,8 +465,7 @@ func parseMatchers(filter iptables.IPHeaderFilter, optVal []byte) ([]iptables.Ma
 
 		// Get the XTEntryMatch.
 		if len(optVal) < linux.SizeOfXTEntryMatch {
-			log.Warningf("netfilter: optVal has insufficient size for entry match: %d", len(optVal))
-			return nil, syserr.ErrInvalidArgument
+			return nil, fmt.Errorf("optVal has insufficient size for entry match: %d", len(optVal))
 		}
 		var match linux.XTEntryMatch
 		buf := optVal[:linux.SizeOfXTEntryMatch]
@@ -516,45 +474,18 @@ func parseMatchers(filter iptables.IPHeaderFilter, optVal []byte) ([]iptables.Ma
 
 		// Check some invariants.
 		if match.MatchSize < linux.SizeOfXTEntryMatch {
-			log.Warningf("netfilter: match size is too small, must be at least %d", linux.SizeOfXTEntryMatch)
-			return nil, syserr.ErrInvalidArgument
+
+			return nil, fmt.Errorf("match size is too small, must be at least %d", linux.SizeOfXTEntryMatch)
 		}
 		if len(optVal) < int(match.MatchSize) {
-			log.Warningf("netfilter: optVal has insufficient size for match: %d", len(optVal))
-			return nil, syserr.ErrInvalidArgument
+			return nil, fmt.Errorf("optVal has insufficient size for match: %d", len(optVal))
 		}
 
-		buf = optVal[linux.SizeOfXTEntryMatch:match.MatchSize]
-		var matcher iptables.Matcher
-		var err error
-		switch match.Name.String() {
-		case matcherNameUDP:
-			if len(buf) < linux.SizeOfXTUDP {
-				log.Warningf("netfilter: optVal has insufficient size for UDP match: %d", len(optVal))
-				return nil, syserr.ErrInvalidArgument
-			}
-			// For alignment reasons, the match's total size may
-			// exceed what's strictly necessary to hold matchData.
-			var matchData linux.XTUDP
-			binary.Unmarshal(buf[:linux.SizeOfXTUDP], usermem.ByteOrder, &matchData)
-			log.Infof("parseMatchers: parsed XTUDP: %+v", matchData)
-			matcher, err = iptables.NewUDPMatcher(filter, iptables.UDPMatcherParams{
-				SourcePortStart:      matchData.SourcePortStart,
-				SourcePortEnd:        matchData.SourcePortEnd,
-				DestinationPortStart: matchData.DestinationPortStart,
-				DestinationPortEnd:   matchData.DestinationPortEnd,
-				InverseFlags:         matchData.InverseFlags,
-			})
-			if err != nil {
-				log.Warningf("netfilter: failed to create UDP matcher: %v", err)
-				return nil, syserr.ErrInvalidArgument
-			}
-
-		default:
-			log.Warningf("netfilter: unsupported matcher with name %q", match.Name.String())
-			return nil, syserr.ErrInvalidArgument
+		// Parse the specific matcher.
+		matcher, err := unmarshalMatcher(match, filter, optVal[linux.SizeOfXTEntryMatch:match.MatchSize])
+		if err != nil {
+			return nil, fmt.Errorf("failed to create matcher: %v", err)
 		}
-
 		matchers = append(matchers, matcher)
 
 		// TODO(gvisor.dev/issue/170): Check the revision field.
@@ -562,8 +493,7 @@ func parseMatchers(filter iptables.IPHeaderFilter, optVal []byte) ([]iptables.Ma
 	}
 
 	if len(optVal) != 0 {
-		log.Warningf("netfilter: optVal should be exhausted after parsing matchers")
-		return nil, syserr.ErrInvalidArgument
+		return nil, errors.New("optVal should be exhausted after parsing matchers")
 	}
 
 	return matchers, nil
@@ -571,11 +501,10 @@ func parseMatchers(filter iptables.IPHeaderFilter, optVal []byte) ([]iptables.Ma
 
 // parseTarget parses a target from optVal. optVal should contain only the
 // target.
-func parseTarget(optVal []byte) (iptables.Target, *syserr.Error) {
+func parseTarget(optVal []byte) (iptables.Target, error) {
 	nflog("set entries: parsing target of size %d", len(optVal))
 	if len(optVal) < linux.SizeOfXTEntryTarget {
-		log.Warningf("netfilter: optVal has insufficient size for entry target %d", len(optVal))
-		return nil, syserr.ErrInvalidArgument
+		return nil, fmt.Errorf("optVal has insufficient size for entry target %d", len(optVal))
 	}
 	var target linux.XTEntryTarget
 	buf := optVal[:linux.SizeOfXTEntryTarget]
@@ -584,8 +513,7 @@ func parseTarget(optVal []byte) (iptables.Target, *syserr.Error) {
 	case "":
 		// Standard target.
 		if len(optVal) != linux.SizeOfXTStandardTarget {
-			log.Warningf("netfilter.SetEntries: optVal has wrong size for standard target %d", len(optVal))
-			return nil, syserr.ErrInvalidArgument
+			return nil, fmt.Errorf("optVal has wrong size for standard target %d", len(optVal))
 		}
 		var standardTarget linux.XTStandardTarget
 		buf = optVal[:linux.SizeOfXTStandardTarget]
@@ -601,15 +529,13 @@ func parseTarget(optVal []byte) (iptables.Target, *syserr.Error) {
 		case iptables.Drop:
 			return iptables.UnconditionalDropTarget{}, nil
 		default:
-			log.Warningf("Unknown verdict: %v", verdict)
-			return nil, syserr.ErrInvalidArgument
+			return nil, fmt.Errorf("Unknown verdict: %v", verdict)
 		}
 
 	case errorTargetName:
 		// Error target.
 		if len(optVal) != linux.SizeOfXTErrorTarget {
-			log.Infof("netfilter.SetEntries: optVal has insufficient size for error target %d", len(optVal))
-			return nil, syserr.ErrInvalidArgument
+			return nil, fmt.Errorf("optVal has insufficient size for error target %d", len(optVal))
 		}
 		var errorTarget linux.XTErrorTarget
 		buf = optVal[:linux.SizeOfXTErrorTarget]
@@ -626,20 +552,17 @@ func parseTarget(optVal []byte) (iptables.Target, *syserr.Error) {
 		case errorTargetName:
 			return iptables.ErrorTarget{}, nil
 		default:
-			log.Infof("Unknown error target %q doesn't exist or isn't supported yet.", errorTarget.Name.String())
-			return nil, syserr.ErrInvalidArgument
+			return nil, fmt.Errorf("unknown error target %q doesn't exist or isn't supported yet.", errorTarget.Name.String())
 		}
 	}
 
 	// Unknown target.
-	log.Infof("Unknown target %q doesn't exist or isn't supported yet.", target.Name.String())
-	return nil, syserr.ErrInvalidArgument
+	return nil, fmt.Errorf("unknown target %q doesn't exist or isn't supported yet.", target.Name.String())
 }
 
-func filterFromIPTIP(iptip linux.IPTIP) (iptables.IPHeaderFilter, *syserr.Error) {
+func filterFromIPTIP(iptip linux.IPTIP) (iptables.IPHeaderFilter, error) {
 	if containsUnsupportedFields(iptip) {
-		log.Warningf("netfilter: unsupported fields in struct iptip: %+v", iptip)
-		return iptables.IPHeaderFilter{}, syserr.ErrInvalidArgument
+		return iptables.IPHeaderFilter{}, fmt.Errorf("unsupported fields in struct iptip: %+v", iptip)
 	}
 	return iptables.IPHeaderFilter{
 		Protocol: tcpip.TransportProtocolNumber(iptip.Protocol),
@@ -677,8 +600,3 @@ func hookFromLinux(hook int) iptables.Hook {
 	}
 	panic(fmt.Sprintf("Unknown hook %d does not correspond to a builtin chain", hook))
 }
-
-// alignUp rounds a length up to an alignment. align must be a power of 2.
-func alignUp(length int, align uint) int {
-	return (length + int(align) - 1) & ^(int(align) - 1)
-}

pkg/sentry/socket/netfilter/tcp_matcher.go

@@ -0,0 +1,143 @@
+// Copyright 2020 The gVisor Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package netfilter
+
+import (
+	"fmt"
+
+	"gvisor.dev/gvisor/pkg/abi/linux"
+	"gvisor.dev/gvisor/pkg/binary"
+	"gvisor.dev/gvisor/pkg/tcpip"
+	"gvisor.dev/gvisor/pkg/tcpip/header"
+	"gvisor.dev/gvisor/pkg/tcpip/iptables"
+	"gvisor.dev/gvisor/pkg/usermem"
+)
+
+const matcherNameTCP = "tcp"
+
+func init() {
+	registerMatchMaker(tcpMarshaler{})
+}
+
+// tcpMarshaler implements matchMaker for TCP matching.
+type tcpMarshaler struct{}
+
+// name implements matchMaker.name.
+func (tcpMarshaler) name() string {
+	return matcherNameTCP
+}
+
+// marshal implements matchMaker.marshal.
+func (tcpMarshaler) marshal(mr iptables.Matcher) []byte {
+	matcher := mr.(*TCPMatcher)
+	xttcp := linux.XTTCP{
+		SourcePortStart:      matcher.sourcePortStart,
+		SourcePortEnd:        matcher.sourcePortEnd,
+		DestinationPortStart: matcher.destinationPortStart,
+		DestinationPortEnd:   matcher.destinationPortEnd,
+	}
+	buf := make([]byte, 0, linux.SizeOfXTTCP)
+	return marshalEntryMatch(matcherNameTCP, binary.Marshal(buf, usermem.ByteOrder, xttcp))
+}
+
+// unmarshal implements matchMaker.unmarshal.
+func (tcpMarshaler) unmarshal(buf []byte, filter iptables.IPHeaderFilter) (iptables.Matcher, error) {
+	if len(buf) < linux.SizeOfXTTCP {
+		return nil, fmt.Errorf("buf has insufficient size for TCP match: %d", len(buf))
+	}
+
+	// For alignment reasons, the match's total size may
+	// exceed what's strictly necessary to hold matchData.
+	var matchData linux.XTTCP
+	binary.Unmarshal(buf[:linux.SizeOfXTTCP], usermem.ByteOrder, &matchData)
+	nflog("parseMatchers: parsed XTTCP: %+v", matchData)
+
+	if matchData.Option != 0 ||
+		matchData.FlagMask != 0 ||
+		matchData.FlagCompare != 0 ||
+		matchData.InverseFlags != 0 {
+		return nil, fmt.Errorf("unsupported TCP matcher flags set")
+	}
+
+	if filter.Protocol != header.TCPProtocolNumber {
+		return nil, fmt.Errorf("TCP matching is only valid for protocol %d.", header.TCPProtocolNumber)
+	}
+
+	return &TCPMatcher{
+		sourcePortStart:      matchData.SourcePortStart,
+		sourcePortEnd:        matchData.SourcePortEnd,
+		destinationPortStart: matchData.DestinationPortStart,
+		destinationPortEnd:   matchData.DestinationPortEnd,
+	}, nil
+}
+
+// TCPMatcher matches TCP packets and their headers. It implements Matcher.
+type TCPMatcher struct {
+	sourcePortStart      uint16
+	sourcePortEnd        uint16
+	destinationPortStart uint16
+	destinationPortEnd   uint16
+}
+
+// Name implements Matcher.Name.
+func (*TCPMatcher) Name() string {
+	return matcherNameTCP
+}
+
+// Match implements Matcher.Match.
+func (tm *TCPMatcher) Match(hook iptables.Hook, pkt tcpip.PacketBuffer, interfaceName string) (bool, bool) {
+	netHeader := header.IPv4(pkt.NetworkHeader)
+
+	if netHeader.TransportProtocol() != header.TCPProtocolNumber {
+		return false, false
+	}
+
+	// We dont't match fragments.
+	if frag := netHeader.FragmentOffset(); frag != 0 {
+		if frag == 1 {
+			return false, true
+		}
+		return false, false
+	}
+
+	// Now we need the transport header. However, this may not have been set
+	// yet.
+	// TODO(gvisor.dev/issue/170): Parsing the transport header should
+	// ultimately be moved into the iptables.Check codepath as matchers are
+	// added.
+	var tcpHeader header.TCP
+	if pkt.TransportHeader != nil {
+		tcpHeader = header.TCP(pkt.TransportHeader)
+	} else {
+		// The TCP header hasn't been parsed yet. We have to do it here.
+		if len(pkt.Data.First()) < header.TCPMinimumSize {
+			// There's no valid TCP header here, so we hotdrop the
+			// packet.
+			return false, true
+		}
+		tcpHeader = header.TCP(pkt.Data.First())
+	}
+
+	// Check whether the source and destination ports are within the
+	// matching range.
+	if sourcePort := tcpHeader.SourcePort(); sourcePort < tm.sourcePortStart || tm.sourcePortEnd < sourcePort {
+		return false, false
+	}
+	if destinationPort := tcpHeader.DestinationPort(); destinationPort < tm.destinationPortStart || tm.destinationPortEnd < destinationPort {
+		return false, false
+	}
+
+	return true, false
+}

pkg/sentry/socket/netfilter/udp_matcher.go

@@ -12,56 +12,89 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package iptables
+package netfilter
 
 import (
 	"fmt"
 
-	"gvisor.dev/gvisor/pkg/log"
+	"gvisor.dev/gvisor/pkg/abi/linux"
+	"gvisor.dev/gvisor/pkg/binary"
 	"gvisor.dev/gvisor/pkg/tcpip"
 	"gvisor.dev/gvisor/pkg/tcpip/header"
+	"gvisor.dev/gvisor/pkg/tcpip/iptables"
+	"gvisor.dev/gvisor/pkg/usermem"
 )
 
-// TODO(gvisor.dev/issue/170): The following per-matcher params should be
-// supported:
-// - Table name
-// - Match size
-// - User size
-// - Hooks
-// - Proto
-// - Family
+const matcherNameUDP = "udp"
 
-// UDPMatcher matches UDP packets and their headers. It implements Matcher.
-type UDPMatcher struct {
-	Data UDPMatcherParams
+func init() {
+	registerMatchMaker(udpMarshaler{})
 }
 
-// UDPMatcherParams are the parameters used to create a UDPMatcher.
-type UDPMatcherParams struct {
-	SourcePortStart      uint16
-	SourcePortEnd        uint16
-	DestinationPortStart uint16
-	DestinationPortEnd   uint16
-	InverseFlags         uint8
+// udpMarshaler implements matchMaker for UDP matching.
+type udpMarshaler struct{}
+
+// name implements matchMaker.name.
+func (udpMarshaler) name() string {
+	return matcherNameUDP
 }
 
-// NewUDPMatcher returns a new instance of UDPMatcher.
-func NewUDPMatcher(filter IPHeaderFilter, data UDPMatcherParams) (Matcher, error) {
-	log.Infof("Adding rule with UDPMatcherParams: %+v", data)
+// marshal implements matchMaker.marshal.
+func (udpMarshaler) marshal(mr iptables.Matcher) []byte {
+	matcher := mr.(*UDPMatcher)
+	xtudp := linux.XTUDP{
+		SourcePortStart:      matcher.sourcePortStart,
+		SourcePortEnd:        matcher.sourcePortEnd,
+		DestinationPortStart: matcher.destinationPortStart,
+		DestinationPortEnd:   matcher.destinationPortEnd,
+	}
+	buf := make([]byte, 0, linux.SizeOfXTUDP)
+	return marshalEntryMatch(matcherNameUDP, binary.Marshal(buf, usermem.ByteOrder, xtudp))
+}
 
-	if data.InverseFlags != 0 {
+// unmarshal implements matchMaker.unmarshal.
+func (udpMarshaler) unmarshal(buf []byte, filter iptables.IPHeaderFilter) (iptables.Matcher, error) {
+	if len(buf) < linux.SizeOfXTUDP {
+		return nil, fmt.Errorf("buf has insufficient size for UDP match: %d", len(buf))
+	}
+
+	// For alignment reasons, the match's total size may exceed what's
+	// strictly necessary to hold matchData.
+	var matchData linux.XTUDP
+	binary.Unmarshal(buf[:linux.SizeOfXTUDP], usermem.ByteOrder, &matchData)
+	nflog("parseMatchers: parsed XTUDP: %+v", matchData)
+
+	if matchData.InverseFlags != 0 {
 		return nil, fmt.Errorf("unsupported UDP matcher inverse flags set")
 	}
 
 	if filter.Protocol != header.UDPProtocolNumber {
-		return nil, fmt.Errorf("UDP matching is only valid for protocol %d", header.UDPProtocolNumber)
+		return nil, fmt.Errorf("UDP matching is only valid for protocol %d.", header.UDPProtocolNumber)
 	}
 
-	return &UDPMatcher{Data: data}, nil
+	return &UDPMatcher{
+		sourcePortStart:      matchData.SourcePortStart,
+		sourcePortEnd:        matchData.SourcePortEnd,
+		destinationPortStart: matchData.DestinationPortStart,
+		destinationPortEnd:   matchData.DestinationPortEnd,
+	}, nil
+}
+
+// UDPMatcher matches UDP packets and their headers. It implements Matcher.
+type UDPMatcher struct {
+	sourcePortStart      uint16
+	sourcePortEnd        uint16
+	destinationPortStart uint16
+	destinationPortEnd   uint16
+}
+
+// Name implements Matcher.Name.
+func (*UDPMatcher) Name() string {
+	return matcherNameUDP
 }
 
 // Match implements Matcher.Match.
-func (um *UDPMatcher) Match(hook Hook, pkt tcpip.PacketBuffer, interfaceName string) (bool, bool) {
+func (um *UDPMatcher) Match(hook iptables.Hook, pkt tcpip.PacketBuffer, interfaceName string) (bool, bool) {
 	netHeader := header.IPv4(pkt.NetworkHeader)
 
 	// TODO(gvisor.dev/issue/170): Proto checks should ultimately be moved
@@ -98,10 +131,10 @@ func (um *UDPMatcher) Match(hook Hook, pkt tcpip.PacketBuffer, interfaceName str
 
 	// Check whether the source and destination ports are within the
 	// matching range.
-	if sourcePort := udpHeader.SourcePort(); sourcePort < um.Data.SourcePortStart || um.Data.SourcePortEnd < sourcePort {
+	if sourcePort := udpHeader.SourcePort(); sourcePort < um.sourcePortStart || um.sourcePortEnd < sourcePort {
 		return false, false
 	}
-	if destinationPort := udpHeader.DestinationPort(); destinationPort < um.Data.DestinationPortStart || um.Data.DestinationPortEnd < destinationPort {
+	if destinationPort := udpHeader.DestinationPort(); destinationPort < um.destinationPortStart || um.destinationPortEnd < destinationPort {
 		return false, false
 	}
 

pkg/tcpip/iptables/BUILD

@@ -8,7 +8,6 @@ go_library(
         "iptables.go",
         "targets.go",
         "types.go",
-        "udp_matcher.go",
     ],
     visibility = ["//visibility:public"],
     deps = [

pkg/tcpip/iptables/types.go

@@ -171,6 +171,9 @@ type IPHeaderFilter struct {
 
 // A Matcher is the interface for matching packets.
 type Matcher interface {
+	// Name returns the name of the Matcher.
+	Name() string
+
 	// Match returns whether the packet matches and whether the packet
 	// should be "hotdropped", i.e. dropped immediately. This is usually
 	// used for suspicious packets.