Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RPC locking #3303

Closed
darosior opened this issue Nov 28, 2019 · 7 comments
Closed

RPC locking #3303

darosior opened this issue Nov 28, 2019 · 7 comments

Comments

@darosior
Copy link
Contributor

With #3129 and #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 ?

@darosior darosior changed the title Wallet locking RPC locking Nov 29, 2019
@cdecker
Copy link
Member

cdecker commented Nov 29, 2019

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
Copy link
Contributor Author

darosior commented Nov 30, 2019

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
Copy link
Contributor Author

darosior commented Dec 1, 2019

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
Copy link
Member

cdecker commented Dec 3, 2019

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
Copy link
Contributor Author

darosior commented Dec 4, 2019

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
Copy link
Member

cdecker commented Dec 9, 2019

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
Copy link
Contributor Author

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.

@darosior darosior closed this as completed Nov 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants