app: don't report ICMPv6 silence that was never measured
The per-network attribution fix worked — the finding named rmnet_data1 instead of the IPv4-only wifi — and immediately exposed a worse problem underneath. Cellular's result was `ok: false` because binding a socket to it failed with EPERM, so no echo request was ever sent; the finding then reported "IPv6 is configured, but ICMPv6 gets no reply" about a network the app had never pinged. That is an assertion about the user's carrier with nothing behind it. `attempted` now travels beside `ok`, set only once sendto has returned, and the finding requires both. Failing to bind is a fact about this app's permissions on this device; it says nothing about the network, and the two must not share a boolean. Verified on hardware with a VPN active: every network fails to bind with EPERM, nothing is sent, and no ICMPv6 finding is emitted — where the previous build would have blamed the carrier. Recorded in build-status: Android blocks per-network binding entirely while a VPN holds the default route, so per-network measurement is unavailable to anyone with one connected. That needs a deliberate answer rather than a silently green run. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
mrambossek
co-authored by
Claude Opus 5
Claude Opus 5
parent
7a5004f293
commit
d9ee8bc2ae
@@ -40,19 +40,20 @@ class IcmpProbe(
|
||||
|
||||
override suspend fun run(ctx: Context, ids: ProbeIds): Test = withContext(Dispatchers.IO) {
|
||||
val b = TestBuilder(type, tier, ids)
|
||||
val perNetwork = LinkedHashMap<String, Triple<String?, Boolean, String>>()
|
||||
val perNetwork = LinkedHashMap<String, Pair<String?, Attempt>>()
|
||||
var anyOk = false
|
||||
val rtts = ArrayList<Double>()
|
||||
|
||||
// Default network first, then each active network explicitly.
|
||||
attempt(null).let { (ok, detail, rtt) ->
|
||||
perNetwork["default"] = Triple(null, ok, detail); if (ok) { anyOk = true; rtt?.let(rtts::add) }
|
||||
attempt(null).let { a ->
|
||||
perNetwork["default"] = null to a
|
||||
if (a.ok) { anyOk = true; a.rttMs?.let(rtts::add) }
|
||||
}
|
||||
for (e in entries) {
|
||||
val label = "${e.model.transport.name.lowercase()}:${e.model.id}"
|
||||
val (ok, detail, rtt) = attempt(e.handle)
|
||||
perNetwork[label] = Triple(e.model.id, ok, detail)
|
||||
if (ok) { anyOk = true; rtt?.let(rtts::add) }
|
||||
val a = attempt(e.handle)
|
||||
perNetwork[label] = e.model.id to a
|
||||
if (a.ok) { anyOk = true; a.rttMs?.let(rtts::add) }
|
||||
}
|
||||
|
||||
// Per-network results are recorded structurally, not just as prose. The aggregate status
|
||||
@@ -62,11 +63,12 @@ class IcmpProbe(
|
||||
val evidence: JsonObject = buildJsonObject {
|
||||
put("target", target)
|
||||
for ((label, r) in perNetwork) {
|
||||
val (netId, ok, detail) = r
|
||||
val (netId, a) = r
|
||||
put(label, buildJsonObject {
|
||||
netId?.let { put("network_ref", it) }
|
||||
put("ok", ok)
|
||||
put("detail", detail)
|
||||
put("ok", a.ok)
|
||||
put("attempted", a.attempted)
|
||||
put("detail", a.detail)
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -80,14 +82,31 @@ class IcmpProbe(
|
||||
b.build(status, evidence = evidence, metrics = metrics)
|
||||
}
|
||||
|
||||
private data class Attempt(val ok: Boolean, val detail: String, val rttMs: Double?)
|
||||
/**
|
||||
* One network's result.
|
||||
*
|
||||
* [attempted] separates "we sent an echo request and heard nothing" from "we never got as far
|
||||
* as sending one". Both leave [ok] false, and collapsing them is how a probe ends up asserting
|
||||
* something about a network it never touched: binding to a non-default network can fail with
|
||||
* EPERM, and reporting that as ICMPv6 silence blames the carrier for the app's own inability
|
||||
* to use the interface.
|
||||
*/
|
||||
private data class Attempt(
|
||||
val ok: Boolean,
|
||||
val attempted: Boolean,
|
||||
val detail: String,
|
||||
val rttMs: Double?,
|
||||
)
|
||||
|
||||
private fun attempt(network: Network?): Attempt {
|
||||
var fd: FileDescriptor? = null
|
||||
var sent = false
|
||||
return try {
|
||||
val proto = if (v6) OsConstants.IPPROTO_ICMPV6 else OsConstants.IPPROTO_ICMP
|
||||
val family = if (v6) OsConstants.AF_INET6 else OsConstants.AF_INET
|
||||
fd = Os.socket(family, OsConstants.SOCK_DGRAM, proto)
|
||||
// Everything up to and including sendto is setup. A failure here means the test did
|
||||
// not run on this network — not that the network stayed silent.
|
||||
network?.bindSocket(fd)
|
||||
Os.setsockoptTimeval(fd, OsConstants.SOL_SOCKET, OsConstants.SO_RCVTIMEO, StructTimeval.fromMillis(3000))
|
||||
val addr = network?.getByName(target) ?: InetAddress.getByName(target)
|
||||
@@ -96,14 +115,16 @@ class IcmpProbe(
|
||||
val packet = buildEchoRequest(v6, ident.toShort(), 1)
|
||||
val t0 = System.nanoTime()
|
||||
Os.sendto(fd, packet, 0, packet.size, 0, addr, 0)
|
||||
sent = true
|
||||
val buf = ByteBuffer.allocate(1500)
|
||||
val received = Os.recvfrom(fd, buf, 0, null)
|
||||
val rttMs = (System.nanoTime() - t0) / 1_000_000.0
|
||||
val replyType = if (received > 0) buf.get(0).toInt() and 0xFF else -1
|
||||
val ok = replyType == (if (v6) 129 else 0)
|
||||
Attempt(ok, "reply type=$replyType rtt_ms=${"%.1f".format(Locale.ROOT, rttMs)} bytes=$received", if (ok) rttMs else null)
|
||||
Attempt(ok, true, "reply type=$replyType rtt_ms=${"%.1f".format(Locale.ROOT, rttMs)} bytes=$received", if (ok) rttMs else null)
|
||||
} catch (e: Throwable) {
|
||||
Attempt(false, "error: ${e.message ?: e.javaClass.simpleName}", null)
|
||||
// A timeout after a successful send is a real "no reply"; anything before it is not.
|
||||
Attempt(false, sent, "error: ${e.message ?: e.javaClass.simpleName}", null)
|
||||
} finally {
|
||||
fd?.let { runCatching { Os.close(it) } }
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user