diff --git a/Cargo.lock b/Cargo.lock index 189bd2f86..3fa5707ae 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -307,9 +307,9 @@ checksum = "ace50bade8e6234aa140d9a2f552bbee1db4d353f69b8217bc503490fc1a9f26" [[package]] name = "axum" -version = "0.7.7" +version = "0.7.9" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "504e3947307ac8326a5437504c517c4b56716c9d98fac0028c2acc7ca47d70ae" +checksum = "edca88bc138befd0323b20752846e6587272d3b03b0343c8ea28a6f819e6e71f" dependencies = [ "async-trait", "axum-core", @@ -639,9 +639,9 @@ checksum = "37b2a672a2cb129a2e41c10b1224bb368f9f37a2b16b612598138befd7b37eb5" [[package]] name = "cc" -version = "1.2.0" +version = "1.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1aeb932158bd710538c73702db6945cb68a8fb08c519e6e12706b94263b36db8" +checksum = "fd9de9f2205d5ef3fd67e685b0df337994ddd4495e2a28d185500d0e1edfea47" dependencies = [ "jobserver", "libc", @@ -732,9 +732,9 @@ dependencies = [ [[package]] name = "clap" -version = "4.5.20" +version = "4.5.21" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b97f376d85a664d5837dbae44bf546e6477a679ff6610010f17276f686d867e8" +checksum = "fb3b4b9e5a7c7514dfa52869339ee98b3156b0bfb4e8a77c4ff4babb64b1604f" dependencies = [ "clap_builder", "clap_derive", @@ -742,9 +742,9 @@ dependencies = [ [[package]] name = "clap_builder" -version = "4.5.20" +version = "4.5.21" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "19bc80abd44e4bed93ca373a0704ccbd1b710dc5749406201bb018272808dc54" +checksum = "b17a95aa67cc7b5ebd32aa5370189aa0d79069ef1c64ce893bd30fb24bff20ec" dependencies = [ "anstream", "anstyle", @@ -766,9 +766,9 @@ dependencies = [ [[package]] name = "clap_lex" -version = "0.7.2" +version = "0.7.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1462739cb27611015575c0c11df5df7601141071f07518d56fcc1be504cbec97" +checksum = "afb84c814227b90d6895e01398aee0d8033c00e7466aca416fb6a8e0eb19d8a7" [[package]] name = "coins-bip32" @@ -1210,7 +1210,7 @@ dependencies = [ [[package]] name = "diesel" version = "2.2.4" -source = "git+https://github.com/diesel-rs/diesel?branch=master#7e55d407f053f03bd9e074616c2f21d7df1038e8" +source = "git+https://github.com/diesel-rs/diesel?branch=master#5a2940eeaa935c0cc0a7fc57ffe012f3aaa80f43" dependencies = [ "diesel_derives", "downcast-rs", @@ -1240,7 +1240,7 @@ dependencies = [ [[package]] name = "diesel_derives" version = "2.2.0" -source = "git+https://github.com/diesel-rs/diesel?branch=master#7e55d407f053f03bd9e074616c2f21d7df1038e8" +source = "git+https://github.com/diesel-rs/diesel?branch=master#5a2940eeaa935c0cc0a7fc57ffe012f3aaa80f43" dependencies = [ "diesel_table_macro_syntax", "dsl_auto_type", @@ -1252,7 +1252,7 @@ dependencies = [ [[package]] name = "diesel_migrations" version = "2.2.0" -source = "git+https://github.com/diesel-rs/diesel?branch=master#7e55d407f053f03bd9e074616c2f21d7df1038e8" +source = "git+https://github.com/diesel-rs/diesel?branch=master#5a2940eeaa935c0cc0a7fc57ffe012f3aaa80f43" dependencies = [ "diesel", "migrations_internals", @@ -1262,7 +1262,7 @@ dependencies = [ [[package]] name = "diesel_table_macro_syntax" version = "0.2.0" -source = "git+https://github.com/diesel-rs/diesel?branch=master#7e55d407f053f03bd9e074616c2f21d7df1038e8" +source = "git+https://github.com/diesel-rs/diesel?branch=master#5a2940eeaa935c0cc0a7fc57ffe012f3aaa80f43" dependencies = [ "syn 2.0.87", ] @@ -1356,7 +1356,7 @@ checksum = "75b325c5dbd37f80359721ad39aca5a29fb04c89279657cffdda8736d0c0b9d2" [[package]] name = "dsl_auto_type" version = "0.1.0" -source = "git+https://github.com/diesel-rs/diesel?branch=master#7e55d407f053f03bd9e074616c2f21d7df1038e8" +source = "git+https://github.com/diesel-rs/diesel?branch=master#5a2940eeaa935c0cc0a7fc57ffe012f3aaa80f43" dependencies = [ "darling", "either", @@ -1916,9 +1916,9 @@ checksum = "0ce7134b9999ecaf8bcd65542e436736ef32ddca1b3e06094cb6ec5755203b80" [[package]] name = "flate2" -version = "1.0.34" +version = "1.0.35" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a1b589b4dc103969ad3cf85c950899926ec64300a1a46d76c03a6072957036f0" +checksum = "c936bfdafb507ebbf50b8074c54fa31c5be9a1e7e5f467dd659697041407d07c" dependencies = [ "crc32fast", "miniz_oxide 0.8.0", @@ -2576,7 +2576,7 @@ dependencies = [ "http 1.1.0", "hyper 1.5.0", "hyper-util", - "rustls 0.23.16", + "rustls 0.23.17", "rustls-pki-types", "tokio", "tokio-rustls 0.26.0", @@ -3082,9 +3082,9 @@ checksum = "bbd2bcb4c963f2ddae06a2efc7e9f3591312473c50c6685e1f298068316e66fe" [[package]] name = "libc" -version = "0.2.162" +version = "0.2.164" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "18d287de67fe55fd7e1581fe933d965a5a9477b38e949cfa9f8574ef01506398" +checksum = "433bfe06b8c75da9b2e3fbea6e5329ff87748f0b144ef75306e674c3f6f7c13f" [[package]] name = "libgit2-sys" @@ -3105,7 +3105,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4979f22fdb869068da03c9f7528f8297c6fd2606bc3a4affe42e6a823fdb8da4" dependencies = [ "cfg-if", - "windows-targets 0.52.6", + "windows-targets 0.48.5", ] [[package]] @@ -3258,7 +3258,7 @@ checksum = "78ca9ab1a0babb1e7d5695e3530886289c18cf2f87ec19a575a0abdce112e3a3" [[package]] name = "migrations_internals" version = "2.2.0" -source = "git+https://github.com/diesel-rs/diesel?branch=master#7e55d407f053f03bd9e074616c2f21d7df1038e8" +source = "git+https://github.com/diesel-rs/diesel?branch=master#5a2940eeaa935c0cc0a7fc57ffe012f3aaa80f43" dependencies = [ "serde", "toml 0.8.19", @@ -3267,7 +3267,7 @@ dependencies = [ [[package]] name = "migrations_macros" version = "2.2.0" -source = "git+https://github.com/diesel-rs/diesel?branch=master#7e55d407f053f03bd9e074616c2f21d7df1038e8" +source = "git+https://github.com/diesel-rs/diesel?branch=master#5a2940eeaa935c0cc0a7fc57ffe012f3aaa80f43" dependencies = [ "migrations_internals", "proc-macro2", @@ -3776,9 +3776,9 @@ checksum = "ff011a302c396a5197692431fc1948019154afc178baf7d8e37367442a4601cf" [[package]] name = "openssl-src" -version = "300.4.0+3.4.0" +version = "300.4.1+3.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a709e02f2b4aca747929cca5ed248880847c650233cf8b8cdc48f40aaf4898a6" +checksum = "faa4eac4138c62414b5622d1b31c5c304f34b406b013c079c2bbc652fdd6678c" dependencies = [ "cc", ] @@ -3844,28 +3844,29 @@ dependencies = [ [[package]] name = "parity-scale-codec" -version = "3.6.12" +version = "3.7.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "306800abfa29c7f16596b5970a588435e3d5b3149683d00c12b699cc19f895ee" +checksum = "8be4817d39f3272f69c59fe05d0535ae6456c2dc2fa1ba02910296c7e0a5c590" dependencies = [ "arrayvec", "bitvec", "byte-slice-cast", "impl-trait-for-tuples", "parity-scale-codec-derive", + "rustversion", "serde", ] [[package]] name = "parity-scale-codec-derive" -version = "3.6.12" +version = "3.7.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d830939c76d294956402033aee57a6da7b438f2294eb94864c37b0569053a42c" +checksum = "8781a75c6205af67215f382092b6e0a4ff3734798523e69073d4bcd294ec767b" dependencies = [ "proc-macro-crate", "proc-macro2", "quote", - "syn 1.0.109", + "syn 2.0.87", ] [[package]] @@ -4346,7 +4347,7 @@ checksum = "0c1318b19085f08681016926435853bbf7858f9c082d0999b80550ff5d9abe15" dependencies = [ "bytes", "heck", - "itertools 0.13.0", + "itertools 0.10.5", "log", "multimap", "once_cell", @@ -4366,7 +4367,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e9552f850d5f0964a4e4d0bf306459ac29323ddfbae05e35a7c0d35cb0803cc5" dependencies = [ "anyhow", - "itertools 0.13.0", + "itertools 0.10.5", "proc-macro2", "quote", "syn 2.0.87", @@ -4733,9 +4734,9 @@ dependencies = [ [[package]] name = "rustix" -version = "0.38.40" +version = "0.38.41" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "99e4ea3e1cdc4b559b8e5650f9c8e5998e3e5c1343b4eaf034565f32318d63c0" +checksum = "d7f649912bc1495e167a6edee79151c84b1bad49748cb4f1f1167f459f6224f6" dependencies = [ "bitflags 2.6.0", "errno", @@ -4758,9 +4759,9 @@ dependencies = [ [[package]] name = "rustls" -version = "0.23.16" +version = "0.23.17" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "eee87ff5d9b36712a58574e12e9f0ea80f915a5b0ac518d322b24a465617925e" +checksum = "7f1a745511c54ba6d4465e8d5dfbd81b45791756de28d4981af70d6dca128f1e" dependencies = [ "log", "once_cell", @@ -5725,7 +5726,7 @@ version = "0.26.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0c7bc40d0e5a97695bb96e27995cd3a08538541b0a846f65bba7a359f36700d4" dependencies = [ - "rustls 0.23.16", + "rustls 0.23.17", "rustls-pki-types", "tokio", ] @@ -6149,9 +6150,9 @@ checksum = "ebc1c04c71510c7f702b52b7c350734c9ff1295c464a03335b00bb84fc54f853" [[package]] name = "uniffi" -version = "0.28.2" +version = "0.28.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "51ce6280c581045879e11b400bae14686a819df22b97171215d15549efa04ddb" +checksum = "4cb08c58c7ed7033150132febe696bef553f891b1ede57424b40d87a89e3c170" dependencies = [ "anyhow", "cargo_metadata 0.15.4", @@ -6163,9 +6164,9 @@ dependencies = [ [[package]] name = "uniffi_bindgen" -version = "0.28.2" +version = "0.28.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5e9f25730c9db2e878521d606f54e921edb719cdd94d735e7f97705d6796d024" +checksum = "cade167af943e189a55020eda2c314681e223f1e42aca7c4e52614c2b627698f" dependencies = [ "anyhow", "askama", @@ -6187,9 +6188,9 @@ dependencies = [ [[package]] name = "uniffi_build" -version = "0.28.2" +version = "0.28.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "88dba57ac699bd8ec53d6a352c8dd0e479b33f698c5659831bb1e4ce468c07bd" +checksum = "4c7cf32576e08104b7dc2a6a5d815f37616e66c6866c2a639fe16e6d2286b75b" dependencies = [ "anyhow", "camino", @@ -6198,9 +6199,9 @@ dependencies = [ [[package]] name = "uniffi_checksum_derive" -version = "0.28.2" +version = "0.28.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d2c801f0f05b06df456a2da4c41b9c2c4fdccc6b9916643c6c67275c4c9e4d07" +checksum = "802d2051a700e3ec894c79f80d2705b69d85844dafbbe5d1a92776f8f48b563a" dependencies = [ "quote", "syn 2.0.87", @@ -6208,9 +6209,9 @@ dependencies = [ [[package]] name = "uniffi_core" -version = "0.28.2" +version = "0.28.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "61049e4db6212d0ede80982adf0e1d6fa224e6118387324c5cfbe3083dfb2252" +checksum = "bc7687007d2546c454d8ae609b105daceb88175477dac280707ad6d95bcd6f1f" dependencies = [ "anyhow", "async-compat", @@ -6223,9 +6224,9 @@ dependencies = [ [[package]] name = "uniffi_macros" -version = "0.28.2" +version = "0.28.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b40fd2249e0c5dcbd2bfa3c263db1ec981f7273dca7f4132bf06a272359a586c" +checksum = "12c65a5b12ec544ef136693af8759fb9d11aefce740fb76916721e876639033b" dependencies = [ "bincode", "camino", @@ -6241,9 +6242,9 @@ dependencies = [ [[package]] name = "uniffi_meta" -version = "0.28.2" +version = "0.28.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c9ad57039b4fafdbf77428d74fff40e0908e5a1731e023c19cfe538f6d4a8ed6" +checksum = "4a74ed96c26882dac1ca9b93ca23c827e284bacbd7ec23c6f0b0372f747d59e4" dependencies = [ "anyhow", "bytes", @@ -6253,9 +6254,9 @@ dependencies = [ [[package]] name = "uniffi_testing" -version = "0.28.2" +version = "0.28.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "21fa171d4d258dc51bbd01893cc9608c1b62273d2f9ea55fb64f639e77824567" +checksum = "6a6f984f0781f892cc864a62c3a5c60361b1ccbd68e538e6c9fbced5d82268ac" dependencies = [ "anyhow", "camino", @@ -6266,9 +6267,9 @@ dependencies = [ [[package]] name = "uniffi_udl" -version = "0.28.2" +version = "0.28.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f52299e247419e7e2934bef2f94d7cccb0e6566f3248b1d48b160d8f369a2668" +checksum = "037820a4cfc4422db1eaa82f291a3863c92c7d1789dc513489c36223f9b4cdfc" dependencies = [ "anyhow", "textwrap", @@ -6651,7 +6652,7 @@ version = "0.1.9" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cf221c93e13a30d793f7645a0e7762c55d169dbb0a49671918a2319d289b10bb" dependencies = [ - "windows-sys 0.59.0", + "windows-sys 0.48.0", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 1c0f868f1..94eb89f26 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -60,11 +60,13 @@ tls_codec = "0.4.1" tokio = { version = "1.35.1", default-features = false } # Changing this version and rustls may potentially break the android build. Use Caution. # Test with Android and Swift first. -# Its probably preferable to one day use https://github.com/rustls/rustls-native-certs +# Its probably preferable to one day use https://github.com/rustls/rustls-platform-verifier # Until then, always test agains iOS/Android after updating these dependencies & making a PR -tonic = { version = "0.12.3", features = ["tls", "tls-native-roots", "tls-webpki-roots"] } -rustls = { version = "=0.23.7", features = ["ring"] } -# Pinned Dependencies +# Related Issues: +# - https://github.com/seanmonstar/reqwest/issues/2159 +# - https://github.com/hyperium/tonic/pull/1974 +# - https://github.com/rustls/rustls-platform-verifier/issues/58 +tonic = { version = "0.12", default-features = false } tracing = { version = "0.1", features = ["log", "release_max_level_debug"] } tracing-subscriber = { version = "0.3", default-features = false } diesel = { version = "2.2", default-features = false } @@ -136,4 +138,3 @@ diesel-wasm-sqlite = { git = "https://github.com/xmtp/diesel-wasm-sqlite", branc diesel = { git = "https://github.com/diesel-rs/diesel", branch = "master" } diesel_derives = { git = "https://github.com/diesel-rs/diesel", branch = "master" } diesel_migrations = { git = "https://github.com/diesel-rs/diesel", branch = "master" } - diff --git a/bindings_ffi/src/lib.rs b/bindings_ffi/src/lib.rs index 34c7842c2..3d16186be 100755 --- a/bindings_ffi/src/lib.rs +++ b/bindings_ffi/src/lib.rs @@ -48,6 +48,8 @@ pub enum GenericError { Association(#[from] xmtp_id::associations::AssociationError), #[error(transparent)] DeviceSync(#[from] xmtp_mls::groups::device_sync::DeviceSyncError), + #[error(transparent)] + Identity(#[from] xmtp_mls::identity::IdentityError), } #[derive(uniffi::Error, thiserror::Error, Debug)] diff --git a/bindings_ffi/src/mls.rs b/bindings_ffi/src/mls.rs index 3eb0c74c9..62d1cfaf9 100644 --- a/bindings_ffi/src/mls.rs +++ b/bindings_ffi/src/mls.rs @@ -364,6 +364,13 @@ impl FfiXmtpClient { Ok(result.into()) } + + pub fn sign_with_installation_key(&self, text: &str) -> Result, GenericError> { + let inner = self.inner_client.as_ref(); + let context = inner.context().public_sign(text)?; + + Ok(context) + } } #[uniffi::export(async_runtime = "tokio")] diff --git a/bindings_node/src/client.rs b/bindings_node/src/client.rs index 296da6480..6ae6a8e25 100644 --- a/bindings_node/src/client.rs +++ b/bindings_node/src/client.rs @@ -289,4 +289,15 @@ impl Client { .map_err(ErrorWrapper::from)?; Ok(state.into_iter().map(Into::into).collect()) } + + #[napi] + pub fn sign_with_installation_key(&self, text: String) -> Result> { + let result = self + .inner_client + .context() + .public_sign(text) + .map_err(ErrorWrapper::from)?; + + Ok(result) + } } diff --git a/xmtp_api_grpc/Cargo.toml b/xmtp_api_grpc/Cargo.toml index b45b56940..7c4ebee2c 100644 --- a/xmtp_api_grpc/Cargo.toml +++ b/xmtp_api_grpc/Cargo.toml @@ -12,15 +12,24 @@ futures.workspace = true hex.workspace = true prost = { workspace = true, features = ["prost-derive"] } tokio = { workspace = true, features = ["macros", "time"] } +tracing.workspace = true +xmtp_proto = { path = "../xmtp_proto", features = ["proto_full"] } +xmtp_v2 = { path = "../xmtp_v2" } +zeroize.workspace = true + +[target.'cfg(not(any(target_os = "android", target_os = "ios")))'.dependencies] tonic = { workspace = true, features = [ + "default", "tls", "tls-native-roots", +] } + +[target.'cfg(any(target_os = "android", target_os = "ios"))'.dependencies] +tonic = { workspace = true, features = [ + "default", + "tls", "tls-webpki-roots", ] } -tracing.workspace = true -xmtp_proto = { path = "../xmtp_proto", features = ["proto_full"] } -xmtp_v2 = { path = "../xmtp_v2" } -zeroize.workspace = true [dev-dependencies] uuid = { workspace = true, features = ["v4"] } diff --git a/xmtp_cryptography/src/basic_credential.rs b/xmtp_cryptography/src/basic_credential.rs index 9518c9a24..292d784ce 100644 --- a/xmtp_cryptography/src/basic_credential.rs +++ b/xmtp_cryptography/src/basic_credential.rs @@ -33,6 +33,7 @@ impl std::fmt::Display for SignerError { } mod private { + /// A rudimentary form of specialization /// this allows implementing CredentialSigning /// on `XmtpInstallationCredential` in foreign crates. @@ -44,11 +45,16 @@ mod private { /// Sign with some public/private keypair credential pub trait CredentialSign { /// the hashed context this credential signature takes place in - // If this is not defined the context will be empty - const CONTEXT: &[u8] = b""; type Error; - fn credential_sign>(&self, text: S) -> Result, Self::Error>; + fn credential_sign>( + &self, + text: S, + ) -> Result, Self::Error>; +} + +pub trait SigningContextProvider { + fn context() -> &'static [u8]; } /// Verify a credential signature with its public key diff --git a/xmtp_id/src/associations/signature.rs b/xmtp_id/src/associations/signature.rs index ff7d68ed6..9450b7aba 100644 --- a/xmtp_id/src/associations/signature.rs +++ b/xmtp_id/src/associations/signature.rs @@ -5,7 +5,8 @@ use sha2::{Digest as _, Sha512}; use std::array::TryFromSliceError; use thiserror::Error; use xmtp_cryptography::{ - CredentialSign, CredentialVerify, SignerError, XmtpInstallationCredential, + CredentialSign, CredentialVerify, SignerError, SigningContextProvider, + XmtpInstallationCredential, }; use xmtp_proto::xmtp::message_contents::{ signed_private_key, SignedPrivateKey as LegacySignedPrivateKeyProto, @@ -16,6 +17,8 @@ use super::{ verified_signature::VerifiedSignature, }; +use ethers::core::k256::ecdsa::Signature as K256Signature; + #[derive(Debug, Error)] pub enum SignatureError { // ethers errors @@ -54,14 +57,19 @@ pub enum SignatureError { /// Xmtp Installation Credential for Specialized for XMTP Identity pub struct InboxIdInstallationCredential; +pub struct InstallationKeyContext; +pub struct PublicContext; + impl CredentialSign for XmtpInstallationCredential { - const CONTEXT: &[u8] = crate::constants::INSTALLATION_KEY_SIGNATURE_CONTEXT; type Error = SignatureError; - fn credential_sign>(&self, text: S) -> Result, Self::Error> { + fn credential_sign>( + &self, + text: S, + ) -> Result, Self::Error> { let mut prehashed: Sha512 = Sha512::new(); prehashed.update(text.as_ref()); - let context = self.with_context(Self::CONTEXT)?; + let context = self.with_context(T::context())?; let sig = context .try_sign_digest(prehashed) .map_err(SignatureError::from)?; @@ -86,6 +94,18 @@ impl CredentialVerify for ed25519_dalek::Verifyin } } +impl SigningContextProvider for InstallationKeyContext { + fn context() -> &'static [u8] { + crate::constants::INSTALLATION_KEY_SIGNATURE_CONTEXT + } +} + +impl SigningContextProvider for PublicContext { + fn context() -> &'static [u8] { + crate::constants::PUBLIC_SIGNATURE_CONTEXT + } +} + #[derive(Clone, Debug, PartialEq)] pub enum SignatureKind { // We might want to have some sort of LegacyErc191 Signature Kind for the `CreateIdentity` signatures only @@ -231,3 +251,89 @@ impl ValidatedLegacySignedPublicKey { self.created_ns } } + +/// Converts a signature to use the lower-s value to prevent signature malleability +pub fn to_lower_s(sig_bytes: &[u8]) -> Result, SignatureError> { + // Check if we have a recovery id byte + let (sig_data, recovery_id) = match sig_bytes.len() { + 64 => (sig_bytes, None), // No recovery id + 65 => (&sig_bytes[..64], Some(sig_bytes[64])), // Recovery id present + _ => return Err(SignatureError::Invalid), + }; + + // Parse the signature bytes into a K256Signature + let sig = K256Signature::try_from(sig_data)?; + + // If s is already normalized (lower-s), return the original bytes + let normalized = match sig.normalize_s() { + None => sig_data.to_vec(), + Some(normalized) => normalized.to_vec(), + }; + + // Add back recovery id if it was present + if let Some(rid) = recovery_id { + let mut result = normalized; + result.push(rid); + Ok(result) + } else { + Ok(normalized) + } +} + +#[cfg(test)] +mod tests { + use super::to_lower_s; + use ethers::core::k256::{ecdsa::Signature as K256Signature, elliptic_curve::scalar::IsHigh}; + use ethers::signers::{LocalWallet, Signer}; + use rand::thread_rng; + + #[tokio::test] + async fn test_to_lower_s() { + // Create a test wallet + let wallet = LocalWallet::new(&mut thread_rng()); + + // Sign a test message + let message = "test message"; + let signature = wallet.sign_message(message).await.unwrap(); + let sig_bytes = signature.to_vec(); + + // Test normalizing an already normalized signature + let normalized = to_lower_s(&sig_bytes).unwrap(); + assert_eq!( + normalized, sig_bytes, + "Already normalized signature should not change" + ); + + // Create a signature with high-s value by manipulating the s component + let mut high_s_sig = sig_bytes.clone(); + // Flip bits in the s component (last 32 bytes) to create a high-s value + for byte in high_s_sig[32..64].iter_mut() { + *byte = !*byte; + } + + // Normalize the manipulated signature + let normalized_high_s = to_lower_s(&high_s_sig).unwrap(); + assert_ne!( + normalized_high_s, high_s_sig, + "High-s signature should be normalized" + ); + + // Verify the normalized signature is valid + let recovered_sig = K256Signature::try_from(&normalized_high_s.as_slice()[..64]).unwrap(); + let is_high: bool = recovered_sig.s().is_high().into(); + assert!(!is_high, "Normalized signature should have low-s value"); + } + + #[test] + fn test_invalid_signature() { + // Test with invalid signature bytes + let invalid_sig = vec![0u8; 65]; + let result = to_lower_s(&invalid_sig); + assert!(result.is_err(), "Should fail with invalid signature"); + + // Test with wrong length + let wrong_length = vec![0u8; 63]; + let result = to_lower_s(&wrong_length); + assert!(result.is_err(), "Should fail with wrong length"); + } +} diff --git a/xmtp_id/src/associations/test_utils.rs b/xmtp_id/src/associations/test_utils.rs index a6cf00f09..0f65a863e 100644 --- a/xmtp_id/src/associations/test_utils.rs +++ b/xmtp_id/src/associations/test_utils.rs @@ -3,7 +3,7 @@ use super::{ builder::SignatureRequest, unsigned_actions::UnsignedCreateInbox, unverified::{UnverifiedAction, UnverifiedCreateInbox, UnverifiedSignature}, - AccountId, + AccountId, InstallationKeyContext, }; use crate::scw_verifier::{SmartContractSignatureVerifier, ValidationResponse, VerifierError}; use ethers::{ @@ -83,7 +83,9 @@ pub async fn add_installation_key_signature( installation_key: &XmtpInstallationCredential, ) { let signature_text = signature_request.signature_text(); - let sig = installation_key.credential_sign(signature_text).unwrap(); + let sig = installation_key + .credential_sign::(signature_text) + .unwrap(); let unverified_sig = UnverifiedSignature::new_installation_key(sig, installation_key.verifying_key()); diff --git a/xmtp_id/src/associations/verified_signature.rs b/xmtp_id/src/associations/verified_signature.rs index 4bfedf405..5f3475e16 100644 --- a/xmtp_id/src/associations/verified_signature.rs +++ b/xmtp_id/src/associations/verified_signature.rs @@ -9,7 +9,8 @@ use xmtp_proto::xmtp::message_contents::SignedPublicKey as LegacySignedPublicKey use crate::scw_verifier::SmartContractSignatureVerifier; use super::{ - AccountId, MemberIdentifier, SignatureError, SignatureKind, ValidatedLegacySignedPublicKey, + to_lower_s, AccountId, MemberIdentifier, SignatureError, SignatureKind, + ValidatedLegacySignedPublicKey, }; #[derive(Debug, Clone)] @@ -43,13 +44,14 @@ impl VerifiedSignature { signature_text: Text, signature_bytes: &[u8], ) -> Result { - let signature = EthersSignature::try_from(signature_bytes)?; + let normalized_signature_bytes = to_lower_s(signature_bytes)?; + let signature = EthersSignature::try_from(normalized_signature_bytes.as_slice())?; let address = h160addr_to_string(signature.recover(signature_text.as_ref())?); Ok(Self::new( MemberIdentifier::Address(address), SignatureKind::Erc191, - signature_bytes.to_vec(), + normalized_signature_bytes.to_vec(), None, )) } @@ -173,7 +175,7 @@ mod tests { sign_with_legacy_key, test_utils::{rand_string, MockSmartContractSignatureVerifier}, verified_signature::VerifiedSignature, - MemberIdentifier, SignatureKind, + InstallationKeyContext, MemberIdentifier, SignatureKind, }, InboxOwner, }; @@ -220,7 +222,9 @@ mod tests { let key = XmtpInstallationCredential::new(); let verifying_key = key.verifying_key(); let signature_text = "test signature text"; - let sig = key.credential_sign(signature_text).unwrap(); + let sig = key + .credential_sign::(signature_text) + .unwrap(); let verified_sig = VerifiedSignature::from_installation_key(signature_text, sig.as_slice(), verifying_key) diff --git a/xmtp_id/src/constants.rs b/xmtp_id/src/constants.rs index 1dbde531f..5298a29cd 100644 --- a/xmtp_id/src/constants.rs +++ b/xmtp_id/src/constants.rs @@ -1,2 +1,3 @@ /// DO NOT CHANGE. SIGNATURES WILL BREAK pub const INSTALLATION_KEY_SIGNATURE_CONTEXT: &[u8] = b"IDENTITY UPDATE SIGNATURE"; +pub const PUBLIC_SIGNATURE_CONTEXT: &[u8] = b"PUBLIC SIGNATURE CONTEXT"; diff --git a/xmtp_mls/src/client.rs b/xmtp_mls/src/client.rs index 6e7972bfb..8a0601f3d 100644 --- a/xmtp_mls/src/client.rs +++ b/xmtp_mls/src/client.rs @@ -1,6 +1,5 @@ use std::{ collections::HashMap, - mem::Discriminant, sync::{ atomic::{AtomicUsize, Ordering}, Arc, @@ -9,14 +8,11 @@ use std::{ use futures::stream::{self, FuturesUnordered, StreamExt}; use openmls::{ - credentials::errors::BasicCredentialError, framing::{MlsMessageBodyIn, MlsMessageIn}, - group::GroupEpoch, messages::Welcome, prelude::tls_codec::{Deserialize, Error as TlsCodecError}, }; use openmls_traits::OpenMlsProvider; -use prost::EncodeError; use thiserror::Error; use tokio::sync::broadcast; @@ -35,26 +31,23 @@ use xmtp_proto::xmtp::mls::api::v1::{ GroupMessage, WelcomeMessage, }; -use crate::storage::wallet_addresses::WalletEntry; use crate::{ api::ApiClientWrapper, - groups::{ - group_permissions::PolicySet, validated_commit::CommitValidationError, GroupError, - GroupMetadataOptions, IntentError, MlsGroup, - }, + groups::{group_permissions::PolicySet, GroupError, GroupMetadataOptions, MlsGroup}, identity::{parse_credential, Identity, IdentityError}, identity_updates::{load_identity_updates, IdentityUpdateError}, intents::Intents, mutex_registry::MutexRegistry, retry::Retry, retry_async, retryable, + storage::wallet_addresses::WalletEntry, storage::{ consent_record::{ConsentState, ConsentType, StoredConsentRecord}, db_connection::DbConnection, group::{GroupMembershipState, GroupQueryArgs, StoredGroup}, group_message::StoredGroupMessage, refresh_state::EntityKind, - sql_key_store, EncryptedMessageStore, StorageError, + EncryptedMessageStore, StorageError, }, subscriptions::LocalEvents, verified_key_package_v2::{KeyPackageVerificationError, VerifiedKeyPackageV2}, @@ -91,8 +84,6 @@ pub enum ClientError { TlsError(#[from] TlsCodecError), #[error("key package verification: {0}")] KeyPackageVerification(#[from] KeyPackageVerificationError), - #[error("syncing errors: {0:?}")] - SyncingError(Vec), #[error("Stream inconsistency error: {0}")] StreamInconsistency(String), #[error("Association error: {0}")] @@ -129,82 +120,6 @@ impl crate::retry::RetryableError for ClientError { } } -/// Errors that can occur when reading and processing a message off the network -#[derive(Debug, Error)] -pub enum MessageProcessingError { - #[error("[{0}] already processed")] - AlreadyProcessed(u64), - #[error("diesel error: {0}")] - Diesel(#[from] diesel::result::Error), - #[error("[{message_time_ns:?}] invalid sender with credential: {credential:?}")] - InvalidSender { - message_time_ns: u64, - credential: Vec, - }, - #[error("invalid payload")] - InvalidPayload, - #[error(transparent)] - Identity(#[from] IdentityError), - #[error("openmls process message error: {0}")] - OpenMlsProcessMessage(#[from] openmls::prelude::ProcessMessageError), - #[error("merge staged commit: {0}")] - MergeStagedCommit(#[from] openmls::group::MergeCommitError), - #[error( - "no pending commit to merge. group epoch is {group_epoch:?} and got {message_epoch:?}" - )] - NoPendingCommit { - message_epoch: GroupEpoch, - group_epoch: GroupEpoch, - }, - #[error("intent error: {0}")] - Intent(#[from] IntentError), - #[error("storage error: {0}")] - Storage(#[from] crate::storage::StorageError), - #[error("TLS Codec error: {0}")] - TlsError(#[from] TlsCodecError), - #[error("unsupported message type: {0:?}")] - UnsupportedMessageType(Discriminant), - #[error("commit validation")] - CommitValidation(#[from] CommitValidationError), - #[error("codec")] - Codec(#[from] crate::codecs::CodecError), - #[error("encode proto: {0}")] - EncodeProto(#[from] EncodeError), - #[error("epoch increment not allowed")] - EpochIncrementNotAllowed, - #[error("Welcome processing error: {0}")] - WelcomeProcessing(Box), - #[error("wrong credential type")] - WrongCredentialType(#[from] BasicCredentialError), - #[error("proto decode error: {0}")] - DecodeError(#[from] prost::DecodeError), - #[error("clear pending commit error: {0}")] - ClearPendingCommit(#[from] sql_key_store::SqlKeyStoreError), - #[error(transparent)] - Group(#[from] Box), - #[error("Serialization/Deserialization Error {0}")] - Serde(#[from] serde_json::Error), - #[error("generic:{0}")] - Generic(String), - #[error("intent is missing staged_commit field")] - IntentMissingStagedCommit, -} - -impl crate::retry::RetryableError for MessageProcessingError { - fn is_retryable(&self) -> bool { - match self { - Self::Group(group_error) => retryable!(group_error), - Self::Identity(identity_error) => retryable!(identity_error), - Self::OpenMlsProcessMessage(err) => retryable!(err), - Self::MergeStagedCommit(err) => retryable!(err), - Self::Diesel(diesel_error) => retryable!(diesel_error), - Self::Storage(s) => retryable!(s), - Self::Generic(err) => err.contains("database is locked"), - _ => false, - } - } -} - impl From for ClientError { fn from(value: String) -> Self { Self::Generic(value) @@ -283,6 +198,10 @@ impl XmtpMlsLocalContext { pub fn signature_request(&self) -> Option { self.identity.signature_request() } + + pub fn public_sign>(&self, text: Text) -> Result, IdentityError> { + self.identity.sign_with_public_context(text) + } } impl Client @@ -843,9 +762,7 @@ where tracing::error!("failed to create group from welcome: {}", err); } - Err(MessageProcessingError::WelcomeProcessing( - Box::new(err) - )) + Err(err) } } }, diff --git a/xmtp_mls/src/groups/mls_sync.rs b/xmtp_mls/src/groups/mls_sync.rs index 30439afb1..4d17fee4f 100644 --- a/xmtp_mls/src/groups/mls_sync.rs +++ b/xmtp_mls/src/groups/mls_sync.rs @@ -1,8 +1,3 @@ -use std::{ - collections::{HashMap, HashSet}, - mem::discriminant, -}; - use super::{ build_extensions_for_admin_lists_update, build_extensions_for_metadata_update, build_extensions_for_permissions_update, build_group_membership_extension, @@ -10,12 +5,10 @@ use super::{ Installation, PostCommitAction, SendMessageIntentData, SendWelcomesAction, UpdateAdminListIntentData, UpdateGroupMembershipIntentData, UpdatePermissionIntentData, }, - validated_commit::extract_group_membership, - GroupError, MlsGroup, ScopedGroupClient, + validated_commit::{extract_group_membership, CommitValidationError}, + GroupError, IntentError, MlsGroup, ScopedGroupClient, }; - use crate::{ - client::MessageProcessingError, codecs::{group_updated::GroupUpdatedCodec, ContentCodec}, configuration::{ GRPC_DATA_LIMIT, MAX_GROUP_SIZE, MAX_INTENT_PUBLISH_ATTEMPTS, MAX_PAST_EPOCHS, @@ -23,8 +16,9 @@ use crate::{ }, groups::{intents::UpdateMetadataIntentData, validated_commit::ValidatedCommit}, hpke::{encrypt_welcome, HpkeError}, - identity::parse_credential, + identity::{parse_credential, IdentityError}, identity_updates::load_identity_updates, + intents::ProcessIntentError, retry::{Retry, RetryableError}, retry_async, storage::{ @@ -33,6 +27,7 @@ use crate::{ group_message::{DeliveryStatus, GroupMessageKind, StoredGroupMessage}, refresh_state::EntityKind, serialization::{db_deserialize, db_serialize}, + sql_key_store, }, subscriptions::LocalEvents, utils::{hash::sha256, id::calculate_message_id}, @@ -41,7 +36,6 @@ use crate::{ }; use crate::{groups::device_sync::DeviceSyncContent, subscriptions::SyncMessage}; use futures::future::try_join_all; -use openmls::framing::WireFormat; use openmls::{ credentials::BasicCredential, extensions::Extensions, @@ -49,15 +43,21 @@ use openmls::{ group::{GroupEpoch, StagedCommit}, key_packages::KeyPackage, prelude::{ - tls_codec::{Deserialize, Serialize}, + tls_codec::{Deserialize, Error as TlsCodecError, Serialize}, LeafNodeIndex, MlsGroup as OpenMlsGroup, MlsMessageBodyIn, MlsMessageIn, PrivateMessageIn, ProcessedMessage, ProcessedMessageContent, Sender, }, treesync::LeafNodeParameters, }; +use openmls::{framing::WireFormat, prelude::BasicCredentialError}; use openmls_traits::{signatures::Signer, OpenMlsProvider}; use prost::bytes::Bytes; use prost::Message; +use std::{ + collections::{HashMap, HashSet}, + mem::{discriminant, Discriminant}, +}; +use thiserror::Error; use tracing::debug; use xmtp_id::{InboxId, InboxIdRef}; use xmtp_proto::xmtp::mls::{ @@ -74,6 +74,86 @@ use xmtp_proto::xmtp::mls::{ }, }; +#[derive(Debug, Error)] +pub enum GroupMessageProcessingError { + #[error("[{0}] already processed")] + AlreadyProcessed(u64), + #[error("diesel error: {0}")] + Diesel(#[from] diesel::result::Error), + #[error("[{message_time_ns:?}] invalid sender with credential: {credential:?}")] + InvalidSender { + message_time_ns: u64, + credential: Vec, + }, + #[error("invalid payload")] + InvalidPayload, + #[error("storage error: {0}")] + Storage(#[from] crate::storage::StorageError), + #[error(transparent)] + Identity(#[from] IdentityError), + #[error("openmls process message error: {0}")] + OpenMlsProcessMessage(#[from] openmls::prelude::ProcessMessageError), + #[error("merge staged commit: {0}")] + MergeStagedCommit(#[from] openmls::group::MergeCommitError), + #[error("TLS Codec error: {0}")] + TlsError(#[from] TlsCodecError), + #[error("unsupported message type: {0:?}")] + UnsupportedMessageType(Discriminant), + #[error("commit validation")] + CommitValidation(#[from] CommitValidationError), + #[error("epoch increment not allowed")] + EpochIncrementNotAllowed, + #[error("clear pending commit error: {0}")] + ClearPendingCommit(#[from] sql_key_store::SqlKeyStoreError), + #[error("Serialization/Deserialization Error {0}")] + Serde(#[from] serde_json::Error), + #[error("intent is missing staged_commit field")] + IntentMissingStagedCommit, + #[error("encode proto: {0}")] + EncodeProto(#[from] prost::EncodeError), + #[error("proto decode error: {0}")] + DecodeProto(#[from] prost::DecodeError), + #[error(transparent)] + Intent(#[from] IntentError), + #[error(transparent)] + Codec(#[from] crate::codecs::CodecError), + #[error("wrong credential type")] + WrongCredentialType(#[from] BasicCredentialError), + #[error(transparent)] + ProcessIntent(#[from] ProcessIntentError), + #[error(transparent)] + AssociationDeserialization(#[from] xmtp_id::associations::DeserializationError), +} + +impl crate::retry::RetryableError for GroupMessageProcessingError { + fn is_retryable(&self) -> bool { + match self { + Self::Diesel(err) => err.is_retryable(), + Self::Storage(err) => err.is_retryable(), + Self::Identity(err) => err.is_retryable(), + Self::OpenMlsProcessMessage(err) => err.is_retryable(), + Self::MergeStagedCommit(err) => err.is_retryable(), + Self::ProcessIntent(err) => err.is_retryable(), + Self::CommitValidation(err) => err.is_retryable(), + Self::ClearPendingCommit(err) => err.is_retryable(), + Self::WrongCredentialType(_) + | Self::Codec(_) + | Self::AlreadyProcessed(_) + | Self::InvalidSender { .. } + | Self::DecodeProto(_) + | Self::InvalidPayload + | Self::Intent(_) + | Self::EpochIncrementNotAllowed + | Self::EncodeProto(_) + | Self::IntentMissingStagedCommit + | Self::Serde(_) + | Self::AssociationDeserialization(_) + | Self::TlsError(_) + | Self::UnsupportedMessageType(_) => false, + } + } +} + use xmtp_proto::xmtp::mls::message_contents::plaintext_envelope::v2::MessageType::{ Reply, Request, }; @@ -276,7 +356,7 @@ where provider: &XmtpOpenMlsProvider, message: ProtocolMessage, envelope: &GroupMessageV1, - ) -> Result { + ) -> Result { let GroupMessageV1 { created_ns: envelope_timestamp_ns, id: ref msg_id, @@ -334,7 +414,7 @@ where let pending_commit = if let Some(staged_commit) = intent.staged_commit { decode_staged_commit(staged_commit)? } else { - return Err(MessageProcessingError::IntentMissingStagedCommit); + return Err(GroupMessageProcessingError::IntentMissingStagedCommit); }; tracing::info!( @@ -404,7 +484,7 @@ where provider: &XmtpOpenMlsProvider, message: PrivateMessageIn, envelope: &GroupMessageV1, - ) -> Result<(), MessageProcessingError> { + ) -> Result<(), GroupMessageProcessingError> { let GroupMessageV1 { created_ns: envelope_timestamp_ns, id: ref msg_id, @@ -450,8 +530,7 @@ where let message_bytes = application_message.into_bytes(); let mut bytes = Bytes::from(message_bytes.clone()); - let envelope = PlaintextEnvelope::decode(&mut bytes) - .map_err(MessageProcessingError::DecodeError)?; + let envelope = PlaintextEnvelope::decode(&mut bytes)?; match envelope.content { Some(Content::V1(V1 { @@ -540,10 +619,10 @@ where } } _ => { - return Err(MessageProcessingError::InvalidPayload); + return Err(GroupMessageProcessingError::InvalidPayload); } }, - None => return Err(MessageProcessingError::InvalidPayload), + None => return Err(GroupMessageProcessingError::InvalidPayload), } } ProcessedMessageContent::ProposalMessage(_proposal_ptr) => { @@ -607,17 +686,17 @@ where provider: &XmtpOpenMlsProvider, envelope: &GroupMessageV1, allow_epoch_increment: bool, - ) -> Result<(), MessageProcessingError> { + ) -> Result<(), GroupMessageProcessingError> { let mls_message_in = MlsMessageIn::tls_deserialize_exact(&envelope.data)?; let message = match mls_message_in.extract() { MlsMessageBodyIn::PrivateMessage(message) => Ok(message), - other => Err(MessageProcessingError::UnsupportedMessageType( + other => Err(GroupMessageProcessingError::UnsupportedMessageType( discriminant(&other), )), }?; if !allow_epoch_increment && message.content_type() == ContentType::Commit { - return Err(MessageProcessingError::EpochIncrementNotAllowed); + return Err(GroupMessageProcessingError::EpochIncrementNotAllowed); } let intent = provider @@ -682,7 +761,7 @@ where self.process_external_message(openmls_group, provider, message, envelope) .await } - Err(err) => Err(MessageProcessingError::Storage(err)), + Err(err) => Err(GroupMessageProcessingError::Storage(err)), } } @@ -692,10 +771,10 @@ where envelope: &GroupMessage, openmls_group: &mut OpenMlsGroup, conn: &DbConnection, - ) -> Result<(), MessageProcessingError> { + ) -> Result<(), GroupMessageProcessingError> { let msgv1 = match &envelope.version { Some(GroupMessageVersion::V1(value)) => value, - _ => return Err(MessageProcessingError::InvalidPayload), + _ => return Err(GroupMessageProcessingError::InvalidPayload), }; let mls_message_in = MlsMessageIn::tls_deserialize_exact(&msgv1.data)?; @@ -716,7 +795,7 @@ where message_entity_kind, last_cursor ); - Err(MessageProcessingError::AlreadyProcessed(msgv1.id)) + Err(GroupMessageProcessingError::AlreadyProcessed(msgv1.id)) } else { self.client .intents() @@ -727,7 +806,7 @@ where |provider| async move { self.process_message(openmls_group, &provider, msgv1, true) .await?; - Ok(()) + Ok::<(), GroupMessageProcessingError>(()) }, ) .await?; @@ -743,7 +822,7 @@ where ) -> Result<(), GroupError> { let mut openmls_group = self.load_mls_group(provider)?; - let mut receive_errors = vec![]; + let mut receive_errors: Vec = vec![]; for message in messages.into_iter() { let result = retry_async!( Retry::default(), @@ -792,7 +871,7 @@ where conn: &DbConnection, validated_commit: ValidatedCommit, timestamp_ns: u64, - ) -> Result, MessageProcessingError> { + ) -> Result, GroupMessageProcessingError> { if validated_commit.is_empty() { return Ok(None); } @@ -1261,7 +1340,7 @@ fn extract_message_sender( openmls_group: &mut OpenMlsGroup, decrypted_message: &ProcessedMessage, message_created_ns: u64, -) -> Result<(InboxId, Vec), MessageProcessingError> { +) -> Result<(InboxId, Vec), GroupMessageProcessingError> { if let Sender::Member(leaf_node_index) = decrypted_message.sender() { if let Some(member) = openmls_group.member_at(*leaf_node_index) { if member.credential.eq(decrypted_message.credential()) { @@ -1273,7 +1352,7 @@ fn extract_message_sender( } let basic_credential = BasicCredential::try_from(decrypted_message.credential().clone())?; - return Err(MessageProcessingError::InvalidSender { + return Err(GroupMessageProcessingError::InvalidSender { message_time_ns: message_created_ns, credential: basic_credential.identity().to_vec(), }); @@ -1396,7 +1475,7 @@ fn get_and_clear_pending_commit( Ok(commit) } -fn decode_staged_commit(data: Vec) -> Result { +fn decode_staged_commit(data: Vec) -> Result { Ok(db_deserialize(&data)?) } diff --git a/xmtp_mls/src/groups/mod.rs b/xmtp_mls/src/groups/mod.rs index 09ca7b772..80a0a02c1 100644 --- a/xmtp_mls/src/groups/mod.rs +++ b/xmtp_mls/src/groups/mod.rs @@ -12,6 +12,7 @@ pub(super) mod subscriptions; pub mod validated_commit; use intents::SendMessageIntentData; +use mls_sync::GroupMessageProcessingError; use openmls::{ credentials::{BasicCredential, CredentialType}, error::LibraryError, @@ -72,7 +73,7 @@ use xmtp_proto::xmtp::mls::{ use crate::{ api::WrappedApiError, - client::{deserialize_welcome, ClientError, MessageProcessingError, XmtpMlsLocalContext}, + client::{deserialize_welcome, ClientError, XmtpMlsLocalContext}, configuration::{ CIPHERSUITE, GROUP_MEMBERSHIP_EXTENSION_ID, GROUP_PERMISSIONS_EXTENSION_ID, MAX_GROUP_SIZE, MAX_PAST_EPOCHS, MUTABLE_METADATA_EXTENSION_ID, @@ -81,6 +82,7 @@ use crate::{ hpke::{decrypt_welcome, HpkeError}, identity::{parse_credential, IdentityError}, identity_updates::{load_identity_updates, InstallationDiffError}, + intents::ProcessIntentError, retry::RetryableError, storage::{ consent_record::{ConsentState, ConsentType, StoredConsentRecord}, @@ -136,9 +138,9 @@ pub enum GroupError { #[error("client: {0}")] Client(#[from] ClientError), #[error("receive error: {0}")] - ReceiveError(#[from] MessageProcessingError), + ReceiveError(#[from] GroupMessageProcessingError), #[error("Receive errors: {0:?}")] - ReceiveErrors(Vec), + ReceiveErrors(Vec), #[error("generic: {0}")] Generic(String), #[error("diesel error {0}")] @@ -180,32 +182,29 @@ pub enum GroupError { NoPSKSupport, #[error("Metadata update must specify a metadata field")] InvalidPermissionUpdate, - #[error("The intent publishing task was cancelled")] - PublishCancelled, - #[error("the publish failed to complete due to panic")] - PublishPanicked, #[error("dm requires target inbox_id")] InvalidDmMissingInboxId, #[error("Missing metadata field {name}")] MissingMetadataField { name: String }, - #[error("Message was processed but is missing")] - MissingMessage, #[error("sql key store error: {0}")] SqlKeyStore(#[from] sql_key_store::SqlKeyStoreError), - #[error("No pending commit found")] - MissingPendingCommit, #[error("Sync failed to wait for intent")] SyncFailedToWait, #[error("cannot change metadata of DM")] DmGroupMetadataForbidden, - #[error("Group intent could not be committed")] + #[error("Missing pending commit")] + MissingPendingCommit, + #[error("Intent not committed")] IntentNotCommitted, + #[error(transparent)] + ProcessIntent(#[from] ProcessIntentError), } impl RetryableError for GroupError { fn is_retryable(&self) -> bool { match self { Self::Api(api_error) => api_error.is_retryable(), + Self::ReceiveErrors(errors) => errors.iter().any(|e| e.is_retryable()), Self::Client(client_error) => client_error.is_retryable(), Self::Diesel(diesel) => diesel.is_retryable(), Self::Storage(storage) => storage.is_retryable(), @@ -216,9 +215,41 @@ impl RetryableError for GroupError { Self::GroupCreate(group) => group.is_retryable(), Self::SelfUpdate(update) => update.is_retryable(), Self::WelcomeError(welcome) => welcome.is_retryable(), + Self::SqlKeyStore(sql) => sql.is_retryable(), + Self::Sync(errs) => errs.iter().any(|e| e.is_retryable()), Self::InstallationDiff(diff) => diff.is_retryable(), Self::CreateGroupContextExtProposalError(create) => create.is_retryable(), - _ => false, + Self::CommitValidation(err) => err.is_retryable(), + Self::WrappedApi(err) => err.is_retryable(), + Self::MessageHistory(err) => err.is_retryable(), + Self::ProcessIntent(err) => err.is_retryable(), + Self::SyncFailedToWait => true, + Self::GroupNotFound + | Self::GroupMetadata(_) + | Self::GroupMutableMetadata(_) + | Self::GroupMutablePermissions(_) + | Self::UserLimitExceeded + | Self::InvalidGroupMembership + | Self::Intent(_) + | Self::CreateMessage(_) + | Self::TlsError(_) + | Self::IntentNotCommitted + | Self::Generic(_) + | Self::InvalidDmMissingInboxId + | Self::MissingSequenceId + | Self::AddressNotFound(_) + | Self::InvalidExtension(_) + | Self::MissingMetadataField { .. } + | Self::DmGroupMetadataForbidden + | Self::Signature(_) + | Self::LeafNodeError(_) + | Self::NoPSKSupport + | Self::MissingPendingCommit + | Self::InvalidPermissionUpdate + | Self::AddressValidation(_) + | Self::InvalidPublicKeys(_) + | Self::CredentialError(_) + | Self::EncodeError(_) => false, } } } @@ -1149,17 +1180,19 @@ impl MlsGroup { } } -fn extract_message_v1(message: GroupMessage) -> Result { +fn extract_message_v1( + message: GroupMessage, +) -> Result { match message.version { Some(GroupMessageVersion::V1(value)) => Ok(value), - _ => Err(MessageProcessingError::InvalidPayload), + _ => Err(GroupMessageProcessingError::InvalidPayload), } } -pub fn extract_group_id(message: &GroupMessage) -> Result, MessageProcessingError> { +pub fn extract_group_id(message: &GroupMessage) -> Result, GroupMessageProcessingError> { match &message.version { Some(GroupMessageVersion::V1(value)) => Ok(value.group_id.clone()), - _ => Err(MessageProcessingError::InvalidPayload), + _ => Err(GroupMessageProcessingError::InvalidPayload), } } @@ -1541,7 +1574,6 @@ pub(crate) mod tests { use crate::{ assert_err, builder::ClientBuilder, - client::MessageProcessingError, codecs::{group_updated::GroupUpdatedCodec, ContentCodec}, groups::{ build_dm_protected_metadata_extension, build_mutable_metadata_extension_default, @@ -1550,6 +1582,7 @@ pub(crate) mod tests { group_mutable_metadata::MetadataField, intents::{PermissionPolicyOption, PermissionUpdateType}, members::{GroupMember, PermissionLevel}, + mls_sync::GroupMessageProcessingError, validate_dm_group, DeliveryStatus, GroupError, GroupMetadataOptions, PreconfiguredPolicies, UpdateAdminListType, }, @@ -1870,7 +1903,7 @@ pub(crate) mod tests { let mut mls_group = alix_group.load_mls_group(&provider).unwrap(); let mut existing_extensions = mls_group.extensions().clone(); let mut group_membership = GroupMembership::new(); - group_membership.add("foo".to_string(), 1); + group_membership.add("deadbeef".to_string(), 1); existing_extensions.add_or_replace(build_group_membership_extension(&group_membership)); mls_group .update_group_context_extensions( @@ -3625,7 +3658,7 @@ pub(crate) mod tests { assert_err!( process_result, - MessageProcessingError::EpochIncrementNotAllowed + GroupMessageProcessingError::EpochIncrementNotAllowed ); } diff --git a/xmtp_mls/src/groups/subscriptions.rs b/xmtp_mls/src/groups/subscriptions.rs index 724fe6df2..5701bb882 100644 --- a/xmtp_mls/src/groups/subscriptions.rs +++ b/xmtp_mls/src/groups/subscriptions.rs @@ -63,13 +63,13 @@ impl MlsGroup { self.process_message(&mut openmls_group, &provider, msgv1, false) .await // NOTE: We want to make sure we retry an error in process_message - .map_err(SubscribeError::Receive) + .map_err(SubscribeError::ReceiveGroup) }) .await }) ); - if let Err(SubscribeError::Receive(_)) = process_result { + if let Err(SubscribeError::ReceiveGroup(_)) = process_result { tracing::debug!( inbox_id = self.client.inbox_id(), group_id = hex::encode(&self.group_id), @@ -99,8 +99,9 @@ impl MlsGroup { inbox_id = self.client.inbox_id(), group_id = hex::encode(&self.group_id), msg_id = msgv1.id, - err = %e, - "process stream entry {:?}", e + err = e.to_string(), + "process stream entry {:?}", + e ); } } diff --git a/xmtp_mls/src/identity.rs b/xmtp_mls/src/identity.rs index e979cedfa..8962532fc 100644 --- a/xmtp_mls/src/identity.rs +++ b/xmtp_mls/src/identity.rs @@ -33,7 +33,7 @@ use tracing::debug; use tracing::info; use xmtp_cryptography::{CredentialSign, XmtpInstallationCredential}; use xmtp_id::associations::unverified::UnverifiedSignature; -use xmtp_id::associations::AssociationError; +use xmtp_id::associations::{AssociationError, InstallationKeyContext, PublicContext}; use xmtp_id::scw_verifier::SmartContractSignatureVerifier; use xmtp_id::{ associations::{ @@ -261,8 +261,8 @@ impl Identity { ) .build(); - let signature = - installation_keys.credential_sign(signature_request.signature_text())?; + let signature = installation_keys + .credential_sign::(signature_request.signature_text())?; signature_request .add_signature( UnverifiedSignature::new_installation_key( @@ -305,7 +305,8 @@ impl Identity { ) .build(); - let sig = installation_keys.credential_sign(signature_request.signature_text())?; + let sig = installation_keys + .credential_sign::(signature_request.signature_text())?; signature_request .add_signature( @@ -361,7 +362,8 @@ impl Identity { ) .build(); - let sig = installation_keys.credential_sign(signature_request.signature_text())?; + let sig = installation_keys + .credential_sign::(signature_request.signature_text())?; // We can pre-sign the request with an installation key signature, since we have access to the key signature_request .add_signature( @@ -409,9 +411,21 @@ impl Identity { /** * Sign the given text with the installation private key. */ - pub(crate) fn sign>(&self, text: Text) -> Result, IdentityError> { + pub(crate) fn sign_identity_update>( + &self, + text: Text, + ) -> Result, IdentityError> { + self.installation_keys + .credential_sign::(text) + .map_err(Into::into) + } + + pub fn sign_with_public_context>( + &self, + text: Text, + ) -> Result, IdentityError> { self.installation_keys - .credential_sign(text) + .credential_sign::(text) .map_err(Into::into) } diff --git a/xmtp_mls/src/identity_updates.rs b/xmtp_mls/src/identity_updates.rs index 741f716af..83c0b9c5f 100644 --- a/xmtp_mls/src/identity_updates.rs +++ b/xmtp_mls/src/identity_updates.rs @@ -259,7 +259,7 @@ where let sig_bytes = self .identity() - .sign(signature_request.signature_text())? + .sign_identity_update(signature_request.signature_text())? .to_vec(); // We can pre-sign the request with an installation key signature, since we have access to the key signature_request diff --git a/xmtp_mls/src/intents.rs b/xmtp_mls/src/intents.rs index 83e3c9bc3..b7b06ef06 100644 --- a/xmtp_mls/src/intents.rs +++ b/xmtp_mls/src/intents.rs @@ -8,13 +8,34 @@ //! Intents are written to local storage (SQLite), before being published to the delivery service via gRPC. An //! intent is fully resolved (success or failure) once it -use std::{future::Future, sync::Arc}; - use crate::{ - client::{MessageProcessingError, XmtpMlsLocalContext}, + client::XmtpMlsLocalContext, + retry::RetryableError, storage::{refresh_state::EntityKind, EncryptedMessageStore}, xmtp_openmls_provider::XmtpOpenMlsProvider, }; +use std::{future::Future, sync::Arc}; +use thiserror::Error; + +#[derive(Debug, Error)] +pub enum ProcessIntentError { + #[error("[{0}] already processed")] + AlreadyProcessed(u64), + #[error("diesel error: {0}")] + Diesel(#[from] diesel::result::Error), + #[error("storage error: {0}")] + Storage(#[from] crate::storage::StorageError), +} + +impl RetryableError for ProcessIntentError { + fn is_retryable(&self) -> bool { + match self { + Self::AlreadyProcessed(_) => false, + Self::Diesel(err) => err.is_retryable(), + Self::Storage(err) => err.is_retryable(), + } + } +} /// Intents holding the context of this Client pub struct Intents { @@ -29,16 +50,20 @@ impl Intents { /// Download all unread welcome messages and convert to groups. /// In a database transaction, increment the cursor for a given entity and /// apply the update after the provided `ProcessingFn` has completed successfully. - pub(crate) async fn process_for_id( + pub(crate) async fn process_for_id( &self, entity_id: &Vec, entity_kind: EntityKind, cursor: u64, process_envelope: ProcessingFn, - ) -> Result + ) -> Result where - Fut: Future>, + Fut: Future>, ProcessingFn: FnOnce(XmtpOpenMlsProvider) -> Fut, + ErrorType: From + + From + + From + + std::fmt::Display, { self.store() .transaction_async(|provider| async move { @@ -47,7 +72,7 @@ impl Intents { .conn_ref() .update_cursor(entity_id, entity_kind, cursor as i64)?; if !is_updated { - return Err(MessageProcessingError::AlreadyProcessed(cursor)); + return Err(ProcessIntentError::AlreadyProcessed(cursor).into()); } process_envelope(provider).await }) diff --git a/xmtp_mls/src/storage/sql_key_store.rs b/xmtp_mls/src/storage/sql_key_store.rs index af7f66258..ae9d2d797 100644 --- a/xmtp_mls/src/storage/sql_key_store.rs +++ b/xmtp_mls/src/storage/sql_key_store.rs @@ -253,7 +253,10 @@ impl RetryableError for SqlKeyStoreError { fn is_retryable(&self) -> bool { match self { SqlKeyStoreError::Storage(err) => retryable!(err), - _ => false, + SqlKeyStoreError::SerializationError => false, + SqlKeyStoreError::UnsupportedMethod => false, + SqlKeyStoreError::UnsupportedValueTypeBytes => false, + SqlKeyStoreError::NotFound => false, } } } diff --git a/xmtp_mls/src/subscriptions.rs b/xmtp_mls/src/subscriptions.rs index 81dd4ba67..e79b39def 100644 --- a/xmtp_mls/src/subscriptions.rs +++ b/xmtp_mls/src/subscriptions.rs @@ -10,8 +10,8 @@ use xmtp_id::scw_verifier::SmartContractSignatureVerifier; use xmtp_proto::{api_client::XmtpMlsStreams, xmtp::mls::api::v1::WelcomeMessage}; use crate::{ - client::{extract_welcome_message, ClientError, MessageProcessingError}, - groups::{subscriptions, GroupError, MlsGroup}, + client::{extract_welcome_message, ClientError}, + groups::{mls_sync::GroupMessageProcessingError, subscriptions, GroupError, MlsGroup}, retry::{Retry, RetryableError}, retry_async, retryable, storage::{ @@ -124,8 +124,8 @@ pub enum SubscribeError { Group(#[from] GroupError), #[error("group message expected in database but is missing")] GroupMessageNotFound, - #[error("processing message in stream: {0}")] - Receive(#[from] MessageProcessingError), + #[error("processing group message in stream: {0}")] + ReceiveGroup(#[from] GroupMessageProcessingError), #[error(transparent)] Database(#[from] diesel::result::Error), #[error(transparent)] @@ -144,7 +144,7 @@ impl RetryableError for SubscribeError { Client(e) => retryable!(e), Group(e) => retryable!(e), GroupMessageNotFound => true, - Receive(e) => retryable!(e), + ReceiveGroup(e) => retryable!(e), Database(e) => retryable!(e), Storage(e) => retryable!(e), Api(e) => retryable!(e),