Skip to content

Commit

Permalink
Warn instead of fail when keychain is unreachable (#78)
Browse files Browse the repository at this point in the history
Warn the user the keychain is unreachable instead of raising an error.

- When the master password is set for the first time (the DB was absent)
or configured to be remembered (`dcli configure save-master-password
true`), this will disable the master password (encrypted using the local
key) save in the DB (that is then useless because the local key can only
be retrieved from the master password). A warning is displayed, so the
user can retry using the keychain.
- When the keychain suddenly starts to fail (`save-master-password` set
to `true` but `getLocalKey()` returns an error) or the master password
encrypted using the local key is absent from the DB, then the keychain
is not disabled, but a warning is displayed, so the user knows what is
happening and how to disable the keychain.

Because the keychain unavailability does not trigger errors anymore, it
has been possible to postpone its access. Thus, this PR resolves #55.

If you think of better warnings, don't hesitate to edit them!
  • Loading branch information
jboillot authored Mar 9, 2023
1 parent fea879e commit 8d9d5db
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 26 deletions.
54 changes: 33 additions & 21 deletions src/crypto/keychainManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,25 @@ import { perform2FAVerification } from '../middleware/perform2FAVerification';

const SERVICE = 'dashlane-cli';

export const setLocalKey = (login: string, shouldNotSaveMasterPassword: boolean, localKey?: Buffer): Buffer => {
export const generateLocalKey = (): Buffer => {
const localKey = crypto.randomBytes(32);
if (!localKey) {
localKey = crypto.randomBytes(32);
if (!localKey) {
throw new Error('Unable to generate AES local key');
}
throw new Error('Unable to generate AES local key');
}
return localKey;
};

if (!shouldNotSaveMasterPassword) {
export const setLocalKey = (login: string, localKey: Buffer, callbackOnError: (errorMessage: string) => void) => {
try {
const entry = new Entry(SERVICE, login);
entry.setPassword(localKey.toString('base64'));
} catch (error) {
let errorMessage = 'unknown error';
if (error instanceof Error) {
errorMessage = error.message;
}
callbackOnError(errorMessage);
}
return localKey;
};

const getLocalKey = (login: string): Buffer | undefined => {
Expand Down Expand Up @@ -77,19 +83,7 @@ const getSecretsWithoutDB = async (
login: string,
shouldNotSaveMasterPassword: boolean
): Promise<Secrets> => {
let localKey: Buffer;
try {
localKey = setLocalKey(login, shouldNotSaveMasterPassword);
} catch (error) {
let errorMessage = 'unknown error';
if (error instanceof Error) {
errorMessage = error.message;
}
winston.debug(`Unable to reach OS keychain: ${errorMessage}`);
throw new Error(
'Your OS keychain is probably unreachable. Install it or disable its usage via `dcli configure save-master-password false`'
);
}
const localKey = generateLocalKey();

// Register the user's device
const { deviceAccessKey, deviceSecretKey, serverKey } = await registerDevice({ login });
Expand All @@ -115,6 +109,13 @@ const getSecretsWithoutDB = async (
const masterPasswordEncrypted = encryptAES(localKey, Buffer.from(masterPassword));
const localKeyEncrypted = encryptAES(derivate, localKey);

if (!shouldNotSaveMasterPassword) {
setLocalKey(login, localKey, (errorMessage) => {
warnUnreachableKeychainDisabled(errorMessage);
shouldNotSaveMasterPassword = true;
});
}

db.prepare('REPLACE INTO device VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)')
.bind(
login,
Expand Down Expand Up @@ -163,7 +164,12 @@ const getSecretsWithoutKeychain = async (login: string, deviceConfiguration: Dev
await decrypt(deviceConfiguration.secretKeyEncrypted, { type: 'alreadyComputed', symmetricKey: localKey })
).toString('hex');

setLocalKey(login, deviceConfiguration.shouldNotSaveMasterPassword, localKey);
if (!deviceConfiguration.shouldNotSaveMasterPassword) {
setLocalKey(login, localKey, (errorMessage) => {
winston.warn(`Unable to reach OS keychain because of error: "${errorMessage}". \
Install it or disable its usage via \`dcli configure save-master-password false\`.`);
});
}

return {
login,
Expand Down Expand Up @@ -270,3 +276,9 @@ export const getSecrets = async (
secretKey,
};
};

export const warnUnreachableKeychainDisabled = (errorMessage: string) => {
winston.warn(`Unable to reach OS keychain because of error: "${errorMessage}", so its use has been disabled. \
To retry using it, please execute \`dcli configure save-master-password true\`. \
Until then, you will have to retype your master password each time you run the CLI.`);
};
16 changes: 11 additions & 5 deletions src/middleware/configure.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import Database from 'better-sqlite3';
import winston from 'winston';
import { encryptAES } from '../crypto/encrypt';
import { deleteLocalKey, setLocalKey } from '../crypto/keychainManager';
import { deleteLocalKey, setLocalKey, warnUnreachableKeychainDisabled } from '../crypto/keychainManager';
import { Secrets } from '../types';

interface ConfigureSaveMasterPassword {
Expand All @@ -17,7 +17,8 @@ interface ConfigureDisableAutoSync {
}

export const configureSaveMasterPassword = (params: ConfigureSaveMasterPassword) => {
const { db, secrets, shouldNotSaveMasterPassword } = params;
const { db, secrets } = params;
let shouldNotSaveMasterPassword = params.shouldNotSaveMasterPassword;

if (shouldNotSaveMasterPassword) {
// Forget the local key stored in the OS keychain because the master password and the DB are enough to retrieve the
Expand All @@ -30,7 +31,7 @@ export const configureSaveMasterPassword = (params: ConfigureSaveMasterPassword)
if (error instanceof Error) {
errorMessage = error.message;
}
winston.debug(`Unable to delete the local key from the keychain: ${errorMessage}`);
winston.warn(`Unable to delete the local key from the keychain: ${errorMessage}`);
}
}

Expand All @@ -41,8 +42,13 @@ export const configureSaveMasterPassword = (params: ConfigureSaveMasterPassword)
// Set encrypted master password in the DB
masterPasswordEncrypted = encryptAES(secrets.localKey, Buffer.from(secrets.masterPassword));

// Set local key in the OS keychain
setLocalKey(secrets.login, shouldNotSaveMasterPassword, secrets.localKey);
if (!shouldNotSaveMasterPassword) {
// Set local key in the OS keychain
setLocalKey(secrets.login, secrets.localKey, (errorMessage: string) => {
warnUnreachableKeychainDisabled(errorMessage);
shouldNotSaveMasterPassword = true;
});
}
}

db.prepare('UPDATE device SET masterPasswordEncrypted = ?, shouldNotSaveMasterPassword = ? WHERE login = ?')
Expand Down

0 comments on commit 8d9d5db

Please sign in to comment.