Merge pull request #1272 from lightning-signer/2022-01-sign-invoice-api

Improve KeysInterface::sign_invoice API

Author: valentinewallace

fuzz/src/chanmon_consistency.rs

@@ -63,6 +63,7 @@ use std::collections::{HashSet, hash_map, HashMap};
 use std::sync::{Arc,Mutex};
 use std::sync::atomic;
 use std::io::Cursor;
+use bitcoin::bech32::u5;
 
 const MAX_FEE: u32 = 10_000;
 struct FuzzEstimator {
@@ -220,7 +221,7 @@ impl KeysInterface for KeyProvider {
 		})
 	}
 
-	fn sign_invoice(&self, _invoice_preimage: Vec<u8>) -> Result<RecoverableSignature, ()> {
+	fn sign_invoice(&self, _hrp_bytes: &[u8], _invoice_data: &[u5]) -> Result<RecoverableSignature, ()> {
 		unreachable!()
 	}
 }

fuzz/src/full_stack.rs

@@ -60,6 +60,7 @@ use std::convert::TryInto;
 use std::cmp;
 use std::sync::{Arc, Mutex};
 use std::sync::atomic::{AtomicU64,AtomicUsize,Ordering};
+use bitcoin::bech32::u5;
 
 #[inline]
 pub fn slice_to_be16(v: &[u8]) -> u16 {
@@ -333,7 +334,7 @@ impl KeysInterface for KeyProvider {
 		))
 	}
 
-	fn sign_invoice(&self, _invoice_preimage: Vec<u8>) -> Result<RecoverableSignature, ()> {
+	fn sign_invoice(&self, _hrp_bytes: &[u8], _invoice_data: &[u5]) -> Result<RecoverableSignature, ()> {
 		unreachable!()
 	}
 }

lightning-invoice/src/lib.rs

@@ -43,6 +43,7 @@ use lightning::ln::features::InvoiceFeatures;
 #[cfg(any(doc, test))]
 use lightning::routing::network_graph::RoutingFees;
 use lightning::routing::router::RouteHint;
+use lightning::util::invoice::construct_invoice_preimage;
 
 use secp256k1::key::PublicKey;
 use secp256k1::{Message, Secp256k1};
