From 8c1b6324d2451c4de31b956bd7b64aa99a892b1d Mon Sep 17 00:00:00 2001 From: kafkade <179981006+kafkade@users.noreply.github.com> Date: Mon, 27 Jul 2026 21:11:23 -0700 Subject: [PATCH] vault identifiers --- CHANGELOG.md | 4 + .../Sources/Views/Sync/SyncSettingsView.swift | 52 +- .../src/app/vault/settings/SyncSettings.tsx | 133 ++++- apps/web/src/contexts/VaultContext.tsx | 63 +- apps/web/src/lib/sync.ts | 28 +- apps/web/src/lib/wasm.ts | 9 +- apps/web/src/types/wasm.d.ts | 3 +- .../swift/Sources/LdgrSwift/ServerSync.swift | 20 +- crates/ldgr-cli/src/commands/devices.rs | 151 ++++- crates/ldgr-cli/src/commands/sync.rs | 419 ++++++++++++-- crates/ldgr-cli/src/sync/bridge.rs | 306 +++++++++- crates/ldgr-cli/tests/server_sync.rs | 10 +- crates/ldgr-cli/tests/sync_round_trip.rs | 5 +- crates/ldgr-core/src/storage/sync.rs | 87 +++ crates/ldgr-core/src/sync/mod.rs | 4 + crates/ldgr-core/src/sync/server/client.rs | 21 +- crates/ldgr-core/src/sync/server/protocol.rs | 33 +- crates/ldgr-core/src/sync/vault_id.rs | 143 +++++ crates/ldgr-ffi/src/sync.rs | 33 +- crates/ldgr-server/src/api/vaults.rs | 37 +- crates/ldgr-server/src/storage/mod.rs | 111 +++- crates/ldgr-server/src/storage/schema.rs | 7 + crates/ldgr-server/tests/common/mod.rs | 7 + .../ldgr-server/tests/device_pairing_e2e.rs | 2 +- .../ldgr-server/tests/ffi_sync_client_e2e.rs | 14 +- .../tests/server_sync_client_e2e.rs | 6 +- crates/ldgr-server/tests/srp_handshake.rs | 7 +- crates/ldgr-server/tests/sync_pipeline_e2e.rs | 2 +- crates/ldgr-server/tests/tenant_isolation.rs | 544 ++++++++++++++++++ crates/ldgr-wasm/src/sync.rs | 23 +- docs/adr/008-self-hosting-and-account-auth.md | 5 + .../011-tenant-scoped-vault-identifiers.md | 202 +++++++ docs/security/threat-model.md | 27 +- docs/self-hosting.md | 28 + docs/sync-setup.md | 41 +- 35 files changed, 2401 insertions(+), 186 deletions(-) create mode 100644 crates/ldgr-core/src/sync/vault_id.rs create mode 100644 crates/ldgr-server/tests/tenant_isolation.rs create mode 100644 docs/adr/011-tenant-scoped-vault-identifiers.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 87a66aa..a205166 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -108,11 +108,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Resolving a sync conflict as "keep remote" now actually re-applies the remote change (via the Swift/iOS bindings it previously only marked the conflict resolved without materializing the remote version) - Sync against a self-hosted `ldgr-server` now lists and pulls every encrypted batch and snapshot in large vaults: blob listings were previously capped at a single server page (~1000 entries), so a vault with more than one page of batches or snapshots would silently drop — and never pull — the overflow. The client now follows the server's continuation cursor across all pages - The `apps/web` admin panel static export builds again: the build now targets the webpack compiler the project's WASM config requires, and Tailwind CSS is aligned to the v3 toolchain its config is written against, so `npm run build` produces the static panel instead of failing +- `ldgr sync pull` no longer reports "Already up to date" while remote batches exist. The CLI built its server transport from the vault id recorded in `sync-config.json` but handed the push/pull engine a freshly recomputed one; when those diverged (after moving or renaming a vault directory) the transport matched neither list prefix and silently returned an empty page. Both now come from a single persisted identifier +- `ldgr sync status` now shows the vault ID for every provider, not just when the config happens to record one ### Security - The local working store (`vault.db`) is now encrypted at rest with SQLCipher (AES-256): the database is keyed by a subkey derived from your vault key via HKDF, so account names, transaction descriptions, amounts, and commodities are no longer readable from disk without your master password. This applies to both the CLI and the shared Rust core used by the iOS/iPadOS app, so anyone with filesystem access to a locked vault — including another local user or a stolen locked laptop — sees only ciphertext. Existing plaintext CLI vaults are upgraded with `ldgr migrate` - The unlocked session key is no longer written to a plaintext `session.json`; it is stored in the operating system keystore (macOS Keychain, Linux kernel keyring, or Windows Credential Manager) while `session.json` holds only non-secret metadata. `ldgr lock` now removes the key from the keystore and clears the session, so locking renders the working store unreadable rather than merely dropping a marker +- Vault identifiers are now random, unguessable, and scoped to the account that owns them, so accounts can no longer collide or interfere on a shared server (ADR-011). Previously the CLI derived a vault's identifier as a djb2 hash of its **directory path** and recomputed it on every command, while the iOS and web apps asked the user to **type one in**. Because nearly every vault lives at the default path, nearly every user derived the *same* identifier — and it was a global primary key, so the first account to register it claimed it server-wide. Any other account then got a conflict that `ldgr sync setup` silently swallowed, after which every push and pull failed with a 404: a permanent, unexplained denial of service, and one an attacker could trigger deliberately since the identifiers were trivially guessable. Identifiers are now 128 bits of CSPRNG output, minted once and persisted inside the vault, and a request for an identifier another account already owns is answered with a freshly minted one instead of a conflict — so squatting achieves nothing and no account can be locked out. Existing vaults keep their identifiers (including Dropbox/WebDAV vaults, which store none, by adopting their path-derived value once and freezing it), so nothing already synced is orphaned. A new device now converges by *adopting* your vault — `ldgr sync setup` adopts the vault registered to your account (asking which, if you have several), and `ldgr devices join` identifies the paired vault by trial-decrypting a batch with the key it just received — instead of relying on two devices happening to hash the same path. The typed "Vault ID" field is gone from the iOS and web sync settings +- Explicit cross-tenant authorization tests now cover every vault-scoped endpoint — batches, snapshots, and devices (read, write, list, and delete) — plus relay offers, asserting that a second authenticated account is refused with `404 Not Found` rather than `403 Forbidden` so the identifier namespace cannot be probed for existence. Authorization was already enforced on all of these; the tests fail loudly if a future handler forgets the ownership check - First-admin bootstrap is now atomic on servers with no `LDGR_ADMIN_EMAIL` configured: the first registered user is elected admin inside a single guarded database transaction, so under concurrent sign-ups at most one admin can ever be created. The previous fallback counted existing users and inserted the new user in separate steps, so two simultaneous registrations could both read zero users and both become admin (and on an unconfigured internet-exposed instance, whoever registered first silently became admin). The env-seeded `LDGR_ADMIN_EMAIL` path is unchanged ## [1.2.0] - 2026-05-30 diff --git a/apps/ios/ldgr/Sources/Views/Sync/SyncSettingsView.swift b/apps/ios/ldgr/Sources/Views/Sync/SyncSettingsView.swift index 4410076..e9e9c3b 100644 --- a/apps/ios/ldgr/Sources/Views/Sync/SyncSettingsView.swift +++ b/apps/ios/ldgr/Sources/Views/Sync/SyncSettingsView.swift @@ -14,6 +14,8 @@ struct SyncSettingsView: View { // Server connection form (non-secret fields are persisted; secrets are not). @State private var baseURL = "" @State private var username = "" + /// Identifier this device's vault is known by on the server. Issued by the + /// server or adopted from the account (ADR-011) — never typed by the user. @State private var vaultId = "" @State private var password = "" @State private var secretKeyInput = "" @@ -142,7 +144,11 @@ struct SyncSettingsView: View { private var connectedServerView: some View { LabeledContent("Server", value: baseURL) LabeledContent("Account", value: username) - LabeledContent("Vault", value: vaultId) + LabeledContent("Vault") { + Text(vaultId) + .font(.caption.monospaced()) + .foregroundStyle(.secondary) + } Button(role: .destructive) { signOut() } label: { @@ -191,11 +197,6 @@ struct SyncSettingsView: View { .autocorrectionDisabled() SecureField("Password", text: $password) .textContentType(.password) - TextField("Vault ID", text: $vaultId) - #if os(iOS) - .textInputAutocapitalization(.never) - #endif - .autocorrectionDisabled() if info.twoSecretAuth { twoSecretButtons(info) @@ -297,9 +298,7 @@ struct SyncSettingsView: View { // MARK: - Form helpers private var canSubmit: Bool { - !username.trimmingCharacters(in: .whitespaces).isEmpty - && !vaultId.trimmingCharacters(in: .whitespaces).isEmpty - && !password.isEmpty + !username.trimmingCharacters(in: .whitespaces).isEmpty && !password.isEmpty } private func loadConfig() { @@ -451,7 +450,7 @@ struct SyncSettingsView: View { // MARK: - Shared sign-in tail - /// Persist the session token + device id, enroll the vault, save the config, + /// Persist the session token + device id, claim the vault, save the config, /// and (re)configure the sync manager. Shared by every auth path. private func finishSignIn( session: LdgrSyncSession, @@ -462,16 +461,15 @@ struct SyncSettingsView: View { throw SyncSettingsError.noToken } - // Best-effort enrollment: create the vault if it doesn't exist yet. - _ = try? await session.createVault(vaultId: vault) - + let resolvedVault = try await claimVault(session: session, existing: vault) let deviceId = try client.syncStatus().deviceId try KeychainManager.storeServerAuthToken(token) try KeychainManager.storeServerDeviceId(deviceId) ServerConfigStore.save( - ServerConfig(baseURL: baseURL, username: user, vaultId: vault) + ServerConfig(baseURL: baseURL, username: user, vaultId: resolvedVault) ) + vaultId = resolvedVault password = "" serverInfo = nil @@ -479,6 +477,32 @@ struct SyncSettingsView: View { await syncManager.refreshStatus(client: client) } + /// Decide which server vault this device syncs to, and claim it (ADR-011). + /// + /// A device that has already synced keeps its identifier. One that has not + /// adopts the account's existing vault rather than creating a second, empty + /// one it would never converge out of. With several vaults on the account + /// the newest is taken, which is right for the case that actually occurs — + /// a fresh device joining the vault its owner most recently created. + /// + /// The identifier the server returns is authoritative: when the requested + /// one is already owned by another account the server mints a substitute + /// instead of failing, so no account can lock another out. + private func claimVault(session: LdgrSyncSession, existing: String) async throws -> String { + var requested: String? = existing.isEmpty ? nil : existing + + if requested == nil { + // A listing failure must not be read as "this account owns no + // vaults" — that would mint a second, empty vault and persist it, + // stranding this device away from the user's real data with no way + // back. Fail the sign-in instead so it can simply be retried. + let owned = try await session.listVaults() + requested = owned.max(by: { $0.createdAt < $1.createdAt })?.id + } + + return try await session.createVault(vaultId: requested) + } + private func signOut() { try? KeychainManager.deleteServerAuthToken() try? KeychainManager.deleteServerDeviceId() diff --git a/apps/web/src/app/vault/settings/SyncSettings.tsx b/apps/web/src/app/vault/settings/SyncSettings.tsx index b361ee8..e1dfc93 100644 --- a/apps/web/src/app/vault/settings/SyncSettings.tsx +++ b/apps/web/src/app/vault/settings/SyncSettings.tsx @@ -4,6 +4,7 @@ import { useEffect, useState } from 'react'; import { useVault } from '@/contexts/VaultContext'; import EmergencyKitView from '@/components/EmergencyKitView'; import type { EmergencyKit, ServerInfo } from '@/lib/wasm'; +import { NEW_VAULT, type RemoteVault } from '@/lib/sync'; const cardClass = 'rounded-lg border border-[var(--color-border)] bg-[var(--color-surface)] p-5 space-y-3'; @@ -32,12 +33,21 @@ export default function SyncSettings() { signInServer, logoutServer, createRemoteVault, + listRemoteVaults, sync, resolveSyncConflict, } = useVault(); const [serverUrl, setServerUrl] = useState(''); + // The vault identifier is issued by the server and adopted from the account + // (ADR-011) — never typed. `vaultId` is empty until this browser has claimed + // or adopted one. const [vaultId, setVaultId] = useState(''); + const [ownedVaults, setOwnedVaults] = useState([]); + // Distinguishes "no vaults on this account" from "we haven't looked yet", so + // the connect button can't be clicked into minting a vault before the picker + // has had a chance to appear. + const [vaultsLoading, setVaultsLoading] = useState(false); const [username, setUsername] = useState(''); const [password, setPassword] = useState(''); const [secretKey, setSecretKey] = useState(''); @@ -56,6 +66,32 @@ export default function SyncSettings() { } }, [serverConfig]); + // Once signed in, discover which vaults the account already owns so a browser + // with no identifier of its own can adopt one instead of stranding itself in a + // second, empty vault. + useEffect(() => { + if (!serverAuthenticated || vaultId) { + setOwnedVaults([]); + setVaultsLoading(false); + return; + } + let cancelled = false; + setVaultsLoading(true); + listRemoteVaults() + .then((vaults) => { + if (!cancelled) setOwnedVaults(vaults); + }) + .catch(() => { + if (!cancelled) setOwnedVaults([]); + }) + .finally(() => { + if (!cancelled) setVaultsLoading(false); + }); + return () => { + cancelled = true; + }; + }, [serverAuthenticated, vaultId, listRemoteVaults]); + const run = async (label: string, fn: () => Promise) => { setBusy(label); setError(null); @@ -113,10 +149,23 @@ export default function SyncSettings() { setMessage('Logged in.'); }); + // Claim a vault for this browser. With no identifier yet and exactly one + // vault on the account, `createRemoteVault` adopts it; otherwise the server + // mints a fresh, random, unguessable one (ADR-011). const handleCreateVault = () => run('create-vault', async () => { - await createRemoteVault(); - setMessage('Remote vault ready.'); + const granted = await createRemoteVault( + ownedVaults.length > 1 ? NEW_VAULT : undefined, + ); + setVaultId(granted); + setMessage(`Vault ready: ${granted}`); + }); + + const handleAdoptVault = (id: string) => + run('create-vault', async () => { + const granted = await createRemoteVault(id); + setVaultId(granted); + setMessage(`Now syncing vault ${granted}.`); }); const handleSync = () => @@ -175,18 +224,14 @@ export default function SyncSettings() { }} /> - + {vaultId && ( +

+ Vault {vaultId} +

