on master help sign in
Security audit 13th pass: fix 23 findings across crypto, server, client

Crypto (R1-R5): - R2: Add "skrepka-msg-v2:" domain-separation tag to message signatures - R3: Pre-allocate decompress buffer to prevent reallocation leak - R4: Fix 100 + compressed_len overflow on 32-bit targets (saturating_sub) - R5: Q49 low-order X25519 check: debug_assert -> runtime warning - R1: Rewrite AAD tests to honestly document defense-in-depth role Client core (C1-C5): - C1: LoadedMessages merges by-id instead of overwrite (message loss fix) - C2: send_text queues text before profile on first contact (profile drop fix) - C4: Add AuthWatchdog symmetric with Poll/Flush watchdogs - C5: Skip redundant idle outbox+cursor writes Server (S1-S8): - S1: 404 forwards now count toward maxForwardRetries; clearFailure only on success - S2: Cap forkForwardsTo fan-out with take maxForwardsPerServer - S4: forwardQueuedToAll replaces per-key sweeps (single mailbox scan) - S6: Add maxMessagesTotal=1000000 global cap to *messages - S7: Hoist Ed25519 verify out of *sessions atomic (two-phase auth) - S8: online+offline same key: online wins, offline filtered Swift shell (C3, C6): - C3: HttpSessionDelegate with per-task 64MiB response size limiting - C6: PhotoPicker loops JPEG quality down until base64 fits MAX_PHOTO_LEN Spec (PROTOCOL.md §3, §10): - Document skrepka-msg-v2 tag, unbounded mailbox, clock jump, active-user metadata leak, forward cancellation blast radius

