From 82a984a94199b5810bad1a6810f264b771df212e Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 18 Jul 2025 11:53:39 +0000 Subject: [PATCH 1/5] Previously-authentication fields in contexts can be reused We recently switched to `ReceiveAuthKey`-based blinded path authentication, removing various fields used to authenticate blinded paths from contexts. In doing so, we left a comment arguing that such fields should not have their TLV IDs reused, however there's no reason for that restriction. If we reuse old blinded paths with those fields set they'll fail the new authentication checks anyway, so the fields should never end up being seen in blinded paths we'd accept. This simply updates the comments. --- lightning/src/blinded_path/message.rs | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/lightning/src/blinded_path/message.rs b/lightning/src/blinded_path/message.rs index 218b2282141..7db5dc00b05 100644 --- a/lightning/src/blinded_path/message.rs +++ b/lightning/src/blinded_path/message.rs @@ -543,14 +543,10 @@ impl_writeable_tlv_based_enum!(MessageContext, {3, DNSResolver} => (), ); -// NOTE: -// Several TLV fields (`nonce`, `hmac`, etc.) were removed in LDK v0.2 -// following the introduction of `ReceiveAuthKey`-based authentication for -// inbound `BlindedMessagePath`s. These fields are now commented out and -// their `type` values must not be reused unless support for LDK v0.2 -// and earlier is fully dropped. -// -// For context-specific removals, see the commented-out fields within each enum variant. +// Note: Several TLV fields (`nonce`, `hmac`, etc.) were removed in LDK v0.2 following the +// introduction of `ReceiveAuthKey`-based authentication for inbound `BlindedMessagePath`s. Because +// we do not support receiving to those contexts anymore (they will fail the `ReceiveAuthKey`-based +// authentication checks), we can reuse those fields here. impl_writeable_tlv_based_enum!(OffersContext, (0, InvoiceRequest) => { (0, nonce, required), @@ -558,12 +554,9 @@ impl_writeable_tlv_based_enum!(OffersContext, (1, OutboundPayment) => { (0, payment_id, required), (1, nonce, required), - // Removed: (2, hmac, option) }, (2, InboundPayment) => { (0, payment_hash, required), - // Removed: (1, nonce, required), - // Removed: (2, hmac, required) }, (3, StaticInvoiceRequested) => { (0, recipient_id, required), @@ -575,12 +568,8 @@ impl_writeable_tlv_based_enum!(OffersContext, impl_writeable_tlv_based_enum!(AsyncPaymentsContext, (0, OutboundPayment) => { (0, payment_id, required), - // Removed: (2, nonce, required), - // Removed: (4, hmac, required), }, (1, InboundPayment) => { - // Removed: (0, nonce, required), - // Removed: (2, hmac, required), (4, path_absolute_expiry, required), }, (2, OfferPaths) => { From c9c28670c5a44d69082d2a53d0d8d806a3e91245 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 18 Jul 2025 11:59:30 +0000 Subject: [PATCH 2/5] Update docs on `verify_inbound_async_payment_context` We recently switched to `ReceiveAuthKey`-based blinded path authentication, removing various fields used to authenticate blinded paths from contexts. In doing so we forgot to update the documentation on `verify_inbound_async_payment_context` to note that it no longer does any cryptographic verification but instead only validates the expiry. Here we update that documentation. --- lightning/src/offers/flow.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lightning/src/offers/flow.rs b/lightning/src/offers/flow.rs index a1b83260dfd..27ed4f42ef0 100644 --- a/lightning/src/offers/flow.rs +++ b/lightning/src/offers/flow.rs @@ -543,13 +543,12 @@ where /// Verifies the provided [`AsyncPaymentsContext`] for an inbound [`HeldHtlcAvailable`] message. /// - /// The context is verified using the `nonce` and `hmac` values, and ensures that the context - /// has not expired based on `path_absolute_expiry`. + /// Because blinded path contexts are verified as a part of onion message processing, this only + /// validates that the context is not yet expired based on `path_absolute_expiry`. /// /// # Errors /// /// Returns `Err(())` if: - /// - The HMAC verification fails for inbound context. /// - The inbound payment context has expired. #[cfg(async_payments)] pub fn verify_inbound_async_payment_context( From 5f14409248f698ef4b13796f36c31c758d500d5e Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 18 Jul 2025 12:00:42 +0000 Subject: [PATCH 3/5] Update `HMAC_INPUT` docs to accurately describe legacy reservations We recently switched to `ReceiveAuthKey`-based blinded path authentication, removing various fields used to authenticate blinded paths from contexts. In doing so we removed no-longer-needed `HMAC_INPUT`s in offer metadata validation, and left a comment noting that previously used values should not be reused. That comment was slightly incorrect as it indicated some kind of "backward compatibility" concern, but of course we broke backwards compatibility when we stopped accepting the previous authentication scheme. Instead, here, we update the comment to note that what we're protecting against is a type confusion attack. --- lightning/src/offers/signer.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lightning/src/offers/signer.rs b/lightning/src/offers/signer.rs index dfdf7fef583..645949ff866 100644 --- a/lightning/src/offers/signer.rs +++ b/lightning/src/offers/signer.rs @@ -42,7 +42,7 @@ const WITH_ENCRYPTED_PAYMENT_ID_HMAC_INPUT: &[u8; 16] = &[4; 16]; // `OffersContext`, but were removed in LDK v0.2 with the introduction of `ReceiveAuthKey`-based // authentication. // Their corresponding values (`[5; 16]` and `[7; 16]`) are now reserved and must not -// be reused to preserve backward compatibility. +// be reused to ensure type confusion attacks are impossible. // // Reserved HMAC_INPUT values — do not reuse: // From 3129ac3e323e70e8dc8db89c798b5694f398d8d0 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 18 Jul 2025 11:56:28 +0000 Subject: [PATCH 4/5] Update `ChaChaPolyWriteAdapter` to bypass crypto when calc'ing len `ChaChaPolyWriteAdapter` is used to wrap `Writeable` objects in an AEAD before serializing them. This is great, except that we sometimes call `::encode` which first calls `Writeable::serialized_length` to calculate the length to pre-allocate in a `Vec` before doing the actual write. Because `ChaChaPolyWriteAdapter` did not override `Writeable::serialized_length`, this led to doing the full cryptographic AEAD encryption and MAC twice, once to calculate the length and once to actually write the data. Instead, here, we override ` Writeable for ChaChaPolyWriteAdapter<'a, T> { Ok(()) } + + fn serialized_length(&self) -> usize { + self.writeable.serialized_length() + 16 + } } /// Encrypts the provided plaintext with the given key using ChaCha20Poly1305 in the modified From 8ee02eb0a6c0c8531430bc67ca750ae9ad33e300 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 18 Jul 2025 12:03:06 +0000 Subject: [PATCH 5/5] Correct `ChaCha[Dual]PolyReadAdapter` extra stream contents logic `ChaCha[Dual]PolyReadAdapter` currently read the encrypted object using `Readable` through the ChaCha stream (including the Poly1305 HMAC), but then consume any remaining bytes directly. This results in any extra bytes not consumed by the desired type's `Readable` being ignored and not included in the HMAC check. This is likely not the desired behavior - if we get some data which has extra slack at the end we ignore, it should still be authenticated as the sender likely thinks that data has meaning and included it in their HMAC check. Luckily, I believe this is currently dead code - `ChaCha[Dual]PolyReadAdapter` are only used for TLV stream reads which consume the full underlying stream. However, if either is used for non-TLV-streams in the future, this may be important. Here we simply push any extra bytes read through the ChaCha20Poly1305 reader, ensuring extra data is included in the HMAC check. --- lightning/src/crypto/streams.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/lightning/src/crypto/streams.rs b/lightning/src/crypto/streams.rs index 38bfd767bd8..b631b7bd645 100644 --- a/lightning/src/crypto/streams.rs +++ b/lightning/src/crypto/streams.rs @@ -128,7 +128,10 @@ impl LengthReadableArgs<([u8; 32], [u8; 32])> for ChaChaDualPolyRea ChaChaDualPolyReader { chacha: &mut chacha, poly: &mut mac, read_len: 0, read: s }; let readable: T = Readable::read(&mut chacha_stream)?; - chacha_stream.read.eat_remaining()?; + while chacha_stream.read.bytes_remain() { + let mut buf = [0; 256]; + chacha_stream.read(&mut buf)?; + } let read_len = chacha_stream.read_len; @@ -203,7 +206,10 @@ impl LengthReadableArgs<[u8; 32]> for ChaChaPolyReadAdapter { let s = FixedLengthReader::new(r, decrypted_len); let mut chacha_stream = ChaChaPolyReader { chacha: &mut chacha, read: s }; let readable: T = Readable::read(&mut chacha_stream)?; - chacha_stream.read.eat_remaining()?; + while chacha_stream.read.bytes_remain() { + let mut buf = [0; 256]; + chacha_stream.read(&mut buf)?; + } let mut tag = [0 as u8; 16]; r.read_exact(&mut tag)?;