diff options
| -rw-r--r-- | README.md | 2 | ||||
| -rw-r--r-- | src/crypto/CMakeLists.txt | 1 | ||||
| -rw-r--r-- | src/crypto/crypto.h | 2 | ||||
| -rw-r--r-- | src/crypto/generic-ops.h | 28 | ||||
| -rw-r--r-- | src/cryptonote_protocol/block_queue.cpp | 39 | ||||
| -rw-r--r-- | src/cryptonote_protocol/block_queue.h | 6 | ||||
| -rw-r--r-- | src/device/device_ledger.cpp | 3 | ||||
| -rw-r--r-- | src/mnemonics/electrum-words.cpp | 3 | ||||
| -rw-r--r-- | src/ringct/rctTypes.h | 15 | ||||
| -rw-r--r-- | tests/performance_tests/equality.h | 72 | ||||
| -rw-r--r-- | tests/performance_tests/main.cpp | 6 | ||||
| -rw-r--r-- | tests/unit_tests/crypto.cpp | 15 |
12 files changed, 164 insertions, 28 deletions
@@ -139,7 +139,7 @@ library archives (`.a`). | libzmq | 3.0.0 | NO | `libzmq3-dev` | `zeromq` | `cppzmq-devel` | NO | ZeroMQ library | | OpenPGM | ? | NO | `libpgm-dev` | `libpgm` | `openpgm-devel` | NO | For ZeroMQ | | libunbound | 1.4.16 | YES | `libunbound-dev` | `unbound` | `unbound-devel` | NO | DNS resolver | -| libsodium | ? | NO | `libsodium-dev` | ? | `libsodium-devel` | NO | libsodium | +| libsodium | ? | NO | `libsodium-dev` | ? | `libsodium-devel` | NO | cryptography | | libunwind | any | NO | `libunwind8-dev` | `libunwind` | `libunwind-devel` | YES | Stack traces | | liblzma | any | NO | `liblzma-dev` | `xz` | `xz-devel` | YES | For libunwind | | libreadline | 6.3.0 | NO | `libreadline6-dev` | `readline` | `readline-devel` | YES | Input editing | diff --git a/src/crypto/CMakeLists.txt b/src/crypto/CMakeLists.txt index 71dcedcab..0c635e7cb 100644 --- a/src/crypto/CMakeLists.txt +++ b/src/crypto/CMakeLists.txt @@ -78,6 +78,7 @@ target_link_libraries(cncrypto PUBLIC epee ${Boost_SYSTEM_LIBRARY} + ${SODIUM_LIBRARY} PRIVATE ${EXTRA_LIBRARIES}) diff --git a/src/crypto/crypto.h b/src/crypto/crypto.h index c1576a218..33cc0a25a 100644 --- a/src/crypto/crypto.h +++ b/src/crypto/crypto.h @@ -283,6 +283,6 @@ namespace crypto { } CRYPTO_MAKE_HASHABLE(public_key) -CRYPTO_MAKE_HASHABLE(secret_key) +CRYPTO_MAKE_HASHABLE_CONSTANT_TIME(secret_key) CRYPTO_MAKE_HASHABLE(key_image) CRYPTO_MAKE_COMPARABLE(signature) diff --git a/src/crypto/generic-ops.h b/src/crypto/generic-ops.h index 62bc758c9..42b98706e 100644 --- a/src/crypto/generic-ops.h +++ b/src/crypto/generic-ops.h @@ -33,19 +33,30 @@ #include <cstddef> #include <cstring> #include <functional> +#include <sodium/crypto_verify_32.h> #define CRYPTO_MAKE_COMPARABLE(type) \ namespace crypto { \ inline bool operator==(const type &_v1, const type &_v2) { \ - return std::memcmp(&_v1, &_v2, sizeof(type)) == 0; \ + return !memcmp(&_v1, &_v2, sizeof(_v1)); \ } \ inline bool operator!=(const type &_v1, const type &_v2) { \ - return std::memcmp(&_v1, &_v2, sizeof(type)) != 0; \ + return !operator==(_v1, _v2); \ } \ } -#define CRYPTO_MAKE_HASHABLE(type) \ -CRYPTO_MAKE_COMPARABLE(type) \ +#define CRYPTO_MAKE_COMPARABLE_CONSTANT_TIME(type) \ +namespace crypto { \ + inline bool operator==(const type &_v1, const type &_v2) { \ + static_assert(sizeof(_v1) == 32, "constant time comparison is only implenmted for 32 bytes"); \ + return crypto_verify_32((const unsigned char*)&_v1, (const unsigned char*)&_v2) == 0; \ + } \ + inline bool operator!=(const type &_v1, const type &_v2) { \ + return !operator==(_v1, _v2); \ + } \ +} + +#define CRYPTO_DEFINE_HASH_FUNCTIONS(type) \ namespace crypto { \ static_assert(sizeof(std::size_t) <= sizeof(type), "Size of " #type " must be at least that of size_t"); \ inline std::size_t hash_value(const type &_v) { \ @@ -60,3 +71,12 @@ namespace std { \ } \ }; \ } + +#define CRYPTO_MAKE_HASHABLE(type) \ +CRYPTO_MAKE_COMPARABLE(type) \ +CRYPTO_DEFINE_HASH_FUNCTIONS(type) + +#define CRYPTO_MAKE_HASHABLE_CONSTANT_TIME(type) \ +CRYPTO_MAKE_COMPARABLE_CONSTANT_TIME(type) \ +CRYPTO_DEFINE_HASH_FUNCTIONS(type) + diff --git a/src/cryptonote_protocol/block_queue.cpp b/src/cryptonote_protocol/block_queue.cpp index c39d67ceb..05f4189fb 100644 --- a/src/cryptonote_protocol/block_queue.cpp +++ b/src/cryptonote_protocol/block_queue.cpp @@ -57,7 +57,11 @@ void block_queue::add_blocks(uint64_t height, std::vector<cryptonote::block_comp bool has_hashes = remove_span(height, &hashes); blocks.insert(span(height, std::move(bcel), connection_id, rate, size)); if (has_hashes) + { + for (const crypto::hash &h: hashes) + requested_hashes.insert(h); set_span_hashes(height, connection_id, hashes); + } } void block_queue::add_blocks(uint64_t height, uint64_t nblocks, const boost::uuids::uuid &connection_id, boost::posix_time::ptime time) @@ -76,11 +80,19 @@ void block_queue::flush_spans(const boost::uuids::uuid &connection_id, bool all) block_map::iterator j = i++; if (j->connection_id == connection_id && (all || j->blocks.size() == 0)) { - blocks.erase(j); + erase_block(j); } } } +void block_queue::erase_block(block_map::iterator j) +{ + CHECK_AND_ASSERT_THROW_MES(j != blocks.end(), "Invalid iterator"); + for (const crypto::hash &h: j->hashes) + requested_hashes.erase(h); + blocks.erase(j); +} + void block_queue::flush_stale_spans(const std::set<boost::uuids::uuid> &live_connections) { boost::unique_lock<boost::recursive_mutex> lock(mutex); @@ -92,7 +104,7 @@ void block_queue::flush_stale_spans(const std::set<boost::uuids::uuid> &live_con block_map::iterator j = i++; if (live_connections.find(j->connection_id) == live_connections.end() && j->blocks.size() == 0) { - blocks.erase(j); + erase_block(j); } } } @@ -106,7 +118,7 @@ bool block_queue::remove_span(uint64_t start_block_height, std::vector<crypto::h { if (hashes) *hashes = std::move(i->hashes); - blocks.erase(i); + erase_block(i); return true; } } @@ -121,7 +133,7 @@ void block_queue::remove_spans(const boost::uuids::uuid &connection_id, uint64_t block_map::iterator j = i++; if (j->connection_id == connection_id && j->start_block_height <= start_block_height) { - blocks.erase(j); + erase_block(j); } } } @@ -160,16 +172,15 @@ std::string block_queue::get_overview() const return s; } +inline bool block_queue::requested_internal(const crypto::hash &hash) const +{ + return requested_hashes.find(hash) != requested_hashes.end(); +} + bool block_queue::requested(const crypto::hash &hash) const { boost::unique_lock<boost::recursive_mutex> lock(mutex); - for (const auto &span: blocks) - { - for (const auto &h: span.hashes) - if (h == hash) - return true; - } - return false; + return requested_internal(hash); } std::pair<uint64_t, uint64_t> block_queue::reserve_span(uint64_t first_block_height, uint64_t last_block_height, uint64_t max_blocks, const boost::uuids::uuid &connection_id, const std::vector<crypto::hash> &block_hashes, boost::posix_time::ptime time) @@ -184,7 +195,7 @@ std::pair<uint64_t, uint64_t> block_queue::reserve_span(uint64_t first_block_hei uint64_t span_start_height = last_block_height - block_hashes.size() + 1; std::vector<crypto::hash>::const_iterator i = block_hashes.begin(); - while (i != block_hashes.end() && requested(*i)) + while (i != block_hashes.end() && requested_internal(*i)) { ++i; ++span_start_height; @@ -256,8 +267,10 @@ void block_queue::set_span_hashes(uint64_t start_height, const boost::uuids::uui if (i->start_block_height == start_height && i->connection_id == connection_id) { span s = *i; - blocks.erase(i); + erase_block(i); s.hashes = std::move(hashes); + for (const crypto::hash &h: s.hashes) + requested_hashes.insert(h); blocks.insert(s); return; } diff --git a/src/cryptonote_protocol/block_queue.h b/src/cryptonote_protocol/block_queue.h index 9059e89ac..9cce95075 100644 --- a/src/cryptonote_protocol/block_queue.h +++ b/src/cryptonote_protocol/block_queue.h @@ -33,6 +33,7 @@ #include <string> #include <vector> #include <set> +#include <unordered_set> #include <boost/thread/recursive_mutex.hpp> #include <boost/uuid/uuid.hpp> @@ -93,7 +94,12 @@ namespace cryptonote bool requested(const crypto::hash &hash) const; private: + void erase_block(block_map::iterator j); + inline bool requested_internal(const crypto::hash &hash) const; + + private: block_map blocks; mutable boost::recursive_mutex mutex; + std::unordered_set<crypto::hash> requested_hashes; }; } diff --git a/src/device/device_ledger.cpp b/src/device/device_ledger.cpp index 7a34dad5e..c4e9e40b7 100644 --- a/src/device/device_ledger.cpp +++ b/src/device/device_ledger.cpp @@ -136,7 +136,8 @@ namespace hw { } bool operator==(const crypto::key_derivation &d0, const crypto::key_derivation &d1) { - return !memcmp(&d0, &d1, sizeof(d0)); + static_assert(sizeof(crypto::key_derivation) == 32, "key_derivation must be 32 bytes"); + return !crypto_verify_32((const unsigned char*)&d0, (const unsigned char*)&d1); } /* ===================================================================== */ diff --git a/src/mnemonics/electrum-words.cpp b/src/mnemonics/electrum-words.cpp index 290f2cb93..3d6338856 100644 --- a/src/mnemonics/electrum-words.cpp +++ b/src/mnemonics/electrum-words.cpp @@ -47,6 +47,7 @@ #include "misc_language.h" #include "crypto/crypto.h" // for declaration of crypto::secret_key #include <fstream> +#include "common/int-util.h" #include "mnemonics/electrum-words.h" #include <stdexcept> #include <boost/filesystem.hpp> @@ -411,7 +412,7 @@ namespace crypto { uint32_t w[4]; - memcpy(&w[0], src + (i * 4), 4); + w[0] = SWAP32LE(*(const uint32_t*)(src + (i * 4))); w[1] = w[0] % word_list_length; w[2] = ((w[0] / word_list_length) + w[1]) % word_list_length; diff --git a/src/ringct/rctTypes.h b/src/ringct/rctTypes.h index a3ccf2e85..452a68eb2 100644 --- a/src/ringct/rctTypes.h +++ b/src/ringct/rctTypes.h @@ -36,6 +36,7 @@ #include <vector> #include <iostream> #include <cinttypes> +#include <sodium/crypto_verify_32.h> extern "C" { #include "crypto/crypto-ops.h" @@ -81,7 +82,7 @@ namespace rct { unsigned char operator[](int i) const { return bytes[i]; } - bool operator==(const key &k) const { return !memcmp(bytes, k.bytes, sizeof(bytes)); } + bool operator==(const key &k) const { return !crypto_verify_32(bytes, k.bytes); } unsigned char bytes[32]; }; typedef std::vector<key> keyV; //vector of keys @@ -524,16 +525,16 @@ namespace rct { static inline const crypto::secret_key rct2sk(const rct::key &k) { return (const crypto::secret_key&)k; } static inline const crypto::key_image rct2ki(const rct::key &k) { return (const crypto::key_image&)k; } static inline const crypto::hash rct2hash(const rct::key &k) { return (const crypto::hash&)k; } - static inline bool operator==(const rct::key &k0, const crypto::public_key &k1) { return !memcmp(&k0, &k1, 32); } - static inline bool operator!=(const rct::key &k0, const crypto::public_key &k1) { return memcmp(&k0, &k1, 32); } + static inline bool operator==(const rct::key &k0, const crypto::public_key &k1) { return !crypto_verify_32(k0.bytes, (const unsigned char*)&k1); } + static inline bool operator!=(const rct::key &k0, const crypto::public_key &k1) { return crypto_verify_32(k0.bytes, (const unsigned char*)&k1); } } namespace cryptonote { - static inline bool operator==(const crypto::public_key &k0, const rct::key &k1) { return !memcmp(&k0, &k1, 32); } - static inline bool operator!=(const crypto::public_key &k0, const rct::key &k1) { return memcmp(&k0, &k1, 32); } - static inline bool operator==(const crypto::secret_key &k0, const rct::key &k1) { return !memcmp(&k0, &k1, 32); } - static inline bool operator!=(const crypto::secret_key &k0, const rct::key &k1) { return memcmp(&k0, &k1, 32); } + static inline bool operator==(const crypto::public_key &k0, const rct::key &k1) { return !crypto_verify_32((const unsigned char*)&k0, k1.bytes); } + static inline bool operator!=(const crypto::public_key &k0, const rct::key &k1) { return crypto_verify_32((const unsigned char*)&k0, k1.bytes); } + static inline bool operator==(const crypto::secret_key &k0, const rct::key &k1) { return !crypto_verify_32((const unsigned char*)&k0, k1.bytes); } + static inline bool operator!=(const crypto::secret_key &k0, const rct::key &k1) { return crypto_verify_32((const unsigned char*)&k0, k1.bytes); } } namespace rct { diff --git a/tests/performance_tests/equality.h b/tests/performance_tests/equality.h new file mode 100644 index 000000000..8d24d7da7 --- /dev/null +++ b/tests/performance_tests/equality.h @@ -0,0 +1,72 @@ +// Copyright (c) 2014-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. +// +// Parts of this file are originally copyright (c) 2012-2013 The Cryptonote developers + +#pragma once + +#include <string.h> +#include <sodium/crypto_verify_32.h> + +struct memcmp32 +{ + static const size_t loop_count = 1000000000; + static int call(const unsigned char *k0, const unsigned char *k1){ return memcmp(k0, k1, 32); } +}; + +struct verify32 +{ + static const size_t loop_count = 10000000; + static int call(const unsigned char *k0, const unsigned char *k1){ return crypto_verify_32(k0, k1); } +}; + +template<typename f, bool equal> +class test_equality +{ +public: + static const size_t loop_count = f::loop_count; + + bool init() + { + for (int n = 0; n < 32; ++n) + k0[n] = n; + for (int n = 0; n < 32; ++n) + k1[n] = equal ? n : n + 1; + return true; + } + + bool test() + { + return equal == !f::call(k0, k1); + } + +private: + unsigned char k0[32]; + unsigned char k1[32]; +}; + diff --git a/tests/performance_tests/main.cpp b/tests/performance_tests/main.cpp index bc3622ea8..1733e3409 100644 --- a/tests/performance_tests/main.cpp +++ b/tests/performance_tests/main.cpp @@ -51,6 +51,7 @@ #include "sc_reduce32.h" #include "cn_fast_hash.h" #include "rct_mlsag.h" +#include "equality.h" namespace po = boost::program_options; @@ -151,6 +152,11 @@ int main(int argc, char** argv) TEST_PERFORMANCE3(filter, test_ringct_mlsag, 1, 10, true); TEST_PERFORMANCE3(filter, test_ringct_mlsag, 1, 100, true); + TEST_PERFORMANCE2(filter, test_equality, memcmp32, true); + TEST_PERFORMANCE2(filter, test_equality, memcmp32, false); + TEST_PERFORMANCE2(filter, test_equality, verify32, false); + TEST_PERFORMANCE2(filter, test_equality, verify32, false); + std::cout << "Tests finished. Elapsed time: " << timer.elapsed_ms() / 1000 << " sec" << std::endl; return 0; diff --git a/tests/unit_tests/crypto.cpp b/tests/unit_tests/crypto.cpp index 9e1680568..29fa88f9d 100644 --- a/tests/unit_tests/crypto.cpp +++ b/tests/unit_tests/crypto.cpp @@ -81,3 +81,18 @@ TEST(Crypto, null_keys) ASSERT_EQ(memcmp(crypto::null_skey.data, zero, 32), 0); ASSERT_EQ(memcmp(crypto::null_pkey.data, zero, 32), 0); } + +TEST(Crypto, verify_32) +{ + // all bytes are treated the same, so we can brute force just one byte + unsigned char k0[32] = {0}, k1[32] = {0}; + for (unsigned int i0 = 0; i0 < 256; ++i0) + { + k0[0] = i0; + for (unsigned int i1 = 0; i1 < 256; ++i1) + { + k1[0] = i1; + ASSERT_EQ(!crypto_verify_32(k0, k1), i0 == i1); + } + } +} |