From 31c585ecdbf11a322f616e7424ab3655cb4d3151 Mon Sep 17 00:00:00 2001 From: spaced4ndy <8711996+spaced4ndy@users.noreply.github.com> Date: Tue, 7 Jul 2026 18:22:58 +0400 Subject: [PATCH] wip --- plans/2026-07-07-signed-files-and-history.md | 63 ++++++++++++++++++++ src/Simplex/Chat/Library/Subscriber.hs | 34 ++++++----- 2 files changed, 83 insertions(+), 14 deletions(-) create mode 100644 plans/2026-07-07-signed-files-and-history.md diff --git a/plans/2026-07-07-signed-files-and-history.md b/plans/2026-07-07-signed-files-and-history.md new file mode 100644 index 0000000000..ce4b20015a --- /dev/null +++ b/plans/2026-07-07-signed-files-and-history.md @@ -0,0 +1,63 @@ +# Signed file integrity + signed history preservation + +Extends [channel message signing](2026-06-04-channel-message-signing.md). Part A must land **before** Part B, because forwarding a signed file post is only meaningful once the signature actually binds the file bytes. Related: [roster catch-up subscribers](2026-06-22-roster-catchup-subscribers.md) (verification depends on the recipient holding the author's key). + +## Part A — Sign the file digest (bug, fix first) + +### Problem + +A "signed" XFTP file message signs nothing about the file itself. The signature covers `XMsgNew`, whose `FileInvitation` is built with `fileDigest = Nothing` (`Types.hs:1524`, `xftpFileInvitation`) and an empty embedded description (`dummyFileDescr`, `Internal.hs:389,410`) — because at send time the file is still uploading async. The real digest/key/servers live only in the later, **unsigned** `XMsgFileDescr` events. So the signature attests only `fileName + fileSize`. A malicious relay can pair the genuine signed `XMsgNew` with a substituted description pointing at different content of the same size, and the recipient displays it as "signed & verified". The file signature is currently meaningless. This affects **live** signed messages, not only history. + +### Fix + +Put the file's content digest into the signed part (`FileInvitation.fileDigest`, already a field, inside `XMsgNew`), and verify the downloaded file against it on receive. + +- Sender: populate `fileDigest` for XFTP sends so it is covered by the message signature. +- Receiver: after the file download completes, compute the received file's digest and compare to the signed `fileDigest`; on mismatch, mark the file invalid / reject and surface an event. Enforce the check when the message is signed; for unsigned messages the digest is informational. +- Compatibility: `fileDigest` is an optional field older clients already ignore — forward-compatible. Verification runs only on clients that support it. + +### Open questions to settle in implementation + +- **Digest form.** Over the decrypted (plaintext) content the recipient can reconstruct, vs the encrypted-file digest already present in the XFTP description. Plaintext is encryption-independent and simplest to verify; confirm against simplexmq's XFTP digest so we reuse rather than recompute. +- **Availability at send.** The invitation is created before the async upload; confirm whether the agent surfaces the digest early enough, or whether it must be computed synchronously before `XMsgNew` is sent. +- **Scope.** Populate `fileDigest` for all XFTP sends (integrity even when unsigned) but gate the reject-on-mismatch behavior on the message being signed. + +### Threat model + +Author honest, relay/forwarder malicious. The relay controls the unsigned description but not the signed invitation. Signing the content digest binds the file end-to-end from author to recipient, independent of any relay. + +## Part B — Preserve signatures in history + +### Problem + +`sendHistory` → `processContentItem` (`Internal.hs:1370`) re-encodes each item's current content via `prepareGroupMsg` and sends it unsigned (`groupMsgSigning False`; the relay has no author key). Catch-up members therefore hold **all** history unsigned, so §7 enforcement (`requireVerifiedEdit`/`requireVerifiedDelete`) never protects their items — and can't heal later, because `updateGroupChatItem_` (`Messages.hs:2766`) does not write `msg_signed`, so a subsequent signed edit does not upgrade the item. Only delivering history signed at creation closes this. + +### Design + +- **Storage.** Two nullable columns on `chat_items`: `item_msg_body`, `item_signatures`. Written by relays only, for content items only (same `msg_content_tag` / `include_in_history` filter history uses). `chat_binding` is not stored — it is derived as `smpEncode(publicGroupId, authorMemberId)` at send time. +- **Capture.** Write the columns in `createNewChatItem_` when the item is signed; overwrite them in `updateGroupChatItem_` when a signed edit is applied. This keeps the stored bytes tracking the **latest** signed event, so history always forwards current content. Thread the raw `(msg_body, signatures)` onto `RcvMessage` (from `saveGroupFwdRcvMsg`'s `verifiedMsgParts`) and use `SndMessage`'s existing `signedMsg_`/`msgBody`; today both carry only the `msgSigned` status. +- **Forward.** In `sendHistory`, for a signed content item with stored bytes, forward the original bytes as a `VMSigned` `XGrpMsgForward` (reuse the live path's `encodeFwdElement` / `sendFwdMemberMessage`) instead of re-encoding; unsigned items keep the current re-encode path. +- **Edits need only the last event.** Forwarding the latest signed event — `XMsgNew` if never edited, else the latest `XMsgUpdate` — is sufficient: on the recipient a forwarded `XMsgUpdate` for a not-yet-existing item hits the create fallback in `groupMessageUpdate` (`Subscriber.hs:2234` `catchCINotFound` → `saveRcvChatItem'` → `createNewRcvChatItem`), which creates the item with the edit's `msgSigned` and content, marked edited. So one `(body, signatures)` per item, no original+edit replay. Self-consistent because a verified item only ever accepts verified edits (`requireVerifiedEdit`). +- **Files.** No special branch. The forwarded signed `XMsgNew` carries `name + size + digest` (from Part A); the description follows via the existing `XMsgFileDescr` path and may be re-forwarded/re-uploaded freely — it is not covered by the signature, and integrity now comes from the signed digest. +- **Compatibility.** Same wire format as live signed messages. Signing is tied to relay channels (`groupMsgSigning` gates on `useRelays'`, not a member version), so any relay subscriber already receives signed live messages; signed history is identical. No new version gate. + +### Result + +Catch-up members hold non-edited and edited signed posts as verified with current content, so §7 enforcement protects their edits/deletes — closing the residual documented in the signing plan. Retention is no longer bounded by the 30-day `messages` pruning. + +## Implementation steps + +Part A: +1. Populate `FileInvitation.fileDigest` for XFTP sends. +2. Verify the downloaded file's digest against the signed value on receive-completion; reject/mark-invalid on mismatch (enforced for signed messages); surface an event. +3. Tests: signed file message verifies; tampered description/content fails verification. + +Part B: +4. Migration: add `item_msg_body`, `item_signatures` to `chat_items` (SQLite + Postgres modules; register in `Migrations.hs`; add to `.cabal`; schema files regenerate via tests). +5. Thread raw signed bytes onto `RcvMessage`; store/overwrite in `createNewChatItem_` and `updateGroupChatItem_` (relay + content-item + signed). +6. `sendHistory`: signed content item with stored bytes → forward `VMSigned`; else re-encode. +7. Tests: catch-up subscriber holds non-edited and edited signed posts as verified/current; a forged unsigned edit/delete of a catch-up item is rejected; a signed file post verifies on catch-up. + +## Docs to update on implementation + +Signing/files/history spec + product docs; move the digest gap from `product/gaps.md` to fixed; cross-link this plan from [channel message signing](2026-06-04-channel-message-signing.md). diff --git a/src/Simplex/Chat/Library/Subscriber.hs b/src/Simplex/Chat/Library/Subscriber.hs index 8f719e10b1..20565ac880 100644 --- a/src/Simplex/Chat/Library/Subscriber.hs +++ b/src/Simplex/Chat/Library/Subscriber.hs @@ -2288,12 +2288,14 @@ processAgentMessageConn cxt user@User {userId} corrId agentConnId agentMessage = _ -> messageError "x.msg.update: invalid message update" $> Nothing where isSender m' = maybe False (\m -> sameMemberId (memberId' m) m') m_ - -- only a verified edit may alter a verified item; reject otherwise (fail-closed) + -- a verified item requires a verified edit (fail-closed): unsigned is a forgery (bad-signature item); signed-but-no-key is unverifiable (drop with a log) requireVerifiedEdit :: ChatDirection 'CTGroup 'MDRcv -> Maybe MsgSigStatus -> CM (Maybe DeliveryTaskContext) -> CM (Maybe DeliveryTaskContext) requireVerifiedEdit cd itemSigned action - | itemSigned == Just MSSVerified && msgSigned /= Just MSSVerified = do - createInternalChatItem user cd (CIRcvGroupEvent RGEMsgBadSignature) (Just brokerTs) - pure Nothing + | itemSigned == Just MSSVerified = + case msgSigned of + Just MSSVerified -> action + Just MSSSignedNoKey -> logWarn "x.msg.update: unverified update of a signed item (no key to verify), dropped" $> Nothing + Nothing -> createInternalChatItem user cd (CIRcvGroupEvent RGEMsgBadSignature) (Just brokerTs) $> Nothing | otherwise = action updateCI :: ShowGroupAsSender -> ChatItem 'CTGroup 'MDRcv -> Maybe GroupChatScopeInfo -> MsgContent -> Maybe Bool -> Maybe MemberId -> CM (Maybe DeliveryTaskContext) updateCI showGroupAsSender ci scopeInfo oldMC itemLive memberId = do @@ -2397,18 +2399,22 @@ processAgentMessageConn cxt user@User {userId} corrId agentConnId agentMessage = | senderRole < GRModerator || senderRole < memberRole = messageError "x.msg.del: message of another member with insufficient member permissions" $> Nothing | otherwise = a - -- only a verified delete may remove a verified item; reject otherwise (fail-closed) + -- a verified item requires a verified delete (fail-closed): unsigned is a forgery (bad-signature item); signed-but-no-key is unverifiable (drop with a log) requireVerifiedDelete :: CChatItem 'CTGroup -> CM (Maybe DeliveryTaskContext) -> CM (Maybe DeliveryTaskContext) requireVerifiedDelete cci@(CChatItem _ ChatItem {chatDir, meta = CIMeta {msgSigned = itemSigned}}) action - | itemSigned == Just MSSVerified && msgSigned /= Just MSSVerified = do - scopeInfo <- withStore $ \db -> getGroupChatScopeInfoForItem db cxt user gInfo (cChatItemId cci) - let cd :: ChatDirection 'CTGroup 'MDRcv - cd = case chatDir of - CIGroupRcv mem -> CDGroupRcv gInfo scopeInfo mem - CIChannelRcv -> CDChannelRcv gInfo scopeInfo - CIGroupSnd -> CDGroupRcv gInfo scopeInfo membership - createInternalChatItem user cd (CIRcvGroupEvent RGEMsgBadSignature) (Just brokerTs) - pure Nothing + | itemSigned == Just MSSVerified = + case msgSigned of + Just MSSVerified -> action + Just MSSSignedNoKey -> logWarn "x.msg.del: unverified delete of a signed item (no key to verify), dropped" $> Nothing + Nothing -> do + scopeInfo <- withStore $ \db -> getGroupChatScopeInfoForItem db cxt user gInfo (cChatItemId cci) + let cd :: ChatDirection 'CTGroup 'MDRcv + cd = case chatDir of + CIGroupRcv mem -> CDGroupRcv gInfo scopeInfo mem + CIChannelRcv -> CDChannelRcv gInfo scopeInfo + CIGroupSnd -> CDGroupRcv gInfo scopeInfo membership + createInternalChatItem user cd (CIRcvGroupEvent RGEMsgBadSignature) (Just brokerTs) + pure Nothing | otherwise = action delete :: CChatItem 'CTGroup -> Bool -> Maybe GroupMember -> CM (Maybe DeliveryTaskContext) delete cci asGroup byGroupMember = do