security: random persisted fallback key for EncryptedStore (not the userData path)

src/main/utils/encrypted-store-crypto.ts

@@ -0,0 +1,62 @@
+// Pure crypto helpers for EncryptedStore's fallback (no OS keychain) path.
+// Extracted so the key handling is unit-testable without Electron.
+//
+// Security: the fallback key must NOT be derivable from a public value. The old
+// code derived it from the (predictable) userData path, so anyone who could read
+// the store file could recompute the key. getOrCreateRandomKey persists a random
+// 32-byte key with owner-only permissions instead; deriveKeyFromPath is kept
+// only to read data written by the old scheme.
+import crypto from 'crypto'
+import fs from 'fs'
+
+const ALGORITHM = 'aes-256-gcm'
+const IV_LENGTH = 16
+const AUTH_TAG_LENGTH = 16
+const KEY_LENGTH = 32
+const LEGACY_APP_SALT = 'mingly-secure-store-v1'
+
+export interface GcmEntry {
+  iv: string
+  data: string
+  tag: string
+}
+
+export function gcmEncrypt(plaintext: string, key: Buffer): GcmEntry {
+  const iv = crypto.randomBytes(IV_LENGTH)
+  const cipher = crypto.createCipheriv(ALGORITHM, key, iv, { authTagLength: AUTH_TAG_LENGTH })
+  const encrypted = Buffer.concat([cipher.update(plaintext, 'utf-8'), cipher.final()])
+  return {
+    iv: iv.toString('base64'),
+    data: encrypted.toString('base64'),
+    tag: cipher.getAuthTag().toString('base64'),
+  }
+}
+
+export function gcmDecrypt(entry: GcmEntry, key: Buffer): string {
+  const decipher = crypto.createDecipheriv(ALGORITHM, key, Buffer.from(entry.iv, 'base64'), { authTagLength: AUTH_TAG_LENGTH })
+  decipher.setAuthTag(Buffer.from(entry.tag, 'base64'))
+  const decrypted = Buffer.concat([decipher.update(Buffer.from(entry.data, 'base64')), decipher.final()])
+  return decrypted.toString('utf-8')
+}
+
+/** Legacy key derived from the userData path — kept ONLY to read old entries. */
+export function deriveKeyFromPath(userDataPath: string): Buffer {
+  return crypto.pbkdf2Sync(userDataPath, LEGACY_APP_SALT, 100_000, KEY_LENGTH, 'sha512')
+}
+
+/**
+ * Load the persisted random fallback key, or generate + persist one with
+ * owner-only (0600) permissions on first use.
+ */
+export function getOrCreateRandomKey(keyPath: string): Buffer {
+  if (fs.existsSync(keyPath)) {
+    const key = fs.readFileSync(keyPath)
+    if (key.length === KEY_LENGTH) return key
+    // Corrupt/short key file — regenerate below.
+  }
+  const key = crypto.randomBytes(KEY_LENGTH)
+  fs.writeFileSync(keyPath, key, { mode: 0o600 })
+  // Defensively tighten perms in case the file already existed with a wider mode.
+  try { fs.chmodSync(keyPath, 0o600) } catch { /* best effort */ }
+  return key
+}

src/main/utils/encrypted-store.ts

@@ -11,21 +11,16 @@
  */
 
 import { app, safeStorage } from 'electron'
-import crypto from 'crypto'
 import fs from 'fs'
 import path from 'path'
