From 87225fad2a468e1516784f13abe8bb946d0172c6 Mon Sep 17 00:00:00 2001
From: Kevin Krakauer <krakauer@google.com>
Date: Mon, 23 Mar 2020 14:33:20 -0700
Subject: iptables: check for truly unconditional rules

We weren't properly checking whether the inserted default rule was
unconditional.
---
 pkg/sentry/socket/netfilter/netfilter.go | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/pkg/sentry/socket/netfilter/netfilter.go b/pkg/sentry/socket/netfilter/netfilter.go
index 41a1ce031..789bb94c8 100644
--- a/pkg/sentry/socket/netfilter/netfilter.go
+++ b/pkg/sentry/socket/netfilter/netfilter.go
@@ -59,6 +59,13 @@ type metadata struct {
 // developing iptables, but can pollute sentry logs otherwise.
 const enableLogging = false
 
+// emptyFilter is for comparison with a rule's filters to determine whether it
+// is also empty. It is immutable.
+var emptyFilter = stack.IPHeaderFilter{
+	Dst:     "\x00\x00\x00\x00",
+	DstMask: "\x00\x00\x00\x00",
+}
+
 // nflog logs messages related to the writing and reading of iptables.
 func nflog(format string, args ...interface{}) {
 	if enableLogging && log.IsLogging(log.Debug) {
@@ -484,7 +491,7 @@ func SetEntries(stk *stack.Stack, optVal []byte) *syserr.Error {
 				}
 				if offset == replace.Underflow[hook] {
 					if !validUnderflow(table.Rules[ruleIdx]) {
-						nflog("underflow for hook %d isn't an unconditional ACCEPT or DROP")
+						nflog("underflow for hook %d isn't an unconditional ACCEPT or DROP", ruleIdx)
 						return syserr.ErrInvalidArgument
 					}
 					table.Underflows[hk] = ruleIdx
@@ -547,7 +554,7 @@ func SetEntries(stk *stack.Stack, optVal []byte) *syserr.Error {
 	// make sure all other chains point to ACCEPT rules.
 	for hook, ruleIdx := range table.BuiltinChains {
 		if hook == stack.Forward || hook == stack.Postrouting {
-			if _, ok := table.Rules[ruleIdx].Target.(stack.AcceptTarget); !ok {
+			if !isUnconditionalAccept(table.Rules[ruleIdx]) {
 				nflog("hook %d is unsupported.", hook)
 				return syserr.ErrInvalidArgument
 			}
@@ -776,6 +783,9 @@ func validUnderflow(rule stack.Rule) bool {
 	if len(rule.Matchers) != 0 {
 		return false
 	}
+	if rule.Filter != emptyFilter {
+		return false
+	}
 	switch rule.Target.(type) {
 	case stack.AcceptTarget, stack.DropTarget:
 		return true
@@ -784,6 +794,14 @@ func validUnderflow(rule stack.Rule) bool {
 	}
 }
 
+func isUnconditionalAccept(rule stack.Rule) bool {
+	if !validUnderflow(rule) {
+		return false
+	}
+	_, ok := rule.Target.(stack.AcceptTarget)
+	return ok
+}
+
 func hookFromLinux(hook int) stack.Hook {
 	switch hook {
 	case linux.NF_INET_PRE_ROUTING:
-- 
cgit v1.2.3