diff --git a/cmd/cli/dns_target_service.go b/cmd/cli/dns_target_service.go index 273cc8a..441984b 100644 --- a/cmd/cli/dns_target_service.go +++ b/cmd/cli/dns_target_service.go @@ -2,71 +2,43 @@ package cli import ( "errors" - "regexp" "strings" ) -var targetServiceOrderEntry = regexp.MustCompile(`^\(([1-9][0-9]*|\*)\) (.+)$`) -var targetServiceDevice = regexp.MustCompile(`^[A-Za-z][A-Za-z0-9]*$`) - // networksetup uses its own service name, which can differ from the Dynamic // Store's UserDefinedName (for example, iPhone USB versus iPhone). // Its listing has no UUID, so require exactly one service on the proven -// device, and require that service to be enabled. +// device, and require that service to be enabled. Do not use patchNetIfaceName: +// its first-enabled-match policy cannot prove a unique target for a DNS write. func targetNetworkServiceName(output, device string) (string, error) { - const header = "An asterisk (*) denotes that a network service is disabled." - lines := strings.Split(strings.TrimSpace(output), "\n") - if len(lines) == 0 || lines[0] != header || !targetServiceDevice.MatchString(device) { - return "", errors.New("invalid network service listing") + if !serviceOrderDeviceName.MatchString(device) { + return "", errors.New("invalid target device") + } + entries, err := parseNetworkServiceOrder(strings.NewReader(output)) + if err != nil { + return "", err } seen := make(map[string]bool) - name, pending := "", "" - disabled := false + name := "" matches := 0 - for _, line := range lines[1:] { - if strings.TrimSpace(line) == "" { + for _, entry := range entries { + key := strings.ToLower(entry.Name) + if seen[key] { + return "", errors.New("ambiguous network service name") + } + seen[key] = true + if entry.Device != device { continue } - if pending == "" { - m := targetServiceOrderEntry.FindStringSubmatch(line) - if m == nil { - return "", errors.New("invalid network service entry") - } - pending = m[2] - disabled = m[1] == "*" - if strings.HasPrefix(pending, "*") { - pending = strings.TrimPrefix(pending, "*") - disabled = true - } - if pending == "" || strings.ContainsAny(pending, "\r\n\x00") || seen[strings.ToLower(pending)] { - return "", errors.New("ambiguous network service name") - } - seen[strings.ToLower(pending)] = true - continue + if strings.ContainsAny(entry.Name, "/{}") { + return "", errors.New("unsafe target service name") } - if !strings.HasPrefix(line, "(Hardware Port: ") || !strings.HasSuffix(line, ")") { - return "", errors.New("invalid network service device") + matches++ + if !entry.Disabled { + name = entry.Name } - at := strings.LastIndex(line, ", Device: ") - if at < 0 { - return "", errors.New("missing network service device") - } - dev := line[at+len(", Device: ") : len(line)-1] - if dev != "" && !targetServiceDevice.MatchString(dev) { - return "", errors.New("invalid network device name") - } - if dev == device { - if strings.ContainsAny(pending, "/{}") { - return "", errors.New("unsafe target service name") - } - matches++ - if !disabled { - name = pending - } - } - pending = "" } - if pending != "" || matches != 1 || name == "" { + if matches != 1 || name == "" { return "", errors.New("network service mapping is missing or ambiguous") } return name, nil diff --git a/cmd/cli/net_darwin.go b/cmd/cli/net_darwin.go index 4856150..35b0f44 100644 --- a/cmd/cli/net_darwin.go +++ b/cmd/cli/net_darwin.go @@ -1,7 +1,6 @@ package cli import ( - "bufio" "bytes" "context" "errors" @@ -32,9 +31,6 @@ const networksetupTimeout = 5 * time.Second // below it, so more than this is a broken command. const maxNetworksetupOutput = 1 << 20 -// maxNetworksetupLine bounds one line, so one token fills no memory. -const maxNetworksetupLine = 64 * 1024 - var errNetworksetupOutputTooLarge = errors.New("networksetup output above the bound") var ( @@ -65,63 +61,6 @@ func networkServiceName(ifaceName string, r io.Reader) string { return serviceNamesByDevice(r)[ifaceName] } -// serviceNamesByDevice maps each device of "networksetup -// -listnetworkserviceorder" to the name of the service that owns it. -func serviceNamesByDevice(r io.Reader) map[string]string { - names := make(map[string]string) - scanner := scanNetworksetup(r) - prevLine := "" - for scanner.Scan() { - line := scanner.Text() - if strings.Contains(line, "*") { - // A disabled service names no device. Without the reset, the device - // line that follows takes the name of the service before it. - prevLine = "" - continue - } - device := serviceOrderDevice(line) - if device == "" { - prevLine = line - continue - } - if _, seen := names[device]; !seen { - names[device] = serviceOrderName(prevLine) - } - prevLine = line - } - if scanner.Err() != nil { - // A truncated read names fewer services than the host has. - return nil - } - return names -} - -// scanNetworksetup reads the output line by line under a bounded buffer. -func scanNetworksetup(r io.Reader) *bufio.Scanner { - scanner := bufio.NewScanner(r) - scanner.Buffer(nil, maxNetworksetupLine) - return scanner -} - -// serviceOrderDevice reads the device out of a line like -// "(Hardware Port: Wi-Fi, Device: en0)". -func serviceOrderDevice(line string) string { - _, after, found := strings.Cut(line, "Device:") - if !found { - return "" - } - return strings.TrimSpace(strings.TrimSuffix(strings.TrimSpace(after), ")")) -} - -// serviceOrderName reads the service name out of a line like "(1) Wi-Fi". -func serviceOrderName(line string) string { - parts := strings.SplitN(line, " ", 2) - if len(parts) != 2 { - return "" - } - return strings.TrimSpace(parts[1]) -} - // parseHardwarePorts maps each device of "networksetup -listallhardwareports" // to its hardware port. Each block names the port before the device. func parseHardwarePorts(r io.Reader) map[string]hardwarePortInfo { diff --git a/cmd/cli/network_service_order.go b/cmd/cli/network_service_order.go new file mode 100644 index 0000000..b025fee --- /dev/null +++ b/cmd/cli/network_service_order.go @@ -0,0 +1,101 @@ +package cli + +import ( + "bufio" + "errors" + "io" + "regexp" + "strings" +) + +// maxNetworksetupLine bounds one line, so one token fills no memory. +const maxNetworksetupLine = 64 * 1024 + +var serviceOrderEntry = regexp.MustCompile(`^\(([1-9][0-9]*|\*)\) (.+)$`) +var serviceOrderDeviceName = regexp.MustCompile(`^[A-Za-z][A-Za-z0-9]*$`) + +type networkServiceEntry struct { + Name, Device string + Disabled bool +} + +// scanNetworksetup reads the output line by line under a bounded buffer. +func scanNetworksetup(r io.Reader) *bufio.Scanner { + scanner := bufio.NewScanner(r) + scanner.Buffer(nil, maxNetworksetupLine) + return scanner +} + +// parseNetworkServiceOrder decodes networksetup -listnetworkserviceorder. +// Selection policy belongs to the caller: retain disabled and duplicate-device +// entries so a DNS writer can reject mappings a first-match reader accepts. +func parseNetworkServiceOrder(r io.Reader) ([]networkServiceEntry, error) { + const header = "An asterisk (*) denotes that a network service is disabled." + scanner := scanNetworksetup(r) + var entries []networkServiceEntry + var pending *networkServiceEntry + headerSeen := false + for scanner.Scan() { + line := scanner.Text() + if strings.TrimSpace(line) == "" { + continue + } + if !headerSeen { + if strings.TrimSpace(line) != header { + return nil, errors.New("invalid network service listing") + } + headerSeen = true + continue + } + if pending == nil { + m := serviceOrderEntry.FindStringSubmatch(line) + if m == nil || strings.ContainsAny(m[2], "\r\n\x00") { + return nil, errors.New("invalid network service entry") + } + // Only the index marks a disabled service. An enabled name may + // itself start with an asterisk; preserve it and its whitespace. + pending = &networkServiceEntry{Name: m[2], Disabled: m[1] == "*"} + continue + } + if !strings.HasPrefix(line, "(Hardware Port: ") || !strings.HasSuffix(line, ")") { + return nil, errors.New("invalid network service device") + } + at := strings.LastIndex(line, ", Device: ") + if at < 0 { + return nil, errors.New("missing network service device") + } + device := line[at+len(", Device: ") : len(line)-1] + if device != "" && !serviceOrderDeviceName.MatchString(device) { + return nil, errors.New("invalid network device name") + } + pending.Device = device + entries = append(entries, *pending) + pending = nil + } + if err := scanner.Err(); err != nil { + return nil, err + } + if !headerSeen || pending != nil { + return nil, errors.New("incomplete network service listing") + } + return entries, nil +} + +// serviceNamesByDevice keeps the first enabled service on each device for +// interface-name lookup and hardware-port metadata, not DNS-target admission. +func serviceNamesByDevice(r io.Reader) map[string]string { + entries, err := parseNetworkServiceOrder(r) + if err != nil { + return nil + } + names := make(map[string]string) + for _, entry := range entries { + if entry.Disabled || entry.Device == "" { + continue + } + if _, seen := names[entry.Device]; !seen { + names[entry.Device] = entry.Name + } + } + return names +} diff --git a/cmd/cli/network_service_order_test.go b/cmd/cli/network_service_order_test.go new file mode 100644 index 0000000..f82fed8 --- /dev/null +++ b/cmd/cli/network_service_order_test.go @@ -0,0 +1,102 @@ +package cli + +import ( + "errors" + "io" + "reflect" + "strings" + "testing" +) + +func TestNetworkServiceOrderEntries(t *testing.T) { + const listing = `An asterisk (*) denotes that a network service is disabled. +(1) *Téléphone personnel +(Hardware Port: USB, Device: en8) +(*) Disabled USB +(Hardware Port: USB, Device: en8) +(2) Wi-Fi +(Hardware Port: Wi-Fi, Device: en0) +(3) Tunnel +(Hardware Port: VPN, Device: ) +` + want := []networkServiceEntry{ + {Name: "*Téléphone personnel", Device: "en8"}, + {Name: "Disabled USB", Device: "en8", Disabled: true}, + {Name: " Wi-Fi", Device: "en0"}, + {Name: "Tunnel"}, + } + got, err := parseNetworkServiceOrder(strings.NewReader(listing)) + if err != nil || !reflect.DeepEqual(got, want) { + t.Fatalf("entries=%+v err=%v", got, err) + } +} + +func TestNetworkServiceOrderSelectionPolicies(t *testing.T) { + base := nativeTargetTestServiceOrder("*iPhone USB", "en8") + for _, tc := range []struct { + name, listing, lookup, target string + }{ + {"enabled asterisk name", base, "*iPhone USB", "*iPhone USB"}, + {"embedded asterisk", nativeTargetTestServiceOrder("My * USB", "en8"), "My * USB", "My * USB"}, + {"name whitespace", nativeTargetTestServiceOrder(" USB ", "en8"), " USB ", " USB "}, + {"disabled", strings.Replace(base, "(1)", "(*)", 1), "", ""}, + {"disabled neighbor", base + "(*) Ethernet\n(Hardware Port: Ethernet, Device: en9)\n", "*iPhone USB", "*iPhone USB"}, + {"enabled then disabled same device", base + "(*) Old USB\n(Hardware Port: USB, Device: en8)\n", "*iPhone USB", ""}, + {"disabled then enabled same device", strings.Replace(base, "(1)", "(*)", 1) + "(2) New USB\n(Hardware Port: USB, Device: en8)\n", "New USB", ""}, + {"two enabled same device", base + "(2) Other USB\n(Hardware Port: USB, Device: en8)\n", "*iPhone USB", ""}, + {"duplicate name on another device", base + "(2) *iPhone USB\n(Hardware Port: USB, Device: en9)\n", "*iPhone USB", ""}, + {"case-insensitive duplicate", base + "(*) *IPHONE USB\n(Hardware Port: USB, Device: en9)\n", "*iPhone USB", ""}, + } { + t.Run(tc.name, func(t *testing.T) { + names := serviceNamesByDevice(strings.NewReader(tc.listing)) + if names["en8"] != tc.lookup { + t.Fatalf("lookup=%v, want %q", names, tc.lookup) + } + if tc.name == "disabled neighbor" { + if _, exists := names["en9"]; exists { + t.Fatal("disabled neighbor inherited an enabled service name") + } + } + got, err := targetNetworkServiceName(tc.listing, "en8") + if got != tc.target || (err != nil) != (tc.target == "") { + t.Fatalf("target=%q err=%v, want %q", got, err, tc.target) + } + }) + } +} + +func TestNetworkServiceOrderRejectsIncompleteReads(t *testing.T) { + base := nativeTargetTestServiceOrder("USB", "en8") + for _, listing := range []string{ + "", "unexpected output", base + "(2) Incomplete\n", + base + "(2) \n(Hardware Port: USB, Device: en9)\n", + base + "(2) Broken\nmissing device line\n", + base + strings.Repeat("x", maxNetworksetupLine+1), + strings.Replace(base, "Device: en8", "Device: en8 extra", 1), + strings.Replace(base, "(1) USB", "(0) USB", 1), + strings.Replace(base, "(1) USB", "(1) USB\x00", 1), + } { + entries, err := parseNetworkServiceOrder(strings.NewReader(listing)) + if err == nil || entries != nil { + t.Fatalf("invalid listing returned entries=%+v err=%v", entries, err) + } + if got := serviceNamesByDevice(strings.NewReader(listing)); got != nil { + t.Fatalf("invalid listing returned partial names: %v", got) + } + if got, err := targetNetworkServiceName(listing, "en8"); err == nil || got != "" { + t.Fatalf("invalid listing admitted target=%q err=%v", got, err) + } + } + readErr := errors.New("interrupted service-order read") + reader := func() io.Reader { return io.MultiReader(strings.NewReader(base), serviceOrderErrorReader{readErr}) } + if entries, err := parseNetworkServiceOrder(reader()); !errors.Is(err, readErr) || entries != nil { + t.Fatalf("partial read returned entries=%+v err=%v", entries, err) + } + if names := serviceNamesByDevice(reader()); names != nil { + t.Fatalf("failed read returned partial names: %v", names) + } +} + +type serviceOrderErrorReader struct{ err error } + +func (r serviceOrderErrorReader) Read([]byte) (int, error) { return 0, r.err }