-
Notifications
You must be signed in to change notification settings - Fork 37.7k
net: No longer send local address in addrMe #8740
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
Conversation
After bitcoin#8594 the addrFrom sent by a node is not used anymore at all, so don't bother sending it. Also mitigates the privacy issue in (bitcoin#8616). It doesn't completely solve the issue as GetLocalAddress is also called in AdvertiseLocal, but at least when advertising addresses it stands out less as *our* address.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK. The addrFrom
value deserialized from the VERSION message (which this corresponds to) is unused.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK
ACK d9c99c3 The new |
ACK (perhaps backport too) |
d9c99c3 net: No longer send local address in addrMe (Wladimir J. van der Laan)
After bitcoin#8594 the addrFrom sent by a node is not used anymore at all, so don't bother sending it. Also mitigates the privacy issue in (bitcoin#8616). It doesn't completely solve the issue as GetLocalAddress is also called in AdvertiseLocal, but at least when advertising addresses it stands out less as *our* address. Github-Pull: bitcoin#8740 Rebased-From: d9c99c3
This is backported in #8772, removing tag |
…ak the local address fa404f1 test: Check that the version message does not leak the local address of the node (MarcoFalke) Pull request description: Add test for bitcoin#8740 ACKs for top commit: theStack: ACK bitcoin@fa404f1 Tree-SHA512: 4d1c10d1c02fba4b51bd8b9eb3a0d9a682b6aac8c3f6924e295fdca3faefa5ecc3eaa87d347cfec5d2b2bc49963c10fe0a37c463f36088ed0304a2e3716b963b
e733939 [Cleanup] Remove CConnman::Copy(Release)NodeVector, now unused (random-zebra) b09da57 [Refactor] Proper CConnman encapsulation of mnsync (random-zebra) 219061e [Refactor] Decouple SyncWithNode from CMasternodeSync::Process() (random-zebra) e1f3620 [Refactor] Proper CConnman encapsulation of proposals / votes sync (random-zebra) f38c9f9 miner: don't try to access connman if it doesn't exist (Fuzzbawls) 92ab619 Address feedback (Fuzzbawls) 9d5346a Do not set an addr time penalty when a peer advertises itself. (Gregory Maxwell) fe12ff9 net: No longer send local address in addrMe (Wladimir J. van der Laan) 14d6917 net: misc header cleanups (Cory Fields) 34ba0de net: make proxy receives interruptible (Cory Fields) 1bd97ef net: make net processing interruptable (Fuzzbawls) e24b4cc net: make net interruptible (Cory Fields) 814d0de net: add CThreadInterrupt and InterruptibleSleep (Cory Fields) 1443f2a net: a few small cleanups before replacing boost threads (Cory Fields) 58eabe4 net: move MAX_FEELER_CONNECTIONS into connman (Cory Fields) 874baba Convert ForEachNode* functions to take a templated function argument rather than a std::function to eliminate std::function overhead (Jeremy Rubin) 9485ab0 Made the ForEachNode* functions in src/net.cpp more pragmatic and self documenting (Jeremy Rubin) c1e59ad minor net cleanups (Fuzzbawls) 07ae004 net: move vNodesDisconnected into CConnman (Cory Fields) 276c946 net: add nSendBufferMaxSize/nReceiveFloodSize to CConnection::Options (Cory Fields) 22a9aff net: Introduce CConnection::Options to avoid passing so many params (Cory Fields) e4891bf net: Drop StartNode/StopNode and use CConnman directly (Cory Fields) 431575c net: pass CClientUIInterface into CConnman (Cory Fields) 48de47e net: Pass best block known height into CConnman (Cory Fields) 15eed91 net: move max/max-outbound to CConnman (Cory Fields) 2bf0921 net: move semOutbound to CConnman (Cory Fields) 481929f net: move nLocalServices/nRelevantServices to CConnman (Cory Fields) bcee6ae net: move SendBufferSize/ReceiveFloodSize to CConnman (Cory Fields) 6865469 net: move send/recv statistics to CConnman (Cory Fields) 1cec418 net: SocketSendData returns written size (Cory Fields) 2bb9dfa net: move messageHandlerCondition to CConnman (Cory Fields) 9c5a0df net: move nLocalHostNonce to CConnman (Cory Fields) a1394ef net: move nLastNodeId to CConnman (Cory Fields) 3804c29 net: move whitelist functions into CConnman (Cory Fields) dbde9be net: create generic functor accessors and move vNodes to CConnman (Cory Fields) 2e02467 net: Add most functions needed for vNodes to CConnman (Cory Fields) 5667e61 net: move added node functions to CConnman (Cory Fields) 37487ed net: Add oneshot functions to CConnman (Cory Fields) facf878 net: move ban and addrman functions into CConnman (Cory Fields) 091aaf2 net: handle nodesignals in CConnman (Cory Fields) 1e9fa0f net: move OpenNetworkConnection into CConnman (Cory Fields) 573200f net: Move socket binding into CConnman (Cory Fields) 7762b97 net: Pass CConnection to wallet rather than using the global (Fuzzbawls) 00591b8 net: Pass CConnman around as needed (Cory Fields) 2cd3d39 net: Add rpc error for missing/disabled p2p functionality (Cory Fields) f08e316 net: Create CConnman to encapsulate p2p connections (Cory Fields) 66337dc net: move CBanDB and CAddrDB out of net.h/cpp (Fuzzbawls) 10969f6 gui: add NodeID to the peer table (Cory Fields) 58044ac Fix some locks (Pieter Wuille) f9f8926 Do not shadow variables in networking code (Pavel Janík) Pull request description: This is the finalization of the upstream PRs being tracked in #1374 to update much of our networking code to more closely follow upstream improvements. The following PRs are included here: - bitcoin#8466 Do not shadow variables in networking code - bitcoin#8606 Fix some locks - bitcoin#8085 Begin encapsulation - bitcoin#9289 drop boost::thread_group - bitcoin#8740 No longer send local address in addrMe - bitcoin#8661 Do not set an addr time penalty when a peer advertises itself Additionally, during conflict resolution and backporting of 8085, the following additional upstream PR was included: - bitcoin#8715 only delete CConnman if it's been created Still TODO in future PRs: - bitcoin#9037 - bitcoin#9441 - bitcoin#9609 - bitcoin#9626 - bitcoin#9708 - bitcoin#12381 - bitcoin#18584 ACKs for top commit: furszy: same here, code ACK e733939. Nice work ☕ . furszy: ACK e733939 . random-zebra: ACK e733939 and merging... Tree-SHA512: 0fc3cca76d9ddc13f75fc9d48c48d215e6c0e0381377c0318436176a0e0ead73b511e0061a4b63f7052874730b6da8b0ffc2c94cba034bcc39aad4212b69ee22
… does not leak the local address fa404f1 test: Check that the version message does not leak the local address of the node (MarcoFalke) Pull request description: Add test for bitcoin#8740 ACKs for top commit: theStack: ACK bitcoin@fa404f1 Tree-SHA512: 4d1c10d1c02fba4b51bd8b9eb3a0d9a682b6aac8c3f6924e295fdca3faefa5ecc3eaa87d347cfec5d2b2bc49963c10fe0a37c463f36088ed0304a2e3716b963b
…eak the local address (#4368) * Backporting Merge bitcoin#18584: test: Check that the version message does not leak the local address fa404f1 test: Check that the version message does not leak the local address of the node (MarcoFalke) Pull request description: Add test for bitcoin#8740 ACKs for top commit: theStack: ACK bitcoin@fa404f1 Tree-SHA512: 4d1c10d1c02fba4b51bd8b9eb3a0d9a682b6aac8c3f6924e295fdca3faefa5ecc3eaa87d347cfec5d2b2bc49963c10fe0a37c463f36088ed0304a2e3716b963b * Merge bitcoin#18584: test: Check that the version message does not leak the local address Co-authored-by: MarcoFalke <falke.marco@gmail.com>
After #8594 the addrFrom sent by a node is not used anymore at all, so don't bother sending it (it was already not used if it is an invalid address so this doesn't disadvantage older versions either).
Also mitigates the privacy issue in (#8616). It doesn't completely solve the issue as GetLocalAddress is also called in AdvertiseLocal, but at least when advertising addresses it stands out less as our address.