From d82b290251397395a994f2e453adb91ed455d8c8 Mon Sep 17 00:00:00 2001 From: Eugene Burkov Date: Wed, 19 Jan 2022 20:45:50 +0300 Subject: [PATCH] Pull request: 4095 fix duplicating port Merge in DNS/adguard-home from 4095-port-3000 to master Updates #4095. Squashed commit of the following: commit 968cc806264898523d29c4ec20b3ce6a69abb09c Author: Eugene Burkov Date: Wed Jan 19 20:26:33 2022 +0300 home: fix typo commit 03c6798db6a4ca726a7b5a683e475a8a74f79fe1 Author: Eugene Burkov Date: Wed Jan 19 20:20:34 2022 +0300 all: more naming imps commit d3d417fcb24a1859f53a743b3533faa81b6bef19 Author: Eugene Burkov Date: Wed Jan 19 20:10:14 2022 +0300 aghalgo: rename into aghalg commit 6e106006d07a747ff4ddf1271532106c3a3e2b20 Author: Eugene Burkov Date: Wed Jan 19 20:05:43 2022 +0300 all: imp names, docs commit 12c8d9fde0d0cc5b953da30b042171ba7c53da5d Author: Eugene Burkov Date: Wed Jan 19 19:57:21 2022 +0300 all: fix log of changes commit 49c7a705b9b1ad8f2ef68fa807f9b6b8c447b421 Author: Eugene Burkov Date: Wed Jan 19 19:51:00 2022 +0300 home: fix duplicating port 3000 --- CHANGELOG.md | 2 + .../{aghalgo/aghalgo.go => aghalg/aghalg.go} | 26 +-- internal/dnsforward/access.go | 12 +- internal/home/config.go | 14 +- internal/home/controlinstall.go | 169 +++++++++++------- internal/home/home.go | 10 +- internal/home/tls.go | 14 +- 7 files changed, 140 insertions(+), 107 deletions(-) rename internal/{aghalgo/aghalgo.go => aghalg/aghalg.go} (62%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8e135c41..7410cd93 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,7 @@ TODO(a.garipov): Remove this deprecation, if v0.108.0 is released before the Go ### Fixed +- Wrong set of ports checked for duplicates during the initial setup ([#4095]). - Incorrectly invalidated service domains ([#4120]). - Poor testing of domain-specific upstream servers ([#4074]). - Omitted aliases of hosts specified by another line within the OS's hosts file @@ -43,6 +44,7 @@ TODO(a.garipov): Remove this deprecation, if v0.108.0 is released before the Go [#3057]: https://github.com/AdguardTeam/AdGuardHome/issues/3057 [#4074]: https://github.com/AdguardTeam/AdGuardHome/issues/4074 [#4079]: https://github.com/AdguardTeam/AdGuardHome/issues/4079 +[#4095]: https://github.com/AdguardTeam/AdGuardHome/issues/4095 [#4120]: https://github.com/AdguardTeam/AdGuardHome/issues/4120 [#4133]: https://github.com/AdguardTeam/AdGuardHome/issues/4133 diff --git a/internal/aghalgo/aghalgo.go b/internal/aghalg/aghalg.go similarity index 62% rename from internal/aghalgo/aghalgo.go rename to internal/aghalg/aghalg.go index 3d937301..c3b6e906 100644 --- a/internal/aghalgo/aghalgo.go +++ b/internal/aghalg/aghalg.go @@ -1,7 +1,7 @@ -// Package aghalgo contains common generic algorithms and data structures. +// Package aghalg contains common generic algorithms and data structures. // // TODO(a.garipov): Update to use type parameters in Go 1.18. -package aghalgo +package aghalg import ( "fmt" @@ -14,20 +14,20 @@ import ( // TODO(a.garipov): Remove in Go 1.18. type comparable = interface{} -// UniquenessValidator allows validating uniqueness of comparable items. -type UniquenessValidator map[comparable]int64 +// UniqChecker allows validating uniqueness of comparable items. +type UniqChecker map[comparable]int64 // Add adds a value to the validator. v must not be nil. -func (v UniquenessValidator) Add(elems ...comparable) { +func (uc UniqChecker) Add(elems ...comparable) { for _, e := range elems { - v[e]++ + uc[e]++ } } // Merge returns a validator containing data from both v and other. -func (v UniquenessValidator) Merge(other UniquenessValidator) (merged UniquenessValidator) { - merged = make(UniquenessValidator, len(v)+len(other)) - for elem, num := range v { +func (uc UniqChecker) Merge(other UniqChecker) (merged UniqChecker) { + merged = make(UniqChecker, len(uc)+len(other)) + for elem, num := range uc { merged[elem] += num } @@ -41,9 +41,9 @@ func (v UniquenessValidator) Merge(other UniquenessValidator) (merged Uniqueness // Validate returns an error enumerating all elements that aren't unique. // isBefore is an optional sorting function to make the error message // deterministic. -func (v UniquenessValidator) Validate(isBefore func(a, b comparable) (less bool)) (err error) { +func (uc UniqChecker) Validate(isBefore func(a, b comparable) (less bool)) (err error) { var dup []comparable - for elem, num := range v { + for elem, num := range uc { if num > 1 { dup = append(dup, elem) } @@ -62,13 +62,13 @@ func (v UniquenessValidator) Validate(isBefore func(a, b comparable) (less bool) return fmt.Errorf("duplicated values: %v", dup) } -// IntIsBefore is a helper sort function for UniquenessValidator.Validate. +// IntIsBefore is a helper sort function for UniqChecker.Validate. // a and b must be of type int. func IntIsBefore(a, b comparable) (less bool) { return a.(int) < b.(int) } -// StringIsBefore is a helper sort function for UniquenessValidator.Validate. +// StringIsBefore is a helper sort function for UniqChecker.Validate. // a and b must be of type string. func StringIsBefore(a, b comparable) (less bool) { return a.(string) < b.(string) diff --git a/internal/dnsforward/access.go b/internal/dnsforward/access.go index f2f2df4a..76145fdf 100644 --- a/internal/dnsforward/access.go +++ b/internal/dnsforward/access.go @@ -7,7 +7,7 @@ import ( "net/http" "strings" - "github.com/AdguardTeam/AdGuardHome/internal/aghalgo" + "github.com/AdguardTeam/AdGuardHome/internal/aghalg" "github.com/AdguardTeam/AdGuardHome/internal/aghhttp" "github.com/AdguardTeam/golibs/log" "github.com/AdguardTeam/golibs/netutil" @@ -214,7 +214,7 @@ func validateAccessSet(list *accessListJSON) (err error) { } merged := allowed.Merge(disallowed) - err = merged.Validate(aghalgo.StringIsBefore) + err = merged.Validate(aghalg.StringIsBefore) if err != nil { return fmt.Errorf("items in allowed and disallowed clients intersect: %w", err) } @@ -223,13 +223,13 @@ func validateAccessSet(list *accessListJSON) (err error) { } // validateStrUniq returns an informative error if clients are not unique. -func validateStrUniq(clients []string) (uv aghalgo.UniquenessValidator, err error) { - uv = make(aghalgo.UniquenessValidator, len(clients)) +func validateStrUniq(clients []string) (uc aghalg.UniqChecker, err error) { + uc = make(aghalg.UniqChecker, len(clients)) for _, c := range clients { - uv.Add(c) + uc.Add(c) } - return uv, uv.Validate(aghalgo.StringIsBefore) + return uc, uc.Validate(aghalg.StringIsBefore) } func (s *Server) handleAccessSet(w http.ResponseWriter, r *http.Request) { diff --git a/internal/home/config.go b/internal/home/config.go index d036a085..a6247164 100644 --- a/internal/home/config.go +++ b/internal/home/config.go @@ -7,7 +7,7 @@ import ( "path/filepath" "sync" - "github.com/AdguardTeam/AdGuardHome/internal/aghalgo" + "github.com/AdguardTeam/AdGuardHome/internal/aghalg" "github.com/AdguardTeam/AdGuardHome/internal/dhcpd" "github.com/AdguardTeam/AdGuardHome/internal/dnsforward" "github.com/AdguardTeam/AdGuardHome/internal/filtering" @@ -288,9 +288,9 @@ func parseConfig() (err error) { return err } - uv := aghalgo.UniquenessValidator{} + uc := aghalg.UniqChecker{} addPorts( - uv, + uc, config.BindPort, config.BetaBindPort, config.DNS.Port, @@ -298,14 +298,14 @@ func parseConfig() (err error) { if config.TLS.Enabled { addPorts( - uv, + uc, config.TLS.PortHTTPS, config.TLS.PortDNSOverTLS, config.TLS.PortDNSOverQUIC, config.TLS.PortDNSCrypt, ) } - if err = uv.Validate(aghalgo.IntIsBefore); err != nil { + if err = uc.Validate(aghalg.IntIsBefore); err != nil { return fmt.Errorf("validating ports: %w", err) } @@ -321,10 +321,10 @@ func parseConfig() (err error) { } // addPorts is a helper for ports validation. It skips zero ports. -func addPorts(uv aghalgo.UniquenessValidator, ports ...int) { +func addPorts(uc aghalg.UniqChecker, ports ...int) { for _, p := range ports { if p != 0 { - uv.Add(p) + uc.Add(p) } } } diff --git a/internal/home/controlinstall.go b/internal/home/controlinstall.go index 5ca99151..b9c302ee 100644 --- a/internal/home/controlinstall.go +++ b/internal/home/controlinstall.go @@ -14,7 +14,7 @@ import ( "strings" "time" - "github.com/AdguardTeam/AdGuardHome/internal/aghalgo" + "github.com/AdguardTeam/AdGuardHome/internal/aghalg" "github.com/AdguardTeam/AdGuardHome/internal/aghhttp" "github.com/AdguardTeam/AdGuardHome/internal/aghnet" "github.com/AdguardTeam/AdGuardHome/internal/version" @@ -73,19 +73,19 @@ func (web *Web) handleInstallGetAddresses(w http.ResponseWriter, r *http.Request } } -type checkConfigReqEnt struct { +type checkConfReqEnt struct { IP net.IP `json:"ip"` Port int `json:"port"` Autofix bool `json:"autofix"` } -type checkConfigReq struct { - Web checkConfigReqEnt `json:"web"` - DNS checkConfigReqEnt `json:"dns"` - SetStaticIP bool `json:"set_static_ip"` +type checkConfReq struct { + Web checkConfReqEnt `json:"web"` + DNS checkConfReqEnt `json:"dns"` + SetStaticIP bool `json:"set_static_ip"` } -type checkConfigRespEnt struct { +type checkConfRespEnt struct { Status string `json:"status"` CanAutofix bool `json:"can_autofix"` } @@ -96,79 +96,110 @@ type staticIPJSON struct { Error string `json:"error"` } -type checkConfigResp struct { - StaticIP staticIPJSON `json:"static_ip"` - Web checkConfigRespEnt `json:"web"` - DNS checkConfigRespEnt `json:"dns"` +type checkConfResp struct { + StaticIP staticIPJSON `json:"static_ip"` + Web checkConfRespEnt `json:"web"` + DNS checkConfRespEnt `json:"dns"` } -// Check if ports are available, respond with results -func (web *Web) handleInstallCheckConfig(w http.ResponseWriter, r *http.Request) { - reqData := checkConfigReq{} - respData := checkConfigResp{} +// validateWeb returns error is the web part if the initial configuration can't +// be set. +func (req *checkConfReq) validateWeb(uc aghalg.UniqChecker) (err error) { + defer func() { err = errors.Annotate(err, "validating ports: %w") }() - err := json.NewDecoder(r.Body).Decode(&reqData) + port := req.Web.Port + addPorts(uc, config.BetaBindPort, port) + if err = uc.Validate(aghalg.IntIsBefore); err != nil { + // Avoid duplicating the error into the status of DNS. + uc[port] = 1 + + return err + } + + switch port { + case 0, config.BindPort: + return nil + default: + // Go on and check the port binding only if it's not zero or won't be + // unbound after install. + } + + return aghnet.CheckPort("tcp", req.Web.IP, port) +} + +// validateDNS returns error if the DNS part of the initial configuration can't +// be set. autofix is true if the port can be unbound by AdGuard Home +// automatically. +func (req *checkConfReq) validateDNS(uc aghalg.UniqChecker) (canAutofix bool, err error) { + defer func() { err = errors.Annotate(err, "validating ports: %w") }() + + port := req.DNS.Port + addPorts(uc, port) + if err = uc.Validate(aghalg.IntIsBefore); err != nil { + return false, err + } + + switch port { + case 0: + return false, nil + case config.BindPort: + // Go on and only check the UDP port since the TCP one is already bound + // by AdGuard Home for web interface. + default: + // Check TCP as well. + err = aghnet.CheckPort("tcp", req.DNS.IP, port) + if err != nil { + return false, err + } + } + + err = aghnet.CheckPort("udp", req.DNS.IP, port) + if !aghnet.IsAddrInUse(err) { + return false, err + } + + // Try to fix automatically. + canAutofix = checkDNSStubListener() + if canAutofix && req.DNS.Autofix { + if derr := disableDNSStubListener(); derr != nil { + log.Error("disabling DNSStubListener: %s", err) + } + + err = aghnet.CheckPort("udp", req.DNS.IP, port) + canAutofix = false + } + + return canAutofix, err +} + +// handleInstallCheckConfig handles the /check_config endpoint. +func (web *Web) handleInstallCheckConfig(w http.ResponseWriter, r *http.Request) { + req := &checkConfReq{} + + err := json.NewDecoder(r.Body).Decode(req) if err != nil { - aghhttp.Error(r, w, http.StatusBadRequest, "Failed to parse 'check_config' JSON data: %s", err) + aghhttp.Error(r, w, http.StatusBadRequest, "decoding the request: %s", err) return } - uv := aghalgo.UniquenessValidator{} - addPorts( - uv, - config.BindPort, - config.BetaBindPort, - reqData.Web.Port, - ) - if err = uv.Validate(aghalgo.IntIsBefore); err != nil { - err = fmt.Errorf("validating ports: %w", err) - respData.Web.Status = err.Error() - } else if reqData.Web.Port != 0 { - err = aghnet.CheckPort("tcp", reqData.Web.IP, reqData.Web.Port) - if err != nil { - respData.Web.Status = err.Error() - } + resp := &checkConfResp{} + uc := aghalg.UniqChecker{} + + if err = req.validateWeb(uc); err != nil { + resp.Web.Status = err.Error() } - addPorts(uv, reqData.DNS.Port) - if err = uv.Validate(aghalgo.IntIsBefore); err != nil { - err = fmt.Errorf("validating ports: %w", err) - respData.DNS.Status = err.Error() - } else if reqData.DNS.Port != 0 { - err = aghnet.CheckPort("udp", reqData.DNS.IP, reqData.DNS.Port) - - if aghnet.IsAddrInUse(err) { - canAutofix := checkDNSStubListener() - if canAutofix && reqData.DNS.Autofix { - - err = disableDNSStubListener() - if err != nil { - log.Error("Couldn't disable DNSStubListener: %s", err) - } - - err = aghnet.CheckPort("udp", reqData.DNS.IP, reqData.DNS.Port) - canAutofix = false - } - - respData.DNS.CanAutofix = canAutofix - } - - if err == nil { - err = aghnet.CheckPort("tcp", reqData.DNS.IP, reqData.DNS.Port) - } - - if err != nil { - respData.DNS.Status = err.Error() - } else if !reqData.DNS.IP.IsUnspecified() { - respData.StaticIP = handleStaticIP(reqData.DNS.IP, reqData.SetStaticIP) - } + if resp.DNS.CanAutofix, err = req.validateDNS(uc); err != nil { + resp.DNS.Status = err.Error() + } else if !req.DNS.IP.IsUnspecified() { + resp.StaticIP = handleStaticIP(req.DNS.IP, req.SetStaticIP) } w.Header().Set("Content-Type", "application/json") - err = json.NewEncoder(w).Encode(respData) + err = json.NewEncoder(w).Encode(resp) if err != nil { - aghhttp.Error(r, w, http.StatusInternalServerError, "Unable to marshal JSON: %s", err) + aghhttp.Error(r, w, http.StatusInternalServerError, "encoding the response: %s", err) return } @@ -494,13 +525,13 @@ func (web *Web) handleInstallCheckConfigBeta(w http.ResponseWriter, r *http.Requ return } - nonBetaReqData := checkConfigReq{ - Web: checkConfigReqEnt{ + nonBetaReqData := checkConfReq{ + Web: checkConfReqEnt{ IP: reqData.Web.IP[0], Port: reqData.Web.Port, Autofix: reqData.Web.Autofix, }, - DNS: checkConfigReqEnt{ + DNS: checkConfReqEnt{ IP: reqData.DNS.IP[0], Port: reqData.DNS.Port, Autofix: reqData.DNS.Autofix, diff --git a/internal/home/home.go b/internal/home/home.go index 05d8c3fe..90674d48 100644 --- a/internal/home/home.go +++ b/internal/home/home.go @@ -19,7 +19,7 @@ import ( "syscall" "time" - "github.com/AdguardTeam/AdGuardHome/internal/aghalgo" + "github.com/AdguardTeam/AdGuardHome/internal/aghalg" "github.com/AdguardTeam/AdGuardHome/internal/aghnet" "github.com/AdguardTeam/AdGuardHome/internal/aghos" "github.com/AdguardTeam/AdGuardHome/internal/dhcpd" @@ -296,23 +296,23 @@ func setupConfig(args options) (err error) { Context.clients.Init(config.Clients, Context.dhcpServer, Context.etcHosts) if args.bindPort != 0 { - uv := aghalgo.UniquenessValidator{} + uc := aghalg.UniqChecker{} addPorts( - uv, + uc, args.bindPort, config.BetaBindPort, config.DNS.Port, ) if config.TLS.Enabled { addPorts( - uv, + uc, config.TLS.PortHTTPS, config.TLS.PortDNSOverTLS, config.TLS.PortDNSOverQUIC, config.TLS.PortDNSCrypt, ) } - if err = uv.Validate(aghalgo.IntIsBefore); err != nil { + if err = uc.Validate(aghalg.IntIsBefore); err != nil { return fmt.Errorf("validating ports: %w", err) } diff --git a/internal/home/tls.go b/internal/home/tls.go index 37e03b0b..a201be14 100644 --- a/internal/home/tls.go +++ b/internal/home/tls.go @@ -20,7 +20,7 @@ import ( "sync" "time" - "github.com/AdguardTeam/AdGuardHome/internal/aghalgo" + "github.com/AdguardTeam/AdGuardHome/internal/aghalg" "github.com/AdguardTeam/AdGuardHome/internal/aghhttp" "github.com/AdguardTeam/AdGuardHome/internal/dnsforward" "github.com/AdguardTeam/golibs/errors" @@ -251,9 +251,9 @@ func (t *TLSMod) handleTLSValidate(w http.ResponseWriter, r *http.Request) { } if setts.Enabled { - uv := aghalgo.UniquenessValidator{} + uc := aghalg.UniqChecker{} addPorts( - uv, + uc, config.BindPort, config.BetaBindPort, config.DNS.Port, @@ -263,7 +263,7 @@ func (t *TLSMod) handleTLSValidate(w http.ResponseWriter, r *http.Request) { setts.PortDNSCrypt, ) - err = uv.Validate(aghalgo.IntIsBefore) + err = uc.Validate(aghalg.IntIsBefore) if err != nil { aghhttp.Error(r, w, http.StatusBadRequest, "validating ports: %s", err) @@ -344,9 +344,9 @@ func (t *TLSMod) handleTLSConfigure(w http.ResponseWriter, r *http.Request) { } if data.Enabled { - uv := aghalgo.UniquenessValidator{} + uc := aghalg.UniqChecker{} addPorts( - uv, + uc, config.BindPort, config.BetaBindPort, config.DNS.Port, @@ -356,7 +356,7 @@ func (t *TLSMod) handleTLSConfigure(w http.ResponseWriter, r *http.Request) { data.PortDNSCrypt, ) - err = uv.Validate(aghalgo.IntIsBefore) + err = uc.Validate(aghalg.IntIsBefore) if err != nil { aghhttp.Error(r, w, http.StatusBadRequest, "%s", err)