ElementsProject / lightning

Core Lightning — Lightning Network implementation focusing on spec compliance and performance
Other
2.84k stars 901 forks source link

RPC locking #3303

Closed darosior closed 3 years ago

darosior commented 4 years ago

With https://github.com/ElementsProject/lightning/pull/3129 and https://github.com/ElementsProject/lightning/pull/2925 merged I'm thinking about finally implementing wallet locking, but I want to reach an agreement upon the design before implementing it.

In Berlin @cdecker and @rustyrussell (iirc) were in favor of the addition of an authentication field to the jsonrpc requests. I'm still thinking that it should just be a local-only lock and unlock mecanism "ala" bitcoin-core (though of course it's not the same security model at all..).

What do you think ? Is there something I missed which makes the lock and unlock way not a good idea ?

cdecker commented 4 years ago

That seems like a good first locking plugin. You'd probably just need to have a single boolean flag locked in the plugin and you'd then filter out any non-read-only RPC calls using rpc_command if I understand your proposal correctly.

It is a bit coarse grained since there is no real concept of identity of a caller, and you'd either unlock all RPC callers or none. I'm assuming here that you're not planning on having something like a cookie that'd re-authenticate callers as the one that unlocked it previously. Is that correct?

With a more fine-grained control using username/password authentication, cookies, or even more flexible schemes like macaroons you could control permissions per user, allow delegation and other things, but for a first PoC I think the binary locked/unlocked plugin is a great start.

darosior commented 4 years ago

Thanks for your feedback.

if I understand your proposal correctly.

You do.

About the fine grained auth and the bakery, I think it would be better for a higher level API, as I assume it would be useful for an exposed one ? I'm not sure about exposing the RPC interface, and I see this first plugin for a local use.

darosior commented 4 years ago

Thinking more about this, we currently don't have any authentication mechanism for plugins themselves.

One could impersonate a plugin by just writing on its stdout. In the case of an RPC locking plugin it would be easy to write before the plugin on its stdout "continue". Even if the plugin then writes the real result afterwards, lightningd would crash but there might be a time window in which a sensitive command could be executed.

Is an authentication challenge for some plugins something we would ?

EDIT: It's straightforward to authenticate lightningd from plugin's viewpoint, but authenticating the plugin from lightningd's viewpoint would require a lot of addition...

cdecker commented 4 years ago

Yes, plugins are pretty much trusted at this point. If a plugin is compromised it can move freely. I was playing with the idea of having a plugin-runner that'd proxy between the plugin and lightningd potentially running as different users and restricting access to the RPC, but I never got around to writing the runner.

If we really wanted a bullet-proof solution I think running the plugins in a seccomp context and restricting their interactions with the rest of the system to just the its stdin and stdout might be nice, but that's a whole load of new things we'd need to plumb through, unless we create the runner

darosior commented 4 years ago

The best I could come up with would be a basic ECDH between the proxy and lightningd : the plugin signals a public key at getmanifest and lightningd responds with its own at init. If signaled at connection establishment then the result content of the JSONRPC response sent by the plugin would be encrypted using the shared secret. That way, the plugin could not be impersonated.

That would be useful for RPC locking, but I wonder if it's not overkill ?

cdecker commented 4 years ago

That seems like overkill. I was talking about the plugin itself being compromised, so any kind of authentication that could be done by a plugin can also be done by an attacker. And I can't see how that could be changed.

Anyway, I think at this point what we'd really want to do is have an authenticating proxy that relays incoming requests to a socket only it can access, rather than intercepting the JSON-RPC commands after they went through the plugin dispatch. Much easier and you can enforce arbitrary policies based on the origin of the request.

rpc_command really is better suited to have simple fail / continue semantics based on some authentication we have piggy-backed on top of the request (notice that this authentication might also be provided by an external proxy that transparently authenticates the caller, similar to the whole service mesh idea popular in kubernetes circles).

darosior commented 4 years ago

Ok

And I can't see how that could be changed.

Fwiw I was thinking that could rely that the auth would be done only once (static plugin), at startup. The auth could not be replaced after startup since lightningd could not be started again without hsm_secret pass.