From 24205d94fe51093067cf9dd64c29d78147e4619c Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Mon, 18 Jan 2021 14:27:00 +0100 Subject: [PATCH] partial bitcoin#20196: fix GetListenPort() to derive the proper port excludes: - 0cfc0cd32239d3c08d2121e028b297022450b320 - 7d64ea4a01920bb55bc6de0de6766712ec792a11 --- src/net.cpp | 12 +++++++- src/net.h | 13 ++++++++- src/test/timedata_tests.cpp | 3 +- src/timedata.cpp | 38 ++++++++++++++++---------- src/timedata.h | 5 ++++ test/functional/p2p_message_capture.py | 2 +- 6 files changed, 55 insertions(+), 18 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index db93469f94..cfc7f85f0d 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -4268,7 +4268,11 @@ void CConnman::UnregisterEvents(CNode *pnode) } #endif } -void CaptureMessage(const CAddress& addr, const std::string& msg_type, const Span& data, bool is_incoming) + +void CaptureMessageToFile(const CAddress& addr, + const std::string& msg_type, + Span data, + bool is_incoming) { // Note: This function captures the message at the time of processing, // not at socket receive/send time. @@ -4295,3 +4299,9 @@ void CaptureMessage(const CAddress& addr, const std::string& msg_type, const Spa ser_writedata32(f, size); f.write(AsBytes(data)); } + +std::function data, + bool is_incoming)> + CaptureMessage = CaptureMessageToFile; diff --git a/src/net.h b/src/net.h index e506033e7a..ad61dcbecb 100644 --- a/src/net.h +++ b/src/net.h @@ -36,6 +36,7 @@ #include #include #include +#include #include #include #include @@ -1528,7 +1529,17 @@ private: std::chrono::microseconds PoissonNextSend(std::chrono::microseconds now, std::chrono::seconds average_interval); /** Dump binary message to file, with timestamp */ -void CaptureMessage(const CAddress& addr, const std::string& msg_type, const Span& data, bool is_incoming); +void CaptureMessageToFile(const CAddress& addr, + const std::string& msg_type, + Span data, + bool is_incoming); + +/** Defaults to `CaptureMessageToFile()`, but can be overridden by unit tests. */ +extern std::function data, + bool is_incoming)> + CaptureMessage; struct NodeEvictionCandidate { diff --git a/src/test/timedata_tests.cpp b/src/test/timedata_tests.cpp index 1dcee23bbb..478d61d5e2 100644 --- a/src/test/timedata_tests.cpp +++ b/src/test/timedata_tests.cpp @@ -96,9 +96,10 @@ BOOST_AUTO_TEST_CASE(addtimedata) // not to fix this because it prevents possible attacks. See the comment in AddTimeData() or issue #4521 // for a more detailed explanation. MultiAddTimeData(2, 100); // filter median is 100 now, but nTimeOffset will not change + // We want this test to end with nTimeOffset==0, otherwise subsequent tests of the suite will fail. BOOST_CHECK_EQUAL(GetTimeOffset(), 0); - // We want this test to end with nTimeOffset==0, otherwise subsequent tests of the suite will fail. + TestOnlyResetTimeData(); } BOOST_AUTO_TEST_SUITE_END() diff --git a/src/timedata.cpp b/src/timedata.cpp index f53fbe231b..bf0427cb95 100644 --- a/src/timedata.cpp +++ b/src/timedata.cpp @@ -39,29 +39,31 @@ int64_t GetAdjustedTime() #define BITCOIN_TIMEDATA_MAX_SAMPLES 200 +static std::set g_sources; +static CMedianFilter g_time_offsets{BITCOIN_TIMEDATA_MAX_SAMPLES, 0}; +static bool g_warning_emitted; + void AddTimeData(const CNetAddr& ip, int64_t nOffsetSample) { LOCK(g_timeoffset_mutex); // Ignore duplicates - static std::set setKnown; - if (setKnown.size() == BITCOIN_TIMEDATA_MAX_SAMPLES) + if (g_sources.size() == BITCOIN_TIMEDATA_MAX_SAMPLES) return; - if (!setKnown.insert(ip).second) + if (!g_sources.insert(ip).second) return; // Add data - static CMedianFilter vTimeOffsets(BITCOIN_TIMEDATA_MAX_SAMPLES, 0); - vTimeOffsets.input(nOffsetSample); - LogPrint(BCLog::NET, "added time data, samples %d, offset %+d (%+d minutes)\n", vTimeOffsets.size(), nOffsetSample, nOffsetSample / 60); + g_time_offsets.input(nOffsetSample); + LogPrint(BCLog::NET, "added time data, samples %d, offset %+d (%+d minutes)\n", g_time_offsets.size(), nOffsetSample, nOffsetSample / 60); // There is a known issue here (see issue #4521): // - // - The structure vTimeOffsets contains up to 200 elements, after which + // - The structure g_time_offsets contains up to 200 elements, after which // any new element added to it will not increase its size, replacing the // oldest element. // // - The condition to update nTimeOffset includes checking whether the - // number of elements in vTimeOffsets is odd, which will never happen after + // number of elements in g_time_offsets is odd, which will never happen after // there are 200 elements. // // But in this case the 'bug' is protective against some attacks, and may @@ -71,9 +73,9 @@ void AddTimeData(const CNetAddr& ip, int64_t nOffsetSample) // So we should hold off on fixing this and clean it up as part of // a timing cleanup that strengthens it in a number of other ways. // - if (vTimeOffsets.size() >= 5 && vTimeOffsets.size() % 2 == 1) { - int64_t nMedian = vTimeOffsets.median(); - std::vector vSorted = vTimeOffsets.sorted(); + if (g_time_offsets.size() >= 5 && g_time_offsets.size() % 2 == 1) { + int64_t nMedian = g_time_offsets.median(); + std::vector vSorted = g_time_offsets.sorted(); // Only let other nodes change our time by so much int64_t max_adjustment = std::max(0, gArgs.GetArg("-maxtimeadjustment", DEFAULT_MAX_TIME_ADJUSTMENT)); if (nMedian >= -max_adjustment && nMedian <= max_adjustment) { @@ -81,8 +83,7 @@ void AddTimeData(const CNetAddr& ip, int64_t nOffsetSample) } else { nTimeOffset = 0; - static bool fDone; - if (!fDone) { + if (!g_warning_emitted) { // If nobody has a time different than ours but within 5 minutes of ours, give a warning bool fMatch = false; for (const int64_t nOffset : vSorted) { @@ -90,7 +91,7 @@ void AddTimeData(const CNetAddr& ip, int64_t nOffsetSample) } if (!fMatch) { - fDone = true; + g_warning_emitted = true; bilingual_str strMessage = strprintf(_("Please check that your computer's date and time are correct! If your clock is wrong, %s will not work properly."), PACKAGE_NAME); SetMiscWarning(strMessage); uiInterface.ThreadSafeMessageBox(strMessage, "", CClientUIInterface::MSG_WARNING); @@ -108,3 +109,12 @@ void AddTimeData(const CNetAddr& ip, int64_t nOffsetSample) } } } + +void TestOnlyResetTimeData() +{ + LOCK(g_timeoffset_mutex); + nTimeOffset = 0; + g_sources.clear(); + g_time_offsets = CMedianFilter{BITCOIN_TIMEDATA_MAX_SAMPLES, 0}; + g_warning_emitted = false; +} diff --git a/src/timedata.h b/src/timedata.h index bc281942a2..902b4dc0f8 100644 --- a/src/timedata.h +++ b/src/timedata.h @@ -75,4 +75,9 @@ int64_t GetTimeOffset(); int64_t GetAdjustedTime(); void AddTimeData(const CNetAddr& ip, int64_t nTime); +/** + * Reset the internal state of GetTimeOffset(), GetAdjustedTime() and AddTimeData(). + */ +void TestOnlyResetTimeData(); + #endif // BITCOIN_TIMEDATA_H diff --git a/test/functional/p2p_message_capture.py b/test/functional/p2p_message_capture.py index 0440a8e447..8a3861dce6 100755 --- a/test/functional/p2p_message_capture.py +++ b/test/functional/p2p_message_capture.py @@ -20,7 +20,7 @@ LENGTH_SIZE = 4 MSGTYPE_SIZE = 12 def mini_parser(dat_file): - """Parse a data file created by CaptureMessage. + """Parse a data file created by CaptureMessageToFile. From the data file we'll only check the structure.