From df5d7ea7b3aeb9e474f5d8d98f87ca6720b4eaab Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Mon, 15 Jul 2024 18:29:41 -0500 Subject: [PATCH] Elide nonce from payer metadata InvoiceRequest and Refund have payer_metadata which consists of an encrypted payment id and a nonce used to derive its signing keys and authenticate any corresponding invoices. Now that the blinded paths include this data in their OffersContext, remove the nonce as it is now redundant. Keep the encrypted payment id as some data is needed in the payer metadata according to the spec. This saves space and prevents de-anonymization attacks as along as the nonce isn't revealed. --- lightning/src/ln/channelmanager.rs | 49 ++++++++++++++----------- lightning/src/ln/offers_tests.rs | 14 +++---- lightning/src/offers/invoice_request.rs | 9 ++--- lightning/src/offers/refund.rs | 14 +++---- lightning/src/offers/signer.rs | 3 +- 5 files changed, 45 insertions(+), 44 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 9fde3e917..a5fe80a3a 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -4203,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) -> Result<(), Bolt12PaymentError> { - let secp_ctx = &self.secp_ctx; - let expanded_key = &self.inbound_payment_key; - match invoice.verify(expanded_key, secp_ctx) { + 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 { + let secp_ctx = &self.secp_ctx; + let expanded_key = &self.inbound_payment_key; + + match context { + OffersContext::Unknown {} if invoice.is_for_refund_without_paths() => { + invoice.verify(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); @@ -10807,24 +10827,9 @@ where } }, OffersMessage::Invoice(invoice) => { - let payer_data = match context { - OffersContext::Unknown {} if invoice.is_for_refund_without_paths() => None, - OffersContext::OutboundPayment { payment_id, nonce } => Some((payment_id, nonce)), - _ => return ResponseInstruction::NoResponse, - }; - - let payment_id = match payer_data { - Some((payment_id, nonce)) => { - if invoice.verify_using_payer_data(payment_id, nonce, expanded_key, secp_ctx) { - payment_id - } else { - return ResponseInstruction::NoResponse; - } - }, - None => match invoice.verify(expanded_key, secp_ctx) { - Ok(payment_id) => payment_id, - Err(()) => return ResponseInstruction::NoResponse, - }, + let payment_id = match self.verify_bolt12_invoice(&invoice, &context) { + Ok(payment_id) => payment_id, + Err(()) => return ResponseInstruction::NoResponse, }; let result = { diff --git a/lightning/src/ln/offers_tests.rs b/lightning/src/ln/offers_tests.rs index 1dcc46777..627fc8126 100644 --- a/lightning/src/ln/offers_tests.rs +++ b/lightning/src/ln/offers_tests.rs @@ -1085,10 +1085,10 @@ fn pays_bolt12_invoice_asynchronously() { let onion_message = alice.onion_messenger.next_onion_message_for_peer(bob_id).unwrap(); bob.onion_messenger.handle_onion_message(&alice_id, &onion_message); - let invoice = match get_event!(bob, Event::InvoiceReceived) { - Event::InvoiceReceived { payment_id: actual_payment_id, invoice, .. } => { + let (invoice, context) = match get_event!(bob, Event::InvoiceReceived) { + Event::InvoiceReceived { payment_id: actual_payment_id, invoice, context, .. } => { assert_eq!(actual_payment_id, payment_id); - invoice + (invoice, context) }, _ => panic!("No Event::InvoiceReceived"), }; @@ -1099,9 +1099,9 @@ fn pays_bolt12_invoice_asynchronously() { assert_eq!(path.introduction_node, IntroductionNode::NodeId(alice_id)); } - assert!(bob.node.send_payment_for_bolt12_invoice(&invoice).is_ok()); + assert!(bob.node.send_payment_for_bolt12_invoice(&invoice, &context).is_ok()); assert_eq!( - bob.node.send_payment_for_bolt12_invoice(&invoice), + bob.node.send_payment_for_bolt12_invoice(&invoice, &context), Err(Bolt12PaymentError::DuplicateInvoice), ); @@ -1112,7 +1112,7 @@ fn pays_bolt12_invoice_asynchronously() { expect_recent_payment!(bob, RecentPaymentDetails::Fulfilled, payment_id); assert_eq!( - bob.node.send_payment_for_bolt12_invoice(&invoice), + bob.node.send_payment_for_bolt12_invoice(&invoice, &context), Err(Bolt12PaymentError::DuplicateInvoice), ); @@ -1121,7 +1121,7 @@ fn pays_bolt12_invoice_asynchronously() { } assert_eq!( - bob.node.send_payment_for_bolt12_invoice(&invoice), + bob.node.send_payment_for_bolt12_invoice(&invoice, &context), Err(Bolt12PaymentError::UnexpectedInvoice), ); } diff --git a/lightning/src/offers/invoice_request.rs b/lightning/src/offers/invoice_request.rs index 2415d5f46..8111e9958 100644 --- a/lightning/src/offers/invoice_request.rs +++ b/lightning/src/offers/invoice_request.rs @@ -1487,10 +1487,7 @@ mod tests { .unwrap() .build().unwrap() .sign(recipient_sign).unwrap(); - match invoice.verify(&expanded_key, &secp_ctx) { - Ok(payment_id) => assert_eq!(payment_id, PaymentId([1; 32])), - Err(()) => panic!("verification failed"), - } + assert!(invoice.verify(&expanded_key, &secp_ctx).is_err()); assert!(invoice.verify_using_payer_data(payment_id, nonce, &expanded_key, &secp_ctx)); // Fails verification with altered fields @@ -1514,7 +1511,7 @@ mod tests { signature_tlv_stream.write(&mut encoded_invoice).unwrap(); let invoice = Bolt12Invoice::try_from(encoded_invoice).unwrap(); - assert!(invoice.verify(&expanded_key, &secp_ctx).is_err()); + assert!(!invoice.verify_using_payer_data(payment_id, nonce, &expanded_key, &secp_ctx)); // Fails verification with altered payer id let ( @@ -1537,7 +1534,7 @@ mod tests { signature_tlv_stream.write(&mut encoded_invoice).unwrap(); let invoice = Bolt12Invoice::try_from(encoded_invoice).unwrap(); - assert!(invoice.verify(&expanded_key, &secp_ctx).is_err()); + assert!(!invoice.verify_using_payer_data(payment_id, nonce, &expanded_key, &secp_ctx)); } #[test] diff --git a/lightning/src/offers/refund.rs b/lightning/src/offers/refund.rs index 6c9226779..a98c606ab 100644 --- a/lightning/src/offers/refund.rs +++ b/lightning/src/offers/refund.rs @@ -191,12 +191,15 @@ macro_rules! refund_builder_methods { ( /// /// Also, sets the metadata when [`RefundBuilder::build`] is called such that it can be used by /// [`Bolt12Invoice::verify`] to determine if the invoice was produced for the refund given an - /// [`ExpandedKey`]. + /// [`ExpandedKey`]. However, if [`RefundBuilder::path`] is called, then the metadata must be + /// included in each [`BlindedPath`] instead. In this case, use + /// [`Bolt12Invoice::verify_using_payer_data`]. /// /// The `payment_id` is encrypted in the metadata and should be unique. This ensures that only /// one invoice will be paid for the refund and that payments can be uniquely identified. /// /// [`Bolt12Invoice::verify`]: crate::offers::invoice::Bolt12Invoice::verify + /// [`Bolt12Invoice::verify_using_payer_data`]: crate::offers::invoice::Bolt12Invoice::verify_using_payer_data /// [`ExpandedKey`]: crate::ln::inbound_payment::ExpandedKey pub fn deriving_payer_id( node_id: PublicKey, expanded_key: &ExpandedKey, nonce: Nonce, @@ -1107,10 +1110,7 @@ mod tests { .unwrap() .build().unwrap() .sign(recipient_sign).unwrap(); - match invoice.verify(&expanded_key, &secp_ctx) { - Ok(payment_id) => assert_eq!(payment_id, PaymentId([1; 32])), - Err(()) => panic!("verification failed"), - } + assert!(invoice.verify(&expanded_key, &secp_ctx).is_err()); assert!(invoice.verify_using_payer_data(payment_id, nonce, &expanded_key, &secp_ctx)); // Fails verification with altered fields @@ -1125,7 +1125,7 @@ mod tests { .unwrap() .build().unwrap() .sign(recipient_sign).unwrap(); - assert!(invoice.verify(&expanded_key, &secp_ctx).is_err()); + assert!(!invoice.verify_using_payer_data(payment_id, nonce, &expanded_key, &secp_ctx)); // Fails verification with altered payer_id let mut tlv_stream = refund.as_tlv_stream(); @@ -1140,7 +1140,7 @@ mod tests { .unwrap() .build().unwrap() .sign(recipient_sign).unwrap(); - assert!(invoice.verify(&expanded_key, &secp_ctx).is_err()); + assert!(!invoice.verify_using_payer_data(payment_id, nonce, &expanded_key, &secp_ctx)); } #[test] diff --git a/lightning/src/offers/signer.rs b/lightning/src/offers/signer.rs index 0f1428760..2e12a1705 100644 --- a/lightning/src/offers/signer.rs +++ b/lightning/src/offers/signer.rs @@ -249,8 +249,7 @@ impl MetadataMaterial { self.hmac.input(DERIVED_METADATA_AND_KEYS_HMAC_INPUT); self.maybe_include_encrypted_payment_id(); - let mut bytes = self.encrypted_payment_id.map(|id| id.to_vec()).unwrap_or(vec![]); - bytes.extend_from_slice(self.nonce.as_slice()); + let bytes = self.encrypted_payment_id.map(|id| id.to_vec()).unwrap_or(vec![]); let hmac = Hmac::from_engine(self.hmac); let privkey = SecretKey::from_slice(hmac.as_byte_array()).unwrap(); -- 2.39.5