Move invoice signing behind KeysInterface

Author: valentinewallace

fuzz/src/chanmon_consistency.rs

@@ -56,6 +56,7 @@ use utils::test_logger;
 use utils::test_persister::TestPersister;
 
 use bitcoin::secp256k1::key::{PublicKey,SecretKey};
+use bitcoin::secp256k1::recovery::RecoverableSignature;
 use bitcoin::secp256k1::Secp256k1;
 
 use std::mem;
@@ -206,6 +207,10 @@ impl KeysInterface for KeyProvider {
 			disable_revocation_policy_check: false,
 		})
 	}
+
+	fn sign_invoice(&self, _invoice_preimage: Vec<u8>) -> Result<RecoverableSignature, DecodeError> {
+		unreachable!()
+	}
 }
 
 impl KeyProvider {

fuzz/src/full_stack.rs

@@ -48,6 +48,7 @@ use utils::test_logger;
 use utils::test_persister::TestPersister;
 
 use bitcoin::secp256k1::key::{PublicKey, SecretKey};
+use bitcoin::secp256k1::recovery::RecoverableSignature;
 use bitcoin::secp256k1::Secp256k1;
 
 use std::cell::RefCell;
@@ -460,6 +461,10 @@ impl KeysInterface for KeyProvider {
     fn read_chan_signer(&self, data: &[u8]) -> Result<EnforcingSigner, DecodeError> {
         EnforcingSigner::read(&mut std::io::Cursor::new(data))
     }
+
+    fn sign_invoice(&self, _invoice_preimage: Vec<u8>) -> Result<RecoverableSignature, DecodeError> {
+        unreachable!()
+    }
 }
 
 #[inline]

lightning-invoice/src/lib.rs

@@ -1291,7 +1291,8 @@ impl<S> Display for SignOrCreationError<S> {
 }
 
 mod utils {
-	use {CreationError, Currency, Invoice, InvoiceBuilder};
+	use {Currency, Invoice, InvoiceBuilder, SemanticError, SignedRawInvoice};
+	use bech32::{FromBase32, ToBase32, u5};
 	use bitcoin_hashes::Hash;
 	use lightning::chain;
 	use lightning::chain::chaininterface::{BroadcasterInterface, FeeEstimator};
@@ -1301,15 +1302,14 @@ mod utils {
 	use lightning::routing::network_graph::RoutingFees;
 	use lightning::routing::router::RouteHintHop;
 	use lightning::util::logger::Logger;
-	use secp256k1::Secp256k1;
 	use std::ops::Deref;
 
 	/// Utility to construct an invoice.
 	#[allow(dead_code)]
 	pub fn from_channelmanager<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>(
 		channelmanager: &ChannelManager<Signer, M, T, K, F, L>, amt_msat: Option<u64>, description: String,
-		network: Currency,
-	) -> Result<Invoice, CreationError>
+		network: Currency, keys_manager: &dyn KeysInterface<Signer = Signer>
+	) -> Result<Invoice, SemanticError>
 	where
 		M::Target: chain::Watch<Signer>,
 		T::Target: BroadcasterInterface,
@@ -1347,7 +1347,6 @@ mod utils {
 			7200, // default invoice expiry is 2 hours
 			0,
 		);
-		let secp_ctx = Secp256k1::new();
 		let our_node_pubkey = channelmanager.get_our_node_id();
 		let mut invoice = InvoiceBuilder::new(network)
 		.description(description)
@@ -1364,9 +1363,32 @@ mod utils {
 			invoice = invoice.route(hint);
 		}
 
-		invoice.build_signed(|msg_hash| {
-			secp_ctx.sign_recoverable(msg_hash, &channelmanager.get_our_node_secret())
-		})
+		let raw_invoice = invoice.build_raw().unwrap();
+		let hrp_str = raw_invoice.hrp.to_string();
+		let hrp_bytes = hrp_str.as_bytes();
+		let data_without_signature = raw_invoice.data.to_base32();
+		let mut invoice_preimage = Vec::<u8>::from(hrp_bytes);
+
+		let mut data_part = Vec::from(data_without_signature);
+		let overhang = (data_part.len() * 5) % 8;
+		if overhang > 0 {
+			// add padding if data does not end at a byte boundary
+			data_part.push(u5::try_from_u8(0).unwrap());
+
+			// if overhang is in (1..3) we need to add u5(0) padding two times
+			if overhang < 3 {
+				data_part.push(u5::try_from_u8(0).unwrap());
+			}
+		}
+
+		invoice_preimage.extend_from_slice(&Vec::<u8>::from_base32(&data_part)
+		  .expect("No padding error may occur due to appended zero above."));
+		let signature = match keys_manager.sign_invoice(invoice_preimage) {
+			Ok(sig) => sig,
+			Err(_) => return Err(SemanticError::InvalidSignature)
+		};
+		let signed_raw_invoice: Result<SignedRawInvoice, SemanticError> = raw_invoice.sign(|_| Ok(signature));
+		Invoice::from_signed(signed_raw_invoice.unwrap())
 	}
 
 }
