From d7518458e9b1166721ebe7157ea3b5efcacbc205 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 need 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 | 4 -- lightning/src/offers/refund.rs | 4 -- lightning/src/offers/signer.rs | 3 +- 5 files changed, 35 insertions(+), 39 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 149e0bf0d32..e74771be8e5 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -4106,15 +4106,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); @@ -10623,24 +10643,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 d36353501a3..034206362fb 100644 --- a/lightning/src/ln/offers_tests.rs +++ b/lightning/src/ln/offers_tests.rs @@ -921,10 +921,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"), }; @@ -935,9 +935,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), ); @@ -948,7 +948,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), ); @@ -957,7 +957,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 b9bf5d5e71d..d6000b1c0e4 100644 --- a/lightning/src/offers/invoice_request.rs +++ b/lightning/src/offers/invoice_request.rs @@ -1483,10 +1483,6 @@ 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_using_payer_data(payment_id, nonce, &expanded_key, &secp_ctx)); // Fails verification with altered fields diff --git a/lightning/src/offers/refund.rs b/lightning/src/offers/refund.rs index fbd4758d6a0..5efc6062a48 100644 --- a/lightning/src/offers/refund.rs +++ b/lightning/src/offers/refund.rs @@ -1106,10 +1106,6 @@ 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_using_payer_data(payment_id, nonce, &expanded_key, &secp_ctx)); // Fails verification with altered fields diff --git a/lightning/src/offers/signer.rs b/lightning/src/offers/signer.rs index f9b8221d968..d072b5a9208 100644 --- a/lightning/src/offers/signer.rs +++ b/lightning/src/offers/signer.rs @@ -244,8 +244,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();