@@ -869,32 +870,9 @@ macro_rules! find_all_extract {
 
 #[allow(missing_docs)]
 impl RawInvoice {
-	/// Construct the invoice's HRP and signatureless data into a preimage to be hashed.
-	pub(crate) fn construct_invoice_preimage(hrp_bytes: &[u8], data_without_signature: &[u5]) -> Vec<u8> {
-		use bech32::FromBase32;
-
-		let mut 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());
-			}
-		}
-
-		preimage.extend_from_slice(&Vec::<u8>::from_base32(&data_part)
-			.expect("No padding error may occur due to appended zero above."));
-		preimage
-	}
-
 	/// Hash the HRP as bytes and signatureless data part.
 	fn hash_from_parts(hrp_bytes: &[u8], data_without_signature: &[u5]) -> [u8; 32] {
-		let preimage = RawInvoice::construct_invoice_preimage(hrp_bytes, data_without_signature);
+		let preimage = construct_invoice_preimage(hrp_bytes, data_without_signature);
 		let mut hash: [u8; 32] = Default::default();
 		hash.copy_from_slice(&sha256::Hash::hash(&preimage)[..]);
 		hash

lightning-invoice/src/utils.rs

@@ -1,6 +1,6 @@
 //! Convenient utilities to create an invoice.
 
-use {CreationError, Currency, DEFAULT_EXPIRY_TIME, Invoice, InvoiceBuilder, SignOrCreationError, RawInvoice};
+use {CreationError, Currency, DEFAULT_EXPIRY_TIME, Invoice, InvoiceBuilder, SignOrCreationError};
 use payment::{Payer, Router};
 
 use bech32::ToBase32;
@@ -118,8 +118,7 @@ where
 	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 invoice_preimage = RawInvoice::construct_invoice_preimage(hrp_bytes, &data_without_signature);
-	let signed_raw_invoice = raw_invoice.sign(|_| keys_manager.sign_invoice(invoice_preimage));
+	let signed_raw_invoice = raw_invoice.sign(|_| keys_manager.sign_invoice(hrp_bytes, &data_without_signature));
 	match signed_raw_invoice {
 		Ok(inv) => Ok(Invoice::from_signed(inv).unwrap()),
 		Err(e) => Err(SignOrCreationError::SignError(e))

lightning/src/chain/keysinterface.rs

@@ -18,6 +18,7 @@ use bitcoin::network::constants::Network;
 use bitcoin::util::bip32::{ExtendedPrivKey, ExtendedPubKey, ChildNumber};
 use bitcoin::util::bip143;
 
+use bitcoin::bech32::u5;
 use bitcoin::hashes::{Hash, HashEngine};
 use bitcoin::hashes::sha256::HashEngine as Sha256State;
 use bitcoin::hashes::sha256::Hash as Sha256;
@@ -42,6 +43,7 @@ use prelude::*;
 use core::sync::atomic::{AtomicUsize, Ordering};
 use io::{self, Error};
 use ln::msgs::{DecodeError, MAX_VALUE_MSAT};
+use util::invoice::construct_invoice_preimage;
 
 /// Used as initial key material, to be expanded into multiple secret keys (but not to be used
 /// directly). This is used within LDK to encrypt/decrypt inbound payment data.
@@ -398,11 +400,12 @@ pub trait KeysInterface {
 	/// 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
+	/// Sign an invoice.
+	/// By parameterizing by the raw invoice bytes 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, ()>;
+	/// The hrp is ascii bytes, while the invoice data is base32.
+	fn sign_invoice(&self, hrp_bytes: &[u8], invoice_data: &[u5]) -> Result<RecoverableSignature, ()>;
 
 	/// Get secret key material as bytes for use in encrypting and decrypting inbound payment data.
 	///
@@ -1105,8 +1108,9 @@ impl KeysInterface for KeysManager {
 		InMemorySigner::read(&mut io::Cursor::new(reader))
 	}
 
-	fn sign_invoice(&self, invoice_preimage: Vec<u8>) -> Result<RecoverableSignature, ()> {
-		Ok(self.secp_ctx.sign_recoverable(&hash_to_message!(&Sha256::hash(&invoice_preimage)), &self.get_node_secret()))
+	fn sign_invoice(&self, hrp_bytes: &[u8], invoice_data: &[u5]) -> Result<RecoverableSignature, ()> {
+		let preimage = construct_invoice_preimage(&hrp_bytes, &invoice_data);
+		Ok(self.secp_ctx.sign_recoverable(&hash_to_message!(&Sha256::hash(&preimage)), &self.get_node_secret()))
 	}
 }
 

lightning/src/ln/channel.rs

@@ -5840,6 +5840,7 @@ mod tests {
 	use bitcoin::hashes::Hash;
 	use bitcoin::hash_types::{Txid, WPubkeyHash};
 	use core::num::NonZeroU8;
+	use bitcoin::bech32::u5;
 	use sync::Arc;
 	use prelude::*;
 
@@ -5884,7 +5885,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, ()> { panic!(); }
+		fn sign_invoice(&self, _hrp_bytes: &[u8], _invoice_data: &[u5]) -> Result<RecoverableSignature, ()> { panic!(); }
 	}
 
 	fn public_from_secret_hex(secp_ctx: &Secp256k1<All>, hex: &str) -> PublicKey {

lightning/src/util/invoice.rs

@@ -0,0 +1,26 @@
+//! Low level invoice utilities.
+
+use bitcoin::bech32::{u5, FromBase32};
+use prelude::*;
+
+/// Construct the invoice's HRP and signatureless data into a preimage to be hashed.
+pub fn construct_invoice_preimage(hrp_bytes: &[u8], data_without_signature: &[u5]) -> Vec<u8> {
+	let mut 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());
+		}
+	}
+
+	preimage.extend_from_slice(&Vec::<u8>::from_base32(&data_part)
+		.expect("No padding error may occur due to appended zero above."));
+	preimage
+}
+

lightning/src/util/mod.rs

@@ -19,6 +19,7 @@ pub mod events;
 pub mod errors;
 pub mod ser;
 pub mod message_signing;
+pub mod invoice;
 
 pub(crate) mod atomic_counter;
 pub(crate) mod byte_utils;

lightning/src/util/test_utils.rs

@@ -47,6 +47,7 @@ use core::time::Duration;
 use sync::{Mutex, Arc};
 use core::sync::atomic::{AtomicBool, AtomicUsize, Ordering};
 use core::{cmp, mem};
+use bitcoin::bech32::u5;
 use chain::keysinterface::{InMemorySigner, KeyMaterial};
 
 pub struct TestVecWriter(pub Vec<u8>);
@@ -87,7 +88,7 @@ impl keysinterface::KeysInterface for OnlyReadsKeysInterface {
 			false
 		))
 	}
-	fn sign_invoice(&self, _invoice_preimage: Vec<u8>) -> Result<RecoverableSignature, ()> { unreachable!(); }
+	fn sign_invoice(&self, _hrp_bytes: &[u8], _invoice_data: &[u5]) -> Result<RecoverableSignature, ()> { unreachable!(); }
 }
 
 pub struct TestChainMonitor<'a> {
@@ -528,8 +529,8 @@ impl keysinterface::KeysInterface for TestKeysInterface {
 		))
 	}
 
-	fn sign_invoice(&self, invoice_preimage: Vec<u8>) -> Result<RecoverableSignature, ()> {
-		self.backing.sign_invoice(invoice_preimage)
+	fn sign_invoice(&self, hrp_bytes: &[u8], invoice_data: &[u5]) -> Result<RecoverableSignature, ()> {
+		self.backing.sign_invoice(hrp_bytes, invoice_data)
 	}
 }