From d71f89e2a26720021f7509f8c1eee87645f48529 Mon Sep 17 00:00:00 2001 From: Dusan Klinec Date: Wed, 28 Nov 2018 22:22:11 +0100 Subject: [PATCH] device/trezor: device/trezor: correct device initialization, status check - checks if the device is in the correct usable state - implements check for the v2.0.9 firmware which does not support payment IDs - simple transacttion check, payment id fmt consistency - minor fixes, refactoring, webusb session counting fix --- cmake/CheckTrezor.cmake | 2 +- src/device_trezor/device_trezor.cpp | 77 +++++++++++++++++++++--- src/device_trezor/device_trezor.hpp | 5 +- src/device_trezor/device_trezor_base.cpp | 58 ++++++++++++++++-- src/device_trezor/device_trezor_base.hpp | 19 +++++- src/device_trezor/trezor/transport.cpp | 2 +- 6 files changed, 140 insertions(+), 23 deletions(-) diff --git a/cmake/CheckTrezor.cmake b/cmake/CheckTrezor.cmake index ea21237fd..bcd3cfc2c 100644 --- a/cmake/CheckTrezor.cmake +++ b/cmake/CheckTrezor.cmake @@ -56,7 +56,7 @@ if(Protobuf_FOUND AND USE_DEVICE_TREZOR AND TREZOR_PYTHON) endif() if(USE_DEVICE_TREZOR_UDP_RELEASE) - add_definitions(-DWITH_DEVICE_TREZOR_UDP_RELEASE=1) + add_definitions(-DUSE_DEVICE_TREZOR_UDP_RELEASE=1) endif() if (Protobuf_INCLUDE_DIR) diff --git a/src/device_trezor/device_trezor.cpp b/src/device_trezor/device_trezor.cpp index 5096fcea8..8868fb995 100644 --- a/src/device_trezor/device_trezor.cpp +++ b/src/device_trezor/device_trezor.cpp @@ -121,7 +121,8 @@ namespace trezor { const boost::optional & network_type){ AUTO_LOCK_CMD(); require_connected(); - test_ping(); + device_state_reset_unsafe(); + require_initialized(); auto req = std::make_shared(); this->set_msg_addr(req.get(), path, network_type); @@ -136,7 +137,8 @@ namespace trezor { const boost::optional & network_type){ AUTO_LOCK_CMD(); require_connected(); - test_ping(); + device_state_reset_unsafe(); + require_initialized(); auto req = std::make_shared(); this->set_msg_addr(req.get(), path, network_type); @@ -152,7 +154,8 @@ namespace trezor { { AUTO_LOCK_CMD(); require_connected(); - test_ping(); + device_state_reset_unsafe(); + require_initialized(); std::shared_ptr req; @@ -238,12 +241,11 @@ namespace trezor { cpend.construction_data = cdata.tx_data; // Transaction check - cryptonote::blobdata tx_blob; - cryptonote::transaction tx_deserialized; - bool r = cryptonote::t_serializable_object_to_blob(cpend.tx, tx_blob); - CHECK_AND_ASSERT_THROW_MES(r, "Transaction serialization failed"); - r = cryptonote::parse_and_validate_tx_from_blob(tx_blob, tx_deserialized); - CHECK_AND_ASSERT_THROW_MES(r, "Transaction deserialization failed"); + try { + transaction_check(cdata, aux_data); + } catch(const std::exception &e){ + throw exc::ProtocolException(std::string("Transaction verification failed: ") + e.what()); + } std::string key_images; bool all_are_txin_to_key = std::all_of(cdata.tx.vin.begin(), cdata.tx.vin.end(), [&](const cryptonote::txin_v& s_e) -> bool @@ -283,7 +285,8 @@ namespace trezor { { AUTO_LOCK_CMD(); require_connected(); - test_ping(); + device_state_reset_unsafe(); + require_initialized(); CHECK_AND_ASSERT_THROW_MES(idx < unsigned_tx.txes.size(), "Invalid transaction index"); signer = std::make_shared(wallet, &unsigned_tx, idx, &aux_data); @@ -294,6 +297,7 @@ namespace trezor { // Step: Init auto init_msg = signer->step_init(); this->set_msg_addr(init_msg.get()); + transaction_pre_check(init_msg); auto response = this->client_exchange(init_msg); signer->step_init_ack(response); @@ -351,6 +355,59 @@ namespace trezor { signer->step_final_ack(ack_final); } + void device_trezor::transaction_pre_check(std::shared_ptr init_msg) + { + CHECK_AND_ASSERT_THROW_MES(init_msg, "TransactionInitRequest is empty"); + CHECK_AND_ASSERT_THROW_MES(init_msg->has_tsx_data(), "TransactionInitRequest has no transaction data"); + CHECK_AND_ASSERT_THROW_MES(m_features, "Device state not initialized"); // make sure the caller did not reset features + const bool nonce_required = init_msg->tsx_data().has_payment_id() && init_msg->tsx_data().payment_id().size() > 0; + + if (nonce_required){ + // Versions 2.0.9 and lower do not support payment ID + CHECK_AND_ASSERT_THROW_MES(m_features->has_major_version() && m_features->has_minor_version() && m_features->has_patch_version(), "Invalid Trezor firmware version information"); + const uint32_t vma = m_features->major_version(); + const uint32_t vmi = m_features->minor_version(); + const uint32_t vpa = m_features->patch_version(); + if (vma < 2 || (vma == 2 && vmi == 0 && vpa <= 9)) { + throw exc::TrezorException("Trezor firmware 2.0.9 and lower does not support transactions with short payment IDs or integrated addresses. Please update."); + } + } + } + + void device_trezor::transaction_check(const protocol::tx::TData & tdata, const hw::tx_aux_data & aux_data) + { + // Simple serialization check + cryptonote::blobdata tx_blob; + cryptonote::transaction tx_deserialized; + bool r = cryptonote::t_serializable_object_to_blob(tdata.tx, tx_blob); + CHECK_AND_ASSERT_THROW_MES(r, "Transaction serialization failed"); + r = cryptonote::parse_and_validate_tx_from_blob(tx_blob, tx_deserialized); + CHECK_AND_ASSERT_THROW_MES(r, "Transaction deserialization failed"); + + // Extras check + std::vector tx_extra_fields; + cryptonote::tx_extra_nonce nonce; + + r = cryptonote::parse_tx_extra(tdata.tx.extra, tx_extra_fields); + CHECK_AND_ASSERT_THROW_MES(r, "tx.extra parsing failed"); + + const bool nonce_required = tdata.tsx_data.has_payment_id() && tdata.tsx_data.payment_id().size() > 0; + const bool has_nonce = cryptonote::find_tx_extra_field_by_type(tx_extra_fields, nonce); + CHECK_AND_ASSERT_THROW_MES(has_nonce == nonce_required, "Transaction nonce presence inconsistent"); + + if (nonce_required){ + const std::string & payment_id = tdata.tsx_data.payment_id(); + if (payment_id.size() == 32){ + crypto::hash payment_id_long{}; + CHECK_AND_ASSERT_THROW_MES(cryptonote::get_payment_id_from_tx_extra_nonce(nonce.nonce, payment_id_long), "Long payment ID not present"); + + } else if (payment_id.size() == 8){ + crypto::hash8 payment_id_short{}; + CHECK_AND_ASSERT_THROW_MES(cryptonote::get_encrypted_payment_id_from_tx_extra_nonce(nonce.nonce, payment_id_short), "Short payment ID not present"); + } + } + } + #else //WITH_DEVICE_TREZOR void register_all(std::map> ®istry) { diff --git a/src/device_trezor/device_trezor.hpp b/src/device_trezor/device_trezor.hpp index a2134574c..1f08be887 100644 --- a/src/device_trezor/device_trezor.hpp +++ b/src/device_trezor/device_trezor.hpp @@ -57,9 +57,8 @@ namespace trezor { */ class device_trezor : public hw::trezor::device_trezor_base, public hw::device_cold { protected: - // To speed up blockchain parsing the view key maybe handle here. - crypto::secret_key viewkey; - bool has_view_key; + void transaction_pre_check(std::shared_ptr init_msg); + void transaction_check(const protocol::tx::TData & tdata, const hw::tx_aux_data & aux_data); public: device_trezor(); diff --git a/src/device_trezor/device_trezor_base.cpp b/src/device_trezor/device_trezor_base.cpp index fddc6082c..5071932ee 100644 --- a/src/device_trezor/device_trezor_base.cpp +++ b/src/device_trezor/device_trezor_base.cpp @@ -65,7 +65,7 @@ namespace trezor { } bool device_trezor_base::set_name(const std::string & name) { - this->full_name = name; + this->m_full_name = name; this->name = ""; auto delim = name.find(':'); @@ -77,10 +77,10 @@ namespace trezor { } const std::string device_trezor_base::get_name() const { - if (this->full_name.empty()) { + if (this->m_full_name.empty()) { return std::string("name).append(">"); } - return this->full_name; + return this->m_full_name; } bool device_trezor_base::init() { @@ -139,6 +139,9 @@ namespace trezor { } bool device_trezor_base::disconnect() { + m_device_state.clear(); + m_features.reset(); + if (m_transport){ try { m_transport->close(); @@ -193,6 +196,25 @@ namespace trezor { } } + void device_trezor_base::require_initialized(){ + if (!m_features){ + throw exc::TrezorException("Device state not initialized"); + } + + if (m_features->has_bootloader_mode() && m_features->bootloader_mode()){ + throw exc::TrezorException("Device is in the bootloader mode"); + } + + if (m_features->has_firmware_present() && !m_features->firmware_present()){ + throw exc::TrezorException("Device has no firmware loaded"); + } + + // Hard requirement on initialized field, has to be there. + if (!m_features->has_initialized() || !m_features->initialized()){ + throw exc::TrezorException("Device is not initialized"); + } + } + void device_trezor_base::call_ping_unsafe(){ auto pingMsg = std::make_shared(); pingMsg->set_message("PING"); @@ -217,7 +239,7 @@ namespace trezor { void device_trezor_base::write_raw(const google::protobuf::Message * msg){ require_connected(); CHECK_AND_ASSERT_THROW_MES(msg, "Empty message"); - this->getTransport()->write(*msg); + this->get_transport()->write(*msg); } GenericMessage device_trezor_base::read_raw(){ @@ -225,7 +247,7 @@ namespace trezor { std::shared_ptr msg_resp; hw::trezor::messages::MessageType msg_resp_type; - this->getTransport()->read(msg_resp, &msg_resp_type); + this->get_transport()->read(msg_resp, &msg_resp_type); return GenericMessage(msg_resp_type, msg_resp); } @@ -314,6 +336,25 @@ namespace trezor { return false; } + void device_trezor_base::device_state_reset_unsafe() + { + require_connected(); + auto initMsg = std::make_shared(); + + if(!m_device_state.empty()) { + initMsg->set_allocated_state(&m_device_state); + } + + m_features = this->client_exchange(initMsg); + initMsg->release_state(); + } + + void device_trezor_base::device_state_reset() + { + AUTO_LOCK_CMD(); + device_state_reset_unsafe(); + } + void device_trezor_base::on_button_request(GenericMessage & resp, const messages::common::ButtonRequest * msg) { CHECK_AND_ASSERT_THROW_MES(msg, "Empty message"); @@ -361,7 +402,13 @@ namespace trezor { // TODO: remove passphrase from memory m.set_passphrase(passphrase.data(), passphrase.size()); } + + if (!m_device_state.empty()){ + m.set_allocated_state(&m_device_state); + } + resp = call_raw(&m); + m.release_state(); } void device_trezor_base::on_passphrase_state_request(GenericMessage & resp, const messages::common::PassphraseStateRequest * msg) @@ -369,6 +416,7 @@ namespace trezor { MDEBUG("on_passhprase_state_request"); CHECK_AND_ASSERT_THROW_MES(msg, "Empty message"); + m_device_state = msg->state(); messages::common::PassphraseStateAck m; resp = call_raw(&m); } diff --git a/src/device_trezor/device_trezor_base.hpp b/src/device_trezor/device_trezor_base.hpp index 4d205ad7a..3c35e8aca 100644 --- a/src/device_trezor/device_trezor_base.hpp +++ b/src/device_trezor/device_trezor_base.hpp @@ -70,8 +70,10 @@ namespace trezor { std::shared_ptr m_transport; i_device_callback * m_callback; - std::string full_name; + std::string m_full_name; std::vector m_wallet_deriv_path; + std::string m_device_state; // returned after passphrase entry, session + std::shared_ptr m_features; // features from the last device reset cryptonote::network_type network_type; @@ -80,8 +82,10 @@ namespace trezor { // void require_connected(); + void require_initialized(); void call_ping_unsafe(); void test_ping(); + void device_state_reset_unsafe(); void ensure_derivation_path() noexcept; // Communication methods @@ -130,7 +134,7 @@ namespace trezor { // Scoped session closer BOOST_SCOPE_EXIT_ALL(&, this) { if (open_session){ - this->getTransport()->close(); + this->get_transport()->close(); } }; @@ -209,7 +213,7 @@ namespace trezor { // Default derivation path for Monero static const uint32_t DEFAULT_BIP44_PATH[2]; - std::shared_ptr getTransport(){ + std::shared_ptr get_transport(){ return m_transport; } @@ -221,6 +225,10 @@ namespace trezor { return m_callback; } + std::shared_ptr & get_features() { + return m_features; + } + void set_derivation_path(const std::string &deriv_path) override; /* ======================================================================= */ @@ -250,6 +258,11 @@ namespace trezor { */ bool ping(); + /** + * Performs Initialize call to the Trezor, resets to known state. + */ + void device_state_reset(); + // Protocol callbacks void on_button_request(GenericMessage & resp, const messages::common::ButtonRequest * msg); void on_pin_request(GenericMessage & resp, const messages::common::PinMatrixRequest * msg); diff --git a/src/device_trezor/trezor/transport.cpp b/src/device_trezor/trezor/transport.cpp index 814537eb6..cd66e59e8 100644 --- a/src/device_trezor/trezor/transport.cpp +++ b/src/device_trezor/trezor/transport.cpp @@ -840,7 +840,7 @@ namespace trezor{ throw exc::DeviceAcquireException("Unable to claim libusb device"); } - m_conn_count += 1; + m_conn_count = 1; m_proto->session_begin(*this); #undef TREZOR_DESTROY_SESSION