Ignore getaddr messages on Outbound connections. #5442
Conversation
|
Hm. Breaking the symmetry here is odd but may be prudent, resulting in one less (of many) fingerprinting vectors. I'll need to consider some if this is tying our hands or would create other problems. |
|
This change makes sense from a minimalism point of view. If we don't send "getaddr" to incoming connections, and have no reason to consider doing that, it doesn't have to be handled. |
|
As we only use explicit 'getaddr' message to get more addresses in case of a nearly-empty addrman, this shouldn't have much impact. ACK |
|
A nearly-empty addrman is what you have on testnet and other less populated networks. addrman still has some pathological behavior in that area, such as trying the same few addresses over and over again. It would be a shame to increase the pathlogical behavior. Not completely convinced increasing the level of assemetry here has value from a Sybil perspective, but it might. |
|
@jgarzik yea, but we don't do it towards inbound peers and probably don't want to due to self-selecting sybil risk. |
|
@jgarzik Between Bitcoin Core clients, this change should have 0 impact. |
|
Today, yes. Do we want to cut off this avenue for obtaining addresses in the future? |
|
On reflection, I think it's not that concerning for the future: You're free to try, the other side might not respond. Plus, we're probably due to create a new address message type (e.g. supports more service information, supports sending pubkeys, supports longer host identifiers (i2p needs 512 bits, tor's upgraded hs will need 256 bits I think)). Another address type could have different rules. So I think if we regret this change we wouldn't regret it enormously. |
|
ACK. |
|
reluctant ut ACK |
| @@ -3923,7 +3923,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, | |||
| } | |||
|
|
|||
|
|
|||
| else if (strCommand == "getaddr") | |||
| else if ((strCommand == "getaddr") && (pfrom->fInbound)) | |||
There was a problem hiding this comment.
Thinking of it, this really needs a comment in the source code why this asymmetric behavior was introduced. Someone reading the code is bound to wonder why.
19d6093 to
cb2aa8b
Compare
The only time when a client sends a "getaddr" message is when he esatblishes an Outbound connection (see ProcessMessage() in src/main.cpp). Another bitcoin client is expected to receive a "getaddr" message only on Inbound connection. Ignoring "gettaddr" requests on Outbound connections can resolve potential privacy issues (and as was said such request normally do not happen anyway).
cb2aa8b to
dca799e
Compare
|
Added a comment in the source code why this asymmetric behavior was introduced. |
|
re-ACK |
dca799e Ignore getaddr messages on Outbound connections. (Ivan Pustogarov)
|
Backported to 0.10 as 200f293 |
The only time when a client sends a "getaddr" message is when he esatblishes an Outbound connection (see ProcessMessage() in src/main.cpp). Another bitcoin client is expected to receive a "getaddr" message only on Inbound connection. Ignoring "gettaddr" requests on Outbound connections can resolve potential privacy issues (and as was said such request normally do not happen anyway). Rebased-From: dca799e Github-Pull: #5442
The only time when a client sends a "getaddr" message is when he esatblishes an Outbound connection (see ProcessMessage() in src/main.cpp). Another bitcoin client is expected to receive a "getaddr" message only on Inbound connection. Ignoring "gettaddr" requests on Outbound connections can resolve potential privacy issues (and as was said such request normally do not happen anyway). Rebased-From: dca799e Github-Pull: bitcoin#5442 (cherry picked from commit 200f293)
The only time when a client sends a "getaddr" message is when he
esatblishes an Outbound connection (see ProcessMessage() in
src/main.cpp). Another bitcoin client is expected to receive a
"getaddr" message only on Inbound connection. Ignoring "gettaddr"
requests on Outbound connections can resolve potential privacy issues
(and as was said such request normally do not happen anyway).