Skip to content

Commit bd3ce45

Browse files
valentinewallacevalentinewallace
committed
Enforce onion message has correct context when peeling
Previously the compiler would not enforce that a peeled onion message had a corresponding context that matched, e.g. an offers message needs an offers blinded path context. Refactor PeeledOnion to have multiple Receive message types such that this is now enforced.
1 parent 74fcc24 commit bd3ce45

File tree

3 files changed

+101
-116
lines changed

3 files changed

+101
-116
lines changed

‎fuzz/src/onion_message.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -343,8 +343,7 @@ mod tests {
343343
assert_eq!(
344344
log_entries.get(&(
345345
"lightning::onion_message::messenger".to_string(),
346-
"Received an onion message with a reply_path: Custom(TestCustomMessage)"
347-
.to_string()
346+
"Received an onion message with a reply_path: TestCustomMessage".to_string()
348347
)),
349348
Some(&1)
350349
);

‎lightning/src/ln/offers_tests.rs

+25-31
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ use core::time::Duration;
4646
use crate::blinded_path::IntroductionNode;
4747
use crate::blinded_path::message::BlindedMessagePath;
4848
use crate::blinded_path::payment::{Bolt12OfferContext, Bolt12RefundContext, PaymentContext};
49-
use crate::blinded_path::message::{MessageContext, OffersContext};
49+
use crate::blinded_path::message::OffersContext;
5050
use crate::events::{ClosureReason, Event, HTLCDestination, PaymentFailureReason, PaymentPurpose};
5151
use crate::ln::channelmanager::{Bolt12PaymentError, MAX_SHORT_LIVED_RELATIVE_EXPIRY, PaymentId, RecentPaymentDetails, RecipientOnionFields, Retry, self};
5252
use crate::types::features::Bolt12InvoiceFeatures;
@@ -60,7 +60,6 @@ use crate::offers::nonce::Nonce;
6060
use crate::offers::parse::Bolt12SemanticError;
6161
use crate::onion_message::messenger::{Destination, PeeledOnion, MessageSendInstructions};
6262
use crate::onion_message::offers::OffersMessage;
63-
use crate::onion_message::packet::ParsedOnionMessageContents;
6463
use crate::routing::gossip::{NodeAlias, NodeId};
6564
use crate::routing::router::{PaymentParameters, RouteParameters, RouteParametersConfig};
6665
use crate::sign::{NodeSigner, Recipient};
@@ -193,9 +192,10 @@ fn claim_bolt12_payment<'a, 'b, 'c>(
193192

194193
fn extract_offer_nonce<'a, 'b, 'c>(node: &Node<'a, 'b, 'c>, message: &OnionMessage) -> Nonce {
195194
match node.onion_messenger.peel_onion_message(message) {
196-
Ok(PeeledOnion::Receive(_, Some(MessageContext::Offers(OffersContext::InvoiceRequest { nonce })), _)) => nonce,
197-
Ok(PeeledOnion::Receive(_, context, _)) => panic!("Unexpected onion message context: {:?}", context),
195+
Ok(PeeledOnion::Offers(_, Some(OffersContext::InvoiceRequest { nonce }), _)) => nonce,
196+
Ok(PeeledOnion::Offers(_, context, _)) => panic!("Unexpected onion message context: {:?}", context),
198197
Ok(PeeledOnion::Forward(_, _)) => panic!("Unexpected onion message forward"),
198+
Ok(_) => panic!("Unexpected onion message"),
199199
Err(e) => panic!("Failed to process onion message {:?}", e),
200200
}
201201
}
@@ -204,34 +204,30 @@ pub(super) fn extract_invoice_request<'a, 'b, 'c>(
204204
node: &Node<'a, 'b, 'c>, message: &OnionMessage
205205
) -> (InvoiceRequest, BlindedMessagePath) {
206206
match node.onion_messenger.peel_onion_message(message) {
207-
Ok(PeeledOnion::Receive(message, _, reply_path)) => match message {
208-
ParsedOnionMessageContents::Offers(offers_message) => match offers_message {
209-
OffersMessage::InvoiceRequest(invoice_request) => (invoice_request, reply_path.unwrap()),
210-
OffersMessage::Invoice(invoice) => panic!("Unexpected invoice: {:?}", invoice),
211-
#[cfg(async_payments)]
212-
OffersMessage::StaticInvoice(invoice) => panic!("Unexpected static invoice: {:?}", invoice),
213-
OffersMessage::InvoiceError(error) => panic!("Unexpected invoice_error: {:?}", error),
214-
},
215-
_ => panic!("Unexpected message {:?}", message),
207+
Ok(PeeledOnion::Offers(message, _, reply_path)) => match message {
208+
OffersMessage::InvoiceRequest(invoice_request) => (invoice_request, reply_path.unwrap()),
209+
OffersMessage::Invoice(invoice) => panic!("Unexpected invoice: {:?}", invoice),
210+
#[cfg(async_payments)]
211+
OffersMessage::StaticInvoice(invoice) => panic!("Unexpected static invoice: {:?}", invoice),
212+
OffersMessage::InvoiceError(error) => panic!("Unexpected invoice_error: {:?}", error),
216213
},
217214
Ok(PeeledOnion::Forward(_, _)) => panic!("Unexpected onion message forward"),
215+
Ok(_) => panic!("Unexpected onion message"),
218216
Err(e) => panic!("Failed to process onion message {:?}", e),
219217
}
220218
}
221219

222220
fn extract_invoice<'a, 'b, 'c>(node: &Node<'a, 'b, 'c>, message: &OnionMessage) -> (Bolt12Invoice, BlindedMessagePath) {
223221
match node.onion_messenger.peel_onion_message(message) {
224-
Ok(PeeledOnion::Receive(message, _, reply_path)) => match message {
225-
ParsedOnionMessageContents::Offers(offers_message) => match offers_message {
226-
OffersMessage::InvoiceRequest(invoice_request) => panic!("Unexpected invoice_request: {:?}", invoice_request),
227-
OffersMessage::Invoice(invoice) => (invoice, reply_path.unwrap()),
228-
#[cfg(async_payments)]
229-
OffersMessage::StaticInvoice(invoice) => panic!("Unexpected static invoice: {:?}", invoice),
230-
OffersMessage::InvoiceError(error) => panic!("Unexpected invoice_error: {:?}", error),
231-
},
232-
_ => panic!("Unexpected message {:?}", message),
222+
Ok(PeeledOnion::Offers(message, _, reply_path)) => match message {
223+
OffersMessage::InvoiceRequest(invoice_request) => panic!("Unexpected invoice_request: {:?}", invoice_request),
224+
OffersMessage::Invoice(invoice) => (invoice, reply_path.unwrap()),
225+
#[cfg(async_payments)]
226+
OffersMessage::StaticInvoice(invoice) => panic!("Unexpected static invoice: {:?}", invoice),
227+
OffersMessage::InvoiceError(error) => panic!("Unexpected invoice_error: {:?}", error),
233228
},
234229
Ok(PeeledOnion::Forward(_, _)) => panic!("Unexpected onion message forward"),
230+
Ok(_) => panic!("Unexpected onion message"),
235231
Err(e) => panic!("Failed to process onion message {:?}", e),
236232
}
237233
}
@@ -240,17 +236,15 @@ fn extract_invoice_error<'a, 'b, 'c>(
240236
node: &Node<'a, 'b, 'c>, message: &OnionMessage
241237
) -> InvoiceError {
242238
match node.onion_messenger.peel_onion_message(message) {
243-
Ok(PeeledOnion::Receive(message, _, _)) => match message {
244-
ParsedOnionMessageContents::Offers(offers_message) => match offers_message {
245-
OffersMessage::InvoiceRequest(invoice_request) => panic!("Unexpected invoice_request: {:?}", invoice_request),
246-
OffersMessage::Invoice(invoice) => panic!("Unexpected invoice: {:?}", invoice),
247-
#[cfg(async_payments)]
248-
OffersMessage::StaticInvoice(invoice) => panic!("Unexpected invoice: {:?}", invoice),
249-
OffersMessage::InvoiceError(error) => error,
250-
},
251-
_ => panic!("Unexpected message: {:?}", message),
239+
Ok(PeeledOnion::Offers(message, _, _)) => match message {
240+
OffersMessage::InvoiceRequest(invoice_request) => panic!("Unexpected invoice_request: {:?}", invoice_request),
241+
OffersMessage::Invoice(invoice) => panic!("Unexpected invoice: {:?}", invoice),
242+
#[cfg(async_payments)]
243+
OffersMessage::StaticInvoice(invoice) => panic!("Unexpected invoice: {:?}", invoice),
244+
OffersMessage::InvoiceError(error) => error,
252245
},
253246
Ok(PeeledOnion::Forward(_, _)) => panic!("Unexpected onion message forward"),
247+
Ok(_) => panic!("Unexpected onion message"),
254248
Err(e) => panic!("Failed to process onion message {:?}", e),
255249
}
256250
}

‎lightning/src/onion_message/messenger.rs

+75-83
Original file line numberDiff line numberDiff line change
@@ -19,16 +19,18 @@ use bitcoin::secp256k1::{self, PublicKey, Scalar, Secp256k1, SecretKey};
1919
use super::async_payments::AsyncPaymentsMessage;
2020
use super::async_payments::AsyncPaymentsMessageHandler;
2121
use super::dns_resolution::{DNSResolverMessage, DNSResolverMessageHandler};
22-
use super::offers::OffersMessageHandler;
22+
use super::offers::{OffersMessage, OffersMessageHandler};
2323
use super::packet::OnionMessageContents;
2424
use super::packet::ParsedOnionMessageContents;
2525
use super::packet::{
2626
ForwardControlTlvs, Packet, Payload, ReceiveControlTlvs, BIG_PACKET_HOP_DATA_LEN,
2727
SMALL_PACKET_HOP_DATA_LEN,
2828
};
29+
#[cfg(async_payments)]
30+
use crate::blinded_path::message::AsyncPaymentsContext;
2931
use crate::blinded_path::message::{
30-
BlindedMessagePath, ForwardTlvs, MessageContext, MessageForwardNode, NextMessageHop,
31-
ReceiveTlvs,
32+
BlindedMessagePath, DNSResolverContext, ForwardTlvs, MessageContext, MessageForwardNode,
33+
NextMessageHop, OffersContext, ReceiveTlvs,
3234
};
3335
use crate::blinded_path::utils;
3436
use crate::blinded_path::{IntroductionNode, NodeIdLookUp};
@@ -897,8 +899,15 @@ pub trait CustomOnionMessageHandler {
897899
pub enum PeeledOnion<T: OnionMessageContents> {
898900
/// Forwarded onion, with the next node id and a new onion
899901
Forward(NextMessageHop, OnionMessage),
900-
/// Received onion message, with decrypted contents, context, and reply path
901-
Receive(ParsedOnionMessageContents<T>, Option<MessageContext>, Option<BlindedMessagePath>),
902+
/// Received offers onion message, with decrypted contents, context, and reply path
903+
Offers(OffersMessage, Option<OffersContext>, Option<BlindedMessagePath>),
904+
/// Received async payments onion message, with decrypted contents, context, and reply path
905+
#[cfg(async_payments)]
906+
AsyncPayments(AsyncPaymentsMessage, AsyncPaymentsContext, Option<BlindedMessagePath>),
907+
/// Received DNS resolver onion message, with decrypted contents, context, and reply path
908+
DNSResolver(DNSResolverMessage, Option<DNSResolverContext>, Option<BlindedMessagePath>),
909+
/// Received custom onion message, with decrypted contents, context, and reply path
910+
Custom(T, Option<Vec<u8>>, Option<BlindedMessagePath>),
902911
}
903912

904913
/// Creates an [`OnionMessage`] with the given `contents` for sending to the destination of
@@ -1073,26 +1082,35 @@ where
10731082
reply_path,
10741083
},
10751084
None,
1076-
)) => match (&message, &context) {
1077-
(_, None) => Ok(PeeledOnion::Receive(message, None, reply_path)),
1078-
(ParsedOnionMessageContents::Offers(_), Some(MessageContext::Offers(_))) => {
1079-
Ok(PeeledOnion::Receive(message, context, reply_path))
1085+
)) => match (message, context) {
1086+
(ParsedOnionMessageContents::Offers(msg), Some(MessageContext::Offers(ctx))) => {
1087+
Ok(PeeledOnion::Offers(msg, Some(ctx), reply_path))
1088+
},
1089+
(ParsedOnionMessageContents::Offers(msg), None) => {
1090+
Ok(PeeledOnion::Offers(msg, None, reply_path))
10801091
},
10811092
#[cfg(async_payments)]
10821093
(
1083-
ParsedOnionMessageContents::AsyncPayments(_),
1084-
Some(MessageContext::AsyncPayments(_)),
1085-
) => Ok(PeeledOnion::Receive(message, context, reply_path)),
1086-
(ParsedOnionMessageContents::Custom(_), Some(MessageContext::Custom(_))) => {
1087-
Ok(PeeledOnion::Receive(message, context, reply_path))
1094+
ParsedOnionMessageContents::AsyncPayments(msg),
1095+
Some(MessageContext::AsyncPayments(ctx)),
1096+
) => Ok(PeeledOnion::AsyncPayments(msg, ctx, reply_path)),
1097+
(ParsedOnionMessageContents::Custom(msg), Some(MessageContext::Custom(ctx))) => {
1098+
Ok(PeeledOnion::Custom(msg, Some(ctx), reply_path))
1099+
},
1100+
(ParsedOnionMessageContents::Custom(msg), None) => {
1101+
Ok(PeeledOnion::Custom(msg, None, reply_path))
10881102
},
1089-
(ParsedOnionMessageContents::DNSResolver(_), Some(MessageContext::DNSResolver(_))) => {
1090-
Ok(PeeledOnion::Receive(message, context, reply_path))
1103+
(
1104+
ParsedOnionMessageContents::DNSResolver(msg),
1105+
Some(MessageContext::DNSResolver(ctx)),
1106+
) => Ok(PeeledOnion::DNSResolver(msg, Some(ctx), reply_path)),
1107+
(ParsedOnionMessageContents::DNSResolver(msg), None) => {
1108+
Ok(PeeledOnion::DNSResolver(msg, None, reply_path))
10911109
},
10921110
_ => {
10931111
log_trace!(
10941112
logger,
1095-
"Received message was sent on a blinded path with the wrong context."
1113+
"Received message was sent on a blinded path with wrong or missing context."
10961114
);
10971115
Err(())
10981116
},
@@ -1894,98 +1912,72 @@ where
18941912
{
18951913
fn handle_onion_message(&self, peer_node_id: PublicKey, msg: &OnionMessage) {
18961914
let logger = WithContext::from(&self.logger, Some(peer_node_id), None, None);
1897-
match self.peel_onion_message(msg) {
1898-
Ok(PeeledOnion::Receive(message, context, reply_path)) => {
1915+
macro_rules! log_receive {
1916+
($message: expr, $with_reply_path: expr) => {
18991917
log_trace!(
19001918
logger,
19011919
"Received an onion message with {} reply_path: {:?}",
1902-
if reply_path.is_some() { "a" } else { "no" },
1903-
message
1920+
if $with_reply_path { "a" } else { "no" },
1921+
$message
19041922
);
1923+
};
1924+
}
19051925

1926+
match self.peel_onion_message(msg) {
1927+
Ok(PeeledOnion::Offers(message, context, reply_path)) => {
1928+
log_receive!(message, reply_path.is_some());
1929+
let responder = reply_path.map(Responder::new);
1930+
let response_instructions =
1931+
self.offers_handler.handle_message(message, context, responder);
1932+
if let Some((msg, instructions)) = response_instructions {
1933+
let _ = self.handle_onion_message_response(msg, instructions);
1934+
}
1935+
},
1936+
#[cfg(async_payments)]
1937+
Ok(PeeledOnion::AsyncPayments(message, context, reply_path)) => {
1938+
log_receive!(message, reply_path.is_some());
19061939
let responder = reply_path.map(Responder::new);
19071940
match message {
1908-
ParsedOnionMessageContents::Offers(msg) => {
1909-
let context = match context {
1910-
None => None,
1911-
Some(MessageContext::Offers(context)) => Some(context),
1912-
_ => {
1913-
debug_assert!(false, "Checked in peel_onion_message");
1914-
return;
1915-
},
1916-
};
1917-
let response_instructions =
1918-
self.offers_handler.handle_message(msg, context, responder);
1919-
if let Some((msg, instructions)) = response_instructions {
1920-
let _ = self.handle_onion_message_response(msg, instructions);
1921-
}
1922-
},
1923-
#[cfg(async_payments)]
1924-
ParsedOnionMessageContents::AsyncPayments(
1925-
AsyncPaymentsMessage::HeldHtlcAvailable(msg),
1926-
) => {
1927-
let context = match context {
1928-
Some(MessageContext::AsyncPayments(context)) => context,
1929-
Some(_) => {
1930-
debug_assert!(false, "Checked in peel_onion_message");
1931-
return;
1932-
},
1933-
None => return,
1934-
};
1941+
AsyncPaymentsMessage::HeldHtlcAvailable(msg) => {
19351942
let response_instructions = self
19361943
.async_payments_handler
19371944
.handle_held_htlc_available(msg, context, responder);
19381945
if let Some((msg, instructions)) = response_instructions {
19391946
let _ = self.handle_onion_message_response(msg, instructions);
19401947
}
19411948
},
1942-
#[cfg(async_payments)]
1943-
ParsedOnionMessageContents::AsyncPayments(
1944-
AsyncPaymentsMessage::ReleaseHeldHtlc(msg),
1945-
) => {
1946-
let context = match context {
1947-
Some(MessageContext::AsyncPayments(context)) => context,
1948-
Some(_) => {
1949-
debug_assert!(false, "Checked in peel_onion_message");
1950-
return;
1951-
},
1952-
None => return,
1953-
};
1949+
AsyncPaymentsMessage::ReleaseHeldHtlc(msg) => {
19541950
self.async_payments_handler.handle_release_held_htlc(msg, context);
19551951
},
1956-
ParsedOnionMessageContents::DNSResolver(DNSResolverMessage::DNSSECQuery(
1957-
msg,
1958-
)) => {
1952+
}
1953+
},
1954+
Ok(PeeledOnion::DNSResolver(message, context, reply_path)) => {
1955+
log_receive!(message, reply_path.is_some());
1956+
let responder = reply_path.map(Responder::new);
1957+
match message {
1958+
DNSResolverMessage::DNSSECQuery(msg) => {
19591959
let response_instructions =
19601960
self.dns_resolver_handler.handle_dnssec_query(msg, responder);
19611961
if let Some((msg, instructions)) = response_instructions {
19621962
let _ = self.handle_onion_message_response(msg, instructions);
19631963
}
19641964
},
1965-
ParsedOnionMessageContents::DNSResolver(DNSResolverMessage::DNSSECProof(
1966-
msg,
1967-
)) => {
1965+
DNSResolverMessage::DNSSECProof(msg) => {
19681966
let context = match context {
1969-
Some(MessageContext::DNSResolver(context)) => context,
1970-
_ => return,
1967+
Some(ctx) => ctx,
1968+
None => return,
19711969
};
19721970
self.dns_resolver_handler.handle_dnssec_proof(msg, context);
19731971
},
1974-
ParsedOnionMessageContents::Custom(msg) => {
1975-
let context = match context {
1976-
None => None,
1977-
Some(MessageContext::Custom(data)) => Some(data),
1978-
_ => {
1979-
debug_assert!(false, "Checked in peel_onion_message");
1980-
return;
1981-
},
1982-
};
1983-
let response_instructions =
1984-
self.custom_handler.handle_custom_message(msg, context, responder);
1985-
if let Some((msg, instructions)) = response_instructions {
1986-
let _ = self.handle_onion_message_response(msg, instructions);
1987-
}
1988-
},
1972+
}
1973+
},
1974+
Ok(PeeledOnion::Custom(message, context, reply_path)) => {
1975+
log_receive!(message, reply_path.is_some());
1976+
let responder = reply_path.map(Responder::new);
1977+
let response_instructions =
1978+
self.custom_handler.handle_custom_message(message, context, responder);
1979+
if let Some((msg, instructions)) = response_instructions {
1980+
let _ = self.handle_onion_message_response(msg, instructions);
19891981
}
19901982
},
19911983
Ok(PeeledOnion::Forward(next_hop, onion_message)) => {

0 commit comments

Comments
 (0)