From 8013a2500e90168701a729319ad980b87e47e4ac Mon Sep 17 00:00:00 2001 From: Nishant Bansal Date: Fri, 21 Aug 2026 01:22:20 +0530 Subject: [PATCH 1/4] smite: add funding_signed oracle Signed-off-by: Nishant Bansal --- smite-scenarios/src/executor.rs | 86 +++++----- smite-scenarios/src/executor/tests.rs | 123 ++++++++++++-- smite/src/channel_tx/commitment.rs | 13 ++ smite/src/oracles.rs | 2 + smite/src/oracles/funding_signed.rs | 233 ++++++++++++++++++++++++++ smite/src/violation.rs | 19 ++- 6 files changed, 415 insertions(+), 61 deletions(-) create mode 100644 smite/src/oracles/funding_signed.rs diff --git a/smite-scenarios/src/executor.rs b/smite-scenarios/src/executor.rs index a63eb3b3..70dad2a4 100644 --- a/smite-scenarios/src/executor.rs +++ b/smite-scenarios/src/executor.rs @@ -18,7 +18,9 @@ use smite::channel_tx::{ build_funding_transaction, }; use smite::noise::{ConnectionError, NoiseConnection}; -use smite::oracles::{AcceptChannelContext, AcceptChannelOracle, Oracle}; +use smite::oracles::{ + AcceptChannelContext, AcceptChannelOracle, FundingSignedContext, FundingSignedOracle, Oracle, +}; use smite::pending_channel::PendingChannel; use smite::violation::Violation; @@ -516,7 +518,11 @@ impl Executor { log::debug!("[{:?}] RecvFundingSigned: waiting", start.elapsed()); let fs: FundingSigned = recv_bolt(&mut self.conn, RECV_IDLE_TIMEOUT)?; log::debug!("[{:?}] RecvFundingSigned: received", start.elapsed()); - verify_funding_signed(&fs, &self.channel_states)?; + FundingSignedOracle.evaluate(&FundingSignedContext { + funding_signed: &fs, + channel: self.channel_states.get(&fs.channel_id), + })?; + record_recv_funding_signed(&mut self.channel_states, &fs); Some(Variable::ChannelId(fs.channel_id)) } @@ -849,17 +855,6 @@ fn build_funding_created( }; let signature = config.sign_counterparty_commitment(&state, &holder); - let channel_id = ChannelId::v1_from_funding_outpoint(config.funding_outpoint); - - // Check whether the funding outpoint is valid and contains the negotiated - // amount and funding script. If not, there is a good chance the target will - // neither complete the funding flow nor send an error message. - let is_funding_outpoint_valid = funding_tx.matches_funding_output( - &open_channel.funding_pubkey, - &accept_channel.funding_pubkey, - open_channel.funding_satoshis, - ); - // Only track a new channel when this negotiation has not built a // `funding_created` yet. If it has, we are likely resending one for the // same `temporary_channel_id` with a different outpoint, which the target @@ -869,6 +864,24 @@ fn build_funding_created( // This also means that building the same message again must not clobber a // channel whose state has already been established (and possibly advanced). if !pending.funding_built { + let channel_id = ChannelId::v1_from_funding_outpoint(config.funding_outpoint); + + // Check whether the funding outpoint is valid and contains the + // negotiated amount and funding script. If not, there is a good chance + // the target will neither complete the funding flow nor send an error + // message. + let is_funding_outpoint_valid = funding_tx.matches_funding_output( + &open_channel.funding_pubkey, + &accept_channel.funding_pubkey, + open_channel.funding_satoshis, + ); + // TODO: Once we support sending malformed signatures, update this state + // when constructing one so that the peer's acceptance can be detected + // as a violation. + let opener_funding_pubkey = + PublicKey::from_secret_key(&Secp256k1::new(), &opener_funding_privkey); + let sent_invalid_signature = opener_funding_pubkey != open_channel.funding_pubkey; + channel_states.entry(channel_id).or_insert_with(|| { ChannelState::new( config, @@ -876,6 +889,7 @@ fn build_funding_created( state, is_funding_outpoint_valid, mined_txids.contains(&funding_outpoint.txid), + sent_invalid_signature, ) }); } @@ -1212,9 +1226,9 @@ fn recv_channel_ready( /// A `channel_ready` is expected when a tracked channel is still at commitment /// number 0, the counterparty's next per-commitment point is unknown, the /// advertised funding outpoint pays the negotiated funding output, the funding -/// transaction was mined only after we sent `funding_created`, and it has at -/// least `minimum_depth` confirmations (as specified in the received -/// `accept_channel`). +/// transaction was mined only after we sent `funding_created`, we have not sent +/// a signature the peer is required to reject, and it has at least +/// `minimum_depth` confirmations (as specified in the received `accept_channel`). fn is_channel_ready_expected( channel_states: &HashMap, bitcoin_cli: &mut impl BitcoinRpc, @@ -1224,34 +1238,12 @@ fn is_channel_ready_expected( && state.next_counterparty_per_commitment_point().is_none() && state.is_funding_outpoint_valid && !state.was_funding_mined_prematurely + && !state.sent_invalid_signature && bitcoin_cli.get_transaction_confirmations(state.config.funding_outpoint.txid) >= state.config.minimum_depth }) } -/// Verifies the counterparty's signature from a `funding_signed` message using -/// the channel state associated with the message's `channel_id`. -/// -/// # Errors -/// -/// Returns [`Violation::UnknownChannel`] if no channel state exists for the -/// given `channel_id`, or [`Violation::InvalidCounterpartySignature`] if the -/// signature is invalid for the holder's initial commitment transaction. -fn verify_funding_signed( - fs: &FundingSigned, - channel_states: &HashMap, -) -> Result<(), Violation> { - let state = channel_states - .get(&fs.channel_id) - .ok_or(Violation::UnknownChannel(fs.channel_id))?; - - state - .config - .verify_counterparty_signature(&state.commitment, &state.holder, &fs.signature) - .then_some(()) - .ok_or(Violation::InvalidCounterpartySignature(fs.channel_id)) -} - /// Records a sent `open_channel`, keyed by `temporary_channel_id`, so the /// funding flow can build commitments from the values actually put on the wire. /// @@ -1297,6 +1289,22 @@ fn record_recv_accept_channel( .accept_channel = Some(accept_channel.clone()); } +/// Records that a `funding_signed` has been accepted for its channel. +/// +/// # Panics +/// +/// Panics if no matching channel exists. This should be unreachable, as +/// `FundingSignedOracle` reports such messages as a [`Violation`]. +fn record_recv_funding_signed( + channel_states: &mut HashMap, + funding_signed: &FundingSigned, +) { + channel_states + .get_mut(&funding_signed.channel_id) + .expect("FundingSignedOracle guaranteed this channel_id exists") + .funding_signed_received = true; +} + /// Extracts a field from a parsed `accept_channel` message. fn extract_field(ac: &AcceptChannel, field: AcceptChannelField) -> Variable { match field { diff --git a/smite-scenarios/src/executor/tests.rs b/smite-scenarios/src/executor/tests.rs index 92bbc936..084ab2d7 100644 --- a/smite-scenarios/src/executor/tests.rs +++ b/smite-scenarios/src/executor/tests.rs @@ -861,12 +861,12 @@ fn execute_send_funding_created_uses_wire_funding_pubkey() { let mut b = ProgramBuilder::new(); let funding = create_funding_tx(&mut b); b.append(Operation::BroadcastTransaction, &[funding.tx]); - let funding_created = send_funding_created_with(&mut b, funding, funding.acceptor_privkey); - b.append(Operation::RecvFundingSigned, &[funding_created.sent]); + send_funding_created_with(&mut b, funding, funding.acceptor_privkey); - // The acceptor's signature still verifies, because the config is built - // from the wire pubkeys rather than from the swapped privkey. - let mut fx = recv_funding_signed_fixture(); + // Signing with a key the peer did not negotiate marks the channel as + // having sent an invalid signature, so receiving a `funding_signed` would + // be a violation. We therefore stop after sending `funding_created`. + let mut fx = Fixture::new().with_negotiation(sample_funding_negotiation()); fx.run(&b.build()); let secp = Secp256k1::new(); @@ -1004,10 +1004,11 @@ fn execute_recv_funding_signed_unknown_channel() { .with_negotiation(sample_funding_negotiation()) .queue(&funding_signed_reply(channel_id)) .run_err(&send_funding_created_and_recv_funding_signed_program()); - assert!(matches!( - err, - ExecuteError::Violation(Violation::UnknownChannel(id)) if id == channel_id - )); + let ExecuteError::Violation(Violation::InvalidFundingSigned(id, reason)) = &err else { + panic!("unexpected error: {err:?}"); + }; + assert_eq!(*id, channel_id); + assert!(reason.contains("unknown channel_id: no funding_created was sent for this channel")); } #[test] @@ -1021,10 +1022,53 @@ fn execute_recv_funding_signed_invalid_signature() { .expect("zero bytes parse as a signature"), })) .run_err(&send_funding_created_and_recv_funding_signed_program()); - assert!(matches!( - err, - ExecuteError::Violation(Violation::InvalidCounterpartySignature(id)) if id == channel_id - )); + let ExecuteError::Violation(Violation::InvalidFundingSigned(id, reason)) = &err else { + panic!("unexpected error: {err:?}"); + }; + assert_eq!(*id, channel_id); + assert!(reason.contains("invalid funding_signed: signature is not valid")); +} + +#[test] +fn execute_recv_funding_signed_after_invalid_funding_created() { + let channel_id = funding_channel_id(); + + // Sign the commitment with the acceptor's private key instead of the + // opener's, so the signature does not match the `funding_pubkey` negotiated + // in `open_channel`. + let mut b = ProgramBuilder::new(); + let funding = create_funding_tx(&mut b); + let funding_created = send_funding_created_with(&mut b, funding, funding.acceptor_privkey); + b.append(Operation::RecvFundingSigned, &[funding_created.sent]); + + let err = recv_funding_signed_fixture().run_err(&b.build()); + let ExecuteError::Violation(Violation::InvalidFundingSigned(id, reason)) = &err else { + panic!("unexpected error: {err:?}"); + }; + assert_eq!(*id, channel_id); + assert!(reason.contains("accepted invalid funding_created: signature is not valid")); +} + +#[test] +fn execute_recv_funding_signed_duplicate() { + let channel_id = funding_channel_id(); + + // Resend the same `funding_created`, which maps to the same channel, and + // receive a valid `funding_signed` for each. + let mut b = ProgramBuilder::new(); + let first = send_funding_created(&mut b); + b.append(Operation::RecvFundingSigned, &[first.sent]); + let second = send_funding_created_with(&mut b, first.tx, first.tx.opener_privkey); + b.append(Operation::RecvFundingSigned, &[second.sent]); + + let err = recv_funding_signed_fixture() + .queue(&funding_signed_reply(channel_id)) + .run_err(&b.build()); + let ExecuteError::Violation(Violation::InvalidFundingSigned(id, reason)) = &err else { + panic!("unexpected error: {err:?}"); + }; + assert_eq!(*id, channel_id); + assert!(reason.contains("duplicate funding_signed: channel already funded")); } #[test] @@ -1134,11 +1178,20 @@ fn execute_send_shutdown_empty_scriptpubkey() { #[test] fn execute_recv_channel_ready_invalid_funding_outpoint_is_noop() { - // Corrupt the negotiated opener funding pubkey so the broadcast funding + // Corrupt the negotiated acceptor funding pubkey so the broadcast funding // transaction's output no longer pays the negotiated 2-of-2 script, // marking the funding outpoint invalid. + // + // We corrupt the acceptor's and not the opener's, since the latter would no + // longer match the resolved opener private key and so also set + // `sent_invalid_signature`, leaving the invalid funding outpoint gate + // unreached. let mut negotiation = sample_funding_negotiation(); - negotiation.open_channel.funding_pubkey = sample_pubkey(1); + negotiation + .accept_channel + .as_mut() + .expect("accept_channel must be present") + .funding_pubkey = sample_pubkey(1); // The corrupted pubkey changes the funding script, so our precomputed // funding_signed signature will no longer verify correctly. That @@ -1159,6 +1212,9 @@ fn execute_recv_channel_ready_invalid_funding_outpoint_is_noop() { // The target's next per-commitment point is still unknown and the queued // `channel_ready` remains untouched. let state = fx.channel_state(&funding_channel_id()); + assert!(!state.is_funding_outpoint_valid); + assert!(!state.was_funding_mined_prematurely); + assert!(!state.sent_invalid_signature); assert!(state.next_counterparty_per_commitment_point().is_none()); assert_eq!(fx.queued_len(), 1); } @@ -1178,6 +1234,9 @@ fn execute_recv_channel_ready_below_minimum_depth_is_noop() { // The target's next per-commitment point is still unknown and the queued // `channel_ready` remains untouched. let state = fx.channel_state(&funding_channel_id()); + assert!(state.is_funding_outpoint_valid); + assert!(!state.was_funding_mined_prematurely); + assert!(!state.sent_invalid_signature); assert!(state.next_counterparty_per_commitment_point().is_none()); assert_eq!(fx.queued_len(), 1); } @@ -1225,11 +1284,45 @@ fn execute_recv_channel_ready_funding_mined_prematurely_is_noop() { // The target's next per-commitment point is still unknown and the queued // `channel_ready` remains untouched. let state = fx.channel_state(&funding_channel_id()); + assert!(state.is_funding_outpoint_valid); assert!(state.was_funding_mined_prematurely); + assert!(!state.sent_invalid_signature); assert!(state.next_counterparty_per_commitment_point().is_none()); assert_eq!(fx.queued_len(), 1); } +#[test] +fn execute_recv_channel_ready_invalid_signature_is_noop() { + let (mut fx, _) = recv_channel_ready_fixture(); + + let mut b = ProgramBuilder::new(); + let funding = create_funding_tx(&mut b); + b.append(Operation::BroadcastTransaction, &[funding.tx]); + // Sign the commitment with the acceptor's private key instead of the + // opener's, so the signature does not match the `funding_pubkey` negotiated + // in `open_channel`. + // + // A `funding_signed` answering a `funding_created` we signed with the wrong + // key is itself a violation, which `FundingSignedOracle` reports. So we + // don't receive one, letting `RecvChannelReady` be reached. + send_funding_created_with(&mut b, funding, funding.acceptor_privkey); + b.append(Operation::MineBlocks(8), &[]); + b.append(Operation::RecvChannelReady, &[]); + + // Having signed with the wrong key, the target does not owe us a + // `channel_ready`, so `RecvChannelReady` must be a no-op. + fx.run(&b.build()); + + // The target's next per-commitment point is still unknown and the queued + // `funding_signed` and `channel_ready` remain untouched. + let state = fx.channel_state(&funding_channel_id()); + assert!(state.is_funding_outpoint_valid); + assert!(!state.was_funding_mined_prematurely); + assert!(state.sent_invalid_signature); + assert!(state.next_counterparty_per_commitment_point().is_none()); + assert_eq!(fx.queued_len(), 2); +} + // -- extract_field tests -- // TODO: Once we can actually construct and send accept_channel messages, it diff --git a/smite/src/channel_tx/commitment.rs b/smite/src/channel_tx/commitment.rs index d4cfebec..3769a9e0 100644 --- a/smite/src/channel_tx/commitment.rs +++ b/smite/src/channel_tx/commitment.rs @@ -133,6 +133,7 @@ struct TxCreationKeys { /// State of a single channel, including its static configuration, holder /// identity, and current commitment state. +#[allow(clippy::struct_excessive_bools)] // Independent flags, not a state machine pub struct ChannelState { /// Channel configuration established at channel creation and unchanged /// for the lifetime of the channel. @@ -159,6 +160,15 @@ pub struct ChannelState { /// after the block height at which they receive `funding_created`, so they /// may never observe it and never send `channel_ready`. pub was_funding_mined_prematurely: bool, + /// Whether we have ever sent a signature the peer was required to reject, + /// such as a `funding_created`, `commitment_signed`, or HTLC signature etc. + /// it cannot verify. Set on the first occurrence and never cleared, since + /// BOLT 2 requires the peer to fail the channel or disconnect in response, + /// any subsequent positive response is therefore a violation. + pub sent_invalid_signature: bool, + /// Whether a `funding_signed` has already been accepted for this channel. + /// Any later one means the target re-signed a channel it already funded. + pub funding_signed_received: bool, } impl Side { @@ -188,6 +198,7 @@ impl ChannelState { commitment: CommitmentState, is_funding_outpoint_valid: bool, was_funding_mined_prematurely: bool, + sent_invalid_signature: bool, ) -> Self { Self { config, @@ -197,6 +208,8 @@ impl ChannelState { acceptor_next_per_commitment_point: None, is_funding_outpoint_valid, was_funding_mined_prematurely, + sent_invalid_signature, + funding_signed_received: false, } } diff --git a/smite/src/oracles.rs b/smite/src/oracles.rs index ee2d7042..937b2fc2 100644 --- a/smite/src/oracles.rs +++ b/smite/src/oracles.rs @@ -3,9 +3,11 @@ //! Oracles evaluate conditions beyond simple crashes. mod accept_channel; +mod funding_signed; use super::violation::Violation; pub use accept_channel::{AcceptChannelContext, AcceptChannelOracle}; +pub use funding_signed::{FundingSignedContext, FundingSignedOracle}; /// `Oracle` evaluates a condition against some context pub trait Oracle { diff --git a/smite/src/oracles/funding_signed.rs b/smite/src/oracles/funding_signed.rs new file mode 100644 index 00000000..55550cd0 --- /dev/null +++ b/smite/src/oracles/funding_signed.rs @@ -0,0 +1,233 @@ +//! BOLT 2 `funding_signed` oracle, for the v1 outbound channel funding flow. + +use super::Oracle; +use crate::bolt::FundingSigned; +use crate::channel_tx::ChannelState; +use crate::violation::Violation; + +/// Context for `FundingSignedOracle` +pub struct FundingSignedContext<'a> { + /// The `funding_signed` received from the peer. + pub funding_signed: &'a FundingSigned, + /// The channel the `funding_signed` belongs to, identified by its + /// `channel_id`, or `None` if no matching `funding_created` was sent. + pub channel: Option<&'a ChannelState>, +} + +/// Checks whether the `funding_created` answered by a `funding_signed` satisfied +/// the BOLT 2 v1 channel establishment requirements for acceptance, and whether +/// the `funding_signed` itself satisfies them. +pub struct FundingSignedOracle; + +impl Oracle> for FundingSignedOracle { + fn evaluate(&self, context: &FundingSignedContext<'_>) -> Result<(), Violation> { + // Check that the `funding_signed` answers a known `funding_created`. + let Some(channel) = context.channel else { + return Err(Violation::InvalidFundingSigned( + context.funding_signed.channel_id, + "unknown channel_id: no funding_created was sent for this channel".to_string(), + )); + }; + + // Check that this is the first `funding_signed` for this channel. + // + // NOTE: Besides a genuine resend, this also catches a valid + // `funding_signed` for a new channel whose channel_id collides with an + // existing one due to the XOR relationship between channel_id and the + // funding outpoint. BOLT 2 does not specify how to handle this, but + // LND, LDK, and Eclair all reject such channels. Since channel_id is + // reused across messages, accepting a collision could cause + // cross-channel state contamination and lead to more serious bugs. Such + // collisions are also extremely unlikely to occur naturally, so the + // target should reject them. + // see: https://github.com/ElementsProject/lightning/issues/9274#issuecomment-5316110622 + if channel.funding_signed_received { + return Err(Violation::InvalidFundingSigned( + context.funding_signed.channel_id, + "duplicate funding_signed: channel already funded".to_string(), + )); + } + + // Check whether we sent a valid signature during `funding_created`. + if channel.sent_invalid_signature { + return Err(Violation::InvalidFundingSigned( + context.funding_signed.channel_id, + "accepted invalid funding_created: signature is not valid".to_string(), + )); + } + + // Check that the `funding_signed` itself is valid. + // + // NOTE: If the colliding channel never received its own + // `funding_signed`, a channel_id collision (see above) surfaces here as + // an invalid signature instead. + if !channel.config.verify_counterparty_signature( + &channel.commitment, + &channel.holder, + &context.funding_signed.signature, + ) { + return Err(Violation::InvalidFundingSigned( + context.funding_signed.channel_id, + "invalid funding_signed: signature is not valid".to_string(), + )); + } + + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::bolt::{COMPACT_SIGNATURE_SIZE, ChannelId, Features}; + use crate::channel_tx::{ChannelConfig, ChannelPartyConfig, HolderIdentity, Side}; + use bitcoin::OutPoint; + use bitcoin::secp256k1::ecdsa::Signature; + use bitcoin::secp256k1::{PublicKey, Secp256k1, SecretKey}; + + fn secret_key(seed: u8) -> SecretKey { + SecretKey::from_slice(&[seed; 32]).expect("valid secret key") + } + + fn pubkey(seed: u8) -> PublicKey { + PublicKey::from_secret_key(&Secp256k1::new(), &secret_key(seed)) + } + + /// Valid channel state for testing. + fn channel_state() -> ChannelState { + let pkey1 = pubkey(1); + let pkey2 = pubkey(2); + + let config = ChannelConfig { + funding_outpoint: OutPoint { + txid: "09b0549b35f14ee862f63bd75811c6c27963c4dea6766ec6836952ec78df1e7e" + .parse() + .expect("valid txid hex"), + vout: 0, + }, + funding_satoshis: 10_000_000, + channel_type: Features::from_bits(&[Features::OPTION_STATIC_REMOTEKEY]), + opener: ChannelPartyConfig { + funding_pubkey: pkey1, + payment_basepoint: pkey1, + revocation_basepoint: pkey1, + delayed_payment_basepoint: pkey1, + dust_limit_satoshis: 546, + to_self_delay: 144, + }, + acceptor: ChannelPartyConfig { + funding_pubkey: pkey2, + payment_basepoint: pkey2, + revocation_basepoint: pkey2, + delayed_payment_basepoint: pkey2, + dust_limit_satoshis: 546, + to_self_delay: 144, + }, + minimum_depth: 8, + }; + let commitment = config + .new_initial_commitment(3_000_000_000, 15_000, pkey1, pkey2) + .expect("valid initial commitment"); + let holder = HolderIdentity { + side: Side::Opener, + funding_privkey: secret_key(1), + }; + + ChannelState::new(config, holder, commitment, true, false, false) + } + + /// Valid `funding_signed` message for testing. + fn funding_signed(channel: &ChannelState) -> FundingSigned { + let acceptor = HolderIdentity { + side: Side::Acceptor, + funding_privkey: secret_key(2), + }; + FundingSigned { + channel_id: ChannelId::v1_from_funding_outpoint(channel.config.funding_outpoint), + signature: channel + .config + .sign_counterparty_commitment(&channel.commitment, &acceptor), + } + } + + #[track_caller] + fn assert_pass(funding_signed: &FundingSigned, channel: Option<&ChannelState>) { + if let Err(err) = FundingSignedOracle.evaluate(&FundingSignedContext { + funding_signed, + channel, + }) { + panic!("expected pass, got: {err}"); + } + } + + #[track_caller] + fn assert_fail(funding_signed: &FundingSigned, channel: Option<&ChannelState>, expected: &str) { + match FundingSignedOracle.evaluate(&FundingSignedContext { + funding_signed, + channel, + }) { + Err(Violation::InvalidFundingSigned(chan_id, reason)) => { + assert_eq!(funding_signed.channel_id, chan_id); + assert!( + reason.contains(expected), + "unexpected failure reason: {reason}" + ); + } + _ => panic!("expected failure: {expected}"), + } + } + + #[test] + fn conforming_funding_signed_passes() { + let channel = channel_state(); + + assert_pass(&funding_signed(&channel), Some(&channel)); + } + + #[test] + fn funding_signed_for_unknown_channel_id() { + assert_fail( + &funding_signed(&channel_state()), + None, + "unknown channel_id: no funding_created was sent for this channel", + ); + } + + #[test] + fn duplicate_funding_signed() { + let mut channel = channel_state(); + channel.funding_signed_received = true; + + assert_fail( + &funding_signed(&channel), + Some(&channel), + "duplicate funding_signed: channel already funded", + ); + } + + #[test] + fn funding_created_with_invalid_signature() { + let mut channel = channel_state(); + channel.sent_invalid_signature = true; + + assert_fail( + &funding_signed(&channel), + Some(&channel), + "accepted invalid funding_created: signature is not valid", + ); + } + + #[test] + fn funding_signed_with_invalid_signature() { + let channel = channel_state(); + let mut fs = funding_signed(&channel); + fs.signature = Signature::from_compact(&[0u8; COMPACT_SIGNATURE_SIZE]) + .expect("zero bytes parse as a signature"); + + assert_fail( + &fs, + Some(&channel), + "invalid funding_signed: signature is not valid", + ); + } +} diff --git a/smite/src/violation.rs b/smite/src/violation.rs index 515c9b52..a2ff8358 100644 --- a/smite/src/violation.rs +++ b/smite/src/violation.rs @@ -36,13 +36,18 @@ pub enum Violation { #[error("invalid accept_channel for temporary_channel_id {0}: {1}")] InvalidAcceptChannel(TemporaryChannelId, String), - /// The target sent a `funding_signed` or `channel_ready` for a `channel_id` - /// we never opened, i.e. one for which no state was ever established. + /// The target's `funding_signed` broke a BOLT 2 requirement, as judged by + /// [`crate::oracles::FundingSignedOracle`]. The reason names the breached + /// requirement, one of: + /// - it names a `channel_id` we sent no `funding_created` for, + /// - it names a `channel_id` that already received a `funding_signed`, + /// - it answers a `funding_created` with an invalid signature, or + /// - its signature is not valid for the holder's commitment transaction. + #[error("invalid funding_signed for channel_id {0}: {1}")] + InvalidFundingSigned(ChannelId, String), + + /// The target sent a `channel_ready` for a `channel_id` we never opened, + /// i.e. one for which no state was ever established. #[error("unknown channel: no tracked state for channel_id {0}")] UnknownChannel(ChannelId), - - /// The target's `funding_signed` signature failed to verify against the - /// holder's initial commitment transaction. - #[error("invalid counterparty signature for channel_id {0}")] - InvalidCounterpartySignature(ChannelId), } From da81c4ab16c8ee47d7f94eb9059322cca8bf6e4c Mon Sep 17 00:00:00 2001 From: Nishant Bansal Date: Thu, 17 Sep 2026 13:32:24 +0530 Subject: [PATCH 2/4] smite-ir: add Malformation for computed message fields Some message fields are computed rather than supplied as IR variables, so no mutator can give them invalid values. Add Malformation to overwrite such a field in the encoded message before sending. MalformableField and MessageType::malformable_fields() live in bolt next to the codecs. Signed-off-by: Nishant Bansal --- smite-ir/src/lib.rs | 3 ++ smite-ir/src/malform.rs | 93 +++++++++++++++++++++++++++++++++++++++++ smite/src/bolt.rs | 32 ++++++++++++++ 3 files changed, 128 insertions(+) create mode 100644 smite-ir/src/malform.rs diff --git a/smite-ir/src/lib.rs b/smite-ir/src/lib.rs index 70992c4b..57414000 100644 --- a/smite-ir/src/lib.rs +++ b/smite-ir/src/lib.rs @@ -7,6 +7,7 @@ //! //! # Modules //! - [`instruction`] - Single IR instruction (operation + input references). +//! - [`malform`] - Byte-level edits applied to encoded messages before sending. //! - [`minimizers`] - Shrink a program while preserving interesting behaviour. //! - [`operation`] - Operations that load, compute, build or act. //! - [`program`] - Ordered list of instructions. @@ -15,6 +16,7 @@ pub mod builder; pub mod generators; pub mod instruction; +pub mod malform; pub mod minimizers; pub mod mutators; pub mod operation; @@ -24,6 +26,7 @@ pub mod variable; pub use builder::ProgramBuilder; pub use generators::Generator; pub use instruction::Instruction; +pub use malform::Malformation; pub use minimizers::Minimizer; pub use mutators::Mutator; pub use operation::Operation; diff --git a/smite-ir/src/malform.rs b/smite-ir/src/malform.rs new file mode 100644 index 00000000..fc3e75f8 --- /dev/null +++ b/smite-ir/src/malform.rs @@ -0,0 +1,93 @@ +//! Byte-level malformation of encoded messages. +//! +//! A few message fields are computed rather than supplied as IR variables, so +//! no mutator can give them an invalid value -- see +//! [`smite::bolt::MalformableField`]. A [`Malformation`] overwrites one such +//! field of an encoded message immediately before it goes on the wire. Which +//! fields those are is fixed per message type by +//! [`smite::bolt::MessageType::malformable_fields`], which lives with the +//! codecs. + +use serde::{Deserialize, Serialize}; + +/// Overwrites one field of an encoded message with the given bytes. +#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)] +pub struct Malformation { + /// Byte offset into the encoded message, including the 2-byte type prefix. + pub offset: u16, + /// Replacement bytes. + pub bytes: Vec, +} + +impl Malformation { + /// Overwrites `msg` from `offset` with the replacement bytes. Returns + /// `true` if the bytes changed. + /// + /// # Panics + /// + /// Panics if the replacement runs past the end of `msg`. Offsets and + /// lengths come from the fixed-size fields of + /// [`smite::bolt::MalformableField`], so a panic here means a field table + /// and the codec it describes have drifted apart. + pub fn apply(&self, msg: &mut [u8]) -> bool { + let start = self.offset as usize; + let end = start + self.bytes.len(); + assert!( + end <= msg.len(), + "malformation of {} bytes at offset {} runs past the end of a {}-byte message", + self.bytes.len(), + self.offset, + msg.len(), + ); + + if msg[start..end] == self.bytes[..] { + return false; + } + + msg[start..end].copy_from_slice(&self.bytes); + true + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn apply_overwrites_field() { + let mut msg = vec![0x00, 0x11, 0x22, 0xaa, 0xaa, 0xaa]; + let malformation = Malformation { + offset: 3, + bytes: vec![0xff, 0xff, 0xff], + }; + + assert!(malformation.apply(&mut msg)); + assert_eq!(msg, vec![0x00, 0x11, 0x22, 0xff, 0xff, 0xff]); + } + + #[test] + fn apply_reports_unchanged_bytes() { + let mut msg = vec![0x00, 0x22, 0xff, 0xff, 0xbb]; + let malformation = Malformation { + offset: 2, + bytes: vec![0xff, 0xff], + }; + + assert!(!malformation.apply(&mut msg)); + assert_eq!(msg, vec![0x00, 0x22, 0xff, 0xff, 0xbb]); + } + + #[test] + #[should_panic( + expected = "malformation of 2 bytes at offset 2 runs past the end of a 3-byte message" + )] + fn apply_past_end_panics() { + let mut msg = vec![0x00, 0x22, 0xaa]; + let malformation = Malformation { + offset: 2, + bytes: vec![0xff, 0xff], + }; + + malformation.apply(&mut msg); + } +} diff --git a/smite/src/bolt.rs b/smite/src/bolt.rs index 978fa434..18192c01 100644 --- a/smite/src/bolt.rs +++ b/smite/src/bolt.rs @@ -138,6 +138,25 @@ pub enum BoltError { }, } +/// A message field whose encoded bytes can be overwritten before the message +/// is sent. +/// +/// These identify fields for which the IR can only construct valid values, so +/// invalid values can only be put on the wire by overwriting their encoded +/// bytes. All other fields are reachable through the IR's parameters. +/// +/// Offsets include the 2-byte message type prefix and therefore index directly +/// into the encoded message. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct MalformableField { + /// Field name to overwrite. + pub name: &'static str, + /// Byte offset in the encoded message, including the message type prefix. + pub offset: u16, + /// Field length in bytes. + pub len: u16, +} + /// A BOLT message type number that displays as `name(type)`, e.g. /// `open_channel(32)`. #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -264,6 +283,19 @@ impl MessageType { _ => "unknown", } } + + /// Returns the fields of this message type whose encoded bytes may be + /// overwritten before the message is sent. + /// + /// # Panics + /// + /// Panics if this message type has no malformable field table. This indicates + /// that a malformation was attached to a message type that is not supported + /// here. + #[must_use] + pub fn malformable_fields(self) -> &'static [MalformableField] { + unreachable!("no malformable field table for message type {self}"); + } } impl std::fmt::Display for MessageType { From 06a6eb82ac176fa5c979dde04e15c978c28a0c05 Mon Sep 17 00:00:00 2001 From: Nishant Bansal Date: Thu, 17 Sep 2026 14:12:36 +0530 Subject: [PATCH 3/4] smite-ir: mutate funding_created malformations Add an optional Malformation to SendFundingCreated so the parameter mutator can overwrite funding_txid, funding_output_index or signature. It can set, replace or clear the malformation, using random bytes, a repeated byte or an interesting integer. FundingCreated now lists these fields and their offsets. The executor does not apply the malformation yet. Signed-off-by: Nishant Bansal --- smite-ir/src/generators/funding_created.rs | 2 +- smite-ir/src/generators/funding_flow.rs | 2 +- smite-ir/src/mutators/operation_param.rs | 99 ++++++++++++++++++- smite-ir/src/operation.rs | 37 +++++-- smite-ir/src/tests.rs | 69 +++++++++++-- smite-scenarios/src/executor.rs | 2 +- smite-scenarios/src/executor/tests.rs | 2 +- .../src/executor/tests/programs.rs | 2 +- smite/src/bolt.rs | 5 +- smite/src/bolt/funding_created.rs | 54 +++++++++- 10 files changed, 249 insertions(+), 25 deletions(-) diff --git a/smite-ir/src/generators/funding_created.rs b/smite-ir/src/generators/funding_created.rs index eb05b2ae..f84163a0 100644 --- a/smite-ir/src/generators/funding_created.rs +++ b/smite-ir/src/generators/funding_created.rs @@ -40,7 +40,7 @@ impl Generator for FundingCreatedGenerator { // Build and send funding_created. let sent_funding_created = builder.append( - Operation::SendFundingCreated, + Operation::SendFundingCreated { malformation: None }, &[ funding_transaction, opener_funding_privkey, diff --git a/smite-ir/src/generators/funding_flow.rs b/smite-ir/src/generators/funding_flow.rs index b07f6791..324f0262 100644 --- a/smite-ir/src/generators/funding_flow.rs +++ b/smite-ir/src/generators/funding_flow.rs @@ -51,7 +51,7 @@ impl Generator for FundingFlowGenerator { // Build and send funding_created. let sent_funding_created = builder.append( - Operation::SendFundingCreated, + Operation::SendFundingCreated { malformation: None }, &[ funding_transaction, funding_privkey, diff --git a/smite-ir/src/mutators/operation_param.rs b/smite-ir/src/mutators/operation_param.rs index 745aa415..0383bed7 100644 --- a/smite-ir/src/mutators/operation_param.rs +++ b/smite-ir/src/mutators/operation_param.rs @@ -3,16 +3,20 @@ use bitcoin::secp256k1::SecretKey; use rand::seq::IteratorRandom; use rand::{Rng, RngExt}; -use smite::bolt::{ChannelTypeVariant, MAX_MESSAGE_SIZE, ShortChannelId}; +use smite::bolt::{ + ChannelTypeVariant, MAX_MESSAGE_SIZE, MalformableField, MessageType, ShortChannelId, +}; use super::Mutator; +use crate::malform::Malformation; use crate::operation::{AcceptChannelField, ShutdownScriptVariant}; use crate::{Operation, Program}; /// Mutates the embedded parameter of a randomly chosen `is_param_mutable` /// instruction. For numeric loads, applies a random arithmetic tweak. For byte /// loads, flips/adds/removes bytes. For extract operations, swaps to a random -/// field with the same output type. +/// field with the same output type. For messages with malformations, randomly +/// sets, replaces, or clears the malformation applied to the encoded message. pub struct OperationParamMutator; impl Mutator for OperationParamMutator { @@ -98,6 +102,11 @@ fn mutate_operation(op: &mut Operation, rng: &mut impl Rng) -> bool { } true } + Operation::SendFundingCreated { malformation } => mutate_malformation( + malformation, + MessageType::FUNDING_CREATED.malformable_fields(), + rng, + ), Operation::SendChannelReady { include_alias } => { // Toggle the SCID alias TLV. Flipping always changes the value; // a random bool could repeat it and waste the mutation. @@ -117,7 +126,6 @@ fn mutate_operation(op: &mut Operation, rng: &mut impl Rng) -> bool { | Operation::BuildAnnouncementSignatures | Operation::SendMessage | Operation::SendOpenChannel - | Operation::SendFundingCreated | Operation::SendShutdown | Operation::RecvAcceptChannel | Operation::RecvFundingSigned @@ -384,6 +392,81 @@ fn mutate_private_key(bytes: &mut [u8; 32], rng: &mut impl Rng) { } } +// -- Malformations -- + +/// Sets, replaces, or clears the malformation of a `Send*` operation. +/// +/// `fields` is the allowlist of fields that can be malformed in the message +/// sent by the operation. Other fields are already reachable through the IR's +/// own parameters, so malforming them would only duplicate the tweaks above. +/// +/// Returns `true` if the malformation changed. +/// +/// # Panics +/// +/// Panics if `fields` is empty or if a chosen field is zero-length. A message +/// type reaching here must have at least one malformable field, and a +/// zero-length field would encode to no replacement bytes at all, so either +/// indicates that the operation and its field table have drifted apart. +fn mutate_malformation( + malformation: &mut Option, + fields: &[MalformableField], + rng: &mut impl Rng, +) -> bool { + assert!(!fields.is_empty(), "no malformable fields for operation"); + + // Half the time, clear instead of replacing, so an already malformed + // message can find its way back to a well-formed encoding. + if malformation.is_some() && rng.random() { + *malformation = None; + return true; + } + + // Otherwise malform a randomly chosen field, overwriting any existing + // malformation. Redrawing the same offset and bytes is not a change. + let field = &fields[rng.random_range(0..fields.len())]; + assert!( + field.len > 0, + "zero-length malformable field {}", + field.name + ); + + let new = Malformation { + offset: field.offset, + bytes: malformed_bytes(field.len as usize, rng), + }; + if malformation.as_ref() == Some(&new) { + return false; + } + *malformation = Some(new); + true +} + +/// Generates `len` replacement bytes for a malformed field. +/// +/// Either fully random bytes or a repeated byte biased toward the interesting +/// byte values. A multi-byte field as wide as an interesting integer can also +/// be replaced by one directly, in big endian. +fn malformed_bytes(len: usize, rng: &mut impl Rng) -> Vec { + let strategies = if matches!(len, 2 | 4 | 8) { 3 } else { 2 }; + match rng.random_range(0..strategies) { + // Fully random bytes. + 0 => { + let mut bytes = vec![0u8; len]; + rng.fill(&mut bytes[..]); + bytes + } + // A repeated byte, biased toward the interesting byte values. + 1 => vec![repeated_byte(rng); len], + // An interesting integer of the field's exact width, in big endian. + _ => match len { + 2 => interesting_u16(rng).to_be_bytes().to_vec(), + 4 => interesting_u32(rng).to_be_bytes().to_vec(), + _ => interesting_u64(rng).to_be_bytes().to_vec(), + }, + } +} + // -- Enum mutations -- /// Swaps to a different `ChannelTypeVariant`. @@ -723,4 +806,14 @@ mod tests { assert!(all_zeros, "mutate_bytes never produced an all-zero field"); assert!(all_ones, "mutate_bytes never produced an all-ones field"); } + + #[test] + fn malformed_bytes_has_requested_len() { + let mut rng = SmallRng::seed_from_u64(0); + for len in [1, 2, 3, 4, 8, 32, 64] { + for _ in 0..100 { + assert_eq!(malformed_bytes(len, &mut rng).len(), len); + } + } + } } diff --git a/smite-ir/src/operation.rs b/smite-ir/src/operation.rs index 22a17899..756d8f6c 100644 --- a/smite-ir/src/operation.rs +++ b/smite-ir/src/operation.rs @@ -16,7 +16,7 @@ use rand::{Rng, RngExt}; use serde::{Deserialize, Serialize}; use smite::bolt::{ChannelTypeVariant, ShortChannelId}; -use super::VariableType; +use super::{Malformation, VariableType}; /// An IR operation. Each instruction in a program contains one operation plus /// input variable indices. @@ -188,7 +188,11 @@ pub enum Operation { /// 0: `funding_transaction` (`FundingTransaction`) /// 1: `opener_funding_privkey` (`PrivateKey`) /// 2: `temporary_channel_id` (`ChannelId`) - SendFundingCreated, + SendFundingCreated { + /// [`Malformation`] to overwrite the derived funding outpoint or + /// signature. + malformation: Option, + }, /// Build and send a `channel_ready` message (BOLT 2, type 36). /// /// The alias TLV is optional in `channel_ready`. Since every `u64` is a @@ -508,6 +512,15 @@ fn format_hex(bytes: &[u8]) -> String { s } +/// Format a malformation as `malformation@=`. Returns +/// `malformation=none` when there is none. +fn format_malformation(malformation: Option<&Malformation>) -> String { + match malformation { + Some(m) => format!("malformation@{}={}", m.offset, format_hex(&m.bytes)), + None => "malformation=none".to_string(), + } +} + /// Print an Operation. Operations that take no variable inputs include parens /// (e.g., `LoadAmount(100000)`, `LoadChainHashFromContext()`). Operations that do take /// inputs omit parens so `Program::Display` can append them `(v0, v1, ...)`. @@ -548,7 +561,13 @@ impl fmt::Display for Operation { Self::BuildAnnouncementSignatures => write!(f, "BuildAnnouncementSignatures"), Self::SendMessage => write!(f, "SendMessage"), Self::SendOpenChannel => write!(f, "SendOpenChannel"), - Self::SendFundingCreated => write!(f, "SendFundingCreated"), + Self::SendFundingCreated { malformation } => { + write!( + f, + "SendFundingCreated{{{}}}", + format_malformation(malformation.as_ref()) + ) + } Self::SendChannelReady { include_alias } => { write!(f, "SendChannelReady{{include_alias={include_alias}}}") } @@ -598,7 +617,7 @@ impl Operation { | Self::MineBlocks(_) | Self::BroadcastTransaction => None, Self::SendOpenChannel => Some(VariableType::SentOpenChannel), - Self::SendFundingCreated => Some(VariableType::SentFundingCreated), + Self::SendFundingCreated { .. } => Some(VariableType::SentFundingCreated), Self::SendShutdown => Some(VariableType::SentShutdown), Self::RecvAcceptChannel => Some(VariableType::AcceptChannel), } @@ -702,7 +721,7 @@ impl Operation { ], Self::SendMessage => vec![VariableType::Message], Self::SendOpenChannel => vec![VariableType::OpenChannelMessage], - Self::SendFundingCreated => vec![ + Self::SendFundingCreated { .. } => vec![ VariableType::FundingTransaction, // funding_transaction VariableType::PrivateKey, // opener_funding_privkey VariableType::ChannelId, // temporary_channel_id @@ -758,7 +777,7 @@ impl Operation { | Self::BuildAnnouncementSignatures | Self::SendMessage | Self::SendOpenChannel - | Self::SendFundingCreated + | Self::SendFundingCreated { .. } | Self::SendChannelReady { .. } | Self::SendShutdown | Self::RecvFundingSigned @@ -806,7 +825,7 @@ impl Operation { Self::CreateFundingTransaction | Self::SendMessage | Self::SendOpenChannel - | Self::SendFundingCreated + | Self::SendFundingCreated { .. } | Self::SendChannelReady { .. } | Self::SendShutdown | Self::RecvAcceptChannel @@ -862,7 +881,7 @@ impl Operation { // the private mempool holds, `BroadcastTransaction` dedups against // it, and `LookupShortChannelId` reads chain state. Self::CreateFundingTransaction - | Self::SendFundingCreated + | Self::SendFundingCreated { .. } | Self::RecvAcceptChannel | Self::RecvFundingSigned | Self::RecvChannelReady @@ -901,6 +920,7 @@ impl Operation { | Self::LoadChannelType(_) | Self::ExtractAcceptChannel(_) | Self::BuildNodeAnnouncement { .. } + | Self::SendFundingCreated { .. } | Self::SendChannelReady { .. } | Self::MineBlocks(_) => true, @@ -914,7 +934,6 @@ impl Operation { | Self::BuildAnnouncementSignatures | Self::SendMessage | Self::SendOpenChannel - | Self::SendFundingCreated | Self::SendShutdown | Self::RecvAcceptChannel | Self::RecvFundingSigned diff --git a/smite-ir/src/tests.rs b/smite-ir/src/tests.rs index 2cb77285..600bb1d1 100644 --- a/smite-ir/src/tests.rs +++ b/smite-ir/src/tests.rs @@ -4,7 +4,7 @@ use bitcoin::secp256k1::SecretKey; use rand::SeedableRng; use rand::rngs::SmallRng; use rand::{Rng, RngExt}; -use smite::bolt::{MAX_MESSAGE_SIZE, ShortChannelId}; +use smite::bolt::{MAX_MESSAGE_SIZE, MessageType, ShortChannelId}; use super::*; use generators::{ @@ -666,7 +666,12 @@ fn postcard_roundtrip() { inputs: vec![], }, Instruction { - operation: Operation::SendFundingCreated, + operation: Operation::SendFundingCreated { + malformation: Some(Malformation { + offset: 34, + bytes: vec![0xff; 32], + }), + }, inputs: vec![11, 0, 3], }, ], @@ -835,7 +840,7 @@ fn displays_send_funding_created_recv_funding_signed_program() { }, // Build and send funding_created. Instruction { - operation: Operation::SendFundingCreated, + operation: Operation::SendFundingCreated { malformation: None }, inputs: vec![4, 0, 5], }, // receive funding_signed. @@ -859,7 +864,7 @@ fn displays_send_funding_created_recv_funding_signed_program() { "v3 = LoadFeeratePerKw(15000)".into(), "v4 = CreateFundingTransaction(v1, v1, v2, v3)".into(), format!("v5 = LoadChannelId(0x{b32})"), - "v6 = SendFundingCreated(v4, v0, v5)".into(), + "v6 = SendFundingCreated{malformation=none}(v4, v0, v5)".into(), "v7 = RecvFundingSigned(v6)".into(), ]; @@ -1247,7 +1252,10 @@ fn generated_funding_created_program_structure() { // Must end with SendFundingCreated, RecvFundingSigned, BroadcastTransaction. assert!( - matches!(ops[ops.len() - 3], Operation::SendFundingCreated), + matches!( + ops[ops.len() - 3], + Operation::SendFundingCreated { malformation: None } + ), "third-to-last instruction should be SendFundingCreated", ); assert!( @@ -1364,7 +1372,7 @@ fn generated_funding_flow_program_structure() { // Key operations must appear in protocol order. let recv_accept_channel = find_operation!(program, Operation::RecvAcceptChannel); - let send_funding_created = find_operation!(program, Operation::SendFundingCreated); + let send_funding_created = find_operation!(program, Operation::SendFundingCreated { .. }); let recv_funding_signed = find_operation!(program, Operation::RecvFundingSigned); let broadcast_transaction = find_operation!(program, Operation::BroadcastTransaction); let send_channel_ready = find_operation!(program, Operation::SendChannelReady { .. }); @@ -2066,6 +2074,55 @@ fn param_mutator_modifies_node_announcement_params() { assert_ne!(*alias, original_alias, "alias never mutated"); } +#[test] +fn param_mutator_malforms_allowlisted_fields_only() { + let mut program = generate_funding_created_program(0); + let send_idx = find_operation!(program, Operation::SendFundingCreated { .. }); + + // Every allowlisted field must be reachable, nothing outside the allowlist + // must be malformed, and the malformation must clear again. + let fields = MessageType::FUNDING_CREATED.malformable_fields(); + let mut unmalformed: Vec<&str> = fields.iter().map(|f| f.name).collect(); + let mut cleared = false; + let mut was_malformed = false; + + let mutator = OperationParamMutator; + let mut rng = SmallRng::seed_from_u64(0); + + for _ in 0..1_000 { + mutator.mutate(&mut program, &mut rng); + + let Operation::SendFundingCreated { malformation } = + &program.instructions[send_idx].operation + else { + panic!("operation type changed"); + }; + let Some(malformation) = malformation else { + cleared |= was_malformed; + continue; + }; + was_malformed = true; + + let field = fields + .iter() + .find(|f| f.offset == malformation.offset && f.len as usize == malformation.bytes.len()) + .unwrap_or_else(|| { + panic!( + "malformation of {} bytes at offset {} is not an allowlisted field", + malformation.bytes.len(), + malformation.offset, + ) + }); + unmalformed.retain(|name| *name != field.name); + } + + assert!( + unmalformed.is_empty(), + "these allowlisted fields were never malformed: {unmalformed:?}", + ); + assert!(cleared, "malformation was never cleared"); +} + #[test] fn param_mutator_preserves_extract_field_type() { // One ExtractAcceptChannel instruction per field variant. diff --git a/smite-scenarios/src/executor.rs b/smite-scenarios/src/executor.rs index 70dad2a4..dfdc83d3 100644 --- a/smite-scenarios/src/executor.rs +++ b/smite-scenarios/src/executor.rs @@ -445,7 +445,7 @@ impl Executor { Some(Variable::SentOpenChannel) } - Operation::SendFundingCreated => { + Operation::SendFundingCreated { .. } => { let fc = build_funding_created( &variables, &instr.inputs, diff --git a/smite-scenarios/src/executor/tests.rs b/smite-scenarios/src/executor/tests.rs index 084ab2d7..17ccdcd4 100644 --- a/smite-scenarios/src/executor/tests.rs +++ b/smite-scenarios/src/executor/tests.rs @@ -910,7 +910,7 @@ fn execute_send_funding_created_after_funding_built_does_not_track_channel() { ], ); b.append( - Operation::SendFundingCreated, + Operation::SendFundingCreated { malformation: None }, &[ second_tx, funding.opener_privkey, diff --git a/smite-scenarios/src/executor/tests/programs.rs b/smite-scenarios/src/executor/tests/programs.rs index f149c283..349296f6 100644 --- a/smite-scenarios/src/executor/tests/programs.rs +++ b/smite-scenarios/src/executor/tests/programs.rs @@ -272,7 +272,7 @@ pub fn send_funding_created_with( ) -> SentFundingCreated { let temporary_channel_id = b.append(Operation::LoadChannelId([0xbb; 32]), &[]); let sent = b.append( - Operation::SendFundingCreated, + Operation::SendFundingCreated { malformation: None }, &[tx.tx, signing_privkey, temporary_channel_id], ); diff --git a/smite/src/bolt.rs b/smite/src/bolt.rs index 18192c01..12ff0010 100644 --- a/smite/src/bolt.rs +++ b/smite/src/bolt.rs @@ -294,7 +294,10 @@ impl MessageType { /// here. #[must_use] pub fn malformable_fields(self) -> &'static [MalformableField] { - unreachable!("no malformable field table for message type {self}"); + match self { + Self::FUNDING_CREATED => FundingCreated::MALFORMABLE_FIELDS, + _ => unreachable!("no malformable field table for message type {self}"), + } } } diff --git a/smite/src/bolt/funding_created.rs b/smite/src/bolt/funding_created.rs index 1bfa0c52..de63dc62 100644 --- a/smite/src/bolt/funding_created.rs +++ b/smite/src/bolt/funding_created.rs @@ -1,8 +1,8 @@ //! BOLT 2 funding created message. -use super::BoltError; use super::types::TemporaryChannelId; use super::wire::WireFormat; +use super::{BoltError, MalformableField}; use bitcoin::Txid; use bitcoin::secp256k1::ecdsa::Signature; @@ -24,6 +24,31 @@ pub struct FundingCreated { } impl FundingCreated { + /// Fields whose encoded bytes may be overwritten before sending: the + /// outpoint is derived from the constructed funding transaction and the + /// signature is computed, so neither is reachable through an IR parameter. + /// See [`MalformableField`]. + pub const FUNDING_TXID_FIELD: MalformableField = MalformableField { + name: "funding_txid", + offset: 34, + len: 32, + }; + pub const FUNDING_OUTPUT_INDEX_FIELD: MalformableField = MalformableField { + name: "funding_output_index", + offset: 66, + len: 2, + }; + pub const SIGNATURE_FIELD: MalformableField = MalformableField { + name: "signature", + offset: 68, + len: 64, + }; + pub const MALFORMABLE_FIELDS: &'static [MalformableField] = &[ + Self::FUNDING_TXID_FIELD, + Self::FUNDING_OUTPUT_INDEX_FIELD, + Self::SIGNATURE_FIELD, + ]; + /// Encodes to wire format (without message type prefix). #[must_use] pub fn encode(&self) -> Vec { @@ -165,4 +190,31 @@ mod tests { Err(BoltError::InvalidSignature(bad_sig)) ); } + + #[test] + fn malformable_field_offsets_match_encoding() { + let msg = sample_funding_created(); + let encoded = crate::bolt::Message::FundingCreated(msg.clone()).encode(); + + let encoded_field = |name: &str| { + let f = FundingCreated::MALFORMABLE_FIELDS + .iter() + .find(|f| f.name == name) + .expect("field is allowlisted"); + &encoded[f.offset as usize..(f.offset + f.len) as usize] + }; + + assert_eq!( + encoded_field("funding_txid"), + msg.funding_txid.to_byte_array() + ); + assert_eq!( + encoded_field("funding_output_index"), + msg.funding_output_index.to_be_bytes() + ); + assert_eq!( + encoded_field("signature"), + msg.signature.serialize_compact() + ); + } } From a8fd818798370ae553108f638a06fdac9b984cf5 Mon Sep 17 00:00:00 2001 From: Nishant Bansal Date: Thu, 17 Sep 2026 15:02:43 +0530 Subject: [PATCH 4/4] smite-scenarios: apply funding_created malformations The executor now applies the malformation before building the commitment, so the channel it tracks matches what was sent. A malformed outpoint changes which channel is tracked and marks the outpoint invalid. A malformed signature is sent unchanged and marks the signature invalid, even if it no longer decodes. Signed-off-by: Nishant Bansal --- smite-scenarios/src/executor.rs | 128 +++++++++++------- smite-scenarios/src/executor/tests.rs | 113 +++++++++++++++- smite-scenarios/src/executor/tests/harness.rs | 16 ++- .../src/executor/tests/programs.rs | 19 ++- 4 files changed, 214 insertions(+), 62 deletions(-) diff --git a/smite-scenarios/src/executor.rs b/smite-scenarios/src/executor.rs index dfdc83d3..1829038b 100644 --- a/smite-scenarios/src/executor.rs +++ b/smite-scenarios/src/executor.rs @@ -26,7 +26,7 @@ use smite::violation::Violation; use super::targets::TargetRpc; use smite_ir::operation::AcceptChannelField; -use smite_ir::{Operation, Program, Variable, VariableType}; +use smite_ir::{Malformation, Operation, Program, Variable, VariableType}; use std::collections::{HashMap, HashSet}; use std::time::Duration; @@ -445,15 +445,15 @@ impl Executor { Some(Variable::SentOpenChannel) } - Operation::SendFundingCreated { .. } => { - let fc = build_funding_created( + Operation::SendFundingCreated { malformation } => { + let encoded = build_funding_created( &variables, &instr.inputs, &mut self.channel_states, &mut self.negotiations, &self.mined_txids, + malformation.as_ref(), )?; - let encoded = Message::FundingCreated(fc).encode(); log::debug!( "[{:?}] SendFundingCreated: {} bytes", start.elapsed(), @@ -772,49 +772,77 @@ fn build_open_channel(variables: &[Option], inputs: &[usize]) -> OpenC /// commitment is built from the negotiated values. `mined_txids` is used to /// determine whether the funding transaction has already been mined. /// -/// If the negotiation for `temporary_channel_id` is incomplete, emits a -/// `funding_created` with the derived outpoint and an all-zero signature. +/// `malformation` overwrites one allowlisted field before the commitment is +/// built, so `channel_states` describes the message that goes on the wire. Only +/// the signature can be malformed into bytes that no longer decode as +/// `funding_created`; those go out as-is, and the channel is still recorded +/// from the negotiated parameters with the signature flagged invalid. +/// +/// If the negotiation for `temporary_channel_id` is incomplete, emits the +/// unsigned message with any malformations applied and leaves `channel_states` +/// untouched. +#[allow(clippy::too_many_lines)] fn build_funding_created( variables: &[Option], inputs: &[usize], channel_states: &mut HashMap, negotiations: &mut HashMap, mined_txids: &HashSet, -) -> Result { + malformation: Option<&Malformation>, +) -> Result, ExecuteError> { let funding_tx = resolve_funding_transaction(variables, inputs[0]); let opener_funding_privkey_bytes = resolve_private_key(variables, inputs[1]); let temporary_channel_id = resolve_channel_id(variables, inputs[2]); - let funding_outpoint = OutPoint { + let resolved_outpoint = OutPoint { txid: funding_tx.tx.compute_txid(), vout: funding_tx.vout, }; - let funding_output_index = u16::try_from(funding_outpoint.vout) - .expect("funding output index of a funding tx must fit in u16"); + + // Malform the message before building the commitment, so the commitment we + // sign and the `channel_id` we track are both derived from the outpoint the + // target sees. + let mut fc = FundingCreated { + temporary_channel_id, + funding_txid: resolved_outpoint.txid, + funding_output_index: u16::try_from(resolved_outpoint.vout) + .expect("funding output index of a funding tx must fit in u16"), + signature: Signature::from_compact(&[0u8; 64]).expect("zero bytes parse as a signature"), + }; + let mut encoded = Message::FundingCreated(fc.clone()).encode(); + if malformation.is_some_and(|m| m.apply(&mut encoded)) + && let Ok(Message::FundingCreated(malformed)) = Message::decode(&encoded) + { + fc = malformed; + } // Without both the recorded `open_channel` and the peer's `accept_channel` // we cannot build the commitment to sign, so fall back to an unsigned // `funding_created` and leave `channel_states` untouched. let Some(pending) = negotiations.get(&temporary_channel_id) else { - return Ok(FundingCreated { - temporary_channel_id, - funding_txid: funding_outpoint.txid, - funding_output_index, - signature: Signature::from_compact(&[0u8; 64]) - .expect("zero bytes parse as a signature"), - }); + return Ok(encoded); }; let open_channel = &pending.open_channel; let Some(accept_channel) = pending.accept_channel.as_ref() else { - return Ok(FundingCreated { - temporary_channel_id, - funding_txid: funding_outpoint.txid, - funding_output_index, - signature: Signature::from_compact(&[0u8; 64]) - .expect("zero bytes parse as a signature"), - }); + return Ok(encoded); }; + // Resolve what the target will derive from the message on the wire. + // + // Malforming `funding_txid` or `funding_output_index` repoints the + // outpoint, so it no longer belongs to the resolved funding transaction. + // + // A signature malformation is instead identified by its targeted field, + // since overwriting the all-zero placeholder with zeros leaves the bytes + // unchanged. + let advertised_outpoint = OutPoint { + txid: fc.funding_txid, + vout: u32::from(fc.funding_output_index), + }; + let is_outpoint_malformed = advertised_outpoint != resolved_outpoint; + let is_signature_malformed = + malformation.is_some_and(|m| m.offset == FundingCreated::SIGNATURE_FIELD.offset); + let opener_funding_privkey = SecretKey::from_slice(&opener_funding_privkey_bytes).expect("valid private key"); @@ -835,7 +863,7 @@ fn build_funding_created( to_self_delay: accept_channel.to_self_delay, }; let config = ChannelConfig { - funding_outpoint, + funding_outpoint: advertised_outpoint, funding_satoshis: open_channel.funding_satoshis, channel_type: Features::from(open_channel.tlvs.channel_type.clone().unwrap_or_default()), opener, @@ -853,7 +881,14 @@ fn build_funding_created( side: Side::Opener, funding_privkey: opener_funding_privkey, }; - let signature = config.sign_counterparty_commitment(&state, &holder); + + // Sign unless the malformation targeted the signature itself, in which case + // the malformed bytes are what goes on the wire. Re-encoding replaces the + // all-zero placeholder while preserving any outpoint malformation. + if !is_signature_malformed { + fc.signature = config.sign_counterparty_commitment(&state, &holder); + encoded = Message::FundingCreated(fc).encode(); + } // Only track a new channel when this negotiation has not built a // `funding_created` yet. If it has, we are likely resending one for the @@ -866,21 +901,27 @@ fn build_funding_created( if !pending.funding_built { let channel_id = ChannelId::v1_from_funding_outpoint(config.funding_outpoint); - // Check whether the funding outpoint is valid and contains the - // negotiated amount and funding script. If not, there is a good chance - // the target will neither complete the funding flow nor send an error - // message. - let is_funding_outpoint_valid = funding_tx.matches_funding_output( - &open_channel.funding_pubkey, - &accept_channel.funding_pubkey, - open_channel.funding_satoshis, - ); - // TODO: Once we support sending malformed signatures, update this state - // when constructing one so that the peer's acceptance can be detected - // as a violation. + // Record how faithful the sent message is, so the tracked channel + // reflects what the target received. + // + // The outpoint is valid only if it survived malformation and the + // resolved transaction pays the negotiated amount to the negotiated + // funding script; otherwise the target may neither complete the funding + // flow nor send an error. + // + // The signature is invalid if it was malformed or was computed with a + // funding key the peer was never told about. + let is_funding_outpoint_valid = !is_outpoint_malformed + && funding_tx.matches_funding_output( + &open_channel.funding_pubkey, + &accept_channel.funding_pubkey, + open_channel.funding_satoshis, + ); + let was_funding_mined_prematurely = mined_txids.contains(&config.funding_outpoint.txid); let opener_funding_pubkey = PublicKey::from_secret_key(&Secp256k1::new(), &opener_funding_privkey); - let sent_invalid_signature = opener_funding_pubkey != open_channel.funding_pubkey; + let sent_invalid_signature = + is_signature_malformed || opener_funding_pubkey != open_channel.funding_pubkey; channel_states.entry(channel_id).or_insert_with(|| { ChannelState::new( @@ -888,7 +929,7 @@ fn build_funding_created( holder, state, is_funding_outpoint_valid, - mined_txids.contains(&funding_outpoint.txid), + was_funding_mined_prematurely, sent_invalid_signature, ) }); @@ -902,12 +943,7 @@ fn build_funding_created( pending.funding_built = true; } - Ok(FundingCreated { - temporary_channel_id, - funding_txid: funding_outpoint.txid, - funding_output_index, - signature, - }) + Ok(encoded) } /// Builds a `ChannelReady` from 3 input variables (wire order). diff --git a/smite-scenarios/src/executor/tests.rs b/smite-scenarios/src/executor/tests.rs index 17ccdcd4..97f6c004 100644 --- a/smite-scenarios/src/executor/tests.rs +++ b/smite-scenarios/src/executor/tests.rs @@ -5,10 +5,11 @@ use std::str::FromStr; use super::*; use bitcoin::Amount; +use bitcoin::hashes::Hash; use bitcoin::secp256k1::{Secp256k1, SecretKey}; use harness::*; use programs::*; -use smite::bolt::{AcceptChannelTlvs, GossipTimestampFilter, Init, Ping}; +use smite::bolt::{AcceptChannelTlvs, BoltError, GossipTimestampFilter, Init, Ping}; use smite_ir::Instruction; use smite_ir::builder::ProgramBuilder; use smite_ir::operation::ShutdownScriptVariant; @@ -861,7 +862,7 @@ fn execute_send_funding_created_uses_wire_funding_pubkey() { let mut b = ProgramBuilder::new(); let funding = create_funding_tx(&mut b); b.append(Operation::BroadcastTransaction, &[funding.tx]); - send_funding_created_with(&mut b, funding, funding.acceptor_privkey); + send_funding_created_with(&mut b, funding, funding.acceptor_privkey, None); // Signing with a key the peer did not negotiate marks the channel as // having sent an invalid signature, so receiving a `funding_signed` would @@ -996,6 +997,105 @@ fn execute_send_funding_created_no_accept_channel() { assert!(fx.channel_states().is_empty()); } +#[test] +fn execute_send_funding_created_malformed_funding_txid() { + let malformed_txid = Txid::from_byte_array([0x00; 32]); + let malformation = Malformation { + offset: FundingCreated::FUNDING_TXID_FIELD.offset, + bytes: malformed_txid.to_byte_array().to_vec(), + }; + + let mut fx = Fixture::new().with_negotiation(sample_funding_negotiation()); + fx.run(&malformed_send_funding_created_program(malformation)); + + let fc: FundingCreated = fx.sent(0); + assert_eq!(fc.funding_txid, malformed_txid); + assert_eq!(fc.funding_output_index, 0); + + // The channel is tracked under the advertised outpoint, not the resolved one. + let advertised_outpoint = OutPoint { + txid: malformed_txid, + vout: 0, + }; + assert_eq!(fx.channel_states().len(), 1); + assert!(!fx.channel_states().contains_key(&funding_channel_id())); + + let state = fx.channel_state(&ChannelId::v1_from_funding_outpoint(advertised_outpoint)); + assert_eq!(state.config.funding_outpoint, advertised_outpoint); + + // The outpoint no longer belongs to the resolved funding transaction, but + // the signature is genuine and covers the advertised outpoint. + assert!(!state.is_funding_outpoint_valid); + assert!(!state.sent_invalid_signature); + let holder = HolderIdentity { + side: Side::Acceptor, + funding_privkey: acceptor_funding_sk(), + }; + assert!( + state + .config + .verify_counterparty_signature(&state.commitment, &holder, &fc.signature) + ); +} + +#[test] +fn execute_send_funding_created_malformed_zero_signature() { + // Overwriting the signature with zeros leaves the all-zero placeholder + // unchanged, so nothing is signed and the zero signature goes out. The + // channel is still tracked, with the signature flagged invalid. + let malformation = Malformation { + offset: FundingCreated::SIGNATURE_FIELD.offset, + bytes: vec![0x00; 64], + }; + + let mut fx = Fixture::new().with_negotiation(sample_funding_negotiation()); + fx.run(&malformed_send_funding_created_program(malformation)); + + let fc: FundingCreated = fx.sent(0); + assert_eq!(fc.signature, Signature::from_compact(&[0u8; 64]).unwrap()); + + // The outpoint is untouched, so the channel is tracked under it. + assert_eq!(fc.funding_txid, funding_outpoint().txid); + assert_eq!(fc.funding_output_index, 0); + assert_eq!(fx.channel_states().len(), 1); + + let state = fx.channel_state(&funding_channel_id()); + assert_eq!(state.config.funding_outpoint, funding_outpoint()); + assert!(state.is_funding_outpoint_valid); + assert!(state.sent_invalid_signature); +} + +#[test] +fn execute_send_funding_created_malformed_undecodable_signature() { + // A signature of all-ones has r and s past the curve order, so the message + // no longer decodes. The bytes still go out as-is and the channel is + // tracked from the negotiated parameters. + let signature_offset = FundingCreated::SIGNATURE_FIELD.offset; + let malformation = Malformation { + offset: signature_offset, + bytes: vec![0xff; 64], + }; + + let mut fx = Fixture::new().with_negotiation(sample_funding_negotiation()); + fx.run(&malformed_send_funding_created_program(malformation)); + + let sent = fx.sent_bytes(0); + assert_eq!( + Message::decode(sent), + Err(BoltError::InvalidSignature([0xff; 64])) + ); + assert_eq!(&sent[signature_offset as usize..], [0xff; 64]); + + // Everything before the signature is unchanged, so the channel is tracked + // under the resolved outpoint with only the signature flagged. + assert_eq!(fx.channel_states().len(), 1); + + let state = fx.channel_state(&funding_channel_id()); + assert_eq!(state.config.funding_outpoint, funding_outpoint()); + assert!(state.is_funding_outpoint_valid); + assert!(state.sent_invalid_signature); +} + #[test] fn execute_recv_funding_signed_unknown_channel() { let channel_id = ChannelId::new([0xbb; 32]); @@ -1038,7 +1138,8 @@ fn execute_recv_funding_signed_after_invalid_funding_created() { // in `open_channel`. let mut b = ProgramBuilder::new(); let funding = create_funding_tx(&mut b); - let funding_created = send_funding_created_with(&mut b, funding, funding.acceptor_privkey); + let funding_created = + send_funding_created_with(&mut b, funding, funding.acceptor_privkey, None); b.append(Operation::RecvFundingSigned, &[funding_created.sent]); let err = recv_funding_signed_fixture().run_err(&b.build()); @@ -1058,7 +1159,7 @@ fn execute_recv_funding_signed_duplicate() { let mut b = ProgramBuilder::new(); let first = send_funding_created(&mut b); b.append(Operation::RecvFundingSigned, &[first.sent]); - let second = send_funding_created_with(&mut b, first.tx, first.tx.opener_privkey); + let second = send_funding_created_with(&mut b, first.tx, first.tx.opener_privkey, None); b.append(Operation::RecvFundingSigned, &[second.sent]); let err = recv_funding_signed_fixture() @@ -1272,7 +1373,7 @@ fn execute_recv_channel_ready_funding_mined_prematurely_is_noop() { // Mine past the negotiated `minimum_depth` *before* sending // `funding_created`. b.append(Operation::MineBlocks(8), &[]); - let funding_created = send_funding_created_with(&mut b, funding, funding.opener_privkey); + let funding_created = send_funding_created_with(&mut b, funding, funding.opener_privkey, None); b.append(Operation::RecvFundingSigned, &[funding_created.sent]); b.append(Operation::RecvChannelReady, &[]); @@ -1305,7 +1406,7 @@ fn execute_recv_channel_ready_invalid_signature_is_noop() { // A `funding_signed` answering a `funding_created` we signed with the wrong // key is itself a violation, which `FundingSignedOracle` reports. So we // don't receive one, letting `RecvChannelReady` be reached. - send_funding_created_with(&mut b, funding, funding.acceptor_privkey); + send_funding_created_with(&mut b, funding, funding.acceptor_privkey, None); b.append(Operation::MineBlocks(8), &[]); b.append(Operation::RecvChannelReady, &[]); diff --git a/smite-scenarios/src/executor/tests/harness.rs b/smite-scenarios/src/executor/tests/harness.rs index f6e3ea44..668f30d5 100644 --- a/smite-scenarios/src/executor/tests/harness.rs +++ b/smite-scenarios/src/executor/tests/harness.rs @@ -225,17 +225,21 @@ impl Fixture { self.executor.conn.sent.len() } - /// Decodes the `n`th message the executor sent, panicking if it is not an - /// `M`. - pub fn sent(&self, n: usize) -> M { - let bytes = self.executor.conn.sent.get(n).unwrap_or_else(|| { + /// Returns the raw bytes of the `n`th message the executor sent. + pub fn sent_bytes(&self, n: usize) -> &[u8] { + self.executor.conn.sent.get(n).unwrap_or_else(|| { panic!( "expected at least {} sent messages, got {}", n + 1, self.sent_len() ) - }); - let msg = Message::decode(bytes).expect("valid message"); + }) + } + + /// Decodes the `n`th message the executor sent, panicking if it is not an + /// `M`. + pub fn sent(&self, n: usize) -> M { + let msg = Message::decode(self.sent_bytes(n)).expect("valid message"); let got = msg.to_string(); M::from_message(msg).unwrap_or_else(|| panic!("expected {}, got {got}", M::TYPE)) } diff --git a/smite-scenarios/src/executor/tests/programs.rs b/smite-scenarios/src/executor/tests/programs.rs index 349296f6..0e88d260 100644 --- a/smite-scenarios/src/executor/tests/programs.rs +++ b/smite-scenarios/src/executor/tests/programs.rs @@ -251,16 +251,16 @@ pub struct SentFundingCreated { } /// Creates a funding transaction, broadcasts it, and sends `funding_created` -/// signed with the opener's funding key. +/// signed with the opener's funding key, with no malformation. pub fn send_funding_created(b: &mut ProgramBuilder) -> SentFundingCreated { let tx = create_funding_tx(b); b.append(Operation::BroadcastTransaction, &[tx.tx]); - send_funding_created_with(b, tx, tx.opener_privkey) + send_funding_created_with(b, tx, tx.opener_privkey, None) } /// Sends `funding_created` for `tx`, signed with the `PrivateKey` variable -/// `signing_privkey`. +/// `signing_privkey`, applying `malformation` to its computed fields. /// /// The `temporary_channel_id` is the one `announced_open_channel` and /// `sample_funding_negotiation` use, so that the executor finds the negotiation @@ -269,10 +269,11 @@ pub fn send_funding_created_with( b: &mut ProgramBuilder, tx: FundingTxVars, signing_privkey: usize, + malformation: Option, ) -> SentFundingCreated { let temporary_channel_id = b.append(Operation::LoadChannelId([0xbb; 32]), &[]); let sent = b.append( - Operation::SendFundingCreated { malformation: None }, + Operation::SendFundingCreated { malformation }, &[tx.tx, signing_privkey, temporary_channel_id], ); @@ -291,6 +292,16 @@ pub fn send_funding_created_program() -> Program { b.build() } +/// A program that sends `funding_created` with `malformation` applied, without +/// awaiting `funding_signed`. +pub fn malformed_send_funding_created_program(malformation: Malformation) -> Program { + let mut b = ProgramBuilder::new(); + let tx = create_funding_tx(&mut b); + send_funding_created_with(&mut b, tx, tx.opener_privkey, Some(malformation)); + + b.build() +} + /// A program that sends `funding_created` and receives the peer's /// `funding_signed`. pub fn send_funding_created_and_recv_funding_signed_program() -> Program {