diff options
| author | fanquake <fanquake@gmail.com> | 2021-12-07 17:36:41 +0800 |
|---|---|---|
| committer | fanquake <fanquake@gmail.com> | 2021-12-07 17:36:53 +0800 |
| commit | e457513eb1bad11482f0820feb0f5810324a9d06 (patch) | |
| tree | 599fc5d9b4505eb8b2bba01c62a02b463eeb7fd8 /src/net_processing.cpp | |
| parent | 084c81c8b6ef4c185cb0554b530fd940fbc631e5 (diff) | |
| parent | 0c85dc30e6b628f7538a67776c7eefcb84ef4f82 (diff) | |
| download | bitcoin-e457513eb1bad11482f0820feb0f5810324a9d06.tar.xz | |
Merge bitcoin/bitcoin#23631: p2p: Don't use timestamps from inbound peers for Adjusted Time
0c85dc30e6b628f7538a67776c7eefcb84ef4f82 p2p: Don't use timestamps from inbound peers (Martin Zumsande)
Pull request description:
`GetAdjustedTime()` (used e.g. in validation and addrman) returns a time with an offset that is influenced by timestamps that our peers have sent us in their version message.
Currently, timestamps from all peers are used for this.
However, I think that it would make sense to ignore the timedata samples from inbound peers, making it much harder for others to influence the Adjusted Time in a targeted way.
With the extra feeler connections (every 2 minutes on average) and extra block-relay-only connections (every 5 minutes on average) there are also now plenty of opportunities to gather a meaningful number of timedata samples from outbound peers.
There are some measures in place to prevent abuse: the `-maxtimeadjustment` parameter with a default of 70 minutes, warnings in cases of large deviations, only using the first 200 samples ([explanation](https://github.com/bitcoin/bitcoin/blob/383d350bd5107bfe00e3b90a00cab9a3c1397c72/src/timedata.cpp#L57-L72)), but I think that only using samples from outbound connections in the first place would be an additional safety measure that would make sense.
See also issue #4521 for further context and links: There have been several discussions in the past about replacing or abolishing the existing timedata system.
ACKs for top commit:
jnewbery:
Concept and code review ACK 0c85dc30e6b628f7538a67776c7eefcb84ef4f82
naumenkogs:
ACK 0c85dc30e6b628f7538a67776c7eefcb84ef4f82
vasild:
ACK 0c85dc30e6b628f7538a67776c7eefcb84ef4f82
Tree-SHA512: 2d6375305bcae034d68b58b7a07777b40ac430dfed554c88e681a048c527536691e1b7d08c0ef995247d356f8e81aa0a4b983bf2674faf6a416264e5f1af0a96
Diffstat (limited to 'src/net_processing.cpp')
| -rw-r--r-- | src/net_processing.cpp | 6 |
1 files changed, 5 insertions, 1 deletions
diff --git a/src/net_processing.cpp b/src/net_processing.cpp index d832ff016b..f352246b0d 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -2683,7 +2683,11 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, int64_t nTimeOffset = nTime - GetTime(); pfrom.nTimeOffset = nTimeOffset; - AddTimeData(pfrom.addr, nTimeOffset); + if (!pfrom.IsInboundConn()) { + // Don't use timedata samples from inbound peers to make it + // harder for others to tamper with our adjusted time. + AddTimeData(pfrom.addr, nTimeOffset); + } // If the peer is old enough to have the old alert system, send it the final alert. if (greatest_common_version <= 70012) { |