home: add tls.admin_listen_addr for separate https admin listener#8356
Open
Raviu56 wants to merge 1 commit intoAdguardTeam:masterfrom
Open
home: add tls.admin_listen_addr for separate https admin listener#8356Raviu56 wants to merge 1 commit intoAdguardTeam:masterfrom
Raviu56 wants to merge 1 commit intoAdguardTeam:masterfrom
Conversation
Add an optional tls.admin_listen_addr (netip.AddrPort) that, when set, runs a dedicated HTTPS server for the admin UI/API on that address, while the HTTPS server on tls.port_https serves DoH only. Both servers share the same TLS certificate. When unset the previous single-server behavior is preserved, so existing installs are unaffected and DNS clients using DoH see no change. Port conflicts involving admin_listen_addr are validated at config parse time (including --check-config), not only as a runtime bind failure. HTTP/3 on tls.port_https mirrors the HTTPS mux split so DoH-over-HTTP/3 clients are not forced through the admin auth middleware and admin routes are not exposed on the DoH port. validateTLSSettings reads AdminListenAddr from the server-side config snapshot, not the frontend payload (the field is json:"-"), so UI port_https changes still trigger the admin-port conflict check. checkPortAvailability validates the admin HTTPS port on its own IP (which may differ from the web API bind host). tlsConfigChanged uses a fresh shutdown context for each HTTPS server so the admin server is not handed an already-cancelled context. registerDoHHandlers reads the admin listen address under config.RLock. Backend + YAML only; no UI changes in this commit. See the CHANGELOG entry for user-facing details. Addresses AdguardTeam#7424 and AdguardTeam#7598. Co-Authored-By: raviu <raviu@protonmail.com>
|
This PR might fix #7773 but 7773 is still a regression. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Addresses #7424 and #7598 by adding a single optional
tls.admin_listen_addrYAML setting (anetip.AddrPort, e.g.127.0.0.1:4443) that decouples the HTTPS admin UI/API from DoH while sharing the same TLS certificate.tls.port_httpsserves both admin UI and DoH, exactly as today. Existing installs are unaffected and DNS clients using DoH see no change.tls.port_httpsserves DoH only (/dns-query, etc.). Admin UI + API are not reachable here.tls.admin_listen_addrserves admin UI + API only. DoH routes are not registered on this listener.This covers both open feature requests without risking DoH clients:
admin_listen_addr: 127.0.0.1:4443→ DoH stays public on 443, admin HTTPS is LAN/VPN-only.admin_listen_addr: 0.0.0.0:443andport_https: 0→ admin HTTPS only, no DoH over HTTPS.HTTPSListenAddrs(DDR SVCB advertisement) and mobileconfig / DNS-encryption display intentionally stay tied toport_https, so DNS clients see no change. HTTP/3 onport_httpsmirrors the HTTPS mux split, so DoH-over-HTTP/3 is not blocked by the admin auth middleware and admin routes are not exposed there.Port conflicts involving
admin_listen_addrare validated at config parse time (including--check-config), not just as a runtime bind failure.checkPortAvailabilityvalidates the admin port on the admin listener's own IP (which may differ from the web API bind host).tlsConfigChangeduses a fresh shutdown context per server so the admin server is not handed an already-cancelled context.Scope: backend + YAML only. No frontend / UI changes.
AdminListenAddris intentionally not exposed in the JSON API (json:"-"); the field is preserved across frontend TLS-settings saves viasetPrivateFieldsAndCompare. Theinternal/nextexperimental code path is not touched. Default plain-HTTP admin behavior (http.address, default:3000) is unchanged; only theforce_httpsredirect target is updated to point at the admin port when the feature is enabled.Example config:
Changes
tlsConfigSettings.AdminListenAddr netip.AddrPort(YAML:admin_listen_addr,json:"-").adminMux(web UI + API + install + control handlers) anddohMux(DoH routes) when the feature is enabled; single shared mux when not.http.Server(adminHTTPSServer) with its own lifecycle, started whenadmin_listen_addris valid and non-zero; shares the cert with the DoH HTTPS server.mustStartHTTP3) mirrorsserveTLSmux selection + auth decision.validateConfig,checkPorts,validatePorts, andcheckPortAvailabilityall cover the admin port (and the admin port uses its own IP in binding availability checks).validateTLSSettingsreadsAdminListenAddrfrom the server-sidetlsConfsnapshot (not the frontend payload) so UIport_httpschanges still trigger the admin-port conflict check.force_httpsplain-HTTP redirect target switches to the admin port when configured.internal/home/adminlistenaddr_internal_test.gocovering the helper, port-conflict detection, config preservation across frontend updates, and mux-split routing.init, lowercase log/error messages, godoc[bracket]links, explicit zero values on error returns).Test results
Three scenarios, 14 assertions, all pass against a binary built from this branch with a shared self-signed cert.
Scenario A — feature off (
admin_listen_addr: "")GET https://:8443/(admin UI)GET https://:8443/dns-query?…(DoH)GET https://:4444/(admin port not bound)Scenario B — feature on (
admin_listen_addr: 127.0.0.1:4444,port_https: 8443,force_https: true)GET https://127.0.0.1:4444/(admin UI)GET https://127.0.0.1:4444/dns-query?…(DoH not served here)GET https://:8443/control/status(admin API not served here)GET https://:8443/dns-query?…(DoH still works)GET http://:3000/login.htmlLocation:headerhttps://…:4444/login.htmlhttps://127.0.0.1:4444/login.htmlScenario C — port conflict (
admin_listen_addr: 127.0.0.1:8443,port_https: 8443)AdGuardHome --check-configvalidating tcp ports: duplicated values: [8443]go test ./...,go vet ./..., andbash scripts/make/go-lint.sh(gofumpt, govulncheck, gocyclo, gocognit, ineffassign, unparam, misspell, gosec, errcheck, staticcheck) are green.Closes #7424
Closes #7598