dnsforward: fix panic; take Host into account

CHANGELOG.md

@@ -14,21 +14,41 @@ and this project adheres to
 <!--
 ## [v0.108.0] - TBA
 
-## [v0.107.25] - 2023-03-09 (APPROX.)
+## [v0.107.26] - 2023-03-09 (APPROX.)
 
-See also the [v0.107.25 GitHub milestone][ms-v0.107.25].
+See also the [v0.107.26 GitHub milestone][ms-v0.107.26].
 
-[ms-v0.107.25]: https://github.com/AdguardTeam/AdGuardHome/milestone/61?closed=1
+[ms-v0.107.26]: https://github.com/AdguardTeam/AdGuardHome/milestone/62?closed=1
 
 NOTE: Add new changes BELOW THIS COMMENT.
 -->
 
+### Fixed
+
+- Failing service installation via script on FreeBSD ([#5431]).
+
+[#5431]: https://github.com/AdguardTeam/AdGuardHome/issues/5431
+
 <!--
 NOTE: Add new changes ABOVE THIS COMMENT.
 -->
 
 
 
+## [v0.107.25] - 2023-02-21
+
+See also the [v0.107.25 GitHub milestone][ms-v0.107.25].
+
+### Fixed
+
+- Panic when using unencrypted DNS-over-HTTPS ([#5518]).
+
+[#5518]: https://github.com/AdguardTeam/AdGuardHome/issues/5518
+
+[ms-v0.107.25]: https://github.com/AdguardTeam/AdGuardHome/milestone/61?closed=1
+
+
+
 ## [v0.107.24] - 2023-02-15
 
 See also the [v0.107.24 GitHub milestone][ms-v0.107.24].
@@ -1669,11 +1689,12 @@ See also the [v0.104.2 GitHub milestone][ms-v0.104.2].
 
 
 <!--
-[Unreleased]: https://github.com/AdguardTeam/AdGuardHome/compare/v0.107.25...HEAD
+[Unreleased]: https://github.com/AdguardTeam/AdGuardHome/compare/v0.107.26...HEAD
-[v0.107.25]:  https://github.com/AdguardTeam/AdGuardHome/compare/v0.107.24...v0.107.25
+[v0.107.26]:  https://github.com/AdguardTeam/AdGuardHome/compare/v0.107.25...v0.107.26
 -->
 
-[Unreleased]: https://github.com/AdguardTeam/AdGuardHome/compare/v0.107.24...HEAD
+[Unreleased]: https://github.com/AdguardTeam/AdGuardHome/compare/v0.107.25...HEAD
+[v0.107.25]:  https://github.com/AdguardTeam/AdGuardHome/compare/v0.107.24...v0.107.25
 [v0.107.24]:  https://github.com/AdguardTeam/AdGuardHome/compare/v0.107.23...v0.107.24
 [v0.107.23]:  https://github.com/AdguardTeam/AdGuardHome/compare/v0.107.22...v0.107.23
 [v0.107.22]:  https://github.com/AdguardTeam/AdGuardHome/compare/v0.107.21...v0.107.22

internal/dnsforward/clientid.go

@@ -147,11 +147,24 @@ func (s *Server) clientIDFromDNSContext(pctx *proxy.DNSContext) (clientID string
 	return clientID, nil
 }
 
-// clientServerName returns the TLS server name based on the protocol.
+// clientServerName returns the TLS server name based on the protocol.  For
+// DNS-over-HTTPS requests, it will return the hostname part of the Host header
+// if there is one.
 func clientServerName(pctx *proxy.DNSContext, proto proxy.Proto) (srvName string, err error) {
 	switch proto {
 	case proxy.ProtoHTTPS:
-		srvName = pctx.HTTPRequest.TLS.ServerName
+		r := pctx.HTTPRequest
+		if connState := r.TLS; connState != nil {
+			srvName = connState.ServerName
+		} else if r.Host != "" {
+			var host string
+			host, err = netutil.SplitHost(r.Host)
+			if err != nil {
+				return "", fmt.Errorf("parsing host: %w", err)
+			}
+
+			srvName = host
+		}
 	case proxy.ProtoQUIC:
 		qConn := pctx.QUICConnection
 		conn, ok := qConn.(quicConnection)

internal/dnsforward/clientid_test.go

@@ -50,136 +50,169 @@ func TestServer_clientIDFromDNSContext(t *testing.T) {
 	testCases := []struct {
 		name         string
 		proto        proxy.Proto
-		hostSrvName  string
+		confSrvName  string
 		cliSrvName   string
 		wantClientID string
 		wantErrMsg   string
+		inclHTTPTLS  bool
 		strictSNI    bool
 	}{{
 		name:         "udp",
 		proto:        proxy.ProtoUDP,
-		hostSrvName:  "",
+		confSrvName:  "",
 		cliSrvName:   "",
 		wantClientID: "",
 		wantErrMsg:   "",
+		inclHTTPTLS:  false,
 		strictSNI:    false,
 	}, {
 		name:         "tls_no_clientid",
 		proto:        proxy.ProtoTLS,
-		hostSrvName:  "example.com",
+		confSrvName:  "example.com",
 		cliSrvName:   "example.com",
 		wantClientID: "",
 		wantErrMsg:   "",
+		inclHTTPTLS:  false,
 		strictSNI:    true,
 	}, {
 		name:         "tls_no_client_server_name",
 		proto:        proxy.ProtoTLS,
-		hostSrvName:  "example.com",
+		confSrvName:  "example.com",
 		cliSrvName:   "",
 		wantClientID: "",
 		wantErrMsg: `clientid check: client server name "" ` +
 			`doesn't match host server name "example.com"`,
+		inclHTTPTLS: false,
 		strictSNI:   true,
 	}, {
 		name:         "tls_no_client_server_name_no_strict",
 		proto:        proxy.ProtoTLS,
-		hostSrvName:  "example.com",
+		confSrvName:  "example.com",
 		cliSrvName:   "",
 		wantClientID: "",
 		wantErrMsg:   "",
+		inclHTTPTLS:  false,
 		strictSNI:    false,
 	}, {
 		name:         "tls_clientid",
 		proto:        proxy.ProtoTLS,
-		hostSrvName:  "example.com",
+		confSrvName:  "example.com",
 		cliSrvName:   "cli.example.com",
 		wantClientID: "cli",
 		wantErrMsg:   "",
+		inclHTTPTLS:  false,
 		strictSNI:    true,
 	}, {
 		name:         "tls_clientid_hostname_error",
 		proto:        proxy.ProtoTLS,
-		hostSrvName:  "example.com",
+		confSrvName:  "example.com",
 		cliSrvName:   "cli.example.net",
 		wantClientID: "",
 		wantErrMsg: `clientid check: client server name "cli.example.net" ` +
 			`doesn't match host server name "example.com"`,
+		inclHTTPTLS: false,
 		strictSNI:   true,
 	}, {
 		name:         "tls_invalid_clientid",
 		proto:        proxy.ProtoTLS,
-		hostSrvName:  "example.com",
+		confSrvName:  "example.com",
 		cliSrvName:   "!!!.example.com",
 		wantClientID: "",
 		wantErrMsg: `clientid check: invalid clientid "!!!": ` +
 			`bad domain name label rune '!'`,
+		inclHTTPTLS: false,
 		strictSNI:   true,
 	}, {
 		name:        "tls_clientid_too_long",
 		proto:       proxy.ProtoTLS,
-		hostSrvName: "example.com",
+		confSrvName: "example.com",
 		cliSrvName: `abcdefghijklmnopqrstuvwxyz0123456789abcdefghijklmno` +
 			`pqrstuvwxyz0123456789.example.com`,
 		wantClientID: "",
 		wantErrMsg: `clientid check: invalid clientid "abcdefghijklmno` +
 			`pqrstuvwxyz0123456789abcdefghijklmnopqrstuvwxyz0123456789": ` +
 			`domain name label is too long: got 72, max 63`,
+		inclHTTPTLS: false,
 		strictSNI:   true,
 	}, {
 		name:         "quic_clientid",
 		proto:        proxy.ProtoQUIC,
-		hostSrvName:  "example.com",
+		confSrvName:  "example.com",
 		cliSrvName:   "cli.example.com",
 		wantClientID: "cli",
 		wantErrMsg:   "",
+		inclHTTPTLS:  false,
 		strictSNI:    true,
 	}, {
 		name:         "tls_clientid_issue3437",
 		proto:        proxy.ProtoTLS,
-		hostSrvName:  "example.com",
+		confSrvName:  "example.com",
 		cliSrvName:   "cli.myexample.com",
 		wantClientID: "",
 		wantErrMsg: `clientid check: client server name "cli.myexample.com" ` +
 			`doesn't match host server name "example.com"`,
+		inclHTTPTLS: false,
 		strictSNI:   true,
 	}, {
 		name:         "tls_case",
 		proto:        proxy.ProtoTLS,
-		hostSrvName:  "example.com",
+		confSrvName:  "example.com",
 		cliSrvName:   "InSeNsItIvE.example.com",
 		wantClientID: "insensitive",
 		wantErrMsg:   ``,
+		inclHTTPTLS:  false,
 		strictSNI:    true,
 	}, {
 		name:         "quic_case",
 		proto:        proxy.ProtoQUIC,
-		hostSrvName:  "example.com",
+		confSrvName:  "example.com",
 		cliSrvName:   "InSeNsItIvE.example.com",
 		wantClientID: "insensitive",
 		wantErrMsg:   ``,
+		inclHTTPTLS:  false,
 		strictSNI:    true,
 	}, {
 		name:         "https_no_clientid",
 		proto:        proxy.ProtoHTTPS,
-		hostSrvName:  "example.com",
+		confSrvName:  "example.com",
 		cliSrvName:   "example.com",
 		wantClientID: "",
 		wantErrMsg:   "",
+		inclHTTPTLS:  true,
 		strictSNI:    true,
 	}, {
 		name:         "https_clientid",
 		proto:        proxy.ProtoHTTPS,
-		hostSrvName:  "example.com",
+		confSrvName:  "example.com",
 		cliSrvName:   "cli.example.com",
 		wantClientID: "cli",
 		wantErrMsg:   "",
+		inclHTTPTLS:  true,
+		strictSNI:    true,
+	}, {
+		name:         "https_issue5518",
+		proto:        proxy.ProtoHTTPS,
+		confSrvName:  "example.com",
+		cliSrvName:   "cli.example.com",
+		wantClientID: "cli",
+		wantErrMsg:   "",
+		inclHTTPTLS:  false,
+		strictSNI:    true,
+	}, {
+		name:         "https_no_host",
+		proto:        proxy.ProtoHTTPS,
+		confSrvName:  "example.com",
+		cliSrvName:   "example.com",
+		wantClientID: "",
+		wantErrMsg:   "",
+		inclHTTPTLS:  false,
 		strictSNI:    true,
 	}}
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
 			tlsConf := TLSConfig{
-				ServerName:     tc.hostSrvName,
+				ServerName:     tc.confSrvName,
 				StrictSNICheck: tc.strictSNI,
 			}
 
@@ -195,7 +228,7 @@ func TestServer_clientIDFromDNSContext(t *testing.T) {
 
 			switch tc.proto {
 			case proxy.ProtoHTTPS:
-				httpReq = newHTTPReq(tc.cliSrvName)
+				httpReq = newHTTPReq(tc.cliSrvName, tc.inclHTTPTLS)
 			case proxy.ProtoQUIC:
 				qconn = testQUICConnection{
 					serverName: tc.cliSrvName,
@@ -222,20 +255,25 @@ func TestServer_clientIDFromDNSContext(t *testing.T) {
 }
 
 // newHTTPReq is a helper to create HTTP requests for tests.
-func newHTTPReq(cliSrvName string) (r *http.Request) {
+func newHTTPReq(cliSrvName string, inclTLS bool) (r *http.Request) {
 	u := &url.URL{
 		Path: "/dns-query",
 	}
 
-	return &http.Request{
+	r = &http.Request{
 		ProtoMajor: 1,
 		ProtoMinor: 1,
 		URL:        u,
 		Host:       cliSrvName,
-		TLS: &tls.ConnectionState{
-			ServerName: cliSrvName,
-		},
 	}
+
+	if inclTLS {
+		r.TLS = &tls.ConnectionState{
+			ServerName: cliSrvName,
+		}
+	}
+
+	return r
 }
 
 func TestClientIDFromDNSContextHTTPS(t *testing.T) {

scripts/make/go-lint.sh

@@ -52,7 +52,7 @@ trap not_found EXIT
 go_version="$( "${GO:-go}" version )"
 readonly go_version
 
-go_min_version='go1.18'
+go_min_version='go1.19.6'
 go_version_msg="
 warning: your go version (${go_version}) is different from the recommended minimal one (${go_min_version}).
 if you have the version installed, please set the GO environment variable.