Pull request: 3835 check ports properly

Merge in DNS/adguard-home from 3835-imp-error-msg to master

Updates AdguardTeam#3835.

Squashed commit of the following:

commit ba31cb6
Merge: 19c7dfc 4be69d3
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Thu Dec 16 20:07:25 2021 +0300

    Merge branch 'master' into 3835-imp-error-msg

commit 19c7dfc
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Thu Dec 16 19:42:10 2021 +0300

    all: imp more

commit 5b9c6a3
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Thu Dec 16 18:57:02 2021 +0300

    all: introduce aghhttp

commit 29caa17
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Thu Dec 16 14:23:53 2021 +0300

    all: imp more

commit 754c020
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Wed Dec 15 20:53:41 2021 +0300

    all: imp code, log changes

commit ec712dd
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Wed Dec 15 18:40:54 2021 +0300

    home: check ports properly

CHANGELOG.md

@@ -49,6 +49,7 @@ and this project adheres to
 
 ### Changed
 
+- Port bindings are now checked for uniqueness ([#3835]).
 - The DNSSEC check now simply checks against the AD flag in the response
   ([#3904]).
 - Client objects in the configuration file are now sorted ([#3933]).
@@ -241,6 +242,7 @@ In this release, the schema version has changed from 10 to 12.
 [#3772]: https://github.com/AdguardTeam/AdGuardHome/issues/3772
 [#3778]: https://github.com/AdguardTeam/AdGuardHome/issues/3778
 [#3815]: https://github.com/AdguardTeam/AdGuardHome/issues/3815
+[#3835]: https://github.com/AdguardTeam/AdGuardHome/issues/3835
 [#3887]: https://github.com/AdguardTeam/AdGuardHome/issues/3887
 [#3890]: https://github.com/AdguardTeam/AdGuardHome/issues/3890
 [#3904]: https://github.com/AdguardTeam/AdGuardHome/issues/3904

internal/aghhttp/aghhttp.go

@@ -0,0 +1,24 @@
+// Package aghhttp provides some common methods to work with HTTP.
+package aghhttp
+
+import (
+	"fmt"
+	"io"
+	"net/http"
+
+	"github.com/AdguardTeam/golibs/log"
+)
+
+// OK responds with word OK.
+func OK(w http.ResponseWriter) {
+	if _, err := io.WriteString(w, "OK\n"); err != nil {
+		log.Error("couldn't write body: %s", err)
+	}
+}
+
+// Error writes formatted message to w and also logs it.
+func Error(r *http.Request, w http.ResponseWriter, code int, format string, args ...interface{}) {
+	text := fmt.Sprintf(format, args...)
+	log.Error("%s %s: %s", r.Method, r.URL, text)
+	http.Error(w, text, code)
+}

internal/aghnet/hostscontainer.go

@@ -431,7 +431,7 @@ func (hp *hostsParser) writeMainHostRule(host string, ip net.IP) (added, addedPt
 		rwSuccess    = "^$dnsrewrite=NOERROR;"
 		rwSuccessPTR = "^$dnsrewrite=NOERROR;PTR;"
 
-		modLen    = len("||") + len(rwSuccess)
+		modLen    = len("||") + len(rwSuccess) + len(";")
 		modLenPTR = len("||") + len(rwSuccessPTR)
 	)
 

internal/aghnet/interfaces.go

@@ -118,3 +118,11 @@ func IfaceDNSIPAddrs(
 
 	return addrs, nil
 }
+
+// interfaceName is a string containing network interface's name.  The name is
+// used in file walking methods.
+type interfaceName string
+
+// Use interfaceName in the OS-independent code since it's actually only used in
+// several OS-dependent implementations which causes linting issues.
+var _ = interfaceName("")

internal/aghnet/net.go

@@ -4,13 +4,11 @@ package aghnet
 import (
 	"encoding/json"
 	"fmt"
+	"io"
 	"net"
-	"os"
 	"os/exec"
-	"runtime"
 	"strings"
 	"syscall"
-	"time"
 
 	"github.com/AdguardTeam/golibs/errors"
 	"github.com/AdguardTeam/golibs/log"
@@ -189,57 +187,30 @@ func GetSubnet(ifaceName string) *net.IPNet {
 	return nil
 }
 
-// CheckPortAvailable - check if TCP port is available
-func CheckPortAvailable(host net.IP, port int) error {
-	ln, err := net.Listen("tcp", netutil.JoinHostPort(host.String(), port))
-	if err != nil {
-		return err
-	}
-	_ = ln.Close()
-
-	// It seems that net.Listener.Close() doesn't close file descriptors right away.
-	// We wait for some time and hope that this fd will be closed.
-	time.Sleep(100 * time.Millisecond)
-	return nil
-}
-
-// CheckPacketPortAvailable - check if UDP port is available
-func CheckPacketPortAvailable(host net.IP, port int) error {
-	ln, err := net.ListenPacket("udp", netutil.JoinHostPort(host.String(), port))
-	if err != nil {
-		return err
+// CheckPort checks if the port is available for binding.
+func CheckPort(network string, ip net.IP, port int) (err error) {
+	var c io.Closer
+	addr := netutil.IPPort{IP: ip, Port: port}.String()
+	switch network {
+	case "tcp":
+		c, err = net.Listen(network, addr)
+	case "udp":
+		c, err = net.ListenPacket(network, addr)
+	default:
+		return nil
 	}
-	_ = ln.Close()
 
-	// It seems that net.Listener.Close() doesn't close file descriptors right away.
-	// We wait for some time and hope that this fd will be closed.
-	time.Sleep(100 * time.Millisecond)
-	return err
+	return errors.WithDeferred(err, closePortChecker(c))
 }
 
-// ErrorIsAddrInUse - check if error is "address already in use"
-func ErrorIsAddrInUse(err error) bool {
-	errOpError, ok := err.(*net.OpError)
-	if !ok {
+// IsAddrInUse checks if err is about unsuccessful address binding.
+func IsAddrInUse(err error) (ok bool) {
+	var sysErr syscall.Errno
+	if !errors.As(err, &sysErr) {
 		return false
 	}
 
-	errSyscallError, ok := errOpError.Err.(*os.SyscallError)
-	if !ok {
-		return false
-	}
-
-	errErrno, ok := errSyscallError.Err.(syscall.Errno)
-	if !ok {
-		return false
-	}
-
-	if runtime.GOOS == "windows" {
-		const WSAEADDRINUSE = 10048
-		return errErrno == WSAEADDRINUSE
-	}
-
-	return errErrno == syscall.EADDRINUSE
+	return isAddrInUse(sysErr)
 }
 
 // SplitHost is a wrapper for net.SplitHostPort for the cases when the hostport

internal/aghnet/net_unix.go

@@ -1,8 +1,19 @@
-//go:build openbsd || freebsd || linux
-// +build openbsd freebsd linux
+//go:build openbsd || freebsd || linux || darwin
+// +build openbsd freebsd linux darwin
 
 package aghnet
 
-// interfaceName is a string containing network interface's name.  The name is
-// used in file walking methods.
-type interfaceName string
+import (
+	"io"
+	"syscall"
+
+	"github.com/AdguardTeam/golibs/errors"
+)
+
+func closePortChecker(c io.Closer) (err error) {
+	return c.Close()
+}
+
+func isAddrInUse(err syscall.Errno) (ok bool) {
+	return errors.Is(err, syscall.EADDRINUSE)
+}

internal/aghnet/net_windows.go

@@ -0,0 +1,45 @@
+//go:build !(linux || darwin || freebsd || openbsd)
+// +build !linux,!darwin,!freebsd,!openbsd
+
+package aghnet
+
+import (
+	"io"
+	"syscall"
+	"time"
+
+	"github.com/AdguardTeam/AdGuardHome/internal/aghos"
+	"github.com/AdguardTeam/golibs/errors"
+	"golang.org/x/sys/windows"
+)
+
+func canBindPrivilegedPorts() (can bool, err error) {
+	return aghos.HaveAdminRights()
+}
+
+func ifaceHasStaticIP(string) (ok bool, err error) {
+	return false, aghos.Unsupported("checking static ip")
+}
+
+func ifaceSetStaticIP(string) (err error) {
+	return aghos.Unsupported("setting static ip")
+}
+
+func closePortChecker(c io.Closer) (err error) {
+	if err = c.Close(); err != nil {
+		return err
+	}
+
+	// It seems that net.Listener.Close() doesn't close file descriptors right
+	// away.  We wait for some time and hope that this fd will be closed.
+	//
+	// TODO(e.burkov):  Investigate the purpose of the line and perhaps use more
+	// reliable approach.
+	time.Sleep(100 * time.Millisecond)
+
+	return nil
+}
+
+func isAddrInUse(err syscall.Errno) (ok bool) {
+	return errors.Is(err, windows.WSAEADDRINUSE)
+}