From fd35e2304ab5223edd9c6caf03a16c19330e972d Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Tue, 15 Oct 2019 18:10:29 +0000 Subject: wallet: fix another facet of "did I get some monero" information leak We get new pool txes before processing any tx, pool or not. This ensures that if we're asked for a password, this does not cause a measurable delay in the txpool query after the last block query. --- src/simplewallet/simplewallet.cpp | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) (limited to 'src/simplewallet') diff --git a/src/simplewallet/simplewallet.cpp b/src/simplewallet/simplewallet.cpp index 03693a57c..7cf0b4913 100644 --- a/src/simplewallet/simplewallet.cpp +++ b/src/simplewallet/simplewallet.cpp @@ -8356,7 +8356,11 @@ bool simple_wallet::get_transfers(std::vector& local_args, std::vec m_in_manual_refresh.store(true, std::memory_order_relaxed); epee::misc_utils::auto_scope_leave_caller scope_exit_handler = epee::misc_utils::create_scope_leave_handler([&](){m_in_manual_refresh.store(false, std::memory_order_relaxed);}); - m_wallet->update_pool_state(); + std::vector> process_txs; + m_wallet->update_pool_state(process_txs); + if (!process_txs.empty()) + m_wallet->process_pool_state(process_txs); + std::list> payments; m_wallet->get_unconfirmed_payments(payments, m_current_subaddress_account, subaddr_indices); for (std::list>::const_iterator i = payments.begin(); i != payments.end(); ++i) { @@ -10002,7 +10006,11 @@ bool simple_wallet::show_transfer(const std::vector &args) try { - m_wallet->update_pool_state(); + std::vector> process_txs; + m_wallet->update_pool_state(process_txs); + if (!process_txs.empty()) + m_wallet->process_pool_state(process_txs); + std::list> pool_payments; m_wallet->get_unconfirmed_payments(pool_payments, m_current_subaddress_account); for (std::list>::const_iterator i = pool_payments.begin(); i != pool_payments.end(); ++i) { -- cgit v1.2.3 From 38f6910481f9bb61c4d6bf8409fddd6426384a58 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Tue, 15 Oct 2019 10:30:50 +0000 Subject: simplewallet: plug a timing leak MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As reported by Tramèr et al, timing of refresh requests can be used to see whether a password was requested (and thus at least one output received) since this will induce a delay in subsequent calls. To avoid this, we schedule calls at a given time instead of sleeping for a set time (which would make delays additive). To further avoid a scheduled call being during the time in which a password is prompted, the actual scheduled time is now randomized. --- src/simplewallet/simplewallet.cpp | 33 ++++++++++++++++++++++++++------- src/simplewallet/simplewallet.h | 8 +++++--- 2 files changed, 31 insertions(+), 10 deletions(-) (limited to 'src/simplewallet') diff --git a/src/simplewallet/simplewallet.cpp b/src/simplewallet/simplewallet.cpp index 7cf0b4913..ea8f6f2f5 100644 --- a/src/simplewallet/simplewallet.cpp +++ b/src/simplewallet/simplewallet.cpp @@ -8807,22 +8807,41 @@ void simple_wallet::check_for_messages() //---------------------------------------------------------------------------------------------------- void simple_wallet::wallet_idle_thread() { + const boost::posix_time::ptime start_time = boost::posix_time::microsec_clock::universal_time(); while (true) { boost::unique_lock lock(m_idle_mutex); if (!m_idle_run.load(std::memory_order_relaxed)) break; + // if another thread was busy (ie, a foreground refresh thread), we'll end up here at + // some random time that's not what we slept for, so we should not call refresh now + // or we'll be leaking that fact through timing + const boost::posix_time::ptime now0 = boost::posix_time::microsec_clock::universal_time(); + const uint64_t dt_actual = (now0 - start_time).total_microseconds() % 1000000; +#ifdef _WIN32 + static const uint64_t threshold = 10000; +#else + static const uint64_t threshold = 2000; +#endif + if (dt_actual < threshold) // if less than a threshold... would a very slow machine always miss it ? + { #ifndef _WIN32 - m_inactivity_checker.do_call(boost::bind(&simple_wallet::check_inactivity, this)); + m_inactivity_checker.do_call(boost::bind(&simple_wallet::check_inactivity, this)); #endif - m_refresh_checker.do_call(boost::bind(&simple_wallet::check_refresh, this)); - m_mms_checker.do_call(boost::bind(&simple_wallet::check_mms, this)); - m_rpc_payment_checker.do_call(boost::bind(&simple_wallet::check_rpc_payment, this)); + m_refresh_checker.do_call(boost::bind(&simple_wallet::check_refresh, this)); + m_mms_checker.do_call(boost::bind(&simple_wallet::check_mms, this)); + m_rpc_payment_checker.do_call(boost::bind(&simple_wallet::check_rpc_payment, this)); - if (!m_idle_run.load(std::memory_order_relaxed)) - break; - m_idle_cond.wait_for(lock, boost::chrono::seconds(1)); + if (!m_idle_run.load(std::memory_order_relaxed)) + break; + } + + // aim for the next multiple of 1 second + const boost::posix_time::ptime now = boost::posix_time::microsec_clock::universal_time(); + const auto dt = (now - start_time).total_microseconds(); + const auto wait = 1000000 - dt % 1000000; + m_idle_cond.wait_for(lock, boost::chrono::microseconds(wait)); } } //---------------------------------------------------------------------------------------------------- diff --git a/src/simplewallet/simplewallet.h b/src/simplewallet/simplewallet.h index e8f96ad54..75bd893d5 100644 --- a/src/simplewallet/simplewallet.h +++ b/src/simplewallet/simplewallet.h @@ -448,10 +448,12 @@ namespace cryptonote std::atomic m_locked; std::atomic m_in_command; + template struct get_random_interval { public: uint64_t operator()() const { return crypto::rand_range(mini, maxi); } }; + epee::math_helper::once_a_time_seconds<1> m_inactivity_checker; - epee::math_helper::once_a_time_seconds<90> m_refresh_checker; - epee::math_helper::once_a_time_seconds<90> m_mms_checker; - epee::math_helper::once_a_time_seconds<90> m_rpc_payment_checker; + epee::math_helper::once_a_time_seconds_range> m_refresh_checker; + epee::math_helper::once_a_time_seconds_range> m_mms_checker; + epee::math_helper::once_a_time_seconds_range> m_rpc_payment_checker; std::atomic m_need_payment; boost::posix_time::ptime m_last_rpc_payment_mining_time; -- cgit v1.2.3