home: add tls.admin_listen_addr for separate https admin listener

CHANGELOG.md

@@ -18,6 +18,10 @@ See also the [v0.107.75 GitHub milestone][ms-v0.107.75].
 NOTE: Add new changes BELOW THIS COMMENT.
 -->
 
+### Added
+
+- New optional `tls.admin_listen_addr` configuration setting (an `IP:port` listen address, e.g. `127.0.0.1:4443`).  When set, AdGuard Home starts a second HTTPS server on this address dedicated to the admin UI and API, while the HTTPS server on `tls.port_https` serves DNS-over-HTTPS only.  Both servers share the same TLS certificate.  When unset (the default), behavior is unchanged: one HTTPS server on `tls.port_https` serves both admin UI and DoH ([#7424], [#7598]).
+
 ### Changed
 
 - `enable_dnssec` in `dns` configuration now defines whether the proxy should set the DO flag in the upstream requests, the default is `true` ([#7046]).
@@ -27,6 +31,8 @@ NOTE: Add new changes BELOW THIS COMMENT.
 - Safe Browsing and Parental Control labels on the General Settings page not updating after changing the UI language.
 
 [#7046]: https://github.com/AdguardTeam/AdGuardHome/issues/7046
+[#7424]: https://github.com/AdguardTeam/AdGuardHome/issues/7424
+[#7598]: https://github.com/AdguardTeam/AdGuardHome/issues/7598
 
 <!--
 NOTE: Add new changes ABOVE THIS COMMENT.

internal/home/adminlistenaddr_internal_test.go

@@ -0,0 +1,237 @@
+package home
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"net/netip"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+// TestAdminListenAddr verifies that [adminListenAddr] returns the configured
+// address only when it is valid and has a non-zero port.
+func TestAdminListenAddr(t *testing.T) {
+	t.Parallel()
+
+	testCases := []struct {
+		name string
+		in   netip.AddrPort
+		want netip.AddrPort
+	}{{
+		name: "zero",
+		in:   netip.AddrPort{},
+		want: netip.AddrPort{},
+	}, {
+		name: "zero_port",
+		in:   netip.AddrPortFrom(netip.MustParseAddr("127.0.0.1"), 0),
+		want: netip.AddrPort{},
+	}, {
+		name: "loopback_port",
+		in:   netip.MustParseAddrPort("127.0.0.1:4443"),
+		want: netip.MustParseAddrPort("127.0.0.1:4443"),
+	}, {
+		name: "unspecified_port",
+		in:   netip.MustParseAddrPort("0.0.0.0:8443"),
+		want: netip.MustParseAddrPort("0.0.0.0:8443"),
+	}}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			conf := &tlsConfigSettings{AdminListenAddr: tc.in}
+			assert.Equal(t, tc.want, adminListenAddr(conf))
+		})
+	}
+
+	t.Run("nil", func(t *testing.T) {
+		t.Parallel()
+
+		assert.Equal(t, netip.AddrPort{}, adminListenAddr(nil))
+	})
+}
+
+// TestValidatePorts_adminHTTPS verifies that [validatePorts] includes the
+// optional dedicated admin HTTPS port in its uniqueness check.
+func TestValidatePorts_adminHTTPS(t *testing.T) {
+	t.Parallel()
+
+	// Baseline ports used below.  These are non-overlapping so that the test
+	// cases below fail or pass solely based on the admin HTTPS port.
+	const (
+		bindPort        tcpPort = 3000
+		dohPort         tcpPort = 443
+		dotPort         tcpPort = 853
+		dnscryptTCPPort tcpPort = 5443
+		dnsPort         udpPort = 53
+		doqPort         udpPort = 784
+	)
+
+	testCases := []struct {
+		name           string
+		adminHTTPSPort tcpPort
+		wantErr        bool
+	}{{
+		name:           "unused",
+		adminHTTPSPort: 0,
+		wantErr:        false,
+	}, {
+		name:           "unique",
+		adminHTTPSPort: 4443,
+		wantErr:        false,
+	}, {
+		name:           "conflicts_with_doh",
+		adminHTTPSPort: dohPort,
+		wantErr:        true,
+	}, {
+		name:           "conflicts_with_webapi",
+		adminHTTPSPort: bindPort,
+		wantErr:        true,
+	}, {
+		name:           "conflicts_with_dot",
+		adminHTTPSPort: dotPort,
+		wantErr:        true,
+	}, {
+		name:           "conflicts_with_plain_dns",
+		adminHTTPSPort: tcpPort(dnsPort),
+		wantErr:        true,
+	}}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			err := validatePorts(
+				bindPort,
+				dohPort,
+				dotPort,
+				dnscryptTCPPort,
+				tc.adminHTTPSPort,
+				dnsPort,
+				doqPort,
+			)
+			if tc.wantErr {
+				assert.Error(t, err)
+			} else {
+				assert.NoError(t, err)
+			}
+		})
+	}
+}
+
+// TestSetPrivateFieldsAndCompare_adminListenAddr verifies that
+// [tlsConfigSettings.setPrivateFieldsAndCompare] preserves the server-side
+// [tlsConfigSettings.AdminListenAddr] value across updates from the frontend
+// and that [cmp.Equal] is able to compare the field.
+func TestSetPrivateFieldsAndCompare_adminListenAddr(t *testing.T) {
+	t.Parallel()
+
+	const testAddr = "127.0.0.1:4443"
+
+	server := &tlsConfigSettings{
+		Enabled:         true,
+		PortHTTPS:       443,
+		AdminListenAddr: netip.MustParseAddrPort(testAddr),
+	}
+
+	// Simulate the frontend sending a new TLS config that does not include
+	// AdminListenAddr (since it is not exposed in the UI).  The server-side
+	// value must be preserved.
+	fromFrontend := &tlsConfigSettings{
+		Enabled:   true,
+		PortHTTPS: 443,
+	}
+
+	equal := server.setPrivateFieldsAndCompare(fromFrontend)
+	assert.True(
+		t,
+		equal,
+		"configs should be equal when only AdminListenAddr is missing from the frontend",
+	)
+	assert.Equal(
+		t,
+		netip.MustParseAddrPort(testAddr),
+		fromFrontend.AdminListenAddr,
+		"server-side AdminListenAddr should be copied into frontend payload",
+	)
+
+	// If the frontend changes an actual user-editable field, the configs must
+	// compare unequal.
+	fromFrontend2 := &tlsConfigSettings{
+		Enabled:   true,
+		PortHTTPS: 8443,
+	}
+
+	equal = server.setPrivateFieldsAndCompare(fromFrontend2)
+	assert.False(t, equal, "configs should be unequal when PortHTTPS differs")
+}
+
+// TestRegisterDoHHandlers_muxSplit verifies that when
+// [tlsConfigSettings.AdminListenAddr] is configured, [registerDoHHandlers]
+// registers DoH routes on the dedicated DoH mux rather than on the shared
+// admin mux.
+func TestRegisterDoHHandlers_muxSplit(t *testing.T) {
+	// Save and restore the globals touched by this test.  The global
+	// [homeContext] cannot be copied by value because it contains mutexes,
+	// so save only the single field that is overwritten.
+	prevWeb := globalContext.web
+	prevConfig := config
+	t.Cleanup(func() {
+		globalContext.web = prevWeb
+		config = prevConfig
+	})
+
+	const dohRoute = "/dns-query"
+
+	testCases := []struct {
+		name            string
+		adminListenAddr netip.AddrPort
+		wantOnAdmin     bool
+		wantOnDoH       bool
+	}{{
+		name:            "disabled",
+		adminListenAddr: netip.AddrPort{},
+		wantOnAdmin:     true,
+		wantOnDoH:       false,
+	}, {
+		name:            "enabled",
+		adminListenAddr: netip.MustParseAddrPort("127.0.0.1:4443"),
+		wantOnAdmin:     false,
+		wantOnDoH:       true,
+	}}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			config = &configuration{
+				TLS: tlsConfigSettings{
+					AdminListenAddr: tc.adminListenAddr,
+				},
+			}
+
+			adminMux := http.NewServeMux()
+			dohMux := http.NewServeMux()
+
+			globalContext.web = &webAPI{
+				conf: &webAPIConfig{
+					mux:    adminMux,
+					dohMux: dohMux,
+				},
+			}
+
+			registerDoHHandlers([]string{dohRoute})
+
+			assert.Equal(t, tc.wantOnAdmin, hasPattern(adminMux, dohRoute))
+			assert.Equal(t, tc.wantOnDoH, hasPattern(dohMux, dohRoute))
+		})
+	}
+}
+
+// hasPattern returns true if pattern is registered on mux, as determined by
+// [http.ServeMux.Handler].
+func hasPattern(mux *http.ServeMux, pattern string) (ok bool) {
+	req := httptest.NewRequest(http.MethodGet, pattern, nil)
+	_, p := mux.Handler(req)
+
+	return p == pattern
+}

