fix: cache a plugin signature only when it verifies for the cached bundle #1066

lib/admin/__tests__/plugin-signature-cache.test.ts

@@ -0,0 +1,62 @@
+// @vitest-environment node
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
+import { createHash, createPublicKey, generateKeyPairSync, verify } from 'node:crypto';
+import { mkdtempSync, rmSync, writeFileSync } from 'node:fs';
+import { tmpdir } from 'node:os';
+import path from 'node:path';
+import { setStoreForTests } from '@/lib/shared-store';
+import { resetLocalStoreForTests } from '@/lib/shared-store/local';
+import { separateFakeStore } from './shared-store-fake';
+
+// Shared mode (#1066) keeps served bundles and their signatures in a
+// per-replica cache keyed by the bundle hash. A signature read while
+// another replica was halfway through an install belongs to other bytes;
+// it must not stick to the cached bundle, or this replica serves the bundle
+// with a signature that never verifies again.
+
+let dir: string;
+const pem = generateKeyPairSync('ed25519').privateKey.export({ type: 'pkcs8', format: 'pem' }) as string;
+const pub = createPublicKey(pem);
+const sha = (s: string) => createHash('sha256').update(s).digest('hex');
+const verifies = (code: string, sig: string | null) => !!sig && verify(null, Buffer.from(code), pub, Buffer.from(sig, 'base64'));
+
+beforeEach(async () => {
+  dir = mkdtempSync(path.join(tmpdir(), 'bw-sig-cache-'));
+  writeFileSync(path.join(dir, 'key.pem'), pem);
+  vi.stubEnv('ADMIN_CONFIG_DIR', path.join(dir, 'config'));
+  vi.stubEnv('PLUGIN_SIGNING_KEY_FILE', path.join(dir, 'key.pem'));
+  resetLocalStoreForTests();
+  setStoreForTests(separateFakeStore(path.join(dir, 'store')));
+  for (const name of ['bulwark.admin.pluginSigning', 'bulwark.admin.bundleCache', 'bulwark.admin.pluginRegistry']) {
+    delete (globalThis as Record<symbol, unknown>)[Symbol.for(name)];
+  }
+  vi.resetModules();
+});
+
+afterEach(() => {
+  setStoreForTests(null);
+  resetLocalStoreForTests();
+  vi.unstubAllEnvs();
+  rmSync(dir, { recursive: true, force: true });
+});
+
+describe('signatures in the bundle cache', () => {
+  it('keeps only a signature that verifies for the cached bundle', async () => {
+    const { getPluginBundle, getPluginSignature } = await import('../plugin-registry');
+    const { pluginDocs } = await import('../extension-store');
+    const { signBytes } = await import('../plugin-signing');
+    const [v1, v2] = ['v1()', 'v2()'];
+    await pluginDocs().put('demo.js', v1);
+    await pluginDocs().put('demo.sig', await signBytes(v1));
+    expect(await getPluginBundle('demo', sha(v1))).toBe(v1);
+
+    // Another replica is halfway through installing v2.
+    await pluginDocs().put('demo.sig', await signBytes(v2));
+    expect(verifies(v1, await getPluginSignature('demo', v1, sha(v1)))).toBe(false);
+
+    // Back to v1's signature: this replica serves it again.
+    await pluginDocs().put('demo.sig', await signBytes(v1));
+    expect(verifies(v1, await getPluginSignature('demo', v1, sha(v1)))).toBe(true);
+  });
+
+});

lib/admin/plugin-registry.ts

@@ -468,7 +468,9 @@ async function storeSignature(id: string, signature: string | null): Promise<voi
  * a later change to the stored bundle stops verifying from then on.
  *
  * `bundleHash` (the registry entry's) lets shared mode answer from this
- * replica's bundle cache.
+ * replica's bundle cache. Only a signature that verifies for the cached
+ * code goes into it: one read while another replica was halfway through an
+ * install belongs to other bytes.
  */
 export async function getPluginSignature(id: string, code: string, bundleHash?: string): Promise<string | null> {
   if (!SAFE_ID_RE.test(id)) return null;
@@ -487,7 +489,12 @@ export async function getPluginSignature(id: string, code: string, bundleHash?:
   } else {
     signature = await signBundle(id, code);
     try {
-      await storeSignature(id, signature);
+      // Only if still missing: a replica that stored one first wins, and
+      // its signature is what every replica serves.
+      if (signature) {
+        const written = await pluginDocs().put(`${id}.sig`, signature, { ifVersion: null });
+        if (!written.ok) signature = written.current?.data.toString('utf-8').trim() || null;
+      }
     } catch (error) {
       if (isStoreUnavailable(error)) throw error;
       // Read-only config, a full disk, a file we may not write: serve the
@@ -499,10 +506,19 @@ export async function getPluginSignature(id: string, code: string, bundleHash?:
       signature = null;
     }
   }
-  if (cached && cached.code === code) cached.signature = signature;
+  if (cached && cached.code === code && signature && (await signatureVerifies(code, signature))) cached.signature = signature;
   return signature;
 }
 
+async function signatureVerifies(code: string, signature: string): Promise<boolean> {
+  try {
+    const { verifyBytes } = await import('./plugin-signing');
+    return await verifyBytes(code, signature);
+  } catch {
+    return false;
+  }
+}
+
 /**
  * The stored bundle of a plugin. With `bundleHash` (the registry entry's),
  * shared mode serves it from this replica's cache once it was fetched and