]> git.bitcoin.ninja Git - rust-lightning/commitdiff
Elide nonce from payer metadata
authorJeffrey Czyz <jkczyz@gmail.com>
Mon, 15 Jul 2024 23:29:41 +0000 (18:29 -0500)
committerJeffrey Czyz <jkczyz@gmail.com>
Mon, 22 Jul 2024 16:38:39 +0000 (11:38 -0500)
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
lightning/src/ln/offers_tests.rs
lightning/src/offers/invoice_request.rs
lightning/src/offers/refund.rs
lightning/src/offers/signer.rs

index 9fde3e917414dc091e74fbc9e4fb1848c403329b..a5fe80a3a7033ebe714cfe299bb23bd434908262 100644 (file)
@@ -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<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() => {
+                               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 = {
index 1dcc4677798cb929916a939d7997f0b803ab2b34..627fc81264662650c350f2c4a17c6266f2d6a302 100644 (file)
@@ -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),
        );
 }
index 2415d5f46b6e1c9f5defbb02409a359cc38d6a04..8111e995813b00f6d9d124426c750474fb6a6324 100644 (file)
@@ -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]
index 6c9226779a13bb7cdf3ed3db69d3a2323f54908a..a98c606abd2a720be4255ad90623716f5ed9efdc 100644 (file)
@@ -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]
index 0f14287608df6cfa2d7379f49365d36e34dc16e4..2e12a17056ecc7ed7e9f1c853ed8a361a137a500 100644 (file)
@@ -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();