internal/home/config.go

@@ -29,6 +29,7 @@ import (
 	"github.com/AdguardTeam/golibs/netutil"
 	"github.com/AdguardTeam/golibs/timeutil"
 	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
 	"github.com/google/renameio/v2/maybe"
 	yaml "go.yaml.in/yaml/v4"
 )
@@ -312,6 +313,18 @@ type tlsConfigSettings struct {
 	// PortHTTPS is the HTTPS port.  If 0, HTTPS will be disabled.
 	PortHTTPS uint16 `yaml:"port_https" json:"port_https,omitempty"`
 
+	// AdminListenAddr is the optional dedicated listen address (IP and port)
+	// for the HTTPS admin UI and API.  When it is valid and non-zero, a
+	// separate HTTPS server is started on this address that serves only admin
+	// UI and API requests, and the HTTPS server on [PortHTTPS] serves DNS-over-
+	// HTTPS only.  Both servers share the same TLS certificate.  When unset,
+	// admin UI and DoH continue to share the HTTPS server on [PortHTTPS].
+	//
+	// This field is intended to be configured via YAML only; it is not exposed
+	// in the web UI.  See https://github.com/AdguardTeam/AdGuardHome/issues/7424
+	// and https://github.com/AdguardTeam/AdGuardHome/issues/7598.
+	AdminListenAddr netip.AddrPort `yaml:"admin_listen_addr" json:"-"`
+
 	// PortDNSOverTLS is the DNS-over-TLS port.  If 0, DoT will be disabled.
 	PortDNSOverTLS uint16 `yaml:"port_dns_over_tls" json:"port_dns_over_tls,omitempty"`
 
@@ -382,6 +395,7 @@ func (c *tlsConfigSettings) clone() (clone *tlsConfigSettings) {
 //	[tlsConfigSettings.DNSCryptConfigFile]
 //	[tlsConfigSettings.OverrideTLSCiphers]
 //	[tlsConfigSettings.PortDNSCrypt]
+//	[tlsConfigSettings.AdminListenAddr]
 //
 // The following properties are skipped as they are set by
 // [tlsManager.loadTLSConfig]:
@@ -393,9 +407,13 @@ func (c *tlsConfigSettings) setPrivateFieldsAndCompare(conf *tlsConfigSettings)
 
 	conf.DNSCryptConfigFile = c.DNSCryptConfigFile
 	conf.PortDNSCrypt = c.PortDNSCrypt
+	conf.AdminListenAddr = c.AdminListenAddr
 
 	// TODO(a.garipov): Define a custom comparer.
-	return cmp.Equal(c, conf)
+	//
+	// [netip.AddrPort] contains unexported fields, so it needs an explicit
+	// equality option.
+	return cmp.Equal(c, conf, cmpopts.EquateComparable(netip.AddrPort{}))
 }
 
 type queryLogConfig struct {
@@ -803,6 +821,10 @@ func validateConfig(ctx context.Context, l *slog.Logger, fileData []byte) (err e
 			tcpPort(config.TLS.PortDNSCrypt),
 		)
 
+		if adminAddr := adminListenAddr(&config.TLS); adminAddr != (netip.AddrPort{}) {
+			addPorts(tcpPorts, tcpPort(adminAddr.Port()))
+		}
+
 		// TODO(e.burkov):  Consider adding a udpPort with the same value when
 		// we add support for HTTP/3 for web admin interface.
 		addPorts(udpPorts, udpPort(config.TLS.PortDNSOverQUIC))

internal/home/control.go

@@ -368,18 +368,7 @@ func (web *webAPI) handleHTTPSRedirect(w http.ResponseWriter, r *http.Request) (
 		return false
 	}
 
-	var (
-		forceHTTPS bool
-		serveHTTP3 bool
-		portHTTPS  uint16
-	)
-	func() {
-		config.RLock()
-		defer config.RUnlock()
-
-		serveHTTP3, portHTTPS = config.DNS.ServeHTTP3, config.TLS.PortHTTPS
-		forceHTTPS = config.TLS.ForceHTTPS && config.TLS.Enabled && config.TLS.PortHTTPS != 0
-	}()
+	forceHTTPS, serveHTTP3, portHTTPS, redirectPort := readHTTPSRedirectConfig()
 
 	respHdr := w.Header()
 
@@ -396,7 +385,7 @@ func (web *webAPI) handleHTTPSRedirect(w http.ResponseWriter, r *http.Request) (
 
 	if forceHTTPS {
 		if r.TLS == nil {
-			u := httpsURL(r.URL, host, portHTTPS)
+			u := httpsURL(r.URL, host, redirectPort)
 			http.Redirect(w, r, u.String(), http.StatusTemporaryRedirect)
 
 			return false
@@ -423,6 +412,32 @@ func (web *webAPI) handleHTTPSRedirect(w http.ResponseWriter, r *http.Request) (
 	return true
 }
 
+// readHTTPSRedirectConfig reads the configuration fields used by
+// [webAPI.handleHTTPSRedirect] under a single read-lock.  redirectPort is the
+// destination port for the plain HTTP → HTTPS redirect: the dedicated admin
+// HTTPS listen address port when configured, and [tlsConfigSettings.PortHTTPS]
+// otherwise.
+func readHTTPSRedirectConfig() (forceHTTPS, serveHTTP3 bool, portHTTPS, redirectPort uint16) {
+	config.RLock()
+	defer config.RUnlock()
+
+	serveHTTP3 = config.DNS.ServeHTTP3
+	portHTTPS = config.TLS.PortHTTPS
+	redirectPort = portHTTPS
+	tlsEnabled := config.TLS.Enabled
+
+	if a := adminListenAddr(&config.TLS); a != (netip.AddrPort{}) {
+		redirectPort = a.Port()
+		forceHTTPS = config.TLS.ForceHTTPS && tlsEnabled
+
+		return forceHTTPS, serveHTTP3, portHTTPS, redirectPort
+	}
+
+	forceHTTPS = config.TLS.ForceHTTPS && tlsEnabled && portHTTPS != 0
+
+	return forceHTTPS, serveHTTP3, portHTTPS, redirectPort
+}
+
 // httpsURL returns a copy of u for redirection to the HTTPS version, taking the
 // hostname and the HTTPS port into account.
 func httpsURL(u *url.URL, host string, portHTTPS uint16) (redirectURL *url.URL) {