Skip to content

Commit b910f8e

Browse files
jkczyzclaude
andcommitted
Remove nonce from outbound payment OffersContexts
Now that the payer nonce is included in the payer metadata of InvoiceRequest and Refund, Bolt12Invoice verification no longer needs the nonce from the blinded path's OffersContext. Remove it from OffersContext::OutboundPaymentForOffer and OffersContext::OutboundPaymentForRefund, along with enqueue_invoice_request's nonce parameter, which only existed to supply it. The nonce in RetryableInvoiceRequest is no longer used either but is still persisted -- and retained when reading state written by prior versions -- so that such versions can retry the payment and verify the resulting invoice after a downgrade. The payment_id is kept in both variants, however. While no longer needed to confirm the invoice is for an invoice request or refund we created, it is checked against the payment id recovered from a received Bolt12Invoice's payer metadata to ensure the invoice arrived over the blinded path created for that payment. This prevents an attacker from reusing the blinded path of one of our payments to deliver another payment's invoice and correlate the two as ours. Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 404d8c6 commit b910f8e

4 files changed

Lines changed: 33 additions & 35 deletions

File tree

lightning/src/blinded_path/message.rs

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -466,15 +466,17 @@ pub enum OffersContext {
466466
OutboundPaymentForRefund {
467467
/// Payment ID used when creating a [`Refund`].
468468
///
469-
/// [`Refund`]: crate::offers::refund::Refund
470-
payment_id: PaymentId,
471-
472-
/// A nonce used for authenticating that a [`Bolt12Invoice`] is for a valid [`Refund`] and
473-
/// for deriving its signing keys.
469+
/// When a [`Bolt12Invoice`] is received, the payment id recovered from its payer metadata
470+
/// must equal this one, confirming the invoice arrived over the blinded path included in the
471+
/// refund for this payment. Without that check, an attacker holding that path could deliver
472+
/// a different payment's invoice over it, and our paying it would reveal that both payments
473+
/// came from us. That the invoice is for a refund we created is verified by
474+
/// [`Bolt12Invoice::verify_using_metadata`] using its payer metadata.
474475
///
475-
/// [`Bolt12Invoice`]: crate::offers::invoice::Bolt12Invoice
476476
/// [`Refund`]: crate::offers::refund::Refund
477-
nonce: Nonce,
477+
/// [`Bolt12Invoice`]: crate::offers::invoice::Bolt12Invoice
478+
/// [`Bolt12Invoice::verify_using_metadata`]: crate::offers::invoice::Bolt12Invoice::verify_using_metadata
479+
payment_id: PaymentId,
478480
},
479481
/// Context used by a [`BlindedMessagePath`] as a reply path for an [`InvoiceRequest`].
480482
///
@@ -487,15 +489,17 @@ pub enum OffersContext {
487489
OutboundPaymentForOffer {
488490
/// Payment ID used when creating an [`InvoiceRequest`].
489491
///
490-
/// [`InvoiceRequest`]: crate::offers::invoice_request::InvoiceRequest
491-
payment_id: PaymentId,
492-
493-
/// A nonce used for authenticating that a [`Bolt12Invoice`] is for a valid
494-
/// [`InvoiceRequest`] and for deriving its signing keys.
492+
/// When a [`Bolt12Invoice`] is received, the payment id recovered from its payer metadata
493+
/// must equal this one, confirming the invoice arrived over the reply path created for this
494+
/// payment. Without that check, an attacker holding this reply path could deliver a
495+
/// different payment's invoice over it, and our paying it would reveal that both payments
496+
/// came from us. That the invoice is for an invoice request we created is verified by
497+
/// [`Bolt12Invoice::verify_using_metadata`] using its payer metadata.
495498
///
496-
/// [`Bolt12Invoice`]: crate::offers::invoice::Bolt12Invoice
497499
/// [`InvoiceRequest`]: crate::offers::invoice_request::InvoiceRequest
498-
nonce: Nonce,
500+
/// [`Bolt12Invoice`]: crate::offers::invoice::Bolt12Invoice
501+
/// [`Bolt12Invoice::verify_using_metadata`]: crate::offers::invoice::Bolt12Invoice::verify_using_metadata
502+
payment_id: PaymentId,
499503
},
500504
/// Context used by a [`BlindedMessagePath`] as a reply path for a [`Bolt12Invoice`].
501505
///
@@ -678,7 +682,6 @@ impl_ser_tlv_based_enum!(OffersContext,
678682
},
679683
(1, OutboundPaymentForRefund) => {
680684
(0, payment_id, required),
681-
(1, nonce, required),
682685
},
683686
(2, InboundPayment) => {
684687
(0, payment_hash, required),
@@ -690,7 +693,6 @@ impl_ser_tlv_based_enum!(OffersContext,
690693
},
691694
(4, OutboundPaymentForOffer) => {
692695
(0, payment_id, required),
693-
(1, nonce, required),
694696
},
695697
);
696698

lightning/src/ln/channelmanager.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15075,13 +15075,13 @@ impl<
1507515075
let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self);
1507615076

1507715077
self.flow.enqueue_invoice_request(
15078-
invoice_request.clone(), payment_id, nonce,
15078+
invoice_request.clone(), payment_id,
1507915079
self.get_peers_for_blinded_path()
1508015080
)?;
1508115081

1508215082
let retryable_invoice_request = RetryableInvoiceRequest {
1508315083
invoice_request: invoice_request.clone(),
15084-
nonce,
15084+
nonce: Some(nonce),
1508515085
needs_retry: true,
1508615086
};
1508715087

@@ -17231,11 +17231,11 @@ impl<
1723117231
for (payment_id, retryable_invoice_request) in
1723217232
self.pending_outbound_payments.release_invoice_requests_awaiting_invoice()
1723317233
{
17234-
let RetryableInvoiceRequest { invoice_request, nonce, .. } = retryable_invoice_request;
17234+
let RetryableInvoiceRequest { invoice_request, .. } = retryable_invoice_request;
1723517235

1723617236
let peers = self.get_peers_for_blinded_path();
1723717237
let enqueue_invreq_res =
17238-
self.flow.enqueue_invoice_request(invoice_request, payment_id, nonce, peers);
17238+
self.flow.enqueue_invoice_request(invoice_request, payment_id, peers);
1723917239
if enqueue_invreq_res.is_err() {
1724017240
log_warn!(
1724117241
self.logger,

lightning/src/ln/outbound_payment.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -173,14 +173,18 @@ pub(crate) enum PendingOutboundPayment {
173173
#[derive(Clone)]
174174
pub(crate) struct RetryableInvoiceRequest {
175175
pub(crate) invoice_request: InvoiceRequest,
176-
pub(crate) nonce: Nonce,
176+
// No longer used, but written so that the payment can be retried after downgrading to a
177+
// version that verifies invoices using the nonce instead of the payer metadata. Set when
178+
// creating an invoice request and otherwise retains the value read from disk, which may have
179+
// been written by such a version.
180+
pub(crate) nonce: Option<Nonce>,
177181
pub(super) needs_retry: bool,
178182
}
179183

180184
impl_ser_tlv_based!(RetryableInvoiceRequest, {
181185
(0, invoice_request, required),
182186
(1, needs_retry, (default_value, true)),
183-
(2, nonce, required),
187+
(2, nonce, option),
184188
});
185189

186190
impl PendingOutboundPayment {

lightning/src/offers/flow.rs

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -503,7 +503,7 @@ impl<MR: MessageRouter, L: Logger> OffersMessageFlow<MR, L> {
503503
None if invoice.is_for_refund_without_paths() => {
504504
invoice.verify_using_metadata(expanded_key, secp_ctx)
505505
},
506-
Some(&OffersContext::OutboundPaymentForOffer { payment_id, .. }) => {
506+
Some(&OffersContext::OutboundPaymentForOffer { payment_id }) => {
507507
if invoice.is_for_offer() {
508508
invoice.verify_using_metadata(expanded_key, secp_ctx).and_then(|extracted| {
509509
(extracted == payment_id).then(|| payment_id).ok_or(())
@@ -512,7 +512,7 @@ impl<MR: MessageRouter, L: Logger> OffersMessageFlow<MR, L> {
512512
Err(())
513513
}
514514
},
515-
Some(&OffersContext::OutboundPaymentForRefund { payment_id, .. }) => {
515+
Some(&OffersContext::OutboundPaymentForRefund { payment_id }) => {
516516
if invoice.is_for_refund() {
517517
invoice.verify_using_metadata(expanded_key, secp_ctx).and_then(|extracted| {
518518
(extracted == payment_id).then(|| payment_id).ok_or(())
@@ -693,7 +693,7 @@ impl<MR: MessageRouter, L: Logger> OffersMessageFlow<MR, L> {
693693

694694
let nonce = Nonce::from_entropy_source(entropy);
695695
let context =
696-
MessageContext::Offers(OffersContext::OutboundPaymentForRefund { payment_id, nonce });
696+
MessageContext::Offers(OffersContext::OutboundPaymentForRefund { payment_id });
697697

698698
// Create the base builder with common properties
699699
let mut builder = RefundBuilder::deriving_signing_pubkey(
@@ -1089,13 +1089,6 @@ impl<MR: MessageRouter, L: Logger> OffersMessageFlow<MR, L> {
10891089
/// over those blinded paths, which can be verified against the intended outbound payment,
10901090
/// ensuring the invoice corresponds to a payment we actually want to make.
10911091
///
1092-
/// # Nonce
1093-
/// The nonce is used to create a unique [`MessageContext`] for the reply paths.
1094-
/// These will be used to verify the corresponding [`Bolt12Invoice`] when it is received.
1095-
///
1096-
/// Note: The provided [`Nonce`] MUST be the same as the [`Nonce`] used for creating the
1097-
/// [`InvoiceRequest`] to ensure correct verification of the corresponding [`Bolt12Invoice`].
1098-
///
10991092
/// See [`OffersMessageFlow::create_invoice_request_builder`] for more details.
11001093
///
11011094
/// # Peers
@@ -1107,11 +1100,10 @@ impl<MR: MessageRouter, L: Logger> OffersMessageFlow<MR, L> {
11071100
/// [`InvoiceError`]: crate::offers::invoice_error::InvoiceError
11081101
/// [`supports_onion_messages`]: crate::types::features::Features::supports_onion_messages
11091102
pub fn enqueue_invoice_request(
1110-
&self, invoice_request: InvoiceRequest, payment_id: PaymentId, nonce: Nonce,
1103+
&self, invoice_request: InvoiceRequest, payment_id: PaymentId,
11111104
peers: Vec<MessageForwardNode>,
11121105
) -> Result<(), Bolt12SemanticError> {
1113-
let context =
1114-
MessageContext::Offers(OffersContext::OutboundPaymentForOffer { payment_id, nonce });
1106+
let context = MessageContext::Offers(OffersContext::OutboundPaymentForOffer { payment_id });
11151107
let reply_paths = self
11161108
.create_blinded_paths(peers, context)
11171109
.map_err(|_| Bolt12SemanticError::MissingPaths)?;

0 commit comments

Comments
 (0)