Merge release-20200211.0-13-g16308b9 (automated)

4 files changed, 170 insertions, 110 deletions

diff --git a/pkg/sentry/socket/netfilter/netfilter.go b/pkg/sentry/socket/netfilter/netfilter.go
index ea02627de..3fc80e0de 100644
--- a/pkg/sentry/socket/netfilter/netfilter.go
+++ b/pkg/sentry/socket/netfilter/netfilter.go
@@ -50,7 +50,9 @@ type metadata struct {
 
 // nflog logs messages related to the writing and reading of iptables.
 func nflog(format string, args ...interface{}) {
-	log.Infof("netfilter: "+format, args...)
+	if log.IsLogging(log.Debug) {
+		log.Debugf("netfilter: "+format, args...)
+	}
 }
 
 // GetInfo returns information about iptables.
@@ -227,19 +229,23 @@ func convertNetstackToBinary(tablename string, table iptables.Table) (linux.Kern
 }
 
 func marshalTarget(target iptables.Target) []byte {
-	switch target.(type) {
-	case iptables.UnconditionalAcceptTarget:
-		return marshalStandardTarget(iptables.Accept)
-	case iptables.UnconditionalDropTarget:
-		return marshalStandardTarget(iptables.Drop)
+	switch tg := target.(type) {
+	case iptables.AcceptTarget:
+		return marshalStandardTarget(iptables.RuleAccept)
+	case iptables.DropTarget:
+		return marshalStandardTarget(iptables.RuleDrop)
 	case iptables.ErrorTarget:
-		return marshalErrorTarget()
+		return marshalErrorTarget(errorTargetName)
+	case iptables.UserChainTarget:
+		return marshalErrorTarget(tg.Name)
+	case iptables.ReturnTarget:
+		return marshalStandardTarget(iptables.RuleReturn)
 	default:
 		panic(fmt.Errorf("unknown target of type %T", target))
 	}
 }
 
-func marshalStandardTarget(verdict iptables.Verdict) []byte {
+func marshalStandardTarget(verdict iptables.RuleVerdict) []byte {
 	nflog("convert to binary: marshalling standard target with size %d", linux.SizeOfXTStandardTarget)
 
 	// The target's name will be the empty string.
@@ -254,14 +260,14 @@ func marshalStandardTarget(verdict iptables.Verdict) []byte {
 	return binary.Marshal(ret, usermem.ByteOrder, target)
 }
 
-func marshalErrorTarget() []byte {
+func marshalErrorTarget(errorName string) []byte {
 	// This is an error target named error
 	target := linux.XTErrorTarget{
 		Target: linux.XTEntryTarget{
 			TargetSize: linux.SizeOfXTErrorTarget,
 		},
 	}
-	copy(target.Name[:], errorTargetName)
+	copy(target.Name[:], errorName)
 	copy(target.Target.Name[:], errorTargetName)
 
 	ret := make([]byte, 0, linux.SizeOfXTErrorTarget)
@@ -270,38 +276,35 @@ func marshalErrorTarget() []byte {
 
 // translateFromStandardVerdict translates verdicts the same way as the iptables
 // tool.
-func translateFromStandardVerdict(verdict iptables.Verdict) int32 {
+func translateFromStandardVerdict(verdict iptables.RuleVerdict) int32 {
 	switch verdict {
-	case iptables.Accept:
+	case iptables.RuleAccept:
 		return -linux.NF_ACCEPT - 1
-	case iptables.Drop:
+	case iptables.RuleDrop:
 		return -linux.NF_DROP - 1
-	case iptables.Queue:
-		return -linux.NF_QUEUE - 1
-	case iptables.Return:
+	case iptables.RuleReturn:
 		return linux.NF_RETURN
-	case iptables.Jump:
+	default:
 		// TODO(gvisor.dev/issue/170): Support Jump.
-		panic("Jump isn't supported yet")
+		panic(fmt.Sprintf("unknown standard verdict: %d", verdict))
 	}
-	panic(fmt.Sprintf("unknown standard verdict: %d", verdict))
 }
 
-// translateToStandardVerdict translates from the value in a
+// translateToStandardTarget translates from the value in a
 // linux.XTStandardTarget to an iptables.Verdict.
-func translateToStandardVerdict(val int32) (iptables.Verdict, error) {
+func translateToStandardTarget(val int32) (iptables.Target, error) {
 	// TODO(gvisor.dev/issue/170): Support other verdicts.
 	switch val {
 	case -linux.NF_ACCEPT - 1:
-		return iptables.Accept, nil
+		return iptables.AcceptTarget{}, nil
 	case -linux.NF_DROP - 1:
-		return iptables.Drop, nil
+		return iptables.DropTarget{}, nil
 	case -linux.NF_QUEUE - 1:
-		return iptables.Invalid, errors.New("unsupported iptables verdict QUEUE")
+		return nil, errors.New("unsupported iptables verdict QUEUE")
 	case linux.NF_RETURN:
-		return iptables.Invalid, errors.New("unsupported iptables verdict RETURN")
+		return iptables.ReturnTarget{}, nil
 	default:
-		return iptables.Invalid, fmt.Errorf("unknown iptables verdict %d", val)
+		return nil, fmt.Errorf("unknown iptables verdict %d", val)
 	}
 }
 
@@ -411,6 +414,10 @@ func SetEntries(stack *stack.Stack, optVal []byte) *syserr.Error {
 					table.BuiltinChains[hk] = ruleIdx
 				}
 				if offset == replace.Underflow[hook] {
+					if !validUnderflow(table.Rules[ruleIdx]) {
+						nflog("underflow for hook %d isn't an unconditional ACCEPT or DROP.")
+						return syserr.ErrInvalidArgument
+					}
 					table.Underflows[hk] = ruleIdx
 				}
 			}
@@ -425,12 +432,34 @@ func SetEntries(stack *stack.Stack, optVal []byte) *syserr.Error {
 		}
 	}
 
+	// Add the user chains.
+	for ruleIdx, rule := range table.Rules {
+		target, ok := rule.Target.(iptables.UserChainTarget)
+		if !ok {
+			continue
+		}
+
+		// We found a user chain. Before inserting it into the table,
+		// check that:
+		// - There's some other rule after it.
+		// - There are no matchers.
+		if ruleIdx == len(table.Rules)-1 {
+			nflog("user chain must have a rule or default policy.")
+			return syserr.ErrInvalidArgument
+		}
+		if len(table.Rules[ruleIdx].Matchers) != 0 {
+			nflog("user chain's first node must have no matcheres.")
+			return syserr.ErrInvalidArgument
+		}
+		table.UserChains[target.Name] = ruleIdx + 1
+	}
+
 	// TODO(gvisor.dev/issue/170): Support other chains.
 	// Since we only support modifying the INPUT chain right now, make sure
 	// all other chains point to ACCEPT rules.
 	for hook, ruleIdx := range table.BuiltinChains {
 		if hook != iptables.Input {
-			if _, ok := table.Rules[ruleIdx].Target.(iptables.UnconditionalAcceptTarget); !ok {
+			if _, ok := table.Rules[ruleIdx].Target.(iptables.AcceptTarget); !ok {
 				nflog("hook %d is unsupported.", hook)
 				return syserr.ErrInvalidArgument
 			}
@@ -519,18 +548,7 @@ func parseTarget(optVal []byte) (iptables.Target, error) {
 		buf = optVal[:linux.SizeOfXTStandardTarget]
 		binary.Unmarshal(buf, usermem.ByteOrder, &standardTarget)
 
-		verdict, err := translateToStandardVerdict(standardTarget.Verdict)
-		if err != nil {
-			return nil, err
-		}
-		switch verdict {
-		case iptables.Accept:
-			return iptables.UnconditionalAcceptTarget{}, nil
-		case iptables.Drop:
-			return iptables.UnconditionalDropTarget{}, nil
-		default:
-			return nil, fmt.Errorf("Unknown verdict: %v", verdict)
-		}
+		return translateToStandardTarget(standardTarget.Verdict)
 
 	case errorTargetName:
 		// Error target.
@@ -548,11 +566,14 @@ func parseTarget(optVal []byte) (iptables.Target, error) {
 		//   somehow fall through every rule.
 		// * To mark the start of a user defined chain. These
 		//   rules have an error with the name of the chain.
-		switch errorTarget.Name.String() {
+		switch name := errorTarget.Name.String(); name {
 		case errorTargetName:
+			nflog("set entries: error target")
 			return iptables.ErrorTarget{}, nil
 		default:
-			return nil, fmt.Errorf("unknown error target %q doesn't exist or isn't supported yet.", errorTarget.Name.String())
+			// User defined chain.
+			nflog("set entries: user-defined target %q", name)
+			return iptables.UserChainTarget{Name: name}, nil
 		}
 	}
 
@@ -585,6 +606,18 @@ func containsUnsupportedFields(iptip linux.IPTIP) bool {
 		iptip.InverseFlags != 0
 }
 
+func validUnderflow(rule iptables.Rule) bool {
+	if len(rule.Matchers) != 0 {
+		return false
+	}
+	switch rule.Target.(type) {
+	case iptables.AcceptTarget, iptables.DropTarget:
+		return true
+	default:
+		return false
+	}
+}
+
 func hookFromLinux(hook int) iptables.Hook {
 	switch hook {
 	case linux.NF_INET_PRE_ROUTING:
diff --git a/pkg/tcpip/iptables/iptables.go b/pkg/tcpip/iptables/iptables.go
index 1b9485bbd..75a433a3b 100644
--- a/pkg/tcpip/iptables/iptables.go
+++ b/pkg/tcpip/iptables/iptables.go
@@ -52,10 +52,10 @@ func DefaultTables() IPTables {
 		Tables: map[string]Table{
 			TablenameNat: Table{
 				Rules: []Rule{
-					Rule{Target: UnconditionalAcceptTarget{}},
-					Rule{Target: UnconditionalAcceptTarget{}},
-					Rule{Target: UnconditionalAcceptTarget{}},
-					Rule{Target: UnconditionalAcceptTarget{}},
+					Rule{Target: AcceptTarget{}},
+					Rule{Target: AcceptTarget{}},
+					Rule{Target: AcceptTarget{}},
+					Rule{Target: AcceptTarget{}},
 					Rule{Target: ErrorTarget{}},
 				},
 				BuiltinChains: map[Hook]int{
@@ -74,8 +74,8 @@ func DefaultTables() IPTables {
 			},
 			TablenameMangle: Table{
 				Rules: []Rule{
-					Rule{Target: UnconditionalAcceptTarget{}},
-					Rule{Target: UnconditionalAcceptTarget{}},
+					Rule{Target: AcceptTarget{}},
+					Rule{Target: AcceptTarget{}},
 					Rule{Target: ErrorTarget{}},
 				},
 				BuiltinChains: map[Hook]int{
@@ -90,9 +90,9 @@ func DefaultTables() IPTables {
 			},
 			TablenameFilter: Table{
 				Rules: []Rule{
-					Rule{Target: UnconditionalAcceptTarget{}},
-					Rule{Target: UnconditionalAcceptTarget{}},
-					Rule{Target: UnconditionalAcceptTarget{}},
+					Rule{Target: AcceptTarget{}},
+					Rule{Target: AcceptTarget{}},
+					Rule{Target: AcceptTarget{}},
 					Rule{Target: ErrorTarget{}},
 				},
 				BuiltinChains: map[Hook]int{
@@ -149,13 +149,11 @@ func (it *IPTables) Check(hook Hook, pkt tcpip.PacketBuffer) bool {
 	for _, tablename := range it.Priorities[hook] {
 		switch verdict := it.checkTable(hook, pkt, tablename); verdict {
 		// If the table returns Accept, move on to the next table.
-		case Accept:
+		case TableAccept:
 			continue
 		// The Drop verdict is final.
-		case Drop:
+		case TableDrop:
 			return false
-		case Stolen, Queue, Repeat, None, Jump, Return, Continue:
-			panic(fmt.Sprintf("Unimplemented verdict %v.", verdict))
 		default:
 			panic(fmt.Sprintf("Unknown verdict %v.", verdict))
 		}
@@ -166,36 +164,58 @@ func (it *IPTables) Check(hook Hook, pkt tcpip.PacketBuffer) bool {
 }
 
 // Precondition: pkt.NetworkHeader is set.
-func (it *IPTables) checkTable(hook Hook, pkt tcpip.PacketBuffer, tablename string) Verdict {
+func (it *IPTables) checkTable(hook Hook, pkt tcpip.PacketBuffer, tablename string) TableVerdict {
 	// Start from ruleIdx and walk the list of rules until a rule gives us
 	// a verdict.
 	table := it.Tables[tablename]
 	for ruleIdx := table.BuiltinChains[hook]; ruleIdx < len(table.Rules); ruleIdx++ {
 		switch verdict := it.checkRule(hook, pkt, table, ruleIdx); verdict {
-		// In either of these cases, this table is done with the packet.
-		case Accept, Drop:
-			return verdict
-		// Continue traversing the rules of the table.
-		case Continue:
+		case RuleAccept:
+			return TableAccept
+
+		case RuleDrop:
+			return TableDrop
+
+		case RuleContinue:
 			continue
-		case Stolen, Queue, Repeat, None, Jump, Return:
-			panic(fmt.Sprintf("Unimplemented verdict %v.", verdict))
+
+		case RuleReturn:
+			// TODO(gvisor.dev/issue/170): We don't implement jump
+			// yet, so any Return is from a built-in chain. That
+			// means we have to to call the underflow.
+			underflow := table.Rules[table.Underflows[hook]]
+			// Underflow is guaranteed to be an unconditional
+			// ACCEPT or DROP.
+			switch v, _ := underflow.Target.Action(pkt); v {
+			case RuleAccept:
+				return TableAccept
+			case RuleDrop:
+				return TableDrop
+			case RuleContinue, RuleReturn:
+				panic("Underflows should only return RuleAccept or RuleDrop.")
+			default:
+				panic(fmt.Sprintf("Unknown verdict: %d", v))
+			}
+
 		default:
-			panic(fmt.Sprintf("Unknown verdict %v.", verdict))
+			panic(fmt.Sprintf("Unknown verdict: %d", verdict))
 		}
+
 	}
 
-	panic(fmt.Sprintf("Traversed past the entire list of iptables rules in table %q.", tablename))
+	// We got through the entire table without a decision. Default to DROP
+	// for safety.
+	return TableDrop
 }
 
 // Precondition: pk.NetworkHeader is set.
-func (it *IPTables) checkRule(hook Hook, pkt tcpip.PacketBuffer, table Table, ruleIdx int) Verdict {
+func (it *IPTables) checkRule(hook Hook, pkt tcpip.PacketBuffer, table Table, ruleIdx int) RuleVerdict {
 	rule := table.Rules[ruleIdx]
 
 	// First check whether the packet matches the IP header filter.
 	// TODO(gvisor.dev/issue/170): Support other fields of the filter.
 	if rule.Filter.Protocol != 0 && rule.Filter.Protocol != header.IPv4(pkt.NetworkHeader).TransportProtocol() {
-		return Continue
+		return RuleContinue
 	}
 
 	// Go through each rule matcher. If they all match, run
@@ -203,10 +223,10 @@ func (it *IPTables) checkRule(hook Hook, pkt tcpip.PacketBuffer, table Table, ru
 	for _, matcher := range rule.Matchers {
 		matches, hotdrop := matcher.Match(hook, pkt, "")
 		if hotdrop {
-			return Drop
+			return RuleDrop
 		}
 		if !matches {
-			return Continue
+			return RuleContinue
 		}
 	}
 
diff --git a/pkg/tcpip/iptables/targets.go b/pkg/tcpip/iptables/targets.go
index 4dd281371..9fc60cfad 100644
--- a/pkg/tcpip/iptables/targets.go
+++ b/pkg/tcpip/iptables/targets.go
@@ -21,20 +21,20 @@ import (
 	"gvisor.dev/gvisor/pkg/tcpip"
 )
 
-// UnconditionalAcceptTarget accepts all packets.
-type UnconditionalAcceptTarget struct{}
+// AcceptTarget accepts packets.
+type AcceptTarget struct{}
 
 // Action implements Target.Action.
-func (UnconditionalAcceptTarget) Action(packet tcpip.PacketBuffer) (Verdict, string) {
-	return Accept, ""
+func (AcceptTarget) Action(packet tcpip.PacketBuffer) (RuleVerdict, string) {
+	return RuleAccept, ""
 }
 
-// UnconditionalDropTarget denies all packets.
-type UnconditionalDropTarget struct{}
+// DropTarget drops packets.
+type DropTarget struct{}
 
 // Action implements Target.Action.
-func (UnconditionalDropTarget) Action(packet tcpip.PacketBuffer) (Verdict, string) {
-	return Drop, ""
+func (DropTarget) Action(packet tcpip.PacketBuffer) (RuleVerdict, string) {
+	return RuleDrop, ""
 }
 
 // ErrorTarget logs an error and drops the packet. It represents a target that
@@ -42,7 +42,26 @@ func (UnconditionalDropTarget) Action(packet tcpip.PacketBuffer) (Verdict, strin
 type ErrorTarget struct{}
 
 // Action implements Target.Action.
-func (ErrorTarget) Action(packet tcpip.PacketBuffer) (Verdict, string) {
-	log.Warningf("ErrorTarget triggered.")
-	return Drop, ""
+func (ErrorTarget) Action(packet tcpip.PacketBuffer) (RuleVerdict, string) {
+	log.Debugf("ErrorTarget triggered.")
+	return RuleDrop, ""
+}
+
+// UserChainTarget marks a rule as the beginning of a user chain.
+type UserChainTarget struct {
+	Name string
+}
+
+// Action implements Target.Action.
+func (UserChainTarget) Action(tcpip.PacketBuffer) (RuleVerdict, string) {
+	panic("UserChainTarget should never be called.")
+}
+
+// ReturnTarget returns from the current chain. If the chain is a built-in, the
+// hook's underflow should be called.
+type ReturnTarget struct{}
+
+// Action implements Target.Action.
+func (ReturnTarget) Action(tcpip.PacketBuffer) (RuleVerdict, string) {
+	return RuleReturn, ""
 }
diff --git a/pkg/tcpip/iptables/types.go b/pkg/tcpip/iptables/types.go
index 7d593c35c..5735d001b 100644
--- a/pkg/tcpip/iptables/types.go
+++ b/pkg/tcpip/iptables/types.go
@@ -56,44 +56,32 @@ const (
 	NumHooks
 )
 
-// A Verdict is returned by a rule's target to indicate how traversal of rules
-// should (or should not) continue.
-type Verdict int
+// A TableVerdict is what a table decides should be done with a packet.
+type TableVerdict int
 
 const (
-	// Invalid indicates an unkonwn or erroneous verdict.
-	Invalid Verdict = iota
+	// TableAccept indicates the packet should continue through netstack.
+	TableAccept TableVerdict = iota
 
-	// Accept indicates the packet should continue traversing netstack as
-	// normal.
-	Accept
-
-	// Drop inicates the packet should be dropped, stopping traversing
-	// netstack.
-	Drop
-
-	// Stolen indicates the packet was co-opted by the target and should
-	// stop traversing netstack.
-	Stolen
-
-	// Queue indicates the packet should be queued for userspace processing.
-	Queue
+	// TableAccept indicates the packet should be dropped.
+	TableDrop
+)
 
-	// Repeat indicates the packet should re-traverse the chains for the
-	// current hook.
-	Repeat
+// A RuleVerdict is what a rule decides should be done with a packet.
+type RuleVerdict int
 
-	// None indicates no verdict was reached.
-	None
+const (
+	// RuleAccept indicates the packet should continue through netstack.
+	RuleAccept RuleVerdict = iota
 
-	// Jump indicates a jump to another chain.
-	Jump
+	// RuleContinue indicates the packet should continue to the next rule.
+	RuleContinue
 
-	// Continue indicates that traversal should continue at the next rule.
-	Continue
+	// RuleDrop indicates the packet should be dropped.
+	RuleDrop
 
-	// Return indicates that traversal should return to the calling chain.
-	Return
+	// RuleReturn indicates the packet should return to the previous chain.
+	RuleReturn
 )
 
 // IPTables holds all the tables for a netstack.
@@ -187,5 +175,5 @@ type Target interface {
 	// Action takes an action on the packet and returns a verdict on how
 	// traversal should (or should not) continue. If the return value is
 	// Jump, it also returns the name of the chain to jump to.
-	Action(packet tcpip.PacketBuffer) (Verdict, string)
+	Action(packet tcpip.PacketBuffer) (RuleVerdict, string)
 }