Skip to content

Commit 3216b02

Browse files
jlucaso1jlucaso1
committed
fix(signal): adversarial-review fixes (DoS, capture gap, input integrity, perf)
- merge_seeds: O(n) insert (existing-index set) instead of O(n²) linear find per seed — removes an authenticated CPU amplification on a large gap. - snapshot_skip: capture new-DH-ratchet gaps decrypted via a promoted previous session too (export self-validation drops wrong-session candidates); gate the previous-session loop on previous_session_count() so the common case is free. - migration: invalid base64 now fails the import instead of silently producing an empty (undecryptable) session. - sidecar key cap raised to wacore's 2050 ceiling so it never drops a key the record still holds. - repersist failure is logged (was silently swallowed); still best-effort. - importLegacySession: doc note that the record is export-round-trip only.
1 parent 8fc5a3e commit 3216b02

3 files changed

Lines changed: 349 additions & 31 deletions

File tree

‎RFC-expose-session-info.md‎

Lines changed: 276 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,276 @@
1+
# RFC: Expose session metadata (`baseKey` + `registrationId`) from the WASM `SessionRecord`
2+
3+
| | |
4+
|---|---|
5+
| **Status** | Proposed — ready for evaluation & implementation |
6+
| **Target repo** | `whatsapp-rust-bridge` (`/home/jlucaso/projects/whatsapp-rust-bridge`) |
7+
| **Driven by** | Baileys (`/home/jlucaso/projects/Baileys`) libsignal → WASM migration (PR `feat-libsignal-wasm`) |
8+
| **Bridge version at time of writing** | npm `0.5.5` (proposed release: `0.5.6`) |
9+
| **Core dep** | `wacore-libsignal` from `github.com/jlucaso1/whatsapp-rust` (locked commit `a03672e`) |
10+
11+
---
12+
13+
## 1. Summary
14+
15+
Add one read-only method, `sessionInfo()`, to the WASM-exported `SessionRecord` class in the bridge. It deserializes the stored session and returns `{ baseKey: Uint8Array, registrationId: number }` for the current open session, or `null` when there is no open session.
16+
17+
All the underlying data **already exists** in the `wacore-libsignal` `SessionRecord` type the bridge wraps; it simply is not crossing the WASM boundary today. This is a small additive change (≈15–25 lines of Rust + a rebuild). No protobuf/schema changes are needed.
18+
19+
---
20+
21+
## 2. Background & context
22+
23+
**Baileys** (a TypeScript WhatsApp Web library) is migrating its Signal protocol implementation from the native JS `libsignal` package to this bridge (`whatsapp-rust-bridge`, Rust→WASM). In the migration, the JS Signal session storage went from "native `SessionRecord` objects" to "opaque serialized bytes (`Uint8Array`) handed to/from the WASM `SignalStorage` adapter".
24+
25+
An adversarial code review of that migration found a behavioral regression (internally tracked as **finding C**):
26+
27+
- The native `libsignal-node` `SessionRecord` exposed `getOpenSession()` whose `indexInfo.baseKey` and `registrationId` Baileys read to power two **retry protections** in incoming-message handling.
28+
- The bridge's WASM `SessionRecord` exposes only `deserialize()`, `serialize()`, and `haveOpenSession()` — **not** the open session's `baseKey` or `registrationId`.
29+
- Because Baileys' `loadSession` now returns raw bytes, its `getSessionInfo()` can no longer obtain those fields and is forced to return `null` always, silently disabling both protections.
30+
31+
This RFC proposes restoring the capability **at the bridge layer**, because that is the only place the data is reachable.
32+
33+
### The two Baileys retry protections that depend on this
34+
35+
In `/home/jlucaso/projects/Baileys/src/Socket/messages-recv.ts` (around lines 1378–1405), `signalRepository.getSessionInfo(participant)` is used to:
36+
37+
1. **Registration-id mismatch** — if the stored session's `registrationId` differs from the `registration` attribute on an incoming retry receipt, the stale session is deleted so it can be re-established.
38+
2. **Base-key collision on retry** — `messageRetryManager.saveBaseKey()` / `hasSameBaseKey()` use the session `baseKey` to detect when a peer is replaying against a session we've already rotated, forcing a fresh session.
39+
40+
Both callers already guard on a `null` result, so today the code is *safe* but *degraded*: these protections are no-ops under the WASM backend.
41+
42+
---
43+
44+
## 3. Problem statement
45+
46+
The Baileys consumer (`/home/jlucaso/projects/Baileys/src/Signal/libsignal.ts`) currently looks like this (post-review, with the degradation made explicit and documented):
47+
48+
```ts
49+
// repository.getSessionInfo
50+
async getSessionInfo() {
51+
// The WASM SignalStorage persists sessions as opaque serialized bytes and the
52+
// whatsapp-rust-bridge SessionRecord API exposes only haveOpenSession()/serialize() —
53+
// it does NOT expose the open session's baseKey or registrationId. The
54+
// registration-id-mismatch and base-key-collision retry protections in messages-recv
55+
// therefore cannot be evaluated under the WASM backend; both call sites guard on a
56+
// null result and simply skip those checks (graceful degradation).
57+
// TODO(libsignal-wasm): expose baseKey + registrationId via the bridge (or persist
58+
// them alongside the session) to restore these retry protections.
59+
return null
60+
}
61+
```
62+
63+
The interface contract it must satisfy (`/home/jlucaso/projects/Baileys/src/Types/Signal.ts`):
64+
65+
```ts
66+
getSessionInfo(jid: string): Promise<{ baseKey: Uint8Array; registrationId: number } | null>
67+
```
68+
69+
We want the bridge to provide enough surface that Baileys can implement this faithfully again.
70+
71+
---
72+
73+
## 4. Why this belongs in the bridge (the data is already there)
74+
75+
The bridge's WASM `SessionRecord` wrapper already deserializes the core type internally. See `/home/jlucaso/projects/whatsapp-rust-bridge/src/session_record.rs`:
76+
77+
```rust
78+
use wacore_libsignal::protocol::SessionRecord as CoreSessionRecord;
79+
80+
#[wasm_bindgen(js_name = SessionRecord)]
81+
pub struct SessionRecord {
82+
pub(crate) serialized_data: Vec<u8>,
83+
}
84+
85+
// existing method — proves the deserialize+query pattern already used here:
86+
#[wasm_bindgen(js_name = haveOpenSession)]
87+
pub fn have_open_session(&self) -> bool {
88+
CoreSessionRecord::deserialize(&self.serialized_data)
89+
.map(|record| record.session_state().is_some())
90+
.unwrap_or(false)
91+
}
92+
```
93+
94+
The core `CoreSessionRecord` (from `wacore-libsignal`) **already exposes** exactly what is needed. Evidence (absolute path, commit `a03672e`):
95+
96+
`/home/jlucaso/.cargo/git/checkouts/whatsapp-rust-acce85bd6aff6984/a03672e/wacore/libsignal/src/protocol/state/session.rs`
97+
98+
| Need | Core method | Line |
99+
|---|---|---|
100+
| open-session presence (→ `null` case) | `SessionRecord::session_state(&self) -> Option<&SessionState>` | `:691` |
101+
| `registrationId` | `SessionRecord::remote_registration_id(&self) -> Result<u32, SignalProtocolError>` | `:854` |
102+
| `baseKey` | `SessionRecord::alice_base_key(&self) -> Result<&[u8], SignalProtocolError>` | `:918` |
103+
104+
`alice_base_key` is the `libsignal-node` `indexInfo.baseKey` equivalent (the X3DH base key that indexes the session). It is stored on **both** the initiator and responder sides (set in the `SessionState` constructor at `state/session.rs:136`, `alice_base_key: Some(alice_base_key.serialize().to_vec())`), so it is a stable per-session identifier — which is exactly how the Baileys collision/mismatch checks use it.
105+
106+
> ⚠️ Line numbers above are for the **locked** checkout `a03672e`. When you implement, confirm the methods still exist on whatever `wacore-libsignal` revision the bridge currently builds against (the `main` branch may have moved). The method *names* are the stable contract.
107+
108+
---
109+
110+
## 5. Proposed change
111+
112+
**File to modify:** `/home/jlucaso/projects/whatsapp-rust-bridge/src/session_record.rs`
113+
Add a new method to the existing `impl SessionRecord` block. Two implementation styles are already established in this codebase; pick whichever fits the house style. **Option A (Tsify) is recommended** for a clean, typed `.d.ts`.
114+
115+
### Option A — Tsify struct (recommended)
116+
117+
The bridge already uses `tsify_next::Tsify` for typed return objects — see `/home/jlucaso/projects/whatsapp-rust-bridge/src/key_helper.rs` (`use tsify_next::Tsify;`, `#[derive(Serialize, Tsify)] #[tsify(into_wasm_abi)]`, `#[tsify(type = "Uint8Array")]`). Mirror that:
118+
119+
```rust
120+
use serde::Serialize;
121+
use tsify_next::Tsify;
122+
123+
#[derive(Debug, Clone, Serialize, Tsify)]
124+
#[tsify(into_wasm_abi)]
125+
pub struct SessionInfo {
126+
#[tsify(type = "Uint8Array")]
127+
pub base_key: Vec<u8>,
128+
pub registration_id: u32,
129+
}
130+
131+
#[wasm_bindgen(js_class = SessionRecord)]
132+
impl SessionRecord {
133+
/// Returns the open session's base key + remote registration id, or `null`
134+
/// (`undefined` on the JS side) when there is no current/open session.
135+
#[wasm_bindgen(js_name = sessionInfo)]
136+
pub fn session_info(&self) -> Result<Option<SessionInfo>, JsValue> {
137+
let record = CoreSessionRecord::deserialize(&self.serialized_data)
138+
.map_err(|e| JsValue::from_str(&e.to_string()))?;
139+
140+
if record.session_state().is_none() {
141+
return Ok(None);
142+
}
143+
144+
let base_key = record
145+
.alice_base_key()
146+
.map_err(|e| JsValue::from_str(&e.to_string()))?
147+
.to_vec();
148+
let registration_id = record
149+
.remote_registration_id()
150+
.map_err(|e| JsValue::from_str(&e.to_string()))?;
151+
152+
Ok(Some(SessionInfo { base_key, registration_id }))
153+
}
154+
}
155+
```
156+
157+
Generated TS (in `/home/jlucaso/projects/whatsapp-rust-bridge/pkg/whatsapp_rust_bridge.d.ts`):
158+
159+
```ts
160+
export interface SessionInfo { base_key: Uint8Array; registration_id: number; }
161+
export class SessionRecord {
162+
// ...existing...
163+
sessionInfo(): SessionInfo | undefined;
164+
}
165+
```
166+
167+
> Note the field names will be `snake_case` (`base_key`, `registration_id`) unless you add `#[serde(rename_all = "camelCase")]` to the struct. **Recommended:** add `#[serde(rename_all = "camelCase")]` so the JS surface is `{ baseKey, registrationId }`, which lets Baileys consume it directly without remapping.
168+
169+
### Option B — `Object` + `Reflect` (matches `session_cipher.rs`)
170+
171+
If you prefer to avoid a new struct, mirror the `{ type, body }` return already in `/home/jlucaso/projects/whatsapp-rust-bridge/src/session_cipher.rs` (around line 63, `Object::new()` + `Reflect::set`, with `#[wasm_bindgen(... typescript_type = "...")]`):
172+
173+
```rust
174+
#[wasm_bindgen(js_name = sessionInfo)]
175+
pub fn session_info(&self) -> Result<JsValue, JsValue> {
176+
let record = CoreSessionRecord::deserialize(&self.serialized_data)
177+
.map_err(|e| JsValue::from_str(&e.to_string()))?;
178+
if record.session_state().is_none() {
179+
return Ok(JsValue::NULL);
180+
}
181+
let base_key = record.alice_base_key().map_err(|e| JsValue::from_str(&e.to_string()))?;
182+
let registration_id = record.remote_registration_id().map_err(|e| JsValue::from_str(&e.to_string()))?;
183+
184+
let obj = js_sys::Object::new();
185+
js_sys::Reflect::set(&obj, &JsValue::from_str("baseKey"), &js_sys::Uint8Array::from(base_key))?;
186+
js_sys::Reflect::set(&obj, &JsValue::from_str("registrationId"), &JsValue::from_f64(registration_id as f64))?;
187+
Ok(obj.into())
188+
}
189+
```
190+
191+
For a typed surface with Option B, attach a `typescript_type = "{ baseKey: Uint8Array; registrationId: number } | null"` annotation (see how `session_cipher.rs` annotates its `EncryptResult`).
192+
193+
### Semantics to preserve
194+
195+
- **No open session ⇒ `null`** (not an empty object, not a throw). Baileys treats `null` as "no info, skip the checks".
196+
- `alice_base_key()` on the core `SessionState` does `unwrap_or(&[])` (`state/session.rs:143`), so an established-but-base-key-less state would yield an empty slice. This is acceptable: Baileys checks `info.baseKey` length/truthiness. If you want to be stricter, return `null` when `base_key.is_empty()` — your call.
197+
198+
---
199+
200+
## 6. Baileys-side consumption (the contract you must satisfy)
201+
202+
Once the bridge ships, `/home/jlucaso/projects/Baileys/src/Signal/libsignal.ts` `getSessionInfo` becomes (with `#[serde(rename_all = "camelCase")]` / Option B):
203+
204+
```ts
205+
async getSessionInfo(jid: string) {
206+
const addr = jidToSignalProtocolAddress(jid).toString()
207+
const session = await storage.loadSession(addr) // Uint8Array | null
208+
if (!session) {
209+
return null
210+
}
211+
return SessionRecord.deserialize(session).sessionInfo() ?? null
212+
}
213+
```
214+
215+
That restores the `{ baseKey, registrationId } | null` contract and re-enables both retry protections. (This Baileys-side change is **out of scope for the bridge agent** — it's listed only so you understand the consumer's expected shape.)
216+
217+
---
218+
219+
## 7. Build, test, release
220+
221+
**Build** (from `/home/jlucaso/projects/whatsapp-rust-bridge`):
222+
223+
```bash
224+
bun run build # prebuild + build:wasm (wasm-pack, dual simd/nosimd) + build:ts + tsc
225+
```
226+
227+
`build:wasm` (`scripts/build-wasm.mjs`) runs `wasm-pack build --target web --out-dir pkg --no-pack --no-opt` and regenerates `pkg/whatsapp_rust_bridge.d.ts`. Verify the new `sessionInfo` / `SessionInfo` appears there.
228+
229+
**Test** — add a case to `/home/jlucaso/projects/whatsapp-rust-bridge/test/session_record.test.ts`. Use `/home/jlucaso/projects/whatsapp-rust-bridge/test/session_builder.test.ts` and `test/helpers/fake_storage.ts` as references for building a *real* established session (process a prekey bundle / run a SessionBuilder), then assert:
230+
231+
- a record with an open session returns `{ baseKey: Uint8Array(len>0), registrationId: <expected u32> }`;
232+
- a freshly-`deserialize([])`'d / empty record returns `null` (mirrors `haveOpenSession() === false`);
233+
- ideally, parity: the `baseKey` bytes and `registrationId` match what `@whiskeysockets/libsignal-node` (already a devDependency) reports via `getOpenSession().indexInfo.baseKey` / `registrationId` for the same established session — this directly proves drop-in equivalence with what Baileys previously read.
234+
235+
**Release**: bump `package.json` `0.5.5 → 0.5.6`, publish, then bump `whatsapp-rust-bridge` in `/home/jlucaso/projects/Baileys/package.json` (currently pinned `0.5.4`) and re-run Baileys' `yarn lint && yarn test`.
236+
237+
---
238+
239+
## 8. Acceptance criteria
240+
241+
- [ ] `SessionRecord.sessionInfo()` exists on the WASM class and is in `pkg/whatsapp_rust_bridge.d.ts`.
242+
- [ ] Returns `{ baseKey: Uint8Array, registrationId: number }` for an open session; `null`/`undefined` when no open session.
243+
- [ ] JS field names are `baseKey` / `registrationId` (camelCase) so Baileys consumes the object directly.
244+
- [ ] New test in `test/session_record.test.ts` covering open-session and no-session cases (parity vs `libsignal-node` if practical).
245+
- [ ] `bun run build` succeeds; existing tests still pass.
246+
- [ ] Version bumped to `0.5.6`.
247+
248+
---
249+
250+
## 9. Risks / open questions
251+
252+
1. **`alice_base_key` vs native `indexInfo.baseKey`** — believed equivalent (both are the X3DH base key stored in the session structure; native libsignal indexes sessions by it). The parity test against `libsignal-node` is the cheapest way to confirm exact byte equivalence. If they ever differ, what matters for Baileys is only that the value is a *stable per-session identifier*, which `alice_base_key` is.
253+
2. **Empty base key** — `alice_base_key()` returns `&[]` when unset; decide whether to surface `null` in that edge case (see §5).
254+
3. **`wacore-libsignal` revision drift** — confirm the three core methods (`session_state`, `remote_registration_id`, `alice_base_key`) exist on the revision the bridge currently builds against; names are the stable contract, line numbers are not.
255+
4. **`Result<Option<Tsify>, JsValue>` ergonomics** — confirm this return shape generates the intended `SessionInfo | undefined` with the bridge's `tsify_next`/`wasm-bindgen` versions; if awkward, fall back to Option B.
256+
257+
---
258+
259+
## Appendix — absolute file reference map
260+
261+
**Bridge (`whatsapp-rust-bridge`)**
262+
- Modify: `/home/jlucaso/projects/whatsapp-rust-bridge/src/session_record.rs`
263+
- Style refs: `/home/jlucaso/projects/whatsapp-rust-bridge/src/key_helper.rs` (Tsify), `/home/jlucaso/projects/whatsapp-rust-bridge/src/session_cipher.rs` (Object+Reflect)
264+
- Generated types: `/home/jlucaso/projects/whatsapp-rust-bridge/pkg/whatsapp_rust_bridge.d.ts`
265+
- Build script: `/home/jlucaso/projects/whatsapp-rust-bridge/scripts/build-wasm.mjs`
266+
- Test target: `/home/jlucaso/projects/whatsapp-rust-bridge/test/session_record.test.ts` (helpers in `test/helpers/`)
267+
- Version: `/home/jlucaso/projects/whatsapp-rust-bridge/package.json`
268+
269+
**Core (`wacore-libsignal`, commit `a03672e`)**
270+
- `/home/jlucaso/.cargo/git/checkouts/whatsapp-rust-acce85bd6aff6984/a03672e/wacore/libsignal/src/protocol/state/session.rs`
271+
- `session_state()` `:691` · `remote_registration_id()` `:854` · `alice_base_key()` `:918` · base-key set in ctor `:136`
272+
273+
**Baileys (consumer)**
274+
- `/home/jlucaso/projects/Baileys/src/Signal/libsignal.ts` (`getSessionInfo`)
275+
- `/home/jlucaso/projects/Baileys/src/Types/Signal.ts` (`getSessionInfo` contract)
276+
- `/home/jlucaso/projects/Baileys/src/Socket/messages-recv.ts` (callers, ~lines 1378–1405)

0 commit comments

Comments
 (0)