router: abstract node key behind ECDH interface #47
Conversation
Roasbeef
left a comment
Roasbeef
left a comment
There was a problem hiding this comment.
Straight forward change, gives us a ton of options in the future w.r.t how we handle onion blobs, etc. I think we'll want a similar change in the area of the switch/router where we need to at times re-derive the shared secret upon restart.
|
cc @joostjager (reviewers thing is messed up on this repo, can't assign ppl easily) |
guggero
force-pushed
the
sign-abstraction
branch
from
e769c54 to
59a66e2
Compare
|
I'm working on a PR that does all this in lnd as well ( Do you think the naming is OK? I'm going to use the same |
| // SingleKeyECDH is an abstraction interface that hides the implementation of an | ||
| // ECDH operation against a specific private key. We use this abstraction for | ||
| // the long term keys which we eventually want to be able to keep in a hardware | ||
| // wallet or HSM. |
There was a problem hiding this comment.
Out of curiosity, which hardware wallets currently support ECDH?
There was a problem hiding this comment.
I have no idea, to be honest. HW wallets are my blind spot. But I imagine implementing that is pretty easy as it's not much different from signing a message.
There was a problem hiding this comment.
Ledger and trezor have support in their APIs which can be leveraged by custom applications. With this though, I think we're more so targeting remote/restricted signers which can emulate these calls by using the existing lnd gRPC interface and sub-servers, or something else entirely.
crypto.go
Outdated
| // the long term keys which we eventually want to be able to keep in a hardware | ||
| // wallet or HSM. | ||
| type SingleKeyECDH interface { | ||
| // PubKey returns the public key of the wrapped, "local" private key. |
There was a problem hiding this comment.
Meaning of 'wrapped' and 'local' not immediately clear. This interface could also be implemented by structs that are not wrapping and/or local?
There was a problem hiding this comment.
Okay, adjusted the comment(s).
With this commit we create a new interface that abstracts away the ECDH implementation. We also create one implementation that satisfies the interface that can be used if the private key is known directly. Later we will change the generateSharedSecret function to use this interface instead of doing the ECDH operation directly.
This is a preparatory commit that adds an error return value to the generateSharedSecret and generateSharedSecrets method. This is needed because the interface we want to abstract the onion key behind has an error return value too.
To be able to eventually extract the private keys out of the node itself into a hardware wallet or HSM, we need to abstract the ECDH operation against the long-term key behind an interface.
guggero
force-pushed
the
sign-abstraction
branch
from
59a66e2 to
d019eaf
Compare
| // the output of a SHA256 hash. | ||
| type Hash256 [sha256.Size]byte | ||
|
|
||
| // SingleKeyECDH is an abstraction interface that hides the implementation of an |
There was a problem hiding this comment.
Wondering still if SingleKeyECDH is the right name. It also suggests some fancy algorithm in which you exchange keys with yourself. Thinking about alternatives. Maybe ExternalECDH or just Signer (even though it isn't really signing)
Yeah, to propagate these changes into the No strong feelings w.r.t naming on my end. |
3 participants
As part of the preparatory work for lightningnetwork/lnd#3929, we remove the node's private key from the
NewRoutermethod and replace it with an interface that abstracts away the ECDH operation.This is only necessary for the long-term node key. All other private keys are just temporary session keys.