Replaces Keytar with safeStorage API

Author: gingi

desktop/src/client/client.module.ts

@@ -24,6 +24,7 @@ import { LocalDataStore } from "./core/local-data-store";
 import { BatchExplorerProperties } from "./core/properties";
 import { ClientTelemetryModule } from "./core/telemetry";
 import { TerminalService } from "./core/terminal";
+import { SafeStorageMainService } from "./core/secure-data-store/safe-storage-main.service";
 import { MenuModule } from "./menu/menu.module";
 import { ProxySettingsManager } from "./proxy";
 
@@ -32,6 +33,7 @@ import { ProxySettingsManager } from "./proxy";
  */
 const servicesToInitialize = [
     TerminalService,
+    SafeStorageMainService,
 ];
 
 // make sure that the services are created on app start

desktop/src/client/core/batch-explorer-application.ts

@@ -24,6 +24,8 @@ import { AADService, AuthenticationWindow } from "./aad";
 import { BatchExplorerInitializer } from "./batch-explorer-initializer";
 import { MainWindowManager } from "./main-window-manager";
 import { StorageBlobAdapter } from "./storage";
+import { SafeStorageMainService } from "./secure-data-store/safe-storage-main.service";
+import { SecureDataStore } from "./secure-data-store/secure-data-store";
 import { filter, first, map } from "rxjs/operators";
 
 const osName = `${os.platform()}-${os.arch()}/${os.release()}`;
@@ -61,6 +63,7 @@ export class BatchExplorerApplication {
         private telemetryService: TelemetryService,
         private telemetryManager: TelemetryManager,
         private storageBlobAdapter: StorageBlobAdapter,
+        private safeStorageMainService: SafeStorageMainService,
         configurationStore: UserConfigurationService<BEUserConfiguration>
     ) {
         this.windows = new MainWindowManager(this, this.telemetryManager);
@@ -81,7 +84,10 @@ export class BatchExplorerApplication {
         this.proxySettings = this.injector.get(ProxySettingsManager);
 
         this.ipcMain.init();
+        this.safeStorageMainService.init();
         await this.aadService.init();
+
+        await this._checkForLegacyCredentials();
         this._registerProtocol();
         this._setupProcessEvents();
         this._registerFileProtocol();
@@ -365,4 +371,12 @@ export class BatchExplorerApplication {
             callback({ cancel: false, requestHeaders: details.requestHeaders });
         });
     }
-}
+
+    private async _checkForLegacyCredentials() {
+        const secureDataStore = this.injector.get(SecureDataStore);
+        if (await secureDataStore.legacyDataDetected()) {
+            log.warn("Logging out to clear legacy encrypted credentials.");
+            await this.aadService.logout();
+        }
+    }
+}

desktop/src/client/core/secure-data-store/crypto-service.spec.ts

