From c4f2a10790d1a8b88506f9ed02908c3ba2fab010 Mon Sep 17 00:00:00 2001 From: mrambossek Date: Sun, 2 Aug 2026 08:14:42 +0200 Subject: [PATCH] app: show what the server reports as facts, not as inputs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The settings card offered three editable boxes and said nothing about the server itself — which addresses a test will actually use, on which ports, what it can measure. That is the part a person checks before trusting a result, and "which address did this come from" is precisely the question a report leaves open. The server now publishes it. The profile's targets carried one IPv4 and a TODO; it reports both families and both alternates, derived from the UDP listen spec rather than configured separately, so the list cannot drift from what is actually bound. No reservation means no alternate is claimed: announcing a second address as the RFC 5780 alternate when none was set aside would promise a redirect the server will not send. The app renders them read-only, in a panel visibly distinct from the fields above. An editable box that changes nothing is worse than no box, and these are facts to read rather than settings to apply. Co-Authored-By: Claude Opus 5 --- .../main/kotlin/app/echo_lot/app/RunStore.kt | 30 ++++++++++ .../main/kotlin/app/echo_lot/app/Settings.kt | 12 ++++ .../kotlin/app/echo_lot/app/SettingsScreen.kt | 28 +++++++++- .../kotlin/app/echo_lot/protocol/Model.kt | 9 +++ server/cmd/echolot-server/main.go | 2 + server/internal/config/addrs_test.go | 56 +++++++++++++++++++ server/internal/config/config.go | 38 +++++++++++++ server/internal/control/control.go | 40 ++++++++++--- 8 files changed, 207 insertions(+), 8 deletions(-) create mode 100644 server/internal/config/addrs_test.go diff --git a/echolot-app/app/src/main/kotlin/app/echo_lot/app/RunStore.kt b/echolot-app/app/src/main/kotlin/app/echo_lot/app/RunStore.kt index 75d16a4..cf0a733 100644 --- a/echolot-app/app/src/main/kotlin/app/echo_lot/app/RunStore.kt +++ b/echolot-app/app/src/main/kotlin/app/echo_lot/app/RunStore.kt @@ -92,6 +92,7 @@ class RunStore(context: Context, private val settings: Settings) { val profile = client().profile(settings.serverCredential) // Learned here so the next run's canary probe knows what to ask for. profile.canaryZone.takeIf { it.isNotBlank() }?.let { settings.canaryZone = it } + settings.serverFacts = describeFacts(profile) val compat = Compat.check(profile, BuildConfig.APP_SEMVER) val head = "${profile.name} · server ${profile.serverVersion} · " + "protocol ${profile.compat.protocolVersion.ifBlank { "unstated" }}" @@ -117,6 +118,32 @@ class RunStore(context: Context, private val settings: Settings) { * no credential — fails later, somewhere else, with an error that points at the wrong thing. * Blocking; callers run it off the main thread. */ + + /** + * Renders what the server says about itself, for display. + * + * Only what a person measuring against it would want to check: which addresses the tests will + * actually use, on which ports, and what the server admits it can do. Addresses first, because + * "which address did this result come from" is the question a report leaves open. + */ + private fun describeFacts(p: app.echo_lot.protocol.Profile): String { + val lines = ArrayList() + lines += "${p.name} · server ${p.serverVersion}" + for (t in p.targets) { + t.ip4?.let { lines += "IPv4 $it" } + t.ip4Alt?.let { lines += "IPv4 $it (alternate, for NAT behaviour tests)" } + t.ip6?.let { lines += "IPv6 $it" } + t.ip6Alt?.let { lines += "IPv6 $it (alternate, for NAT behaviour tests)" } + lines += "ports udp ${t.udpPort} · tcp ${t.tcpPort} · stun ${t.stunPort}" + } + if (p.canaryZone.isNotBlank()) lines += "DNS canary zone ${p.canaryZone}" + if (p.capabilities.isNotEmpty()) { + lines += "can measure ${p.capabilities.joinToString(", ")}" + } + val b = StringBuilder(lines.joinToString("\n")) + return b.toString().trimEnd() + } + fun enroll(link: String, deviceName: String?): String { val parsed = app.echo_lot.protocol.EnrollmentLink.parse(link) ?: return "That does not look like an Echolot enrollment link. It should start with " + @@ -124,6 +151,8 @@ class RunStore(context: Context, private val settings: Settings) { return try { val enrolled = parsed.redeem(deviceName, BuildConfig.APP_SEMVER) val compat = Compat.check(enrolled.profile, BuildConfig.APP_SEMVER) + settings.serverFacts = describeFacts(enrolled.profile) + enrolled.profile.canaryZone.takeIf { it.isNotBlank() }?.let { settings.canaryZone = it } settings.serverUrl = enrolled.controlUrl settings.serverPublicUrl = enrolled.publicUrl settings.serverPin = enrolled.pin @@ -154,6 +183,7 @@ class RunStore(context: Context, private val settings: Settings) { val client = client() val profile = client.profile(settings.serverCredential) profile.canaryZone.takeIf { it.isNotBlank() }?.let { settings.canaryZone = it } + settings.serverFacts = describeFacts(profile) // Compatibility before policy: an incompatible server may well advertise an upload // policy it would never actually apply to us. diff --git a/echolot-app/app/src/main/kotlin/app/echo_lot/app/Settings.kt b/echolot-app/app/src/main/kotlin/app/echo_lot/app/Settings.kt index 4d5ec8e..b1f7e9d 100644 --- a/echolot-app/app/src/main/kotlin/app/echo_lot/app/Settings.kt +++ b/echolot-app/app/src/main/kotlin/app/echo_lot/app/Settings.kt @@ -108,6 +108,17 @@ class Settings(context: Context) { get() = (prefs.getString(SERVER_PUBLIC_URL, "") ?: "").ifBlank { serverUrl } set(v) = prefs.edit().putString(SERVER_PUBLIC_URL, v.trim()).apply() + /** + * What the server said about itself, last time it was asked: addresses, ports, capabilities. + * + * Cached as a rendered block rather than as fields, because it is shown and never acted on — + * these are facts to read, not settings to apply, and storing them as settings would invite + * exactly the confusion of an editable box that changes nothing. + */ + var serverFacts: String + get() = prefs.getString(SERVER_FACTS, "") ?: "" + set(v) = prefs.edit().putString(SERVER_FACTS, v).apply() + var serverCredential: String get() = prefs.getString(SERVER_CRED, "") ?: "" set(v) = prefs.edit().putString(SERVER_CRED, v.trim()).apply() @@ -184,6 +195,7 @@ class Settings(context: Context) { const val SERVER_PIN = "server_pin" const val SERVER_CRED = "server_credential" const val SERVER_PUBLIC_URL = "server_public_url" + const val SERVER_FACTS = "server_facts" const val CANARY_ZONE = "server_canary_zone" const val PENDING_VERIFIER = "pending_auth_verifier" const val PENDING_STATE = "pending_auth_state" diff --git a/echolot-app/app/src/main/kotlin/app/echo_lot/app/SettingsScreen.kt b/echolot-app/app/src/main/kotlin/app/echo_lot/app/SettingsScreen.kt index b87c77e..58485c8 100644 --- a/echolot-app/app/src/main/kotlin/app/echo_lot/app/SettingsScreen.kt +++ b/echolot-app/app/src/main/kotlin/app/echo_lot/app/SettingsScreen.kt @@ -11,6 +11,7 @@ import androidx.compose.foundation.layout.Spacer import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.height import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.shape.RoundedCornerShape import androidx.compose.foundation.rememberScrollState import androidx.compose.foundation.verticalScroll import androidx.compose.material3.Button @@ -19,6 +20,7 @@ import androidx.compose.material3.FilterChip import androidx.compose.material3.LocalContentColor import androidx.compose.material3.MaterialTheme import androidx.compose.material3.OutlinedTextField +import androidx.compose.material3.Surface import androidx.compose.material3.Switch import androidx.compose.material3.Text import androidx.compose.material3.TextButton @@ -76,7 +78,9 @@ fun SettingsScreen( // Enrolling is asynchronous, so these are re-read when its result lands rather than when the // button is pressed — reading them immediately showed the previous server's values and looked // exactly like an enrollment that had silently done nothing. - androidx.compose.runtime.LaunchedEffect(enrollStatus) { + var serverFacts by remember { mutableStateOf(settings.serverFacts) } + androidx.compose.runtime.LaunchedEffect(enrollStatus, serverStatus) { + serverFacts = settings.serverFacts serverUrl = settings.serverPublicUrl serverPin = settings.serverPin serverCred = settings.serverCredential @@ -251,6 +255,28 @@ fun SettingsScreen( }, label = { Text("Server URL") }, singleLine = true, modifier = Modifier.fillMaxWidth(), ) + // What the server reports about itself. Read-only on purpose: these are facts to + // check, not settings to apply, and an editable box that changes nothing is worse + // than no box at all. + if (serverFacts.isNotBlank()) { + Surface( + color = MaterialTheme.colorScheme.surfaceVariant, + shape = RoundedCornerShape(6.dp), + modifier = Modifier.fillMaxWidth(), + ) { + Column(Modifier.padding(12.dp), verticalArrangement = Arrangement.spacedBy(4.dp)) { + Text( + "What this server reports", + style = MaterialTheme.typography.labelLarge, + ) + Text( + serverFacts, + style = MaterialTheme.typography.bodySmall, + fontFamily = FontFamily.Monospace, + ) + } + } + } if (settings.serverUrl.isNotBlank() && settings.serverUrl != settings.serverPublicUrl) { Text( "Connects to ${settings.serverUrl} — this server publishes one name and " + diff --git a/echolot-app/core-protocol/src/main/kotlin/app/echo_lot/protocol/Model.kt b/echolot-app/core-protocol/src/main/kotlin/app/echo_lot/protocol/Model.kt index 515476a..f26d123 100644 --- a/echolot-app/core-protocol/src/main/kotlin/app/echo_lot/protocol/Model.kt +++ b/echolot-app/core-protocol/src/main/kotlin/app/echo_lot/protocol/Model.kt @@ -29,6 +29,15 @@ data class Target( val id: String, val ip4: String? = null, val ip6: String? = null, + /** + * The second address, which RFC 5780 behaviour discovery redirects to. + * + * Worth surfacing rather than treating as an implementation detail: a report that says "the + * server did not answer" means something different depending on which of its addresses was + * asked, and an operator reading one needs to be able to tell. + */ + @SerialName("ip4_alt") val ip4Alt: String? = null, + @SerialName("ip6_alt") val ip6Alt: String? = null, @SerialName("udp_port") val udpPort: Int = 0, @SerialName("tcp_port") val tcpPort: Int = 0, @SerialName("stun_port") val stunPort: Int = 0, diff --git a/server/cmd/echolot-server/main.go b/server/cmd/echolot-server/main.go index 4e13307..727bba2 100644 --- a/server/cmd/echolot-server/main.go +++ b/server/cmd/echolot-server/main.go @@ -260,7 +260,9 @@ func serve(cfg *config.Config) error { "every upload will be refused") } + ip4, ip6, ip4Alt, ip6Alt := cfg.MeasurementAddrs() ctl := &control.Server{ + IP4: ip4, IP6: ip6, IP4Alt: ip4Alt, IP6Alt: ip6Alt, Store: st, Sessions: sessions, Name: cfg.Name, UDPPort: mustPort(firstAddr(cfg.UDPListen)), TCPPort: mustPort(firstAddr(cfg.TCPListen)), StunPort: mustPort(firstAddr(cfg.StunListen)), PinB64: pin, CertChain: cert.Certificate, diff --git a/server/internal/config/addrs_test.go b/server/internal/config/addrs_test.go new file mode 100644 index 0000000..8a3a358 --- /dev/null +++ b/server/internal/config/addrs_test.go @@ -0,0 +1,56 @@ +// SPDX-FileCopyrightText: 2026 Echolot contributors +// SPDX-License-Identifier: GPL-3.0-or-later + +package config + +import "testing" + +// The fixture is fmr's real UDP listen spec, because the point of deriving these from the bound +// listeners is that they cannot disagree with what the server actually answers on. +const fmrUDP = "89.185.109.150:8442,89.185.109.151:8442," + + "[2001:1ad0:c4fe:6767::150]:8442,[2001:1ad0:c4fe:6767::151]:8442" + +func TestMeasurementAddrsSplitsPrimaryFromReserved(t *testing.T) { + c := &Config{ + UDPListen: fmrUDP, + ReservedAddrs: "89.185.109.151,2001:1ad0:c4fe:6767::151", + } + ip4, ip6, ip4Alt, ip6Alt := c.MeasurementAddrs() + for _, tc := range []struct{ got, want, name string }{ + {ip4, "89.185.109.150", "ip4"}, + {ip6, "2001:1ad0:c4fe:6767::150", "ip6"}, + {ip4Alt, "89.185.109.151", "ip4_alt"}, + {ip6Alt, "2001:1ad0:c4fe:6767::151", "ip6_alt"}, + } { + if tc.got != tc.want { + t.Errorf("%s = %q, want %q", tc.name, tc.got, tc.want) + } + } +} + +func TestMeasurementAddrsWithNothingReserved(t *testing.T) { + // No reservation means no alternate: reporting a second address as the RFC 5780 alternate + // when it was never set aside for that would tell a client to expect a redirect that the + // server has no intention of sending. + c := &Config{UDPListen: fmrUDP} + ip4, ip6, ip4Alt, ip6Alt := c.MeasurementAddrs() + if ip4 == "" || ip6 == "" { + t.Fatalf("primaries should still be found: ip4=%q ip6=%q", ip4, ip6) + } + if ip4Alt != "" || ip6Alt != "" { + t.Errorf("no address is reserved, so there is no alternate; got %q / %q", ip4Alt, ip6Alt) + } +} + +func TestMeasurementAddrsIgnoresWhatItCannotRead(t *testing.T) { + // A wildcard bind names no address, and a hostname is not resolved here. Either would be a + // guess presented to clients as fact. + c := &Config{UDPListen: ":8442,probe.example.net:8442,89.185.109.150:8442"} + ip4, ip6, _, _ := c.MeasurementAddrs() + if ip4 != "89.185.109.150" { + t.Errorf("ip4 = %q, want the one address that was actually spelled out", ip4) + } + if ip6 != "" { + t.Errorf("ip6 = %q, want empty — none was configured", ip6) + } +} diff --git a/server/internal/config/config.go b/server/internal/config/config.go index 89789d9..4c4d3dd 100644 --- a/server/internal/config/config.go +++ b/server/internal/config/config.go @@ -322,6 +322,44 @@ func (c *Config) Listeners() []Listener { } } +// MeasurementAddrs picks out the addresses this server can be measured on, by family, splitting +// primaries from the reserved alternates. +// +// Derived from what is actually bound rather than configured separately: a second list of the +// server's own addresses is a second thing to keep in step, and the copy that drifts is the one +// clients are told about. +func (c *Config) MeasurementAddrs() (ip4, ip6, ip4Alt, ip6Alt string) { + reserved := map[string]bool{} + for _, ip := range c.ReservedIPs() { + reserved[ip.String()] = true + } + // The UDP data plane binds every address a client may be pointed at, which makes it the + // honest source for this. + for _, a := range Addrs(c.UDPListen) { + host, _, err := net.SplitHostPort(a) + if err != nil { + continue + } + host = strings.Trim(host, "[]") + ip := net.ParseIP(host) + if ip == nil { + continue + } + alt := reserved[ip.String()] + switch { + case ip.To4() != nil && alt && ip4Alt == "": + ip4Alt = host + case ip.To4() != nil && !alt && ip4 == "": + ip4 = host + case ip.To4() == nil && alt && ip6Alt == "": + ip6Alt = host + case ip.To4() == nil && !alt && ip6 == "": + ip6 = host + } + } + return +} + // Addrs splits a comma-separated listen spec into individual addresses. // Explicit per-address binds matter on multi-IP hosts: a wildcard bind // (":8443") would also claim addresses reserved for other purposes (e.g. an diff --git a/server/internal/control/control.go b/server/internal/control/control.go index b84dd9a..76079a5 100644 --- a/server/internal/control/control.go +++ b/server/internal/control/control.go @@ -80,6 +80,10 @@ type Server struct { CanaryQueries func(sessionPrefix string) any // CanaryZone is surfaced in the profile so the app knows what to query. CanaryZone string + // The addresses this server can be measured on. The "_alt" pair is the second address + // RFC 5780 behaviour discovery redirects to, and the one reserved from services so that + // nothing answering there is itself a measurement. + IP4, IP6, IP4Alt, IP6Alt string // ProvenGood reports the server's self-test signal (may be nil). Surfaced // in the profile so a client can trust — or skip — MTU tests: if the // server's own egress isn't full-MTU, client MTU results measure the @@ -614,13 +618,7 @@ func (s *Server) profile(w http.ResponseWriter, r *http.Request) { // modified builds and gives clients provenance for the measurement. "source_url": "", // TODO: stamp from build metadata "capabilities": s.Capabilities, - "targets": []map[string]any{{ - "id": s.Name, - "ip4": host, // TODO: explicit configured addresses, v6, second STUN addr - "udp_port": s.UDPPort, - "tcp_port": s.TCPPort, - "stun_port": s.StunPort, - }}, + "targets": []map[string]any{s.target(host)}, "pins": []string{"pin-sha256:" + s.PinB64}, "next_pins": []string{}, "canary_zone": s.CanaryZone, @@ -838,6 +836,34 @@ func EnrollmentURI(publicURL, pinB64, token string) string { "&t=" + url.QueryEscape(token) } +// target describes where this server can be measured, so a client can say which address a result +// came from instead of "the server". +// +// The alternates matter as much as the primaries: RFC 5780 behaviour discovery needs a second +// address to redirect to, and an operator reading a report needs to know which of their addresses +// a finding refers to. [fallback] is used only when nothing was configured explicitly, so a server +// that has not been told its own addresses still answers with something usable. +func (s *Server) target(fallback string) map[string]any { + t := map[string]any{ + "id": s.Name, + "udp_port": s.UDPPort, + "tcp_port": s.TCPPort, + "stun_port": s.StunPort, + } + ip4 := s.IP4 + if ip4 == "" { + ip4 = fallback + } + for k, v := range map[string]string{ + "ip4": ip4, "ip6": s.IP6, "ip4_alt": s.IP4Alt, "ip6_alt": s.IP6Alt, + } { + if v != "" { + t[k] = v + } + } + return t +} + // upstreamJSON renders the upstream tally with the derived figures already computed, so every // consumer does not have to repeat (and risk fumbling) the same arithmetic. func upstreamJSON(sess *session.Session) map[string]any {