From a6aeeae23936eb84c452e4b1da99103ca8f79cd3 Mon Sep 17 00:00:00 2001 From: JC Brand Date: Wed, 5 Aug 2026 10:25:56 +0200 Subject: [PATCH 1/3] fix: Require worker replies to assert success The reply protocol inferred success from a missing error field, so a reply carrying neither resolved the job. verifySignature reports success by resolving with no value, which made an unintelligible reply indistinguishable from a valid signature. The worker now sends `ok: true` on success and a non-empty error string on failure. The manager resolves only on ok === true and treats anything else as a transport failure. --- CHANGELOG.md | 2 ++ src/curve25519_worker.ts | 21 +++++++++--- src/curve25519_worker_manager.ts | 29 ++++++++++++---- test/worker-backend.ts | 59 ++++++++++++++++++++++++++++---- 4 files changed, 92 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a656854..21f4abe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,8 @@ - A per-operation timeout (default 10s, `startWorker(url, { timeout })`, `0` to disable) catches a worker that loads but then hangs and never replies. It is treated as a transport failure and falls back like the others. +- The worker reply protocol now asserts success explicitly (`ok: true`) instead of + inferring it from the absence of an error. - New build `libomemo.js/worker-client` (ESM): the same public API as the default build but with **no bundled WebAssembly**. Its local backend is a stub that throws until `startWorker(url)` is called, so the worker carries the only copy diff --git a/src/curve25519_worker.ts b/src/curve25519_worker.ts index 77fc4d5..86ce496 100644 --- a/src/curve25519_worker.ts +++ b/src/curve25519_worker.ts @@ -17,25 +17,36 @@ const ALLOWED_METHODS = new Set([ "ed25519PubKeyToCurvePubKey", ]); +/** + * Reduce a thrown value to a non-empty message string. + */ +function toErrorMessage(error: unknown): string { + if (error instanceof Error && error.message) return error.message; + if (typeof error === "string" && error) return error; + return "curve25519 worker operation failed"; +} + self.onmessage = (e: MessageEvent) => { const { id, methodName, args } = e.data; if (!ALLOWED_METHODS.has(methodName)) { - postMessage({ id, error: "Unsupported method." }); + postMessage({ id, ok: false, error: "Unsupported method." }); return; } const method = curve[methodName]; if (typeof method !== "function") { - postMessage({ id, error: "Unsupported method." }); + postMessage({ id, ok: false, error: "Unsupported method." }); return; } Promise.resolve((method.bind(curve) as (...a: unknown[]) => Promise)(...args)) .then((result: unknown) => { - postMessage({ id, result }); + // `ok: true` is what makes success explicit rather than inferred from + // the absence of an error. See the manager's #onMessage. + postMessage({ id, ok: true, result }); }) - .catch((error: Error) => { - postMessage({ id, error: error.message }); + .catch((error: unknown) => { + postMessage({ id, ok: false, error: toErrorMessage(error) }); }); }; diff --git a/src/curve25519_worker_manager.ts b/src/curve25519_worker_manager.ts index 2f2deb4..46f123a 100644 --- a/src/curve25519_worker_manager.ts +++ b/src/curve25519_worker_manager.ts @@ -19,6 +19,7 @@ interface Job { interface WorkerResponse { id: number; + ok?: boolean; // Explicit flag. Success must be asserted by the worker, not inferred. result?: unknown; error?: string; } @@ -35,8 +36,9 @@ class WorkerTransportError extends Error {} /** * Low-level message transport to the curve25519 Web Worker. Each `post()` is one * request/reply keyed by id. A normal error reply rejects just that job (an - * operation error). A worker-level failure rejects every pending job with a - * WorkerTransportError and notifies the owner through `onTransportError`. + * operation error). A worker-level failure (a crash, or a malformed reply) + * rejects every pending job with a WorkerTransportError and notifies the owner + * through `onTransportError`. */ class Curve25519Worker { #jobs = new Map(); @@ -57,19 +59,32 @@ class Curve25519Worker { } #onMessage(data: WorkerResponse): void { - const job = this.#take(data.id); + const job = this.#take(data?.id); if (!job) return; - if (data.error !== undefined) { - job.reject(new Error(data.error)); // operation error: caller must not fall back - } else { + + if (data.ok === true) { job.resolve(data.result); + return; + } + if (typeof data.error === "string" && data.error !== "") { + job.reject(new Error(data.error)); // operation error: caller must not fall back + return; } + + // Neither a well-formed success nor a well-formed failure. + // Fail closed as a transport error. + const malformed = new WorkerTransportError("curve25519 worker sent a malformed reply"); + job.reject(malformed); + this.#fail(malformed); } // Remove a job from the pending set and cancel its timeout, returning it. - #take(id: number): Job | undefined { + #take(id: number | undefined): Job | undefined { + if (id === undefined) return undefined; + const job = this.#jobs.get(id); if (!job) return undefined; + this.#jobs.delete(id); if (job.timer !== undefined) clearTimeout(job.timer); return job; diff --git a/test/worker-backend.ts b/test/worker-backend.ts index b527de4..26929cd 100644 --- a/test/worker-backend.ts +++ b/test/worker-backend.ts @@ -22,10 +22,16 @@ interface WorkerRequest { } interface Reply { + ok?: boolean; result?: unknown; error?: string; } +/** A well-formed success reply, i.e. one carrying the explicit `ok: true`. */ +function ok(result?: unknown): Reply { + return { ok: true, result }; +} + /** A responder decides how the fake worker answers a request. Returning * `undefined` leaves the request unanswered, simulating a hung worker. */ type Responder = (methodName: string, args: unknown[]) => Reply | undefined; @@ -83,7 +89,7 @@ function buf(len: number, fill = 0): ArrayBuffer { /** A well-formed keypair reply, so createKeyPair dispatch resolves cleanly. */ function keyPairReply(): Reply { - return { result: { pubKey: buf(33, 5), privKey: buf(32, 1) } }; + return ok({ pubKey: buf(33, 5), privKey: buf(32, 1) }); } describe("WorkerCurveBackend dispatch (Node, fake worker)", function () { @@ -111,7 +117,7 @@ describe("WorkerCurveBackend dispatch (Node, fake worker)", function () { // A sentinel pubKey the local wasm could never produce; if we get it back, // the operation genuinely ran in the worker rather than falling back. const sentinelPub = buf(33, 0xab); - worker.responder = () => ({ result: { pubKey: sentinelPub, privKey: buf(32, 0xcd) } }); + worker.responder = () => ok({ pubKey: sentinelPub, privKey: buf(32, 0xcd) }); const key = await internalCrypto.createKeyPair(); @@ -124,15 +130,16 @@ describe("WorkerCurveBackend dispatch (Node, fake worker)", function () { it("maps each CurveBackend operation to its worker protocol method name", async function () { startWorker(WORKER_URL); const worker = FakeWorker.only(); - worker.responder = () => ({ result: buf(32) }); + worker.responder = () => ok(buf(32)); await internalCrypto.createKeyPair(buf(32)); await internalCrypto.ECDHE(buf(33, 5), buf(32)); await internalCrypto.Ed25519Sign(buf(32), buf(4)); - // A defined result (not an error reply) makes verify resolve, not reject. - worker.responder = () => ({ result: undefined }); + // verifySignature succeeds by resolving with no value, so only `ok: true` + // distinguishes this from a malformed reply. + worker.responder = () => ok(undefined); await internalCrypto.Ed25519Verify(buf(33, 5), buf(4), buf(64)); - worker.responder = () => ({ result: buf(32) }); + worker.responder = () => ok(buf(32)); await internalCrypto.curvePubKeyToEd25519PubKey(buf(33, 5)); await internalCrypto.ed25519PubKeyToCurvePubKey(buf(32)); @@ -149,7 +156,7 @@ describe("WorkerCurveBackend dispatch (Node, fake worker)", function () { it("propagates an operation error from the worker without falling back", async function () { startWorker(WORKER_URL); - FakeWorker.only().responder = () => ({ error: "Invalid signature" }); + FakeWorker.only().responder = () => ({ ok: false, error: "Invalid signature" }); let caught: Error | undefined; try { @@ -218,6 +225,44 @@ describe("WorkerCurveBackend dispatch (Node, fake worker)", function () { expect(errorSpy.mock.calls.length).to.equal(1); }); + it("fails closed on a reply that does not assert success", async function () { + // The pre-fix protocol inferred success from the absence of an error, so a + // reply carrying neither (a stale worker build, a truncated or foreign + // message) resolved the job. For verifySignature, which reports success by + // resolving with no value, that is an accepted signature. Every shape below + // must therefore be treated as a transport failure, never as a result. + const malformed: Reply[] = [ + { result: undefined }, // no `ok`: indistinguishable from a valid verify + { result: "whatever" }, + { ok: false, error: "" }, // an Error built with no message + { ok: false }, // a thrown non-Error, whose `.message` was undefined + ]; + + for (const reply of malformed) { + stopWorker(); + FakeWorker.created = []; + errorSpy.mockClear(); + startWorker(WORKER_URL); + const worker = FakeWorker.only(); + worker.responder = () => reply; + + let caught: Error | undefined; + try { + // Falls back to the real local wasm, which rejects these junk inputs. + await internalCrypto.Ed25519Verify(buf(33, 5), buf(4), buf(64)); + } catch (e) { + caught = e as Error; + } + + expect(caught, `reply ${JSON.stringify(reply)} must not verify`).to.be.instanceOf( + Error + ); + // An unintelligible worker is dropped, not trusted for later replies. + expect(worker.terminated).to.equal(true); + expect(errorSpy.mock.calls.length).to.equal(1); + } + }); + it("stopWorker terminates a healthy worker and reverts to the local backend", async function () { startWorker(WORKER_URL); const worker = FakeWorker.only(); From 2c73263464b9b3e36eabc8efcadb3350514e3a45 Mon Sep 17 00:00:00 2001 From: JC Brand Date: Wed, 5 Aug 2026 10:48:03 +0200 Subject: [PATCH 2/3] fix: Do not disable worker offload after a single timeout A timeout rejected every pending job, terminated the worker, and latched the backend to the fallback for good. The default build then moved all private key operations back onto the main thread, and the worker-client build, whose fallback throws, failed every later operation for the lifetime of the page. A timer firing more than 250ms past its deadline now counts as a stalled main thread and grants the job one more interval. A lone timeout fails only its own operation. The worker is dropped after two consecutive timeouts or a crash, then rebuilt on a later operation with backoff. stopWorker() remains the only permanent stop. --- CHANGELOG.md | 1 + README.md | 5 + src/curve25519_worker_manager.ts | 151 ++++++++++++++++++++++++++----- test/worker-backend.ts | 69 +++++++++++++- 4 files changed, 201 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 21f4abe..d5c4729 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ It is treated as a transport failure and falls back like the others. - The worker reply protocol now asserts success explicitly (`ok: true`) instead of inferring it from the absence of an error. +- Worker failures no longer disable offloading for the lifetime of the page. - New build `libomemo.js/worker-client` (ESM): the same public API as the default build but with **no bundled WebAssembly**. Its local backend is a stub that throws until `startWorker(url)` is called, so the worker carries the only copy diff --git a/README.md b/README.md index 61654be..03fe7f9 100644 --- a/README.md +++ b/README.md @@ -279,6 +279,10 @@ falls back to the main-thread WebAssembly, so crypto keeps working. Errors the worker reports for a specific operation (an invalid signature, for example) are propagated unchanged and do not trigger a fallback. +**Important**: The worker script runs inside the trust boundary and it receives +raw private keys. Serve it as a trusted, same-origin script under your own CSP, +and do not build its URL from remote or user-supplied input. + ### The `worker-client` build (no bundled WebAssembly) For apps that always run a worker, `libomemo.js/worker-client` is a second build @@ -293,6 +297,7 @@ const identityKeyPair = await KeyHelper.generateIdentityKeyPair(); Any operation attempted before a worker is started (or while the worker is unavailable) rejects with a clear error, since there is no main-thread fallback. + This build is ESM-only and intended for the browser; under Node, use the default build. diff --git a/src/curve25519_worker_manager.ts b/src/curve25519_worker_manager.ts index 46f123a..a9321ba 100644 --- a/src/curve25519_worker_manager.ts +++ b/src/curve25519_worker_manager.ts @@ -11,10 +11,35 @@ import { getLocalCurveBackend, resetCurveBackend, setCurveBackend } from "./cryp */ const DEFAULT_WORKER_TIMEOUT_MS = 10_000; +/** + * How many consecutive timeouts, with no reply of any kind in between, mean the + * worker is hung rather than briefly slow. A single timeout only fails its own + * operation (which then completes on the fallback) and leaves the worker in place. + * Reaching this count tears it down, so later operations go to the fallback. + */ +const MAX_CONSECUTIVE_TIMEOUTS = 2; + +/** + * How far past its own deadline a timer may fire before we stop trusting it. + */ +const TIMER_OVERSHOOT_GRACE_MS = 250; + +/** + * Backoff bounds for rebuilding a worker that failed. The delay doubles per + * consecutive failure so a burst of operations cannot spawn a worker each, and + * resets as soon as a worker completes a round trip. + */ +const WORKER_RESTART_BASE_DELAY_MS = 1_000; +const WORKER_RESTART_MAX_DELAY_MS = 60_000; + interface Job { resolve: (result: unknown) => void; reject: (error: Error) => void; timer: ReturnType | undefined; + /** When this job's current timer is due to fire, for the overshoot check. */ + deadline: number; + /** Whether the one-shot grace period for a blocked main thread has been used. */ + regranted: boolean; } interface WorkerResponse { @@ -36,14 +61,15 @@ class WorkerTransportError extends Error {} /** * Low-level message transport to the curve25519 Web Worker. Each `post()` is one * request/reply keyed by id. A normal error reply rejects just that job (an - * operation error). A worker-level failure (a crash, or a malformed reply) - * rejects every pending job with a WorkerTransportError and notifies the owner - * through `onTransportError`. + * operation error), as does a lone timeout. A worker-level failure (a crash, a + * malformed reply, or repeated timeouts) rejects every pending job with a + * WorkerTransportError and notifies the owner through `onTransportError`. */ class Curve25519Worker { #jobs = new Map(); #jobId = 0; #dead = false; + #consecutiveTimeouts = 0; readonly #timeoutMs: number; readonly worker: Worker; onTransportError: (error: WorkerTransportError) => void = () => {}; @@ -62,6 +88,9 @@ class Curve25519Worker { const job = this.#take(data?.id); if (!job) return; + // A reply of any shape proves the worker is alive and draining its queue. + this.#consecutiveTimeouts = 0; + if (data.ok === true) { job.resolve(data.result); return; @@ -90,11 +119,35 @@ class Curve25519Worker { return job; } - // A job whose reply did not arrive in time: the worker is hung, so treat it as - // a transport failure (which fails every pending job and latches to fallback). + /** + * A job whose reply did not arrive in time. This fails only that job, which the + * backend then completes on the fallback. The worker itself is kept unless it + * misses MAX_CONSECUTIVE_TIMEOUTS replies in a row. A single slow operation, or + * a timer distorted by a blocked main thread, must not cost us the worker. + */ #onTimeout(id: number): void { - if (!this.#jobs.has(id)) return; - this.#fail(new WorkerTransportError("curve25519 worker timed out")); + const job = this.#jobs.get(id); + if (!job) return; + + // The timer fired far later than it was due, so it measured a stalled main + // thread rather than a stalled worker. Give the job one more full interval: + // the reply may already be queued behind this callback. + if (!job.regranted && Date.now() - job.deadline > TIMER_OVERSHOOT_GRACE_MS) { + job.regranted = true; + job.deadline = Date.now() + this.#timeoutMs; + job.timer = setTimeout(() => this.#onTimeout(id), this.#timeoutMs); + return; + } + + this.#jobs.delete(id); // its timer has already fired, nothing to clear + this.#consecutiveTimeouts++; + job.reject(new WorkerTransportError("curve25519 worker timed out")); + + // Repeated timeouts with no reply in between: the worker really is hung, so + // stop routing to it instead of making every later operation wait one out. + if (this.#consecutiveTimeouts >= MAX_CONSECUTIVE_TIMEOUTS) { + this.#fail(new WorkerTransportError("curve25519 worker timed out repeatedly")); + } } #fail(error: WorkerTransportError): void { @@ -116,11 +169,17 @@ class Curve25519Worker { return new Promise((resolve, reject) => { const id = this.#jobId++; - const timer = - this.#timeoutMs > 0 && isFinite(this.#timeoutMs) - ? setTimeout(() => this.#onTimeout(id), this.#timeoutMs) - : undefined; - this.#jobs.set(id, { resolve, reject, timer }); + const timed = this.#timeoutMs > 0 && isFinite(this.#timeoutMs); + const timer = timed + ? setTimeout(() => this.#onTimeout(id), this.#timeoutMs) + : undefined; + this.#jobs.set(id, { + resolve, + reject, + timer, + deadline: timed ? Date.now() + this.#timeoutMs : Infinity, + regranted: false, + }); this.worker.postMessage({ id, methodName, args }); }); } @@ -147,20 +206,47 @@ class Curve25519Worker { * share their names with the worker protocol. */ class WorkerCurveBackend implements CurveBackend { - #transport: Curve25519Worker | null = null; + readonly #url: string; readonly #fallback: CurveBackend; + readonly #timeoutMs: number; + #transport: Curve25519Worker | null = null; + #stopped = false; #loggedFailure = false; + #restartAttempts = 0; + #nextAttemptAt = 0; constructor(url: string, fallback: CurveBackend, timeoutMs: number) { + this.#url = url; this.#fallback = fallback; + this.#timeoutMs = timeoutMs; + this.#connect(); + } + + /** + * Build the worker, or return null if we should not try right now. + * + * A failed worker is not permanent. `#nextAttemptAt` backs off so a burst of + * operations cannot spawn a worker each, but a worker that was merely + * unreachable for a while is picked up again on a later operation. + * That recovery matters most in the `worker-client` build, whose + * fallback throws by design. Without it, one transport failure would break every + * subsequent OMEMO operation for the lifetime of the page. + */ + #connect(): Curve25519Worker | null { + if (this.#stopped) return null; + if (Date.now() < this.#nextAttemptAt) return null; + try { - const transport = new Curve25519Worker(url, timeoutMs); + const transport = new Curve25519Worker(this.#url, this.#timeoutMs); transport.onTransportError = (err) => this.#onTransportError(err); this.#transport = transport; + return transport; } catch (err) { + // `new Worker()` throws synchronously for a malformed URL or a CSP block. this.#onTransportError( err instanceof WorkerTransportError ? err : new WorkerTransportError(String(err)) ); + return null; } } @@ -173,6 +259,17 @@ class WorkerCurveBackend implements CurveBackend { ); } + // Back off before the next attempt, doubling per consecutive failure, so a + // permanently broken worker URL costs one spawn per interval rather than one + // per operation. #run resets this the moment a worker completes a round trip. + this.#nextAttemptAt = + Date.now() + + Math.min( + WORKER_RESTART_BASE_DELAY_MS * 2 ** this.#restartAttempts, + WORKER_RESTART_MAX_DELAY_MS + ); + this.#restartAttempts++; + // Terminate the failed worker before dropping the reference. A timed-out // worker is still a live thread, and `onerror` fires for any uncaught error // without killing the worker, so neither case is self-cleaning. @@ -182,15 +279,18 @@ class WorkerCurveBackend implements CurveBackend { } async #run(methodName: string, args: unknown[], local: () => Promise): Promise { - const transport = this.#transport; + const transport = this.#transport ?? this.#connect(); if (!transport) return local(); try { - return (await transport.post(methodName, args)) as T; + const result = (await transport.post(methodName, args)) as T; + this.#restartAttempts = 0; // a completed round trip: this worker works + return result; } catch (error) { - // The worker died mid-call; #onTransportError has already latched us - // to the fallback, so just complete this operation there. + // The worker died mid-call; #onTransportError has already dropped it and + // scheduled the next attempt, so just complete this operation locally. if (error instanceof WorkerTransportError) return local(); + this.#restartAttempts = 0; // an operation error also proves it is alive throw error; // operation error: propagate } } @@ -230,6 +330,7 @@ class WorkerCurveBackend implements CurveBackend { } terminate(): void { + this.#stopped = true; // an explicit stop must not be undone by #connect this.#transport?.terminate(); this.#transport = null; } @@ -241,11 +342,19 @@ let activeWorkerBackend: WorkerCurveBackend | null = null; * Offload curve operations to the Web Worker at `url` (typically the bundled * `dist/libomemo-worker.js`). Subsequent OMEMO crypto runs off the main thread. * If the worker cannot be loaded, or a call does not get a reply within - * `options.timeout` ms, calls fall back to the local backend: the bundled wasm in - * the default build, or a thrown error in the `worker-client` build, which has no + * `options.timeout` ms, that call falls back to the local backend: the bundled wasm + * in the default build, or a thrown error in the `worker-client` build, which has no * local wasm. * - * @param url URL of the worker script (must be same-origin / CSP-permitted). + * A failure is not permanent. A single timeout fails only its own operation; the + * worker is dropped after repeated timeouts or a crash, and is then rebuilt + * automatically on a later operation (with backoff), so a transient outage does not + * disable offloading, or, in the `worker-client` build, all cryptography, for the + * lifetime of the page. `stopWorker()` is the only permanent stop. + * + * @param url URL of the worker script. It receives raw private keys, so it must + * be a trusted same-origin, CSP-permitted script, not a URL derived + * from remote input. * @param options.timeout Per-operation reply timeout in milliseconds * (default 10000). Pass 0 to disable the timeout. */ diff --git a/test/worker-backend.ts b/test/worker-backend.ts index 26929cd..1b5c09f 100644 --- a/test/worker-backend.ts +++ b/test/worker-backend.ts @@ -39,6 +39,8 @@ type Responder = (methodName: string, args: unknown[]) => Reply | undefined; class FakeWorker { static created: FakeWorker[] = []; static constructShouldThrow = false; + /** Responder handed to every worker built from here on, for lazily-built workers. */ + static autoResponder: Responder | undefined; readonly url: string; readonly posted: WorkerRequest[] = []; @@ -54,6 +56,7 @@ class FakeWorker { throw new Error("failed to construct worker (bad URL / CSP)"); } this.url = url; + this.responder = FakeWorker.autoResponder; FakeWorker.created.push(this); } @@ -101,6 +104,7 @@ describe("WorkerCurveBackend dispatch (Node, fake worker)", function () { globalThis.Worker = FakeWorker as unknown as typeof Worker; FakeWorker.created = []; FakeWorker.constructShouldThrow = false; + FakeWorker.autoResponder = undefined; // Suppress and observe the single fallback log line. errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); }); @@ -198,11 +202,33 @@ describe("WorkerCurveBackend dispatch (Node, fake worker)", function () { expect(worker.terminated).to.equal(true); }); - it("times out a hung worker, falls back, and terminates it (leak fix)", async function () { + it("a single timeout fails only that op and keeps the worker", async function () { + // A lone timeout may just be a slow op or a main thread that was blocked + // long enough to distort the timer. Tearing the worker down for it would + // silently move all later crypto back onto the main thread, so it must not. startWorker(WORKER_URL, { timeout: 25 }); - const worker = FakeWorker.only(); // responder unset: the worker never replies + const worker = FakeWorker.only(); // responder unset: this one op hangs const key = await internalCrypto.createKeyPair(); + expect(key.pubKey.byteLength).to.equal(33); // rescued by the timeout + fallback + expect(worker.terminated).to.equal(false); + + // The worker is still in service, and a reply clears the timeout tally. + const sentinelPub = buf(33, 0xab); + worker.responder = () => ok({ pubKey: sentinelPub, privKey: buf(32, 0xcd) }); + const second = await internalCrypto.createKeyPair(); + expect(new Uint8Array(second.pubKey)).to.deep.equal(new Uint8Array(sentinelPub)); + }); + + it("times out repeatedly, falls back, and terminates the hung worker (leak fix)", async function () { + startWorker(WORKER_URL, { timeout: 25 }); + const worker = FakeWorker.only(); // responder unset: the worker never replies + + await internalCrypto.createKeyPair(); // first strike: op falls back, worker kept + expect(worker.terminated).to.equal(false); + expect(errorSpy.mock.calls.length).to.equal(0); + + const key = await internalCrypto.createKeyPair(); // second strike: really hung expect(key.pubKey.byteLength).to.equal(33); // rescued by the timeout + fallback expect(errorSpy.mock.calls.length).to.equal(1); @@ -214,14 +240,17 @@ describe("WorkerCurveBackend dispatch (Node, fake worker)", function () { startWorker(WORKER_URL, { timeout: 25 }); const worker = FakeWorker.only(); - await internalCrypto.createKeyPair(); // trips the timeout, latches to local + await internalCrypto.createKeyPair(); // first timeout + await internalCrypto.createKeyPair(); // second: drops the worker expect(errorSpy.mock.calls.length).to.equal(1); - // Subsequent calls must not re-post to the dead worker nor log again. + // Within the restart backoff, calls must not re-post to the dead worker, + // build a replacement, nor log again. const postedBefore = worker.posted.length; const key = await internalCrypto.createKeyPair(); expect(key.pubKey.byteLength).to.equal(33); expect(worker.posted.length).to.equal(postedBefore); + expect(FakeWorker.created).to.have.length(1); expect(errorSpy.mock.calls.length).to.equal(1); }); @@ -263,6 +292,38 @@ describe("WorkerCurveBackend dispatch (Node, fake worker)", function () { } }); + it("rebuilds the worker after the backoff instead of falling back forever", async function () { + // A transport failure must not disable offloading for the life of the page. + // It is the difference between a transient outage and permanently degraded + // crypto, and in the worker-client build (whose fallback throws) between a + // blip and every later OMEMO operation failing. + let now = 1_000_000; + const nowSpy = vi.spyOn(Date, "now").mockImplementation(() => now); + try { + FakeWorker.constructShouldThrow = true; + startWorker(WORKER_URL); + expect(errorSpy.mock.calls.length).to.equal(1); + + FakeWorker.constructShouldThrow = false; + const sentinelPub = buf(33, 0xab); + FakeWorker.autoResponder = () => ok({ pubKey: sentinelPub, privKey: buf(32, 0xcd) }); + + // Inside the backoff window: no rebuild attempt, straight to local. + const local = await internalCrypto.createKeyPair(); + expect(FakeWorker.created).to.have.length(0); + expect(new Uint8Array(local.pubKey)).to.not.deep.equal(new Uint8Array(sentinelPub)); + + // Past it: the next operation rebuilds the worker and runs there again. + now += 5_000; + const offloaded = await internalCrypto.createKeyPair(); + expect(FakeWorker.created).to.have.length(1); + expect(new Uint8Array(offloaded.pubKey)).to.deep.equal(new Uint8Array(sentinelPub)); + expect(errorSpy.mock.calls.length).to.equal(1); // still just the one log + } finally { + nowSpy.mockRestore(); + } + }); + it("stopWorker terminates a healthy worker and reverts to the local backend", async function () { startWorker(WORKER_URL); const worker = FakeWorker.only(); From 05c7eb59e8128e71afb28613913192b77ee9e188 Mon Sep 17 00:00:00 2001 From: JC Brand Date: Wed, 5 Aug 2026 11:27:48 +0200 Subject: [PATCH 3/3] feat: Report when curve operations stop running in the worker A worker failure moves every private key operation back onto the main thread, which the default build announced with a single console.error. Now that a dropped worker is rebuilt automatically, offloading can stop and resume several times in one page, and none of that was observable to the consumer. startWorker takes an onStatusChange callback that fires on each transition with { offloaded, error }. Logging follows the same edges, so a repeatedly failing worker is announced once rather than never or every time. --- CHANGELOG.md | 2 + README.md | 15 ++++++ src/curve25519_worker_manager.ts | 78 ++++++++++++++++++++++++++------ src/index.ts | 9 +++- src/types.ts | 24 ++++++++++ test/worker-backend.ts | 71 ++++++++++++++++++++++++++++- 6 files changed, 182 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d5c4729..483edae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,8 @@ - The worker reply protocol now asserts success explicitly (`ok: true`) instead of inferring it from the absence of an error. - Worker failures no longer disable offloading for the lifetime of the page. +- New `startWorker(url, { onStatusChange })` reports when operations start or stop + running in the worker. - New build `libomemo.js/worker-client` (ESM): the same public API as the default build but with **no bundled WebAssembly**. Its local backend is a stub that throws until `startWorker(url)` is called, so the worker carries the only copy diff --git a/README.md b/README.md index 03fe7f9..93ab002 100644 --- a/README.md +++ b/README.md @@ -279,6 +279,21 @@ falls back to the main-thread WebAssembly, so crypto keeps working. Errors the worker reports for a specific operation (an invalid signature, for example) are propagated unchanged and do not trigger a fallback. +Whether operations are currently offloaded is observable. In the default build +a fallback means private key operations have moved onto the main thread, which +some applications will want to surface or act on: + +```js +startWorker("/path/to/libomemo-worker.js", { + onStatusChange: ({ offloaded, error }) => { + if (!offloaded) console.warn("OMEMO crypto is on the main thread", error); + }, +}); +``` + +The callback is edge-triggered. It fires when offloading starts or stops, not per +operation, and not for `stopWorker()`. + **Important**: The worker script runs inside the trust boundary and it receives raw private keys. Serve it as a trusted, same-origin script under your own CSP, and do not build its URL from remote or user-supplied input. diff --git a/src/curve25519_worker_manager.ts b/src/curve25519_worker_manager.ts index a9321ba..667ac29 100644 --- a/src/curve25519_worker_manager.ts +++ b/src/curve25519_worker_manager.ts @@ -1,4 +1,4 @@ -import { CurveBackend, KeyPair } from "./types"; +import { CurveBackend, KeyPair, StartWorkerOptions, WorkerStatus } from "./types"; import { getLocalCurveBackend, resetCurveBackend, setCurveBackend } from "./crypto"; /** @@ -209,19 +209,56 @@ class WorkerCurveBackend implements CurveBackend { readonly #url: string; readonly #fallback: CurveBackend; readonly #timeoutMs: number; + readonly #onStatusChange: ((status: WorkerStatus) => void) | undefined; #transport: Curve25519Worker | null = null; #stopped = false; - #loggedFailure = false; #restartAttempts = 0; #nextAttemptAt = 0; - - constructor(url: string, fallback: CurveBackend, timeoutMs: number) { + #offloaded = true; // Whether operations are currently reaching the worker. + + constructor( + url: string, + fallback: CurveBackend, + timeoutMs: number, + onStatusChange?: (status: WorkerStatus) => void + ) { this.#url = url; this.#fallback = fallback; this.#timeoutMs = timeoutMs; + // Assigned before #connect so a worker that fails to construct is reported. + this.#onStatusChange = onStatusChange; this.#connect(); } + /** + * Record where operations are running, announcing only genuine transitions. + * + * The consumer's callback is untrusted. It runs inside our dispatch path, so a + * throw from it must not fail the operation that triggered it or surface as an + * unhandled rejection. + */ + #setOffloaded(offloaded: boolean, error?: Error): void { + if (this.#offloaded === offloaded) return; + this.#offloaded = offloaded; + + if (offloaded) { + console.info( + "libomemo.js: the curve25519 worker recovered; operations are offloaded again." + ); + } else { + console.error( + "libomemo.js: the curve25519 worker failed; falling back to the local backend.", + error + ); + } + + try { + this.#onStatusChange?.(error ? { offloaded, error } : { offloaded }); + } catch { + // A broken status handler is the consumer's problem, not the crypto's. + } + } + /** * Build the worker, or return null if we should not try right now. * @@ -251,13 +288,7 @@ class WorkerCurveBackend implements CurveBackend { } #onTransportError(error: Error): void { - if (!this.#loggedFailure) { - this.#loggedFailure = true; - console.error( - "libomemo.js: the curve25519 worker failed; falling back to the local backend.", - error - ); - } + this.#setOffloaded(false, error); // Back off before the next attempt, doubling per consecutive failure, so a // permanently broken worker URL costs one spawn per interval rather than one @@ -284,17 +315,23 @@ class WorkerCurveBackend implements CurveBackend { try { const result = (await transport.post(methodName, args)) as T; - this.#restartAttempts = 0; // a completed round trip: this worker works + this.#onRoundTrip(); return result; } catch (error) { // The worker died mid-call; #onTransportError has already dropped it and // scheduled the next attempt, so just complete this operation locally. if (error instanceof WorkerTransportError) return local(); - this.#restartAttempts = 0; // an operation error also proves it is alive + this.#onRoundTrip(); // an operation error also proves it is alive throw error; // operation error: propagate } } + /** A completed round trip: this worker works, so clear the backoff and report it. */ + #onRoundTrip(): void { + this.#restartAttempts = 0; + this.#setOffloaded(true); + } + createKeyPair(privKey: ArrayBuffer): Promise { return this.#run("createKeyPair", [privKey], () => this.#fallback.createKeyPair(privKey)); } @@ -352,16 +389,27 @@ let activeWorkerBackend: WorkerCurveBackend | null = null; * disable offloading, or, in the `worker-client` build, all cryptography, for the * lifetime of the page. `stopWorker()` is the only permanent stop. * + * Because that movement is invisible from the outside, and in the default build it + * means private-key operations have moved onto the main thread, pass + * `options.onStatusChange` to observe it rather than relying on the console. + * * @param url URL of the worker script. It receives raw private keys, so it must * be a trusted same-origin, CSP-permitted script, not a URL derived * from remote input. * @param options.timeout Per-operation reply timeout in milliseconds * (default 10000). Pass 0 to disable the timeout. + * @param options.onStatusChange Called whenever operations start or stop running in + * the worker. See {@link WorkerStatus}. */ -export function startWorker(url: string, options: { timeout?: number } = {}): void { +export function startWorker(url: string, options: StartWorkerOptions = {}): void { stopWorker(); const timeoutMs = options.timeout ?? DEFAULT_WORKER_TIMEOUT_MS; - const backend = new WorkerCurveBackend(url, getLocalCurveBackend(), timeoutMs); + const backend = new WorkerCurveBackend( + url, + getLocalCurveBackend(), + timeoutMs, + options.onStatusChange + ); activeWorkerBackend = backend; setCurveBackend(backend); } diff --git a/src/index.ts b/src/index.ts index 5cc3c81..56725c5 100644 --- a/src/index.ts +++ b/src/index.ts @@ -13,7 +13,14 @@ export { startWorker, stopWorker } from "./curve25519_worker_manager"; export { curvePubKeyToEd25519PubKey, ed25519PubKeyToCurvePubKey } from "./crypto"; export { BaseKeyType, ChainType } from "./types"; -export type { KeyPair, PreKey, SignedPreKey, PublicPreKey } from "./types"; +export type { + KeyPair, + PreKey, + SignedPreKey, + PublicPreKey, + StartWorkerOptions, + WorkerStatus, +} from "./types"; export { SessionRecord } from "./session/record"; export { default as InMemoryStore } from "./session/store"; diff --git a/src/types.ts b/src/types.ts index e75ce17..7c6088d 100644 --- a/src/types.ts +++ b/src/types.ts @@ -45,6 +45,30 @@ export interface InternalCryptoInterface { * The `internalCrypto` layer fills defaults (e.g. generates the private key for a keypair) * before delegating, so no backend needs its own RNG and the two backends stay interchangeable. */ + +/** + * Where curve operations are running, reported to `startWorker`'s `onStatusChange` + * whenever that changes. + */ +export interface WorkerStatus { + offloaded: boolean; // True while operations run in the worker + error?: Error; // Why offloading stopped. Present only when `offloaded` is false +} + +export interface StartWorkerOptions { + /** + * Per-operation reply timeout in milliseconds (default 10000). Pass 0 to + * disable the timeout. + */ + timeout?: number; + /** + * Called when operations start or stop running in the worker. Edge-triggered: + * it fires on a change, not per operation, and not for `stopWorker()`, which + * the caller already knows about. Throwing from it is contained and ignored. + */ + onStatusChange?: (status: WorkerStatus) => void; +} + export interface CurveBackend { createKeyPair(privKey: ArrayBuffer): Promise; ECDHE(pubKey: ArrayBuffer, privKey: ArrayBuffer): Promise; diff --git a/test/worker-backend.ts b/test/worker-backend.ts index 1b5c09f..5cb71f3 100644 --- a/test/worker-backend.ts +++ b/test/worker-backend.ts @@ -2,6 +2,7 @@ import { expect } from "chai"; import { beforeEach, afterEach, vi } from "vitest"; import { internalCrypto } from "../src/crypto.js"; import { startWorker, stopWorker } from "../src/curve25519_worker_manager.js"; +import type { WorkerStatus } from "../src/types.js"; /** * Node-runnable unit tests for the worker dispatch state machine @@ -98,6 +99,7 @@ function keyPairReply(): Reply { describe("WorkerCurveBackend dispatch (Node, fake worker)", function () { let savedWorker: typeof Worker; let errorSpy: ReturnType; + let infoSpy: ReturnType; beforeEach(function () { savedWorker = globalThis.Worker; @@ -105,13 +107,15 @@ describe("WorkerCurveBackend dispatch (Node, fake worker)", function () { FakeWorker.created = []; FakeWorker.constructShouldThrow = false; FakeWorker.autoResponder = undefined; - // Suppress and observe the single fallback log line. + // Suppress and observe the fallback log line. errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + infoSpy = vi.spyOn(console, "info").mockImplementation(() => {}); }); afterEach(function () { stopWorker(); errorSpy.mockRestore(); + infoSpy.mockRestore(); globalThis.Worker = savedWorker; }); @@ -324,6 +328,71 @@ describe("WorkerCurveBackend dispatch (Node, fake worker)", function () { } }); + it("reports offload transitions to onStatusChange, in both directions", async function () { + // Whether crypto is running in the worker is a security property in the + // default build (private keys on the main thread or not), and the automatic + // rebuild means it can change more than once. A console line is not an + // observable, so a consumer needs to be told. + let now = 1_000_000; + const nowSpy = vi.spyOn(Date, "now").mockImplementation(() => now); + const seen: WorkerStatus[] = []; + try { + startWorker(WORKER_URL, { onStatusChange: (s) => seen.push({ ...s }) }); + const worker = FakeWorker.only(); + worker.responder = () => keyPairReply(); + + // Neither a healthy start nor a healthy op is a transition. + await internalCrypto.createKeyPair(); + expect(seen).to.have.length(0); + + worker.emitError("worker boom"); + expect(seen).to.have.length(1); + expect(seen[0].offloaded).to.equal(false); + expect(seen[0].error).to.be.instanceOf(Error); + + // Still down: staying in a state is not a transition, so no repeat. + await internalCrypto.createKeyPair(); + expect(seen).to.have.length(1); + + // Rebuilt and answering again, so the consumer can clear what it showed. + now += 5_000; + FakeWorker.autoResponder = () => keyPairReply(); + await internalCrypto.createKeyPair(); + expect(seen).to.have.length(2); + expect(seen[1].offloaded).to.equal(true); + expect(seen[1].error).to.equal(undefined); + } finally { + nowSpy.mockRestore(); + } + }); + + it("does not report stopWorker(), which the caller already knows about", async function () { + const seen: WorkerStatus[] = []; + startWorker(WORKER_URL, { onStatusChange: (s) => seen.push({ ...s }) }); + FakeWorker.only().responder = () => keyPairReply(); + await internalCrypto.createKeyPair(); + + stopWorker(); + expect(seen).to.have.length(0); + }); + + it("contains a throwing onStatusChange handler", async function () { + // The handler runs inside the dispatch path, so a consumer bug must not + // fail the operation that triggered it. + startWorker(WORKER_URL, { + onStatusChange: () => { + throw new Error("consumer handler blew up"); + }, + }); + const worker = FakeWorker.only(); // responder unset: the op stays in flight + + const pending = internalCrypto.createKeyPair(); + worker.emitError("worker boom"); + + const key = await pending; + expect(key.pubKey.byteLength).to.equal(33); // completed on the local backend + }); + it("stopWorker terminates a healthy worker and reverts to the local backend", async function () { startWorker(WORKER_URL); const worker = FakeWorker.only();