feat: receiver recovers both the local identity and the account#137
feat: receiver recovers both the local identity and the account#137kaichaosun wants to merge 6 commits into
Conversation
kaichaosun
force-pushed
the
accounts-content
branch
from
7b6e8f1 to
2e8d9fb
Compare
jazzz
left a comment
jazzz
left a comment
There was a problem hiding this comment.
Love the ambition here, but there are some blockers with the current approach.
The core issue is that the account crate seems focused on creating credentials. Thats not a terrible approach, however its not quite what we are looking for. This moves message validation to the moment the credential is created, which causes issues with the authentication model.
We need lazy evaluation at the Client level, in order to support revocation.
| /// MLS guarantees the message was signed by the sender leaf's key, which is | ||
| /// the device (LocalIdentity) key. We pair that authoritative key with the |
There was a problem hiding this comment.
[Boulder] This statement is not true. MLS content messages are not signed.
There was a problem hiding this comment.
It looks processed messages are checked with signing methods,
https://github.com/openmls/openmls/blob/main/openmls/src/group/mls_group/processing.rs#L591
| let Sender::Member(leaf_index) = processed.sender() else { | ||
| return None; | ||
| }; | ||
| // The leaf's signature key is the device key MLS verified the message | ||
| // against — the trustworthy LocalIdentity, not anything self-asserted in | ||
| // the credential content. | ||
| let member = self.mls_group.member_at(*leaf_index)?; | ||
| let key_bytes: [u8; 32] = member.signature_key.as_slice().try_into().ok()?; | ||
| let device_key = Ed25519VerifyingKey::from_bytes(&key_bytes).ok()?; | ||
| logos_account::resolve_sender(processed.credential().serialized_content(), &device_key).ok() |
There was a problem hiding this comment.
[Boulder] Do not perform the leaf_key lookup manually. Use the populated Credential field
There was a problem hiding this comment.
ProcessedMessage doesn't expose the signing key, credential() returns a Credential, which is just { credential_type, serialized_credential_content } (our account claim); the device key isn't on it.
OpenMLS builds a CredentialWithKey during verification but doesn't surface it on the processed message. The leaf's signature_key is only available via the ratchet tree (member_at).
If we instead embedded the device key in the credential content and read it back, it'd be self-asserted. MLS doesn't enforce that content matches the leaf's real signature_key, so a member could put a different key there.
Is there a populated field you had in mind that I'm missing?
| // de-mls carries only the sender's member-id (the LocalIdentity | ||
| // display string); it does not yet expose an account-bound | ||
| // credential the way GroupV1 does. Surface the LocalIdentity and | ||
| // treat it as its own Account (the testnet 1:1 case). |
There was a problem hiding this comment.
[!] Demls encodes the credential a s ConversationMessage::sender. You can retrieve a credential by deserializing into an Openmls::Credential
There was a problem hiding this comment.
de-mls verifies each message against its own generated leaf signing key, which is not libchat's device key.
So unlike group-v1 where the MLS leaf key is the device key, groupv2 has no link between the key it verifies and libchat's account/device model. That's the whole gap.
I updated a bit, but looks like there are more work needed, and I'm not sure if I'm in the correct direction. To split the concern, let's keep the scope of this PR on group-v1.
There was a problem hiding this comment.
groupv2 has no link between the key it verifies and libchat's account/device model. That's the whole gap.
GroupV2 will use the same keypackages and MLS config as GroupV1. Its some work to migrate the code, but that is the integration point. Crendentials will contain an LocalSignerId, and an AccountId.
de-mls verifies each message against its own generated leaf signing key
For completeness this is technically not true. What is true is that openmls associates a received content message with a given credential. If the wiring from openmls::ProcessedMessage to ConvoOutcome may be missing, happy to complete this if that would help.
Let's keep the scope of this PR on group-v1.
Agreed. Or go even higher, and implement strictly at the Core/Client interface and the wiring can be completed later.
| let raya_sender = harness | ||
| .saro() | ||
| .sender_of(&convo_id, M_R1) | ||
| .expect("Saro should see who sent Raya's message") | ||
| .clone(); | ||
| assert_eq!( | ||
| raya_sender.account, raya_sender.local_identity, | ||
| "single-key testnet account resolves Account == LocalIdentity" | ||
| ); |
There was a problem hiding this comment.
[Dust] Consider extending TestClient::check to accept a, expected sender. If a test checks that a message arrived it should also check the sender is valid
| /// Produce the Account's endorsement of `device`, using the Account's signing | ||
| /// capability. `sign` is the account authority's signer (a local key on testnet, | ||
| /// or an external wallet/enclave), so the closure is fallible. | ||
| pub fn endorse_local_identity<E>( | ||
| account: &Ed25519VerifyingKey, | ||
| device: &Ed25519VerifyingKey, | ||
| sign: impl FnOnce(&[u8]) -> Result<Ed25519Signature, E>, | ||
| ) -> Result<Ed25519Signature, E> { | ||
| sign(&endorsement_message(account, device)) | ||
| } |
There was a problem hiding this comment.
[Pebble] This approach does not meet the requirements. Introducing signed bundled, embedded with each payload makes revocation nearly impossible.
https://roadmap.logos.co/messaging/furps/application/group_chat. -> F6
- Account should support a mechanism for authorizing a LocalSigner.
- Account creates an AccountRecord and submits it to the AccountRegistry.
- AccountRecord(AccountData, Signature_over_account_data);
- AccountData( account_public_key, lamport_timestamp, number of entries, AuthorizedSignerPublicKey, AuthorizedSignerPublicKey ... );
There was a problem hiding this comment.
Yeah, good to know that, it was proposed before I read your message to use account service as the verification check for device -> account.
I just updated to the new approach.
kaichaosun
force-pushed
the
accounts-content
branch
from
3b83873 to
a5d7604
Compare
2 participants
The changes binds the Account into the MLS leaf credential so a receiver recovers both — verifiably, with no directory round-trip.
logos-account:credentialmodule —encode_credentialbuild an account-endorsed credential;resolve_senderverifies it against the leaf device key and returnsMessageSender { account, local_identity }.conversations:get_credentialemits the account-bound credential;GroupV1resolves and attaches the sender;ConvoOutcomegainssender. GroupV2 surfaces the LocalIdentity (account resolution is a de-mls follow-up).logos-chat:Event::MessageReceivedgainssender.Tests: credential codec units + GroupV1 integration assert the verified sender and that distinct identities resolve to distinct accounts.
Related to #109