Authenticate use of offer blinded paths #3139
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -64,6 +64,7 @@ use crate::ln::wire::Encode; | |
| use crate::offers::invoice::{BlindedPayInfo, Bolt12Invoice, DEFAULT_RELATIVE_EXPIRY, DerivedSigningPubkey, ExplicitSigningPubkey, InvoiceBuilder, UnsignedBolt12Invoice}; | ||
| use crate::offers::invoice_error::InvoiceError; | ||
| use crate::offers::invoice_request::{DerivedPayerId, InvoiceRequestBuilder}; | ||
| use crate::offers::nonce::Nonce; | ||
| use crate::offers::offer::{Offer, OfferBuilder}; | ||
| use crate::offers::parse::Bolt12SemanticError; | ||
| use crate::offers::refund::{Refund, RefundBuilder}; | ||
|
|
@@ -2254,7 +2255,10 @@ where | |
| event_persist_notifier: Notifier, | ||
| needs_persist_flag: AtomicBool, | ||
|
|
||
| #[cfg(not(any(test, feature = "_test_utils")))] | ||
| pending_offers_messages: Mutex<Vec<PendingOnionMessage<OffersMessage>>>, | ||
| #[cfg(any(test, feature = "_test_utils"))] | ||
| pub(crate) pending_offers_messages: Mutex<Vec<PendingOnionMessage<OffersMessage>>>, | ||
|
|
||
| /// Tracks the message events that are to be broadcasted when we are connected to some peer. | ||
| pending_broadcast_messages: Mutex<Vec<MessageSendEvent>>, | ||
|
|
@@ -4199,15 +4203,35 @@ where | |
| /// whether or not the payment was successful. | ||
| /// | ||
| /// [timer tick]: Self::timer_tick_occurred | ||
| pub fn send_payment_for_bolt12_invoice( | ||
| &self, invoice: &Bolt12Invoice, context: &OffersContext, | ||
| ) -> Result<(), Bolt12PaymentError> { | ||
| match self.verify_bolt12_invoice(invoice, context) { | ||
| Ok(payment_id) => self.send_payment_for_verified_bolt12_invoice(invoice, payment_id), | ||
| Err(()) => Err(Bolt12PaymentError::UnexpectedInvoice), | ||
| } | ||
| } | ||
|
|
||
| fn verify_bolt12_invoice( | ||
| &self, invoice: &Bolt12Invoice, context: &OffersContext, | ||
| ) -> Result<PaymentId, ()> { | ||
| let secp_ctx = &self.secp_ctx; | ||
| let expanded_key = &self.inbound_payment_key; | ||
|
|
||
| match context { | ||
| OffersContext::Unknown {} if invoice.is_for_refund_without_paths() => { | ||
|
There was a problem hiding this comment. Are we planning to also stop using There was a problem hiding this comment.
Yeah, looks like I need to update that to use
This code here will stay the same since the context is from the refund's blinded path -- or There was a problem hiding this comment. Ah okay. To me it reads like using an There was a problem hiding this comment. Yeah, I think we wanted to avoid having an There was a problem hiding this comment. But if we're avoiding an There was a problem hiding this comment. Right, I'm in agreement that once we remove the last use of There was a problem hiding this comment. Ah okay, thanks for that clarification. |
||
| invoice.verify_using_metadata(expanded_key, secp_ctx) | ||
| }, | ||
| OffersContext::OutboundPayment { payment_id, nonce } => { | ||
| invoice | ||
| .verify_using_payer_data(*payment_id, *nonce, expanded_key, secp_ctx) | ||
| .then(|| *payment_id) | ||
| .ok_or(()) | ||
| }, | ||
| _ => Err(()), | ||
| } | ||
| } | ||
|
|
||
| fn send_payment_for_verified_bolt12_invoice(&self, invoice: &Bolt12Invoice, payment_id: PaymentId) -> Result<(), Bolt12PaymentError> { | ||
| let best_block_height = self.best_block.read().unwrap().height; | ||
| let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self); | ||
|
|
@@ -8784,13 +8808,12 @@ macro_rules! create_offer_builder { ($self: ident, $builder: ty) => { | |
| let entropy = &*$self.entropy_source; | ||
| let secp_ctx = &$self.secp_ctx; | ||
|
|
||
| let nonce = Nonce::from_entropy_source(entropy); | ||
| let context = OffersContext::InvoiceRequest { nonce }; | ||
| let path = $self.create_blinded_paths_using_absolute_expiry(context, absolute_expiry) | ||
| .and_then(|paths| paths.into_iter().next().ok_or(())) | ||
| .map_err(|_| Bolt12SemanticError::MissingPaths)?; | ||
| let builder = OfferBuilder::deriving_signing_pubkey(node_id, expanded_key, nonce, secp_ctx) | ||
| .chain_hash($self.chain_hash) | ||
| .path(path); | ||
|
|
||
|
|
@@ -8858,13 +8881,14 @@ macro_rules! create_refund_builder { ($self: ident, $builder: ty) => { | |
| let entropy = &*$self.entropy_source; | ||
| let secp_ctx = &$self.secp_ctx; | ||
|
|
||
| let nonce = Nonce::from_entropy_source(entropy); | ||
| let context = OffersContext::OutboundPayment { payment_id, nonce }; | ||
| let path = $self.create_blinded_paths_using_absolute_expiry(context, Some(absolute_expiry)) | ||
| .and_then(|paths| paths.into_iter().next().ok_or(())) | ||
| .map_err(|_| Bolt12SemanticError::MissingPaths)?; | ||
|
|
||
| let builder = RefundBuilder::deriving_payer_id( | ||
| node_id, expanded_key, nonce, secp_ctx, amount_msats, payment_id | ||
| )? | ||
| .chain_hash($self.chain_hash) | ||
| .absolute_expiry(absolute_expiry) | ||
|
|
@@ -8973,8 +8997,9 @@ where | |
| let entropy = &*self.entropy_source; | ||
| let secp_ctx = &self.secp_ctx; | ||
|
|
||
| let nonce = Nonce::from_entropy_source(entropy); | ||
| let builder: InvoiceRequestBuilder<DerivedPayerId, secp256k1::All> = offer | ||
| .request_invoice_deriving_payer_id(expanded_key, nonce, secp_ctx, payment_id)? | ||
| .into(); | ||
| let builder = builder.chain_hash(self.chain_hash)?; | ||
|
|
||
|
|
@@ -8992,8 +9017,9 @@ where | |
| }; | ||
| let invoice_request = builder.build_and_sign()?; | ||
|
|
||
| let context = OffersContext::OutboundPayment { payment_id, nonce }; | ||
| let reply_paths = self.create_blinded_paths(context) | ||
| .map_err(|_| Bolt12SemanticError::MissingPaths)?; | ||
|
|
||
| let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self); | ||
|
|
||
|
|
@@ -10692,7 +10718,7 @@ where | |
|
|
||
| let abandon_if_payment = |context| { | ||
| match context { | ||
| OffersContext::OutboundPayment { payment_id, .. } => self.abandon_payment(payment_id), | ||
| _ => {}, | ||
| } | ||
| }; | ||
|
|
@@ -10703,19 +10729,32 @@ where | |
| Some(responder) => responder, | ||
| None => return ResponseInstruction::NoResponse, | ||
| }; | ||
|
|
||
| let nonce = match context { | ||
| OffersContext::Unknown {} if invoice_request.metadata().is_some() => None, | ||
| OffersContext::InvoiceRequest { nonce } => Some(nonce), | ||
| _ => return ResponseInstruction::NoResponse, | ||
| }; | ||
|
|
||
| let invoice_request = match nonce { | ||
valentinewallace marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| Some(nonce) => match invoice_request.verify_using_recipient_data( | ||
| nonce, expanded_key, secp_ctx, | ||
| ) { | ||
| Ok(invoice_request) => invoice_request, | ||
| Err(()) => return ResponseInstruction::NoResponse, | ||
| }, | ||
| None => match invoice_request.verify_using_metadata(expanded_key, secp_ctx) { | ||
| Ok(invoice_request) => invoice_request, | ||
| Err(()) => return ResponseInstruction::NoResponse, | ||
|
There was a problem hiding this comment. FWIW I don't think there's anything wrong with ignoring the BOLTs here - as long as we handle "unsolicited invoice_request that is for an offer we didnt built" and "invoice_request that we received over the wrong path" the same we're fine. Not sure it matters much if we think this case isn't going to be a common one that we need to worry about making sure we get good errors back, but if we think it'd be helpful we should just do it. There was a problem hiding this comment. Currently, this will only be triggered for older offers. When we add support for producing offers without blinded paths (i.e. with a public node id), it will also be hit. So at that point someone could de-anonymize the older offer by sending an invoice request for it to the node id given in the newer offer. There was a problem hiding this comment. Hmm? Not sure I understand the scenario you're describing - are you saying if we issue two offers, one with a blinded path and one that publishes our node, someone can use the one with the blinded path and the one that publishes our node somehow to get different behavior? There was a problem hiding this comment. Ah, no, I was saying they could determine that they came from the same issuer. Though, that is actually a different problem than what you are asking about and one we probably can't solve. Sorry, think I was confused about your comment. Were you saying we should continue to send an There was a problem hiding this comment. I think so? Like, if we think it doesn't matter if we pass back an error then no need, but if we think we may hit those cases and we should pass back an error then we should ignore the BOLTs here. There was a problem hiding this comment. Yeah, probably not useful to send an |
||
| }, | ||
| }; | ||
|
|
||
| let amount_msats = match InvoiceBuilder::<DerivedSigningPubkey>::amount_msats( | ||
| &invoice_request.inner | ||
| ) { | ||
| Ok(amount_msats) => amount_msats, | ||
| Err(error) => return responder.respond(OffersMessage::InvoiceError(error.into())), | ||
| }; | ||
|
|
||
| let relative_expiry = DEFAULT_RELATIVE_EXPIRY.as_secs() as u32; | ||
| let (payment_hash, payment_secret) = match self.create_inbound_payment( | ||
|
|
@@ -10788,24 +10827,28 @@ where | |
| } | ||
| }, | ||
| OffersMessage::Invoice(invoice) => { | ||
| let payment_id = match self.verify_bolt12_invoice(&invoice, &context) { | ||
| Ok(payment_id) => payment_id, | ||
| Err(()) => return ResponseInstruction::NoResponse, | ||
| }; | ||
|
|
||
| let result = { | ||
| let features = self.bolt12_invoice_features(); | ||
| if invoice.invoice_features().requires_unknown_bits_from(&features) { | ||
| Err(InvoiceError::from(Bolt12SemanticError::UnknownRequiredFeatures)) | ||
| } else if self.default_configuration.manually_handle_bolt12_invoices { | ||
| let event = Event::InvoiceReceived { | ||
| payment_id, invoice, context, responder, | ||
| }; | ||
| self.pending_events.lock().unwrap().push_back((event, None)); | ||
| return ResponseInstruction::NoResponse; | ||
| } else { | ||
| self.send_payment_for_verified_bolt12_invoice(&invoice, payment_id) | ||
| .map_err(|e| { | ||
| log_trace!(self.logger, "Failed paying invoice: {:?}", e); | ||
| InvoiceError::from_string(format!("{:?}", e)) | ||
| }) | ||
| } | ||
| }; | ||
|
|
||
| match result { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.