From cd0b5a1a4c47a1cd02000ffa1d4980821b3d536f Mon Sep 17 00:00:00 2001 From: selsta Date: Wed, 9 Jan 2019 09:20:53 +0100 Subject: [PATCH] device: proper handling of user input (1) If the user denies something on the Ledger, a proper error message is now shown. (2) Ledger doesn't time out anymore while waiting on user input. (3) Lower the timeout to 2 seconds, this is enough for normal Ledger <-> System communication. --- src/device/device_io.hpp | 2 +- src/device/device_io_hid.cpp | 8 ++++++-- src/device/device_io_hid.hpp | 2 +- src/device/device_ledger.cpp | 39 +++++++++++++++++++++++++++++++----- src/device/device_ledger.hpp | 1 + 5 files changed, 43 insertions(+), 9 deletions(-) diff --git a/src/device/device_io.hpp b/src/device/device_io.hpp index 96163a211..1d5e3564c 100644 --- a/src/device/device_io.hpp +++ b/src/device/device_io.hpp @@ -50,7 +50,7 @@ namespace hw { virtual void disconnect() = 0; virtual bool connected() const = 0; - virtual int exchange(unsigned char *command, unsigned int cmd_len, unsigned char *response, unsigned int max_resp_len) = 0; + virtual int exchange(unsigned char *command, unsigned int cmd_len, unsigned char *response, unsigned int max_resp_len, bool user_input) = 0; }; }; }; diff --git a/src/device/device_io_hid.cpp b/src/device/device_io_hid.cpp index 1aadfb9ea..36c7a241b 100644 --- a/src/device/device_io_hid.cpp +++ b/src/device/device_io_hid.cpp @@ -148,7 +148,7 @@ namespace hw { return this->usb_device != NULL; } - int device_io_hid::exchange(unsigned char *command, unsigned int cmd_len, unsigned char *response, unsigned int max_resp_len) { + int device_io_hid::exchange(unsigned char *command, unsigned int cmd_len, unsigned char *response, unsigned int max_resp_len, bool user_input) { unsigned char buffer[400]; unsigned char padding_buffer[MAX_BLOCK+1]; unsigned int result; @@ -177,7 +177,11 @@ namespace hw { //get first response memset(buffer, 0, sizeof(buffer)); - hid_ret = hid_read_timeout(this->usb_device, buffer, MAX_BLOCK, this->timeout); + if (!user_input) { + hid_ret = hid_read_timeout(this->usb_device, buffer, MAX_BLOCK, this->timeout); + } else { + hid_ret = hid_read(this->usb_device, buffer, MAX_BLOCK); + } ASSERT_X(hid_ret>=0, "Unable to read hidapi response. Error "+std::to_string(result)+": "+ safe_hid_error(this->usb_device)); result = (unsigned int)hid_ret; io_hid_log(1, buffer, result); diff --git a/src/device/device_io_hid.hpp b/src/device/device_io_hid.hpp index bb0f0a814..c47eefad2 100644 --- a/src/device/device_io_hid.hpp +++ b/src/device/device_io_hid.hpp @@ -100,7 +100,7 @@ namespace hw { void connect(void *params); void connect(unsigned int vid, unsigned int pid, boost::optional interface_number, boost::optional usage_page); bool connected() const; - int exchange(unsigned char *command, unsigned int cmd_len, unsigned char *response, unsigned int max_resp_len); + int exchange(unsigned char *command, unsigned int cmd_len, unsigned char *response, unsigned int max_resp_len, bool user_input); void disconnect(); void release(); }; diff --git a/src/device/device_ledger.cpp b/src/device/device_ledger.cpp index 20dc79929..bfbcf691d 100644 --- a/src/device/device_ledger.cpp +++ b/src/device/device_ledger.cpp @@ -176,7 +176,7 @@ namespace hw { #define INS_GET_RESPONSE 0xc0 - device_ledger::device_ledger(): hw_device(0x0101, 0x05, 64, 120000) { + device_ledger::device_ledger(): hw_device(0x0101, 0x05, 64, 2000) { this->id = device_id++; this->reset_buffer(); this->mode = NONE; @@ -235,6 +235,9 @@ namespace hw { /* IO */ /* ======================================================================= */ + #define IO_SW_DENY 0x6982 + #define IO_SECRET_KEY 0x02 + void device_ledger::logCMD() { if (apdu_verbose) { char strbuffer[1024]; @@ -283,7 +286,12 @@ namespace hw { void device_ledger::send_simple(unsigned char ins, unsigned char p1) { this->length_send = set_command_header_noopt(ins, p1); - this->exchange(); + if (ins == INS_GET_KEY && p1 == IO_SECRET_KEY) { + // export view key user input + this->exchange_wait_on_input(); + } else { + this->exchange(); + } } bool device_ledger::reset() { @@ -294,7 +302,7 @@ namespace hw { unsigned int device_ledger::exchange(unsigned int ok, unsigned int mask) { logCMD(); - this->length_recv = hw_device.exchange(this->buffer_send, this->length_send, this->buffer_recv, BUFFER_SEND_SIZE); + this->length_recv = hw_device.exchange(this->buffer_send, this->length_send, this->buffer_recv, BUFFER_SEND_SIZE, false); ASSERT_X(this->length_recv>=2, "Communication error, less than tow bytes received"); this->length_recv -= 2; @@ -305,6 +313,25 @@ namespace hw { return this->sw; } + unsigned int device_ledger::exchange_wait_on_input(unsigned int ok, unsigned int mask) { + logCMD(); + unsigned int deny = 0; + this->length_recv = hw_device.exchange(this->buffer_send, this->length_send, this->buffer_recv, BUFFER_SEND_SIZE, true); + ASSERT_X(this->length_recv>=2, "Communication error, less than two bytes received"); + + this->length_recv -= 2; + this->sw = (this->buffer_recv[length_recv]<<8) | this->buffer_recv[length_recv+1]; + if (this->sw == IO_SW_DENY) { + // cancel on device + deny = 1; + } else { + ASSERT_SW(this->sw,ok,msk); + } + + logRESP(); + return deny; + } + void device_ledger::reset_buffer() { this->length_send = 0; memset(this->buffer_send, 0, BUFFER_SEND_SIZE); @@ -1260,7 +1287,8 @@ namespace hw { this->buffer_send[4] = offset-5; this->length_send = offset; - this->exchange(); + // check fee user input + CHECK_AND_ASSERT_THROW_MES(this->exchange_wait_on_input() == 0, "Fee denied on device."); //pseudoOuts if (type == rct::RCTTypeSimple) { @@ -1328,7 +1356,8 @@ namespace hw { this->buffer_send[4] = offset-5; this->length_send = offset; - this->exchange(); + // check transaction user input + CHECK_AND_ASSERT_THROW_MES(this->exchange_wait_on_input() == 0, "Transaction denied on device."); #ifdef DEBUG_HWDEVICE hw::ledger::log_hexbuffer("Prehash AKV input", (char*)&this->buffer_recv[64], 3*32); #endif diff --git a/src/device/device_ledger.hpp b/src/device/device_ledger.hpp index 9f731cde9..de38d5c27 100644 --- a/src/device/device_ledger.hpp +++ b/src/device/device_ledger.hpp @@ -94,6 +94,7 @@ namespace hw { void logCMD(void); void logRESP(void); unsigned int exchange(unsigned int ok=0x9000, unsigned int mask=0xFFFF); + unsigned int exchange_wait_on_input(unsigned int ok=0x9000, unsigned int mask=0xFFFF); void reset_buffer(void); int set_command_header(unsigned char ins, unsigned char p1 = 0x00, unsigned char p2 = 0x00); int set_command_header_noopt(unsigned char ins, unsigned char p1 = 0x00, unsigned char p2 = 0x00);