Skip to content

feat: remove identities module#1756

Merged
SimonThormeyer merged 23 commits intomainfrom
simon/remove-ciphersuite-from-mls-init-WPB-21629
Jan 21, 2026
Merged

feat: remove identities module#1756
SimonThormeyer merged 23 commits intomainfrom
simon/remove-ciphersuite-from-mls-init-WPB-21629

Conversation

@SimonThormeyer
Copy link
Copy Markdown
Member

What's new in this PR


PR Submission Checklist for internal contributors
  • The PR Title
    • conforms to the style of semantic commits messages¹ supported in Wire's Github Workflow²
    • contains a reference JIRA issue number like SQPIT-764
    • answers the question: If merged, this PR will: ... ³
  1. https://sparkbox.com/foundry/semantic_commit_messages
  2. https://github.com/wireapp/.github#usage
  3. E.g. feat(conversation-list): Sort conversations by most emojis in the title #SQPIT-764.

@SimonThormeyer SimonThormeyer force-pushed the simon/remove-ciphersuite-from-mls-init-WPB-21629 branch from 248d04c to 0f07043 Compare January 16, 2026 16:14
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jan 16, 2026

🐰 Bencher Report

Branchsimon/remove-ciphersuite-from-mls-init-WPB-21629
Testbedubuntu-latest

⚠️ WARNING: No Threshold found!

Without a Threshold, no Alerts will ever be generated.

Click here to create a new Threshold
For more information, see the Threshold documentation.
To only post results if a Threshold exists, set the --ci-only-thresholds flag.

