From bf03063310b71f58b5694e207bdc928e976377d3 Mon Sep 17 00:00:00 2001 From: Orbital Date: Fri, 19 Jan 2024 19:00:12 -0600 Subject: [PATCH 1/3] multi: add verification details to invoice request Once we get an invoice returned from the offer creator, we'll be able to verify that the invoice is a response to the invoice request we sent. We can also use the PaymentId we set to verify we didn't already pay the invoice. --- src/lib.rs | 10 +++++++++- src/lndk_offers.rs | 18 +++++++++++++++--- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 4286b634..49f875a1 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -14,12 +14,14 @@ use bitcoin::network::constants::Network; use bitcoin::secp256k1::{Error as Secp256k1Error, PublicKey}; use home::home_dir; use lightning::blinded_path::BlindedPath; +use lightning::ln::inbound_payment::ExpandedKey; use lightning::ln::peer_handler::IgnoringMessageHandler; use lightning::offers::offer::Offer; use lightning::onion_message::{ DefaultMessageRouter, Destination, OffersMessage, OffersMessageHandler, OnionMessenger, PendingOnionMessage, }; +use lightning::sign::{EntropySource, KeyMaterial}; use log::{error, info, LevelFilter}; use log4rs::append::console::ConsoleAppender; use log4rs::append::file::FileAppender; @@ -183,6 +185,7 @@ pub struct OfferHandler { active_offers: Mutex>, pending_messages: Mutex>>, messenger_utils: MessengerUtilities, + expanded_key: ExpandedKey, } #[derive(Clone)] @@ -199,10 +202,15 @@ pub struct PayOfferParams { impl OfferHandler { pub fn new() -> Self { + let messenger_utils = MessengerUtilities::new(); + let random_bytes = messenger_utils.get_secure_random_bytes(); + let expanded_key = ExpandedKey::new(&KeyMaterial(random_bytes)); + OfferHandler { active_offers: Mutex::new(HashMap::new()), pending_messages: Mutex::new(Vec::new()), - messenger_utils: MessengerUtilities::new(), + messenger_utils, + expanded_key, } } diff --git a/src/lndk_offers.rs b/src/lndk_offers.rs index 3fa25eb3..34c73892 100644 --- a/src/lndk_offers.rs +++ b/src/lndk_offers.rs @@ -7,11 +7,13 @@ use bitcoin::secp256k1::schnorr::Signature; use bitcoin::secp256k1::{Error as Secp256k1Error, PublicKey, Secp256k1}; use futures::executor::block_on; use lightning::blinded_path::BlindedPath; +use lightning::ln::channelmanager::PaymentId; use lightning::offers::invoice_request::{InvoiceRequest, UnsignedInvoiceRequest}; use lightning::offers::merkle::SignError; use lightning::offers::offer::{Amount, Offer}; use lightning::offers::parse::{Bolt12ParseError, Bolt12SemanticError}; use lightning::onion_message::{Destination, OffersMessage, PendingOnionMessage}; +use lightning::sign::EntropySource; use log::error; use std::error::Error; use std::fmt::Display; @@ -151,7 +153,7 @@ impl OfferHandler { &self, mut signer: impl MessageSigner + std::marker::Send + 'static, offer: Offer, - metadata: Vec, + _metadata: Vec, network: Network, msats: u64, ) -> Result> { @@ -169,8 +171,19 @@ impl OfferHandler { let pubkey = PublicKey::from_slice(&pubkey_bytes).expect("failed to deserialize public key"); + // Generate a new payment id for this payment. + let bytes = self.messenger_utils.get_secure_random_bytes(); + // We need to add some metadata to the invoice request to help with verification of the invoice + // once returned from the offer maker. Once we get an invoice back, this metadata will help us + // to determine: 1) That the invoice is truly for the invoice request we sent. 2) We don't pay + // duplicate invoices. let unsigned_invoice_req = offer - .request_invoice(metadata, pubkey) + .request_invoice_deriving_metadata( + pubkey, + &self.expanded_key, + &self.messenger_utils, + PaymentId(bytes), + ) .unwrap() .chain(network) .unwrap() @@ -349,7 +362,6 @@ impl PeerConnector for Client { let list_req = ListPeersRequest { ..Default::default() }; - self.lightning() .list_peers(list_req) .await From a6c72c6508bff7d8a126522fa976a5b117471855 Mon Sep 17 00:00:00 2001 From: Orbital Date: Wed, 24 Jan 2024 15:00:23 -0600 Subject: [PATCH 2/3] multi: split off uir signing portion of create_invoice_request into a method We split the unsigned invoice request (uir) signing portion into another function to make it easier to mock. Earlier we had hardcoded a signature for our lndk_offers.rs unit tests. But now that we've changed the uir being signed (by adding verification metadata), the hardcoded signature is invalid. We refactor our mocking to get rid of this hardcoded signature data completely. --- src/lib.rs | 2 +- src/lnd.rs | 8 +++- src/lndk_offers.rs | 116 +++++++++++++++++++++++++++------------------ 3 files changed, 78 insertions(+), 48 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 49f875a1..e5cba4d5 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -184,7 +184,7 @@ enum OfferState { pub struct OfferHandler { active_offers: Mutex>, pending_messages: Mutex>>, - messenger_utils: MessengerUtilities, + pub messenger_utils: MessengerUtilities, expanded_key: ExpandedKey, } diff --git a/src/lnd.rs b/src/lnd.rs index 1eb6161b..5c198cae 100644 --- a/src/lnd.rs +++ b/src/lnd.rs @@ -1,3 +1,4 @@ +use crate::OfferError; use async_trait::async_trait; use bitcoin::bech32::u5; use bitcoin::hashes::sha256::Hash; @@ -8,7 +9,7 @@ use bitcoin::secp256k1::{self, PublicKey, Scalar, Secp256k1}; use futures::executor::block_on; use lightning::ln::msgs::UnsignedGossipMessage; use lightning::offers::invoice::UnsignedBolt12Invoice; -use lightning::offers::invoice_request::UnsignedInvoiceRequest; +use lightning::offers::invoice_request::{InvoiceRequest, UnsignedInvoiceRequest}; use lightning::sign::{KeyMaterial, NodeSigner, Recipient}; use std::cell::RefCell; use std::collections::HashMap; @@ -195,6 +196,11 @@ pub trait MessageSigner { merkle_hash: Hash, tag: String, ) -> Result, Status>; + fn sign_uir( + &mut self, + key_loc: KeyLocator, + unsigned_invoice_req: UnsignedInvoiceRequest, + ) -> Result>; } /// PeerConnector provides a layer of abstraction over the LND API for connecting to a peer. diff --git a/src/lndk_offers.rs b/src/lndk_offers.rs index 34c73892..bf091fc2 100644 --- a/src/lndk_offers.rs +++ b/src/lndk_offers.rs @@ -195,24 +195,9 @@ impl OfferHandler { // To create a valid invoice request, we also need to sign it. This is spawned in a blocking // task because we need to call block_on on sign_message so that sign_closure can be a // synchronous closure. - task::spawn_blocking(move || { - let sign_closure = |msg: &UnsignedInvoiceRequest| { - let tagged_hash = msg.as_ref(); - let tag = tagged_hash.tag().to_string(); - - let signature = - block_on(signer.sign_message(key_loc, tagged_hash.merkle_root(), tag)) - .map_err(|_| Secp256k1Error::InvalidSignature)?; - - Signature::from_slice(&signature) - }; - - unsigned_invoice_req - .sign(sign_closure) - .map_err(OfferError::SignError) - }) - .await - .unwrap() + task::spawn_blocking(move || signer.sign_uir(key_loc, unsigned_invoice_req)) + .await + .unwrap() } /// create_reply_path creates a blinded path to provide to the offer maker when requesting an @@ -428,12 +413,34 @@ impl MessageSigner for Client { let resp_inner = resp.into_inner(); Ok(resp_inner.signature) } + + fn sign_uir( + &mut self, + key_loc: KeyLocator, + unsigned_invoice_req: UnsignedInvoiceRequest, + ) -> Result> { + let sign_closure = |msg: &UnsignedInvoiceRequest| { + let tagged_hash = msg.as_ref(); + let tag = tagged_hash.tag().to_string(); + + let signature = block_on(self.sign_message(key_loc, tagged_hash.merkle_root(), tag)) + .map_err(|_| Secp256k1Error::InvalidSignature)?; + + Signature::from_slice(&signature) + }; + + unsigned_invoice_req + .sign(sign_closure) + .map_err(OfferError::SignError) + } } #[cfg(test)] mod tests { use super::*; - use bitcoin::secp256k1::{KeyPair, Secp256k1, SecretKey}; + use bitcoin::secp256k1::{Error as Secp256k1Error, KeyPair, Secp256k1, SecretKey}; + use core::convert::Infallible; + use lightning::offers::merkle::SignError; use lightning::offers::offer::{OfferBuilder, Quantity}; use mockall::mock; use std::collections::HashMap; @@ -464,8 +471,23 @@ mod tests { "0313ba7ccbd754c117962b9afab6c2870eb3ef43f364a9f6c43d0fabb4553776ba".to_string() } - fn get_signature() -> String { - "28b937976a29c15827433086440b36c2bec6ca5bd977557972dca8641cd59ffba50daafb8ee99a19c950976b46f47d9e7aa716652e5657dfc555b82eff467f18".to_string() + fn get_invoice_request(offer: Offer, amount: u64) -> InvoiceRequest { + let secp_ctx = Secp256k1::new(); + let keys = KeyPair::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()); + let pubkey = PublicKey::from(keys); + offer + .request_invoice(vec![42; 64], pubkey) + .unwrap() + .chain(Network::Regtest) + .unwrap() + .amount_msats(amount) + .unwrap() + .build() + .unwrap() + .sign::<_, Infallible>(|message| { + Ok(secp_ctx.sign_schnorr_no_aux_rand(message.as_ref().as_digest(), &keys)) + }) + .expect("failed verifying signature") } mock! { @@ -475,6 +497,7 @@ mod tests { impl MessageSigner for TestBolt12Signer { async fn derive_key(&mut self, key_loc: KeyLocator) -> Result, Status>; async fn sign_message(&mut self, key_loc: KeyLocator, merkle_hash: Hash, tag: String) -> Result, Status>; + fn sign_uir(&mut self, key_loc: KeyLocator, unsigned_invoice_req: UnsignedInvoiceRequest) -> Result>; } } @@ -494,25 +517,27 @@ mod tests { let mut signer_mock = MockTestBolt12Signer::new(); signer_mock.expect_derive_key().returning(|_| { - Ok(PublicKey::from_str(&get_pubkey()) - .unwrap() - .serialize() - .to_vec()) + let pubkey = PublicKey::from_str(&get_pubkey()).unwrap(); + Ok(pubkey.serialize().to_vec()) }); - signer_mock.expect_sign_message().returning(|_, _, _| { - Ok(Signature::from_str(&get_signature()) - .unwrap() - .as_ref() - .to_vec()) - }); + let offer = decode(get_offer()).unwrap(); + let offer_amount = offer.amount().unwrap(); + let amount = match offer_amount { + Amount::Bitcoin { amount_msats } => *amount_msats, + _ => panic!("unexpected amount type"), + }; + + signer_mock + .expect_sign_uir() + .returning(move |_, _| Ok(get_invoice_request(offer.clone(), amount))); let offer = decode(get_offer()).unwrap(); let handler = OfferHandler::new(); - assert!(handler - .create_invoice_request(signer_mock, offer, vec![], Network::Regtest, 10000) - .await - .is_ok()) + let resp = handler + .create_invoice_request(signer_mock, offer, vec![], Network::Regtest, amount) + .await; + assert!(resp.is_ok()) } #[tokio::test] @@ -523,17 +548,14 @@ mod tests { .expect_derive_key() .returning(|_| Err(Status::unknown("error testing"))); - signer_mock.expect_sign_message().returning(|_, _, _| { - Ok(Signature::from_str(&get_signature()) - .unwrap() - .as_ref() - .to_vec()) - }); + signer_mock + .expect_sign_uir() + .returning(move |_, _| Ok(get_invoice_request(decode(get_offer()).unwrap(), 10000))); let offer = decode(get_offer()).unwrap(); let handler = OfferHandler::new(); assert!(handler - .create_invoice_request(signer_mock, offer, vec![], Network::Regtest, 10000) + .create_invoice_request(signer_mock, offer, vec![], Network::Regtest, 10000,) .await .is_err()) } @@ -549,14 +571,16 @@ mod tests { .to_vec()) }); - signer_mock - .expect_sign_message() - .returning(|_, _, _| Err(Status::unknown("error testing"))); + signer_mock.expect_sign_uir().returning(move |_, _| { + Err(OfferError::SignError(SignError::Signing( + Secp256k1Error::InvalidSignature, + ))) + }); let offer = decode(get_offer()).unwrap(); let handler = OfferHandler::new(); assert!(handler - .create_invoice_request(signer_mock, offer, vec![], Network::Regtest, 10000) + .create_invoice_request(signer_mock, offer, vec![], Network::Regtest, 10000,) .await .is_err()) } From df53046443bab3c3c5d1c920e5b9d7fac8f01fe5 Mon Sep 17 00:00:00 2001 From: Orbital Date: Wed, 7 Feb 2024 19:50:58 -0600 Subject: [PATCH 3/3] offers: verify invoice upon return --- src/lib.rs | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index e5cba4d5..e7e253fb 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -11,11 +11,12 @@ use crate::lnd::{ use crate::lndk_offers::OfferError; use crate::onion_messenger::MessengerUtilities; use bitcoin::network::constants::Network; -use bitcoin::secp256k1::{Error as Secp256k1Error, PublicKey}; +use bitcoin::secp256k1::{Error as Secp256k1Error, PublicKey, Secp256k1}; use home::home_dir; use lightning::blinded_path::BlindedPath; use lightning::ln::inbound_payment::ExpandedKey; use lightning::ln::peer_handler::IgnoringMessageHandler; +use lightning::offers::invoice_error::InvoiceError; use lightning::offers::offer::Offer; use lightning::onion_message::{ DefaultMessageRouter, Destination, OffersMessage, OffersMessageHandler, OnionMessenger, @@ -238,7 +239,20 @@ impl OffersMessageHandler for OfferHandler { log::error!("Invoice request received, payment not yet supported."); None } - OffersMessage::Invoice(_invoice) => None, + OffersMessage::Invoice(invoice) => { + let secp_ctx = &Secp256k1::new(); + // We verify that this invoice is a response to the invoice request we just sent. + match invoice.verify(&self.expanded_key, secp_ctx) { + // TODO: Eventually when we allow for multiple payments in flight, we can use the + // returned payment id below to check if we already processed an invoice for + // this payment. Right now it's safe to let this be because we won't try to pay + // a second invoice (if it comes through). + Ok(_payment_id) => Some(OffersMessage::Invoice(invoice)), + Err(()) => Some(OffersMessage::InvoiceError(InvoiceError::from_string( + String::from("invoice verification failure"), + ))), + } + } OffersMessage::InvoiceError(_error) => None, } }