macOS: share network service-order parsing

Decode networksetup entries once for interface lookup and DNS-target
selection. Preserve first-enabled lookup and fail-closed DNS uniqueness
as separate policies. Treat only the (*) index as disabled, preserving
asterisks and whitespace in enabled service names.

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

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 {

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
+}

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 }