lightning-kmp icon indicating copy to clipboard operation
lightning-kmp copied to clipboard

Refactor key management

Open pgrange opened this issue 1 year ago • 0 comments

⚠️ Disclaimer:

  • Being my first time with Kotlin, code might not be very idiomatic;
  • It might be the case that this topic is already somewhere in your roadmap and you might have given it some though about some target architecture that could be very different from what I propose here;
  • I've done my best to carefully craft each commit but, please, feel free to share any suggestion that could make this PR easier to review;
  • any other feedback you can think of are welcome.

Why

I'm planning to explore alternative implementations of a KeyManager and especially what can be done with hardware. As a pre-requisite, I believe some refactoring is needed to ensure that:

  1. the KeyManager interface is fully agnostic to the key material implementation (currently there are some dependencies to DeterministicWallet);
  2. private key material does not leak outside LocalKeyManager (currently DeterministicWallet.PrivateKey appears in Transactions, KeyManager, Channel, Bolt3Derivation, RouteBlinding for instance.

I've noticed key material is way more concealed in eclair-core (the android-phoenix branch).

What

In the target architecture, the user application would be in charge of instantiating the LocalKeyManager as it's the application (or any lightning-kmp client for that matter) that's in charge of initiating, backing up or restoring the key material (in the form of a seed for instance). Then the application trusts lightning-kmp for the lightning logic, lightning-kmp using some abstract KeyManager implemented by LocalKeyManager:

block-beta
columns 3
Start(("User")) space:2
u2a<[" "]>(down) space:2

Application                                            a2l<["uses"]>(right)       LNKMP["Lighning Logic"]
downAgain<["Instantiates"]>(down) space                                l2k<["uses"]>(down)
LocalKeyManager                                 k2l<["delegates"]>(left) KeyManager 

style Start fill:#969;

That way, we may replace LocalKeyManager by alternative implementations like, for instance, a HardwareKeyManager or a WhateverKeyManager:

block-beta
columns 3
LNKMP["Lighning Logic"]:3
KeyManager:3
LocalKeyManager HardwareKeyManager WhateverKeyManager

How

To limit the exposure of the key material, I've decided to create PrivateKeyDescriptor and ExtendedPrivateKeyDescriptor which are two interfaces, describing a private key that can be implemented to suit different crypto engines.

I've only implemented them interfaces for the LocalKeyManager. LocalKeyManager and the local package is the only places where such keys can be instantiated and private key material used for cryptography.

There are some cases where exporting private key material seems ok:

  • NodeKeys, which are used for network authentication and communication;
  • shaSeed, used for commitment points and eventually shared with channel peers;

It's also the case that Phoenix exports private key material to cypher cloud stored data for instance. In which case it seems very handy to rely on the KeyManager of lightning-kmp so that there is no need for another seed, or secret to backup/restore. But to open the way to alternative implementation, I've introduced the deterministicKeyMaterial() function.

Also note that, regarding the way Phoenix uses lightning-kmp, this PR introduces a breaking change. See commit Refactor key management - BREAKING API.

After this PR, to implement a KeyManager, one will have to also implement both interfaces as needed:

  • PrivateKeyDescriptor
    • publicKey
    • deriveForRevocation
    • deriveForCommitment
    • sign
    • signInputTaprootScriptPath
    • signMusig2TaprootInput
    • generateMusig2Nonce
  • ExtendedPrivateKeyDescriptor
    • privateKey
    • publicKey
    • derive
    • derivePrivateKey

Notes

I've tried to carefully craft each commit to ease the review process. I suggest to review each commit separately instead of just the whole PR outcome as they come with detailed explanation of what's done and why it's done that way:

  • Refactor key management - fundingKey/paymentKey is the first commit, it introduces PrivateKeyDescriptor and handles the easiest part with the funding and payment keys which are used without basic derivation strategies;
  • Refactor key management - Bolt3 focuses on everything that's Bolt3 related and, in particular it introduces two new implementations of PrivateKeyDescriptor to handle revocation and commitment derivations;
  • Refactor key management - swap in protocol focuses on the swap in protocol and introduces ExtendedPrivateKeyDescriptor;
  • Refactor key management - Bip84 focuses on Bip84
  • Refactor key management - encapsulate Transactions.sign moves Transactions.sign() into the PrivateKeyDescriptor interface and is the first commit to migrate key operation into this interface
  • Refactor key management - segregate instantiate() to LocalKeyManager make it so that only the LocalKeyManager or code in the local package can instantiate actual private key material
  • Refactor key management - segregate instantiate() again does the same thing but for extended private keys
  • Refactor key management - BREAKING API introduces a breaking change. The commit message describes what kind of changes (which look minimal) are to be done to adapt the code of Phoenix to this breaking change.

pgrange avatar Feb 29 '24 14:02 pgrange