Merge bitcoin/bitcoin#23695: p2p: Always serialize local timestamp for version msg
fa1dc9b36ap2p: Always serialize local timestamp for version msg (MarcoFalke) Pull request description: Currently we serialize the local time when connecting to outbound connections and the "adjusted network" time when someone connects to us. I presume the reason is to avoid a fingerprint in case the local time is misconfigured. However, the fingerprint still exits when: * The local time goes out-of-sync after timedata is filled up, in which case the adjusted time is *not* adjusted. See comment in `src/timedata.cpp`. (In practise I expect no adjustment to happen after timedata is filled up by one entry more than half its size). * The local time is off by more than 70 minutes. See `DEFAULT_MAX_TIME_ADJUSTMENT`. While there is a warning in this case, the warning might be missed by the node operator. * The adjusted time is poisoned by an attacker. This is only a theoretical concern after commite457513eb1. Using the adjusted time does help in a the case where the local time is off by a constant less than 70 minutes and the node quickly connects to 5 outbound peers to retrieve the adjusted time. Still, I think using `GetAdjustedTime` here gives a false sense of security. It will be better for node operators to instead set the correct time. ACKs for top commit: naumenkogs: ACKfa1dc9b36alaanwj: Code review ACKfa1dc9b36aw0xlt: crACKfa1dc9bTree-SHA512: 70a0f4ab3500e6ddcde291620e35273018cefd1d9e94b91ad333e360139ed18862718bb1a9854af2bf79990bf74b05d95492f77d0747c7b9bdd276c020116dcb
This commit is contained in:
1 file changed
+7
-4
@@ -386,7 +386,7 @@ private:
|
|||||||
EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
|
EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
|
||||||
|
|
||||||
/** Send a version message to a peer */
|
/** Send a version message to a peer */
|
||||||
void PushNodeVersion(CNode& pnode, int64_t nTime);
|
void PushNodeVersion(CNode& pnode);
|
||||||
|
|
||||||
/** Send a ping message every PING_INTERVAL or if requested via RPC. May
|
/** Send a ping message every PING_INTERVAL or if requested via RPC. May
|
||||||
* mark the peer to be disconnected if a ping has timed out.
|
* mark the peer to be disconnected if a ping has timed out.
|
||||||
@@ -1090,12 +1090,13 @@ void PeerManagerImpl::FindNextBlocksToDownload(NodeId nodeid, unsigned int count
|
|||||||
|
|
||||||
} // namespace
|
} // namespace
|
||||||
|
|
||||||
void PeerManagerImpl::PushNodeVersion(CNode& pnode, int64_t nTime)
|
void PeerManagerImpl::PushNodeVersion(CNode& pnode)
|
||||||
{
|
{
|
||||||
// Note that pnode->GetLocalServices() is a reflection of the local
|
// Note that pnode->GetLocalServices() is a reflection of the local
|
||||||
// services we were offering when the CNode object was created for this
|
// services we were offering when the CNode object was created for this
|
||||||
// peer.
|
// peer.
|
||||||
uint64_t my_services{pnode.GetLocalServices()};
|
uint64_t my_services{pnode.GetLocalServices()};
|
||||||
|
const int64_t nTime{count_seconds(GetTime<std::chrono::seconds>())};
|
||||||
uint64_t nonce = pnode.GetLocalNonce();
|
uint64_t nonce = pnode.GetLocalNonce();
|
||||||
const int nNodeStartingHeight{m_best_height};
|
const int nNodeStartingHeight{m_best_height};
|
||||||
NodeId nodeid = pnode.GetId();
|
NodeId nodeid = pnode.GetId();
|
||||||
@@ -1167,7 +1168,7 @@ void PeerManagerImpl::InitializeNode(CNode *pnode)
|
|||||||
m_peer_map.emplace_hint(m_peer_map.end(), nodeid, std::move(peer));
|
m_peer_map.emplace_hint(m_peer_map.end(), nodeid, std::move(peer));
|
||||||
}
|
}
|
||||||
if (!pnode->IsInboundConn()) {
|
if (!pnode->IsInboundConn()) {
|
||||||
PushNodeVersion(*pnode, GetTime());
|
PushNodeVersion(*pnode);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -2599,7 +2600,9 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
|
|||||||
|
|
||||||
// Inbound peers send us their version message when they connect.
|
// Inbound peers send us their version message when they connect.
|
||||||
// We send our version message in response.
|
// We send our version message in response.
|
||||||
if (pfrom.IsInboundConn()) PushNodeVersion(pfrom, GetAdjustedTime());
|
if (pfrom.IsInboundConn()) {
|
||||||
|
PushNodeVersion(pfrom);
|
||||||
|
}
|
||||||
|
|
||||||
// Change version
|
// Change version
|
||||||
const int greatest_common_version = std::min(nVersion, PROTOCOL_VERSION);
|
const int greatest_common_version = std::min(nVersion, PROTOCOL_VERSION);
|
||||||
|
|||||||
Reference in new issue
Block a user