-
-
Notifications
You must be signed in to change notification settings - Fork 823
Add support for dehydrated devices #5239
Changes from all commits
1c2e05e
4398f1e
add36ca
b1b7215
4e2397a
744f464
503f329
0db81e4
dd57e94
bd9cebe
c1765c8
49cc62c
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 |
---|---|---|
|
@@ -24,15 +24,21 @@ import {encodeBase64} from "matrix-js-sdk/src/crypto/olmlib"; | |
import { isSecureBackupRequired } from './utils/WellKnownUtils'; | ||
import AccessSecretStorageDialog from './components/views/dialogs/security/AccessSecretStorageDialog'; | ||
import RestoreKeyBackupDialog from './components/views/dialogs/security/RestoreKeyBackupDialog'; | ||
import SettingsStore from "./settings/SettingsStore"; | ||
|
||
// This stores the secret storage private keys in memory for the JS SDK. This is | ||
// only meant to act as a cache to avoid prompting the user multiple times | ||
// during the same single operation. Use `accessSecretStorage` below to scope a | ||
// single secret storage operation, as it will clear the cached keys once the | ||
// operation ends. | ||
let secretStorageKeys = {}; | ||
let secretStorageKeyInfo = {}; | ||
let secretStorageBeingAccessed = false; | ||
|
||
let nonInteractive = false; | ||
|
||
let dehydrationCache = {}; | ||
|
||
function isCachingAllowed() { | ||
return secretStorageBeingAccessed; | ||
} | ||
|
@@ -66,6 +72,20 @@ async function confirmToDismiss() { | |
return !sure; | ||
} | ||
|
||
function makeInputToKey(keyInfo) { | ||
return async ({ passphrase, recoveryKey }) => { | ||
if (passphrase) { | ||
return deriveKey( | ||
passphrase, | ||
keyInfo.passphrase.salt, | ||
keyInfo.passphrase.iterations, | ||
); | ||
} else { | ||
return decodeRecoveryKey(recoveryKey); | ||
} | ||
}; | ||
} | ||
|
||
async function getSecretStorageKey({ keys: keyInfos }, ssssItemName) { | ||
uhoreg marked this conversation as resolved.
Show resolved
Hide resolved
|
||
const keyInfoEntries = Object.entries(keyInfos); | ||
if (keyInfoEntries.length > 1) { | ||
|
@@ -78,17 +98,18 @@ async function getSecretStorageKey({ keys: keyInfos }, ssssItemName) { | |
return [keyId, secretStorageKeys[keyId]]; | ||
} | ||
|
||
const inputToKey = async ({ passphrase, recoveryKey }) => { | ||
if (passphrase) { | ||
return deriveKey( | ||
passphrase, | ||
keyInfo.passphrase.salt, | ||
keyInfo.passphrase.iterations, | ||
); | ||
} else { | ||
return decodeRecoveryKey(recoveryKey); | ||
if (dehydrationCache.key) { | ||
if (await MatrixClientPeg.get().checkSecretStorageKey(dehydrationCache.key, keyInfo)) { | ||
cacheSecretStorageKey(keyId, dehydrationCache.key, keyInfo); | ||
return [keyId, dehydrationCache.key]; | ||
} | ||
}; | ||
} | ||
|
||
if (nonInteractive) { | ||
throw new Error("Could not unlock non-interactively"); | ||
} | ||
|
||
const inputToKey = makeInputToKey(keyInfo); | ||
const { finished } = Modal.createTrackedDialog("Access Secret Storage dialog", "", | ||
AccessSecretStorageDialog, | ||
/* props= */ | ||
|
@@ -118,14 +139,56 @@ async function getSecretStorageKey({ keys: keyInfos }, ssssItemName) { | |
const key = await inputToKey(input); | ||
|
||
// Save to cache to avoid future prompts in the current session | ||
cacheSecretStorageKey(keyId, key); | ||
cacheSecretStorageKey(keyId, key, keyInfo); | ||
|
||
return [keyId, key]; | ||
} | ||
|
||
function cacheSecretStorageKey(keyId, key) { | ||
export async function getDehydrationKey(keyInfo, checkFunc) { | ||
const inputToKey = makeInputToKey(keyInfo); | ||
const { finished } = Modal.createTrackedDialog("Access Secret Storage dialog", "", | ||
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 will ask for the dehydration key using UI that talks about Secure Backup, which worries me that users will get lost in yet another key... If we are not yet taking steps to unify 4S and dehydration key (so the dehydration key may exist and be different), then it seems like we need fresh designs for the UI that asks for it, so as to avoid confusion. 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 suppose since there's a feature flag now, we wouldn't need to block merging on those designs, but it feels like a concern to me that would block enabling it later. |
||
AccessSecretStorageDialog, | ||
/* props= */ | ||
{ | ||
keyInfo, | ||
checkPrivateKey: async (input) => { | ||
const key = await inputToKey(input); | ||
try { | ||
checkFunc(key); | ||
return true; | ||
} catch (e) { | ||
return false; | ||
} | ||
}, | ||
}, | ||
/* className= */ null, | ||
/* isPriorityModal= */ false, | ||
/* isStaticModal= */ false, | ||
/* options= */ { | ||
onBeforeClose: async (reason) => { | ||
if (reason === "backgroundClick") { | ||
return confirmToDismiss(); | ||
} | ||
return true; | ||
}, | ||
}, | ||
); | ||
const [input] = await finished; | ||
if (!input) { | ||
throw new AccessCancelledError(); | ||
} | ||
const key = await inputToKey(input); | ||
|
||
// need to copy the key because rehydration (unpickling) will clobber it | ||
dehydrationCache = {key: new Uint8Array(key), keyInfo}; | ||
|
||
return key; | ||
} | ||
|
||
function cacheSecretStorageKey(keyId, key, keyInfo) { | ||
if (isCachingAllowed()) { | ||
secretStorageKeys[keyId] = key; | ||
secretStorageKeyInfo[keyId] = keyInfo; | ||
} | ||
} | ||
|
||
|
@@ -176,6 +239,7 @@ export const crossSigningCallbacks = { | |
getSecretStorageKey, | ||
cacheSecretStorageKey, | ||
onSecretRequested, | ||
getDehydrationKey, | ||
}; | ||
|
||
export async function promptForBackupPassphrase() { | ||
|
@@ -262,6 +326,18 @@ export async function accessSecretStorage(func = async () => { }, forceReset = f | |
await cli.bootstrapSecretStorage({ | ||
getKeyBackupPassphrase: promptForBackupPassphrase, | ||
}); | ||
|
||
const keyId = Object.keys(secretStorageKeys)[0]; | ||
if (keyId && SettingsStore.getValue("feature_dehydration")) { | ||
const dehydrationKeyInfo = | ||
secretStorageKeyInfo[keyId] && secretStorageKeyInfo[keyId].passphrase | ||
? {passphrase: secretStorageKeyInfo[keyId].passphrase} | ||
: {}; | ||
console.log("Setting dehydration key"); | ||
await cli.setDehydrationKey(secretStorageKeys[keyId], dehydrationKeyInfo, "Backup device"); | ||
jryans marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} else { | ||
console.log("Not setting dehydration key: no SSSS key found"); | ||
} | ||
} | ||
|
||
// `return await` needed here to ensure `finally` block runs after the | ||
|
@@ -272,6 +348,57 @@ export async function accessSecretStorage(func = async () => { }, forceReset = f | |
secretStorageBeingAccessed = false; | ||
if (!isCachingAllowed()) { | ||
secretStorageKeys = {}; | ||
secretStorageKeyInfo = {}; | ||
} | ||
} | ||
} | ||
|
||
// FIXME: this function name is a bit of a mouthful | ||
export async function tryToUnlockSecretStorageWithDehydrationKey(client) { | ||
jryans marked this conversation as resolved.
Show resolved
Hide resolved
|
||
const key = dehydrationCache.key; | ||
let restoringBackup = false; | ||
if (key && await client.isSecretStorageReady()) { | ||
console.log("Trying to set up cross-signing using dehydration key"); | ||
secretStorageBeingAccessed = true; | ||
nonInteractive = true; | ||
try { | ||
await client.checkOwnCrossSigningTrust(); | ||
|
||
// we also need to set a new dehydrated device to replace the | ||
// device we rehydrated | ||
const dehydrationKeyInfo = | ||
dehydrationCache.keyInfo && dehydrationCache.keyInfo.passphrase | ||
? {passphrase: dehydrationCache.keyInfo.passphrase} | ||
: {}; | ||
await client.setDehydrationKey(key, dehydrationKeyInfo, "Backup device"); | ||
|
||
// and restore from backup | ||
const backupInfo = await client.getKeyBackupVersion(); | ||
if (backupInfo) { | ||
restoringBackup = true; | ||
// don't await, because this can take a long time | ||
client.restoreKeyBackupWithSecretStorage(backupInfo) | ||
.finally(() => { | ||
secretStorageBeingAccessed = false; | ||
nonInteractive = false; | ||
if (!isCachingAllowed()) { | ||
secretStorageKeys = {}; | ||
secretStorageKeyInfo = {}; | ||
} | ||
}); | ||
} | ||
} finally { | ||
dehydrationCache = {}; | ||
// the secret storage cache is needed for restoring from backup, so | ||
// don't clear it yet if we're restoring from backup | ||
if (!restoringBackup) { | ||
secretStorageBeingAccessed = false; | ||
nonInteractive = false; | ||
if (!isCachingAllowed()) { | ||
secretStorageKeys = {}; | ||
secretStorageKeyInfo = {}; | ||
} | ||
} | ||
} | ||
} | ||
} |
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.
Is there a way to know that device ID came from rehydration? That would allow us to know the same information as
freshLogin
without needing an additional flag to track it.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.
Sorry, I don't really understand this comment. The
freshLogin
flag tells us whether we just logged in or not. And if so, then it tries to rehydrate the device. So at the point where thefreshLogin
flag is set, we haven't tried to rehydrate yet. We only want to try to rehydrate if we just logged in so that we don't replace a device that is already in use with the dehydrated device.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.
I guess I am trying to ask if there is any other way to know whether a rehydration is needed, instead of adding a new flag
freshLogin
. It would be nice to skip this work if it is not needed, for example.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.
I see. There's two different issues here. The first is whether there's a dehydrated device ready to be rehydrated. If there isn't a dehydrated device, the
rehydrateDevice
function will basically only make one call to the server and then return, so it shouldn't be much work.The second issue is whether we just logged in, which is what the
freshLogin
flag checks for. If, for example, someone reloads Element, we don't want it to try to dehydrate a device even if there is one available. We want it to keep using the device it was already using. So we only check for a dehydrated device if the user just logged in.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.
Okay, I think I am following. As an alternative to storing
mx_fresh_login
in local storage when it is fresh, could we instead rely on knowing that the_restoreFromLocalStorage
means we must not be a fresh login, and so it could pass somethingfreshLogin: false
through to_doSetLoggedIn
without needing extra storage...?Sorry if this feels a bit pedantic... 😅 I'm trying to finding chances to reduce complexity (even if a tiny amount), since login is already very complex.
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.
Unfortunately, we can't do that. When we do an SSO login, it saves the state to storage, and then reloads the page (why? I have no idea), so that means that it will
_restoreFromLocalStorage
even though it's a fresh login.