From 8a73f5d242540320f7b142cdb79bf7f547a3f2d1 Mon Sep 17 00:00:00 2001 From: Glenn Willen Date: Fri, 28 Aug 2020 05:01:37 -0700 Subject: [PATCH] When decoding or signing a PSBT, check blinded values. When we decode or sign a PSBT given to us via RPC, first check that, if it contains blinded output values, they verifiably match the unblinded output values contained in the original transaction proposal. Otherwise fail. --- src/blind.cpp | 68 ++++++++++++++++++++++++++++++++++++++ src/blind.h | 8 +++++ src/rpc/rawtransaction.cpp | 36 ++++++++++++++++++++ src/rpc/rawtransaction.h | 3 ++ src/wallet/rpcwallet.cpp | 1 + 5 files changed, 116 insertions(+) diff --git a/src/blind.cpp b/src/blind.cpp index e9db1a034f..e99395225d 100644 --- a/src/blind.cpp +++ b/src/blind.cpp @@ -36,6 +36,74 @@ public: static Blind_ECC_Init ecc_init_on_load; +bool VerifyConfidentialPair(const CConfidentialValue& conf_value, const CConfidentialAsset& conf_asset, const CAmount& claimed_value, const CAsset& claimed_asset, const uint256& value_blinding_factor, const uint256& asset_blinding_factor) { + if (conf_value.IsNull() || conf_asset.IsNull() || claimed_asset.IsNull()) { + return false; + } + + if (conf_value.IsExplicit()) { + // Match behavior of UnblindConfidentialPair + return false; + } + if (conf_asset.IsExplicit() && conf_asset.GetAsset() != claimed_asset) { + return false; + } + + // Just to be safe + if (!MoneyRange(claimed_value)) { + return false; + } + + // Valid asset commitment? + secp256k1_generator observed_gen; + if (conf_asset.IsCommitment()) { + if (secp256k1_generator_parse(secp256k1_blind_context, &observed_gen, &conf_asset.vchCommitment[0]) != 1) + return false; + } else if (conf_asset.IsExplicit()) { + if (secp256k1_generator_generate(secp256k1_blind_context, &observed_gen, conf_asset.GetAsset().begin()) != 1) + return false; + } + + // Valid value commitment? + secp256k1_pedersen_commitment value_commit; + if (secp256k1_pedersen_commitment_parse(secp256k1_blind_context, &value_commit, conf_value.vchCommitment.data()) != 1) { + return false; + } + + const unsigned char *asset_type = claimed_asset.id.begin(); + const unsigned char *asset_blinder = asset_blinding_factor.begin(); + secp256k1_generator recalculated_gen; + if (secp256k1_generator_generate_blinded(secp256k1_blind_context, &recalculated_gen, asset_type, asset_blinder) != 1) { + return false; + } + + // Serialize both generators then compare + unsigned char observed_generator[33]; + unsigned char derived_generator[33]; + secp256k1_generator_serialize(secp256k1_blind_context, observed_generator, &observed_gen); + secp256k1_generator_serialize(secp256k1_blind_context, derived_generator, &recalculated_gen); + if (memcmp(observed_generator, derived_generator, sizeof(observed_generator))) { + return false; + } + + const unsigned char *value_blinder = value_blinding_factor.begin(); + secp256k1_pedersen_commitment recalculated_commit; + if(secp256k1_pedersen_commit(secp256k1_blind_context, &recalculated_commit, value_blinder, claimed_value, &observed_gen) != 1) { + return false; + } + + // Serialize both value commitments then compare + unsigned char claimed_commitment[33]; + unsigned char derived_commitment[33]; + secp256k1_pedersen_commitment_serialize(secp256k1_blind_context, claimed_commitment, &value_commit); + secp256k1_pedersen_commitment_serialize(secp256k1_blind_context, derived_commitment, &recalculated_commit); + if (memcmp(claimed_commitment, derived_commitment, sizeof(claimed_commitment))) { + return false; + } + + return true; +} + bool UnblindConfidentialPair(const CKey& blinding_key, const CConfidentialValue& conf_value, const CConfidentialAsset& conf_asset, const CConfidentialNonce& nonce_commitment, const CScript& committedScript, const std::vector& vchRangeproof, CAmount& amount_out, uint256& blinding_factor_out, CAsset& asset_out, uint256& asset_blinding_factor_out) { if (!blinding_key.IsValid() || vchRangeproof.size() == 0) { diff --git a/src/blind.h b/src/blind.h index 13f335e426..462638f06e 100644 --- a/src/blind.h +++ b/src/blind.h @@ -22,6 +22,14 @@ static const size_t SURJECTION_PROOF_SIZE = 67; // 32 bytes of asset type, 32 bytes of asset blinding factor in sidechannel static const size_t SIDECHANNEL_MSG_SIZE = 64; +/* + * Verify a pair of confidential asset and value, given the blinding factors for both. + * Unlike UnblindConfidentialPair, this does _not_ require the recipient's blinding + * key, but it _does_ require the blinding factors be provided (rather than extracting + * them from the rangeproof.) +*/ +bool VerifyConfidentialPair(const CConfidentialValue& conf_value, const CConfidentialAsset& conf_asset, const CAmount& claimed_value, const CAsset& claimed_asset, const uint256& value_blinding_factor, const uint256& asset_blinding_factor); + /* * Unblind a pair of confidential asset and value. * Note that unblinded data will only be outputted if *BOTH* asset and value could be unblinded. diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 7959a60533..8bb7fa6c9d 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -74,6 +74,41 @@ static void TxToJSON(const CTransaction& tx, const uint256 hashBlock, UniValue& } } +void RPCCheckPSBTBlinding(const PartiallySignedTransaction& psbtx) { + // Plausibly, we may want a way to let the user continue anyway. However, we + // want to fail by default, to make it as hard as possible to do something + // really dangerous. And since this way of handling blinded PSBTs is going + // away "real soon now" in favor of a better one, no sense in trying too + // hard about it. + + for (size_t i = 0; i < psbtx.outputs.size(); ++i) { + const PSBTOutput& output = psbtx.outputs[i]; + const CTxOut& txo = psbtx.tx->vout[i]; + + if (txo.nValue.IsCommitment() || txo.nAsset.IsCommitment()) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "PSBT's 'tx' field may not have pre-blinded outputs."); + } + + if (!output.value_commitment.IsCommitment() && + !output.asset_commitment.IsCommitment() && + output.value_blinding_factor.IsNull() && + output.asset_blinding_factor.IsNull()) { + // Nothing blinded, nothing to check. + continue; + } else if (!output.value_commitment.IsCommitment() || + !output.asset_commitment.IsCommitment() || + output.value_blinding_factor.IsNull() || + output.asset_blinding_factor.IsNull()) { + // Something blinded, but not everything? That's not expected. + throw JSONRPCError(RPC_INVALID_PARAMETER, "PSBT has a partially-blinded output. Blinded outputs must be fully blinded."); + } + + if (!VerifyConfidentialPair(output.value_commitment, output.asset_commitment, txo.nValue.GetAmount(), txo.nAsset.GetAsset(), output.value_blinding_factor, output.asset_blinding_factor)) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "PSBT's 'tx' field output values do not match blinded output values (or are invalid in some way)! Either there is a bug, or the blinder is attacking you."); + } + } +} + static UniValue getrawtransaction(const JSONRPCRequest& request) { if (request.fHelp || request.params.size() < 1 || request.params.size() > 3) @@ -1722,6 +1757,7 @@ UniValue decodepsbt(const JSONRPCRequest& request) if (!DecodeBase64PSBT(psbtx, request.params[0].get_str(), error)) { throw JSONRPCError(RPC_DESERIALIZATION_ERROR, strprintf("TX decode failed %s", error)); } + RPCCheckPSBTBlinding(psbtx); UniValue result(UniValue::VOBJ); diff --git a/src/rpc/rawtransaction.h b/src/rpc/rawtransaction.h index 42784aeb46..58db5b14bd 100644 --- a/src/rpc/rawtransaction.h +++ b/src/rpc/rawtransaction.h @@ -13,6 +13,9 @@ namespace interfaces { class Chain; } // namespace interfaces +/** Check that the blinder did not tamper with the values in a blinded PSBT. */ +void RPCCheckPSBTBlinding(const PartiallySignedTransaction& psbtx); + /** Sign a transaction with the given keystore and previous transactions */ UniValue SignTransaction(interfaces::Chain& chain, CMutableTransaction& mtx, const UniValue& prevTxs, CBasicKeyStore *keystore, bool tempKeystore, const UniValue& hashType); diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 8a92943573..a1225fefc6 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -4561,6 +4561,7 @@ UniValue walletsignpsbt(const JSONRPCRequest& request) if (!DecodeBase64PSBT(psbtx, request.params[0].get_str(), error)) { throw JSONRPCError(RPC_DESERIALIZATION_ERROR, strprintf("TX decode failed %s", error)); } + RPCCheckPSBTBlinding(psbtx); // Get the sighash type int nHashType = ParseSighashString(request.params[1]);