commitee54351bc0cf38d3e2235a103914d8997a51ba0e authorLancelot <montelot-dev@typeable.io> dateThu 16 Jul 2026, 03:24 (3 months ago) parent3258116
7 files changed+400−176
PROTOCOL.md+10−5 apple/SkrepkaApp/Effects.swift+65−12 apple/SkrepkaApp/PhotoPicker.swift+17−1 core/src/app.rs+75−15 core/src/crypto.rs+43−40 server.knot+190−103 server.o
PROTOCOL.md+10−5
@@ -118,12 +118,12 @@ The wire-visible "envelope" carries only the recipient and the opaque blob. The
118118 ```
119119 compressed = zstd(plaintext_bytes) // standard zstd frame (embeds content size)
120120 ```
121−5. Sign the recipient pubkey concatenated with the compressed plaintext and padding. Binding the signature to the recipient prevents the same blob from being replayed against another recipient:
121+5. Sign a domain-separation tag, the recipient pubkey, and the compressed plaintext with padding. The `"skrepka-msg-v2:"` prefix is a domain-separation tag distinguishing message signatures from auth-challenge signatures (which begin with `"skrepka-auth-v1:"`, §6), so a signature produced in one context can never validate in the other. Binding the signature to the recipient prevents the same blob from being replayed against another recipient:
122122 ```
123123 compressed_len = uint32_be(len(compressed))
124124 padding = random_bytes(target_blob_size - unpadded_blob_size)
125125 signature = ed25519_sign(sender_ed25519_private,
126− recipient_ed25519_public || compressed_len || compressed || padding)
126+ "skrepka-msg-v2:" || recipient_ed25519_public || compressed_len || compressed || padding)
127127 ```
1281286. Build the inner buffer that will be encrypted:
129129 ```
@@ -156,7 +156,7 @@ The wire-visible "envelope" carries only the recipient and the opaque blob. The
1561562. Derive the recipient's X25519 private key from the recipient's Ed25519 private key, compute the raw shared secret with `ephemeral_public`, and HKDF-derive the same `key`.
1571573. Reconstruct the AAD: `version_byte || ephemeral_public || nonce || recipient_ed25519_public`. AEAD-decrypt the ciphertext with this AAD to recover the inner buffer.
1581584. Split: 32-byte `sender_ed25519_public`, 64-byte `signature`, 4-byte `compressed_len` (big-endian u32), then `compressed_len` bytes of `compressed` (the remainder is padding).
159−5. Verify the signature over `recipient_ed25519_public || inner[96..]` (i.e. `compressed_len || compressed || padding`) using `sender_ed25519_public`. Reject the message on failure.
159+5. Verify the signature over `"skrepka-msg-v2:" || recipient_ed25519_public || inner[96..]` (i.e. the domain-separation tag followed by `compressed_len || compressed || padding`) using `sender_ed25519_public`. Reject the message on failure.
1601606. zstd-decompress the `compressed` bytes (only the `compressed_len`-length slice, not the trailing padding) to recover the plaintext payload.
161161
162162### Encrypted Blob Format
@@ -378,7 +378,7 @@ When a client connects, it proves ownership of its keypair:
378378 ```
379379 The challenge is a lowercase hex string of at least 32 chars (128 bits of entropy). The reference server uses 48 hex chars (192 bits). The challenge MUST be unique per challenge request and consumed on first use (verified).
380380
381−3. Client signs the **UTF-8 bytes of `"skrepka-auth-v1:" + server_host + ":" + challenge`** with its Ed25519 private key and submits. `server_host` is the bare lowercased hostname of the server the client dialed (no scheme, no port, no trailing dot). The `skrepka-auth-v1:` prefix is a domain-separation tag distinguishing auth signatures from message signatures (which sign `recipientPub ++ compressed`, §4):
381+3. Client signs the **UTF-8 bytes of `"skrepka-auth-v1:" + server_host + ":" + challenge`** with its Ed25519 private key and submits. `server_host` is the bare lowercased hostname of the server the client dialed (no scheme, no port, no trailing dot). The `skrepka-auth-v1:` prefix is a domain-separation tag distinguishing auth signatures from message signatures (which begin with `"skrepka-msg-v2:"` and sign `recipientPub ++ compressed`, §3):
382382 ```
383383 POST /auth/verify
384384 {
@@ -458,6 +458,7 @@ The client makes a blocking POST. The server holds the connection open until new
458458```
459459
460460- `cursor` is a server-side **strictly-monotonic sequence**, seeded from the wall-clock millisecond at assignment but bumped to `previous + 1` whenever messages arrive within the same millisecond. Two messages therefore never share a `receivedAt`, so advancing the cursor past one message can never silently drop another. Treat it as an opaque checkpoint, not a clock.
461+- The server clamps the supplied cursor to `max(0, min(max(t, curSeq), cursor))`, where `t` is the current wall-clock millisecond and `curSeq` is the last assigned sequence — note the ceiling is `max(t, curSeq)`, not just `curSeq`, so under idle the clamp allows acking up to the current wall-clock, while under sustained bursts (where the sequence has run ahead of the clock) it still refuses to ack beyond what has actually been issued.
461462- Advancing past a message acts as an **implicit ack**: when the next poll arrives with a `cursor` greater than or equal to a stored message's `receivedAt`, the server drops the stored message **and any pending federation forwards for it**.
462463- If events are available immediately, the server responds right away. Otherwise it parks the connection on a row-level wait against the mailbox and returns when a new matching message lands or after 25 s, whichever comes first.
463464- An empty page (`events: []`) is returned on timeout, with the cursor echoed back.
@@ -688,7 +689,9 @@ Skrepka protects message **content and sender identity** from servers and networ
688689| **Presence gossip leaks location** | Federated peers learn which server a user is currently connected to. A presence row is anchored for up to 90 min (`onlineGossipTtl`), which outlives the 1 h session, so location metadata — and forwards aimed at the user — can linger for up to ~90 min after the user disconnects. |
689690| **No durable delivery** | If the sender's server goes down or all forward retries fail, messages may be lost. |
690691| **Mailbox eviction under flood (DoS)** | The per-recipient mailbox cap (`maxMessagesPerRecipient`, default 10 000) evicts the oldest messages when the cap is reached. An authenticated attacker (pubkeys are free to mint) can flood a victim's mailbox with junk, pushing out genuine undelivered messages. The victim's client only sees junk; the genuine messages are permanently destroyed. At 60 requests/min × 100 messages = 6 000/min, the attacker fills the cap in ~2 minutes (per IP; multiple IPs are faster). This is an accepted design tradeoff of v0.1 — a real fix requires per-sender quotas or pricing. |
692+| **Unbounded mailbox storage** | The per-recipient mailbox cap (`maxMessagesPerRecipient`) bounds each recipient but not the count of distinct recipient keys. An attacker can fill the server's disk by sending messages to many distinct invalid pubkeys, bounded only by the 30-day TTL and rate limits. The reference server now enforces a global `maxMessagesTotal` cap (default 1 000 000) that evicts the globally-oldest rows on overflow, symmetric with the other state refs. |
691693| **CPU amplification via dedup scan** | `appendMessage` performs an O(n) full-scan dedup check on every message (§6). Under a burst of 100-message batches, each message triggers a scan of the entire mailbox. With large mailboxes this is quadratic and amplifies CPU cost per request. Bounded by rate limits (60/min per IP) and the fact that the attacker must be authenticated, but the amplification is real. |
694+| **Clock forward-jump pins the sequence counter** | The `receivedAt` sequence counter is a forward-only ratchet: `max(wall_clock, previous + 1)`. A single NTP forward-jump (server migration, misconfiguration) pins the counter at the jumped value, delaying all TTL pruning until real time catches up. Backward jumps are safe (the counter dominates). |
692695| **Challenge pool floodable (soft DoS)** | The challenge pool is globally capped at `maxChallenges` (default 10 000) with per-IP rate limiting. An attacker with many IPs — or behind a proxy with XFF spoofing when `trustForwardedFor = true` — can fill the pool with challenges for random pubkeys, causing legitimate challenges to be LRU-evicted. Authentication still works but with extra latency (clients retry). This is a soft DoS. |
693696| **Rate limit counts requests, not messages** | The per-IP send rate limit (60/min) counts requests, and each request can carry up to 100 messages. The effective message rate is therefore 60 × 100 = 6 000 messages/min per IP (bounded by the body-size cap), much higher than the per-request limit suggests. An attacker with multiple IPs can send millions of messages per minute. |
694697| **Session pool Sybil-floodable** | The global session pool (`maxSessions`, default 10 000) uses LRU eviction. An attacker with enough IPs can generate thousands of keypairs, authenticate each one, and fill the pool — evicting legitimate users' sessions. Users re-authenticate automatically (they get `401` and retry), so this is a soft DoS adding latency and load. Per-IP session caps were considered and rejected because they collapse behind carrier-grade NAT. |
@@ -698,7 +701,7 @@ Skrepka protects message **content and sender identity** from servers and networ
698701| **No identity revocation** | There is no in-protocol mechanism to revoke or invalidate a compromised identity key. A stolen key allows full takeover: decrypt all captured messages, send messages as the user, poll their mailbox, and receive their messages via presence redirect. The only defense is out-of-band communication. See §11 (Future Extensions — Key Rotation) for the intended direction. |
699702| **Unauthenticated session relay** (mitigated) | The auth signature is bound to the target server's hostname under a domain-separation tag (§6), so a malicious/relay server can no longer forward another server's challenge and replay the resulting signature to impersonate the client elsewhere. Residual caveat: the binding is to the hostname the client dialed, so it assumes the client reaches each server under its true `serverHost` (DNS/TLS integrity). |
700703| **Same-recipient replay** | Nothing stops a captured blob from being re-delivered to its original recipient. The damage is bounded per payload type: `text` is deduplicated by `id` (the reference client maintains a persistent `seen_ids` set that survives `trim_history`), `delivery.ack` is idempotent, and `profile` is guarded by the per-contact `ts` staleness check of §4. |
701−| **Open federation — gossip redirect** | `/federation/gossip` is unauthenticated; an attacker announces the victim online at a host it controls and origin relays forward the victim's queued ciphertext there. See *Open federation* below. |
704+| **Open federation — gossip redirect** | `/federation/gossip` is unauthenticated; an attacker announces the victim online at a host it controls and origin relays forward the victim's queued ciphertext there. **Active users are also affected**: even when the recipient is locally online and actively polling, a forged remote presence row causes each new incoming message to be duplicated to the attacker's server — the victim receives everything normally but the attacker silently collects ciphertext metadata for the live inbound stream. See *Open federation* below. |
702705| **Open federation — forward injection** | `/federation/forward` is unauthenticated; any host can inject arbitrary blobs into a currently-online recipient's mailbox. See *Open federation* below. |
703706| **Bearer token theft = full impersonation** | Sessions are bearer-token-based: anyone who obtains the token can use it. The token is bound to `(pubkey, ip, expiry)`, but the `pubkey` binding is for authorization (self-send rejection), not for authenticating the request. If an attacker steals a bearer token (e.g., from a compromised proxy's access log), they can poll the victim's mailbox and send messages as the victim — modulo IP binding. Token expiry (1 hour) is the only automatic recovery. Consider shorter token lifetimes or per-request signing. |
704707| **Token entropy depends on runtime CSPRNG** | Session and challenge tokens are generated by the server's runtime random source. Implementations SHOULD verify that their RNG is cryptographically secure (e.g., backed by `/dev/urandom` or an OS-provided CSPRNG). |
@@ -721,6 +724,8 @@ to every relay it can reach. Each relay holding mail for the victim now forwards
721724
722725**2. Forward injection — spam into an online recipient's mailbox.** `/federation/forward` is unauthenticated, so any host can inject arbitrary blobs into the mailbox of any recipient who is currently locally online. Injected junk is undecryptable, so a client drops it silently on ingest — but it occupies the recipient's poll stream and the relay's mailbox until the cursor advances past it. The only bounds are the per-source-IP rate limit (`forwardLimit`), the SSRF deny-list (which constrains where a relay will *send*, not who may call it), and the `no_presence` gate (which limits injection to online recipients).
723726
727+**3. Forward cancellation — deleting queued mail-in-flight.** The offline-event forward cancellation (§7) **widens** the unauthenticated-gossip blast radius from writing presence to **deleting queued forwards** — an attacker claiming `fromServer: X` can cancel pending forwards to any real relay X by announcing an `offline` event for the relevant key, so mail queued for delivery to X is dropped from the origin's outbox (the message itself stays in `*messages` and is re-forwarded if the key reappears on another peer, so this delays rather than destroys delivery).
728+
724729**Mitigation path.** Both attacks are accepted limitations of v0.1's unauthenticated federation. The intended fix is peer authentication — signed presence gossip and authenticated forward requests — or an explicit peer allow-list gate before `forwardQueuedToServer`. Until then, operators who federate only with trusted peers (or not at all) are unaffected.
725730
726731### Recommendations for Implementers
apple/SkrepkaApp/Effects.swift+65−12
@@ -5,19 +5,43 @@ import Skrepka
55
66// MARK: - HTTP
77
8−/// Cancels every HTTP redirect instead of following it.
8+/// Hard cap on any HTTP response body we will buffer. A hostile relay
9+/// can answer any endpoint with a multi-gigabyte body; without this cap
10+/// `URLSession.data(for:)` would OOM the app before the core's own
11+/// `MAX_POLL_RESPONSE_BYTES` check (which runs on the wrong side of the
12+/// FFI boundary to prevent the allocation) ever fires.
13+private let maxResponseBytes = 64 * 1024 * 1024 // 64 MiB
14+
15+/// The one session delegate, handling two defenses against an untrusted relay.
916///
10−/// URLSession replays the original request's headers onto a redirect target, including
11−/// `Authorization: Bearer <session_token>`. A relay is untrusted infrastructure: a hostile
12−/// (or compromised) one could answer `/poll` or `/messages` with a 302 to a host it chooses
17+/// **Redirect cancellation.** URLSession replays the original request's headers onto a
18+/// redirect target, including `Authorization: Bearer <session_token>`. A hostile (or
19+/// compromised) relay could answer `/poll` or `/messages` with a 302 to a host it chooses
1320/// and harvest the bearer token from the request we would dutifully re-send there. Sessions
1421/// are bound to a single relay by design (§6), so a cross-host redirect is never legitimate
15−/// — refusing it costs nothing and closes the token-exfiltration path.
22+/// — refusing it costs nothing and closes the token-exfiltration path. Returning `nil` from
23+/// the completion handler does not error the task: it stops the redirect and hands the 3xx
24+/// response itself back to the caller, which the core sees as a non-2xx status and treats as
25+/// a failed request.
1626///
17−/// Returning `nil` from the completion handler does not error the task: it stops the
18−/// redirect and hands the 3xx response itself back to the caller, which the core sees as a
19−/// non-2xx status and treats as a failed request.
20−private final class NoRedirectDelegate: NSObject, URLSessionTaskDelegate {
27+/// **Response size limiting.** As body chunks arrive we accumulate the byte count per task
28+/// and cancel the task the moment it crosses `maxResponseBytes`, so a multi-gigabyte body
29+/// never fully materializes in memory. Cancelling surfaces to the caller as a `URLError`
30+/// with code `.cancelled`, which `Http.perform` maps to a transport error.
31+private final class HttpSessionDelegate: NSObject, URLSessionTaskDelegate, URLSessionDataDelegate {
32+ let maxResponseBytes: Int
33+
34+ /// Per-task accumulated byte counts. The delegate is shared across every concurrent
35+ /// request, so a single shared counter would conflate their bodies — key by task instead.
36+ /// Guarded by `lock` because delegate callbacks arrive on the session's own (non-main)
37+ /// queue and may interleave across tasks.
38+ private var taskSizes: [URLSessionTask: Int] = [:]
39+ private let lock = NSLock()
40+
41+ init(maxResponseBytes: Int) {
42+ self.maxResponseBytes = maxResponseBytes
43+ }
44+
2145 func urlSession(
2246 _ session: URLSession,
2347 task: URLSessionTask,
@@ -27,18 +51,43 @@ private final class NoRedirectDelegate: NSObject, URLSessionTaskDelegate {
2751 ) {
2852 completionHandler(nil)
2953 }
54+
55+ func urlSession(
56+ _ session: URLSession,
57+ dataTask: URLSessionDataTask,
58+ didReceive data: Data
59+ ) {
60+ lock.lock()
61+ let total = (taskSizes[dataTask] ?? 0) + data.count
62+ taskSizes[dataTask] = total
63+ let exceeded = total > maxResponseBytes
64+ lock.unlock()
65+ if exceeded {
66+ dataTask.cancel()
67+ }
68+ }
69+
70+ func urlSession(
71+ _ session: URLSession,
72+ task: URLSessionTask,
73+ didCompleteWithError error: Error?
74+ ) {
75+ lock.lock()
76+ taskSizes.removeValue(forKey: task)
77+ lock.unlock()
78+ }
3079}
3180
3281enum Http {
3382 /// Strongly held by `session` for the app's lifetime — the session is never invalidated,
3483 /// so the delegate is never released. That is the intended shape here, not a leak.
35− private static let redirectDelegate = NoRedirectDelegate()
84+ private static let httpDelegate = HttpSessionDelegate(maxResponseBytes: maxResponseBytes)
3685
3786 /// A private, ephemeral session. `URLSession.shared` would hand every relay a
3887 /// persistent cookie jar and an on-disk response cache — a relay is untrusted
3988 /// infrastructure and must not be able to pin a stable identifier on the client
4089 /// or leave polled ciphertext lying around on disk. It would also follow redirects,
41− /// which is what `NoRedirectDelegate` is here to prevent.
90+ /// which is what `HttpSessionDelegate` is here to prevent.
4291 private static let session: URLSession = {
4392 let config = URLSessionConfiguration.ephemeral
4493 config.httpCookieAcceptPolicy = .never
@@ -48,7 +97,7 @@ enum Http {
4897 config.requestCachePolicy = .reloadIgnoringLocalAndRemoteCacheData
4998 config.timeoutIntervalForRequest = 70 // server long-polls for ~25s
5099 config.tlsMinimumSupportedProtocolVersion = .TLSv12
51− return URLSession(configuration: config, delegate: redirectDelegate, delegateQueue: nil)
100+ return URLSession(configuration: config, delegate: httpDelegate, delegateQueue: nil)
52101 }()
53102
54103 static func perform(_ req: HttpRequest) async -> HttpResult {
@@ -72,6 +121,10 @@ enum Http {
72121 headers: headers(of: http),
73122 body: [UInt8](data)
74123 ))
124+ } catch let error as URLError where error.code == .cancelled {
125+ // The size-limit delegate cancels the task when the body exceeds the cap.
126+ // Treat it as a transport error so the core backs off and retries.
127+ return .err(.io("response body exceeded \(maxResponseBytes) bytes"))
75128 } catch let error as URLError where error.code == .timedOut {
76129 return .err(.timeout)
77130 } catch {
apple/SkrepkaApp/PhotoPicker.swift+17−1
@@ -62,7 +62,23 @@ struct PhotoPicker: UIViewControllerRepresentable {
6262 guard let source = CGImageSourceCreateWithURL(url as CFURL, nil),
6363 let thumb = CGImageSourceCreateThumbnailAtIndex(source, 0, options as CFDictionary)
6464 else { return nil }
65− return UIImage(cgImage: thumb).jpegData(compressionQuality: 0.7)?.base64EncodedString()
65+ let image = UIImage(cgImage: thumb)
66+ // Reduce quality until the base64 fits within the protocol cap (MAX_PHOTO_LEN,
67+ // 64 KiB). A high-detail 256px frame can exceed the cap even at q0.7; without
68+ // this loop `SaveProfile` rejects the whole profile with no recourse for the user.
69+ let maxBase64Len = 64 * 1024
70+ var quality: CGFloat = 0.7
71+ while quality > 0.1 {
72+ if let jpegData = image.jpegData(compressionQuality: quality) {
73+ let base64 = jpegData.base64EncodedString()
74+ if base64.count <= maxBase64Len {
75+ return base64
76+ }
77+ }
78+ quality -= 0.1
79+ }
80+ // Last resort: the smallest quality that still produces something.
81+ return image.jpegData(compressionQuality: 0.1)?.base64EncodedString()
6682 }
6783 }
6884}
core/src/app.rs+75−15
@@ -105,6 +105,12 @@ const POLL_WATCHDOG_MS: u64 = 90_000;
105105/// one costs a redundant request.
106106const FLUSH_WATCHDOG_MS: u64 = 90_000;
107107
108+/// How long an auth round-trip may stay in flight before we assume the
109+/// shell lost it. Same rationale as `POLL_WATCHDOG_MS`: `authenticating`
110+/// gates the whole auth flow, and without a watchdog a dropped effect
111+/// leaves it stuck forever. Kept above the 70 s shell HTTP timeout.
112+const AUTH_WATCHDOG_MS: u64 = 90_000;
113+
108114/// Messages per `/messages` request.
109115///
110116/// The relay rejects a longer batch outright (`413 batch_too_large`,
@@ -259,6 +265,11 @@ pub enum Event {
259265 #[serde(skip)]
260266 #[facet(skip)]
261267 FlushWatchdog(u64),
268+ /// Fires `AUTH_WATCHDOG_MS` after generation `n`'s auth round-trip is
269+ /// issued; a no-op unless that exact auth is somehow still in flight.
270+ #[serde(skip)]
271+ #[facet(skip)]
272+ AuthWatchdog(u64),
262273 #[serde(skip)]
263274 #[facet(skip)]
264275 ChallengeResult(u64, #[facet(opaque)] HttpResult),
@@ -804,7 +815,29 @@ impl App for Skrepka {
804815 Event::LoadedMessages(peer, res) => {
805816 match parse_kv::<Vec<StoredMessage>>(res) {
806817 Ok(Some(list)) => {
807− model.messages.insert(peer, list);
818+ // Merge: a poll that beat this lazy load may have already
819+ // inserted messages for this peer. Overwriting with the
820+ // on-disk list (which the GET read before the poll wrote)
821+ // would silently destroy the freshly-ingested messages.
822+ // Union by id, keeping both the on-disk history and any
823+ // in-memory additions.
824+ match model.messages.get_mut(&peer) {
825+ Some(existing) => {
826+ let existing_ids: HashSet<&String> =
827+ existing.iter().map(|m| &m.id).collect();
828+ let new_msgs: Vec<StoredMessage> = list
829+ .into_iter()
830+ .filter(|m| !existing_ids.contains(&m.id))
831+ .collect();
832+ existing.extend(new_msgs);
833+ // Re-sort by ts to maintain the invariant.
834+ existing.sort_by_key(|m| m.ts);
835+ trim_history(existing);
836+ }
837+ None => {
838+ model.messages.insert(peer, list);
839+ }
840+ }
808841 }
809842 Ok(None) => {}
810843 Err(()) => {
@@ -1165,6 +1198,11 @@ impl App for Skrepka {
11651198 let gen = model.next_auth_gen();
11661199 req.build()
11671200 .then_send(move |r| Event::ChallengeResult(gen, r))
1201+ .and(
1202+ Time::notify_after(Duration::from_millis(AUTH_WATCHDOG_MS))
1203+ .0
1204+ .then_send(move |_| Event::AuthWatchdog(gen)),
1205+ )
11681206 .and(render())
11691207 }
11701208 Err(_) => {
@@ -1361,6 +1399,7 @@ impl App for Skrepka {
13611399 return self.backoff_poll(model);
13621400 }
13631401 };
1402+ let cursor_before = model.cursor;
13641403 let cmd = self.ingest_poll(model, parsed);
13651404 // The re-poll hangs off the *cursor write*, not off this event.
13661405 // Polling again is what acks the batch we just took — the server
@@ -1372,10 +1411,17 @@ impl App for Skrepka {
13721411 // `polling` stays `false`, and the loop is dead until the next
13731412 // foreground `Connect`. The watchdog fires `Poll` if the loop is
13741413 // still idle — same pattern as the poll/send HTTP watchdogs.
1375− cmd.and(
1414+ //
1415+ // Skip the cursor write when the cursor did not advance (an empty
1416+ // or fully-deduped page): there is nothing new to persist, so
1417+ // re-poll directly. The watchdog is still armed as a fallback.
1418+ let cursor_advanced = model.cursor != cursor_before;
1419+ cmd.and(if cursor_advanced {
13761420 KeyValue::set(K_CURSOR, json_bytes(&model.cursor))
1377− .then_send(Event::SavedCursor),
1378− )
1421+ .then_send(Event::SavedCursor)
1422+ } else {
1423+ Command::event(Event::Poll)
1424+ })
13791425 .and(
13801426 Time::notify_after(Duration::from_millis(POLL_WATCHDOG_MS))
13811427 .0
@@ -1402,6 +1448,15 @@ impl App for Skrepka {
14021448 model.abandon_flush();
14031449 self.flush_next(model)
14041450 }
1451+ Event::AuthWatchdog(gen) => {
1452+ // Only the auth this watchdog was armed for, and only while it is
1453+ // still the current one and still in flight.
1454+ if gen != model.auth_gen || !model.authenticating {
1455+ return Command::done();
1456+ }
1457+ model.abandon_auth();
1458+ Command::event(Event::Authenticate)
1459+ }
14051460 Event::SendResult(gen, Ok(resp)) => {
14061461 // A result for a send we already abandoned (a watchdog gave up
14071462 // on it, or a server change invalidated it). Dropping it is safe
@@ -1794,10 +1849,17 @@ impl Skrepka {
17941849 .insert(peer.clone(), Contact::new(peer.clone(), String::new(), ts));
17951850 }
17961851
1797− // On first contact, share our profile *before* the text so the
1798− // recipient's first impression is a name and avatar, not a truncated
1799− // @p that gets replaced a moment later. Both are queued behind any
1800− // in-flight items, so the ordering is preserved through the outbox.
1852+ let text = Payload::Text { id, body };
1853+ model.outbox.push_back(OutboxItem::new(
1854+ peer.clone(),
1855+ Arc::new(protocol::serialize_payload(&text, ts)),
1856+ ));
1857+
1858+ // On first contact, share our profile *after* the text. The text
1859+ // auto-creates the contact on the recipient's side; the profile that
1860+ // follows in the same batch then applies to the now-existing contact.
1861+ // (Previously profile was queued before the text, but the recipient's
1862+ // ingest drops profiles from non-contacts, so the profile was lost.)
18011863 if was_new {
18021864 let profile = Payload::Profile {
18031865 display_name: model.profile.display_name.clone(),
@@ -1810,12 +1872,6 @@ impl Skrepka {
18101872 ));
18111873 }
18121874
1813− let text = Payload::Text { id, body };
1814− model.outbox.push_back(OutboxItem::new(
1815− peer.clone(),
1816− Arc::new(protocol::serialize_payload(&text, ts)),
1817− ));
1818−
18191875 Command::all([
18201876 self.persist_messages(model, &peer),
18211877 self.persist_contacts(model),
@@ -2051,6 +2107,7 @@ impl Skrepka {
20512107 let now = now_ms();
20522108 let mut touched_convos: HashSet<String> = HashSet::new();
20532109 let mut contacts_dirty = false;
2110+ let mut outbox_dirty = false;
20542111 // sender hex -> message ids needing acks
20552112 let mut ack_targets: BTreeMap<String, Vec<String>> = BTreeMap::new();
20562113 // Q17: Per-sender deferred ack accumulation. Collected during the event
@@ -2352,6 +2409,7 @@ impl Skrepka {
23522409 Arc::new(protocol::serialize_payload(&payload, ack_ts)),
23532410 ));
23542411 }
2412+ outbox_dirty = true;
23552413 }
23562414
23572415 // Persist everything touched.
@@ -2366,7 +2424,9 @@ impl Skrepka {
23662424 if contacts_dirty {
23672425 cmds.push(self.persist_contacts(model));
23682426 }
2369− cmds.push(self.persist_outbox(model));
2427+ if outbox_dirty {
2428+ cmds.push(self.persist_outbox(model));
2429+ }
23702430 cmds.push(Command::event(Event::StartFlush));
23712431 Command::all(cmds)
23722432 }
core/src/crypto.rs+43−40
@@ -31,6 +31,13 @@ const HKDF_INFO: &[u8] = b"skrepka-v2";
3131/// Domain-separation tag for the auth challenge signature (PROTOCOL.md §6).
3232const AUTH_TAG: &str = "skrepka-auth-v1:";
3333
34+/// Domain-separation tag for per-message signatures (PROTOCOL.md §3).
35+/// Prevents cross-protocol signature reuse: an auth challenge signature
36+/// begins with "skrepka-auth-v1:" while a message signature begins with
37+/// this tag, so the same key cannot sign in one context and have it
38+/// validate in another.
39+const MSG_TAG: &[u8] = b"skrepka-msg-v2:";
40+
3441/// Wire-format version byte prepended to every encrypted blob (PROTOCOL.md §3).
3542///
3643/// Version 0x02 adds AEAD associated data (AAD): the cleartext header
@@ -58,9 +65,10 @@ fn aad(version: u8, eph_pub: &[u8; 32], nonce: &[u8; NONCE_LEN], recipient_pub:
5865pub const ED25519_SECRET_LEN: usize = 64;
5966pub const ED25519_PUBLIC_LEN: usize = 32;
6067pub const NONCE_LEN: usize = 24;
61−/// 1 (version) + 32 (eph) + 24 (nonce) + 32 (sender pub) + 64 (sig) + 4 (compressed_len)
62−/// + 16 (AEAD tag). The 4-byte compressed_len field sits inside the AEAD ciphertext.
63−/// The version byte, ephemeral pubkey, nonce, and recipient pubkey are AEAD AAD (v0x02).
68+/// `1+32+24+32+64+4+16` bytes = version, eph pubkey, nonce, sender pub, sig,
69+/// compressed_len, AEAD tag. The 4-byte compressed_len field sits inside the AEAD
70+/// ciphertext. The version byte, ephemeral pubkey, nonce, and recipient pubkey are
71+/// AEAD AAD (v0x02).
6472pub const MIN_BLOB_LEN: usize = 1 + 32 + NONCE_LEN + 32 + 64 + 4 + 16;
6573
6674/// Blob sizes are padded up to these boundaries so the on-wire length does not
@@ -233,11 +241,12 @@ impl Identity {
233241 fn with_x25519(secret_key: [u8; ED25519_SECRET_LEN]) -> Self {
234242 let x25519_priv = *ed25519_sk_to_x25519(&secret_key);
235243 let x25519_pub = x25519_dalek::x25519(x25519_priv, x25519_dalek::X25519_BASEPOINT_BYTES);
236− debug_assert!(
237− !x25519_pub.iter().all(|b| *b == 0),
238− "derived X25519 public key is all-zero (low-order point) — \
239− this identity cannot receive messages"
240− );
244+ if x25519_pub.iter().all(|b| *b == 0) {
245+ eprintln!(
246+ "warning: derived X25519 public key is all-zero (low-order point) — \
247+ this identity cannot receive messages"
248+ );
249+ }
241250 Identity {
242251 secret_key,
243252 x25519_priv,
@@ -346,7 +355,7 @@ fn decompress(data: &[u8]) -> Result<Vec<u8>, CryptoError> {
346355 decoder
347356 .window_log_max(21)
348357 .map_err(|_| CryptoError::Decompress)?;
349− let mut out = Vec::new();
358+ let mut out = Vec::with_capacity(MAX_PLAINTEXT_LEN + 1);
350359 (&mut decoder)
351360 .take(MAX_PLAINTEXT_LEN as u64 + 1)
352361 .read_to_end(&mut out)
@@ -394,7 +403,8 @@ pub fn encrypt(
394403
395404 // Sign recipient_pub || compressed_len_bytes || compressed || padding
396405 // (i.e. recipient_pub || everything after the 96-byte header in inner).
397− let mut signed = Zeroizing::new(Vec::with_capacity(32 + 4 + compressed.len() + padding.len()));
406+ let mut signed = Zeroizing::new(Vec::with_capacity(MSG_TAG.len() + 32 + 4 + compressed.len() + padding.len()));
407+ signed.extend_from_slice(MSG_TAG);
398408 signed.extend_from_slice(recipient_ed_pub);
399409 signed.extend_from_slice(&compressed_len_bytes);
400410 signed.extend_from_slice(&compressed);
@@ -505,7 +515,7 @@ pub fn decrypt(recipient: &Identity, blob: &[u8]) -> Result<Decrypted, CryptoErr
505515 inner[98],
506516 inner[99],
507517 ]) as usize;
508− if 100 + compressed_len > inner.len() {
518+ if compressed_len > inner.len().saturating_sub(100) {
509519 inner.zeroize();
510520 return Err(CryptoError::Decrypt);
511521 }
@@ -515,7 +525,8 @@ pub fn decrypt(recipient: &Identity, blob: &[u8]) -> Result<Decrypted, CryptoErr
515525 // (covers compressed_len_bytes || compressed || padding).
516526 let verifying = VerifyingKey::from_bytes(&sender_pub).map_err(|_| CryptoError::BadSignature)?;
517527 let signature = ed25519_dalek::Signature::from_bytes(&sig_bytes);
518− let mut signed = Zeroizing::new(Vec::with_capacity(32 + (inner.len() - 96)));
528+ let mut signed = Zeroizing::new(Vec::with_capacity(MSG_TAG.len() + 32 + (inner.len() - 96)));
529+ signed.extend_from_slice(MSG_TAG);
519530 signed.extend_from_slice(&recipient.public_key());
520531 signed.extend_from_slice(&inner[96..]);
521532 verifying
@@ -808,61 +819,53 @@ mod tests {
808819 assert!(id.sign_challenge("relay.example.com", &at_cap).is_ok());
809820 }
810821
811− /// N1: AAD prevents silent `to`-swap censorship. A blob encrypted to Bob
812− /// cannot be decrypted by Charlie even if a relay changes the envelope's
813− /// `to` field to Charlie's pubkey — the AAD binds the recipient identity,
814− /// so the AEAD tag check fails before any inner content is revealed.
822+ /// A blob encrypted to Bob cannot be decrypted by Charlie — the key
823+ /// derivation itself binds the recipient (the AEAD key is a function
824+ /// of the recipient's private key and the ephemeral public key). The
825+ /// AAD additionally authenticates the recipient pubkey, but the tag
826+ /// check would fail even without it, because Charlie derives a
827+ /// different key. The AAD is defense-in-depth: it makes the binding
828+ /// explicit rather than implicit in the KDF.
815829 #[test]
816− fn aad_prevents_to_swap_censorship() {
830+ fn wrong_recipient_cannot_decrypt_aead_fails() {
817831 let alice = Identity::generate(&mut rng(1));
818832 let bob = Identity::generate(&mut rng(2));
819833 let charlie = Identity::generate(&mut rng(3));
820834
821− // Alice encrypts a message to Bob.
822835 let blob = encrypt(&mut rng(4), &alice, &bob.public_key(), b"hi bob").unwrap();
823−
824− // Charlie tries to decrypt the blob meant for Bob. The AAD includes
825− // Bob's pubkey (from the blob's perspective the recipient is Bob),
826− // but Charlie's identity derives a different key. This must fail at
827− // AEAD decryption, not at signature verification.
828836 assert_eq!(
829837 decrypt(&charlie, &blob).unwrap_err(),
830838 CryptoError::Decrypt,
831− "wrong recipient must fail AEAD, not reach signature check"
839+ "wrong recipient must fail AEAD — key derivation binds the recipient"
832840 );
833841 }
834842
835− /// N1: Tampering with the version byte causes AEAD failure (the version
836− /// is part of the AAD, not just checked separately).
843+ /// Tampering with the version byte causes failure via the explicit
844+ /// version check (`blob[0] != WIRE_VERSION`) before the AEAD even runs.
845+ /// The AAD includes the version as defense-in-depth, but the explicit
846+ /// check fires first.
837847 #[test]
838− fn aad_binds_version_byte() {
848+ fn tampered_version_byte_is_rejected() {
839849 let alice = Identity::generate(&mut rng(1));
840850 let bob = Identity::generate(&mut rng(2));
841851 let mut blob = encrypt(&mut rng(3), &alice, &bob.public_key(), b"hi").unwrap();
842−
843− // Flip the version byte. The separate check (`blob[0] != WIRE_VERSION`)
844− // would catch this, but the AAD also includes it, so even if the
845− // version check were removed, AEAD would still fail.
846852 blob[0] ^= 0xff;
847853 assert_eq!(
848854 decrypt(&bob, &blob).unwrap_err(),
849855 CryptoError::Decrypt,
850− "version byte tampering must fail AEAD"
856+ "version byte tampering is rejected"
851857 );
852858 }
853859
854− /// N1: Tampering with the ephemeral pubkey causes AEAD failure (it's
855− /// part of the AAD, not just used for key derivation).
860+ /// Tampering with the ephemeral pubkey causes key derivation to produce
861+ /// a different key, so the AEAD tag check fails. The AAD additionally
862+ /// authenticates the eph_pub, but the KDF mismatch is what actually
863+ /// catches this.
856864 #[test]
857− fn aad_binds_ephemeral_pubkey() {
865+ fn tampered_ephemeral_pubkey_is_rejected() {
858866 let alice = Identity::generate(&mut rng(1));
859867 let bob = Identity::generate(&mut rng(2));
860868 let mut blob = encrypt(&mut rng(3), &alice, &bob.public_key(), b"hi").unwrap();
861−
862− // Corrupt one byte of the ephemeral pubkey. Even though the key
863− // derivation would produce a different key (causing AEAD failure
864− // anyway), the AAD also includes the eph_pub, so this is
865− // cryptographically bound to the ciphertext.
866869 blob[5] ^= 0x01;
867870 assert!(decrypt(&bob, &blob).is_err());
868871 }
server.knot+190−103
@@ -189,6 +189,15 @@ maxPresencePerServer = 5000
189189-- the same argument for sessions on `maxSessions`, and PROTOCOL.md §10.
190190maxMessagesPerRecipient : Int
191191maxMessagesPerRecipient = 10000
192+-- Global ceiling on total stored messages across all recipients (S6). Every
193+-- other state ref has a global cap (*sessions, *challenges, *presence,
194+-- *forwards); *messages had only a per-recipient cap, so total storage was
195+-- bounded only by (distinct recipient keys x maxMessagesPerRecipient), and an
196+-- attacker minting fresh invalid pubkeys could fill the disk (bounded only by
197+-- rate limits and the 30-day TTL). On overflow this evicts the globally-oldest
198+-- rows, symmetric with the other refs. See PROTOCOL.md §10.
199+maxMessagesTotal : Int
200+maxMessagesTotal = 1000000
192201-- Validity window for an auth challenge before it must be re-issued (1 min).
193202challengeExpiry : Int<Ms>
194203challengeExpiry = 60000
@@ -957,7 +966,15 @@ appendMessage = \toKey encryptedBlob t -> do
957966 let excess = max 0 (count forRecipient - (maxMessagesPerRecipient - 1))
958967 let evicted = map (\m -> m.encryptedBlob) (take excess (sortBy (\m -> m.receivedAt) forRecipient))
959968 let kept = filter (\m -> not (m.toKey == toKey && elem m.encryptedBlob evicted)) msgs
960− *messages = union kept [{toKey, encryptedBlob, receivedAt: seqTs, deliveredTo: []}])
969+ -- Global cap (S6): after the per-recipient eviction, bound the total mailbox
970+ -- across all recipients by dropping the globally-oldest rows. Only runs on
971+ -- overflow, so the O(n log n) sort is off the hot path. Dropped rows may
972+ -- belong to other recipients; their orphaned *forwards are reclaimed by
973+ -- `pruneExpired`, and their lost `deliveredTo` is harmless (the blob is gone).
974+ let globallyCapped = if count kept > maxMessagesTotal
975+ then drop (count kept - maxMessagesTotal) (sortBy (\m -> m.receivedAt) kept)
976+ else kept
977+ *messages = union globallyCapped [{toKey, encryptedBlob, receivedAt: seqTs, deliveredTo: []}])
961978
962979-- Record that `server` has accepted a federated forward of this message, so a
963980-- later presence re-announce of `toKey` doesn't re-forward the same blob to a peer
@@ -1073,56 +1090,70 @@ sendBatchedGossip = \peers events -> case localServerName of
10731090-- comes online: any messages already buffered for that pubkey are enqueued
10741091-- as forwards to the newly-known server.
10751092
1076−-- Attempt every forward in `batch` to one peer, in order, and report whether any
1077−-- attempt was a genuine peer failure. Drops successful entries from `*forwards` and
1078−-- records the peer on the message's `deliveredTo`; on failure bumps the retry count
1079−-- and gives up after `maxForwardRetries`.
1093+-- Attempt every forward in `batch` to one peer, in order, and report both whether
1094+-- any attempt was a genuine peer failure (`anyFailed`) and whether any attempt
1095+-- genuinely succeeded (`anySucceeded`). Drops successful entries from `*forwards`
1096+-- and records the peer on the message's `deliveredTo`; every attempt (success,
1097+-- failure, or 404) bumps the retry count and gives up after `maxForwardRetries`.
10801098--
10811099-- INVARIANT — this must not touch `*forwardFailures`. The caller records at most one
1082−-- failure per peer per sweep, from the Bool returned here; recording per failing
1083−-- *message* would let one unreachable peer with N buffered messages take N backoff
1084−-- steps in a single sweep and pin itself at `retryBackoffMax` for `forwardFailureMaxAge`.
1085−-- The counter counts consecutive failed *sweeps*.
1100+-- failure per peer per sweep, from the `anyFailed` flag returned here; recording per
1101+-- failing *message* would let one unreachable peer with N buffered messages take N
1102+-- backoff steps in a single sweep and pin itself at `retryBackoffMax` for
1103+-- `forwardFailureMaxAge`. The counter counts consecutive failed *sweeps*.
10861104--
10871105-- A 404 is NOT a peer failure. `handleForwardMessage` answers 404 `no_presence`
10881106-- whenever the recipient is not *currently* online at that peer, which is an
10891107-- expected, retryable outcome (PROTOCOL.md §7) — the peer is healthy and answering.
1090−-- But `fetch` returns `Err` for every status >= 400, so treating any `Err` as a
1091−-- failure would feed a routine 404 into `recordFailure`, and `*forwardFailures` is
1092−-- shared with gossip: one stale-presence 404 would suppress all forwards *and* all
1093−-- gossip to that peer for a full backoff step (30 s–8 min). So a 404 leaves the peer
1094−-- unpenalized and the forward row untouched — no retry bump either, since the row is
1095−-- waiting on the recipient's presence, not on a delivery that went wrong.
1108+-- `fetch` returns `Err` for every status >= 400, so treating any `Err` as a failure
1109+-- would feed a routine 404 into `recordFailure`, and `*forwardFailures` is shared
1110+-- with gossip: one stale-presence 404 would suppress all forwards *and* all gossip to
1111+-- that peer for a full backoff step (30 s–8 min). So a 404 leaves the peer's backoff
1112+-- untouched (`anyFailed` stays false for it) and never clears it (`anySucceeded`
1113+-- stays false too).
1114+--
1115+-- But a 404 DOES bump the forward's retry count now (S1). It previously left the row
1116+-- entirely untouched, so a forward whose recipient never came online at the peer was
1117+-- retried every sweep for the full 30-day message TTL — the counter never advanced,
1118+-- so `maxForwardRetries` was never reached. Counting the attempt (without charging
1119+-- the peer's backoff) lets the forward hit the cap and be dropped after
1120+-- `maxForwardRetries` sweeps while still not penalizing a healthy peer.
10961121--
10971122-- Recursive rather than `forEach` because the result has to be accumulated across
10981123-- iterations; `sortBy` fixes the iteration order that `head`/`drop 1` then walk.
1099−retryForwardsTo : Text -> [ForwardEntry] -> IO {rw *forwards, rw *messages, network} Bool
1124+retryForwardsTo : Text -> [ForwardEntry] -> IO {rw *forwards, rw *messages, network} {anyFailed: Bool, anySucceeded: Bool}
11001125retryForwardsTo = \server batch -> case head batch of
1101− Nothing {} -> yield (False {})
1126+ Nothing {} -> yield {anyFailed: False {}, anySucceeded: False {}}
11021127 Just {value: f} -> do
11031128 result <- fetch (federationUrl server) (ForwardMessage {f.toKey, f.encryptedBlob})
11041129 let status = case result of
11051130 Err {error: e} -> e.status
11061131 Ok _ -> 200
11071132 let success = status < 400
1108− -- Recipient simply isn't online at the peer yet: hold the row, blame no one.
1133+ -- Recipient simply isn't online at the peer yet: hold the peer blameless, but
1134+ -- still count the attempt so the row can't outlive the message TTL (S1).
11091135 let deferred = status == 404
11101136 let isMatch = forwardMatches f.toKey f.encryptedBlob server
11111137 let isDone = \x -> isMatch x && (success || x.retries + 1 >= maxForwardRetries)
11121138 atomic (do
1113− when (not deferred) (do
1114− cur <- *forwards
1115− *forwards = map (\x -> if isMatch x
1116− then {x.toServer, x.toKey, x.encryptedBlob, retries: x.retries + 1}
1117− else x) (filter (\x -> not isDone x) cur))
1139+ cur <- *forwards
1140+ -- Count the attempt regardless of outcome: a 404 still consumes a retry slot
1141+ -- so a forward that never finds the recipient online is eventually dropped
1142+ -- (S1). On success or after `maxForwardRetries`, `isDone` filters the row out
1143+ -- first, so the increment then no-ops on an absent row.
1144+ *forwards = map (\x -> if isMatch x
1145+ then {x.toServer, x.toKey, x.encryptedBlob, retries: x.retries + 1}
1146+ else x) (filter (\x -> not isDone x) cur)
11181147 when success (markDelivered f.toKey f.encryptedBlob server))
1119− restFailed <- retryForwardsTo server (drop 1 batch)
1120− yield ((not success && not deferred) || restFailed)
1148+ rest <- retryForwardsTo server (drop 1 batch)
1149+ yield {anyFailed: (not success && not deferred) || rest.anyFailed, anySucceeded: success || rest.anySucceeded}
11211150
11221151-- Walk pending forwards grouped by destination and retry each peer's backlog (up
11231152-- to maxBatchSize per peer) via `/federation/forward`. One failure record per peer
1124−-- per sweep: a sweep in which every attempt succeeded clears the peer's backoff, a
1125−-- sweep with any failure takes exactly one backoff step (see `retryForwardsTo`).
1153+-- per sweep: a sweep with any genuine failure takes exactly one backoff step; a
1154+-- sweep with at least one genuine success clears the peer's backoff; an all-404
1155+-- sweep leaves the backoff untouched, neither charging nor clearing it (S1). See
1156+-- `retryForwardsTo`.
11261157retryPendingForwards : IO {rw *forwardFailures, rw *forwards, rw *messages, network, clock} {}
11271158retryPendingForwards = do
11281159 t <- now
@@ -1144,13 +1175,17 @@ retryPendingForwards = do
11441175 -- `maxForwardRetries` (dropped). `ForwardEntry` carries no timestamp, so key on
11451176 -- the unique blob.
11461177 let batch = take maxBatchSize (sortBy (\f -> f.encryptedBlob) (filter (\f -> f.toServer == server) fwds))
1147− anyFailed <- retryForwardsTo server batch
1148− -- `count batch > 0` guards the clear: `maxBatchSize` is startup-overridable,
1149− -- and at 0 every batch is empty — an empty sweep must not be read as a
1150− -- success and reset a failing peer's backoff.
1151− if anyFailed
1178+ result <- retryForwardsTo server batch
1179+ -- Clear the peer's backoff only on a sweep with at least one genuine success
1180+ -- (S1). An all-404 sweep (recipient not online at the peer) is neither a
1181+ -- failure nor a success: clearing backoff on it would reset the peer to
1182+ -- "fresh" every sweep, firing a retry every `backgroundPruneInterval` for the
1183+ -- full 30-day message TTL. `count batch > 0` also guards against reading an
1184+ -- empty sweep (possible with a startup-overridden `maxBatchSize` of 0) as
1185+ -- meaningful.
1186+ if result.anyFailed
11521187 then recordFailure server t
1153− else when (count batch > 0) (clearFailure server t)))
1188+ else when (count batch > 0 && result.anySucceeded) (clearFailure server t)))
11541189
11551190-- Forward queued messages to a server when a remote key comes online.
11561191-- Skips messages already delivered to `server` (tracked via Message.deliveredTo)
@@ -1173,6 +1208,32 @@ forwardQueuedToServer = \pubkey serverName -> atomic (do
11731208 let newFwds = map (\m -> {toServer: server, m.toKey, m.encryptedBlob, retries: 0}) (take budget (sortBy (\m -> m.receivedAt) candidates))
11741209 *forwards = capForwards (union fwds newFwds))
11751210
1211+-- Batched version of `forwardQueuedToServer`: stage forwards for *all* newly-online
1212+-- keys in a single mailbox scan (S4). Calling `forwardQueuedToServer` per key scans
1213+-- `*messages` once per key, which is why `handleRecvGossip` previously capped the
1214+-- number of keys at `maxNewlyOnlineForwards` (N13) — but that cap silently deferred
1215+-- delivery past the 10th key, and a re-announce within the TTL never recovered it
1216+-- (the `deliveredTo`/presence state made the key look already-handled). A single
1217+-- pass bounds the cost to O(|messages|) regardless of how many keys came online.
1218+--
1219+-- SECURITY: shares `forwardQueuedToServer`'s exposure — it routes on gossip-written
1220+-- presence. The same `deliveredTo` / `notYetForwarded` / per-peer budget guards
1221+-- apply; do not widen them without revisiting the gossip-redirect note above.
1222+forwardQueuedToAll : [PubkeyHex] -> ServerName -> IO {rw *forwards, r *messages} {}
1223+forwardQueuedToAll = \pubkeys serverName -> atomic (do
1224+ let server = (serverName : Text)
1225+ msgs <- *messages
1226+ fwds <- *forwards
1227+ let existing = countWhere (\f -> f.toServer == server) fwds
1228+ let budget = max 0 (maxForwardsPerServer - existing)
1229+ let notYetForwarded = \m -> not any (forwardMatches m.toKey m.encryptedBlob server) fwds
1230+ let notYetDelivered = \m -> not any (\d -> d.server == server) m.deliveredTo
1231+ -- `elem m.toKey pubkeys` is O(|pubkeys|), but `pubkeys` is bounded by
1232+ -- `maxGossipEvents` (100), so the scan stays O(|messages|) overall.
1233+ let candidates = filter (\m -> elem m.toKey pubkeys && notYetForwarded m && notYetDelivered m) msgs
1234+ let newFwds = map (\m -> {toServer: server, m.toKey, m.encryptedBlob, retries: 0}) (take budget (sortBy (\m -> m.receivedAt) candidates))
1235+ *forwards = capForwards (union fwds newFwds))
1236+
11761237-- ============================================
11771238-- Handler: challenge / verify
11781239-- ============================================
@@ -1223,68 +1284,82 @@ handleVerifyAuth = \pubkey challenge signature revokeOthers clientIp -> do
12231284 -- relay server cannot replay the signature to a different server. We verify
12241285 -- against our own configured hostname (port stripped), which a relay cannot forge.
12251286 --
1226− -- Signature verification is deferred until *after* the challenge lookup confirms
1227− -- a matching challenge exists (inside the atomic below). This avoids spending
1228− -- expensive Ed25519 CPU on /auth/verify calls that have no matching challenge,
1229− -- which would otherwise be a cheap CPU-DoS vector. The trade-off is that
1230− -- `verify` runs inside the STM transaction and may re-run on retries — but
1231− -- retries only happen on concurrent writes to *challenges/*sessions, which
1232− -- are rare, so the expected retry count is ~1.
1287+ -- Signature verification is split out of the session-install transaction (S7).
1288+ -- Phase 1 is a cheap read-only atomic that confirms a matching, unexpired,
1289+ -- IP-bound challenge exists — the original CPU-DoS guard, which still spends no
1290+ -- Ed25519 CPU on /auth/verify calls that have no matching challenge. Phase 2 runs
1291+ -- the expensive `verify` *outside* any atomic, so an STM retry on the phase-3
1292+ -- install transaction (triggered by concurrent writes to *challenges/*sessions)
1293+ -- never re-executes it — previously `verify` sat inside that transaction and every
1294+ -- retry re-ran the Ed25519 check. Phase 3 re-checks the challenge before consuming
1295+ -- it, closing the TOCTOU window opened between phases 1 and 3.
12331296 let signedText = "skrepka-auth-v1:" ++ authHost serverHost ++ ":" ++ challenge
1234− result <- atomic (do
1297+ challengeOk <- atomic (do
12351298 ch <- *challenges
12361299 let same = \c -> c.pubkey == pubkey && c.challenge == challenge
1237− let matchingChallenge = findFirst same ch
1238− case matchingChallenge of
1239− Nothing {} -> yield (Nothing {})
1240− Just {value: c} -> do
1241− if not (c.expiresAt > t && matchIp clientIp c.ip)
1242− then yield (Nothing {})
1243− else do
1244− -- Verify the signature only now that a matching, unexpired, IP-bound
1245− -- challenge exists.
1246− let sigValid = case bytesFromHex pubkey of
1247− Just {value: pk} -> case bytesFromHex signature of
1248− Just {value: sigBytes} -> verify pk (textToBytes signedText) sigBytes
1249− _ -> False {}
1250− _ -> False {}
1251− if not sigValid
1252− then yield (Nothing {})
1253− else do
1254− -- Consume the challenge and optionally revoke other sessions + challenges.
1255− -- When `revokeOthers` is set, drop all other sessions AND outstanding
1256− -- challenges for this pubkey, so a captured challenge cannot be
1257− -- redeemed for a new session after revocation (Q35).
1258− *challenges = if revokeOthers
1259− then filter (\x -> not (x.pubkey == pubkey)) ch
1260− else filter (\x -> not same x) ch
1261− ses <- *sessions
1262− let pruned = if revokeOthers
1263− then filter (\s -> s.pubkey != pubkey) ses
1264− else capPerKey ses pubkey maxSessionsPerKey
1265− let newSession = {token, pubkey, expiresAt: expiry, ip: clientIp}
1266− -- LRU-evicts the soonest-to-expire session when the pool is full; this global
1267− -- eviction is Sybil-floodable, see the LIMITATION note on `maxSessions`.
1268− let pooled = union pruned [newSession]
1269− -- Count what `capList` is about to shed, so the handler can warn about it
1270− -- below. That flood has no other observable signature: evicted users just
1271− -- start seeing 401s and silently re-authenticate, which looks like normal
1272− -- traffic in the access log. A sustained nonzero rate here is the alert.
1273− let evicted = max 0 (count pooled - maxSessions)
1274− *sessions = capList pooled maxSessions
1275− peers <- refreshLocalPresence pubkey expiry
1276− yield (Just {value: {peers, evicted}}))
1277− case result of
1278− Nothing {} -> yield (Err {error: {status: 401, message: "invalid"}})
1279− Just {value: r} -> do
1280− -- Warn outside the `atomic` block, never inside: an STM transaction can
1281− -- retry, and a console write is not rolled back with it — logging inside
1282− -- would emit one line per retry attempt rather than one per eviction.
1283− -- Include the requesting pubkey and IP for audit (Q38): an operator
1284− -- investigating a Sybil flood needs to know which identities are responsible.
1285− when (r.evicted > 0) (logWarn ("session pool full: evicted " ++ show r.evicted ++ " session(s) to seat a new login (pubkey=" ++ pubkey ++ ", ip=" ++ clientIp ++ ", maxSessions=" ++ show maxSessions ++ ")"))
1286− when federationEnabled (fork (sendBatchedGossip r.peers [{eventType: "online", pubkey: Just {value: pubkey}}]))
1287− yield (Ok {value: {token, expiresAt: expiry}})
1300+ case findFirst ch same of
1301+ Nothing {} -> yield (False {})
1302+ Just {value: c} -> yield (c.expiresAt > t && matchIp clientIp c.ip))
1303+ if not challengeOk
1304+ then yield (Err {error: {status: 401, message: "invalid"}})
1305+ else do
1306+ -- Phase 2: verify OUTSIDE any atomic. Expensive, and must not be re-executed on
1307+ -- an STM retry of the install transaction below (S7).
1308+ let sigValid = case bytesFromHex pubkey of
1309+ Just {value: pk} -> case bytesFromHex signature of
1310+ Just {value: sigBytes} -> verify pk (textToBytes signedText) sigBytes
1311+ _ -> False {}
1312+ _ -> False {}
1313+ if not sigValid
1314+ then yield (Err {error: {status: 401, message: "invalid"}})
1315+ else do
1316+ -- Phase 3: re-check the challenge (it may have expired or been consumed by a
1317+ -- concurrent verify since phase 1), consume it, and install the session. The
1318+ -- body is cheap, so an STM retry here costs almost nothing.
1319+ result <- atomic (do
1320+ ch <- *challenges
1321+ let same = \c -> c.pubkey == pubkey && c.challenge == challenge
1322+ let matchingChallenge = findFirst ch same
1323+ case matchingChallenge of
1324+ Nothing {} -> yield (Nothing {})
1325+ Just {value: c} -> do
1326+ if not (c.expiresAt > t && matchIp clientIp c.ip)
1327+ then yield (Nothing {})
1328+ else do
1329+ -- Consume the challenge and optionally revoke other sessions + challenges.
1330+ -- When `revokeOthers` is set, drop all other sessions AND outstanding
1331+ -- challenges for this pubkey, so a captured challenge cannot be
1332+ -- redeemed for a new session after revocation (Q35).
1333+ *challenges = if revokeOthers
1334+ then filter (\x -> not (x.pubkey == pubkey)) ch
1335+ else filter (\x -> not same x) ch
1336+ ses <- *sessions
1337+ let pruned = if revokeOthers
1338+ then filter (\s -> s.pubkey != pubkey) ses
1339+ else capPerKey ses pubkey maxSessionsPerKey
1340+ let newSession = {token, pubkey, expiresAt: expiry, ip: clientIp}
1341+ -- LRU-evicts the soonest-to-expire session when the pool is full; this global
1342+ -- eviction is Sybil-floodable, see the LIMITATION note on `maxSessions`.
1343+ let pooled = union pruned [newSession]
1344+ -- Count what `capList` is about to shed, so the handler can warn about it
1345+ -- below. That flood has no other observable signature: evicted users just
1346+ -- start seeing 401s and silently re-authenticate, which looks like normal
1347+ -- traffic in the access log. A sustained nonzero rate here is the alert.
1348+ let evicted = max 0 (count pooled - maxSessions)
1349+ *sessions = capList pooled maxSessions
1350+ peers <- refreshLocalPresence pubkey expiry
1351+ yield (Just {value: {peers, evicted}}))
1352+ case result of
1353+ Nothing {} -> yield (Err {error: {status: 401, message: "invalid"}})
1354+ Just {value: r} -> do
1355+ -- Warn outside the `atomic` block, never inside: an STM transaction can
1356+ -- retry, and a console write is not rolled back with it — logging inside
1357+ -- would emit one line per retry attempt rather than one per eviction.
1358+ -- Include the requesting pubkey and IP for audit (Q38): an operator
1359+ -- investigating a Sybil flood needs to know which identities are responsible.
1360+ when (r.evicted > 0) (logWarn ("session pool full: evicted " ++ show r.evicted ++ " session(s) to seat a new login (pubkey=" ++ pubkey ++ ", ip=" ++ clientIp ++ ", maxSessions=" ++ show maxSessions ++ ")"))
1361+ when federationEnabled (fork (sendBatchedGossip r.peers [{eventType: "online", pubkey: Just {value: pubkey}}]))
1362+ yield (Ok {value: {token, expiresAt: expiry}})
12881363
12891364-- ============================================
12901365-- Handler: poll
@@ -1438,10 +1513,15 @@ handleSendMessages = \messages authorization clientIp -> do
14381513 Nothing {} -> do
14391514 committed <- atomic (do
14401515 pres <- *presence
1516+ -- S2: cap the immediate fan-out at `maxForwardsPerServer` servers per
1517+ -- recipient. The post-commit `forkForwardsTo` fires against this list
1518+ -- directly (bypassing the `*forwards` queue and its per-peer cap), so
1519+ -- without a bound here a recipient gossiped as present on thousands of
1520+ -- servers would spawn one immediate HTTP forward per server per message.
14411521 let staged = map (\m -> {
14421522 m.to,
14431523 m.encryptedBlob,
1444− remoteServers: collectPeers (filter (\p -> p.pubkey == m.to && p.expiresAt > t) pres) (\p -> p.server)
1524+ remoteServers: take maxForwardsPerServer (collectPeers (filter (\p -> p.pubkey == m.to && p.expiresAt > t) pres) (\p -> p.server))
14451525 }) messages
14461526 forEach staged (\c -> do
14471527 -- Read `*messages` *before* the append: `delivered` below must see the
@@ -1546,7 +1626,7 @@ forkForwardsTo = \to encryptedBlob servers -> forEach servers (\server -> when (
15461626-- fromServer) row touched by this batch, then re-adds fresh-TTL rows for
15471627-- pubkeys with an `online` event (so Online wins on collision). For keys
15481628-- that transition offline → online, flushes any locally-buffered messages via
1549−-- `forwardQueuedToServer`. Events with an unrecognized `eventType` or no
1629+-- `forwardQueuedToAll`. Events with an unrecognized `eventType` or no
15501630-- `pubkey` are skipped (forward-compat) rather than rejecting the batch.
15511631--
15521632-- SECURITY: this is the gossip-redirect ingress described in the section header
@@ -1557,7 +1637,7 @@ handleRecvGossip : [GossipEvent] -> ServerName -> IO {r *messages, rw *forwards,
15571637handleRecvGossip = \events fromServer -> do
15581638 t <- now
15591639 -- Widen the peer-supplied `ServerName` once (see `type ServerName`);
1560− -- `forwardQueuedToServer` below still takes the refined name.
1640+ -- `forwardQueuedToAll` below still takes the refined name.
15611641 let from = (fromServer : Text)
15621642 if not federationEnabled
15631643 then yield (Err {error: {status: 403, message: "federation_disabled"}})
@@ -1598,21 +1678,28 @@ handleRecvGossip = \events fromServer -> do
15981678 let localRows = filter (\p -> isLocalServer p.server) merged
15991679 let remoteRows = filter (\p -> not isLocalServer p.server) merged
16001680 *presence = union localRows (capList (capPerServer remoteRows from maxPresencePerServer) maxPresence)
1681+ -- S8: Make online+offline for the same key mutually exclusive within one
1682+ -- batch. If a batch carries both events for a key, online wins — the presence
1683+ -- merge above already added the online row — so that key must NOT also travel
1684+ -- the offline path and cancel its own freshly-staged forwards.
1685+ let onlineKeys = map (\p -> p.pubkey) collected.added
1686+ let offlineOnly = filter (\pk -> not elem pk onlineKeys) collected.offline
16011687 -- N14: Cancel pending forwards to `from` for offline keys. Without this,
16021688 -- forwards to a peer that just went offline retry for up to 80 minutes,
16031689 -- wasting maxForwardsPerServer slots and generating failed HTTP requests.
16041690 -- The message stays in *messages, so it's re-forwarded if the key
16051691 -- re-appears on another peer.
1606− when (count collected.offline > 0) (do
1692+ when (count offlineOnly > 0) (do
16071693 fwds <- *forwards
1608− *forwards = filter (\f -> not (f.toServer == from && elem f.toKey collected.offline)) fwds)
1694+ *forwards = filter (\f -> not (f.toServer == from && elem f.toKey offlineOnly)) fwds)
16091695 yield (filter (\pk -> not any (\p -> p.pubkey == pk && p.server == from && p.expiresAt > t) pres) (map (\p -> p.pubkey) collected.added)))
1610− -- N13: Cap the number of newly-online keys that trigger forward sweeps.
1611− -- Without this, 100 online events = 100 mailbox scans. The remaining keys
1612− -- are still in `*presence` — their forwards will be staged when a poll
1613− -- refreshes presence or the next gossip re-announces them.
1614− let toForward = take maxNewlyOnlineForwards newlyOnline
1615− fork (forEach toForward (\pk -> forwardQueuedToServer pk fromServer))
1696+ -- S4: forward queued mail for ALL newly-online keys in one mailbox pass. The
1697+ -- old per-key cap (maxNewlyOnlineForwards, N13) bounded amplification but
1698+ -- silently deferred delivery past the 10th key, and a re-announce within the
1699+ -- TTL never recovered it (the deliveredTo/presence state made the key look
1700+ -- already-handled). `forwardQueuedToAll` bounds the cost to O(|messages|)
1701+ -- regardless of how many keys came online.
1702+ fork (forwardQueuedToAll newlyOnline fromServer)
16161703 yield (Ok {value: {}})
16171704
16181705-- POST /federation/forward: receive a single federated message destined for
server.onew file

Binary file not shown.