-
-// Legacy constants (for migration only)
-const LEGACY_ALGORITHM = 'aes-256-gcm'
-const LEGACY_IV_LENGTH = 16
-const LEGACY_AUTH_TAG_LENGTH = 16
-const LEGACY_KEY_LENGTH = 32
-const LEGACY_APP_SALT = 'mingly-secure-store-v1'
+import { gcmEncrypt, gcmDecrypt, deriveKeyFromPath, getOrCreateRandomKey } from './encrypted-store-crypto'
 
 interface LegacyEncryptedEntry {
   iv: string
   data: string
   tag: string
+  /** 'random' = new per-install fallback key; absent = legacy path-derived key. */
+  keyId?: 'random'
 }
 
 interface SafeStorageEntry {
@@ -46,15 +41,19 @@ export class EncryptedStore {
   private entries: Record<string, StoreEntry> = {}
   private useSafeStorage: boolean
   private legacyKey: Buffer | null = null
+  private fallbackKey: Buffer | null = null
 
   constructor(filename: string = 'secure-keys.enc.json') {
     const userDataPath = app.getPath('userData')
     this.storePath = path.join(userDataPath, filename)
     this.useSafeStorage = safeStorage.isEncryptionAvailable()
 
     if (!this.useSafeStorage) {
-      // Fallback: derive legacy key for environments without safeStorage
+      // Fallback (no OS keychain): use a persisted RANDOM key, not one derived
+      // from the predictable userData path. The path-derived key is kept only
+      // to read entries written by the old scheme.
       this.legacyKey = this.deriveLegacyKey(userDataPath)
+      this.fallbackKey = getOrCreateRandomKey(this.storePath + '.key')
       console.warn('[EncryptedStore] safeStorage unavailable — using AES-256-GCM fallback')
     }
 
@@ -64,13 +63,23 @@ export class EncryptedStore {
 
   /** Derive legacy encryption key (for migration and fallback) */
   private deriveLegacyKey(userDataPath: string): Buffer {
-    return crypto.pbkdf2Sync(
-      userDataPath,
-      LEGACY_APP_SALT,
-      100_000,
-      LEGACY_KEY_LENGTH,
-      'sha512'
-    )
+    return deriveKeyFromPath(userDataPath)
+  }
+
+  private ensureFallbackKey(): Buffer {
+    if (!this.fallbackKey) this.fallbackKey = getOrCreateRandomKey(this.storePath + '.key')
+    return this.fallbackKey
+  }
+
+  private ensurePathKey(): Buffer {
+    if (!this.legacyKey) this.legacyKey = deriveKeyFromPath(app.getPath('userData'))
+    return this.legacyKey
+  }
+
+  /** Pick the decryption key: random fallback key for current-scheme entries,
+   *  else the legacy path-derived key for old data. */
+  private keyFor(entry: LegacyEncryptedEntry): Buffer {
+    return entry.keyId === 'random' ? this.ensureFallbackKey() : this.ensurePathKey()
   }
 
   private loadFromDisk(): void {
@@ -103,17 +112,13 @@ export class EncryptedStore {
   private migrateToSafeStorage(): void {
     if (!this.useSafeStorage) return
 
-    // Need legacy key to decrypt old entries
-    const userDataPath = app.getPath('userData')
-    const legacyKey = this.deriveLegacyKey(userDataPath)
-
     let migrated = 0
     for (const [key, entry] of Object.entries(this.entries)) {
       if (isSafeStorageEntry(entry)) continue // Already migrated
 
       // Decrypt with legacy AES-256-GCM
       try {
-        const plaintext = this.decryptLegacy(entry as LegacyEncryptedEntry, legacyKey)
+        const plaintext = this.decryptLegacy(entry as LegacyEncryptedEntry)
         // Re-encrypt with safeStorage
         const encrypted = safeStorage.encryptString(plaintext)
         this.entries[key] = { encrypted: encrypted.toString('base64'), v: 2 }
@@ -129,44 +134,14 @@ export class EncryptedStore {
     }
   }
 
-  /** Decrypt a legacy AES-256-GCM entry */
-  private decryptLegacy(entry: LegacyEncryptedEntry, encryptionKey: Buffer): string {
-    const iv = Buffer.from(entry.iv, 'base64')
-    const data = Buffer.from(entry.data, 'base64')
-    const tag = Buffer.from(entry.tag, 'base64')
-
-    const decipher = crypto.createDecipheriv(LEGACY_ALGORITHM, encryptionKey, iv, {
-      authTagLength: LEGACY_AUTH_TAG_LENGTH
-    })
-    decipher.setAuthTag(tag)
-
-    const decrypted = Buffer.concat([
-      decipher.update(data),
-      decipher.final()
-    ])
-
-    return decrypted.toString('utf-8')
+  /** Decrypt an AES-256-GCM entry, picking the right key by its keyId. */
+  private decryptLegacy(entry: LegacyEncryptedEntry): string {
+    return gcmDecrypt(entry, this.keyFor(entry))
   }
 
-  /** Encrypt with legacy AES-256-GCM (fallback only) */
+  /** Encrypt with the random fallback key (used only when safeStorage is off). */
   private encryptLegacy(plaintext: string): LegacyEncryptedEntry {
-    if (!this.legacyKey) throw new Error('Legacy key not available')
-
-    const iv = crypto.randomBytes(LEGACY_IV_LENGTH)
-    const cipher = crypto.createCipheriv(LEGACY_ALGORITHM, this.legacyKey, iv, {
-      authTagLength: LEGACY_AUTH_TAG_LENGTH
-    })
-
-    const encrypted = Buffer.concat([
-      cipher.update(plaintext, 'utf-8'),
-      cipher.final()
-    ])
-
-    return {
-      iv: iv.toString('base64'),
-      data: encrypted.toString('base64'),
-      tag: cipher.getAuthTag().toString('base64')
-    }
+    return { ...gcmEncrypt(plaintext, this.ensureFallbackKey()), keyId: 'random' }
   }
 
   get(key: string): string | undefined {
@@ -179,15 +154,8 @@ export class EncryptedStore {
         return safeStorage.decryptString(buffer)
       }
 
-      // Legacy fallback
-      if (this.legacyKey) {
-        return this.decryptLegacy(entry as LegacyEncryptedEntry, this.legacyKey)
-      }
-
-      // safeStorage available but entry is legacy — should have been migrated
-      const userDataPath = app.getPath('userData')
-      const legacyKey = this.deriveLegacyKey(userDataPath)
-      return this.decryptLegacy(entry as LegacyEncryptedEntry, legacyKey)
+      // Legacy / fallback entry — keyFor picks the right key by keyId.
+      return this.decryptLegacy(entry as LegacyEncryptedEntry)
     } catch {
       // Decrypt failed — likely caused by adhoc re-signing after rebuild.
       // Remove the corrupted entry so the user can re-enter the key cleanly.

tests/unit/encrypted-store-crypto.test.ts

@@ -0,0 +1,39 @@
+import { describe, it, expect, afterEach } from 'vitest'
+import { mkdtempSync, rmSync, statSync, existsSync } from 'fs'
+import { tmpdir } from 'os'
+import { join } from 'path'
+import { gcmEncrypt, gcmDecrypt, deriveKeyFromPath, getOrCreateRandomKey } from '../../src/main/utils/encrypted-store-crypto'
+
+const tmps: string[] = []
+const mkTmp = () => { const d = mkdtempSync(join(tmpdir(), 'es-')); tmps.push(d); return d }
+afterEach(() => { for (const d of tmps.splice(0)) rmSync(d, { recursive: true, force: true }) })
+
+describe('encrypted-store crypto', () => {
+  it('round-trips with a random key', () => {
+    const key = getOrCreateRandomKey(join(mkTmp(), 'k'))
+    const entry = gcmEncrypt('s3cret', key)
+    expect(gcmDecrypt(entry, key)).toBe('s3cret')
+  })
+  it('a wrong key fails the auth tag (no silent mis-decrypt)', () => {
+    const e = gcmEncrypt('x', getOrCreateRandomKey(join(mkTmp(), 'a')))
+    expect(() => gcmDecrypt(e, getOrCreateRandomKey(join(mkTmp(), 'b')))).toThrow()
+  })
+  it('getOrCreateRandomKey persists and returns the SAME key, not derivable from a path', () => {
+    const p = join(mkTmp(), 'k')
+    const k1 = getOrCreateRandomKey(p)
+    const k2 = getOrCreateRandomKey(p)
+    expect(k1.equals(k2)).toBe(true)
+    expect(k1.length).toBe(32)
+    expect(existsSync(p)).toBe(true)
+  })
+  it('writes the key file with owner-only (0600) permissions', () => {
+    const p = join(mkTmp(), 'k')
+    getOrCreateRandomKey(p)
+    expect(statSync(p).mode & 0o777).toBe(0o600)
+  })
+  it('legacy path-derived data still decrypts (backward compatibility)', () => {
+    const legacy = deriveKeyFromPath('/some/userData/path')
+    const entry = gcmEncrypt('old', legacy)
+    expect(gcmDecrypt(entry, deriveKeyFromPath('/some/userData/path'))).toBe('old')
+  })
+})