From 90a16b119f3f35900d0bfc304e8cae2dad20a84b Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Tue, 10 Apr 2018 12:20:31 +0100 Subject: [PATCH 1/2] crypto: fix initialization order issue with random mutex --- src/crypto/crypto.cpp | 15 +++++++++++---- src/crypto/crypto.h | 10 ++++------ 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/src/crypto/crypto.cpp b/src/crypto/crypto.cpp index 494027560..4937a234a 100644 --- a/src/crypto/crypto.cpp +++ b/src/crypto/crypto.cpp @@ -70,8 +70,6 @@ namespace crypto { #include "random.h" } - boost::mutex random_lock; - static inline unsigned char *operator &(ec_point &point) { return &reinterpret_cast(point); } @@ -88,6 +86,13 @@ namespace crypto { return &reinterpret_cast(scalar); } + void generate_random_bytes_thread_safe(size_t N, uint8_t *bytes) + { + static boost::mutex random_lock; + boost::lock_guard lock(random_lock); + generate_random_bytes_not_thread_safe(N, bytes); + } + /* generate a random 32-byte (256-bit) integer and copy it to res */ static inline void random_scalar_not_thread_safe(ec_scalar &res) { unsigned char tmp[64]; @@ -96,8 +101,10 @@ namespace crypto { memcpy(&res, tmp, 32); } static inline void random_scalar(ec_scalar &res) { - boost::lock_guard lock(random_lock); - random_scalar_not_thread_safe(res); + unsigned char tmp[64]; + generate_random_bytes_thread_safe(64, tmp); + sc_reduce(tmp); + memcpy(&res, tmp, 32); } void hash_to_scalar(const void *data, size_t length, ec_scalar &res) { diff --git a/src/crypto/crypto.h b/src/crypto/crypto.h index 81ebfb9e2..9ea0f2ec0 100644 --- a/src/crypto/crypto.h +++ b/src/crypto/crypto.h @@ -53,8 +53,6 @@ namespace crypto { #include "random.h" } - extern boost::mutex random_lock; - #pragma pack(push, 1) POD_CLASS ec_point { char data[32]; @@ -149,11 +147,12 @@ namespace crypto { const public_key *const *, std::size_t, const signature *); }; + void generate_random_bytes_thread_safe(size_t N, uint8_t *bytes); + /* Generate N random bytes */ inline void rand(size_t N, uint8_t *bytes) { - boost::lock_guard lock(random_lock); - generate_random_bytes_not_thread_safe(N, bytes); + generate_random_bytes_thread_safe(N, bytes); } /* Generate a value filled with random bytes. @@ -161,8 +160,7 @@ namespace crypto { template typename std::enable_if::value, T>::type rand() { typename std::remove_cv::type res; - boost::lock_guard lock(random_lock); - generate_random_bytes_not_thread_safe(sizeof(T), &res); + generate_random_bytes_thread_safe(sizeof(T), (uint8_t*)&res); return res; } From 6a61f520e2569f6a690f5c637c6314fb81b0b538 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Mon, 9 Apr 2018 18:42:22 +0100 Subject: [PATCH 2/2] unit_tests: add ringdb unit tests --- src/wallet/ringdb.cpp | 18 +++- src/wallet/ringdb.h | 1 + tests/unit_tests/CMakeLists.txt | 3 +- tests/unit_tests/ringdb.cpp | 164 ++++++++++++++++++++++++++++++++ 4 files changed, 181 insertions(+), 5 deletions(-) create mode 100644 tests/unit_tests/ringdb.cpp diff --git a/src/wallet/ringdb.cpp b/src/wallet/ringdb.cpp index 44992520f..3f2634c8b 100644 --- a/src/wallet/ringdb.cpp +++ b/src/wallet/ringdb.cpp @@ -190,7 +190,8 @@ namespace tools { ringdb::ringdb(std::string filename, const std::string &genesis): - filename(filename) + filename(filename), + env(NULL) { MDB_txn *txn; bool tx_active = false; @@ -227,9 +228,18 @@ ringdb::ringdb(std::string filename, const std::string &genesis): ringdb::~ringdb() { - mdb_dbi_close(env, dbi_rings); - mdb_dbi_close(env, dbi_blackballs); - mdb_env_close(env); + close(); +} + +void ringdb::close() +{ + if (env) + { + mdb_dbi_close(env, dbi_rings); + mdb_dbi_close(env, dbi_blackballs); + mdb_env_close(env); + env = NULL; + } } bool ringdb::add_rings(const crypto::chacha_key &chacha_key, const cryptonote::transaction_prefix &tx) diff --git a/src/wallet/ringdb.h b/src/wallet/ringdb.h index 2bd1ac149..6b4bce124 100644 --- a/src/wallet/ringdb.h +++ b/src/wallet/ringdb.h @@ -41,6 +41,7 @@ namespace tools { public: ringdb(std::string filename, const std::string &genesis); + void close(); ~ringdb(); bool add_rings(const crypto::chacha_key &chacha_key, const cryptonote::transaction_prefix &tx); diff --git a/tests/unit_tests/CMakeLists.txt b/tests/unit_tests/CMakeLists.txt index 9c58536c9..935093184 100644 --- a/tests/unit_tests/CMakeLists.txt +++ b/tests/unit_tests/CMakeLists.txt @@ -68,7 +68,8 @@ set(unit_tests_sources varint.cpp ringct.cpp output_selection.cpp - vercmp.cpp) + vercmp.cpp + ringdb.cpp) set(unit_tests_headers unit_tests_utils.h) diff --git a/tests/unit_tests/ringdb.cpp b/tests/unit_tests/ringdb.cpp new file mode 100644 index 000000000..d50d61b0f --- /dev/null +++ b/tests/unit_tests/ringdb.cpp @@ -0,0 +1,164 @@ +// Copyright (c) 2018, The Monero Project +// +// All rights reserved. +// +// Redistribution and use in source and binary forms, with or without modification, are +// permitted provided that the following conditions are met: +// +// 1. Redistributions of source code must retain the above copyright notice, this list of +// conditions and the following disclaimer. +// +// 2. Redistributions in binary form must reproduce the above copyright notice, this list +// of conditions and the following disclaimer in the documentation and/or other +// materials provided with the distribution. +// +// 3. Neither the name of the copyright holder nor the names of its contributors may be +// used to endorse or promote products derived from this software without specific +// prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY +// EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF +// MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL +// THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, +// PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS +// INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, +// STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF +// THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +#include +#include +#include +#include +#include +#include + +#include "gtest/gtest.h" + +#include "string_tools.h" +#include "crypto/crypto.h" +#include "crypto/random.h" +#include "crypto/chacha.h" +#include "wallet/ringdb.h" + +static crypto::chacha_key generate_chacha_key() +{ + uint8_t key[CHACHA_KEY_SIZE]; + crypto::rand(CHACHA_KEY_SIZE, key); + crypto::chacha_key chacha_key; + memcpy(&chacha_key, key, CHACHA_KEY_SIZE); + return chacha_key; +} + +static crypto::key_image generate_key_image() +{ + return crypto::rand(); +} + +static crypto::public_key generate_output() +{ + return crypto::rand(); +} + + +static const crypto::chacha_key KEY_1 = generate_chacha_key(); +static const crypto::chacha_key KEY_2 = generate_chacha_key(); +static const crypto::key_image KEY_IMAGE_1 = generate_key_image(); +static const crypto::public_key OUTPUT_1 = generate_output(); +static const crypto::public_key OUTPUT_2 = generate_output(); + +class RingDB: public tools::ringdb +{ +public: + RingDB(const char *genesis = ""): tools::ringdb(make_filename(), genesis) { } + ~RingDB() { close(); boost::filesystem::remove_all(filename); free(filename); } + +private: + std::string make_filename() + { + boost::filesystem::path path = tools::get_default_data_dir(); + path /= "fake"; +#if defined(__MINGW32__) || defined(__MINGW__) + filename = tempnam(path.string().c_str(), "ringdb-test-"); + EXPECT_TRUE(filename != NULL); +#else + path /= "ringdb-test-XXXXXX"; + filename = strdup(path.string().c_str()); + EXPECT_TRUE(mkdtemp(filename) != NULL); +#endif + return filename; + } + +private: + char *filename; +}; + +TEST(ringdb, not_found) +{ + RingDB ringdb; + std::vector outs; + ASSERT_FALSE(ringdb.get_ring(KEY_1, KEY_IMAGE_1, outs)); +} + +TEST(ringdb, found) +{ + RingDB ringdb; + std::vector outs, outs2; + outs.push_back(43); outs.push_back(7320); outs.push_back(8429); + ASSERT_TRUE(ringdb.set_ring(KEY_1, KEY_IMAGE_1, outs, false)); + ASSERT_TRUE(ringdb.get_ring(KEY_1, KEY_IMAGE_1, outs2)); + ASSERT_EQ(outs, outs2); +} + +TEST(ringdb, convert) +{ + RingDB ringdb; + std::vector outs, outs2; + outs.push_back(43); outs.push_back(7320); outs.push_back(8429); + ASSERT_TRUE(ringdb.set_ring(KEY_1, KEY_IMAGE_1, outs, true)); + ASSERT_TRUE(ringdb.get_ring(KEY_1, KEY_IMAGE_1, outs2)); + ASSERT_EQ(outs2.size(), 3); + ASSERT_EQ(outs2[0], 43); + ASSERT_EQ(outs2[1], 43+7320); + ASSERT_EQ(outs2[2], 43+7320+8429); +} + +TEST(ringdb, different_genesis) +{ + RingDB ringdb; + std::vector outs, outs2; + outs.push_back(43); outs.push_back(7320); outs.push_back(8429); + ASSERT_TRUE(ringdb.set_ring(KEY_1, KEY_IMAGE_1, outs, false)); + ASSERT_FALSE(ringdb.get_ring(KEY_2, KEY_IMAGE_1, outs2)); +} + +TEST(blackball, not_found) +{ + RingDB ringdb; + ASSERT_TRUE(ringdb.blackball(OUTPUT_1)); + ASSERT_FALSE(ringdb.blackballed(OUTPUT_2)); +} + +TEST(blackball, found) +{ + RingDB ringdb; + ASSERT_TRUE(ringdb.blackball(OUTPUT_1)); + ASSERT_TRUE(ringdb.blackballed(OUTPUT_1)); +} + +TEST(blackball, unblackball) +{ + RingDB ringdb; + ASSERT_TRUE(ringdb.blackball(OUTPUT_1)); + ASSERT_TRUE(ringdb.unblackball(OUTPUT_1)); + ASSERT_FALSE(ringdb.blackballed(OUTPUT_1)); +} + +TEST(blackball, clear) +{ + RingDB ringdb; + ASSERT_TRUE(ringdb.blackball(OUTPUT_1)); + ASSERT_TRUE(ringdb.clear_blackballs()); + ASSERT_FALSE(ringdb.blackballed(OUTPUT_1)); +} +