@@ -1,37 +1,43 @@
-import { CryptoService } from "./crypto-service";
+import { CryptoService, UnsupportedEncryptionVersionError } from "./crypto-service";
 
 describe("CryptoService", () => {
     let service: CryptoService;
     let keytarSpy;
 
-    let masterKey: string | null = null;
+    let masterKeys: { [key: string]: string | null } = {};
 
     beforeEach(() => {
-        // Fake testing key needs to be 32 characters long
-        masterKey = "------fake-key-for-testing------";
+        // Current key is 64 chars (32 bytes hex)
+        masterKeys = {
+            "BatchExplorer:master-v2": "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"
+        };
+
         keytarSpy = {
-            setPassword: jasmine.createSpy("setPassword").and.callFake((x) => {
-                masterKey = x;
-                return Promise.resolve();
+            setPassword: jasmine.createSpy("setPassword").and.callFake(async (service, account, password) => {
+                const key = `${service}:${account}`;
+                masterKeys[key] = password;
+            }),
+            getPassword: jasmine.createSpy("getPassword").and.callFake(async (service, account) => {
+                const key = `${service}:${account}`;
+                return masterKeys[key] || null;
             }),
-            getPassword: jasmine.createSpy("getPassword").and.callFake(() => Promise.resolve(masterKey)),
         };
 
         service = new CryptoService(keytarSpy);
     });
 
-    it("sets a new master password if not is found", async () => {
-        masterKey = null;
+    it("sets a new master password if not found", async () => {
+        masterKeys["BatchExplorer:master-v2"] = null;
         service = new CryptoService(keytarSpy);
-        const key = await (service as any)._masterKey;
+        const key = await (service as any)._ensureMasterKey();
         expect(key).not.toBeFalsy();
-        expect(key!.length).toBe(32);
+        expect(key.length).toBe(64);
     });
 
     it("retrieve the master password", async () => {
-        const key = await (service as any)._masterKey;
+        const key = await (service as any)._ensureMasterKey();
         expect(key).not.toBeFalsy();
-        expect(key!.length).toBe(32);
+        expect(key.length).toBe(64);
     });
 
     it("encrypt and decrypt a string correctly", async () => {
@@ -41,4 +47,23 @@ describe("CryptoService", () => {
         const decrypted = await service.decrypt(encrypted);
         expect(decrypted).toEqual(message);
     });
+
+    it("rejects legacy encrypted data", async () => {
+        // Simulate legacy format: no version header, just [iv(16)] + [encrypted_data]
+        const crypto = require("crypto");
+        const legacyKeyHex = "0123456789abcdef0123456789abcdef"; // 32-char hex string
+
+        // Create legacy encrypted data that's long enough to pass length check
+        const message = "legacy message that needs to be long enough for the minimum length";
+        let iv: Buffer;
+        do {
+            iv = crypto.randomBytes(16);
+        } while (iv[0] <= 10);
+
+        const cipher = crypto.createCipheriv("aes-256-ctr", legacyKeyHex, iv);
+        const legacyEncrypted = Buffer.concat([iv, cipher.update(Buffer.from(message)), cipher.final()]);
+
+        await expectAsync(service.decrypt(legacyEncrypted.toString("base64")))
+            .toBeRejectedWithError(/Unsupported encryption version.*Please re-authenticate/);
+    });
 });

desktop/src/client/core/secure-data-store/crypto-service.ts

@@ -6,27 +6,57 @@ import { KeytarService } from "./keytar.service";
  * Keytar service and key to save the master key
  */
 const BATCH_APPLICATION = "BatchExplorer";
-const KEYTAR_KEY = "master";
+const KEYTAR_KEY = "master-v2";
 
 /**
  * Length of the initialization vector
  */
 const IV_BYTES = 16;
 
 /**
- * Algorithm to use when encrypting
+ * Current encryption algorithm: AES-256-GCM with 32-byte key
  */
-const ENCRYPT_ALGORITHM = "aes-256-ctr";
+const ALGORITHM = "aes-256-gcm";
+const ALGORITHM_VERSION = 2;
+
+/**
+ * GCM tag length for authenticated encryption
+ */
+const GCM_TAG_LENGTH = 16;
+
+/**
+ * Version header length (1 byte)
+ */
+const VERSION_HEADER_LENGTH = 1;
+
+/**
+ * Key size in bytes (32 bytes = 256 bits)
+ */
+const KEY_BYTES = 32;
 
 // What encoding to use when converting buffer to string
 const DEFAULT_STRING_ENCODING = "base64";
 
+export class UnsupportedEncryptionVersionError extends Error {
+    constructor(version: number) {
+        super(`Unsupported encryption version: ${version}. Please re-authenticate.`);
+        this.name = "UnsupportedEncryptionVersionError";
+    }
+}
+
 @Injectable({ providedIn: "root" })
 export class CryptoService {
-    private _masterKey: Promise<string>;
+    private _masterKey: Promise<string> | null = null;
 
     constructor(private keytar: KeytarService) {
-        this._masterKey = this._loadMasterKey();
+        // Don't load keys in constructor - defer until first use to avoid IPC initialization issues
+    }
+
+    private _ensureMasterKey(): Promise<string> {
+        if (!this._masterKey) {
+            this._masterKey = this._loadMasterKey();
+        }
+        return this._masterKey;
     }
 
     public async encrypt(content: Buffer): Promise<Buffer>;
@@ -52,33 +82,56 @@ export class CryptoService {
     }
 
     private async _encryptBuffer(content: Buffer): Promise<Buffer> {
-        const key = await this._masterKey;
+        const keyHex = await this._ensureMasterKey();
+        const key = Buffer.from(keyHex, "hex") as crypto.CipherKey;
         const iv = this._getIV();
-        const cipher = crypto.createCipheriv(ENCRYPT_ALGORITHM, key, iv);
-        return Buffer.concat([iv, cipher.update(content), cipher.final()]);
+
+        const cipher = crypto.createCipheriv(ALGORITHM as any, key, iv as crypto.BinaryLike);
+        const encrypted = cipher.update(content as crypto.BinaryLike);
+        cipher.final();
+        const tag = cipher.getAuthTag();
+
+        // Format: [version(1)] + [iv(16)] + [tag(16)] + [encrypted_data]
+        return Buffer.concat([
+            new Uint8Array([ALGORITHM_VERSION]),
+            iv,
+            tag,
+            encrypted
+        ]);
     }
 
     private async _decryptBuffer(content: Buffer): Promise<Buffer> {
-        const key = await this._masterKey;
-        const iv = content.slice(0, IV_BYTES);
-        const decipher = crypto.createDecipheriv(ENCRYPT_ALGORITHM, key, iv);
-        return Buffer.concat([decipher.update(content.slice(16)), decipher.final()]);
+        // Verify minimum length and version header
+        if (content.length < VERSION_HEADER_LENGTH + IV_BYTES + GCM_TAG_LENGTH) {
+            throw new Error("Invalid encrypted data: content too short");
+        }
+
+        const version = content[0];
+        if (version !== ALGORITHM_VERSION) {
+            throw new UnsupportedEncryptionVersionError(version);
+        }
+
+        const keyHex = await this._ensureMasterKey();
+        const key = Buffer.from(keyHex, "hex") as crypto.CipherKey;
+        const iv = content.slice(VERSION_HEADER_LENGTH, VERSION_HEADER_LENGTH + IV_BYTES);
+        const tag = content.slice(VERSION_HEADER_LENGTH + IV_BYTES, VERSION_HEADER_LENGTH + IV_BYTES + GCM_TAG_LENGTH);
+        const encrypted = content.slice(VERSION_HEADER_LENGTH + IV_BYTES + GCM_TAG_LENGTH);
+
+        const decipher = crypto.createDecipheriv(ALGORITHM as any, key, iv as crypto.BinaryLike);
+        decipher.setAuthTag(tag);
+        return Buffer.concat([decipher.update(encrypted), decipher.final()]);
     }
 
     private async _loadMasterKey(): Promise<string> {
         let masterKey = await this.keytar.getPassword(BATCH_APPLICATION, KEYTAR_KEY);
         if (!masterKey) {
-            masterKey = this._generateMasterKey();
+            masterKey = crypto.randomBytes(KEY_BYTES).toString("hex");
             await this.keytar.setPassword(BATCH_APPLICATION, KEYTAR_KEY, masterKey);
         }
         return masterKey;
     }
 
-    private _generateMasterKey() {
-        return crypto.randomBytes(16).toString("hex");
-    }
-
-    private _getIV() {
+    private _getIV(): Buffer {
         return crypto.randomBytes(IV_BYTES);
     }
 }

desktop/src/client/core/secure-data-store/index.ts

@@ -1,2 +1,4 @@
-export * from "./secure-data-store";
 export * from "./crypto-service";
+export * from "./keytar.service";
+export * from "./safe-storage-main.service";
+export * from "./secure-data-store";

desktop/src/client/core/secure-data-store/keytar.service.spec.ts

@@ -0,0 +1,87 @@
+import { KeytarService } from "./keytar.service";
+
+describe("KeytarService", () => {
+    let service: KeytarService;
+    let safeStorageSpy;
+
+    beforeEach(() => {
+        safeStorageSpy = {
+            setPassword: jasmine.createSpy("setPassword").and.returnValue(Promise.resolve()),
+            getPassword: jasmine.createSpy("getPassword").and.returnValue(Promise.resolve(null))
+        };
+
+        service = new KeytarService(safeStorageSpy);
+    });
+
+    describe("setPassword", () => {
+        it("should call safeStorage.setPassword with correct arguments", async () => {
+            await service.setPassword("testService", "testAccount", "testPassword");
+
+            expect(safeStorageSpy.setPassword).toHaveBeenCalledWith(
+                "testService:testAccount",
+                "testPassword"
+            );
+        });
+
+        it("should not throw when safeStorage.setPassword succeeds", async () => {
+            await expectAsync(service.setPassword("testService", "testAccount", "testPassword"))
+                .toBeResolved();
+        });
+
+        it("should catch and log errors when safeStorage.setPassword fails", async () => {
+            const consoleWarnSpy = spyOn(console, "warn");
+            const error = new Error("Encryption not available");
+            safeStorageSpy.setPassword.and.returnValue(Promise.reject(error));
+
+            await service.setPassword("testService", "testAccount", "testPassword");
+
+            expect(consoleWarnSpy).toHaveBeenCalledWith(
+                "Failed to store credentials securely for testService:testAccount:",
+                "Encryption not available"
+            );
+        });
+
+        it("should not throw when safeStorage.setPassword fails", async () => {
+            safeStorageSpy.setPassword.and.returnValue(
+                Promise.reject(new Error("Encryption not available"))
+            );
+
+            await expectAsync(service.setPassword("testService", "testAccount", "testPassword"))
+                .toBeResolved();
+        });
+    });
+
+    describe("getPassword", () => {
+        it("should call safeStorage.getPassword with correct arguments", async () => {
+            await service.getPassword("testService", "testAccount");
+
+            expect(safeStorageSpy.getPassword).toHaveBeenCalledWith(
+                "testService:testAccount"
+            );
+        });
+
+        it("should return the password from safeStorage.getPassword", async () => {
+            safeStorageSpy.getPassword.and.returnValue(Promise.resolve("retrievedPassword"));
+
+            const result = await service.getPassword("testService", "testAccount");
+
+            expect(result).toBe("retrievedPassword");
+        });
+
+        it("should return null when password is not found", async () => {
+            safeStorageSpy.getPassword.and.returnValue(Promise.resolve(null));
+
+            const result = await service.getPassword("testService", "testAccount");
+
+            expect(result).toBeNull();
+        });
+
+        it("should propagate errors from safeStorage.getPassword", async () => {
+            const error = new Error("Storage error");
+            safeStorageSpy.getPassword.and.returnValue(Promise.reject(error));
+
+            await expectAsync(service.getPassword("testService", "testAccount"))
+                .toBeRejectedWith(error);
+        });
+    });
+});