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

Add security key support #496

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all 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
15 changes: 15 additions & 0 deletions _locales/en/messages.json
Original file line number Diff line number Diff line change
Expand Up @@ -353,5 +353,20 @@
},
"algorithm": {
"message": "Algorithm"
},
"operationFailed": {
"message": "Operation failed."
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Crowdin is going to flood my email again :(

use updateFailure

},
"noPassphraseWarning": {
"message": "You must set a passphrase before enable security key."
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"message": "You must set a passphrase before enable security key."
"message": "You must set a password before adding a security key."

},
"securityKeyEnabled": {
"message": "Security key enabled."
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"message": "Security key enabled."
"message": "Security key added."

},
"alreadyEnabledSecurityKeyWarning": {
"message": "You have already enabled security key, do you want to continue?"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"message": "You have already enabled security key, do you want to continue?"
"message": "Your old security key will not work, are you sure you want to continue?"

},
"updatePassphraseDisableSecurityKeyWarning": {
"message": "Update passphrase will reset security key, do you want to continue?"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"message": "Update passphrase will reset security key, do you want to continue?"
"message": This action will also remove your security key, do you want to continue?"

}
}
13 changes: 13 additions & 0 deletions sass/_ui.scss
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,19 @@ $theme-map: null;
padding: 10px;
}

.button-u2f {
@extend .button;
margin: 20px 97px;
width: 84px;
border: none !important;
padding: 0;
}

.button-u2f svg {
width: 64px;
height: 64px;
}

.input {
display: block;
margin: 15px 10px 20px 10px;
Expand Down
30 changes: 30 additions & 0 deletions src/components/Popup/EnterPasswordPage.vue
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,23 @@
i18n.phrase_not_match
}}</label>
<div class="button-small" v-on:click="applyPassphrase()">{{ i18n.ok }}</div>
<div
class="button-u2f"
v-on:click="applySecurityKey()"
v-show="securityKeyEnabled && u2fSupportedPlatform"
>
<IconWindowsHello v-if="u2fSupportedPlatform === 'Win'" />
<IconTouchId v-if="u2fSupportedPlatform === 'Mac'" />
</div>
</div>
</template>
<script lang="ts">
import Vue from "vue";
import { mapState } from "vuex";

import IconWindowsHello from "../../../svg/windows-hello.svg";
import IconTouchId from "../../../svg/touch-id.svg";

