diff options
author | Ghanan Gowripalan <ghanan@google.com> | 2021-05-14 13:20:24 -0700 |
---|---|---|
committer | gVisor bot <gvisor-bot@google.com> | 2021-05-14 13:22:58 -0700 |
commit | 600d14f83e1dca4816ab4bba8aa8eb2c90a5c466 (patch) | |
tree | 9d22b4ae646d0b226be2e9299d30bdaff94261be /pkg/sentry/fsimpl | |
parent | 2ac6b76884c7a8c8b788b4c376098ea48b9fe610 (diff) |
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
Diffstat (limited to 'pkg/sentry/fsimpl')
-rw-r--r-- | pkg/sentry/fsimpl/proc/tasks_sys.go | 18 | ||||
-rw-r--r-- | pkg/sentry/fsimpl/proc/tasks_sys_test.go | 2 |
2 files changed, 6 insertions, 14 deletions
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)) |