Since 0.16 Bitcoin can be stuck connected to adversarial/3rdparty node #29506

issue litecomb openend this issue on February 28, 2024
  1. litecomb commented at 6:44 pm on February 28, 2024: none

    Is there an existing issue for this?

    • I have searched the existing issues

    Current behaviour

    There is a bug in net_processing.cpp logic. We might be m_protect=true because that feature sees us as unsynced (easy to simulate/spoof by an adversary) Or this is actually possible:

    0             if (state == nullptr) return; // shouldn't be possible, but just in case
    

    Secondly something sets fDisconnect=true (could be anything) So basically we kind of accumulate 200 remote bitcoins connected to us which want to disconnect from us but simultaneously can’t. I mean on the live network we have 200 uploader slots so 200 bitcoins become connected to us who really can’t disconnect from us. Kind of silly bug in bitcoin. fDisconnect=true kills any messages to us except ping. m_protect=true (theoretically kills the actual disconnect)

    Expected behaviour

    Eventually the remote bitcoin should disconnect from us anyway, as we consume it’s slot forever.

    Steps to reproduce

    1. Adversary connects to Bitcoin
    2. Stays connected to bitcoin forever.

    Relevant log output

    No response

    How did you obtain Bitcoin Core

    Compiled from source

    What version of Bitcoin Core are you using?

    25

    Operating system and version

    ubuntu

    Machine specifications

    No response

  2. mzumsande commented at 6:58 pm on February 28, 2024: contributor

    I don’t understand this. A node sets m_protect=true for a a maximum of 4 outbound peers (MAX_OUTBOUND_PEERS_TO_PROTECT_FROM_DISCONNECT) that have announced headers to us that have at least the work of our tip. See here for the code on master. It is also never set for inbound peers, so if an “Adversary connects to Bitcoin” it won’t be protected by it.

    Moreover, even if it m_protect was set it only protects from being disconnected due to not keeping up with the best chain (PeerManagerImpl::ConsiderEviction). It doesn’t protect against disconnections for other reasons.

  3. litecomb commented at 8:04 pm on February 28, 2024: none
    I understand what do you mean. I take back that it’s caused by m_protect, but this bug behaves identically to m_protect =?= true and fDisconnect=?=true. Also in our tests it applies to both inbound and outbound peers, the bitcoin core peer stays connected, is pinging/ponging and never disconnects (but doesn’t respond to any packets).
  4. mzumsande commented at 8:42 pm on February 28, 2024: contributor

    Also in our tests it applies to both inbound and outbound peers

    Inbound eviction only happens when the node is full, replacing existing peers. There is a rather complicated algorithm for that protecting peers with respect to certain criteria to establish a good diversity of inbound peers, see CConnman::AttemptToEvictConnection(). But when the node is not at capacity, no inbound peer gets disconnected, which is intentional.

    Automatic outbound peers get disconnected if they fall behind our tip (PeerManagerImpl::ConsiderEviction). Of course, that requires you to have gotten a new tip from someone else in the meantime. Manually chosen outbound peers (“addnode”) are not disconnected this way.

    See https://github.com/bitcoin-core/bitcoin-devwiki/wiki/P2P-Design-Philosophy for a more in-depth essay on that.

  5. litecomb commented at 8:52 pm on February 28, 2024: none

    But when the node is not at capacity, no inbound peer gets disconnected, which is intentional.

    Okay, not a bug for inbound case. Still, the bad outbound case remains.

    Essay says:

    so we won’t stay connected to 9 peers for long, in theory

    My tests show otherwise. You stay connected to an adversary (i.e. not Bitcoin Core) while not communicating new blocks or anything (except pings pongs) indefinitely. On Bitcoin Core - Bitcoin Core (the honest case) this bug doesn’t manifest.

  6. maflcko commented at 8:38 am on February 29, 2024: member

    My tests show otherwise.

    Can you share your tests? Ideally as a python functional test with the P2P interface mock, or otherwise exact steps to reproduce?

  7. maflcko added the label P2P on Feb 29, 2024
  8. litecomb commented at 1:29 am on March 22, 2024: none
    I would love to, currently it’s in golang. Actually what bugs me the most that the remote bitcoins cease sending new block headers to me after a long time. But they keep pinging me.
  9. willcl-ark commented at 3:52 pm on October 14, 2024: member

    The problem is not easily reproducible.

    Please open a new issue (or leave a comment in here if you want this re-opened) if you experience the problem again.

  10. willcl-ark closed this on Oct 14, 2024


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: 2024-12-11 03:12 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me