export default Vue.extend({
data: function() {
return {
Expand All @@ -29,12 +40,31 @@ export default Vue.extend({
computed: {
wrongPassword() {
return this.$store.state.accounts.wrongPassword;
},
securityKeyEnabled() {
return !!localStorage.securityTokenEncryptedKey;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

!!?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

!!localStorage.securityTokenEncryptedKey always has bool value

},
u2fSupportedPlatform() {
if (navigator.platform.indexOf("Win") > -1) {
return "Win";
} else if (navigator.platform.indexOf("Mac") > -1) {
return "Mac";
} else {
return null;
}
}
},
methods: {
applyPassphrase() {
this.$store.dispatch("accounts/applyPassphrase", this.password);
},
applySecurityKey() {
this.$store.dispatch("accounts/applySecurityKey");
}
},
components: {
IconWindowsHello,
IconTouchId
}
});
</script>
51 changes: 51 additions & 0 deletions src/components/Popup/SetPasswordPage.vue
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,22 @@
>
{{ i18n.ok }}
</div>
<div
class="button-u2f"
v-on:click="enableSecurityKey()"
v-show="encryption.getEncryptionStatus() && u2fSupportedPlatform"
>
<IconWindowsHello v-if="u2fSupportedPlatform === 'Win'" />
<IconTouchId v-if="u2fSupportedPlatform === 'Mac'" />
</div>
</div>
</template>
<script lang="ts">
import Vue from "vue";

import IconWindowsHello from "../../../svg/windows-hello.svg";
import IconTouchId from "../../../svg/touch-id.svg";

export default Vue.extend({
data: function() {
return {
Expand All @@ -38,8 +49,20 @@ export default Vue.extend({
};
},
computed: {
encryption: function() {
return this.$store.state.accounts.encryption;
},
enforcePassword: function() {
return this.$store.state.menu.enforcePassword;
},
u2fSupportedPlatform() {
if (navigator.platform.indexOf("Win") > -1) {
return "Win";
} else if (navigator.platform.indexOf("Mac") > -1) {
return "Mac";
} else {
return null;
}
}
},
methods: {
Expand All @@ -60,12 +83,40 @@ export default Vue.extend({
return;
}

if (
localStorage.securityTokenEncryptedKey &&
!(await this.$store.dispatch(
"notification/confirm",
this.i18n.updatePassphraseDisableSecurityKeyWarning
))
) {
return;
}

this.$store.commit("currentView/changeView", "LoadingPage");
await this.$store.dispatch("accounts/changePassphrase", this.phrase);
this.$store.commit("notification/alert", this.i18n.updateSuccess);
this.$store.commit("style/hideInfo");
return;
},
async enableSecurityKey() {
if (
!localStorage.securityTokenEncryptedKey ||
(await this.$store.dispatch(
"notification/confirm",
this.i18n.alreadyEnabledSecurityKeyWarning
))
) {
const result = await this.$store.dispatch("accounts/enableSecurityKey");
this.$store.commit("notification/alert", result);
this.$store.commit("style/hideInfo");
}
return;
}
},
components: {
IconWindowsHello,
IconTouchId
}
});
</script>
135 changes: 135 additions & 0 deletions src/store/Accounts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,9 @@ export class Accounts implements Module {

// remove cached passphrase in old version
localStorage.removeItem("encodedPhrase");

// remove security token encrypted key in old version
localStorage.removeItem("securityTokenEncryptedKey");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no older version that uses this key?

},
updateEntries: async (state: ActionContext<AccountsState, {}>) => {
const entries = await this.getEntries();
Expand Down Expand Up @@ -483,6 +486,138 @@ export class Accounts implements Module {
});
});
}
},
enableSecurityKey: async (state: ActionContext<AccountsState, {}>) => {
const createCredentialOptions: CredentialCreationOptions = {
publicKey: {
rp: {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

id: chrome.runtime.id,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is different between browsers. Will this break backups between Chrome <=> Firefox if enabled?

name: "Authenticator"
},
user: {
name: "Authenticator",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to remove the name / displayname since we don't actually know any of this info?

displayName: "Authenticator",
id: new Uint8Array(16)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it really ok for us to just have blank ids?

Copy link
Member Author

@Sneezry Sneezry Jun 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought it will be some random chars without setting a value just like C language. I will update to use a random string.

},
challenge: new Uint8Array(16),
Copy link
Member

@mymindstorm mymindstorm Jun 8, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should have some random data. we should verify that the key is working and response is signed properly

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't care about the credential, we are only intereted in credential id

pubKeyCredParams: [
{ type: "public-key", alg: -7 },
{ type: "public-key", alg: -35 },
{ type: "public-key", alg: -36 },
{ type: "public-key", alg: -257 },
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{ type: "public-key", alg: -258 },
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{ type: "public-key", alg: -259 },
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{ type: "public-key", alg: -37 },
{ type: "public-key", alg: -38 },
{ type: "public-key", alg: -39 }
],
authenticatorSelection: {
authenticatorAttachment: "platform",
requireResidentKey: false,
userVerification: "required"
}
}
};

let credentialInfo: Credential | null = null;

try {
credentialInfo = await navigator.credentials.create(
createCredentialOptions
);
} catch (e) {
console.error(e);
}

if (credentialInfo === null) {
return chrome.i18n.getMessage("operationFailed");
} else {
const cachedPassphrase = await this.getCachedPassphrase();
const encKey = await BrowserStorage.getKey();
if (encKey === null) {
return chrome.i18n.getMessage("noPassphraseWarning");
}
const encryptedKey = CryptoJS.AES.encrypt(
CryptoJS.enc.Hex.parse(cachedPassphrase),
credentialInfo.id
).toString();
localStorage.securityTokenEncryptedKey = encryptedKey;
return chrome.i18n.getMessage("securityKeyEnabled");
}
},
applySecurityKey: async (state: ActionContext<AccountsState, {}>) => {
const encKey = await BrowserStorage.getKey();
if (!encKey) {
// TODO: alert to ask customer enter password first to migrate
return;
}

let credentialInfo: Credential | null = null;

try {
credentialInfo = await navigator.credentials.get({
publicKey: {
challenge: new Uint8Array(16),
rpId: chrome.runtime.id,
userVerification: "required"
}
});
} catch (e) {
console.error(e);
}

if (credentialInfo === null) {
state.commit("currentView/changeView", "EnterPasswordPage", {
root: true
});
return;
}

const credentialId = credentialInfo.id;

const key = CryptoJS.AES.decrypt(
localStorage.securityTokenEncryptedKey,
credentialId
).toString();
const isCorrectPassword = await new Promise(
(resolve: (value: string) => void) => {
const iframe = document.getElementById("argon-sandbox");
const message = {
action: "verify",
value: key,
hash: encKey.hash
};
if (iframe) {
window.addEventListener("message", response => {
resolve(response.data.response);
});
// eslint-disable-next-line @typescript-eslint/ban-ts-ignore
//@ts-ignore
iframe.contentWindow.postMessage(message, "*");
}
}
);

if (!isCorrectPassword) {
state.commit("wrongPassword");
state.commit("currentView/changeView", "EnterPasswordPage", {
root: true
});
return;
}

state.state.encryption.updateEncryptionPassword(key);
await state.dispatch("updateEntries");

if (!state.getters.currentlyEncrypted) {
chrome.runtime.sendMessage({
action: "cachePassphrase",
value: key
});
}

state.commit("style/hideInfo", true, { root: true });
return;
}
},
namespaced: true
Expand Down
37 changes: 37 additions & 0 deletions svg/touch-id.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 1 addition & 0 deletions svg/windows-hello.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.