Slate: match a participant on both keys and validate the count

find_index_matching_context accepted an entry matching either our excess
or our nonce, while fill_round_2 requires both, so the two could disagree
on which entry is ours and signing could return success without adding a
signature. Both also indexed participant_data by position up to
num_participants, which a counterparty can send short.

Match on both keys and iterate the entries the slate carries. Require the
participant count to be exact before signing a contract, and check for two
participants in the contract tx log before pairing them with 'xor 1'.

libwallet/src/contract/slate.rs

@@ -197,6 +197,16 @@ where
 	K: Keychain,
 {
 	debug!("contract::slate::sign => called");
+	// The counterparty controls the participant list, so require it to hold exactly the
+	// participants the slate declares before we sign over it. Setup has already added our
+	// own entry by this point.
+	if slate.participant_data.len() != slate.num_participants as usize {
+		return Err(Error::GenericError(format!(
+			"Expected {} participant(s) before signing, found {}",
+			slate.num_participants,
+			slate.participant_data.len()
+		)));
+	}
 	let keychain = w.keychain(keychain_mask)?;
 	slate.fill_round_2(&keychain, &context.sec_key, &context.sec_nonce)?;
 	debug!(

libwallet/src/contract/utils.rs

@@ -97,8 +97,16 @@ where
 			)?;
 			let sender_address = OnionV3Address::from_private(&sender_key.0)?;
 
-			// We're looking for the OTHER party here, the recipient
+			// We're looking for the OTHER party here, the recipient. The 'xor 1' pairing
+			// only holds for a two-party slate, so check that before indexing: a slate
+			// with a shorter participant list would otherwise panic here.
 			let sender_index = slate.find_index_matching_context(&keychain, context)?;
+			if slate.participant_data.len() != 2 {
+				return Err(Error::GenericError(format!(
+					"Expected 2 participants for a payment proof, found {}",
+					slate.participant_data.len()
+				)));
+			}
 			let recipient_index = sender_index ^ 1;
 
 			tx_log_entry.payment_proof = Some(StoredProofInfo {

libwallet/src/slate.rs

@@ -420,14 +420,15 @@ impl Slate {
 	where
 		K: Keychain,
 	{
-		for i in 0..self.num_participants() as usize {
-			let calc_pub_excess = PublicKey::from_secret_key(keychain.secp(), &context.sec_key)?;
-			let calc_pub_nonce = PublicKey::from_secret_key(keychain.secp(), &context.sec_nonce)?;
-
+		let calc_pub_excess = PublicKey::from_secret_key(keychain.secp(), &context.sec_key)?;
+		let calc_pub_nonce = PublicKey::from_secret_key(keychain.secp(), &context.sec_nonce)?;
+		// Iterate over the entries the slate actually carries rather than indexing by
+		// position up to num_participants, which a counterparty can send short. Both keys
+		// have to match, as fill_round_2 requires, so an entry sharing only one of them is
+		// not mistaken for ours.
+		for (i, p) in self.participant_data.iter().enumerate() {
 			// find my entry
-			if self.participant_data[i].public_blind_excess == calc_pub_excess
-				|| self.participant_data[i].public_nonce == calc_pub_nonce
-			{
+			if p.public_blind_excess == calc_pub_excess && p.public_nonce == calc_pub_nonce {
 				return Ok(i);
 			}
 		}
@@ -458,12 +459,11 @@ impl Slate {
 		)?;
 		let pub_excess = PublicKey::from_secret_key(keychain.secp(), &sec_key)?;
 		let pub_nonce = PublicKey::from_secret_key(keychain.secp(), &sec_nonce)?;
-		for i in 0..self.num_participants() as usize {
+		// As in find_index_matching_context, iterate the entries we actually have
+		for p in self.participant_data.iter_mut() {
 			// find my entry
-			if self.participant_data[i].public_blind_excess == pub_excess
-				&& self.participant_data[i].public_nonce == pub_nonce
-			{
-				self.participant_data[i].part_sig = Some(sig_part);
+			if p.public_blind_excess == pub_excess && p.public_nonce == pub_nonce {
+				p.part_sig = Some(sig_part);
 				break;
 			}
 		}
@@ -1614,3 +1614,71 @@ impl From<OutputFeaturesV4> for OutputFeatures {
 		}
 	}
 }
+
+#[cfg(test)]
+mod test {
+	use super::*;
+	use crate::grin_keychain::ExtKeychain;
+
+	// A counterparty controls the participant list, so it can be shorter than
+	// num_participants claims, or hold an entry that matches only one of our two keys.
+	fn context_and_keys() -> (ExtKeychain, Context, PublicKey, PublicKey) {
+		let keychain = ExtKeychain::from_random_seed(true).unwrap();
+		let parent_key_id = ExtKeychain::derive_key_id(1, 0, 0, 0, 0);
+		let context = Context::new(keychain.secp(), &parent_key_id, true, true);
+		let our_excess = PublicKey::from_secret_key(keychain.secp(), &context.sec_key).unwrap();
+		let our_nonce = PublicKey::from_secret_key(keychain.secp(), &context.sec_nonce).unwrap();
+		(keychain, context, our_excess, our_nonce)
+	}
+
+	#[test]
+	fn find_index_handles_short_participant_list() {
+		let (keychain, context, _, _) = context_and_keys();
+		// A key that matches neither of ours, so the search runs off the end of the list
+		let other_id = ExtKeychain::derive_key_id(1, 1, 0, 0, 0);
+		let other_key = keychain
+			.derive_key(0, &other_id, SwitchCommitmentType::Regular)
+			.unwrap();
+		let other = PublicKey::from_secret_key(keychain.secp(), &other_key).unwrap();
+
+		let mut slate = Slate::blank(2, false);
+		// Claims two participants but carries one, and it is not ours
+		slate.num_participants = 2;
+		slate.participant_data.push(ParticipantData {
+			public_blind_excess: other,
+			public_nonce: other,
+			part_sig: None,
+		});
+		assert!(slate
+			.find_index_matching_context(&keychain, &context)
+			.is_err());
+	}
+
+	#[test]
+	fn find_index_requires_both_keys_to_match() {
+		let (keychain, context, our_excess, our_nonce) = context_and_keys();
+		let mut slate = Slate::blank(2, false);
+		// Shares our nonce but not our excess, so it is not our entry
+		slate.participant_data.push(ParticipantData {
+			public_blind_excess: our_nonce,
+			public_nonce: our_nonce,
+			part_sig: None,
+		});
+		assert!(slate
+			.find_index_matching_context(&keychain, &context)
+			.is_err());
+
+		// Both keys match, so this one is ours
+		slate.participant_data.push(ParticipantData {
+			public_blind_excess: our_excess,
+			public_nonce: our_nonce,
+			part_sig: None,
+		});
+		assert_eq!(
+			slate
+				.find_index_matching_context(&keychain, &context)
+				.unwrap(),
+			1
+		);
+	}
+}