Merge pull request #6329

6bfcd3101 Updates InProofV1, OutProofV1, and ReserveProofV1 to new V2 variants that include all public proof parameters in Schnorr challenges, along with hash function domain separators. Includes new randomized unit tests. (Sarang Noether)

4 files changed, 167 insertions, 26 deletions

diff --git a/src/crypto/crypto.cpp b/src/crypto/crypto.cpp
index 1e4a6d33f..4cfe83d54 100644
--- a/src/crypto/crypto.cpp
+++ b/src/crypto/crypto.cpp
@@ -43,6 +43,8 @@
 #include "crypto.h"
 #include "hash.h"
 
+#include "cryptonote_config.h"
+
 namespace {
   static void local_abort(const char *msg)
   {
@@ -261,11 +263,24 @@ namespace crypto {
     ec_point comm;
   };
 
+  // Used in v1 tx proofs
+  struct s_comm_2_v1 {
+    hash msg;
+    ec_point D;
+    ec_point X;
+    ec_point Y;
+  };
+
+  // Used in v1/v2 tx proofs
   struct s_comm_2 {
     hash msg;
     ec_point D;
     ec_point X;
     ec_point Y;
+    hash sep; // domain separation
+    ec_point R;
+    ec_point A;
+    ec_point B;
   };
 
   void crypto_ops::generate_signature(const hash &prefix_hash, const public_key &pub, const secret_key &sec, signature &sig) {
@@ -321,6 +336,86 @@ namespace crypto {
     return sc_isnonzero(&c) == 0;
   }
 
+  // Generate a proof of knowledge of `r` such that (`R = rG` and `D = rA`) or (`R = rB` and `D = rA`) via a Schnorr proof
+  // This handles use cases for both standard addresses and subaddresses
+  //
+  // NOTE: This generates old v1 proofs, and is for TESTING ONLY
+  void crypto_ops::generate_tx_proof_v1(const hash &prefix_hash, const public_key &R, const public_key &A, const boost::optional<public_key> &B, const public_key &D, const secret_key &r, signature &sig) {
+    // sanity check
+    ge_p3 R_p3;
+    ge_p3 A_p3;
+    ge_p3 B_p3;
+    ge_p3 D_p3;
+    if (ge_frombytes_vartime(&R_p3, &R) != 0) throw std::runtime_error("tx pubkey is invalid");
+    if (ge_frombytes_vartime(&A_p3, &A) != 0) throw std::runtime_error("recipient view pubkey is invalid");
+    if (B && ge_frombytes_vartime(&B_p3, &*B) != 0) throw std::runtime_error("recipient spend pubkey is invalid");
+    if (ge_frombytes_vartime(&D_p3, &D) != 0) throw std::runtime_error("key derivation is invalid");
+#if !defined(NDEBUG)
+    {
+      assert(sc_check(&r) == 0);
+      // check R == r*G or R == r*B
+      public_key dbg_R;
+      if (B)
+      {
+        ge_p2 dbg_R_p2;
+        ge_scalarmult(&dbg_R_p2, &r, &B_p3);
+        ge_tobytes(&dbg_R, &dbg_R_p2);
+      }
+      else
+      {
+        ge_p3 dbg_R_p3;
+        ge_scalarmult_base(&dbg_R_p3, &r);
+        ge_p3_tobytes(&dbg_R, &dbg_R_p3);
+      }
+      assert(R == dbg_R);
+      // check D == r*A
+      ge_p2 dbg_D_p2;
+      ge_scalarmult(&dbg_D_p2, &r, &A_p3);
+      public_key dbg_D;
+      ge_tobytes(&dbg_D, &dbg_D_p2);
+      assert(D == dbg_D);
+    }
+#endif
+
+    // pick random k
+    ec_scalar k;
+    random_scalar(k);
+    
+    s_comm_2_v1 buf;
+    buf.msg = prefix_hash;
+    buf.D = D;
+    
+    if (B)
+    {
+      // compute X = k*B
+      ge_p2 X_p2;
+      ge_scalarmult(&X_p2, &k, &B_p3);
+      ge_tobytes(&buf.X, &X_p2);
+    }
+    else
+    {
+      // compute X = k*G
+      ge_p3 X_p3;
+      ge_scalarmult_base(&X_p3, &k);
+      ge_p3_tobytes(&buf.X, &X_p3);
+    }
+    
+    // compute Y = k*A
+    ge_p2 Y_p2;
+    ge_scalarmult(&Y_p2, &k, &A_p3);
+    ge_tobytes(&buf.Y, &Y_p2);
+
+    // sig.c = Hs(Msg || D || X || Y) 
+    hash_to_scalar(&buf, sizeof(buf), sig.c);
+
+    // sig.r = k - sig.c*r
+    sc_mulsub(&sig.r, &sig.c, &unwrap(r), &k);
+  }
+
+  // Generate a proof of knowledge of `r` such that (`R = rG` and `D = rA`) or (`R = rB` and `D = rA`) via a Schnorr proof
+  // This handles use cases for both standard addresses and subaddresses
+  //
+  // Generates only proofs for InProofV2 and OutProofV2
   void crypto_ops::generate_tx_proof(const hash &prefix_hash, const public_key &R, const public_key &A, const boost::optional<public_key> &B, const public_key &D, const secret_key &r, signature &sig) {
     // sanity check
     ge_p3 R_p3;
@@ -362,10 +457,20 @@ namespace crypto {
     ec_scalar k;
     random_scalar(k);
     
+    // if B is not present
+    static const ec_point zero = {{ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 }};
+
     s_comm_2 buf;
     buf.msg = prefix_hash;
     buf.D = D;
-
+    buf.R = R;
+    buf.A = A;
+    if (B)
+        buf.B = *B;
+    else
+        buf.B = zero;
+    cn_fast_hash(config::HASH_KEY_TXPROOF_V2, sizeof(config::HASH_KEY_TXPROOF_V2)-1, buf.sep);
+    
     if (B)
     {
       // compute X = k*B
@@ -386,7 +491,7 @@ namespace crypto {
     ge_scalarmult(&Y_p2, &k, &A_p3);
     ge_tobytes(&buf.Y, &Y_p2);
 
-    // sig.c = Hs(Msg || D || X || Y)
+    // sig.c = Hs(Msg || D || X || Y || sep || R || A || B) 
     hash_to_scalar(&buf, sizeof(buf), sig.c);
 
     // sig.r = k - sig.c*r
@@ -395,7 +500,8 @@ namespace crypto {
     memwipe(&k, sizeof(k));
   }
 
-  bool crypto_ops::check_tx_proof(const hash &prefix_hash, const public_key &R, const public_key &A, const boost::optional<public_key> &B, const public_key &D, const signature &sig) {
+  // Verify a proof: either v1 (version == 1) or v2 (version == 2)
+  bool crypto_ops::check_tx_proof(const hash &prefix_hash, const public_key &R, const public_key &A, const boost::optional<public_key> &B, const public_key &D, const signature &sig, const int version) {
     // sanity check
     ge_p3 R_p3;
     ge_p3 A_p3;
@@ -467,14 +573,31 @@ namespace crypto {
     ge_p2 Y_p2;
     ge_p1p1_to_p2(&Y_p2, &Y_p1p1);
 
-    // compute c2 = Hs(Msg || D || X || Y)
+    // Compute hash challenge
+    // for v1, c2 = Hs(Msg || D || X || Y)
+    // for v2, c2 = Hs(Msg || D || X || Y || sep || R || A || B)
+
+    // if B is not present
+    static const ec_point zero = {{ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 }};
+
     s_comm_2 buf;
     buf.msg = prefix_hash;
     buf.D = D;
+    buf.R = R;
+    buf.A = A;
+    if (B)
+        buf.B = *B;
+    else
+        buf.B = zero;
+    cn_fast_hash(config::HASH_KEY_TXPROOF_V2, sizeof(config::HASH_KEY_TXPROOF_V2)-1, buf.sep);
     ge_tobytes(&buf.X, &X_p2);
     ge_tobytes(&buf.Y, &Y_p2);
     ec_scalar c2;
-    hash_to_scalar(&buf, sizeof(s_comm_2), c2);
+
+    // Hash depends on version
+    if (version == 1) hash_to_scalar(&buf, sizeof(s_comm_2) - 3*sizeof(ec_point) - sizeof(hash), c2);
+    else if (version == 2) hash_to_scalar(&buf, sizeof(s_comm_2), c2);
+    else return false;
 
     // test if c2 == sig.c
     sc_sub(&c2, &c2, &sig.c);
diff --git a/src/crypto/crypto.h b/src/crypto/crypto.h
index 70d463a16..7ddc0150f 100644
--- a/src/crypto/crypto.h
+++ b/src/crypto/crypto.h
@@ -132,8 +132,10 @@ namespace crypto {
     friend bool check_signature(const hash &, const public_key &, const signature &);
     static void generate_tx_proof(const hash &, const public_key &, const public_key &, const boost::optional<public_key> &, const public_key &, const secret_key &, signature &);
     friend void generate_tx_proof(const hash &, const public_key &, const public_key &, const boost::optional<public_key> &, const public_key &, const secret_key &, signature &);
-    static bool check_tx_proof(const hash &, const public_key &, const public_key &, const boost::optional<public_key> &, const public_key &, const signature &);
-    friend bool check_tx_proof(const hash &, const public_key &, const public_key &, const boost::optional<public_key> &, const public_key &, const signature &);
+    static void generate_tx_proof_v1(const hash &, const public_key &, const public_key &, const boost::optional<public_key> &, const public_key &, const secret_key &, signature &);
+    friend void generate_tx_proof_v1(const hash &, const public_key &, const public_key &, const boost::optional<public_key> &, const public_key &, const secret_key &, signature &);
+    static bool check_tx_proof(const hash &, const public_key &, const public_key &, const boost::optional<public_key> &, const public_key &, const signature &, const int);
+    friend bool check_tx_proof(const hash &, const public_key &, const public_key &, const boost::optional<public_key> &, const public_key &, const signature &, const int);
     static void generate_key_image(const public_key &, const secret_key &, key_image &);
     friend void generate_key_image(const public_key &, const secret_key &, key_image &);
     static void generate_ring_signature(const hash &, const key_image &,
@@ -248,8 +250,11 @@ namespace crypto {
   inline void generate_tx_proof(const hash &prefix_hash, const public_key &R, const public_key &A, const boost::optional<public_key> &B, const public_key &D, const secret_key &r, signature &sig) {
     crypto_ops::generate_tx_proof(prefix_hash, R, A, B, D, r, sig);
   }
-  inline bool check_tx_proof(const hash &prefix_hash, const public_key &R, const public_key &A, const boost::optional<public_key> &B, const public_key &D, const signature &sig) {
-    return crypto_ops::check_tx_proof(prefix_hash, R, A, B, D, sig);
+  inline void generate_tx_proof_v1(const hash &prefix_hash, const public_key &R, const public_key &A, const boost::optional<public_key> &B, const public_key &D, const secret_key &r, signature &sig) {
+    crypto_ops::generate_tx_proof_v1(prefix_hash, R, A, B, D, r, sig);
+  }
+  inline bool check_tx_proof(const hash &prefix_hash, const public_key &R, const public_key &A, const boost::optional<public_key> &B, const public_key &D, const signature &sig, const int version) {
+    return crypto_ops::check_tx_proof(prefix_hash, R, A, B, D, sig, version);
   }
 
   /* To send money to a key:
diff --git a/src/cryptonote_config.h b/src/cryptonote_config.h
index f88f622f2..8c4e61d4d 100644
--- a/src/cryptonote_config.h
+++ b/src/cryptonote_config.h
@@ -224,6 +224,7 @@ namespace config
   const unsigned char HASH_KEY_RPC_PAYMENT_NONCE = 0x58;
   const unsigned char HASH_KEY_MEMORY = 'k';
   const unsigned char HASH_KEY_MULTISIG[] = {'M', 'u', 'l', 't' , 'i', 's', 'i', 'g', 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 };
+  const unsigned char HASH_KEY_TXPROOF_V2[] = "TXPROOF_V2";
 
   namespace testnet
   {
diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp
index d7ed3e999..760693b60 100644
--- a/src/wallet/wallet2.cpp
+++ b/src/wallet/wallet2.cpp
@@ -11425,7 +11425,7 @@ std::string wallet2::get_tx_proof(const cryptonote::transaction &tx, const crypt
         hwdev.generate_tx_proof(prefix_hash, tx_pub_key, address.m_view_public_key, boost::none, shared_secret[i], additional_tx_keys[i - 1], sig[i]);
       }
     }
-    sig_str = std::string("OutProofV1");
+    sig_str = std::string("OutProofV2");
   }
   else
   {
@@ -11461,7 +11461,7 @@ std::string wallet2::get_tx_proof(const cryptonote::transaction &tx, const crypt
         hwdev.generate_tx_proof(prefix_hash, address.m_view_public_key, additional_tx_pub_keys[i - 1], boost::none, shared_secret[i], a, sig[i]);
       }
     }
-    sig_str = std::string("InProofV1");
+    sig_str = std::string("InProofV2");
   }
   const size_t num_sigs = shared_secret.size();
 
@@ -11540,8 +11540,14 @@ bool wallet2::check_tx_proof(const crypto::hash &txid, const cryptonote::account
 
 bool wallet2::check_tx_proof(const cryptonote::transaction &tx, const cryptonote::account_public_address &address, bool is_subaddress, const std::string &message, const std::string &sig_str, uint64_t &received) const
 {
+  // InProofV1, InProofV2, OutProofV1, OutProofV2
   const bool is_out = sig_str.substr(0, 3) == "Out";
-  const std::string header = is_out ? "OutProofV1" : "InProofV1";
+  const std::string header = is_out ? sig_str.substr(0,10) : sig_str.substr(0,9);
+  int version = 2; // InProofV2
+  if (is_out && sig_str.substr(8,2) == "V1") version = 1; // OutProofV1
+  else if (is_out) version = 2; // OutProofV2
+  else if (sig_str.substr(7,2) == "V1") version = 1; // InProofV1
+
   const size_t header_len = header.size();
   THROW_WALLET_EXCEPTION_IF(sig_str.size() < header_len || sig_str.substr(0, header_len) != header, error::wallet_internal_error,
     "Signature header check error");
@@ -11588,27 +11594,27 @@ bool wallet2::check_tx_proof(const cryptonote::transaction &tx, const cryptonote
   if (is_out)
   {
     good_signature[0] = is_subaddress ?
-      crypto::check_tx_proof(prefix_hash, tx_pub_key, address.m_view_public_key, address.m_spend_public_key, shared_secret[0], sig[0]) :
-      crypto::check_tx_proof(prefix_hash, tx_pub_key, address.m_view_public_key, boost::none, shared_secret[0], sig[0]);
+      crypto::check_tx_proof(prefix_hash, tx_pub_key, address.m_view_public_key, address.m_spend_public_key, shared_secret[0], sig[0], version) :
+      crypto::check_tx_proof(prefix_hash, tx_pub_key, address.m_view_public_key, boost::none, shared_secret[0], sig[0], version);
 
     for (size_t i = 0; i < additional_tx_pub_keys.size(); ++i)
     {
       good_signature[i + 1] = is_subaddress ?
-        crypto::check_tx_proof(prefix_hash, additional_tx_pub_keys[i], address.m_view_public_key, address.m_spend_public_key, shared_secret[i + 1], sig[i + 1]) :
-        crypto::check_tx_proof(prefix_hash, additional_tx_pub_keys[i], address.m_view_public_key, boost::none, shared_secret[i + 1], sig[i + 1]);
+        crypto::check_tx_proof(prefix_hash, additional_tx_pub_keys[i], address.m_view_public_key, address.m_spend_public_key, shared_secret[i + 1], sig[i + 1], version) :
+        crypto::check_tx_proof(prefix_hash, additional_tx_pub_keys[i], address.m_view_public_key, boost::none, shared_secret[i + 1], sig[i + 1], version);
     }
   }
   else
   {
     good_signature[0] = is_subaddress ?
-      crypto::check_tx_proof(prefix_hash, address.m_view_public_key, tx_pub_key, address.m_spend_public_key, shared_secret[0], sig[0]) :
-      crypto::check_tx_proof(prefix_hash, address.m_view_public_key, tx_pub_key, boost::none, shared_secret[0], sig[0]);
+      crypto::check_tx_proof(prefix_hash, address.m_view_public_key, tx_pub_key, address.m_spend_public_key, shared_secret[0], sig[0], version) :
+      crypto::check_tx_proof(prefix_hash, address.m_view_public_key, tx_pub_key, boost::none, shared_secret[0], sig[0], version);
 
     for (size_t i = 0; i < additional_tx_pub_keys.size(); ++i)
     {
       good_signature[i + 1] = is_subaddress ?
-        crypto::check_tx_proof(prefix_hash, address.m_view_public_key, additional_tx_pub_keys[i], address.m_spend_public_key, shared_secret[i + 1], sig[i + 1]) :
-        crypto::check_tx_proof(prefix_hash, address.m_view_public_key, additional_tx_pub_keys[i], boost::none, shared_secret[i + 1], sig[i + 1]);
+        crypto::check_tx_proof(prefix_hash, address.m_view_public_key, additional_tx_pub_keys[i], address.m_spend_public_key, shared_secret[i + 1], sig[i + 1], version) :
+        crypto::check_tx_proof(prefix_hash, address.m_view_public_key, additional_tx_pub_keys[i], boost::none, shared_secret[i + 1], sig[i + 1], version);
     }
   }
 
@@ -11746,7 +11752,7 @@ std::string wallet2::get_reserve_proof(const boost::optional<std::pair<uint32_t,
   std::ostringstream oss;
   boost::archive::portable_binary_oarchive ar(oss);
   ar << proofs << subaddr_spendkeys;
-  return "ReserveProofV1" + tools::base58::encode(oss.str());
+  return "ReserveProofV2" + tools::base58::encode(oss.str());
 }
 
 bool wallet2::check_reserve_proof(const cryptonote::account_public_address &address, const std::string &message, const std::string &sig_str, uint64_t &total, uint64_t &spent)
@@ -11755,12 +11761,18 @@ bool wallet2::check_reserve_proof(const cryptonote::account_public_address &addr
   THROW_WALLET_EXCEPTION_IF(!check_connection(&rpc_version), error::wallet_internal_error, "Failed to connect to daemon: " + get_daemon_address());
   THROW_WALLET_EXCEPTION_IF(rpc_version < MAKE_CORE_RPC_VERSION(1, 0), error::wallet_internal_error, "Daemon RPC version is too old");
 
-  static constexpr char header[] = "ReserveProofV1";
-  THROW_WALLET_EXCEPTION_IF(!boost::string_ref{sig_str}.starts_with(header), error::wallet_internal_error,
+  static constexpr char header_v1[] = "ReserveProofV1";
+  static constexpr char header_v2[] = "ReserveProofV2"; // assumes same length as header_v1
+  THROW_WALLET_EXCEPTION_IF(!boost::string_ref{sig_str}.starts_with(header_v1) && !boost::string_ref{sig_str}.starts_with(header_v2), error::wallet_internal_error,
     "Signature header check error");
+  int version = 2; // assume newest version
+  if (boost::string_ref{sig_str}.starts_with(header_v1))
+      version = 1;
+  else if (boost::string_ref{sig_str}.starts_with(header_v2))
+      version = 2;
 
   std::string sig_decoded;
-  THROW_WALLET_EXCEPTION_IF(!tools::base58::decode(sig_str.substr(std::strlen(header)), sig_decoded), error::wallet_internal_error,
+  THROW_WALLET_EXCEPTION_IF(!tools::base58::decode(sig_str.substr(std::strlen(header_v1)), sig_decoded), error::wallet_internal_error,
     "Signature decoding error");
 
   std::istringstream iss(sig_decoded);
@@ -11841,9 +11853,9 @@ bool wallet2::check_reserve_proof(const cryptonote::account_public_address &addr
     const std::vector<crypto::public_key> additional_tx_pub_keys = get_additional_tx_pub_keys_from_extra(tx);
 
     // check singature for shared secret
-    ok = crypto::check_tx_proof(prefix_hash, address.m_view_public_key, tx_pub_key, boost::none, proof.shared_secret, proof.shared_secret_sig);
+    ok = crypto::check_tx_proof(prefix_hash, address.m_view_public_key, tx_pub_key, boost::none, proof.shared_secret, proof.shared_secret_sig, version);
     if (!ok && additional_tx_pub_keys.size() == tx.vout.size())
-      ok = crypto::check_tx_proof(prefix_hash, address.m_view_public_key, additional_tx_pub_keys[proof.index_in_tx], boost::none, proof.shared_secret, proof.shared_secret_sig);
+      ok = crypto::check_tx_proof(prefix_hash, address.m_view_public_key, additional_tx_pub_keys[proof.index_in_tx], boost::none, proof.shared_secret, proof.shared_secret_sig, version);
     if (!ok)
       return false;