f! remove new ser version + allow downgrade in onchain tx handler

Author: wpaulino

lightning/src/chain/onchaintx.rs

@@ -36,7 +36,7 @@ use crate::chain::keysinterface::{Sign, KeysInterface};
 use crate::chain::package::PackageSolvingData;
 use crate::chain::package::PackageTemplate;
 use crate::util::logger::Logger;
-use crate::util::ser::{Readable, ReadableArgs, MaybeReadable, Writer, Writeable};
+use crate::util::ser::{Readable, ReadableArgs, MaybeReadable, Writer, Writeable, VecWriter};
 use crate::util::byte_utils;
 
 use crate::io;
@@ -250,7 +250,7 @@ pub struct OnchainTxHandler<ChannelSigner: Sign> {
 	pub(super) secp_ctx: Secp256k1<secp256k1::All>,
 }
 
-const SERIALIZATION_VERSION: u8 = 2;
+const SERIALIZATION_VERSION: u8 = 1;
 const MIN_SERIALIZATION_VERSION: u8 = 1;
 
 impl<ChannelSigner: Sign> OnchainTxHandler<ChannelSigner> {
@@ -265,6 +265,13 @@ impl<ChannelSigner: Sign> OnchainTxHandler<ChannelSigner> {
 
 		self.channel_transaction_parameters.write(writer)?;
 
+		let mut key_data = VecWriter(Vec::new());
+		self.signer.write(&mut key_data)?;
+		assert!(key_data.0.len() < core::usize::MAX);
+		assert!(key_data.0.len() < core::u32::MAX as usize);
+		(key_data.0.len() as u32).write(writer)?;
+		writer.write_all(&key_data.0[..])?;
+
 		writer.write_all(&byte_utils::be64_to_array(self.pending_claim_requests.len() as u64))?;
 		for (ref ancestor_claim_txid, request) in self.pending_claim_requests.iter() {
 			ancestor_claim_txid.write(writer)?;
@@ -303,7 +310,7 @@ impl<'a, K: KeysInterface> ReadableArgs<(&'a K, u64, [u8; 32])> for OnchainTxHan
 		let channel_value_satoshis = args.1;
 		let channel_keys_id = args.2;
 
-		let ver = read_ver_prefix!(reader, SERIALIZATION_VERSION);
+		let _ver = read_ver_prefix!(reader, SERIALIZATION_VERSION);
 
 		let destination_script = Readable::read(reader)?;
 
@@ -314,21 +321,19 @@ impl<'a, K: KeysInterface> ReadableArgs<(&'a K, u64, [u8; 32])> for OnchainTxHan
 
 		let channel_parameters = Readable::read(reader)?;
 
-		let mut signer: K::Signer;
-		if ver <= 1 {
-			let keys_len: u32 = Readable::read(reader)?;
-			let mut keys_data = Vec::with_capacity(cmp::min(keys_len as usize, MAX_ALLOC_SIZE));
-			while keys_data.len() != keys_len as usize {
-				// Read 1KB at a time to avoid accidentally allocating 4GB on corrupted channel keys
-				let mut data = [0; 1024];
-				let read_slice = &mut data[0..cmp::min(1024, keys_len as usize - keys_data.len())];
-				reader.read_exact(read_slice)?;
-				keys_data.extend_from_slice(read_slice);
-			}
-			signer = keys_manager.read_chan_signer(&keys_data)?;
-		} else {
-			signer = keys_manager.derive_channel_signer(channel_value_satoshis, channel_keys_id);
+		// Read the serialized signer bytes, but don't deserialize them, as we'll obtain our signer
+		// by re-deriving the private key material.
+		let keys_len: u32 = Readable::read(reader)?;
+		let mut keys_data = Vec::with_capacity(cmp::min(keys_len as usize, MAX_ALLOC_SIZE));
+		while keys_data.len() != keys_len as usize {
+			// Read 1KB at a time to avoid accidentally allocating 4GB on corrupted channel keys
+			let mut data = [0; 1024];
+			let read_slice = &mut data[0..cmp::min(1024, keys_len as usize - keys_data.len())];
+			reader.read_exact(read_slice)?;
+			keys_data.extend_from_slice(read_slice);
 		}
+
+		let mut signer = keys_manager.derive_channel_signer(channel_value_satoshis, Some(channel_keys_id));
 		signer.ready_channel(&channel_parameters);
 
 		let pending_claim_requests_len: u64 = Readable::read(reader)?;