diff --git a/cmd/cli/cli.go b/cmd/cli/cli.go index 602c391..c69d4f2 100644 --- a/cmd/cli/cli.go +++ b/cmd/cli/cli.go @@ -86,12 +86,18 @@ _/ ___\ __\_ __ \ | / __ | ` func curVersion() string { + // Ensure version has proper "v" prefix for semantic versioning + // This is needed because some build systems may provide version without the "v" prefix if version != "dev" && !strings.HasPrefix(version, "v") { version = "v" + version } + // Return version directly if it's not empty and not a dev build + // This avoids unnecessary commit hash concatenation for release versions if version != "" && version != "dev" { return version } + // Truncate commit hash to 7 characters for readability + // Git commit hashes are typically 40 characters, but 7 is sufficient for identification if len(commit) > 7 { commit = commit[:7] } @@ -608,6 +614,10 @@ func processCDFlags(cfg *ctrld.Config) (*controld.ResolverConfig, error) { bo.LogLongerThan = 30 * time.Second ctx := ctrld.LoggerCtx(context.Background(), logger) resolverConfig, err := controld.FetchResolverConfig(ctx, cdUID, appVersion, cdDev) + + // Retry logic for network errors using bootstrap DNS + // This is needed because the initial DNS resolution might fail due to network issues + // or DNS server unavailability, but bootstrap DNS can provide alternative resolution for { if errUrlNetworkError(err) { bo.BackOff(ctx, err) @@ -632,6 +642,8 @@ func processCDFlags(cfg *ctrld.Config) (*controld.ResolverConfig, error) { logger.Info().Msg("generating ctrld config from Control-D configuration") + // Reset config to ensure clean state before applying Control-D settings + // This prevents mixing of old configuration with new Control-D settings *cfg = ctrld.Config{} // Fetch config, unmarshal to cfg. if resolverConfig.Ctrld.CustomConfig != "" { @@ -662,6 +674,8 @@ func processCDFlags(cfg *ctrld.Config) (*controld.ResolverConfig, error) { return "" } + // Initialize upstream configuration with Control-D resolver settings + // This creates the primary DNS resolver configuration for the proxy cfg.Upstream = make(map[string]*ctrld.UpstreamConfig) cfg.Upstream["0"] = &ctrld.UpstreamConfig{ BootstrapIP: bootstrapIP(resolverConfig.DOH), @@ -669,10 +683,16 @@ func processCDFlags(cfg *ctrld.Config) (*controld.ResolverConfig, error) { Type: cdUpstreamProto, Timeout: 5000, } + + // Create exclusion rules for domains that should bypass Control-D + // These domains will be resolved using the system's default DNS servers rules := make([]ctrld.Rule, 0, len(resolverConfig.Exclude)) for _, domain := range resolverConfig.Exclude { rules = append(rules, ctrld.Rule{domain: []string{}}) } + + // Initialize listener configuration with policy rules + // This sets up the DNS proxy listener with the exclusion policy cfg.Listener = make(map[string]*ctrld.ListenerConfig) lc := &ctrld.ListenerConfig{ Policy: &ctrld.ListenerPolicyConfig{ @@ -1175,6 +1195,9 @@ func tryUpdateListenerConfig(cfg *ctrld.Config, notifyFunc func(), fatal bool) ( il := mainLog.Load() if isMobile() { // On Mobile, only use first listener, ignore others. + // This is needed because mobile platforms have limited resources and + // multiple listeners can cause conflicts with system DNS services and + // likely don't work anyway. firstLn := cfg.FirstListener() for k := range cfg.Listener { if cfg.Listener[k] != firstLn { @@ -1182,6 +1205,8 @@ func tryUpdateListenerConfig(cfg *ctrld.Config, notifyFunc func(), fatal bool) ( } } if cdMode { + // Use mobile-specific listener settings for Control-D mode + // Mobile platforms require specific IP/port combinations to avoid permission issues. firstLn.IP = mobileListenerIp() firstLn.Port = mobileListenerPort() clear(lcc) @@ -1273,6 +1298,9 @@ func tryUpdateListenerConfig(cfg *ctrld.Config, notifyFunc func(), fatal bool) ( ok = false break } + + // Try standard port 53 first for better compatibility + // This is the most common DNS port and has the highest chance of working if tryAllPort53 { tryAllPort53 = false if check.IP { @@ -1286,6 +1314,9 @@ func tryUpdateListenerConfig(cfg *ctrld.Config, notifyFunc func(), fatal bool) ( } continue } + + // Try localhost as fallback for security and compatibility + // Localhost is often available even when other addresses are blocked if tryLocalhost { tryLocalhost = false if check.IP { @@ -1299,6 +1330,9 @@ func tryUpdateListenerConfig(cfg *ctrld.Config, notifyFunc func(), fatal bool) ( } continue } + + // Try random IP/port combinations as last resort + // This ensures the service can start even in constrained environments if check.IP && !isZeroIP { // for "0.0.0.0" or "::", we only need to try new port. listener.IP = randomLocalIP() } else { @@ -1326,6 +1360,7 @@ func tryUpdateListenerConfig(cfg *ctrld.Config, notifyFunc func(), fatal bool) ( } // Specific case for systemd-resolved. + // systemd-resolved has specific requirements for DNS forwarding that we must handle if useSystemdResolved { if listener := cfg.FirstListener(); listener != nil && listener.Port == 53 { n := listeners[0] diff --git a/config.go b/config.go index 8b359ed..41e6793 100644 --- a/config.go +++ b/config.go @@ -351,6 +351,9 @@ func (uc *UpstreamConfig) Init(ctx context.Context) { } } if uc.IPStack == "" { + // Set default IP stack based on upstream type + // Control-D upstreams use split stack for better IPv4/IPv6 handling, + // while other upstreams use both stacks for maximum compatibility if uc.IsControlD() { uc.IPStack = IpStackSplit } else { diff --git a/doh.go b/doh.go index 6fbfb71..86b9fb5 100644 --- a/doh.go +++ b/doh.go @@ -53,6 +53,9 @@ var EncodeArchNameMap = map[string]string{ var DecodeArchNameMap = map[string]string{} func init() { + // Create reverse mappings for OS and architecture names + // This is needed because the API expects encoded values, but we need to decode + // them back to their original form for processing for k, v := range EncodeOsNameMap { DecodeOsNameMap[v] = k } diff --git a/internal/clientinfo/arp_unix.go b/internal/clientinfo/arp_unix.go index f5d8f88..51c934a 100644 --- a/internal/clientinfo/arp_unix.go +++ b/internal/clientinfo/arp_unix.go @@ -20,6 +20,8 @@ func (a *arpDiscover) scan() { } // trim brackets + // Unix "arp -an" output formats IP addresses with parentheses like "(192.168.1.1)" + // We need to remove these brackets for proper IP parsing ip := strings.ReplaceAll(fields[1], "(", "") ip = strings.ReplaceAll(ip, ")", "") diff --git a/internal/clientinfo/arp_windows.go b/internal/clientinfo/arp_windows.go index 016b752..c037b29 100644 --- a/internal/clientinfo/arp_windows.go +++ b/internal/clientinfo/arp_windows.go @@ -17,10 +17,14 @@ func (a *arpDiscover) scan() { continue // empty lines } if line[0] != ' ' { + // Mark that we've found an interface header line + // Windows "arp -a" output has interface headers followed by ARP entries header = true // "Interface:" lines, next is header line. continue } if header { + // Skip the header line that follows interface names + // These lines contain column headers like "Internet Address" and "Physical Address" header = false // header lines continue } diff --git a/internal/clientinfo/client_info.go b/internal/clientinfo/client_info.go index a66830b..fd67a05 100644 --- a/internal/clientinfo/client_info.go +++ b/internal/clientinfo/client_info.go @@ -99,9 +99,13 @@ type Table struct { func NewTable(cfg *ctrld.Config, selfIP, cdUID string, ns []string, logger *ctrld.Logger) *Table { refreshInterval := cfg.Service.DiscoverRefreshInterval + // Set default refresh interval if not configured + // This ensures client discovery continues to work even without explicit configuration if refreshInterval <= 0 { refreshInterval = 2 * 60 // 2 minutes } + // Use no-op logger if none provided + // This prevents nil pointer dereferences when logging is not configured if logger == nil { logger = ctrld.NopLogger } @@ -274,6 +278,7 @@ func (t *Table) init() { host, port = h, p } // Only use valid ip:port pair. + // Invalid nameservers can cause PTR discovery to fail silently if _, portErr := strconv.Atoi(port); portErr == nil && port != "0" && net.ParseIP(host) != nil { nss = append(nss, net.JoinHostPort(host, port)) } else { @@ -465,6 +470,7 @@ func (t *Table) ListClients() []*Client { for _, c := range ipMap { // If we found a client with empty hostname, use hostname from // an existed client which has the same MAC address. + // This helps fill in missing hostnames when multiple IPs share the same MAC if cFromMac := clientsByMAC[c.Mac]; cFromMac != nil && c.Hostname == "" { c.Hostname = cFromMac.Hostname } diff --git a/internal/clientinfo/dhcp.go b/internal/clientinfo/dhcp.go index b387806..88a4b5e 100644 --- a/internal/clientinfo/dhcp.go +++ b/internal/clientinfo/dhcp.go @@ -141,6 +141,9 @@ func (d *dhcp) lookupIPByHostname(name string, v6 bool) string { return true } if addr, err := netip.ParseAddr(key.(string)); err == nil && addr.Is6() == v6 { + // Categorize addresses into RFC1918 (private) and public + // RFC1918 addresses are prioritized because they're more likely to be + // the actual client IP in most network configurations if addr.IsPrivate() { rfc1918Addrs = append(rfc1918Addrs, addr) } else { @@ -264,6 +267,8 @@ func (d *dhcp) iscDHCPReadClientInfoReader(reader io.Reader) error { } switch fields[0] { case "lease": + // Normalize IP address to lowercase for consistent comparison + // DHCP lease files may contain mixed-case IP addresses ip = normalizeIP(strings.ToLower(fields[1])) if net.ParseIP(ip) == nil { d.logger.Warn().Msgf("invalid ip address entry: %q", ip) @@ -271,6 +276,8 @@ func (d *dhcp) iscDHCPReadClientInfoReader(reader io.Reader) error { } case "hardware": if len(fields) >= 3 { + // Convert MAC to lowercase and remove trailing semicolon + // DHCP lease files use semicolon-terminated MAC addresses mac = strings.ToLower(strings.TrimRight(fields[2], ";")) if _, err := net.ParseMAC(mac); err != nil { // Invalid dhcp, skip. @@ -278,6 +285,8 @@ func (d *dhcp) iscDHCPReadClientInfoReader(reader io.Reader) error { } } case "client-hostname": + // Remove quotes and semicolons from hostname + // DHCP lease files may quote hostnames and add semicolons hostname = strings.Trim(fields[1], `";`) } } diff --git a/internal/clientinfo/hostsfile.go b/internal/clientinfo/hostsfile.go index 4dc6f35..bcf1bff 100644 --- a/internal/clientinfo/hostsfile.go +++ b/internal/clientinfo/hostsfile.go @@ -165,6 +165,8 @@ func parseHostEntriesConfFromReader(r io.Reader) map[string][]string { for scanner.Scan() { line := scanner.Text() if after, found := strings.CutPrefix(line, "local-zone:"); found { + // Extract local zone name for domain suffix removal + // This is needed because unbound appends the local zone to hostnames after = strings.TrimSpace(after) fields := strings.Fields(after) if len(fields) > 1 { @@ -177,6 +179,8 @@ func parseHostEntriesConfFromReader(r io.Reader) map[string][]string { if !found { continue } + // Clean up the parsed data by removing whitespace and quotes + // This ensures consistent formatting for hostname processing after = strings.TrimSpace(after) after = strings.Trim(after, `"`) fields := strings.Fields(after) @@ -184,6 +188,8 @@ func parseHostEntriesConfFromReader(r io.Reader) map[string][]string { continue } ip := fields[0] + // Remove local zone suffix from hostname for cleaner lookups + // Unbound adds the local zone to hostnames, but we want just the base name name := strings.TrimSuffix(fields[1], "."+localZone) hostsMap[ip] = append(hostsMap[ip], name) } diff --git a/internal/clientinfo/mdns.go b/internal/clientinfo/mdns.go index ebdfabc..b1bfaaf 100644 --- a/internal/clientinfo/mdns.go +++ b/internal/clientinfo/mdns.go @@ -219,6 +219,8 @@ func (m *mdns) probe(conns []*net.UDPConn, remoteAddr net.Addr) error { for _, conn := range conns { _ = conn.SetWriteDeadline(time.Now().Add(time.Second * 30)) if _, werr := conn.WriteTo(buf, remoteAddr); werr != nil { + // Capture the last write error for reporting + // Multiple connections may fail, but we only report the last error err = werr } } diff --git a/internal/clientinfo/ndp.go b/internal/clientinfo/ndp.go index 87f86fe..7da7f8f 100644 --- a/internal/clientinfo/ndp.go +++ b/internal/clientinfo/ndp.go @@ -174,6 +174,9 @@ func (nd *ndpDiscover) scanUnix(r io.Reader) { } if mac := parseMAC(fields[1]); mac != "" { ip := fields[0] + // Remove interface suffix from IPv6 addresses + // Unix systems append interface names to IPv6 addresses (e.g., "fe80::1%eth0") + // This suffix needs to be removed for proper IP parsing if idx := strings.IndexByte(ip, '%'); idx != -1 { ip = ip[:idx] } @@ -192,11 +195,15 @@ func normalizeMac(mac string) string { return mac } // Windows use "-" instead of ":" as separator. + // This normalization is needed because different operating systems use different + // separators for MAC addresses, but net.ParseMAC expects ":" format mac = strings.ReplaceAll(mac, "-", ":") parts := strings.Split(mac, ":") if len(parts) != 6 { return "" } + // Pad single-digit hex values with leading zero + // This ensures consistent formatting for MAC address parsing for i, c := range parts { if len(c) == 1 { parts[i] = "0" + c diff --git a/internal/clientinfo/ptr_lookup.go b/internal/clientinfo/ptr_lookup.go index b4783bd..4d45971 100644 --- a/internal/clientinfo/ptr_lookup.go +++ b/internal/clientinfo/ptr_lookup.go @@ -105,11 +105,9 @@ func (p *ptrDiscover) lookupIPByHostname(name string, v6 bool) string { if value == name { if addr, err := netip.ParseAddr(key.(string)); err == nil && addr.Is6() == v6 { ip = addr.String() - //lint:ignore S1008 This is used for readable. - if addr.IsLoopback() { // Continue searching if this is loopback address. - return true - } - return false + // Continue searching if this is a loopback address + // We prefer non-loopback addresses as they're more likely to be the actual client IP + return addr.IsLoopback() // Continue searching if this is loopback address. } } return true diff --git a/internal/controld/config.go b/internal/controld/config.go index 813fcd5..77cebb0 100644 --- a/internal/controld/config.go +++ b/internal/controld/config.go @@ -233,6 +233,8 @@ func apiTransport(loggerCtx context.Context, cdDev bool) *http.Transport { } // Separate IPv4 and IPv6 addresses + // This separation is needed because different network stacks may have different + // connectivity to IPv4 vs IPv6, so we try them separately for better reliability var ipv4s, ipv6s []string for _, ip := range ips { if strings.Contains(ip, ":") { diff --git a/nameservers_windows.go b/nameservers_windows.go index b02be53..b19c5ad 100644 --- a/nameservers_windows.go +++ b/nameservers_windows.go @@ -165,6 +165,9 @@ func getDNSServers(ctx context.Context) ([]string, error) { if info.DomainControllerAddress != nil { dcAddr := windows.UTF16PtrToString(info.DomainControllerAddress) + // Remove "\\" prefix from domain controller address + // Windows domain controller addresses are returned with "\\" prefix, + // but we need just the IP address for DNS resolution dcAddr = strings.TrimPrefix(dcAddr, "\\\\") logger.Debug().Msgf("Found domain controller address: %s", dcAddr) if ip := net.ParseIP(dcAddr); ip != nil { diff --git a/resolver.go b/resolver.go index 1c4bf28..0565c2b 100644 --- a/resolver.go +++ b/resolver.go @@ -126,10 +126,11 @@ func InitializeOsResolver(ctx context.Context, guardAgainstNoNameservers bool) [ // - First available LAN servers are saved and store. // - Later calls, if no LAN servers available, the saved servers above will be used. func initializeOsResolver(servers []string) []string { - var lanNss, publicNss []string - // First categorize servers + // Categorize DNS servers into LAN and public servers + // This is needed because LAN servers should be tried first for better performance, + // while public servers serve as fallback for external queries for _, ns := range servers { addr, err := netip.ParseAddr(ns) if err != nil { @@ -143,6 +144,8 @@ func initializeOsResolver(servers []string) []string { } } + // Ensure we have at least one public DNS server as fallback + // This prevents DNS resolution failures when no public servers are configured if len(publicNss) == 0 { publicNss = []string{controldPublicDnsWithPort} }