refactor: errors per module [WPB-14354]

crypto/src/e2e_identity/enabled.rs

@@ -1,8 +1,9 @@
 //! Utility for clients to get the current state of E2EI when the app resumes
 
-use super::error::Result;
+use super::error::{Error, Result};
 use crate::context::CentralContext;
-use crate::prelude::{Client, CryptoError, MlsCentral, MlsCredentialType};
+use crate::mls;
+use crate::prelude::{Client, MlsCentral, MlsCredentialType};
 use openmls_traits::types::SignatureScheme;
 
 impl CentralContext {
@@ -26,12 +27,15 @@ impl Client {
             .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.into()),
+            Err(e) => Err(Error::mls_client("finding most recent x509 credential bundle")(e)),
             Ok(_) => Ok(true),
         }
     }

crypto/src/e2e_identity/error.rs

@@ -75,6 +75,15 @@ pub enum Error {
     /// 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.
@@ -88,3 +97,9 @@ impl From<crate::CryptoError> for Error {
         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 }
+    }
+}

crypto/src/e2e_identity/rotate.rs

@@ -149,15 +149,17 @@ impl CentralContext {
                 cs.signature_algorithm(),
                 cert_bundle,
             )
-            .await?;
+            .await
+            .map_err(Error::mls_client("saving new x509 credential bundle"))?;
 
         let commits = self.e2ei_update_all(client, &new_cb).await?;
 
         let key_package_refs_to_remove = self.find_key_packages_to_remove(&new_cb).await?;
 
         let new_key_packages = client
             .generate_new_keypackages(&self.mls_provider().await?, cs, &new_cb, new_key_packages_count)
