Skip to content

Commit

Permalink
Add explicit error about invalid private key (#1020)
Browse files Browse the repository at this point in the history
Add an explicit error about private key being invalid.
  • Loading branch information
Hinton authored Sep 5, 2024
1 parent 7ee604f commit 80a7d84
Show file tree
Hide file tree
Showing 7 changed files with 63 additions and 21 deletions.
6 changes: 4 additions & 2 deletions crates/bitwarden-core/src/auth/auth_request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ use bitwarden_crypto::{
#[cfg(feature = "internal")]
use bitwarden_crypto::{EncString, KeyDecryptable, SymmetricCryptoKey};

#[cfg(feature = "internal")]
use crate::client::encryption_settings::EncryptionSettingsError;
use crate::{error::Error, Client};

#[cfg_attr(feature = "uniffi", derive(uniffi::Record))]
Expand Down Expand Up @@ -49,7 +51,7 @@ pub(crate) fn new_auth_request(email: &str) -> Result<AuthRequestResponse, Error
pub(crate) fn auth_request_decrypt_user_key(
private_key: String,
user_key: AsymmetricEncString,
) -> Result<SymmetricCryptoKey, Error> {
) -> Result<SymmetricCryptoKey, EncryptionSettingsError> {
let key = AsymmetricCryptoKey::from_der(&STANDARD.decode(private_key)?)?;
let mut key: Vec<u8> = user_key.decrypt_with_key(&key)?;

Expand All @@ -62,7 +64,7 @@ pub(crate) fn auth_request_decrypt_master_key(
private_key: String,
master_key: AsymmetricEncString,
user_key: EncString,
) -> Result<SymmetricCryptoKey, Error> {
) -> Result<SymmetricCryptoKey, EncryptionSettingsError> {
use bitwarden_crypto::MasterKey;

let key = AsymmetricCryptoKey::from_der(&STANDARD.decode(private_key)?)?;
Expand Down
22 changes: 19 additions & 3 deletions crates/bitwarden-core/src/client/encryption_settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,24 @@ use std::collections::HashMap;
use bitwarden_crypto::{AsymmetricCryptoKey, CryptoError, KeyContainer, SymmetricCryptoKey};
#[cfg(feature = "internal")]
use bitwarden_crypto::{AsymmetricEncString, EncString, MasterKey};
use thiserror::Error;
use uuid::Uuid;

#[cfg(feature = "internal")]
use crate::error::Result;

#[derive(Debug, Error)]
pub enum EncryptionSettingsError {
#[error("Cryptography error, {0}")]
Crypto(#[from] bitwarden_crypto::CryptoError),

#[error(transparent)]
InvalidBase64(#[from] base64::DecodeError),

#[error("Invalid private key")]
InvalidPrivateKey,
}

#[derive(Clone)]
pub struct EncryptionSettings {
user_key: SymmetricCryptoKey,
Expand All @@ -28,7 +41,7 @@ impl EncryptionSettings {
master_key: MasterKey,
user_key: EncString,
private_key: EncString,
) -> Result<Self> {
) -> Result<Self, EncryptionSettingsError> {
// Decrypt the user key
let user_key = master_key.decrypt_user_key(user_key)?;
Self::new_decrypted_key(user_key, private_key)
Expand All @@ -42,12 +55,15 @@ impl EncryptionSettings {
pub(crate) fn new_decrypted_key(
user_key: SymmetricCryptoKey,
private_key: EncString,
) -> Result<Self> {
) -> Result<Self, EncryptionSettingsError> {
use bitwarden_crypto::KeyDecryptable;

let private_key = {
let dec: Vec<u8> = private_key.decrypt_with_key(&user_key)?;
Some(AsymmetricCryptoKey::from_der(&dec)?)
Some(
AsymmetricCryptoKey::from_der(&dec)
.map_err(|_| EncryptionSettingsError::InvalidPrivateKey)?,
)
};

Ok(EncryptionSettings {
Expand Down
18 changes: 10 additions & 8 deletions crates/bitwarden-core/src/client/internal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,18 @@ use uuid::Uuid;

#[cfg(feature = "secrets")]
use super::login_method::ServiceAccountLoginMethod;
use super::{encryption_settings::EncryptionSettings, login_method::LoginMethod};
#[cfg(feature = "internal")]
use super::{flags::Flags, login_method::UserLoginMethod};
#[cfg(feature = "internal")]
use crate::error::Error;
use crate::{
auth::renew::renew_token,
client::{encryption_settings::EncryptionSettings, login_method::LoginMethod},
error::{Result, VaultLocked},
DeviceType,
};
#[cfg(feature = "internal")]
use crate::{
client::encryption_settings::EncryptionSettingsError,
client::{flags::Flags, login_method::UserLoginMethod},
error::Error,
};

#[derive(Debug, Clone)]
pub struct ApiConfigurations {
Expand Down Expand Up @@ -179,7 +181,7 @@ impl InternalClient {
master_key: MasterKey,
user_key: EncString,
private_key: EncString,
) -> Result<()> {
) -> Result<(), EncryptionSettingsError> {
*self
.encryption_settings
.write()
Expand All @@ -197,7 +199,7 @@ impl InternalClient {
&self,
user_key: SymmetricCryptoKey,
private_key: EncString,
) -> Result<()> {
) -> Result<(), EncryptionSettingsError> {
*self
.encryption_settings
.write()
Expand All @@ -214,7 +216,7 @@ impl InternalClient {
pin_key: PinKey,
pin_protected_user_key: EncString,
private_key: EncString,
) -> Result<()> {
) -> Result<(), EncryptionSettingsError> {
let decrypted_user_key = pin_key.decrypt_user_key(pin_protected_user_key)?;
self.initialize_user_crypto_decrypted_key(decrypted_user_key, private_key)
}
Expand Down
7 changes: 7 additions & 0 deletions crates/bitwarden-core/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ use reqwest::StatusCode;
use thiserror::Error;
use validator::ValidationErrors;

#[cfg(feature = "internal")]
use crate::client::encryption_settings::EncryptionSettingsError;

#[derive(Debug, Error)]
pub enum Error {
#[error(transparent)]
Expand Down Expand Up @@ -56,6 +59,10 @@ pub enum Error {

#[error("Internal error: {0}")]
Internal(Cow<'static, str>),

#[cfg(feature = "internal")]
#[error(transparent)]
EncryptionSettings(#[from] EncryptionSettingsError),
}

impl From<String> for Error {
Expand Down
7 changes: 5 additions & 2 deletions crates/bitwarden-core/src/mobile/client_crypto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
use bitwarden_crypto::{AsymmetricEncString, EncString};

use super::crypto::{derive_key_connector, DeriveKeyConnectorRequest};
use crate::Client;
use crate::{client::encryption_settings::EncryptionSettingsError, Client};
#[cfg(feature = "internal")]
use crate::{
error::Result,
Expand All @@ -18,7 +18,10 @@ pub struct ClientCrypto<'a> {
}

impl<'a> ClientCrypto<'a> {
pub async fn initialize_user_crypto(&self, req: InitUserCryptoRequest) -> Result<()> {
pub async fn initialize_user_crypto(
&self,
req: InitUserCryptoRequest,
) -> Result<(), EncryptionSettingsError> {
initialize_user_crypto(self.client, req).await
}

Expand Down
7 changes: 5 additions & 2 deletions crates/bitwarden-core/src/mobile/crypto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use schemars::JsonSchema;
use serde::{Deserialize, Serialize};

use crate::{
client::{LoginMethod, UserLoginMethod},
client::{encryption_settings::EncryptionSettingsError, LoginMethod, UserLoginMethod},
error::{Error, Result},
Client,
};
Expand Down Expand Up @@ -86,7 +86,10 @@ pub enum AuthRequestMethod {
},
}

pub async fn initialize_user_crypto(client: &Client, req: InitUserCryptoRequest) -> Result<()> {
pub async fn initialize_user_crypto(
client: &Client,
req: InitUserCryptoRequest,
) -> Result<(), EncryptionSettingsError> {
use bitwarden_crypto::{DeviceKey, PinKey};

use crate::auth::{auth_request_decrypt_master_key, auth_request_decrypt_user_key};
Expand Down
17 changes: 13 additions & 4 deletions crates/bitwarden-uniffi/src/crypto.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
use std::sync::Arc;

use bitwarden::mobile::crypto::{
DeriveKeyConnectorRequest, DerivePinKeyResponse, InitOrgCryptoRequest, InitUserCryptoRequest,
UpdatePasswordResponse,
use bitwarden::{
mobile::crypto::{
DeriveKeyConnectorRequest, DerivePinKeyResponse, InitOrgCryptoRequest,
InitUserCryptoRequest, UpdatePasswordResponse,
},
Error,
};
use bitwarden_crypto::{AsymmetricEncString, EncString};

Expand All @@ -16,7 +19,13 @@ impl ClientCrypto {
/// Initialization method for the user crypto. Needs to be called before any other crypto
/// operations.
pub async fn initialize_user_crypto(&self, req: InitUserCryptoRequest) -> Result<()> {
Ok(self.0 .0.crypto().initialize_user_crypto(req).await?)
Ok(self
.0
.0
.crypto()
.initialize_user_crypto(req)
.await
.map_err(Error::EncryptionSettings)?)
}

/// Initialization method for the organization crypto. Needs to be called after
Expand Down

0 comments on commit 80a7d84

Please sign in to comment.