From d73d2cf3a24383a4aaafcf6d4077706b833df418 Mon Sep 17 00:00:00 2001 From: thorved <54140516+thorved@users.noreply.github.com> Date: Mon, 6 Oct 2025 11:47:12 +0530 Subject: [PATCH 1/2] Fix OIDC credential persistence issue The issue was that OIDC users were getting a new random Data Encryption Key (DEK) on every login, which made previously encrypted credentials inaccessible. Changes: - Modified setupOIDCUserEncryption() to persist the DEK encrypted with a system-derived key - Updated authenticateOIDCUser() to properly retrieve and use the persisted DEK - Ensured OIDC users now have the same encryption persistence as password-based users This fix ensures that credentials created by OIDC users remain accessible across multiple login sessions. --- src/backend/utils/user-crypto.ts | 46 +++++++++++++++++++++++++------- 1 file changed, 37 insertions(+), 9 deletions(-) diff --git a/src/backend/utils/user-crypto.ts b/src/backend/utils/user-crypto.ts index 4164ece4..b9010456 100644 --- a/src/backend/utils/user-crypto.ts +++ b/src/backend/utils/user-crypto.ts @@ -70,7 +70,34 @@ class UserCrypto { } async setupOIDCUserEncryption(userId: string): Promise { - const DEK = crypto.randomBytes(UserCrypto.DEK_LENGTH); + // Check if DEK already exists for this OIDC user + const existingKEKSalt = await this.getKEKSalt(userId); + const existingEncryptedDEK = await this.getEncryptedDEK(userId); + + let DEK: Buffer; + + if (existingKEKSalt && existingEncryptedDEK) { + // User already has a persisted DEK, retrieve it + const systemKey = this.deriveOIDCSystemKey(userId); + DEK = this.decryptDEK(existingEncryptedDEK, systemKey); + systemKey.fill(0); + } else { + // First time setup - create and persist new DEK + DEK = crypto.randomBytes(UserCrypto.DEK_LENGTH); + + // Generate a KEK salt for OIDC user (using a deterministic approach) + const kekSalt = await this.generateKEKSalt(); + await this.storeKEKSalt(userId, kekSalt); + + // Derive system key for OIDC user + const systemKey = this.deriveOIDCSystemKey(userId); + + // Encrypt and store the DEK + const encryptedDEK = this.encryptDEK(DEK, systemKey); + await this.storeEncryptedDEK(userId, encryptedDEK); + + systemKey.fill(0); + } const now = Date.now(); this.userSessions.set(userId, { @@ -135,34 +162,34 @@ class UserCrypto { async authenticateOIDCUser(userId: string): Promise { try { const kekSalt = await this.getKEKSalt(userId); - if (!kekSalt) { - await this.setupOIDCUserEncryption(userId); - return true; - } - - const systemKey = this.deriveOIDCSystemKey(userId); const encryptedDEK = await this.getEncryptedDEK(userId); - if (!encryptedDEK) { - systemKey.fill(0); + + if (!kekSalt || !encryptedDEK) { + // First time login or missing encryption data - set up encryption await this.setupOIDCUserEncryption(userId); return true; } + // Retrieve persisted DEK + const systemKey = this.deriveOIDCSystemKey(userId); const DEK = this.decryptDEK(encryptedDEK, systemKey); systemKey.fill(0); if (!DEK || DEK.length === 0) { + // DEK decryption failed - recreate encryption await this.setupOIDCUserEncryption(userId); return true; } const now = Date.now(); + // Clear any existing session const oldSession = this.userSessions.get(userId); if (oldSession) { oldSession.dataKey.fill(0); } + // Create new session with the persisted DEK this.userSessions.set(userId, { dataKey: Buffer.from(DEK), lastActivity: now, @@ -173,6 +200,7 @@ class UserCrypto { return true; } catch (error) { + // On error, set up fresh encryption await this.setupOIDCUserEncryption(userId); return true; } -- 2.49.1 From 9906b940b7fd3804ab0698cbae2ed316ca809bf7 Mon Sep 17 00:00:00 2001 From: thorved <54140516+thorved@users.noreply.github.com> Date: Mon, 6 Oct 2025 12:24:44 +0530 Subject: [PATCH 2/2] Fix race condition and remove redundant kekSalt for OIDC users Critical fixes: 1. Race Condition Mitigation: - Added read-after-write verification in setupOIDCUserEncryption() - Ensures session uses the DEK that's actually in the database - Prevents data loss when concurrent logins occur for new OIDC users - If race is detected, discards generated DEK and uses stored one 2. Remove Redundant kekSalt Logic: - Removed unnecessary kekSalt generation and checks for OIDC users - kekSalt is not used in OIDC key derivation (uses userId as salt) - Reduces database operations from 4 to 2 per authentication - Simplifies code and removes potential confusion 3. Improved Error Handling: - systemKey cleanup moved to finally block - Ensures sensitive key material is always cleared from memory These changes ensure data consistency and prevent potential data loss in high-concurrency scenarios. --- src/backend/utils/user-crypto.ts | 38 +++++++++++++++++++------------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/src/backend/utils/user-crypto.ts b/src/backend/utils/user-crypto.ts index b9010456..5b7394a9 100644 --- a/src/backend/utils/user-crypto.ts +++ b/src/backend/utils/user-crypto.ts @@ -70,13 +70,11 @@ class UserCrypto { } async setupOIDCUserEncryption(userId: string): Promise { - // Check if DEK already exists for this OIDC user - const existingKEKSalt = await this.getKEKSalt(userId); const existingEncryptedDEK = await this.getEncryptedDEK(userId); let DEK: Buffer; - if (existingKEKSalt && existingEncryptedDEK) { + if (existingEncryptedDEK) { // User already has a persisted DEK, retrieve it const systemKey = this.deriveOIDCSystemKey(userId); DEK = this.decryptDEK(existingEncryptedDEK, systemKey); @@ -84,19 +82,30 @@ class UserCrypto { } else { // First time setup - create and persist new DEK DEK = crypto.randomBytes(UserCrypto.DEK_LENGTH); - - // Generate a KEK salt for OIDC user (using a deterministic approach) - const kekSalt = await this.generateKEKSalt(); - await this.storeKEKSalt(userId, kekSalt); - - // Derive system key for OIDC user const systemKey = this.deriveOIDCSystemKey(userId); - // Encrypt and store the DEK - const encryptedDEK = this.encryptDEK(DEK, systemKey); - await this.storeEncryptedDEK(userId, encryptedDEK); + try { + const encryptedDEK = this.encryptDEK(DEK, systemKey); + await this.storeEncryptedDEK(userId, encryptedDEK); - systemKey.fill(0); + // MITIGATION: Read back the stored DEK to ensure we use the one that won the race. + const storedEncryptedDEK = await this.getEncryptedDEK(userId); + if ( + storedEncryptedDEK && + storedEncryptedDEK.data !== encryptedDEK.data + ) { + // We lost the race. Use the DEK from the database. + DEK.fill(0); // Discard our generated DEK. + DEK = this.decryptDEK(storedEncryptedDEK, systemKey); + } else if (!storedEncryptedDEK) { + // This is an unexpected state; the store operation should have worked. + throw new Error( + "Failed to store and retrieve user encryption key.", + ); + } + } finally { + systemKey.fill(0); + } } const now = Date.now(); @@ -161,10 +170,9 @@ class UserCrypto { async authenticateOIDCUser(userId: string): Promise { try { - const kekSalt = await this.getKEKSalt(userId); const encryptedDEK = await this.getEncryptedDEK(userId); - if (!kekSalt || !encryptedDEK) { + if (!encryptedDEK) { // First time login or missing encryption data - set up encryption await this.setupOIDCUserEncryption(userId); return true; -- 2.49.1