-            .await?;
+            .await
+            .map_err(Error::mls_client("generating new keypackages"))?;
 
         Ok(MlsRotateBundle {
             commits,

crypto/src/error.rs

@@ -246,6 +246,9 @@ pub enum CryptoError {
     /// Invalid Context. This context has been finished and can no longer be used.
     #[error("This context has already been finished and can no longer be used.")]
     InvalidContext,
+    /// Something happened in the MLS client code
+    #[error(transparent)]
+    MlsClient(#[from] crate::mls::client::error::Error),
 }
 
 impl From<MlsError> for CryptoError {

crypto/src/mls/client/error.rs

@@ -2,9 +2,117 @@
 
 pub type Result<T, E = Error> = core::result::Result<T, E>;
 
-/// MLS errors
+/// MLS client errors
 #[derive(Debug, thiserror::Error)]
 pub enum Error {
-    #[error("Supplied User Id was not valid")]
+    #[error("Supplied user id was not valid")]
     InvalidUserId,
+    #[error("X509 certificate bundle set was empty")]
+    NoX509CertificateBundle,
+    /// Tried to insert an already existing CredentialBundle
+    #[error("Tried to insert an already existing CredentialBundle")]
+    CredentialBundleConflict,
+    /// A MLS operation was requested but MLS hasn't been initialized on this instance
+    #[error("A MLS operation was requested but MLS hasn't been initialized on this instance")]
+    MlsNotInitialized,
+    /// A Credential was not found locally which is very likely an implementation error
+    #[error("A Credential of type {0:?} was not found locally which is very likely an implementation error")]
+    CredentialNotFound(crate::prelude::MlsCredentialType),
+    /// A key store operation failed
+    //
+    // This uses a `Box<dyn>` pattern because we do not directly import `keystore` from here right now,
+    // and it feels a bit silly to add the dependency only for this.
+    #[error("{context}")]
+    Keystore {
+        #[source]
+        source: Box<dyn std::error::Error + Send + Sync>,
+        context: &'static str,
+    },
+    /// Serializing an item for tls
+    #[error("Serializing {item} for TLS")]
+    TlsSerialize {
+        item: &'static str,
+        #[source]
+        source: tls_codec::Error,
+    },
+    /// Deserializing an item for tls
+    #[error("Deserializing {item} for TLS")]
+    TlsDeserialize {
+        item: &'static str,
+        #[source]
+        source: tls_codec::Error,
+    },
+    /// Keypackage list was empty
+    #[error("Keypackage list was empty")]
+    EmptyKeypackageList,
+    /// Computing Keypackage Hashref
+    #[error("Computing keypackage hashref")]
+    ComputeKeypackageHashref(#[source] openmls::error::LibraryError),
+    /// The keystore has no knowledge of such client; this shouldn't happen as Client::init is failsafe (find-else-create)
+    #[error("The provided client signature has not been found in the keystore")]
+    ClientSignatureNotFound,
+    /// Client was unexpectedly ready.
+    ///
+    /// This indicates an invalid calling pattern.
+    #[error("Client was unexpectedly ready")]
+    UnexpectedlyReady,
+    /// The keystore already has a stored identity. As such, we cannot create a new raw identity
+    #[error("The keystore already contains a stored identity. Cannot create a new one!")]
+    IdentityAlreadyPresent,
+    /// Generating random client id
+    #[error("Generating random client id")]
+    GenerateRandomClientId(#[source] mls_crypto_provider::MlsProviderError),
+    /// This error occurs when we cannot find any provisional keypair in the store, indicating that the `generate_raw_keypair` method hasn't been called.
+    #[error(
+        r#"The externally-generated client ID initialization cannot continue - there's no provisional keypair in-store!
+
+        Have you called `CoreCrypto::generate_raw_keypair` ?"#
+    )]
+    NoProvisionalIdentityFound,
+    /// This error occurs when during the MLS external client generation, we end up with more than one client identity in store.
+    ///
+    /// This is usually not possible, unless there's some kind of concurrency issue
+    /// on the consumer (creating an ext-gen client AND a normal one at the same time for instance)
+    #[error(
+        "Somehow CoreCrypto holds more than one MLS identity. Something might've gone very wrong with this client!"
+    )]
+    TooManyIdentitiesPresent,
+    /// The supplied credential does not match the id or signature schemes provided.
+    #[error("The supplied credential does not match the id or signature schemes provided")]
+    WrongCredential,
+    /// Generating signature keypair
+    #[error("Generating signature keypair")]
+    GeneratingSignatureKeypair(#[source] openmls_traits::types::CryptoError),
+    /// 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 keystore<E>(context: &'static str) -> impl FnOnce(E) -> Self
+    where
+        E: 'static + std::error::Error + Send + Sync,
+    {
+        move |err| Self::Keystore {
+            context,
+            source: Box::new(err),
+        }
+    }
+
+    pub(crate) fn tls_serialize(item: &'static str) -> impl FnOnce(tls_codec::Error) -> Self {
+        move |source| Self::TlsSerialize { item, source }
+    }
+
+    pub(crate) fn tls_deserialize(item: &'static str) -> impl FnOnce(tls_codec::Error) -> Self {
+        move |source| Self::TlsDeserialize { item, source }
+    }
 }

crypto/src/mls/client/id.rs

@@ -14,7 +14,7 @@
 // You should have received a copy of the GNU General Public License
 // along with this program. If not, see http://www.gnu.org/licenses/.
 
-use crate::CryptoError;
+use super::error::Error;
 
 /// A Client identifier
 ///
@@ -69,7 +69,7 @@ impl std::fmt::Display for ClientId {
 }
 
 impl std::str::FromStr for ClientId {
-    type Err = CryptoError;
+    type Err = Error;
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         Ok(Self(

crypto/src/mls/client/identifier.rs

@@ -1,8 +1,8 @@
-use super::CredentialBundle;
-use crate::{
-    prelude::CryptoError,
-    prelude::{CertificateBundle, Client, ClientId, CryptoResult},
+use super::{
+    error::{Error, Result},
+    CredentialBundle,
 };
+use crate::prelude::{CertificateBundle, Client, ClientId};
 use mls_crypto_provider::MlsCryptoProvider;
 use openmls_traits::types::SignatureScheme;
 use std::collections::{HashMap, HashSet};
@@ -20,14 +20,14 @@ pub enum ClientIdentifier {
 impl ClientIdentifier {
     /// Extract the unique [ClientId] from an identifier. Use with parsimony as, in case of a x509
     /// certificate this leads to parsing the certificate
-    pub fn get_id(&self) -> CryptoResult<std::borrow::Cow<ClientId>> {
+    pub fn get_id(&self) -> Result<std::borrow::Cow<ClientId>> {
         match self {
             ClientIdentifier::Basic(id) => Ok(std::borrow::Cow::Borrowed(id)),
             ClientIdentifier::X509(certs) => {
                 // since ClientId has uniqueness constraints, it is the same for all certificates.
                 // hence no need to compute it for every certificate then verify its uniqueness
                 // that's not a getter's job
-                let cert = certs.values().next().ok_or(CryptoError::ImplementationError)?;
+                let cert = certs.values().next().ok_or(Error::NoX509CertificateBundle)?;
                 let id = cert.get_client_id()?;
                 Ok(std::borrow::Cow::Owned(id))
             }
@@ -40,11 +40,11 @@ impl ClientIdentifier {
         self,
         backend: &MlsCryptoProvider,
         signature_schemes: HashSet<SignatureScheme>,
-    ) -> CryptoResult<Vec<(SignatureScheme, ClientId, CredentialBundle)>> {
+    ) -> Result<Vec<(SignatureScheme, ClientId, CredentialBundle)>> {
         match self {
             ClientIdentifier::Basic(id) => signature_schemes.iter().try_fold(
                 Vec::with_capacity(signature_schemes.len()),
-                |mut acc, &sc| -> CryptoResult<_> {
+                |mut acc, &sc| -> Result<_> {
                     let cb = Client::new_basic_credential_bundle(&id, sc, backend)?;
                     acc.push((sc, id.clone(), cb));
                     Ok(acc)
@@ -54,7 +54,7 @@ impl ClientIdentifier {
                 let cap = certs.len();
                 certs
                     .into_iter()
-                    .try_fold(Vec::with_capacity(cap), |mut acc, (sc, cert)| -> CryptoResult<_> {
+                    .try_fold(Vec::with_capacity(cap), |mut acc, (sc, cert)| -> Result<_> {
                         let id = cert.get_client_id()?;
                         let cb = Client::new_x509_credential_bundle(cert)?;
                         acc.push((sc, id, cb));