diff --git a/core/src/api/keys.rs b/core/src/api/keys.rs index 570788a53..d15a4043e 100644 --- a/core/src/api/keys.rs +++ b/core/src/api/keys.rs @@ -57,7 +57,7 @@ pub(crate) fn mount() -> RouterBuilder { }) // do not unlock the key manager until this route returns true .library_query("isUnlocked", |t| { - t(|_, _: (), library| async move { Ok(library.key_manager.is_unlocked().await?) }) + t(|_, _: (), library| async move { Ok(library.key_manager.is_unlocked().await) }) }) // this is so we can show the key as mounted in the UI .library_query("listMounted", |t| { @@ -121,7 +121,7 @@ pub(crate) fn mount() -> RouterBuilder { }) .library_mutation("syncKeyToLibrary", |t| { t(|_, key_uuid: Uuid, library| async move { - let key = library.key_manager.sync_to_database(key_uuid)?; + let key = library.key_manager.sync_to_database(key_uuid).await?; // does not check that the key doesn't exist before writing write_storedkey_to_db(&library.db, &key).await?; @@ -132,10 +132,11 @@ pub(crate) fn mount() -> RouterBuilder { }) .library_mutation("updateAutomountStatus", |t| { t(|_, args: AutomountUpdateArgs, library| async move { - if !library.key_manager.is_memory_only(args.uuid)? { + if !library.key_manager.is_memory_only(args.uuid).await? { library .key_manager - .change_automount_status(args.uuid, args.status)?; + .change_automount_status(args.uuid, args.status) + .await?; library .db @@ -155,7 +156,7 @@ pub(crate) fn mount() -> RouterBuilder { }) .library_mutation("deleteFromLibrary", |t| { t(|_, key_uuid: Uuid, library| async move { - if !library.key_manager.is_memory_only(key_uuid)? { + if !library.key_manager.is_memory_only(key_uuid).await? { library .db .key() @@ -266,8 +267,11 @@ pub(crate) fn mount() -> RouterBuilder { .await?; if args.library_sync { - write_storedkey_to_db(&library.db, &library.key_manager.access_keystore(uuid)?) - .await?; + write_storedkey_to_db( + &library.db, + &library.key_manager.access_keystore(uuid).await?, + ) + .await?; if args.automount { library diff --git a/core/src/object/fs/encrypt.rs b/core/src/object/fs/encrypt.rs index 9dc1f8783..37e0c8fc0 100644 --- a/core/src/object/fs/encrypt.rs +++ b/core/src/object/fs/encrypt.rs @@ -38,7 +38,7 @@ pub struct Metadata { pub path_id: i32, pub name: String, pub hidden: bool, - pub favourite: bool, + pub favorite: bool, pub important: bool, pub note: Option, pub date_created: chrono::DateTime, @@ -87,9 +87,12 @@ impl StatefulJob for FileEncryptorJob { if !info.path_data.is_dir { // handle overwriting checks, and making sure there's enough available space - let user_key = key_manager.access_keymount(state.init.key_uuid)?.hashed_key; + let user_key = key_manager + .access_keymount(state.init.key_uuid) + .await? + .hashed_key; - let user_key_details = key_manager.access_keystore(state.init.key_uuid)?; + let user_key_details = key_manager.access_keystore(state.init.key_uuid).await?; let output_path = state.init.output_path.clone().map_or_else( || { @@ -155,7 +158,7 @@ impl StatefulJob for FileEncryptorJob { path_id: state.init.path_id, name: info.path_data.materialized_path.clone(), hidden: object.hidden, - favourite: object.favorite, + favorite: object.favorite, important: object.important, note: object.note, date_created: object.date_created, diff --git a/crates/crypto/src/error.rs b/crates/crypto/src/error.rs index 0170ad82f..c0dcb8896 100644 --- a/crates/crypto/src/error.rs +++ b/crates/crypto/src/error.rs @@ -16,36 +16,29 @@ pub type Result = std::result::Result; /// This enum defines all possible errors that this crate can give #[derive(Error, Debug)] pub enum Error { + // crypto primitive errors (STREAM, hashing) #[error("there was an error while password hashing")] PasswordHash, - #[error("I/O error: {0}")] - Io(#[from] std::io::Error), #[error("error while encrypting")] Encrypt, #[error("error while decrypting")] Decrypt, #[error("nonce length mismatch")] NonceLengthMismatch, - #[error("invalid file header")] - FileHeader, #[error("error initialising stream encryption/decryption")] StreamModeInit, - #[error("wrong password provided")] - IncorrectPassword, + + // header errors #[error("no keyslots available")] NoKeyslots, - #[error("mismatched data length while converting vec to array")] - VecArrSizeMismatch, - #[error("error while parsing preview media length")] - MediaLengthParse, #[error("no preview media found")] NoPreviewMedia, - #[error("error while serializing/deserializing an item")] - Serialization, #[error("no metadata found")] NoMetadata, #[error("tried adding too many keyslots to a header")] TooManyKeyslots, + + // key manager #[error("requested key wasn't found in the key manager")] KeyNotFound, #[error("key is already mounted")] @@ -58,38 +51,34 @@ pub enum Error { KeyAlreadyQueued, #[error("no default key has been set")] NoDefaultKeySet, - #[error("no master password has been provided to the keymanager")] - NoMasterPassword, - #[error("mismatch between supplied keys and the keystore")] - KeystoreMismatch, - #[error("mutex lock error")] - MutexLock, + #[error("keymanager is not unlocked")] + NotUnlocked, #[error("no verification key")] NoVerificationKey, #[error("key isn't flagged as memory only")] KeyNotMemoryOnly, - #[error("wrong information provided to the key manager")] - IncorrectKeymanagerDetails, + + // general errors + #[error("I/O error: {0}")] + Io(#[from] std::io::Error), + #[error("mismatched data length while converting vec to array")] + VecArrSizeMismatch, + #[error("incorrect password/details were provided")] + IncorrectPassword, + #[error("error while serializing/deserializing an item")] + Serialization, #[error("string parse error")] StringParse(#[from] FromUtf8Error), + // keyring #[cfg(target_os = "linux")] #[error("error with the linux keyring: {0}")] LinuxKeyringError(#[from] secret_service::Error), - #[cfg(any(target_os = "macos", target_os = "ios"))] #[error("error with the apple keyring: {0}")] AppleKeyringError(#[from] security_framework::base::Error), - #[error("generic keyring error")] KeyringError, - #[error("keyring not available on this platform")] KeyringNotSupported, } - -impl From> for Error { - fn from(_: std::sync::PoisonError) -> Self { - Self::MutexLock - } -} diff --git a/crates/crypto/src/header/file.rs b/crates/crypto/src/header/file.rs index 3ae4accde..93ba8039d 100644 --- a/crates/crypto/src/header/file.rs +++ b/crates/crypto/src/header/file.rs @@ -256,7 +256,7 @@ impl FileHeader { reader.read_exact(&mut magic_bytes).await?; if magic_bytes != MAGIC_BYTES { - return Err(Error::FileHeader); + return Err(Error::Serialization); } let mut version = [0u8; 2]; diff --git a/crates/crypto/src/keys/keymanager.rs b/crates/crypto/src/keys/keymanager.rs index 262bc3c99..38233042e 100644 --- a/crates/crypto/src/keys/keymanager.rs +++ b/crates/crypto/src/keys/keymanager.rs @@ -152,6 +152,28 @@ impl KeyManager { Ok(()) } + // This verifies that the key manager is unlocked before continuing the calling function. + pub async fn ensure_unlocked(&self) -> Result<()> { + self.is_unlocked() + .await + .then_some(()) + .ok_or(Error::NotUnlocked) + } + + // This verifies that the target key is not already queued before continuing the operation. + pub fn ensure_not_queued(&self, uuid: Uuid) -> Result<()> { + (!self.is_queued(uuid)) + .then_some(()) + .ok_or(Error::KeyAlreadyMounted) + } + + // This verifies that the target key is not already mounted before continuing the operation. + pub fn ensure_not_mounted(&self, uuid: Uuid) -> Result<()> { + (!self.keymount.contains_key(&uuid)) + .then_some(()) + .ok_or(Error::KeyAlreadyMounted) + } + pub async fn keyring_retrieve( &self, library_uuid: Uuid, @@ -319,6 +341,8 @@ impl KeyManager { /// This function removes a key from the keystore, the keymount and it's unset as the default. pub async fn remove_key(&self, uuid: Uuid) -> Result<()> { + self.ensure_unlocked().await?; + if self.keystore.contains_key(&uuid) { // if key is default, clear it // do this manually to prevent deadlocks @@ -348,6 +372,8 @@ impl KeyManager { hashing_algorithm: HashingAlgorithm, library_uuid: Uuid, ) -> Result { + self.ensure_unlocked().await?; + let secret_key = SecretKey::generate(); let content_salt = Salt::generate(); @@ -429,6 +455,8 @@ impl KeyManager { secret_key: SecretKeyString, // at the time of the backup stored_keys: &[StoredKey], // from the backup ) -> Result> { + self.ensure_unlocked().await?; + // this backup should contain a verification key, which will tell us the algorithm+hashing algorithm let secret_key = secret_key.into(); @@ -541,7 +569,7 @@ impl KeyManager { /// /// Only provide the secret key if it should not/can not be sourced from an OS keychain (e.g. web, OS keychains not enabled/available, etc). /// - /// This minimises the risk of an attacker obtaining the master password, as both of these are required to unlock the vault (and both should be stored separately). + /// This minimizes the risk of an attacker obtaining the master password, as both of these are required to unlock the vault (and both should be stored separately). /// /// Both values need to be correct, otherwise this function will return a generic error. /// @@ -563,11 +591,7 @@ impl KeyManager { .as_ref() .map_or(Err(Error::NoVerificationKey), |k| Ok(k.clone()))?; - if self.is_unlocked().await? { - return Err(Error::KeyAlreadyMounted); - } else if self.is_queued(verification_key.uuid) { - return Err(Error::KeyAlreadyQueued); - } + self.ensure_not_queued(verification_key.uuid)?; let secret_key = if let Some(secret_key) = provided_secret_key.clone() { secret_key.into() @@ -615,7 +639,7 @@ impl KeyManager { .await .map_err(|_| { self.remove_from_queue(verification_key.uuid).ok(); - Error::IncorrectKeymanagerDetails + Error::IncorrectPassword })?; *self.root_key.lock().await = Some( @@ -663,11 +687,9 @@ impl KeyManager { /// /// We could add a log to this, so that the user can view mounts pub async fn mount(&self, uuid: Uuid) -> Result<()> { - if self.keymount.get(&uuid).is_some() { - return Err(Error::KeyAlreadyMounted); - } else if self.is_queued(uuid) { - return Err(Error::KeyAlreadyQueued); - } + self.ensure_unlocked().await?; + self.ensure_not_mounted(uuid)?; + self.ensure_not_queued(uuid)?; if let Some(stored_key) = self.keystore.get(&uuid) { match stored_key.version { @@ -738,6 +760,8 @@ impl KeyManager { /// /// The master password/salt needs to be present, so we are able to decrypt the key itself from the stored key. pub async fn get_key(&self, uuid: Uuid) -> Result { + self.ensure_unlocked().await?; + if let Some(stored_key) = self.keystore.get(&uuid) { let master_key = StreamDecryption::decrypt_bytes( Key::derive( @@ -790,6 +814,8 @@ impl KeyManager { automount: bool, content_salt: Option, ) -> Result { + self.ensure_unlocked().await?; + let uuid = Uuid::new_v4(); // Generate items we'll need for encryption @@ -851,14 +877,18 @@ impl KeyManager { /// This function is for accessing the internal keymount. /// /// We could add a log to this, so that the user can view accesses - pub fn access_keymount(&self, uuid: Uuid) -> Result { + pub async fn access_keymount(&self, uuid: Uuid) -> Result { + self.ensure_unlocked().await?; + self.keymount .get(&uuid) .map_or(Err(Error::KeyNotFound), |v| Ok(v.clone())) } /// This function is for accessing a `StoredKey`. - pub fn access_keystore(&self, uuid: Uuid) -> Result { + pub async fn access_keystore(&self, uuid: Uuid) -> Result { + self.ensure_unlocked().await?; + self.keystore .get(&uuid) .map_or(Err(Error::KeyNotFound), |v| Ok(v.clone())) @@ -866,6 +896,8 @@ impl KeyManager { /// This allows you to set the default key pub async fn set_default(&self, uuid: Uuid) -> Result<()> { + self.ensure_unlocked().await?; + if self.keystore.contains_key(&uuid) { *self.default.lock().await = Some(uuid); Ok(()) @@ -876,16 +908,14 @@ impl KeyManager { /// This allows you to get the default key's ID pub async fn get_default(&self) -> Result { + self.ensure_unlocked().await?; + self.default.lock().await.ok_or(Error::NoDefaultKeySet) } /// This should ONLY be used internally. async fn get_root_key(&self) -> Result { - self.root_key - .lock() - .await - .clone() - .ok_or(Error::NoMasterPassword) + self.root_key.lock().await.clone().ok_or(Error::NotUnlocked) } pub async fn get_verification_key(&self) -> Result { @@ -896,13 +926,17 @@ impl KeyManager { .ok_or(Error::NoVerificationKey) } - pub fn is_memory_only(&self, uuid: Uuid) -> Result { + pub async fn is_memory_only(&self, uuid: Uuid) -> Result { + self.ensure_unlocked().await?; + self.keystore .get(&uuid) .map_or(Err(Error::KeyNotFound), |v| Ok(v.memory_only)) } - pub fn change_automount_status(&self, uuid: Uuid, status: bool) -> Result<()> { + pub async fn change_automount_status(&self, uuid: Uuid, status: bool) -> Result<()> { + self.ensure_unlocked().await?; + let updated_key = self .keystore .get(&uuid) @@ -933,8 +967,8 @@ impl KeyManager { /// This function is for converting a memory-only key to a saved key which syncs to the library. /// /// The returned value needs to be written to the database. - pub fn sync_to_database(&self, uuid: Uuid) -> Result { - if !self.is_memory_only(uuid)? { + pub async fn sync_to_database(&self, uuid: Uuid) -> Result { + if !self.is_memory_only(uuid).await? { return Err(Error::KeyNotMemoryOnly); } @@ -961,8 +995,8 @@ impl KeyManager { } /// This function is used for checking if the key manager is unlocked. - pub async fn is_unlocked(&self) -> Result { - Ok(self.root_key.lock().await.is_some()) + pub async fn is_unlocked(&self) -> bool { + self.root_key.lock().await.is_some() } /// This function is used for unmounting all keys at once. @@ -985,12 +1019,10 @@ impl KeyManager { /// This function returns a Vec of `StoredKey`s, so you can write them somewhere/update the database with them/etc /// /// The database and keystore should be in sync at ALL times (unless the user chose an in-memory only key) - #[must_use] pub fn dump_keystore(&self) -> Vec { self.keystore.iter().map(|key| key.clone()).collect() } - #[must_use] pub fn get_mounted_uuids(&self) -> Vec { self.keymount.iter().map(|key| key.uuid).collect() } diff --git a/packages/interface/src/components/explorer/ExplorerContextMenu.tsx b/packages/interface/src/components/explorer/ExplorerContextMenu.tsx index ac8b16fe2..f7278e5d3 100644 --- a/packages/interface/src/components/explorer/ExplorerContextMenu.tsx +++ b/packages/interface/src/components/explorer/ExplorerContextMenu.tsx @@ -348,7 +348,7 @@ export function FileItemContextMenu({ data, ...props }: FileItemContextMenuProps dialogManager.create((dp) => ( )); diff --git a/packages/interface/src/components/key/KeyManager.tsx b/packages/interface/src/components/key/KeyManager.tsx index 3da1eff5f..eec5b0ef0 100644 --- a/packages/interface/src/components/key/KeyManager.tsx +++ b/packages/interface/src/components/key/KeyManager.tsx @@ -140,9 +140,11 @@ export function KeyManager(props: KeyManagerProps) { - - - + {isUnlocked && ( + + + + )} diff --git a/packages/interface/src/screens/settings/library/KeysSetting.tsx b/packages/interface/src/screens/settings/library/KeysSetting.tsx index 9a96fa0aa..357549492 100644 --- a/packages/interface/src/screens/settings/library/KeysSetting.tsx +++ b/packages/interface/src/screens/settings/library/KeysSetting.tsx @@ -210,9 +210,11 @@ export default function KeysSettings() { } /> -
- -
+ {isUnlocked && ( +
+ +
+ )} {keyringSk?.data && ( <>