Legacy key issues (#767)

bindings_ffi/src/mls.rs

@@ -64,8 +64,6 @@ pub async fn create_client(
     legacy_identity_source: LegacyIdentitySource,
     legacy_signed_private_key_proto: Option<Vec<u8>>,
 ) -> Result<Arc<FfiXmtpClient>, GenericError> {
-    init_logger(logger);
-
     log::info!(
         "Creating API client for host: {}, isSecure: {}",
         host,
@@ -832,6 +830,7 @@ mod tests {
     };
 
     use super::{create_client, FfiMessage, FfiMessageCallback, FfiXmtpClient};
+    use ethers::utils::hex;
     use ethers_core::rand::{
         self,
         distributions::{Alphanumeric, DistString},
@@ -977,32 +976,19 @@ mod tests {
 
     #[tokio::test]
     #[ignore]
-    // This test needs to be updated to use the real address for the legacy signed private key
     async fn test_legacy_identity() {
-        let inbox_id = "pseudo-hex";
-        let legacy_signed_private_key_proto = vec![
-            8, 128, 154, 196, 133, 220, 244, 197, 216, 23, 18, 34, 10, 32, 214, 70, 104, 202, 68,
-            204, 25, 202, 197, 141, 239, 159, 145, 249, 55, 242, 147, 126, 3, 124, 159, 207, 96,
-            135, 134, 122, 60, 90, 82, 171, 131, 162, 26, 153, 1, 10, 79, 8, 128, 154, 196, 133,
-            220, 244, 197, 216, 23, 26, 67, 10, 65, 4, 232, 32, 50, 73, 113, 99, 115, 168, 104,
-            229, 206, 24, 217, 132, 223, 217, 91, 63, 137, 136, 50, 89, 82, 186, 179, 150, 7, 127,
-            140, 10, 165, 117, 233, 117, 196, 134, 227, 143, 125, 210, 187, 77, 195, 169, 162, 116,
-            34, 20, 196, 145, 40, 164, 246, 139, 197, 154, 233, 190, 148, 35, 131, 240, 106, 103,
-            18, 70, 18, 68, 10, 64, 90, 24, 36, 99, 130, 246, 134, 57, 60, 34, 142, 165, 221, 123,
-            63, 27, 138, 242, 195, 175, 212, 146, 181, 152, 89, 48, 8, 70, 104, 94, 163, 0, 25,
-            196, 228, 190, 49, 108, 141, 60, 174, 150, 177, 115, 229, 138, 92, 105, 170, 226, 204,
-            249, 206, 12, 37, 145, 3, 35, 226, 15, 49, 20, 102, 60, 16, 1,
-        ];
+        let account_address = "0x0bD00B21aF9a2D538103c3AAf95Cb507f8AF1B28".to_lowercase();
+        let legacy_keys = hex::decode("0880bdb7a8b3f6ede81712220a20ad528ea38ce005268c4fb13832cfed13c2b2219a378e9099e48a38a30d66ef991a96010a4c08aaa8e6f5f9311a430a41047fd90688ca39237c2899281cdf2756f9648f93767f91c0e0f74aed7e3d3a8425e9eaa9fa161341c64aa1c782d004ff37ffedc887549ead4a40f18d1179df9dff124612440a403c2cb2338fb98bfe5f6850af11f6a7e97a04350fc9d37877060f8d18e8f66de31c77b3504c93cf6a47017ea700a48625c4159e3f7e75b52ff4ea23bc13db77371001").unwrap();
 
         let client = create_client(
             Box::new(MockLogger {}),
             xmtp_api_grpc::LOCALHOST_ADDRESS.to_string(),
             false,
             Some(tmp_path()),
             None,
-            inbox_id.to_string(),
+            account_address.to_string(),
             LegacyIdentitySource::KeyGenerator,
-            Some(legacy_signed_private_key_proto),
+            Some(legacy_keys),
         )
         .await
         .unwrap();

xmtp_id/src/associations/signature.rs

@@ -1,6 +1,5 @@
-use std::array::TryFromSliceError;
-
 use crate::constants::INSTALLATION_KEY_SIGNATURE_CONTEXT;
+use std::array::TryFromSliceError;
 
 use super::MemberIdentifier;
 use async_trait::async_trait;
@@ -322,17 +321,21 @@ impl LegacyDelegatedSignature {
 #[async_trait]
 impl Signature for LegacyDelegatedSignature {
     async fn recover_signer(&self) -> Result<MemberIdentifier, SignatureError> {
+        // TODO: Actually verify the private key signature, in addition to extracting the wallet
+        // address from the SignedPublicKey
         // 1. Verify the RecoverableEcdsaSignature
-        let legacy_signer = self.legacy_key_signature.recover_signer().await?;
+        // let legacy_signer = self.legacy_key_signature.recover_signer().await?;
 
         // 2. Verify the [LegacySignedPublicKeyProto] and make sure it matches to the legacy_signer
         let signed_public_key: ValidatedLegacySignedPublicKey =
             self.signed_public_key_proto.clone().try_into()?;
-        if MemberIdentifier::Address(signed_public_key.account_address()) != legacy_signer {
-            return Err(SignatureError::Invalid);
-        }
+        // if MemberIdentifier::Address(signed_public_key.account_address()) != legacy_signer {
+        //     // return Err(SignatureError::Invalid);
+        // }
 
-        Ok(legacy_signer)
+        Ok(MemberIdentifier::Address(
+            signed_public_key.account_address(),
+        ))
     }
 
     fn signature_kind(&self) -> SignatureKind {

xmtp_mls/src/builder.rs

@@ -113,7 +113,7 @@ where
 mod tests {
     use xmtp_api_grpc::grpc_api_helper::Client as GrpcClient;
     use xmtp_cryptography::utils::generate_local_wallet;
-    use xmtp_id::associations::RecoverableEcdsaSignature;
+    use xmtp_id::associations::{generate_inbox_id, RecoverableEcdsaSignature};
 
     use super::{ClientBuilder, IdentityStrategy};
     use crate::{
@@ -179,6 +179,29 @@ mod tests {
         assert!(!client.installation_public_key().is_empty());
     }
 
+    #[tokio::test]
+    async fn legacy_keys() {
+        // This test is supposed to be run with a fresh state where account_address has not been registered.
+        // Subsequent runs will use a different code path.
+        let account_address = "0x0bD00B21aF9a2D538103c3AAf95Cb507f8AF1B28".to_lowercase();
+        let legacy_keys = hex::decode("0880bdb7a8b3f6ede81712220a20ad528ea38ce005268c4fb13832cfed13c2b2219a378e9099e48a38a30d66ef991a96010a4c08aaa8e6f5f9311a430a41047fd90688ca39237c2899281cdf2756f9648f93767f91c0e0f74aed7e3d3a8425e9eaa9fa161341c64aa1c782d004ff37ffedc887549ead4a40f18d1179df9dff124612440a403c2cb2338fb98bfe5f6850af11f6a7e97a04350fc9d37877060f8d18e8f66de31c77b3504c93cf6a47017ea700a48625c4159e3f7e75b52ff4ea23bc13db77371001").unwrap();
+        let client = ClientBuilder::new(IdentityStrategy::CreateIfNotFound(
+            account_address.to_string(),
+            Some(legacy_keys),
+        ))
+        .temp_store()
+        .local_grpc()
+        .await
+        .build()
+        .await
+        .unwrap();
+
+        assert_eq!(
+            client.inbox_id(),
+            generate_inbox_id(&account_address.to_string(), &0)
+        );
+    }
+
     #[tokio::test]
     async fn identity_persistence_test() {
         let tmpdb = tmp_path();

xmtp_mls/src/identity.rs

@@ -9,12 +9,12 @@ use crate::{
     configuration::{CIPHERSUITE, GROUP_MEMBERSHIP_EXTENSION_ID, MUTABLE_METADATA_EXTENSION_ID},
     storage::StorageError,
     xmtp_openmls_provider::XmtpOpenMlsProvider,
-    InboxOwner, XmtpApi,
+    XmtpApi,
 };
 use crate::{builder::ClientBuilderError, storage::EncryptedMessageStore};
 use crate::{Fetch, Store};
 use ed25519_dalek::SigningKey;
-use ethers::signers::{LocalWallet, WalletError};
+use ethers::signers::WalletError;
 use log::debug;
 use log::info;
 use openmls::prelude::tls_codec::Serialize;
@@ -34,6 +34,7 @@ use openmls_traits::OpenMlsProvider;
 use prost::Message;
 use sha2::{Digest, Sha512};
 use thiserror::Error;
+use xmtp_id::associations::ValidatedLegacySignedPublicKey;
 use xmtp_id::{
     associations::{
         builder::{SignatureRequest, SignatureRequestBuilder, SignatureRequestError},
@@ -111,6 +112,8 @@ pub enum IdentityError {
     #[error(transparent)]
     SignatureRequestBuilder(#[from] SignatureRequestError),
     #[error(transparent)]
+    Signature(#[from] xmtp_id::associations::SignatureError),
+    #[error(transparent)]
     BasicCredential(#[from] BasicCredentialError),
     #[error("Legacy key re-use")]
     LegacyKeyReuse,
@@ -230,6 +233,7 @@ impl Identity {
                     .await?,
                 ))
                 .await?;
+
             let identity_update = signature_request.build_identity_update()?;
             api_client.publish_identity_update(identity_update).await?;
 
@@ -239,7 +243,6 @@ impl Identity {
                 credential: create_credential(inbox_id)?,
                 signature_request: None,
             };
-
             Ok(identity)
         } else {
             let nonce = rand::random::<u64>();
@@ -410,14 +413,15 @@ async fn sign_with_installation_key(
 fn legacy_key_to_address(legacy_signed_private_key: Vec<u8>) -> Result<String, IdentityError> {
     let legacy_signed_private_key_proto =
         LegacySignedPrivateKeyProto::decode(legacy_signed_private_key.as_slice())?;
-    let signed_private_key::Union::Secp256k1(secp256k1) = legacy_signed_private_key_proto
-        .union
-        .ok_or(IdentityError::MalformedLegacyKey(
-            "Missing secp256k1.union field".to_string(),
-        ))?;
-    let legacy_private_key = secp256k1.bytes;
-    let wallet: LocalWallet = LocalWallet::from_bytes(&legacy_private_key)?;
-    Ok(wallet.get_address())
+    let validated_legacy_public_key: ValidatedLegacySignedPublicKey =
+        legacy_signed_private_key_proto
+            .public_key
+            .ok_or(IdentityError::MalformedLegacyKey(
+                "Missing public_key field".to_string(),
+            ))?
+            .try_into()?;
+
+    Ok(validated_legacy_public_key.account_address())
 }
 
 async fn sign_with_legacy_key(