Skip to content

Commit 35019fa

Browse files
kari-tskari-ts
committed
android: fix local endpoint parsing and add regression ifaceparse_test
Android reports interface addresses in CIDR form. Recent changes (see 9c933a0#diff-1d627686c31972e04ef60d7d301e8a2a93714c60096a50055a6bbe9aa041ca8fL105, 9c933a0#diff-1d627686c31972e04ef60d7d301e8a2a93714c60096a50055a6bbe9aa041ca8fL105) parsed addresses as CIDR prefixes, resulting in prefix base being advertised as a local endpoint. This change: -instead of parsing CIDR prefixes, the parser strips the /NN portion and uses netip.ParseAddr to parse the host IP only -extracts the interface parsing into a helper in a new package to make it testable on non-Android platforms -adds a unit test to verify that host addresses are present and prefix-base ones are not Fixes tailscale/tailscale#16836 Signed-off-by: kari-ts <kari@tailscale.com>
1 parent d0442f7 commit 35019fa

3 files changed

Lines changed: 204 additions & 75 deletions

File tree

‎libtailscale/ifaceparse/ifaceparse.go‎

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
// Copyright (c) Tailscale Inc & AUTHORS
2+
// SPDX-License-Identifier: BSD-3-Clause
3+
4+
package ifaceparse
5+
6+
import (
7+
"fmt"
8+
"net"
9+
"net/netip"
10+
"strings"
11+
12+
"tailscale.com/net/netmon"
13+
)
14+
15+
type ParseStats struct {
16+
LinesTotal int
17+
LinesParsed int
18+
LinesSkipped int
19+
AddrsTotal int
20+
AddrsParsed int
21+
AddrsSkipped int
22+
}
23+
24+
// ParseInterfacesAsNetmon parses the string returned by Android's GetInterfacesAsString()
25+
// into netmon.Interface values.
26+
func ParseInterfacesAsNetmon(ifaceString string) ([]netmon.Interface, ParseStats, error) {
27+
var ifaces []netmon.Interface
28+
var st ParseStats
29+
30+
for _, line := range strings.Split(ifaceString, "\n") {
31+
// Example of the strings we're processing:
32+
// wlan0 30 1500 true true false false true | fe80::2f60:2c82:4163:8389%wlan0/64 10.1.10.131/24
33+
// r_rmnet_data0 21 1500 true false false false false | fe80::9318:6093:d1ad:ba7f%r_rmnet_data0/64
34+
// mnet_data2 12 1500 true false false false false | fe80::3c8c:44dc:46a9:9907%rmnet_data2/64
35+
if strings.TrimSpace(line) == "" {
36+
continue
37+
}
38+
st.LinesTotal++
39+
40+
fields := strings.Split(line, "|")
41+
if len(fields) != 2 {
42+
st.LinesSkipped++
43+
continue
44+
}
45+
46+
var name string
47+
var index, mtu int
48+
var up, broadcast, loopback, pointToPoint, multicast bool
49+
if _, err := fmt.Sscanf(fields[0], "%s %d %d %t %t %t %t %t",
50+
&name, &index, &mtu, &up, &broadcast, &loopback, &pointToPoint, &multicast); err != nil {
51+
st.LinesSkipped++
52+
continue
53+
}
54+
55+
newIf := netmon.Interface{
56+
Interface: &net.Interface{
57+
Name: name,
58+
Index: index,
59+
MTU: mtu,
60+
},
61+
AltAddrs: []net.Addr{}, // non-nil to avoid Go using netlink
62+
}
63+
if up {
64+
newIf.Flags |= net.FlagUp
65+
}
66+
if broadcast {
67+
newIf.Flags |= net.FlagBroadcast
68+
}
69+
if loopback {
70+
newIf.Flags |= net.FlagLoopback
71+
}
72+
if pointToPoint {
73+
newIf.Flags |= net.FlagPointToPoint
74+
}
75+
if multicast {
76+
newIf.Flags |= net.FlagMulticast
77+
}
78+
79+
addrField := strings.TrimSpace(fields[1])
80+
addrTokens := strings.Fields(addrField)
81+
st.AddrsTotal += len(addrTokens)
82+
83+
for _, addr := range addrTokens {
84+
// addr is typically in CIDR form: "10.0.0.159/8" or "2601:.../64".
85+
// For endpoints we want the host IP, not the CIDR prefix base, so strip "/NN".
86+
ipStr, _, _ := strings.Cut(addr, "/")
87+
88+
na, err := netip.ParseAddr(ipStr) // supports IPv6 zones (e.g. "%wlan0")
89+
if err != nil {
90+
st.AddrsSkipped++
91+
continue
92+
}
93+
st.AddrsParsed++
94+
95+
newIf.AltAddrs = append(newIf.AltAddrs, &net.IPAddr{
96+
IP: net.IP(na.AsSlice()),
97+
Zone: na.Zone(),
98+
})
99+
}
100+
101+
ifaces = append(ifaces, newIf)
102+
st.LinesParsed++
103+
}
104+
105+
return ifaces, st, nil
106+
}

‎libtailscale/ifaceparse/ifaceparse_test.go‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
// Copyright (c) Tailscale Inc & AUTHORS
2+
// SPDX-License-Identifier: BSD-3-Clause
3+
4+
package ifaceparse
5+
6+
import (
7+
"net"
8+
"sort"
9+
"strings"
10+
"testing"
11+
12+
"tailscale.com/net/netmon"
13+
)
14+
15+
func TestParseInterfacesAsNetmon_StripsCIDRAndDoesNotUsePrefixBase(t *testing.T) {
16+
ifaceString := strings.Join([]string{
17+
// This includes both IPv4 + IPv6 CIDRs and an IPv6 zone.
18+
// We are guarding against accidentally turning these into
19+
// prefix-base addresses like 10.1.10.0 or 2601:...:2640::.
20+
"wlan0 30 1500 true true false false true | fe80::2f60:2c82:4163:8389%wlan0/64 10.1.10.131/24 2601:647:6801:2640:842b:a104:7efe:3f74/64",
21+
"",
22+
}, "\n")
23+
24+
ifaces, st, err := ParseInterfacesAsNetmon(ifaceString)
25+
if err != nil {
26+
t.Fatalf("ParseInterfacesAsNetmon: %v", err)
27+
}
28+
if len(ifaces) != 1 {
29+
t.Fatalf("expected 1 interface, got %d", len(ifaces))
30+
}
31+
if ifaces[0].Name != "wlan0" {
32+
t.Fatalf("expected interface wlan0, got %q", ifaces[0].Name)
33+
}
34+
35+
if st.LinesTotal != 1 || st.LinesParsed != 1 || st.LinesSkipped != 0 {
36+
t.Fatalf("unexpected line stats: %+v", st)
37+
}
38+
if st.AddrsTotal != 3 {
39+
t.Fatalf("expected 3 addr tokens, got %d (stats=%+v)", st.AddrsTotal, st)
40+
}
41+
if st.AddrsParsed < 2 {
42+
t.Fatalf("expected at least 2 parsed addrs, got %d (stats=%+v)", st.AddrsParsed, st)
43+
}
44+
45+
got := collectAltAddrStrings(t, ifaces[0])
46+
47+
// Must contain host IPs (not prefix base).
48+
if !got["10.1.10.131"] {
49+
t.Fatalf("missing host IPv4 10.1.10.131; got=%v", keys(got))
50+
}
51+
if !got["2601:647:6801:2640:842b:a104:7efe:3f74"] {
52+
t.Fatalf("missing host IPv6; got=%v", keys(got))
53+
}
54+
55+
// Must NOT contain prefix-base addresses.
56+
if got["10.1.10.0"] {
57+
t.Fatalf("saw prefix-base IPv4 10.1.10.0; got=%v", keys(got))
58+
}
59+
if got["2601:647:6801:2640::"] {
60+
t.Fatalf("saw prefix-base IPv6 2601:...::; got=%v", keys(got))
61+
}
62+
}
63+
64+
// collectAltAddrStrings formats AltAddrs into comparable strings.
65+
func collectAltAddrStrings(t *testing.T, ifc netmon.Interface) map[string]bool {
66+
t.Helper()
67+
68+
out := map[string]bool{}
69+
for _, a := range ifc.AltAddrs {
70+
v, ok := a.(*net.IPAddr)
71+
if !ok {
72+
t.Fatalf("unexpected AltAddrs type: %T", a)
73+
}
74+
out[v.IP.String()] = true
75+
if v.Zone != "" {
76+
out[v.IP.String()+"%"+v.Zone] = true
77+
}
78+
}
79+
return out
80+
}
81+
82+
func keys(m map[string]bool) []string {
83+
out := make([]string, 0, len(m))
84+
for k := range m {
85+
out = append(out, k)
86+
}
87+
sort.Strings(out)
88+
return out
89+
}

‎libtailscale/net.go‎

Lines changed: 9 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,12 @@ import (
77
"errors"
88
"fmt"
99
"log"
10-
"net"
1110
"net/netip"
1211
"runtime/debug"
1312
"strings"
1413
"syscall"
1514

15+
"github.com/tailscale/tailscale-android/libtailscale/ifaceparse"
1616
"github.com/tailscale/wireguard-go/tun"
1717
"tailscale.com/net/dns"
1818
"tailscale.com/net/netmon"
@@ -42,85 +42,19 @@ var vpnService = &VpnService{}
4242

4343
// Report interfaces in the device in net.Interface format.
4444
func (a *App) getInterfaces() ([]netmon.Interface, error) {
45-
var ifaces []netmon.Interface
46-
4745
ifaceString, err := a.appCtx.GetInterfacesAsString()
4846
if err != nil {
49-
return ifaces, err
47+
return nil, err
5048
}
5149

52-
for _, iface := range strings.Split(ifaceString, "\n") {
53-
// Example of the strings we're processing:
54-
// wlan0 30 1500 true true false false true | fe80::2f60:2c82:4163:8389%wlan0/64 10.1.10.131/24
55-
// r_rmnet_data0 21 1500 true false false false false | fe80::9318:6093:d1ad:ba7f%r_rmnet_data0/64
56-
// mnet_data2 12 1500 true false false false false | fe80::3c8c:44dc:46a9:9907%rmnet_data2/64
57-
58-
if strings.TrimSpace(iface) == "" {
59-
continue
60-
}
61-
62-
fields := strings.Split(iface, "|")
63-
if len(fields) != 2 {
64-
log.Printf("getInterfaces: unable to split %q", iface)
65-
continue
66-
}
67-
68-
var name string
69-
var index, mtu int
70-
var up, broadcast, loopback, pointToPoint, multicast bool
71-
_, err := fmt.Sscanf(fields[0], "%s %d %d %t %t %t %t %t",
72-
&name, &index, &mtu, &up, &broadcast, &loopback, &pointToPoint, &multicast)
73-
if err != nil {
74-
log.Printf("getInterfaces: unable to parse %q: %v", iface, err)
75-
continue
76-
}
77-
78-
newIf := netmon.Interface{
79-
Interface: &net.Interface{
80-
Name: name,
81-
Index: index,
82-
MTU: mtu,
83-
},
84-
AltAddrs: []net.Addr{}, // non-nil to avoid Go using netlink
85-
}
86-
if up {
87-
newIf.Flags |= net.FlagUp
88-
}
89-
if broadcast {
90-
newIf.Flags |= net.FlagBroadcast
91-
}
92-
if loopback {
93-
newIf.Flags |= net.FlagLoopback
94-
}
95-
if pointToPoint {
96-
newIf.Flags |= net.FlagPointToPoint
97-
}
98-
if multicast {
99-
newIf.Flags |= net.FlagMulticast
100-
}
101-
102-
addrs := strings.Trim(fields[1], " \n")
103-
for _, addr := range strings.Split(addrs, " ") {
104-
pfx, err := netip.ParsePrefix(addr)
105-
var ip net.IP
106-
if pfx.Addr().Is4() {
107-
v4 := pfx.Addr().As4()
108-
ip = net.IP(v4[:])
109-
} else {
110-
v6 := pfx.Addr().As16()
111-
ip = net.IP(v6[:])
112-
}
113-
if err == nil {
114-
newIf.AltAddrs = append(newIf.AltAddrs, &net.IPAddr{
115-
IP: ip,
116-
Zone: pfx.Addr().Zone(),
117-
})
118-
}
119-
}
120-
121-
ifaces = append(ifaces, newIf)
50+
ifaces, st, err := ifaceparse.ParseInterfacesAsNetmon(ifaceString)
51+
if err != nil {
52+
return nil, err
53+
}
54+
if st.LinesSkipped > 0 || st.AddrsSkipped > 0 {
55+
log.Printf("getInterfaces: parsed %d/%d lines, %d/%d addrs (skipped %d lines, %d addrs)",
56+
st.LinesParsed, st.LinesTotal, st.AddrsParsed, st.AddrsTotal, st.LinesSkipped, st.AddrsSkipped)
12257
}
123-
12458
return ifaces, nil
12559
}
12660

0 commit comments

Comments
 (0)