diff --git a/app/build.gradle.kts b/app/build.gradle.kts index 721949d..0fbd998 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -33,8 +33,8 @@ android { // โš ๏ธ Android also refuses to install an APK whose `versionCode` is // lower than the installed one, so a code that goes backwards makes a // release uninstallable for everybody already on the higher number. - versionCode = 7 - versionName = "0.7" + versionCode = 8 + versionName = "0.8" testInstrumentationRunner = "androidx.test.runner.AndroidJUnitRunner" } @@ -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..6978b07 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. @@ -416,24 +440,18 @@ class CallManager( var onStaleOffer: ((Long, Long) -> Unit)? = null /** Accept the call that is ringing. */ - fun accept(turn: JSONObject?) { - val (fromUser, offer) = pendingOffer ?: return - pendingOffer = null - val sdp = offer.sdp ?: return - state = State.CONNECTING - armConnectGuard() - val pc = open(turn, fromUser, offer.callId, offer.video) ?: run { - state = State.IDLE - return - } - current = Active(offer.callId, fromUser, incoming = true, video = offer.video) - pc.setRemoteDescription( - SimpleSdpObserver(), - SessionDescription(SessionDescription.Type.OFFER, sdp), - ) - pc.createAnswer(object : SimpleSdpObserver() { + /** + * Build and send the answer. Split out so it can only be reached from + * `setRemoteDescription`'s success callback โ€” the ordering IS the fix. + */ + private fun answerNow( + pc: PeerConnection, + fromUser: Long, + offer: Content.CallSignal, + ) { + pc.createAnswer(object : SimpleSdpObserver("answer:create") { 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 { @@ -447,6 +465,39 @@ class CallManager( }, MediaConstraints()) } + fun accept(turn: JSONObject?) { + val (fromUser, offer) = pendingOffer ?: return + pendingOffer = null + val sdp = offer.sdp ?: return + state = State.CONNECTING + armConnectGuard() + val pc = open(turn, fromUser, offer.callId, offer.video) ?: run { + state = State.IDLE + return + } + current = Active(offer.callId, fromUser, incoming = true, video = offer.video) + // ๐Ÿ”ด **`createAnswer` waits for the remote description to be APPLIED.** + // These two were fired back to back: `setRemoteDescription` is + // asynchronous, and WebRTC cannot answer an offer it has not finished + // reading. When it lost that race `createAnswer` failed, + // `setLocalDescription` never ran, and the send path found no local + // description and gave up โ€” "NOT SENDING the offer/answer ... the other + // side will never see this call". Accepting a call did nothing, the + // caller rang on until it timed out, and neither end said why. + // + // โš ๏ธ It is a RACE, which is what made it look intermittent: on a quiet + // device the description often lands first and the call connects, so + // the same build both works and fails depending on load. + pc.setRemoteDescription( + object : SimpleSdpObserver("answer:setRemote") { + override fun onSetSuccess() { + main.post { answerNow(pc, fromUser, offer) } + } + }, + SessionDescription(SessionDescription.Type.OFFER, sdp), + ) + } + /** Refuse a ringing call, and say so rather than letting it ring out. */ fun decline() { val (fromUser, offer) = pendingOffer ?: return @@ -614,9 +665,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/ChatScreens.kt b/app/src/main/kotlin/io/flamenet/messenger/app/ChatScreens.kt index d21e040..359f374 100644 --- a/app/src/main/kotlin/io/flamenet/messenger/app/ChatScreens.kt +++ b/app/src/main/kotlin/io/flamenet/messenger/app/ChatScreens.kt @@ -712,6 +712,7 @@ fun ColumnScope.AccountScreen( var displayName by remember { mutableStateOf("") } var password by remember { mutableStateOf("") } var confirmingDelete by remember { mutableStateOf(false) } + var confirmingErase by remember { mutableStateOf(false) } Text("Account", style = MaterialTheme.typography.headlineMedium) Text( @@ -876,9 +877,28 @@ fun ColumnScope.AccountScreen( SettingsSection { SettingsRow( label = "Sign out", - description = "Erases this phone's keys and stored messages. They cannot be recovered.", + description = "Ends this session and keeps your messages on this phone, " + + "so signing back in picks up where you left off.", tag = "sign-out", - onClick = model::signOut, + onClick = { model.signOut() }, + ) + // ๐Ÿ”ด **The one that erases is its own row, and is not the default.** + // Both used to be the same button: signing out for an afternoon + // destroyed this phone's identity, so coming back made every contact + // accept a safety-number change first. Erasing is a real thing people + // need โ€” handing a phone on โ€” but it is the rarer thing, and it must be + // chosen rather than stumbled into. Confirms in place, like the pair + // below it, because a dialog is dismissed reflexively. + SettingsRow( + label = if (confirmingErase) "Really erase โ€” this cannot be undone" + else "Sign out and erase this device", + description = "Also removes this phone's keys and stored messages. " + + "Choose this if you are handing the phone on.", + tag = "sign-out-erase", + onClick = { + if (confirmingErase) model.signOut(erasingDevice = true) + else confirmingErase = true + }, ) SettingsRow( label = if (confirmingDelete) "Really delete โ€” this cannot be undone" @@ -1083,6 +1103,13 @@ private fun ColumnScope.AppLockSection() { * โš ๏ธ There is no "public" option and cannot be one. A story only ever goes to * contacts, because it has to be encrypted to somebody in particular. */ +/** โš ๏ธ Plain words: "168 hours" would make the reader do the division. */ +private fun storyLifetimeLabel(hours: Int): String = when { + hours < 24 -> if (hours == 1) "1 hour" else "$hours hours" + hours == 24 -> "24 hours" + else -> (hours / 24).let { if (it == 1) "1 day" else "$it days" } +} + @Composable private fun ColumnScope.StoryAudienceSection( model: MessengerViewModel, @@ -1139,6 +1166,42 @@ private fun ColumnScope.StoryAudienceSection( color = Palette.SecondaryText, ) + Spacer(Modifier.height(Space.lg)) + Text("How long your stories last", style = MaterialTheme.typography.titleMedium) + + // ๐Ÿ”ด A day used to be the only answer. The expiry rides inside the sealed + // story, so this is the poster's choice travelling with every copy โ€” not a + // rule the relay is being asked to keep. + val storyContext = LocalContext.current + val mediaSettings = remember { MediaSettings(storyContext) } + var storyHours by remember { mutableStateOf(mediaSettings.storyLifetimeHours) } + Row( + modifier = Modifier.fillMaxWidth().padding(vertical = Space.sm), + horizontalArrangement = Arrangement.spacedBy(Space.sm), + ) { + MediaSettings.STORY_LIFETIME_CHOICES.forEach { hours -> + KitChip( + text = storyLifetimeLabel(hours), + selected = storyHours == hours, + onClick = { + storyHours = hours + mediaSettings.storyLifetimeHours = hours + }, + tag = "story-lifetime-$hours", + ) + } + } + Text( + // โš ๏ธ Says what it does NOT promise. "Disappears after" alone reads as a + // guarantee about other people's phones, and a viewer who screenshots a + // story has it for good. + "Stories you post from now on disappear after this long, on your device " + + "and your contacts'. Ones already posted keep the time they were " + + "sent with.", + fontSize = 14.sp, + color = Palette.SecondaryText, + ) + // The list only earns its place when the mode uses one. if (audience.mode != Session.AudienceMode.EVERYONE) { if (phase.buddies.isEmpty()) { 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/MediaScrub.kt b/app/src/main/kotlin/io/flamenet/messenger/app/MediaScrub.kt new file mode 100644 index 0000000..ab37457 --- /dev/null +++ b/app/src/main/kotlin/io/flamenet/messenger/app/MediaScrub.kt @@ -0,0 +1,116 @@ +package io.flamenet.messenger.app + +import android.media.MediaCodec +import android.media.MediaExtractor +import android.media.MediaFormat +import android.media.MediaMuxer +import java.io.File +import java.nio.ByteBuffer + +/** + * Strip everything from a video except the pictures and the sound. + * + * ๐Ÿ”ด **A clip from the camera roll says where it was shot.** Android writes the + * capture location into the MP4 container โ€” the `ยฉxyz` / `location` atom โ€” and + * the story path used to hand the picked file over byte for byte, so posting a + * video told every viewer the coordinates of wherever the person happened to + * be. Nothing on screen suggested it and no setting controlled it. Photos were + * already safe by accident, because [ImagePrep] re-encodes them through a + * bitmap and `Bitmap.compress` writes no EXIF; video had no equivalent step at + * all. + * + * โš ๏ธ **A remux, not a transcode.** The samples are copied across untouched, so + * there is no quality loss and no codec work โ€” the container is rebuilt and the + * metadata simply is not carried over. That is cheap enough to do on every + * post, which matters: a scrub people wait for is a scrub somebody eventually + * makes optional. + * + * โš ๏ธ Rotation IS carried over, deliberately. It says which way up the phone + * was, not where it was, and dropping it plays every portrait clip on its side. + */ +object MediaScrub { + + /** + * @param source the picked video's bytes. + * @param rotationDegrees from `METADATA_KEY_VIDEO_ROTATION`, re-applied to + * the rebuilt file. + * @return scrubbed bytes, or null if the file could not be rebuilt โ€” the + * caller must then refuse the post rather than fall back to the original. + */ + fun videoWithoutMetadata( + source: ByteArray, + rotationDegrees: Int, + cacheDir: File, + ): ByteArray? { + val input = File.createTempFile("scrub-in", ".mp4", cacheDir) + val output = File.createTempFile("scrub-out", ".mp4", cacheDir) + var extractor: MediaExtractor? = null + var muxer: MediaMuxer? = null + var started = false + return try { + input.writeBytes(source) + + extractor = MediaExtractor() + extractor.setDataSource(input.absolutePath) + muxer = MediaMuxer(output.absolutePath, MediaMuxer.OutputFormat.MUXER_OUTPUT_MPEG_4) + + // Audio and video only. Anything else in the file โ€” timed text, a + // sensor track, whatever a particular camera app decided to add โ€” + // is exactly the kind of passenger this exists to leave behind. + val trackMap = HashMap() + var bufferSize = DEFAULT_BUFFER + for (i in 0 until extractor.trackCount) { + val format = extractor.getTrackFormat(i) + val mime = format.getString(MediaFormat.KEY_MIME) ?: continue + if (!mime.startsWith("video/") && !mime.startsWith("audio/")) continue + if (format.containsKey(MediaFormat.KEY_MAX_INPUT_SIZE)) { + bufferSize = maxOf(bufferSize, format.getInteger(MediaFormat.KEY_MAX_INPUT_SIZE)) + } + extractor.selectTrack(i) + trackMap[i] = muxer.addTrack(format) + } + if (trackMap.isEmpty()) return null + + muxer.setOrientationHint(rotationDegrees) + muxer.start() + started = true + + val buffer = ByteBuffer.allocate(bufferSize) + val info = MediaCodec.BufferInfo() + while (true) { + val track = extractor.sampleTrackIndex + if (track < 0) break + val size = extractor.readSampleData(buffer, 0) + if (size < 0) break + val target = trackMap[track] + if (target != null) { + info.offset = 0 + info.size = size + info.presentationTimeUs = extractor.sampleTime + // โš ๏ธ Mapped rather than assigned across. The extractor's + // flags and the codec's are different vocabularies that + // happen to agree on the value of "sync sample" today. + info.flags = + if (extractor.sampleFlags and MediaExtractor.SAMPLE_FLAG_SYNC != 0) { + MediaCodec.BUFFER_FLAG_KEY_FRAME + } else { + 0 + } + muxer.writeSampleData(target, buffer, info) + } + extractor.advance() + } + output.readBytes() + } catch (failure: Exception) { + null + } finally { + if (started) runCatching { muxer?.stop() } + runCatching { muxer?.release() } + runCatching { extractor?.release() } + runCatching { input.delete() } + runCatching { output.delete() } + } + } + + private const val DEFAULT_BUFFER = 1 shl 20 +} diff --git a/app/src/main/kotlin/io/flamenet/messenger/app/MediaSettings.kt b/app/src/main/kotlin/io/flamenet/messenger/app/MediaSettings.kt index a4e0fe5..e1f066f 100644 --- a/app/src/main/kotlin/io/flamenet/messenger/app/MediaSettings.kt +++ b/app/src/main/kotlin/io/flamenet/messenger/app/MediaSettings.kt @@ -24,6 +24,31 @@ class MediaSettings(context: Context) { get() = prefs.getBoolean(FULL_QUALITY, false) set(value) = prefs.edit().putBoolean(FULL_QUALITY, value).apply() + /** + * How long a story posted from this phone stays visible, in hours. + * + * ๐Ÿ”ด **A day was the only answer the app had.** The lifetime was a + * constant, so the one thing people actually want to change about a story + * โ€” how long it lasts โ€” was the one thing they could not. + * + * โš ๏ธ The choice is the SENDER's and rides inside the sealed envelope + * (`exp`, ยง6.6), so it does not ask the relay to keep a promise: every copy + * carries the expiry its poster chose. + * + * โš ๏ธ Clamped to the offered set on read. A value from a future build, or a + * corrupted preference, must not post a story that expires before anybody + * sees it โ€” which is what a raw 0 would do. + */ + var storyLifetimeHours: Int + get() { + val stored = prefs.getInt(STORY_LIFETIME_HOURS, STORY_LIFETIME_DEFAULT) + return if (stored in STORY_LIFETIME_CHOICES) stored else STORY_LIFETIME_DEFAULT + } + set(value) = prefs.edit().putInt(STORY_LIFETIME_HOURS, value).apply() + + /** Seconds, as [io.flamenet.client.Conversations.postStory] wants it. */ + val storyLifetimeSeconds: Double get() = storyLifetimeHours * 60.0 * 60.0 + /** * Refuse to send unless the relay can prove the recipient's keys appear in * the log it publishes. @@ -61,6 +86,10 @@ class MediaSettings(context: Context) { private const val PREFS = "media-settings" private const val REQUIRE_KEY_LOG = "requireKeyLog" private const val FULL_QUALITY = "full_quality" + private const val STORY_LIFETIME_HOURS = "stories.lifetimeHours" + const val STORY_LIFETIME_DEFAULT = 24 + /** โš ๏ธ Same set as iOS, so the two apps offer the same thing. */ + val STORY_LIFETIME_CHOICES = listOf(1, 6, 24, 72, 168) private const val LINK_PREVIEWS = "link_previews" } } 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..620f1a1 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.") @@ -490,10 +506,26 @@ class MessengerViewModel(app: Application) : AndroidViewModel(app) { } val vault = vault ?: error("the vault is not open") - val device = identity ?: DeviceIdentity.create().also { - vault.saveIdentity(it) - identity = it - } + // ๐Ÿ”ด **The stored identity FIRST, and this line is why signing out and + // back in used to cost every contact a prompt.** Sign-in only ever + // consulted the in-memory field, and sign-out clears it โ€” so it minted + // a fresh identity and overwrote the vault's, making this phone a brand + // new device on every sign-in. Contacts saw a safety-number change, + // sends were refused until they accepted it, and the relay's public key + // log had not yet caught up with the key being offered, so even after + // accepting, the next send failed on "the keys offered by this contact + // are not in the public log". + // + // โš ๏ธ It was harmless right up until it wasn't: sign-out used to destroy + // the vault, so there was never a stored identity to find and minting + // one was correct. Keeping the device across a plain sign-out is what + // turned this into the bug it always looked like. + val device = identity + ?: vault.loadIdentity()?.also { identity = it } + ?: DeviceIdentity.create().also { + vault.saveIdentity(it) + identity = it + } val relay = RelayClient(config.baseUrl, username = account.username, appPassword = credential) relay.useRelayAuth(device) @@ -1376,7 +1408,7 @@ class MessengerViewModel(app: Application) : AndroidViewModel(app) { fun deleteAccount() = viewModelScope.launch { val current = session ?: return@launch val done = withContext(Dispatchers.IO) { runCatching { current.api.deleteAccount() } } - done.onSuccess { signOut() } + done.onSuccess { signOut(erasingDevice = true) } .onFailure { failure -> update { it.copy(error = describe(failure)) } } } @@ -1947,7 +1979,9 @@ class MessengerViewModel(app: Application) : AndroidViewModel(app) { val done = withContext(Dispatchers.IO) { runCatching { current.postStory(bytes, mime, width, height, caption, recipients, - duration, streamUrl) + duration, streamUrl, + lifetimeSeconds = MediaSettings(getApplication()) + .storyLifetimeSeconds) } } done.onSuccess { posted -> @@ -2438,13 +2472,45 @@ class MessengerViewModel(app: Application) : AndroidViewModel(app) { + "new device.") } - fun signOut() = viewModelScope.launch { + /** + * End the session, and optionally erase this phone with it. + * + * ๐Ÿ”ด **Signing out used to cost every one of your contacts a prompt.** It + * destroyed the vault unconditionally, so signing back in was a genuinely + * NEW device: every contact had the old key pinned, saw a safety-number + * change, and could not message you until they accepted it. For someone + * stepping away for an afternoon that is a strange price to pay, and it is + * why signing out and back in kept looking like the app breaking. + * + * Two doors now. Plain sign-out ends the session and keeps the device, so + * coming back is silent. Erasing is for handing the phone on, and for + * deleting the account, and the settings screen says which is which. + * + * โš ๏ธ Keeping the keys does not leave the mailbox reachable: a relay token + * is minted against the account credential, which this clears either way. + */ + fun signOut(erasingDevice: Boolean = false) = viewModelScope.launch { polling?.cancel() polling = null + val current = session + val myDevice = identity?.deviceId session = null - withContext(Dispatchers.IO) { KeystoreVault.destroy(getApplication()) } - identity = null - vault = KeystoreVault.open(getApplication()) + if (erasingDevice) { + withContext(Dispatchers.IO) { + // โš ๏ธ Tell the relay BEFORE destroying the keys that authorise + // saying it. Otherwise the account goes on publishing a device + // nobody will ever read again, and fan-out is per device โ€” so + // contacts keep addressing copies to a dead mailbox while the + // relay reports every one of them delivered. Best effort: a bad + // network must not leave the phone half-erased. + if (myDevice != null) runCatching { current?.revokeDevice(myDevice) } + KeystoreVault.destroy(getApplication()) + } + identity = null + vault = KeystoreVault.open(getApplication()) + } else { + identity = null + } _phase.value = Phase.SignedOut() } @@ -2460,7 +2526,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 +2547,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..4374c31 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) @@ -749,6 +762,13 @@ class Session( if (gid != null) { val known = groupRoster(gid) if (known == null || message.fromUser !in known) { + // โš ๏ธ Refusing is correct โ€” but refusing in silence is what + // made a sealed group look like a group nobody could talk + // in. Say which group and which sender, never the message. + System.err.println( + "fnmsg-recv: NOT FILED into group ${gid.take(8)} โ€” " + + "sender ${message.fromUser} is not in the roster " + + "(roster ${known?.size ?: -1} member(s))") continue } // โš ๏ธ Every group envelope carries the group's name, and this is @@ -886,8 +906,16 @@ class Session( duration: Double = 0.0, /** A live stream this story embeds (ยง6.13); [bytes] is then the poster. */ streamUrl: String? = null, + /** + * โš ๏ธ Passed explicitly rather than left to the client's default. The + * default is 24 hours and the setting exists precisely so that is not + * the only answer; a call site that omits it silently ignores what the + * person chose. + */ + lifetimeSeconds: Double, ): io.flamenet.client.Conversations.StoryPosted { val posted = conversations.postStory(bytes, mime, width, height, caption, toUserIds, + lifetimeSeconds = lifetimeSeconds, duration = duration, streamUrl = streamUrl) if (posted.reached > 0) { val now = System.currentTimeMillis() / 1000.0 @@ -1231,6 +1259,16 @@ class Session( @Volatile var groupNamesUpdated: Boolean = false private fun groupRoster(gid: String): Set? { + // ๐Ÿ”ด **A SEALED group's roster is not the relay's to give.** ยง8.9b + // exists so the relay holds opaque tags and no membership at all, so + // `listGroups()` returns an EMPTY member list for one โ€” and the caller + // reads that as "the sender is not a member" and drops the message. + // Every message in every sealed group was discarded on arrival, in + // silence, by the check meant to protect it. The sealed roster is the + // authoritative list here and the only one that exists. + if (vault.groupSecrets(gid) != null) { + return sealedGroups().firstOrNull { it.id == gid }?.memberIds?.toSet() + } val now = System.currentTimeMillis() val cached = groupRosterCache if (cached != null && now - cached.first < 30_000) return cached.second[gid] diff --git a/app/src/main/kotlin/io/flamenet/messenger/app/StoryMedia.kt b/app/src/main/kotlin/io/flamenet/messenger/app/StoryMedia.kt index a13b8aa..be3fb99 100644 --- a/app/src/main/kotlin/io/flamenet/messenger/app/StoryMedia.kt +++ b/app/src/main/kotlin/io/flamenet/messenger/app/StoryMedia.kt @@ -28,14 +28,23 @@ import java.io.File /** * Staging a picked story, image or video, from either entry point. * - * Videos are validated, not transcoded: this platform has no built-in - * exporter the way AVFoundation is one, and pulling in a transcoding stack - * is a decision for later. So the caps are enforced honestly โ€” a clip too - * long or too heavy is refused with the reason, and a clip that fits goes - * out exactly as picked. + * Videos are validated and scrubbed, not transcoded: this platform has no + * built-in exporter the way AVFoundation is one, and pulling in a transcoding + * stack is a decision for later. So the caps are enforced honestly โ€” a clip too + * long or too heavy is refused with the reason โ€” and a clip that fits is + * remuxed to drop the container metadata before it goes out. See [MediaScrub]; + * "exactly as picked" used to include the capture location. */ object StoryMedia { + /** Duration in seconds, display dimensions, and the container rotation. */ + private data class Meta( + val seconds: Double, + val width: Int, + val height: Int, + val rotation: Int, + ) + /** A story is a moment, not a film โ€” same cap as iOS. */ const val MAX_DURATION_S = 30 /** Under the relay's 10 MB attachment cap, with room for the seal. */ @@ -66,27 +75,40 @@ object StoryMedia { ?.toIntOrNull() ?: 0 // A portrait phone clip is stored landscape plus a rotation. if (rotation == 90 || rotation == 270) { val t = w; w = h; h = t } - Triple(durationMs / 1000.0, w, h) + // โš ๏ธ The rotation is carried out of here now, not just used to + // swap the dimensions: rebuilding the container to strip its + // metadata drops the orientation with everything else, and a + // portrait story would play on its side. + Meta(durationMs / 1000.0, w, h, rotation) }.getOrNull().also { runCatching { retriever.release() } } if (meta == null) { model.reportStoryProblem("That video couldn't be read.") return } - val (seconds, w, h) = meta + val (seconds, w, h, rotation) = meta if (seconds > MAX_DURATION_S + 0.5) { model.reportStoryProblem( "Stories are capped at $MAX_DURATION_S seconds โ€” this one is " + "${seconds.toInt()}. Trim it and try again.") return } - val bytes = runCatching { + val picked = runCatching { context.contentResolver.openInputStream(uri)?.use { it.readBytes() } }.getOrNull() - if (bytes == null) { + if (picked == null) { model.reportStoryProblem("That video couldn't be read.") return } + // ๐Ÿ”ด **Where it was shot does not go with it.** See [MediaScrub]. + // A failure here refuses the post rather than falling back to the + // original: "we could not remove the location, so we sent it + // anyway" is the one outcome this must never produce. + val bytes = MediaScrub.videoWithoutMetadata(picked, rotation, context.cacheDir) + if (bytes == null) { + model.reportStoryProblem("That video couldn't be prepared for sending.") + return + } if (bytes.size > MAX_BYTES) { model.reportStoryProblem( "That video is too large to send โ€” the cap is about 9 MB. " @@ -97,16 +119,25 @@ object StoryMedia { return } - val bytes = runCatching { + val picked = runCatching { context.contentResolver.openInputStream(uri)?.use { it.readBytes() } }.getOrNull() - if (bytes == null) { + if (picked == null) { model.reportStoryProblem("That file couldn't be read.") return } - val bounds = android.graphics.BitmapFactory.Options().apply { inJustDecodeBounds = true } - android.graphics.BitmapFactory.decodeByteArray(bytes, 0, bytes.size, bounds) - model.stageStory(bytes, mime, bounds.outWidth, bounds.outHeight) + // ๐Ÿ”ด **Prepared, not posted as picked.** A story photo was the one + // image path in the app that never went through [ImagePrep], so it was + // uploaded with its EXIF block intact โ€” including, for anything shot on + // the phone, the GPS coordinates. Chat photos have always been + // re-encoded and were safe by consequence; a story, which is shown to + // every contact at once, was not. + val prepared = ImagePrep.prepare(picked, MediaSettings(context).fullQuality) + if (prepared == null) { + model.reportStoryProblem("That picture couldn't be read.") + return + } + model.stageStory(prepared.bytes, prepared.mime, prepared.width, prepared.height) } /** The first frame, for the composer's preview of a staged video. */ diff --git a/client/src/main/kotlin/io/flamenet/client/Conversations.kt b/client/src/main/kotlin/io/flamenet/client/Conversations.kt index ae9bf85..1b5ee14 100644 --- a/client/src/main/kotlin/io/flamenet/client/Conversations.kt +++ b/client/src/main/kotlin/io/flamenet/client/Conversations.kt @@ -501,7 +501,11 @@ class Conversations( * request happens to verify is the mistake this whole section exists to * avoid. */ - private fun guardIdentity(toUserId: Long, bundles: List) { + private fun guardIdentity( + toUserId: Long, + bundles: List, + servedFnid: String? = null, + ) { val theirs = bundles.filter { it.deviceId != identity.deviceId } if (theirs.isEmpty()) return @@ -513,7 +517,11 @@ class Conversations( ) { throw IdentityChangedException(toUserId) } - theirs.forEach { trust.enforceKeyLogIfServed(it, toUserId) } + // The address just served wins over the pinned one: a device that + // registered a moment ago is logged under whatever the account claims + // NOW, and the pin may predate the claim. + val fnid = servedFnid ?: pinned + theirs.forEach { trust.enforceKeyLogIfServed(it, toUserId, fnid) } } /** @@ -524,6 +532,28 @@ class Conversations( fun sendPlaintextForGroupInvite(toUserId: Long, text: String): Sent = sendPlaintext(toUserId, text) + /** + * Whether an envelope is worth interrupting somebody for. + * + * ๐Ÿ”ด **Everything this app sends was.** The relay holds ciphertext and + * cannot tell a call answer from a birthday message, so it announced every + * envelope as "New message" โ€” and call answers, call ends, delivery keys, + * key-log gossip, reactions and avatars are all envelopes. One call + * produced a burst of banners on the far end for things nobody sent. + * + * โš ๏ธ A plain string is a message: text from an older build carries no + * envelope at all, and the safe reading of something unrecognised is that a + * person meant to say it. + */ + private fun warrantsAlert(body: String): Boolean { + if (!body.startsWith("""{"fnc":""")) return true + val content = Content.parse(body) + // Only the OFFER announces itself. An answer and a hang-up are the far + // end's half of a conversation already on screen. + content.call?.let { return it.action == Content.CallSignal.OFFER } + return content.isRenderable + } + private fun sendPlaintext(toUserId: Long, text: String): Sent { // ๐Ÿ”ด **The DIRECTORY, not the key route.** `/e2e/keys` pops a one-time // prekey per device and is metered per contact pair โ€” calling it on @@ -567,13 +597,34 @@ class Conversations( val needsEstablish = stubs.any { it.deviceId != identity.deviceId && existingSession(it.deviceId) == null } + // โš ๏ธ The address comes back with the bundles and is REMEMBERED. The + // directory route serves no fnid, so a later send that needs no bundle + // would otherwise be unable to check the log again. + var servedFnid: String? = null val bundles = if (!needsEstablish) stubs else { - val full = relay.fetchKeys(toUserId).associateBy { it.deviceId } + val keys = relay.fetchKeys(toUserId) + servedFnid = keys.fnid + keys.fnid?.let { trust.rememberFnid(toUserId, it) } + val full = keys.bundles.associateBy { it.deviceId } // Union: every device the directory knows, carrying a full bundle // 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, servedFnid) + } 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 +645,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 +679,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,10 +702,23 @@ 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, + silent = !warrantsAlert(text), // โš ๏ธ The key only when there is no capability. Sending both // would let the relay accept on the key when the capability // had been revoked, so it would never learn it was refused @@ -651,7 +726,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 +1164,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 +1258,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. // @@ -1188,7 +1291,7 @@ class Conversations( /** How many prekeys we have left with the relay, and a top-up when it is low. */ fun replenishPreKeysIfLow(threshold: Int = 5, batch: Int = 20): Int { - val mine = relay.fetchKeys(myUserId ?: return 0) + val mine = relay.fetchKeys(myUserId ?: return 0).bundles .firstOrNull { it.deviceId == identity.deviceId } ?: return 0 // The relay pops one per fetch, so seeing none here means the pool was // empty before this call, not that this call drained it. 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/client/src/main/kotlin/io/flamenet/client/RelayClient.kt b/client/src/main/kotlin/io/flamenet/client/RelayClient.kt index 5b6c85f..61e7ca4 100644 --- a/client/src/main/kotlin/io/flamenet/client/RelayClient.kt +++ b/client/src/main/kotlin/io/flamenet/client/RelayClient.kt @@ -138,10 +138,31 @@ class RelayClient( * starting a session. Polling it drains the pool and pushes every peer onto * the last-resort key, which costs initial forward secrecy for everyone. */ - fun fetchKeys(userId: Long): List { + /** A peer's bundles, and the address their key-log entries are filed under. */ + data class Keys(val fnid: String?, val bundles: List) + + /** + * ๐Ÿ”ด **The `fnid` is not decoration; without it the key log cannot be + * checked.** The relay files a leaf under `fnid:
` once an account + * has claimed one, and under `local:/` before that. A + * verifier that does not know the address computes the other subject, finds + * no matching leaf, and refuses to send โ€” reporting "has published keys, + * but none match what this relay served" for a conversation in which + * nothing is wrong. + * + * This response has always carried it and this method always dropped it, so + * every newly registered device on an account with an address was + * unverifiable and therefore unsendable-to. It presents as messaging + * breaking the moment somebody reinstalls. + */ + fun fetchKeys(userId: Long): Keys { val body = getJson("/e2e/keys/$userId") val devices = body.optJSONArray("devices") ?: JSONArray() - return (0 until devices.length()).map { PeerBundle.fromWire(devices.getJSONObject(it)) } + return Keys( + fnid = body.optString("fnid").ifEmpty { null }, + bundles = (0 until devices.length()) + .map { PeerBundle.fromWire(devices.getJSONObject(it)) }, + ) } /** `GET /e2e/devices/{user}` โ€” who a user is, costing no prekey. */ @@ -196,6 +217,19 @@ class RelayClient( * revocation is the whole lever ยง21 adds. */ deliveryToken: String? = null, + /** + * Deliver without announcing: protocol traffic, not a message. + * + * ๐Ÿ”ด **The relay announced everything, because it cannot tell.** It + * holds ciphertext, so a call answer, a delivery key and a birthday + * message are the same bytes to it โ€” and every one of them raised + * "New message" on the far end. One call produced a burst of banners + * for things nobody sent. The sender is the only party that knows. + * + * โš ๏ธ Advisory and unauthenticated. A sender that lies suppresses its + * own notifications and nobody else's. + */ + silent: Boolean = false, ): JSONObject = postJson("/e2e/messages", JSONObject() .put("to", toUser) @@ -203,6 +237,7 @@ class RelayClient( if (deliveryToken != null) body.put("delivery_token", deliveryToken) else if (deliveryKey != null) body.put("delivery_key", deliveryKey) else body.put("from_device", fromDevice) + if (silent) body.put("silent", true) } .put("messages", JSONArray().also { array -> envelopes.forEach { diff --git a/client/src/main/kotlin/io/flamenet/client/Trust.kt b/client/src/main/kotlin/io/flamenet/client/Trust.kt index 506bb48..3d48183 100644 --- a/client/src/main/kotlin/io/flamenet/client/Trust.kt +++ b/client/src/main/kotlin/io/flamenet/client/Trust.kt @@ -247,6 +247,21 @@ class Trust( /** The address this contact was pinned under, if any. */ fun pinnedFnid(userId: Long): String? = vault.loadTrust(userId)?.fnid + /** + * Record the address the relay served for this contact. + * + * โš ๏ธ Not a trust decision โ€” it changes nothing about which identity keys + * are accepted. It only records the SUBJECT their key-log leaves are filed + * under, which a verifier needs and the device directory does not serve. + * Storing it means a later send that establishes no new session can still + * check the log. + */ + fun rememberFnid(userId: Long, fnid: String) { + val existing = vault.loadTrust(userId) ?: return + if (existing.fnid == fnid) return + vault.saveTrust(userId, existing.copy(fnid = fnid)) + } + /** Trust as it stands, for UI that has not triggered a fetch. */ fun state(userId: Long): IdentityTrust { val t = vault.loadTrust(userId) ?: return IdentityTrust.UNKNOWN @@ -289,8 +304,25 @@ class Trust( * the tick in place would show "verified" for something nobody verified. */ fun acceptIdentityChange(userId: Long, handle: String? = null) { - val offered = relay.fetchKeys(userId).map { it.identitySigning } - if (offered.isEmpty()) return + // โš ๏ธ The address is taken from the SAME response that carries the keys. + // Accepting a change and then filing it under a stale address leaves + // the key log uncheckable for the very keys just accepted. + val served = relay.fetchKeys(userId) + val offered = served.bundles.map { it.identitySigning } + // ๐Ÿ”ด **An empty answer is a FAILURE, not an acceptance of nothing.** + // This returned quietly, so a fetch that was rate-limited, timed out or + // 500'd left the pin exactly as it was โ€” while the UI reported the + // change accepted. The person taps "accept", nothing happens, every + // message stays refused, and the app insists the problem is on the + // other end. Observed after a burst of retries tripped the per-contact + // key-fetch limit, which is precisely when somebody is trying to + // recover a conversation and most needs this to work. + if (offered.isEmpty()) { + throw RelayException.Malformed( + "the server returned no keys for this contact, so there was " + + "nothing to accept โ€” try again in a moment") + } + served.fnid?.let { rememberFnid(userId, it) } // Re-resolve rather than keep the old address: if what changed WAS the // address, keeping it would leave every later check recomputing against // a name this account no longer publishes under, and honest keys would diff --git a/client/src/test/kotlin/io/flamenet/client/LiveKeyLogTest.kt b/client/src/test/kotlin/io/flamenet/client/LiveKeyLogTest.kt index 03dd1b6..9666227 100644 --- a/client/src/test/kotlin/io/flamenet/client/LiveKeyLogTest.kt +++ b/client/src/test/kotlin/io/flamenet/client/LiveKeyLogTest.kt @@ -85,7 +85,7 @@ class LiveKeyLogTest { // was served. Bob genuinely published this key a moment ago, so the // relay can prove it โ€” and if this throws, enforcement is refusing // honest keys, which is worse than not enforcing at all. - val bundle = alice.relay.fetchKeys(bob.userId).single { it.deviceId == bob.identity.deviceId } + val bundle = alice.relay.fetchKeys(bob.userId).bundles.single { it.deviceId == bob.identity.deviceId } val trust = alice.trust() trust.enforceKeyLog(bundle, bob.userId) @@ -109,7 +109,7 @@ class LiveKeyLogTest { val alice = Actor(url, "c") val bob = Actor(url, "d") - val real = alice.relay.fetchKeys(bob.userId).single { it.deviceId == bob.identity.deviceId } + val real = alice.relay.fetchKeys(bob.userId).bundles.single { it.deviceId == bob.identity.deviceId } val substituted = io.flamenet.e2e.PeerBundle( deviceId = real.deviceId, identityDH = real.identityDH, @@ -141,7 +141,7 @@ class LiveKeyLogTest { val bob = Actor(url, "f") val trust = alice.trust() - val first = alice.relay.fetchKeys(bob.userId).single { it.deviceId == bob.identity.deviceId } + val first = alice.relay.fetchKeys(bob.userId).bundles.single { it.deviceId == bob.identity.deviceId } trust.enforceKeyLog(first, bob.userId) val pinned = alice.vault.loadKeyLogPin(alice.relay.host)!! @@ -149,7 +149,7 @@ class LiveKeyLogTest { // second call must produce and verify a real consistency proof from the // pinned size to the new one. val second = Actor(url, "g") - val third = alice.relay.fetchKeys(second.userId) + val third = alice.relay.fetchKeys(second.userId).bundles .single { it.deviceId == second.identity.deviceId } trust.enforceKeyLog(third, second.userId) @@ -175,7 +175,7 @@ class LiveKeyLogTest { relay = alice.relay.host, pin = Vault.KeyLogPin(head.getInt("size"), Wire.b64(ByteArray(32) { 0x5A })) ) - val bundle = alice.relay.fetchKeys(bob.userId).single { it.deviceId == bob.identity.deviceId } + val bundle = alice.relay.fetchKeys(bob.userId).bundles.single { it.deviceId == bob.identity.deviceId } assertFailsWith { alice.trust().enforceKeyLog(bundle, bob.userId) } @@ -218,7 +218,7 @@ class LiveKeyLogTest { .also { it.useRelayAuth(second) } .registerDevice(second, second.generateOneTimePreKeys(3)) - val bundle = alice.relay.fetchKeys(bob.userId).single { it.deviceId == second.deviceId } + val bundle = alice.relay.fetchKeys(bob.userId).bundles.single { it.deviceId == second.deviceId } val trust = alice.trust() // 1. Nothing pinned: the honest key is refused, and says why โ€” there @@ -254,7 +254,7 @@ class LiveKeyLogTest { val bob = Actor(url, "o") val trust = alice.trust() - val bundle = alice.relay.fetchKeys(bob.userId).single { it.deviceId == bob.identity.deviceId } + val bundle = alice.relay.fetchKeys(bob.userId).bundles.single { it.deviceId == bob.identity.deviceId } // Pin Bob under an address, with his real keys. alice.vault.saveTrust( diff --git a/client/src/test/kotlin/io/flamenet/client/LiveSealedSenderTest.kt b/client/src/test/kotlin/io/flamenet/client/LiveSealedSenderTest.kt index 88eceae..fe4cedb 100644 --- a/client/src/test/kotlin/io/flamenet/client/LiveSealedSenderTest.kt +++ b/client/src/test/kotlin/io/flamenet/client/LiveSealedSenderTest.kt @@ -145,7 +145,7 @@ class LiveSealedSenderTest { // A submission presenting the OLD key must be refused outright. Sent // through the client rather than by hand, so this exercises the same // path a real sealed send takes. - val bundle = alice.relay.fetchKeys(bob.userId).first { it.deviceId == bob.identity.deviceId } + val bundle = alice.relay.fetchKeys(bob.userId).bundles.first { it.deviceId == bob.identity.deviceId } val envelope = io.flamenet.e2e.E2ESession.initiate(alice.identity, bundle).encrypt("stale") val blob = SealedSender.seal( envelope, alice.userId, alice.identity.deviceId, bundle.deviceId, bundle.identityDH) diff --git a/client/src/test/kotlin/io/flamenet/client/LiveShapesTest.kt b/client/src/test/kotlin/io/flamenet/client/LiveShapesTest.kt index 75bde53..7d32a2d 100644 --- a/client/src/test/kotlin/io/flamenet/client/LiveShapesTest.kt +++ b/client/src/test/kotlin/io/flamenet/client/LiveShapesTest.kt @@ -381,7 +381,7 @@ class LiveShapesTest { // Reachable means a stranger can fetch a bundle WITH a one-time prekey. // A device published without them can send and can never be started // with, which looks identical from the account's own side. - val bundles = relay.fetchKeys(account.userId) + val bundles = relay.fetchKeys(account.userId).bundles val mine = bundles.firstOrNull { it.deviceId == identity.deviceId } assertTrue(mine != null, "$base served no bundle for the device just registered") assertTrue(mine!!.oneTimePreKey != null, "$base served no one-time prekey") diff --git a/client/src/test/kotlin/io/flamenet/client/LiveSignUpTest.kt b/client/src/test/kotlin/io/flamenet/client/LiveSignUpTest.kt index cfd13a9..f0715a3 100644 --- a/client/src/test/kotlin/io/flamenet/client/LiveSignUpTest.kt +++ b/client/src/test/kotlin/io/flamenet/client/LiveSignUpTest.kt @@ -59,7 +59,7 @@ class LiveSignUpTest { // The device is only really registered if a stranger can fetch a bundle // for it โ€” that is what makes this phone reachable. - val bundles = client.fetchKeys(account.userId) + val bundles = client.fetchKeys(account.userId).bundles assertTrue(bundles.any { it.deviceId == identity.deviceId }, "the relay did not serve a bundle for the device we just registered") assertTrue(bundles.first { it.deviceId == identity.deviceId }.oneTimePreKey != null, diff --git a/client/src/test/kotlin/io/flamenet/client/RelayClientTest.kt b/client/src/test/kotlin/io/flamenet/client/RelayClientTest.kt index 0ed399c..0d3b5bd 100644 --- a/client/src/test/kotlin/io/flamenet/client/RelayClientTest.kt +++ b/client/src/test/kotlin/io/flamenet/client/RelayClientTest.kt @@ -293,7 +293,11 @@ class RelayClientTest { .put("opk", JSONObject().put("id", oneTime.id).put("pub", Wire.b64(oneTime.publicKey))) server.enqueue(json("""{"devices":[$wire]}""")) - val bundles = client().fetchKeys(42) + // โš ๏ธ `fetchKeys` answers with the fnid alongside the bundles now โ€” the + // address is what the key log is checked against, and returning only + // the bundles threw it away at the one point it is available. + val fetched = client().fetchKeys(42) + val bundles = fetched.bundles assertEquals(1, bundles.size) assertEquals(peer.deviceId, bundles[0].deviceId) assertEquals(oneTime.id, bundles[0].oneTimePreKeyId) 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") diff --git a/interop/src/main/kotlin/io/flamenet/interop/Main.kt b/interop/src/main/kotlin/io/flamenet/interop/Main.kt index 6a246ff..8a7c88d 100644 --- a/interop/src/main/kotlin/io/flamenet/interop/Main.kt +++ b/interop/src/main/kotlin/io/flamenet/interop/Main.kt @@ -107,7 +107,7 @@ private fun handle(req: JSONObject): JSONObject = when (req.getString("cmd")) { "encrypt" -> encrypt(req) "decrypt" -> decrypt(req) // --- the session layer, which is what an app actually uses ------------ - "session_device" -> sessionDevice() + "session_device" -> sessionDevice(req) "session_initiate" -> sessionInitiate(req) "session_respond" -> sessionRespond(req) "session_encrypt" -> sessionEncrypt(req) @@ -149,6 +149,14 @@ private fun newDevice(req: JSONObject): JSONObject { // v3, the legacy paths would stop being covered on the day v3 landed โ€” // which is the day they matter most. val postQuantum = req.optBoolean("pq", true) + // ๐Ÿ”ด **A peer can be asked to advertise an OLDER version, and it has to be + // possible to ask.** The standing rule is that the current version and the + // one before it interoperate โ€” so when v4 lands, v3 and v4 must talk, and + // the only way to keep that honest is a harness that can stand up a peer + // on the previous version and hold a real conversation with it. Without + // this the suite can only ever test the newest code against itself, which + // is the shape of every compatibility bug that has ever shipped. + val proto = req.optInt("proto", if (postQuantum) 3 else 1) val id = next("kotlin-device-") val device = Device( deviceId = id, @@ -183,9 +191,13 @@ private fun newDevice(req: JSONObject): JSONObject { // Without `caps_sig` the web engine's `bundleFromWire` discards the // whole v2 block and `negotiate` returns v1, which is exactly what this // harness was quietly doing before. + // โš ๏ธ Signed over the version actually claimed. A signature over a + // different number is exactly what `provenVersion` exists to reject, + // and a harness that got this wrong would "prove" compatibility by + // silently falling back to v1 on every run. .putOpt("caps_sig", if (postQuantum) b64(Ed25519.sign( - device.ikSig, CAPS_DOMAIN + byteArrayOf(3.toByte()))) else null) - .put("proto", if (postQuantum) 3 else 1) + device.ikSig, CAPS_DOMAIN + byteArrayOf(proto.toByte()))) else null) + .put("proto", proto) return JSONObject().put("device", id).put("bundle", bundle) } @@ -338,12 +350,29 @@ private val identities = HashMap() private val liveSessions = HashMap() /** A device and the bundle wire a relay would serve for it, one prekey included. */ -private fun sessionDevice(): JSONObject { +private fun sessionDevice(req: JSONObject): JSONObject { val identity = DeviceIdentity.create() identities[identity.deviceId] = identity val oneTime = identity.generateOneTimePreKeys(1).first() val wire = identity.registrationWire() .put("opk", JSONObject().put("id", oneTime.id).put("pub", b64(oneTime.publicKey))) + // ๐Ÿ”ด **Stand up a peer on an OLDER version, for the compatibility rule.** + // The engine publishes whatever it currently implements, which is right for + // a device and useless for testing the release before it โ€” and the standing + // rule is that the current version and the one before it interoperate. + // + // โš ๏ธ The capability signature is REMADE over the number actually claimed. + // Leaving the original in place would advertise one version and sign + // another, which `provenVersion` rejects by falling back to v1 โ€” so the + // test would quietly pass while exercising a pairing nobody ships. + // Overriding lives here rather than in `registrationWire` on purpose: the + // engine should publish what it implements, not what a test wants. + if (req.has("proto")) { + val proto = req.getInt("proto") + wire.put("proto", proto) + wire.put("caps_sig", b64(Ed25519.sign( + identity.identitySigning, CAPS_DOMAIN + byteArrayOf(proto.toByte())))) + } return JSONObject().put("device", identity.deviceId).put("bundle", wire) } @@ -359,6 +388,7 @@ private fun sessionInitiate(req: JSONObject): JSONObject { // layer where the one bug this protocol has actually shipped lives: a // client and a relay disagreeing about a field name, caught by nothing // until a live round trip. + .put("version", session.version) .put("envelope", session.encrypt(req.getString("text")).toJson()) } @@ -371,6 +401,12 @@ private fun sessionRespond(req: JSONObject): JSONObject { return JSONObject() .put("session", id) .put("ad", b64(session.associatedData)) + // ๐Ÿ”ด **What the session ANSWERED with, not what the harness assumes.** + // Without this the interop test could not see the version a responder + // settled on, and a responder that capped itself below the initiator + // shipped: iOS ran the session as v3, this side as v2, and every + // message sent into it was refused by the peer as a downgrade. + .put("version", session.version) .put("text", session.decrypt(envelope)) } @@ -407,9 +443,12 @@ private fun openSealed(req: JSONObject): JSONObject { .put("envelope", opened.envelope.toJson()) } -private fun sessionEncrypt(req: JSONObject): JSONObject = JSONObject() - .put("envelope", liveSessions.getValue(req.getString("session")) - .encrypt(req.getString("text")).toJson()) +private fun sessionEncrypt(req: JSONObject): JSONObject = + liveSessions.getValue(req.getString("session")).let { session -> + JSONObject() + .put("version", session.version) + .put("envelope", session.encrypt(req.getString("text")).toJson()) + } private fun sessionDecrypt(req: JSONObject): JSONObject = JSONObject() .put("text", liveSessions.getValue(req.getString("session")) @@ -426,8 +465,19 @@ private fun sessionDecrypt(req: JSONObject): JSONObject = JSONObject() private fun sessionReload(req: JSONObject): JSONObject { val id = req.getString("session") val archived = liveSessions.getValue(id).archive() - liveSessions[id] = E2ESession.fromArchive(JSONObject(archived.toString())) - return JSONObject().put("reloaded", true).put("bytes", archived.toString().length) + // ๐Ÿ”ด **`as_version` exists to reproduce a phone that stored a session + // BEFORE the responder fix, and it is the only way to test the upgrade.** + // The version is persisted, so a session written by the old build restores + // as v2 no matter how new the code reading it is โ€” updating the app does + // NOT repair a conversation already on disk. That population is every + // existing user, and nothing else in this harness can produce them. + val forced = JSONObject(archived.toString()) + if (req.has("as_version")) forced.put("version", req.getInt("as_version")) + liveSessions[id] = E2ESession.fromArchive(forced) + return JSONObject() + .put("reloaded", true) + .put("version", liveSessions.getValue(id).version) + .put("bytes", archived.toString().length) } private fun accountKdf(req: JSONObject): JSONObject {