diff --git a/PARITY.md b/PARITY.md index 8737a4c..17f455f 100644 --- a/PARITY.md +++ b/PARITY.md @@ -339,22 +339,49 @@ the messenger rather than staying separate apps. | Files engine: names, key wrapping, chunked content | ✅ pinned to the **browser's** published vectors, not to the Swift engine | | Files engine: KDF and identity record | ✅ an identity written by the browser unlocks here | | Local vault: add, organise, delete | ✅ index, folders, both storage modes, bulk delete with its report | -| Files screen: set up, unlock, browse, add, folders, delete, promote | ✅ ⚠️ **not yet run on a device** — see below | -| Bell–LaPadula labels and clearance | 🟡 lattice, encoding and clearance in Kotlin, pinned to independently-computed vectors; the ★-property is unenforced on **both** clients — android#15 | -| Share a file into a chat or group | ⚪ | +| Files screen: set up, unlock, browse, add, folders, delete, promote | ✅ **verified on a device** — set up, encrypt, lock, cold-start reload, link via SAF | +| The security lattice (labels, clearance) | ✅ encoding and clearance in Kotlin, pinned to independently-computed vectors. ⚠️ **Not Bell–LaPadula** — the ★-property is deliberately unenforced on both clients; see android#15 | +| Share a file into a chat or group | ✅ both directions — send from the vault, save a received one back at the label it arrived with | | Wallet engine: BIP-39, Base58, SLIP-0010 | ✅ pinned to the published vectors | | Wallet engine: address derivation and signing | ✅ BIP-44 path, Base58 address, and the engine's own RFC 8032 signs — no API-33 floor | | Wallet: BIP-39 seed, create and restore | ✅ create, import, restore, sealed at rest, with the phrase shown once and explained | | Wallet: fresh receive address per receive | ✅ counter in the vault; the screen says why the address changes | | Wallet: balance and transactions from a node | ✅ both, with **unread drawn as an em dash and never as zero** | -⚠️ **The Files screen compiles and every layer under it is tested, but it has -not been exercised on a device or emulator.** The machine was carrying an iOS -simulator at 99% CPU when it landed, and booting an emulator on top of that -produces a flaky run rather than evidence. Two things specifically need a real -device before this can be called done: the Storage Access Framework picker and -its persistable read grant, and whether a reopened app can still read a linked -file after a reboot. Neither can be faked on the JVM. +🔴 **The device run found a bug no JVM test could have.** Android's bundled +`org.json` coerces a JSON `null` to the four-character string `"null"`; the Maven +`org.json` the tests run against returns `""`. Every nullable field in the vault +index therefore passed on a laptop and broke on a phone: + +- `parentID` threw `Invalid UUID string: null`, so **a vault holding any + top-level file failed to load at all.** Adding the first file worked, because + it was still in memory; reopening the app lost everything, with an error + nobody could act on. +- `wrappedContentKey` was worse because it did NOT throw — `"null"` is valid + base64, so the node came back with a plausible three-byte content key and + simply refused to decrypt, for ever. +- `bookmark` produced a reference pointing at `"null"`, permanently missing. + +Absence is now decided by `JSONObject.isNull`, never by comparing a decoded +string to a sentinel, and the literal `"null"` is rejected too. Two regression +tests reproduce the device's behaviour on the JVM by writing the literal string +the platform hands over — feeding a real JSON `null` would pass with the broken +code and prove nothing. + +⚠️ **Worth a sweep.** `optString` on a nullable field is wrong everywhere in +this app, not only here, and it fails only on the platform it ships to. + +**What the device run covered** (emulator, API 35): set-up with the passphrase, +a wrong passphrase refused, the SAF `OpenDocument` picker with the display name +pre-filled, encrypting into the vault at a chosen classification, lock and +unlock, and a cold-start reload of the vault. On disk: the blob begins `ENCL\x01`, +both wrapped keys carry version `0x11` and the name `0x01`, the filename is not +in the clear, and the plaintext is absent from the entire app container. Linking +a file works and reports "Nothing was copied". + +⚠️ **Still owed: whether a persistable read grant survives a reboot.** The +emulator reboot ran at a machine load of 38 and the launcher itself ANR'd, which +is not a condition that produces evidence about anything. 🔴 **Android asks for a Files passphrase, exactly as iOS does, and this was a decision rather than a port.** The Android wallet sits behind the Keystore-sealed @@ -365,6 +392,22 @@ inner one for convenience would be a real reduction and not a refactor — a vau that opens because the phone is open is a folder with a lock drawn on it. +🔴 **Files is NOT a mandatory access control system, and nothing should say it +is.** The sharing path forced the question android#15 asked, and the answer was +already written on the wire format: *the label is a MARKING, not a permission.* +The content key travels in the same envelope as the file — by design, because +that is what makes sending work — so no client-side refusal is a control. There +is no trusted computing base: the person holds every key, on a device they +control, running a client they could modify. **A suggestion dressed as a control +is worse than no control**, because it is the one people plan around. + +What Files does provide, and what may be claimed: cryptographic read separation +(no label key, no plaintext — enforced by mathematics, not by a check), and a +classification that travels with a file and is preserved on the recipient's +phone. Bell–LaPadula's ★-property is deliberately absent on both clients, and +the test in `LabelTest` asserts that absence so it fails loudly if one platform +implements it alone. + 🔴 **The Files crypto is held to `vectors/files-v1.json`, which the JavaScript engine in the browser produced — a third implementation, not the Swift one.** Checking Kotlin against Swift would only establish that the two agree, and two 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 49275b0..81bcd05 100644 --- a/app/src/main/kotlin/io/flamenet/messenger/app/ChatScreens.kt +++ b/app/src/main/kotlin/io/flamenet/messenger/app/ChatScreens.kt @@ -1974,6 +1974,9 @@ fun ColumnScope.ChatScreen( // this screen that did not belong to that row. var showStickers by remember { mutableStateOf(false) } var showEmoji by remember { mutableStateOf(false) } + var showAttach by remember { mutableStateOf(false) } + /** The Files picker, opened from the attach panel. */ + var pickingFile by remember { mutableStateOf(false) } var micGranted by remember { mutableStateOf( @@ -2074,8 +2077,19 @@ fun ColumnScope.ChatScreen( verticalAlignment = Alignment.CenterVertically, horizontalArrangement = Arrangement.spacedBy(Space.sm), ) { - IconAction(icon = KitIcon.Attach, label = "Attach a photo", tag = "attach", - onClick = { pickImage.launch("image/*") }) + // 🔴 **A choice, not a shortcut to the photo picker.** This was labelled + // "Attach a photo" and launched the gallery — which is also exactly what + // the camera button beside it does, so the row had two controls doing + // one thing and no way at all to send a document. Opening a small panel + // keeps the kit's row at its four controls while giving the second kind + // of attachment somewhere to live. + IconAction(icon = KitIcon.Attach, + label = if (showAttach) "Close attachments" else "Attach", + tag = "attach", + onClick = { + showAttach = !showAttach + if (showAttach) { showEmoji = false; showStickers = false } + }) // ⚠️ `BasicTextField` inside our own pill rather than a Material text // field with its shape overridden: `OutlinedTextField` reserves room @@ -2186,6 +2200,25 @@ fun ColumnScope.ChatScreen( } } + if (showAttach) { + AttachPanel( + model = model, + onPhoto = { showAttach = false; pickImage.launch("image/*") }, + onFiles = { showAttach = false; pickingFile = true }, + ) + } + + if (pickingFile) { + PickFileToSendSheet( + model = model, + onCancel = { pickingFile = false }, + onPick = { entry -> + pickingFile = false + model.sendFileFromVault(entry, chat) + }, + ) + } + if (showStickers) { StickerPicker { sticker -> // A sticker IS the message. There is nothing to add it to and @@ -2408,7 +2441,10 @@ private fun Bubble( io.flamenet.e2e.Attachment.AUDIO -> VoiceBubble(ref, model) // A document, not a picture: AttachedImage would download // a PDF and draw nothing. - io.flamenet.e2e.Attachment.FILE -> FileBubble(ref, line.mine) + io.flamenet.e2e.Attachment.FILE -> FileBubble( + ref, line.mine, + onSave = if (line.mine) null else { { model.saveAttachmentToFiles(ref) } }, + ) else -> AttachedImage(ref, model) } } @@ -3783,7 +3819,11 @@ fun ColumnScope.MailboxesSection( * other half is built. */ @Composable -private fun FileBubble(ref: io.flamenet.e2e.Attachment.Ref, mine: Boolean) { +private fun FileBubble( + ref: io.flamenet.e2e.Attachment.Ref, + mine: Boolean, + onSave: (() -> Unit)? = null, +) { val onColour = if (mine) MaterialTheme.colorScheme.onPrimary else MaterialTheme.colorScheme.onSurface Row( @@ -3835,6 +3875,22 @@ private fun FileBubble(ref: io.flamenet.e2e.Attachment.Ref, mine: Boolean) { ) } } + // 🔴 **Offered on a received file only.** Saving something already + // in the vault back into it would make a second, unrelated copy at + // whatever label this bubble happens to carry — which is how one + // document quietly becomes two at different classifications. + if (onSave != null) { + Text( + "Save to Files", + fontSize = 12.sp, + fontWeight = FontWeight.Medium, + color = onColour, + modifier = Modifier + .clickable(onClick = onSave) + .padding(top = 2.dp) + .testTag("chat.file.save"), + ) + } } } } @@ -3844,14 +3900,19 @@ private fun FileBubble(ref: io.flamenet.e2e.Attachment.Ref, mine: Boolean) { * ask on either side any more. An ordinal outside the set is shown as itself * rather than guessed at — inventing "Secret" for a level this deployment calls * something else would be worse than a number. + * + * 🔴 **One vocabulary, not two.** This was a second `when` listing the same + * four names — agreeing with [FileLevels] by coincidence, with nothing making + * them agree. Two places to change is how one of them gets changed: rename a + * level for the Files screen and the chat bubble keeps showing the old word for + * the same number, on the same phone, in the same conversation. + * + * ⚠️ It matters more than a rename. The ORDINAL is what travels on the wire, + * so the name is the only thing telling somebody what a number means — and two + * answers to that is worse than none. */ -private fun classificationName(ordinal: Int): String = when (ordinal) { - 0 -> "Unclassified" - 1 -> "Internal" - 2 -> "Confidential" - 3 -> "Restricted" - else -> "Level $ordinal" -} +private fun classificationName(ordinal: Int): String = + FileLevels.levelName(ordinal.toShort()) /** * What is being typed after an unfinished `@`, or null when there isn't one. diff --git a/app/src/main/kotlin/io/flamenet/messenger/app/FilesScreen.kt b/app/src/main/kotlin/io/flamenet/messenger/app/FilesScreen.kt index 1625f37..321833f 100644 --- a/app/src/main/kotlin/io/flamenet/messenger/app/FilesScreen.kt +++ b/app/src/main/kotlin/io/flamenet/messenger/app/FilesScreen.kt @@ -539,3 +539,115 @@ private fun ColumnScope.LevelPicker(selected: Short, onPick: (Short) -> Unit) { } } } + +// ---- Into and out of a conversation -------------------------------------- + +/** + * What can be attached to a message. + * + * ⚠️ Two kinds, and they are not variations of one thing: a photo comes from + * the gallery and is re-encoded for sending; a document comes from the vault + * and carries a classification with it. + */ +@Composable +fun ColumnScope.AttachPanel( + model: MessengerViewModel, + onPhoto: () -> Unit, + onFiles: () -> Unit, +) { + Column( + modifier = Modifier + .fillMaxWidth() + .clip(RoundedCornerShape(Radius.small)) + .background(Palette.SurfaceAlt) + .padding(Space.md) + .testTag("attach-panel"), + verticalArrangement = Arrangement.spacedBy(Space.sm), + ) { + KitSecondaryButton(text = "Photo", onClick = onPhoto, + modifier = Modifier.fillMaxWidth(), tag = "attach-photo") + KitSecondaryButton( + text = if (model.filesUnlocked()) "From Files" else "From Files (locked)", + onClick = onFiles, + modifier = Modifier.fillMaxWidth(), tag = "attach-files", + ) + } +} + +/** + * Choose a document from the vault, from inside a conversation. + * + * 🔴 **Sending is a downgrade, and the sheet says so before the tap.** The + * content key travels in the same envelope as the message, so a classification + * does not stop the recipient opening it — it goes so their phone files it the + * same way. Every word here has to carry that, because the alternative is a + * person believing "Restricted" did something on the way. + * + * ⚠️ **Never asks for the Files passphrase.** A conversation that prompted for + * one would be a conversation that has started collecting one, and Files goes + * out of its way not to hold it. Locked says so and points at the Files screen. + */ +@Composable +fun ColumnScope.PickFileToSendSheet( + model: MessengerViewModel, + onCancel: () -> Unit, + onPick: (LocalVault.Entry) -> Unit, +) { + val unlocked = model.filesUnlocked() + // Read once when the sheet opens: the vault does not change underneath it, + // and re-reading on every recomposition means decrypting every filename + // again for each frame. + val entries = remember(unlocked) { if (unlocked) model.filesForSending() else emptyList() } + + Column( + modifier = Modifier + .fillMaxWidth() + .clip(RoundedCornerShape(Radius.small)) + .background(Palette.SurfaceAlt) + .padding(Space.md) + .testTag("pick-file-sheet"), + ) { + Text("Send a file", fontWeight = FontWeight.Medium, fontSize = 16.sp) + Spacer(Modifier.height(Space.sm)) + + if (!unlocked) { + Text( + "Files is locked. Open Files from your account and unlock it, " + + "then come back — your passphrase isn't asked for here.", + fontSize = 13.sp, color = Palette.SecondaryText, + ) + } else if (entries.isEmpty()) { + Text("Nothing in your files yet.", fontSize = 13.sp, color = Palette.SecondaryText) + } else { + Text( + "Whoever you send this to will be able to open it. The " + + "classification travels with the file so it's filed the " + + "same way on their phone — it doesn't stop them reading it.", + fontSize = 12.sp, color = Palette.SecondaryText, + ) + Spacer(Modifier.height(Space.sm)) + for (entry in entries) { + Row( + modifier = Modifier + .fillMaxWidth() + .clickable { onPick(entry) } + .padding(vertical = Space.sm) + .testTag("pick-file-row"), + verticalAlignment = Alignment.CenterVertically, + ) { + Column(Modifier.weight(1f)) { + Text(entry.name ?: "Unnamed", fontSize = 15.sp, + maxLines = 1, overflow = TextOverflow.Ellipsis) + Text( + FileLevels.levelName(entry.label.classification), + fontSize = 11.sp, color = Palette.SecondaryText, + ) + } + } + } + } + Spacer(Modifier.height(Space.sm)) + KitSecondaryButton(text = "Cancel", onClick = onCancel, + modifier = Modifier.fillMaxWidth(), tag = "pick-file-cancel") + } +} 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 2ccd0ed..396ffc1 100644 --- a/app/src/main/kotlin/io/flamenet/messenger/app/MessengerViewModel.kt +++ b/app/src/main/kotlin/io/flamenet/messenger/app/MessengerViewModel.kt @@ -1649,6 +1649,112 @@ class MessengerViewModel(app: Application) : AndroidViewModel(app) { ) } + // -- Files, into and out of a conversation -------------------------------- + + /** + * Whether a conversation can offer to send from Files right now. + * + * ⚠️ **A chat never asks for the Files passphrase.** A conversation that + * prompted for one would be a conversation that has started collecting one, + * and Files goes out of its way not to hold it. Locked means the picker says + * so and points at the Files screen. + */ + fun filesUnlocked(): Boolean = filesVault != null + + /** Everything sendable, anywhere in the tree. */ + fun filesForSending(): List = + filesVault?.let { runCatching { it.allFiles() }.getOrDefault(emptyList()) } ?: emptyList() + + /** + * Send a document from the vault into this conversation. + * + * 🔴 **This is a downgrade, and the screen says so before the tap rather + * than after.** The content key goes in the same envelope, so the + * classification does not stop the recipient reading it — it travels so + * their phone files it the same way. Nothing here refuses on the label, + * because nothing here could: refusing in a client whose user holds the + * keys is a suggestion, and one dressed as a control is worse than none. + */ + fun sendFileFromVault(entry: LocalVault.Entry, chat: Phase.Chat) = + viewModelScope.launch { + val open = filesVault ?: return@launch + val live = session ?: return@launch + filesUpdate { it.copy(busy = "Sending…", problem = null, notice = null) } + val done = withContext(Dispatchers.IO) { + runCatching { + val bytes = open.contents(entry.node) + val name = entry.name ?: "file" + val label = entry.label + val group = chat.group + if (group != null) { + live.sendGroupFile(group, bytes, name, + label.classification.toInt(), label.compartments.sorted()) + } else { + live.sendFile(chat.peerId ?: return@runCatching, + bytes, name, + label.classification.toInt(), label.compartments.sorted()) + } + } + } + done.fold( + onSuccess = { + filesUpdate { it.copy(busy = null) } + update { + it.copy(openChat = it.openChat?.copy(lines = live.history(chat.threadId))) + } + }, + onFailure = { e -> + filesUpdate { + it.copy(busy = null, + problem = "Couldn't send that file. ${e.message ?: ""}".trim()) + } + }, + ) + } + + /** + * Save a received document into the vault. + * + * 🔴 **Always an encrypted copy, never a link.** There is no file on this + * phone to point at — the bytes arrived over the wire — so the two storage + * modes are not a choice here. + * + * ⚠️ **Filed under the label it arrived with, not under one this device + * chose.** Re-labelling somebody else's document on receipt would quietly + * launder its classification, and the number is the only thing that survived + * the trip. + */ + fun saveAttachmentToFiles(ref: io.flamenet.e2e.Attachment.Ref) = viewModelScope.launch { + val open = filesVault + if (open == null) { + filesUpdate { + it.copy(problem = "Files is locked. Open Files and unlock it, then " + + "come back — your passphrase isn't asked for here.") + } + return@launch + } + val live = session ?: return@launch + filesUpdate { it.copy(busy = "Saving…", problem = null, notice = null) } + val done = withContext(Dispatchers.IO) { + runCatching { + val bytes = live.attachmentBytes(ref, getApplication().cacheDir) + val name = ref.filename.ifEmpty { "Received file" } + open.add(bytes, name, + Label(ref.classification.toShort(), ref.compartments.toSet())) + name + } + } + done.fold( + onSuccess = { name -> filesUpdate { it.copy(busy = null, notice = "Saved $name to Files.") } }, + onFailure = { e -> + filesUpdate { + it.copy(busy = null, + problem = "Couldn't save that file. ${e.message ?: ""}".trim()) + } + }, + ) + } + fun filesClearMessages() = filesUpdate { it.copy(problem = null, notice = null) } private fun finishFilesWork(result: Result<*>, notice: String?) { 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 04b964f..e16351e 100644 --- a/app/src/main/kotlin/io/flamenet/messenger/app/Session.kt +++ b/app/src/main/kotlin/io/flamenet/messenger/app/Session.kt @@ -360,6 +360,61 @@ class Session( return sent } + /** + * Send a document from the Files vault. + * + * 🔴 **The label travels; it does not gate.** The content key is in the same + * envelope as the message, so the recipient can open this whatever their + * clearance — the classification goes so their phone files it the same way, + * and every sentence on the way to this call has to say that rather than + * implying a control. See `Attachment.FILE`. + * + * ⚠️ `application/octet-stream` and not a guess from the extension. A + * document announced as `image/png` renders as a broken image rather than a + * file somebody can save, and the vault stores no MIME type to be right + * with. + */ + fun sendFile( + peerUserId: Long, + bytes: ByteArray, + name: String, + classification: Int, + compartments: List, + ): io.flamenet.client.Conversations.Sent { + val sent = conversations.sendAttachment( + peerUserId, bytes, "application/octet-stream", Attachment.FILE, + filename = name, classification = classification, compartments = compartments) + record(Vault.threadForUser(peerUserId), "", mine = true, + attachment = sent.attachment, + delivery = if (sent.reachedSomeone) Delivery.DELIVERED else Delivery.PENDING) + return sent + } + + /** + * Send a document to a group. + * + * ⚠️ **Sealed and uploaded once, then fanned out** — one blob, one key, one + * upload, referenced by every member's envelope. And the count is worth + * saying out loud before tapping: a Restricted file into a group of eleven + * hands the key to eleven people. + */ + fun sendGroupFile( + group: RelayClient.Group, + bytes: ByteArray, + name: String, + classification: Int, + compartments: List, + ): Conversations.Sent { + val sent = conversations.sendGroupAttachment( + group, userId, bytes, "application/octet-stream", width = 0, height = 0, + kind = Attachment.FILE, filename = name, + classification = classification, compartments = compartments) + record(Vault.threadForGroup(group.id), "", mine = true, + attachment = sent.attachment, author = userId, + delivery = if (sent.reachedSomeone) Delivery.DELIVERED else Delivery.PENDING) + return sent + } + /** Bytes for an attachment, cached on disk after the first fetch. */ fun attachmentBytes(ref: Attachment.Ref, cacheDir: java.io.File): ByteArray { val cached = java.io.File(cacheDir, "att-" + ref.id.replace('/', '_')) diff --git a/app/src/test/kotlin/io/flamenet/messenger/app/FileShareWireTest.kt b/app/src/test/kotlin/io/flamenet/messenger/app/FileShareWireTest.kt new file mode 100644 index 0000000..2654bfd --- /dev/null +++ b/app/src/test/kotlin/io/flamenet/messenger/app/FileShareWireTest.kt @@ -0,0 +1,109 @@ +package io.flamenet.messenger.app + +import io.flamenet.e2e.Attachment +import io.flamenet.e2e.Content +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNotNull +import kotlin.test.assertTrue + +/** + * What a document sent from Files actually puts on the wire. + * + * 🔴 **The classification is the only thing that survives the trip.** The + * recipient's phone files the document at whatever number arrives — there is + * nothing else to consult and no server to ask — so a label dropped in transit + * does not error anywhere: the file simply arrives Unclassified and gets filed + * that way, on the most permissive setting it could possibly have. + * + * ⚠️ These check the ENVELOPE. `Session.sendFile` needs a relay and a vault, so + * the send itself is covered on device; the shape that crosses to the other app + * is what an interop bug would live in, and it is checkable here. + */ +class FileShareWireTest { + + private fun ref(classification: Int, compartments: List, name: String = "Q3.txt") = + Attachment.Ref( + kind = Attachment.FILE, id = "blob-1", + key = ByteArray(32) { 1 }, digest = ByteArray(32) { 2 }, + bytes = 4096, mime = "application/octet-stream", + filename = name, classification = classification, compartments = compartments, + ) + + private fun roundTrip(r: Attachment.Ref): Attachment.Ref { + val parsed = assertNotNull(Content.parse(Content.attachment(r))) + return assertNotNull(parsed.attachment, "the attachment did not survive the envelope") + } + + @Test + fun `a file keeps its classification and its compartments`() { + val back = roundTrip(ref(3, listOf(1, 2))) + assertEquals(3, back.classification, "the classification was lost in the envelope") + assertEquals(listOf(1, 2), back.compartments) + assertEquals("Q3.txt", back.filename) + } + + /** + * 🔴 **Unclassified must round-trip as a real value, not as an absence.** + * Zero is the default for a missing field, so a sender that dropped the key + * and a sender that meant zero are indistinguishable at the other end. This + * pins that the zero case still parses as a file rather than as something + * the recipient has to guess about. + */ + @Test + fun `an unclassified file is still a file, not a missing label`() { + val back = roundTrip(ref(0, emptyList())) + assertEquals(0, back.classification) + assertTrue(back.compartments.isEmpty()) + assertEquals(Attachment.FILE, back.kind) + } + + /** + * ⚠️ A file envelope always carries a caption, even when nobody wrote one. + * Builds predating this attachment kind show the caption and drop the rest, + * so an empty one reaches them as "[unsupported attachment]" with no clue + * what was sent. + */ + @Test + fun `a file with no caption still says what it is to an older build`() { + val parsed = assertNotNull(Content.parse(Content.attachment(ref(1, emptyList())))) + assertTrue( + parsed.text.contains("Q3.txt"), + "an older build would render this as an unsupported attachment: '${parsed.text}'", + ) + } + + @Test + fun `a caption somebody wrote is not replaced by the filename`() { + val parsed = assertNotNull( + Content.parse(Content.attachment(ref(1, emptyList()), caption = "the numbers"))) + assertEquals("the numbers", parsed.text) + } + + /** + * ⚠️ Compartment order is normalised on the way out, so two senders who + * chose the same areas produce the same bytes. Unsorted, the label would + * still be *correct* and would still encode differently — which matters + * because the canonical encoding is authenticated data elsewhere. + */ + @Test + fun `compartments arrive in a stable order`() { + assertEquals(listOf(1, 2, 5), roundTrip(ref(2, listOf(5, 1, 2))).compartments.sorted()) + } + + /** + * 🔴 An image must NOT gain a label. `label` is written only for the file + * kind — a photo carrying `cls: 0` would tell the other end there is a + * classification here to respect, and there is not. + */ + @Test + fun `an image carries no label at all`() { + val image = Attachment.Ref( + kind = Attachment.IMAGE, id = "b", key = ByteArray(32), digest = ByteArray(32), + bytes = 10, mime = "image/jpeg", width = 4, height = 4, + ) + val json = org.json.JSONObject(Content.attachment(image)) + assertTrue(!json.has("cls"), "an image envelope is claiming a classification") + assertTrue(!json.has("cmp"), "an image envelope is claiming compartments") + } +} diff --git a/client/src/main/kotlin/io/flamenet/client/Conversations.kt b/client/src/main/kotlin/io/flamenet/client/Conversations.kt index 87bf21b..feaf925 100644 --- a/client/src/main/kotlin/io/flamenet/client/Conversations.kt +++ b/client/src/main/kotlin/io/flamenet/client/Conversations.kt @@ -270,6 +270,18 @@ class Conversations( height: Int = 0, duration: Double = 0.0, groupId: String? = null, + /** File only: the document's own name. */ + filename: String = "", + /** + * File only: the label it was filed under. + * + * 🔴 **A MARKING, not a permission.** The content key rides in the same + * envelope, so whoever can read the message can read the file. This + * travels so the recipient can file it the same way — it must never be + * described, here or on screen, as something that stops them opening it. + */ + classification: Int = 0, + compartments: List = emptyList(), ): Sent { val sealed = Attachment.seal(data) val id = relay.uploadAttachment(sealed.blob) @@ -277,6 +289,8 @@ class Conversations( kind = kind, id = id, key = sealed.key, digest = sealed.digest, bytes = sealed.blob.size, mime = mime, width = width, height = height, duration = duration, + filename = filename, classification = classification, + compartments = compartments.sorted(), ) return sendPlaintext( toUserId, @@ -301,12 +315,19 @@ class Conversations( width: Int, height: Int, caption: String? = null, + kind: String = Attachment.IMAGE, + filename: String = "", + /** See the note on [sendAttachment]: a marking, not a permission. */ + classification: Int = 0, + compartments: List = emptyList(), ): Sent { val sealed = Attachment.seal(data) val id = relay.uploadAttachment(sealed.blob) val ref = Attachment.Ref( - kind = Attachment.IMAGE, id = id, key = sealed.key, digest = sealed.digest, + kind = kind, id = id, key = sealed.key, digest = sealed.digest, bytes = sealed.blob.size, mime = mime, width = width, height = height, + filename = filename, classification = classification, + compartments = compartments.sorted(), ) val body = Content.attachment(ref, caption, group.id, groupName = vault.groupName(group.id)) diff --git a/client/src/main/kotlin/io/flamenet/client/LocalVault.kt b/client/src/main/kotlin/io/flamenet/client/LocalVault.kt index 09fb810..41044b8 100644 --- a/client/src/main/kotlin/io/flamenet/client/LocalVault.kt +++ b/client/src/main/kotlin/io/flamenet/client/LocalVault.kt @@ -276,23 +276,54 @@ class LocalVault( .put("storage", node.storage.wire) .put("bookmark", node.bookmark ?: JSONObject.NULL) + /** + * 🔴 **`optString` is not safe on a nullable field, and the JVM will not + * tell you.** Android's bundled `org.json` coerces a JSON `null` to the + * four-character string `"null"`; the Maven `org.json` these tests run + * against returns `""`. Every nullable field below therefore passed on a + * laptop and broke on a phone: + * + * - `parentID` threw `Invalid UUID string: null` — so a vault holding any + * top-level file failed to load AT ALL. Adding the first file worked, + * because it was already in memory; reopening the app lost everything, + * with an error nobody could act on. + * - `wrappedContentKey` was worse, because it did NOT throw: `"null"` is + * valid base64, so the node came back with a plausible three-byte content + * key and simply refused to decrypt. + * - `bookmark` produced a reference pointing at `"null"`, permanently + * missing. + * + * ⚠️ So absence is decided by [JSONObject.isNull], never by comparing a + * decoded string to a sentinel. The literal `"null"` is rejected as well: + * it is not a valid value for any of these fields, and tolerating it costs + * nothing next to reintroducing this. + * + * Found by running the app on a device (android!52). No JVM test could have + * caught it, which is the argument for the device run, not against the + * tests. + */ + private fun optional(o: JSONObject, key: String): String? { + if (o.isNull(key)) return null + val value = o.optString(key, "") + return value.takeIf { it.isNotEmpty() && it != "null" } + } + private fun decode(o: JSONObject): Node { val bits = o.optJSONArray("compartments") ?: JSONArray() return Node( id = UUID.fromString(o.getString("id")), - parentId = o.optString("parentID", "").takeIf { it.isNotEmpty() }?.let(UUID::fromString), + parentId = optional(o, "parentID")?.let(UUID::fromString), isFolder = o.optBoolean("isFolder", false), classification = o.getInt("classification").toShort(), compartments = (0 until bits.length()).map { bits.getInt(it) }, wrappedNodeKey = unb64(o.getString("wrappedNodeKey")), nameDigest = unb64(o.getString("nameDigest")), wrappedName = unb64(o.getString("wrappedName")), - wrappedContentKey = o.optString("wrappedContentKey", "") - .takeIf { it.isNotEmpty() }?.let(::unb64), + wrappedContentKey = optional(o, "wrappedContentKey")?.let(::unb64), byteCount = o.optInt("byteCount", 0), addedAtMillis = o.optLong("addedAt", 0L), - storage = Storage.parse(o.optString("storage", "").takeIf { it.isNotEmpty() }), - bookmark = o.optString("bookmark", "").takeIf { it.isNotEmpty() }, + storage = Storage.parse(optional(o, "storage")), + bookmark = optional(o, "bookmark"), ) } @@ -344,6 +375,28 @@ class LocalVault( found } + /** + * Every file in the whole tree, for a picker that has no business showing + * folders. + * + * 🔴 **The whole tree, not the top of it.** iOS's picker lists `list(nil)`, + * so a document filed into a folder cannot be sent from a conversation at + * all — and nothing says so; the file is simply absent from the list, which + * reads as "it isn't there". Filed as ios#68. + * + * ⚠️ Unreadable and missing rows are dropped here rather than shown + * disabled. Everywhere else this class insists on showing a row you cannot + * open — because hiding it would leave somebody unable to see that anything + * is there. A picker is the exception: its only purpose is choosing + * something to send, and a row that can never be chosen is furniture. + */ + fun allFiles(): List = lock.withLock { + nodes.filter { !it.isFolder } + .map { Entry(it, nameOf(it), isMissing = isMissing(it)) } + .filter { it.name != null && !it.isMissing } + .sortedWith(compareBy(String.CASE_INSENSITIVE_ORDER) { it.name ?: "" }) + } + fun contents(of: Node): ByteArray = lock.withLock { readContents(of) } private fun readContents(node: Node): ByteArray { diff --git a/client/src/test/kotlin/io/flamenet/client/LocalVaultTest.kt b/client/src/test/kotlin/io/flamenet/client/LocalVaultTest.kt index c6f151b..c7d54e3 100644 --- a/client/src/test/kotlin/io/flamenet/client/LocalVaultTest.kt +++ b/client/src/test/kotlin/io/flamenet/client/LocalVaultTest.kt @@ -160,6 +160,63 @@ class LocalVaultTest { assertEquals(LocalVault.Storage.VAULT, reopen().list(null).single().storage) } + /** + * 🔴 **The bug the JVM could not see.** Android's bundled `org.json` + * coerces a JSON `null` to the four-character string `"null"`; the Maven + * `org.json` this suite runs against returns `""`. Every nullable field in + * the index therefore passed here and broke on a phone — a vault with any + * top-level file threw `Invalid UUID string: null` on load, so adding the + * first file worked and reopening the app lost everything. + * + * ⚠️ **This test writes the literal string `"null"` on purpose.** That is + * exactly what the decoder is handed on a device, so putting it in the JSON + * here is what makes the JVM run reproduce the device's behaviour. Feeding + * a real JSON `null` — which is what the vault actually writes — would pass + * with the broken code and prove nothing. + */ + @Test + fun `a nullable field decodes as absent however the platform hands it over`() { + val node = vault.add("x".toByteArray(), "top.txt", secret) + val reference = File(root, "index.json").readText() + + for (shape in listOf( + reference.replace("\"parentID\":null", "\"parentID\":\"null\""), + reference.replace("\"parentID\":null", ""). // key missing entirely + replace(",,", ","), + reference, + )) { + File(root, "index.json").writeText(shape) + val reopened = reopen() + assertEquals(1, reopened.count, "the index failed to load: $shape") + assertNull(assertNotNull(reopened.node(node.id)).parentId) + assertEquals("top.txt", reopened.list(null).single().name) + } + } + + /** + * 🔴 The one that would NOT have thrown. `"null"` is valid base64, so a + * content key decoded from it is a plausible three bytes — and the file + * simply refuses to decrypt, for ever, with nothing to explain why. + */ + @Test + fun `a bookmark or content key of the literal string null is treated as absent`() { + val node = vault.add("x".toByteArray(), "f.txt", secret) + val index = File(root, "index.json") + index.writeText( + index.readText() + .replace("\"bookmark\":null", "\"bookmark\":\"null\"") + .replace(Regex("\"wrappedContentKey\":\"[^\"]*\""), + "\"wrappedContentKey\":\"null\"") + ) + val reopened = reopen() + val back = assertNotNull(reopened.node(node.id)) + assertNull(back.bookmark) + assertNull(back.wrappedContentKey, + "a three-byte content key decoded from the string \"null\" was accepted") + // And the failure is one somebody can act on, not a decryption error. + assertFailsWith { reopened.contents(back) } + } + @Test fun `an empty directory loads as an empty vault rather than failing`() { val fresh = File(root, "elsewhere").also { it.mkdirs() } @@ -468,6 +525,52 @@ class LocalVaultTest { assertEquals(listOf("b.txt"), reopen().list(null).map { it.name }) } + // ---- The picker ------------------------------------------------------ + + /** + * 🔴 **The whole tree, not the top of it.** iOS's picker lists the root + * only, so a document filed into a folder cannot be sent from a + * conversation at all — and nothing says so: the file is simply absent, + * which reads as "it isn't there". Filed as ios#68. + */ + @Test + fun `everything sendable is offered, however deep it is filed`() { + val folder = vault.createFolder("Reports", open) + val inner = vault.createFolder("2026", open, parentId = folder.id) + vault.add("a".toByteArray(), "top.txt", open) + vault.add("b".toByteArray(), "buried.txt", secret, parentId = inner.id) + + assertEquals(listOf("buried.txt", "top.txt"), vault.allFiles().map { it.name }) + } + + /** Folders are places, not files. Sending one means nothing. */ + @Test + fun `folders are not offered`() { + vault.createFolder("Reports", open) + assertTrue(vault.allFiles().isEmpty()) + } + + /** + * ⚠️ **The one place this class hides a row it cannot read.** Everywhere + * else an unreadable row is shown, because hiding it would leave somebody + * unable to see that anything is there. A picker's only purpose is choosing + * something to send, and a row that can never be chosen is furniture. + */ + @Test + fun `rows that could never be sent are left out of the picker`() { + refs.files["doc://gone"] = "x".toByteArray() + vault.reference("doc://gone", "vanished.pdf", open) + vault.add("readable".toByteArray(), "fine.txt", open) + refs.files.remove("doc://gone") + + assertEquals(listOf("fine.txt"), vault.allFiles().map { it.name }, + "a file that has moved was offered as sendable") + + val stranger = LocalVault(root, ByteArray(32) { 99 }, refs).also { it.load() } + assertTrue(stranger.allFiles().isEmpty(), + "a row this device cannot decrypt was offered as sendable") + } + // ---- The tree -------------------------------------------------------- @Test diff --git a/engine/src/main/kotlin/io/flamenet/e2e/Vault.kt b/engine/src/main/kotlin/io/flamenet/e2e/Vault.kt index aa8f5ae..393688d 100644 --- a/engine/src/main/kotlin/io/flamenet/e2e/Vault.kt +++ b/engine/src/main/kotlin/io/flamenet/e2e/Vault.kt @@ -461,15 +461,42 @@ class Vault(private val storage: Storage, private val key: SecretKey) { Trust( identities = (0 until a.length()).map { a.getString(it) }, verified = o.optBoolean("verified", false), - fnid = o.optString("fnid").ifEmpty { null }, + // 🔴 **`"null"` is rejected, and that is not paranoia — records + // written that way are already on phones.** `saveTrust` used to + // write an explicit JSON null here, and Android's `org.json` + // coerces that to the four-character string on the way back out. + // See the note on [saveTrust]; this is the half that repairs what + // has already been stored. + fnid = o.optString("fnid").takeIf { it.isNotEmpty() && it != "null" }, changed = o.optBoolean("changed", false), ) } + /** + * 🔴 **The key is OMITTED when there is no address, never written as an + * explicit JSON null.** Android's bundled `org.json` reads a JSON null back + * as the four-character string `"null"`, so a contact pinned before their + * fnid was known reloaded with `fnid = "null"` — a non-null value that is + * not an address. What that caused: + * + * - `Trust.pin` compares a newly offered fnid against the pinned one and + * treats a difference as a different person. `"null" != "fnid:…"`, so the + * first time a real address arrived the contact was marked **CHANGED** — + * the "security keys changed" warning, which is supposed to mean an + * attack, fired on an ordinary first sighting. + * - `rememberIdentity` returns early once `fnid != null`, so the real + * address was never resolved and the record stayed wrong for good. + * - `pinnedFnid` fed `"null"` into key-log verification, which recomputes a + * leaf against the address a contact was accepted under. Every check + * would have been against the wrong subject. + * + * An absent key reads as `""` on both platforms, which is what the reader + * has always expected. + */ fun saveTrust(userId: Long, trust: Trust) = writeJson(trustName(userId), JSONObject() .put("identities", org.json.JSONArray(trust.identities)) .put("verified", trust.verified) - .put("fnid", trust.fnid ?: JSONObject.NULL) + .also { if (trust.fnid != null) it.put("fnid", trust.fnid) } .put("changed", trust.changed)) fun forgetTrust(userId: Long) = storage.delete(trustName(userId)) diff --git a/engine/src/test/kotlin/io/flamenet/e2e/VaultTest.kt b/engine/src/test/kotlin/io/flamenet/e2e/VaultTest.kt index 003acc3..da915ad 100644 --- a/engine/src/test/kotlin/io/flamenet/e2e/VaultTest.kt +++ b/engine/src/test/kotlin/io/flamenet/e2e/VaultTest.kt @@ -315,4 +315,66 @@ class VaultTest { Vault(storage, secret).saveIdentity(identity) assertNotEquals(Bytes.b64(first), Bytes.b64(storage.records.getValue("identity"))) } + + // ---- Trust records and the platform's JSON --------------------------- + + /** + * 🔴 **A contact with no known address must come back with NO address.** + * + * `saveTrust` used to write `"fnid": null` explicitly, and Android's + * bundled `org.json` reads a JSON null back as the four-character string + * `"null"` — a non-null value that is not an address. `Trust.pin` compares a + * newly offered fnid against the pinned one and treats a difference as a + * different person, so the first time a real address arrived the contact was + * marked **CHANGED**: the "security keys changed" warning, which is meant to + * mean an attack, fired on an ordinary first sighting. + * + * ⚠️ The JVM's `org.json` returns `""` there, so this passed on a laptop and + * failed on a phone. The two tests below cover both halves — what is + * written now, and what has to be tolerated because it is already on + * devices in the field. + */ + @Test + fun `a trust record with no address writes no address at all`() { + val storage = MapStorage() + val secret = key() + Vault(storage, secret).saveTrust(7, Vault.Trust(listOf("aaa"), verified = false, fnid = null)) + + assertNull(Vault(storage, secret).loadTrust(7)?.fnid, + "an absent address came back as a value") + } + + /** + * ⚠️ **This is the test that reproduces the device**, and the one above is + * not. On the JVM a real JSON null reads back as `""` whether the writer + * omits the key or not, so the round trip above passes with the bug intact. + * Handing the reader the literal string is what Android actually does, so + * writing it here is what makes a laptop run tell the truth. + */ + + @Test + fun `a record already holding the literal string null reads as no address`() { + // 🔴 Records written the old way exist on phones already. The reader has + // to repair them, or those contacts stay permanently mis-addressed — + // `rememberIdentity` returns early once `fnid != null`, so nothing would + // ever ask again. + val storage = MapStorage() + val secret = key() + val vault = Vault(storage, secret) + vault.saveTrust(7, Vault.Trust(listOf("aaa"), verified = false, fnid = "null")) + + assertNull(Vault(storage, secret).loadTrust(7)?.fnid, + "the literal string \"null\" was accepted as somebody's global address") + } + + @Test + fun `a real address still round-trips`() { + val storage = MapStorage() + val secret = key() + Vault(storage, secret).saveTrust( + 7, Vault.Trust(listOf("aaa"), verified = true, fnid = "fnid:abc123")) + val back = Vault(storage, secret).loadTrust(7) + assertEquals("fnid:abc123", back?.fnid) + assertTrue(back!!.verified) + } }