fix: stop a stuck certificate call holding mail rendering, and let a server's certificate access be taken back

lib/__tests__/plugin-keystore-runtime.test.ts

@@ -71,21 +71,29 @@ describe('api.keystore in the sandbox', () => {
     expect(g.__pluginApi!.host.features.keystore).toBeFalsy();
   }, 30_000);
 
-  it('no call gives up while the host has not answered: a PIN can take minutes', async () => {
+  it('a list has the ordinary RPC timeout: it never waits on a card', async () => {
+    await boot({ keystore: true });
+    vi.useFakeTimers();
+    const listing = g.__pluginApi!.keystore.list();
+    const outcome = listing.then(() => 'answered', (error: Error) => error.message);
+    await vi.advanceTimersByTimeAsync(31_000);
+    await expect(outcome).resolves.toMatch(/timed out/);
+  }, 30_000);
+
+  it('no key operation gives up while the host has not answered: a PIN can take minutes', async () => {
     await boot({ keystore: true });
     vi.useFakeTimers();
     const api = g.__pluginApi!.keystore;
     const digest = new Uint8Array(32);
     const settled: string[] = [];
     const watch = (name: string, promise: Promise<unknown>) => promise.then(() => settled.push(name), () => settled.push(`${name} rejected`));
-    watch('list', api.list());
     watch('sign', api.sign('id', digest, { scheme: 'rsa-pkcs1', hash: 'SHA-256' }));
     watch('decrypt', api.decrypt('id', new Uint8Array(256), { scheme: 'rsa-pkcs1' }));
     watch('agree', api.agree('id', new Uint8Array(91)));
     // Past the RPC's 30 s and past the network calls' 120 s.
     await vi.advanceTimersByTimeAsync(600_000);
     expect(settled).toEqual([]);
-    expect(requests().map((request) => request.method)).toEqual(['keystore.list', 'keystore.sign', 'keystore.decrypt', 'keystore.agree']);
+    expect(requests().map((request) => request.method)).toEqual(['keystore.sign', 'keystore.decrypt', 'keystore.agree']);
   }, 30_000);
 
   it('passes the bytes as bytes, and hands an error back with the code as its message', async () => {

lib/__tests__/plugin-keystore.test.ts

@@ -281,6 +281,59 @@ describe('a keystore call and the hook timer', () => {
     expect(hasDialogFor(ID)).toBe(false);
   });
 
+  it('a list takes no hold: it never waits on a card, and a hung list must not stop the hook timer', async () => {
+    const bridge = fakeBridge();
+    bridge.list.mockImplementation(() => new Promise(() => {}));
+    installBridge(bridge);
+    renderHooks.onRenderEmailBody.register(ID, async (body: object) => {
+      await dispatchApiCall(plugin(), 'keystore.list', [], { privileged: true });
+      return { ...body, html: 'never' };
+    });
+    const ran = renderHooks.onRenderEmailBody.transform({ html: 'as it was' });
+    expect(hasDialogFor(ID)).toBe(false);
+    await vi.advanceTimersByTimeAsync(6000);
+    await expect(ran).resolves.toEqual({ html: 'as it was' });
+    expect(console.error).toHaveBeenCalled();
+  });
+
+  it('a call that never answers cannot hold the hook for ever: there is an absolute deadline', async () => {
+    const bridge = fakeBridge();
+    bridge.decrypt.mockImplementation(() => new Promise(() => {}));
+    installBridge(bridge);
+    // This call never ends, so its hold never does: an id of its own.
+    const hung = plugin({ id: 'hung-plugin' });
+    renderHooks.onRenderEmailBody.register(hung.id, async (body: object) => {
+      await dispatchApiCall(hung, 'keystore.decrypt', ['id', new Uint8Array(256), { scheme: 'rsa-pkcs1' }], { privileged: true });
+      return { ...body, html: 'never' };
+    });
+    const ran = renderHooks.onRenderEmailBody.transform({ html: 'as it was' });
+    let settled = false;
+    void ran.then(() => { settled = true; });
+    // Longer than the shell's own 5 minute cap, which would normally end it first.
+    await vi.advanceTimersByTimeAsync(5 * 60_000);
+    expect(settled).toBe(false);
+    await vi.advanceTimersByTimeAsync(11 * 60_000);
+    expect(settled).toBe(true);
+    await expect(ran).resolves.toEqual({ html: 'as it was' });
+    expect(console.error).toHaveBeenCalled();
+  });
+
+  it('holds that keep coming and going do not reset the deadline', async () => {
+    renderHooks.onRenderEmailBody.register(ID, () => new Promise<object>(() => {}));
+    let release = holdForUser(ID);
+    const ran = renderHooks.onRenderEmailBody.transform({ html: 'as it was' });
+    let settled = false;
+    void ran.then(() => { settled = true; });
+    for (let minute = 0; minute < 20 && !settled; minute += 1) {
+      await vi.advanceTimersByTimeAsync(4000);
+      release();
+      release = holdForUser(ID);
+      await vi.advanceTimersByTimeAsync(60_000 - 4000);
+    }
+    release();
+    expect(settled).toBe(true);
+  });
+
   it('a hook that hangs with no keystore call still times out', async () => {
     renderHooks.onRenderEmailBody.register(ID, () => new Promise<object>(() => {}));
     const ran = renderHooks.onRenderEmailBody.transform({ html: 'as it was' });

lib/desktop/types.ts

@@ -141,8 +141,13 @@ export interface DesktopKeystoreCertificate {
   signSchemes: string[];
   decryptSchemes: string[];
   canAgree: boolean;
+  /** The key can also sign the user in somewhere: the user is asked per use. */
+  askEachUse: boolean;
 }
 
+/** Whether this origin may use the keystore at all. `app` is standalone. */
+export type DesktopKeystoreAccessState = 'app' | 'policy' | 'stored' | 'off';
+
 export type DesktopKeystoreSignScheme =
   | { scheme: 'rsa-pkcs1'; hash: DesktopKeystoreHash }
   /** MGF1 on the same hash, salt as long as the hash. */
@@ -158,11 +163,13 @@ export type DesktopKeystoreDecryptScheme =
 /**
  * The OS certificate store and smart cards. Present only where the native helper
  * loaded and the OS has one. Failures reject with a bare code as the message:
- * keystore:cancelled, not-found, unsupported, locked, invalid, failed. The key never
- * leaves the OS, and its own prompts stay on, so a call may take as long as a PIN.
+ * keystore:cancelled, not-found, unsupported, locked, invalid, failed. The shell asks the
+ * user in a native dialog before any of this, on top of which the OS has its own prompts,
+ * so a call may take a while. On an instance it stays off until policy or the user allows
+ * it. The key never leaves the OS.
  */
 export interface DesktopKeystoreBridge {
-  /** Certificates fit for mail that have a private key. Never prompts. */
+  /** Certificates fit for mail that have a private key. The OS doesn't prompt. */
   list: () => Promise<DesktopKeystoreCertificate[]>;
   /** RSA gives the raw signature, ECDSA a DER ECDSA-Sig-Value. */
   sign: (id: string, digest: Uint8Array, scheme: DesktopKeystoreSignScheme) => Promise<Uint8Array>;
@@ -170,6 +177,9 @@ export interface DesktopKeystoreBridge {
   decrypt: (id: string, data: Uint8Array, scheme: DesktopKeystoreDecryptScheme) => Promise<Uint8Array>;
   /** The raw shared secret, big-endian. The peer's key is an EC SubjectPublicKeyInfo. */
   agree: (id: string, peerPublicKey: Uint8Array) => Promise<Uint8Array>;
+  accessState: () => Promise<DesktopKeystoreAccessState>;
+  /** Takes back a stored yes and this session's grants. Only ever narrows. */
+  revokeAccess: () => Promise<void>;
 }
 
 export interface BulwarkDesktopBridge {

lib/plugin-hooks.ts

@@ -71,6 +71,13 @@ const INTERCEPT_TIMEOUT_MS = 60_000;
 // counted them toward auto-disable.
 const BLOB_UPLOAD_TIMEOUT_MS = 15 * 60_000;
 
+/**
+ * The longest a hook may stay waiting on a dialog or an OS prompt, from its start. A
+ * person answers well within this. A plugin whose dialog is never answered, or a call the
+ * OS never finishes, must not hold mail rendering or a send forever.
+ */
+const HELD_HOOK_DEADLINE_MS = 10 * 60_000;
+
 class HookTimeoutError extends Error {}
 
 /** Tells the hook timer when a plugin is waiting on the user. */
@@ -97,20 +104,30 @@ function withTimeout<T>(pluginId: string, promise: T | Promise<T>, ms: number =
   if (!(promise instanceof Promise)) return Promise.resolve(promise);
   return new Promise<T>((resolve, reject) => {
     let timer: ReturnType<typeof setTimeout> | undefined;
+    let deadline: ReturnType<typeof setTimeout> | undefined;
     let stopWaiting: (() => void) | undefined;
+    const started = Date.now();
     const arm = () => {
       stopWaiting = undefined;
       timer = setTimeout(expire, ms);
     };
     const expire = () => {
       if (dialogGate?.hasOpenDialog(pluginId)) {
+        // One deadline from the start, however often holds come and go. A bus with a
+        // longer budget of its own has at least that.
+        deadline ??= setTimeout(() => {
+          settle();
+          reject(new HookTimeoutError(`Hook still waiting on the user after ${Date.now() - started}ms`));
+        }, Math.max(0, started + Math.max(HELD_HOOK_DEADLINE_MS, ms) - Date.now()));
         stopWaiting = dialogGate.whenDialogsClosed(pluginId, arm);
         return;
       }
+      clearTimeout(deadline);
       reject(new HookTimeoutError(`Hook timed out after ${ms}ms`));
     };
     const settle = () => {
       clearTimeout(timer);
+      clearTimeout(deadline);
       stopWaiting?.();
     };
     arm();

lib/plugin-sandbox/host-api-keystore.ts

@@ -3,9 +3,9 @@
 // private key never reaches the webmail, and the OS's own prompts stay on.
 //
 // Privileged tier only (host-api.ts), so a user-uploaded plugin never gets here.
-// A privileged frame is same-origin and can reach window.parent.bulwarkDesktop
-// directly; these checks are for the front door, and the safeguards that hold in
-// both cases are in the shell and the OS.
+// None of this is a security boundary: a privileged frame is same-origin and can reach
+// window.parent.bulwarkDesktop directly. The checks are for the front door. What holds in
+// both cases is in the shell: a native dialog in main (keystore-consent.ts) and the OS.
 //
 // RPC errors reach the frame as message strings only, so the code is the message:
 // keystore:cancelled, keystore:not-found, keystore:unsupported, keystore:locked,
@@ -118,8 +118,10 @@ export async function doKeystoreCall(plugin: InstalledPlugin, method: string, ar
   // The OS may be showing a PIN dialog right now, which the host can't see. While it
   // is, the plugin is waiting on the user, not hung: this holds the hook timer the
   // way one of its own dialogs does, so a card's PIN during onRenderEmailBody doesn't
-  // count towards auto-disable.
-  const release = holdForUser(plugin.id);
+  // count towards auto-disable. Not for a list: it never touches a card, and a plugin
+  // shouldn't be able to stop the timer with it. The hold has an absolute deadline
+  // (plugin-hooks.ts), and the shell gives up on a stuck call before that.
+  const release = method === 'keystore.list' ? () => {} : holdForUser(plugin.id);
   try {
     return await run();
   } catch (error) {

lib/plugin-sandbox/runtime.tsx

@@ -508,14 +508,15 @@ function buildCoreApi(manifest: PluginManifest) {
     },
     /**
      * The OS certificate store and smart cards, in the desktop app. Privileged tier
-     * with the `keystore` permission, when api.host.features.keystore is true. No
-     * timeout: the OS may be showing a PIN prompt, and the host always answers.
-     * Errors are bare codes: keystore:cancelled, not-found, unsupported, locked,
-     * invalid, failed.
+     * with the `keystore` permission, when api.host.features.keystore is true. The
+     * shell asks the user first, so any call can answer keystore:cancelled. Key
+     * operations have no timeout here: the OS may be showing a PIN prompt, and the shell
+     * gives up by itself. Errors are bare codes: keystore:cancelled, not-found,
+     * unsupported, locked, invalid, failed.
      */
     keystore: {
-      /** Certificates fit for mail that have a private key. Never prompts. */
-      list: () => callApi('keystore.list', [], 0),
+      /** Certificates fit for mail that have a private key. The OS doesn't prompt, and it has the usual timeout. */
+      list: () => callApi('keystore.list', []),
       /** Signs a digest. scheme: { scheme: 'rsa-pkcs1' | 'rsa-pss' | 'ecdsa', hash }. RSA gives the raw signature, ECDSA DER. */
       sign: (id: string, digest: Uint8Array, scheme: unknown) => callApi('keystore.sign', [id, digest, scheme], 0) as Promise<Uint8Array>,
       /** Unwraps a content key. scheme: { scheme: 'rsa-pkcs1' } or { scheme: 'rsa-oaep', hash, label? }. */