Skip to content

feat: implement new credential management api [WPB-19572] [WPB-15973] #1522

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.

Sign up for GitHub

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

Jump to bottom
Draft
wants to merge 16 commits into
base: main
Choose a base branch
from
Draft

feat: implement new credential management api [WPB-19572] [WPB-15973] #1522

wants to merge 16 commits into from
+1,717 −1,200

Conversation

coriolinus
Copy link
Contributor

@coriolinus coriolinus commented Oct 2, 2025 •
edited
Loading

What's new in this PR

  • Expose a Credential type as a first-class struct.
  • Credential has ciphersuite not just signature_scheme see here: if an identity uses signature schemes as the keys to an identity map, then it cannot have two identities with different ciphersuites which share the same signature scheme. And if that is impossible, then storing only the signature scheme is sufficient.
  • basic credential types can be instantiated from all bindings
  • tests of creating the credential type from all bindings
    • ts
    • kotlin
    • swift
  • Add a CredentialRef type from which we can get a credential from the db
  • Add Session methods:
    • add_credential <- check the client id matches, stores in keystore
    • remove_credential <- must first check that the credential is not used in any conversation and remove both the credential and all key packages that were generated from it
    • find_credentials <- optional filters: credential_type, ciphersuite, signature scheme
    • get_credentials <- all credentials without filtering
  • tests:
    • add_credential
      • ts
      • kotlin
      • swift
    • remove_credential
      • ts
      • kotlin
      • swift
    • find_credentials
      • ts
      • kotlin
      • swift
    • get_credentials
      • ts
      • kotlin
      • swift
  • rm existing credential API
  • adjust existing tests which required old credential API
  • changelog

@coriolinus
chore(mls-provider): impl OpenMlsCrypto for &MlsCryptoProvider
79dff56 
This passthrough implementation means that an `&MlsCryptoProvider` instance,
ubiquitous in the `core-crypto` codebase as `backend`, can be used
anywhere the `OpenMlsCrypto` trait is needed.
@coriolinus
chore: impl Obfuscate for SignatureKeyPair
c2cc125
@coriolinus
chore: enable Clone impl for SignatureKeyPair
feb2e72
@coriolinus
feat: expose Credential type
0c64ea8 
This type is a first-class credential type exposed to the world.
It can be created independently of any client instance and the
database; it lives in memory only.

Strongly based on the old `CredentialBundle` type.
@coriolinus coriolinus force-pushed the prgn/feat/19573-credential-management-api branch from e241695 to e0c43af Compare October 2, 2025 15:23
@github-actions GitHub Actions
Copy link

github-actions bot commented Oct 2, 2025 •
edited
Loading

🐰 Bencher Report

Branchprgn/feat/19573-credential-management-api
Testbedubuntu-latest
Click to view all benchmark results
BenchmarkLatencymicroseconds (µs)
Commit add f(group size)/cs1/mem/1002📈 view plot
🚷 view threshold		17,887.00 µs
Commit add f(group size)/cs1/mem/2📈 view plot
🚷 view threshold		742.69 µs
Commit add f(group size)/cs1/mem/202📈 view plot
🚷 view threshold		4,400.80 µs
Commit add f(group size)/cs1/mem/402📈 view plot
🚷 view threshold		7,984.50 µs
Commit add f(group size)/cs1/mem/602📈 view plot
🚷 view threshold		12,104.00 µs
Commit add f(group size)/cs1/mem/802📈 view plot
🚷 view threshold		15,215.00 µs
Commit add f(number clients)/cs1/mem/1002📈 view plot
🚷 view threshold		1,008,300.00 µs
Commit add f(number clients)/cs1/mem/2📈 view plot
🚷 view threshold		751.59 µs
Commit add f(number clients)/cs1/mem/202📈 view plot
🚷 view threshold		80,345.00 µs
Commit add f(number clients)/cs1/mem/402📈 view plot
🚷 view threshold		218,300.00 µs
Commit add f(number clients)/cs1/mem/602📈 view plot
🚷 view threshold		426,450.00 µs
Commit add f(number clients)/cs1/mem/802📈 view plot
🚷 view threshold		677,340.00 µs
Commit pending proposals f(group size)/cs1/mem/1002📈 view plot
🚷 view threshold		117,040.00 µs
Commit pending proposals f(group size)/cs1/mem/2📈 view plot
🚷 view threshold		23,176.00 µs
Commit pending proposals f(group size)/cs1/mem/202📈 view plot
🚷 view threshold		42,109.00 µs
Commit pending proposals f(group size)/cs1/mem/402📈 view plot
🚷 view threshold		58,015.00 µs
Commit pending proposals f(group size)/cs1/mem/602📈 view plot
🚷 view threshold		77,739.00 µs
Commit pending proposals f(group size)/cs1/mem/802📈 view plot
🚷 view threshold		93,082.00 µs
Commit pending proposals f(pending size)/cs1/mem/1📈 view plot
🚷 view threshold		17,731.00 µs
Commit pending proposals f(pending size)/cs1/mem/101📈 view plot
🚷 view threshold		116,210.00 µs
Commit pending proposals f(pending size)/cs1/mem/21📈 view plot
🚷 view threshold		35,535.00 µs
Commit pending proposals f(pending size)/cs1/mem/41📈 view plot
🚷 view threshold		55,920.00 µs
Commit pending proposals f(pending size)/cs1/mem/61📈 view plot
🚷 view threshold		75,058.00 µs
Commit pending proposals f(pending size)/cs1/mem/81📈 view plot
🚷 view threshold		95,603.00 µs
Commit remove f(group size)/cs1/mem/1002📈 view plot
🚷 view threshold		11,300.00 µs
Commit remove f(group size)/cs1/mem/2📈 view plot
🚷 view threshold		579.81 µs
Commit remove f(group size)/cs1/mem/202📈 view plot
🚷 view threshold		2,483.80 µs
Commit remove f(group size)/cs1/mem/402📈 view plot
🚷 view threshold		4,352.20 µs
Commit remove f(group size)/cs1/mem/602📈 view plot
🚷 view threshold		6,752.40 µs
Commit remove f(group size)/cs1/mem/802📈 view plot
🚷 view threshold		8,837.60 µs
Commit remove f(number clients)/cs1/mem/1002📈 view plot
🚷 view threshold		14,122.00 µs
Commit remove f(number clients)/cs1/mem/2📈 view plot
🚷 view threshold		135,350.00 µs
Commit remove f(number clients)/cs1/mem/202📈 view plot
🚷 view threshold		111,290.00 µs
Commit remove f(number clients)/cs1/mem/402📈 view plot
🚷 view threshold		86,817.00 µs
Commit remove f(number clients)/cs1/mem/602📈 view plot
🚷 view threshold		63,120.00 µs
Commit remove f(number clients)/cs1/mem/802📈 view plot
🚷 view threshold		38,287.00 µs
Commit update f(group size)/cs1/mem/1002📈 view plot
🚷 view threshold		135,310.00 µs
Commit update f(group size)/cs1/mem/2📈 view plot
🚷 view threshold		757.35 µs
Commit update f(group size)/cs1/mem/202📈 view plot
🚷 view threshold		28,175.00 µs
Commit update f(group size)/cs1/mem/402📈 view plot
🚷 view threshold		55,228.00 µs
Commit update f(group size)/cs1/mem/602📈 view plot
🚷 view threshold		83,144.00 µs
Commit update f(group size)/cs1/mem/802📈 view plot
🚷 view threshold		109,230.00 µs
🐰 View full continuous benchmarking report in Bencher

