Skip to content
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

Fix --anonymous-inbound data leak [0.18] #9633

Open
wants to merge 1 commit into
base: release-v0.18
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 23 additions & 10 deletions src/p2p/net_node.inl
Original file line number Diff line number Diff line change
Expand Up @@ -2474,6 +2474,7 @@ namespace nodetool

//fill response
const epee::net_utils::zone zone_type = context.m_remote_address.get_zone();
const bool is_public = (zone_type == epee::net_utils::zone::public_);
network_zone& zone = m_network_zones.at(zone_type);

//will add self to peerlist if in same zone as outgoing later in this function
Expand All @@ -2483,26 +2484,38 @@ namespace nodetool
std::vector<peerlist_entry> local_peerlist_new;
zone.m_peerlist.get_peerlist_head(local_peerlist_new, true, max_peerlist_size);

/* Tor/I2P nodes receiving connections via forwarding (from tor/i2p daemon)
do not know the address of the connecting peer. This is relayed to them,
iff the node has setup an inbound hidden service. The other peer will have
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

small typo here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see the typo, can you elaborate?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought iff is supposed to be if but seems it's an abbreviation I wasn't aware of.

to use the random peer_id value to link the two. My initial thought is that
the inbound peer should leave the other side marked as `<unknown tor host>`,
etc., because someone could give faulty addresses over Tor/I2P to get the
real peer with that identity banned/blacklisted.
\note Insert into `local_peerlist_new` so that it is only sent once like
the other peers. */
if(outgoing_to_same_zone)
local_peerlist_new.push_back(
peerlist_entry{zone.m_our_address, zone.m_config.m_peer_id, (is_public ? std::time(nullptr) : 0)}
);

//only include out peers we did not already send
rsp.local_peerlist_new.reserve(local_peerlist_new.size());
for (auto &pe: local_peerlist_new)
{
if (!context.sent_addresses.insert(pe.adr).second)
continue;
rsp.local_peerlist_new.push_back(std::move(pe));

if (!is_public)
rsp.local_peerlist_new.back().last_seen = 0;
}
m_payload_handler.get_payload_sync_data(rsp.payload_data);

/* Tor/I2P nodes receiving connections via forwarding (from tor/i2p daemon)
do not know the address of the connecting peer. This is relayed to them,
iff the node has setup an inbound hidden service. The other peer will have
to use the random peer_id value to link the two. My initial thought is that
the inbound peer should leave the other side marked as `<unknown tor host>`,
etc., because someone could give faulty addresses over Tor/I2P to get the
real peer with that identity banned/blacklisted. */

if(outgoing_to_same_zone)
rsp.local_peerlist_new.push_back(peerlist_entry{zone.m_our_address, zone.m_config.m_peer_id, std::time(nullptr)});
// randomize so location of local inbound is not easily found
std::shuffle(
rsp.local_peerlist_new.begin(), rsp.local_peerlist_new.end(), crypto::random_device{}
);

LOG_DEBUG_CC(context, "COMMAND_TIMED_SYNC");
return 1;
Expand Down
Loading