+ )} + {!serverInfo && ( - +
+ {/* Several vaults on one account (#296): the user picks which one + this browser syncs to, rather than typing an identifier. */} + {!vaultId && ownedVaults.length > 1 && ( +
+

+ This account has more than one vault. Choose the one to sync + in this browser: +

+ {ownedVaults.map((v) => ( + + ))} +
+ )} + +
+ {!vaultId && ( + + )} + +
)} diff --git a/apps/web/src/contexts/VaultContext.tsx b/apps/web/src/contexts/VaultContext.tsx index 26796dd..275a2d8 100644 --- a/apps/web/src/contexts/VaultContext.tsx +++ b/apps/web/src/contexts/VaultContext.tsx @@ -37,7 +37,9 @@ import { listOpenConflicts, resolveConflict, makeFetchCallback, + NEW_VAULT, type ServerConfig, + type RemoteVault, type SyncOutcome, type ConflictRow, } from '@/lib/sync'; @@ -103,7 +105,13 @@ interface VaultContextValue { secretKey?: string | null, ) => Promise; logoutServer: () => Promise; - createRemoteVault: () => Promise; + /** + * Claim or adopt this browser's server vault, returning the identifier the + * server put in force. Pass an id to adopt a specific vault of the account's. + */ + createRemoteVault: (adoptVaultId?: string) => Promise; + /** The vaults this account owns, newest first. */ + listRemoteVaults: () => Promise; sync: () => Promise; resolveSyncConflict: (id: string, keepRemote: boolean) => Promise; @@ -470,17 +478,65 @@ export function VaultProvider({ children }: { children: ReactNode }) { await saveVault(); }, [db, saveVault]); - const createRemoteVault = useCallback(async () => { + /** + * Claim this browser's vault on the server (ADR-011). + * + * A browser that has already synced keeps its identifier. One that has not + * adopts the account's existing vault instead of creating a second, empty one + * — otherwise it would never converge with the user's other devices. With + * several vaults on the account (#296) the caller must pick one; we take the + * most recently created as the default and surface the rest for selection. + * + * The identifier the server returns is authoritative: when the requested one + * is already owned by another account the server mints a substitute rather + * than failing, so no account can lock another out. + */ + /** The vaults this account owns, newest first. */ + const listRemoteVaults = useCallback(async (): Promise => { if (!db || !serverConfig) throw new Error('Configure the server first'); const client = clientRef.current ?? buildClient(serverConfig, loadToken(db)); clientRef.current = client; - await client.createVault(serverConfig.vaultId); + const parsed = JSON.parse(await client.listVaults()) as RemoteVault[]; + return parsed.sort((a, b) => b.created_at.localeCompare(a.created_at)); }, [db, serverConfig, buildClient]); + const createRemoteVault = useCallback( + async (adoptVaultId?: string): Promise => { + if (!db || !serverConfig) throw new Error('Configure the server first'); + const client = + clientRef.current ?? buildClient(serverConfig, loadToken(db)); + clientRef.current = client; + + let requested = adoptVaultId === NEW_VAULT ? '' : (adoptVaultId ?? serverConfig.vaultId); + if (!requested && adoptVaultId !== NEW_VAULT) { + const owned = await listRemoteVaults(); + if (owned.length === 1) { + requested = owned[0].id; + } else if (owned.length > 1) { + // Minting here would strand this browser in a fresh, empty vault with + // no UI to switch away from it. Make the caller choose explicitly. + throw new Error( + 'This account has several vaults — choose which one to sync here.', + ); + } + } + + const granted = await client.createVault(requested || undefined); + const updated = { ...serverConfig, vaultId: granted }; + saveServerConfig(db, updated); + setServerConfig(updated); + await saveVault(); + return granted; + }, + [db, serverConfig, buildClient, listRemoteVaults, saveVault], + ); + const sync = useCallback(async (): Promise => { if (!wasm || !vault || !db || !serverConfig) throw new Error('Sync is not configured'); + if (!serverConfig.vaultId) + throw new Error('No vault claimed yet — connect this browser to a vault first'); const client = clientRef.current ?? buildClient(serverConfig, loadToken(db)); clientRef.current = client; @@ -604,6 +660,7 @@ export function VaultProvider({ children }: { children: ReactNode }) { signInServer, logoutServer, createRemoteVault, + listRemoteVaults, sync, resolveSyncConflict, getBalanceReport, diff --git a/apps/web/src/lib/sync.ts b/apps/web/src/lib/sync.ts index 7f9453f..e0276ae 100644 --- a/apps/web/src/lib/sync.ts +++ b/apps/web/src/lib/sync.ts @@ -62,9 +62,27 @@ const OP_TO_WIRE: Record = { export interface ServerConfig { serverUrl: string; username: string; + /** + * Identifier this vault is known by on the server (ADR-011). Empty until the + * account is signed in and a vault has been claimed or adopted — it is never + * typed by the user, and never derived from anything guessable. + */ vaultId: string; } +/** + * Sentinel for `createRemoteVault`: mint a brand-new vault instead of adopting + * one, even when the account already owns several. Distinct from `''`, which + * means "work out which vault to adopt". + */ +export const NEW_VAULT = '\u0000new'; + +/** A vault the signed-in account owns, as returned by `listVaults`. */ +export interface RemoteVault { + id: string; + created_at: string; +} + export interface SyncOutcome { pushed: number; applied: number; @@ -688,19 +706,21 @@ export async function runSync( export function loadServerConfig(db: Database): ServerConfig | null { const serverUrl = getState(db, CFG_SERVER_URL); - const vaultId = getState(db, CFG_VAULT_ID); - if (!serverUrl || !vaultId) return null; + if (!serverUrl) return null; return { serverUrl, username: getState(db, CFG_USERNAME) ?? '', - vaultId, + // Absent until the first successful claim; sync refuses to run without it. + vaultId: getState(db, CFG_VAULT_ID) ?? '', }; } export function saveServerConfig(db: Database, cfg: ServerConfig): void { setState(db, CFG_SERVER_URL, cfg.serverUrl); setState(db, CFG_USERNAME, cfg.username); - setState(db, CFG_VAULT_ID, cfg.vaultId); + // Only ever overwrite a known identifier with another known one — a config + // save that predates the first claim must not wipe it. + if (cfg.vaultId) setState(db, CFG_VAULT_ID, cfg.vaultId); } /** diff --git a/apps/web/src/lib/wasm.ts b/apps/web/src/lib/wasm.ts index 1b63242..cf7d29c 100644 --- a/apps/web/src/lib/wasm.ts +++ b/apps/web/src/lib/wasm.ts @@ -93,7 +93,14 @@ export interface WasmSyncClient { serverInfo(): Promise; /** Liveness probe (`GET /server/ping`); returns a JSON string. */ ping(): Promise; - createVault(vaultId: string): Promise; + /** + * Claim a vault, returning the identifier the server put in force. Pass + * `undefined` to have the server mint a random one (ADR-011); the result is + * authoritative and may differ from `vaultId`. + */ + createVault(vaultId?: string): Promise; + /** List this account's vaults; returns a JSON array of `{ id, created_at }`. */ + listVaults(): Promise; putBatch( vaultId: string, deviceId: string, diff --git a/apps/web/src/types/wasm.d.ts b/apps/web/src/types/wasm.d.ts index 25f1942..cde7af9 100644 --- a/apps/web/src/types/wasm.d.ts +++ b/apps/web/src/types/wasm.d.ts @@ -56,7 +56,8 @@ declare module '*pkg/ldgr_wasm' { ): Promise; serverInfo(): Promise; ping(): Promise; - createVault(vaultId: string): Promise; + createVault(vaultId?: string): Promise; + listVaults(): Promise; putBatch( vaultId: string, deviceId: string, diff --git a/bindings/swift/Sources/LdgrSwift/ServerSync.swift b/bindings/swift/Sources/LdgrSwift/ServerSync.swift index 65d5fc0..f64c1bf 100644 --- a/bindings/swift/Sources/LdgrSwift/ServerSync.swift +++ b/bindings/swift/Sources/LdgrSwift/ServerSync.swift @@ -19,7 +19,7 @@ /// let client = LdgrSync.makeClient(baseURL: URL(string: "https://sync.example.com")!) /// _ = try await client.register(username: "alice", password: Data("pw".utf8)) /// try await client.login(username: "alice", password: Data("pw".utf8)) -/// try await client.createVault(vaultId: "vault-1") +/// let vaultId = try await client.createVault() /// // `ciphertext` is an encrypted batch blob produced by the vault crypto layer. /// _ = try await client.putBatch(vaultId: "vault-1", deviceId: "dev-a", /// batchId: "batch-0001", ciphertext: ciphertext) @@ -518,12 +518,26 @@ public final class LdgrSyncSession: @unchecked Sendable { } } - /// Create a vault on the server (idempotent enrollment). Returns its path. + /// Claim a vault on the server, returning the identifier it put in force. + /// + /// Pass `nil` to have the server mint a random identifier (ADR-011). The + /// returned identifier is authoritative and may differ from `vaultId`: when + /// the requested one is already owned by another account the server mints a + /// substitute rather than failing, so no account can lock another out. + /// Callers must persist what they get back. @discardableResult - public func createVault(vaultId: String) async throws -> String { + public func createVault(vaultId: String? = nil) async throws -> String { try await mapping { try await client.createVault(vaultId: vaultId) } } + /// The vaults this account owns. + /// + /// A device with no identifier of its own adopts one of these rather than + /// creating a second, empty vault it would never converge out of. + public func listVaults() async throws -> [FfiVault] { + try await mapping { try await client.listVaults() } + } + /// Register/refresh this device's record on the server. public func putDevice(vaultId: String, deviceId: String, encryptedInfo: Data) async throws { try await mapping { diff --git a/crates/ldgr-cli/src/commands/devices.rs b/crates/ldgr-cli/src/commands/devices.rs index 4b8185c..bc98131 100644 --- a/crates/ldgr-cli/src/commands/devices.rs +++ b/crates/ldgr-cli/src/commands/devices.rs @@ -21,11 +21,12 @@ use std::time::Duration; use anyhow::{Context, Result, bail}; +use ldgr_core::sync::framing::open_batch_with_session_key; use ldgr_core::sync::pairing::{ Initiation, JoinerSession, PairingCode, deliver_vault_key, initiate_pairing, poll_joiner_hello, poll_vault_key, respond_pairing, }; -use ldgr_core::sync::server::ServerSyncClient; +use ldgr_core::sync::server::{ListBatchesQuery, ServerSyncClient}; use ldgr_core::sync::transport::{DeviceInfo, TransportConfig}; use crate::sync::server::ReqwestSender; @@ -51,7 +52,15 @@ struct ServerContext { /// Device pairing is only available on the self-hosted `ldgr-server` transport, /// so this fails with a clear message for other providers or when sync has not /// been set up. -fn load_server_context(vault_dir: &Path) -> Result { +/// +/// When `conn` is supplied the vault identifier comes from the vault itself +/// (ADR-011), which is authoritative — a device that adopted a different vault +/// while pairing has updated it there but not in `sync-config.json`. Callers +/// that hold no unlocked connection fall back to the configured copy. +fn load_server_context( + vault_dir: &Path, + conn: Option<&rusqlite::Connection>, +) -> Result { let config_path = vault_dir.join("sync-config.json"); if !config_path.exists() { bail!( @@ -63,7 +72,7 @@ fn load_server_context(vault_dir: &Path) -> Result { let config: TransportConfig = serde_json::from_str(&json).context("failed to parse sync config")?; - let (base_url, vault_id) = match config { + let (base_url, configured_vault_id) = match config { TransportConfig::Server { base_url, vault_id, .. } => (base_url, vault_id), @@ -74,6 +83,11 @@ fn load_server_context(vault_dir: &Path) -> Result { ), }; + let vault_id = match conn { + Some(conn) => crate::sync::bridge::resolve_vault_id(conn, vault_dir)?, + None => configured_vault_id, + }; + let creds_path = vault_dir.join(CREDENTIALS_FILE); if !creds_path.exists() { bail!( @@ -108,7 +122,7 @@ pub fn run_list(vault_path: &Path) -> Result<()> { let vault_dir = crate::session::resolve_vault_dir(Some(vault_path)); let this_device = crate::sync::bridge::resolve_device_id(&conn, &vault_dir).ok(); - let ctx = load_server_context(&vault_dir)?; + let ctx = load_server_context(&vault_dir, Some(&conn))?; let rt = runtime()?; let devices = rt .block_on(ctx.client.list_devices(&ctx.vault_id)) @@ -168,10 +182,10 @@ fn hex_decode(s: &str) -> Option> { /// code + copyable pairing token. Once the new device joins, the vault key is /// encrypted under the shared secret and delivered over the relay. pub fn run_add(vault_path: &Path) -> Result<()> { - let (_conn, vault_key) = crate::db::require_unlocked_db_with_key(vault_path)?; + let (conn, vault_key) = crate::db::require_unlocked_db_with_key(vault_path)?; let vault_dir = crate::session::resolve_vault_dir(Some(vault_path)); - let ctx = load_server_context(&vault_dir)?; + let ctx = load_server_context(&vault_dir, Some(&conn))?; let rt = runtime()?; rt.block_on(async { @@ -221,7 +235,14 @@ pub fn run_add(vault_path: &Path) -> Result<()> { /// Consumes the pairing code shown by `ldgr devices add`, derives the shared /// secret, and waits for the encrypted vault key on the relay. On success the /// key is installed into the local session so `ldgr sync pull` can materialize -/// the vault. +/// the vault, and this device adopts the **paired vault's** identifier. +/// +/// That adoption is what makes multi-device sync converge. Every device that +/// shares a vault must address the same server vault; before ADR-011 they only +/// agreed by accident, because each independently hashed the same default vault +/// directory path. A joining device now has its own random identifier, so +/// without adopting the paired one it would sync into a second, empty vault and +/// never see the other device's data. pub fn run_join(vault_path: &Path, payload: &str) -> Result<()> { let vault_dir = crate::session::resolve_vault_dir(Some(vault_path)); @@ -232,10 +253,24 @@ pub fn run_join(vault_path: &Path, payload: &str) -> Result<()> { ) })?; - let ctx = load_server_context(&vault_dir)?; + // Open the working store up front, and require it. Two reasons, both load- + // bearing: + // + // - the adoption below has to write to it, and once `create_session` swaps + // the session key for the paired device's, this vault's own SQLCipher + // store can no longer be opened (its file key is derived from the *local* + // vault key), so there is no second chance later; + // - failing here happens *before* `respond_pairing` consumes the pairing + // code, so the user can simply unlock and retry with the same code instead + // of having to generate a fresh one on the other device. + let conn = crate::db::require_unlocked_db(vault_path).context( + "the vault must be unlocked to join a pairing — run `ldgr unlock` first, \ + then re-run `ldgr devices join`", + )?; + let ctx = load_server_context(&vault_dir, Some(&conn))?; let rt = runtime()?; - let vault_key = rt.block_on(async { + let (vault_key, paired_vault_id) = rt.block_on(async { let session: JoinerSession = respond_pairing(&ctx.client, &code) .await .map_err(|e| anyhow::anyhow!("failed to join pairing: {e}"))?; @@ -255,10 +290,10 @@ pub fn run_join(vault_path: &Path, payload: &str) -> Result<()> { }) .await?; + let paired = discover_paired_vault(&ctx, &key).await; + // Best-effort: register this device so it appears in `devices list`. - if let Ok(conn) = crate::db::require_unlocked_db(vault_path) - && let Ok(device_id) = crate::sync::bridge::resolve_device_id(&conn, &vault_dir) - { + if let Ok(device_id) = crate::sync::bridge::resolve_device_id(&conn, &vault_dir) { let info = DeviceInfo { device_id: device_id.clone(), name: hostname(), @@ -267,18 +302,30 @@ pub fn run_join(vault_path: &Path, payload: &str) -> Result<()> { vector_clock: ldgr_core::sync::VectorClock::default(), }; if let Ok(bytes) = serde_json::to_vec(&info) { - let _ = ctx - .client - .put_device(&ctx.vault_id, &device_id, &bytes) - .await; + let target = paired.as_deref().unwrap_or(&ctx.vault_id); + let _ = ctx.client.put_device(target, &device_id, &bytes).await; } } - Ok::<[u8; 32], anyhow::Error>(key) + Ok::<([u8; 32], Option), anyhow::Error>((key, paired)) })?; - // Install the received vault key into the local session so subsequent - // commands (notably `ldgr sync pull`) can use it. + // Point this device at the vault it was just paired into, updating both + // stores together. `sync_state` takes precedence over `sync-config.json` + // when the vault id is resolved, so writing only the config would leave + // push/pull silently addressing this device's own (empty) vault while + // `devices remove` addressed the paired one. + if let Some(paired) = paired_vault_id.filter(|p| *p != ctx.vault_id) { + crate::sync::bridge::persist_vault_id(&conn, &paired)?; + rewrite_configured_vault_id(&vault_dir, &paired)?; + println!(); + println!("✓ Adopted the paired vault `{paired}`."); + } + + // Install the received vault key **last**. It replaces the session key with + // the paired device's, after which the local working store is no longer + // openable, so nothing below this point may touch the database. + drop(conn); crate::session::create_session( &vault_dir, vault_path, @@ -293,10 +340,74 @@ pub fn run_join(vault_path: &Path, payload: &str) -> Result<()> { Ok(()) } +/// Work out which of the account's vaults the received key belongs to. +/// +/// The key itself is the proof: a batch that decrypts under it is a batch from +/// the paired vault. So we try one batch per candidate and adopt the vault that +/// opens — no extra pairing-protocol fields, and no chance of the user picking +/// the wrong vault by hand. +/// +/// Falls back to the account's only vault when there is exactly one, and to +/// `None` (keep the current identifier) when the choice is genuinely ambiguous — +/// several vaults, none with a batch to test against yet. +async fn discover_paired_vault(ctx: &ServerContext, vault_key: &[u8; 32]) -> Option { + let vaults = ctx.client.list_vaults().await.ok()?; + + match vaults.len() { + 0 => return None, + 1 => return Some(vaults[0].id.clone()), + _ => {} + } + + for vault in &vaults { + let query = ListBatchesQuery { + limit: Some(1), + ..ListBatchesQuery::default() + }; + let Ok(metas) = ctx.client.list_remote_batches(&vault.id, &query).await else { + continue; + }; + let Some(meta) = metas.first() else { continue }; + let Ok(ciphertext) = ctx + .client + .get_batch(&vault.id, &meta.device_id, &meta.batch_id) + .await + else { + continue; + }; + if open_batch_with_session_key(vault_key, &ciphertext).is_ok() { + return Some(vault.id.clone()); + } + } + + println!(); + println!("⚠ This account has several vaults and none could be matched to the"); + println!(" key you just received. Staying on `{}`.", ctx.vault_id); + println!(" Run `ldgr sync status` to check which vault this device syncs to."); + None +} + +/// Update the `vault_id` recorded in `sync-config.json` so it agrees with the +/// vault's own identifier. Leaves every other field untouched. +fn rewrite_configured_vault_id(vault_dir: &Path, vault_id: &str) -> Result<()> { + let config_path = vault_dir.join("sync-config.json"); + let json = std::fs::read_to_string(&config_path).context("failed to read sync config")?; + let mut config: TransportConfig = + serde_json::from_str(&json).context("failed to parse sync config")?; + + if let TransportConfig::Server { vault_id: id, .. } = &mut config { + vault_id.clone_into(id); + } + + let updated = + serde_json::to_string_pretty(&config).context("failed to serialize sync config")?; + std::fs::write(&config_path, updated).context("failed to write sync config") +} + /// `ldgr devices remove ` — revoke a device. pub fn run_remove(vault_path: &Path, device_id: &str) -> Result<()> { let vault_dir = crate::session::resolve_vault_dir(Some(vault_path)); - let ctx = load_server_context(&vault_dir)?; + let ctx = load_server_context(&vault_dir, None)?; print!("Remove device `{device_id}`? This revokes its access. [y/N]: "); io::stdout().flush()?; diff --git a/crates/ldgr-cli/src/commands/sync.rs b/crates/ldgr-cli/src/commands/sync.rs index 2da5f68..aa0048e 100644 --- a/crates/ldgr-cli/src/commands/sync.rs +++ b/crates/ldgr-cli/src/commands/sync.rs @@ -51,6 +51,18 @@ pub fn run_setup(vault_path: &Path) -> Result<()> { _ => bail!("Invalid choice. Please enter 1, 2, or 3."), }; + // Freeze this vault's identifier *before* the new config overwrites the old + // one (ADR-011). Dropbox and WebDAV have no server to issue an identifier + // and record none in their config, so the choice is made here: a vault that + // synced before the upgrade adopts the path-derived identifier its blobs are + // already filed under, and one that never synced mints a random one. Doing + // this after the write would see the fresh config and wrongly treat a + // brand-new vault as an upgrading one. The ldgr-server path is excluded + // because `setup_server` has already claimed an identifier with the server. + if !matches!(config, TransportConfig::Server { .. }) { + crate::sync::bridge::resolve_vault_id(&db, &vault_dir)?; + } + // Save config let config_path = vault_dir.join("sync-config.json"); let json = serde_json::to_string_pretty(&config).context("failed to serialize config")?; @@ -221,6 +233,114 @@ fn print_server_info(info: &ServerInfo) { } } +/// Decide which server vault this device should sync to, and claim it. +/// +/// A device that has already synced keeps its identifier. A device that has not +/// adopts one from the account instead of inventing a second one — otherwise a +/// newly paired device would quietly create an empty vault alongside the real +/// one and never converge. Multi-vault accounts (#296) are disambiguated +/// interactively. +/// +/// Returns the identifier the **server** put in force, which is authoritative: +/// when the requested one is already owned by another account the server mints a +/// substitute rather than failing, so no account can lock another out (ADR-011). +async fn claim_server_vault( + client: &ServerSyncClient, + conn: &rusqlite::Connection, + vault_dir: &Path, +) -> Result { + let existing = crate::sync::bridge::existing_vault_id(conn, vault_dir)?; + + let requested = if let Some(id) = existing { + Some(id) + } else { + // A listing failure must abort, never fall through to "the account has + // no vaults". Treating a timeout or 5xx as "none" would mint a second, + // empty vault and freeze it locally — the exact convergence failure this + // whole flow exists to prevent, with no way back afterwards. + let remote = client.list_vaults().await.map_err(|e| { + anyhow::anyhow!( + "could not check which vaults this account owns: {e}\n\ + Setup stopped rather than risk creating a second, empty vault. \ + Check your connection and re-run `ldgr sync setup`." + ) + })?; + match remote.len() { + // No vault on the account yet — let the server mint one. + 0 => None, + 1 => { + let id = remote[0].id.clone(); + println!(); + println!("Adopting the vault already registered to this account."); + Some(id) + } + _ => Some(prompt_pick_vault(&remote)?), + } + }; + + let granted = match client.create_vault(requested.as_deref()).await { + Ok(vault) => vault.id, + // A pre-ADR-011 server can't mint identifiers — its `vault_id` field is + // required, so axum rejects the body outright (422 from the extractor, + // 400 if it ever reaches the handler). Pick one locally and retry. + Err(ServerSyncError::Http { + status: 400 | 422, .. + }) if requested.is_none() => { + let local = ldgr_core::sync::generate_vault_id(); + client + .create_vault(Some(&local)) + .await + .map_err(|e| anyhow::anyhow!("failed to create vault: {e}"))? + .id + } + // A pre-ADR-011 server answers 409 when the vault already exists, which + // for an identifier we already hold means "yours, already registered". + // Upgrading the client before the server must not lock the user out of + // re-authenticating, so keep the identifier we asked for. + Err(ServerSyncError::Http { status: 409, .. }) => match requested.as_deref() { + Some(id) => id.to_string(), + None => bail!("failed to create vault: server reported a conflict"), + }, + Err(e) => bail!("failed to create vault: {e}"), + }; + + if requested.as_deref().is_some_and(|r| r != granted) { + println!(); + println!("Note: that vault identifier is already in use by another account."); + println!("This vault has been registered as `{granted}` instead."); + } + + crate::sync::bridge::persist_vault_id(conn, &granted)?; + Ok(granted) +} + +/// Ask which of an account's several vaults this device should sync to. +fn prompt_pick_vault(vaults: &[ldgr_core::sync::server::VaultResponse]) -> Result { + println!(); + println!("This account has more than one vault. Choose the one to sync here:"); + for (i, v) in vaults.iter().enumerate() { + println!(" {}. {} (created {})", i + 1, v.id, v.created_at); + } + println!(" {}. Create a new vault", vaults.len() + 1); + println!(); + print!("Vault [1-{}]: ", vaults.len() + 1); + io::stdout().flush()?; + + let mut choice = String::new(); + io::stdin().read_line(&mut choice)?; + let index: usize = choice + .trim() + .parse() + .ok() + .filter(|n| (1..=vaults.len() + 1).contains(n)) + .context("invalid choice")?; + + if index == vaults.len() + 1 { + return Ok(ldgr_core::sync::generate_vault_id()); + } + Ok(vaults[index - 1].id.clone()) +} + /// Two-secret (2SKD, ADR-008 + #296) onboarding: sign up (generating a Secret /// Key + Emergency Kit) or sign in — either on a device that already stores the /// Secret Key, or a new device where the user supplies it. @@ -246,7 +366,6 @@ fn setup_server_2skd( bail!("Password cannot be empty."); } - let vault_id = get_vault_id(vault_dir); let device_id = crate::sync::bridge::resolve_device_id(conn, vault_dir)?; // If this device already stored a Secret Key (and its account KDF), sign in @@ -279,13 +398,9 @@ fn setup_server_2skd( .await? }; - // Ensure the vault exists (idempotent — a 409 means it already does). - if let Err(e) = client.create_vault(&vault_id).await { - match e { - ServerSyncError::Http { status: 409, .. } => {} - other => bail!("failed to create vault: {other}"), - } - } + // Adopt or claim this device's vault (ADR-011). + let vault_id = claim_server_vault(&client, conn, vault_dir).await?; + // Best-effort device registration; the encrypted blob is sent on push. let _ = client.put_device(&vault_id, &device_id, b"{}").await; @@ -293,14 +408,15 @@ fn setup_server_2skd( .token() .map(str::to_string) .ok_or_else(|| anyhow::anyhow!("server did not return a session token"))?; - Ok::<(String, String, Option), anyhow::Error>(( + Ok::<(String, String, Option, String), anyhow::Error>(( token, secret_key_text, account_kdf, + vault_id, )) })?; - let (session_token, secret_key_text, account_kdf) = outcome; + let (session_token, secret_key_text, account_kdf, vault_id) = outcome; println!(); println!("✓ Authenticated to {base_url} and registered device {device_id}."); @@ -562,10 +678,9 @@ fn setup_server_single_secret( bail!("Password cannot be empty."); } - let vault_id = get_vault_id(vault_dir); let device_id = crate::sync::bridge::resolve_device_id(conn, vault_dir)?; - let session_token = rt.block_on(async { + let (session_token, vault_id) = rt.block_on(async { let sender = ReqwestSender::new(base_url.clone()); let mut client = ServerSyncClient::new(sender); @@ -595,13 +710,8 @@ fn setup_server_single_secret( Err(e) => bail!("login failed: {e}"), } - // Ensure the vault exists (idempotent — a 409 means it already does). - if let Err(e) = client.create_vault(&vault_id).await { - match e { - ServerSyncError::Http { status: 409, .. } => {} - other => bail!("failed to create vault: {other}"), - } - } + // Adopt or claim this device's vault (ADR-011). + let vault_id = claim_server_vault(&client, conn, vault_dir).await?; // Register this device. The real encrypted device blob is uploaded on // `sync push`; this is a best-effort placeholder so the device is known. @@ -611,7 +721,7 @@ fn setup_server_single_secret( .token() .map(str::to_string) .ok_or_else(|| anyhow::anyhow!("server did not return a session token"))?; - Ok::(token) + Ok::<(String, String), anyhow::Error>((token, vault_id)) })?; println!(); @@ -769,12 +879,12 @@ pub fn run_push(vault_path: &Path) -> Result<()> { let vault_dir = crate::session::resolve_vault_dir(Some(vault_path)); let config = load_config(&vault_dir)?; - let vault_id = get_vault_id(&vault_dir); + let vault_id = crate::sync::bridge::resolve_vault_id(&conn, &vault_dir)?; let device_id = crate::sync::bridge::resolve_device_id(&conn, &vault_dir)?; let rt = tokio::runtime::Runtime::new().context("failed to create runtime")?; rt.block_on(async { - let transport = create_transport(&config, &vault_dir)?; + let transport = create_transport(&config, &vault_dir, &vault_id)?; println!("Pushing changes to {}…", config.provider().as_str()); @@ -826,12 +936,12 @@ pub fn run_pull(vault_path: &Path) -> Result<()> { let vault_dir = crate::session::resolve_vault_dir(Some(vault_path)); let config = load_config(&vault_dir)?; - let vault_id = get_vault_id(&vault_dir); + let vault_id = crate::sync::bridge::resolve_vault_id(&conn, &vault_dir)?; let device_id = crate::sync::bridge::resolve_device_id(&conn, &vault_dir)?; let rt = tokio::runtime::Runtime::new().context("failed to create runtime")?; rt.block_on(async { - let transport = create_transport(&config, &vault_dir)?; + let transport = create_transport(&config, &vault_dir, &vault_id)?; println!("Pulling changes from {}…", config.provider().as_str()); @@ -880,6 +990,7 @@ pub fn run_status(vault_path: &Path) -> Result<()> { let config = load_config(&vault_dir)?; let device_id = crate::sync::bridge::resolve_device_id(&conn, &vault_dir)?; + let vault_id = crate::sync::bridge::resolve_vault_id(&conn, &vault_dir)?; let last_sync = crate::sync::bridge::last_sync_at(&conn)?; let pending_push = ldgr_core::storage::sync::pending_event_count(&conn)?; let conflicts = ldgr_core::storage::sync::unresolved_conflict_count(&conn)?; @@ -887,6 +998,7 @@ pub fn run_status(vault_path: &Path) -> Result<()> { println!("Sync Status"); println!("════════════"); println!(" Provider: {}", config.provider().as_str()); + println!(" Vault ID: {vault_id}"); println!(" Device ID: {device_id}"); println!(" Last sync: {}", last_sync.as_deref().unwrap_or("never")); println!(" Pending push: {pending_push} event(s)"); @@ -916,17 +1028,13 @@ pub fn run_status(vault_path: &Path) -> Result<()> { } } TransportConfig::Server { - base_url, - username, - vault_id, - .. + base_url, username, .. } => { println!(); println!(" Server URL: {base_url}"); if let Some(user) = username { println!(" Username: {user}"); } - println!(" Vault ID: {vault_id}"); } } @@ -1011,27 +1119,25 @@ fn load_config(vault_dir: &Path) -> Result { serde_json::from_str(&json).context("failed to parse sync config") } -fn get_vault_id(vault_dir: &Path) -> String { - let path_str = vault_dir.to_string_lossy(); - let hash = simple_hash(path_str.as_bytes()); - format!("vault_{hash:016x}") -} - -fn simple_hash(data: &[u8]) -> u64 { - let mut hash: u64 = 5381; - for &b in data { - hash = hash.wrapping_mul(33).wrapping_add(u64::from(b)); - } - hash -} - fn hostname() -> String { std::env::var("COMPUTERNAME") .or_else(|_| std::env::var("HOSTNAME")) .unwrap_or_else(|_| "unknown".to_string()) } -fn create_transport(config: &TransportConfig, vault_dir: &Path) -> Result> { +/// Build the configured blob transport for `vault_id`. +/// +/// `vault_id` is the identifier resolved from the vault +/// ([`resolve_vault_id`](crate::sync::bridge::resolve_vault_id)), not the copy +/// echoed into `sync-config.json`. The two must be the same value: the server +/// transport matches incoming list prefixes against its own id, and a mismatch +/// used to fall through to the "no such prefix" arm — making `sync pull` report +/// "Already up to date" while remote batches existed. +fn create_transport( + config: &TransportConfig, + vault_dir: &Path, + vault_id: &str, +) -> Result> { let policy = ldgr_core::sync::RetryPolicy::default(); match config { @@ -1071,9 +1177,7 @@ fn create_transport(config: &TransportConfig, vault_dir: &Path) -> Result { + TransportConfig::Server { base_url, .. } => { // The SRP session token is a secret kept in sync-credentials.json, // written at `sync setup` after login (same pattern as Dropbox). let creds_path = vault_dir.join(CREDENTIALS_FILE); @@ -1097,9 +1201,228 @@ fn create_transport(config: &TransportConfig, vault_dir: &Path) -> Result String { + let config = Config { + bind_addr: "127.0.0.1:0".parse().unwrap(), + db_path: ":memory:".into(), + session_ttl_hours: 720, + relay_ttl_minutes: 10, + max_blob_bytes: 50 * 1024 * 1024, + srp_handshake_ttl_secs: 120, + registration_policy: RegistrationPolicy::Open, + admin_email: None, + allowed_origins: Vec::new(), + default_user_quota_bytes: 1024 * 1024 * 1024, + server_name: "vault-claim-test".into(), + }; + let state = Arc::new(AppState { + db: ServerDb::open(":memory:").expect("open in-memory db"), + srp_handshakes: SrpHandshakeStore::new(Duration::from_mins(2)), + config, + }); + let app = ldgr_server::api::router(state); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0") + .await + .expect("bind ephemeral port"); + let addr = listener.local_addr().expect("local addr"); + tokio::spawn(async move { + axum::serve(listener, app).await.expect("serve"); + }); + format!("http://{addr}") + } + + async fn account(base_url: &str, username: &str) -> ServerSyncClient { + let mut client = ServerSyncClient::new(ReqwestSender::new(base_url.to_string())); + let password = b"correct horse battery staple"; + client.register(username, password).await.expect("register"); + client.login(username, password).await.expect("login"); + client + } + + /// A local vault that already proposes `vault_id`, as an upgrading client + /// would: an unlocked working store plus a `sync-config.json` naming it. + fn configured_vault(vault_id: &str) -> (tempfile::TempDir, rusqlite::Connection) { + let dir = tempfile::tempdir().expect("temp dir"); + let conn = rusqlite::Connection::open_in_memory().expect("in-memory SQLite"); + ldgr_core::storage::schema::initialize(&conn).expect("schema"); + + let config = TransportConfig::Server { + base_url: "https://sync.example.com".into(), + username: Some("user@example.com".into()), + vault_id: vault_id.into(), + device_id: "device-a".into(), + }; + std::fs::write( + dir.path().join("sync-config.json"), + serde_json::to_string_pretty(&config).unwrap(), + ) + .unwrap(); + (dir, conn) + } + + fn stored_vault_id(conn: &rusqlite::Connection) -> Option { + sync_storage::get_state(conn, sync_storage::VAULT_ID_KEY).expect("read vault id state") + } + + /// The identifier the client proposed is free, so the server grants it and + /// the client keeps addressing its existing blobs. + #[tokio::test] + async fn client_keeps_its_identifier_when_the_server_grants_it() { + let base_url = spawn_server().await; + let client = account(&base_url, "solo@example.com").await; + let (dir, conn) = configured_vault(COLLIDING_ID); + + let granted = claim_server_vault(&client, &conn, dir.path()) + .await + .expect("claim"); + + assert_eq!(granted, COLLIDING_ID, "a free identifier must be granted"); + assert_eq!(stored_vault_id(&conn).as_deref(), Some(COLLIDING_ID)); + } + + /// The crux of ADR-011's client contract: when the server substitutes an + /// identifier, the client must persist and use **what came back**, not what + /// it asked for. Persisting its own proposal would leave it addressing + /// another account's vault and silently 404 on every push and pull. + #[tokio::test] + async fn client_persists_the_server_returned_identifier_not_its_own_proposal() { + let base_url = spawn_server().await; + + // Another account claims the identifier first — the squatting scenario. + let first = account(&base_url, "first@example.com").await; + let taken = first + .create_vault(Some(COLLIDING_ID)) + .await + .expect("first claim") + .id; + assert_eq!(taken, COLLIDING_ID); + + // A second account's client proposes the very same identifier. + let second = account(&base_url, "second@example.com").await; + let (dir, conn) = configured_vault(COLLIDING_ID); + + let granted = claim_server_vault(&second, &conn, dir.path()) + .await + .expect("a taken identifier must not fail setup"); + + assert_ne!( + granted, COLLIDING_ID, + "server handed over a taken identifier" + ); + assert_eq!( + stored_vault_id(&conn).as_deref(), + Some(granted.as_str()), + "client persisted its own proposal instead of the server's answer — \ + every push and pull would 404" + ); + + // The identifier it persisted actually works, which is what the + // assertion above is really protecting. + second + .put_batch(&granted, "device-2", "batch-1", b"ciphertext") + .await + .expect("the persisted identifier must be usable"); + + // And a re-run is idempotent: the client now proposes the substitute it + // stored, and the server returns that same vault rather than minting a + // third one. + let again = claim_server_vault(&second, &conn, dir.path()) + .await + .expect("re-claim"); + assert_eq!(again, granted); + assert_eq!( + second.list_vaults().await.expect("list").len(), + 1, + "re-running setup forked the vault" + ); + } + + /// Re-running setup on a device that already owns its vault must return the + /// same vault, never mint a second one — otherwise one vault silently forks + /// into two and the data is stranded. + #[tokio::test] + async fn owner_reclaim_is_idempotent_and_never_forks_the_vault() { + let base_url = spawn_server().await; + let client = account(&base_url, "owner@example.com").await; + let (dir, conn) = configured_vault(COLLIDING_ID); + + let first = claim_server_vault(&client, &conn, dir.path()) + .await + .expect("first claim"); + client + .put_batch(&first, "device-1", "batch-1", b"ciphertext") + .await + .expect("push"); + + let second = claim_server_vault(&client, &conn, dir.path()) + .await + .expect("second claim"); + + assert_eq!(second, first, "re-claim minted a different vault"); + assert_eq!( + client.list_vaults().await.expect("list").len(), + 1, + "re-claim forked the vault" + ); + assert_eq!( + client + .get_batch(&first, "device-1", "batch-1") + .await + .expect("data survives a re-claim"), + b"ciphertext" + ); + } + + /// A device with no identifier of its own adopts the account's existing + /// vault instead of minting a second, empty one it would never converge + /// out of. + #[tokio::test] + async fn a_new_device_adopts_the_accounts_existing_vault() { + let base_url = spawn_server().await; + let client = account(&base_url, "twodevice@example.com").await; + + let original = client.create_vault(None).await.expect("device A").id; + + // Device B: unlocked store, no sync config, no stored identifier. + let dir = tempfile::tempdir().unwrap(); + let conn = rusqlite::Connection::open_in_memory().unwrap(); + ldgr_core::storage::schema::initialize(&conn).unwrap(); + + let adopted = claim_server_vault(&client, &conn, dir.path()) + .await + .expect("device B claim"); + + assert_eq!( + adopted, original, + "device B did not converge on device A's vault" + ); + assert_eq!(stored_vault_id(&conn).as_deref(), Some(original.as_str())); + assert_eq!(client.list_vaults().await.expect("list").len(), 1); + } +} diff --git a/crates/ldgr-cli/src/sync/bridge.rs b/crates/ldgr-cli/src/sync/bridge.rs index 2f6a3f0..65c7710 100644 --- a/crates/ldgr-cli/src/sync/bridge.rs +++ b/crates/ldgr-cli/src/sync/bridge.rs @@ -32,7 +32,7 @@ use ldgr_core::sync::pipeline::{ IngestOutcome, export_pending_batch_with_session_key, ingest_batch_with_session_key, }; use ldgr_core::sync::transport::{ - batch_path, batches_prefix, device_batches_prefix, parse_batch_path, + TransportConfig, batch_path, batches_prefix, device_batches_prefix, parse_batch_path, }; use super::BlobTransport; @@ -44,6 +44,9 @@ const LAST_SYNC_AT_KEY: &str = "cli_last_sync_at"; /// Legacy file (pre-unification) that held a CLI-only device id. const LEGACY_DEVICE_ID_FILE: &str = "device-id"; +/// Sync configuration file, read only to recover a pre-ADR-011 vault id. +const SYNC_CONFIG_FILE: &str = "sync-config.json"; + /// Build a [`sync_storage::SyncContext`] for the next local mutation: the /// vault's DB device id plus a freshly-ticked Lamport clock. /// @@ -84,6 +87,102 @@ pub fn resolve_device_id(conn: &Connection, vault_dir: &Path) -> Result sync_storage::device_id(conn).context("failed to resolve device id") } +/// Resolve this vault's sync identifier, adopting a pre-ADR-011 one on upgrade. +/// +/// The identifier is **persisted** in `sync_state` and never re-derived. Earlier +/// builds recomputed it on every call as a djb2 hash of the vault *directory +/// path*, which made it predictable, enumerable, and — because nearly every user +/// keeps the vault at the default path — identical across unrelated accounts, so +/// the first account to register it on a shared server locked everyone else out. +/// +/// Resolution order, first match wins: +/// +/// 1. the identifier already stored in `sync_state`; +/// 2. the one persisted in `sync-config.json` by a configured server transport; +/// 3. for an already-configured Dropbox/WebDAV vault, which stores no identifier +/// anywhere, the legacy path-derived value — recomputed **once** and then +/// frozen, so existing remote blobs under `vault_/` stay reachable; +/// 4. otherwise a fresh random identifier. +/// +/// Steps 2 and 3 are the upgrade path: without them an existing user would get a +/// brand-new identifier and be silently orphaned from everything they had synced. +pub fn resolve_vault_id(conn: &Connection, vault_dir: &Path) -> Result { + if let Some(existing) = existing_vault_id(conn, vault_dir)? { + return Ok(existing); + } + sync_storage::vault_id(conn).context("failed to resolve vault id") +} + +/// The identifier this vault is already known by, without minting one. +/// +/// Returns `None` only for a vault that has never synced, which is the single +/// case where a caller may choose a brand-new identifier (or adopt one from the +/// server). Steps 1-3 of [`resolve_vault_id`]. +pub fn existing_vault_id(conn: &Connection, vault_dir: &Path) -> Result> { + if let Some(id) = sync_storage::get_state(conn, sync_storage::VAULT_ID_KEY) + .context("failed to read vault id state")? + { + return Ok(Some(id)); + } + + let Some(legacy) = legacy_vault_id(vault_dir) else { + return Ok(None); + }; + let adopted = sync_storage::adopt_vault_id(conn, &legacy) + .context("failed to adopt the existing vault id")?; + Ok(Some(adopted)) +} + +/// Record the identifier the server put in force. +/// +/// The server's response is authoritative — it may hand back a different +/// identifier than the one requested when that one is already owned by another +/// account — so setup persists what it receives rather than what it asked for. +pub fn persist_vault_id(conn: &Connection, id: &str) -> Result<()> { + sync_storage::set_vault_id(conn, id).context("failed to persist the vault id") +} + +/// The identifier an already-configured vault used before ADR-011, or `None` +/// when sync was never set up (in which case a fresh random id is correct). +/// +/// Migration-only. Nothing else may derive an identifier from the vault path. +fn legacy_vault_id(vault_dir: &Path) -> Option { + let config_path = vault_dir.join(SYNC_CONFIG_FILE); + if !config_path.exists() { + return None; + } + + // A server transport persisted its identifier; prefer that exact value over + // re-deriving, since the vault directory may have moved since setup. + let configured = std::fs::read_to_string(&config_path) + .ok() + .and_then(|json| serde_json::from_str::(&json).ok()); + if let Some(TransportConfig::Server { vault_id, .. }) = configured + && !vault_id.is_empty() + { + return Some(vault_id); + } + + // Dropbox/WebDAV store no identifier, so reproduce the one their blobs are + // already filed under. + Some(path_derived_vault_id(vault_dir)) +} + +/// Reproduce the pre-ADR-011 path-derived identifier (djb2 over the vault +/// directory path). +/// +/// Migration-only, and deliberately private: this is exactly the weak, +/// guessable, collision-prone derivation ADR-011 removes. It exists so an +/// upgrading Dropbox/WebDAV vault can adopt the identifier its existing remote +/// blobs are stored under, once, before freezing it in `sync_state`. +fn path_derived_vault_id(vault_dir: &Path) -> String { + let mut hash: u64 = 5381; + for &b in vault_dir.to_string_lossy().as_bytes() { + hash = hash.wrapping_mul(33).wrapping_add(u64::from(b)); + } + format!("vault_{hash:016x}") +} + /// Summary of a `push` run. #[derive(Debug, Clone, Copy, Default)] pub struct PushSummary { @@ -244,3 +343,208 @@ fn save_ingested_batches(conn: &Connection, ids: &[String]) -> Result<()> { sync_storage::set_state(conn, INGESTED_BATCHES_KEY, &json) .context("failed to persist ingested-batch cursor") } + +#[cfg(test)] +mod tests { + use super::*; + + fn vault_db() -> Connection { + let conn = Connection::open_in_memory().expect("in-memory SQLite"); + ldgr_core::storage::schema::initialize(&conn).expect("schema"); + conn + } + + fn write_config(dir: &Path, config: &TransportConfig) { + std::fs::write( + dir.join(SYNC_CONFIG_FILE), + serde_json::to_string_pretty(config).unwrap(), + ) + .unwrap(); + } + + fn server_config(vault_id: &str) -> TransportConfig { + TransportConfig::Server { + base_url: "https://sync.example.com".into(), + username: Some("user@example.com".into()), + vault_id: vault_id.into(), + device_id: "device-a".into(), + } + } + + // ── Branch 4: never-synced vault ──────────────────────────────────────── + + #[test] + fn unconfigured_vault_mints_a_random_id_once() { + let dir = tempfile::tempdir().unwrap(); + let conn = vault_db(); + + let first = resolve_vault_id(&conn, dir.path()).unwrap(); + assert!( + ldgr_core::sync::is_random_vault_id(&first), + "expected a random id, got {first}" + ); + + let second = resolve_vault_id(&conn, dir.path()).unwrap(); + assert_eq!( + first, second, + "the identifier must be persisted, not re-minted" + ); + } + + #[test] + fn two_vaults_at_the_same_path_get_different_ids() { + // The whole point of ADR-011: the identifier no longer depends on where + // the vault lives, so two accounts on the default path cannot collide. + let dir = tempfile::tempdir().unwrap(); + let a = resolve_vault_id(&vault_db(), dir.path()).unwrap(); + let b = resolve_vault_id(&vault_db(), dir.path()).unwrap(); + assert_ne!(a, b); + } + + // ── Branch 2: upgrading a configured server vault ─────────────────────── + + #[test] + fn configured_server_vault_keeps_its_identifier() { + let dir = tempfile::tempdir().unwrap(); + write_config(dir.path(), &server_config("vault_0123456789abcdef")); + let conn = vault_db(); + + let resolved = resolve_vault_id(&conn, dir.path()).unwrap(); + assert_eq!( + resolved, "vault_0123456789abcdef", + "an upgrading client must keep the identifier its blobs are filed under" + ); + } + + #[test] + fn configured_server_identifier_survives_moving_the_vault_directory() { + // The persisted identifier wins over anything derived from the path, so + // renaming the vault directory no longer silently repoints sync. + let original = tempfile::tempdir().unwrap(); + write_config( + original.path(), + &server_config("v1_abcdefabcdefabcdefabcdefabcdefab"), + ); + let conn = vault_db(); + let before = resolve_vault_id(&conn, original.path()).unwrap(); + + let moved = tempfile::tempdir().unwrap(); + write_config( + moved.path(), + &server_config("v1_abcdefabcdefabcdefabcdefabcdefab"), + ); + let after = resolve_vault_id(&conn, moved.path()).unwrap(); + + assert_eq!(before, after); + assert_eq!(after, "v1_abcdefabcdefabcdefabcdefabcdefab"); + } + + // ── Branch 3: upgrading a configured Dropbox/WebDAV vault ─────────────── + + #[test] + fn configured_blob_provider_adopts_the_path_derived_identifier() { + // Dropbox and WebDAV store no identifier anywhere, so an upgrading vault + // must reproduce the one its remote blobs already live under — otherwise + // everything previously synced becomes unreachable. + let dir = tempfile::tempdir().unwrap(); + write_config( + dir.path(), + &TransportConfig::WebDav { + base_url: "https://dav.example.com".into(), + username: Some("user".into()), + }, + ); + let conn = vault_db(); + + let resolved = resolve_vault_id(&conn, dir.path()).unwrap(); + assert_eq!(resolved, path_derived_vault_id(dir.path())); + assert!(ldgr_core::sync::is_legacy_vault_id(&resolved)); + } + + #[test] + fn an_adopted_legacy_identifier_is_frozen_not_re_derived() { + let dir = tempfile::tempdir().unwrap(); + write_config( + dir.path(), + &TransportConfig::Dropbox { + app_key: "key".into(), + account_hint: None, + }, + ); + let conn = vault_db(); + let adopted = resolve_vault_id(&conn, dir.path()).unwrap(); + + // Moving the vault afterwards must NOT change the identifier — the old + // derivation would have silently orphaned every synced blob. + let moved = tempfile::tempdir().unwrap(); + write_config( + moved.path(), + &TransportConfig::Dropbox { + app_key: "key".into(), + account_hint: None, + }, + ); + assert_eq!(resolve_vault_id(&conn, moved.path()).unwrap(), adopted); + } + + // ── Branch 1 / precedence ─────────────────────────────────────────────── + + #[test] + fn a_stored_identifier_wins_over_the_configuration() { + let dir = tempfile::tempdir().unwrap(); + write_config(dir.path(), &server_config("vault_0123456789abcdef")); + let conn = vault_db(); + + persist_vault_id(&conn, "v1_11112222333344445555666677778888").unwrap(); + assert_eq!( + resolve_vault_id(&conn, dir.path()).unwrap(), + "v1_11112222333344445555666677778888" + ); + } + + #[test] + fn existing_vault_id_reports_none_only_for_a_never_synced_vault() { + let dir = tempfile::tempdir().unwrap(); + let conn = vault_db(); + assert!(existing_vault_id(&conn, dir.path()).unwrap().is_none()); + + write_config(dir.path(), &server_config("vault_0123456789abcdef")); + assert_eq!( + existing_vault_id(&conn, dir.path()).unwrap().as_deref(), + Some("vault_0123456789abcdef") + ); + } + + #[test] + fn a_fresh_setup_mints_rather_than_deriving() { + // `sync setup` freezes the identifier before writing the new config, so + // a vault that has never synced mints a random one even for Dropbox and + // WebDAV — which record no identifier of their own. Resolving against an + // empty directory reproduces that pre-write state. + let dir = tempfile::tempdir().unwrap(); + let conn = vault_db(); + let minted = resolve_vault_id(&conn, dir.path()).unwrap(); + assert!(ldgr_core::sync::is_random_vault_id(&minted), "{minted}"); + + // Writing the config afterwards must not change it. + write_config( + dir.path(), + &TransportConfig::Dropbox { + app_key: "key".into(), + account_hint: None, + }, + ); + assert_eq!(resolve_vault_id(&conn, dir.path()).unwrap(), minted); + } + + #[test] + fn path_derivation_reproduces_the_pre_adr_011_identifier() { + // Pins the exact legacy algorithm (djb2, seed 5381, multiplier 33) so a + // future refactor cannot silently change which blobs a legacy vault + // adopts. This value is the hash of the literal path below. + assert_eq!( + path_derived_vault_id(Path::new("/home/user/.ldgr")), + "vault_9eeca790b42e1fb1" + ); + } +} diff --git a/crates/ldgr-cli/tests/server_sync.rs b/crates/ldgr-cli/tests/server_sync.rs index 7be2487..f0e9a1c 100644 --- a/crates/ldgr-cli/tests/server_sync.rs +++ b/crates/ldgr-cli/tests/server_sync.rs @@ -76,7 +76,10 @@ async fn server_transport_round_trips_batch_and_snapshot() { let mut client = ServerSyncClient::new(sender); client.register(username, password).await.expect("register"); client.login(username, password).await.expect("login"); - client.create_vault(vault_id).await.expect("create vault"); + client + .create_vault(Some(vault_id)) + .await + .expect("create vault"); client.token().expect("session token").to_string() }; @@ -166,7 +169,10 @@ async fn server_transport_lists_all_batches_across_pages() { let mut client = ServerSyncClient::new(sender); client.register(username, password).await.expect("register"); client.login(username, password).await.expect("login"); - client.create_vault(vault_id).await.expect("create vault"); + client + .create_vault(Some(vault_id)) + .await + .expect("create vault"); client.token().expect("session token").to_string() }; diff --git a/crates/ldgr-cli/tests/sync_round_trip.rs b/crates/ldgr-cli/tests/sync_round_trip.rs index 2883727..1df045a 100644 --- a/crates/ldgr-cli/tests/sync_round_trip.rs +++ b/crates/ldgr-cli/tests/sync_round_trip.rs @@ -94,7 +94,10 @@ async fn provision(base_url: &str, username: &str, vault_id: &str) -> String { .login(username, b"correct horse battery staple") .await .expect("login"); - client.create_vault(vault_id).await.expect("create vault"); + client + .create_vault(Some(vault_id)) + .await + .expect("create vault"); client.token().expect("session token").to_string() } diff --git a/crates/ldgr-core/src/storage/sync.rs b/crates/ldgr-core/src/storage/sync.rs index 2c24974..c79023f 100644 --- a/crates/ldgr-core/src/storage/sync.rs +++ b/crates/ldgr-core/src/storage/sync.rs @@ -278,6 +278,54 @@ pub fn device_id(conn: &Connection) -> Result { Ok(id) } +/// Key under which the vault's sync identifier is persisted in `sync_state`. +pub const VAULT_ID_KEY: &str = "vault_id"; + +/// Get this vault's sync identifier, minting a random one if it isn't set yet +/// (ADR-011). +/// +/// The identifier is persisted rather than derived, so it survives moving or +/// renaming the vault directory. Earlier clients recomputed it as a hash of the +/// vault path on every call, which made it predictable and — since most vaults +/// live at the default path — identical across unrelated accounts. +/// +/// Callers upgrading an already-synced vault must seed the existing identifier +/// with [`adopt_vault_id`] *before* the first call here, otherwise this mints a +/// new one and the vault is orphaned from its already-uploaded blobs. +pub fn vault_id(conn: &Connection) -> Result { + if let Some(id) = get_state(conn, VAULT_ID_KEY)? { + return Ok(id); + } + + let id = crate::sync::vault_id::generate_vault_id(); + set_state(conn, VAULT_ID_KEY, &id)?; + Ok(id) +} + +/// Persist `id` as this vault's sync identifier, but only if none is set yet. +/// +/// Returns the identifier now in force. Used for two things: adopting the +/// legacy path-derived identifier of an already-synced vault on upgrade, and +/// adopting the identifier of an existing server vault when a newly paired +/// device joins. Never overwrites an established identifier, so it is safe to +/// call unconditionally. +pub fn adopt_vault_id(conn: &Connection, id: &str) -> Result { + if let Some(existing) = get_state(conn, VAULT_ID_KEY)? { + return Ok(existing); + } + set_state(conn, VAULT_ID_KEY, id)?; + Ok(id.to_string()) +} + +/// Replace this vault's sync identifier, even if one is already set. +/// +/// Only for the two cases where the previous identifier is known to be wrong: +/// the server handed back a different identifier than the one requested, or a +/// freshly paired device must switch to the vault it just received the key for. +pub fn set_vault_id(conn: &Connection, id: &str) -> Result<(), StorageError> { + set_state(conn, VAULT_ID_KEY, id) +} + /// Count events originating from a specific device (synced or not). /// /// Used by the sync pipeline to compute this device's own vector-clock @@ -483,6 +531,45 @@ mod tests { assert!(!id1.is_empty()); } + #[test] + fn vault_id_auto_generates_and_is_stable() { + let conn = setup_db(); + let id1 = vault_id(&conn).unwrap(); + let id2 = vault_id(&conn).unwrap(); + assert_eq!(id1, id2, "vault id must be persisted, not regenerated"); + assert!(crate::sync::vault_id::is_random_vault_id(&id1), "{id1}"); + } + + #[test] + fn vault_id_differs_between_vaults() { + let a = vault_id(&setup_db()).unwrap(); + let b = vault_id(&setup_db()).unwrap(); + assert_ne!(a, b, "two vaults must not share an identifier"); + } + + #[test] + fn adopt_vault_id_seeds_only_when_unset() { + let conn = setup_db(); + // A legacy identifier adopted on upgrade wins over minting a new one. + let adopted = adopt_vault_id(&conn, "vault_0123456789abcdef").unwrap(); + assert_eq!(adopted, "vault_0123456789abcdef"); + assert_eq!(vault_id(&conn).unwrap(), "vault_0123456789abcdef"); + + // A second adoption must not clobber the established identifier. + let again = adopt_vault_id(&conn, "v1_ffffffffffffffffffffffffffffffff").unwrap(); + assert_eq!(again, "vault_0123456789abcdef"); + } + + #[test] + fn set_vault_id_overrides_an_established_id() { + let conn = setup_db(); + let original = vault_id(&conn).unwrap(); + set_vault_id(&conn, "v1_ffffffffffffffffffffffffffffffff").unwrap(); + let updated = vault_id(&conn).unwrap(); + assert_ne!(updated, original); + assert_eq!(updated, "v1_ffffffffffffffffffffffffffffffff"); + } + #[test] fn lamport_clock_ticks() { let conn = setup_db(); diff --git a/crates/ldgr-core/src/sync/mod.rs b/crates/ldgr-core/src/sync/mod.rs index a57b647..1af9087 100644 --- a/crates/ldgr-core/src/sync/mod.rs +++ b/crates/ldgr-core/src/sync/mod.rs @@ -11,6 +11,7 @@ pub mod onboarding; pub mod payload; pub mod snapshot; pub mod transport; +pub mod vault_id; /// Server sync protocol (SRP-6a client + endpoint types). Requires the `sync` /// feature, which pulls in big-integer arithmetic for SRP. @@ -49,3 +50,6 @@ pub use transport::{ RemoteSnapshotMeta, RetryPolicy, SyncCheckpoint, TransportConfig, TransportErrorKind, TransportProvider, }; +pub use vault_id::{ + MAX_VAULT_ID_LEN, generate_vault_id, is_legacy_vault_id, is_random_vault_id, is_valid_vault_id, +}; diff --git a/crates/ldgr-core/src/sync/server/client.rs b/crates/ldgr-core/src/sync/server/client.rs index 37601f2..b9cd73d 100644 --- a/crates/ldgr-core/src/sync/server/client.rs +++ b/crates/ldgr-core/src/sync/server/client.rs @@ -428,9 +428,22 @@ impl ServerSyncClient { // ── Vaults ────────────────────────────────────────────────────────────── /// Create a vault. - pub async fn create_vault(&self, vault_id: &str) -> Result { + /// + /// Pass `None` to have the server mint a random identifier (ADR-011), or + /// `Some(id)` to request a specific one — which an already-synced vault must + /// do so its existing blobs stay addressable. + /// + /// The returned [`VaultResponse::id`] is **authoritative** and may differ + /// from `vault_id`: when the requested identifier is already owned by + /// another account the server mints a fresh one rather than failing, so no + /// account can lock another out of an identifier. Callers must persist the + /// identifier they get back. + pub async fn create_vault( + &self, + vault_id: Option<&str>, + ) -> Result { let body = CreateVaultRequest { - vault_id: vault_id.to_string(), + vault_id: vault_id.map(str::to_string), }; self.post_json(&format!("{API_PREFIX}/vaults"), &body, true) .await @@ -971,7 +984,7 @@ mod tests { fn authenticated_call_requires_token() { let sender = MockSender::new(vec![]); let client = ServerSyncClient::new(sender); - let err = block_on(client.create_vault("v1")).unwrap_err(); + let err = block_on(client.create_vault(Some("v1"))).unwrap_err(); assert_eq!(err, ServerSyncError::NotAuthenticated); } @@ -982,7 +995,7 @@ mod tests { &serde_json::json!({ "id": "v1", "created_at": "t" }), )]); let client = ServerSyncClient::with_token(sender, "tok-abc".into()); - let resp = block_on(client.create_vault("v1")).expect("create"); + let resp = block_on(client.create_vault(Some("v1"))).expect("create"); assert_eq!(resp.id, "v1"); let req = client.sender.last_request(); diff --git a/crates/ldgr-core/src/sync/server/protocol.rs b/crates/ldgr-core/src/sync/server/protocol.rs index e33f2d8..3c70d37 100644 --- a/crates/ldgr-core/src/sync/server/protocol.rs +++ b/crates/ldgr-core/src/sync/server/protocol.rs @@ -236,12 +236,25 @@ pub struct Pong { // ── Vaults ────────────────────────────────────────────────────────────────────── /// `POST /api/v1/vaults` request body. +/// +/// `vault_id` is optional (ADR-011): a client that has no identifier yet omits +/// it and the server mints a random one, returned in [`VaultResponse::id`]. +/// Clients that already hold an identifier — including pre-ADR-011 clients with +/// a path-derived one — still send it so their existing blobs stay addressable. +/// Either way the response is authoritative: the server may hand back a +/// different identifier than the one requested (see [`VaultResponse`]). #[derive(Debug, Clone, Serialize, Deserialize)] pub struct CreateVaultRequest { - pub vault_id: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub vault_id: Option, } /// Vault descriptor returned by create/list. +/// +/// `id` is authoritative. On create it may differ from the requested identifier +/// when that identifier is already owned by another account — the server mints a +/// fresh one instead of failing, so one account can never lock another out of an +/// identifier (ADR-011). Clients must persist what they receive here. #[derive(Debug, Clone, Serialize, Deserialize)] pub struct VaultResponse { pub id: String, @@ -478,10 +491,26 @@ mod tests { assert_eq!(wire.to_account_kdf().unwrap(), kdf); } + #[test] + fn create_vault_request_omits_absent_vault_id() { + // A server-assigned identifier is requested by sending no field at all, + // which pre-ADR-011 servers reject — clients retry with an id. + let json = serde_json::to_string(&CreateVaultRequest { vault_id: None }).unwrap(); + assert_eq!(json, "{}"); + + // And an empty body still parses, so old clients that send an id and new + // clients that don't are both accepted. + let parsed: CreateVaultRequest = serde_json::from_str("{}").unwrap(); + assert!(parsed.vault_id.is_none()); + let legacy: CreateVaultRequest = + serde_json::from_str(r#"{"vault_id":"vault_0123456789abcdef"}"#).unwrap(); + assert_eq!(legacy.vault_id.as_deref(), Some("vault_0123456789abcdef")); + } + #[test] fn vault_and_blob_types_round_trip() { round_trip(&CreateVaultRequest { - vault_id: "v1".into(), + vault_id: Some("v1".into()), }); round_trip(&VaultResponse { id: "v1".into(), diff --git a/crates/ldgr-core/src/sync/vault_id.rs b/crates/ldgr-core/src/sync/vault_id.rs new file mode 100644 index 0000000..3860dd2 --- /dev/null +++ b/crates/ldgr-core/src/sync/vault_id.rs @@ -0,0 +1,143 @@ +//! Vault identifiers: the stable, unguessable handle a vault is known by on a +//! sync server (ADR-011). +//! +//! Pure computation — generation and validation only. Persistence lives in +//! [`crate::storage::sync::vault_id`] (CLI/native) or in platform storage +//! (iOS `UserDefaults`, web `sync_state`). +//! +//! ## Why not derive the id +//! +//! Earlier clients derived the id as a djb2 hash of the vault *directory path* +//! (`vault_{hash:016x}`). That made the id predictable, enumerable, and — because +//! most users keep the vault at the default path — **identical across accounts**, +//! so the first account to claim it on a shared server locked every other account +//! out of that id permanently. Identifiers are now random, so two vaults collide +//! only with probability 2⁻¹²⁸. +//! +//! Deriving the id from the vault key was considered and rejected: it would +//! couple a server-visible identifier to key material and break under any future +//! vault-key rotation. + +use rand::Rng; + +/// Prefix marking a random, ADR-011 vault identifier. +const PREFIX: &str = "v1_"; + +/// Bytes of entropy in a generated identifier. +const ENTROPY_BYTES: usize = 16; + +/// Prefix used by pre-ADR-011 path-derived identifiers. +const LEGACY_PREFIX: &str = "vault_"; + +/// Maximum accepted identifier length, matching the server's validation. +pub const MAX_VAULT_ID_LEN: usize = 128; + +/// Generate a fresh, random vault identifier. +/// +/// The result is `v1_` followed by 32 lowercase hex characters (128 bits of +/// CSPRNG entropy) — unguessable, and stable once persisted by the caller. +#[must_use] +pub fn generate_vault_id() -> String { + const HEX: &[u8; 16] = b"0123456789abcdef"; + + let mut bytes = [0u8; ENTROPY_BYTES]; + rand::rng().fill_bytes(&mut bytes); + + let mut id = String::with_capacity(PREFIX.len() + ENTROPY_BYTES * 2); + id.push_str(PREFIX); + for b in bytes { + id.push(char::from(HEX[usize::from(b >> 4)])); + id.push(char::from(HEX[usize::from(b & 0x0f)])); + } + id +} + +/// Whether `id` is a well-formed identifier the server will accept. +/// +/// Accepts both random ADR-011 identifiers and the legacy path-derived ones, so +/// existing vaults keep working. The character set is restricted to +/// `[A-Za-z0-9_-]` because the identifier is interpolated into blob paths and +/// URL path segments. +#[must_use] +pub fn is_valid_vault_id(id: &str) -> bool { + !id.is_empty() + && id.len() <= MAX_VAULT_ID_LEN + && id + .bytes() + .all(|b| b.is_ascii_alphanumeric() || b == b'_' || b == b'-') +} + +/// Whether `id` looks like a pre-ADR-011 path-derived identifier. +/// +/// Such identifiers are grandfathered — they keep addressing already-synced +/// data — but clients never mint new ones. +#[must_use] +pub fn is_legacy_vault_id(id: &str) -> bool { + id.strip_prefix(LEGACY_PREFIX) + .is_some_and(|rest| rest.len() == 16 && rest.bytes().all(|b| b.is_ascii_hexdigit())) +} + +/// Whether `id` is a random identifier minted under ADR-011. +#[must_use] +pub fn is_random_vault_id(id: &str) -> bool { + id.strip_prefix(PREFIX).is_some_and(|rest| { + rest.len() == ENTROPY_BYTES * 2 && rest.bytes().all(|b| b.is_ascii_hexdigit()) + }) +} + +#[cfg(test)] +mod tests { + use std::collections::HashSet; + + use super::*; + + #[test] + fn generated_ids_have_the_documented_shape() { + let id = generate_vault_id(); + assert!(id.starts_with(PREFIX), "missing prefix: {id}"); + assert_eq!(id.len(), PREFIX.len() + 32); + assert!(is_random_vault_id(&id)); + assert!(is_valid_vault_id(&id)); + assert!(!is_legacy_vault_id(&id)); + } + + #[test] + fn generated_ids_are_unique() { + let ids: HashSet = (0..1000).map(|_| generate_vault_id()).collect(); + assert_eq!(ids.len(), 1000, "generated ids collided"); + } + + #[test] + fn generated_ids_are_not_all_zero() { + // Guards against an RNG that silently yields zeroed buffers. + let id = generate_vault_id(); + assert_ne!(id, format!("{PREFIX}{}", "0".repeat(32))); + } + + #[test] + fn legacy_ids_are_recognized_and_still_valid() { + let legacy = "vault_0123456789abcdef"; + assert!(is_legacy_vault_id(legacy)); + assert!(is_valid_vault_id(legacy)); + assert!(!is_random_vault_id(legacy)); + } + + #[test] + fn legacy_detection_rejects_near_misses() { + assert!(!is_legacy_vault_id("vault_0123456789abcde"), "too short"); + assert!(!is_legacy_vault_id("vault_0123456789abcdef0"), "too long"); + assert!(!is_legacy_vault_id("vault_0123456789abcdeg"), "not hex"); + assert!(!is_legacy_vault_id("v1_0123456789abcdef")); + } + + #[test] + fn validation_rejects_empty_oversized_and_path_traversing_ids() { + assert!(!is_valid_vault_id("")); + assert!(!is_valid_vault_id(&"a".repeat(MAX_VAULT_ID_LEN + 1))); + assert!(is_valid_vault_id(&"a".repeat(MAX_VAULT_ID_LEN))); + assert!(!is_valid_vault_id("../etc/passwd")); + assert!(!is_valid_vault_id("vault/batches")); + assert!(!is_valid_vault_id("vault id")); + assert!(!is_valid_vault_id("vault.id")); + } +} diff --git a/crates/ldgr-ffi/src/sync.rs b/crates/ldgr-ffi/src/sync.rs index bd8f23c..ea6a27a 100644 --- a/crates/ldgr-ffi/src/sync.rs +++ b/crates/ldgr-ffi/src/sync.rs @@ -267,6 +267,13 @@ pub struct FfiDevice { pub encrypted_info: String, } +/// A vault the authenticated account owns. +#[derive(Debug, Clone, uniffi::Record)] +pub struct FfiVault { + pub id: String, + pub created_at: String, +} + /// Server discovery document (`GET /server/info`), mirrors core `ServerInfo`. #[derive(Debug, Clone, uniffi::Record)] pub struct FfiServerInfo { @@ -480,10 +487,30 @@ impl LdgrSyncClient { // ── Vaults ─────────────────────────────────────────────────────────────── - /// Create a vault. Returns the server-assigned vault id. - pub async fn create_vault(&self, vault_id: String) -> Result { + /// Claim a vault, returning the identifier the server put in force. + /// + /// Pass `None` to have the server mint a random identifier (ADR-011). The + /// returned identifier is authoritative and may differ from `vault_id` — + /// callers must persist what they get back. + pub async fn create_vault(&self, vault_id: Option) -> Result { + let client = self.inner.lock().await; + Ok(client.create_vault(vault_id.as_deref()).await?.id) + } + + /// List the vaults this account owns. + /// + /// A device with no identifier of its own adopts one of these rather than + /// creating a second, empty vault it would never converge out of (ADR-011). + pub async fn list_vaults(&self) -> Result, FfiSyncError> { let client = self.inner.lock().await; - Ok(client.create_vault(&vault_id).await?.id) + let vaults = client.list_vaults().await?; + Ok(vaults + .into_iter() + .map(|v| FfiVault { + id: v.id, + created_at: v.created_at, + }) + .collect()) } // ── Batches (opaque ciphertext) ────────────────────────────────────────── diff --git a/crates/ldgr-server/src/api/vaults.rs b/crates/ldgr-server/src/api/vaults.rs index 99ab192..aa10d40 100644 --- a/crates/ldgr-server/src/api/vaults.rs +++ b/crates/ldgr-server/src/api/vaults.rs @@ -11,7 +11,10 @@ use crate::state::SharedState; #[derive(Deserialize)] pub struct CreateVaultRequest { - pub vault_id: String, + /// Identifier the client wants to keep. Absent when the client has none yet + /// and wants the server to mint one (ADR-011). + #[serde(default)] + pub vault_id: Option, } #[derive(Serialize)] @@ -20,24 +23,42 @@ pub struct VaultResponse { pub created_at: String, } +/// `POST /api/v1/vaults` +/// +/// Claim a vault for the authenticated account. The response `id` is +/// authoritative and may differ from the requested one — see +/// [`ServerDb::claim_vault`](crate::storage::ServerDb::claim_vault) for the +/// resolution order and why a taken identifier mints a new one instead of +/// returning a conflict. pub async fn create_vault( State(state): State, AuthUser(user_id): AuthUser, Json(req): Json, ) -> Result<(StatusCode, Json), ServerError> { - if req.vault_id.is_empty() || req.vault_id.len() > 128 { - return Err(ServerError::BadRequest( - "vault_id must be 1-128 characters".into(), - )); + // Only the length contract is enforced here, matching what pre-ADR-011 + // servers rejected. The character-set rule lives in `claim_vault` so it can + // gate *new* identifiers without rejecting one the account already owns — + // the iOS and web clients used to let users type anything, so identifiers + // like `Family Vault` exist in the wild and must stay claimable. + if let Some(requested) = req.vault_id.as_deref() + && (requested.is_empty() || requested.len() > ldgr_core::sync::MAX_VAULT_ID_LEN) + { + return Err(ServerError::BadRequest(format!( + "vault_id must be 1-{} characters", + ldgr_core::sync::MAX_VAULT_ID_LEN + ))); } - let created_at = state.db.create_vault(&req.vault_id, &user_id).await?; + let vault = state + .db + .claim_vault(req.vault_id.as_deref(), &user_id) + .await?; Ok(( StatusCode::CREATED, Json(VaultResponse { - id: req.vault_id, - created_at, + id: vault.id, + created_at: vault.created_at, }), )) } diff --git a/crates/ldgr-server/src/storage/mod.rs b/crates/ldgr-server/src/storage/mod.rs index fbd89ca..7037510 100644 --- a/crates/ldgr-server/src/storage/mod.rs +++ b/crates/ldgr-server/src/storage/mod.rs @@ -198,6 +198,11 @@ impl ServerDb { // schema again is enough to add them to a legacy DB. conn.execute_batch( "CREATE UNIQUE INDEX IF NOT EXISTS idx_users_email ON users(email); + -- Tenant-scoped vault lookups (ADR-011). Additive and safe on a + -- pre-ADR-011 database: `vaults.id` was already a global primary + -- key there, so (user_id, id) is trivially unique and no existing + -- row can violate it. + CREATE UNIQUE INDEX IF NOT EXISTS idx_vaults_user_id ON vaults(user_id, id); CREATE TABLE IF NOT EXISTS settings ( key TEXT PRIMARY KEY, value TEXT NOT NULL @@ -867,29 +872,105 @@ impl ServerDb { // ── Vaults ──────────────────────────────────────────────────────────────── - pub async fn create_vault(&self, id: &str, user_id: &str) -> Result { + /// Claim a vault identifier for `user_id`, returning the identifier now in + /// force (ADR-011). + /// + /// Resolution order, evaluated under a single connection lock so concurrent + /// claims cannot race past each other: + /// + /// 1. the account already owns `requested` — return it unchanged, whatever + /// it looks like, so re-running setup on a configured device is + /// idempotent even for a hand-typed identifier from an older client; + /// 2. `requested` is free and well-formed — claim it (this is how a + /// pre-ADR-011 client keeps its path-derived identifier, and therefore + /// its already-uploaded blobs); + /// 3. `requested` is owned by a **different** account, is absent, or is not + /// path-safe — mint a fresh random identifier instead. + /// + /// Step 3 is the anti-squatting rule. Returning a conflict here is what + /// allowed the first account to claim a guessable, path-derived identifier + /// to permanently lock every other account out of it. It also keeps a + /// path-unsafe identifier from ever entering the blob namespace, without + /// failing the request. + pub async fn claim_vault( + &self, + requested: Option<&str>, + user_id: &str, + ) -> Result { let conn = self.conn.clone(); - let id = id.to_string(); + let requested = requested.map(String::from); let user_id = user_id.to_string(); let created_at = chrono::Utc::now().to_rfc3339(); - let ts = created_at.clone(); + tokio::task::spawn_blocking(move || { let conn = conn .lock() .map_err(|e| ServerError::Internal(format!("lock poisoned: {e}")))?; - conn.execute( - "INSERT INTO vaults (id, user_id, created_at) VALUES (?1, ?2, ?3)", - params![id, user_id, ts], - ) - .map_err(|e| match e { - rusqlite::Error::SqliteFailure(err, _) - if err.code == rusqlite::ErrorCode::ConstraintViolation => - { - ServerError::Conflict("vault already exists".into()) + + if let Some(id) = requested.as_deref() { + let owner: Option<(String, String)> = conn + .query_row( + "SELECT user_id, created_at FROM vaults WHERE id = ?1", + params![id], + |row| Ok((row.get(0)?, row.get(1)?)), + ) + .optional()?; + + match owner { + // Already ours — idempotent. + Some((owner_id, existing_created_at)) if owner_id == user_id => { + return Ok(Vault { + id: id.to_string(), + user_id, + created_at: existing_created_at, + }); + } + // Free, and safe to interpolate into a blob path. + None if ldgr_core::sync::is_valid_vault_id(id) => { + // `INSERT OR IGNORE` rather than a plain insert: another + // server process sharing this database file could have + // claimed the identifier between the SELECT and here, and + // that must mint a substitute like any other taken + // identifier, not surface a constraint violation. + let inserted = conn.execute( + "INSERT OR IGNORE INTO vaults (id, user_id, created_at) \ + VALUES (?1, ?2, ?3)", + params![id, user_id, created_at], + )?; + if inserted == 1 { + return Ok(Vault { + id: id.to_string(), + user_id, + created_at, + }); + } + } + // Taken by another account, or not path-safe — fall through + // and mint a substitute rather than failing the request. + Some(_) | None => {} } - other => ServerError::from(other), - })?; - Ok(created_at) + } + + // Mint a random identifier. Retry on the (astronomically unlikely) + // collision rather than surfacing a constraint violation. + for _ in 0..8 { + let id = ldgr_core::sync::generate_vault_id(); + let inserted = conn.execute( + "INSERT OR IGNORE INTO vaults (id, user_id, created_at) VALUES (?1, ?2, ?3)", + params![id, user_id, created_at], + )?; + if inserted == 1 { + return Ok(Vault { + id, + user_id, + created_at, + }); + } + } + + Err(ServerError::Internal( + "failed to mint a unique vault identifier".into(), + )) }) .await? } diff --git a/crates/ldgr-server/src/storage/schema.rs b/crates/ldgr-server/src/storage/schema.rs index 04aa393..0eba3f7 100644 --- a/crates/ldgr-server/src/storage/schema.rs +++ b/crates/ldgr-server/src/storage/schema.rs @@ -67,6 +67,13 @@ CREATE TABLE IF NOT EXISTS vaults ( CREATE INDEX IF NOT EXISTS idx_vaults_user ON vaults(user_id); +-- Vault identifiers are tenant-scoped (ADR-011): a vault is only ever addressed +-- as (owning account, id), which is what every API lookup joins on. `id` stays +-- the global primary key so `blobs.path` (`{vault_id}/...`) remains unambiguous +-- across accounts; identifiers minted since ADR-011 carry 128 bits of entropy, +-- so the global namespace cannot be enumerated or squatted. +CREATE UNIQUE INDEX IF NOT EXISTS idx_vaults_user_id ON vaults(user_id, id); + CREATE TABLE IF NOT EXISTS blobs ( path TEXT PRIMARY KEY, vault_id TEXT NOT NULL REFERENCES vaults(id) ON DELETE CASCADE, diff --git a/crates/ldgr-server/tests/common/mod.rs b/crates/ldgr-server/tests/common/mod.rs index 0199a9f..53250ed 100644 --- a/crates/ldgr-server/tests/common/mod.rs +++ b/crates/ldgr-server/tests/common/mod.rs @@ -118,6 +118,13 @@ pub fn client() -> ServerSyncClient { ServerSyncClient::new(RouterSender::new()) } +/// A [`ServerSyncClient`] sharing an **already booted** server with another +/// client, so two accounts can be exercised against the same database. Used by +/// the tenant-isolation suite. +pub fn client_on(sender: &RouterSender) -> ServerSyncClient { + ServerSyncClient::new(sender.clone()) +} + /// Derive the master auth key (`MK_auth`) from a password, as a client would. pub fn auth_key(password: &[u8]) -> AuthKey { let mk = derive_master_key(password, b"argon-salt-16byte", &Argon2Params::test()) diff --git a/crates/ldgr-server/tests/device_pairing_e2e.rs b/crates/ldgr-server/tests/device_pairing_e2e.rs index b4ebdf4..5fe58c7 100644 --- a/crates/ldgr-server/tests/device_pairing_e2e.rs +++ b/crates/ldgr-server/tests/device_pairing_e2e.rs @@ -117,7 +117,7 @@ async fn devices_list_register_and_remove_round_trip() { let (a, _b) = two_devices().await; let vault = "vault-pair"; - a.create_vault(vault).await.expect("create vault"); + a.create_vault(Some(vault)).await.expect("create vault"); // No devices registered yet. let devices = a.list_devices(vault).await.expect("list empty"); diff --git a/crates/ldgr-server/tests/ffi_sync_client_e2e.rs b/crates/ldgr-server/tests/ffi_sync_client_e2e.rs index 99d999b..674c4c0 100644 --- a/crates/ldgr-server/tests/ffi_sync_client_e2e.rs +++ b/crates/ldgr-server/tests/ffi_sync_client_e2e.rs @@ -224,7 +224,9 @@ async fn ffi_single_secret_full_round_trip() { let token = c.token().await.expect("token present after login"); let vault = "vault-1".to_string(); - c.create_vault(vault.clone()).await.expect("create vault"); + c.create_vault(Some(vault.clone())) + .await + .expect("create vault"); // Device A: a real on-disk vault. Compose its pending events into a REAL // encrypted batch blob via the #201 pipeline (through the FFI `LdgrVault` @@ -342,7 +344,9 @@ async fn ffi_two_secret_full_round_trip() { assert!(c.is_authenticated().await); let vault = "vault-2skd".to_string(); - c.create_vault(vault.clone()).await.expect("create vault"); + c.create_vault(Some(vault.clone())) + .await + .expect("create vault"); // Push a REAL encrypted batch (composed from an on-disk vault) so the 2SKD // auth path is exercised against genuine ciphertext, not synthetic bytes. @@ -424,7 +428,9 @@ async fn ffi_real_batch_conflict_surfaces_through_transport() { let token = c.token().await.expect("token present after login"); let vault = "vault-conflict".to_string(); - c.create_vault(vault.clone()).await.expect("create vault"); + c.create_vault(Some(vault.clone())) + .await + .expect("create vault"); // Device A: a real vault with a transaction; capture its id + session key. let dir_a = tempfile::tempdir().unwrap(); @@ -507,7 +513,7 @@ async fn ffi_real_batch_conflict_surfaces_through_transport() { async fn ffi_unauthenticated_call_is_rejected() { let (c, _s) = client_with_sender(); // No register/login: the core client guards locally before sending. - let result = c.create_vault("nope".into()).await; + let result = c.create_vault(Some("nope".into())).await; assert!( matches!(result, Err(FfiSyncError::NotAuthenticated)), "expected NotAuthenticated, got {result:?}" diff --git a/crates/ldgr-server/tests/server_sync_client_e2e.rs b/crates/ldgr-server/tests/server_sync_client_e2e.rs index f2c8386..d36c6fb 100644 --- a/crates/ldgr-server/tests/server_sync_client_e2e.rs +++ b/crates/ldgr-server/tests/server_sync_client_e2e.rs @@ -34,7 +34,7 @@ async fn single_secret_full_round_trip() { assert!(c.is_authenticated()); let vault = "vault-1"; - c.create_vault(vault).await.expect("create vault"); + c.create_vault(Some(vault)).await.expect("create vault"); // Upload an encrypted batch and read it back byte-for-byte. let device = "device-a"; @@ -109,7 +109,7 @@ async fn two_secret_full_round_trip() { // An authenticated call must succeed and a blob must round-trip. let vault = "vault-2skd"; - c.create_vault(vault).await.expect("create vault"); + c.create_vault(Some(vault)).await.expect("create vault"); let device = "device-2skd"; let batch = "batch-2skd-1"; @@ -148,6 +148,6 @@ async fn login_2skd_with_wrong_secret_key_is_rejected() { async fn unauthenticated_call_is_rejected() { let c = client(); // No register/login: the client guards locally before sending. - let result = c.create_vault("nope").await; + let result = c.create_vault(Some("nope")).await; assert_eq!(result.unwrap_err(), ServerSyncError::NotAuthenticated); } diff --git a/crates/ldgr-server/tests/srp_handshake.rs b/crates/ldgr-server/tests/srp_handshake.rs index 5b4d50e..27b6c6e 100644 --- a/crates/ldgr-server/tests/srp_handshake.rs +++ b/crates/ldgr-server/tests/srp_handshake.rs @@ -139,7 +139,10 @@ async fn authenticated_vault_and_batch_flow() { client.login("carol", b"s3cr3t").await.expect("login"); // Create a vault, then list it back. - let vault = client.create_vault("vault-1").await.expect("create vault"); + let vault = client + .create_vault(Some("vault-1")) + .await + .expect("create vault"); assert_eq!(vault.id, "vault-1"); let vaults = client.list_vaults().await.expect("list vaults"); @@ -175,7 +178,7 @@ async fn authenticated_vault_and_batch_flow() { async fn unauthenticated_request_without_token_fails_locally() { let client = ServerSyncClient::new(RouterTransport::new()); let err = client - .create_vault("vault-x") + .create_vault(Some("vault-x")) .await .expect_err("should fail"); assert!(matches!(err, ServerSyncError::NotAuthenticated)); diff --git a/crates/ldgr-server/tests/sync_pipeline_e2e.rs b/crates/ldgr-server/tests/sync_pipeline_e2e.rs index b38ab6a..ca3db60 100644 --- a/crates/ldgr-server/tests/sync_pipeline_e2e.rs +++ b/crates/ldgr-server/tests/sync_pipeline_e2e.rs @@ -202,7 +202,7 @@ async fn logged_in_client(vault_id: &str) -> ServerSyncClient(label: &str, result: Result) { + match result { + Err(ServerSyncError::Http { status: 404, .. }) => {} + Err(other) => panic!("{label}: expected HTTP 404, got {other:?}"), + Ok(value) => panic!("{label}: CROSS-TENANT ACCESS SUCCEEDED, returned {value:?}"), + } +} + +/// Register and log in `username` on the shared server. +async fn account( + client: &mut ServerSyncClient, + username: &str, + password: &[u8], +) { + client.register(username, password).await.expect("register"); + client.login(username, password).await.expect("login"); + assert!(client.is_authenticated(), "{username} should be logged in"); +} + +/// Boot one server, register account A with a populated vault and account B as +/// an unrelated tenant. Returns `(alice, mallory, alice_vault_id)`. +async fn two_tenants() -> ( + ServerSyncClient, + ServerSyncClient, + String, +) { + let sender = RouterSender::new(); + + let mut alice = client_on(&sender); + account(&mut alice, "alice", b"alice-correct-horse").await; + + let vault = alice + .create_vault(None) + .await + .expect("alice creates a vault") + .id; + + alice + .put_batch(&vault, A_DEVICE, A_BATCH, A_BLOB) + .await + .expect("alice pushes a batch"); + alice + .put_snapshot(&vault, A_SNAPSHOT, A_BLOB) + .await + .expect("alice pushes a snapshot"); + alice + .put_device(&vault, A_DEVICE, b"alice-encrypted-device-info") + .await + .expect("alice registers a device"); + + let mut mallory = client_on(&sender); + account(&mut mallory, "mallory", b"mallory-correct-horse").await; + + (alice, mallory, vault) +} + +// ── Vault-scoped endpoints ────────────────────────────────────────────────────── + +#[tokio::test] +async fn other_tenant_is_denied_on_every_batch_endpoint() { + let (_alice, mallory, vault) = two_tenants().await; + + assert_denied( + "GET batch", + mallory.get_batch(&vault, A_DEVICE, A_BATCH).await, + ); + assert_denied( + "PUT batch", + mallory + .put_batch(&vault, A_DEVICE, "batch-injected", B_BLOB) + .await, + ); + assert_denied( + "LIST batches", + mallory + .list_batches(&vault, &ListBatchesQuery::default()) + .await, + ); +} + +#[tokio::test] +async fn other_tenant_is_denied_on_every_snapshot_endpoint() { + let (_alice, mallory, vault) = two_tenants().await; + + assert_denied( + "GET snapshot", + mallory.get_snapshot(&vault, A_SNAPSHOT).await, + ); + assert_denied( + "PUT snapshot", + mallory + .put_snapshot(&vault, "snapshot-injected", B_BLOB) + .await, + ); + assert_denied( + "LIST snapshots", + mallory + .list_snapshots(&vault, &ListSnapshotsQuery::default()) + .await, + ); +} + +#[tokio::test] +async fn other_tenant_is_denied_on_every_device_endpoint() { + let (_alice, mallory, vault) = two_tenants().await; + + assert_denied("LIST devices", mallory.list_devices(&vault).await); + assert_denied( + "PUT device", + mallory.put_device(&vault, "device-injected", b"info").await, + ); + assert_denied( + "DELETE device", + mallory.delete_device(&vault, A_DEVICE).await, + ); +} + +#[tokio::test] +async fn other_tenant_cannot_see_the_vault_in_its_own_listing() { + let (_alice, mallory, vault) = two_tenants().await; + + let listed = mallory.list_vaults().await.expect("list own vaults"); + assert!( + !listed.iter().any(|v| v.id == vault), + "another tenant's vault leaked into the listing: {listed:?}" + ); +} + +/// Positive control: the denials above must come from *authorization*, not from +/// a broken fixture. The owner still reaches everything after B's attempts, and +/// none of B's writes landed. +#[tokio::test] +async fn owner_still_reaches_everything_after_the_denied_attempts() { + let (alice, mallory, vault) = two_tenants().await; + + let _ = mallory + .put_batch(&vault, A_DEVICE, "batch-injected", B_BLOB) + .await; + let _ = mallory.delete_device(&vault, A_DEVICE).await; + + assert_eq!( + alice + .get_batch(&vault, A_DEVICE, A_BATCH) + .await + .expect("owner reads its batch"), + A_BLOB + ); + assert_eq!( + alice + .get_snapshot(&vault, A_SNAPSHOT) + .await + .expect("owner reads its snapshot"), + A_BLOB + ); + + let batches = alice + .list_batches(&vault, &ListBatchesQuery::default()) + .await + .expect("owner lists batches"); + assert_eq!( + batches.entries.len(), + 1, + "a rejected cross-tenant write still landed: {batches:?}" + ); + + let devices = alice + .list_devices(&vault) + .await + .expect("owner lists devices"); + assert_eq!( + devices.len(), + 1, + "a rejected cross-tenant delete still took effect: {devices:?}" + ); +} + +// ── Identifier squatting ──────────────────────────────────────────────────────── + +#[tokio::test] +async fn claiming_another_tenants_vault_id_yields_a_different_vault() { + let (alice, mallory, vault) = two_tenants().await; + + // Mallory asks for Alice's identifier verbatim. The server must neither + // hand it over nor fail: it mints Mallory a fresh identifier instead. + let claimed = mallory + .create_vault(Some(&vault)) + .await + .expect("claiming a taken id must succeed with a substitute, not error") + .id; + + assert_ne!( + claimed, vault, + "server handed a tenant an identifier already owned by another account" + ); + + // The substitute is a real, usable vault of Mallory's own. + mallory + .put_batch(&claimed, "device-m", "batch-m", B_BLOB) + .await + .expect("mallory can use the vault she was given"); + + // And Alice's vault is still hers alone. + assert_denied( + "GET batch after squat attempt", + mallory.get_batch(&vault, A_DEVICE, A_BATCH).await, + ); + assert_eq!( + alice + .get_batch(&vault, A_DEVICE, A_BATCH) + .await + .expect("owner unaffected by the squat attempt"), + A_BLOB + ); +} + +/// The bug this issue exists to fix, from the victim's side: a second account +/// asking for an identifier that a *first* account already took must still end +/// up with a working vault. Before #298 the server returned a conflict, which +/// clients swallowed at setup and then failed on at every push. +#[tokio::test] +async fn a_taken_identifier_never_locks_a_tenant_out() { + let sender = RouterSender::new(); + + // Both accounts derive the same identifier — exactly what the old + // hash-of-the-default-vault-path scheme produced for every user. + let shared = "vault_5f2e1c0a9b8d7e6f"; + + let mut first = client_on(&sender); + account(&mut first, "first", b"first-correct-horse").await; + let first_vault = first + .create_vault(Some(shared)) + .await + .expect("first claim") + .id; + assert_eq!(first_vault, shared, "an unclaimed id should be granted"); + + let mut second = client_on(&sender); + account(&mut second, "second", b"second-correct-horse").await; + let second_vault = second + .create_vault(Some(shared)) + .await + .expect("second account must not be locked out") + .id; + + assert_ne!(second_vault, first_vault); + second + .put_batch(&second_vault, "device-2", "batch-2", B_BLOB) + .await + .expect("second account can sync into its own vault"); + first + .put_batch(&first_vault, "device-1", "batch-1", A_BLOB) + .await + .expect("first account is unaffected"); +} + +#[tokio::test] +async fn reclaiming_an_owned_identifier_is_idempotent() { + let (alice, _mallory, vault) = two_tenants().await; + + // Re-running setup on an already-configured device must return the same + // vault, not mint a second one and orphan the uploaded blobs. + let again = alice + .create_vault(Some(&vault)) + .await + .expect("re-claim own vault") + .id; + assert_eq!(again, vault); + + let vaults = alice.list_vaults().await.expect("list own vaults"); + assert_eq!( + vaults.len(), + 1, + "re-claiming duplicated the vault: {vaults:?}" + ); + assert_eq!( + alice + .get_batch(&vault, A_DEVICE, A_BATCH) + .await + .expect("blobs survive a re-claim"), + A_BLOB + ); +} + +#[tokio::test] +async fn minted_identifiers_are_random_and_unguessable() { + let sender = RouterSender::new(); + let mut c = client_on(&sender); + account(&mut c, "randy", b"randy-correct-horse").await; + + let mut ids = std::collections::HashSet::new(); + for _ in 0..16 { + let id = c.create_vault(None).await.expect("mint a vault").id; + assert!( + ldgr_core::sync::is_random_vault_id(&id), + "server minted a non-random identifier: {id}" + ); + assert!(ids.insert(id), "server minted a duplicate identifier"); + } +} + +#[tokio::test] +async fn path_unsafe_identifiers_are_substituted_never_stored() { + let sender = RouterSender::new(); + let mut c = client_on(&sender); + account(&mut c, "picky", b"picky-correct-horse").await; + + // A path separator would let a caller escape its own blob namespace. The + // server never stores one — but it substitutes rather than failing, so an + // older client that sends something odd still ends up with a usable vault. + for bad in ["../etc/passwd", "vault/batches", "vault id", "vault.id"] { + let granted = c + .create_vault(Some(bad)) + .await + .expect("an unusable identifier must be substituted, not rejected") + .id; + assert_ne!(granted, bad, "server stored a path-unsafe identifier"); + assert!( + ldgr_core::sync::is_random_vault_id(&granted), + "expected a minted substitute, got {granted}" + ); + } + + let owned = c.list_vaults().await.expect("list own vaults"); + assert!( + owned + .iter() + .all(|v| ldgr_core::sync::is_valid_vault_id(&v.id)), + "a path-unsafe identifier reached the vaults table: {owned:?}" + ); +} + +/// Length is still a hard contract, matching what pre-ADR-011 servers enforced. +#[tokio::test] +async fn empty_and_oversized_identifiers_are_rejected() { + let sender = RouterSender::new(); + let mut c = client_on(&sender); + account(&mut c, "lengthy", b"lengthy-correct-horse").await; + + for bad in [String::new(), "a".repeat(129)] { + match c.create_vault(Some(&bad)).await { + Err(ServerSyncError::Http { status: 400, .. }) => {} + other => panic!( + "expected 400 for a {}-char vault_id, got {other:?}", + bad.len() + ), + } + } +} + +/// Older iOS and web builds let users type any vault identifier they liked, so +/// values like `Family Vault` exist on deployed servers. Tightening the +/// character set must not lock those accounts out of their own vault. +#[tokio::test] +async fn a_hand_typed_legacy_identifier_stays_claimable_by_its_owner() { + let path = std::env::temp_dir().join(format!("ldgr-legacy-id-{}.db", uuid::Uuid::now_v7())); + let path_str = path.to_str().unwrap().to_string(); + + // Seed the pre-ADR-011 row directly — today's server would never mint it. + { + let conn = rusqlite::Connection::open(&path_str).unwrap(); + conn.execute_batch( + "CREATE TABLE users ( + id TEXT PRIMARY KEY, username TEXT UNIQUE NOT NULL, + salt BLOB NOT NULL, verifier BLOB NOT NULL, created_at TEXT NOT NULL + ); + CREATE TABLE vaults ( + id TEXT PRIMARY KEY, + user_id TEXT NOT NULL REFERENCES users(id) ON DELETE CASCADE, + created_at TEXT NOT NULL + ); + INSERT INTO users (id, username, salt, verifier, created_at) + VALUES ('u1', 'vintage', x'5a', x'5a', '2020-01-01T00:00:00Z'); + INSERT INTO users (id, username, salt, verifier, created_at) + VALUES ('u2', 'newcomer', x'5b', x'5b', '2020-01-02T00:00:00Z'); + INSERT INTO vaults (id, user_id, created_at) + VALUES ('Family Vault', 'u1', '2020-01-01T00:00:00Z');", + ) + .unwrap(); + } + + let db = ldgr_server::storage::ServerDb::open(&path_str).expect("open + migrate"); + let reclaimed = db + .claim_vault(Some("Family Vault"), "u1") + .await + .expect("re-claim"); + assert_eq!( + reclaimed.id, "Family Vault", + "the owner was refused its own vault — sign-in would break for this account" + ); + + // A *different* account asking for the same identifier still gets a + // substitute, and the substitute is always path-safe. + let other = db + .claim_vault(Some("Family Vault"), "u2") + .await + .expect("claim"); + assert_ne!(other.id, "Family Vault"); + assert!(ldgr_core::sync::is_random_vault_id(&other.id)); + + drop(db); + let _ = std::fs::remove_file(&path); + let _ = std::fs::remove_file(format!("{path_str}-wal")); + let _ = std::fs::remove_file(format!("{path_str}-shm")); +} + +// ── Schema migration ──────────────────────────────────────────────────────────── + +/// A server that predates ADR-011 must pick up the tenant-scoped vault index in +/// place, without losing the vaults its users already registered. +#[tokio::test] +async fn legacy_database_gains_the_tenant_scoped_vault_index() { + let path = std::env::temp_dir().join(format!("ldgr-vault-migrate-{}.db", uuid::Uuid::now_v7())); + let path_str = path.to_str().unwrap().to_string(); + + // Hand-build a pre-ADR-011 database: vaults keyed globally, no + // (user_id, id) index, holding one already-registered path-derived vault. + { + let conn = rusqlite::Connection::open(&path_str).unwrap(); + conn.execute_batch( + "CREATE TABLE users ( + id TEXT PRIMARY KEY, username TEXT UNIQUE NOT NULL, + salt BLOB NOT NULL, verifier BLOB NOT NULL, created_at TEXT NOT NULL + ); + CREATE TABLE vaults ( + id TEXT PRIMARY KEY, + user_id TEXT NOT NULL REFERENCES users(id) ON DELETE CASCADE, + created_at TEXT NOT NULL + ); + INSERT INTO users (id, username, salt, verifier, created_at) + VALUES ('u1', 'legacy', x'5a', x'5a', '2020-01-01T00:00:00Z'); + INSERT INTO vaults (id, user_id, created_at) + VALUES ('vault_5f2e1c0a9b8d7e6f', 'u1', '2020-01-01T00:00:00Z');", + ) + .unwrap(); + } + + let db = ldgr_server::storage::ServerDb::open(&path_str).expect("open + migrate"); + + // The legacy vault survives and is still addressable by its old identifier. + let vaults = db.list_user_vaults("u1").await.expect("list legacy vaults"); + assert_eq!(vaults.len(), 1); + assert_eq!(vaults[0].id, "vault_5f2e1c0a9b8d7e6f"); + + // Re-claiming it is idempotent, so an un-upgraded client keeps working. + let reclaimed = db + .claim_vault(Some("vault_5f2e1c0a9b8d7e6f"), "u1") + .await + .expect("legacy client re-claims its vault"); + assert_eq!(reclaimed.id, "vault_5f2e1c0a9b8d7e6f"); + + { + let conn = rusqlite::Connection::open(&path_str).unwrap(); + let index_exists: i64 = conn + .query_row( + "SELECT COUNT(*) FROM sqlite_master \ + WHERE type = 'index' AND name = 'idx_vaults_user_id'", + [], + |row| row.get(0), + ) + .unwrap(); + assert_eq!(index_exists, 1, "tenant-scoped vault index was not created"); + } + + // Re-opening an already-migrated database must not fail. + drop(db); + let _ = ldgr_server::storage::ServerDb::open(&path_str).expect("re-open is idempotent"); + + let _ = std::fs::remove_file(&path); + let _ = std::fs::remove_file(format!("{path_str}-wal")); + let _ = std::fs::remove_file(format!("{path_str}-shm")); +} + +// ── Relay (device pairing) ────────────────────────────────────────────────────── + +/// Relay offers are keyed by an opaque offer id, so this locks in that they are +/// also bound to the account that created them — otherwise a third party who +/// learned an offer id could intercept a device-pairing exchange. +#[tokio::test] +async fn other_tenant_cannot_touch_a_relay_offer() { + let (alice, mallory, _vault) = two_tenants().await; + + let offer = alice + .create_offer(b"alice-encrypted-offer") + .await + .expect("alice opens a pairing offer"); + + assert_denied("GET offer", mallory.get_offer(&offer.offer_id).await); + assert_denied( + "POST offer response", + mallory + .post_offer_response(&offer.offer_id, b"mallory-hijack") + .await, + ); + assert_denied( + "GET offer response", + mallory.get_offer_response(&offer.offer_id).await, + ); + + // The owner's exchange still completes untouched. + alice + .post_offer_response(&offer.offer_id, b"alice-encrypted-response") + .await + .expect("owner responds to its own offer"); + assert_eq!( + alice + .get_offer_response(&offer.offer_id) + .await + .expect("owner reads its own response"), + b"alice-encrypted-response" + ); +} diff --git a/crates/ldgr-wasm/src/sync.rs b/crates/ldgr-wasm/src/sync.rs index e5ebcf2..bc1763b 100644 --- a/crates/ldgr-wasm/src/sync.rs +++ b/crates/ldgr-wasm/src/sync.rs @@ -359,16 +359,33 @@ impl WasmSyncClient { let pong = client.ping().await.map_err(sync_err)?; serde_json::to_string(&pong).map_err(|e| JsError::new(&format!("serialization error: {e}"))) } + /// Claim a vault, returning the identifier the server put in force. + /// + /// Pass `undefined` to have the server mint a random identifier (ADR-011). + /// The result is authoritative and may differ from `vaultId` — callers must + /// persist what they get back. #[wasm_bindgen(js_name = createVault)] - pub async fn create_vault(&self, vault_id: String) -> Result<(), JsError> { + pub async fn create_vault(&self, vault_id: Option) -> Result { let client = self.client(); client - .create_vault(&vault_id) + .create_vault(vault_id.as_deref()) .await - .map(|_| ()) + .map(|v| v.id) .map_err(sync_err) } + /// List the vaults this account owns, as a JSON array of + /// `{ id, created_at }`. A device with no identifier of its own uses this to + /// adopt an existing vault rather than creating a second, empty one + /// (ADR-011). + #[wasm_bindgen(js_name = listVaults)] + pub async fn list_vaults(&self) -> Result { + let client = self.client(); + let vaults = client.list_vaults().await.map_err(sync_err)?; + serde_json::to_string(&vaults) + .map_err(|e| JsError::new(&format!("serialization error: {e}"))) + } + /// Upload an encrypted event batch (put-if-absent). #[wasm_bindgen(js_name = putBatch)] pub async fn put_batch( diff --git a/docs/adr/008-self-hosting-and-account-auth.md b/docs/adr/008-self-hosting-and-account-auth.md index af309cd..f373c3c 100644 --- a/docs/adr/008-self-hosting-and-account-auth.md +++ b/docs/adr/008-self-hosting-and-account-auth.md @@ -315,6 +315,11 @@ The change is **additive and backward compatible** at the server schema level. ## See also +- [ADR-011: Stable, Tenant-Scoped Vault Identifiers](011-tenant-scoped-vault-identifiers.md) — + refines the multi-tenant story this ADR opens up. ADR-008 makes an instance + multi-account; ADR-011 makes vault identifiers random, server-issued, and + scoped to the owning account, so two accounts on one instance cannot collide, + enumerate each other, or squat an identifier. - [Self-Hosting guide](../self-hosting.md) — the operator's walkthrough that applies this ADR: deploy, first-run admin onboarding, registration policy, adding users, the two-secret account model + Emergency Kit, and the threat diff --git a/docs/adr/011-tenant-scoped-vault-identifiers.md b/docs/adr/011-tenant-scoped-vault-identifiers.md new file mode 100644 index 0000000..f1cbb91 --- /dev/null +++ b/docs/adr/011-tenant-scoped-vault-identifiers.md @@ -0,0 +1,202 @@ +# ADR-011: Stable, Tenant-Scoped Vault Identifiers + +**Status**: Accepted +**Date**: 2026-07-27 +**Decision makers**: @kafkade + +## Context + +Every vault that syncs is known to the server by a **vault identifier**. It appears in the URL +of every vault-scoped endpoint (`/api/v1/vaults/{vault_id}/batches/…`) and it is the first path +segment of every blob in every transport's blob store: + +```text +{vault_id}/ + batches/{device_id}/{batch_id}.enc + snapshots/{snapshot_id}.enc + devices/{device_id}.json.enc +``` + +Up to v1.2.0 that identifier was **derived, not stored**. The CLI computed it as a djb2 hash of +the vault *directory path* — `vault_{hash:016x}` — and recomputed it on every command. The iOS +and web clients did something worse: they asked the **user to type it** into a free-text field. + +Three problems follow, and they compound. + +**It is guessable.** A 64-bit non-cryptographic hash of a short, highly predictable string is +trivially enumerable, and a hand-typed identifier is worse still — real users type `vault`, +`personal`, `main`, or their email address. The identifier also leaks that it is path-derived. + +**It collides across accounts.** Nearly every user keeps the vault at the default path, so +nearly every user derived the *same* identifier. The identifier was a global primary key, so the +first account to register it claimed it for the whole server. + +**The collision was a permanent denial of service, and it failed silently.** A second account +registering the same identifier got a `409 Conflict`, which the CLI deliberately swallowed +("idempotent — a 409 means it already does") — so `ldgr sync setup` reported success. Every +subsequent `sync push` and `sync pull` then failed with `404` from the ownership check, because +the vault existed but belonged to someone else. On any shared or multi-tenant host, one account +could permanently lock every other account out of the default identifier, and the victim had no +way to tell why. + +Authorization itself was **not** broken: `require_vault_access` was already called by all nine +vault-scoped handlers, and relay offers were already bound to the creating account. There was no +cross-tenant read. What was missing was unguessable, stable, per-account identifiers — and any +test that would fail if a future handler forgot the ownership check. + +### The constraint that makes this non-trivial + +The obvious fix — "generate a random UUID per device" — **breaks multi-device sync**. Two +devices must map the same logical vault to the same server vault or they never converge: the +second device would create its own empty vault and silently sync into the void. + +The old scheme only worked by accident. Two devices agreed *because they hashed the same default +path* — the exact mechanism that made two accounts collide. Removing the collision therefore +means replacing the convergence mechanism too, not just the identifier. + +## Decision + +### 1. Identifiers are random and persisted, never derived + +A vault identifier is `v1_` followed by 32 hex characters — 128 bits from the platform CSPRNG, +minted by `ldgr_core::sync::generate_vault_id()`. It is written once to the vault's `sync_state` +table and read thereafter, mirroring how `device_id` already works. Nothing derives it from a +path, a name, or anything else a third party could reproduce. + +Vault-key derivation (`HKDF(vault_key, "ldgr-vault-id")`) was considered and rejected. It is +elegant — it converges across devices for free — but it couples a server-visible identifier to +key material and would break under any future vault-key rotation. + +### 2. The server's response is authoritative, and a taken identifier is never a conflict + +`POST /api/v1/vaults` takes an **optional** `vault_id`. `ServerDb::claim_vault` resolves it under +a single connection lock, so concurrent claims cannot race: + +| Request | Outcome | +| --- | --- | +| identifier already owned by this account | returned unchanged, whatever it looks like (re-running setup is idempotent) | +| identifier free and path-safe | granted | +| identifier owned by **another** account | a fresh random identifier is minted and returned | +| identifier not path-safe | a fresh random identifier is minted and returned | +| no identifier supplied | a fresh random identifier is minted and returned | + +The character-set rule gates only *new* claims. Older iOS and web builds let users +type any identifier they liked, so values like `Family Vault` exist on deployed +servers; refusing to re-claim one would lock that account out of a vault it +demonstrably owns. Only the length contract (1-128 characters) is still a hard +rejection, matching what pre-ADR-011 servers enforced. + +The third row is the fix. Returning a conflict is what let one account lock out another; minting +a substitute means a squatter gains nothing and a victim is never blocked. Clients persist the +identifier they receive, not the one they asked for. + +### 3. Lookups stay scoped to the authenticated account + +Every vault-scoped handler calls `require_vault_access`, which answers `404 Not Found` — never +`403 Forbidden` — for a vault the caller does not own, so the identifier namespace cannot be +probed for existence. + +To be precise about where the security actually comes from, because it is easy to point at the +wrong thing: the properties this ADR claims rest on four mechanisms, and the schema is not one +of them. + +1. Identifiers carry 128 bits of CSPRNG entropy, so they cannot be guessed or enumerated. +2. The server mints a substitute instead of returning a conflict, so squatting cannot deny + service to anyone. +3. `require_vault_access` scopes every vault-scoped lookup to the authenticated account — this + predates the ADR and was already correct. +4. The cross-tenant regression tests in `crates/ldgr-server/tests/tenant_isolation.rs` fail + loudly if a future handler forgets (3). + +The `UNIQUE(user_id, id)` index added alongside is **redundant belt-and-braces**, not a control: +`vaults.id` is already a global primary key, so `(user_id, id)` cannot be violated by any row +that the primary key would admit. It is kept purely as intent documentation — it states the +tenant-scoped shape of the data in the schema itself, and it is what a future composite-key +migration would need anyway. Removing it would change no behaviour. + +`vaults.id` deliberately remains the **global** primary key. Relaxing it to a composite key would +let two accounts hold the same identifier, and because `blobs.path` is literally +`{vault_id}/…` that would *create* a cross-tenant read hole where none exists — closing it again +would mean re-keying `blobs` and `devices` onto an internal surrogate and rewriting every stored +blob. With 128-bit random identifiers the global namespace is collision-free by construction, so +the rewrite would buy no security. + +### 4. Devices converge by adopting, not by re-deriving + +A device that has already synced keeps its identifier. A device that has not **adopts** one: + +- **`ldgr sync setup`** lists the account's vaults after login. None → let the server mint one. + Exactly one → adopt it. Several (accounts are multi-vault since #296) → ask which. +- **`ldgr devices join`** already receives the vault key over the end-to-end encrypted relay. The + key itself identifies the vault: the joiner tries one batch from each candidate vault and + adopts the one that decrypts. No new pairing-protocol fields, and no chance of the user + choosing wrong. It falls back to the account's only vault, and declines to guess when several + vaults exist and none has a batch to test against. +- **iOS and web** replace the typed field with the same adopt-or-create flow. + +## Consequences + +### Positive + +- Identifiers carry 128 bits of entropy: not guessable, not enumerable, and they leak nothing + about where the vault lives on disk. +- Squatting is impossible. Two accounts cannot collide by construction, and even a deliberately + copied identifier only produces a fresh vault for the copier. +- The identifier survives moving or renaming the vault directory. Under the old scheme that + silently repointed sync at a different, empty namespace. +- It fixes a latent silent failure: the CLI used to build its blob transport from the identifier + in `sync-config.json` but hand the push/pull bridge a freshly recomputed one. When those + diverged, the server transport matched neither list prefix and returned an empty page, so + `ldgr sync pull` reported "Already up to date" while remote batches existed. +- Users no longer invent identifiers, so a whole class of "I typed the wrong vault name" support + problems disappears. + +### Negative / trade-offs + +- A second device can no longer join by typing the same string. It must either pair + (`ldgr devices add` / `join`) or sign in and adopt — which is correct, since a device without + the vault key could never read the data anyway, but it is a visible workflow change. +- Vault identifiers are opaque, so a user cannot recognise their vault at a glance. The CLI + prints it in `ldgr sync status` and both apps show it once connected. +- `vaults.id` staying globally unique means the tenant scoping is enforced by the ownership check + and the random namespace rather than by the primary key. This is a deliberate trade recorded in + §3, not an oversight. + +## Migration + +No existing user may be orphaned from data they have already synced. + +**Server.** Additive only: one guarded `CREATE UNIQUE INDEX IF NOT EXISTS idx_vaults_user_id ON +vaults(user_id, id)` in `ServerDb::migrate`. Safe on a v1.2.0 database because `vaults.id` was +already a global primary key there, so `(user_id, id)` is trivially unique and no existing row +can violate it. No table rebuild and no blob rewrite. + +**Clients.** `resolve_vault_id` adopts the identifier an upgrading vault is already filed under, +first match winning: + +1. the identifier stored in `sync_state`; +2. the one persisted in `sync-config.json` by a configured server transport; +3. for a configured Dropbox/WebDAV vault — which stores no identifier anywhere — the legacy + path-derived value, recomputed **once** and then frozen; +4. otherwise a fresh random identifier. + +Steps 2 and 3 are the upgrade path. Without them an upgrading user would silently get a new +identifier and lose sight of everything already uploaded. The legacy djb2 derivation survives as +a private, migration-only helper with a test pinning its exact output, so a future refactor +cannot quietly change which blobs a legacy vault adopts. + +**Compatibility.** An old client keeps sending its identifier and, if free, still gets it — +unchanged behaviour. In the other direction, a new client talking to a **pre-ADR-011 server** +handles both of that server's behaviours: omitting `vault_id` trips its required-field +validation (axum's `Json` extractor rejects the body with `422`, or the handler answers `400`), +so the client retries once with a locally minted identifier; and re-claiming an identifier it +already holds returns `409 Conflict`, which the client reads as "already registered, and ours" +rather than an error — otherwise upgrading the client before the server would lock users out of +re-authenticating when their session token expires. + +## Related + +- ADR-003 (sync and conflict resolution), ADR-004 (data model), ADR-005 (platform boundaries), + ADR-008 (self-hosting and account auth — this refines the multi-tenant story). +- Issues #298 (this ADR) and #296 (multi-vault accounts, which is why disambiguation is needed). +- This corresponds to finding F9 of the private security assessment. diff --git a/docs/security/threat-model.md b/docs/security/threat-model.md index 0e853fc..3d0cc07 100644 --- a/docs/security/threat-model.md +++ b/docs/security/threat-model.md @@ -148,7 +148,26 @@ secrets cannot leak into logs or crash dumps. These are **best-effort** mitigati cannot defeat a dump taken at the exact moment keys are live, and the OS may page memory to swap outside ldgr's control. Classified partial, honestly. -### 4.5 Malicious app co-resident on the same device — **out of scope** +### 4.5 Another tenant on a shared sync server — **in scope** + +*Capabilities:* a fully authenticated account on the same self-hosted or multi-tenant +server, able to make any authenticated API call and to guess or enumerate identifiers. + +*Analysis:* every vault-scoped endpoint (batches, snapshots, devices) checks that the +authenticated account owns the vault before doing anything, and answers `404 Not Found` +rather than `403 Forbidden`, so a tenant cannot even confirm whether another tenant's +vault exists. Relay offers used for device pairing are likewise bound to the account that +created them. Vault identifiers carry 128 bits of CSPRNG entropy and are issued by the +server, so they cannot be guessed, enumerated, or claimed — and a request for an +identifier another account already holds is answered with a freshly minted one instead of +a conflict, so a hostile tenant cannot deny service by squatting on it. Even a tenant who +learns another's identifier reads nothing, because the ownership check is independent of +how the identifier was obtained. **A co-tenant cannot read, write, or block another +tenant's data.** Residual exposure: a co-tenant on a shared instance can still consume +shared resources (disk, bandwidth) up to their quota. See +[ADR-011](../adr/011-tenant-scoped-vault-identifiers.md). + +### 4.6 Malicious app co-resident on the same device — **out of scope** *Capabilities:* another application running on the same device, possibly with elevated or root privileges. @@ -169,8 +188,9 @@ Out of scope; see [§6](#6-what-the-vault-does-not-protect-against). | In transit | **Network interception** | Client-side AES-256-GCM applied before the transport; TLS as a second layer; GCM tags detect tampering | | At rest | **At-rest exposure** (stolen disk/file) | Argon2id-derived MEK wraps the Vault Key; all items AES-256-GCM-encrypted; metadata encrypted too | | At rest | **Offline brute force** | Argon2id memory-hard KDF makes each password guess costly; per-platform parameters tuned for resistance (§7) | +| Shared server | **Cross-tenant access & identifier squatting** | Every vault-scoped lookup is scoped to the authenticated account and answers `404` for anything else; vault identifiers are 128-bit random and server-issued, so they cannot be guessed or claimed (ADR-011) | -In all four cases the attacker is left with authenticated ciphertext and, at most, +In all five cases the attacker is left with authenticated ciphertext and, at most, coarse-grained metadata (bucketed sizes, item counts, sync timing). --- @@ -188,7 +208,7 @@ security model. keys or plaintext directly. The vault format assumes the code decrypting it is honest. - **A compromised or rooted operating system.** Root-level access can read process memory, intercept syscalls, or harvest cached keys (including a biometric-unlock MEK in the OS - keychain). Application-level encryption cannot defeat a hostile OS (see §4.5). + keychain). Application-level encryption cannot defeat a hostile OS (see §4.6). - **Rubber-hose cryptanalysis.** Coercion, legal compulsion, or extortion to reveal your password or recovery key is outside any cryptographic defense. - **Lost password *and* lost recovery key — unrecoverable by design.** There is no master @@ -281,6 +301,7 @@ Stated openly so reviewers know the current boundaries: | Network attacker (MITM) | ✅ Yes | Data encrypted before transport + TLS; per-blob GCM auth tags detect tampering | Traffic analysis; availability (drop/delay) | | Device thief — locked device / disk image | ✅ Yes (at rest) | Argon2id-wrapped Vault Key; all items + metadata encrypted | Offline guessing of weak passwords; out of scope if captured unlocked | | Forensic examiner — memory / swap dump | ⚠️ Partial | `Zeroize`/`ZeroizeOnDrop`, idle auto-lock, `Debug` redaction | Keys live in RAM while unlocked; OS may page to swap | +| Another tenant on a shared sync server | ✅ Yes | Every vault lookup scoped to the authenticated account (404, never 403); 128-bit server-issued vault identifiers; a taken identifier is re-minted, never a conflict (ADR-011) | Shared-resource consumption up to quota | | Malicious co-resident / rooted OS | ❌ No | Relies on OS process/sandbox boundary | Full compromise if the OS is hostile | | Keylogger / screen capture | ❌ No | None (endpoint trust assumed) | Password and plaintext fully exposed | | Compromised app binary (supply chain) | ❌ No | Reproducible builds / signing are process controls, not format guarantees | Malicious binary can exfiltrate keys/plaintext | diff --git a/docs/self-hosting.md b/docs/self-hosting.md index e8708d8..df83d2d 100644 --- a/docs/self-hosting.md +++ b/docs/self-hosting.md @@ -166,6 +166,27 @@ Compose recreates the `server` container with the new image; the `ldgr-data` volume (your database) is preserved. Back up before upgrading (below), and pin `LDGR_VERSION` so upgrades are deliberate rather than implicit. +### Upgrading past v1.2.0: vault identifiers + +Schema migrations run automatically when the server starts, and this one is purely +additive — a unique index on `vaults(user_id, id)`. No table is rebuilt and no blob is +rewritten, so the upgrade is fast regardless of how much data you host, and rolling +back to the previous image works too (older builds simply ignore the extra index). + +What changes operationally: vault identifiers are now random, unguessable, and issued +by the server rather than derived by the client +(see [ADR-011](adr/011-tenant-scoped-vault-identifiers.md)). Before this release nearly +every client derived the *same* identifier from its default vault path, so on a shared +server the first account to register it locked every other account out of that +identifier permanently — `sync setup` appeared to succeed and every later push or pull +then failed with a 404. That failure mode is gone: a request for an identifier another +account already owns is now answered with a freshly minted one instead of a conflict. + +Existing vaults keep their identifiers, so your users need to do nothing. Users still +stuck behind a squatted identifier are unblocked as soon as they update their client — +it will be issued a working identifier on the next `sync setup`. Because they were +never able to sync under the squatted identifier, there is no data to migrate. + ## Backup and restore All server state lives in the named volume `ldgr-data` (the SQLite database at @@ -450,6 +471,13 @@ For the full rationale see | Encrypted vault blobs | No — AES-256-GCM ciphertext, size-bucket padded | | `(salt, verifier)` per account | No — the verifier is `g^x mod N`, not your password | | Email / username | Identity only | +| Vault identifiers | Random 128-bit values; they reveal nothing about the vault or where it lives | + +**Tenant isolation.** Every vault-scoped endpoint checks that the authenticated +account owns the vault, and answers `404 Not Found` rather than `403 Forbidden` so the +identifier namespace cannot be probed for existence. Identifiers carry 128 bits of +entropy, so one account can neither guess another's nor claim it +([ADR-011](adr/011-tenant-scoped-vault-identifiers.md)). **What the server never sees:** your password, your Account Secret Key, your encryption keys, or any plaintext financial data. With SRP-6a your password is diff --git a/docs/sync-setup.md b/docs/sync-setup.md index 9a2122b..d7c5072 100644 --- a/docs/sync-setup.md +++ b/docs/sync-setup.md @@ -27,8 +27,8 @@ All encryption happens on your devices. └──────────────┘ ``` -Both devices use the **same account** and the **same vault ID**; the server only -relays encrypted batches between them. +Both devices are signed in to the **same account** and connected to the **same vault**; +the server only relays encrypted batches between them. --- @@ -180,8 +180,20 @@ How you authenticate depends on what your server advertises at Either way your password never leaves the device (SRP-6a): the server stores only a verifier, never the password itself (see [Threat-model recap](#threat-model-recap)). -Pick a **vault ID** and use the **same vault ID and the same account** on every device -you want to keep in sync. +### Vault identifiers + +You never choose or type a vault identifier. Each vault is issued a random, unguessable +one the first time it connects to a server, and it is stored inside the vault from then +on — so it survives moving or renaming your vault directory, and two accounts can never +collide on a shared server (see [ADR-011](adr/011-tenant-scoped-vault-identifiers.md)). + +A new device connects to your existing vault in one of two ways: sign in to the same +account and it adopts the vault already registered there (picking one if you have +several), or pair it with `ldgr devices add` / `join`, which transfers the vault key and +the vault identifier together. + +Upgrading from v1.2.0 or earlier keeps your existing identifier, so everything you have +already synced stays reachable — there is nothing to do. > **Save your Emergency Kit.** The Secret Key is shown **once**, at sign-up. If you > lose it you can still use every device already signed in, but you won't be able to @@ -194,14 +206,18 @@ In the vault's **Settings → Sync (ldgr-server)** panel: 1. Enter the **Server URL** (e.g. `https://sync.example.com`) and click **Connect**. The panel shows the server name, protocol version, and whether it uses two-secret auth. -2. Fill in **Vault ID**, **Username**, and **Password**. +2. Fill in **Username** and **Password**. 3. On your first device, click **Create Account**. On a two-secret server the app generates your Secret Key and shows the **Emergency Kit** — save it (copy / download / print) before continuing. 4. On later devices, click **Sign In**. On a new device the app prompts for your **Secret Key** (paste it from the Kit); after that it's remembered for that device. +5. Click **Connect this browser to a vault**. If your account already has one vault the + browser adopts it; if it has several you pick which one; if it has none the server + issues a new one. -The panel shows **🟢 Authenticated** with a short device ID when you're signed in. +The panel shows **🟢 Authenticated** with a short device ID when you're signed in, and +the vault identifier once one is in force. ### iOS / iPadOS app @@ -209,7 +225,7 @@ In **Sync** settings: 1. Enter the **Server URL** and tap **Connect** to validate it and read the server's capabilities. -2. Enter **Username**, **Password**, and **Vault ID**. +2. Enter **Username** and **Password**. 3. Tap **Create Account** on your first device — the app generates your Secret Key and presents the **Emergency Kit** (share sheet / screenshot / QR) to save once. On a new device tap **Sign In on This Device** and enter the **Secret Key** from your @@ -246,7 +262,7 @@ On success it saves a non-secret `sync-config.json` and the SRP session token in ```sh ldgr sync push # upload your local encrypted batches ldgr sync pull # download other devices' encrypted batches -ldgr sync status # show provider, device ID, last sync, pending counts +ldgr sync status # show provider, vault ID, device ID, last sync, pending counts ``` To pair another device without re-entering your account credentials by hand, use @@ -270,9 +286,10 @@ ldgr devices remove # deregister a device ## Step 3 — Sync a transaction between two devices -This walkthrough uses two clients signed in to the **same account** with the **same -vault ID** (two browsers, two devices, or one of each). The Web and iOS/macOS apps -both apply pulled changes; the CLI does not yet (see the note above). +This walkthrough uses two clients signed in to the **same account** and connected to the +**same vault** (two browsers, two devices, or one of each). You never type a vault +identifier — each client adopts one at sign-in, or receives it while pairing. The Web +and iOS/macOS apps both apply pulled changes; the CLI does not yet (see the note above). 1. **Device A** — add a transaction in the app as you normally would. 2. **Device A** — open Sync and click **Sync now** (Web) or **Sync Now** (iOS/macOS). @@ -391,7 +408,7 @@ single-secret (password-only) SRP-6a automatically. | `registration is invite-only` (403) on register | Default policy. Set `LDGR_REGISTRATION=open`, or register your first/admin account first, or issue an invite token via the admin API. | | Other devices can't reach the server | Running directly binds loopback (`127.0.0.1`). Set `LDGR_BIND_ADDR=0.0.0.0:8080` or front it with a reverse proxy. | | TLS / certificate errors | The server is plain HTTP. Terminate HTTPS at a reverse proxy (Caddy/nginx/Traefik) and point clients at the `https://` proxy URL. | -| A transaction won't appear on the other device | Confirm both devices use the **same account** and the **same vault ID**, and that you ran **Sync now** on both. Remember the CLI does not apply pulled batches yet — use the Web or iOS/macOS app. | +| A transaction won't appear on the other device | Confirm both devices are signed in to the **same account** and report the **same vault ID** (`ldgr sync status`, or the Sync panel in the apps), and that you ran **Sync now** on both. If they differ, the second device created its own vault — pair it with `ldgr devices join` instead. Remember the CLI does not apply pulled batches yet — use the Web or iOS/macOS app. | | Data lost after restarting the container | Persist the database with a volume: `-v ldgr-data:/data`. | | Login token stopped working after ~30 days | Sessions expire per `LDGR_SESSION_TTL_HOURS` (default 720 h). Sign in again. |