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

Upload key package before publishing identity updates #962

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 3 additions & 2 deletions mls_validation_service/src/handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,8 @@ async fn validate_inbox_id_key_package(
error_message: "".into(),
credential: Some(kp.credential),
installation_public_key: kp.installation_public_key,
expiration: kp.inner.life_time().not_after(),
// We are deprecating the expiration field and key package lifetimes, so stop checking for its existence
expiration: 0,
})
}

Expand Down Expand Up @@ -377,7 +378,7 @@ fn validate_key_package(key_package_bytes: Vec<u8>) -> Result<ValidateKeyPackage
installation_id: verified_key_package.installation_id(),
account_address: verified_key_package.account_address,
credential_identity_bytes: basic_credential.identity().to_vec(),
expiration: verified_key_package.inner.life_time().not_after(),
expiration: 0,
})
}

Expand Down
106 changes: 36 additions & 70 deletions xmtp_api_grpc_gateway/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 6 additions & 9 deletions xmtp_mls/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -419,24 +419,21 @@ where
.collect())
}

/// Register the identity with the network
/// Callers should always check the result of [`text_to_sign`](Self::text_to_sign) before invoking this function.
///
/// If `text_to_sign` returns `None`, then the wallet signature is not required and this function can be called with `None`.
///
/// If `text_to_sign` returns `Some`, then the caller should sign the text with their wallet and pass the signature to this function.
/// Upload a Key Package to the network and publish the signed identity update
/// from the provided SignatureRequest
pub async fn register_identity(
&self,
signature_request: SignatureRequest,
) -> Result<(), ClientError> {
log::info!("registering identity");
self.apply_signature_request(signature_request).await?;
let connection = self.store().conn()?;
let provider = self.mls_provider(connection);
// Register the identity before applying the signature request
let provider = self.mls_provider(self.store().conn()?);
self.identity()
.register(&provider, &self.api_client)
.await?;

self.apply_signature_request(signature_request).await?;

Ok(())
}

Expand Down
8 changes: 5 additions & 3 deletions xmtp_mls/src/identity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -269,9 +269,8 @@ impl Identity {
.await?,
))
.await?;
let identity_update = signature_request.build_identity_update()?;
api_client.publish_identity_update(identity_update).await?;

// Make sure to register the identity before applying the signature request
let identity = Self {
inbox_id: inbox_id.clone(),
installation_keys: signature_keys,
Expand All @@ -281,6 +280,9 @@ impl Identity {

identity.register(provider, api_client).await?;

let identity_update = signature_request.build_identity_update()?;
api_client.publish_identity_update(identity_update).await?;

Ok(identity)
} else {
if inbox_id != generate_inbox_id(&address, &nonce) {
Expand Down Expand Up @@ -424,7 +426,7 @@ impl Identity {
}
let kp = self.new_key_package(provider)?;
let kp_bytes = kp.tls_serialize_detached()?;
api_client.register_installation(kp_bytes, true).await?;
api_client.upload_key_package(kp_bytes, true).await?;

Ok(StoredIdentity::from(self).store(provider.conn_ref())?)
}
Expand Down
3 changes: 0 additions & 3 deletions xmtp_mls/src/verified_key_package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,6 @@ impl VerifiedKeyPackage {
),
);
}
if !kp.life_time().is_valid() {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not so worried about removing these checks, since no key packages are more than a few months old. They all have 6 month expirations, so none will be expired until long after this code is widely deployed.

return Err(KeyPackageVerificationError::InvalidLifetime);
}

Ok(Self::new(kp, account_address))
}
Expand Down
4 changes: 0 additions & 4 deletions xmtp_mls/src/verified_key_package_v2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,6 @@ impl TryFrom<KeyPackage> for VerifiedKeyPackageV2 {
let pub_key_bytes = leaf_node.signature_key().as_slice().to_vec();
let credential = MlsCredential::decode(basic_credential.identity())?;

if !kp.life_time().is_valid() {
return Err(KeyPackageVerificationError::InvalidLifetime);
}

Ok(Self::new(kp, credential, pub_key_bytes))
}
}