Don't read forwarding from netstack in sentry

https://www.kernel.org/doc/Documentation/networking/ip-sysctl.txt:
  /proc/sys/net/ipv4/* Variables:

  ip_forward - BOOLEAN
    0 - disabled (default)
    not 0 - enabled

    Forward Packets between interfaces.

    This variable is special, its change resets all configuration
    parameters to their default state (RFC1122 for hosts, RFC1812
    for routers)

/proc/sys/net/ipv4/ip_forward only does work when its value is changed
and always returns the last written value. The last written value may
not reflect the current state of the netstack (e.g. when `ip_forward`
was written a value of "1" then disable forwarding on an interface)
so there is no need for sentry to probe netstack to get the current
forwarding state of interfaces.

```
~$ cat /proc/sys/net/ipv4/ip_forward
0
~$ sudo bash -c "echo 1 > /proc/sys/net/ipv4/ip_forward"
~$ cat /proc/sys/net/ipv4/ip_forward
1
~$ sudo sysctl -a | grep ipv4 | grep forward
net.ipv4.conf.all.forwarding = 1
net.ipv4.conf.default.forwarding = 1
net.ipv4.conf.eno1.forwarding = 1
net.ipv4.conf.lo.forwarding = 1
net.ipv4.conf.wlp1s0.forwarding = 1
net.ipv4.ip_forward = 1
net.ipv4.ip_forward_update_priority = 1
net.ipv4.ip_forward_use_pmtu = 0
~$ sudo sysctl -w net.ipv4.conf.wlp1s0.forwarding=0
net.ipv4.conf.wlp1s0.forwarding = 0
~$ sudo sysctl -a | grep ipv4 | grep forward
net.ipv4.conf.all.forwarding = 1
net.ipv4.conf.default.forwarding = 1
net.ipv4.conf.eno1.forwarding = 1
net.ipv4.conf.lo.forwarding = 1
net.ipv4.conf.wlp1s0.forwarding = 0
net.ipv4.ip_forward = 1
net.ipv4.ip_forward_update_priority = 1
net.ipv4.ip_forward_use_pmtu = 0
~$ cat /proc/sys/net/ipv4/ip_forward
1
~$ sudo bash -c "echo 1 > /proc/sys/net/ipv4/ip_forward"
~$ sudo sysctl -a | grep ipv4 | grep forward
net.ipv4.conf.all.forwarding = 1
net.ipv4.conf.default.forwarding = 1
net.ipv4.conf.eno1.forwarding = 1
net.ipv4.conf.lo.forwarding = 1
net.ipv4.conf.wlp1s0.forwarding = 0
net.ipv4.ip_forward = 1
net.ipv4.ip_forward_update_priority = 1
net.ipv4.ip_forward_use_pmtu = 0
~$ sudo bash -c "echo 0 > /proc/sys/net/ipv4/ip_forward"
~$ sudo sysctl -a | grep ipv4 | grep forward
sysctl: unable to open directory "/proc/sys/fs/binfmt_misc/"
net.ipv4.conf.all.forwarding = 0
net.ipv4.conf.default.forwarding = 0
net.ipv4.conf.eno1.forwarding = 0
net.ipv4.conf.lo.forwarding = 0
net.ipv4.conf.wlp1s0.forwarding = 0
net.ipv4.ip_forward = 0
net.ipv4.ip_forward_update_priority = 1
net.ipv4.ip_forward_use_pmtu = 0
~$ cat /proc/sys/net/ipv4/ip_forward
0
```

In the above example we can see that writing "1" to
/proc/sys/net/ipv4/ip_forward configures the stack to be a router (all
interfaces are configured to enable forwarding). However, if we manually
update an interace (`wlp1s0`) to not forward packets,
/proc/sys/net/ipv4/ip_forward continues to return the last written value
of "1", even though not all interfaces will forward packets.

Also note that writing the same value twice has no effect; work is
performed iff the value changes.

This change also removes the 'unset' state from sentry's ip forwarding
data structures as an 'unset' ip forwarding value is the same as leaving
forwarding disabled as the stack is always brought up with forwarding
initially disabled; disabling forwarding on a newly created stack is a
no-op.

PiperOrigin-RevId: 373853106

9 files changed, 12 insertions, 74 deletions

diff --git a/pkg/sentry/fs/proc/sys_net.go b/pkg/sentry/fs/proc/sys_net.go
index 1d09afdd7..4893af56b 100644
--- a/pkg/sentry/fs/proc/sys_net.go
+++ b/pkg/sentry/fs/proc/sys_net.go
@@ -403,7 +403,7 @@ type ipForwarding struct {
 	// enabled stores the IPv4 forwarding state on save.
 	// We must save/restore this here, since a netstack instance
 	// is created on restore.
-	enabled *bool
+	enabled bool
 }
 
 func newIPForwardingInode(ctx context.Context, msrc *fs.MountSource, s inet.Stack) *fs.Inode {
@@ -461,13 +461,8 @@ func (f *ipForwardingFile) Read(ctx context.Context, _ *fs.File, dst usermem.IOS
 		return 0, io.EOF
 	}
 
-	if f.ipf.enabled == nil {
-		enabled := f.stack.Forwarding(ipv4.ProtocolNumber)
-		f.ipf.enabled = &enabled
-	}
-
 	val := "0\n"
-	if *f.ipf.enabled {
+	if f.ipf.enabled {
 		// Technically, this is not quite compatible with Linux. Linux
 		// stores these as an integer, so if you write "2" into
 		// ip_forward, you should get 2 back.
@@ -494,11 +489,8 @@ func (f *ipForwardingFile) Write(ctx context.Context, _ *fs.File, src usermem.IO
 	if err != nil {
 		return n, err
 	}
-	if f.ipf.enabled == nil {
-		f.ipf.enabled = new(bool)
-	}
-	*f.ipf.enabled = v != 0
-	return n, f.stack.SetForwarding(ipv4.ProtocolNumber, *f.ipf.enabled)
+	f.ipf.enabled = v != 0
+	return n, f.stack.SetForwarding(ipv4.ProtocolNumber, f.ipf.enabled)
 }
 
 // portRangeInode implements fs.InodeOperations. It provides and allows
diff --git a/pkg/sentry/fs/proc/sys_net_state.go b/pkg/sentry/fs/proc/sys_net_state.go
index 4cb4741af..51d2be647 100644
--- a/pkg/sentry/fs/proc/sys_net_state.go
+++ b/pkg/sentry/fs/proc/sys_net_state.go
@@ -47,9 +47,7 @@ func (s *tcpSack) afterLoad() {
 
 // afterLoad is invoked by stateify.
 func (ipf *ipForwarding) afterLoad() {
-	if ipf.enabled != nil {
-		if err := ipf.stack.SetForwarding(ipv4.ProtocolNumber, *ipf.enabled); err != nil {
-			panic(fmt.Sprintf("failed to set IPv4 forwarding [%v]: %v", *ipf.enabled, err))
-		}
+	if err := ipf.stack.SetForwarding(ipv4.ProtocolNumber, ipf.enabled); err != nil {
+		panic(fmt.Sprintf("ipf.stack.SetForwarding(%d, %t): %s", ipv4.ProtocolNumber, ipf.enabled, err))
 	}
 }
diff --git a/pkg/sentry/fsimpl/proc/tasks_sys.go b/pkg/sentry/fsimpl/proc/tasks_sys.go
index 9b14dd6b9..88ab49048 100644
--- a/pkg/sentry/fsimpl/proc/tasks_sys.go
+++ b/pkg/sentry/fsimpl/proc/tasks_sys.go
@@ -365,27 +365,22 @@ func (d *tcpMemData) writeSizeLocked(size inet.TCPBufferSize) error {
 }
 
 // ipForwarding implements vfs.WritableDynamicBytesSource for
-// /proc/sys/net/ipv4/ip_forwarding.
+// /proc/sys/net/ipv4/ip_forward.
 //
 // +stateify savable
 type ipForwarding struct {
 	kernfs.DynamicBytesFile
 
 	stack   inet.Stack `state:"wait"`
-	enabled *bool
+	enabled bool
 }
 
 var _ vfs.WritableDynamicBytesSource = (*ipForwarding)(nil)
 
 // Generate implements vfs.DynamicBytesSource.Generate.
 func (ipf *ipForwarding) Generate(ctx context.Context, buf *bytes.Buffer) error {
-	if ipf.enabled == nil {
-		enabled := ipf.stack.Forwarding(ipv4.ProtocolNumber)
-		ipf.enabled = &enabled
-	}
-
 	val := "0\n"
-	if *ipf.enabled {
+	if ipf.enabled {
 		// Technically, this is not quite compatible with Linux. Linux stores these
 		// as an integer, so if you write "2" into tcp_sack, you should get 2 back.
 		// Tough luck.
@@ -414,11 +409,8 @@ func (ipf *ipForwarding) Write(ctx context.Context, src usermem.IOSequence, offs
 	if err != nil {
 		return 0, err
 	}
-	if ipf.enabled == nil {
-		ipf.enabled = new(bool)
-	}
-	*ipf.enabled = v != 0
-	if err := ipf.stack.SetForwarding(ipv4.ProtocolNumber, *ipf.enabled); err != nil {
+	ipf.enabled = v != 0
+	if err := ipf.stack.SetForwarding(ipv4.ProtocolNumber, ipf.enabled); err != nil {
 		return 0, err
 	}
 	return n, nil
diff --git a/pkg/sentry/fsimpl/proc/tasks_sys_test.go b/pkg/sentry/fsimpl/proc/tasks_sys_test.go
index 6cee22823..19b012f7d 100644
--- a/pkg/sentry/fsimpl/proc/tasks_sys_test.go
+++ b/pkg/sentry/fsimpl/proc/tasks_sys_test.go
@@ -132,7 +132,7 @@ func TestConfigureIPForwarding(t *testing.T) {
 		t.Run(c.comment, func(t *testing.T) {
 			s.IPForwarding = c.initial
 
-			file := &ipForwarding{stack: s, enabled: &c.initial}
+			file := &ipForwarding{stack: s, enabled: c.initial}
 
 			// Write the values.
 			src := usermem.BytesIOSequence([]byte(c.str))
diff --git a/pkg/sentry/inet/inet.go b/pkg/sentry/inet/inet.go
index 6b71bd3a9..80dda1559 100644
--- a/pkg/sentry/inet/inet.go
+++ b/pkg/sentry/inet/inet.go
@@ -88,9 +88,6 @@ type Stack interface {
 	// for restoring a stack after a save.
 	RestoreCleanupEndpoints([]stack.TransportEndpoint)
 
-	// Forwarding returns if packet forwarding between NICs is enabled.
-	Forwarding(protocol tcpip.NetworkProtocolNumber) bool
-
 	// SetForwarding enables or disables packet forwarding between NICs.
 	SetForwarding(protocol tcpip.NetworkProtocolNumber, enable bool) error
 
diff --git a/pkg/sentry/inet/test_stack.go b/pkg/sentry/inet/test_stack.go
index 03e2608c2..218d9dafc 100644
--- a/pkg/sentry/inet/test_stack.go
+++ b/pkg/sentry/inet/test_stack.go
@@ -154,11 +154,6 @@ func (s *TestStack) CleanupEndpoints() []stack.TransportEndpoint {
 // RestoreCleanupEndpoints implements inet.Stack.RestoreCleanupEndpoints.
 func (s *TestStack) RestoreCleanupEndpoints([]stack.TransportEndpoint) {}
 
-// Forwarding implements inet.Stack.Forwarding.
-func (s *TestStack) Forwarding(protocol tcpip.NetworkProtocolNumber) bool {
-	return s.IPForwarding
-}
-
 // SetForwarding implements inet.Stack.SetForwarding.
 func (s *TestStack) SetForwarding(protocol tcpip.NetworkProtocolNumber, enable bool) error {
 	s.IPForwarding = enable
diff --git a/pkg/sentry/socket/hostinet/BUILD b/pkg/sentry/socket/hostinet/BUILD
index 2e3064565..3c6511ead 100644
--- a/pkg/sentry/socket/hostinet/BUILD
+++ b/pkg/sentry/socket/hostinet/BUILD
@@ -39,8 +39,6 @@ go_library(
         "//pkg/syserr",
         "//pkg/syserror",
         "//pkg/tcpip",
-        "//pkg/tcpip/network/ipv4",
-        "//pkg/tcpip/network/ipv6",
         "//pkg/tcpip/stack",
         "//pkg/usermem",
         "//pkg/waiter",
diff --git a/pkg/sentry/socket/hostinet/stack.go b/pkg/sentry/socket/hostinet/stack.go
index 393a1ab3a..cbb1e905d 100644
--- a/pkg/sentry/socket/hostinet/stack.go
+++ b/pkg/sentry/socket/hostinet/stack.go
@@ -35,8 +35,6 @@ import (
 	"gvisor.dev/gvisor/pkg/syserr"
 	"gvisor.dev/gvisor/pkg/syserror"
 	"gvisor.dev/gvisor/pkg/tcpip"
-	"gvisor.dev/gvisor/pkg/tcpip/network/ipv4"
-	"gvisor.dev/gvisor/pkg/tcpip/network/ipv6"
 	"gvisor.dev/gvisor/pkg/tcpip/stack"
 	"gvisor.dev/gvisor/pkg/usermem"
 )
@@ -66,8 +64,6 @@ type Stack struct {
 	tcpSACKEnabled bool
 	netDevFile     *os.File
 	netSNMPFile    *os.File
-	ipv4Forwarding bool
-	ipv6Forwarding bool
 }
 
 // NewStack returns an empty Stack containing no configuration.
@@ -127,13 +123,6 @@ func (s *Stack) Configure() error {
 		s.netSNMPFile = f
 	}
 
-	s.ipv6Forwarding = false
-	if ipForwarding, err := ioutil.ReadFile("/proc/sys/net/ipv6/conf/all/forwarding"); err == nil {
-		s.ipv6Forwarding = strings.TrimSpace(string(ipForwarding)) != "0"
-	} else {
-		log.Warningf("Failed to read if ipv6 forwarding is enabled, setting to false")
-	}
-
 	return nil
 }
 
@@ -492,19 +481,6 @@ func (s *Stack) CleanupEndpoints() []stack.TransportEndpoint { return nil }
 // RestoreCleanupEndpoints implements inet.Stack.RestoreCleanupEndpoints.
 func (s *Stack) RestoreCleanupEndpoints([]stack.TransportEndpoint) {}
 
-// Forwarding implements inet.Stack.Forwarding.
-func (s *Stack) Forwarding(protocol tcpip.NetworkProtocolNumber) bool {
-	switch protocol {
-	case ipv4.ProtocolNumber:
-		return s.ipv4Forwarding
-	case ipv6.ProtocolNumber:
-		return s.ipv6Forwarding
-	default:
-		log.Warningf("Forwarding(%v) failed: unsupported protocol", protocol)
-		return false
-	}
-}
-
 // SetForwarding implements inet.Stack.SetForwarding.
 func (s *Stack) SetForwarding(tcpip.NetworkProtocolNumber, bool) error {
 	return syserror.EACCES
diff --git a/pkg/sentry/socket/netstack/stack.go b/pkg/sentry/socket/netstack/stack.go
index 9cc1c57d7..eef5e6519 100644
--- a/pkg/sentry/socket/netstack/stack.go
+++ b/pkg/sentry/socket/netstack/stack.go
@@ -458,16 +458,6 @@ func (s *Stack) RestoreCleanupEndpoints(es []stack.TransportEndpoint) {
 	s.Stack.RestoreCleanupEndpoints(es)
 }
 
-// Forwarding implements inet.Stack.Forwarding.
-func (s *Stack) Forwarding(protocol tcpip.NetworkProtocolNumber) bool {
-	switch protocol {
-	case ipv4.ProtocolNumber, ipv6.ProtocolNumber:
-		return s.Stack.Forwarding(protocol)
-	default:
-		panic(fmt.Sprintf("Forwarding(%v) failed: unsupported protocol", protocol))
-	}
-}
-
 // SetForwarding implements inet.Stack.SetForwarding.
 func (s *Stack) SetForwarding(protocol tcpip.NetworkProtocolNumber, enable bool) error {
 	if err := s.Stack.SetForwardingDefaultAndAllNICs(protocol, enable); err != nil {