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

refactor: errors per module [WPB-14354] #791

Merged
merged 31 commits into from
Dec 20, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
4d5244b
build(mls/client): byte literals exist so that humans do not have to …
coriolinus Nov 27, 2024
d548298
refactor!(mls): simplify `get_conversation` api
coriolinus Nov 27, 2024
807a9d1
refactor!(e2e_identity): module uses a module-specific error type
coriolinus Nov 27, 2024
c4a2b1c
refactor!(mls/client): client module uses a module-internal error type
coriolinus Nov 28, 2024
a782d53
refactor!(mls/conversation): conversation module uses a module-intern…
coriolinus Nov 29, 2024
2361365
refactor!(mls/credential): credential module uses a module-internal e…
coriolinus Nov 29, 2024
37730a7
refactor!(mls): mls root module uses a module-internal error type
coriolinus Nov 29, 2024
3aa7e84
refactor!(core-crypto): root module uses module-internal error type
coriolinus Dec 3, 2024
5d0541a
refactor!(errors): reexport each from module root
coriolinus Dec 4, 2024
b0dbc11
refactor(errors): extract `LeafError` to consolidate error leaves fro…
coriolinus Dec 4, 2024
7dd921f
refactor!(errors): centralize recursive error definitions to simplify
coriolinus Dec 4, 2024
a612ad8
refactor!(errors): centralize `KeystoreError`
coriolinus Dec 4, 2024
373ac0a
refactor(errors): better organize top-level errors
coriolinus Dec 5, 2024
4bd521a
fix(errors): make core-crypto compile again for `wasm32-unknown-unknown`
coriolinus Dec 5, 2024
1a083e3
refactor(errors): move `error::Error::ConversationNotFound` into `Lea…
coriolinus Dec 5, 2024
b9a9190
refactor(e2e_identity/errors): audit error type and simplify where po…
coriolinus Dec 5, 2024
cb7821c
refactor(mls/client/errors): audit error type and simplify where poss…
coriolinus Dec 5, 2024
17e4e89
refactor(mls/conversation/errors): audit error type and simplify wher…
coriolinus Dec 5, 2024
5b68c75
partial(errors): get some core-crypto test cases passing again
coriolinus Dec 6, 2024
65f672b
fix(errors): fix remaining failing test cases
coriolinus Dec 9, 2024
81dd49a
refactor(crypto-ffi): `core-crypto-ffi` builds given new errors
coriolinus Dec 12, 2024
7fdeeb4
fix: fix `check` ci action
coriolinus Dec 13, 2024
d0db260
fix: make leaf node validation tests pass
coriolinus Dec 13, 2024
15cbf13
build: reduce visibility of unreachable pub items
coriolinus Dec 13, 2024
e1d6dc5
fix: cause doctests to build/pass
coriolinus Dec 13, 2024
eab63e3
fix: ensure everything still builds in arbitray feature combinations
coriolinus Dec 13, 2024
4181deb
fix(kotlin): cause kotlin to build again
coriolinus Dec 13, 2024
a61739b
chore: rm legacy error type
coriolinus Dec 18, 2024
c84da0b
chore: typos and vocab fixes
coriolinus Dec 18, 2024
8cd3806
fixup: `innermost_source_matches` can handle dereferencing boxed errors
coriolinus Dec 18, 2024
e93cee6
chore: remove error variants already removed by `c5482e5`
coriolinus Dec 18, 2024
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
6 changes: 3 additions & 3 deletions crypto-ffi/src/generic/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ use crate::UniffiCustomTypeConverter;
pub use core_crypto::prelude::ConversationId;
use core_crypto::{
prelude::{
ClientIdentifier, CryptoError, E2eIdentityError, EntropySeed, KeyPackageIn, KeyPackageRef,
ClientIdentifier, CryptoError, EntropySeed, Error, KeyPackageIn, KeyPackageRef,
MlsBufferedConversationDecryptMessage, MlsCentral, MlsCentralConfiguration, MlsCiphersuite, MlsCommitBundle,
MlsConversationConfiguration, MlsConversationCreationMessage, MlsConversationDecryptMessage,
MlsConversationInitBundle, MlsCustomConfiguration, MlsGroupInfoBundle, MlsProposalBundle, MlsRotateBundle,
Expand Down Expand Up @@ -223,8 +223,8 @@ impl From<CryptoError> for CoreCryptoError {
}
}

impl From<E2eIdentityError> for CoreCryptoError {
fn from(e: E2eIdentityError) -> Self {
impl From<Error> for CoreCryptoError {
fn from(e: Error) -> Self {
Self::E2eiError(e.to_string())
}
}
Expand Down
35 changes: 12 additions & 23 deletions crypto/src/e2e_identity/conversation_state.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use crate::{
mls::credential::ext::CredentialExt,
prelude::{ConversationId, CryptoResult, MlsCentral, MlsConversation, MlsCredentialType},
MlsError,
prelude::{ConversationId, MlsCentral, MlsConversation, MlsCredentialType},
};

use mls_crypto_provider::MlsCryptoProvider;
Expand All @@ -16,6 +15,8 @@ use openmls::{
treesync::RatchetTree,
};

use super::error::Result;

/// Indicates the state of a Conversation regarding end-to-end identity.
///
/// Note: this does not check pending state (pending commit, pending proposals) so it does not
Expand All @@ -34,7 +35,7 @@ pub enum E2eiConversationState {
impl CentralContext {
/// Indicates when to mark a conversation as not verified i.e. when not all its members have a X509
/// Credential generated by Wire's end-to-end identity enrollment
pub async fn e2ei_conversation_state(&self, id: &ConversationId) -> CryptoResult<E2eiConversationState> {
pub async fn e2ei_conversation_state(&self, id: &ConversationId) -> Result<E2eiConversationState> {
let conversation = self.get_conversation(id).await?;
let conversation_guard = conversation.read().await;
conversation_guard
Expand All @@ -43,10 +44,7 @@ impl CentralContext {
}

/// See [MlsCentral::e2ei_verify_group_state].
pub async fn e2ei_verify_group_state(
&self,
group_info: VerifiableGroupInfo,
) -> CryptoResult<E2eiConversationState> {
pub async fn e2ei_verify_group_state(&self, group_info: VerifiableGroupInfo) -> Result<E2eiConversationState> {
let mls_provider = self.mls_provider().await?;
let auth_service = mls_provider.authentication_service();
auth_service.refresh_time_of_interest().await;
Expand Down Expand Up @@ -74,16 +72,13 @@ impl CentralContext {
&self,
group_info: VerifiableGroupInfo,
credential_type: MlsCredentialType,
) -> CryptoResult<E2eiConversationState> {
) -> Result<E2eiConversationState> {
let cs = group_info.ciphersuite().into();
// Not verifying the supplied the GroupInfo here could let attackers lure the clients about
// the e2ei state of a conversation and as a consequence degrade this conversation for all
// participants once joining it.
// This 👇 verifies the GroupInfo and the RatchetTree btw
let rt = group_info
.take_ratchet_tree(&self.mls_provider().await?, false)
.await
.map_err(MlsError::from)?;
let rt = group_info.take_ratchet_tree(&self.mls_provider().await?, false).await?;
let mls_provider = self.mls_provider().await?;
let auth_service = mls_provider.authentication_service().borrow().await;
get_credential_in_use_in_ratchet_tree(cs, rt, credential_type, auth_service.as_ref()).await
Expand All @@ -92,10 +87,7 @@ impl CentralContext {

impl MlsCentral {
/// Verifies a Group state before joining it
pub async fn e2ei_verify_group_state(
&self,
group_info: VerifiableGroupInfo,
) -> CryptoResult<E2eiConversationState> {
pub async fn e2ei_verify_group_state(&self, group_info: VerifiableGroupInfo) -> Result<E2eiConversationState> {
self.mls_backend
.authentication_service()
.refresh_time_of_interest()
Expand Down Expand Up @@ -128,16 +120,13 @@ impl MlsCentral {
&self,
group_info: VerifiableGroupInfo,
credential_type: MlsCredentialType,
) -> CryptoResult<E2eiConversationState> {
) -> Result<E2eiConversationState> {
let cs = group_info.ciphersuite().into();
// Not verifying the supplied the GroupInfo here could let attackers lure the clients about
// the e2ei state of a conversation and as a consequence degrade this conversation for all
// participants once joining it.
// This 👇 verifies the GroupInfo and the RatchetTree btw
let rt = group_info
.take_ratchet_tree(&self.mls_backend, false)
.await
.map_err(MlsError::from)?;
let rt = group_info.take_ratchet_tree(&self.mls_backend, false).await?;
get_credential_in_use_in_ratchet_tree(
cs,
rt,
Expand All @@ -149,7 +138,7 @@ impl MlsCentral {
}

impl MlsConversation {
async fn e2ei_conversation_state(&self, backend: &MlsCryptoProvider) -> CryptoResult<E2eiConversationState> {
async fn e2ei_conversation_state(&self, backend: &MlsCryptoProvider) -> Result<E2eiConversationState> {
backend.authentication_service().refresh_time_of_interest().await;
Ok(compute_state(
self.ciphersuite(),
Expand All @@ -166,7 +155,7 @@ async fn get_credential_in_use_in_ratchet_tree(
ratchet_tree: RatchetTree,
credential_type: MlsCredentialType,
env: Option<&wire_e2e_identity::prelude::x509::revocation::PkiEnvironment>,
) -> CryptoResult<E2eiConversationState> {
) -> Result<E2eiConversationState> {
let credentials = ratchet_tree.iter().filter_map(|n| match n {
Some(Node::LeafNode(ln)) => Some(ln.credential()),
_ => None,
Expand Down
26 changes: 12 additions & 14 deletions crypto/src/e2e_identity/crypto.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use super::error::*;
use crate::{prelude::MlsCiphersuite, CryptoError, CryptoResult, MlsError};
use crate::prelude::MlsCiphersuite;
use mls_crypto_provider::{MlsCryptoProvider, PkiKeypair, RustCrypto};
use openmls_basic_credential::SignatureKeyPair as OpenMlsSignatureKeyPair;
use openmls_traits::{
Expand All @@ -14,32 +14,32 @@ impl super::E2eiEnrollment {
pub(super) fn new_sign_key(
ciphersuite: MlsCiphersuite,
backend: &MlsCryptoProvider,
) -> CryptoResult<E2eiSignatureKeypair> {
) -> Result<E2eiSignatureKeypair> {
let (sk, _) = backend
.crypto()
.signature_key_gen(ciphersuite.signature_algorithm())
.map_err(MlsError::from)?;
.map_err(Error::SignatureKeyGen)?;
E2eiSignatureKeypair::try_new(ciphersuite.signature_algorithm(), sk)
}

pub(super) fn get_sign_key_for_mls(&self) -> CryptoResult<Vec<u8>> {
pub(super) fn get_sign_key_for_mls(&self) -> Result<Vec<u8>> {
let sk = match self.ciphersuite.signature_algorithm() {
SignatureScheme::ECDSA_SECP256R1_SHA256 | SignatureScheme::ECDSA_SECP384R1_SHA384 => self.sign_sk.to_vec(),
SignatureScheme::ECDSA_SECP521R1_SHA512 => RustCrypto::normalize_p521_secret_key(&self.sign_sk).to_vec(),
SignatureScheme::ED25519 => RustCrypto::normalize_ed25519_key(self.sign_sk.as_slice())
.map_err(MlsError::from)?
.map_err(Error::NormalizingEd25519Key)?
.to_bytes()
.to_vec(),
SignatureScheme::ED448 => return Err(E2eIdentityError::NotYetSupported.into()),
SignatureScheme::ED448 => return Err(Error::NotYetSupported.into()),
};
Ok(sk)
}
}

impl TryFrom<MlsCiphersuite> for JwsAlgorithm {
type Error = E2eIdentityError;
type Error = Error;

fn try_from(cs: MlsCiphersuite) -> E2eIdentityResult<Self> {
fn try_from(cs: MlsCiphersuite) -> Result<Self> {
let cs = openmls_traits::types::Ciphersuite::from(cs);
Ok(match cs {
Ciphersuite::MLS_128_DHKEMX25519_AES128GCM_SHA256_Ed25519
Expand All @@ -48,9 +48,7 @@ impl TryFrom<MlsCiphersuite> for JwsAlgorithm {
Ciphersuite::MLS_256_DHKEMP384_AES256GCM_SHA384_P384 => JwsAlgorithm::P384,
Ciphersuite::MLS_256_DHKEMP521_AES256GCM_SHA512_P521 => JwsAlgorithm::P521,
Ciphersuite::MLS_256_DHKEMX448_AES256GCM_SHA512_Ed448
| Ciphersuite::MLS_256_DHKEMX448_CHACHA20POLY1305_SHA512_Ed448 => {
return Err(E2eIdentityError::NotYetSupported)
}
| Ciphersuite::MLS_256_DHKEMX448_CHACHA20POLY1305_SHA512_Ed448 => return Err(Error::NotYetSupported),
})
}
}
Expand All @@ -60,16 +58,16 @@ impl TryFrom<MlsCiphersuite> for JwsAlgorithm {
pub struct E2eiSignatureKeypair(Vec<u8>);

impl E2eiSignatureKeypair {
pub fn try_new(sc: SignatureScheme, sk: Vec<u8>) -> CryptoResult<Self> {
pub fn try_new(sc: SignatureScheme, sk: Vec<u8>) -> Result<Self> {
let keypair = PkiKeypair::new(sc, sk)?;
Ok(Self(keypair.signing_key_bytes()))
}
}

impl TryFrom<&OpenMlsSignatureKeyPair> for E2eiSignatureKeypair {
type Error = CryptoError;
type Error = Error;

fn try_from(kp: &OpenMlsSignatureKeyPair) -> CryptoResult<Self> {
fn try_from(kp: &OpenMlsSignatureKeyPair) -> Result<Self> {
Self::try_new(kp.signature_scheme(), kp.private().to_vec())
}
}
27 changes: 17 additions & 10 deletions crypto/src/e2e_identity/enabled.rs
Original file line number Diff line number Diff line change
@@ -1,44 +1,49 @@
//! Utility for clients to get the current state of E2EI when the app resumes

use super::error::{Error, Result};
use crate::context::CentralContext;
use crate::prelude::{Client, CryptoError, CryptoResult, MlsCentral, MlsCredentialType};
use crate::mls;
use crate::prelude::{Client, MlsCentral, MlsCredentialType};
use openmls_traits::types::SignatureScheme;

impl CentralContext {
/// See [MlsCentral::e2ei_is_enabled]
pub async fn e2ei_is_enabled(&self, signature_scheme: SignatureScheme) -> CryptoResult<bool> {
pub async fn e2ei_is_enabled(&self, signature_scheme: SignatureScheme) -> Result<bool> {
let client = self.mls_client().await?;
client.e2ei_is_enabled(signature_scheme).await
}
}

impl MlsCentral {
/// Returns true when end-to-end-identity is enabled for the given SignatureScheme
pub async fn e2ei_is_enabled(&self, signature_scheme: SignatureScheme) -> CryptoResult<bool> {
pub async fn e2ei_is_enabled(&self, signature_scheme: SignatureScheme) -> Result<bool> {
self.mls_client.e2ei_is_enabled(signature_scheme).await
}
}

impl Client {
async fn e2ei_is_enabled(&self, signature_scheme: SignatureScheme) -> CryptoResult<bool> {
async fn e2ei_is_enabled(&self, signature_scheme: SignatureScheme) -> Result<bool> {
let x509_result = self
.find_most_recent_credential_bundle(signature_scheme, MlsCredentialType::X509)
.await;
match x509_result {
Err(CryptoError::CredentialNotFound(MlsCredentialType::X509)) => {
Err(mls::client::error::Error::CredentialNotFound(MlsCredentialType::X509)) => {
self.find_most_recent_credential_bundle(signature_scheme, MlsCredentialType::Basic)
.await?;
.await
.map_err(Error::mls_client(
"finding most recent basic credential bundle after searching for x509",
))?;
Ok(false)
}
Err(e) => Err(e),
Err(e) => Err(Error::mls_client("finding most recent x509 credential bundle")(e)),
Ok(_) => Ok(true),
}
}
}

#[cfg(test)]
mod tests {
use crate::{prelude::MlsCredentialType, test_utils::*, CryptoError};
use crate::{e2e_identity::error::Error, prelude::MlsCredentialType, test_utils::*, CryptoError};
use openmls_traits::types::SignatureScheme;
use wasm_bindgen_test::*;

Expand Down Expand Up @@ -66,7 +71,8 @@ mod tests {
Box::pin(async move {
assert!(matches!(
cc.context.e2ei_is_enabled(case.signature_scheme()).await.unwrap_err(),
CryptoError::MlsNotInitialized
Error::CryptoError(boxed)
if matches!(*boxed, CryptoError::MlsNotInitialized)
));
})
})
Expand All @@ -85,7 +91,8 @@ mod tests {
};
assert!(matches!(
cc.context.e2ei_is_enabled(other_sc).await.unwrap_err(),
CryptoError::CredentialNotFound(_)
Error::CryptoError(boxed)
if matches!(*boxed, CryptoError::CredentialNotFound(_))
));
})
})
Expand Down
68 changes: 63 additions & 5 deletions crypto/src/e2e_identity/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,12 @@
use crate::prelude::MlsCredentialType;
use core_crypto_keystore::CryptoKeystoreError;

/// Wrapper over a [Result] of an end to end identity error
pub type E2eIdentityResult<T> = Result<T, E2eIdentityError>;
/// Wrapper over a [Result][core::result::Result] of an end to end identity error
pub type Result<T, E = Error> = core::result::Result<T, E>;

/// End to end identity errors
#[derive(Debug, thiserror::Error)]
#[cfg_attr(feature = "uniffi", derive(uniffi::Error))]
#[cfg_attr(feature = "uniffi", uniffi(flat_error))]
pub enum E2eIdentityError {
pub enum Error {
/// Client misused this library
#[error("Incorrect usage of this API")]
ImplementationError,
Expand Down Expand Up @@ -44,4 +42,64 @@ pub enum E2eIdentityError {
/// We already have an ACME Root Trust Anchor registered. Cannot proceed but this is usually indicative of double registration and can be ignored
#[error("We already have an ACME Root Trust Anchor registered. Cannot proceed but this is usually indicative of double registration and can be ignored")]
TrustAnchorAlreadyRegistered,
/// Getting MLS ratchet tree
#[error("Getting MLS ratchet tree")]
MlsRatchetTree(#[from] openmls::messages::group_info::GroupInfoError),
/// Generating signature key
#[error("Generating signature key")]
SignatureKeyGen(#[source] openmls_traits::types::CryptoError),
#[error("Normalizing ed25519 key")]
/// Normalizing ed25519 key
NormalizingEd25519Key(#[source] openmls_traits::types::CryptoError),
#[error("Generating new PKI keypair")]
/// Generating new PKi keypair
GeneratePkiKeypair(#[from] mls_crypto_provider::MlsProviderError),
/// The encountered ClientId does not match Wire's definition
#[error("The encountered ClientId does not match Wire's definition")]
InvalidClientId,
/// see [`x509_cert::der::Error`]
#[error(transparent)]
X509CertDerError(#[from] x509_cert::der::Error),
/// This function accepts a list of IDs as a parameter, but that list was empty
#[error("This function accepts a list of IDs as a parameter, but that list was empty")]
EmptyInputIdList,
/// Getting user identities
#[error("Getting user identities")]
GetUserIdentities(#[source] crate::mls::client::error::Error),
/// PKI environment must ben set before calling this function
#[error("PKI Environment must be set before calling this function")]
PkiEnvironmentUnset,
/// Computing key package hash ref
#[error("Computing key package hash ref")]
KeyPackageHashRef(#[from] openmls::error::LibraryError),
/// Serializing key package for TLS
#[error("Serializing key package for TLS")]
TlsSerializingKeyPackage(#[from] tls_codec::Error),
/// Something in the MLS client went wrong
#[error("{context}")]
MlsClient {
/// What was happening when the error was thrown
context: &'static str,
/// The inner error which was produced
#[source]
source: crate::mls::client::error::Error,
},
/// Compatibility wrapper
///
/// This should be removed before merging this branch, but it allows an easier migration path to module-specific errors.
#[deprecated]
#[error(transparent)]
CryptoError(Box<crate::CryptoError>),
}

impl From<crate::CryptoError> for Error {
fn from(value: crate::CryptoError) -> Self {
Self::CryptoError(Box::new(value))
}
}

impl Error {
pub(crate) fn mls_client(context: &'static str) -> impl FnOnce(crate::mls::client::error::Error) -> Self {
move |source| Self::MlsClient { context, source }
}
}
Loading