wallet: fix output collision detection for view wallets

View wallets do not have the spend secret key, and are thus
unable to derive key images for incoming outputs. Moreover,
a previous patch set key images to zero as a means to mark
an output as having an unknown key image, so they could be
filled in when importing key images at a later time. That
later patch caused spurious collisions. We now use public
keys to detect duplicate outputs. Public keys obtained from
the blockchain are checked to be identical to the ones
derived locally, so can't be spoofed.
This commit is contained in:
moneromooo-monero 2016-11-07 18:50:05 +00:00
parent 1372f255af
commit c80f4d416d
No known key found for this signature in database
GPG key ID: 686F07454D6CEFC3
3 changed files with 47 additions and 21 deletions

View file

@ -236,6 +236,6 @@ namespace crypto {
} }
} }
CRYPTO_MAKE_COMPARABLE(public_key) CRYPTO_MAKE_HASHABLE(public_key)
CRYPTO_MAKE_HASHABLE(key_image) CRYPTO_MAKE_HASHABLE(key_image)
CRYPTO_MAKE_COMPARABLE(signature) CRYPTO_MAKE_COMPARABLE(signature)

View file

@ -250,8 +250,6 @@ bool wallet2::wallet_generate_key_image_helper(const cryptonote::account_keys& a
{ {
if (!cryptonote::generate_key_image_helper(ack, tx_public_key, real_output_index, in_ephemeral, ki)) if (!cryptonote::generate_key_image_helper(ack, tx_public_key, real_output_index, in_ephemeral, ki))
return false; return false;
if (m_watch_only)
memset(&ki, 0, 32);
return true; return true;
} }
//---------------------------------------------------------------------------------------------------- //----------------------------------------------------------------------------------------------------
@ -484,12 +482,12 @@ void wallet2::process_new_transaction(const cryptonote::transaction& tx, const s
THROW_WALLET_EXCEPTION_IF(tx.vout.size() <= o, error::wallet_internal_error, "wrong out in transaction: internal index=" + THROW_WALLET_EXCEPTION_IF(tx.vout.size() <= o, error::wallet_internal_error, "wrong out in transaction: internal index=" +
std::to_string(o) + ", total_outs=" + std::to_string(tx.vout.size())); std::to_string(o) + ", total_outs=" + std::to_string(tx.vout.size()));
auto kit = m_key_images.find(ki[o]); auto kit = m_pub_keys.find(in_ephemeral[o].pub);
THROW_WALLET_EXCEPTION_IF(kit != m_key_images.end() && kit->second >= m_transfers.size(), THROW_WALLET_EXCEPTION_IF(kit != m_pub_keys.end() && kit->second >= m_transfers.size(),
error::wallet_internal_error, std::string("Unexpected transfer index from key image: ") error::wallet_internal_error, std::string("Unexpected transfer index from public key: ")
+ "got " + (kit == m_key_images.end() ? "<none>" : boost::lexical_cast<std::string>(kit->second)) + "got " + (kit == m_pub_keys.end() ? "<none>" : boost::lexical_cast<std::string>(kit->second))
+ ", m_transfers.size() is " + boost::lexical_cast<std::string>(m_transfers.size())); + ", m_transfers.size() is " + boost::lexical_cast<std::string>(m_transfers.size()));
if (kit == m_key_images.end()) if (kit == m_pub_keys.end())
{ {
if (!pool) if (!pool)
{ {
@ -501,6 +499,7 @@ void wallet2::process_new_transaction(const cryptonote::transaction& tx, const s
td.m_tx = (const cryptonote::transaction_prefix&)tx; td.m_tx = (const cryptonote::transaction_prefix&)tx;
td.m_txid = txid(); td.m_txid = txid();
td.m_key_image = ki[o]; td.m_key_image = ki[o];
td.m_key_image_known = !m_watch_only;
td.m_amount = tx.vout[o].amount; td.m_amount = tx.vout[o].amount;
if (td.m_amount == 0) if (td.m_amount == 0)
{ {
@ -520,6 +519,7 @@ void wallet2::process_new_transaction(const cryptonote::transaction& tx, const s
} }
set_unspent(m_transfers.size()-1); set_unspent(m_transfers.size()-1);
m_key_images[td.m_key_image] = m_transfers.size()-1; m_key_images[td.m_key_image] = m_transfers.size()-1;
m_pub_keys[in_ephemeral[o].pub] = m_transfers.size()-1;
LOG_PRINT_L0("Received money: " << print_money(td.amount()) << ", with tx: " << txid()); LOG_PRINT_L0("Received money: " << print_money(td.amount()) << ", with tx: " << txid());
if (0 != m_callback) if (0 != m_callback)
m_callback->on_money_received(height, tx, td.m_amount); m_callback->on_money_received(height, tx, td.m_amount);
@ -527,14 +527,14 @@ void wallet2::process_new_transaction(const cryptonote::transaction& tx, const s
} }
else if (m_transfers[kit->second].m_spent || m_transfers[kit->second].amount() >= tx.vout[o].amount) else if (m_transfers[kit->second].m_spent || m_transfers[kit->second].amount() >= tx.vout[o].amount)
{ {
LOG_ERROR("key image " << epee::string_tools::pod_to_hex(kit->first) LOG_ERROR("Public key " << epee::string_tools::pod_to_hex(kit->first)
<< " from received " << print_money(tx.vout[o].amount) << " output already exists with " << " from received " << print_money(tx.vout[o].amount) << " output already exists with "
<< (m_transfers[kit->second].m_spent ? "spent" : "unspent") << " " << (m_transfers[kit->second].m_spent ? "spent" : "unspent") << " "
<< print_money(m_transfers[kit->second].amount()) << ", received output ignored"); << print_money(m_transfers[kit->second].amount()) << ", received output ignored");
} }
else else
{ {
LOG_ERROR("key image " << epee::string_tools::pod_to_hex(kit->first) LOG_ERROR("Public key " << epee::string_tools::pod_to_hex(kit->first)
<< " from received " << print_money(tx.vout[o].amount) << " output already exists with " << " from received " << print_money(tx.vout[o].amount) << " output already exists with "
<< print_money(m_transfers[kit->second].amount()) << ", replacing with new output"); << print_money(m_transfers[kit->second].amount()) << ", replacing with new output");
// The new larger output replaced a previous smaller one // The new larger output replaced a previous smaller one
@ -565,7 +565,7 @@ void wallet2::process_new_transaction(const cryptonote::transaction& tx, const s
td.m_mask = rct::identity(); td.m_mask = rct::identity();
td.m_rct = false; td.m_rct = false;
} }
THROW_WALLET_EXCEPTION_IF(td.m_key_image != ki[o], error::wallet_internal_error, "Inconsistent key images"); THROW_WALLET_EXCEPTION_IF(td.get_public_key() != in_ephemeral[o].pub, error::wallet_internal_error, "Inconsistent public keys");
THROW_WALLET_EXCEPTION_IF(td.m_spent, error::wallet_internal_error, "Inconsistent spent status"); THROW_WALLET_EXCEPTION_IF(td.m_spent, error::wallet_internal_error, "Inconsistent spent status");
LOG_PRINT_L0("Received money: " << print_money(td.amount()) << ", with tx: " << txid()); LOG_PRINT_L0("Received money: " << print_money(td.amount()) << ", with tx: " << txid());
@ -1345,7 +1345,13 @@ void wallet2::detach_blockchain(uint64_t height)
auto it_ki = m_key_images.find(m_transfers[i].m_key_image); auto it_ki = m_key_images.find(m_transfers[i].m_key_image);
THROW_WALLET_EXCEPTION_IF(it_ki == m_key_images.end(), error::wallet_internal_error, "key image not found"); THROW_WALLET_EXCEPTION_IF(it_ki == m_key_images.end(), error::wallet_internal_error, "key image not found");
m_key_images.erase(it_ki); m_key_images.erase(it_ki);
++transfers_detached; }
for(size_t i = i_start; i!= m_transfers.size();i++)
{
auto it_pk = m_pub_keys.find(m_transfers[i].get_public_key());
THROW_WALLET_EXCEPTION_IF(it_pk == m_pub_keys.end(), error::wallet_internal_error, "public key not found");
m_pub_keys.erase(it_pk);
} }
m_transfers.erase(it, m_transfers.end()); m_transfers.erase(it, m_transfers.end());
@ -1382,6 +1388,7 @@ bool wallet2::clear()
m_blockchain.clear(); m_blockchain.clear();
m_transfers.clear(); m_transfers.clear();
m_key_images.clear(); m_key_images.clear();
m_pub_keys.clear();
m_unconfirmed_txs.clear(); m_unconfirmed_txs.clear();
m_payments.clear(); m_payments.clear();
m_tx_keys.clear(); m_tx_keys.clear();
@ -2158,13 +2165,11 @@ void wallet2::rescan_spent()
std::to_string(daemon_resp.spent_status.size()) + ", expected " + std::to_string(key_images.size())); std::to_string(daemon_resp.spent_status.size()) + ", expected " + std::to_string(key_images.size()));
// update spent status // update spent status
key_image zero_ki;
memset(&zero_ki, 0, 32);
for (size_t i = 0; i < m_transfers.size(); ++i) for (size_t i = 0; i < m_transfers.size(); ++i)
{ {
transfer_details& td = m_transfers[i]; transfer_details& td = m_transfers[i];
// a view wallet may not know about key images // a view wallet may not know about key images
if (td.m_key_image == zero_ki) if (!td.m_key_image_known)
continue; continue;
if (td.m_spent != (daemon_resp.spent_status[i] != COMMAND_RPC_IS_KEY_IMAGE_SPENT::UNSPENT)) if (td.m_spent != (daemon_resp.spent_status[i] != COMMAND_RPC_IS_KEY_IMAGE_SPENT::UNSPENT))
{ {
@ -4276,10 +4281,7 @@ std::vector<std::pair<crypto::key_image, crypto::signature>> wallet2::export_key
cryptonote::keypair in_ephemeral; cryptonote::keypair in_ephemeral;
cryptonote::generate_key_image_helper(m_account.get_keys(), tx_pub_key, td.m_internal_output_index, in_ephemeral, ki); cryptonote::generate_key_image_helper(m_account.get_keys(), tx_pub_key, td.m_internal_output_index, in_ephemeral, ki);
bool zero_key_image = true; THROW_WALLET_EXCEPTION_IF(td.m_key_image_known && ki != td.m_key_image,
for (size_t i = 0; i < sizeof(td.m_key_image); ++i)
zero_key_image &= (td.m_key_image.data[i] == 0);
THROW_WALLET_EXCEPTION_IF(!zero_key_image && ki != td.m_key_image,
error::wallet_internal_error, "key_image generated not matched with cached key image"); error::wallet_internal_error, "key_image generated not matched with cached key image");
THROW_WALLET_EXCEPTION_IF(in_ephemeral.pub != pkey, THROW_WALLET_EXCEPTION_IF(in_ephemeral.pub != pkey,
error::wallet_internal_error, "key_image generated ephemeral public key not matched with output_key"); error::wallet_internal_error, "key_image generated ephemeral public key not matched with output_key");
@ -4335,7 +4337,10 @@ uint64_t wallet2::import_key_images(const std::vector<std::pair<crypto::key_imag
} }
for (size_t n = 0; n < signed_key_images.size(); ++n) for (size_t n = 0; n < signed_key_images.size(); ++n)
{
m_transfers[n].m_key_image = signed_key_images[n].first; m_transfers[n].m_key_image = signed_key_images[n].first;
m_transfers[n].m_key_image_known = true;
}
m_daemon_rpc_mutex.lock(); m_daemon_rpc_mutex.lock();
bool r = epee::net_utils::invoke_http_json_remote_command2(m_daemon_address + "/is_key_image_spent", req, daemon_resp, m_http_client, 200000); bool r = epee::net_utils::invoke_http_json_remote_command2(m_daemon_address + "/is_key_image_spent", req, daemon_resp, m_http_client, 200000);
@ -4401,6 +4406,7 @@ size_t wallet2::import_outputs(const std::vector<tools::wallet2::transfer_detail
"Public key wasn't found in the transaction extra at index " + i); "Public key wasn't found in the transaction extra at index " + i);
cryptonote::generate_key_image_helper(m_account.get_keys(), pub_key_field.pub_key, td.m_internal_output_index, in_ephemeral, td.m_key_image); cryptonote::generate_key_image_helper(m_account.get_keys(), pub_key_field.pub_key, td.m_internal_output_index, in_ephemeral, td.m_key_image);
td.m_key_image_known = true;
THROW_WALLET_EXCEPTION_IF(in_ephemeral.pub != boost::get<cryptonote::txout_to_key>(td.m_tx.vout[td.m_internal_output_index].target).key, THROW_WALLET_EXCEPTION_IF(in_ephemeral.pub != boost::get<cryptonote::txout_to_key>(td.m_tx.vout[td.m_internal_output_index].target).key,
error::wallet_internal_error, "key_image generated ephemeral public key not matched with output_key at index " + i); error::wallet_internal_error, "key_image generated ephemeral public key not matched with output_key at index " + i);

View file

@ -109,9 +109,11 @@ namespace tools
rct::key m_mask; rct::key m_mask;
uint64_t m_amount; uint64_t m_amount;
bool m_rct; bool m_rct;
bool m_key_image_known;
bool is_rct() const { return m_rct; } bool is_rct() const { return m_rct; }
uint64_t amount() const { return m_amount; } uint64_t amount() const { return m_amount; }
const crypto::public_key &get_public_key() const { return boost::get<const cryptonote::txout_to_key>(m_tx.vout[m_internal_output_index].target).key; }
}; };
struct payment_details struct payment_details
@ -409,6 +411,19 @@ namespace tools
a & m_unconfirmed_payments; a & m_unconfirmed_payments;
if(ver < 14) if(ver < 14)
return; return;
if(ver < 15)
{
// we're loading an older wallet without a pubkey map, rebuild it
for (size_t i = 0; i < m_transfers.size(); ++i)
{
const transfer_details &td = m_transfers[i];
const cryptonote::tx_out &out = td.m_tx.vout[td.m_internal_output_index];
const cryptonote::txout_to_key &o = boost::get<const cryptonote::txout_to_key>(out.target);
m_pub_keys.emplace(o.key, i);
}
return;
}
a & m_pub_keys;
} }
/*! /*!
@ -543,6 +558,7 @@ namespace tools
transfer_container m_transfers; transfer_container m_transfers;
payment_container m_payments; payment_container m_payments;
std::unordered_map<crypto::key_image, size_t> m_key_images; std::unordered_map<crypto::key_image, size_t> m_key_images;
std::unordered_map<crypto::public_key, size_t> m_pub_keys;
cryptonote::account_public_address m_account_public_address; cryptonote::account_public_address m_account_public_address;
std::unordered_map<crypto::hash, std::string> m_tx_notes; std::unordered_map<crypto::hash, std::string> m_tx_notes;
uint64_t m_upper_transaction_size_limit; //TODO: auto-calc this value or request from daemon, now use some fixed value uint64_t m_upper_transaction_size_limit; //TODO: auto-calc this value or request from daemon, now use some fixed value
@ -567,8 +583,8 @@ namespace tools
bool m_confirm_missing_payment_id; bool m_confirm_missing_payment_id;
}; };
} }
BOOST_CLASS_VERSION(tools::wallet2, 14) BOOST_CLASS_VERSION(tools::wallet2, 15)
BOOST_CLASS_VERSION(tools::wallet2::transfer_details, 4) BOOST_CLASS_VERSION(tools::wallet2::transfer_details, 5)
BOOST_CLASS_VERSION(tools::wallet2::payment_details, 1) BOOST_CLASS_VERSION(tools::wallet2::payment_details, 1)
BOOST_CLASS_VERSION(tools::wallet2::unconfirmed_transfer_details, 6) BOOST_CLASS_VERSION(tools::wallet2::unconfirmed_transfer_details, 6)
BOOST_CLASS_VERSION(tools::wallet2::confirmed_transfer_details, 3) BOOST_CLASS_VERSION(tools::wallet2::confirmed_transfer_details, 3)
@ -597,6 +613,7 @@ namespace boost
{ {
x.m_rct = x.m_tx.vout[x.m_internal_output_index].amount == 0; x.m_rct = x.m_tx.vout[x.m_internal_output_index].amount == 0;
} }
x.m_key_image_known = true;
} }
template <class Archive> template <class Archive>
@ -644,6 +661,9 @@ namespace boost
return; return;
} }
a & x.m_rct; a & x.m_rct;
if (ver < 5)
return;
a & x.m_key_image_known;
} }
template <class Archive> template <class Archive>