From 80ff0344aebbdebdfa7433d855b0aa9de6c4bed3 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Wed, 14 Dec 2016 16:41:37 -0800 Subject: Dont deserialize nVersion into CNode, should fix #9212 --- src/net_processing.cpp | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) (limited to 'src/net_processing.cpp') diff --git a/src/net_processing.cpp b/src/net_processing.cpp index b9667eb6c..6dcb907bc 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1199,7 +1199,8 @@ bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vR CAddress addrFrom; uint64_t nNonce = 1; uint64_t nServiceInt; - vRecv >> pfrom->nVersion >> nServiceInt >> nTime >> addrMe; + int nVersion; + vRecv >> nVersion >> nServiceInt >> nTime >> addrMe; pfrom->nServices = ServiceFlags(nServiceInt); if (!pfrom->fInbound) { @@ -1214,18 +1215,18 @@ bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vR return false; } - if (pfrom->nVersion < MIN_PEER_PROTO_VERSION) + if (nVersion < MIN_PEER_PROTO_VERSION) { // disconnect from peers older than this proto version - LogPrintf("peer=%d using obsolete version %i; disconnecting\n", pfrom->id, pfrom->nVersion); + LogPrintf("peer=%d using obsolete version %i; disconnecting\n", pfrom->id, nVersion); connman.PushMessage(pfrom, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::REJECT, strCommand, REJECT_OBSOLETE, strprintf("Version must be %d or greater", MIN_PEER_PROTO_VERSION))); pfrom->fDisconnect = true; return false; } - if (pfrom->nVersion == 10300) - pfrom->nVersion = 300; + if (nVersion == 10300) + nVersion = 300; if (!vRecv.empty()) vRecv >> addrFrom >> nNonce; if (!vRecv.empty()) { @@ -1277,7 +1278,8 @@ bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vR // Change version connman.PushMessage(pfrom, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::VERACK)); - int nSendVersion = std::min(pfrom->nVersion, PROTOCOL_VERSION); + int nSendVersion = std::min(nVersion, PROTOCOL_VERSION); + pfrom->nVersion = nVersion; pfrom->SetSendVersion(nSendVersion); if (!pfrom->fInbound) -- cgit v1.2.3 From 2046617b5e06ddb7f960b28219c155995542f029 Mon Sep 17 00:00:00 2001 From: Cory Fields Date: Wed, 18 Jan 2017 18:15:00 -0500 Subject: net: deserialize the entire version message locally This avoids having some vars set if the version negotiation fails. Also copy it all into CNode at the same site. nVersion and fSuccessfullyConnected are set last, as they are the gates for the other vars. Make them atomic for that reason. --- src/net_processing.cpp | 61 ++++++++++++++++++++++++++++---------------------- 1 file changed, 34 insertions(+), 27 deletions(-) (limited to 'src/net_processing.cpp') diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 6dcb907bc..d1e6b1ae0 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1199,16 +1199,23 @@ bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vR CAddress addrFrom; uint64_t nNonce = 1; uint64_t nServiceInt; + ServiceFlags nServices; int nVersion; + int nSendVersion; + std::string strSubVer; + int nStartingHeight = -1; + bool fRelay = true; + vRecv >> nVersion >> nServiceInt >> nTime >> addrMe; - pfrom->nServices = ServiceFlags(nServiceInt); + nSendVersion = std::min(nVersion, PROTOCOL_VERSION); + nServices = ServiceFlags(nServiceInt); if (!pfrom->fInbound) { - connman.SetServices(pfrom->addr, pfrom->nServices); + connman.SetServices(pfrom->addr, nServices); } - if (pfrom->nServicesExpected & ~pfrom->nServices) + if (pfrom->nServicesExpected & ~nServices) { - LogPrint("net", "peer=%d does not offer the expected services (%08x offered, %08x expected); disconnecting\n", pfrom->id, pfrom->nServices, pfrom->nServicesExpected); + LogPrint("net", "peer=%d does not offer the expected services (%08x offered, %08x expected); disconnecting\n", pfrom->id, nServices, pfrom->nServicesExpected); connman.PushMessage(pfrom, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::REJECT, strCommand, REJECT_NONSTANDARD, strprintf("Expected to offer services %08x", pfrom->nServicesExpected))); pfrom->fDisconnect = true; @@ -1230,20 +1237,13 @@ bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vR if (!vRecv.empty()) vRecv >> addrFrom >> nNonce; if (!vRecv.empty()) { - vRecv >> LIMITED_STRING(pfrom->strSubVer, MAX_SUBVERSION_LENGTH); - pfrom->cleanSubVer = SanitizeString(pfrom->strSubVer); + vRecv >> LIMITED_STRING(strSubVer, MAX_SUBVERSION_LENGTH); } if (!vRecv.empty()) { - vRecv >> pfrom->nStartingHeight; + vRecv >> nStartingHeight; } - { - LOCK(pfrom->cs_filter); - if (!vRecv.empty()) - vRecv >> pfrom->fRelayTxes; // set to true after we get the first filter* message - else - pfrom->fRelayTxes = true; - } - + if (!vRecv.empty()) + vRecv >> fRelay; // Disconnect if we connected to ourself if (pfrom->fInbound && !connman.CheckIncomingNonce(nNonce)) { @@ -1252,7 +1252,6 @@ bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vR return true; } - pfrom->addrLocal = addrMe; if (pfrom->fInbound && addrMe.IsRoutable()) { SeenLocal(addrMe); @@ -1262,9 +1261,25 @@ bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vR if (pfrom->fInbound) PushNodeVersion(pfrom, connman, GetAdjustedTime()); - pfrom->fClient = !(pfrom->nServices & NODE_NETWORK); + connman.PushMessage(pfrom, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::VERACK)); + + pfrom->nServices = nServices; + pfrom->addrLocal = addrMe; + pfrom->strSubVer = strSubVer; + pfrom->cleanSubVer = SanitizeString(strSubVer); + pfrom->nStartingHeight = nStartingHeight; + pfrom->fClient = !(nServices & NODE_NETWORK); + { + LOCK(pfrom->cs_filter); + pfrom->fRelayTxes = fRelay; // set to true after we get the first filter* message + } + + // Change version + pfrom->SetSendVersion(nSendVersion); + pfrom->nVersion = nVersion; + pfrom->fSuccessfullyConnected = true; - if((pfrom->nServices & NODE_WITNESS)) + if((nServices & NODE_WITNESS)) { LOCK(cs_main); State(pfrom->GetId())->fHaveWitness = true; @@ -1276,12 +1291,6 @@ bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vR UpdatePreferredDownload(pfrom, State(pfrom->GetId())); } - // Change version - connman.PushMessage(pfrom, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::VERACK)); - int nSendVersion = std::min(nVersion, PROTOCOL_VERSION); - pfrom->nVersion = nVersion; - pfrom->SetSendVersion(nSendVersion); - if (!pfrom->fInbound) { // Advertise our address @@ -1309,8 +1318,6 @@ bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vR connman.MarkAddressGood(pfrom->addr); } - pfrom->fSuccessfullyConnected = true; - std::string remoteAddr; if (fLogIPs) remoteAddr = ", peeraddr=" + pfrom->addr.ToString(); @@ -1352,7 +1359,7 @@ bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vR if (strCommand == NetMsgType::VERACK) { - pfrom->SetRecvVersion(std::min(pfrom->nVersion, PROTOCOL_VERSION)); + pfrom->SetRecvVersion(std::min(pfrom->nVersion.load(), PROTOCOL_VERSION)); if (!pfrom->fInbound) { // Mark this node as currently connected, so we update its timestamp later. -- cgit v1.2.3 From 7a8c2519015650acd51eaf42719f04e53f839bbe Mon Sep 17 00:00:00 2001 From: Cory Fields Date: Thu, 26 Jan 2017 12:35:49 -0500 Subject: net: Disallow sending messages until the version handshake is complete This is a change in behavior, though it's much more sane now than before. --- src/net_processing.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'src/net_processing.cpp') diff --git a/src/net_processing.cpp b/src/net_processing.cpp index d1e6b1ae0..a7acd6edf 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1277,7 +1277,6 @@ bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vR // Change version pfrom->SetSendVersion(nSendVersion); pfrom->nVersion = nVersion; - pfrom->fSuccessfullyConnected = true; if((nServices & NODE_WITNESS)) { @@ -1387,6 +1386,7 @@ bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vR nCMPCTBLOCKVersion = 1; connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::SENDCMPCT, fAnnounceUsingCMPCTBLOCK, nCMPCTBLOCKVersion)); } + pfrom->fSuccessfullyConnected = true; } @@ -2725,8 +2725,8 @@ bool SendMessages(CNode* pto, CConnman& connman, std::atomic& interruptMsg { const Consensus::Params& consensusParams = Params().GetConsensus(); { - // Don't send anything until we get its version message - if (pto->nVersion == 0 || pto->fDisconnect) + // Don't send anything until the version handshake is complete + if (!pto->fSuccessfullyConnected || pto->fDisconnect) return true; // If we get here, the outgoing message serialization version is set and can't change. -- cgit v1.2.3