From 15d0c2c35955d9fb72de61b9e22bddcb1a3ea6b0 Mon Sep 17 00:00:00 2001 From: Nishant Bansal Date: Wed, 7 Oct 2026 15:31:45 +0530 Subject: [PATCH 1/2] smite-scenarios: track pubkeys revealed on the wire This prepares for an oracle that detects pubkey reuse by the target. Signed-off-by: Nishant Bansal --- smite-scenarios/src/executor.rs | 105 +++++++++++++++++- smite-scenarios/src/executor/tests.rs | 96 +++++++++++++++- smite-scenarios/src/executor/tests/harness.rs | 2 +- smite/src/bolt/accept_channel.rs | 13 +++ smite/src/bolt/open_channel.rs | 13 +++ smite/src/pending_channel.rs | 54 ++++++++- 6 files changed, 273 insertions(+), 10 deletions(-) diff --git a/smite-scenarios/src/executor.rs b/smite-scenarios/src/executor.rs index bf6f6b90..2cd2fcad 100644 --- a/smite-scenarios/src/executor.rs +++ b/smite-scenarios/src/executor.rs @@ -21,7 +21,7 @@ use smite::noise::{ConnectionError, NoiseConnection}; use smite::oracles::{ AcceptChannelContext, AcceptChannelOracle, FundingSignedContext, FundingSignedOracle, Oracle, }; -use smite::pending_channel::PendingChannel; +use smite::pending_channel::{KeyOrigin, PendingChannel}; use smite::violation::Violation; use super::targets::TargetRpc; @@ -253,6 +253,10 @@ pub struct Executor { /// `temporary_channel_id`, so the funding flow can build commitments from /// the parameters actually sent on the wire. negotiations: HashMap, + /// Every pubkey revealed on the wire by either side, mapped to its origin. + /// We may send the same pubkey more than once, keeping its first origin. + /// The target must not send a pubkey whose private key is or will be known. + revealed_pubkeys: HashMap, /// Transactions stored outside Bitcoin Core's mempool, typically because they /// were rejected by mempool policy, to be included in the next `MineBlocks` /// operation. Each is stored as `(txid, raw_hex)`: re-signing the same @@ -272,6 +276,14 @@ impl Executor { /// program context, and target RPC handle. Channel state and negotiations /// start empty. pub fn new(conn: C, bitcoin_cli: B, rpc: R, context: ProgramContext) -> Self { + let revealed_pubkeys = HashMap::from([( + context.target_pubkey, + KeyOrigin::TargetStatic { + channel: TemporaryChannelId::ALL, + field: "node_pubkey", + }, + )]); + Self { conn, bitcoin_cli, @@ -279,6 +291,7 @@ impl Executor { context, channel_states: HashMap::new(), negotiations: HashMap::new(), + revealed_pubkeys, private_mempool: Vec::new(), unmined_txids: HashSet::new(), mined_txids: HashSet::new(), @@ -436,7 +449,12 @@ impl Executor { Operation::SendOpenChannel => { let oc = resolve_open_channel_message(&variables, instr.inputs[0]); - record_send_open_channel(&mut self.negotiations, oc); + record_send_open_channel( + &mut self.negotiations, + &mut self.revealed_pubkeys, + oc, + variables.len(), + ); let encoded = Message::OpenChannel(oc.clone()).encode(); log::debug!( "[{:?}] SendOpenChannel: {} bytes", @@ -471,6 +489,8 @@ impl Executor { &instr.inputs, *include_alias, &mut self.channel_states, + &mut self.revealed_pubkeys, + variables.len(), ); let encoded = Message::ChannelReady(cr).encode(); log::debug!( @@ -508,7 +528,11 @@ impl Executor { negotiation: self.negotiations.get(&ac.temporary_channel_id), negotiated_features: &self.context.negotiated_features, })?; - record_recv_accept_channel(&mut self.negotiations, &ac); + record_recv_accept_channel( + &mut self.negotiations, + &mut self.revealed_pubkeys, + &ac, + ); Some(Variable::AcceptChannel(ac)) } @@ -532,7 +556,11 @@ impl Executor { Operation::RecvChannelReady => { if is_channel_ready_expected(&self.channel_states, &mut self.bitcoin_cli) { log::debug!("[{:?}] RecvChannelReady: waiting", start.elapsed()); - recv_channel_ready(&mut self.conn, &mut self.channel_states)?; + recv_channel_ready( + &mut self.conn, + &mut self.channel_states, + &mut self.revealed_pubkeys, + )?; log::debug!("[{:?}] RecvChannelReady: received", start.elapsed()); } None @@ -914,11 +942,16 @@ fn build_funding_created( } /// Builds a `ChannelReady` from 3 input variables (wire order). +/// +/// `instruction` is the index of the `SendChannelReady` instruction, recorded +/// as the origin of the `second_per_commitment_point` we reveal. fn build_channel_ready( variables: &[Option], inputs: &[usize], include_alias: bool, channel_states: &mut HashMap, + revealed_pubkeys: &mut HashMap, + instruction: usize, ) -> ChannelReady { let channel_id = resolve_channel_id(variables, inputs[0]); let second_per_commitment_point = resolve_pubkey(variables, inputs[1]); @@ -939,6 +972,13 @@ fn build_channel_ready( } } + // Record the sent `second_per_commitment_point` as ours, whether or not the + // channel state above is updated. We may send a pubkey many times, but only + // its first origin is recorded. + revealed_pubkeys + .entry(second_per_commitment_point) + .or_insert(KeyOrigin::Ours { instruction }); + ChannelReady { channel_id, second_per_commitment_point, @@ -1203,7 +1243,8 @@ fn recv_bolt( /// Receives and decodes a `channel_ready` message. /// /// The `second_per_commitment_point` is recorded as the counterparty's next -/// per-commitment point on the channel it identifies. +/// per-commitment point on the channel it identifies, and in `revealed_pubkeys` +/// as a per-commitment point the target revealed. /// /// # Errors /// @@ -1213,6 +1254,7 @@ fn recv_bolt( fn recv_channel_ready( conn: &mut impl Connection, channel_states: &mut HashMap, + revealed_pubkeys: &mut HashMap, ) -> Result<(), ExecuteError> { let cr: ChannelReady = recv_bolt(conn, RECV_CHANNEL_READY_TIMEOUT)?; @@ -1221,6 +1263,25 @@ fn recv_channel_ready( .ok_or(Violation::UnknownChannel(cr.channel_id))?; *state.next_counterparty_per_commitment_point_mut() = Some(cr.second_per_commitment_point); + // Record the `second_per_commitment_point` as a per-commitment point the + // target revealed, for commitment number 1. It is unique, so record its + // only origin. + // + // TODO: Once we have the channel_ready oracle, flag a reused + // `second_per_commitment_point`, i.e. one already present in + // `revealed_pubkeys`, unless the entry is a `TargetPcp` for this same + // channel and commitment number. That exception allows the two legitimate + // repeats: the target may resend `channel_ready` with a different alias but + // the same point, and we may receive the same point again after a + // reconnection. + revealed_pubkeys.insert( + cr.second_per_commitment_point, + KeyOrigin::TargetPcp { + channel: cr.channel_id, + commitment_number: 1, + }, + ); + Ok(()) } @@ -1254,10 +1315,25 @@ fn is_channel_ready_expected( /// it is left untouched, preserving the first `open_channel`. Once a /// `funding_created` has been built, it is overwritten, allowing the /// `temporary_channel_id` to be reused for a new negotiation. +/// +/// Either way, all six pubkeys are recorded in `revealed_pubkeys` as ours, +/// keeping only the first origin of each. `instruction` is the index of the +/// `SendOpenChannel` instruction. fn record_send_open_channel( negotiations: &mut HashMap, + revealed_pubkeys: &mut HashMap, open_channel: &OpenChannel, + instruction: usize, ) { + // Record the sent pubkeys as ours, whether or not the negotiation below is + // updated. We may send a pubkey many times but only its first origin is + // recorded. + for pubkey in open_channel.pubkeys() { + revealed_pubkeys + .entry(pubkey) + .or_insert(KeyOrigin::Ours { instruction }); + } + if negotiations .get(&open_channel.temporary_channel_id) .is_some_and(|pending| !pending.funding_built) @@ -1276,7 +1352,7 @@ fn record_send_open_channel( } /// Pairs a received `accept_channel` with the recorded `open_channel` of the -/// same `temporary_channel_id`. +/// same `temporary_channel_id`, and records its pubkeys in `revealed_pubkeys`. /// /// # Panics /// @@ -1284,12 +1360,29 @@ fn record_send_open_channel( /// `AcceptChannelOracle` reports such messages as a [`Violation`]. fn record_recv_accept_channel( negotiations: &mut HashMap, + revealed_pubkeys: &mut HashMap, accept_channel: &AcceptChannel, ) { negotiations .get_mut(&accept_channel.temporary_channel_id) .expect("AcceptChannelOracle guaranteed this temporary_channel_id exists") .accept_channel = Some(accept_channel.clone()); + + let channel = accept_channel.temporary_channel_id; + // Static pubkeys may recur across channels, so keep the first origin. + for (field, pubkey) in accept_channel.static_pubkeys() { + revealed_pubkeys + .entry(pubkey) + .or_insert(KeyOrigin::TargetStatic { channel, field }); + } + // A per-commitment point is unique, so record its only origin. + revealed_pubkeys.insert( + accept_channel.first_per_commitment_point, + KeyOrigin::TargetPcp { + channel, + commitment_number: 0, + }, + ); } /// Records that a `funding_signed` has been accepted for its channel. diff --git a/smite-scenarios/src/executor/tests.rs b/smite-scenarios/src/executor/tests.rs index 3f030aa4..cd18bd20 100644 --- a/smite-scenarios/src/executor/tests.rs +++ b/smite-scenarios/src/executor/tests.rs @@ -8,7 +8,7 @@ use bitcoin::Amount; use bitcoin::secp256k1::{Secp256k1, SecretKey}; use harness::*; use programs::*; -use smite::bolt::{AcceptChannelTlvs, GossipTimestampFilter, Init, Ping}; +use smite::bolt::{AcceptChannelTlvs, ChannelReadyTlvs, GossipTimestampFilter, Init, Ping}; use smite_ir::Instruction; use smite_ir::builder::ProgramBuilder; use smite_ir::operation::ShutdownScriptVariant; @@ -498,6 +498,35 @@ fn execute_recv_accept_channel_rejects_reuse_before_funding() { )); } +#[test] +fn execute_recv_accept_channel_allows_reused_static_pubkeys() { + let first_id = TemporaryChannelId::new([0xbb; 32]); + let second_id = TemporaryChannelId::new([0xcc; 32]); + let accept_channel = sample_accept_channel(); + + // Run two negotiations with different temporary channel IDs. The second + // `accept_channel` reuses the first's static pubkeys, but uses a fresh + // `first_per_commitment_point` + let mut b = ProgramBuilder::new(); + negotiate_channel(&mut b, &announced_open_channel()); + let mut second_open = announced_open_channel(); + second_open.message.temporary_channel_id = second_id; + negotiate_channel(&mut b, &second_open); + + let mut fx = Fixture::new() + .queue(&Message::AcceptChannel(accept_channel.clone())) + .queue(&Message::AcceptChannel(AcceptChannel { + temporary_channel_id: second_id, + first_per_commitment_point: sample_pubkey(8), + ..accept_channel + })); + fx.run(&b.build()); + + assert_eq!(fx.queued_len(), 0); + assert!(fx.negotiation(&first_id).accept_channel.is_some()); + assert!(fx.negotiation(&second_id).accept_channel.is_some()); +} + #[test] fn execute_records_only_first_open_channel_for_duplicate_id_before_funding() { let temporary_channel_id = TemporaryChannelId::new([0xbb; 32]); @@ -1266,6 +1295,71 @@ fn execute_recv_channel_ready_invalid_signature_is_noop() { assert_eq!(fx.queued_len(), 2); } +#[test] +fn execute_recv_channel_ready_resend_with_new_alias() { + // We will carry forward two funding flows with different temporary channel + // IDs and funding amounts and hence different funded channel IDs. + let first_negotiation = sample_funding_negotiation(); + let target_pcp = sample_pubkey(9); + let second_id = TemporaryChannelId::new([0xcc; 32]); + // A second UTXO so the program can build a second funding transaction. + let mut second_utxo = sample_utxo(); + second_utxo.amount = Amount::from_sat(20_010_000); + second_utxo.outpoint.vout = 1; + // A second negotiation with a different temporary channel ID and funding + // amount, resulting in a different channel ID and allowing the second UTXO + // to fund it. + let mut second_negotiation = sample_funding_negotiation(); + second_negotiation.open_channel.temporary_channel_id = second_id; + second_negotiation.open_channel.funding_satoshis = 20_000_000; + second_negotiation + .accept_channel + .as_mut() + .unwrap() + .temporary_channel_id = second_id; + + // Create the funding transactions and send funding_created for both + // negotiations. Do not wait for funding_signed, since that is not the goal + // of this test. Once the funding transactions have enough confirmations, + // receive channel_ready for both negotiations. The target, instead of + // sending channel_ready for the second funding flow, sends a duplicate + // channel_ready for the first funding flow with a different alias but + // otherwise identical contents. + let mut b = ProgramBuilder::new(); + send_funding_created(&mut b); + let tx = create_funding_tx_with(&mut b, 20_000_000, 15_000); + let second_temp_chan_id = b.append(Operation::LoadChannelId(second_id.0), &[]); + b.append( + Operation::SendFundingCreated, + &[tx.tx, tx.opener_privkey, second_temp_chan_id], + ); + b.append(Operation::MineBlocks(6), &[]); + b.append(Operation::RecvChannelReady, &[]); + b.append(Operation::RecvChannelReady, &[]); + + let mut fx = Fixture::new() + .with_utxos(vec![sample_utxo(), second_utxo]) + .with_negotiation(first_negotiation) + .with_negotiation(second_negotiation) + .queue(&channel_ready_reply(target_pcp)) + .queue(&Message::ChannelReady(ChannelReady { + channel_id: funding_channel_id(), + second_per_commitment_point: target_pcp, + tlvs: ChannelReadyTlvs { + short_channel_id: Some(ShortChannelId::from_u64(0)), + }, + })); + fx.run(&b.build()); + + let state = fx.channel_state(&funding_channel_id()); + assert_eq!( + *state.next_counterparty_per_commitment_point(), + Some(target_pcp) + ); + assert_eq!(fx.queued_len(), 0); + assert_eq!(fx.channel_states().len(), 2); +} + // -- extract_field tests -- // TODO: Once we can actually construct and send accept_channel messages, it diff --git a/smite-scenarios/src/executor/tests/harness.rs b/smite-scenarios/src/executor/tests/harness.rs index 668e6f1a..6ef8fe6e 100644 --- a/smite-scenarios/src/executor/tests/harness.rs +++ b/smite-scenarios/src/executor/tests/harness.rs @@ -453,7 +453,7 @@ pub fn recv_funding_signed_fixture() -> Fixture { /// A [`recv_funding_signed_fixture`] with the target's `channel_ready` queued /// too, plus the per-commitment point it carries for the assertions. pub fn recv_channel_ready_fixture() -> (Fixture, PublicKey) { - let target_pcp = sample_pubkey(1); + let target_pcp = sample_pubkey(7); let fx = recv_funding_signed_fixture().queue(&channel_ready_reply(target_pcp)); (fx, target_pcp) diff --git a/smite/src/bolt/accept_channel.rs b/smite/src/bolt/accept_channel.rs index 9fa072a4..cd927b3a 100644 --- a/smite/src/bolt/accept_channel.rs +++ b/smite/src/bolt/accept_channel.rs @@ -60,6 +60,19 @@ pub struct AcceptChannelTlvs { } impl AcceptChannel { + /// Returns the channel acceptor's's static pubkeys (`funding_pubkey` and + /// basepoints) paired with their field names in wire order. + #[must_use] + pub fn static_pubkeys(&self) -> [(&'static str, PublicKey); 5] { + [ + ("funding_pubkey", self.funding_pubkey), + ("revocation_basepoint", self.revocation_basepoint), + ("payment_basepoint", self.payment_basepoint), + ("delayed_payment_basepoint", self.delayed_payment_basepoint), + ("htlc_basepoint", self.htlc_basepoint), + ] + } + /// Encodes to wire format (without message type prefix). #[must_use] pub fn encode(&self) -> Vec { diff --git a/smite/src/bolt/open_channel.rs b/smite/src/bolt/open_channel.rs index 93820ff6..5c6c6cad 100644 --- a/smite/src/bolt/open_channel.rs +++ b/smite/src/bolt/open_channel.rs @@ -67,6 +67,19 @@ pub struct OpenChannelTlvs { } impl OpenChannel { + /// Returns the channel initiator's pubkeys in wire order. + #[must_use] + pub fn pubkeys(&self) -> [PublicKey; 6] { + [ + self.funding_pubkey, + self.revocation_basepoint, + self.payment_basepoint, + self.delayed_payment_basepoint, + self.htlc_basepoint, + self.first_per_commitment_point, + ] + } + /// Encodes to wire format (without message type prefix). #[must_use] pub fn encode(&self) -> Vec { diff --git a/smite/src/pending_channel.rs b/smite/src/pending_channel.rs index bacb467e..2d0fcf65 100644 --- a/smite/src/pending_channel.rs +++ b/smite/src/pending_channel.rs @@ -1,9 +1,12 @@ //! BOLT 2 channel negotiation state. //! //! Remembers the `open_channel`/`accept_channel` parameters of each channel -//! being established, so later steps can build commitments from them. +//! being established, so later steps can build commitments from them. It also +//! tracks the origin of each pubkey sent on the wire, so pubkey reuse can be +//! reported. -use crate::bolt::{AcceptChannel, OpenChannel}; +use crate::bolt::{AcceptChannel, ChannelId, OpenChannel, TemporaryChannelId}; +use std::fmt; /// Negotiation parameters for a channel being established. /// @@ -15,3 +18,50 @@ pub struct PendingChannel { pub accept_channel: Option, pub funding_built: bool, } + +/// The origin of a pubkey sent on the wire. +/// +/// A static pubkey may be reused across channels, but a per-commitment point's +/// secret is eventually revealed, so it must be unique. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum KeyOrigin { + /// A pubkey we sent, identified by the first instruction that put it on the + /// wire. We know its private key. + Ours { instruction: usize }, + /// A static pubkey (funding key or basepoint) sent by the target. It may + /// recur across channels, so this records the first channel it was sent on. + TargetStatic { + /// The channel's `temporary_channel_id`. + channel: TemporaryChannelId, + /// The message field containing the pubkey. + field: &'static str, + }, + /// A per-commitment point sent by the target. + TargetPcp { + /// The channel's `temporary_channel_id` before funding and `channel_id` + /// after funding. + channel: ChannelId, + /// The commitment number for which the point is used. + commitment_number: u64, + }, +} + +impl fmt::Display for KeyOrigin { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + KeyOrigin::Ours { instruction } => { + write!(f, "our key from instruction {instruction}") + } + KeyOrigin::TargetStatic { channel, field } => { + write!(f, "target's {field} on channel {channel}") + } + KeyOrigin::TargetPcp { + channel, + commitment_number, + } => write!( + f, + "target's per-commitment point on channel {channel} at commitment number {commitment_number}" + ), + } + } +} From 57f21954c3bb8ec2dd8103754b464cc8c81b8ef5 Mon Sep 17 00:00:00 2001 From: Nishant Bansal Date: Wed, 7 Oct 2026 15:39:41 +0530 Subject: [PATCH 2/2] smite: add pubkey reuse check to accept_channel oracle Signed-off-by: Nishant Bansal --- smite-scenarios/src/executor.rs | 1 + smite-scenarios/src/executor/tests.rs | 157 +++++++++++++++++ smite/src/oracles/accept_channel.rs | 244 ++++++++++++++++++++++++-- smite/src/violation.rs | 5 +- 4 files changed, 393 insertions(+), 14 deletions(-) diff --git a/smite-scenarios/src/executor.rs b/smite-scenarios/src/executor.rs index 2cd2fcad..c1d68e42 100644 --- a/smite-scenarios/src/executor.rs +++ b/smite-scenarios/src/executor.rs @@ -527,6 +527,7 @@ impl Executor { accept_channel: &ac, negotiation: self.negotiations.get(&ac.temporary_channel_id), negotiated_features: &self.context.negotiated_features, + revealed_pubkeys: &self.revealed_pubkeys, })?; record_recv_accept_channel( &mut self.negotiations, diff --git a/smite-scenarios/src/executor/tests.rs b/smite-scenarios/src/executor/tests.rs index cd18bd20..f65d05cd 100644 --- a/smite-scenarios/src/executor/tests.rs +++ b/smite-scenarios/src/executor/tests.rs @@ -498,6 +498,63 @@ fn execute_recv_accept_channel_rejects_reuse_before_funding() { )); } +#[test] +fn execute_recv_accept_channel_rejects_reused_our_pubkey() { + let temporary_channel_id = TemporaryChannelId::new([0xbb; 32]); + + // Echo back the opener's (our) `funding_pubkey` in the `accept_channel`. + let oc = SampleOpenChannel::new(PointSource::Secret([0x11; 32])); + let reused = oc.message.funding_pubkey; + + let mut b = ProgramBuilder::new(); + let open_channel = send_open_channel(&mut b, &oc); + b.append(Operation::RecvAcceptChannel, &[open_channel.sent]); + + let err = Fixture::new() + .queue(&Message::AcceptChannel(AcceptChannel { + first_per_commitment_point: reused, + ..sample_accept_channel() + })) + .run_err(&b.build()); + + let ExecuteError::Violation(Violation::InvalidAcceptChannel(id, reason)) = &err else { + panic!("unexpected error: {err:?}"); + }; + assert_eq!(*id, temporary_channel_id); + assert!(reason.contains(&format!( + "pubkey reuse: first_per_commitment_point {reused} reuses our key from instruction {}", + open_channel.sent, + ))); +} + +#[test] +fn execute_recv_accept_channel_rejects_reused_target_node_pubkey() { + let temporary_channel_id = TemporaryChannelId::new([0xbb; 32]); + + let oc = SampleOpenChannel::new(PointSource::Secret([0x11; 32])); + let reused = sample_context().target_pubkey; + + let mut b = ProgramBuilder::new(); + let open_channel = send_open_channel(&mut b, &oc); + b.append(Operation::RecvAcceptChannel, &[open_channel.sent]); + + let err = Fixture::new() + .queue(&Message::AcceptChannel(AcceptChannel { + first_per_commitment_point: reused, + ..sample_accept_channel() + })) + .run_err(&b.build()); + + let ExecuteError::Violation(Violation::InvalidAcceptChannel(id, reason)) = &err else { + panic!("unexpected error: {err:?}"); + }; + assert_eq!(*id, temporary_channel_id); + assert!(reason.contains(&format!( + "pubkey reuse: first_per_commitment_point {reused} reuses target's node_pubkey on channel {}", + TemporaryChannelId::ALL + ))); +} + #[test] fn execute_recv_accept_channel_allows_reused_static_pubkeys() { let first_id = TemporaryChannelId::new([0xbb; 32]); @@ -527,6 +584,106 @@ fn execute_recv_accept_channel_allows_reused_static_pubkeys() { assert!(fx.negotiation(&second_id).accept_channel.is_some()); } +#[test] +fn execute_recv_accept_channel_rejects_reused_target_pcp() { + let first_id = TemporaryChannelId::new([0xbb; 32]); + let second_id = TemporaryChannelId::new([0xcc; 32]); + let accept_channel = sample_accept_channel(); + + // Run two negotiations with different temporary channel IDs but the same + // pubkeys. + let mut b = ProgramBuilder::new(); + negotiate_channel(&mut b, &announced_open_channel()); + let mut second_open = announced_open_channel(); + second_open.message.temporary_channel_id = second_id; + negotiate_channel(&mut b, &second_open); + + let err = Fixture::new() + .queue(&Message::AcceptChannel(accept_channel.clone())) + .queue(&Message::AcceptChannel(AcceptChannel { + temporary_channel_id: second_id, + ..accept_channel + })) + .run_err(&b.build()); + + let ExecuteError::Violation(Violation::InvalidAcceptChannel(id, reason)) = &err else { + panic!("unexpected error: {err:?}"); + }; + assert_eq!(*id, second_id); + assert!(reason.contains(&format!( + "pubkey reuse: first_per_commitment_point {} reuses target's per-commitment point on channel {first_id} at commitment number 0", + accept_channel.first_per_commitment_point, + ))); +} + +#[test] +fn execute_recv_accept_channel_rejects_reused_our_channel_ready_pubkey() { + let our_sk = SecretKey::from_slice(&[0x42; 32]).expect("valid secret key"); + + // Send a `channel_ready` revealing a point we hold the secret for, then + // negotiate a channel. + let mut b = ProgramBuilder::new(); + let channel_id = b.append(Operation::LoadChannelId([0xaa; 32]), &[]); + let sk = b.append(Operation::LoadPrivateKey(our_sk.secret_bytes()), &[]); + let our_point = b.append(Operation::DerivePoint, &[sk]); + let alias = b.append(Operation::LoadShortChannelId(0), &[]); + let sent_channel_ready = b.append( + Operation::SendChannelReady { + include_alias: true, + }, + &[channel_id, our_point, alias], + ); + negotiate_channel(&mut b, &announced_open_channel()); + + // The target reuses our point as its `htlc_basepoint`. + let accept_channel = AcceptChannel { + htlc_basepoint: PublicKey::from_secret_key(&Secp256k1::new(), &our_sk), + ..sample_accept_channel() + }; + let err = Fixture::new() + .queue(&Message::AcceptChannel(accept_channel.clone())) + .run_err(&b.build()); + + let ExecuteError::Violation(Violation::InvalidAcceptChannel(id, reason)) = &err else { + panic!("unexpected error: {err:?}"); + }; + assert_eq!(*id, accept_channel.temporary_channel_id); + assert!(reason.contains(&format!( + "pubkey reuse: htlc_basepoint {} reuses our key from instruction {sent_channel_ready}", + accept_channel.htlc_basepoint, + ))); +} + +#[test] +fn execute_recv_accept_channel_rejects_reused_target_channel_ready_pubkey() { + let (fx, target_pcp) = recv_channel_ready_fixture(); + let accept_channel = AcceptChannel { + revocation_basepoint: target_pcp, + ..sample_accept_channel() + }; + + let mut b = ProgramBuilder::new(); + let funding_created = send_funding_created(&mut b); + b.append(Operation::RecvFundingSigned, &[funding_created.sent]); + b.append(Operation::MineBlocks(6), &[]); + b.append(Operation::RecvChannelReady, &[]); + negotiate_channel(&mut b, &announced_open_channel()); + + let err = fx + .queue(&Message::AcceptChannel(accept_channel.clone())) + .run_err(&b.build()); + + let ExecuteError::Violation(Violation::InvalidAcceptChannel(id, reason)) = &err else { + panic!("unexpected error: {err:?}"); + }; + assert_eq!(*id, accept_channel.temporary_channel_id); + assert!(reason.contains(&format!( + "pubkey reuse: revocation_basepoint {} reuses target's per-commitment point on channel {} at commitment number 1", + accept_channel.revocation_basepoint, + funding_channel_id() + ))); +} + #[test] fn execute_records_only_first_open_channel_for_duplicate_id_before_funding() { let temporary_channel_id = TemporaryChannelId::new([0xbb; 32]); diff --git a/smite/src/oracles/accept_channel.rs b/smite/src/oracles/accept_channel.rs index 1a603c09..50bc9e45 100644 --- a/smite/src/oracles/accept_channel.rs +++ b/smite/src/oracles/accept_channel.rs @@ -6,11 +6,13 @@ use crate::bolt::{ is_acceptable_shutdown_script, is_standard_shutdown_script, }; use crate::channel_tx::CommitmentCost; -use crate::pending_channel::PendingChannel; +use crate::pending_channel::{KeyOrigin, PendingChannel}; use crate::violation::Violation; use bitcoin::Amount; use bitcoin::hex::DisplayHex; +use bitcoin::secp256k1::PublicKey; +use std::collections::HashMap; // Constants from the BOLT 2 `open_channel` and `accept_channel` requirements: // https://github.com/lightning/bolts/blob/master/02-peer-protocol.md#requirements-8 @@ -40,12 +42,16 @@ pub struct AcceptChannelContext<'a> { pub negotiation: Option<&'a PendingChannel>, /// Features negotiated between the target node and Smite. pub negotiated_features: &'a Features, + /// Every pubkey revealed on the wire by either side, mapped to its first + /// origin. + pub revealed_pubkeys: &'a HashMap, } /// Checks whether the `open_channel` answered by an `accept_channel` satisfied /// the BOLT 2 v1 channel establishment requirements for acceptance, whether the -/// `accept_channel` itself satisfies them, and that the negotiated -/// `temporary_channel_id` was not reused. +/// `accept_channel` itself satisfies them, that the negotiated +/// `temporary_channel_id` was not reused, and that the `accept_channel` reuses +/// no pubkey whose private key we know or will learn. pub struct AcceptChannelOracle; impl Oracle> for AcceptChannelOracle { @@ -94,6 +100,17 @@ impl Oracle> for AcceptChannelOracle { )); } + // Check that the `accept_channel` reuses no pubkey whose private key we + // know or will learn. + if let Err(reason) = + verify_safe_pubkey_reuse(context.accept_channel, context.revealed_pubkeys) + { + return Err(Violation::InvalidAcceptChannel( + context.accept_channel.temporary_channel_id, + format!("pubkey reuse: {reason}"), + )); + } + Ok(()) } } @@ -423,6 +440,51 @@ fn verify_initial_commitment( Ok(()) } +/// Verifies that the `accept_channel` reuses no pubkey whose private key we +/// know or will eventually learn, returning an error if it reuses one, or +/// `Ok(())` if it reuses none. +/// +/// Static pubkeys are checked against our keys and the target's per-commitment +/// points. The `first_per_commitment_point` is checked against all revealed +/// keys and this message's own static pubkeys. See [`KeyOrigin`] for the +/// rationale. +fn verify_safe_pubkey_reuse( + accept_channel: &AcceptChannel, + revealed_pubkeys: &HashMap, +) -> Result<(), String> { + // A static pubkey may repeat another, but must not match a key we hold the + // private key for or a per-commitment point the target will reveal. + for (field, pubkey) in accept_channel.static_pubkeys() { + if let Some(origin) = revealed_pubkeys.get(&pubkey) { + match origin { + KeyOrigin::Ours { .. } | KeyOrigin::TargetPcp { .. } => { + return Err(format!("{field} {pubkey} reuses {origin}")); + } + KeyOrigin::TargetStatic { .. } => {} + } + } + } + + // The `first_per_commitment_point` must not reuse a previously revealed key + // or any static pubkey from this message, since its secret is eventually + // revealed. + let first_per_commitment_point = accept_channel.first_per_commitment_point; + if let Some(origin) = revealed_pubkeys.get(&first_per_commitment_point) { + return Err(format!( + "first_per_commitment_point {first_per_commitment_point} reuses {origin}" + )); + } + for (field, pubkey) in accept_channel.static_pubkeys() { + if pubkey == first_per_commitment_point { + return Err(format!( + "first_per_commitment_point {first_per_commitment_point} reuses {field}" + )); + } + } + + Ok(()) +} + /// Returns the maximum funding amount allowed by the negotiated features. fn max_funding_satoshis(negotiated_features: &Features) -> u64 { if negotiated_features.supports_feature(Features::OPTION_SUPPORT_LARGE_CHANNEL) { @@ -446,7 +508,7 @@ mod tests { use super::*; use crate::bolt::{AcceptChannelTlvs, CHAIN_HASH_SIZE, OpenChannelTlvs, TemporaryChannelId}; use bitcoin::hashes::Hash; - use bitcoin::secp256k1::{PublicKey, Secp256k1, SecretKey}; + use bitcoin::secp256k1::{Secp256k1, SecretKey}; use bitcoin::{PubkeyHash, ScriptBuf, WPubkeyHash}; fn pubkey(seed: u8) -> PublicKey { @@ -485,7 +547,6 @@ mod tests { /// Valid `accept_channel` message for testing. fn accept_channel() -> AcceptChannel { - let key = pubkey(2); AcceptChannel { temporary_channel_id: TemporaryChannelId::new([1u8; 32]), dust_limit_satoshis: 546, @@ -495,12 +556,12 @@ mod tests { minimum_depth: 6, to_self_delay: 144, max_accepted_htlcs: 483, - funding_pubkey: key, - revocation_basepoint: key, - payment_basepoint: key, - delayed_payment_basepoint: key, - htlc_basepoint: key, - first_per_commitment_point: key, + funding_pubkey: pubkey(2), + revocation_basepoint: pubkey(3), + payment_basepoint: pubkey(4), + delayed_payment_basepoint: pubkey(5), + htlc_basepoint: pubkey(6), + first_per_commitment_point: pubkey(7), tlvs: AcceptChannelTlvs { upfront_shutdown_script: None, channel_type: Some(vec![0x10, 0x00]), @@ -535,11 +596,13 @@ mod tests { accept_channel: &AcceptChannel, negotiation: Option<&PendingChannel>, negotiated_features: &Features, + revealed_pubkeys: &HashMap, ) { if let Err(err) = AcceptChannelOracle.evaluate(&AcceptChannelContext { accept_channel, negotiation, negotiated_features, + revealed_pubkeys, }) { panic!("expected pass, got: {err}"); } @@ -550,12 +613,14 @@ mod tests { accept_channel: &AcceptChannel, negotiation: Option<&PendingChannel>, negotiated_features: &Features, + revealed_pubkeys: &HashMap, expected: &str, ) { match AcceptChannelOracle.evaluate(&AcceptChannelContext { accept_channel, negotiation, negotiated_features, + revealed_pubkeys, }) { Err(Violation::InvalidAcceptChannel(chan_id, reason)) => { assert_eq!(accept_channel.temporary_channel_id, chan_id); @@ -574,6 +639,7 @@ mod tests { &accept_channel(), Some(&pending_negotiation(open_channel())), &sample_negotiated_features(), + &HashMap::new(), ); } @@ -591,6 +657,7 @@ mod tests { &ac, Some(&pending_negotiation(oc)), &sample_negotiated_features(), + &HashMap::new(), ); } @@ -606,6 +673,7 @@ mod tests { &ac, Some(&pending_negotiation(oc)), &sample_negotiated_features(), + &HashMap::new(), ); } @@ -621,7 +689,12 @@ mod tests { let mut ac = accept_channel(); ac.tlvs.upfront_shutdown_script = Some(segwit_script); - assert_pass(&ac, Some(&pending_negotiation(oc)), &negotiated_features); + assert_pass( + &ac, + Some(&pending_negotiation(oc)), + &negotiated_features, + &HashMap::new(), + ); } #[test] @@ -630,6 +703,7 @@ mod tests { &accept_channel(), None, &sample_negotiated_features(), + &HashMap::new(), "unknown temporary_channel_id: no open_channel was sent for this negotiation", ); } @@ -643,6 +717,7 @@ mod tests { &accept_channel(), Some(&pending_negotiation(open_channel())), &negotiated_features, + &HashMap::new(), "invalid open_channel: option_dual_fund has been negotiated", ); } @@ -656,6 +731,7 @@ mod tests { &accept_channel(), Some(&pending_negotiation(oc)), &sample_negotiated_features(), + &HashMap::new(), "invalid open_channel: chain_hash aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa is not the chain hash", ); } @@ -669,6 +745,7 @@ mod tests { &accept_channel(), Some(&pending_negotiation(oc)), &sample_negotiated_features(), + &HashMap::new(), "invalid open_channel: funding_satoshis 16777216 exceeds maximum funding of 16777215 sat", ); } @@ -685,6 +762,7 @@ mod tests { &accept_channel(), Some(&pending_negotiation(oc)), &negotiated_features, + &HashMap::new(), "invalid open_channel: funding_satoshis 2100000000000001 exceeds maximum funding of 2100000000000000 sat", ); } @@ -698,6 +776,7 @@ mod tests { &accept_channel(), Some(&pending_negotiation(oc)), &sample_negotiated_features(), + &HashMap::new(), "invalid open_channel: push_msat 10000000001 exceeds funding amount", ); } @@ -711,6 +790,7 @@ mod tests { &accept_channel(), Some(&pending_negotiation(oc)), &sample_negotiated_features(), + &HashMap::new(), "invalid open_channel: channel_reserve_satoshis 10000000 is not below funding_satoshis 10000000", ); } @@ -727,6 +807,7 @@ mod tests { &accept_channel(), Some(&pending_negotiation(oc)), &negotiated_features, + &HashMap::new(), "invalid open_channel: upfront_shutdown_script is not valid", ); } @@ -740,6 +821,7 @@ mod tests { &accept_channel(), Some(&pending_negotiation(open_channel())), &negotiated_features, + &HashMap::new(), "invalid open_channel: open_channel does not include upfront_shutdown_script", ); } @@ -753,6 +835,7 @@ mod tests { &accept_channel(), Some(&pending_negotiation(oc)), &sample_negotiated_features(), + &HashMap::new(), "invalid open_channel: open_channel does not include a channel_type", ); } @@ -768,6 +851,7 @@ mod tests { &accept_channel(), Some(&pending_negotiation(oc)), &negotiated_features, + &HashMap::new(), "invalid open_channel: channel_type contains features that were not negotiated", ); } @@ -788,6 +872,7 @@ mod tests { &accept_channel(), Some(&pending_negotiation(oc)), &negotiated_features, + &HashMap::new(), "invalid open_channel: channel_type is not a known variant", ); } @@ -802,6 +887,7 @@ mod tests { &accept_channel(), Some(&pending_negotiation(oc)), &sample_negotiated_features(), + &HashMap::new(), "invalid open_channel: zero_fee_commitments requires feerate_per_kw to be 0", ); } @@ -815,6 +901,7 @@ mod tests { &accept_channel(), Some(&pending_negotiation(oc)), &sample_negotiated_features(), + &HashMap::new(), "invalid open_channel: feerate_per_kw must be non-zero without zero_fee_commitments", ); } @@ -828,6 +915,7 @@ mod tests { &accept_channel(), Some(&pending_negotiation(oc)), &sample_negotiated_features(), + &HashMap::new(), "invalid open_channel: to_self_delay 2017 exceeds the maximum of 2016 blocks", ); } @@ -842,6 +930,7 @@ mod tests { &accept_channel(), Some(&pending_negotiation(oc)), &sample_negotiated_features(), + &HashMap::new(), "invalid open_channel: option_scid_alias requires the channel to be private", ); } @@ -855,6 +944,7 @@ mod tests { &accept_channel(), Some(&pending_negotiation(oc)), &sample_negotiated_features(), + &HashMap::new(), "invalid open_channel: max_accepted_htlcs 484 exceeds the limit of 483", ); } @@ -870,6 +960,7 @@ mod tests { &accept_channel(), Some(&pending_negotiation(oc)), &sample_negotiated_features(), + &HashMap::new(), "invalid open_channel: max_accepted_htlcs 115 exceeds the limit of 114", ); } @@ -883,6 +974,7 @@ mod tests { &accept_channel(), Some(&pending_negotiation(oc)), &sample_negotiated_features(), + &HashMap::new(), "invalid open_channel: dust_limit_satoshis 10001 exceeds the maximum of 10000 sat", ); } @@ -896,6 +988,7 @@ mod tests { &accept_channel(), Some(&pending_negotiation(oc)), &sample_negotiated_features(), + &HashMap::new(), "invalid open_channel: dust_limit_satoshis 353 is below the minimum of 354 sat", ); } @@ -909,6 +1002,7 @@ mod tests { &accept_channel(), Some(&pending_negotiation(oc)), &sample_negotiated_features(), + &HashMap::new(), "invalid open_channel: opener balance 10000 sat cannot cover the commitment fee", ); } @@ -923,6 +1017,7 @@ mod tests { &accept_channel(), Some(&pending_negotiation(oc)), &sample_negotiated_features(), + &HashMap::new(), "invalid open_channel: opener balance 17000 sat cannot cover anchor cost of 660 sat (after fee deduction)", ); } @@ -936,6 +1031,7 @@ mod tests { &accept_channel(), Some(&pending_negotiation(oc)), &sample_negotiated_features(), + &HashMap::new(), "invalid open_channel: neither side exceeds channel reserve", ); } @@ -956,6 +1052,7 @@ mod tests { &ac, Some(&pending_negotiation(oc)), &negotiated_features, + &HashMap::new(), "invalid accept_channel: upfront_shutdown_script is not valid", ); } @@ -973,6 +1070,7 @@ mod tests { &accept_channel(), Some(&pending_negotiation(oc)), &negotiated_features, + &HashMap::new(), "accept_channel does not include upfront_shutdown_script", ); } @@ -986,6 +1084,7 @@ mod tests { &ac, Some(&pending_negotiation(open_channel())), &sample_negotiated_features(), + &HashMap::new(), "invalid accept_channel: accept_channel does not include a channel_type", ); } @@ -999,6 +1098,7 @@ mod tests { &ac, Some(&pending_negotiation(open_channel())), &sample_negotiated_features(), + &HashMap::new(), "invalid accept_channel: accept_channel channel_type does not match open_channel", ); } @@ -1012,6 +1112,7 @@ mod tests { &accept_channel(), Some(&pending_negotiation(oc)), &sample_negotiated_features(), + &HashMap::new(), ); } @@ -1027,6 +1128,7 @@ mod tests { &ac, Some(&pending_negotiation(oc)), &sample_negotiated_features(), + &HashMap::new(), "invalid accept_channel: option_zeroconf requires minimum_depth to be 0", ); } @@ -1041,6 +1143,7 @@ mod tests { &ac, Some(&pending_negotiation(oc)), &sample_negotiated_features(), + &HashMap::new(), "invalid accept_channel: channel_reserve_satoshis 545 is below the open_channel dust_limit_satoshis 546", ); } @@ -1055,6 +1158,7 @@ mod tests { &ac, Some(&pending_negotiation(open_channel())), &sample_negotiated_features(), + &HashMap::new(), "invalid accept_channel: dust_limit_satoshis 5000 exceeds channel_reserve_satoshis 4000", ); } @@ -1068,6 +1172,7 @@ mod tests { &ac, Some(&pending_negotiation(open_channel())), &sample_negotiated_features(), + &HashMap::new(), "invalid accept_channel: max_accepted_htlcs 484 exceeds the limit of 483", ); } @@ -1086,6 +1191,7 @@ mod tests { &ac, Some(&pending_negotiation(oc)), &sample_negotiated_features(), + &HashMap::new(), "invalid accept_channel: max_accepted_htlcs 115 exceeds the limit of 114", ); } @@ -1099,6 +1205,7 @@ mod tests { &ac, Some(&pending_negotiation(open_channel())), &sample_negotiated_features(), + &HashMap::new(), "invalid accept_channel: max_accepted_htlcs 0 leaves the channel unable to carry HTLCs", ); } @@ -1113,6 +1220,7 @@ mod tests { &ac, Some(&pending_negotiation(open_channel())), &sample_negotiated_features(), + &HashMap::new(), "invalid accept_channel: dust_limit_satoshis 10001 exceeds the maximum of 10000 sat", ); } @@ -1126,6 +1234,7 @@ mod tests { &ac, Some(&pending_negotiation(open_channel())), &sample_negotiated_features(), + &HashMap::new(), "invalid accept_channel: dust_limit_satoshis 353 is below the minimum of 354 sat", ); } @@ -1139,6 +1248,7 @@ mod tests { &ac, Some(&pending_negotiation(open_channel())), &sample_negotiated_features(), + &HashMap::new(), "invalid accept_channel: htlc_minimum_msat 100000001 exceeds max_htlc_value_in_flight_msat 100000000", ); } @@ -1154,6 +1264,7 @@ mod tests { &ac, Some(&pending_negotiation(oc)), &sample_negotiated_features(), + &HashMap::new(), "invalid accept_channel: htlc_minimum_msat 10000000001 exceeds the open_channel funding amount 10000000000 msat", ); } @@ -1167,6 +1278,7 @@ mod tests { &ac, Some(&pending_negotiation(open_channel())), &sample_negotiated_features(), + &HashMap::new(), "invalid accept_channel: to_self_delay must be non-zero", ); } @@ -1180,6 +1292,7 @@ mod tests { &ac, Some(&pending_negotiation(open_channel())), &sample_negotiated_features(), + &HashMap::new(), "invalid accept_channel: neither side exceeds channel reserve", ); } @@ -1201,6 +1314,7 @@ mod tests { &ac, Some(&pending_negotiation(oc)), &sample_negotiated_features(), + &HashMap::new(), ); } @@ -1218,6 +1332,7 @@ mod tests { &ac, Some(&pending_negotiation(oc)), &sample_negotiated_features(), + &HashMap::new(), ); } @@ -1230,6 +1345,7 @@ mod tests { &accept_channel(), Some(&negotiation), &sample_negotiated_features(), + &HashMap::new(), "temporary_channel_id reuse: previous negotiation has not reached funding_created", ); } @@ -1244,6 +1360,110 @@ mod tests { &accept_channel(), Some(&negotiation), &sample_negotiated_features(), + &HashMap::new(), + ); + } + + #[test] + fn static_pubkey_reuse_our_key() { + let ac = accept_channel(); + let revealed_pubkeys = + HashMap::from([(ac.htlc_basepoint, KeyOrigin::Ours { instruction: 7 })]); + + assert_fail( + &ac, + Some(&pending_negotiation(open_channel())), + &sample_negotiated_features(), + &revealed_pubkeys, + &format!( + "pubkey reuse: htlc_basepoint {} reuses our key from instruction 7", + ac.htlc_basepoint, + ), + ); + } + + #[test] + fn static_pubkey_reuse_target_per_commitment_point() { + let ac = accept_channel(); + let previous_channel_id = TemporaryChannelId::new([2u8; 32]); + let revealed_pubkeys = HashMap::from([( + ac.revocation_basepoint, + KeyOrigin::TargetPcp { + channel: previous_channel_id, + commitment_number: 1, + }, + )]); + + assert_fail( + &ac, + Some(&pending_negotiation(open_channel())), + &sample_negotiated_features(), + &revealed_pubkeys, + &format!( + "pubkey reuse: revocation_basepoint {} reuses target's per-commitment point on channel {previous_channel_id} at commitment number 1", + ac.revocation_basepoint, + ), + ); + } + + #[test] + fn static_pubkey_reuse_target_static_pubkey() { + let ac = accept_channel(); + let previous_channel_id = TemporaryChannelId::new([2u8; 32]); + let revealed_pubkeys = HashMap::from([( + ac.payment_basepoint, + KeyOrigin::TargetStatic { + channel: previous_channel_id, + field: "payment_basepoint", + }, + )]); + + assert_pass( + &ac, + Some(&pending_negotiation(open_channel())), + &sample_negotiated_features(), + &revealed_pubkeys, + ); + } + + #[test] + fn first_per_commitment_point_reuse_target_static_pubkey() { + let ac = accept_channel(); + let previous_channel_id = TemporaryChannelId::new([2u8; 32]); + let revealed_pubkeys = HashMap::from([( + ac.first_per_commitment_point, + KeyOrigin::TargetStatic { + channel: previous_channel_id, + field: "funding_pubkey", + }, + )]); + + assert_fail( + &ac, + Some(&pending_negotiation(open_channel())), + &sample_negotiated_features(), + &revealed_pubkeys, + &format!( + "pubkey reuse: first_per_commitment_point {} reuses target's funding_pubkey on channel {previous_channel_id}", + ac.first_per_commitment_point, + ), + ); + } + + #[test] + fn first_per_commitment_point_reuses_own_static_pubkey() { + let mut ac = accept_channel(); + ac.first_per_commitment_point = ac.funding_pubkey; + + assert_fail( + &ac, + Some(&pending_negotiation(open_channel())), + &sample_negotiated_features(), + &HashMap::new(), + &format!( + "pubkey reuse: first_per_commitment_point {} reuses funding_pubkey", + ac.first_per_commitment_point, + ), ); } } diff --git a/smite/src/violation.rs b/smite/src/violation.rs index a2ff8358..a0987795 100644 --- a/smite/src/violation.rs +++ b/smite/src/violation.rs @@ -31,8 +31,9 @@ pub enum Violation { /// requirement, one of: /// - it names a `temporary_channel_id` we sent no `open_channel` for, /// - it accepts an `open_channel` BOLT 2 required it to reject, - /// - its own fields breach the `accept_channel` requirements, or - /// - it reuses a `temporary_channel_id` still awaiting `funding_created`. + /// - its own fields breach the `accept_channel` requirements, + /// - it reuses a `temporary_channel_id` still awaiting `funding_created`, or + /// - it reuses a pubkey whose private key we know or will learn. #[error("invalid accept_channel for temporary_channel_id {0}: {1}")] InvalidAcceptChannel(TemporaryChannelId, String),