RIP Esplora, configure bitcoin-d as chain source. #21
Conversation
IIUC, the file-based config approach will allow for using Esplora if desired? |
I don't think it makes much sense to use ldk-server with esplora given myriad of problems related to scaling/trust/rate-limiting. |
| @@ -24,7 +24,7 @@ fn main() { | |||
|
|
|||
| if args.len() < 6 { | |||
| eprintln!( | |||
| "Usage: {} storage_path listening_addr rest_svc_addr network esplora_server_url", | |||
| "Usage: {} storage_path listening_addr rest_svc_addr network bitcoind_rpc_addr bitcoind_rpc_user bitcoind_rpc_password", | |||
There was a problem hiding this comment.
Let's not take a plaintext password from command line here, please. Let's parse username password from the default .cookie file and let the user supply an override path.
There was a problem hiding this comment.
That does sound like a good idea, but this needs more thought.
iiuc, the .cookie is ephemeral and is meant for local-only access; it is read-only if we use the same user for bitcoin-d as the LDK daemon.
For remote access, I think the RPC user and pass in bitcoin.conf need to be set.
Similarly, it is equivalent to have an rpc-user and rpc-pass in ldk-server.conf.
Generally, just relying on RPC (pass) isn't a sufficient security measure; most people need to configure bitcoind with secure network access in addition to RPC authentication.
I checked this for both LND and c-lightning; they do it the same way i.e., credentials in a file: https://docs.corelightning.org/reference/lightningd-config.
We should probably switch to rpc-auth instead of rpc-user-password for the bitcoind connection.
Currently, given that the cookie method restricts bitcoin-d usage to local-only, I don't think we should restrict ourselves to that method.
There was a problem hiding this comment.
Similarly, it is equivalent to have an rpc-user and rpc-pass in ldk-server.conf.
Mhh, I tend to disagree. Supplying these parameters via shell input means that they might get inadvertently distributed across different hosts/history files. IMO, there is a notable difference between reading RPC credentials from (likely hopefully protected by 700 permissions) config file and reading them as arguments, which might leak to other users on the same host through ps or similar.
We should probably switch to rpc-auth instead of rpc-user-password for the bitcoind connection.
Not sure how rpc-auth is different here? It's just the username and the hashed password in a single string, which is exactly the .cookie format?
There was a problem hiding this comment.
Mhh, I tend to disagree. Supplying these parameters via shell input means that they might get inadvertently distributed across different hosts/history files. IMO, there is a notable difference between reading RPC credentials from (likely hopefully protected by 700 permissions) config file
As noted in main PR description, CLI reading is a stop-gap solution until the next PR which introduces read all args from file.
All the CLI options are moving to a config file. Hence it is is equivalent.
Not sure how rpc-auth is different here? It's just the username and the hashed password in a single string, which is exactly the .cookie format?
RPCAuth refers https://github.com/bitcoin/bitcoin/blob/master/share/rpcauth/rpcauth.py , where we configure
`rpcauth=$username:$SALT$hash
It is different from .cookie since .cookie is ephemeral and is meant for local access only.
Afaik, rpc-auth is the preffered method for accessing bitcoind remotely: https://bitcoin.stackexchange.com/a/90420
There was a problem hiding this comment.
As noted in main PR description, CLI reading is a stop-gap solution until the next PR which introduces read all args from file. All the CLI options are moving to a config file. Hence it is is equivalent.
Is there a pressing need for this PR? Why not just do the next one instead?
There was a problem hiding this comment.
Is there a pressing need for this PR? Why not just do the next one instead?
I don't see a reason to block this on next one, given they are independent from functionality perspective.
Reading from config is in milestone-3 (#11), and i will have to prioritize it. It also helps me if all the possible config options are checked-in more or less, before i think about config-file schema. Also hoping that it is easier to test with bitcoind than setup local esplora.
Given I have already said that we will move all args to config file, and only take config-file path as input, i assume we can move forward with that?
There was a problem hiding this comment.
RPCAuth refers https://github.com/bitcoin/bitcoin/blob/master/share/rpcauth/rpcauth.py , where we configure `rpcauth=$username:$SALT$hash
It is different from
.cookiesince .cookie is ephemeral and is meant for local access only.Afaik, rpc-auth is the preffered method for accessing bitcoind remotely: https://bitcoin.stackexchange.com/a/90420
It's literally the same scheme. The .cookie just houses rpcauth token with user __cookie__ and a random password. If we implement reading .cookie files first, we'd get a way to supply rpcauth params from file for free.
In any case, I don't want to hold up this PR over it if we agree that this is a stop-gap solution that will be dropped in the future anyways.
There was a problem hiding this comment.
Will defer to y'all, but note we already have code supporting cookie-based auth in LDK sample.
There was a problem hiding this comment.
It's literally the same scheme.
The reason the cookie doesn't work for us is that it is ephemeral.
Not really an option for us as default unless we support other methods.
|
How do Core, LND, and CLN handle it?
…On Thu, Nov 7, 2024 at 2:16 AM Elias Rohrer ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In server/src/main.rs
<#21 (comment)>
:
> @@ -24,7 +24,7 @@ fn main() {
if args.len() < 6 {
eprintln!(
- "Usage: {} storage_path listening_addr rest_svc_addr network esplora_server_url",
+ "Usage: {} storage_path listening_addr rest_svc_addr network bitcoind_rpc_addr bitcoind_rpc_user bitcoind_rpc_password",
Similarly, it is equivalent to have an rpc-user and rpc-pass in
ldk-server.conf.
Mhh, I tend to disagree. Supplying these parameters via shell input means
that they might get inadvertently distributed across different
hosts/history files. IMO, there *is* a notable difference between reading
RPC credentials from (likely hopefully protected by 700 permissions)
config file and reading them as arguments, which might leak to other users
on the same host through ps or similar.
We should probably switch to rpc-auth instead of rpc-user-password for the
bitcoind connection.
Not sure what you're referring to as rpc-auth here?
—
Reply to this email directly, view it on GitHub
<#21 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AACPUA4SFBZAFGLEQHT5BNLZ7M4WTAVCNFSM6AAAAABRHG5U56VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDIMRQGUZDGNBYGA>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
They offer multiple options but usually storing credentials in a config (or in Core's case dedicated I think we should implement multiple ways eventually, but reading from command line shouldn't be the default, IMO. |
As mentioned in main PR description, CLI reading is a stop-gap solution until the next PR which introduces read all args from file. Reading from file is the default. Reading from |
|
Squash fixups early to avoid another review cycle. |
Not too worried about increasing number of cli args,
In later PRs we move the whole config to a file and only take config file path as input.