Add hmac over encrypted value during transaction

src/cryptonote_core/cryptonote_tx_utils.cpp

@@ -612,7 +612,7 @@ namespace cryptonote
   {
     hw::device &hwdev = sender_account_keys.get_device();
     hwdev.open_tx(tx_key);
-
+    try {
       // figure out if we need to make additional tx pubkeys
       size_t num_stdaddresses = 0;
       size_t num_subaddresses = 0;
@@ -629,6 +629,10 @@ namespace cryptonote
       bool r = construct_tx_with_tx_key(sender_account_keys, subaddresses, sources, destinations, change_addr, extra, tx, unlock_time, tx_key, additional_tx_keys, rct, rct_config, msout);
       hwdev.close_tx();
       return r;
+    } catch(...) {
+      hwdev.close_tx();
+      throw;
+    }
   }
   //---------------------------------------------------------------
   bool construct_tx(const account_keys& sender_account_keys, std::vector<tx_source_entry>& sources, const std::vector<tx_destination_entry>& destinations, const boost::optional<cryptonote::account_public_address>& change_addr, const std::vector<uint8_t> &extra, transaction& tx, uint64_t unlock_time)

src/device/device_ledger.cpp

@@ -64,6 +64,41 @@ namespace hw {
       crypto::secret_key dbg_spendkey;
     #endif
 
+    /* ===================================================================== */
+    /* ===                        hmacmap                               ==== */
+    /* ===================================================================== */
+
+
+    SecHMAC::SecHMAC(const uint8_t s[32], const uint8_t h[32]) {
+        memcpy(this->sec, s, 32);
+        memcpy(this->hmac, h, 32);
+    }
+
+    void  HMACmap::find_mac(const uint8_t sec[32], uint8_t hmac[32]) {
+      size_t sz = hmacs.size();
+      log_hexbuffer("find_mac: lookup for ", (char*)sec,32);
+      for (size_t i=0; i<sz; i++) {
+       log_hexbuffer("find_mac:   - try ",(char*)hmacs[i].sec,32);
+        if (memcmp(sec, hmacs[i].sec, 32) == 0) {
+          memcpy(hmac, hmacs[i].hmac, 32);
+          log_hexbuffer("find_mac:   - found ",(char*)hmacs[i].hmac,32);
+          return;
+        }
+
+      }
+      throw std::runtime_error("Protocol error: try to send untrusted secret");
+    }
+
+    void  HMACmap::add_mac(const uint8_t sec[32], const uint8_t hmac[32]) {
+      log_hexbuffer("add_mac: sec  ", (char*)sec,32);
+      log_hexbuffer("add_mac: hmac ", (char*)hmac,32);
+      hmacs.push_back(SecHMAC(sec,hmac));
+    }
+
+    void HMACmap::clear() {
+      hmacs.clear();
+    }
+
     /* ===================================================================== */
     /* ===                        Keymap                                ==== */
     /* ===================================================================== */
@@ -205,6 +240,7 @@ namespace hw {
       this->reset_buffer();      
       this->mode = NONE;
       this->has_view_key = false;
+      this->tx_in_progress = false;
       MDEBUG( "Device "<<this->id <<" Created");
     }
 
@@ -317,6 +353,26 @@ namespace hw {
       }
     }
 
