-
Notifications
You must be signed in to change notification settings - Fork 791
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -353,5 +353,20 @@ | |||||
}, | ||||||
"algorithm": { | ||||||
"message": "Algorithm" | ||||||
}, | ||||||
"operationFailed": { | ||||||
"message": "Operation failed." | ||||||
}, | ||||||
"noPassphraseWarning": { | ||||||
"message": "You must set a passphrase before enable security key." | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
}, | ||||||
"securityKeyEnabled": { | ||||||
"message": "Security key enabled." | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
}, | ||||||
"alreadyEnabledSecurityKeyWarning": { | ||||||
"message": "You have already enabled security key, do you want to continue?" | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
}, | ||||||
"updatePassphraseDisableSecurityKeyWarning": { | ||||||
"message": "Update passphrase will reset security key, do you want to continue?" | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
} | ||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 { | ||
|
@@ -29,12 +40,31 @@ export default Vue.extend({ | |
computed: { | ||
wrongPassword() { | ||
return this.$store.state.accounts.wrongPassword; | ||
}, | ||
securityKeyEnabled() { | ||
return !!localStorage.securityTokenEncryptedKey; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
}, | ||
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> |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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(); | ||
|
@@ -483,6 +486,138 @@ export class Accounts implements Module { | |
}); | ||
}); | ||
} | ||
}, | ||
enableSecurityKey: async (state: ActionContext<AccountsState, {}>) => { | ||
const createCredentialOptions: CredentialCreationOptions = { | ||
publicKey: { | ||
rp: { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
id: chrome.runtime.id, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it really ok for us to just have blank ids? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 }, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is deprecated There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's the only supported algorithm for Windows Hello. https://docs.microsoft.com/en-us/microsoft-edge/dev-guide/windows-integration/web-authentication |
||
{ type: "public-key", alg: -258 }, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not recommended |
||
{ type: "public-key", alg: -259 }, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not recommended |
||
{ 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 | ||
|
There was a problem hiding this comment.
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