feat: add unified keystore connection [WPB-23773] by coriolinus · Pull Request #1933 · wireapp/core-crypto

coriolinus · 2026-03-11T16:23:44Z

What's new in this PR

Create a rusqlite connection for the keystore that's usable both in native and wasm compilation environments.

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: ... ³

https://sparkbox.com/foundry/semantic_commit_messages
https://github.com/wireapp/.github#usage
E.g. feat(conversation-list): Sort conversations by most emojis in the title #SQPIT-764.

keystore/src/unified_connection/filesystem.rs

keystore/src/unified_connection/mod.rs

keystore/src/entities/helpers.rs

keystore/src/entities/mls.rs

SimonThormeyer · 2026-03-17T09:21:02Z

keystore/src/entities/mls.rs

+    fn save(&self, tx: &rusqlite::Transaction) -> crate::CryptoKeystoreResult<()> {
+        let hash = crate::Sha256Hash::hash_from(&self.pk);
+        let mut stmt =
+            tx.prepare_cached("INSERT OR REPLACE INTO mls_hpke_private_keys (pk_sha256, pk, sk) VALUES (?, ?, ?)")?;


optional: wanna use named parameters here, just to make it more robust against errors when refactoring?

SimonThormeyer · 2026-03-17T09:21:42Z

keystore/src/entities/mls.rs

+    fn save(&self, tx: &rusqlite::Transaction) -> crate::CryptoKeystoreResult<()> {
+        let hash = crate::Sha256Hash::hash_from(&self.pk);
+        let mut stmt = tx
+            .prepare_cached("INSERT OR REPLACE INTO mls_encryption_keypairs (pk_sha256, pk, sk) VALUES (?, ?, ?)")?;


optional: wanna use named parameters here, just to make it more robust against errors when refactoring?

SimonThormeyer · 2026-03-17T09:22:04Z

keystore/src/entities/mls.rs

+    fn save(&self, tx: &rusqlite::Transaction) -> crate::CryptoKeystoreResult<()> {
+        let hash = crate::Sha256Hash::hash_from(&self.psk_id);
+        let mut stmt =
+            tx.prepare_cached("INSERT OR REPLACE INTO mls_psk_bundles (id_sha256, psk_id, psk) VALUES (?, ?, ?)")?;


optional: wanna use named parameters here, just to make it more robust against errors when refactoring?

SimonThormeyer · 2026-03-17T09:22:28Z

keystore/src/entities/proteus.rs

+    type AutoGeneratedFields = ();
+
+    fn save(&self, tx: &rusqlite::Transaction) -> crate::CryptoKeystoreResult<()> {
+        let mut stmt = tx.prepare_cached("INSERT INTO proteus_prekeys (id, key) VALUES (?, ?)")?;


optional: wanna use named parameters here, just to make it more robust against errors when refactoring?

SimonThormeyer · 2026-03-17T09:22:36Z

keystore/src/entities/proteus.rs

+    type AutoGeneratedFields = ();
+
+    fn save(&self, tx: &rusqlite::Transaction) -> crate::CryptoKeystoreResult<()> {
+        let mut stmt = tx.prepare_cached("INSERT INTO proteus_identities (sk, pk) VALUES (?, ?)")?;


optional: wanna use named parameters here, just to make it more robust against errors when refactoring?

keystore/src/entities/mls/e2ei_acme_ca.rs

keystore/Cargo.toml

keystore/src/unified_connection/mod.rs

SimonThormeyer

Just a few comments/questions, nothing blocking.
Great work! Looking forward to what's coming next.

coriolinus · 2026-03-17T11:54:36Z

I'm going to skip the named parameters suggestions for now, for two reasons:

These impls are easier to understand when they're as close as possible to a copy-paste job of the existing impls as possible
Each of the suggested named parameters impls is a one-liner; IMO that's simple enough that positional parameters are sufficient.

This isn't really part of the connection anyway.

This type wraps a bare rusqlite `Connection` when in memory or running locally, and one with a IDB VFS when on `target_os = "unknown"`. It also refactors the initialization a bit to minimize code duplication. The `ConnectionWrapper` and `RusqliteConnection` items aren't really final; they work for the moment, but I don't love them. Need to put a bit more thought into how precisely I want this all to work.

…ction This may not be the optimal placement for these; I want to revisit the project layout again after we have deleted the old connection code. For now, they're all in place at least.

We need to have some kind of abstraction in the unified database, because we want to support operations like "wipe" which involve simply deleting the database. But there's no particular reason that abstraction needs to include the actual connection, so we separate that part out. This turns out to simplify everything.

…atabase We can't fully support this yet without adjusting certain intefaces in `KeystoreTransaction`, but we can get it started.

Like it or not, we still need this capability at least on known-os platforms, so here it is.

This means we don't have to mess with the `FetchFromDatabase` trait or any writeable entity methods; people can just use the relevant `UnifiedEntity` traits directly. We probably want to change this up in the future. This approach means that I save a lot of work right now messing with traits which are in flux, but it also means that it's possible to bypass the CC transaction entirely and just do writes directly to the database. See WPB-24064. In the meantime, this is worth it just because it means we don't have to write _any_ entity-specific code on the `Database` for now.

Major changes: 1. everything is sync because sqlite is not async 2. connection, transaction are specified explicitly instead of as associated types 3. eliminated `EntityBase`

These act like the old generic helpers, but tuned for the new unified entity traits.

… entities

It's much easier to work on a bunch of individual focused files than a few massive files containing a ton of entities each.

These files only contain the one entity, so name them appropriately.

Somehow I'd configured out an essential trait import also...

Turns out that refinery is very strict about migration files, hashing the file itself to determine what the migration looks like. So adding a trailing newline there breaks our ability to ever migrate from pre-v9 databases, because the migration hash differs. So we need to retain the old migrations byte-for-byte.

coriolinus force-pushed the prgn/feat/23773-unified-keystore-connection branch 2 times, most recently from 8045494 to c25589b Compare March 16, 2026 11:46

coriolinus marked this pull request as ready for review March 16, 2026 13:11

coriolinus requested a review from a team March 16, 2026 13:11

SimonThormeyer reviewed Mar 16, 2026

View reviewed changes

keystore/src/unified_connection/filesystem.rs Show resolved Hide resolved