github/Bose-SoundTouch
1
0
Fork 0
mirror of https://github.com/gesellix/Bose-SoundTouch.git synced 2026-08-18 08:36:13 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(health): detect advertised-HTTPS-URL / listener port mismatch (#355)

Browse Source 
Follow-up to the previous commit. The cert-chain check dialed the
advertised HTTPS URL, whose port defaults to 443 when the URL omits it
(splitHTTPSHostPort). The advertised URL comes from
--https-server-url / HTTPS_SERVER_URL / the settings file and is not
editable in the web UI, so when it lost its port it silently pointed the
check (and speakers) at 443 while the real listener was on 8443 — the
exact "port 443" complaint in issue #355.

Thread the actual HTTPS listener port into the check (new
Server.SetHTTPSListenAddr, wired from config.httpsAddr). When the dial
fails and the advertised port differs from the listener port, emit a
mismatch-specific warning that names both ports and offers the corrected
HTTPS_SERVER_URL, while still deferring to reverse-proxy setups. A
reachable endpoint never reaches this branch.

Reproduced locally on a clean data dir: seeding a port-less
https_server_url with the listener on 8443 previously errored on
:443; it now warns with both ports and the fix. Regression tests added.

refs #355

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Tobias Gesellchen
2026-07-04 17:45:41 +02:00
co-authored by Claude Opus 4.8
parent 9d69b61779
commit 433a779998
4 changed files with 112 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/service/handlers/server.go
+31
View File
@@ -41,6 +41,7 @@ type Server struct {
mu sync.RWMutex
serverURL string
httpsServerURL string
httpsListenAddr string
discovering bool
redactLogs bool
logBodies bool
@@ -149,6 +150,7 @@ func NewServer(ds *datastore.DataStore, sm *setup.Manager, serverURL string, red
_, httpsURL := s.GetSettings()
return httpsURL
},
s.actualHTTPSPort,
s.loadOwnCACert,
)
health.RegisterCACertExpiryCheck(s.healthRegistry, s.loadOwnCACert, s.ownCACertPath)
@@ -801,6 +803,35 @@ func (s *Server) SetHTTPServerURL(url string) {
s.httpsServerURL = url
}
// SetHTTPSListenAddr records the address the HTTPS listener is bound
// to (e.g. ":8443"). The cert-chain health check uses its port to
// detect an advertised-URL/listener port mismatch (issue #355).
func (s *Server) SetHTTPSListenAddr(addr string) {
s.mu.Lock()
defer s.mu.Unlock()
s.httpsListenAddr = addr
}
// actualHTTPSPort returns the port the HTTPS listener is bound to, or
// "" if unknown/unparseable.
func (s *Server) actualHTTPSPort() string {
s.mu.RLock()
addr := s.httpsListenAddr
s.mu.RUnlock()
if addr == "" {
return ""
}
_, port, err := net.SplitHostPort(addr)
if err != nil {
return ""
}
return port
}
// SetRecorder sets the recorder for the server.
func (s *Server) SetRecorder(r *proxy.Recorder) {
s.mu.Lock()
pkg/service/health/checks_cert_chain.go
+53 -13
View File
@@ -29,27 +29,40 @@ const CheckIDCertChain = "service_cert_chain"
// something else → warning with an `openssl s_client`
// investigation prompt (foreign chain / reverse proxy /
// ingress cert).
// - endpoint not reachable from inside the service (no cert
// presented) → warning, not error: the advertised HTTPS URL
// is often intentionally unreachable from the service itself
// (reverse proxy, Docker-published port, LAN-only hostname).
// - endpoint not reachable AND the advertised URL's port differs
// from the actual listener port → warning naming both ports
// and the fix (issue #355: the advertised URL, invisible in
// the web UI, defaulted to 443 while the listener was on 8443).
// - endpoint not reachable with no port mismatch → warning, not
// error: the advertised HTTPS URL is often intentionally
// unreachable from the service itself (reverse proxy,
// Docker-published port, LAN-only hostname).
// - HTTPS URL not configured → skip silently.
//
// caCertFn returns AfterTouch's own CA leaf certificate (nil if
// unavailable). It's called per check run; the handler-side
// implementation caches the parse via sync.Once so we don't
// re-read the PEM on every poll.
func RegisterCertChainCheck(r *Registry, httpsURLFn func() string, caCertFn func() *x509.Certificate) {
// actualHTTPSPortFn returns the port the HTTPS listener is actually
// bound to (e.g. "8443"), or "" if unknown. It lets the check
// distinguish a genuine "advertised URL points at the wrong port"
// misconfiguration from an intentionally-unreachable advertised URL.
func RegisterCertChainCheck(r *Registry, httpsURLFn, actualHTTPSPortFn func() string, caCertFn func() *x509.Certificate) {
r.Register(Check{
ID: CheckIDCertChain,
Title: "HTTPS endpoint TLS configuration",
Run: func() []Finding {
return runCertChainCheck(httpsURLFn(), caCertFn)
actualPort := ""
if actualHTTPSPortFn != nil {
actualPort = actualHTTPSPortFn()
}
return runCertChainCheck(httpsURLFn(), actualPort, caCertFn)
},
})
}
func runCertChainCheck(httpsURL string, caCertFn func() *x509.Certificate) []Finding {
func runCertChainCheck(httpsURL, actualHTTPSPort string, caCertFn func() *x509.Certificate) []Finding {
if strings.TrimSpace(httpsURL) == "" {
return nil
}
@@ -87,12 +100,39 @@ func runCertChainCheck(httpsURL string, caCertFn func() *x509.Certificate) []Fin
leaf := leafFromVerifyError(err)
if leaf == nil {
// The dial failed before any certificate was presented
// (connection refused, timeout, handshake reset). From
// inside the service we can't tell "the endpoint is down"
// apart from "the advertised HTTPS URL simply isn't
// reachable from here" — the latter is a normal, healthy
// setup (TLS terminated by a reverse proxy, or a
// Docker-published port / external hostname that only
// (connection refused, timeout, handshake reset).
//
// Special-case the misconfiguration behind issue #355: the
// HTTPS URL the service advertises (and points speakers at)
// carries a different port than the one the listener is
// actually bound to. This is easy to hit because the URL
// comes from --https-server-url / HTTPS_SERVER_URL / the
// settings file and, when it omits the port, defaults to
// 443 here — while the listener stays on --https-port
// (default 8443). The URL isn't editable in the web UI, so
// the mismatch is otherwise invisible. Call it out with the
// fix. (A reachable endpoint never gets here — a working
// reverse proxy on the advertised port dials fine above.)
if actualHTTPSPort != "" && port != actualHTTPSPort {
return []Finding{{
Severity: SeverityWarning,
Message: fmt.Sprintf("Configured HTTPS URL %s uses port %s, but the service is listening on port %s and nothing answered on port %s.", httpsURL, port, actualHTTPSPort, port),
Details: fmt.Sprintf("Speakers are pointed at the configured HTTPS URL, so if its port doesn't match the listener they connect to the wrong place. This URL is set via --https-server-url / HTTPS_SERVER_URL (or the persisted settings file) and isn't editable in the web UI, which makes the mismatch easy to miss. Set it to include the listener's port, e.g. https://%s:%s. If a reverse proxy intentionally forwards port %s to %s, this is expected and can be ignored. Underlying error: %v.",
host, actualHTTPSPort, port, actualHTTPSPort, err),
ManualCommands: []ManualCommand{{
Label: "Point the HTTPS URL at the listener's port (restart required):",
Command: fmt.Sprintf("HTTPS_SERVER_URL=https://%s:%s", host, actualHTTPSPort),
Hint: "Or pass --https-server-url. Skip this if a reverse proxy already maps the ports.",
}},
}}
}
// Otherwise: a reachability failure with no port mismatch to
// point at. From inside the service we can't tell "the
// endpoint is down" apart from "the advertised HTTPS URL
// simply isn't reachable from here" — the latter is a
// normal, healthy setup (TLS terminated by a reverse proxy,
// or a Docker-published port / external hostname that only
// resolves on the LAN). Reporting a hard error there is a
// false alarm (issue #355), so this is a warning with the
// context to tell the two apart.
pkg/service/health/checks_cert_chain_test.go
+27 -6
View File
@@ -16,19 +16,40 @@ import (
)
func TestCertChain_EmptyURLSkips(t *testing.T) {
got := runCertChainCheck("", nil)
got := runCertChainCheck("", "", nil)
if len(got) != 0 {
t.Errorf("expected no findings for empty URL, got %+v", got)
}
}
func TestCertChain_UnparseableURLWarns(t *testing.T) {
got := runCertChainCheck("://nope", nil)
got := runCertChainCheck("://nope", "", nil)
if len(got) != 1 || got[0].Severity != SeverityWarning {
t.Fatalf("expected one warning, got %+v", got)
}
}
func TestCertChain_PortMismatch_NamesBothPortsAndFix(t *testing.T) {
// Regression for issue #355: the advertised HTTPS URL carries a
// different port (here the 443 default, from a port-less URL)
// than the actual listener (8443). When that endpoint is also
// unreachable, the warning must name both ports and offer the
// corrected HTTPS_SERVER_URL, since the URL isn't editable in
// the web UI. 127.0.0.1:443 is unreachable in the test sandbox.
got := runCertChainCheck("https://127.0.0.1/", "8443", nil)
if len(got) != 1 || got[0].Severity != SeverityWarning {
t.Fatalf("expected one warning for port mismatch, got %+v", got)
}
if !strings.Contains(got[0].Message, "443") || !strings.Contains(got[0].Message, "8443") {
t.Errorf("expected message to name both the advertised (443) and listener (8443) ports, got %q", got[0].Message)
}
if len(got[0].ManualCommands) == 0 || !strings.Contains(got[0].ManualCommands[0].Command, "https://127.0.0.1:8443") {
t.Errorf("expected a corrected HTTPS_SERVER_URL suggestion, got %+v", got[0].ManualCommands)
}
}
func TestCertChain_UnreachableEndpoint_IsWarningNotError(t *testing.T) {
// Regression for issue #355: the service dialing its own
// configured HTTPS URL and finding it unreachable is NOT a hard
@@ -39,7 +60,7 @@ func TestCertChain_UnreachableEndpoint_IsWarningNotError(t *testing.T) {
// explains the expected case and offers a client-side check.
//
// 127.0.0.1:1 refuses; using https:// to force the TLS path.
got := runCertChainCheck("https://127.0.0.1:1/", nil)
got := runCertChainCheck("https://127.0.0.1:1/", "", nil)
if len(got) != 1 || got[0].Severity != SeverityWarning {
t.Fatalf("expected one warning for unreachable endpoint, got %+v", got)
}
@@ -60,7 +81,7 @@ func TestCertChain_SelfSigned_SubjectEqualsIssuerFallback(t *testing.T) {
// No CA provided → fallback to Subject==Issuer heuristic.
// This is informational, not a warning — a self-signed
// AfterTouch chain is the expected default deployment shape.
got := runCertChainCheck(srv.URL, nil)
got := runCertChainCheck(srv.URL, "", nil)
if len(got) != 1 || got[0].Severity != SeverityInfo {
t.Fatalf("expected one info finding for self-signed cert, got %+v", got)
}
@@ -111,7 +132,7 @@ func TestCertChain_LeafSignedByOwnCA_IsInformationalNotAWarning(t *testing.T) {
srv.StartTLS()
defer srv.Close()
got := runCertChainCheck(srv.URL, func() *x509.Certificate { return ca })
got := runCertChainCheck(srv.URL, "", func() *x509.Certificate { return ca })
if len(got) != 1 {
t.Fatalf("expected one finding, got %+v", got)
}
@@ -160,7 +181,7 @@ func TestCertChain_ForeignChain_SuggestsOpenSSL(t *testing.T) {
// Different CA — pretend it's "our" AfterTouch CA.
_, ourCA := generateInternalCA(t)
got := runCertChainCheck(srv.URL, func() *x509.Certificate { return ourCA })
got := runCertChainCheck(srv.URL, "", func() *x509.Certificate { return ourCA })
if len(got) != 1 || got[0].Severity != SeverityWarning {
t.Fatalf("expected one warning, got %+v", got)
}