Merge pull request #2715 from valentinewallace/2023-11-skimmed-fees
authorMatt Corallo <649246+TheBlueMatt@users.noreply.github.com>
Sun, 12 Nov 2023 20:27:25 +0000 (20:27 +0000)
committerGitHub <noreply@github.com>
Sun, 12 Nov 2023 20:27:25 +0000 (20:27 +0000)
Complete underpaying HTLCs support

fuzz/README.md
lightning/src/ln/channel.rs
lightning/src/ln/channelmanager.rs
lightning/src/ln/mod.rs
lightning/src/ln/msgs.rs
lightning/src/ln/onion_utils.rs
lightning/src/offers/invoice.rs
lightning/src/offers/invoice_request.rs
lightning/src/offers/merkle.rs
lightning/src/onion_message/mod.rs

index bfc8fa5f4bfb5c6ae71b5d1262d17bb831d1ee52..987288b5d932f908f456bde2dc34d6f3e2289928 100644 (file)
@@ -1,22 +1,23 @@
 # Fuzzing
 
-Fuzz tests generate a ton of random parameter arguments to the program and then validate that none cause it to crash.
+Fuzz tests generate a ton of random parameter arguments to the program and then validate that none 
+cause it to crash.
 
 ## How does it work?
 
-Typically, Travis CI will run `travis-fuzz.sh` on one of the environments the automated tests are configured for.
-This is the most time-consuming component of the continuous integration workflow, so it is recommended that you detect
-issues locally, and Travis merely acts as a sanity check. Fuzzing is further only effective with
-a lot of CPU time, indicating that if crash scenarios are discovered on Travis with its low
-runtime constraints, the crash is caused relatively easily.
+Typically, CI will run `ci-fuzz.sh` on one of the environments the automated tests are
+configured for. Fuzzing is further only effective with a lot of CPU time, indicating that if crash 
+scenarios are discovered on CI with its low runtime constraints, the crash is caused relatively
+easily.
 
 ## How do I run fuzz tests locally?
 
-You typically won't need to run the entire combination of different fuzzing tools. For local execution, `honggfuzz`
-should be more than sufficient. 
+We support multiple fuzzing engines such as `honggfuzz`, `libFuzzer` and `AFL`. You typically won't 
+need to run the entire suite of different fuzzing tools. For local execution, `honggfuzz`should be 
+more than sufficient. 
 
 ### Setup
-
+#### Honggfuzz
 To install `honggfuzz`, simply run
 
 ```shell
@@ -31,9 +32,18 @@ cargo update -p honggfuzz --precise "0.5.52"
 cargo install --force honggfuzz --version "0.5.52"
 ```
 
+#### cargo-fuzz / libFuzzer
+To install `cargo-fuzz`, simply run
+
+```shell
+cargo update
+cargo install --force cargo-fuzz
+```
+
 ### Execution
 
-To run the Hongg fuzzer, do
+#### Honggfuzz
+To run fuzzing using `honggfuzz`, do
 
 ```shell
 export CPU_COUNT=1 # replace as needed
@@ -46,19 +56,39 @@ cargo hfuzz run $TARGET
 
 (Or, for a prettier output, replace the last line with `cargo --color always hfuzz run $TARGET`.)
 
+#### cargo-fuzz / libFuzzer
+To run fuzzing using `cargo-fuzz / libFuzzer`, run
+
+```shell
+rustup install nightly # Note: libFuzzer requires a nightly version of rust.
+cargo +nightly fuzz run --features "libfuzzer_fuzz" msg_ping_target
+```
+Note: If you encounter a `SIGKILL` during run/build check for OOM in kernel logs and consider 
+increasing RAM size for VM.
+
+If you wish to just generate fuzzing binary executables for `libFuzzer` and not run them:
+```shell 
+cargo +nightly fuzz build --features "libfuzzer_fuzz" msg_ping_target 
+# Generates binary artifact in path ./target/aarch64-unknown-linux-gnu/release/msg_ping_target
+# Exact path depends on your system architecture.
+```
+You can upload the build artifact generated above to `ClusterFuzz` for distributed fuzzing.
+
+### List Fuzzing Targets
 To see a list of available fuzzing targets, run:
 
 ```shell
 ls ./src/bin/
 ```
 
-## A fuzz test failed on Travis, what do I do?
+## A fuzz test failed, what do I do?
 
-You're trying to create a PR, but need to find the underlying cause of that pesky fuzz failure blocking the merge?
+You're trying to create a PR, but need to find the underlying cause of that pesky fuzz failure 
+blocking the merge?
 
 Worry not, for this is easily traced.
 
-If your Travis output log looks like this:
+If your output log looks like this:
 
 ```
 Size:639 (i,b,hw,ed,ip,cmp): 0/0/0/0/0/1, Tot:0/0/0/2036/5/28604
