Always pad BlindedMessagePath hop data to a consistent length

If we're building a blinded message path with extra dummy hops, we
have to ensure we at least hide the length of the data in pre-final
hops as otherwise the dummy hops are trivially obvious. Here we do
so, taking an extra `bool` parameter to `BlindedMessagePath`
constructors to decide whether to pad every hop to the existing
`MESSAGE_PADDING_ROUND_OFF` or whether to only ensure that each
non-final hop has an identical hop data length.

In cases where the `DefaultMessageRouter` opts to use compact
paths, it now also selects compact padding, whether short channel
IDs are available or not.

Author: TheBlueMatt

lightning-dns-resolver/src/lib.rs

@@ -236,6 +236,7 @@ mod test {
 				recipient,
 				local_node_receive_key,
 				context,
+				false,
 				&keys,
 				secp_ctx,
 			)])
@@ -345,6 +346,7 @@ mod test {
 			payer_id,
 			receive_key,
 			query_context,
+			false,
 			&*payer_keys,
 			&secp_ctx,
 		);

lightning/src/blinded_path/message.rs

@@ -54,21 +54,37 @@ impl Readable for BlindedMessagePath {
 
 impl BlindedMessagePath {
 	/// Create a one-hop blinded path for a message.
+	///
+	/// `compact_padding` selects between space-inefficient padding that better hides contents and
+	/// a space-constrained padding that does very little to hide the contents, especially for the
+	/// last hop. It should only be set when the blinded path needs to be as compact as possible.
 	pub fn one_hop<ES: Deref, T: secp256k1::Signing + secp256k1::Verification>(
 		recipient_node_id: PublicKey, local_node_receive_key: ReceiveAuthKey,
-		context: MessageContext, entropy_source: ES, secp_ctx: &Secp256k1<T>,
+		context: MessageContext, compact_padding: bool, entropy_source: ES, secp_ctx: &Secp256k1<T>,
 	) -> Self
 	where
 		ES::Target: EntropySource,
 	{
-		Self::new(&[], recipient_node_id, local_node_receive_key, context, entropy_source, secp_ctx)
+		Self::new(
+			&[],
+			recipient_node_id,
+			local_node_receive_key,
+			context,
+			compact_padding,
+			entropy_source,
+			secp_ctx,
+		)
 	}
 
 	/// Create a path for an onion message, to be forwarded along `node_pks`.
+	///
+	/// `compact_padding` selects between space-inefficient padding that better hides contents and
+	/// a space-constrained padding that does very little to hide the contents, especially for the
+	/// last hop. It should only be set when the blinded path needs to be as compact as possible.
 	pub fn new<ES: Deref, T: secp256k1::Signing + secp256k1::Verification>(
 		intermediate_nodes: &[MessageForwardNode], recipient_node_id: PublicKey,
-		local_node_receive_key: ReceiveAuthKey, context: MessageContext, entropy_source: ES,
-		secp_ctx: &Secp256k1<T>,
+		local_node_receive_key: ReceiveAuthKey, context: MessageContext, compact_padding: bool,
+		entropy_source: ES, secp_ctx: &Secp256k1<T>,
 	) -> Self
 	where
 		ES::Target: EntropySource,
@@ -79,19 +95,24 @@ impl BlindedMessagePath {
 			0,
 			local_node_receive_key,
 			context,
+			compact_padding,
 			entropy_source,
 			secp_ctx,
 		)
 	}
 
 	/// Same as [`BlindedMessagePath::new`], but allows specifying a number of dummy hops.
 	///
-	/// Note:
-	/// At most [`MAX_DUMMY_HOPS_COUNT`] dummy hops can be added to the blinded path.
+	///
+	/// `compact_padding` selects between space-inefficient padding that better hides contents and
+	/// a space-constrained padding that does very little to hide the contents, especially for the
+	/// last hop. It should only be set when the blinded path needs to be as compact as possible.
+	///
+	/// Note: At most [`MAX_DUMMY_HOPS_COUNT`] dummy hops can be added to the blinded path.
 	pub fn new_with_dummy_hops<ES: Deref, T: secp256k1::Signing + secp256k1::Verification>(
 		intermediate_nodes: &[MessageForwardNode], recipient_node_id: PublicKey,
 		dummy_hop_count: usize, local_node_receive_key: ReceiveAuthKey, context: MessageContext,
-		entropy_source: ES, secp_ctx: &Secp256k1<T>,
+		compact_padding: bool, entropy_source: ES, secp_ctx: &Secp256k1<T>,
 	) -> Self
 	where
 		ES::Target: EntropySource,
@@ -114,6 +135,7 @@ impl BlindedMessagePath {
 				context,
 				&blinding_secret,
 				local_node_receive_key,
+				compact_padding,
 			),
 		})
 	}
