Upgrade VSS encryption and obfuscation scheme #627

tnull · 2025-08-22T09:14:07Z

In this PR we account for vss-client being renamed to vss-client-ng, upgrade to the latest v0.4.0 release, as well as fix minor issues with the data encryption and key obfuscation scheme currently employed by VssStore.

As these are breaking changes, we'll also include a migration procedure as part of this PR.

ldk-reviews-bot · 2025-08-22T09:14:10Z

👋 Thanks for assigning @jkczyz as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

tnull · 2025-11-12T11:52:27Z

Oh, seems when we also obfuscate the namespaces we might end up with keys that are too long. Will need to get back to the drawing board once more.

jkczyz · 2025-11-13T22:14:17Z

src/io/vss_store.rs

 }

 struct VssStoreInner {
+	schema_version: VssSchemaVersion,


Don't have a strong opinion, but would it be worth using a trait to avoid all the conditional logic?

jkczyz · 2025-11-13T22:23:14Z

src/io/vss_store.rs

+		let runtime_handle = internal_runtime.handle();
+		let schema_store_id = store_id.clone();
+		let schema_key_obfuscator = KeyObfuscator::new(obfuscation_master_key);
+		let (schema_version, blocking_client) = tokio::task::block_in_place(move || {


Ah, I guess we can't if we are determining at runtime. Could maybe still have separate structs V0 and V1 structs with static methods to keep the call sites cleaner.

Hmm, not sure if duplicating all the struct boilerplate and passing in all the fields currently living on self would be much cleaner?

src/io/vss_store.rs

Cargo.toml

src/io/vss_store.rs

Cargo.toml

src/io/vss_store.rs

tnull

Addressed pending feedback

Cargo.toml

src/io/vss_store.rs

We introduce an `enum VssSchemaVersion` that will allow us to discern different behaviors based on the schema version based on a backwards compatible manner.

tnull · 2025-11-17T15:08:51Z

Rebased after #689 has been merged.

tankyleo

reviewed the rebase, good to squash

tnull · 2025-11-17T18:44:05Z

reviewed the rebase, good to squash

Squashed without further changes.

We bump our `vss-client` dependency to include the changes to the `StorableBuilder` interface. Previously, we the `vss-client` didn't allow to set `ChaCha20Poly1305RFC`'s `aad` field, which had the `tag` not commit to any particular key. This would allow a malicious VSS provider to substitute blobs stored under a different key without the client noticing. Here, we now set the `aad` field to the key under which the `Storable` will be stored, ensuring that the retrieved data was originally stored under the key we expected, if `VssSchemaVersion::V1` is set. We also now obfuscate primary and secondary namespaces in the persisted keys, if `VssSchemaVersion::V1` is set. We also account for `StorableBuilder` now taking `data_decryption_key` by reference on `build`/`deconstruct`.

While having it in `VssStoreInner` makes more sense, we now opt to construt the client (soon, clients) in `VssStore` and then hand it down to `VssStoreInner`. That will allow us to use the client once for checking the schema version before actually instantiating `VssStoreInner`.

We previously attempted to drop the internal runtime from `VssStore`, resulting into blocking behavior. While we recently made changes that improved our situation (having VSS CI pass again pretty reliably), we just ran into yet another case where the VSS CI hung (cf. https://github.com/lightningdevkit/vss-server/actions/runs/19023212819/job/54322173817?pr=59). Here we attempt to restore even more of the original pre- ab3d78d / lightningdevkit#623 behavior to get rid of the reappearing blocking behavior, i.e., only use the internal runtime in `VssStore`.

Now that we rely on `reqwest` v0.12.* retry logic as well as client-side timeouts, we can address the remaining TODOs here and simply drop the redundant `tokio::timeout`s we previously added as a safeguard to blocking tasks (even though in the worst cases we saw they never actually fired).

To avoid any blocking cross-runtime behavior that could arise from reusing a single client's TCP connections in different runtime contexts, we here split out the `VssStore` behavior to use one dedicated `VssClient` per context. I.e., we're now using two connections/connection pools and make sure only the `blocking_client` is used in `KVStoreSync` contexts, and `async_client` in `KVStore` contexts.

Since we just made some breaking changes to how exactly we persist data via VSS (now using an `aad` that commits to the key and also obfuscating namespaces), we have to detect which schema version we're on to ensure backwards compatibility. To this end, we here start reading a persisted `vss_schema_version` key in `VssStore::new`. If it is present, we just return the encoded value (right now that can only be V1). If it is not present, it can either mean we run for the first time *or* we're on V0, which we determine checking if anything related to the `bdk_wallet` descriptors are present in the store. If we're running for the first time, we also persist the schema version to save us these rather inefficient steps on following startups.

We add a test case that ensures that a node started and persisted on LDK Node v0.6.2 can still be successfully started with the new schema changes. Co-authored by Claude AI

This is close to the backwards compatibility test we just added for v0, now just making sure we can actually read the data we persisted with our current (V1+) code. Co-authored by Claude AI

tnull added the blocked on upstream label Aug 22, 2025

tnull marked this pull request as draft August 22, 2025 09:14

tnull force-pushed the 2025-08-upgrade-vss-encryption-obfuscation-scheme branch from ac8ae88 to 3490f2a Compare August 22, 2025 09:53

tnull force-pushed the 2025-08-upgrade-vss-encryption-obfuscation-scheme branch 2 times, most recently from bec8829 to e977a6d Compare November 6, 2025 11:41

tnull removed the blocked on upstream label Nov 6, 2025

tnull mentioned this pull request Nov 6, 2025

Only use internal runtime in VssStore #693

Closed

tnull force-pushed the 2025-08-upgrade-vss-encryption-obfuscation-scheme branch 3 times, most recently from c7fc423 to dd9a9e0 Compare November 6, 2025 11:57

tnull added this to Weekly Goals Nov 6, 2025

tnull moved this to Goal: Merge in Weekly Goals Nov 6, 2025

tnull self-assigned this Nov 6, 2025

tnull force-pushed the 2025-08-upgrade-vss-encryption-obfuscation-scheme branch 6 times, most recently from 96db24d to e248dc5 Compare November 10, 2025 15:48

tnull mentioned this pull request Nov 10, 2025

LDK Node v0.7 Tracking issue #443

Closed

8 tasks

tnull added this to the 0.7 milestone Nov 10, 2025

tnull linked an issue Nov 10, 2025 that may be closed by this pull request

Have VSS persistence retry forever #380

Closed

tnull mentioned this pull request Nov 10, 2025

Have VSS persistence retry forever #380

Closed

tankyleo self-requested a review November 10, 2025 21:59

tnull force-pushed the 2025-08-upgrade-vss-encryption-obfuscation-scheme branch 2 times, most recently from 70f30cc to 85fd473 Compare November 12, 2025 10:51

tnull requested a review from jkczyz November 12, 2025 11:46

tnull force-pushed the 2025-08-upgrade-vss-encryption-obfuscation-scheme branch from 173bfbc to c84a361 Compare November 12, 2025 19:10

tnull force-pushed the 2025-08-upgrade-vss-encryption-obfuscation-scheme branch from 564456b to bc2b4e5 Compare November 13, 2025 10:56

jkczyz reviewed Nov 13, 2025

View reviewed changes

tankyleo reviewed Nov 14, 2025

View reviewed changes

tnull force-pushed the 2025-08-upgrade-vss-encryption-obfuscation-scheme branch from bc2b4e5 to 7d78977 Compare November 14, 2025 09:02

tnull commented Nov 14, 2025

View reviewed changes

tnull mentioned this pull request Nov 14, 2025

feat: Added in-memory storage for testing purposes lightningdevkit/vss-server#59

Open

tankyleo reviewed Nov 14, 2025

View reviewed changes

src/io/vss_store.rs Show resolved Hide resolved

src/io/vss_store.rs Show resolved Hide resolved

src/io/vss_store.rs Outdated Show resolved Hide resolved

tnull force-pushed the 2025-08-upgrade-vss-encryption-obfuscation-scheme branch from 7d78977 to 867f0ec Compare November 15, 2025 17:58

Introduce schema versioning

3d5013b

We introduce an `enum VssSchemaVersion` that will allow us to discern different behaviors based on the schema version based on a backwards compatible manner.

tnull force-pushed the 2025-08-upgrade-vss-encryption-obfuscation-scheme branch from 867f0ec to 040dcf1 Compare November 17, 2025 15:07

tnull requested review from jkczyz and tankyleo November 17, 2025 15:08

tankyleo reviewed Nov 17, 2025

View reviewed changes

tnull force-pushed the 2025-08-upgrade-vss-encryption-obfuscation-scheme branch from 040dcf1 to 94318be Compare November 17, 2025 18:43

tnull added 9 commits November 17, 2025 19:44

Bump retries and timeouts considerably

37ee256

Add test ensuring backwards compatibility with VSS schema v0

8e5503c

We add a test case that ensures that a node started and persisted on LDK Node v0.6.2 can still be successfully started with the new schema changes. Co-authored by Claude AI

Add simple test ensuring we can restart from a VSS backend

d2153f2

This is close to the backwards compatibility test we just added for v0, now just making sure we can actually read the data we persisted with our current (V1+) code. Co-authored by Claude AI

tnull force-pushed the 2025-08-upgrade-vss-encryption-obfuscation-scheme branch from 94318be to d2153f2 Compare November 17, 2025 18:45

tankyleo approved these changes Nov 17, 2025

View reviewed changes

tnull merged commit 90e9736 into lightningdevkit:main Nov 17, 2025
10 of 17 checks passed

github-project-automation bot moved this from Goal: Merge to Done in Weekly Goals Nov 17, 2025

andrei-21 mentioned this pull request Nov 17, 2025

Obfuscate keys and encrypt payload in VSS backend breez/breez-sdk-ldk#17

Merged

Upgrade VSS encryption and obfuscation scheme #627

Upgrade VSS encryption and obfuscation scheme #627

Uh oh!

Conversation

tnull commented Aug 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ldk-reviews-bot commented Aug 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tnull commented Nov 12, 2025

Uh oh!

jkczyz Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

jkczyz Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

tnull Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tnull left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tnull commented Nov 17, 2025

Uh oh!

tankyleo left a comment

Choose a reason for hiding this comment

Uh oh!

tnull commented Nov 17, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

tnull commented Aug 22, 2025 •

edited

Loading

ldk-reviews-bot commented Aug 22, 2025 •

edited

Loading