Click to view all benchmark results
BenchmarkLatencymicroseconds (µs)
Commit add f(group size)/cs1/mem/1002📈 view plot
⚠️ NO THRESHOLD
13,913.00 µs
Commit add f(group size)/cs1/mem/2📈 view plot
⚠️ NO THRESHOLD
632.79 µs
Commit add f(group size)/cs1/mem/202📈 view plot
⚠️ NO THRESHOLD
3,401.30 µs
Commit add f(group size)/cs1/mem/402📈 view plot
⚠️ NO THRESHOLD
6,052.40 µs
Commit add f(group size)/cs1/mem/602📈 view plot
⚠️ NO THRESHOLD
9,318.60 µs
Commit add f(group size)/cs1/mem/802📈 view plot
⚠️ NO THRESHOLD
12,546.00 µs
Commit add f(number clients)/cs1/mem/1002📈 view plot
⚠️ NO THRESHOLD
780,090.00 µs
Commit add f(number clients)/cs1/mem/2📈 view plot
⚠️ NO THRESHOLD
685.71 µs
Commit add f(number clients)/cs1/mem/202📈 view plot
⚠️ NO THRESHOLD
65,613.00 µs
Commit add f(number clients)/cs1/mem/402📈 view plot
⚠️ NO THRESHOLD
172,470.00 µs
Commit add f(number clients)/cs1/mem/602📈 view plot
⚠️ NO THRESHOLD
336,670.00 µs
Commit add f(number clients)/cs1/mem/802📈 view plot
⚠️ NO THRESHOLD
537,750.00 µs
Commit pending proposals f(group size)/cs1/mem/1002📈 view plot
⚠️ NO THRESHOLD
88,829.00 µs
Commit pending proposals f(group size)/cs1/mem/2📈 view plot
⚠️ NO THRESHOLD
18,784.00 µs
Commit pending proposals f(group size)/cs1/mem/202📈 view plot
⚠️ NO THRESHOLD
32,888.00 µs
Commit pending proposals f(group size)/cs1/mem/402📈 view plot
⚠️ NO THRESHOLD
43,933.00 µs
Commit pending proposals f(group size)/cs1/mem/602📈 view plot
⚠️ NO THRESHOLD
59,130.00 µs
Commit pending proposals f(group size)/cs1/mem/802📈 view plot
⚠️ NO THRESHOLD
70,285.00 µs
Commit pending proposals f(pending size)/cs1/mem/1📈 view plot
⚠️ NO THRESHOLD
13,615.00 µs
Commit pending proposals f(pending size)/cs1/mem/101📈 view plot
⚠️ NO THRESHOLD
87,591.00 µs
Commit pending proposals f(pending size)/cs1/mem/21📈 view plot
⚠️ NO THRESHOLD
26,823.00 µs
Commit pending proposals f(pending size)/cs1/mem/41📈 view plot
⚠️ NO THRESHOLD
43,772.00 µs
Commit pending proposals f(pending size)/cs1/mem/61📈 view plot
⚠️ NO THRESHOLD
57,858.00 µs
Commit pending proposals f(pending size)/cs1/mem/81📈 view plot
⚠️ NO THRESHOLD
73,030.00 µs
Commit remove f(group size)/cs1/mem/1002📈 view plot
⚠️ NO THRESHOLD
8,766.10 µs
Commit remove f(group size)/cs1/mem/2📈 view plot
⚠️ NO THRESHOLD
485.79 µs
Commit remove f(group size)/cs1/mem/202📈 view plot
⚠️ NO THRESHOLD
1,824.40 µs
Commit remove f(group size)/cs1/mem/402📈 view plot
⚠️ NO THRESHOLD
3,244.50 µs
Commit remove f(group size)/cs1/mem/602📈 view plot
⚠️ NO THRESHOLD
4,913.20 µs
Commit remove f(group size)/cs1/mem/802📈 view plot
⚠️ NO THRESHOLD
6,458.50 µs
Commit remove f(number clients)/cs1/mem/1002📈 view plot
⚠️ NO THRESHOLD
11,301.00 µs
Commit remove f(number clients)/cs1/mem/2📈 view plot
⚠️ NO THRESHOLD
115,550.00 µs
Commit remove f(number clients)/cs1/mem/202📈 view plot
⚠️ NO THRESHOLD
94,729.00 µs
Commit remove f(number clients)/cs1/mem/402📈 view plot
⚠️ NO THRESHOLD
74,030.00 µs
Commit remove f(number clients)/cs1/mem/602📈 view plot
⚠️ NO THRESHOLD
53,138.00 µs
Commit remove f(number clients)/cs1/mem/802📈 view plot
⚠️ NO THRESHOLD
32,154.00 µs
Commit update f(group size)/cs1/mem/1002📈 view plot
⚠️ NO THRESHOLD
115,560.00 µs
Commit update f(group size)/cs1/mem/2📈 view plot
⚠️ NO THRESHOLD
651.84 µs
Commit update f(group size)/cs1/mem/202📈 view plot
⚠️ NO THRESHOLD
23,909.00 µs
Commit update f(group size)/cs1/mem/402📈 view plot
⚠️ NO THRESHOLD
46,945.00 µs
Commit update f(group size)/cs1/mem/602📈 view plot
⚠️ NO THRESHOLD
70,531.00 µs
Commit update f(group size)/cs1/mem/802📈 view plot
⚠️ NO THRESHOLD
92,988.00 µs
🐰 View full continuous benchmarking report in Bencher

@SimonThormeyer SimonThormeyer force-pushed the simon/remove-ciphersuite-from-mls-init-WPB-21629 branch from 0f07043 to 7475791 Compare January 19, 2026 13:28
Comment thread crypto/src/mls/session/credential.rs Outdated
Comment thread crypto/src/mls/session/credential.rs Outdated
@coriolinus
Copy link
Copy Markdown
Contributor

WDYT of excluding the public key from the credential ref--it might be large--and instead just storing the Sha256Sum in there instead?

@SimonThormeyer SimonThormeyer force-pushed the simon/remove-ciphersuite-from-mls-init-WPB-21629 branch from 7475791 to 2d4b6fe Compare January 19, 2026 14:02
@SimonThormeyer
Copy link
Copy Markdown
Member Author

