Skip to content
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

[SG-649] Add confirmation dialog and tweak shared key retrieval #3451

Merged
merged 5 commits into from
Sep 15, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions apps/desktop/native-messaging-test-runner/src/bw-handshake.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import * as config from "./variables";
LogUtils.logSuccess("Received response to handshake request");
if (response.status === "success") {
LogUtils.logSuccess("Handshake success response");
} else if (response.error === "canceled") {
LogUtils.logWarning("Handshake canceled by user");
} else {
LogUtils.logError("Handshake failure response");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ NodeIPC.config.maxRetries = 0;
NodeIPC.config.silent = true;

const DESKTOP_APP_PATH = `${homedir}/tmp/app.bitwarden`;
const DEFAULT_MESSAGE_TIMEOUT = 10 * 1000; // 10 seconds
differsthecat marked this conversation as resolved.
Show resolved Hide resolved
const DEFAULT_MESSAGE_TIMEOUT = 10 * 1500; // 15 seconds

export type MessageHandler = (MessageCommon) => void;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ import IPCService from "./ipcService";
import * as config from "./variables";

type HandshakePayload = {
status: "success" | "canceled";
status?: "success";
differsthecat marked this conversation as resolved.
Show resolved Hide resolved
error?: "canceled";
differsthecat marked this conversation as resolved.
Show resolved Hide resolved
sharedKey?: string;
};

Expand Down
5 changes: 5 additions & 0 deletions apps/desktop/src/app/accounts/settings.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,11 @@ export class SettingsComponent implements OnInit {
await this.stateService.setEnableDuckDuckGoBrowserIntegration(
this.enableDuckDuckGoBrowserIntegration
);

if (!this.enableBrowserIntegration) {
await this.stateService.setDuckDuckGoSharedKey(null);
}

this.messagingService.send(
this.enableDuckDuckGoBrowserIntegration
? "enableDuckDuckGoBrowserIntegration"
Expand Down
1 change: 1 addition & 0 deletions apps/desktop/src/app/services/services.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ const RELOAD_CALLBACK = new InjectionToken<() => any>("RELOAD_CALLBACK");
PolicyServiceAbstraction,
MessagingServiceAbstraction,
PasswordGenerationService,
I18nServiceAbstraction,
],
},
],
Expand Down
6 changes: 6 additions & 0 deletions apps/desktop/src/locales/en/messages.json
Original file line number Diff line number Diff line change
Expand Up @@ -1603,6 +1603,12 @@
"verifyBrowserDesc": {
"message": "Please ensure the shown fingerprint is identical to the fingerprint showed in the browser extension."
},
"verifyDDGBrowserTitle": {
"message": "DuckDuckGo browser wants to connect to Bitwarden"
},
"verifyDDGBrowserDesc": {
"message": "Would you like to approve this request?"
},
"biometricsNotEnabledTitle": {
"message": "Biometrics not enabled"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,6 @@ import { MessageCommon } from "./messageCommon";

export type UnencryptedMessageResponse = MessageCommon &
(
| {
payload: {
status: "canceled";
};
}
| {
payload: {
status: "success";
Expand All @@ -15,7 +10,7 @@ export type UnencryptedMessageResponse = MessageCommon &
}
| {
payload: {
error: "locked" | "cannot-decrypt" | "version-discrepancy";
error: "canceled" | "locked" | "cannot-decrypt" | "version-discrepancy";
};
}
);
78 changes: 54 additions & 24 deletions apps/desktop/src/services/nativeMessageHandler.service.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import { Injectable } from "@angular/core";
import { ipcRenderer } from "electron";
import Swal from "sweetalert2";

import { CipherService } from "@bitwarden/common/abstractions/cipher.service";
import { CryptoService } from "@bitwarden/common/abstractions/crypto.service";
import { CryptoFunctionService } from "@bitwarden/common/abstractions/cryptoFunction.service";
import { I18nService } from "@bitwarden/common/abstractions/i18n.service";
import { MessagingService } from "@bitwarden/common/abstractions/messaging.service";
import { PasswordGenerationService } from "@bitwarden/common/abstractions/passwordGeneration.service";
import { PolicyService } from "@bitwarden/common/abstractions/policy/policy.service.abstraction";
Expand Down Expand Up @@ -44,8 +46,8 @@ export class NativeMessageHandler {
private cipherService: CipherService,
private policyService: PolicyService,
private messagingService: MessagingService,

private passwordGenerationService: PasswordGenerationService
private passwordGenerationService: PasswordGenerationService,
private i18nService: I18nService
) {}

async handleMessage(message: Message) {
Expand All @@ -68,7 +70,6 @@ export class NativeMessageHandler {
}

private async handleDecryptedMessage(message: UnencryptedMessage) {
let encryptedSecret: ArrayBuffer;
const { messageId, payload } = message;
const { publicKey } = payload;
if (!publicKey) {
Expand All @@ -91,20 +92,56 @@ export class NativeMessageHandler {
messageId: messageId,
version: NativeMessagingVersion.Latest,
payload: {
status: "canceled",
error: "canceled",
},
});

return;
}

// Ask for confirmation from user
this.messagingService.send("setFocus");
const submitted = await Swal.fire({
titleText: this.i18nService.t("verifyDDGBrowserTitle"),
html: this.i18nService.t("verifyDDGBrowserDesc"),
showCancelButton: true,
cancelButtonText: this.i18nService.t("no"),
showConfirmButton: true,
confirmButtonText: this.i18nService.t("yes"),
allowOutsideClick: false,
});

if (submitted.value !== true) {
this.sendResponse({
messageId: messageId,
version: NativeMessagingVersion.Latest,
payload: {
error: "canceled",
},
});
return;
}

const secret = await this.cryptoFunctionService.randomBytes(64);
this.ddgSharedSecret = new SymmetricCryptoKey(secret);
encryptedSecret = await this.cryptoFunctionService.rsaEncrypt(
const sharedKeyB64 = new SymmetricCryptoKey(secret).toJSON().keyB64;

await this.stateService.setDuckDuckGoSharedKey(sharedKeyB64);

const encryptedSecret = await this.cryptoFunctionService.rsaEncrypt(
secret,
remotePublicKey,
EncryptionAlgorithm
);

this.sendResponse({
messageId: messageId,
version: NativeMessagingVersion.Latest,
payload: {
status: "success",
sharedKey: Utils.fromBufferToB64(encryptedSecret),
},
});
} catch (error) {
this.sendResponse({
messageId: messageId,
Expand All @@ -114,17 +151,6 @@ export class NativeMessageHandler {
},
});
}

await this.stateService.setDuckDuckGoSharedKey(Utils.fromBufferToB64(encryptedSecret));

this.sendResponse({
messageId: messageId,
version: NativeMessagingVersion.Latest,
payload: {
status: "success",
sharedKey: Utils.fromBufferToB64(encryptedSecret),
},
});
}

private async handleEncryptedMessage(message: EncryptedMessage) {
Expand Down Expand Up @@ -304,14 +330,18 @@ export class NativeMessageHandler {

private async decryptPayload(message: EncryptedMessage): Promise<DecryptedCommandData> {
if (!this.ddgSharedSecret) {
this.sendResponse({
messageId: message.messageId,
version: NativeMessagingVersion.Latest,
payload: {
error: "cannot-decrypt",
},
});
return;
const storedKey = await this.stateService.getDuckDuckGoSharedKey();
if (storedKey == null) {
this.sendResponse({
messageId: message.messageId,
version: NativeMessagingVersion.Latest,
payload: {
error: "cannot-decrypt",
},
});
return;
}
this.ddgSharedSecret = SymmetricCryptoKey.fromJSON({ keyB64: storedKey });
}

return JSON.parse(
Expand Down