From 1b70d9ee8f47df33fe70d7396e2a023feaea6be5 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Wed, 1 Sep 2021 20:22:49 +0000 Subject: [PATCH] Require user cooperative close payout scripts to be Segwit There is little reason for users to be paying out to non-Segwit scripts when closing channels at this point. Given we will soon, in rare cases, force-close during shutdown when a counterparty closes to a non-Segwit script, we should also require it of our own users. --- lightning/src/ln/channel.rs | 28 +++++++++++----------- lightning/src/ln/script.rs | 46 ++++++++----------------------------- 2 files changed, 22 insertions(+), 52 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index f02818b26..57da708ba 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -26,7 +26,7 @@ use ln::{PaymentPreimage, PaymentHash}; use ln::features::{ChannelFeatures, InitFeatures}; use ln::msgs; use ln::msgs::{DecodeError, OptionalField, DataLossProtect}; -use ln::script::ShutdownScript; +use ln::script::{self, ShutdownScript}; use ln::channelmanager::{CounterpartyForwardingInfo, PendingHTLCStatus, HTLCSource, HTLCFailReason, HTLCFailureMsg, PendingHTLCInfo, RAACommitmentOrder, BREAKDOWN_TIMEOUT, MIN_CLTV_EXPIRY_DELTA, MAX_LOCAL_BREAKDOWN_TIMEOUT}; use ln::chan_utils::{CounterpartyCommitmentSecrets, TxCreationKeys, HTLCOutputInCommitment, HTLC_SUCCESS_TX_WEIGHT, HTLC_TIMEOUT_TX_WEIGHT, make_funding_redeemscript, ChannelPublicKeys, CommitmentTransaction, HolderCommitmentTransaction, ChannelTransactionParameters, CounterpartyChannelTransactionParameters, MAX_HTLCS, get_commitment_transaction_number_obscure_factor, ClosingTransaction}; use ln::chan_utils; @@ -44,7 +44,6 @@ use util::scid_utils::scid_from_parts; use io; use prelude::*; use core::{cmp,mem,fmt}; -use core::convert::TryFrom; use core::ops::Deref; #[cfg(any(test, feature = "fuzztarget", debug_assertions))] use sync::Mutex; @@ -896,10 +895,10 @@ impl Channel { if script.len() == 0 { None } else { - match ShutdownScript::try_from((script.clone(), their_features)) { - Ok(shutdown_script) => Some(shutdown_script.into_inner()), - Err(_) => return Err(ChannelError::Close(format!("Peer is signaling upfront_shutdown but has provided an unacceptable scriptpubkey format: {}", script))), + if !script::is_bolt2_compliant(&script, their_features) { + return Err(ChannelError::Close(format!("Peer is signaling upfront_shutdown but has provided an unacceptable scriptpubkey format: {}", script))) } + Some(script.clone()) } }, // Peer is signaling upfront shutdown but don't opt-out with correct mechanism (a.k.a 0-length script). Peer looks buggy, we fail the channel @@ -1641,10 +1640,10 @@ impl Channel { if script.len() == 0 { None } else { - match ShutdownScript::try_from((script.clone(), their_features)) { - Ok(shutdown_script) => Some(shutdown_script.into_inner()), - Err(_) => return Err(ChannelError::Close(format!("Peer is signaling upfront_shutdown but has provided an unacceptable scriptpubkey format: {}", script))), + if !script::is_bolt2_compliant(&script, their_features) { + return Err(ChannelError::Close(format!("Peer is signaling upfront_shutdown but has provided an unacceptable scriptpubkey format: {}", script))); } + Some(script.clone()) } }, // Peer is signaling upfront shutdown but don't opt-out with correct mechanism (a.k.a 0-length script). Peer looks buggy, we fail the channel @@ -3494,17 +3493,16 @@ impl Channel { } assert_eq!(self.channel_state & ChannelState::ShutdownComplete as u32, 0); - let shutdown_scriptpubkey = match ShutdownScript::try_from((msg.scriptpubkey.clone(), their_features)) { - Ok(script) => script.into_inner(), - Err(_) => return Err(ChannelError::Close(format!("Got a nonstandard scriptpubkey ({}) from remote peer", msg.scriptpubkey.to_bytes().to_hex()))), - }; + if !script::is_bolt2_compliant(&msg.scriptpubkey, their_features) { + return Err(ChannelError::Close(format!("Got a nonstandard scriptpubkey ({}) from remote peer", msg.scriptpubkey.to_bytes().to_hex()))); + } if self.counterparty_shutdown_scriptpubkey.is_some() { - if Some(&shutdown_scriptpubkey) != self.counterparty_shutdown_scriptpubkey.as_ref() { - return Err(ChannelError::Close(format!("Got shutdown request with a scriptpubkey ({}) which did not match their previous scriptpubkey.", shutdown_scriptpubkey.to_bytes().to_hex()))); + if Some(&msg.scriptpubkey) != self.counterparty_shutdown_scriptpubkey.as_ref() { + return Err(ChannelError::Close(format!("Got shutdown request with a scriptpubkey ({}) which did not match their previous scriptpubkey.", msg.scriptpubkey.to_bytes().to_hex()))); } } else { - self.counterparty_shutdown_scriptpubkey = Some(shutdown_scriptpubkey); + self.counterparty_shutdown_scriptpubkey = Some(msg.scriptpubkey.clone()); } // If we have any LocalAnnounced updates we'll probably just get back an update_fail_htlc diff --git a/lightning/src/ln/script.rs b/lightning/src/ln/script.rs index 4e81d76ad..00ad48bb1 100644 --- a/lightning/src/ln/script.rs +++ b/lightning/src/ln/script.rs @@ -3,7 +3,7 @@ use bitcoin::blockdata::opcodes::all::OP_PUSHBYTES_0 as SEGWIT_V0; use bitcoin::blockdata::script::{Builder, Script}; use bitcoin::hashes::Hash; -use bitcoin::hash_types::{PubkeyHash, ScriptHash, WPubkeyHash, WScriptHash}; +use bitcoin::hash_types::{WPubkeyHash, WScriptHash}; use bitcoin::secp256k1::key::PublicKey; use ln::features::InitFeatures; @@ -66,16 +66,6 @@ impl ShutdownScript { Self(ShutdownScriptImpl::Legacy(pubkey)) } - /// Generates a P2PKH script pubkey from the given [`PubkeyHash`]. - pub fn new_p2pkh(pubkey_hash: &PubkeyHash) -> Self { - Self(ShutdownScriptImpl::Bolt2(Script::new_p2pkh(pubkey_hash))) - } - - /// Generates a P2SH script pubkey from the given [`ScriptHash`]. - pub fn new_p2sh(script_hash: &ScriptHash) -> Self { - Self(ShutdownScriptImpl::Bolt2(Script::new_p2sh(script_hash))) - } - /// Generates a P2WPKH script pubkey from the given [`WPubkeyHash`]. pub fn new_p2wpkh(pubkey_hash: &WPubkeyHash) -> Self { Self(ShutdownScriptImpl::Bolt2(Script::new_v0_wpkh(pubkey_hash))) @@ -126,7 +116,9 @@ impl ShutdownScript { } } -fn is_bolt2_compliant(script: &Script, features: &InitFeatures) -> bool { +/// Check if a given script is compliant with BOLT 2's shutdown script requirements for the given +/// counterparty features. +pub(crate) fn is_bolt2_compliant(script: &Script, features: &InitFeatures) -> bool { if script.is_p2pkh() || script.is_p2sh() || script.is_v0_p2wpkh() || script.is_v0_p2wsh() { true } else if features.supports_shutdown_anysegwit() { @@ -136,6 +128,8 @@ fn is_bolt2_compliant(script: &Script, features: &InitFeatures) -> bool { } } +// Note that this is only for our own shutdown scripts. Counterparties are still allowed to send us +// non-witness shutdown scripts which this rejects. impl TryFrom