+    void device_ledger::send_secret(const unsigned char sec[32], int &offset) {
+      MDEBUG("send_secret: " << this->tx_in_progress);
+      memmove(this->buffer_send+offset, sec, 32);
+      offset +=32;
+      if (this->tx_in_progress) {
+        this->hmac_map.find_mac((uint8_t*)sec, this->buffer_send+offset);
+        offset += 32;
+      }
+    }
+
+    void device_ledger::receive_secret(unsigned char sec[32], int &offset) {
+      MDEBUG("receive_secret: " << this->tx_in_progress);
+      memmove(sec, this->buffer_recv+offset, 32);
+      offset += 32;
+      if (this->tx_in_progress) {
+        this->hmac_map.add_mac((uint8_t*)sec, this->buffer_recv+offset);
+        offset += 32;
+      }
+    }
+
     bool device_ledger::reset() {
       reset_buffer();
       int offset = set_command_header_noopt(INS_RESET);
@@ -573,8 +629,7 @@ namespace hw {
         memmove(this->buffer_send+offset, pub.data, 32);
         offset += 32;
         //derivation
-        memmove(this->buffer_send+offset, derivation.data, 32);
+        this->send_secret((unsigned char*)derivation.data, offset);
-        offset += 32;
         //index
         this->buffer_send[offset+0] = output_index>>24;
         this->buffer_send[offset+1] = output_index>>16;
@@ -706,8 +761,7 @@ namespace hw {
 
         int offset = set_command_header_noopt(INS_GET_SUBADDRESS_SECRET_KEY);
         //sec
-        memmove(this->buffer_send+offset, sec.data, 32);
+        this->send_secret((unsigned char*)sec.data, offset);
-        offset += 32;
         //index
         static_assert(sizeof(cryptonote::subaddress_index) == 8, "cryptonote::subaddress_index shall be 8 bytes length");
         memmove(this->buffer_send+offset, &index, sizeof(cryptonote::subaddress_index));
@@ -717,7 +771,8 @@ namespace hw {
         this->length_send = offset;
         this->exchange();
 
-        memmove(sub_sec.data,  &this->buffer_recv[0],  32);
+        offset = 0;
+        this->receive_secret((unsigned char*)sub_sec.data,  offset);
 
         #ifdef DEBUG_HWDEVICE
         crypto::secret_key            sub_sec_clear =   hw::ledger::decrypt(sub_sec);
@@ -737,8 +792,7 @@ namespace hw {
 
         offset = set_command_header_noopt(INS_VERIFY_KEY);
         //sec
-        memmove(this->buffer_send+offset, secret_key.data, 32);
+        this->send_secret((unsigned char*)secret_key.data, offset);
-        offset += 32;
         //pub
         memmove(this->buffer_send+offset, public_key.data, 32);
         offset += 32;
@@ -774,9 +828,7 @@ namespace hw {
         memmove(this->buffer_send+offset, P.bytes, 32);
         offset += 32;
         //sec
-        memmove(this->buffer_send+offset, a.bytes, 32);
+        this->send_secret(a.bytes, offset);
-        offset += 32;
-
 
         this->buffer_send[4] = offset-5;
         this->length_send = offset;
@@ -805,8 +857,7 @@ namespace hw {
 
         int offset = set_command_header_noopt(INS_SECRET_SCAL_MUL_BASE);
         //sec
-        memmove(this->buffer_send+offset, a.bytes, 32);
+        this->send_secret(a.bytes, offset);
-        offset += 32;
 
         this->buffer_send[4] = offset-5;
         this->length_send = offset;
@@ -824,6 +875,7 @@ namespace hw {
 
     bool device_ledger::sc_secret_add( crypto::secret_key &r, const crypto::secret_key &a, const crypto::secret_key &b) {
         AUTO_LOCK_CMD();
+        int offset;
 
         #ifdef DEBUG_HWDEVICE
         const crypto::secret_key a_x = hw::ledger::decrypt(a);
@@ -836,20 +888,19 @@ namespace hw {
         log_hexbuffer("sc_secret_add: [[OUT]] aG", (char*)r_x.data, 32);       
         #endif
 
-        int offset = set_command_header_noopt(INS_SECRET_KEY_ADD);
+        offset = set_command_header_noopt(INS_SECRET_KEY_ADD);
         //sec key
-        memmove(this->buffer_send+offset, a.data, 32);
+        this->send_secret((unsigned char*)a.data, offset);
-        offset += 32;
         //sec key
-        memmove(this->buffer_send+offset, b.data, 32);
+        this->send_secret((unsigned char*)b.data, offset);
-        offset += 32;
 
         this->buffer_send[4] = offset-5;
         this->length_send = offset;
         this->exchange();
 
-        //pub key
+        //sec key
-        memmove(r.data, &this->buffer_recv[0], 32);
+        offset = 0;
+        this->receive_secret((unsigned char*)r.data, offset);
 
         #ifdef DEBUG_HWDEVICE
         crypto::secret_key r_clear = hw::ledger::decrypt(r);
@@ -861,6 +912,8 @@ namespace hw {
 
     crypto::secret_key  device_ledger::generate_keys(crypto::public_key &pub, crypto::secret_key &sec, const crypto::secret_key& recovery_key, bool recover) {
         AUTO_LOCK_CMD();
+        int offset;
+
         if (recover) {
            throw std::runtime_error("device generate key does not support recover");
         }
@@ -877,9 +930,11 @@ namespace hw {
 
         send_simple(INS_GENERATE_KEYPAIR);
 
+        offset = 0;
         //pub key
         memmove(pub.data, &this->buffer_recv[0], 32);
-        memmove(sec.data, &this->buffer_recv[32], 32);
+        offset += 32;
+        this->receive_secret((unsigned char*)sec.data, offset);
 
         #ifdef DEBUG_HWDEVICE
         crypto::secret_key sec_clear = hw::ledger::decrypt(sec);
@@ -922,15 +977,16 @@ namespace hw {
         memmove(this->buffer_send+offset, pub.data, 32);
         offset += 32;
          //sec
-        memmove(this->buffer_send+offset, sec.data, 32);
+        this->send_secret((unsigned char*)sec.data, offset);
-        offset += 32;
 
         this->buffer_send[4] = offset-5;
         this->length_send = offset;
         this->exchange();
 
+        offset = 0;
         //derivattion data
-        memmove(derivation.data, &this->buffer_recv[0], 32);
+        this->receive_secret((unsigned char*)derivation.data, offset);
+
         r = true;
       }
       #ifdef DEBUG_HWDEVICE
@@ -978,9 +1034,9 @@ namespace hw {
         #endif
 
         int offset = set_command_header_noopt(INS_DERIVATION_TO_SCALAR);
-        //derivattion
+        //derivation
-        memmove(this->buffer_send+offset, derivation.data, 32);
+        this->send_secret((unsigned char*)derivation.data, offset);
-        offset += 32;
+
         //index
         this->buffer_send[offset+0] = output_index>>24;
         this->buffer_send[offset+1] = output_index>>16;
@@ -992,8 +1048,9 @@ namespace hw {
         this->length_send = offset;
         this->exchange();
 
-        //derivattion data
+        //derivation data
-        memmove(res.data, &this->buffer_recv[0], 32);
+        offset = 0;
+        this->receive_secret((unsigned char*)res.data, offset);
 
         #ifdef DEBUG_HWDEVICE
         crypto::ec_scalar res_clear  = hw::ledger::decrypt(res);
@@ -1020,8 +1077,7 @@ namespace hw {
 
         int offset = set_command_header_noopt(INS_DERIVE_SECRET_KEY);
         //derivation
-        memmove(this->buffer_send+offset, derivation.data, 32);
+        this->send_secret((unsigned char*)derivation.data, offset);
-        offset += 32;
         //index
         this->buffer_send[offset+0] = output_index>>24;
         this->buffer_send[offset+1] = output_index>>16;
@@ -1029,15 +1085,15 @@ namespace hw {
         this->buffer_send[offset+3] = output_index>>0;
         offset += 4;
         //sec
-        memmove(this->buffer_send+offset, sec.data, 32);
+        this->send_secret((unsigned char*)sec.data, offset);
-        offset += 32;
 
         this->buffer_send[4] = offset-5;
         this->length_send = offset;
         this->exchange();
 
-        //pub key
+        offset = 0;
-        memmove(derived_sec.data, &this->buffer_recv[0], 32);
+        //sec key
+        this->receive_secret((unsigned char*)derived_sec.data, offset);
 
         #ifdef DEBUG_HWDEVICE
         crypto::secret_key derived_sec_clear = hw::ledger::decrypt(derived_sec);
@@ -1064,8 +1120,7 @@ namespace hw {
 
         int offset = set_command_header_noopt(INS_DERIVE_PUBLIC_KEY);
         //derivation
-        memmove(this->buffer_send+offset, derivation.data, 32);
+        this->send_secret((unsigned char*)derivation.data, offset);
-        offset += 32;
         //index
         this->buffer_send[offset+0] = output_index>>24;
         this->buffer_send[offset+1] = output_index>>16;
@@ -1106,8 +1161,7 @@ namespace hw {
 
         int offset = set_command_header_noopt(INS_SECRET_KEY_TO_PUBLIC_KEY);
         //sec key
-        memmove(this->buffer_send+offset, sec.data, 32);
+        this->send_secret((unsigned char*)sec.data, offset);
-        offset += 32;
 
         this->buffer_send[4] = offset-5;
         this->length_send = offset;
@@ -1141,8 +1195,7 @@ namespace hw {
         memmove(this->buffer_send+offset, pub.data, 32);
         offset += 32;
         //sec
-        memmove(this->buffer_send+offset, sec.data, 32);
+        this->send_secret((unsigned char*)sec.data, offset);
-        offset += 32;
 
         this->buffer_send[4] = offset-5;
         this->length_send = offset;
@@ -1211,8 +1264,7 @@ namespace hw {
       memmove(&this->buffer_send[offset], D.data, 32);
       offset += 32;
       // r
-      memmove(&this->buffer_send[offset], r.data, 32);
+      this->send_secret((unsigned char*)r.data, offset);
-      offset += 32;
 
       this->buffer_send[4] = offset-5;
       this->length_send = offset;
@@ -1233,8 +1285,10 @@ namespace hw {
 
     bool device_ledger::open_tx(crypto::secret_key &tx_key) {
         AUTO_LOCK_CMD();
-
+        this->lock();
         key_map.clear();
+        hmac_map.clear();
+        this->tx_in_progress = true;
         int offset = set_command_header_noopt(INS_OPEN_TX, 0x01);
 
         //account
@@ -1248,7 +1302,13 @@ namespace hw {
         this->length_send = offset;
         this->exchange();
 
-        memmove(tx_key.data, &this->buffer_recv[32], 32);
+        //skip R, receive: r, r_hmac, fake_a, a_hmac, fake_b, hmac_b
+        unsigned char tmp[32];
+        offset = 32;
+        this->receive_secret((unsigned char*)tx_key.data, offset);
+        this->receive_secret(tmp, offset);
+        this->receive_secret(tmp, offset);
+
         #ifdef DEBUG_HWDEVICE
         const crypto::secret_key r_x = hw::ledger::decrypt(tx_key); 
         log_hexbuffer("open_tx: [[OUT]] R ", (char*)&this->buffer_recv[0], 32);
@@ -1276,8 +1336,7 @@ namespace hw {
         memmove(&this->buffer_send[offset], public_key.data, 32);
         offset += 32;
         //sec
-        memmove(&this->buffer_send[offset], secret_key.data, 32);
+        this->send_secret((unsigned char*)secret_key.data, offset);
-        offset += 32;
         //id
         memmove(&this->buffer_send[offset], payment_id.data, 8);
         offset += 8;
@@ -1365,8 +1424,7 @@ namespace hw {
       this->buffer_send[offset+3] = tx_version>>0;
       offset += 4;
       //tx_key
-      memmove(&this->buffer_send[offset], tx_key.data, 32);
+      this->send_secret((unsigned char*)tx_key.data, offset);
-      offset += 32;
       //txkey_pub
       memmove(&this->buffer_send[offset], txkey_pub.data, 32);
       offset += 32;
@@ -1394,11 +1452,11 @@ namespace hw {
       offset++;
       //additional_tx_key
       if (need_additional_txkeys) {
-        memmove(&this->buffer_send[offset], additional_txkey.sec.data, 32);
+        this->send_secret((unsigned char*)additional_txkey.sec.data, offset);
       } else {
         memset(&this->buffer_send[offset], 0, 32);
-      }
         offset += 32;
+      }
 
       this->buffer_send[4] = offset-5;
       this->length_send = offset;
@@ -1411,9 +1469,8 @@ namespace hw {
       {
         ASSERT_X(recv_len>=32, "Not enought data from device");
         crypto::secret_key scalar1;
-        memmove(scalar1.data, &this->buffer_recv[offset],32);
+        this->receive_secret((unsigned char*)scalar1.data, offset);
         amount_keys.push_back(rct::sk2rct(scalar1));
-        offset += 32;
         recv_len -= 32;
       }
       ASSERT_X(recv_len>=32, "Not enought data from device");
@@ -1464,8 +1521,7 @@ namespace hw {
         rct::key mask;
         int offset = set_command_header_noopt(INS_GEN_COMMITMENT_MASK);
         // AKout
-        memmove(this->buffer_send+offset, AKout.bytes, 32);
+        this->send_secret(AKout.bytes, offset);
-        offset += 32;
 
         this->buffer_send[4] = offset-5;
         this->length_send = offset;
@@ -1494,8 +1550,7 @@ namespace hw {
         this->buffer_send[offset] = short_amount?0x02:0x00;
         offset += 1;        
         // AKout
-        memmove(this->buffer_send+offset, AKout.bytes, 32);
+        this->send_secret(AKout.bytes, offset);
-        offset += 32;
         //mask k
         memmove(this->buffer_send+offset, unmasked.mask.bytes, 32);
         offset += 32;
@@ -1535,8 +1590,7 @@ namespace hw {
         this->buffer_send[offset] = short_amount?0x02:0x00;
         offset += 1;        
         // AKout
-        memmove(this->buffer_send+offset, AKout.bytes, 32);
+        this->send_secret(AKout.bytes, offset);
-        offset += 32;
         //mask k
         memmove(this->buffer_send+offset, masked.mask.bytes, 32);
         offset += 32;
@@ -1649,7 +1703,6 @@ namespace hw {
           this->buffer_send[offset] = (i==outputs_size-1)? 0x00:0x80 ;
           this->buffer_send[offset] |= (type==rct::RCTTypeBulletproof2)?0x02:0x00;
           offset += 1;
-          if (found) {
           //is_subaddress
           this->buffer_send[offset] = outKeys.is_subaddress;
           offset++;
@@ -1663,12 +1716,8 @@ namespace hw {
           memmove(this->buffer_send+offset, outKeys.Bout.bytes, 32);
           offset+=32;
           //AKout
-            memmove(this->buffer_send+offset, outKeys.AKout.bytes, 32);
+          this->send_secret(outKeys.AKout.bytes, offset);
-            offset+=32;
+
-          } else {
-            // dummy: is_subaddress Aout Bout AKout
-            offset += 2+32*3;
-          }
           //C
           memmove(this->buffer_send+offset, data+C_offset,32);
           offset += 32;
@@ -1760,17 +1809,19 @@ namespace hw {
         memmove(this->buffer_send+offset, H.bytes, 32);
         offset += 32;
         //mask xin
-        memmove(this->buffer_send+offset, xx.bytes, 32);
+        this->send_secret(xx.bytes, offset);
-        offset += 32;
 
         this->buffer_send[4] = offset-5;
         this->length_send = offset;
         this->exchange();
 
-        memmove(a.bytes,   &this->buffer_recv[32*0], 32);
+        offset = 0;
-        memmove(aG.bytes,  &this->buffer_recv[32*1], 32);
+        this->receive_secret(a.bytes, offset);
-        memmove(aHP.bytes, &this->buffer_recv[32*2], 32);
+        memmove(aG.bytes,  &this->buffer_recv[offset], 32);
-        memmove(II.bytes,  &this->buffer_recv[32*3], 32);
+        offset +=32;
+        memmove(aHP.bytes, &this->buffer_recv[offset], 32);
+        offset +=32;
+        memmove(II.bytes,  &this->buffer_recv[offset], 32);
 
         #ifdef DEBUG_HWDEVICE
         a_x = hw::ledger::decrypt(a);
@@ -1788,6 +1839,7 @@ namespace hw {
 
     bool device_ledger::mlsag_prepare(rct::key &a, rct::key &aG) {
         AUTO_LOCK_CMD();
+        int offset;
 
         #ifdef DEBUG_HWDEVICE
         rct::key a_x;
@@ -1796,8 +1848,9 @@ namespace hw {
 
         send_simple(INS_MLSAG, 0x01);
 
-        memmove(a.bytes,   &this->buffer_recv[32*0], 32);
+        offset = 0;
-        memmove(aG.bytes,  &this->buffer_recv[32*1], 32);
+        this->receive_secret(a.bytes, offset);
+        memmove(aG.bytes,  &this->buffer_recv[offset], 32);
 
         #ifdef DEBUG_HWDEVICE
         a_x = hw::ledger::decrypt(a);
@@ -1870,11 +1923,9 @@ namespace hw {
           }
           offset += 1;
           //xx
-          memmove(this->buffer_send+offset, xx[j].bytes, 32);
+          this->send_secret(xx[j].bytes, offset);
-          offset += 32;
           //alpa
-          memmove(this->buffer_send+offset, alpha[j].bytes, 32);
+          this->send_secret(alpha[j].bytes, offset);
-          offset += 32;
 
           this->buffer_send[4] = offset-5;
           this->length_send = offset;
@@ -1900,6 +1951,10 @@ namespace hw {
     bool device_ledger::close_tx() {
         AUTO_LOCK_CMD();
         send_simple(INS_CLOSE_TX);
+        key_map.clear();
+        hmac_map.clear();
+        this->tx_in_progress = false;
+        this->unlock();
         return true;
     }
 

src/device/device_ledger.hpp

@@ -90,6 +90,25 @@ namespace hw {
         void log();
     };
 
+    class SecHMAC {
+    public:
+        uint32_t  sec[32];
+        uint32_t  hmac[32];
+
+        SecHMAC(const uint8_t s[32], const uint8_t m[32]);
+
+    };
+
+    class HMACmap {
+    public:
+        std::vector<SecHMAC>  hmacs;
+
+        void find_mac(const uint8_t sec[32], uint8_t hmac[32]) ;
+        void add_mac(const uint8_t sec[32], const uint8_t hmac[32]) ;
+        void clear() ;
+    };
+
+
     #define BUFFER_SEND_SIZE 262
     #define BUFFER_RECV_SIZE 262
 
@@ -115,15 +134,21 @@ namespace hw {
         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);
         void send_simple(unsigned char ins, unsigned char p1 = 0x00);
-
+        void send_secret(const unsigned char sec[32], int &offset);
+        void receive_secret(unsigned char sec[32], int &offset);
 
         // hw running mode
         device_mode mode;
+        bool tx_in_progress;
+
         // map public destination key to ephemeral destination key
         Keymap key_map;
         bool  add_output_key_mapping(const crypto::public_key &Aout, const crypto::public_key &Bout, const bool is_subaddress, const bool is_change,
                                      const bool need_additional, const size_t real_output_index,
                                      const rct::key &amount_key,  const crypto::public_key &out_eph_public_key);
+        //hmac for some encrypted value
+        HMACmap hmac_map;
+
         // To speed up blockchain parsing the view key maybe handle here.
         crypto::secret_key viewkey;
         bool has_view_key;