cmd/cli: guard against nil client info

Though it's only possible raised in testing, still better to be safe.

cmd/cli/cli.go

@@ -146,6 +146,7 @@ func initCLI() {
 	_ = runCmd.Flags().MarkHidden("iface")
 	runCmd.Flags().StringVarP(&cdUpstreamProto, "proto", "", ctrld.ResolverTypeDOH, `Control D upstream type, either "doh" or "doh3"`)
 
+	runCmd.FParseErrWhitelist = cobra.FParseErrWhitelist{UnknownFlags: true}
 	rootCmd.AddCommand(runCmd)
 
 	startCmd := &cobra.Command{
@@ -206,7 +207,11 @@ func initCLI() {
 					defaultConfigFile = filepath.Join(dir, defaultConfigFile)
 				}
 				sc.Arguments = append(sc.Arguments, "--homedir="+dir)
-				sockPath := filepath.Join(dir, ctrldLogUnixSock)
+				sockDir := dir
+				if d, err := socketDir(); err == nil {
+					sockDir = d
+				}
+				sockPath := filepath.Join(sockDir, ctrldLogUnixSock)
 				_ = os.Remove(sockPath)
 				go func() {
 					defer func() {
@@ -393,7 +398,7 @@ func initCLI() {
 				{s.Start, true},
 			}
 			if doTasks(tasks) {
-				dir, err := userHomeDir()
+				dir, err := socketDir()
 				if err != nil {
 					mainLog.Load().Warn().Err(err).Msg("Service was restarted, but could not ping the control server")
 					return
@@ -416,7 +421,7 @@ func initCLI() {
 		Short: "Reload the ctrld service",
 		Args:  cobra.NoArgs,
 		Run: func(cmd *cobra.Command, args []string) {
-			dir, err := userHomeDir()
+			dir, err := socketDir()
 			if err != nil {
 				mainLog.Load().Fatal().Err(err).Msg("failed to find ctrld home dir")
 			}
@@ -688,7 +693,7 @@ NOTE: Uninstalling will set DNS to values provided by DHCP.`,
 			checkHasElevatedPrivilege()
 		},
 		Run: func(cmd *cobra.Command, args []string) {
-			dir, err := userHomeDir()
+			dir, err := socketDir()
 			if err != nil {
 				mainLog.Load().Fatal().Err(err).Msg("failed to find ctrld home dir")
 			}
@@ -790,7 +795,11 @@ func run(appCallback *AppCallback, stopCh chan struct{}) {
 			homedir = dir
 		}
 	}
-	sockPath := filepath.Join(homedir, ctrldLogUnixSock)
+	sockDir := homedir
+	if d, err := socketDir(); err == nil {
+		sockDir = d
+	}
+	sockPath := filepath.Join(sockDir, ctrldLogUnixSock)
 	if addr, err := net.ResolveUnixAddr("unix", sockPath); err == nil {
 		if conn, err := net.Dial(addr.Network(), addr.String()); err == nil {
 			lc := &logConn{conn: conn}
@@ -842,7 +851,7 @@ func run(appCallback *AppCallback, stopCh chan struct{}) {
 	}
 
 	p.router = router.New(&cfg, cdUID != "")
-	cs, err := newControlServer(filepath.Join(homedir, ctrldControlUnixSock))
+	cs, err := newControlServer(filepath.Join(sockDir, ctrldControlUnixSock))
 	if err != nil {
 		mainLog.Load().Warn().Err(err).Msg("could not create control server")
 	}
@@ -1295,7 +1304,7 @@ func selfCheckStatus(s service.Service) service.Status {
 	if status != service.StatusRunning {
 		return status
 	}
-	dir, err := userHomeDir()
+	dir, err := socketDir()
 	if err != nil {
 		mainLog.Load().Error().Err(err).Msg("failed to check ctrld listener status: could not get home directory")
 		return service.StatusUnknown
@@ -1447,6 +1456,19 @@ func userHomeDir() (string, error) {
 	return dir, nil
 }
 
+// socketDir returns directory that ctrld will create socket file for running controlServer.
+func socketDir() (string, error) {
+	switch {
+	case runtime.GOOS == "windows", isMobile():
+		return userHomeDir()
+	}
+	dir := "/var/run"
+	if ok, _ := dirWritable(dir); !ok {
+		return userHomeDir()
+	}
+	return dir, nil
+}
+
 // tryReadingConfig is like tryReadingConfigWithNotice, with notice set to false.
 func tryReadingConfig(writeDefaultConfig bool) {
 	tryReadingConfigWithNotice(writeDefaultConfig, false)

cmd/cli/dns_proxy.go

@@ -61,6 +61,7 @@ type upstreamForResult struct {
 	matchedNetwork string
 	matchedRule    string
 	matched        bool
+	srcAddr        string
 }
 
 func (p *prog) serveDNS(listenerNum string) error {
@@ -97,9 +98,9 @@ func (p *prog) serveDNS(listenerNum string) error {
 		ci.ClientIDPref = p.cfg.Service.ClientIDPref
 		stripClientSubnet(m)
 		remoteAddr := spoofRemoteAddr(w.RemoteAddr(), ci)
-		fmtSrcToDest := fmtRemoteToLocal(listenerNum, remoteAddr.String(), w.LocalAddr().String())
+		fmtSrcToDest := fmtRemoteToLocal(listenerNum, ci.Hostname, remoteAddr.String())
 		t := time.Now()
-		ctrld.Log(ctx, mainLog.Load().Debug(), "%s received query: %s %s", fmtSrcToDest, dns.TypeToString[q.Qtype], domain)
+		ctrld.Log(ctx, mainLog.Load().Info(), "QUERY: %s: %s %s", fmtSrcToDest, dns.TypeToString[q.Qtype], domain)
 		res := p.upstreamFor(ctx, listenerNum, listenerConfig, remoteAddr, ci.Mac, domain)
 		var answer *dns.Msg
 		if !res.matched && listenerConfig.Restricted {
@@ -200,7 +201,7 @@ func (p *prog) upstreamFor(ctx context.Context, defaultUpstreamNum string, lc *c
 	matchedNetwork := "no network"
 	matchedRule := "no rule"
 	matched := false
-	res = &upstreamForResult{}
+	res = &upstreamForResult{srcAddr: addr.String()}
 
 	defer func() {
 		res.upstreams = upstreams
@@ -377,7 +378,7 @@ func (p *prog) proxy(ctx context.Context, req *proxyRequest) *dns.Msg {
 	// 4. Try remote upstream.
 	isLanOrPtrQuery := false
 	if req.ufr.matched {
-		ctrld.Log(ctx, mainLog.Load().Info(), "%s, %s, %s -> %v", req.ufr.matchedPolicy, req.ufr.matchedNetwork, req.ufr.matchedRule, upstreams)
+		ctrld.Log(ctx, mainLog.Load().Debug(), "%s, %s, %s -> %v", req.ufr.matchedPolicy, req.ufr.matchedNetwork, req.ufr.matchedRule, upstreams)
 	} else {
 		switch {
 		case isPrivatePtrLookup(req.msg):
@@ -386,16 +387,16 @@ func (p *prog) proxy(ctx context.Context, req *proxyRequest) *dns.Msg {
 				return answer
 			}
 			upstreams, upstreamConfigs = p.upstreamsAndUpstreamConfigForLanAndPtr(upstreams, upstreamConfigs)
-			ctrld.Log(ctx, mainLog.Load().Info(), "private PTR lookup, using upstreams: %v", upstreams)
+			ctrld.Log(ctx, mainLog.Load().Debug(), "private PTR lookup, using upstreams: %v", upstreams)
 		case isLanHostnameQuery(req.msg):
 			isLanOrPtrQuery = true
 			if answer := p.proxyLanHostnameQuery(ctx, req.msg); answer != nil {
 				return answer
 			}
 			upstreams, upstreamConfigs = p.upstreamsAndUpstreamConfigForLanAndPtr(upstreams, upstreamConfigs)
-			ctrld.Log(ctx, mainLog.Load().Info(), "lan hostname lookup, using upstreams: %v", upstreams)
+			ctrld.Log(ctx, mainLog.Load().Debug(), "lan hostname lookup, using upstreams: %v", upstreams)
 		default:
-			ctrld.Log(ctx, mainLog.Load().Info(), "no explicit policy matched, using default routing -> %v", upstreams)
+			ctrld.Log(ctx, mainLog.Load().Debug(), "no explicit policy matched, using default routing -> %v", upstreams)
 		}
 	}
 
@@ -503,6 +504,11 @@ func (p *prog) proxy(ctx context.Context, req *proxyRequest) *dns.Msg {
 			p.cache.Add(dnscache.NewKey(req.msg, upstreams[n]), dnscache.NewValue(answer, expired))
 			ctrld.Log(ctx, mainLog.Load().Debug(), "add cached response")
 		}
+		hostname := ""
+		if req.ci != nil {
+			hostname = req.ci.Hostname
+		}
+		ctrld.Log(ctx, mainLog.Load().Info(), "REPLY: %s -> %s (%s): %s", upstreams[n], req.ufr.srcAddr, hostname, dns.RcodeToString[answer.Rcode])
 		return answer
 	}
 	ctrld.Log(ctx, mainLog.Load().Error(), "all %v endpoints failed", upstreams)
@@ -564,8 +570,8 @@ func wildcardMatches(wildcard, domain string) bool {
 	return false
 }
 
-func fmtRemoteToLocal(listenerNum, remote, local string) string {
-	return fmt.Sprintf("%s -> listener.%s: %s:", remote, listenerNum, local)
+func fmtRemoteToLocal(listenerNum, hostname, remote string) string {
+	return fmt.Sprintf("%s (%s) -> listener.%s", remote, hostname, listenerNum)
 }
 
 func requestID() string {

cmd/cli/prog.go

@@ -525,6 +525,7 @@ var (
 	windowsENETUNREACH  = syscall.Errno(10051)
 	windowsEINVAL       = syscall.Errno(10022)
 	windowsEADDRINUSE   = syscall.Errno(10048)
+	windowsEHOSTUNREACH = syscall.Errno(10065)
 )
 
 func errUrlNetworkError(err error) bool {
@@ -547,7 +548,8 @@ func errNetworkError(err error) bool {
 			errors.Is(opErr.Err, syscall.ENETUNREACH),
 			errors.Is(opErr.Err, windowsENETUNREACH),
 			errors.Is(opErr.Err, windowsEINVAL),
-			errors.Is(opErr.Err, windowsECONNREFUSED):
+			errors.Is(opErr.Err, windowsECONNREFUSED),
+			errors.Is(opErr.Err, windowsEHOSTUNREACH):
 			return true
 		}
 	}

cmd/cli/upstream_monitor.go

@@ -3,7 +3,6 @@ package cli
 import (
 	"context"
 	"sync"
-	"sync/atomic"
 	"time"
 
 	"github.com/miekg/dns"
@@ -22,45 +21,52 @@ const (
 type upstreamMonitor struct {
 	cfg *ctrld.Config
 
-	down       map[string]*atomic.Bool
-	failureReq map[string]*atomic.Uint64
-
-	mu       sync.Mutex
-	checking map[string]bool
+	mu         sync.Mutex
+	checking   map[string]bool
+	down       map[string]bool
+	failureReq map[string]uint64
 }
 
 func newUpstreamMonitor(cfg *ctrld.Config) *upstreamMonitor {
 	um := &upstreamMonitor{
 		cfg:        cfg,
-		down:       make(map[string]*atomic.Bool),
-		failureReq: make(map[string]*atomic.Uint64),
 		checking:   make(map[string]bool),
+		down:       make(map[string]bool),
+		failureReq: make(map[string]uint64),
 	}
 	for n := range cfg.Upstream {
 		upstream := upstreamPrefix + n
-		um.down[upstream] = new(atomic.Bool)
-		um.failureReq[upstream] = new(atomic.Uint64)
+		um.reset(upstream)
 	}
-	um.down[upstreamOS] = new(atomic.Bool)
-	um.failureReq[upstreamOS] = new(atomic.Uint64)
+	um.reset(upstreamOS)
 	return um
 }
 
 // increaseFailureCount increase failed queries count for an upstream by 1.
 func (um *upstreamMonitor) increaseFailureCount(upstream string) {
-	failedCount := um.failureReq[upstream].Add(1)
-	um.down[upstream].Store(failedCount >= maxFailureRequest)
+	um.mu.Lock()
+	defer um.mu.Unlock()
+
+	um.failureReq[upstream] += 1
+	failedCount := um.failureReq[upstream]
+	um.down[upstream] = failedCount >= maxFailureRequest
 }
 
 // isDown reports whether the given upstream is being marked as down.
 func (um *upstreamMonitor) isDown(upstream string) bool {
-	return um.down[upstream].Load()
+	um.mu.Lock()
+	defer um.mu.Unlock()
+
+	return um.down[upstream]
 }
 
 // reset marks an upstream as up and set failed queries counter to zero.
 func (um *upstreamMonitor) reset(upstream string) {
-	um.failureReq[upstream].Store(0)
-	um.down[upstream].Store(false)
+	um.mu.Lock()
+	defer um.mu.Unlock()
+
+	um.failureReq[upstream] = 0
+	um.down[upstream] = false
 }
 
 // checkUpstream checks the given upstream status, periodically sending query to upstream
@@ -74,6 +80,11 @@ func (um *upstreamMonitor) checkUpstream(upstream string, uc *ctrld.UpstreamConf
 	}
 	um.checking[upstream] = true
 	um.mu.Unlock()
+	defer func() {
+		um.mu.Lock()
+		um.checking[upstream] = false
+		um.mu.Unlock()
+	}()
 
 	resolver, err := ctrld.NewResolver(uc)
 	if err != nil {

docs/config.md

@@ -412,7 +412,7 @@ If set to `true`, makes the listener `REFUSED` DNS queries from all source IP ad
 - Default: false
 
 ### allow_wan_clients
-The listener `REFUSED` DNS queries from WAN clients by default. If set to `true`, makes the listener replies to them.
+The listener will refuse DNS queries from WAN IPs using `REFUSED` RCODE by default. Set to `true` to disable this behavior, but this is not recommended. 
 
 - Type: bool
 - Required: no

doh.go

@@ -146,61 +146,67 @@ func (r *dohResolver) Resolve(ctx context.Context, msg *dns.Msg) (*dns.Msg, erro
 	return answer, nil
 }
 
+// addHeader adds necessary HTTP header to request based on upstream config.
 func addHeader(ctx context.Context, req *http.Request, uc *UpstreamConfig) {
-	req.Header.Set("Content-Type", headerApplicationDNS)
-	req.Header.Set("Accept", headerApplicationDNS)
-
 	printed := false
+	dohHeader := make(http.Header)
 	if uc.UpstreamSendClientInfo() {
 		if ci, ok := ctx.Value(ClientInfoCtxKey{}).(*ClientInfo); ok && ci != nil {
 			printed = ci.Mac != "" || ci.IP != "" || ci.Hostname != ""
 			switch {
 			case uc.isControlD():
-				addControlDHeaders(req, ci)
+				dohHeader = newControlDHeaders(ci)
 			case uc.isNextDNS():
-				addNextDNSHeaders(req, ci)
+				dohHeader = newNextDNSHeaders(ci)
 			}
 		}
 	}
 	if printed {
-		Log(ctx, ProxyLogger.Load().Debug().Interface("header", req.Header), "sending request header")
+		Log(ctx, ProxyLogger.Load().Debug(), "sending request header: %v", dohHeader)
 	}
+	dohHeader.Set("Content-Type", headerApplicationDNS)
+	dohHeader.Set("Accept", headerApplicationDNS)
+	req.Header = dohHeader
 }
 
-// addControlDHeaders set DoH/Doh3 HTTP request headers for ControlD upstream.
-func addControlDHeaders(req *http.Request, ci *ClientInfo) {
-	req.Header.Set(dohOsHeader, dohOsHeaderValue())
+// newControlDHeaders returns DoH/Doh3 HTTP request headers for ControlD upstream.
+func newControlDHeaders(ci *ClientInfo) http.Header {
+	header := make(http.Header)
+	header.Set(dohOsHeader, dohOsHeaderValue())
 	if ci.Mac != "" {
-		req.Header.Set(dohMacHeader, ci.Mac)
+		header.Set(dohMacHeader, ci.Mac)
 	}
 	if ci.IP != "" {
-		req.Header.Set(dohIPHeader, ci.IP)
+		header.Set(dohIPHeader, ci.IP)
 	}
 	if ci.Hostname != "" {
-		req.Header.Set(dohHostHeader, ci.Hostname)
+		header.Set(dohHostHeader, ci.Hostname)
 	}
 	if ci.Self {
-		req.Header.Set(dohOsHeader, dohOsHeaderValue())
+		header.Set(dohOsHeader, dohOsHeaderValue())
 	}
 	switch ci.ClientIDPref {
 	case "mac":
-		req.Header.Set(dohClientIDPrefHeader, "1")
+		header.Set(dohClientIDPrefHeader, "1")
 	case "host":
-		req.Header.Set(dohClientIDPrefHeader, "2")
+		header.Set(dohClientIDPrefHeader, "2")
 	}
+	return header
 }
 
-// addNextDNSHeaders set DoH/Doh3 HTTP request headers for nextdns upstream.
+// newNextDNSHeaders returns DoH/Doh3 HTTP request headers for nextdns upstream.
 // https://github.com/nextdns/nextdns/blob/v1.41.0/resolver/doh.go#L100
-func addNextDNSHeaders(req *http.Request, ci *ClientInfo) {
+func newNextDNSHeaders(ci *ClientInfo) http.Header {
+	header := make(http.Header)
 	if ci.Mac != "" {
 		// https: //github.com/nextdns/nextdns/blob/v1.41.0/run.go#L543
-		req.Header.Set("X-Device-Model", "mac:"+ci.Mac[:8])
+		header.Set("X-Device-Model", "mac:"+ci.Mac[:8])
 	}
 	if ci.IP != "" {
-		req.Header.Set("X-Device-Ip", ci.IP)
+		header.Set("X-Device-Ip", ci.IP)
 	}
 	if ci.Hostname != "" {
-		req.Header.Set("X-Device-Name", ci.Hostname)
+		header.Set("X-Device-Name", ci.Hostname)
 	}
+	return header
 }