Allow whitelisting outgoing connections #17167

pull luke-jr wants to merge 3 commits into bitcoin:master from luke-jr:whitelist_outgoing changing 7 files +80 −34
  1. luke-jr commented at 9:35 PM on October 16, 2019: member

    Rethinking/rewrite of #10594 in light of whitelist flags: new flags are added for "in" and "out" to specify the connection direction matched. Both can be specified to match either direction.

  2. luke-jr force-pushed on Oct 16, 2019
  3. fanquake added the label P2P on Oct 16, 2019
  4. luke-jr force-pushed on Oct 16, 2019
  5. luke-jr force-pushed on Oct 16, 2019
  6. DrahtBot commented at 11:30 PM on October 16, 2019: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #25515 ([draft] PeerManager unit tests by dergoegge)
    • #25514 (net processing: Move CNode::nServices and CNode::nLocalServices to Peer by dergoegge)
    • #25500 (refactor: Move inbound eviction logic to its own translation unit by dergoegge)
    • #25268 (refactor: Introduce EvictionManager by dergoegge)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

  7. luke-jr force-pushed on Oct 17, 2019
  8. luke-jr force-pushed on Oct 17, 2019
  9. laanwj commented at 8:19 AM on October 17, 2019: member

    Concept ACK.

    As for implementation, I'm not sure about having In and Out as permissions.

    This doesn't allow having different flags for a IP for incoming and outgoing. Not sure that's ever necessary, but this seems a bit muddled to me. Might also consider e.g. whiteconnect (for outgoing connections) could have a completely different flag list structure from the normal whitelist (for incoming connections).

    Edit: in a way you already do this by having separate connOptions.vWhitelistedRangeOutgoing versus connOptions.vWhitelistedRange, it's just not exposed all the way.

  10. luke-jr commented at 12:50 PM on October 17, 2019: member

    As you noticed, the implementation already doesn't treat in/out as permissions.

    Using -whitelist for both allows for easily specifying the same flags for both in,out. I can't think of any benefit to having separate options. (I'm not sure why we'd want to support different sets of flags...)

  11. DrahtBot added the label Needs rebase on Oct 30, 2019
  12. luke-jr force-pushed on Nov 4, 2019
  13. DrahtBot removed the label Needs rebase on Nov 4, 2019
  14. luke-jr force-pushed on Nov 15, 2019
  15. DrahtBot added the label Needs rebase on Dec 4, 2019
  16. luke-jr force-pushed on Mar 28, 2020
  17. luke-jr force-pushed on Aug 20, 2020
  18. luke-jr commented at 7:43 PM on August 20, 2020: member

    Rebased

  19. luke-jr force-pushed on Aug 20, 2020
  20. luke-jr cross-referenced this on Aug 20, 2020 from issue net: Replace enum CConnMan::NumConnections with enum class ConnectionDirection by luke-jr
  21. DrahtBot cross-referenced this on Aug 20, 2020 from issue RPC: getpeerinfo: Deprecate "whitelisted" field (replaced by "permissions") by luke-jr
  22. DrahtBot cross-referenced this on Aug 20, 2020 from issue [RPC] Add connection type to getpeerinfo, improve logs by amitiuttarwar
  23. DrahtBot removed the label Needs rebase on Aug 20, 2020
  24. DrahtBot cross-referenced this on Aug 20, 2020 from issue Per-Peer Message Capture by troygiorshev
  25. DrahtBot cross-referenced this on Aug 20, 2020 from issue rpc, cli: add network in/out connections to `getnetworkinfo` and `-getinfo` by jonatack
  26. DrahtBot cross-referenced this on Aug 20, 2020 from issue [tests] Allow outbound & block-relay-only connections in functional tests. by amitiuttarwar
  27. DrahtBot cross-referenced this on Aug 20, 2020 from issue qt: Deduplicate NumConnections enum by hebasto
  28. DrahtBot cross-referenced this on Aug 20, 2020 from issue p2p: Move all header verification into the network layer, extend logging by troygiorshev
  29. DrahtBot cross-referenced this on Aug 20, 2020 from issue bloom: use Span instead of std::vector for `insert` and `contains` [ZAP3] by jb55
  30. DrahtBot cross-referenced this on Aug 21, 2020 from issue interfaces: Expose settings.json methods to GUI by ryanofsky
  31. DrahtBot cross-referenced this on Aug 31, 2020 from issue rpc: remove deprecated CRPCCommand constructor by maflcko
  32. DrahtBot cross-referenced this on Sep 3, 2020 from issue refactor: Add ALLOW_LIST flags and enforce usage in CheckArgFlags by ryanofsky
  33. DrahtBot cross-referenced this on Sep 3, 2020 from issue util: Fix -norpcwhitelist, -norpcallowip, and similar corner case behavior by ryanofsky
  34. DrahtBot added the label Needs rebase on Sep 4, 2020
  35. maflcko commented at 10:58 AM on September 5, 2020: member

    Previously whitelisting outgoing connections wasn't possible, so no guards have been added to protect those from disconnection. Would be good to revisit those places and evaluate if guards are needed.

    For example commit c60fd71a65e841efe187992f46c583a704cc37f5 by @sdaftuar checks for pfrom->fWhitelisted, but the commit immediately after it (5a6d00c6defc587e22c93e63029fdd538ce8858d) removes the check.

  36. luke-jr commented at 4:33 PM on September 10, 2020: member

    @MarcoFalke Even on inbound peers, noban isn't absolute. There are many places that can be made more noban-friendly, but due to the complexity of at least a few of them, I think they would be better as followup PRs.

  37. luke-jr force-pushed on Sep 10, 2020
  38. DrahtBot removed the label Needs rebase on Sep 10, 2020
  39. luke-jr force-pushed on Sep 10, 2020
  40. luke-jr force-pushed on Sep 11, 2020
  41. DrahtBot cross-referenced this on Sep 19, 2020 from issue signet mining utility by ajtowns
  42. DrahtBot cross-referenced this on Sep 19, 2020 from issue Coinstats Index by fjahr
  43. DrahtBot cross-referenced this on Sep 22, 2020 from issue Assert that RPCArg names are equal to CRPCCommand ones (net, rpcwallet) by maflcko
  44. DrahtBot cross-referenced this on Sep 23, 2020 from issue net: Add CNode::ConnectedThroughNetwork member function by hebasto
  45. DrahtBot cross-referenced this on Sep 23, 2020 from issue rpc: Add dumpcoinstats by fjahr
  46. DrahtBot added the label Needs rebase on Sep 23, 2020
  47. luke-jr force-pushed on Oct 30, 2020
  48. DrahtBot removed the label Needs rebase on Oct 30, 2020
  49. DrahtBot cross-referenced this on Oct 31, 2020 from issue rpc, net: Expose connections_onion_only in getnetworkinfo RPC output by hebasto
  50. DrahtBot cross-referenced this on Oct 31, 2020 from issue net: Add blockfilters white{bind,list} permission flag by luke-jr
  51. DrahtBot cross-referenced this on Nov 11, 2020 from issue Follow-ups to 19107 by troygiorshev
  52. DrahtBot cross-referenced this on Nov 25, 2020 from issue refactor: Move node and wallet code out of src/interfaces by ryanofsky
  53. DrahtBot cross-referenced this on Nov 25, 2020 from issue multiprocess: Add basic spawn and IPC support by ryanofsky
  54. DrahtBot cross-referenced this on Nov 26, 2020 from issue multiprocess: Add bitcoin-gui -ipcconnect option by ryanofsky
  55. DrahtBot cross-referenced this on Nov 26, 2020 from issue multiprocess: Add bitcoin-wallet -ipcconnect option by ryanofsky
  56. DrahtBot cross-referenced this on Nov 26, 2020 from issue Multiprocess bitcoin by ryanofsky
  57. DrahtBot added the label Needs rebase on Dec 1, 2020
  58. luke-jr force-pushed on Mar 4, 2021
  59. DrahtBot removed the label Needs rebase on Mar 4, 2021
  60. maflcko referenced this in commit 8c049fe9af on Mar 7, 2021
  61. rebroad commented at 10:12 AM on March 19, 2021: contributor

    More reasons to merge than to not merge, based on my understanding of the use of whitelist (which is essentially peers you trust/own), so untested ACK.

  62. DrahtBot cross-referenced this on Mar 22, 2021 from issue p2p, refactor: make NetPermissionFlags an enum class by jonatack
  63. DrahtBot cross-referenced this on Apr 9, 2021 from issue p2p, bugfix: use NetPermissions::HasFlag() in CConnman::Bind() by jonatack
  64. jonatack commented at 9:41 PM on April 9, 2021: contributor

    I've looking at the net permissions lately, will review.

  65. DrahtBot cross-referenced this on Apr 13, 2021 from issue refactor: Remove settings merge reverse precedence code by ryanofsky
  66. DrahtBot cross-referenced this on Apr 13, 2021 from issue util: Forbid ambiguous multiple assignments in config file by ryanofsky
  67. DrahtBot cross-referenced this on Apr 15, 2021 from issue net: expand Sock and fuzz-test more of CConnman by vasild
  68. DrahtBot added the label Needs rebase on Apr 30, 2021
  69. luke-jr commented at 1:23 AM on July 7, 2021: member

    Rebased

  70. luke-jr force-pushed on Jul 7, 2021
  71. DrahtBot removed the label Needs rebase on Jul 7, 2021
  72. DrahtBot cross-referenced this on Jul 7, 2021 from issue Make all networking code mockable by vasild
  73. DrahtBot cross-referenced this on Nov 18, 2021 from issue scripted-diff: Use clang-tidy syntax for C++ named arguments by maflcko
  74. DrahtBot cross-referenced this on Nov 27, 2021 from issue Use Sock in CNode by vasild
  75. DrahtBot added the label Needs rebase on Feb 4, 2022
  76. net: Move PF_ISIMPLICIT interpretation from AcceptConnection to new InitializePermissionFlags 1daa9d1a91
  77. net: Move extra service flag into InitializePermissionFlags a2a21f995f
  78. Accept "in" and "out" flags to -whitelist to allow whitelisting outgoing connections 36cc299bae
  79. luke-jr force-pushed on May 12, 2022
  80. DrahtBot removed the label Needs rebase on May 12, 2022
  81. maflcko commented at 1:26 PM on May 17, 2022: member

    Concept ACK

  82. maflcko cross-referenced this on May 17, 2022 from issue refactor: Introduce PeerManagerImpl::RejectIncomingTxs by maflcko
  83. luke-jr referenced this in commit facd0cd196 on May 21, 2022
  84. luke-jr referenced this in commit 06163b0921 on May 21, 2022
  85. luke-jr referenced this in commit 90bb84efb7 on May 21, 2022
  86. maflcko referenced this in commit fa2b5fe0c1 on May 27, 2022
  87. maflcko referenced this in commit fafddafc2c on Jun 14, 2022
  88. maflcko referenced this in commit ede9089096 on Jun 15, 2022
  89. sidhujag referenced this in commit a46aa0a2b9 on Jun 15, 2022
  90. luke-jr cross-referenced this on Jun 18, 2022 from issue I2P: add support for transient addresses for outbound connections by vasild
  91. DrahtBot cross-referenced this on Jun 22, 2022 from issue refactor: Introduce EvictionManager by dergoegge
  92. ghost commented at 2:19 AM on June 23, 2022: none

    Concept ACK

  93. vasild commented at 11:36 AM on June 28, 2022: contributor

    This would allow us to use transient (aka one-time / random / disposable) I2P addresses when making outgoing connections to all nodes, except to whitelisted ones for which we would use our persistent I2P address.

  94. DrahtBot cross-referenced this on Jun 29, 2022 from issue refactor: Move inbound eviction logic to its own translation unit by dergoegge
  95. DrahtBot cross-referenced this on Jun 30, 2022 from issue net, test: Virtualise CConnman and add initial PeerManager unit tests by dergoegge
  96. DrahtBot cross-referenced this on Jun 30, 2022 from issue net processing: Move CNode::nServices and CNode::nLocalServices to Peer by dergoegge
  97. DrahtBot commented at 6:13 PM on July 7, 2022: contributor

    <!--cf906140f33d8803c4a75a2196329ecb-->

    🐙 This pull request conflicts with the target branch and needs rebase.

    <sub>Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a "draft".</sub>

  98. DrahtBot added the label Needs rebase on Jul 7, 2022
  99. janus referenced this in commit 646a84b1a1 on Aug 4, 2022
  100. achow101 commented at 6:03 PM on October 12, 2022: member

    Closing this as it has not had any activity in a while. If you are interested in continuing work on this, please leave a comment so that it can be reopened.

  101. achow101 closed this on Oct 12, 2022
  102. brunoerg commented at 7:03 PM on October 24, 2022: contributor

    @luke-jr Are you intending to work on this?

  103. ghost commented at 7:58 PM on October 24, 2022: none

    Seems a little weird because I tried to filter bitcoin core pull requests with is:pr is:open sort:updated-asc and first PR in the result has 1 Concept ACK from dev who no longer contributes to this repo in reviews and second was only active for a few days/weeks. Those comments were posted in May 2022 and PR is in draft mode.

    Whereas, this PR got last 2 Concept ACK or comments agreeing from active contributors in May 2022. And a Concept ACK from present maintainer.

    @luke-jr Are you intending to work on this?

    Anyway, PR author has not commented since Jul 7, 2021 so we cannot blame others for this PR to get more review and eventually merged.

    Whitelist could be a taboo in bitcoin, however this is already possible manually. This PR makes it easier to manage your outgoing connections. I haven't reviewed each commit but I conceptually agree with the idea.

  104. luke-jr commented at 11:30 PM on October 24, 2022: member

    @brunoerg I have never stopped maintaining it.

  105. brunoerg commented at 11:47 PM on October 24, 2022: contributor

    @luke-jr Cool, just to know, because it's been closed and I am interesting on it.

  106. brunoerg commented at 11:47 PM on October 24, 2022: contributor

    Concept ACK

  107. brunoerg cross-referenced this on Nov 1, 2022 from issue rpc, p2p: add `addpermissionflags` RPC and allow whitelisting outbound by brunoerg
  108. bitcoin deleted a comment on Nov 2, 2022
  109. brunoerg cross-referenced this on Feb 16, 2023 from issue p2p: Allow whitelisting outgoing connections by brunoerg
  110. vijaydasmp referenced this in commit 91535fda98 on Aug 8, 2023
  111. vijaydasmp referenced this in commit f0079dcf4a on Aug 8, 2023
  112. vijaydasmp referenced this in commit 6728914385 on Aug 9, 2023
  113. bitcoin locked this on Nov 2, 2023
Contributors
luke-jrDrahtBotlaanwjmaflckorebroadjonatackghostvasildachow101brunoerg
Labels
P2PNeeds rebase
Linked (view graph)
#10594 Bugfix: net: Apply whitelisting criteria to outgoing connections#19771 net: Replace enum CConnMan::NumConnections with enum class ConnectionDirection#25156 refactor: Introduce PeerManagerImpl::RejectIncomingTxs#25355 I2P: add support for transient addresses for outbound connections#26441 rpc, p2p: add `addpermissionflags` RPC and allow whitelisting outbound#27114 p2p: Allow whitelisting manual connections
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-05-19 06:54 UTC