@coriolinus coriolinus force-pushed the prgn/feat/19573-credential-management-api branch from 619e5a4 to 0aebd21 Compare October 2, 2025 16:13
@coriolinus
chore: adjust CC to handle new type names
2084922 
Note that this includes renaming a bunch of keystore entities
from `MlsFoo` to `StoredFoo`, because

1. Those were not properly types owned by MLS
2. They were causing conflicts with actual types owned by MLS
3. The whole situation there was just confusing.
@coriolinus coriolinus force-pushed the prgn/feat/19573-credential-management-api branch from 0aebd21 to 367b0c9 Compare October 6, 2025 08:43
@coriolinus
refactor: swap meaning of Ciphersuite vs MlsCiphersuite
2136b5e 
Previously `MlsCiphersuite` was defined in core-crypto,
and `Ciphersuite` was defined in openmls, which was insane.

This commit adjusts such that `Ciphersuite` is defined in core-crypto,
and `MlsCiphersuite` is a module-level import alias for the definition
in openmls.
@coriolinus coriolinus force-pushed the prgn/feat/19573-credential-management-api branch from 367b0c9 to 2136b5e Compare October 6, 2025 08:56
@coriolinus coriolinus force-pushed the prgn/feat/19573-credential-management-api branch from b2a54e3 to 9d9929f Compare October 6, 2025 12:21
@coriolinus
refactor: MlsCredentialType -> CredentialType
637fcaf 
It's crazy how bad the naming scheme had been. Sometimes in the past
we had `MlsFoo` meaning "the foo entity in the keystore"; in this
case, we had `MlsCredentialType` meaning "the credential type entity
defined in core-crypto."

So we change it: places where we need to reference the Openmls
credential type use it as `MlsCredentialType`. Places using our
own credential type just call it `CredentialType`.
@coriolinus
refactor: drop MlsCredentialType
1b75809 
It provided us exactly one thing: it didn't handle the "unknown" case,
because it incorrectly featured a bug which would trigger a runtime
panic if one was ever encountered.

That was obviously not ideal, so it seemed best to simplify the
type system by dropping it entirely in favor of the upstream type.
@coriolinus
chore: simplify keystore StoredCredential, StoredSignatureKeypair…
53e3083 
… impls
@coriolinus
chore: cargo fmt --all
bdfa4bd
@coriolinus
feat: add the ability to search for credentials per the current impl
65cbf8b 
The bit of our current implementation which handles finding credentials
is split across two different functions in `Session`; it's hard to follow.
But I am fairly confident that this replicates the current logic.

The bit where keypairs get distributed across signing schemes and saved
credentials seems very weird to me. I'd like to refactor that bit of
the database at some point and make it make more sense.
@coriolinus coriolinus force-pushed the prgn/feat/19573-credential-management-api branch from 851c3aa to 65cbf8b Compare October 7, 2025 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@coriolinus