From ee17d773b7de4e77d3f5d045dade83d0e7767674 Mon Sep 17 00:00:00 2001 From: Paul Hitt Date: Tue, 1 Sep 2026 09:12:28 -0400 Subject: [PATCH] A session we ANSWER can now be v3, and receive failures say why MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `E2ESession.respond` hard-coded `version = if (pq == null) 1 else 2`, so a session this device answered could never be v3 — while `initiate` twelve lines above correctly returns 3 whenever the post-quantum ratchet is on. A conversation iOS started therefore ran as v3 on one side and v2 on the other, and every message this device sent into it was refused by the peer as a downgrade (§13.4) and dropped on arrival. iOS to Android worked; Android to iOS did not, for as long as the pairing existed. Kotlin talking to Kotlin never noticed, because both ends drive the ratchet from their own PQ state and ignore the label. Only a peer that enforces the version sees it. `HybridSessionTest` asserted the wrong value — `assertEquals(2, bobSide.version)`, two lines below requiring the initiator to be 3 — so the suite passed while encoding the defect. Corrected, with the reason recorded next to it. The version now follows the initiator rather than assuming 3 whenever PQ is present: a v2 peer sends a PQ block too, and answering that in v3 breaks the pairing in the other direction. Receive-side failures are no longer swallowed. `catch (_: Exception) {}` threw away the reason for every undecryptable envelope, which left this half of every delivery investigation unreadable — the relay says delivered, the app shows nothing, and the one place that knew why discarded it. The envelope is still skipped; it now names the id, the device and the exception. Sends report the protocol shape they emit, and receives report which of the three chains opened them. Also here: - proguard-rules.pro, which did not exist while build.gradle.kts referenced it. R8 accepts a missing entry in silence, so release builds ran on the platform defaults and answering a call aborted the process inside WebRTC's JNI_OnLoad — it resolves Java classes by name, and R8 had renamed them. The build now fails if the file is missing rather than shipping without it. - Server refusals read as sentences. `RelayException.Http` had no case in `describe()`, so choosing a taken handle showed "relay returned 409:" and a JSON fragment, which says nothing about picking another name. Transport failures showed a Java networking string naming an address the person never typed. - Switching between signing in and creating an account clears the last error. It belonged to the other mode and stayed on screen, so taking the advice it gave looked like it had failed. Co-Authored-By: Claude Opus 4.8 --- app/build.gradle.kts | 11 +++ app/proguard-rules.pro | 50 +++++++++++++ .../io/flamenet/messenger/app/CallManager.kt | 55 +++++++++++--- .../io/flamenet/messenger/app/MainActivity.kt | 8 ++- .../messenger/app/MessengerViewModel.kt | 44 +++++++++++- .../io/flamenet/messenger/app/Session.kt | 15 +++- .../io/flamenet/client/Conversations.kt | 71 ++++++++++++++++++- .../kotlin/io/flamenet/client/RelayAuth.kt | 29 +++++++- .../main/kotlin/io/flamenet/e2e/E2ESession.kt | 19 ++++- .../io/flamenet/e2e/HybridSessionTest.kt | 14 +++- 10 files changed, 296 insertions(+), 20 deletions(-) create mode 100644 app/proguard-rules.pro diff --git a/app/build.gradle.kts b/app/build.gradle.kts index 721949d..7e22a6e 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -105,6 +105,17 @@ android { // Debug keeps all four, because that is what runs on an emulator. ndk { abiFilters += setOf("arm64-v8a", "armeabi-v7a") } isMinifyEnabled = true + // 🔴 **R8 accepts a missing rules file in silence.** This one was + // referenced here and did not exist, so the release build shipped + // with the platform defaults alone — and answering a call aborted + // the process, because WebRTC's native JNI_OnLoad resolves its Java + // classes by name and R8 had renamed them. Nothing failed, nothing + // warned, and no debug test could see it. Fail the build instead. + file("proguard-rules.pro").takeIf { it.isFile } + ?: throw GradleException( + "app/proguard-rules.pro is missing. The release build needs it — " + + "without the org.webrtc keep rules, answering a call aborts the " + + "process inside JNI_OnLoad with no Kotlin stack trace.") proguardFiles(getDefaultProguardFile("proguard-android-optimize.txt"), "proguard-rules.pro") signingConfig = signingConfigs.findByName("release") } diff --git a/app/proguard-rules.pro b/app/proguard-rules.pro new file mode 100644 index 0000000..c8f421a --- /dev/null +++ b/app/proguard-rules.pro @@ -0,0 +1,50 @@ +# R8 rules for the release build. +# +# 🔴 **This file did not exist, and `build.gradle.kts` referenced it anyway.** +# R8 accepts a missing `proguardFiles` entry in silence, so the release build +# ran with the platform defaults alone and nobody was told. What that cost: +# answering a call on a release build killed the process instantly — +# +# Fatal signal 6 (SIGABRT), thread DefaultDispatch +# JNI_OnLoad <- libjingle_peerconnection_so.so +# System.loadLibrary +# org.webrtc.PeerConnectionFactory +# +# — because WebRTC's native `JNI_OnLoad` resolves its Java classes **by name**, +# and R8 had renamed them. The lookup fails and the library aborts the process +# rather than throwing, so there is no Kotlin stack trace and nothing in the +# call log: the app is gone before it can write one. Reported as "calling from +# iOS to Android crashes Android when I hit answer" and, from the other side, +# "iOS shows 2 calls, Android shows 0". One cause, two symptoms. +# +# ⚠️ **Debug builds cannot catch this and never will.** `connectedDebugAndroidTest` +# runs the unminified variant, where every one of these classes still has its +# real name. Every call test this project has ever run was blind to it. A +# release-build call test is the only thing that would have. + +# --- WebRTC ---------------------------------------------------------------- +# Everything, members included: the native side looks up classes, methods AND +# fields by their source names. Narrowing this to class names alone is the +# tempting half-fix and it still aborts, in a different frame. +-keep class org.webrtc.** { *; } +-dontwarn org.webrtc.** + +# Any class holding native methods must keep those names, or the JNI binding +# that pairs them by signature finds nothing. +-keepclasseswithmembernames class * { + native ; +} + +# WebRTC marks its JNI entry points with this annotation. Keeping the +# annotation itself is what lets the rule above stay meaningful after R8's +# attribute stripping. +-keepattributes *Annotation*,InnerClasses,Signature,EnclosingMethod + +# --- Conscrypt ------------------------------------------------------------- +# ⚠️ The engine selects Conscrypt explicitly by provider name (see +# android/PARITY.md). Renaming its classes turns that selection into a silent +# fallback to the JVM's own provider, which is a different implementation with +# different behaviour — the exact class of divergence the parity tests exist +# to catch, arriving only in release. +-keep class org.conscrypt.** { *; } +-dontwarn org.conscrypt.** diff --git a/app/src/main/kotlin/io/flamenet/messenger/app/CallManager.kt b/app/src/main/kotlin/io/flamenet/messenger/app/CallManager.kt index 24f0ce0..b00417e 100644 --- a/app/src/main/kotlin/io/flamenet/messenger/app/CallManager.kt +++ b/app/src/main/kotlin/io/flamenet/messenger/app/CallManager.kt @@ -149,7 +149,21 @@ class CallManager( onGathered = null gatherGuard?.let { main.removeCallbacks(it) } gatherGuard = null - val sdp = connection?.localDescription?.description ?: return + // 🔴 **This `?: return` sent nobody anything, silently.** After the + // 8s gather timeout the local description can still be null — because + // setLocalDescription had not finished, or had FAILED and said nothing + // (see SimpleSdpObserver). The call then dies here: no offer, no + // envelope on the relay, no ring on the other phone, and no error. + // Verified against the database — zero envelopes had ever been created + // between two accounts that had "placed" several calls. + val sdp = connection?.localDescription?.description ?: run { + android.util.Log.w("fnmsg-call", "NOT SENDING the offer/answer: no localDescription after " + + "gathering (state=${connection?.iceGatheringState()}). " + + "The other side will never see this call.") + return + } + android.util.Log.i("fnmsg-call", "sending signal after gathering, sdp=${sdp.length} bytes, " + + "state=${connection?.iceGatheringState()}") send(sdp) } @@ -213,7 +227,15 @@ class CallManager( continualGatheringPolicy = PeerConnection.ContinualGatheringPolicy.GATHER_ONCE } val observer = object : PeerConnection.Observer { - override fun onIceCandidate(candidate: IceCandidate?) = Unit + override fun onIceCandidate(candidate: IceCandidate?) { + // 🔴 The candidate TYPE is the whole diagnosis. `host` only + // means we never reached STUN/TURN and cannot cross a NAT; + // `relay` means TURN answered and the call can connect even + // when a direct path is impossible. + val sdp = candidate?.sdp ?: return + val kind = Regex("typ (\\w+)").find(sdp)?.groupValues?.get(1) ?: "?" + android.util.Log.i("fnmsg-call", "ice candidate: typ=$kind") + } override fun onIceCandidatesRemoved(candidates: Array?) = Unit override fun onSignalingChange(state: PeerConnection.SignalingState?) = Unit // 🔴 **Nothing may throw out of here, and nothing may call back @@ -222,6 +244,7 @@ class CallManager( // out — so ANY escaping Kotlin exception aborts the process: // SIGABRT, no AndroidRuntime trace, nothing to catch. override fun onIceConnectionChange(newState: PeerConnection.IceConnectionState?) { + android.util.Log.i("fnmsg-call", "ice connection -> $newState") runCatching { when (newState) { PeerConnection.IceConnectionState.CONNECTED, @@ -239,6 +262,7 @@ class CallManager( } override fun onIceConnectionReceivingChange(receiving: Boolean) = Unit override fun onIceGatheringChange(newState: PeerConnection.IceGatheringState?) { + android.util.Log.i("fnmsg-call", "ice gathering -> $newState") // Hop to the main thread rather than touching the connection // from here — same JNI rule as above: no WebRTC calls, and no // exceptions, from inside a WebRTC callback. @@ -327,7 +351,7 @@ class CallManager( } pc.createOffer(object : SimpleSdpObserver() { override fun onCreateSuccess(sdp: SessionDescription) { - pc.setLocalDescription(SimpleSdpObserver(), sdp) + pc.setLocalDescription(SimpleSdpObserver("offer:setLocal"), sdp) // The complete SDP — with candidates — exists only after // gathering; see onGathered. DIALLING holds until the answer // arrives, so the caller's screen says what is actually true. @@ -428,12 +452,12 @@ class CallManager( } current = Active(offer.callId, fromUser, incoming = true, video = offer.video) pc.setRemoteDescription( - SimpleSdpObserver(), + SimpleSdpObserver("answer:setRemote"), SessionDescription(SessionDescription.Type.OFFER, sdp), ) pc.createAnswer(object : SimpleSdpObserver() { override fun onCreateSuccess(answer: SessionDescription) { - pc.setLocalDescription(SimpleSdpObserver(), answer) + pc.setLocalDescription(SimpleSdpObserver("answer:setLocal"), answer) // Same as the offer: the answer is complete only after // gathering, and one sent early names no address to reach. main.post { @@ -614,9 +638,24 @@ object PendingIncomingCall { } /** Most SDP callbacks have nothing to say; this stops four stubs per call site. */ -private open class SimpleSdpObserver : SdpObserver { +/** + * 🔴 **This discarded every SDP failure, and that is why calls were undebuggable.** + * `createOffer` or `setLocalDescription` could fail for any reason and nothing + * anywhere recorded it — the offer then simply never went out, the callee never + * rang, and the database showed no envelope had ever been created. Four hours of + * "the call failed" with no cause, because the two callbacks that knew returned + * `Unit`. + * + * ⚠️ A `label` so the log says WHICH step failed. "setLocalDescription failed" + * and "createAnswer failed" send you to different places. + */ +private open class SimpleSdpObserver(private val label: String = "sdp") : SdpObserver { override fun onCreateSuccess(sdp: SessionDescription) = Unit override fun onSetSuccess() = Unit - override fun onCreateFailure(error: String?) = Unit - override fun onSetFailure(error: String?) = Unit + override fun onCreateFailure(error: String?) { + android.util.Log.w("fnmsg-call", "$label: createOffer/createAnswer FAILED: $error") + } + override fun onSetFailure(error: String?) { + android.util.Log.w("fnmsg-call", "$label: setLocal/setRemoteDescription FAILED: $error") + } } diff --git a/app/src/main/kotlin/io/flamenet/messenger/app/MainActivity.kt b/app/src/main/kotlin/io/flamenet/messenger/app/MainActivity.kt index ab36742..f6f67f8 100644 --- a/app/src/main/kotlin/io/flamenet/messenger/app/MainActivity.kt +++ b/app/src/main/kotlin/io/flamenet/messenger/app/MainActivity.kt @@ -479,7 +479,13 @@ private fun ColumnScope.SignIn(model: MessengerViewModel, phase: MessengerViewMo } TextButton( - onClick = { registering = !registering }, + onClick = { + registering = !registering + // The last attempt's message was about the OTHER mode; leaving it + // up makes switching look like it failed. + ageError = null + model.clearAuthError() + }, modifier = Modifier.fillMaxWidth().testTag("toggle-register"), ) { Text(if (registering) "I already have an account" else "Create an account") diff --git a/app/src/main/kotlin/io/flamenet/messenger/app/MessengerViewModel.kt b/app/src/main/kotlin/io/flamenet/messenger/app/MessengerViewModel.kt index 78a5a0a..1bf3b85 100644 --- a/app/src/main/kotlin/io/flamenet/messenger/app/MessengerViewModel.kt +++ b/app/src/main/kotlin/io/flamenet/messenger/app/MessengerViewModel.kt @@ -404,6 +404,22 @@ class MessengerViewModel(app: Application) : AndroidViewModel(app) { ) } + /** + * Forget whatever the last sign-in attempt said. + * + * 🔴 **Switching between signing in and creating an account has to do + * this.** The message belongs to the mode it was produced in — "that + * handle is already taken" is advice about signing up, and it stayed on + * screen after the person took that advice and switched to signing in, + * where it reads as a refusal of the thing they are now trying to do. + */ + fun clearAuthError() { + val current = _phase.value + if (current is Phase.SignedOut && current.error != null) { + _phase.value = current.copy(error = null) + } + } + fun signIn(handle: String, passphrase: String, register: Boolean, email: String = "") { val config = site ?: run { _phase.value = Phase.SignedOut(error = "That doesn't look like a server address.") @@ -2460,7 +2476,11 @@ class MessengerViewModel(app: Application) : AndroidViewModel(app) { is AccountsClient.Failure.HandleTaken -> "That handle is already taken." is AccountsClient.Failure.RateLimited -> "Too many attempts. Wait a few minutes." - is AccountsClient.Failure.Transport -> failure.message.orEmpty() + // ⚠️ Not the underlying message: at this layer it is a Java networking + // string ("Failed to connect to /10.0.2.2:443"), which names an address + // the person never typed. + is AccountsClient.Failure.Transport -> + "Couldn't reach that server. Check the address and your connection." // ⚠️ NOT "that passphrase is wrong". The relay rejects a TOKEN, which // the user never typed and cannot check. Telling them their passphrase // is wrong when their session merely aged out sends them off to change @@ -2477,7 +2497,27 @@ class MessengerViewModel(app: Application) : AndroidViewModel(app) { if (mins == 1) "." else "s." } ?: "Too many attempts. Wait a few minutes." is Vault.Corrupt -> "This phone's stored keys could not be read." - else -> failure.message ?: "Something went wrong reaching that server." + + // 🔴 **Anything not named above used to reach the screen verbatim.** + // `RelayException.Http` carries "relay returned 409: {body}" as its + // message, so somebody choosing a handle that was already in use was + // shown a status code and a JSON fragment. Nothing in that tells them + // to pick a different name, which is the entire content of the answer. + // + // ⚠️ The body is deliberately dropped rather than appended. It is + // written for whoever is reading the logs, and half of these carry the + // handle back in it. + is RelayException.Http -> when (failure.status) { + 409 -> "That handle is already taken. Try a different one." + 400, 422 -> "That server couldn't accept those details. " + + "Check the handle and try again." + 403 -> "That isn't allowed on this server." + in 500..599 -> "That server is having trouble right now. " + + "Try again in a moment." + else -> "That server couldn't complete this. Try again in a moment." + } + is RelayException.Malformed -> "That server sent back something unexpected." + else -> "Something went wrong reaching that server." } /** diff --git a/app/src/main/kotlin/io/flamenet/messenger/app/Session.kt b/app/src/main/kotlin/io/flamenet/messenger/app/Session.kt index f471d32..3d510ab 100644 --- a/app/src/main/kotlin/io/flamenet/messenger/app/Session.kt +++ b/app/src/main/kotlin/io/flamenet/messenger/app/Session.kt @@ -630,8 +630,21 @@ class Session( } /** TURN credentials, or null when this relay has none configured. */ + /** + * 🔴 **A null here silently downgrades every call to STUN-only.** + * `CallManager.iceServers(null)` falls back to Google's STUN server, which + * cannot relay — so two peers behind NAT never connect, the call "just + * fails", and coturn records nothing because it was never contacted. The + * failure was swallowed by `getOrNull()`, so nothing anywhere said why. + */ fun turnCredentials(): org.json.JSONObject? = - runCatching { relay.turnCredentials() }.getOrNull() + runCatching { relay.turnCredentials() } + .onFailure { + android.util.Log.w("fnmsg-call", "no TURN credentials " + + "(${it.javaClass.simpleName}: ${it.message}) — this call will use " + + "STUN only and will not connect across NAT", it) + } + .getOrNull() fun sendGroup(group: RelayClient.Group, text: String): Conversations.Sent { val sent = conversations.sendToGroup(group, text, userId) diff --git a/client/src/main/kotlin/io/flamenet/client/Conversations.kt b/client/src/main/kotlin/io/flamenet/client/Conversations.kt index ae9bf85..72b057a 100644 --- a/client/src/main/kotlin/io/flamenet/client/Conversations.kt +++ b/client/src/main/kotlin/io/flamenet/client/Conversations.kt @@ -573,7 +573,21 @@ class Conversations( // where the key route returned one. stubs.map { full[it.deviceId] ?: it } } - guardIdentity(toUserId, bundles) + // 🔴 **The send path had no logging at all, and that is why a message + // that never left the phone was indistinguishable from one that did.** + // The relay records refusals, so an absent POST looks identical to a + // successful one from the server side; only this side knows. + System.err.println("fnmsg-send: to=$toUserId stubs=${stubs.size} " + + "bundles=${bundles.size} mine=${identity.deviceId}") + try { + guardIdentity(toUserId, bundles) + } catch (e: Exception) { + System.err.println("fnmsg-send: REFUSED by guardIdentity " + + "(${e.javaClass.simpleName}: ${e.message}) — nothing will be sent. " + + "This fires when the peer's identity key changed, e.g. after they " + + "reinstalled or the account was recreated.") + throw e + } val delivered = mutableListOf() val failed = LinkedHashMap() val outgoing = mutableListOf() @@ -594,6 +608,8 @@ class Conversations( // above call this once per member with no try around it — throwing // would abandon everyone after the first person who is signed out. if (bundles.none { it.deviceId != identity.deviceId }) { + System.err.println("fnmsg-send: NOT SENDING — the recipient has no " + + "device other than ours (bundles=${bundles.map { it.deviceId }})") failed["user:$toUserId"] = "not signed in on any device" return Sent(delivered, failed, sealed = false) } @@ -626,6 +642,15 @@ class Conversations( bundle.deviceId, wrap(pre, bundle, sealed)) } val envelope = session.encrypt(text) + // ⚠️ Protocol shape only — version and whether the post-quantum + // half of the ratchet step is on the wire. A v3 peer refuses a + // step without it, and nothing on this side reported what we + // actually emitted. + System.err.println( + "fnmsg-send: envelope v${envelope.version} ${envelope.type} " + + "n=${envelope.header.n} pqdh=${envelope.header.pqdh != null} " + + "pqct=${envelope.header.pqct != null} " + + "sessionVersion=${session.version}") // Saved BEFORE the network call. If the relay accepts the // envelope and we then crash, the peer has a message our // ratchet has not advanced past, and nothing either side can @@ -640,7 +665,19 @@ class Conversations( } } + if (outgoing.isEmpty()) { + System.err.println("fnmsg-send: NOT SENDING — every device failed to " + + "encrypt: $failed") + } if (outgoing.isNotEmpty()) { + System.err.println("fnmsg-send: POSTing ${outgoing.size} envelope(s) to " + + "/e2e/messages for user $toUserId sealed=$sealed " + + "toDevices=${outgoing.map { it.deviceId }.distinct()}") + // 🔴 The OUTCOME, not just the attempt. "POSTing" told us the code + // reached the call; it did not tell us whether the request left the + // phone. A DNS blip or a dropped connection throws here, and every + // caller above treats a thrown send as a failed message without + // ever saying what failed. try { relay.sendEnvelopes( toUserId, outgoing, identity.deviceId, @@ -651,7 +688,15 @@ class Conversations( deliveryKey = if (peerToken == null) peerKey?.let(Wire::b64) else null, deliveryToken = peerToken, ) - } catch (e: RelayException.Unauthorized) { + System.err.println("fnmsg-send: relay ACCEPTED ${outgoing.size} envelope(s)") + } catch (e: Exception) { + if (e !is RelayException.Unauthorized) { + System.err.println("fnmsg-send: SEND FAILED " + + "(${e.javaClass.simpleName}: ${e.message}) — the message did " + + "not leave this phone") + throw e + } + @Suppress("NAME_SHADOWING") val e = e // 🔴 **A refused capability means the batch was revoked.** // Retrying with the rest of it for ever is how a conversation // quietly stops working, so the batch goes and the SAME @@ -1081,6 +1126,11 @@ class Conversations( openWith(E2ESession.respond(identity, envelope), envelope) } else null + System.err.println( + "fnmsg-recv: envelope ${message.id} opened by " + + (if (fromCurrent != null) "current" else if (fromRetired != null) "retired" + else if (fromFresh != null) "FRESH handshake" else "nothing") + + " (sealed=$sealed, handshake=${envelope.handshake != null})") val plaintext = when { fromCurrent != null -> { vault.saveSession(fromDevice, fromCurrent.session) @@ -1170,7 +1220,22 @@ class Conversations( sealed = sealed, senderProven = senderProven, at = message.createdAt, fromAddress = fromAddress) - } catch (_: Exception) { + } catch (failure: Exception) { + // 🔴 **Skipped, but never again in silence.** Swallowing this + // whole is what made Android's half of every delivery + // investigation unreadable: the relay says delivered, the app + // shows nothing, and the one place that knew why threw the + // reason away. The message is still skipped — that part was + // right — but it now says which envelope, from which device, + // and what actually failed. + // + // ⚠️ The exception type and its message only. Neither carries + // plaintext or key material; `IdentityChangedException` carries + // a user id and the rest are fixed strings. + System.err.println( + "fnmsg-recv: DROPPED envelope ${message.id} from " + + "${message.fromDevice.take(8)} — " + + "${failure.javaClass.simpleName}: ${failure.message}") // Deliberately swallowed and skipped. One undecryptable message // is not a reason to stop reading the inbox. // diff --git a/client/src/main/kotlin/io/flamenet/client/RelayAuth.kt b/client/src/main/kotlin/io/flamenet/client/RelayAuth.kt index 659c783..da3827d 100644 --- a/client/src/main/kotlin/io/flamenet/client/RelayAuth.kt +++ b/client/src/main/kotlin/io/flamenet/client/RelayAuth.kt @@ -50,17 +50,40 @@ class RelayAuth( * A valid token, minting one when the cache is empty or close to expiry. * Null when this deployment does not issue them. */ + /** Plain Kotlin module: no android.util.Log here. On device this lands in + * logcat as `W/System.err`, which is what matters — it is READABLE. */ + private fun warn(message: String) = System.err.println("fnmsg-auth: $message") + fun token(now: Long = System.currentTimeMillis() / 1000): String? { if (unavailable) return null cached?.let { if (it.expiresAt - SLACK_SECONDS > now) return it.token } val minted = try { mint(deviceId, Wire.b64(identityKey.publicKey)) - } catch (_: RelayException.NotFound) { + } catch (e: RelayException.NotFound) { + // 🔴 **Say it.** Every failure here used to be swallowed in + // silence, and the consequence was invisible: with no token the + // client falls back to Basic, the relay refuses `/e2e/*` — which it + // does by design — and the user sees "the call failed", "could not + // finish setting up", or an inbox that 410s and wipes itself. Four + // hours of debugging produced symptoms and no cause, because the + // one line that knew what went wrong discarded it. + warn("no token: relay has no /account/token route; " + + "falling back to Basic, which /e2e/* refuses. Calls and device " + + "registration will fail until this is fixed." + " [" + e + "]") unavailable = true return null - } catch (_: Exception) { + } catch (e: Exception) { + warn("no token: minting failed (${e.javaClass.simpleName}: " + + "${e.message}); falling back to Basic, which /e2e/* refuses.") return null - } ?: return null + } ?: run { + // ⚠️ A non-2xx that is not 404 arrives here as null, not as a + // throw, so this branch is NOT unreachable — it was the quietest + // path of the three. + warn("no token: /account/token answered a non-success " + + "status; falling back to Basic, which /e2e/* refuses.") + return null + } cached = Cached(minted.token, now + minted.lifetimeSeconds) return minted.token } diff --git a/engine/src/main/kotlin/io/flamenet/e2e/E2ESession.kt b/engine/src/main/kotlin/io/flamenet/e2e/E2ESession.kt index 59fbd07..101ff49 100644 --- a/engine/src/main/kotlin/io/flamenet/e2e/E2ESession.kt +++ b/engine/src/main/kotlin/io/flamenet/e2e/E2ESession.kt @@ -173,7 +173,24 @@ class E2ESession private constructor( associatedData = X3DH.associatedData( theirIdentitySigning, identity.identitySigningPublic, pq?.second), pendingHandshake = null, - version = if (pq == null) 1 else 2, + // 🔴 **Honour what the initiator declared; this could never + // return 3.** `initiate` above sets 3 whenever the PQ ratchet + // is on, but responding capped at 2 — so a session this device + // ANSWERED was labelled v2 while the far side ran it as v3. + // Every message then arrived at a v3 peer without the + // post-quantum material a v3 step requires, and was refused as + // a downgrade (§13.4) — correctly, and for ever. iOS-initiated + // conversations were one-way: iOS to Android worked, Android to + // iOS was silently dropped on arrival. + // + // ⚠️ Reads the initiator's version rather than assuming 3 + // whenever PQ is present: a v2 peer sends a PQ block too, and + // answering it in v3 breaks the pairing in the other direction. + version = when { + pq == null -> 1 + envelope.version >= 3 -> 3 + else -> 2 + }, ) return session } diff --git a/engine/src/test/kotlin/io/flamenet/e2e/HybridSessionTest.kt b/engine/src/test/kotlin/io/flamenet/e2e/HybridSessionTest.kt index b736ed8..805a237 100644 --- a/engine/src/test/kotlin/io/flamenet/e2e/HybridSessionTest.kt +++ b/engine/src/test/kotlin/io/flamenet/e2e/HybridSessionTest.kt @@ -98,7 +98,19 @@ class HybridSessionTest { val delivered = Envelope.fromPayload(envelope.toPayload()) val bobSide = E2ESession.respond(bob, delivered) - assertEquals(2, bobSide.version) + // 🔴 **3, and asserting 2 here is what hid a real bug for weeks.** + // The initiator is v3 two assertions above; a responder that answers + // the same handshake as v2 disagrees with its own peer about which + // protocol the session runs. Kotlin-to-Kotlin never noticed, because + // both ends drive the ratchet off their own PQ state and ignore the + // label — so this test round-tripped happily while encoding the defect. + // + // A v3 peer that does enforce it does notice: iOS refuses a ratchet + // step with no post-quantum material as a downgrade, so every message + // Android sent into an iOS-initiated session was dropped on arrival and + // the conversation ran one way only. + assertEquals(3, bobSide.version, + "answering a v3 handshake makes a v3 session, not a v2 one") assertEquals("post-quantum hello", bobSide.decrypt(delivered)) assertContentEquals(session.associatedData, bobSide.associatedData, "both sides must bind the same ciphertext digest")