@@ -1696,7 +1718,7 @@ mod test {
 		let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
 		let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
 		let _chan = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
-		let invoice = ::utils::from_channelmanager(&nodes[1].node, Some(10_000), "test".to_string(), Currency::BitcoinTestnet).unwrap();
+		let invoice = ::utils::from_channelmanager(&nodes[1].node, Some(10_000), "test".to_string(), Currency::BitcoinTestnet, nodes[1].keys_manager).unwrap();
 		assert_eq!(invoice.amount_pico_btc(), Some(100_000));
 		assert_eq!(invoice.min_final_cltv_expiry(), Some(&9));
 		assert_eq!(invoice.description(), InvoiceDescription::Direct(&Description("test".to_string())));

lightning/src/chain/keysinterface.rs

@@ -25,7 +25,8 @@ use bitcoin::hashes::sha256d::Hash as Sha256dHash;
 use bitcoin::hash_types::WPubkeyHash;
 
 use bitcoin::secp256k1::key::{SecretKey, PublicKey};
-use bitcoin::secp256k1::{Secp256k1, Signature, Signing};
+use bitcoin::secp256k1::{Message, Secp256k1, Signature, Signing};
+use bitcoin::secp256k1::recovery::RecoverableSignature;
 use bitcoin::secp256k1;
 
 use util::{byte_utils, transaction_utils};
@@ -391,6 +392,12 @@ pub trait KeysInterface: Send + Sync {
 	/// contain no versioning scheme. You may wish to include your own version prefix and ensure
 	/// you've read all of the provided bytes to ensure no corruption occurred.
 	fn read_chan_signer(&self, reader: &[u8]) -> Result<Self::Signer, DecodeError>;
+
+	/// Sign an invoice's preimage (note that this is the preimage of the invoice, not the HTLC's
+	/// preimage). By parameterizing by the preimage instead of the hash, we allow implementors of
+	/// this trait to parse the invoice and make sure they're signing what they expect, rather than
+	/// blindly signing the hash.
+	fn sign_invoice(&self, invoice_preimage: Vec<u8>) -> Result<RecoverableSignature, DecodeError>;
 }
 
 #[derive(Clone)]
@@ -1047,6 +1054,15 @@ impl KeysInterface for KeysManager {
 	fn read_chan_signer(&self, reader: &[u8]) -> Result<Self::Signer, DecodeError> {
 		InMemorySigner::read(&mut std::io::Cursor::new(reader))
 	}
+
+	fn sign_invoice(&self, invoice_preimage: Vec<u8>) -> Result<RecoverableSignature, DecodeError> {
+		let secp_ctx = Secp256k1::new();
+		let mut raw_hash: [u8; 32] = Default::default();
+		raw_hash.copy_from_slice(&Sha256::hash(&invoice_preimage)[..]);
+		let msg_hash = Message::from_slice(&raw_hash[..])
+			.expect("Hash is 32 bytes long, same as MESSAGE_SIZE");
+		Ok(secp_ctx.sign_recoverable(&msg_hash, &self.get_node_secret()))
+	}
 }
 
 // Ensure that BaseSign can have a vtable

lightning/src/ln/channel.rs

@@ -4843,6 +4843,7 @@ mod tests {
 	use bitcoin::secp256k1::{Secp256k1, Message, Signature, All};
 	use bitcoin::secp256k1::ffi::Signature as FFISignature;
 	use bitcoin::secp256k1::key::{SecretKey,PublicKey};
+	use bitcoin::secp256k1::recovery::RecoverableSignature;
 	use bitcoin::hashes::sha256::Hash as Sha256;
 	use bitcoin::hashes::Hash;
 	use bitcoin::hash_types::{Txid, WPubkeyHash};
@@ -4888,6 +4889,7 @@ mod tests {
 		}
 		fn get_secure_random_bytes(&self) -> [u8; 32] { [0; 32] }
 		fn read_chan_signer(&self, _data: &[u8]) -> Result<Self::Signer, DecodeError> { panic!(); }
+		fn sign_invoice(&self, _invoice_preimage: Vec<u8>) -> Result<RecoverableSignature, DecodeError> { panic!(); }
 	}
 
 	fn public_from_secret_hex(secp_ctx: &Secp256k1<All>, hex: &str) -> PublicKey {

lightning/src/util/test_utils.rs

@@ -32,6 +32,7 @@ use bitcoin::network::constants::Network;
 use bitcoin::hash_types::{BlockHash, Txid};
 
 use bitcoin::secp256k1::{SecretKey, PublicKey, Secp256k1, Signature};
+use bitcoin::secp256k1::recovery::RecoverableSignature;
 
 use regex;
 
@@ -75,6 +76,7 @@ impl keysinterface::KeysInterface for OnlyReadsKeysInterface {
 	fn read_chan_signer(&self, reader: &[u8]) -> Result<Self::Signer, msgs::DecodeError> {
 		EnforcingSigner::read(&mut std::io::Cursor::new(reader))
 	}
+	fn sign_invoice(&self, _invoice_preimage: Vec<u8>) -> Result<RecoverableSignature, msgs::DecodeError> { unreachable!(); }
 }
 
 pub struct TestChainMonitor<'a> {
@@ -483,6 +485,10 @@ impl keysinterface::KeysInterface for TestKeysInterface {
 			disable_revocation_policy_check: self.disable_revocation_policy_check,
 		})
 	}
+
+	fn sign_invoice(&self, invoice_preimage: Vec<u8>) -> Result<RecoverableSignature, msgs::DecodeError> {
+		self.backing.sign_invoice(invoice_preimage)
+	}
 }