From 5278d314d500be001b685f3c715c23d44056c9f3 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 19 Jun 2024 11:29:20 -0500 Subject: [PATCH 01/29] Change Nonce visibility to pub A nonce is generated in OfferBuilder::deriving_signing_pubkey from an EntropySource for use in Offer::metadata. The same nonce will need to be included as recipient data in any blinded paths in the Offer. Increase the visibility to allow for this. --- lightning/src/ln/inbound_payment.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lightning/src/ln/inbound_payment.rs b/lightning/src/ln/inbound_payment.rs index a85d93b7135..ba6729290d3 100644 --- a/lightning/src/ln/inbound_payment.rs +++ b/lightning/src/ln/inbound_payment.rs @@ -104,7 +104,7 @@ impl ExpandedKey { /// [`Offer::metadata`]: crate::offers::offer::Offer::metadata /// [`Offer::signing_pubkey`]: crate::offers::offer::Offer::signing_pubkey #[derive(Clone, Copy, Debug, PartialEq)] -pub(crate) struct Nonce(pub(crate) [u8; Self::LENGTH]); +pub struct Nonce(pub(crate) [u8; Self::LENGTH]); impl Nonce { /// Number of bytes in the nonce. From 0a5918e477a56a2132c2ebcbe9e3c2d3c9e70341 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 19 Jun 2024 11:40:34 -0500 Subject: [PATCH 02/29] Reorder imports --- lightning/src/ln/inbound_payment.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/lightning/src/ln/inbound_payment.rs b/lightning/src/ln/inbound_payment.rs index ba6729290d3..b00bf786a3d 100644 --- a/lightning/src/ln/inbound_payment.rs +++ b/lightning/src/ln/inbound_payment.rs @@ -13,12 +13,13 @@ use bitcoin::hashes::{Hash, HashEngine}; use bitcoin::hashes::cmp::fixed_time_eq; use bitcoin::hashes::hmac::{Hmac, HmacEngine}; use bitcoin::hashes::sha256::Hash as Sha256; -use crate::sign::{KeyMaterial, EntropySource}; -use crate::ln::types::{PaymentHash, PaymentPreimage, PaymentSecret}; -use crate::ln::msgs; -use crate::ln::msgs::MAX_VALUE_MSAT; + use crate::crypto::chacha20::ChaCha20; use crate::crypto::utils::hkdf_extract_expand_5x; +use crate::ln::msgs; +use crate::ln::msgs::MAX_VALUE_MSAT; +use crate::ln::types::{PaymentHash, PaymentPreimage, PaymentSecret}; +use crate::sign::{KeyMaterial, EntropySource}; use crate::util::errors::APIError; use crate::util::logger::Logger; From d7aeaa0aadaece9921b9317a661bbeb4f2580cb6 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 19 Jun 2024 11:51:53 -0500 Subject: [PATCH 03/29] Move Nonce to a separate offers sub-module Nonce is used when constructing Offer::metadata and will soon be need when constructing BlindedPath for use in authentication. Move it to separate module now that it is public and will be more widely used. --- lightning/src/ln/inbound_payment.rs | 48 +------------------ lightning/src/offers/invoice_request.rs | 3 +- lightning/src/offers/mod.rs | 1 + lightning/src/offers/nonce.rs | 64 +++++++++++++++++++++++++ lightning/src/offers/offer.rs | 3 +- lightning/src/offers/refund.rs | 3 +- lightning/src/offers/signer.rs | 3 +- 7 files changed, 74 insertions(+), 51 deletions(-) create mode 100644 lightning/src/offers/nonce.rs diff --git a/lightning/src/ln/inbound_payment.rs b/lightning/src/ln/inbound_payment.rs index b00bf786a3d..2d807d55070 100644 --- a/lightning/src/ln/inbound_payment.rs +++ b/lightning/src/ln/inbound_payment.rs @@ -19,6 +19,7 @@ use crate::crypto::utils::hkdf_extract_expand_5x; use crate::ln::msgs; use crate::ln::msgs::MAX_VALUE_MSAT; use crate::ln::types::{PaymentHash, PaymentPreimage, PaymentSecret}; +use crate::offers::nonce::Nonce; use crate::sign::{KeyMaterial, EntropySource}; use crate::util::errors::APIError; use crate::util::logger::Logger; @@ -97,53 +98,6 @@ impl ExpandedKey { } } -/// A 128-bit number used only once. -/// -/// Needed when constructing [`Offer::metadata`] and deriving [`Offer::signing_pubkey`] from -/// [`ExpandedKey`]. Must not be reused for any other derivation without first hashing. -/// -/// [`Offer::metadata`]: crate::offers::offer::Offer::metadata -/// [`Offer::signing_pubkey`]: crate::offers::offer::Offer::signing_pubkey -#[derive(Clone, Copy, Debug, PartialEq)] -pub struct Nonce(pub(crate) [u8; Self::LENGTH]); - -impl Nonce { - /// Number of bytes in the nonce. - pub const LENGTH: usize = 16; - - /// Creates a `Nonce` from the given [`EntropySource`]. - pub fn from_entropy_source(entropy_source: ES) -> Self - where - ES::Target: EntropySource, - { - let mut bytes = [0u8; Self::LENGTH]; - let rand_bytes = entropy_source.get_secure_random_bytes(); - bytes.copy_from_slice(&rand_bytes[..Self::LENGTH]); - - Nonce(bytes) - } - - /// Returns a slice of the underlying bytes of size [`Nonce::LENGTH`]. - pub fn as_slice(&self) -> &[u8] { - &self.0 - } -} - -impl TryFrom<&[u8]> for Nonce { - type Error = (); - - fn try_from(bytes: &[u8]) -> Result { - if bytes.len() != Self::LENGTH { - return Err(()); - } - - let mut copied_bytes = [0u8; Self::LENGTH]; - copied_bytes.copy_from_slice(bytes); - - Ok(Self(copied_bytes)) - } -} - enum Method { LdkPaymentHash = 0, UserPaymentHash = 1, diff --git a/lightning/src/offers/invoice_request.rs b/lightning/src/offers/invoice_request.rs index cdf94a2e80d..b9ccc5f4d7d 100644 --- a/lightning/src/offers/invoice_request.rs +++ b/lightning/src/offers/invoice_request.rs @@ -68,10 +68,11 @@ use crate::blinded_path::BlindedPath; use crate::ln::types::PaymentHash; use crate::ln::channelmanager::PaymentId; use crate::ln::features::InvoiceRequestFeatures; -use crate::ln::inbound_payment::{ExpandedKey, IV_LEN, Nonce}; +use crate::ln::inbound_payment::{ExpandedKey, IV_LEN}; use crate::ln::msgs::DecodeError; use crate::offers::invoice::BlindedPayInfo; use crate::offers::merkle::{SignError, SignFn, SignatureTlvStream, SignatureTlvStreamRef, TaggedHash, self}; +use crate::offers::nonce::Nonce; use crate::offers::offer::{Offer, OfferContents, OfferId, OfferTlvStream, OfferTlvStreamRef}; use crate::offers::parse::{Bolt12ParseError, ParsedMessage, Bolt12SemanticError}; use crate::offers::payer::{PayerContents, PayerTlvStream, PayerTlvStreamRef}; diff --git a/lightning/src/offers/mod.rs b/lightning/src/offers/mod.rs index e5e894e2a12..e4fe7d789db 100644 --- a/lightning/src/offers/mod.rs +++ b/lightning/src/offers/mod.rs @@ -20,6 +20,7 @@ pub mod invoice_error; mod invoice_macros; pub mod invoice_request; pub mod merkle; +pub mod nonce; pub mod parse; mod payer; pub mod refund; diff --git a/lightning/src/offers/nonce.rs b/lightning/src/offers/nonce.rs new file mode 100644 index 00000000000..965a39d84ea --- /dev/null +++ b/lightning/src/offers/nonce.rs @@ -0,0 +1,64 @@ +// This file is Copyright its original authors, visible in version control +// history. +// +// This file is licensed under the Apache License, Version 2.0 or the MIT license +// , at your option. +// You may not use this file except in accordance with one or both of these +// licenses. + +//! A number used only once. + +use crate::sign::EntropySource; +use core::ops::Deref; + +#[allow(unused_imports)] +use crate::prelude::*; + +/// A 128-bit number used only once. +/// +/// Needed when constructing [`Offer::metadata`] and deriving [`Offer::signing_pubkey`] from +/// [`ExpandedKey`]. Must not be reused for any other derivation without first hashing. +/// +/// [`Offer::metadata`]: crate::offers::offer::Offer::metadata +/// [`Offer::signing_pubkey`]: crate::offers::offer::Offer::signing_pubkey +/// [`ExpandedKey`]: crate::ln::inbound_payment::ExpandedKey +#[derive(Clone, Copy, Debug, PartialEq)] +pub struct Nonce(pub(crate) [u8; Self::LENGTH]); + +impl Nonce { + /// Number of bytes in the nonce. + pub const LENGTH: usize = 16; + + /// Creates a `Nonce` from the given [`EntropySource`]. + pub fn from_entropy_source(entropy_source: ES) -> Self + where + ES::Target: EntropySource, + { + let mut bytes = [0u8; Self::LENGTH]; + let rand_bytes = entropy_source.get_secure_random_bytes(); + bytes.copy_from_slice(&rand_bytes[..Self::LENGTH]); + + Nonce(bytes) + } + + /// Returns a slice of the underlying bytes of size [`Nonce::LENGTH`]. + pub fn as_slice(&self) -> &[u8] { + &self.0 + } +} + +impl TryFrom<&[u8]> for Nonce { + type Error = (); + + fn try_from(bytes: &[u8]) -> Result { + if bytes.len() != Self::LENGTH { + return Err(()); + } + + let mut copied_bytes = [0u8; Self::LENGTH]; + copied_bytes.copy_from_slice(bytes); + + Ok(Self(copied_bytes)) + } +} diff --git a/lightning/src/offers/offer.rs b/lightning/src/offers/offer.rs index 253de8652bb..03e1e92a482 100644 --- a/lightning/src/offers/offer.rs +++ b/lightning/src/offers/offer.rs @@ -90,9 +90,10 @@ use crate::io; use crate::blinded_path::BlindedPath; use crate::ln::channelmanager::PaymentId; use crate::ln::features::OfferFeatures; -use crate::ln::inbound_payment::{ExpandedKey, IV_LEN, Nonce}; +use crate::ln::inbound_payment::{ExpandedKey, IV_LEN}; use crate::ln::msgs::{DecodeError, MAX_VALUE_MSAT}; use crate::offers::merkle::{TaggedHash, TlvStream}; +use crate::offers::nonce::Nonce; use crate::offers::parse::{Bech32Encode, Bolt12ParseError, Bolt12SemanticError, ParsedMessage}; use crate::offers::signer::{Metadata, MetadataMaterial, self}; use crate::util::ser::{HighZeroBytesDroppedBigSize, Readable, WithoutLength, Writeable, Writer}; diff --git a/lightning/src/offers/refund.rs b/lightning/src/offers/refund.rs index 624036d1957..6a14d287170 100644 --- a/lightning/src/offers/refund.rs +++ b/lightning/src/offers/refund.rs @@ -95,10 +95,11 @@ use crate::blinded_path::BlindedPath; use crate::ln::types::PaymentHash; use crate::ln::channelmanager::PaymentId; use crate::ln::features::InvoiceRequestFeatures; -use crate::ln::inbound_payment::{ExpandedKey, IV_LEN, Nonce}; +use crate::ln::inbound_payment::{ExpandedKey, IV_LEN}; use crate::ln::msgs::{DecodeError, MAX_VALUE_MSAT}; use crate::offers::invoice::BlindedPayInfo; use crate::offers::invoice_request::{InvoiceRequestTlvStream, InvoiceRequestTlvStreamRef}; +use crate::offers::nonce::Nonce; use crate::offers::offer::{OfferTlvStream, OfferTlvStreamRef}; use crate::offers::parse::{Bech32Encode, Bolt12ParseError, Bolt12SemanticError, ParsedMessage}; use crate::offers::payer::{PayerContents, PayerTlvStream, PayerTlvStreamRef}; diff --git a/lightning/src/offers/signer.rs b/lightning/src/offers/signer.rs index fff0514564c..50016a051bd 100644 --- a/lightning/src/offers/signer.rs +++ b/lightning/src/offers/signer.rs @@ -16,8 +16,9 @@ use bitcoin::hashes::sha256::Hash as Sha256; use bitcoin::secp256k1::{Keypair, PublicKey, Secp256k1, SecretKey, self}; use core::fmt; use crate::ln::channelmanager::PaymentId; -use crate::ln::inbound_payment::{ExpandedKey, IV_LEN, Nonce}; +use crate::ln::inbound_payment::{ExpandedKey, IV_LEN}; use crate::offers::merkle::TlvRecord; +use crate::offers::nonce::Nonce; use crate::util::ser::Writeable; use crate::prelude::*; From 219691f9ef7b709fbb128cd39e9559ee02b50e18 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 19 Jun 2024 12:42:16 -0500 Subject: [PATCH 04/29] Pass Nonce directly to OfferBuilder When using OfferBuilder::deriving_signing_pubkey, the nonce generated needs to be the same one included in any OfferBuilder::paths. This is because the nonce is used along with the offer TLVs to derive a signing pubkey and will soon be elided from the metadata entirely. --- lightning/src/ln/channelmanager.rs | 7 ++-- lightning/src/offers/invoice.rs | 8 ++-- lightning/src/offers/invoice_request.rs | 5 ++- lightning/src/offers/offer.rs | 22 +++++----- lightning/src/offers/static_invoice.rs | 55 ++++++++++++++----------- 5 files changed, 51 insertions(+), 46 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 9bceb5816b6..34267adeda9 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -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}; @@ -8784,13 +8785,11 @@ 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 path = $self.create_blinded_paths_using_absolute_expiry(OffersContext::Unknown {}, 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, entropy, secp_ctx - ) + let builder = OfferBuilder::deriving_signing_pubkey(node_id, expanded_key, nonce, secp_ctx) .chain_hash($self.chain_hash) .path(path); diff --git a/lightning/src/offers/invoice.rs b/lightning/src/offers/invoice.rs index 3f96e703b2a..cfe97afd109 100644 --- a/lightning/src/offers/invoice.rs +++ b/lightning/src/offers/invoice.rs @@ -1416,6 +1416,7 @@ mod tests { use crate::ln::msgs::DecodeError; use crate::offers::invoice_request::InvoiceRequestTlvStreamRef; use crate::offers::merkle::{SignError, SignatureTlvStreamRef, TaggedHash, self}; + use crate::offers::nonce::Nonce; use crate::offers::offer::{Amount, OfferTlvStreamRef, Quantity}; use crate::prelude::*; #[cfg(not(c_bindings))] @@ -1752,6 +1753,7 @@ mod tests { let node_id = recipient_pubkey(); let expanded_key = ExpandedKey::new(&KeyMaterial([42; 32])); let entropy = FixedEntropy {}; + let nonce = Nonce::from_entropy_source(&entropy); let secp_ctx = Secp256k1::new(); let blinded_path = BlindedPath { @@ -1765,8 +1767,7 @@ mod tests { #[cfg(c_bindings)] use crate::offers::offer::OfferWithDerivedMetadataBuilder as OfferBuilder; - let offer = OfferBuilder - ::deriving_signing_pubkey(node_id, &expanded_key, &entropy, &secp_ctx) + let offer = OfferBuilder::deriving_signing_pubkey(node_id, &expanded_key, nonce, &secp_ctx) .amount_msats(1000) .path(blinded_path) .build().unwrap(); @@ -1785,8 +1786,7 @@ mod tests { let expanded_key = ExpandedKey::new(&KeyMaterial([41; 32])); assert!(invoice_request.verify(&expanded_key, &secp_ctx).is_err()); - let offer = OfferBuilder - ::deriving_signing_pubkey(node_id, &expanded_key, &entropy, &secp_ctx) + let offer = OfferBuilder::deriving_signing_pubkey(node_id, &expanded_key, nonce, &secp_ctx) .amount_msats(1000) // Omit the path so that node_id is used for the signing pubkey instead of deriving .build().unwrap(); diff --git a/lightning/src/offers/invoice_request.rs b/lightning/src/offers/invoice_request.rs index b9ccc5f4d7d..3b45f9511b5 100644 --- a/lightning/src/offers/invoice_request.rs +++ b/lightning/src/offers/invoice_request.rs @@ -1217,6 +1217,7 @@ mod tests { use crate::ln::msgs::{DecodeError, MAX_VALUE_MSAT}; use crate::offers::invoice::{Bolt12Invoice, SIGNATURE_TAG as INVOICE_SIGNATURE_TAG}; use crate::offers::merkle::{SignError, SignatureTlvStreamRef, TaggedHash, self}; + use crate::offers::nonce::Nonce; use crate::offers::offer::{Amount, OfferTlvStreamRef, Quantity}; #[cfg(not(c_bindings))] use { @@ -2274,12 +2275,12 @@ mod tests { let node_id = recipient_pubkey(); let expanded_key = ExpandedKey::new(&KeyMaterial([42; 32])); let entropy = FixedEntropy {}; + let nonce = Nonce::from_entropy_source(&entropy); let secp_ctx = Secp256k1::new(); #[cfg(c_bindings)] use crate::offers::offer::OfferWithDerivedMetadataBuilder as OfferBuilder; - let offer = OfferBuilder - ::deriving_signing_pubkey(node_id, &expanded_key, &entropy, &secp_ctx) + let offer = OfferBuilder::deriving_signing_pubkey(node_id, &expanded_key, nonce, &secp_ctx) .chain(Network::Testnet) .amount_msats(1000) .supported_quantity(Quantity::Unbounded) diff --git a/lightning/src/offers/offer.rs b/lightning/src/offers/offer.rs index 03e1e92a482..674cfeaa12c 100644 --- a/lightning/src/offers/offer.rs +++ b/lightning/src/offers/offer.rs @@ -243,9 +243,9 @@ macro_rules! offer_explicit_metadata_builder_methods { ( macro_rules! offer_derived_metadata_builder_methods { ($secp_context: ty) => { /// Similar to [`OfferBuilder::new`] except, if [`OfferBuilder::path`] is called, the signing - /// pubkey is derived from the given [`ExpandedKey`] and [`EntropySource`]. This provides - /// recipient privacy by using a different signing pubkey for each offer. Otherwise, the - /// provided `node_id` is used for the signing pubkey. + /// pubkey is derived from the given [`ExpandedKey`] and [`Nonce`]. This provides recipient + /// privacy by using a different signing pubkey for each offer. Otherwise, the provided + /// `node_id` is used for the signing pubkey. /// /// Also, sets the metadata when [`OfferBuilder::build`] is called such that it can be used by /// [`InvoiceRequest::verify`] to determine if the request was produced for the offer given an @@ -253,11 +253,10 @@ macro_rules! offer_derived_metadata_builder_methods { ($secp_context: ty) => { /// /// [`InvoiceRequest::verify`]: crate::offers::invoice_request::InvoiceRequest::verify /// [`ExpandedKey`]: crate::ln::inbound_payment::ExpandedKey - pub fn deriving_signing_pubkey( - node_id: PublicKey, expanded_key: &ExpandedKey, entropy_source: ES, + pub fn deriving_signing_pubkey( + node_id: PublicKey, expanded_key: &ExpandedKey, nonce: Nonce, secp_ctx: &'a Secp256k1<$secp_context> - ) -> Self where ES::Target: EntropySource { - let nonce = Nonce::from_entropy_source(entropy_source); + ) -> Self { let derivation_material = MetadataMaterial::new(nonce, expanded_key, IV_BYTES, None); let metadata = Metadata::DerivedSigningPubkey(derivation_material); Self { @@ -1164,6 +1163,7 @@ mod tests { use crate::ln::features::OfferFeatures; use crate::ln::inbound_payment::ExpandedKey; use crate::ln::msgs::{DecodeError, MAX_VALUE_MSAT}; + use crate::offers::nonce::Nonce; use crate::offers::parse::{Bolt12ParseError, Bolt12SemanticError}; use crate::offers::test_utils::*; use crate::util::ser::{BigSize, Writeable}; @@ -1278,12 +1278,12 @@ mod tests { let node_id = recipient_pubkey(); let expanded_key = ExpandedKey::new(&KeyMaterial([42; 32])); let entropy = FixedEntropy {}; + let nonce = Nonce::from_entropy_source(&entropy); let secp_ctx = Secp256k1::new(); #[cfg(c_bindings)] use super::OfferWithDerivedMetadataBuilder as OfferBuilder; - let offer = OfferBuilder - ::deriving_signing_pubkey(node_id, &expanded_key, &entropy, &secp_ctx) + let offer = OfferBuilder::deriving_signing_pubkey(node_id, &expanded_key, nonce, &secp_ctx) .amount_msats(1000) .build().unwrap(); assert_eq!(offer.signing_pubkey(), Some(node_id)); @@ -1329,6 +1329,7 @@ mod tests { let node_id = recipient_pubkey(); let expanded_key = ExpandedKey::new(&KeyMaterial([42; 32])); let entropy = FixedEntropy {}; + let nonce = Nonce::from_entropy_source(&entropy); let secp_ctx = Secp256k1::new(); let blinded_path = BlindedPath { @@ -1342,8 +1343,7 @@ mod tests { #[cfg(c_bindings)] use super::OfferWithDerivedMetadataBuilder as OfferBuilder; - let offer = OfferBuilder - ::deriving_signing_pubkey(node_id, &expanded_key, &entropy, &secp_ctx) + let offer = OfferBuilder::deriving_signing_pubkey(node_id, &expanded_key, nonce, &secp_ctx) .amount_msats(1000) .path(blinded_path) .build().unwrap(); diff --git a/lightning/src/offers/static_invoice.rs b/lightning/src/offers/static_invoice.rs index 69f4073e678..7c084ba3e25 100644 --- a/lightning/src/offers/static_invoice.rs +++ b/lightning/src/offers/static_invoice.rs @@ -565,6 +565,7 @@ mod tests { use crate::offers::invoice::InvoiceTlvStreamRef; use crate::offers::merkle; use crate::offers::merkle::{SignatureTlvStreamRef, TaggedHash}; + use crate::offers::nonce::Nonce; use crate::offers::offer::{Offer, OfferBuilder, OfferTlvStreamRef, Quantity}; use crate::offers::parse::{Bolt12ParseError, Bolt12SemanticError}; use crate::offers::static_invoice::{ @@ -608,13 +609,13 @@ mod tests { let now = now(); let expanded_key = ExpandedKey::new(&KeyMaterial([42; 32])); let entropy = FixedEntropy {}; + let nonce = Nonce::from_entropy_source(&entropy); let secp_ctx = Secp256k1::new(); - let offer = - OfferBuilder::deriving_signing_pubkey(node_id, &expanded_key, &entropy, &secp_ctx) - .path(blinded_path()) - .build() - .unwrap(); + let offer = OfferBuilder::deriving_signing_pubkey(node_id, &expanded_key, nonce, &secp_ctx) + .path(blinded_path()) + .build() + .unwrap(); StaticInvoiceBuilder::for_offer_using_derived_keys( &offer, @@ -647,13 +648,13 @@ mod tests { let now = now(); let expanded_key = ExpandedKey::new(&KeyMaterial([42; 32])); let entropy = FixedEntropy {}; + let nonce = Nonce::from_entropy_source(&entropy); let secp_ctx = Secp256k1::new(); - let offer = - OfferBuilder::deriving_signing_pubkey(node_id, &expanded_key, &entropy, &secp_ctx) - .path(blinded_path()) - .build() - .unwrap(); + let offer = OfferBuilder::deriving_signing_pubkey(node_id, &expanded_key, nonce, &secp_ctx) + .path(blinded_path()) + .build() + .unwrap(); let invoice = StaticInvoiceBuilder::for_offer_using_derived_keys( &offer, @@ -742,13 +743,14 @@ mod tests { let now = now(); let expanded_key = ExpandedKey::new(&KeyMaterial([42; 32])); let entropy = FixedEntropy {}; + let nonce = Nonce::from_entropy_source(&entropy); let secp_ctx = Secp256k1::new(); let future_expiry = Duration::from_secs(u64::max_value()); let past_expiry = Duration::from_secs(0); let valid_offer = - OfferBuilder::deriving_signing_pubkey(node_id, &expanded_key, &entropy, &secp_ctx) + OfferBuilder::deriving_signing_pubkey(node_id, &expanded_key, nonce, &secp_ctx) .path(blinded_path()) .absolute_expiry(future_expiry) .build() @@ -769,7 +771,7 @@ mod tests { assert_eq!(invoice.absolute_expiry(), Some(future_expiry)); let expired_offer = - OfferBuilder::deriving_signing_pubkey(node_id, &expanded_key, &entropy, &secp_ctx) + OfferBuilder::deriving_signing_pubkey(node_id, &expanded_key, nonce, &secp_ctx) .path(blinded_path()) .absolute_expiry(past_expiry) .build() @@ -797,10 +799,11 @@ mod tests { let now = now(); let expanded_key = ExpandedKey::new(&KeyMaterial([42; 32])); let entropy = FixedEntropy {}; + let nonce = Nonce::from_entropy_source(&entropy); let secp_ctx = Secp256k1::new(); let valid_offer = - OfferBuilder::deriving_signing_pubkey(node_id, &expanded_key, &entropy, &secp_ctx) + OfferBuilder::deriving_signing_pubkey(node_id, &expanded_key, nonce, &secp_ctx) .path(blinded_path()) .build() .unwrap(); @@ -860,10 +863,11 @@ mod tests { let now = now(); let expanded_key = ExpandedKey::new(&KeyMaterial([42; 32])); let entropy = FixedEntropy {}; + let nonce = Nonce::from_entropy_source(&entropy); let secp_ctx = Secp256k1::new(); let valid_offer = - OfferBuilder::deriving_signing_pubkey(node_id, &expanded_key, &entropy, &secp_ctx) + OfferBuilder::deriving_signing_pubkey(node_id, &expanded_key, nonce, &secp_ctx) .path(blinded_path()) .build() .unwrap(); @@ -916,10 +920,11 @@ mod tests { let now = now(); let expanded_key = ExpandedKey::new(&KeyMaterial([42; 32])); let entropy = FixedEntropy {}; + let nonce = Nonce::from_entropy_source(&entropy); let secp_ctx = Secp256k1::new(); let offer_with_extra_chain = - OfferBuilder::deriving_signing_pubkey(node_id, &expanded_key, &entropy, &secp_ctx) + OfferBuilder::deriving_signing_pubkey(node_id, &expanded_key, nonce, &secp_ctx) .path(blinded_path()) .chain(Network::Bitcoin) .chain(Network::Testnet) @@ -947,13 +952,13 @@ mod tests { let now = now(); let expanded_key = ExpandedKey::new(&KeyMaterial([42; 32])); let entropy = FixedEntropy {}; + let nonce = Nonce::from_entropy_source(&entropy); let secp_ctx = Secp256k1::new(); - let offer = - OfferBuilder::deriving_signing_pubkey(node_id, &expanded_key, &entropy, &secp_ctx) - .path(blinded_path()) - .build() - .unwrap(); + let offer = OfferBuilder::deriving_signing_pubkey(node_id, &expanded_key, nonce, &secp_ctx) + .path(blinded_path()) + .build() + .unwrap(); const TEST_RELATIVE_EXPIRY: u32 = 3600; let invoice = StaticInvoiceBuilder::for_offer_using_derived_keys( @@ -988,13 +993,13 @@ mod tests { let now = now(); let expanded_key = ExpandedKey::new(&KeyMaterial([42; 32])); let entropy = FixedEntropy {}; + let nonce = Nonce::from_entropy_source(&entropy); let secp_ctx = Secp256k1::new(); - let offer = - OfferBuilder::deriving_signing_pubkey(node_id, &expanded_key, &entropy, &secp_ctx) - .path(blinded_path()) - .build() - .unwrap(); + let offer = OfferBuilder::deriving_signing_pubkey(node_id, &expanded_key, nonce, &secp_ctx) + .path(blinded_path()) + .build() + .unwrap(); let invoice = StaticInvoiceBuilder::for_offer_using_derived_keys( &offer, From e15641504894d7f6e0c0492db569a13d117eb084 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 19 Jun 2024 19:18:26 -0500 Subject: [PATCH 05/29] Add InvoiceRequest::verify_using_recipient_data Invoice requests are authenticated by checking the metadata in the corresponding offer. For offers using blinded paths, this will simply be a 128-bit nonce. Allows checking this nonce explicitly instead of the metadata. This will be used by an upcoming change that includes the nonce in the offer's blinded paths instead of the metadata, which mitigate de-anonymization attacks. --- lightning/src/offers/invoice_request.rs | 37 ++++++++++++++++++++++-- lightning/src/offers/offer.rs | 38 +++++++++++++++++++++---- lightning/src/offers/signer.rs | 22 ++++++++++++++ 3 files changed, 88 insertions(+), 9 deletions(-) diff --git a/lightning/src/offers/invoice_request.rs b/lightning/src/offers/invoice_request.rs index 3b45f9511b5..8c09721f282 100644 --- a/lightning/src/offers/invoice_request.rs +++ b/lightning/src/offers/invoice_request.rs @@ -774,9 +774,11 @@ macro_rules! invoice_request_respond_with_explicit_signing_pubkey_methods { ( } } macro_rules! invoice_request_verify_method { ($self: ident, $self_type: ty) => { - /// Verifies that the request was for an offer created using the given key. Returns the verified - /// request which contains the derived keys needed to sign a [`Bolt12Invoice`] for the request - /// if they could be extracted from the metadata. + /// Verifies that the request was for an offer created using the given key by checking the + /// metadata from the offer. + /// + /// Returns the verified request which contains the derived keys needed to sign a + /// [`Bolt12Invoice`] for the request if they could be extracted from the metadata. /// /// [`Bolt12Invoice`]: crate::offers::invoice::Bolt12Invoice pub fn verify< @@ -800,6 +802,35 @@ macro_rules! invoice_request_verify_method { ($self: ident, $self_type: ty) => { }) } + /// Verifies that the request was for an offer created using the given key by checking a nonce + /// included with the [`BlindedPath`] for which the request was sent through. + /// + /// Returns the verified request which contains the derived keys needed to sign a + /// [`Bolt12Invoice`] for the request if they could be extracted from the metadata. + /// + /// [`Bolt12Invoice`]: crate::offers::invoice::Bolt12Invoice + pub fn verify_using_recipient_data< + #[cfg(not(c_bindings))] + T: secp256k1::Signing + >( + $self: $self_type, nonce: Nonce, key: &ExpandedKey, + #[cfg(not(c_bindings))] + secp_ctx: &Secp256k1, + #[cfg(c_bindings)] + secp_ctx: &Secp256k1, + ) -> Result { + let (offer_id, keys) = $self.contents.inner.offer.verify_using_recipient_data( + &$self.bytes, nonce, key, secp_ctx + )?; + Ok(VerifiedInvoiceRequest { + offer_id, + #[cfg(not(c_bindings))] + inner: $self, + #[cfg(c_bindings)] + inner: $self.clone(), + keys, + }) + } } } #[cfg(not(c_bindings))] diff --git a/lightning/src/offers/offer.rs b/lightning/src/offers/offer.rs index 674cfeaa12c..6ac97a2d749 100644 --- a/lightning/src/offers/offer.rs +++ b/lightning/src/offers/offer.rs @@ -913,18 +913,28 @@ impl OfferContents { self.signing_pubkey } - /// Verifies that the offer metadata was produced from the offer in the TLV stream. pub(super) fn verify( &self, bytes: &[u8], key: &ExpandedKey, secp_ctx: &Secp256k1 ) -> Result<(OfferId, Option), ()> { - match self.metadata() { + self.verify_using_metadata(bytes, self.metadata.as_ref(), key, secp_ctx) + } + + pub(super) fn verify_using_recipient_data( + &self, bytes: &[u8], nonce: Nonce, key: &ExpandedKey, secp_ctx: &Secp256k1 + ) -> Result<(OfferId, Option), ()> { + self.verify_using_metadata(bytes, Some(&Metadata::RecipientData(nonce)), key, secp_ctx) + } + + /// Verifies that the offer metadata was produced from the offer in the TLV stream. + fn verify_using_metadata( + &self, bytes: &[u8], metadata: Option<&Metadata>, key: &ExpandedKey, secp_ctx: &Secp256k1 + ) -> Result<(OfferId, Option), ()> { + match metadata { Some(metadata) => { let tlv_stream = TlvStream::new(bytes).range(OFFER_TYPES).filter(|record| { match record.r#type { OFFER_METADATA_TYPE => false, - OFFER_NODE_ID_TYPE => { - !self.metadata.as_ref().unwrap().derives_recipient_keys() - }, + OFFER_NODE_ID_TYPE => !metadata.derives_recipient_keys(), _ => true, } }); @@ -933,7 +943,7 @@ impl OfferContents { None => return Err(()), }; let keys = signer::verify_recipient_metadata( - metadata, key, IV_BYTES, signing_pubkey, tlv_stream, secp_ctx + metadata.as_ref(), key, IV_BYTES, signing_pubkey, tlv_stream, secp_ctx )?; let offer_id = OfferId::from_valid_invreq_tlv_stream(bytes); @@ -1296,6 +1306,14 @@ mod tests { Err(_) => panic!("unexpected error"), } + // Fails verification when using the wrong method + let invoice_request = offer.request_invoice(vec![1; 32], payer_pubkey()).unwrap() + .build().unwrap() + .sign(payer_sign).unwrap(); + assert!( + invoice_request.verify_using_recipient_data(nonce, &expanded_key, &secp_ctx).is_err() + ); + // Fails verification with altered offer field let mut tlv_stream = offer.as_tlv_stream(); tlv_stream.amount = Some(100); @@ -1357,6 +1375,14 @@ mod tests { Err(_) => panic!("unexpected error"), } + let invoice_request = offer.request_invoice(vec![1; 32], payer_pubkey()).unwrap() + .build().unwrap() + .sign(payer_sign).unwrap(); + match invoice_request.verify_using_recipient_data(nonce, &expanded_key, &secp_ctx) { + Ok(invoice_request) => assert_eq!(invoice_request.offer_id, offer.id()), + Err(_) => panic!("unexpected error"), + } + // Fails verification with altered offer field let mut tlv_stream = offer.as_tlv_stream(); tlv_stream.amount = Some(100); diff --git a/lightning/src/offers/signer.rs b/lightning/src/offers/signer.rs index 50016a051bd..25b4c52865d 100644 --- a/lightning/src/offers/signer.rs +++ b/lightning/src/offers/signer.rs @@ -43,6 +43,9 @@ pub(super) enum Metadata { /// Metadata as parsed, supplied by the user, or derived from the message contents. Bytes(Vec), + /// Metadata for deriving keys included as recipient data in a blinded path. + RecipientData(Nonce), + /// Metadata to be derived from message contents and given material. Derived(MetadataMaterial), @@ -54,6 +57,7 @@ impl Metadata { pub fn as_bytes(&self) -> Option<&Vec> { match self { Metadata::Bytes(bytes) => Some(bytes), + Metadata::RecipientData(_) => None, Metadata::Derived(_) => None, Metadata::DerivedSigningPubkey(_) => None, } @@ -62,6 +66,7 @@ impl Metadata { pub fn has_derivation_material(&self) -> bool { match self { Metadata::Bytes(_) => false, + Metadata::RecipientData(_) => false, Metadata::Derived(_) => true, Metadata::DerivedSigningPubkey(_) => true, } @@ -75,6 +80,7 @@ impl Metadata { // derived, as wouldn't be the case if a Metadata::Bytes with length PaymentId::LENGTH + // Nonce::LENGTH had been set explicitly. Metadata::Bytes(bytes) => bytes.len() == PaymentId::LENGTH + Nonce::LENGTH, + Metadata::RecipientData(_) => false, Metadata::Derived(_) => false, Metadata::DerivedSigningPubkey(_) => true, } @@ -88,6 +94,7 @@ impl Metadata { // derived, as wouldn't be the case if a Metadata::Bytes with length Nonce::LENGTH had // been set explicitly. Metadata::Bytes(bytes) => bytes.len() == Nonce::LENGTH, + Metadata::RecipientData(_) => true, Metadata::Derived(_) => false, Metadata::DerivedSigningPubkey(_) => true, } @@ -96,6 +103,7 @@ impl Metadata { pub fn without_keys(self) -> Self { match self { Metadata::Bytes(_) => self, + Metadata::RecipientData(_) => self, Metadata::Derived(_) => self, Metadata::DerivedSigningPubkey(material) => Metadata::Derived(material), } @@ -106,6 +114,7 @@ impl Metadata { ) -> (Self, Option) { match self { Metadata::Bytes(_) => (self, None), + Metadata::RecipientData(_) => (self, None), Metadata::Derived(mut metadata_material) => { tlv_stream.write(&mut metadata_material.hmac).unwrap(); (Metadata::Bytes(metadata_material.derive_metadata()), None) @@ -126,10 +135,22 @@ impl Default for Metadata { } } +impl AsRef<[u8]> for Metadata { + fn as_ref(&self) -> &[u8] { + match self { + Metadata::Bytes(bytes) => &bytes, + Metadata::RecipientData(nonce) => &nonce.0, + Metadata::Derived(_) => &[], + Metadata::DerivedSigningPubkey(_) => &[], + } + } +} + impl fmt::Debug for Metadata { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { Metadata::Bytes(bytes) => bytes.fmt(f), + Metadata::RecipientData(Nonce(bytes)) => bytes.fmt(f), Metadata::Derived(_) => f.write_str("Derived"), Metadata::DerivedSigningPubkey(_) => f.write_str("DerivedSigningPubkey"), } @@ -145,6 +166,7 @@ impl PartialEq for Metadata { } else { false }, + Metadata::RecipientData(_) => false, Metadata::Derived(_) => false, Metadata::DerivedSigningPubkey(_) => false, } From c0cae08d66d122784dc010113155e528ac40ab00 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Thu, 11 Jul 2024 14:00:38 -0500 Subject: [PATCH 06/29] Assert and document valid Metadata states Metadata is an internal type used within Offer messages. For any constructed message, Metadata::Bytes is always used. The other variants are used during construction or verification time. Document this and debug_assert!(false) accordingly. --- lightning/src/offers/signer.rs | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/lightning/src/offers/signer.rs b/lightning/src/offers/signer.rs index 25b4c52865d..e161a72379e 100644 --- a/lightning/src/offers/signer.rs +++ b/lightning/src/offers/signer.rs @@ -41,15 +41,23 @@ const WITH_ENCRYPTED_PAYMENT_ID_HMAC_INPUT: &[u8; 16] = &[4; 16]; #[derive(Clone)] pub(super) enum Metadata { /// Metadata as parsed, supplied by the user, or derived from the message contents. + /// + /// This is the terminal variant; any `Metadata` in a created message will always use this. Bytes(Vec), /// Metadata for deriving keys included as recipient data in a blinded path. + /// + /// This variant should only be used at verification time, never when building. RecipientData(Nonce), /// Metadata to be derived from message contents and given material. + /// + /// This variant should only be used at building time. Derived(MetadataMaterial), /// Metadata and signing pubkey to be derived from message contents and given material. + /// + /// This variant should only be used at building time. DerivedSigningPubkey(MetadataMaterial), } @@ -57,16 +65,14 @@ impl Metadata { pub fn as_bytes(&self) -> Option<&Vec> { match self { Metadata::Bytes(bytes) => Some(bytes), - Metadata::RecipientData(_) => None, - Metadata::Derived(_) => None, - Metadata::DerivedSigningPubkey(_) => None, + _ => { debug_assert!(false); None }, } } pub fn has_derivation_material(&self) -> bool { match self { Metadata::Bytes(_) => false, - Metadata::RecipientData(_) => false, + Metadata::RecipientData(_) => { debug_assert!(false); false }, Metadata::Derived(_) => true, Metadata::DerivedSigningPubkey(_) => true, } @@ -103,7 +109,7 @@ impl Metadata { pub fn without_keys(self) -> Self { match self { Metadata::Bytes(_) => self, - Metadata::RecipientData(_) => self, + Metadata::RecipientData(_) => { debug_assert!(false); self }, Metadata::Derived(_) => self, Metadata::DerivedSigningPubkey(material) => Metadata::Derived(material), } @@ -114,7 +120,7 @@ impl Metadata { ) -> (Self, Option) { match self { Metadata::Bytes(_) => (self, None), - Metadata::RecipientData(_) => (self, None), + Metadata::RecipientData(_) => { debug_assert!(false); (self, None) }, Metadata::Derived(mut metadata_material) => { tlv_stream.write(&mut metadata_material.hmac).unwrap(); (Metadata::Bytes(metadata_material.derive_metadata()), None) @@ -140,8 +146,8 @@ impl AsRef<[u8]> for Metadata { match self { Metadata::Bytes(bytes) => &bytes, Metadata::RecipientData(nonce) => &nonce.0, - Metadata::Derived(_) => &[], - Metadata::DerivedSigningPubkey(_) => &[], + Metadata::Derived(_) => { debug_assert!(false); &[] }, + Metadata::DerivedSigningPubkey(_) => { debug_assert!(false); &[] }, } } } From c58a1bbaa58618af9b27078b60ca409d69e1e42f Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Tue, 9 Jul 2024 13:31:34 -0500 Subject: [PATCH 07/29] Clean up MessageContext docs --- lightning/src/blinded_path/message.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/lightning/src/blinded_path/message.rs b/lightning/src/blinded_path/message.rs index fe1a8b76681..08e556a9c28 100644 --- a/lightning/src/blinded_path/message.rs +++ b/lightning/src/blinded_path/message.rs @@ -85,20 +85,20 @@ impl Writeable for ReceiveTlvs { } } -/// Represents additional data included by the recipient in a [`BlindedPath`]. +/// Additional data included by the recipient in a [`BlindedPath`]. /// -/// This data is encrypted by the recipient and remains invisible to anyone else. -/// It is included in the [`BlindedPath`], making it accessible again to the recipient -/// whenever the [`BlindedPath`] is used. -/// The recipient can authenticate the message and utilize it for further processing -/// if needed. +/// This data is encrypted by the recipient and will be given to the corresponding message handler +/// when handling a message sent over the [`BlindedPath`]. The recipient can use this data to +/// authenticate the message or for further processing if needed. #[derive(Clone, Debug)] pub enum MessageContext { - /// Represents the data specific to [`OffersMessage`] + /// Context specific to an [`OffersMessage`]. /// /// [`OffersMessage`]: crate::onion_message::offers::OffersMessage Offers(OffersContext), - /// Represents custom data received in a Custom Onion Message. + /// Context specific to a [`CustomOnionMessageHandler::CustomMessage`]. + /// + /// [`CustomOnionMessageHandler::CustomMessage`]: crate::onion_message::messenger::CustomOnionMessageHandler::CustomMessage Custom(Vec), } From 7904e3c7d291a01113b236f3b959c75f78d2349f Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Tue, 9 Jul 2024 13:34:14 -0500 Subject: [PATCH 08/29] Wrap docs at 100 characters --- lightning/src/blinded_path/message.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lightning/src/blinded_path/message.rs b/lightning/src/blinded_path/message.rs index 08e556a9c28..893e5c3a639 100644 --- a/lightning/src/blinded_path/message.rs +++ b/lightning/src/blinded_path/message.rs @@ -108,9 +108,9 @@ pub enum MessageContext { #[derive(Clone, Debug)] pub enum OffersContext { /// Represents an unknown BOLT12 payment context. - /// This variant is used when a message is sent without - /// using a [`BlindedPath`] or over one created prior to - /// LDK version 0.0.124. + /// + /// This variant is used when a message is sent without using a [`BlindedPath`] or over one + /// created prior to LDK version 0.0.124. Unknown {}, /// Represents an outbound BOLT12 payment context. OutboundPayment { From f546aad5dc2eb36a9c3a8349fc16430207fffc18 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Tue, 9 Jul 2024 13:41:42 -0500 Subject: [PATCH 09/29] Expand OffersContext::OutboundPayment docs --- lightning/src/blinded_path/message.rs | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/lightning/src/blinded_path/message.rs b/lightning/src/blinded_path/message.rs index 893e5c3a639..a8942ceeca3 100644 --- a/lightning/src/blinded_path/message.rs +++ b/lightning/src/blinded_path/message.rs @@ -112,9 +112,21 @@ pub enum OffersContext { /// This variant is used when a message is sent without using a [`BlindedPath`] or over one /// created prior to LDK version 0.0.124. Unknown {}, - /// Represents an outbound BOLT12 payment context. + /// Context used by a [`BlindedPath`] within a [`Refund`] or as a reply path for an + /// [`InvoiceRequest`]. + /// + /// This variant is intended to be received when handling a [`Bolt12Invoice`] or an + /// [`InvoiceError`]. + /// + /// [`Refund`]: crate::offers::refund::Refund + /// [`InvoiceRequest`]: crate::offers::invoice_request::InvoiceRequest + /// [`Bolt12Invoice`]: crate::offers::invoice::Bolt12Invoice + /// [`InvoiceError`]: crate::offers::invoice_error::InvoiceError OutboundPayment { - /// Payment ID of the outbound BOLT12 payment. + /// Payment ID used when creating a [`Refund`] or [`InvoiceRequest`]. + /// + /// [`Refund`]: crate::offers::refund::Refund + /// [`InvoiceRequest`]: crate::offers::invoice_request::InvoiceRequest payment_id: PaymentId }, } From 1ff8c8d009d2d48dad31d08a9805ed7a47593002 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Thu, 18 Jul 2024 17:33:00 -0500 Subject: [PATCH 10/29] Fix grammar --- lightning/src/blinded_path/message.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lightning/src/blinded_path/message.rs b/lightning/src/blinded_path/message.rs index a8942ceeca3..8423902b232 100644 --- a/lightning/src/blinded_path/message.rs +++ b/lightning/src/blinded_path/message.rs @@ -102,7 +102,7 @@ pub enum MessageContext { Custom(Vec), } -/// Contains the data specific to [`OffersMessage`] +/// Contains data specific to an [`OffersMessage`]. /// /// [`OffersMessage`]: crate::onion_message::offers::OffersMessage #[derive(Clone, Debug)] From a5145e4360f391427db7f95064de3af910064d4a Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Thu, 18 Jul 2024 17:34:51 -0500 Subject: [PATCH 11/29] Fix OffersContext::Unknown docs --- lightning/src/blinded_path/message.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lightning/src/blinded_path/message.rs b/lightning/src/blinded_path/message.rs index 8423902b232..c910689cebe 100644 --- a/lightning/src/blinded_path/message.rs +++ b/lightning/src/blinded_path/message.rs @@ -107,7 +107,7 @@ pub enum MessageContext { /// [`OffersMessage`]: crate::onion_message::offers::OffersMessage #[derive(Clone, Debug)] pub enum OffersContext { - /// Represents an unknown BOLT12 payment context. + /// Represents an unknown BOLT12 message context. /// /// This variant is used when a message is sent without using a [`BlindedPath`] or over one /// created prior to LDK version 0.0.124. From 6a546189e4cc06d59c456d15d1d1fa63e855757d Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 3 Jul 2024 13:47:07 -0500 Subject: [PATCH 12/29] Add OffersContext::InvoiceRequest To authenticate that an InvoiceRequest is for a valid Offer, include the nonce from the Offer::metadata in the Offer::paths. This can be used to prevent de-anonymization attacks where an attacker sends requests using self-constructed paths to nodes near the Offer::paths' introduction nodes. --- lightning/src/blinded_path/message.rs | 20 +++++++++++++++++++- lightning/src/ln/channelmanager.rs | 3 ++- lightning/src/offers/nonce.rs | 15 +++++++++++++++ 3 files changed, 36 insertions(+), 2 deletions(-) diff --git a/lightning/src/blinded_path/message.rs b/lightning/src/blinded_path/message.rs index c910689cebe..74d31c5b99e 100644 --- a/lightning/src/blinded_path/message.rs +++ b/lightning/src/blinded_path/message.rs @@ -22,6 +22,7 @@ use crate::io; use crate::io::Cursor; use crate::ln::channelmanager::PaymentId; use crate::ln::onion_utils; +use crate::offers::nonce::Nonce; use crate::onion_message::packet::ControlTlvs; use crate::sign::{NodeSigner, Recipient}; use crate::crypto::streams::ChaChaPolyReadAdapter; @@ -112,6 +113,20 @@ pub enum OffersContext { /// This variant is used when a message is sent without using a [`BlindedPath`] or over one /// created prior to LDK version 0.0.124. Unknown {}, + /// Context used by a [`BlindedPath`] within an [`Offer`]. + /// + /// This variant is intended to be received when handling an [`InvoiceRequest`]. + /// + /// [`Offer`]: crate::offers::offer::Offer + /// [`InvoiceRequest`]: crate::offers::invoice_request::InvoiceRequest + InvoiceRequest { + /// A nonce used for authenticating that an [`InvoiceRequest`] is for a valid [`Offer`] and + /// for deriving the offer's signing keys. + /// + /// [`InvoiceRequest`]: crate::offers::invoice_request::InvoiceRequest + /// [`Offer`]: crate::offers::offer::Offer + nonce: Nonce, + }, /// Context used by a [`BlindedPath`] within a [`Refund`] or as a reply path for an /// [`InvoiceRequest`]. /// @@ -138,7 +153,10 @@ impl_writeable_tlv_based_enum!(MessageContext, impl_writeable_tlv_based_enum!(OffersContext, (0, Unknown) => {}, - (1, OutboundPayment) => { + (1, InvoiceRequest) => { + (0, nonce, required), + }, + (2, OutboundPayment) => { (0, payment_id, required), }, ); diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 34267adeda9..9f05877b951 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -8786,7 +8786,8 @@ macro_rules! create_offer_builder { ($self: ident, $builder: ty) => { let secp_ctx = &$self.secp_ctx; let nonce = Nonce::from_entropy_source(entropy); - let path = $self.create_blinded_paths_using_absolute_expiry(OffersContext::Unknown {}, absolute_expiry) + 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) diff --git a/lightning/src/offers/nonce.rs b/lightning/src/offers/nonce.rs index 965a39d84ea..be4c1d3d254 100644 --- a/lightning/src/offers/nonce.rs +++ b/lightning/src/offers/nonce.rs @@ -9,7 +9,10 @@ //! A number used only once. +use crate::io::{self, Read}; +use crate::ln::msgs::DecodeError; use crate::sign::EntropySource; +use crate::util::ser::{Readable, Writeable, Writer}; use core::ops::Deref; #[allow(unused_imports)] @@ -62,3 +65,15 @@ impl TryFrom<&[u8]> for Nonce { Ok(Self(copied_bytes)) } } + +impl Writeable for Nonce { + fn write(&self, w: &mut W) -> Result<(), io::Error> { + self.0.write(w) + } +} + +impl Readable for Nonce { + fn read(r: &mut R) -> Result { + Ok(Nonce(Readable::read(r)?)) + } +} From 35b75fd1fdae40e0d2316da6ddcc725d35c60c50 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Tue, 2 Jul 2024 16:55:59 -0500 Subject: [PATCH 13/29] Authenticate InvoiceRequest using OfferContext When an InvoiceRequest is handled with an OfferContext, use the containing nonce to verify that it is for a valid Offer. Otherwise, fall back to using Offer::metadata, which also contains the nonce. The latter is useful for supporting offers without blinded paths or those created prior to including an OffersContext in their blinded paths. --- lightning/src/ln/channelmanager.rs | 32 ++++++++++++++++++------- lightning/src/offers/invoice_request.rs | 2 +- 2 files changed, 25 insertions(+), 9 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 9f05877b951..80faffc0d7d 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -10703,19 +10703,35 @@ 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 { + 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(expanded_key, secp_ctx) { + Ok(invoice_request) => invoice_request, + Err(()) => { + let error = Bolt12SemanticError::InvalidMetadata; + return responder.respond(OffersMessage::InvoiceError(error.into())); + }, + }, + }; + let amount_msats = match InvoiceBuilder::::amount_msats( - &invoice_request + &invoice_request.inner ) { Ok(amount_msats) => amount_msats, Err(error) => return responder.respond(OffersMessage::InvoiceError(error.into())), }; - let invoice_request = match invoice_request.verify(expanded_key, secp_ctx) { - Ok(invoice_request) => invoice_request, - Err(()) => { - let error = Bolt12SemanticError::InvalidMetadata; - 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( diff --git a/lightning/src/offers/invoice_request.rs b/lightning/src/offers/invoice_request.rs index 8c09721f282..6011ca79b22 100644 --- a/lightning/src/offers/invoice_request.rs +++ b/lightning/src/offers/invoice_request.rs @@ -613,7 +613,7 @@ pub struct VerifiedInvoiceRequest { pub offer_id: OfferId, /// The verified request. - inner: InvoiceRequest, + pub(crate) inner: InvoiceRequest, /// Keys used for signing a [`Bolt12Invoice`] if they can be derived. /// From bf428478679193118e8d705aaeb5c98801496450 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Thu, 20 Jun 2024 11:43:57 -0500 Subject: [PATCH 14/29] Elide metadata from Offer with derived keys When an Offer uses blinded paths, its metadata consists of a nonce used to derive its signing keys. Now that the blinded paths contain this nonce, elide the metadata as it is now redundant. This saves space and also makes it impossible to derive the signing keys if an invoice request is received with the incorrect nonce. The nonce shouldn't be revealed in this case either to prevent de-anonymization attacks. --- lightning/src/offers/invoice.rs | 6 ++-- lightning/src/offers/invoice_request.rs | 8 +++-- lightning/src/offers/offer.rs | 45 ++++++++++++++++--------- lightning/src/offers/static_invoice.rs | 22 +++++++++--- 4 files changed, 55 insertions(+), 26 deletions(-) diff --git a/lightning/src/offers/invoice.rs b/lightning/src/offers/invoice.rs index cfe97afd109..6100f02ca82 100644 --- a/lightning/src/offers/invoice.rs +++ b/lightning/src/offers/invoice.rs @@ -1776,7 +1776,7 @@ mod tests { .sign(payer_sign).unwrap(); if let Err(e) = invoice_request.clone() - .verify(&expanded_key, &secp_ctx).unwrap() + .verify_using_recipient_data(nonce, &expanded_key, &secp_ctx).unwrap() .respond_using_derived_keys_no_std(payment_paths(), payment_hash(), now()).unwrap() .build_and_sign(&secp_ctx) { @@ -1784,7 +1784,9 @@ mod tests { } let expanded_key = ExpandedKey::new(&KeyMaterial([41; 32])); - assert!(invoice_request.verify(&expanded_key, &secp_ctx).is_err()); + assert!( + invoice_request.verify_using_recipient_data(nonce, &expanded_key, &secp_ctx).is_err() + ); let offer = OfferBuilder::deriving_signing_pubkey(node_id, &expanded_key, nonce, &secp_ctx) .amount_msats(1000) diff --git a/lightning/src/offers/invoice_request.rs b/lightning/src/offers/invoice_request.rs index 6011ca79b22..0fee4683c50 100644 --- a/lightning/src/offers/invoice_request.rs +++ b/lightning/src/offers/invoice_request.rs @@ -605,8 +605,9 @@ pub struct InvoiceRequest { signature: Signature, } -/// An [`InvoiceRequest`] that has been verified by [`InvoiceRequest::verify`] and exposes different -/// ways to respond depending on whether the signing keys were derived. +/// An [`InvoiceRequest`] that has been verified by [`InvoiceRequest::verify`] or +/// [`InvoiceRequest::verify_using_recipient_data`] and exposes different ways to respond depending +/// on whether the signing keys were derived. #[derive(Clone, Debug)] pub struct VerifiedInvoiceRequest { /// The identifier of the [`Offer`] for which the [`InvoiceRequest`] was made. @@ -737,7 +738,8 @@ macro_rules! invoice_request_respond_with_explicit_signing_pubkey_methods { ( /// # Note /// /// If the originating [`Offer`] was created using [`OfferBuilder::deriving_signing_pubkey`], - /// then use [`InvoiceRequest::verify`] and [`VerifiedInvoiceRequest`] methods instead. + /// then first use [`InvoiceRequest::verify`] or [`InvoiceRequest::verify_using_recipient_data`] + /// and then [`VerifiedInvoiceRequest`] methods instead. /// /// [`Bolt12Invoice::created_at`]: crate::offers::invoice::Bolt12Invoice::created_at /// [`OfferBuilder::deriving_signing_pubkey`]: crate::offers::offer::OfferBuilder::deriving_signing_pubkey diff --git a/lightning/src/offers/offer.rs b/lightning/src/offers/offer.rs index 6ac97a2d749..1911940eb04 100644 --- a/lightning/src/offers/offer.rs +++ b/lightning/src/offers/offer.rs @@ -249,9 +249,12 @@ macro_rules! offer_derived_metadata_builder_methods { ($secp_context: ty) => { /// /// Also, sets the metadata when [`OfferBuilder::build`] is called such that it can be used by /// [`InvoiceRequest::verify`] to determine if the request was produced for the offer given an - /// [`ExpandedKey`]. + /// [`ExpandedKey`]. However, if [`OfferBuilder::path`] is called, then the metadata will not be + /// set and must be included in each [`BlindedPath`] instead. In this case, use + /// [`InvoiceRequest::verify_using_recipient_data`]. /// /// [`InvoiceRequest::verify`]: crate::offers::invoice_request::InvoiceRequest::verify + /// [`InvoiceRequest::verify_using_recipient_data`]: crate::offers::invoice_request::InvoiceRequest::verify_using_recipient_data /// [`ExpandedKey`]: crate::ln::inbound_payment::ExpandedKey pub fn deriving_signing_pubkey( node_id: PublicKey, expanded_key: &ExpandedKey, nonce: Nonce, @@ -384,9 +387,12 @@ macro_rules! offer_builder_methods { ( } fn build_without_checks($($self_mut)* $self: $self_type) -> Offer { - // Create the metadata for stateless verification of an InvoiceRequest. if let Some(mut metadata) = $self.offer.metadata.take() { + // Create the metadata for stateless verification of an InvoiceRequest. if metadata.has_derivation_material() { + + // Don't derive keys if no blinded paths were given since this means the signing + // pubkey must be the node id of an announced node. if $self.offer.paths.is_none() { metadata = metadata.without_keys(); } @@ -398,14 +404,17 @@ macro_rules! offer_builder_methods { ( tlv_stream.node_id = None; } + // Either replace the signing pubkey with the derived pubkey or include the metadata + // for verification. In the former case, the blinded paths must include + // `OffersContext::InvoiceRequest` instead. let (derived_metadata, keys) = metadata.derive_from(tlv_stream, $self.secp_ctx); - metadata = derived_metadata; - if let Some(keys) = keys { - $self.offer.signing_pubkey = Some(keys.public_key()); + match keys { + Some(keys) => $self.offer.signing_pubkey = Some(keys.public_key()), + None => $self.offer.metadata = Some(derived_metadata), } + } else { + $self.offer.metadata = Some(metadata); } - - $self.offer.metadata = Some(metadata); } let mut bytes = Vec::new(); @@ -667,9 +676,9 @@ impl Offer { #[cfg(async_payments)] pub(super) fn verify( - &self, key: &ExpandedKey, secp_ctx: &Secp256k1 + &self, nonce: Nonce, key: &ExpandedKey, secp_ctx: &Secp256k1 ) -> Result<(OfferId, Option), ()> { - self.contents.verify(&self.bytes, key, secp_ctx) + self.contents.verify_using_recipient_data(&self.bytes, nonce, key, secp_ctx) } } @@ -1296,6 +1305,7 @@ mod tests { let offer = OfferBuilder::deriving_signing_pubkey(node_id, &expanded_key, nonce, &secp_ctx) .amount_msats(1000) .build().unwrap(); + assert!(offer.metadata().is_some()); assert_eq!(offer.signing_pubkey(), Some(node_id)); let invoice_request = offer.request_invoice(vec![1; 32], payer_pubkey()).unwrap() @@ -1365,23 +1375,22 @@ mod tests { .amount_msats(1000) .path(blinded_path) .build().unwrap(); + assert!(offer.metadata().is_none()); assert_ne!(offer.signing_pubkey(), Some(node_id)); let invoice_request = offer.request_invoice(vec![1; 32], payer_pubkey()).unwrap() .build().unwrap() .sign(payer_sign).unwrap(); - match invoice_request.verify(&expanded_key, &secp_ctx) { + match invoice_request.verify_using_recipient_data(nonce, &expanded_key, &secp_ctx) { Ok(invoice_request) => assert_eq!(invoice_request.offer_id, offer.id()), Err(_) => panic!("unexpected error"), } + // Fails verification when using the wrong method let invoice_request = offer.request_invoice(vec![1; 32], payer_pubkey()).unwrap() .build().unwrap() .sign(payer_sign).unwrap(); - match invoice_request.verify_using_recipient_data(nonce, &expanded_key, &secp_ctx) { - Ok(invoice_request) => assert_eq!(invoice_request.offer_id, offer.id()), - Err(_) => panic!("unexpected error"), - } + assert!(invoice_request.verify(&expanded_key, &secp_ctx).is_err()); // Fails verification with altered offer field let mut tlv_stream = offer.as_tlv_stream(); @@ -1394,7 +1403,9 @@ mod tests { .request_invoice(vec![1; 32], payer_pubkey()).unwrap() .build().unwrap() .sign(payer_sign).unwrap(); - assert!(invoice_request.verify(&expanded_key, &secp_ctx).is_err()); + assert!( + invoice_request.verify_using_recipient_data(nonce, &expanded_key, &secp_ctx).is_err() + ); // Fails verification with altered signing pubkey let mut tlv_stream = offer.as_tlv_stream(); @@ -1408,7 +1419,9 @@ mod tests { .request_invoice(vec![1; 32], payer_pubkey()).unwrap() .build().unwrap() .sign(payer_sign).unwrap(); - assert!(invoice_request.verify(&expanded_key, &secp_ctx).is_err()); + assert!( + invoice_request.verify_using_recipient_data(nonce, &expanded_key, &secp_ctx).is_err() + ); } #[test] diff --git a/lightning/src/offers/static_invoice.rs b/lightning/src/offers/static_invoice.rs index 7c084ba3e25..555b878a670 100644 --- a/lightning/src/offers/static_invoice.rs +++ b/lightning/src/offers/static_invoice.rs @@ -22,6 +22,7 @@ use crate::offers::invoice_macros::{invoice_accessors_common, invoice_builder_me use crate::offers::merkle::{ self, SignError, SignFn, SignatureTlvStream, SignatureTlvStreamRef, TaggedHash, }; +use crate::offers::nonce::Nonce; use crate::offers::offer::{ Amount, Offer, OfferContents, OfferTlvStream, OfferTlvStreamRef, Quantity, }; @@ -97,7 +98,7 @@ impl<'a> StaticInvoiceBuilder<'a> { pub fn for_offer_using_derived_keys( offer: &'a Offer, payment_paths: Vec<(BlindedPayInfo, BlindedPath)>, message_paths: Vec, created_at: Duration, expanded_key: &ExpandedKey, - secp_ctx: &Secp256k1, + nonce: Nonce, secp_ctx: &Secp256k1, ) -> Result { if offer.chains().len() > 1 { return Err(Bolt12SemanticError::UnexpectedChain); @@ -111,7 +112,7 @@ impl<'a> StaticInvoiceBuilder<'a> { offer.signing_pubkey().ok_or(Bolt12SemanticError::MissingSigningPubkey)?; let keys = offer - .verify(&expanded_key, &secp_ctx) + .verify(nonce, &expanded_key, &secp_ctx) .map_err(|()| Bolt12SemanticError::InvalidMetadata)? .1 .ok_or(Bolt12SemanticError::MissingSigningPubkey)?; @@ -623,6 +624,7 @@ mod tests { vec![blinded_path()], now, &expanded_key, + nonce, &secp_ctx, ) .unwrap() @@ -662,6 +664,7 @@ mod tests { vec![blinded_path()], now, &expanded_key, + nonce, &secp_ctx, ) .unwrap() @@ -672,7 +675,7 @@ mod tests { invoice.write(&mut buffer).unwrap(); assert_eq!(invoice.bytes, buffer.as_slice()); - assert!(invoice.metadata().is_some()); + assert_eq!(invoice.metadata(), None); assert_eq!(invoice.amount(), None); assert_eq!(invoice.description(), None); assert_eq!(invoice.offer_features(), &OfferFeatures::empty()); @@ -698,13 +701,12 @@ mod tests { ); let paths = vec![blinded_path()]; - let metadata = vec![42; 16]; assert_eq!( invoice.as_tlv_stream(), ( OfferTlvStreamRef { chains: None, - metadata: Some(&metadata), + metadata: None, currency: None, amount: None, description: None, @@ -762,6 +764,7 @@ mod tests { vec![blinded_path()], now, &expanded_key, + nonce, &secp_ctx, ) .unwrap() @@ -782,6 +785,7 @@ mod tests { vec![blinded_path()], now, &expanded_key, + nonce, &secp_ctx, ) .unwrap() @@ -815,6 +819,7 @@ mod tests { vec![blinded_path()], now, &expanded_key, + nonce, &secp_ctx, ) { assert_eq!(e, Bolt12SemanticError::MissingPaths); @@ -829,6 +834,7 @@ mod tests { Vec::new(), now, &expanded_key, + nonce, &secp_ctx, ) { assert_eq!(e, Bolt12SemanticError::MissingPaths); @@ -849,6 +855,7 @@ mod tests { vec![blinded_path()], now, &expanded_key, + nonce, &secp_ctx, ) { assert_eq!(e, Bolt12SemanticError::MissingPaths); @@ -886,6 +893,7 @@ mod tests { vec![blinded_path()], now, &expanded_key, + nonce, &secp_ctx, ) { assert_eq!(e, Bolt12SemanticError::MissingSigningPubkey); @@ -906,6 +914,7 @@ mod tests { vec![blinded_path()], now, &expanded_key, + nonce, &secp_ctx, ) { assert_eq!(e, Bolt12SemanticError::InvalidMetadata); @@ -937,6 +946,7 @@ mod tests { vec![blinded_path()], now, &expanded_key, + nonce, &secp_ctx, ) { assert_eq!(e, Bolt12SemanticError::UnexpectedChain); @@ -967,6 +977,7 @@ mod tests { vec![blinded_path()], now, &expanded_key, + nonce, &secp_ctx, ) .unwrap() @@ -1007,6 +1018,7 @@ mod tests { vec![blinded_path()], now, &expanded_key, + nonce, &secp_ctx, ) .unwrap() From 9d463408c8097c47a8e8a38e95daee49e7a7fb92 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Fri, 19 Jul 2024 09:56:11 -0500 Subject: [PATCH 15/29] Rename InvoiceRequest::verify --- lightning/src/ln/channelmanager.rs | 2 +- lightning/src/offers/invoice.rs | 2 +- lightning/src/offers/invoice_request.rs | 14 +++++++------ lightning/src/offers/offer.rs | 26 ++++++++++++------------- 4 files changed, 23 insertions(+), 21 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 80faffc0d7d..b5356a85347 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -10717,7 +10717,7 @@ where Ok(invoice_request) => invoice_request, Err(()) => return ResponseInstruction::NoResponse, }, - None => match invoice_request.verify(expanded_key, secp_ctx) { + None => match invoice_request.verify_using_metadata(expanded_key, secp_ctx) { Ok(invoice_request) => invoice_request, Err(()) => { let error = Bolt12SemanticError::InvalidMetadata; diff --git a/lightning/src/offers/invoice.rs b/lightning/src/offers/invoice.rs index 6100f02ca82..2d34b1c0977 100644 --- a/lightning/src/offers/invoice.rs +++ b/lightning/src/offers/invoice.rs @@ -1797,7 +1797,7 @@ mod tests { .sign(payer_sign).unwrap(); match invoice_request - .verify(&expanded_key, &secp_ctx).unwrap() + .verify_using_metadata(&expanded_key, &secp_ctx).unwrap() .respond_using_derived_keys_no_std(payment_paths(), payment_hash(), now()) { Ok(_) => panic!("expected error"), diff --git a/lightning/src/offers/invoice_request.rs b/lightning/src/offers/invoice_request.rs index 0fee4683c50..39eb157d76f 100644 --- a/lightning/src/offers/invoice_request.rs +++ b/lightning/src/offers/invoice_request.rs @@ -605,7 +605,7 @@ pub struct InvoiceRequest { signature: Signature, } -/// An [`InvoiceRequest`] that has been verified by [`InvoiceRequest::verify`] or +/// An [`InvoiceRequest`] that has been verified by [`InvoiceRequest::verify_using_metadata`] or /// [`InvoiceRequest::verify_using_recipient_data`] and exposes different ways to respond depending /// on whether the signing keys were derived. #[derive(Clone, Debug)] @@ -738,8 +738,9 @@ macro_rules! invoice_request_respond_with_explicit_signing_pubkey_methods { ( /// # Note /// /// If the originating [`Offer`] was created using [`OfferBuilder::deriving_signing_pubkey`], - /// then first use [`InvoiceRequest::verify`] or [`InvoiceRequest::verify_using_recipient_data`] - /// and then [`VerifiedInvoiceRequest`] methods instead. + /// then first use [`InvoiceRequest::verify_using_metadata`] or + /// [`InvoiceRequest::verify_using_recipient_data`] and then [`VerifiedInvoiceRequest`] methods + /// instead. /// /// [`Bolt12Invoice::created_at`]: crate::offers::invoice::Bolt12Invoice::created_at /// [`OfferBuilder::deriving_signing_pubkey`]: crate::offers::offer::OfferBuilder::deriving_signing_pubkey @@ -783,7 +784,7 @@ macro_rules! invoice_request_verify_method { ($self: ident, $self_type: ty) => { /// [`Bolt12Invoice`] for the request if they could be extracted from the metadata. /// /// [`Bolt12Invoice`]: crate::offers::invoice::Bolt12Invoice - pub fn verify< + pub fn verify_using_metadata< #[cfg(not(c_bindings))] T: secp256k1::Signing >( @@ -793,7 +794,8 @@ macro_rules! invoice_request_verify_method { ($self: ident, $self_type: ty) => { #[cfg(c_bindings)] secp_ctx: &Secp256k1, ) -> Result { - let (offer_id, keys) = $self.contents.inner.offer.verify(&$self.bytes, key, secp_ctx)?; + let (offer_id, keys) = + $self.contents.inner.offer.verify_using_metadata(&$self.bytes, key, secp_ctx)?; Ok(VerifiedInvoiceRequest { offer_id, #[cfg(not(c_bindings))] @@ -2326,7 +2328,7 @@ mod tests { .payer_note("0".repeat(PAYER_NOTE_LIMIT * 2)) .build().unwrap() .sign(payer_sign).unwrap(); - match invoice_request.verify(&expanded_key, &secp_ctx) { + match invoice_request.verify_using_metadata(&expanded_key, &secp_ctx) { Ok(invoice_request) => { let fields = invoice_request.fields(); assert_eq!(invoice_request.offer_id, offer.id()); diff --git a/lightning/src/offers/offer.rs b/lightning/src/offers/offer.rs index 1911940eb04..94d5175a5b6 100644 --- a/lightning/src/offers/offer.rs +++ b/lightning/src/offers/offer.rs @@ -248,12 +248,12 @@ macro_rules! offer_derived_metadata_builder_methods { ($secp_context: ty) => { /// `node_id` is used for the signing pubkey. /// /// Also, sets the metadata when [`OfferBuilder::build`] is called such that it can be used by - /// [`InvoiceRequest::verify`] to determine if the request was produced for the offer given an - /// [`ExpandedKey`]. However, if [`OfferBuilder::path`] is called, then the metadata will not be - /// set and must be included in each [`BlindedPath`] instead. In this case, use - /// [`InvoiceRequest::verify_using_recipient_data`]. + /// [`InvoiceRequest::verify_using_metadata`] to determine if the request was produced for the + /// offer given an [`ExpandedKey`]. However, if [`OfferBuilder::path`] is called, then the + /// metadata will not be set and must be included in each [`BlindedPath`] instead. In this case, + /// use [`InvoiceRequest::verify_using_recipient_data`]. /// - /// [`InvoiceRequest::verify`]: crate::offers::invoice_request::InvoiceRequest::verify + /// [`InvoiceRequest::verify_using_metadata`]: crate::offers::invoice_request::InvoiceRequest::verify_using_metadata /// [`InvoiceRequest::verify_using_recipient_data`]: crate::offers::invoice_request::InvoiceRequest::verify_using_recipient_data /// [`ExpandedKey`]: crate::ln::inbound_payment::ExpandedKey pub fn deriving_signing_pubkey( @@ -922,20 +922,20 @@ impl OfferContents { self.signing_pubkey } - pub(super) fn verify( + pub(super) fn verify_using_metadata( &self, bytes: &[u8], key: &ExpandedKey, secp_ctx: &Secp256k1 ) -> Result<(OfferId, Option), ()> { - self.verify_using_metadata(bytes, self.metadata.as_ref(), key, secp_ctx) + self.verify(bytes, self.metadata.as_ref(), key, secp_ctx) } pub(super) fn verify_using_recipient_data( &self, bytes: &[u8], nonce: Nonce, key: &ExpandedKey, secp_ctx: &Secp256k1 ) -> Result<(OfferId, Option), ()> { - self.verify_using_metadata(bytes, Some(&Metadata::RecipientData(nonce)), key, secp_ctx) + self.verify(bytes, Some(&Metadata::RecipientData(nonce)), key, secp_ctx) } /// Verifies that the offer metadata was produced from the offer in the TLV stream. - fn verify_using_metadata( + fn verify( &self, bytes: &[u8], metadata: Option<&Metadata>, key: &ExpandedKey, secp_ctx: &Secp256k1 ) -> Result<(OfferId, Option), ()> { match metadata { @@ -1311,7 +1311,7 @@ mod tests { let invoice_request = offer.request_invoice(vec![1; 32], payer_pubkey()).unwrap() .build().unwrap() .sign(payer_sign).unwrap(); - match invoice_request.verify(&expanded_key, &secp_ctx) { + match invoice_request.verify_using_metadata(&expanded_key, &secp_ctx) { Ok(invoice_request) => assert_eq!(invoice_request.offer_id, offer.id()), Err(_) => panic!("unexpected error"), } @@ -1335,7 +1335,7 @@ mod tests { .request_invoice(vec![1; 32], payer_pubkey()).unwrap() .build().unwrap() .sign(payer_sign).unwrap(); - assert!(invoice_request.verify(&expanded_key, &secp_ctx).is_err()); + assert!(invoice_request.verify_using_metadata(&expanded_key, &secp_ctx).is_err()); // Fails verification with altered metadata let mut tlv_stream = offer.as_tlv_stream(); @@ -1349,7 +1349,7 @@ mod tests { .request_invoice(vec![1; 32], payer_pubkey()).unwrap() .build().unwrap() .sign(payer_sign).unwrap(); - assert!(invoice_request.verify(&expanded_key, &secp_ctx).is_err()); + assert!(invoice_request.verify_using_metadata(&expanded_key, &secp_ctx).is_err()); } #[test] @@ -1390,7 +1390,7 @@ mod tests { let invoice_request = offer.request_invoice(vec![1; 32], payer_pubkey()).unwrap() .build().unwrap() .sign(payer_sign).unwrap(); - assert!(invoice_request.verify(&expanded_key, &secp_ctx).is_err()); + assert!(invoice_request.verify_using_metadata(&expanded_key, &secp_ctx).is_err()); // Fails verification with altered offer field let mut tlv_stream = offer.as_tlv_stream(); From f537abd960d83f39bf4b1a66e76560136d8be199 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Thu, 18 Jul 2024 17:53:39 -0500 Subject: [PATCH 16/29] Add docs to Metadata::without_keys --- lightning/src/offers/signer.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lightning/src/offers/signer.rs b/lightning/src/offers/signer.rs index e161a72379e..ca78bc422eb 100644 --- a/lightning/src/offers/signer.rs +++ b/lightning/src/offers/signer.rs @@ -106,6 +106,11 @@ impl Metadata { } } + /// Indicates that signing keys should not be derived when calling [`derive_from`]. Only + /// applicable to state [`Metadata::DerivedSigningPubkey`]; calling this in other states will + /// result in no change. + /// + /// [`derive_from`]: Self::derive_from pub fn without_keys(self) -> Self { match self { Metadata::Bytes(_) => self, From c2a120eeefa5d875557a8601c38555a472098854 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Tue, 2 Jul 2024 17:27:39 -0500 Subject: [PATCH 17/29] Authenticate Bolt12Invoice using OfferContext When a Bolt12Invoice is handled with an OfferContext, use the containing payment_id to verify that it is for a pending outbound payment. Only invoices for refunds without any blinded paths can be verified without an OfferContext. --- lightning/src/ln/channelmanager.rs | 12 ++++++++++++ lightning/src/offers/invoice.rs | 7 +++++++ 2 files changed, 19 insertions(+) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index b5356a85347..6290a9d7e07 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -10804,8 +10804,20 @@ where } }, OffersMessage::Invoice(invoice) => { + let expected_payment_id = match context { + OffersContext::Unknown {} if invoice.is_for_refund_without_paths() => None, + OffersContext::OutboundPayment { payment_id } => Some(payment_id), + _ => return ResponseInstruction::NoResponse, + }; + let result = match invoice.verify(expanded_key, secp_ctx) { Ok(payment_id) => { + if let Some(expected_payment_id) = expected_payment_id { + if payment_id != expected_payment_id { + return ResponseInstruction::NoResponse; + } + } + let features = self.bolt12_invoice_features(); if invoice.invoice_features().requires_unknown_bits_from(&features) { Err(InvoiceError::from(Bolt12SemanticError::UnknownRequiredFeatures)) diff --git a/lightning/src/offers/invoice.rs b/lightning/src/offers/invoice.rs index 2d34b1c0977..69eafbdc549 100644 --- a/lightning/src/offers/invoice.rs +++ b/lightning/src/offers/invoice.rs @@ -787,6 +787,13 @@ impl Bolt12Invoice { (payer_tlv_stream, offer_tlv_stream, invoice_request_tlv_stream, invoice_tlv_stream, signature_tlv_stream) } + + pub(crate) fn is_for_refund_without_paths(&self) -> bool { + match self.contents { + InvoiceContents::ForOffer { .. } => false, + InvoiceContents::ForRefund { .. } => self.message_paths().is_empty(), + } + } } impl PartialEq for Bolt12Invoice { From 559daeb2ae727dce41a30ae790b4ba71c7284c88 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 3 Jul 2024 10:50:02 -0500 Subject: [PATCH 18/29] Don't send InvoiceError on failed authentication When an invoice or invoice request cannot be authenticated from an OffersContext, simply do not respond instead of sending an InvoiceError message. According to BOLT4, messages sent over a blinded path not intended for its use MUST be ignored. --- lightning/src/ln/channelmanager.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 6290a9d7e07..b941a7932d8 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -10719,10 +10719,7 @@ where }, None => match invoice_request.verify_using_metadata(expanded_key, secp_ctx) { Ok(invoice_request) => invoice_request, - Err(()) => { - let error = Bolt12SemanticError::InvalidMetadata; - return responder.respond(OffersMessage::InvoiceError(error.into())); - }, + Err(()) => return ResponseInstruction::NoResponse, }, }; @@ -10833,7 +10830,7 @@ where }) } }, - Err(()) => Err(InvoiceError::from_string("Unrecognized invoice".to_owned())), + Err(()) => return ResponseInstruction::NoResponse, }; match result { From bdf333045cf43f0e46ddf5e7db8c415c7684cc4f Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 3 Jul 2024 18:09:25 -0500 Subject: [PATCH 19/29] Add failure tests for offer message authentication --- lightning/src/ln/channelmanager.rs | 3 + lightning/src/ln/offers_tests.rs | 342 ++++++++++++++++++++++++++++- 2 files changed, 344 insertions(+), 1 deletion(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index b941a7932d8..837a8927dcd 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -2255,7 +2255,10 @@ where event_persist_notifier: Notifier, needs_persist_flag: AtomicBool, + #[cfg(not(any(test, feature = "_test_utils")))] pending_offers_messages: Mutex>>, + #[cfg(any(test, feature = "_test_utils"))] + pub(crate) pending_offers_messages: Mutex>>, /// Tracks the message events that are to be broadcasted when we are connected to some peer. pending_broadcast_messages: Mutex>, diff --git a/lightning/src/ln/offers_tests.rs b/lightning/src/ln/offers_tests.rs index cdd78d02ca8..1dcc4677798 100644 --- a/lightning/src/ln/offers_tests.rs +++ b/lightning/src/ln/offers_tests.rs @@ -54,7 +54,7 @@ use crate::offers::invoice::Bolt12Invoice; use crate::offers::invoice_error::InvoiceError; use crate::offers::invoice_request::{InvoiceRequest, InvoiceRequestFields}; use crate::offers::parse::Bolt12SemanticError; -use crate::onion_message::messenger::PeeledOnion; +use crate::onion_message::messenger::{Destination, PeeledOnion}; use crate::onion_message::offers::OffersMessage; use crate::onion_message::packet::ParsedOnionMessageContents; use crate::routing::gossip::{NodeAlias, NodeId}; @@ -1234,6 +1234,346 @@ fn creates_refund_with_blinded_path_using_unannounced_introduction_node() { } } +/// Check that authentication fails when an invoice request is handled using the wrong context +/// (i.e., was sent directly or over an unexpected blinded path). +#[test] +fn fails_authentication_when_handling_invoice_request() { + let mut accept_forward_cfg = test_default_channel_config(); + accept_forward_cfg.accept_forwards_to_priv_channels = true; + + let mut features = channelmanager::provided_init_features(&accept_forward_cfg); + features.set_onion_messages_optional(); + features.set_route_blinding_optional(); + + let chanmon_cfgs = create_chanmon_cfgs(6); + let node_cfgs = create_node_cfgs(6, &chanmon_cfgs); + + *node_cfgs[1].override_init_features.borrow_mut() = Some(features); + + let node_chanmgrs = create_node_chanmgrs( + 6, &node_cfgs, &[None, Some(accept_forward_cfg), None, None, None, None] + ); + let nodes = create_network(6, &node_cfgs, &node_chanmgrs); + + create_unannounced_chan_between_nodes_with_value(&nodes, 0, 1, 10_000_000, 1_000_000_000); + create_unannounced_chan_between_nodes_with_value(&nodes, 2, 3, 10_000_000, 1_000_000_000); + create_announced_chan_between_nodes_with_value(&nodes, 1, 2, 10_000_000, 1_000_000_000); + create_announced_chan_between_nodes_with_value(&nodes, 1, 4, 10_000_000, 1_000_000_000); + create_announced_chan_between_nodes_with_value(&nodes, 1, 5, 10_000_000, 1_000_000_000); + create_announced_chan_between_nodes_with_value(&nodes, 2, 4, 10_000_000, 1_000_000_000); + create_announced_chan_between_nodes_with_value(&nodes, 2, 5, 10_000_000, 1_000_000_000); + + let (alice, bob, charlie, david) = (&nodes[0], &nodes[1], &nodes[2], &nodes[3]); + let alice_id = alice.node.get_our_node_id(); + let bob_id = bob.node.get_our_node_id(); + let charlie_id = charlie.node.get_our_node_id(); + let david_id = david.node.get_our_node_id(); + + disconnect_peers(alice, &[charlie, david, &nodes[4], &nodes[5]]); + disconnect_peers(david, &[bob, &nodes[4], &nodes[5]]); + + let offer = alice.node + .create_offer_builder(None) + .unwrap() + .amount_msats(10_000_000) + .build().unwrap(); + assert_eq!(offer.metadata(), None); + assert_ne!(offer.signing_pubkey(), Some(alice_id)); + assert!(!offer.paths().is_empty()); + for path in offer.paths() { + assert_eq!(path.introduction_node, IntroductionNode::NodeId(bob_id)); + } + + let invalid_path = alice.node + .create_offer_builder(None) + .unwrap() + .build().unwrap() + .paths().first().unwrap() + .clone(); + assert_eq!(invalid_path.introduction_node, IntroductionNode::NodeId(bob_id)); + + // Send the invoice request directly to Alice instead of using a blinded path. + let payment_id = PaymentId([1; 32]); + david.node.pay_for_offer(&offer, None, None, None, payment_id, Retry::Attempts(0), None) + .unwrap(); + expect_recent_payment!(david, RecentPaymentDetails::AwaitingInvoice, payment_id); + + connect_peers(david, alice); + #[cfg(not(c_bindings))] { + david.node.pending_offers_messages.lock().unwrap().first_mut().unwrap().destination = + Destination::Node(alice_id); + } + #[cfg(c_bindings)] { + david.node.pending_offers_messages.lock().unwrap().first_mut().unwrap().1 = + Destination::Node(alice_id); + } + + let onion_message = david.onion_messenger.next_onion_message_for_peer(alice_id).unwrap(); + alice.onion_messenger.handle_onion_message(&david_id, &onion_message); + + let (invoice_request, reply_path) = extract_invoice_request(alice, &onion_message); + assert_eq!(invoice_request.amount_msats(), None); + assert_ne!(invoice_request.payer_id(), david_id); + assert_eq!(reply_path.introduction_node, IntroductionNode::NodeId(charlie_id)); + + assert_eq!(alice.onion_messenger.next_onion_message_for_peer(charlie_id), None); + + david.node.abandon_payment(payment_id); + get_event!(david, Event::InvoiceRequestFailed); + + // Send the invoice request to Alice using an invalid blinded path. + let payment_id = PaymentId([2; 32]); + david.node.pay_for_offer(&offer, None, None, None, payment_id, Retry::Attempts(0), None) + .unwrap(); + expect_recent_payment!(david, RecentPaymentDetails::AwaitingInvoice, payment_id); + + #[cfg(not(c_bindings))] { + david.node.pending_offers_messages.lock().unwrap().first_mut().unwrap().destination = + Destination::BlindedPath(invalid_path); + } + #[cfg(c_bindings)] { + david.node.pending_offers_messages.lock().unwrap().first_mut().unwrap().1 = + Destination::BlindedPath(invalid_path); + } + + connect_peers(david, bob); + + let onion_message = david.onion_messenger.next_onion_message_for_peer(bob_id).unwrap(); + bob.onion_messenger.handle_onion_message(&david_id, &onion_message); + + let onion_message = bob.onion_messenger.next_onion_message_for_peer(alice_id).unwrap(); + alice.onion_messenger.handle_onion_message(&bob_id, &onion_message); + + let (invoice_request, reply_path) = extract_invoice_request(alice, &onion_message); + assert_eq!(invoice_request.amount_msats(), None); + assert_ne!(invoice_request.payer_id(), david_id); + assert_eq!(reply_path.introduction_node, IntroductionNode::NodeId(charlie_id)); + + assert_eq!(alice.onion_messenger.next_onion_message_for_peer(charlie_id), None); +} + +/// Check that authentication fails when an invoice is handled using the wrong context (i.e., was +/// sent over an unexpected blinded path). +#[test] +fn fails_authentication_when_handling_invoice_for_offer() { + let mut accept_forward_cfg = test_default_channel_config(); + accept_forward_cfg.accept_forwards_to_priv_channels = true; + + let mut features = channelmanager::provided_init_features(&accept_forward_cfg); + features.set_onion_messages_optional(); + features.set_route_blinding_optional(); + + let chanmon_cfgs = create_chanmon_cfgs(6); + let node_cfgs = create_node_cfgs(6, &chanmon_cfgs); + + *node_cfgs[1].override_init_features.borrow_mut() = Some(features); + + let node_chanmgrs = create_node_chanmgrs( + 6, &node_cfgs, &[None, Some(accept_forward_cfg), None, None, None, None] + ); + let nodes = create_network(6, &node_cfgs, &node_chanmgrs); + + create_unannounced_chan_between_nodes_with_value(&nodes, 0, 1, 10_000_000, 1_000_000_000); + create_unannounced_chan_between_nodes_with_value(&nodes, 2, 3, 10_000_000, 1_000_000_000); + create_announced_chan_between_nodes_with_value(&nodes, 1, 2, 10_000_000, 1_000_000_000); + create_announced_chan_between_nodes_with_value(&nodes, 1, 4, 10_000_000, 1_000_000_000); + create_announced_chan_between_nodes_with_value(&nodes, 1, 5, 10_000_000, 1_000_000_000); + create_announced_chan_between_nodes_with_value(&nodes, 2, 4, 10_000_000, 1_000_000_000); + create_announced_chan_between_nodes_with_value(&nodes, 2, 5, 10_000_000, 1_000_000_000); + + let (alice, bob, charlie, david) = (&nodes[0], &nodes[1], &nodes[2], &nodes[3]); + let alice_id = alice.node.get_our_node_id(); + let bob_id = bob.node.get_our_node_id(); + let charlie_id = charlie.node.get_our_node_id(); + let david_id = david.node.get_our_node_id(); + + disconnect_peers(alice, &[charlie, david, &nodes[4], &nodes[5]]); + disconnect_peers(david, &[bob, &nodes[4], &nodes[5]]); + + let offer = alice.node + .create_offer_builder(None) + .unwrap() + .amount_msats(10_000_000) + .build().unwrap(); + assert_ne!(offer.signing_pubkey(), Some(alice_id)); + assert!(!offer.paths().is_empty()); + for path in offer.paths() { + assert_eq!(path.introduction_node, IntroductionNode::NodeId(bob_id)); + } + + // Initiate an invoice request, but abandon tracking it. + let payment_id = PaymentId([1; 32]); + david.node.pay_for_offer(&offer, None, None, None, payment_id, Retry::Attempts(0), None) + .unwrap(); + david.node.abandon_payment(payment_id); + get_event!(david, Event::InvoiceRequestFailed); + + // Don't send the invoice request, but grab its reply path to use with a different request. + let invalid_reply_path = { + let mut pending_offers_messages = david.node.pending_offers_messages.lock().unwrap(); + let pending_invoice_request = pending_offers_messages.pop().unwrap(); + pending_offers_messages.clear(); + #[cfg(not(c_bindings))] { + pending_invoice_request.reply_path + } + #[cfg(c_bindings)] { + pending_invoice_request.2 + } + }; + + let payment_id = PaymentId([2; 32]); + david.node.pay_for_offer(&offer, None, None, None, payment_id, Retry::Attempts(0), None) + .unwrap(); + expect_recent_payment!(david, RecentPaymentDetails::AwaitingInvoice, payment_id); + + // Swap out the reply path to force authentication to fail when handling the invoice since it + // will be sent over the wrong blinded path. + { + let mut pending_offers_messages = david.node.pending_offers_messages.lock().unwrap(); + let mut pending_invoice_request = pending_offers_messages.first_mut().unwrap(); + #[cfg(not(c_bindings))] { + pending_invoice_request.reply_path = invalid_reply_path; + } + #[cfg(c_bindings)] { + pending_invoice_request.2 = invalid_reply_path; + } + } + + connect_peers(david, bob); + + let onion_message = david.onion_messenger.next_onion_message_for_peer(bob_id).unwrap(); + bob.onion_messenger.handle_onion_message(&david_id, &onion_message); + + connect_peers(alice, charlie); + + let onion_message = bob.onion_messenger.next_onion_message_for_peer(alice_id).unwrap(); + alice.onion_messenger.handle_onion_message(&bob_id, &onion_message); + + let (invoice_request, reply_path) = extract_invoice_request(alice, &onion_message); + assert_eq!(invoice_request.amount_msats(), None); + assert_ne!(invoice_request.payer_id(), david_id); + assert_eq!(reply_path.introduction_node, IntroductionNode::NodeId(charlie_id)); + + let onion_message = alice.onion_messenger.next_onion_message_for_peer(charlie_id).unwrap(); + charlie.onion_messenger.handle_onion_message(&alice_id, &onion_message); + + let onion_message = charlie.onion_messenger.next_onion_message_for_peer(david_id).unwrap(); + david.onion_messenger.handle_onion_message(&charlie_id, &onion_message); + + expect_recent_payment!(david, RecentPaymentDetails::AwaitingInvoice, payment_id); +} + +/// Check that authentication fails when an invoice is handled using the wrong context (i.e., was +/// sent directly or over an unexpected blinded path). +#[test] +fn fails_authentication_when_handling_invoice_for_refund() { + let mut accept_forward_cfg = test_default_channel_config(); + accept_forward_cfg.accept_forwards_to_priv_channels = true; + + let mut features = channelmanager::provided_init_features(&accept_forward_cfg); + features.set_onion_messages_optional(); + features.set_route_blinding_optional(); + + let chanmon_cfgs = create_chanmon_cfgs(6); + let node_cfgs = create_node_cfgs(6, &chanmon_cfgs); + + *node_cfgs[1].override_init_features.borrow_mut() = Some(features); + + let node_chanmgrs = create_node_chanmgrs( + 6, &node_cfgs, &[None, Some(accept_forward_cfg), None, None, None, None] + ); + let nodes = create_network(6, &node_cfgs, &node_chanmgrs); + + create_unannounced_chan_between_nodes_with_value(&nodes, 0, 1, 10_000_000, 1_000_000_000); + create_unannounced_chan_between_nodes_with_value(&nodes, 2, 3, 10_000_000, 1_000_000_000); + create_announced_chan_between_nodes_with_value(&nodes, 1, 2, 10_000_000, 1_000_000_000); + create_announced_chan_between_nodes_with_value(&nodes, 1, 4, 10_000_000, 1_000_000_000); + create_announced_chan_between_nodes_with_value(&nodes, 1, 5, 10_000_000, 1_000_000_000); + create_announced_chan_between_nodes_with_value(&nodes, 2, 4, 10_000_000, 1_000_000_000); + create_announced_chan_between_nodes_with_value(&nodes, 2, 5, 10_000_000, 1_000_000_000); + + let (alice, bob, charlie, david) = (&nodes[0], &nodes[1], &nodes[2], &nodes[3]); + let alice_id = alice.node.get_our_node_id(); + let charlie_id = charlie.node.get_our_node_id(); + let david_id = david.node.get_our_node_id(); + + disconnect_peers(alice, &[charlie, david, &nodes[4], &nodes[5]]); + disconnect_peers(david, &[bob, &nodes[4], &nodes[5]]); + + let absolute_expiry = Duration::from_secs(u64::MAX); + let payment_id = PaymentId([1; 32]); + let refund = david.node + .create_refund_builder(10_000_000, absolute_expiry, payment_id, Retry::Attempts(0), None) + .unwrap() + .build().unwrap(); + assert_ne!(refund.payer_id(), david_id); + assert!(!refund.paths().is_empty()); + for path in refund.paths() { + assert_eq!(path.introduction_node, IntroductionNode::NodeId(charlie_id)); + } + expect_recent_payment!(david, RecentPaymentDetails::AwaitingInvoice, payment_id); + + // Send the invoice directly to David instead of using a blinded path. + let expected_invoice = alice.node.request_refund_payment(&refund).unwrap(); + + connect_peers(david, alice); + #[cfg(not(c_bindings))] { + alice.node.pending_offers_messages.lock().unwrap().first_mut().unwrap().destination = + Destination::Node(david_id); + } + #[cfg(c_bindings)] { + alice.node.pending_offers_messages.lock().unwrap().first_mut().unwrap().1 = + Destination::Node(david_id); + } + + let onion_message = alice.onion_messenger.next_onion_message_for_peer(david_id).unwrap(); + david.onion_messenger.handle_onion_message(&alice_id, &onion_message); + + let (invoice, _) = extract_invoice(david, &onion_message); + assert_eq!(invoice, expected_invoice); + + expect_recent_payment!(david, RecentPaymentDetails::AwaitingInvoice, payment_id); + david.node.abandon_payment(payment_id); + get_event!(david, Event::InvoiceRequestFailed); + + // Send the invoice to David using an invalid blinded path. + let invalid_path = refund.paths().first().unwrap().clone(); + let payment_id = PaymentId([2; 32]); + let refund = david.node + .create_refund_builder(10_000_000, absolute_expiry, payment_id, Retry::Attempts(0), None) + .unwrap() + .build().unwrap(); + assert_ne!(refund.payer_id(), david_id); + assert!(!refund.paths().is_empty()); + for path in refund.paths() { + assert_eq!(path.introduction_node, IntroductionNode::NodeId(charlie_id)); + } + + let expected_invoice = alice.node.request_refund_payment(&refund).unwrap(); + + #[cfg(not(c_bindings))] { + alice.node.pending_offers_messages.lock().unwrap().first_mut().unwrap().destination = + Destination::BlindedPath(invalid_path); + } + #[cfg(c_bindings)] { + alice.node.pending_offers_messages.lock().unwrap().first_mut().unwrap().1 = + Destination::BlindedPath(invalid_path); + } + + connect_peers(alice, charlie); + + let onion_message = alice.onion_messenger.next_onion_message_for_peer(charlie_id).unwrap(); + charlie.onion_messenger.handle_onion_message(&alice_id, &onion_message); + + let onion_message = charlie.onion_messenger.next_onion_message_for_peer(david_id).unwrap(); + david.onion_messenger.handle_onion_message(&charlie_id, &onion_message); + + let (invoice, _) = extract_invoice(david, &onion_message); + assert_eq!(invoice, expected_invoice); + + expect_recent_payment!(david, RecentPaymentDetails::AwaitingInvoice, payment_id); +} + /// Fails creating or paying an offer when a blinded path cannot be created because no peers are /// connected. #[test] From fd596c31b504adfddf8478ef59451a62a3d2c597 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Fri, 12 Jul 2024 11:14:18 -0500 Subject: [PATCH 20/29] Pass Nonce directly to InvoiceRequestBuilder When using InvoiceRequestBuilder::deriving_payer_id, the nonce generated needs to be the same one included in any reply path. This is because the nonce is used along with the invoice request TLVs to derive a payer id. While this data is also included in the payer_metadata, including it in the blinded path would allow reducing the amount of data needed there to just enough to provide entropy (i.e., 16 bytes). This is more important for Refund because it can be transmitted via a QR code. But using the same payer_metadata structure for both InvoiceRequest and Refund would be beneficial to avoid more code. --- lightning/src/ln/channelmanager.rs | 3 ++- lightning/src/offers/invoice_request.rs | 22 ++++++++++------------ lightning/src/offers/offer.rs | 24 ++++++++---------------- 3 files changed, 20 insertions(+), 29 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 837a8927dcd..6e6b0ceb6ee 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -8976,8 +8976,9 @@ where let entropy = &*self.entropy_source; let secp_ctx = &self.secp_ctx; + let nonce = Nonce::from_entropy_source(entropy); let builder: InvoiceRequestBuilder = offer - .request_invoice_deriving_payer_id(expanded_key, entropy, secp_ctx, payment_id)? + .request_invoice_deriving_payer_id(expanded_key, nonce, secp_ctx, payment_id)? .into(); let builder = builder.chain_hash(self.chain_hash)?; diff --git a/lightning/src/offers/invoice_request.rs b/lightning/src/offers/invoice_request.rs index 39eb157d76f..b8e47bac59e 100644 --- a/lightning/src/offers/invoice_request.rs +++ b/lightning/src/offers/invoice_request.rs @@ -61,8 +61,6 @@ use bitcoin::blockdata::constants::ChainHash; use bitcoin::network::Network; use bitcoin::secp256k1::{Keypair, PublicKey, Secp256k1, self}; use bitcoin::secp256k1::schnorr::Signature; -use core::ops::Deref; -use crate::sign::EntropySource; use crate::io; use crate::blinded_path::BlindedPath; use crate::ln::types::PaymentHash; @@ -171,11 +169,10 @@ macro_rules! invoice_request_explicit_payer_id_builder_methods { ($self: ident, } #[cfg_attr(c_bindings, allow(dead_code))] - pub(super) fn deriving_metadata( - offer: &'a Offer, payer_id: PublicKey, expanded_key: &ExpandedKey, entropy_source: ES, + pub(super) fn deriving_metadata( + offer: &'a Offer, payer_id: PublicKey, expanded_key: &ExpandedKey, nonce: Nonce, payment_id: PaymentId, - ) -> Self where ES::Target: EntropySource { - let nonce = Nonce::from_entropy_source(entropy_source); + ) -> Self { let payment_id = Some(payment_id); let derivation_material = MetadataMaterial::new(nonce, expanded_key, IV_BYTES, payment_id); let metadata = Metadata::Derived(derivation_material); @@ -201,11 +198,10 @@ macro_rules! invoice_request_derived_payer_id_builder_methods { ( $self: ident, $self_type: ty, $secp_context: ty ) => { #[cfg_attr(c_bindings, allow(dead_code))] - pub(super) fn deriving_payer_id( - offer: &'a Offer, expanded_key: &ExpandedKey, entropy_source: ES, + pub(super) fn deriving_payer_id( + offer: &'a Offer, expanded_key: &ExpandedKey, nonce: Nonce, secp_ctx: &'b Secp256k1<$secp_context>, payment_id: PaymentId - ) -> Self where ES::Target: EntropySource { - let nonce = Nonce::from_entropy_source(entropy_source); + ) -> Self { let payment_id = Some(payment_id); let derivation_material = MetadataMaterial::new(nonce, expanded_key, IV_BYTES, payment_id); let metadata = Metadata::DerivedSigningPubkey(derivation_material); @@ -1403,6 +1399,7 @@ mod tests { let payer_id = payer_pubkey(); let expanded_key = ExpandedKey::new(&KeyMaterial([42; 32])); let entropy = FixedEntropy {}; + let nonce = Nonce::from_entropy_source(&entropy); let secp_ctx = Secp256k1::new(); let payment_id = PaymentId([1; 32]); @@ -1410,7 +1407,7 @@ mod tests { .amount_msats(1000) .build().unwrap(); let invoice_request = offer - .request_invoice_deriving_metadata(payer_id, &expanded_key, &entropy, payment_id) + .request_invoice_deriving_metadata(payer_id, &expanded_key, nonce, payment_id) .unwrap() .build().unwrap() .sign(payer_sign).unwrap(); @@ -1476,6 +1473,7 @@ mod tests { fn builds_invoice_request_with_derived_payer_id() { let expanded_key = ExpandedKey::new(&KeyMaterial([42; 32])); let entropy = FixedEntropy {}; + let nonce = Nonce::from_entropy_source(&entropy); let secp_ctx = Secp256k1::new(); let payment_id = PaymentId([1; 32]); @@ -1483,7 +1481,7 @@ mod tests { .amount_msats(1000) .build().unwrap(); let invoice_request = offer - .request_invoice_deriving_payer_id(&expanded_key, &entropy, &secp_ctx, payment_id) + .request_invoice_deriving_payer_id(&expanded_key, nonce, &secp_ctx, payment_id) .unwrap() .build_and_sign() .unwrap(); diff --git a/lightning/src/offers/offer.rs b/lightning/src/offers/offer.rs index 94d5175a5b6..b5e9154d53a 100644 --- a/lightning/src/offers/offer.rs +++ b/lightning/src/offers/offer.rs @@ -82,10 +82,8 @@ use bitcoin::network::Network; use bitcoin::secp256k1::{Keypair, PublicKey, Secp256k1, self}; use core::hash::{Hash, Hasher}; use core::num::NonZeroU64; -use core::ops::Deref; use core::str::FromStr; use core::time::Duration; -use crate::sign::EntropySource; use crate::io; use crate::blinded_path::BlindedPath; use crate::ln::channelmanager::PaymentId; @@ -699,25 +697,22 @@ macro_rules! request_invoice_derived_payer_id { ($self: ident, $builder: ty) => /// [`Bolt12Invoice::verify`]: crate::offers::invoice::Bolt12Invoice::verify /// [`ExpandedKey`]: crate::ln::inbound_payment::ExpandedKey pub fn request_invoice_deriving_payer_id< - 'a, 'b, ES: Deref, + 'a, 'b, #[cfg(not(c_bindings))] T: secp256k1::Signing >( - &'a $self, expanded_key: &ExpandedKey, entropy_source: ES, + &'a $self, expanded_key: &ExpandedKey, nonce: Nonce, #[cfg(not(c_bindings))] secp_ctx: &'b Secp256k1, #[cfg(c_bindings)] secp_ctx: &'b Secp256k1, payment_id: PaymentId - ) -> Result<$builder, Bolt12SemanticError> - where - ES::Target: EntropySource, - { + ) -> Result<$builder, Bolt12SemanticError> { if $self.offer_features().requires_unknown_bits() { return Err(Bolt12SemanticError::UnknownRequiredFeatures); } - Ok(<$builder>::deriving_payer_id($self, expanded_key, entropy_source, secp_ctx, payment_id)) + Ok(<$builder>::deriving_payer_id($self, expanded_key, nonce, secp_ctx, payment_id)) } } } @@ -728,18 +723,15 @@ macro_rules! request_invoice_explicit_payer_id { ($self: ident, $builder: ty) => /// Useful for recurring payments using the same `payer_id` with different invoices. /// /// [`InvoiceRequest::payer_id`]: crate::offers::invoice_request::InvoiceRequest::payer_id - pub fn request_invoice_deriving_metadata( - &$self, payer_id: PublicKey, expanded_key: &ExpandedKey, entropy_source: ES, + pub fn request_invoice_deriving_metadata( + &$self, payer_id: PublicKey, expanded_key: &ExpandedKey, nonce: Nonce, payment_id: PaymentId - ) -> Result<$builder, Bolt12SemanticError> - where - ES::Target: EntropySource, - { + ) -> Result<$builder, Bolt12SemanticError> { if $self.offer_features().requires_unknown_bits() { return Err(Bolt12SemanticError::UnknownRequiredFeatures); } - Ok(<$builder>::deriving_metadata($self, payer_id, expanded_key, entropy_source, payment_id)) + Ok(<$builder>::deriving_metadata($self, payer_id, expanded_key, nonce, payment_id)) } /// Creates an [`InvoiceRequestBuilder`] for the offer with the given `metadata` and `payer_id`, From 114954cbace0b26bc99205c493ce53dc9bf42183 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Fri, 12 Jul 2024 17:26:30 -0500 Subject: [PATCH 21/29] Pass Nonce directly to RefundBuilder When using RefundBuilder::deriving_payer_id, the nonce generated needs to be the same one included in any RefundBuilder::paths. This is because the nonce is used along with the refund TLVs to derive a payer id and will soon be used to authenticate any invoices. --- lightning/src/ln/channelmanager.rs | 3 ++- lightning/src/offers/refund.rs | 14 ++++++++------ 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 6e6b0ceb6ee..1988aba4a26 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -8861,13 +8861,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 }; 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, entropy, secp_ctx, amount_msats, payment_id + node_id, expanded_key, nonce, secp_ctx, amount_msats, payment_id )? .chain_hash($self.chain_hash) .absolute_expiry(absolute_expiry) diff --git a/lightning/src/offers/refund.rs b/lightning/src/offers/refund.rs index 6a14d287170..d5171b3a692 100644 --- a/lightning/src/offers/refund.rs +++ b/lightning/src/offers/refund.rs @@ -197,15 +197,14 @@ macro_rules! refund_builder_methods { ( /// /// [`InvoiceRequest`]: crate::offers::invoice_request::InvoiceRequest /// [`ExpandedKey`]: crate::ln::inbound_payment::ExpandedKey - pub fn deriving_payer_id( - node_id: PublicKey, expanded_key: &ExpandedKey, entropy_source: ES, + pub fn deriving_payer_id( + node_id: PublicKey, expanded_key: &ExpandedKey, nonce: Nonce, secp_ctx: &'a Secp256k1<$secp_context>, amount_msats: u64, payment_id: PaymentId - ) -> Result where ES::Target: EntropySource { + ) -> Result { if amount_msats > MAX_VALUE_MSAT { return Err(Bolt12SemanticError::InvalidAmount); } - let nonce = Nonce::from_entropy_source(entropy_source); let payment_id = Some(payment_id); let derivation_material = MetadataMaterial::new(nonce, expanded_key, IV_BYTES, payment_id); let metadata = Metadata::DerivedSigningPubkey(derivation_material); @@ -940,6 +939,7 @@ mod tests { use crate::ln::inbound_payment::ExpandedKey; use crate::ln::msgs::{DecodeError, MAX_VALUE_MSAT}; use crate::offers::invoice_request::InvoiceRequestTlvStreamRef; + use crate::offers::nonce::Nonce; use crate::offers::offer::OfferTlvStreamRef; use crate::offers::parse::{Bolt12ParseError, Bolt12SemanticError}; use crate::offers::payer::PayerTlvStreamRef; @@ -1029,11 +1029,12 @@ mod tests { let node_id = payer_pubkey(); let expanded_key = ExpandedKey::new(&KeyMaterial([42; 32])); let entropy = FixedEntropy {}; + let nonce = Nonce::from_entropy_source(&entropy); let secp_ctx = Secp256k1::new(); let payment_id = PaymentId([1; 32]); let refund = RefundBuilder - ::deriving_payer_id(node_id, &expanded_key, &entropy, &secp_ctx, 1000, payment_id) + ::deriving_payer_id(node_id, &expanded_key, nonce, &secp_ctx, 1000, payment_id) .unwrap() .build().unwrap(); assert_eq!(refund.payer_id(), node_id); @@ -1083,6 +1084,7 @@ mod tests { let node_id = payer_pubkey(); let expanded_key = ExpandedKey::new(&KeyMaterial([42; 32])); let entropy = FixedEntropy {}; + let nonce = Nonce::from_entropy_source(&entropy); let secp_ctx = Secp256k1::new(); let payment_id = PaymentId([1; 32]); @@ -1096,7 +1098,7 @@ mod tests { }; let refund = RefundBuilder - ::deriving_payer_id(node_id, &expanded_key, &entropy, &secp_ctx, 1000, payment_id) + ::deriving_payer_id(node_id, &expanded_key, nonce, &secp_ctx, 1000, payment_id) .unwrap() .path(blinded_path) .build().unwrap(); From 868fee7d2d98d1a5dd971a90b71b6016d3cfca29 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Fri, 12 Jul 2024 12:16:23 -0500 Subject: [PATCH 22/29] Add Bolt12Invoice::verify_using_payer_data Invoices are authenticated by checking the payer metadata in the corresponding invoice request or refund. For all invoices requests and for refunds using blinded paths, this will be the encrypted payment id and a 128-bit nonce. Allows checking the unencrypted payment id and nonce explicitly instead of the payer metadata. This will be used by an upcoming change that includes the payment id and nonce in the invoice request's reply path and the refund's blinded paths instead of completely in the payer metadata, which mitigates de-anonymization attacks. --- lightning/src/offers/invoice.rs | 55 +++++++++++++++---------- lightning/src/offers/invoice_request.rs | 8 ++-- lightning/src/offers/refund.rs | 8 ++-- lightning/src/offers/signer.rs | 23 +++++++++++ 4 files changed, 63 insertions(+), 31 deletions(-) diff --git a/lightning/src/offers/invoice.rs b/lightning/src/offers/invoice.rs index 69eafbdc549..5ea2c2e6041 100644 --- a/lightning/src/offers/invoice.rs +++ b/lightning/src/offers/invoice.rs @@ -119,11 +119,12 @@ use crate::ln::msgs::DecodeError; use crate::offers::invoice_macros::{invoice_accessors_common, invoice_builder_methods_common}; use crate::offers::invoice_request::{INVOICE_REQUEST_PAYER_ID_TYPE, INVOICE_REQUEST_TYPES, IV_BYTES as INVOICE_REQUEST_IV_BYTES, InvoiceRequest, InvoiceRequestContents, InvoiceRequestTlvStream, InvoiceRequestTlvStreamRef}; use crate::offers::merkle::{SignError, SignFn, SignatureTlvStream, SignatureTlvStreamRef, TaggedHash, TlvStream, WithoutSignatures, self}; +use crate::offers::nonce::Nonce; use crate::offers::offer::{Amount, OFFER_TYPES, OfferTlvStream, OfferTlvStreamRef, Quantity}; use crate::offers::parse::{Bolt12ParseError, Bolt12SemanticError, ParsedMessage}; use crate::offers::payer::{PAYER_METADATA_TYPE, PayerTlvStream, PayerTlvStreamRef}; use crate::offers::refund::{IV_BYTES as REFUND_IV_BYTES, Refund, RefundContents}; -use crate::offers::signer; +use crate::offers::signer::{Metadata, self}; use crate::util::ser::{HighZeroBytesDroppedBigSize, Iterable, Readable, SeekReadable, WithoutLength, Writeable, Writer}; use crate::util::string::PrintableString; @@ -770,12 +771,31 @@ impl Bolt12Invoice { self.tagged_hash.as_digest().as_ref().clone() } - /// Verifies that the invoice was for a request or refund created using the given key. Returns - /// the associated [`PaymentId`] to use when sending the payment. + /// Verifies that the invoice was for a request or refund created using the given key by + /// checking the payer metadata from the invoice request. + /// + /// Returns the associated [`PaymentId`] to use when sending the payment. pub fn verify( &self, key: &ExpandedKey, secp_ctx: &Secp256k1 ) -> Result { - self.contents.verify(TlvStream::new(&self.bytes), key, secp_ctx) + let metadata = match &self.contents { + InvoiceContents::ForOffer { invoice_request, .. } => &invoice_request.inner.payer.0, + InvoiceContents::ForRefund { refund, .. } => &refund.payer.0, + }; + self.contents.verify(TlvStream::new(&self.bytes), metadata, key, secp_ctx) + } + + /// Verifies that the invoice was for a request or refund created using the given key by + /// checking a payment id and nonce included with the [`BlindedPath`] for which the invoice was + /// sent through. + pub fn verify_using_payer_data( + &self, payment_id: PaymentId, nonce: Nonce, key: &ExpandedKey, secp_ctx: &Secp256k1 + ) -> bool { + let metadata = Metadata::payer_data(payment_id, nonce, key); + match self.contents.verify(TlvStream::new(&self.bytes), &metadata, key, secp_ctx) { + Ok(extracted_payment_id) => payment_id == extracted_payment_id, + Err(()) => false, + } } pub(crate) fn as_tlv_stream(&self) -> FullInvoiceTlvStreamRef { @@ -1006,35 +1026,28 @@ impl InvoiceContents { } fn verify( - &self, tlv_stream: TlvStream<'_>, key: &ExpandedKey, secp_ctx: &Secp256k1 + &self, tlv_stream: TlvStream<'_>, metadata: &Metadata, key: &ExpandedKey, + secp_ctx: &Secp256k1 ) -> Result { let offer_records = tlv_stream.clone().range(OFFER_TYPES); let invreq_records = tlv_stream.range(INVOICE_REQUEST_TYPES).filter(|record| { match record.r#type { PAYER_METADATA_TYPE => false, // Should be outside range - INVOICE_REQUEST_PAYER_ID_TYPE => !self.derives_keys(), + INVOICE_REQUEST_PAYER_ID_TYPE => !metadata.derives_payer_keys(), _ => true, } }); let tlv_stream = offer_records.chain(invreq_records); - let (metadata, payer_id, iv_bytes) = match self { - InvoiceContents::ForOffer { invoice_request, .. } => { - (invoice_request.metadata(), invoice_request.payer_id(), INVOICE_REQUEST_IV_BYTES) - }, - InvoiceContents::ForRefund { refund, .. } => { - (refund.metadata(), refund.payer_id(), REFUND_IV_BYTES) - }, + let payer_id = self.payer_id(); + let iv_bytes = match self { + InvoiceContents::ForOffer { .. } => INVOICE_REQUEST_IV_BYTES, + InvoiceContents::ForRefund { .. } => REFUND_IV_BYTES, }; - signer::verify_payer_metadata(metadata, key, iv_bytes, payer_id, tlv_stream, secp_ctx) - } - - fn derives_keys(&self) -> bool { - match self { - InvoiceContents::ForOffer { invoice_request, .. } => invoice_request.derives_keys(), - InvoiceContents::ForRefund { refund, .. } => refund.derives_keys(), - } + signer::verify_payer_metadata( + metadata.as_ref(), key, iv_bytes, payer_id, tlv_stream, secp_ctx, + ) } fn as_tlv_stream(&self) -> PartialInvoiceTlvStreamRef { diff --git a/lightning/src/offers/invoice_request.rs b/lightning/src/offers/invoice_request.rs index b8e47bac59e..2415d5f46b6 100644 --- a/lightning/src/offers/invoice_request.rs +++ b/lightning/src/offers/invoice_request.rs @@ -636,7 +636,7 @@ pub(super) struct InvoiceRequestContents { #[derive(Clone, Debug)] #[cfg_attr(test, derive(PartialEq))] pub(super) struct InvoiceRequestContentsWithoutPayerId { - payer: PayerContents, + pub(super) payer: PayerContents, pub(super) offer: OfferContents, chain: Option, amount_msats: Option, @@ -953,10 +953,6 @@ impl InvoiceRequestContents { self.inner.metadata() } - pub(super) fn derives_keys(&self) -> bool { - self.inner.payer.0.derives_payer_keys() - } - pub(super) fn chain(&self) -> ChainHash { self.inner.chain() } @@ -1421,6 +1417,7 @@ mod tests { 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 let ( @@ -1494,6 +1491,7 @@ mod tests { 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 let ( diff --git a/lightning/src/offers/refund.rs b/lightning/src/offers/refund.rs index d5171b3a692..fbd4758d6a0 100644 --- a/lightning/src/offers/refund.rs +++ b/lightning/src/offers/refund.rs @@ -415,7 +415,7 @@ pub struct Refund { #[derive(Clone, Debug)] #[cfg_attr(test, derive(PartialEq))] pub(super) struct RefundContents { - payer: PayerContents, + pub(super) payer: PayerContents, // offer fields description: String, absolute_expiry: Option, @@ -727,10 +727,6 @@ impl RefundContents { self.payer_note.as_ref().map(|payer_note| PrintableString(payer_note.as_str())) } - pub(super) fn derives_keys(&self) -> bool { - self.payer.0.derives_payer_keys() - } - pub(super) fn as_tlv_stream(&self) -> RefundTlvStreamRef { let payer = PayerTlvStreamRef { metadata: self.payer.0.as_bytes(), @@ -1049,6 +1045,7 @@ mod tests { 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)); let mut tlv_stream = refund.as_tlv_stream(); tlv_stream.2.amount = Some(2000); @@ -1113,6 +1110,7 @@ mod tests { 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 let mut tlv_stream = refund.as_tlv_stream(); diff --git a/lightning/src/offers/signer.rs b/lightning/src/offers/signer.rs index ca78bc422eb..0f14287608d 100644 --- a/lightning/src/offers/signer.rs +++ b/lightning/src/offers/signer.rs @@ -50,6 +50,11 @@ pub(super) enum Metadata { /// This variant should only be used at verification time, never when building. RecipientData(Nonce), + /// Metadata for deriving keys included as payer data in a blinded path. + /// + /// This variant should only be used at verification time, never when building. + PayerData([u8; PaymentId::LENGTH + Nonce::LENGTH]), + /// Metadata to be derived from message contents and given material. /// /// This variant should only be used at building time. @@ -62,6 +67,16 @@ pub(super) enum Metadata { } impl Metadata { + pub fn payer_data(payment_id: PaymentId, nonce: Nonce, expanded_key: &ExpandedKey) -> Self { + let encrypted_payment_id = expanded_key.crypt_for_offer(payment_id.0, nonce); + + let mut bytes = [0u8; PaymentId::LENGTH + Nonce::LENGTH]; + bytes[..PaymentId::LENGTH].copy_from_slice(encrypted_payment_id.as_slice()); + bytes[PaymentId::LENGTH..].copy_from_slice(nonce.as_slice()); + + Metadata::PayerData(bytes) + } + pub fn as_bytes(&self) -> Option<&Vec> { match self { Metadata::Bytes(bytes) => Some(bytes), @@ -73,6 +88,7 @@ impl Metadata { match self { Metadata::Bytes(_) => false, Metadata::RecipientData(_) => { debug_assert!(false); false }, + Metadata::PayerData(_) => { debug_assert!(false); false }, Metadata::Derived(_) => true, Metadata::DerivedSigningPubkey(_) => true, } @@ -87,6 +103,7 @@ impl Metadata { // Nonce::LENGTH had been set explicitly. Metadata::Bytes(bytes) => bytes.len() == PaymentId::LENGTH + Nonce::LENGTH, Metadata::RecipientData(_) => false, + Metadata::PayerData(_) => true, Metadata::Derived(_) => false, Metadata::DerivedSigningPubkey(_) => true, } @@ -101,6 +118,7 @@ impl Metadata { // been set explicitly. Metadata::Bytes(bytes) => bytes.len() == Nonce::LENGTH, Metadata::RecipientData(_) => true, + Metadata::PayerData(_) => false, Metadata::Derived(_) => false, Metadata::DerivedSigningPubkey(_) => true, } @@ -115,6 +133,7 @@ impl Metadata { match self { Metadata::Bytes(_) => self, Metadata::RecipientData(_) => { debug_assert!(false); self }, + Metadata::PayerData(_) => { debug_assert!(false); self }, Metadata::Derived(_) => self, Metadata::DerivedSigningPubkey(material) => Metadata::Derived(material), } @@ -126,6 +145,7 @@ impl Metadata { match self { Metadata::Bytes(_) => (self, None), Metadata::RecipientData(_) => { debug_assert!(false); (self, None) }, + Metadata::PayerData(_) => { debug_assert!(false); (self, None) }, Metadata::Derived(mut metadata_material) => { tlv_stream.write(&mut metadata_material.hmac).unwrap(); (Metadata::Bytes(metadata_material.derive_metadata()), None) @@ -151,6 +171,7 @@ impl AsRef<[u8]> for Metadata { match self { Metadata::Bytes(bytes) => &bytes, Metadata::RecipientData(nonce) => &nonce.0, + Metadata::PayerData(bytes) => bytes.as_slice(), Metadata::Derived(_) => { debug_assert!(false); &[] }, Metadata::DerivedSigningPubkey(_) => { debug_assert!(false); &[] }, } @@ -162,6 +183,7 @@ impl fmt::Debug for Metadata { match self { Metadata::Bytes(bytes) => bytes.fmt(f), Metadata::RecipientData(Nonce(bytes)) => bytes.fmt(f), + Metadata::PayerData(bytes) => bytes.fmt(f), Metadata::Derived(_) => f.write_str("Derived"), Metadata::DerivedSigningPubkey(_) => f.write_str("DerivedSigningPubkey"), } @@ -178,6 +200,7 @@ impl PartialEq for Metadata { false }, Metadata::RecipientData(_) => false, + Metadata::PayerData(_) => false, Metadata::Derived(_) => false, Metadata::DerivedSigningPubkey(_) => false, } From 14634c6ae12897a39e44ac7760b49130a88bd2c9 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Fri, 12 Jul 2024 16:59:22 -0500 Subject: [PATCH 23/29] Add nonce to OffersContext::OutboundPayment To authenticate that a Bolt12Invoice is for a valid InvoiceRequest or Refund, include the nonce from the payer_metadata in the InvoiceRequest reply path or Refund::paths, respectively. This can be used to prevent de-anonymization attacks where an attacker sends invoices using self-constructed paths to nodes near the blinded paths' introduction nodes. --- lightning/src/blinded_path/message.rs | 11 ++++++++++- lightning/src/ln/channelmanager.rs | 11 ++++++----- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/lightning/src/blinded_path/message.rs b/lightning/src/blinded_path/message.rs index 74d31c5b99e..04cb867dac1 100644 --- a/lightning/src/blinded_path/message.rs +++ b/lightning/src/blinded_path/message.rs @@ -142,7 +142,15 @@ pub enum OffersContext { /// /// [`Refund`]: crate::offers::refund::Refund /// [`InvoiceRequest`]: crate::offers::invoice_request::InvoiceRequest - payment_id: PaymentId + payment_id: PaymentId, + + /// A nonce used for authenticating that a [`Bolt12Invoice`] is for a valid [`Refund`] or + /// [`InvoiceRequest`] and for deriving their signing keys. + /// + /// [`Bolt12Invoice`]: crate::offers::invoice::Bolt12Invoice + /// [`Refund`]: crate::offers::refund::Refund + /// [`InvoiceRequest`]: crate::offers::invoice_request::InvoiceRequest + nonce: Nonce, }, } @@ -158,6 +166,7 @@ impl_writeable_tlv_based_enum!(OffersContext, }, (2, OutboundPayment) => { (0, payment_id, required), + (1, nonce, required), }, ); diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 1988aba4a26..9aa80348eac 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -8862,7 +8862,7 @@ macro_rules! create_refund_builder { ($self: ident, $builder: ty) => { let secp_ctx = &$self.secp_ctx; let nonce = Nonce::from_entropy_source(entropy); - let context = OffersContext::OutboundPayment { payment_id }; + 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)?; @@ -8997,8 +8997,9 @@ where }; let invoice_request = builder.build_and_sign()?; - let context = OffersContext::OutboundPayment { payment_id }; - let reply_paths = self.create_blinded_paths(context).map_err(|_| Bolt12SemanticError::MissingPaths)?; + 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); @@ -10697,7 +10698,7 @@ where let abandon_if_payment = |context| { match context { - OffersContext::OutboundPayment { payment_id } => self.abandon_payment(payment_id), + OffersContext::OutboundPayment { payment_id, .. } => self.abandon_payment(payment_id), _ => {}, } }; @@ -10808,7 +10809,7 @@ where OffersMessage::Invoice(invoice) => { let expected_payment_id = match context { OffersContext::Unknown {} if invoice.is_for_refund_without_paths() => None, - OffersContext::OutboundPayment { payment_id } => Some(payment_id), + OffersContext::OutboundPayment { payment_id, .. } => Some(payment_id), _ => return ResponseInstruction::NoResponse, }; From 2c2f3fe80cbe6f66961fc2ca98d6c46943bad55d Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Fri, 12 Jul 2024 18:16:44 -0500 Subject: [PATCH 24/29] Authenticate Bolt12Invoice using BlindedPath data When a Bolt12Invoice is handled with an OfferContext, use both the containing payment_id and nonce to verify that it is for a pending outbound payment. Previously, the nonce the payment_id were taken from the payer_metadata and the latter was compared against the payment_id in the OfferContext. The payer_metadata thus no longer needs to include either when a blinded path is used. However, some payer_metadata will still be needed as per the spec. --- lightning/src/ln/channelmanager.rs | 51 ++++++++++++++++-------------- 1 file changed, 28 insertions(+), 23 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 9aa80348eac..20bcbdc308c 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -10807,36 +10807,41 @@ where } }, OffersMessage::Invoice(invoice) => { - let expected_payment_id = match context { + let payer_data = match context { OffersContext::Unknown {} if invoice.is_for_refund_without_paths() => None, - OffersContext::OutboundPayment { payment_id, .. } => Some(payment_id), + OffersContext::OutboundPayment { payment_id, nonce } => Some((payment_id, nonce)), _ => return ResponseInstruction::NoResponse, }; - let result = match invoice.verify(expanded_key, secp_ctx) { - Ok(payment_id) => { - if let Some(expected_payment_id) = expected_payment_id { - if payment_id != expected_payment_id { - return ResponseInstruction::NoResponse; - } - } - - 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, responder }; - self.pending_events.lock().unwrap().push_back((event, None)); - 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 { - 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)) - }) + return ResponseInstruction::NoResponse; } }, - Err(()) => return ResponseInstruction::NoResponse, + None => match invoice.verify(expanded_key, secp_ctx) { + 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, 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 { From e6ee1943e0ed33370b20ebd88b188ca9a2e344b5 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Mon, 15 Jul 2024 18:22:43 -0500 Subject: [PATCH 25/29] Include OffersContext in Event::InvoiceReceived When authenticating that an invoice is for a valid invoice request, the payer metadata is needed. Some of this data will be removed in the next commit and instead be included in the message context of the request's reply path. Add this data to Event::InvoiceReceived so that asynchronous invoice handling can verify properly. --- lightning/src/blinded_path/message.rs | 2 +- lightning/src/events/mod.rs | 14 +++++++++++--- lightning/src/ln/channelmanager.rs | 4 +++- lightning/src/offers/nonce.rs | 2 +- 4 files changed, 16 insertions(+), 6 deletions(-) diff --git a/lightning/src/blinded_path/message.rs b/lightning/src/blinded_path/message.rs index 04cb867dac1..15bf1a94940 100644 --- a/lightning/src/blinded_path/message.rs +++ b/lightning/src/blinded_path/message.rs @@ -106,7 +106,7 @@ pub enum MessageContext { /// Contains data specific to an [`OffersMessage`]. /// /// [`OffersMessage`]: crate::onion_message::offers::OffersMessage -#[derive(Clone, Debug)] +#[derive(Clone, Debug, Eq, PartialEq)] pub enum OffersContext { /// Represents an unknown BOLT12 message context. /// diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs index 9b4c203c30d..9aa449efbaa 100644 --- a/lightning/src/events/mod.rs +++ b/lightning/src/events/mod.rs @@ -18,6 +18,7 @@ pub mod bump_transaction; pub use bump_transaction::BumpTransactionEvent; +use crate::blinded_path::message::OffersContext; use crate::blinded_path::payment::{Bolt12OfferContext, Bolt12RefundContext, PaymentContext, PaymentContextRef}; use crate::chain::transaction; use crate::ln::channelmanager::{InterceptId, PaymentId, RecipientOnionFields}; @@ -806,6 +807,10 @@ pub enum Event { payment_id: PaymentId, /// The invoice to pay. invoice: Bolt12Invoice, + /// The context of the [`BlindedPath`] used to send the invoice. + /// + /// [`BlindedPath`]: crate::blinded_path::BlindedPath + context: OffersContext, /// A responder for replying with an [`InvoiceError`] if needed. /// /// `None` if the invoice wasn't sent with a reply path. @@ -1648,12 +1653,13 @@ impl Writeable for Event { (0, peer_node_id, required), }); }, - &Event::InvoiceReceived { ref payment_id, ref invoice, ref responder } => { + &Event::InvoiceReceived { ref payment_id, ref invoice, ref context, ref responder } => { 41u8.write(writer)?; write_tlv_fields!(writer, { (0, payment_id, required), (2, invoice, required), - (4, responder, option), + (4, context, required), + (6, responder, option), }); }, &Event::FundingTxBroadcastSafe { ref channel_id, ref user_channel_id, ref funding_txo, ref counterparty_node_id, ref former_temporary_channel_id} => { @@ -2107,11 +2113,13 @@ impl MaybeReadable for Event { _init_and_read_len_prefixed_tlv_fields!(reader, { (0, payment_id, required), (2, invoice, required), - (4, responder, option), + (4, context, required), + (6, responder, option), }); Ok(Some(Event::InvoiceReceived { payment_id: payment_id.0.unwrap(), invoice: invoice.0.unwrap(), + context: context.0.unwrap(), responder, })) }; diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 20bcbdc308c..9fde3e91741 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -10832,7 +10832,9 @@ where 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, responder }; + let event = Event::InvoiceReceived { + payment_id, invoice, context, responder, + }; self.pending_events.lock().unwrap().push_back((event, None)); return ResponseInstruction::NoResponse; } else { diff --git a/lightning/src/offers/nonce.rs b/lightning/src/offers/nonce.rs index be4c1d3d254..1dd21e6c83d 100644 --- a/lightning/src/offers/nonce.rs +++ b/lightning/src/offers/nonce.rs @@ -26,7 +26,7 @@ use crate::prelude::*; /// [`Offer::metadata`]: crate::offers::offer::Offer::metadata /// [`Offer::signing_pubkey`]: crate::offers::offer::Offer::signing_pubkey /// [`ExpandedKey`]: crate::ln::inbound_payment::ExpandedKey -#[derive(Clone, Copy, Debug, PartialEq)] +#[derive(Clone, Copy, Debug, Eq, PartialEq)] pub struct Nonce(pub(crate) [u8; Self::LENGTH]); impl Nonce { From 4ed37d8e588b2bc6aa6cc1fabf26a63e10374c25 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Fri, 19 Jul 2024 11:50:07 -0500 Subject: [PATCH 26/29] Correct docs --- lightning/src/offers/refund.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lightning/src/offers/refund.rs b/lightning/src/offers/refund.rs index fbd4758d6a0..6c9226779a1 100644 --- a/lightning/src/offers/refund.rs +++ b/lightning/src/offers/refund.rs @@ -189,13 +189,14 @@ macro_rules! refund_builder_methods { ( /// different payer id for each refund, assuming a different nonce is used. Otherwise, the /// provided `node_id` is used for the payer id. /// - /// Also, sets the metadata when [`RefundBuilder::build`] is called such that it can be used to - /// verify that an [`InvoiceRequest`] was produced for the refund given an [`ExpandedKey`]. + /// 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`]. /// /// 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. /// - /// [`InvoiceRequest`]: crate::offers::invoice_request::InvoiceRequest + /// [`Bolt12Invoice::verify`]: crate::offers::invoice::Bolt12Invoice::verify /// [`ExpandedKey`]: crate::ln::inbound_payment::ExpandedKey pub fn deriving_payer_id( node_id: PublicKey, expanded_key: &ExpandedKey, nonce: Nonce, From df5d7ea7b3aeb9e474f5d8d98f87ca6720b4eaab Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Mon, 15 Jul 2024 18:29:41 -0500 Subject: [PATCH 27/29] 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 9fde3e91741..a5fe80a3a70 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 1dcc4677798..627fc812646 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 2415d5f46b6..8111e995813 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 6c9226779a1..a98c606abd2 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 0f14287608d..2e12a17056e 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(); From 718bc4766496c2f7b8fad8c6b9337a1c8f28fb4b Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Fri, 19 Jul 2024 11:17:51 -0500 Subject: [PATCH 28/29] Rename Bolt12Invoice::verify --- lightning/src/ln/channelmanager.rs | 2 +- lightning/src/offers/invoice.rs | 2 +- lightning/src/offers/invoice_request.rs | 8 ++++---- lightning/src/offers/offer.rs | 7 ++++--- lightning/src/offers/refund.rs | 16 ++++++++-------- 5 files changed, 18 insertions(+), 17 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index a5fe80a3a70..3e4ac872e1b 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -4220,7 +4220,7 @@ where match context { OffersContext::Unknown {} if invoice.is_for_refund_without_paths() => { - invoice.verify(expanded_key, secp_ctx) + invoice.verify_using_metadata(expanded_key, secp_ctx) }, OffersContext::OutboundPayment { payment_id, nonce } => { invoice diff --git a/lightning/src/offers/invoice.rs b/lightning/src/offers/invoice.rs index 5ea2c2e6041..e1f30138212 100644 --- a/lightning/src/offers/invoice.rs +++ b/lightning/src/offers/invoice.rs @@ -775,7 +775,7 @@ impl Bolt12Invoice { /// checking the payer metadata from the invoice request. /// /// Returns the associated [`PaymentId`] to use when sending the payment. - pub fn verify( + pub fn verify_using_metadata( &self, key: &ExpandedKey, secp_ctx: &Secp256k1 ) -> Result { let metadata = match &self.contents { diff --git a/lightning/src/offers/invoice_request.rs b/lightning/src/offers/invoice_request.rs index 8111e995813..4ad99645002 100644 --- a/lightning/src/offers/invoice_request.rs +++ b/lightning/src/offers/invoice_request.rs @@ -1413,7 +1413,7 @@ mod tests { .unwrap() .build().unwrap() .sign(recipient_sign).unwrap(); - match invoice.verify(&expanded_key, &secp_ctx) { + match invoice.verify_using_metadata(&expanded_key, &secp_ctx) { Ok(payment_id) => assert_eq!(payment_id, PaymentId([1; 32])), Err(()) => panic!("verification failed"), } @@ -1440,7 +1440,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_metadata(&expanded_key, &secp_ctx).is_err()); // Fails verification with altered metadata let ( @@ -1463,7 +1463,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_metadata(&expanded_key, &secp_ctx).is_err()); } #[test] @@ -1487,7 +1487,7 @@ mod tests { .unwrap() .build().unwrap() .sign(recipient_sign).unwrap(); - assert!(invoice.verify(&expanded_key, &secp_ctx).is_err()); + assert!(invoice.verify_using_metadata(&expanded_key, &secp_ctx).is_err()); assert!(invoice.verify_using_payer_data(payment_id, nonce, &expanded_key, &secp_ctx)); // Fails verification with altered fields diff --git a/lightning/src/offers/offer.rs b/lightning/src/offers/offer.rs index b5e9154d53a..29220125f66 100644 --- a/lightning/src/offers/offer.rs +++ b/lightning/src/offers/offer.rs @@ -685,8 +685,9 @@ macro_rules! request_invoice_derived_payer_id { ($self: ident, $builder: ty) => /// - derives the [`InvoiceRequest::payer_id`] such that a different key can be used for each /// request, /// - sets [`InvoiceRequest::payer_metadata`] when [`InvoiceRequestBuilder::build`] is called - /// such that it can be used by [`Bolt12Invoice::verify`] to determine if the invoice was - /// requested using a base [`ExpandedKey`] from which the payer id was derived, and + /// such that it can be used by [`Bolt12Invoice::verify_using_metadata`] to determine if the + /// invoice was requested using a base [`ExpandedKey`] from which the payer id was derived, + /// and /// - includes the [`PaymentId`] encrypted in [`InvoiceRequest::payer_metadata`] so that it can /// be used when sending the payment for the requested invoice. /// @@ -694,7 +695,7 @@ macro_rules! request_invoice_derived_payer_id { ($self: ident, $builder: ty) => /// /// [`InvoiceRequest::payer_id`]: crate::offers::invoice_request::InvoiceRequest::payer_id /// [`InvoiceRequest::payer_metadata`]: crate::offers::invoice_request::InvoiceRequest::payer_metadata - /// [`Bolt12Invoice::verify`]: crate::offers::invoice::Bolt12Invoice::verify + /// [`Bolt12Invoice::verify_using_metadata`]: crate::offers::invoice::Bolt12Invoice::verify_using_metadata /// [`ExpandedKey`]: crate::ln::inbound_payment::ExpandedKey pub fn request_invoice_deriving_payer_id< 'a, 'b, diff --git a/lightning/src/offers/refund.rs b/lightning/src/offers/refund.rs index a98c606abd2..9cfa3147c63 100644 --- a/lightning/src/offers/refund.rs +++ b/lightning/src/offers/refund.rs @@ -190,15 +190,15 @@ macro_rules! refund_builder_methods { ( /// provided `node_id` is used for the payer id. /// /// 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`]. However, if [`RefundBuilder::path`] is called, then the metadata must be - /// included in each [`BlindedPath`] instead. In this case, use + /// [`Bolt12Invoice::verify_using_metadata`] to determine if the invoice was produced for the + /// refund given an [`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_metadata`]: crate::offers::invoice::Bolt12Invoice::verify_using_metadata /// [`Bolt12Invoice::verify_using_payer_data`]: crate::offers::invoice::Bolt12Invoice::verify_using_payer_data /// [`ExpandedKey`]: crate::ln::inbound_payment::ExpandedKey pub fn deriving_payer_id( @@ -1045,7 +1045,7 @@ mod tests { .unwrap() .build().unwrap() .sign(recipient_sign).unwrap(); - match invoice.verify(&expanded_key, &secp_ctx) { + match invoice.verify_using_metadata(&expanded_key, &secp_ctx) { Ok(payment_id) => assert_eq!(payment_id, PaymentId([1; 32])), Err(()) => panic!("verification failed"), } @@ -1062,7 +1062,7 @@ mod tests { .unwrap() .build().unwrap() .sign(recipient_sign).unwrap(); - assert!(invoice.verify(&expanded_key, &secp_ctx).is_err()); + assert!(invoice.verify_using_metadata(&expanded_key, &secp_ctx).is_err()); // Fails verification with altered metadata let mut tlv_stream = refund.as_tlv_stream(); @@ -1077,7 +1077,7 @@ mod tests { .unwrap() .build().unwrap() .sign(recipient_sign).unwrap(); - assert!(invoice.verify(&expanded_key, &secp_ctx).is_err()); + assert!(invoice.verify_using_metadata(&expanded_key, &secp_ctx).is_err()); } #[test] @@ -1110,7 +1110,7 @@ mod tests { .unwrap() .build().unwrap() .sign(recipient_sign).unwrap(); - assert!(invoice.verify(&expanded_key, &secp_ctx).is_err()); + assert!(invoice.verify_using_metadata(&expanded_key, &secp_ctx).is_err()); assert!(invoice.verify_using_payer_data(payment_id, nonce, &expanded_key, &secp_ctx)); // Fails verification with altered fields From 825bda03c9354d79e1095501e22c24f13be4c7c6 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 17 Jul 2024 15:38:15 -0500 Subject: [PATCH 29/29] Add pending changelog for BOLT12 authentication --- pending_changelog/3139-bolt12-compatability.txt | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 pending_changelog/3139-bolt12-compatability.txt diff --git a/pending_changelog/3139-bolt12-compatability.txt b/pending_changelog/3139-bolt12-compatability.txt new file mode 100644 index 00000000000..7907ccf6b1f --- /dev/null +++ b/pending_changelog/3139-bolt12-compatability.txt @@ -0,0 +1,10 @@ +## Backwards Compatibility + * BOLT12 `Offers` created in prior versions are still valid but are at risk of + de-anonymization attacks. + * BOLT12 outbound payments in state `RecentPaymentDetails::AwaitingInvoice` are + considered invalid by `ChannelManager`. Any invoices received through the + corresponding `InvoiceRequest` reply path will be ignored. + * BOLT12 `Refund`s created in prior versions with non-empty `Refund::paths` are + considered invalid by `ChannelManager`. Any invoices sent through those + blinded paths will be ignored. `Refund`'s without blinded paths are + unaffected.