Page MenuHomePhabricator

[Identity] Add X3DH RPCs
ClosedPublic

Authored by jon on Mar 13 2023, 10:28 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Nov 10, 9:34 AM
Unknown Object (File)
Fri, Nov 8, 3:32 AM
Unknown Object (File)
Wed, Nov 6, 6:33 AM
Unknown Object (File)
Wed, Nov 6, 2:51 AM
Unknown Object (File)
Tue, Nov 5, 10:17 PM
Unknown Object (File)
Tue, Nov 5, 10:17 PM
Unknown Object (File)
Tue, Nov 5, 10:17 PM
Unknown Object (File)
Tue, Nov 5, 10:17 PM
Subscribers
None

Details

Summary

Separate diff for discussion around X3DH actions

Part of https://linear.app/comm/issue/ENG-3264

Depends on D7003

Test Plan

N/A, meant for consensus around data-plane considerations
of key exchanges.

Diff Detail

Repository
rCOMM Comm
Branch
jonringer/client-proto (branched from master)
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

shared/protos/identity_client.proto
251 ↗(On Diff #23700)

This appears to be the name of a string... did you mean to use RemoteDeviceInfo as a type? If not, should it be lowercased?

jon marked an inline comment as done.

Correct KeyserverKeysResponse structure

ashoat requested changes to this revision.Mar 13 2023, 11:50 AM

Mostly requesting changes to get rid of the keyserverID concept

shared/protos/identity_client.proto
247 ↗(On Diff #23701)

We don't have a concept of a "keyserver ID". Instead, each user may have a keyserver, and that keyserver is generally addressable via the user's ID.

Can we update this to use the same identifier we've defined on line 233?

251 ↗(On Diff #23701)

Users are not required to have a keyserver. What's our behavior if the requested keyserver does not exist? I'm guessing we send an error of some sort?

267 ↗(On Diff #23701)

A little weird that this contains optional string onetimePrekey. Should we consider introducing a new type that is like PreKeyResponse, but only contains preKey and preKeySignature?

This revision now requires changes to proceed.Mar 13 2023, 11:50 AM

Fix refreshPreKeys message. response -> upload

shared/protos/identity_client.proto
247 ↗(On Diff #23701)

From the whitepaper:

Keyserver. A keyserver is a primary device that can also host communities. Keyservers are necessary to host communities because they can handle background processing and respond to queries on-demand. As keyservers are self-hosted, Comm is not able to control who has access to the data on a keyserver. Multiple users can share the same keyserver.

This makes it sound like:

  • Keyservers are independent of a particular user (shared)
  • All keyservers are primary devices (not yet implemented)

But your feedback, makes some additional assumptions:

  • An (already established) user can have 0 or 1 keyservers (no more).
  • All keyservers are directly associated (in some way) with one and only one user.

This brings up a few questions for me for the many-keyserver world:

  • Can you register a new keyserver without an existing user? If you can't, what does a user on one keyserver have to do with another? In discord, when you start a server, you just get added as the initial admin, but not much more. The originating user can delete their account, or leave the server, and the server still moves on.
  • What happens if i want to spin up another keyserver for a different community (e.g. Nix keyserver vs rust keyserver)? Seems like I would need to make another user, as pulling keyserver information by userID would need to return more than one result otherwise.
251 ↗(On Diff #23701)

correct, there would be a grpc::not_found error issued.

267 ↗(On Diff #23701)

This should have been upload, thanks.

ashoat requested changes to this revision.Mar 13 2023, 12:35 PM

I think it would be easier to make the requested change RE keyserverID and follow up with questions you might have for me separately.

Can you register a new keyserver without an existing user? If you can't, what does a user on one keyserver have to do with another? In discord, when you start a server, you just get added as the initial admin, but not much more. The originating user can delete their account, or leave the server, and the server still moves on.

No, you can't. Not sure what the second question is about. User B uses user A's keyserver by merit of being in a community hosted on that keyserver.

What happens if i want to spin up another keyserver for a different community (e.g. Nix keyserver vs rust keyserver)? Seems like I would need to make another user, as pulling keyserver information by userID would need to return more than one result otherwise.

Yes, that's right.

This revision now requires changes to proceed.Mar 13 2023, 12:35 PM
shared/protos/identity_client.proto
282 ↗(On Diff #23703)

Not sure how this is better than PreKeyResponse. Effectively copy-pasting my feedback below:

A little weird that this contains repeated string onetimePrekeys = 3. Should we consider introducing a new type that is like PreKeyRegistrationUpload, but only contains preKey and preKeySignature?

Use userID to find KeyserverInfo

ashoat requested changes to this revision.Mar 13 2023, 1:16 PM

Got no response to this one, requesting changes again

shared/protos/identity_client.proto
282 ↗(On Diff #23703)

Feedback

This revision now requires changes to proceed.Mar 13 2023, 1:16 PM
jon marked 4 inline comments as done.

Use username or wallet ID for looking up keyserver info

ashoat requested changes to this revision.Mar 13 2023, 3:01 PM
ashoat added inline comments.
shared/protos/identity_client.proto
282 ↗(On Diff #23703)

Feedback

This revision now requires changes to proceed.Mar 13 2023, 3:01 PM
jon added inline comments.
shared/protos/identity_client.proto
282 ↗(On Diff #23703)

I don't feel super strong either way, you could just have 0 one-time keys being passed.

Have prekey refresh only pass prekeys and not onetime keys

This revision is now accepted and ready to land.Mar 14 2023, 12:21 PM
This revision was automatically updated to reflect the committed changes.