WDYT of excluding the public key from the credential ref--it might be large--and instead just storing the Sha256Sum in there instead?

Sounds good! Would you mind if we did that as a separate item?

@coriolinus
Copy link
Copy Markdown
Contributor

Sounds good! Would you mind if we did that as a separate item?

That's fine, I was just thinking of it here because it was on my mental queue of things to do once we got rid of the "get most recent commit" functionality, which this PR implements.

@SimonThormeyer SimonThormeyer force-pushed the simon/remove-ciphersuite-from-mls-init-WPB-21629 branch 5 times, most recently from b7a775a to 4a4d932 Compare January 20, 2026 15:49
@SimonThormeyer SimonThormeyer marked this pull request as ready for review January 20, 2026 15:50
@SimonThormeyer SimonThormeyer requested a review from a team January 20, 2026 15:50
@SimonThormeyer SimonThormeyer force-pushed the simon/remove-ciphersuite-from-mls-init-WPB-21629 branch from afa8e34 to d62693e Compare January 20, 2026 15:59
Copy link
Copy Markdown
Contributor

@coriolinus coriolinus left a comment

Choose a reason for hiding this comment

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

This is great! A few nits, but I think this is a real improvement.

Now that the identities module is gone, I'm pretty sure that the door is now open to delete all the 1s delays per added credential, because we don't need them to have unique created_at values anymore. I'm agnostic about whether you want to do that in this PR or in a follow-up though.

Comment thread crypto/src/mls/conversation/mod.rs Outdated
Comment on lines -242 to -243
/// If no credentials are present in the keystore, then one _must_ be created and added to the
/// session before it can be used.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This comment seems relevant still.

Copy link
Copy Markdown
Member Author

@SimonThormeyer SimonThormeyer Jan 21, 2026

Choose a reason for hiding this comment

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

Really? Now that mls_init() doesn't do anything with credentials anymore it seems a little off to mention them here.

@SimonThormeyer SimonThormeyer force-pushed the simon/remove-ciphersuite-from-mls-init-WPB-21629 branch 2 times, most recently from d54dcaa to 353d7d5 Compare January 21, 2026 08:01
Removing this function is the first step to unlock removal of the
`identities` module which would allow querying for most recently added
credentials.

`e2ei_rotate()` would just take the most recent x509 credential from the
DB and set it for the given conversation. This removes that implicit
behavior. To allow that, we're now returning a credential ref from
`save_x509_credential()`.

Note: we're taking a shortcut here on the FFI layer - we're not anymore
returning the `NewCrlDistributionPoints` from `save_x509_credential()`.
However, CRL handling is going to be rewored in WPB-19580, so it's not
worth bothering too much.
This is, much like the parent commit, just to produce an intermediate
state after this PR that enables passing tests without _recent
credential_ logic. This method is going to disappear by the latest in
WPB-19579.
No more implicit _most recent_ credential logic
This is the only real use case for the `identities` module
`find_credentials()` does a superset of what this function used to do -
except relying on the implicit _most recent_ logic.
In the parent commit, we just replaced all instances of
`find_most_recent_credential()` with
`Conversation.find_current_credential()` to cause the code to compile.

This is good enough in some cases, but in others we have to be smarter
than that:

When updating credentials, we need to ensure that we're signing with the
signature key of the leaf node we're inserting, but
`find_current_credential()` gives us the old one.
This commit takes care of that.
@SimonThormeyer SimonThormeyer force-pushed the simon/remove-ciphersuite-from-mls-init-WPB-21629 branch from 353d7d5 to d4ddda1 Compare January 21, 2026 08:52
@SimonThormeyer SimonThormeyer merged commit d4ddda1 into main Jan 21, 2026
54 of 55 checks passed
@SimonThormeyer SimonThormeyer deleted the simon/remove-ciphersuite-from-mls-init-WPB-21629 branch January 21, 2026 09:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants