diff --git a/src/Makefile.test.include b/src/Makefile.test.include index 96e2f4ddfd66..e8e3cda233fe 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -126,7 +126,6 @@ BITCOIN_TESTS =\ test/raii_event_tests.cpp \ test/random_tests.cpp \ test/rbf_tests.cpp \ - test/readwritefile_tests.cpp \ test/rest_tests.cpp \ test/result_tests.cpp \ test/reverselock_tests.cpp \ diff --git a/src/i2p.cpp b/src/i2p.cpp index 1e3e76872398..c891562d00bf 100644 --- a/src/i2p.cpp +++ b/src/i2p.cpp @@ -434,9 +434,9 @@ void Session::CreateIfNotCreatedAlready() } else { // Read our persistent destination (private key) from disk or generate // one and save it to disk. Then use it when creating the session. - std::optional data{ReadBinaryFile(m_private_key_file)}; - if (data) { - m_private_key.assign(data->begin(), data->end()); + const auto& [read_ok, data] = ReadBinaryFile(m_private_key_file); + if (read_ok) { + m_private_key.assign(data.begin(), data.end()); } else { GenerateAndSavePrivateKey(*sock); } diff --git a/src/key.h b/src/key.h index dbe527dd88b0..200289d920c3 100644 --- a/src/key.h +++ b/src/key.h @@ -213,6 +213,19 @@ class CKey * Returns whether the public key as an even Y coordinate. */ bool HasEvenY() const; + + template + inline void Unserialize(Stream& s) { + s >> fCompressed; + MakeKeyData(); + s >> MakeWritableByteSpan(*keydata); + } + + template + void Serialize(Stream &s) const { + s << fCompressed; + ::Serialize(s, MakeUCharSpan(*this)); + } }; CKey GenerateRandomKey(bool compressed = true) noexcept; diff --git a/src/node/sv2_template_provider.cpp b/src/node/sv2_template_provider.cpp index dbfa012466fd..0b24b17fa457 100644 --- a/src/node/sv2_template_provider.cpp +++ b/src/node/sv2_template_provider.cpp @@ -26,23 +26,22 @@ using node::Sv2SubmitSolutionMsg; Sv2TemplateProvider::Sv2TemplateProvider(ChainstateManager& chainman, CTxMemPool& mempool) : m_chainman{chainman}, m_mempool{mempool} { // Read static key if cached - auto data{ReadBinaryFile>(GetStaticKeyFile())}; - if (data) { - if (data->size() != 32) { - LogPrintLevel(BCLog::SV2, BCLog::Level::Error, - "Failed to load static key from %s: size %d != 32\n", - fs::PathToString(GetStaticKeyFile()), data->size()); - } + try { + AutoFile{fsbridge::fopen(GetStaticKeyFile(), "wb")} >> m_static_key; LogPrintLevel(BCLog::SV2, BCLog::Level::Debug, "Reading cached static key from %s\n", fs::PathToString(GetStaticKeyFile())); - m_static_key.Set(data->begin(), data->end(), /*fCompressedIn=*/true); - } else { + } catch (const std::ios_base::failure&) { + // File is not expected to exist the first time. + // In the unlikely event that loading an existing key fails, create a new one. + } + if (!m_static_key.IsValid()) { m_static_key = GenerateRandomKey(); - std::vector data(m_static_key.begin(), m_static_key.end()); - if (WriteBinaryFile(GetStaticKeyFile(), data)) { - LogPrintLevel(BCLog::SV2, BCLog::Level::Debug, "Generated static key, saved to %s\n", fs::PathToString(GetStaticKeyFile())); - } else { + try { + AutoFile{fsbridge::fopen(GetStaticKeyFile(), "wb")} << m_static_key; + } catch (const std::ios_base::failure&) { LogPrintLevel(BCLog::SV2, BCLog::Level::Error, "Error writing static key to %s\n", fs::PathToString(GetStaticKeyFile())); + // Continue, because this is not a critical failure. } + LogPrintLevel(BCLog::SV2, BCLog::Level::Debug, "Generated static key, saved to %s\n", fs::PathToString(GetStaticKeyFile())); } LogPrintLevel(BCLog::SV2, BCLog::Level::Info, "Static key: %s\n", HexStr(m_static_key.GetPubKey())); @@ -51,23 +50,21 @@ Sv2TemplateProvider::Sv2TemplateProvider(ChainstateManager& chainman, CTxMemPool // Load authority key if cached CKey authority_key; - auto auth_key_data{ReadBinaryFile>(GetAuthorityKeyFile())}; - if (auth_key_data) { - if (auth_key_data->size() != 32) { - LogPrintLevel(BCLog::SV2, BCLog::Level::Error, - "Failed to load authority key from %s: size %d != 32\n", - fs::PathToString(GetAuthorityKeyFile()), auth_key_data->size()); - } - LogPrintLevel(BCLog::SV2, BCLog::Level::Debug, "Reading cached authority key from %s\n", fs::PathToString(GetAuthorityKeyFile())); - authority_key.Set(auth_key_data->begin(), auth_key_data->end(), /*fCompressedIn=*/true); - } else { + try { + AutoFile{fsbridge::fopen(GetAuthorityKeyFile(), "wb")} >> authority_key; + } catch (const std::ios_base::failure&) { + // File is not expected to exist the first time. + // In the unlikely event that loading an existing key fails, create a new one. + } + if (!authority_key.IsValid()) { authority_key = GenerateRandomKey(); - std::vector data(m_static_key.begin(), m_static_key.end()); - if (WriteBinaryFile(GetStaticKeyFile(), data)) { - LogPrintLevel(BCLog::SV2, BCLog::Level::Debug, "Generated authority key, saved to %s\n", fs::PathToString(GetAuthorityKeyFile())); - } else { + try { + AutoFile{fsbridge::fopen(GetAuthorityKeyFile(), "wb")} << authority_key; + } catch (const std::ios_base::failure&) { LogPrintLevel(BCLog::SV2, BCLog::Level::Error, "Error writing authority key to %s\n", fs::PathToString(GetAuthorityKeyFile())); + // Continue, because this is not a critical failure. } + LogPrintLevel(BCLog::SV2, BCLog::Level::Debug, "Generated authority key, saved to %s\n", fs::PathToString(GetAuthorityKeyFile())); } // SRI uses base58 encoded x-only pubkeys in its configuration files LogPrintLevel(BCLog::SV2, BCLog::Level::Info, "Authority key: %s\n", EncodeBase58(XOnlyPubKey(m_static_key.GetPubKey()))); diff --git a/src/test/key_tests.cpp b/src/test/key_tests.cpp index 03397ca1fcfb..3cf408168c56 100644 --- a/src/test/key_tests.cpp +++ b/src/test/key_tests.cpp @@ -398,4 +398,26 @@ BOOST_AUTO_TEST_CASE(ecdh_test) BOOST_CHECK(std::memcmp(&ecdh_output_1[0], &ecdh_output_2[0], 32) == 0); } +BOOST_AUTO_TEST_CASE(key_serialization) +{ + { + CKey key{GenerateRandomKey()}; + DataStream s{}; + s << key; + CKey key_copy; + s >> key_copy; + BOOST_CHECK(key == key_copy); + } + + { + CKey key{GenerateRandomKey(/*compressed=*/false)}; + DataStream s{}; + s << key; + CKey key_copy; + s >> key_copy; + BOOST_CHECK(key == key_copy); + } + +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/readwritefile_tests.cpp b/src/test/readwritefile_tests.cpp deleted file mode 100644 index 9756b83a9eb5..000000000000 --- a/src/test/readwritefile_tests.cpp +++ /dev/null @@ -1,86 +0,0 @@ -// Copyright (c) 2024 The Bitcoin Core developers -// Distributed under the MIT software license, see the accompanying -// file COPYING or http://www.opensource.org/licenses/mit-license.php. - -#include -#include -#include - -#include -#include -#include -#include -#include - -#include - -BOOST_FIXTURE_TEST_SUITE(readwritefile_tests, BasicTestingSetup) - -BOOST_AUTO_TEST_CASE(util_ReadBinaryFile) -{ - fs::path tmpfolder = m_args.GetDataDirBase(); - fs::path tmpfile = tmpfolder / "read_binary.dat"; - std::string expected_text; - for (int i = 0; i < 30; i++) { - expected_text += "0123456789"; - } - { - std::ofstream file{tmpfile}; - file << expected_text; - } - { - // read all contents in file - auto text{ReadBinaryFile(tmpfile)}; - BOOST_REQUIRE(text); - BOOST_CHECK_EQUAL(text.value(), expected_text); - } - { - // read half contents in file - auto text{ReadBinaryFile(tmpfile, expected_text.size() / 2)}; - BOOST_REQUIRE(text); - BOOST_CHECK_EQUAL(text.value(), expected_text.substr(0, expected_text.size() / 2)); - } - { - // read from non-existent file - fs::path invalid_file = tmpfolder / "invalid_binary.dat"; - auto text{ReadBinaryFile(invalid_file)}; - BOOST_REQUIRE(!text); - } - { - std::vector expected_data; - for (int i = 0; i < 30; i++) { - // store result as bytes (ASCII "0" is character 48) - expected_data.insert(expected_data.end(), {48,49,50,51,52,53,54,55,56,57}); - } - auto data{ReadBinaryFile>(tmpfile)}; - BOOST_REQUIRE(data); - BOOST_REQUIRE(data.value() == expected_data); - } -} - -BOOST_AUTO_TEST_CASE(util_WriteBinaryFile) -{ - fs::path tmpfolder = m_args.GetDataDirBase(); - fs::path tmpfile = tmpfolder / "write_binary.dat"; - std::string expected_text = "bitcoin"; - { - auto valid = WriteBinaryFile(tmpfile, expected_text); - std::string actual_text; - std::ifstream file{tmpfile}; - file >> actual_text; - BOOST_CHECK(valid); - BOOST_CHECK_EQUAL(actual_text, expected_text); - } - { - std::vector bytes{98, 105, 116, 99, 111, 105, 110}; // "bitcoin" - auto valid = WriteBinaryFile(tmpfile, bytes); - std::string actual_text; - std::ifstream file{tmpfile}; - file >> actual_text; - BOOST_CHECK(valid); - BOOST_CHECK_EQUAL(actual_text, expected_text); - } - -} - -BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index 17f7bfaba5ed..47808a2a5811 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -15,6 +15,7 @@ #include // For MessageSign(), MessageVerify(), MESSAGE_MAGIC #include #include +#include #include #include #include @@ -1744,6 +1745,52 @@ BOOST_AUTO_TEST_CASE(util_ParseByteUnits) BOOST_CHECK(!ParseByteUnits("1x", noop)); } +BOOST_AUTO_TEST_CASE(util_ReadBinaryFile) +{ + fs::path tmpfolder = m_args.GetDataDirBase(); + fs::path tmpfile = tmpfolder / "read_binary.dat"; + std::string expected_text; + for (int i = 0; i < 30; i++) { + expected_text += "0123456789"; + } + { + std::ofstream file{tmpfile}; + file << expected_text; + } + { + // read all contents in file + auto [valid, text] = ReadBinaryFile(tmpfile); + BOOST_CHECK(valid); + BOOST_CHECK_EQUAL(text, expected_text); + } + { + // read half contents in file + auto [valid, text] = ReadBinaryFile(tmpfile, expected_text.size() / 2); + BOOST_CHECK(valid); + BOOST_CHECK_EQUAL(text, expected_text.substr(0, expected_text.size() / 2)); + } + { + // read from non-existent file + fs::path invalid_file = tmpfolder / "invalid_binary.dat"; + auto [valid, text] = ReadBinaryFile(invalid_file); + BOOST_CHECK(!valid); + BOOST_CHECK(text.empty()); + } +} + +BOOST_AUTO_TEST_CASE(util_WriteBinaryFile) +{ + fs::path tmpfolder = m_args.GetDataDirBase(); + fs::path tmpfile = tmpfolder / "write_binary.dat"; + std::string expected_text = "bitcoin"; + auto valid = WriteBinaryFile(tmpfile, expected_text); + std::string actual_text; + std::ifstream file{tmpfile}; + file >> actual_text; + BOOST_CHECK(valid); + BOOST_CHECK_EQUAL(actual_text, expected_text); +} + BOOST_AUTO_TEST_CASE(clearshrink_test) { { diff --git a/src/torcontrol.cpp b/src/torcontrol.cpp index 36cdf69d81dc..442c1c4d422d 100644 --- a/src/torcontrol.cpp +++ b/src/torcontrol.cpp @@ -328,10 +328,10 @@ TorController::TorController(struct event_base* _base, const std::string& tor_co LogPrintf("tor: Initiating connection to Tor control port %s failed\n", m_tor_control_center); } // Read service private key if cached - auto data{ReadBinaryFile(GetPrivateKeyFile())}; - if (data) { + std::pair pkf = ReadBinaryFile(GetPrivateKeyFile()); + if (pkf.first) { LogPrint(BCLog::TOR, "Reading cached private key from %s\n", fs::PathToString(GetPrivateKeyFile())); - private_key = data.value(); + private_key = pkf.second; } } @@ -586,15 +586,15 @@ void TorController::protocolinfo_cb(TorControlConnection& _conn, const TorContro } else if (methods.count("SAFECOOKIE")) { // Cookie: hexdump -e '32/1 "%02x""\n"' ~/.tor/control_auth_cookie LogPrint(BCLog::TOR, "Using SAFECOOKIE authentication, reading cookie authentication from %s\n", cookiefile); - auto status_cookie = ReadBinaryFile(fs::PathFromString(cookiefile), TOR_COOKIE_SIZE); - if (status_cookie && status_cookie->size() == TOR_COOKIE_SIZE) { - // _conn.Command("AUTHENTICATE " + HexStr(status_cookie), std::bind(&TorController::auth_cb, this, std::placeholders::_1, std::placeholders::_2)); - cookie = std::vector(status_cookie->begin(), status_cookie->end()); + std::pair status_cookie = ReadBinaryFile(fs::PathFromString(cookiefile), TOR_COOKIE_SIZE); + if (status_cookie.first && status_cookie.second.size() == TOR_COOKIE_SIZE) { + // _conn.Command("AUTHENTICATE " + HexStr(status_cookie.second), std::bind(&TorController::auth_cb, this, std::placeholders::_1, std::placeholders::_2)); + cookie = std::vector(status_cookie.second.begin(), status_cookie.second.end()); clientNonce = std::vector(TOR_NONCE_SIZE, 0); GetRandBytes(clientNonce); _conn.Command("AUTHCHALLENGE SAFECOOKIE " + HexStr(clientNonce), std::bind(&TorController::authchallenge_cb, this, std::placeholders::_1, std::placeholders::_2)); } else { - if (status_cookie) { + if (status_cookie.first) { LogPrintf("tor: Authentication cookie %s is not exactly %i bytes, as is required by the spec\n", cookiefile, TOR_COOKIE_SIZE); } else { LogPrintf("tor: Authentication cookie %s could not be opened (check permissions)\n", cookiefile); diff --git a/src/util/readwritefile.cpp b/src/util/readwritefile.cpp index 7bfbd8a8da86..773d7ad1cccd 100644 --- a/src/util/readwritefile.cpp +++ b/src/util/readwritefile.cpp @@ -12,34 +12,29 @@ #include #include #include -#include -template -std::optional ReadBinaryFile(const fs::path& filename, size_t maxsize) +std::pair ReadBinaryFile(const fs::path &filename, size_t maxsize) { FILE *f = fsbridge::fopen(filename, "rb"); - if (f == nullptr) return {}; - T output{}; + if (f == nullptr) + return std::make_pair(false,""); + std::string retval; char buffer[128]; do { - const size_t n = fread(buffer, 1, std::min(sizeof(buffer), maxsize - output.size()), f); + const size_t n = fread(buffer, 1, std::min(sizeof(buffer), maxsize - retval.size()), f); // Check for reading errors so we don't return any data if we couldn't // read the entire file (or up to maxsize) if (ferror(f)) { fclose(f); - return {}; + return std::make_pair(false,""); } - output.insert(output.end(), buffer, buffer + n); - } while (!feof(f) && output.size() < maxsize); + retval.append(buffer, buffer+n); + } while (!feof(f) && retval.size() < maxsize); fclose(f); - return output; + return std::make_pair(true,retval); } -template std::optional ReadBinaryFile(const fs::path &filename, size_t maxsize); -template std::optional> ReadBinaryFile(const fs::path &filename, size_t maxsize); - -template -bool WriteBinaryFile(const fs::path& filename, const T& data) +bool WriteBinaryFile(const fs::path &filename, const std::string &data) { FILE *f = fsbridge::fopen(filename, "wb"); if (f == nullptr) @@ -53,6 +48,3 @@ bool WriteBinaryFile(const fs::path& filename, const T& data) } return true; } - -template bool WriteBinaryFile(const fs::path& filename, const std::string& data); -template bool WriteBinaryFile(const fs::path& filename, const std::vector& data); diff --git a/src/util/readwritefile.h b/src/util/readwritefile.h index 23f08bde7e19..6ddfcb4f35a2 100644 --- a/src/util/readwritefile.h +++ b/src/util/readwritefile.h @@ -8,27 +8,21 @@ #include #include -#include #include #include -/** - * Read full contents of a file and return one of the following formats: - * 1. std::vector - * 2. std::string +/** Read full contents of a file and return them in a std::string. + * Returns a pair . + * If an error occurred, status will be false, otherwise status will be true and the data will be returned in string. * - * @param[in] filename Filename. Returns false it doesn't exist. - * @param[in] maxsize Puts a maximum size limit on the file that is read. If the file - * is larger than this, truncated data (with len > maxsize) will be returned. - * @return result successful, {} otherwise + * @param maxsize Puts a maximum size limit on the file that is read. If the file is larger than this, truncated data + * (with len > maxsize) will be returned. */ -template -std::optional ReadBinaryFile(const fs::path& filename, size_t maxsize=std::numeric_limits::max()); +std::pair ReadBinaryFile(const fs::path &filename, size_t maxsize=std::numeric_limits::max()); -/** Write contents of std::string or std::vector to a file. +/** Write contents of std::string to a file. * @return true on success. */ -template -bool WriteBinaryFile(const fs::path& filename, const T& data); +bool WriteBinaryFile(const fs::path &filename, const std::string &data); #endif // BITCOIN_UTIL_READWRITEFILE_H