diff --git a/mm2src/coins/qrc20.rs b/mm2src/coins/qrc20.rs index ad01bba16f..07aa04aad2 100644 --- a/mm2src/coins/qrc20.rs +++ b/mm2src/coins/qrc20.rs @@ -842,7 +842,8 @@ impl SwapOps for Qrc20Coin { }, }; let fee_tx_hash = fee_tx.hash().reversed().into(); - let inputs_signed_by_pub = check_all_utxo_inputs_signed_by_pub(fee_tx, validate_fee_args.expected_sender)?; + let inputs_signed_by_pub = + check_all_utxo_inputs_signed_by_pub(self, fee_tx, validate_fee_args.expected_sender).await?; if !inputs_signed_by_pub { return MmError::err(ValidatePaymentError::WrongPaymentTx( "The dex fee was sent from wrong address".to_string(), diff --git a/mm2src/coins/utxo/rpc_clients/electrum_rpc/client.rs b/mm2src/coins/utxo/rpc_clients/electrum_rpc/client.rs index e7c5311db2..3cf3996f09 100644 --- a/mm2src/coins/utxo/rpc_clients/electrum_rpc/client.rs +++ b/mm2src/coins/utxo/rpc_clients/electrum_rpc/client.rs @@ -806,8 +806,12 @@ impl UtxoRpcClientOps for ElectrumClient { // If the plain pubkey is available, fetch the UTXOs found in P2PK outputs as well (if any). if let Some(pubkey) = address.pubkey() { - let p2pk_output_script = output_script_p2pk(pubkey); - output_scripts.push(p2pk_output_script); + // We don't want to show P2PK outputs along with segwit ones (P2WPKH). + // Allow listing the P2PK outputs only if the address is not segwit (i.e. show P2PK outputs along with P2PKH). + if !address.addr_format().is_segwit() { + let p2pk_output_script = output_script_p2pk(pubkey); + output_scripts.push(p2pk_output_script); + } } let this = self.clone(); @@ -937,8 +941,11 @@ impl UtxoRpcClientOps for ElectrumClient { // If the plain pubkey is available, fetch the balance found in P2PK output as well (if any). if let Some(pubkey) = address.pubkey() { - let p2pk_output_script = output_script_p2pk(pubkey); - hashes.push(hex::encode(electrum_script_hash(&p2pk_output_script))); + // Show the balance in P2PK outputs only for the non-segwit legacy addresses (P2PKH). + if !address.addr_format().is_segwit() { + let p2pk_output_script = output_script_p2pk(pubkey); + hashes.push(hex::encode(electrum_script_hash(&p2pk_output_script))); + } } let this = self.clone(); diff --git a/mm2src/coins/utxo/utxo_common.rs b/mm2src/coins/utxo/utxo_common.rs index 2e792f05ac..c6eed83761 100644 --- a/mm2src/coins/utxo/utxo_common.rs +++ b/mm2src/coins/utxo/utxo_common.rs @@ -2019,19 +2019,100 @@ pub async fn send_maker_refunds_payment( refund_htlc_payment(coin, args).await.map(|tx| tx.into()) } +/// Sets the amount of the input at the given index to the value of the corresponding output in the previous transaction. +/// +/// This invokes the RPC client to fetch the previous transaction and extract the output value. +pub async fn set_index_amount_from_prev_tx( + rpc_client: &UtxoRpcClientEnum, + signer: &mut TransactionInputSigner, + idx: usize, +) -> Result<(), String> { + let inputs_len = signer.inputs.len(); + let input = signer.inputs.get_mut(idx).ok_or_else(|| { + format!( + "Input index {} out of bounds for transaction with {} inputs", + idx, inputs_len + ) + })?; + let prev_output_tx_hash = input.previous_output.hash.reversed().into(); + let prev_output_index = input.previous_output.index as usize; + let prev_tx_hex = rpc_client + .get_transaction_bytes(&prev_output_tx_hash) + .compat() + .await + .map_err(|e| format!("Failed to get prev tx hex: {e}"))?; + let prev_tx: UtxoTx = deserialize(prev_tx_hex.0.as_slice()) + .map_err(|e| format!("Failed to deserialize prev tx {}: {}", prev_output_tx_hash, e))?; + let prev_output = prev_tx.outputs.get(prev_output_index).ok_or_else(|| { + format!( + "Prev tx output index {} out of bounds for tx {}", + input.previous_output.index, + prev_tx.hash() + ) + })?; + input.amount = prev_output.value; + Ok(()) +} + +/// Verifies that the script that spends a P2PK is signed by the expected pubkey. +fn verify_p2pk_input_pubkey( + script: &Script, + expected_pubkey: &Public, + unsigned_tx: &TransactionInputSigner, + index: usize, + signature_version: SignatureVersion, + fork_id: u32, +) -> Result { + // Extract the signature from the scriptSig. + let signature = script.extract_signature()?; + // Validate the signature. + try_s!(SecpSignature::from_der(&signature[..signature.len() - 1])); + let signature = signature.into(); + // Make sure we have no more instructions. P2PK scriptSigs consist of a single instruction only containing the signature. + if script.get_instruction(1).is_some() { + return ERR!("Unexpected instruction at position 2 of script {:?}", script); + }; + // Get the scriptPub for this input. We need it to get the transaction sig_hash to sign (but actually "to verify" in this case). + let pubkey = expected_pubkey + .to_secp256k1_pubkey() + .map_err(|e| ERRL!("Error converting plain pubkey to secp256k1 pubkey: {}", e))?; + // P2PK scriptPub has two valid possible formats depending on whether the public key is written in compressed or uncompressed form. + let possible_pubkey_scripts = [ + Builder::build_p2pk(&Public::Compressed(pubkey.serialize().into())), + Builder::build_p2pk(&Public::Normal(pubkey.serialize_uncompressed().into())), + ]; + for pubkey_script in possible_pubkey_scripts { + // Get the transaction hash that has been signed in the scriptSig. + let hash = match signature_hash_to_sign( + unsigned_tx, + index, + &pubkey_script, + signature_version, + SIGHASH_ALL, + fork_id, + ) { + Ok(hash) => hash, + Err(e) => return ERR!("Error calculating signature hash: {}", e), + }; + // Verify that the signature is valid for the transaction hash with respect to the expected public key. + return match expected_pubkey.verify(&hash, &signature) { + Ok(true) => Ok(true), + // The signature is invalid for this pubkey, try the other possible pubkey script. + Ok(false) => continue, + Err(e) => ERR!("Error verifying signature: {}", e), + }; + } + + // Both possible pubkey scripts failed to verify the signature. + Ok(false) +} + /// Extracts pubkey from script sig fn pubkey_from_script_sig(script: &Script) -> Result { - match script.get_instruction(0) { - Some(Ok(instruction)) => match instruction.opcode { - Opcode::OP_PUSHBYTES_70 | Opcode::OP_PUSHBYTES_71 | Opcode::OP_PUSHBYTES_72 => match instruction.data { - Some(bytes) => try_s!(SecpSignature::from_der(&bytes[..bytes.len() - 1])), - None => return ERR!("No data at instruction 0 of script {:?}", script), - }, - _ => return ERR!("Unexpected opcode {:?}", instruction.opcode), - }, - Some(Err(e)) => return ERR!("Error {} on getting instruction 0 of script {:?}", e, script), - None => return ERR!("None instruction 0 of script {:?}", script), - }; + // Extract the signature from the scriptSig. + let signature = script.extract_signature()?; + // Validate the signature. + try_s!(SecpSignature::from_der(&signature[..signature.len() - 1])); let pubkey = match script.get_instruction(1) { Some(Ok(instruction)) => match instruction.opcode { @@ -2087,18 +2168,65 @@ where } } -pub fn check_all_utxo_inputs_signed_by_pub( +/// This function is used to check that all inputs are signed/owned by the expected pubkey. +/// +/// It's used to verify that all the inputs of the taker-sent dex fee are signed/owned by the taker's pubkey. +/// It's used also by watcher to verify that all the taker payment inputs are signed/owned by the taker's pubkey. +/// The `expected_pub` should be the taker's pubkey in compressed (33-byte) format. +pub async fn check_all_utxo_inputs_signed_by_pub( + coin: &T, tx: &UtxoTx, expected_pub: &[u8], ) -> Result> { - for input in &tx.inputs { + let expected_pub = + H264::from_slice(expected_pub).map_to_mm(|e| ValidatePaymentError::TxDeserializationError(e.to_string()))?; + let mut unsigned_tx: Option = None; + + for (idx, input) in tx.inputs.iter().enumerate() { + let script = Script::from(input.script_sig.clone()); + + // This handles the case where the input is a P2PK input. + if !input.has_witness() && script.does_script_spend_p2pk() { + let unsigned_tx = unsigned_tx.get_or_insert_with(|| tx.clone().into()); + // If the transaction is overwintered, we need to set the consensus branch id and the input's amount. + // This is needed for the sighash calculation. + if unsigned_tx.overwintered { + set_index_amount_from_prev_tx(&coin.as_ref().rpc_client, unsigned_tx, idx) + .await + .map_err(|e| { + ValidatePaymentError::TxDeserializationError(format!( + "Failed to set index amount for input {}: {}", + idx, e + )) + })?; + unsigned_tx.consensus_branch_id = coin.as_ref().conf.consensus_branch_id; + } + // Verfiy that the P2PK input's scriptSig corresponds to the expected public key. + let successful_verification = verify_p2pk_input_pubkey( + &script, + &Public::Compressed(expected_pub), + unsigned_tx, + idx, + coin.as_ref().conf.signature_version, + coin.as_ref().conf.fork_id, + ) + .map_to_mm(ValidatePaymentError::TxDeserializationError)?; + if successful_verification { + // No pubkey extraction for P2PK inputs. Continue. + continue; + } + return Ok(false); + } + let pubkey = if input.has_witness() { + // Extract the pubkey from a P2WPKH scriptSig. pubkey_from_witness_script(&input.script_witness).map_to_mm(ValidatePaymentError::TxDeserializationError)? } else { - let script: Script = input.script_sig.clone().into(); + // Extract the pubkey from a P2PKH scriptSig. pubkey_from_script_sig(&script).map_to_mm(ValidatePaymentError::TxDeserializationError)? }; - if *pubkey != expected_pub { + + if pubkey != expected_pub { return Ok(false); } } @@ -2146,7 +2274,8 @@ pub fn watcher_validate_taker_fee( }; let taker_fee_tx: UtxoTx = deserialize(tx_from_rpc.hex.0.as_slice())?; - let inputs_signed_by_pub = check_all_utxo_inputs_signed_by_pub(&taker_fee_tx, &sender_pubkey)?; + let inputs_signed_by_pub = + check_all_utxo_inputs_signed_by_pub(&coin, &taker_fee_tx, &sender_pubkey).await?; if !inputs_signed_by_pub { return MmError::err(ValidatePaymentError::WrongPaymentTx(format!( "{}: Taker fee does not belong to the verified public key", @@ -2278,17 +2407,18 @@ pub fn validate_fee( ) -> ValidatePaymentFut<()> { let dex_address = try_f!(dex_address(&coin).map_to_mm(ValidatePaymentError::InternalError)); let burn_address = try_f!(burn_address(&coin).map_to_mm(ValidatePaymentError::InternalError)); - let inputs_signed_by_pub = try_f!(check_all_utxo_inputs_signed_by_pub(&tx, sender_pubkey)); - if !inputs_signed_by_pub { - return Box::new(futures01::future::err( - ValidatePaymentError::WrongPaymentTx(format!( - "{INVALID_SENDER_ERR_LOG}: Taker payment does not belong to the verified public key" - )) - .into(), - )); - } + let sender_pubkey = sender_pubkey.to_vec(); let fut = async move { + match check_all_utxo_inputs_signed_by_pub(&coin, &tx, &sender_pubkey).await { + Ok(true) => {}, + Ok(false) => { + return MmError::err(ValidatePaymentError::WrongPaymentTx(format!( + "{INVALID_SENDER_ERR_LOG}: Taker payment does not belong to the verified public key" + ))) + }, + Err(e) => return Err(e), + }; let tx_from_rpc = coin .as_ref() .rpc_client @@ -2394,7 +2524,8 @@ pub fn watcher_validate_taker_payment( let coin = coin.clone(); let fut = async move { - let inputs_signed_by_pub = check_all_utxo_inputs_signed_by_pub(&taker_payment_tx, &input.taker_pub)?; + let inputs_signed_by_pub = + check_all_utxo_inputs_signed_by_pub(&coin, &taker_payment_tx, &input.taker_pub).await?; if !inputs_signed_by_pub { return MmError::err(ValidatePaymentError::WrongPaymentTx(format!( "{INVALID_SENDER_ERR_LOG}: Taker payment does not belong to the verified public key" @@ -5288,6 +5419,52 @@ fn test_pubkey_from_script_sig() { pubkey_from_script_sig(&script_sig_err).unwrap_err(); } +#[test] +fn test_verify_p2pk_input_pubkey() { + // 65-byte (uncompressed) pubkey example. + // https://mempool.space/tx/1db6251a9afce7025a2061a19e63c700dffc3bec368bd1883decfac353357a9d + let tx: UtxoTx = "0100000001740443e82e526cef440ed590d1c43a67f509424134542de092e5ae68721575d60100000049483045022078e86c021003cca23842d4b2862dfdb68d2478a98c08c10dcdffa060e55c72be022100f6a41da12cdc2e350045f4c97feeab76a7c0ab937bd8a9e507293ce6d37c9cc201ffffffff0200f2052a010000001976a91431891996d28cc0214faa3760a765b40846bd035888ac00ba1dd2050000004341049464205950188c29d377eebca6535e0f3699ce4069ecd77ffebfbd0bcf95e3c134cb7d2742d800a12df41413a09ef87a80516353a2f0a280547bb5512dc03da8ac00000000".into(); + let script_sig = tx.inputs[0].script_sig.clone().into(); + let expected_pub = Public::Normal("049464205950188c29d377eebca6535e0f3699ce4069ecd77ffebfbd0bcf95e3c134cb7d2742d800a12df41413a09ef87a80516353a2f0a280547bb5512dc03da8".into()); + let unsigned_tx: TransactionInputSigner = tx.into(); + let successful_verification = + verify_p2pk_input_pubkey(&script_sig, &expected_pub, &unsigned_tx, 0, SignatureVersion::Base, 0).unwrap(); + assert!(successful_verification); + + // 33-byte (compressed) pubkey example. + // https://kmdexplorer.io/tx/07ceb50f9eedc3b820e48dc1e5250f6625115afe4ace3089bfcc66b34f5d4344 + let tx: UtxoTx = "0400008085202f89013683897bf3bfb1e217663aa9591bd73c9eb105f8c8471e88dbe7152ca7627a19050000004948304502210087100bf4a665ebab3cc6d3472068905bdc6c6def37e432597e78e2ccc4da017a02205b5f0800cabe84bc49b5eb0997926b48dfee3b8ca5a31623ae9506272f8a5cd501ffffffff0288130000000000002321020e46e79a2a8d12b9b5d12c7a91adb4e454edfae43c0a0cb805427d2ac7613fd9ac0000000000000000226a20976bd7ad5596ac3521fd90295e753b1096e4eb90ad9ded1170b2ed81f810df5fc0dbf36752ea42000000000000000000000000".into(); + let script_sig = tx.inputs[0].script_sig.clone().into(); + let expected_pub = Public::Compressed("02f9a7b49282885cd03969f1f5478287497bc8edfceee9eac676053c107c5fcdaf".into()); + let mut unsigned_tx: TransactionInputSigner = tx.into(); + // For overwintered transactions, the amount must be set, as wel as the consensus branch id. + unsigned_tx.inputs[0].amount = 10000; + unsigned_tx.consensus_branch_id = 0x76b8_09bb; + let successful_verification = + verify_p2pk_input_pubkey(&script_sig, &expected_pub, &unsigned_tx, 0, SignatureVersion::Base, 0).unwrap(); + assert!(successful_verification); +} + +#[test] +fn test_check_all_utxo_inputs_signed_by_pub_overwintered() { + use super::utxo_tests::electrum_client_for_test; + use common::block_on; + + // We need a running electrum client for this test to test the functionality of fetching a tx from the network, parsing it, and using its input amount for sig_hash calculations. + let client = UtxoRpcClientEnum::Electrum(electrum_client_for_test(&[ + "electrum3.cipig.net:10001", + "electrum1.cipig.net:10001", + "electrum2.cipig.net:10001", + ])); + let mut fields = utxo_coin_fields_for_test(client, None, false); + fields.conf.ticker = "KMD".to_owned(); + let coin = utxo_coin_from_fields(fields); + + let tx: UtxoTx = "0400008085202f89013683897bf3bfb1e217663aa9591bd73c9eb105f8c8471e88dbe7152ca7627a19050000004948304502210087100bf4a665ebab3cc6d3472068905bdc6c6def37e432597e78e2ccc4da017a02205b5f0800cabe84bc49b5eb0997926b48dfee3b8ca5a31623ae9506272f8a5cd501ffffffff0288130000000000002321020e46e79a2a8d12b9b5d12c7a91adb4e454edfae43c0a0cb805427d2ac7613fd9ac0000000000000000226a20976bd7ad5596ac3521fd90295e753b1096e4eb90ad9ded1170b2ed81f810df5fc0dbf36752ea42000000000000000000000000".into(); + let expected_pub = Public::Compressed("02f9a7b49282885cd03969f1f5478287497bc8edfceee9eac676053c107c5fcdaf".into()); + assert!(block_on(check_all_utxo_inputs_signed_by_pub(&coin, &tx, &expected_pub)).unwrap()); +} + #[test] fn test_tx_v_size() { // Multiple legacy inputs with P2SH and P2PKH output diff --git a/mm2src/mm2_bitcoin/keys/src/public.rs b/mm2src/mm2_bitcoin/keys/src/public.rs index cb801585d8..c21eda0259 100644 --- a/mm2src/mm2_bitcoin/keys/src/public.rs +++ b/mm2src/mm2_bitcoin/keys/src/public.rs @@ -3,8 +3,8 @@ use crypto::dhash160; use hash::{H160, H264, H520}; use hex::ToHex; use secp256k1::{recovery::{RecoverableSignature, RecoveryId}, - Message as SecpMessage, PublicKey, Signature as SecpSignature}; -use std::{fmt, ops}; + Error as SecpError, Message as SecpMessage, PublicKey, Signature as SecpSignature}; +use std::{fmt, ops::Deref}; use {CompactSignature, Error, Message, Signature}; /// Secret public key @@ -80,9 +80,12 @@ impl Public { Public::Normal(_) => None, } } + + #[inline(always)] + pub fn to_secp256k1_pubkey(&self) -> Result { PublicKey::from_slice(self.deref()) } } -impl ops::Deref for Public { +impl Deref for Public { type Target = [u8]; fn deref(&self) -> &Self::Target { diff --git a/mm2src/mm2_bitcoin/script/src/script.rs b/mm2src/mm2_bitcoin/script/src/script.rs index c5e4b57d0f..8614961879 100644 --- a/mm2src/mm2_bitcoin/script/src/script.rs +++ b/mm2src/mm2_bitcoin/script/src/script.rs @@ -513,6 +513,30 @@ impl Script { script.sigops_count(true) } + + /// Extracts the signature from a scriptSig at instruction 0. + /// + /// Usable for P2PK and P2PKH scripts. + pub fn extract_signature(&self) -> Result, String> { + match self.get_instruction(0) { + Some(Ok(instruction)) => match instruction.opcode { + Opcode::OP_PUSHBYTES_70 | Opcode::OP_PUSHBYTES_71 | Opcode::OP_PUSHBYTES_72 => match instruction.data { + Some(bytes) => Ok(bytes.to_vec()), + None => Err(format!("No data at instruction 0 of script {:?}", self)), + }, + opcode => Err(format!("Unexpected opcode {:?}", opcode)), + }, + Some(Err(e)) => Err(format!("Error {} on getting instruction 0 of script {:?}", e, self)), + None => Err(format!("None instruction 0 of script {:?}", self)), + } + } + + /// Checks if a scriptSig is a script that spends a P2PK output. + pub fn does_script_spend_p2pk(&self) -> bool { + // P2PK scriptSig is just a single signature. The script should consist of a single push bytes + // instruction with the data as the signature. + self.extract_signature().is_ok() && self.get_instruction(1).is_none() + } } pub struct Instructions<'a> { @@ -971,4 +995,13 @@ OP_ADD } assert_eq!(max_idx, 3); } + + #[test] + fn test_does_script_spend_p2pk() { + let script_sig = Script::from("473044022071edae37cf518e98db3f7637b9073a7a980b957b0c7b871415dbb4898ec3ebdc022031b402a6b98e64ffdf752266449ca979a9f70144dba77ed7a6a25bfab11648f6012103ad6f89abc2e5beaa8a3ac28e22170659b3209fe2ddf439681b4b8f31508c36fa"); + assert!(!script_sig.does_script_spend_p2pk()); + // The scriptSig of the input spent from: https://mempool.space/tx/1db6251a9afce7025a2061a19e63c700dffc3bec368bd1883decfac353357a9d + let script_sig = Script::from("483045022078e86c021003cca23842d4b2862dfdb68d2478a98c08c10dcdffa060e55c72be022100f6a41da12cdc2e350045f4c97feeab76a7c0ab937bd8a9e507293ce6d37c9cc201"); + assert!(script_sig.does_script_spend_p2pk()); + } }