@@ -66,13 +96,13 @@ Seen a crash. Terminating all fuzzing threads
 
 … # a lot of lines in between
 
-<0x0000555555565559> [func:UNKNOWN file: line:0 module:/home/travis/build/rust-bitcoin/rust-lightning/fuzz/hfuzz_target/x86_64-unknown-linux-gnu/release/full_stack_target]
+<0x0000555555565559> [func:UNKNOWN file: line:0 module:./rust-lightning/fuzz/hfuzz_target/x86_64-unknown-linux-gnu/release/full_stack_target]
 <0x0000000000000000> [func:UNKNOWN file: line:0 module:UNKNOWN]
 =====================================================================
 2d3136383734090101010101010101010101010101010101010101010101
 010101010100040101010101010101010101010103010101010100010101
 0069d07c319a4961
-The command "if [ "$(rustup show | grep default | grep stable)" != "" ]; then cd fuzz && cargo test --verbose && ./travis-fuzz.sh; fi" exited with 1.
+The command "if [ "$(rustup show | grep default | grep stable)" != "" ]; then cd fuzz && cargo test --verbose && ./ci-fuzz.sh; fi" exited with 1.
 ```
 
 Note that the penultimate stack trace line ends in `release/full_stack_target]`. That indicates that
index 1b5d6cfa7bfe575a7c7ea75a8bd59193ca97ce08..1e1b09f9ea8d8d69af86523750349cd1e11f8e5c 100644 (file)
@@ -48,6 +48,7 @@ use crate::util::scid_utils::scid_from_parts;
 use crate::io;
 use crate::prelude::*;
 use core::{cmp,mem,fmt};
+use core::convert::TryInto;
 use core::ops::Deref;
 #[cfg(any(test, fuzzing, debug_assertions))]
 use crate::sync::Mutex;
@@ -1195,8 +1196,8 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider  {
                match self.config.options.max_dust_htlc_exposure {
                        MaxDustHTLCExposure::FeeRateMultiplier(multiplier) => {
                                let feerate_per_kw = fee_estimator.bounded_sat_per_1000_weight(
-                                       ConfirmationTarget::OnChainSweep);
-                               feerate_per_kw as u64 * multiplier
+                                       ConfirmationTarget::OnChainSweep) as u64;
+                               feerate_per_kw.saturating_mul(multiplier)
                        },
                        MaxDustHTLCExposure::FixedLimitMsat(limit) => limit,
                }
@@ -1780,14 +1781,14 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider  {
                         context.holder_dust_limit_satoshis       + dust_buffer_feerate * htlc_timeout_tx_weight(context.get_channel_type()) / 1000)
                };
                let on_counterparty_dust_htlc_exposure_msat = inbound_stats.on_counterparty_tx_dust_exposure_msat + outbound_stats.on_counterparty_tx_dust_exposure_msat;
-               if on_counterparty_dust_htlc_exposure_msat as i64 + htlc_success_dust_limit as i64 * 1000 - 1 > max_dust_htlc_exposure_msat as i64 {
+               if on_counterparty_dust_htlc_exposure_msat as i64 + htlc_success_dust_limit as i64 * 1000 - 1 > max_dust_htlc_exposure_msat.try_into().unwrap_or(i64::max_value()) {
                        remaining_msat_below_dust_exposure_limit =
                                Some(max_dust_htlc_exposure_msat.saturating_sub(on_counterparty_dust_htlc_exposure_msat));
                        dust_exposure_dust_limit_msat = cmp::max(dust_exposure_dust_limit_msat, htlc_success_dust_limit * 1000);
                }
 
                let on_holder_dust_htlc_exposure_msat = inbound_stats.on_holder_tx_dust_exposure_msat + outbound_stats.on_holder_tx_dust_exposure_msat;
-               if on_holder_dust_htlc_exposure_msat as i64 + htlc_timeout_dust_limit as i64 * 1000 - 1 > max_dust_htlc_exposure_msat as i64 {
+               if on_holder_dust_htlc_exposure_msat as i64 + htlc_timeout_dust_limit as i64 * 1000 - 1 > max_dust_htlc_exposure_msat.try_into().unwrap_or(i64::max_value()) {
                        remaining_msat_below_dust_exposure_limit = Some(cmp::min(
                                remaining_msat_below_dust_exposure_limit.unwrap_or(u64::max_value()),
                                max_dust_htlc_exposure_msat.saturating_sub(on_holder_dust_htlc_exposure_msat)));
index 3d698603551d75d1d8a4a58e788de3e74ed4a72d..ce79f25400a53645b8fccf2f71ddb7ca48f46e64 100644 (file)
@@ -3430,12 +3430,10 @@ where
                let prng_seed = self.entropy_source.get_secure_random_bytes();
                let session_priv = SecretKey::from_slice(&session_priv_bytes[..]).expect("RNG is busted");
 
-               let onion_keys = onion_utils::construct_onion_keys(&self.secp_ctx, &path, &session_priv)
-                       .map_err(|_| APIError::InvalidRoute{err: "Pubkey along hop was maliciously selected".to_owned()})?;
-               let (onion_payloads, htlc_msat, htlc_cltv) = onion_utils::build_onion_payloads(path, total_value, recipient_onion, cur_height, keysend_preimage)?;
-
-               let onion_packet = onion_utils::construct_onion_packet(onion_payloads, onion_keys, prng_seed, payment_hash)
-                       .map_err(|_| APIError::InvalidRoute { err: "Route size too large considering onion data".to_owned()})?;
+               let (onion_packet, htlc_msat, htlc_cltv) = onion_utils::create_payment_onion(
+                       &self.secp_ctx, &path, &session_priv, total_value, recipient_onion, cur_height,
+                       payment_hash, keysend_preimage, prng_seed
+               )?;
 
                let err: Result<(), _> = loop {
                        let (counterparty_node_id, id) = match self.short_to_chan_info.read().unwrap().get(&path.hops.first().unwrap().short_channel_id) {
index 6a9a10c80c060b41f3a11fc128add17c99a93553..f9d364ed0db848b746ae65266f68df83d8264e82 100644 (file)
@@ -39,6 +39,7 @@ pub(crate) mod onion_utils;
 mod outbound_payment;
 pub mod wire;
 
+pub use onion_utils::create_payment_onion;
 // Older rustc (which we support) refuses to let us call the get_payment_preimage_hash!() macro
 // without the node parameter being mut. This is incorrect, and thus newer rustcs will complain
 // about an unnecessary mut. Thus, we silence the unused_mut warning in two test modules below.
index 6043bf35b99d4d85ae79a5656cc75b82e7d6c09e..d43ad6d0aebc2fd372c5c7837349a9beac0ebedf 100644 (file)
@@ -1674,17 +1674,21 @@ pub use self::fuzzy_internal_msgs::*;
 #[cfg(not(fuzzing))]
 pub(crate) use self::fuzzy_internal_msgs::*;
 
+/// BOLT 4 onion packet including hop data for the next peer.
 #[derive(Clone)]
-pub(crate) struct OnionPacket {
-       pub(crate) version: u8,
+pub struct OnionPacket {
+       /// BOLT 4 version number.
+       pub version: u8,
        /// In order to ensure we always return an error on onion decode in compliance with [BOLT
        /// #4](https://github.com/lightning/bolts/blob/master/04-onion-routing.md), we have to
        /// deserialize `OnionPacket`s contained in [`UpdateAddHTLC`] messages even if the ephemeral
        /// public key (here) is bogus, so we hold a [`Result`] instead of a [`PublicKey`] as we'd
        /// like.
-       pub(crate) public_key: Result<PublicKey, secp256k1::Error>,
-       pub(crate) hop_data: [u8; 20*65],
-       pub(crate) hmac: [u8; 32],
+       pub public_key: Result<PublicKey, secp256k1::Error>,
+       /// 1300 bytes encrypted payload for the next hop.
+       pub hop_data: [u8; 20*65],
+       /// HMAC to verify the integrity of hop_data.
+       pub hmac: [u8; 32],
 }
 
 impl onion_utils::Packet for OnionPacket {
index 7f694565f325a8c73463d48d0deea4c23400a57c..e952bd8e2e125c5d8c16ef6f8419e1c8a2579ba0 100644 (file)
@@ -935,6 +935,27 @@ pub(crate) fn decode_next_payment_hop<NS: Deref>(
        }
 }
 
+/// Build a payment onion, returning the first hop msat and cltv values as well.
+/// `cur_block_height` should be set to the best known block height + 1.
+pub fn create_payment_onion<T: secp256k1::Signing>(
+       secp_ctx: &Secp256k1<T>, path: &Path, session_priv: &SecretKey, total_msat: u64,
+       recipient_onion: RecipientOnionFields, cur_block_height: u32, payment_hash: &PaymentHash,
+       keysend_preimage: &Option<PaymentPreimage>, prng_seed: [u8; 32]
+) -> Result<(msgs::OnionPacket, u64, u32), APIError> {
+       let onion_keys = construct_onion_keys(&secp_ctx, &path, &session_priv)
+               .map_err(|_| APIError::InvalidRoute{
+                       err: "Pubkey along hop was maliciously selected".to_owned()
+               })?;
+       let (onion_payloads, htlc_msat, htlc_cltv) = build_onion_payloads(
+               &path, total_msat, recipient_onion, cur_block_height, keysend_preimage
+       )?;
+       let onion_packet = construct_onion_packet(onion_payloads, onion_keys, prng_seed, payment_hash)
+               .map_err(|_| APIError::InvalidRoute{
+                       err: "Route size too large considering onion data".to_owned()
+               })?;
+       Ok((onion_packet, htlc_msat, htlc_cltv))
+}
+
 pub(crate) fn decode_next_untagged_hop<T, R: ReadableArgs<T>, N: NextPacketBytes>(shared_secret: [u8; 32], hop_data: &[u8], hmac_bytes: [u8; 32], read_args: T) -> Result<(R, Option<([u8; 32], N)>), OnionDecodeErr> {
        decode_next_hop(shared_secret, hop_data, hmac_bytes, None, read_args)
 }
index c6fd5d056491560b5b3f630564fe97af93bea372..5bd5a95d8e627a3e69964db540fbffa554bbddf4 100644 (file)
@@ -439,6 +439,7 @@ impl UnsignedBolt12Invoice {
                        bytes: self.bytes,
                        contents: self.contents,
                        signature,
+                       tagged_hash: self.tagged_hash,
                })
        }
 }
@@ -463,6 +464,7 @@ pub struct Bolt12Invoice {
        bytes: Vec<u8>,
        contents: InvoiceContents,
        signature: Signature,
+       tagged_hash: TaggedHash,
 }
 
 /// The contents of an [`Bolt12Invoice`] for responding to either an [`Offer`] or a [`Refund`].
@@ -707,7 +709,7 @@ impl Bolt12Invoice {
 
        /// Hash that was used for signing the invoice.
        pub fn signable_hash(&self) -> [u8; 32] {
-               merkle::message_digest(SIGNATURE_TAG, &self.bytes).as_ref().clone()
+               self.tagged_hash.as_digest().as_ref().clone()
        }
 
        /// Verifies that the invoice was for a request or refund created using the given key. Returns
@@ -1212,11 +1214,11 @@ impl TryFrom<ParsedMessage<FullInvoiceTlvStream>> for Bolt12Invoice {
                        None => return Err(Bolt12ParseError::InvalidSemantics(Bolt12SemanticError::MissingSignature)),
                        Some(signature) => signature,
                };
-               let message = TaggedHash::new(SIGNATURE_TAG, &bytes);
+               let tagged_hash = TaggedHash::new(SIGNATURE_TAG, &bytes);
                let pubkey = contents.fields().signing_pubkey;
-               merkle::verify_signature(&signature, message, pubkey)?;
+               merkle::verify_signature(&signature, &tagged_hash, pubkey)?;
 
-               Ok(Bolt12Invoice { bytes, contents, signature })
+               Ok(Bolt12Invoice { bytes, contents, signature, tagged_hash })
        }
 }
 
@@ -1431,7 +1433,7 @@ mod tests {
                assert_eq!(invoice.signing_pubkey(), recipient_pubkey());
 
                let message = TaggedHash::new(SIGNATURE_TAG, &invoice.bytes);
-               assert!(merkle::verify_signature(&invoice.signature, message, recipient_pubkey()).is_ok());
+               assert!(merkle::verify_signature(&invoice.signature, &message, recipient_pubkey()).is_ok());
 
                let digest = Message::from_slice(&invoice.signable_hash()).unwrap();
                let pubkey = recipient_pubkey().into();
@@ -1528,7 +1530,7 @@ mod tests {
                assert_eq!(invoice.signing_pubkey(), recipient_pubkey());
 
                let message = TaggedHash::new(SIGNATURE_TAG, &invoice.bytes);
-               assert!(merkle::verify_signature(&invoice.signature, message, recipient_pubkey()).is_ok());
+               assert!(merkle::verify_signature(&invoice.signature, &message, recipient_pubkey()).is_ok());
 
                assert_eq!(
                        invoice.as_tlv_stream(),
index bd6d58371a9f316cccf6b362cc4825f27171f177..9ddd741a1d5428745e2bd1d840fa14eb4a1eee08 100644 (file)
@@ -876,7 +876,7 @@ impl TryFrom<Vec<u8>> for InvoiceRequest {
                        Some(signature) => signature,
                };
                let message = TaggedHash::new(SIGNATURE_TAG, &bytes);
-               merkle::verify_signature(&signature, message, contents.payer_id)?;
+               merkle::verify_signature(&signature, &message, contents.payer_id)?;
 
                Ok(InvoiceRequest { bytes, contents, signature })
        }
@@ -1013,7 +1013,7 @@ mod tests {
                assert_eq!(invoice_request.payer_note(), None);
 
                let message = TaggedHash::new(SIGNATURE_TAG, &invoice_request.bytes);
-               assert!(merkle::verify_signature(&invoice_request.signature, message, payer_pubkey()).is_ok());
+               assert!(merkle::verify_signature(&invoice_request.signature, &message, payer_pubkey()).is_ok());
 
                assert_eq!(
                        invoice_request.as_tlv_stream(),
index 7390b58fef8ef780a68f58aa50438dcbe1979d71..7330541220f625240dee1db5e13539a73aaea824 100644 (file)
@@ -30,20 +30,41 @@ tlv_stream!(SignatureTlvStream, SignatureTlvStreamRef, SIGNATURE_TYPES, {
 ///
 /// [BIP 340]: https://github.com/bitcoin/bips/blob/master/bip-0340.mediawiki
 /// [BOLT 12]: https://github.com/rustyrussell/lightning-rfc/blob/guilt/offers/12-offer-encoding.md#signature-calculation
-#[derive(Debug, PartialEq)]
-pub struct TaggedHash(Message);
+#[derive(Clone, Debug, PartialEq)]
+pub struct TaggedHash {
+       tag: &'static str,
+       merkle_root: sha256::Hash,
+       digest: Message,
+}
 
 impl TaggedHash {
        /// Creates a tagged hash with the given parameters.
        ///
        /// Panics if `tlv_stream` is not a well-formed TLV stream containing at least one TLV record.
-       pub(super) fn new(tag: &str, tlv_stream: &[u8]) -> Self {
-               Self(message_digest(tag, tlv_stream))
+       pub(super) fn new(tag: &'static str, tlv_stream: &[u8]) -> Self {
+               let tag_hash = sha256::Hash::hash(tag.as_bytes());
+               let merkle_root = root_hash(tlv_stream);
+               let digest = Message::from_slice(&tagged_hash(tag_hash, merkle_root)).unwrap();
+               Self {
+                       tag,
+                       merkle_root,
+                       digest,
+               }
        }
 
        /// Returns the digest to sign.
        pub fn as_digest(&self) -> &Message {
-               &self.0
+               &self.digest
+       }
+
+       /// Returns the tag used in the tagged hash.
+       pub fn tag(&self) -> &str {
+               &self.tag
+       }
+
+       /// Returns the merkle root used in the tagged hash.
+       pub fn merkle_root(&self) -> sha256::Hash {
+               self.merkle_root
        }
 }
 
@@ -91,7 +112,7 @@ where
 /// Verifies the signature with a pubkey over the given message using a tagged hash as the message
 /// digest.
 pub(super) fn verify_signature(
-       signature: &Signature, message: TaggedHash, pubkey: PublicKey,
+       signature: &Signature, message: &TaggedHash, pubkey: PublicKey,
 ) -> Result<(), secp256k1::Error> {
        let digest = message.as_digest();
        let pubkey = pubkey.into();
@@ -99,12 +120,6 @@ pub(super) fn verify_signature(
        secp_ctx.verify_schnorr(signature, digest, &pubkey)
 }
 
-pub(super) fn message_digest(tag: &str, bytes: &[u8]) -> Message {
-       let tag = sha256::Hash::hash(tag.as_bytes());
-       let merkle_root = root_hash(bytes);
-       Message::from_slice(&tagged_hash(tag, merkle_root)).unwrap()
-}
-
 /// Computes a merkle root hash for the given data, which must be a well-formed TLV stream
 /// containing at least one TLV record.
 fn root_hash(data: &[u8]) -> sha256::Hash {
@@ -258,12 +273,13 @@ mod tests {
        use super::{SIGNATURE_TYPES, TlvStream, WithoutSignatures};
 
        use bitcoin::hashes::{Hash, sha256};
-       use bitcoin::secp256k1::{KeyPair, Secp256k1, SecretKey};
+       use bitcoin::secp256k1::{KeyPair, Message, Secp256k1, SecretKey};
        use bitcoin::secp256k1::schnorr::Signature;
        use core::convert::Infallible;
        use crate::offers::offer::{Amount, OfferBuilder};
        use crate::offers::invoice_request::InvoiceRequest;
        use crate::offers::parse::Bech32Encode;
+       use crate::offers::test_utils::{payer_pubkey, recipient_pubkey};
        use crate::util::ser::Writeable;
 
        #[test]
@@ -322,6 +338,25 @@ mod tests {
                );
        }
 
+        #[test]
+        fn compute_tagged_hash() {
+                let unsigned_invoice_request = OfferBuilder::new("foo".into(), recipient_pubkey())
+                        .amount_msats(1000)
+                        .build().unwrap()
+                        .request_invoice(vec![1; 32], payer_pubkey()).unwrap()
+                        .payer_note("bar".into())
+                        .build().unwrap();
+
+                // Simply test that we can grab the tag and merkle root exposed by the accessor
+                // functions, then use them to succesfully compute a tagged hash.
+                let tagged_hash = unsigned_invoice_request.as_ref();
+                let expected_digest = unsigned_invoice_request.as_ref().as_digest();
+                let tag = sha256::Hash::hash(tagged_hash.tag().as_bytes());
+                let actual_digest = Message::from_slice(&super::tagged_hash(tag, tagged_hash.merkle_root()))
+                        .unwrap();
+                assert_eq!(*expected_digest, actual_digest);
+        }
+
        #[test]
        fn skips_encoding_signature_tlv_records() {
                let secp_ctx = Secp256k1::new();
index ff6e0cd8e5d59a44324838a9156bce79ae0ea45e..d106a542fd368156f0a4f7959c37919d8e1821ec 100644 (file)
@@ -28,6 +28,7 @@ mod functional_tests;
 
 // Re-export structs so they can be imported with just the `onion_message::` module prefix.
 pub use self::messenger::{CustomOnionMessageHandler, DefaultMessageRouter, Destination, MessageRouter, OnionMessageContents, OnionMessagePath, OnionMessenger, PeeledOnion, PendingOnionMessage, SendError};
+pub use self::messenger::{create_onion_message, peel_onion_message};
 #[cfg(not(c_bindings))]
 pub use self::messenger::{SimpleArcOnionMessenger, SimpleRefOnionMessenger};
 pub use self::offers::{OffersMessage, OffersMessageHandler};