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

plugins/bcli: load RPC password from stdin instead of an argument #5509

Conversation

seberm
Copy link
Contributor

@seberm seberm commented Aug 9, 2022

Passing a RPC password via cli argument (-rpcpassword) can leak auth secrets to other users on a system (at least on Linux). This PR tries to pass the RPC password through a stdin using a -stdinrpcpass

Refs.:

Fixes #3984

Copy link
Contributor

@rustyrussell rustyrussell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor changes only...

plugins/bcli.c Outdated Show resolved Hide resolved
plugins/bcli.c Outdated Show resolved Hide resolved
plugins/bcli.c Outdated Show resolved Hide resolved
@seberm seberm force-pushed the feature/load-rpc-password-from-stdin branch 3 times, most recently from 078fdba to da47e8e Compare August 10, 2022 11:46
@seberm
Copy link
Contributor Author

seberm commented Aug 10, 2022

Everything should be fixed now, thanks for your review!

@rustyrussell
Copy link
Contributor

Hmm can you add a Changelog-Fixed: bcli: don't expose bitcoin RPC password on commandline to the commit message, so we make sure it gets noted in the CHANGELOG.md for next release?

Thanks!
Ack da47e8e

@rustyrussell rustyrussell added this to the v22.10 milestone Aug 10, 2022
@seberm seberm force-pushed the feature/load-rpc-password-from-stdin branch from da47e8e to af4335c Compare August 10, 2022 17:28
@seberm
Copy link
Contributor Author

seberm commented Aug 10, 2022

Sure, I've added the changelog info to commit message and I've fixed the order of includes.

Copy link
Collaborator

@vincenzopalazzo vincenzopalazzo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK af4335c

@vincenzopalazzo
Copy link
Collaborator

We can add the magic keyword Fixes #3984 to avoid to leave the issue open?

@seberm
Copy link
Contributor Author

seberm commented Aug 11, 2022

I've added the Fixes keyword into my first comment. It seems the #3984 issue is now linked to close automatically.

Changelog-Fixed: bcli: don't expose bitcoin RPC password on commandline
@rustyrussell rustyrussell force-pushed the feature/load-rpc-password-from-stdin branch from af4335c to be29c1f Compare September 14, 2022 21:39
@rustyrussell rustyrussell self-assigned this Sep 14, 2022
@rustyrussell
Copy link
Contributor

Trivial rebase, should kick CI.

Ack be29c1f

@rustyrussell rustyrussell merged commit 0a85677 into ElementsProject:master Sep 15, 2022
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

Successfully merging this pull request may close these issues.

Avoid leaking bitcoin auth secrets to other users
3 participants