Error if onion payloads exceed max length on packet construction.

Ensure that if we call construct_onion_packet and friends where payloads are
too large for the allotted packet length, we'll fail to construct. Previously,
senders would happily construct invalid packets by array-shifting the final
node's HMAC out of the packet when adding an intermediate onion layer, causing
the receiver to error with "final payload provided for us as an intermediate
node."

diff --git a/lightning/src/ln/onion_payment.rs b/lightning/src/ln/onion_payment.rs
index 07c08d57995b6938c78214067a54c9f7ae74c7b1..7c0d32a52a53bccab19177fa014b90e477cd317c 100644 (file)
--- a/lightning/src/ln/onion_payment.rs
+++ b/lightning/src/ln/onion_payment.rs
@@ -449,7 +449,7 @@ pub(super) fn check_incoming_htlc_cltv(
 mod tests {
        use bitcoin::hashes::Hash;
        use bitcoin::hashes::sha256::Hash as Sha256;
-       use bitcoin::secp256k1::{PublicKey, SecretKey};
+       use bitcoin::secp256k1::{PublicKey, Secp256k1, SecretKey};
        use crate::ln::{PaymentPreimage, PaymentHash, PaymentSecret};
        use crate::ln::ChannelId;
        use crate::ln::channelmanager::RecipientOnionFields;
@@ -459,6 +459,38 @@ mod tests {
        use crate::routing::router::{Path, RouteHop};
        use crate::util::test_utils;
 
+       #[test]
+       fn fail_construct_onion_on_too_big_payloads() {
+               // Ensure that if we call `construct_onion_packet` and friends where payloads are too large for
+               // the allotted packet length, we'll fail to construct. Previously, senders would happily
+               // construct invalid packets by array-shifting the final node's HMAC out of the packet when
+               // adding an intermediate onion layer, causing the receiver to error with "final payload
+               // provided for us as an intermediate node."
+               let secp_ctx = Secp256k1::new();
+               let bob = crate::sign::KeysManager::new(&[2; 32], 42, 42);
+               let bob_pk = PublicKey::from_secret_key(&secp_ctx, &bob.get_node_secret_key());
+               let charlie = crate::sign::KeysManager::new(&[3; 32], 42, 42);
+               let charlie_pk = PublicKey::from_secret_key(&secp_ctx, &charlie.get_node_secret_key());
+
+               let (
+                       session_priv, total_amt_msat, cur_height, mut recipient_onion, keysend_preimage, payment_hash,
+                       prng_seed, hops, ..
+               ) = payment_onion_args(bob_pk, charlie_pk);
+
+               // Ensure the onion will not fit all the payloads by adding a large custom TLV.
+               recipient_onion.custom_tlvs.push((13377331, vec![0; 1156]));
+
+               let path = Path { hops, blinded_tail: None, };
+               let onion_keys = super::onion_utils::construct_onion_keys(&secp_ctx, &path, &session_priv).unwrap();
+               let (onion_payloads, ..) = super::onion_utils::build_onion_payloads(
+                       &path, total_amt_msat, recipient_onion, cur_height + 1, &Some(keysend_preimage)
+               ).unwrap();
+
+               assert!(super::onion_utils::construct_onion_packet(
+                               onion_payloads, onion_keys, prng_seed, &payment_hash
+               ).is_err());
+       }
+
        #[test]
        fn test_peel_payment_onion() {
                use super::*;

diff --git a/lightning/src/ln/onion_utils.rs b/lightning/src/ln/onion_utils.rs
index a9f8074f5c1bda36f06f78158b61e09f96d3318f..ea906c9e32af86b2b1d21e3322eba082a4c195ea 100644 (file)
--- a/lightning/src/ln/onion_utils.rs
+++ b/lightning/src/ln/onion_utils.rs
@@ -323,8 +323,6 @@ fn construct_onion_packet_with_init_noise<HD: Writeable, P: Packet>(
 
                let mut pos = 0;
                for (i, (payload, keys)) in payloads.iter().zip(onion_keys.iter()).enumerate() {
-                       if i == payloads.len() - 1 { break; }
-
                        let mut chacha = ChaCha20::new(&keys.rho, &[0u8; 8]);
                        for _ in 0..(packet_data.len() - pos) { // TODO: Batch this.
                                let mut dummy = [0; 1];
@@ -338,6 +336,8 @@ fn construct_onion_packet_with_init_noise<HD: Writeable, P: Packet>(
                                return Err(());
                        }
 
+                       if i == payloads.len() - 1 { break; }
+
                        res.resize(pos, 0u8);
                        chacha.process_in_place(&mut res);
                }