From 226486eb9ae90a32c3bb6857b9dbc3dec605597c Mon Sep 17 00:00:00 2001 From: Andrew Dunham Date: Tue, 16 Apr 2024 16:06:13 -0400 Subject: [PATCH] net/interfaces: handle removed interfaces in State.Equal This wasn't previously handling the case where an interface in s2 was removed and not present in s1, and would cause the Equal method to incorrectly return that the states were equal. Updates tailscale/corp#19124 Signed-off-by: Andrew Dunham Change-Id: I3af22bc631015d1ddd0a1d01bfdf312161b9532d --- net/interfaces/interfaces.go | 7 +++++++ net/interfaces/interfaces_test.go | 30 ++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/net/interfaces/interfaces.go b/net/interfaces/interfaces.go index 8a45f5673..0682374a0 100644 --- a/net/interfaces/interfaces.go +++ b/net/interfaces/interfaces.go @@ -343,6 +343,13 @@ func (s *State) Equal(s2 *State) bool { s.PAC != s2.PAC { return false } + // If s2 has more interfaces than s, it's not equal. + if len(s.Interface) != len(s2.Interface) || len(s.InterfaceIPs) != len(s2.InterfaceIPs) { + return false + } + // Now that we know that both states have the same number of + // interfaces, we can check each interface in s against s2. If it's not + // present or not exactly equal, then the states are not equal. for iname, i := range s.Interface { i2, ok := s2.Interface[iname] if !ok { diff --git a/net/interfaces/interfaces_test.go b/net/interfaces/interfaces_test.go index 4925600d3..08bfc2b68 100644 --- a/net/interfaces/interfaces_test.go +++ b/net/interfaces/interfaces_test.go @@ -286,6 +286,13 @@ func TestStateString(t *testing.T) { // tests (*State).Equal func TestEqual(t *testing.T) { + pfxs := func(addrs ...string) (ret []netip.Prefix) { + for _, addr := range addrs { + ret = append(ret, netip.MustParsePrefix(addr)) + } + return ret + } + tests := []struct { name string s1, s2 *State @@ -362,6 +369,29 @@ func TestEqual(t *testing.T) { }, want: false, }, + + // See tailscale/corp#19124 + { + name: "interface-removed", + s1: &State{ + InterfaceIPs: map[string][]netip.Prefix{ + "rmnet16": pfxs("2607:1111:2222:3333:4444:5555:6666:7777/64"), + "rmnet17": pfxs("2607:9999:8888:7777:666:5555:4444:3333/64"), + "tun0": pfxs("100.64.1.2/32", "fd7a:115c:a1e0::1/128"), + "v4-rmnet16": pfxs("192.0.0.4/32"), + "wlan0": pfxs("10.0.0.111/24"), // removed below + }, + }, + s2: &State{ + InterfaceIPs: map[string][]netip.Prefix{ + "rmnet16": pfxs("2607:1111:2222:3333:4444:5555:6666:7777/64"), + "rmnet17": pfxs("2607:9999:8888:7777:666:5555:4444:3333/64"), + "tun0": pfxs("100.64.1.2/32", "fd7a:115c:a1e0::1/128"), + "v4-rmnet16": pfxs("192.0.0.4/32"), + }, + }, + want: false, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) {