@@ -714,7 +736,7 @@ pub const MAX_DUMMY_HOPS_COUNT: usize = 10;
 pub(super) fn blinded_hops<T: secp256k1::Signing + secp256k1::Verification>(
 	secp_ctx: &Secp256k1<T>, intermediate_nodes: &[MessageForwardNode],
 	recipient_node_id: PublicKey, dummy_hop_count: usize, context: MessageContext,
-	session_priv: &SecretKey, local_node_receive_key: ReceiveAuthKey,
+	session_priv: &SecretKey, local_node_receive_key: ReceiveAuthKey, compact_padding: bool,
 ) -> Vec<BlindedHop> {
 	let dummy_count = cmp::min(dummy_hop_count, MAX_DUMMY_HOPS_COUNT);
 	let pks = intermediate_nodes
@@ -724,9 +746,8 @@ pub(super) fn blinded_hops<T: secp256k1::Signing + secp256k1::Verification>(
 			core::iter::repeat((recipient_node_id, Some(local_node_receive_key))).take(dummy_count),
 		)
 		.chain(core::iter::once((recipient_node_id, Some(local_node_receive_key))));
-	let is_compact = intermediate_nodes.iter().any(|node| node.short_channel_id.is_some());
 
-	let tlvs = pks
+	let intermediate_tlvs = pks
 		.clone()
 		.skip(1) // The first node's TLVs contains the next node's pubkey
 		.zip(intermediate_nodes.iter().map(|node| node.short_channel_id))
@@ -737,18 +758,44 @@ pub(super) fn blinded_hops<T: secp256k1::Signing + secp256k1::Verification>(
 		.map(|next_hop| {
 			ControlTlvs::Forward(ForwardTlvs { next_hop, next_blinding_override: None })
 		})
-		.chain((0..dummy_count).map(|_| ControlTlvs::Dummy))
-		.chain(core::iter::once(ControlTlvs::Receive(ReceiveTlvs { context: Some(context) })));
-
-	if is_compact {
-		let path = pks.zip(tlvs);
-		utils::construct_blinded_hops(secp_ctx, path, session_priv)
+		.chain((0..dummy_count).map(|_| ControlTlvs::Dummy));
+
+	let max_intermediate_len =
+		intermediate_tlvs.clone().map(|tlvs| tlvs.serialized_length()).max().unwrap_or(0);
+	let have_intermediate_one_byte_smaller =
+		intermediate_tlvs.clone().any(|tlvs| tlvs.serialized_length() == max_intermediate_len - 1);
+
+	let round_off = if compact_padding {
+		// We can only pad by a minimum of two bytes. Thus, if there are any intermediate hops that
+		// need to be padded by exactly one byte, we have to instead pad everything by two.
+		if have_intermediate_one_byte_smaller {
+			max_intermediate_len + 2
+		} else {
+			max_intermediate_len
+		}
 	} else {
-		let path =
-			pks.zip(tlvs.map(|tlv| BlindedPathWithPadding {
-				tlvs: tlv,
-				round_off: MESSAGE_PADDING_ROUND_OFF,
-			}));
-		utils::construct_blinded_hops(secp_ctx, path, session_priv)
-	}
+		MESSAGE_PADDING_ROUND_OFF
+	};
+
+	let tlvs = intermediate_tlvs.map(|tlvs| {
+		let res = BlindedPathWithPadding {
+			tlvs,
+			round_off,
+		};
+		if compact_padding {
+			debug_assert_eq!(res.serialized_length(), max_intermediate_len);
+		} else {
+			// We don't currently ever push extra stuff to intermediate hops, so simply assert that
+			// the fully-padded hops are always `MESSAGE_PADDING_ROUND_OFF` long.
+			debug_assert_eq!(res.serialized_length(), MESSAGE_PADDING_ROUND_OFF);
+		}
+		res
+	})
+	.chain(core::iter::once(BlindedPathWithPadding {
+		tlvs: ControlTlvs::Receive(ReceiveTlvs { context: Some(context) }),
+		round_off: if compact_padding { 0 } else { MESSAGE_PADDING_ROUND_OFF },
+	}));
+
+	let path = pks.zip(tlvs);
+	utils::construct_blinded_hops(secp_ctx, path, session_priv)
 }

lightning/src/blinded_path/utils.rs

@@ -256,9 +256,12 @@ impl<T: Writeable> Writeable for BlindedPathWithPadding<T> {
 		let tlv_length = self.tlvs.serialized_length();
 		let total_length = tlv_length + TLV_OVERHEAD;
 
-		let padding_length = total_length.div_ceil(self.round_off) * self.round_off - total_length;
-
-		let padding = Some(BlindedPathPadding::new(padding_length));
+		let padding = if self.round_off == 0 || tlv_length % self.round_off == 0 {
+			None
+		} else {
+			let length = total_length.div_ceil(self.round_off) * self.round_off - total_length;
+			Some(BlindedPathPadding::new(length))
+		};
 
 		encode_tlv_stream!(writer, {
 			(1, padding, option),

lightning/src/offers/flow.rs

@@ -1319,6 +1319,7 @@ where
 			num_dummy_hops,
 			self.receive_auth_key,
 			context,
+			false,
 			&*entropy,
 			&self.secp_ctx,
 		)