Skip to content

Reduce fingerprinting through timestamps in 'addr' messages. #5860

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.

Sign up for GitHub

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

Jump to bottom
Merged
laanwj merged 1 commit into from
Mar 17, 2015
Merged

Reduce fingerprinting through timestamps in 'addr' messages. #5860

laanwj merged 1 commit into from
Mar 17, 2015

Conversation

@sipa
Copy link
Member

@sipa sipa commented Mar 6, 2015

Suggested by @jonasnick.

@fanquake
Copy link
Member

fanquake commented Mar 8, 2015

Travis failure

Cleaning up
terminate called after throwing an instance of 'boost::exception_detail::clone_impl<boost::exception_detail::error_info_injector<boost::lock_error> >'
  what():  boost: mutex lock failed in pthread_mutex_lock: Invalid argument
terminate called after throwing an instance of 'boost::exception_detail::clone_impl<boost::exception_detail::error_info_injector<boost::lock_error> >'
  what():  boost: mutex lock failed in pthread_mutex_lock: Invalid argument
*** glibc detected *** /home/travis/build/bitcoin/bitcoin/bitcoin-x86_64-unknown-linux-gnu/src/bitcoind: malloc(): memory corruption: 0x00007fd8131c3ab1 ***

@laanwj laanwj added this to the 0.10.0 milestone Mar 9, 2015
@laanwj laanwj added the P2P label Mar 9, 2015
@Diapolo
Copy link

Diapolo commented Mar 9, 2015

@laanwj Isn't 0.10.0 released ;)? Why did you tag this 0.10.0 then?

@laanwj
Copy link
Member

laanwj commented Mar 9, 2015

@Diapolo Don't be so pedantic, I'm using it as marker for the next version on the 0.10 branch.

@Diapolo
Copy link

Diapolo commented Mar 9, 2015

@laanwj Just asking, no offense :).

@laanwj
Copy link
Member

laanwj commented Mar 9, 2015

@Diapolo okay :)

@sipa @fanquake Tried a Travis respin, still same problem on same configuration.

@laanwj
Copy link
Member

laanwj commented Mar 11, 2015

I'm also getting these errors when running the RPC tests locally with this patch:

bitcoind: /usr/include/boost/thread/pthread/recursive_mutex.hpp:110: void boost::recursive_mutex::lock(): Assertion `!pthread_mutex_lock(&m)' failed.
bitcoind: /usr/include/boost/thread/pthread/recursive_mutex.hpp:110: void boost::recursive_mutex::lock(): Assertion `!pthread_mutex_lock(&m)' failed.

This appears to be a problem with destruction order. By moving addrman.Connected(addr); to the CNode destructor, addrman is called when each node is deleted. This may cause a use-after-free of a mutex?

@jgarzik
Copy link
Contributor

jgarzik commented Mar 11, 2015

concept ACK

@sipa
Copy link
Member Author

sipa commented Mar 11, 2015

Changed approach.

@jonasnick
Copy link
Contributor

jonasnick commented Mar 11, 2015

To give a bit of context, without this PR it is possible to learn all outbound peers of a node by repeatedly sending 'getaddr' messages. See the histogram for the timestamps of a full node and a client client.

@sipa sipa force-pushed the addrfinger branch 2 times, most recently from 1880f2c to d88011e Compare March 12, 2015 13:01
@laanwj
Copy link
Member

laanwj commented Mar 13, 2015

utACK

laanwj
laanwj reviewed Mar 16, 2015
View reviewed changes
src/main.cpp Outdated
Copy link
Member

@laanwj laanwj Mar 16, 2015

Choose a reason for hiding this comment

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

This comment is no longer correct, should probably be Update the connected status for this node

Copy link
Member Author

@sipa sipa Mar 17, 2015

Choose a reason for hiding this comment

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

Done.

@jonasnick
Copy link
Contributor

jonasnick commented Mar 16, 2015

untested ack

@laanwj laanwj merged commit 9c27379 into bitcoin:master Mar 17, 2015
laanwj added a commit that referenced this pull request Mar 17, 2015
@laanwj
Merge pull request #5860
93a8c46 
9c27379 Reduce fingerprinting through timestamps in 'addr' messages. (Pieter Wuille)
sipa added a commit that referenced this pull request Mar 17, 2015
@sipa @laanwj
Reduce fingerprinting through timestamps in 'addr' messages.
ca301bf 
Suggested by Jonas Nick.

Rebased-From: 9c27379
Github-Pull: #5860
@laanwj
Copy link
Member

laanwj commented Mar 17, 2015

Cherry-picked to 0.10 as ca301bf

laanwj added a commit that referenced this pull request Apr 6, 2015
@laanwj
doc: Credit Jonas Nick in release notes
149c1d8 
For discovering the vulnerability and discussing the fix that led to PR #5860.
@maflcko maflcko mentioned this pull request Jul 18, 2019
Merged
reddink pushed a commit to reddcoin-project/reddcoin-3.10 that referenced this pull request Jul 11, 2020
@sipa @reddink
Reduce fingerprinting through timestamps in 'addr' messages.
bc820a9 
Suggested by Jonas Nick.

Rebased-From: 9c27379
Github-Pull: bitcoin#5860
(cherry picked from commit ca301bf)
reddink pushed a commit to reddcoin-project/reddcoin-3.10 that referenced this pull request Jul 14, 2020
@sipa @reddink
Reduce fingerprinting through timestamps in 'addr' messages.
5ddae18 
Suggested by Jonas Nick.

Rebased-From: 9c27379
Github-Pull: bitcoin#5860
(cherry picked from commit ca301bf)
@jnewbery jnewbery mentioned this pull request Aug 18, 2020
Merged
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

P2P

Projects

None yet

Milestone

0.10.0

Development

Successfully merging this pull request may close these issues.

6 participants

@sipa @fanquake @Diapolo @laanwj @jgarzik @jonasnick