-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
wtclientrpc: prevent watchtower connections to local addresses #9230
base: master
Are you sure you want to change the base?
wtclientrpc: prevent watchtower connections to local addresses #9230
Conversation
Adds validation to prevent watchtower client connections to local addresses by: - Implementing IsLocalAddress() to detect localhost/local network addresses - Adding check in AddTower RPC to reject local tower connections - Including comprehensive unit tests for address validation This helps prevent security issues from misconfigured watchtower setups that accidentally expose local addresses.
Important Review skippedAuto reviews are limited to specific labels. 🏷️ Labels to auto review (1)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
@ellemouton Please review. |
@anibilthare - I think the issue was more about connecting to your own watchtower on the same LND node. ie, connecting the wtclient to the watchtower server on the same LND node |
Thanks for the quick response @ellemouton !! I chose to check for all local addresses (rather than just same-node) for these reasons:
Would you prefer we narrow the scope to only prevent same-node connections? I'm open to adjusting the approach if you think the current implementation is too broad. |
@ellemouton Any feedback ? |
@anibilthare - my concern is that this makes testing hard/impossible. We could just log a warning instead? Your points are all valid but i think the user should have the flexibility to run things how they want to & they themselves should be aware of the risks of certain run configurations imo, the original issue isnt really an issue anyways... maybe others disagree. |
Change Description
Fixes 5522
Adds validation to prevent watchtower client connections to local addresses by:
This helps prevent security issues from misconfigured watchtower setups that accidentally expose local addresses.
Steps to Test
Steps for reviewers to follow to test the change.