-
Notifications
You must be signed in to change notification settings - Fork 0
Harden Encryption Key Management and Storage #356
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| import { describe, it, expect, beforeEach, afterEach } from 'vitest'; | ||
| import { getOrCreateKey, encrypt, decrypt, hasKey, deleteKey } from '../crypto'; | ||
|
|
||
| describe('Crypto Utilities', () => { | ||
| const TEST_KEY_ID = 'test-encryption-key'; | ||
|
|
||
| beforeEach(async () => { | ||
| await deleteKey(TEST_KEY_ID); | ||
| }); | ||
|
|
||
| afterEach(async () => { | ||
| await deleteKey(TEST_KEY_ID); | ||
| }); | ||
|
|
||
| it('should create and store a non-extractable key', async () => { | ||
| const key = await getOrCreateKey(TEST_KEY_ID); | ||
| expect(key.extractable).toBe(false); | ||
| expect(key.type).toBe('secret'); | ||
| expect(key.algorithm.name).toBe('AES-GCM'); | ||
|
|
||
| const exists = await hasKey(TEST_KEY_ID); | ||
| expect(exists).toBe(true); | ||
|
|
||
| const sameKey = await getOrCreateKey(TEST_KEY_ID); | ||
| // When retrieved from IndexedDB, it might not be the exact same object reference | ||
| // but should have the same properties. | ||
| expect(sameKey.extractable).toBe(false); | ||
| expect(sameKey.algorithm.name).toBe('AES-GCM'); | ||
| }); | ||
|
|
||
| it('should encrypt and decrypt values', async () => { | ||
| const key = await getOrCreateKey(TEST_KEY_ID); | ||
| const plaintext = 'secret message 123'; | ||
|
|
||
| const encrypted = await encrypt(plaintext, key); | ||
| expect(encrypted).not.toBe(plaintext); | ||
| expect(typeof encrypted).toBe('string'); | ||
|
|
||
| const decrypted = await decrypt(encrypted, key); | ||
| expect(decrypted).toBe(plaintext); | ||
| }); | ||
|
|
||
| it('should throw error when decrypting with wrong key', async () => { | ||
| const key1 = await getOrCreateKey(TEST_KEY_ID); | ||
| const plaintext = 'secret'; | ||
| const encrypted = await encrypt(plaintext, key1); | ||
|
|
||
| await deleteKey(TEST_KEY_ID); | ||
| const key2 = await getOrCreateKey(TEST_KEY_ID); // Different key | ||
|
|
||
| await expect(decrypt(encrypted, key2)).rejects.toThrow(); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,103 @@ | ||
| import { describe, it, expect, beforeEach } from 'vitest'; | ||
| import { keyStore } from '../key-store'; | ||
| import { deleteKey, hasKey, getOrCreateKey } from '../crypto'; | ||
|
|
||
| describe('KeyStore Secure Migration', () => { | ||
| const CRYPTO_KEY_ID = 'dks:key-store:encryption-key'; | ||
|
|
||
| beforeEach(async () => { | ||
| await deleteKey(CRYPTO_KEY_ID); | ||
| // Clear the legacy key from IndexedDB if it exists (mocking legacy state) | ||
| const request = indexedDB.open('dks:key-store', 1); | ||
| await new Promise<void>((resolve, reject) => { | ||
| request.onsuccess = (e: Event) => { | ||
| const db = (e.target as IDBOpenDBRequest).result; | ||
| if (db.objectStoreNames.contains('keys')) { | ||
| const tx = db.transaction('keys', 'readwrite'); | ||
| tx.objectStore('keys').delete('__encryption_key__'); | ||
| tx.oncomplete = () => resolve(); | ||
| } else { | ||
| resolve(); | ||
| } | ||
| }; | ||
| request.onerror = reject; | ||
| request.onupgradeneeded = (e: IDBVersionChangeEvent) => { | ||
| const db = (e.target as IDBOpenDBRequest).result; | ||
| if (!db.objectStoreNames.contains('keys')) { | ||
| db.createObjectStore('keys', { keyPath: 'id' }); | ||
| } | ||
| }; | ||
| }); | ||
| }); | ||
|
|
||
| it('should encrypt new values using non-extractable keys', async () => { | ||
| await keyStore.set('test-key', 'test-value'); | ||
| const value = await keyStore.get('test-key'); | ||
| expect(value).toBe('test-value'); | ||
|
|
||
| const key = await getOrCreateKey(CRYPTO_KEY_ID); | ||
| expect(key.extractable).toBe(false); | ||
| }); | ||
|
|
||
| it('should migrate legacy JWK key to secure crypto-store', async () => { | ||
| // 1. Manually put a legacy JWK into the old store | ||
| const legacyKey = await crypto.subtle.generateKey( | ||
| { name: 'AES-GCM', length: 256 }, | ||
| true, | ||
| ['encrypt', 'decrypt'] | ||
| ); | ||
| const jwk = await crypto.subtle.exportKey('jwk', legacyKey); | ||
|
|
||
| const openReq = indexedDB.open('dks:key-store', 1); | ||
| await new Promise<void>((resolve) => { | ||
| openReq.onsuccess = (e: Event) => { | ||
| const db = (e.target as IDBOpenDBRequest).result; | ||
| const tx = db.transaction('keys', 'readwrite'); | ||
| tx.objectStore('keys').put({ id: '__encryption_key__', value: JSON.stringify(jwk) }); | ||
| tx.oncomplete = () => resolve(); | ||
| }; | ||
| }); | ||
|
|
||
| // 2. Encrypt something with the legacy key (mocking legacy encrypted data) | ||
| // We need to use the exact same format: enc:v1:<iv+ciphertext> | ||
| const iv = crypto.getRandomValues(new Uint8Array(12)); | ||
| const encoded = new TextEncoder().encode('legacy-data'); | ||
| const encrypted = await crypto.subtle.encrypt({ name: 'AES-GCM', iv }, legacyKey, encoded); | ||
| const combined = new Uint8Array(iv.length + encrypted.byteLength); | ||
| combined.set(iv, 0); | ||
| combined.set(new Uint8Array(encrypted), iv.length); | ||
| const legacyEncryptedValue = `enc:v1:${btoa(String.fromCharCode(...combined))}`; | ||
|
|
||
| await new Promise<void>((resolve) => { | ||
| const openReq2 = indexedDB.open('dks:key-store', 1); | ||
| openReq2.onsuccess = (e: Event) => { | ||
| const db = (e.target as IDBOpenDBRequest).result; | ||
| const tx = db.transaction('keys', 'readwrite'); | ||
| tx.objectStore('keys').put({ id: 'legacy-item', value: legacyEncryptedValue }); | ||
| tx.oncomplete = () => resolve(); | ||
| }; | ||
| }); | ||
|
|
||
| // 3. Access the item via keyStore — it should trigger migration and still be able to decrypt | ||
| const value = await keyStore.get('legacy-item'); | ||
| expect(value).toBe('legacy-data'); | ||
|
|
||
| // 4. Verify migration happened | ||
| const hasSecureKey = await hasKey(CRYPTO_KEY_ID); | ||
| expect(hasSecureKey).toBe(true); | ||
| const secureKey = await getOrCreateKey(CRYPTO_KEY_ID); | ||
| expect(secureKey.extractable).toBe(false); | ||
|
|
||
| // 5. Verify legacy key is gone | ||
| const openReq3 = indexedDB.open('dks:key-store', 1); | ||
| const legacyKeyStored = await new Promise<unknown>((resolve) => { | ||
| openReq3.onsuccess = (e: Event) => { | ||
| const db = (e.target as IDBOpenDBRequest).result; | ||
| const tx = db.transaction('keys', 'readonly'); | ||
| const req = tx.objectStore('keys').get('__encryption_key__'); | ||
| req.onsuccess = () => resolve(req.result); | ||
| }; | ||
| }); | ||
| expect(legacyKeyStored).toBeUndefined(); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,147 @@ | ||
| const DB_NAME = 'dks:crypto-store'; | ||
| const DB_VERSION = 1; | ||
| const STORE_NAME = 'keys'; | ||
|
|
||
| /** | ||
| * Open the IndexedDB database for secure key storage. | ||
| * IndexedDB supports storing CryptoKey objects directly via structured clone. | ||
| */ | ||
| const openDB = (): Promise<IDBDatabase> => | ||
| new Promise((resolve, reject) => { | ||
| const request = indexedDB.open(DB_NAME, DB_VERSION); | ||
| request.onerror = () => reject(new Error(String(request.error))); | ||
| request.onsuccess = () => resolve(request.result); | ||
| request.onupgradeneeded = () => { | ||
| const db = request.result; | ||
| if (!db.objectStoreNames.contains(STORE_NAME)) { | ||
| db.createObjectStore(STORE_NAME, { keyPath: 'id' }); | ||
| } | ||
| }; | ||
| }); | ||
|
|
||
| /** | ||
| * Get or create a non-extractable AES-GCM encryption key. | ||
| * By setting extractable to false, the raw key bytes cannot be retrieved | ||
| * via crypto.subtle.exportKey, providing stronger protection against key theft. | ||
| */ | ||
| export async function getOrCreateKey(id: string, options: { extractable?: boolean } = {}): Promise<CryptoKey> { | ||
| const db = await openDB(); | ||
| const stored = await new Promise<{ id: string; key: CryptoKey } | undefined>((resolve, reject) => { | ||
| const tx = db.transaction(STORE_NAME, 'readonly'); | ||
| const store = tx.objectStore(STORE_NAME); | ||
| const request = store.get(id); | ||
| request.onsuccess = () => resolve(request.result as { id: string; key: CryptoKey } | undefined); | ||
| request.onerror = () => reject(new Error(String(request.error))); | ||
| }); | ||
|
|
||
| if (stored?.key) { | ||
| return stored.key; | ||
| } | ||
|
|
||
| const extractable = options.extractable ?? false; | ||
| const key = await crypto.subtle.generateKey( | ||
| { name: 'AES-GCM', length: 256 }, | ||
| extractable, | ||
| ['encrypt', 'decrypt'], | ||
| ); | ||
|
|
||
| await new Promise<void>((resolve, reject) => { | ||
| const tx = db.transaction(STORE_NAME, 'readwrite'); | ||
| const store = tx.objectStore(STORE_NAME); | ||
| store.put({ id, key }); | ||
| tx.oncomplete = () => resolve(); | ||
| tx.onerror = () => reject(new Error(String(tx.error))); | ||
| }); | ||
|
|
||
| return key; | ||
| } | ||
|
Comment on lines
+27
to
+57
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
|
||
| /** | ||
| * Import an existing key and store it as a non-extractable key. | ||
| */ | ||
| export async function importAndStoreKey(id: string, jwk: JsonWebKey, options: { extractable?: boolean } = {}): Promise<CryptoKey> { | ||
| const db = await openDB(); | ||
| const extractable = options.extractable ?? false; | ||
| const key = await crypto.subtle.importKey( | ||
| 'jwk', | ||
| jwk, | ||
| { name: 'AES-GCM', length: 256 }, | ||
| extractable, | ||
| ['encrypt', 'decrypt'], | ||
| ); | ||
|
|
||
| await new Promise<void>((resolve, reject) => { | ||
| const tx = db.transaction(STORE_NAME, 'readwrite'); | ||
| const store = tx.objectStore(STORE_NAME); | ||
| store.put({ id, key }); | ||
| tx.oncomplete = () => resolve(); | ||
| tx.onerror = () => reject(new Error(String(tx.error))); | ||
| }); | ||
|
|
||
| return key; | ||
| } | ||
|
Comment on lines
+62
to
+82
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
|
||
| /** | ||
| * Encrypt a plaintext string using AES-GCM and the provided key. | ||
| * Returns a base64 string containing the IV and ciphertext. | ||
| */ | ||
| export async function encrypt(plaintext: string, key: CryptoKey): Promise<string> { | ||
| const iv = crypto.getRandomValues(new Uint8Array(12)); | ||
| const encoded = new TextEncoder().encode(plaintext); | ||
| const encrypted = await crypto.subtle.encrypt( | ||
| { name: 'AES-GCM', iv }, | ||
| key, | ||
| encoded, | ||
| ); | ||
|
|
||
| const combined = new Uint8Array(iv.length + encrypted.byteLength); | ||
| combined.set(iv, 0); | ||
| combined.set(new Uint8Array(encrypted), iv.length); | ||
|
|
||
| return btoa(String.fromCharCode(...combined)); | ||
| } | ||
|
Comment on lines
+88
to
+102
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
|
||
| /** | ||
| * Decrypt an encrypted base64 string using AES-GCM and the provided key. | ||
| */ | ||
| export async function decrypt(encrypted: string, key: CryptoKey): Promise<string> { | ||
| const combined = Uint8Array.from(atob(encrypted), (c) => c.charCodeAt(0)); | ||
| const iv = combined.slice(0, 12); | ||
| const ciphertext = combined.slice(12); | ||
|
|
||
| const decrypted = await crypto.subtle.decrypt( | ||
| { name: 'AES-GCM', iv }, | ||
| key, | ||
| ciphertext, | ||
| ); | ||
|
|
||
| return new TextDecoder().decode(decrypted); | ||
| } | ||
|
Comment on lines
+107
to
+119
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
|
||
| /** | ||
| * Check if a key exists in the store. | ||
| */ | ||
| export async function hasKey(id: string): Promise<boolean> { | ||
| const db = await openDB(); | ||
| return new Promise((resolve, reject) => { | ||
| const tx = db.transaction(STORE_NAME, 'readonly'); | ||
| const store = tx.objectStore(STORE_NAME); | ||
| const request = store.count(id); | ||
| request.onsuccess = () => resolve(request.result > 0); | ||
| request.onerror = () => reject(new Error(String(request.error))); | ||
| }); | ||
| } | ||
|
Comment on lines
+124
to
+133
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
|
||
| /** | ||
| * Delete a key from the store. | ||
| */ | ||
| export async function deleteKey(id: string): Promise<void> { | ||
| const db = await openDB(); | ||
| return new Promise((resolve, reject) => { | ||
| const tx = db.transaction(STORE_NAME, 'readwrite'); | ||
| const store = tx.objectStore(STORE_NAME); | ||
| store.delete(id); | ||
| tx.oncomplete = () => resolve(); | ||
| tx.onerror = () => reject(new Error(String(tx.error))); | ||
| }); | ||
| } | ||
|
Comment on lines
+138
to
+147
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is considered a best practice to avoid 'polluting' the global scope with variables that are intended to be local to the script. Global variables created from a script can produce name collisions with global variables created from another script, which will usually lead to runtime errors or unexpected behavior. It is mostly useful for browser scripts.