Skip to content

Sanity check for ChannelManager and KeysInterface #1250

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 22, 2022
Merged

Sanity check for ChannelManager and KeysInterface #1250

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 20 additions & 7 deletions lightning/src/chain/keysinterface.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -526,7 +526,8 @@ impl InMemorySigner {
/// described by descriptor, returning the witness stack for the input.
///
/// Returns an Err if the input at input_idx does not exist, has a non-empty script_sig,
/// or is not spending the outpoint described by `descriptor.outpoint`.
/// is not spending the outpoint described by `descriptor.outpoint`,
/// or if an output descriptor script_pubkey does not match the one we can spend.
pub fn sign_counterparty_payment_input<C: Signing>(&self, spend_tx: &Transaction, input_idx: usize, descriptor: &StaticPaymentOutputDescriptor, secp_ctx: &Secp256k1<C>) -> Result<Vec<Vec<u8>>, ()> {
// TODO: We really should be taking the SigHashCache as a parameter here instead of
// spend_tx, but ideally the SigHashCache would expose the transaction's inputs read-only
Expand All @@ -540,6 +541,9 @@ impl InMemorySigner {
let witness_script = bitcoin::Address::p2pkh(&::bitcoin::PublicKey{compressed: true, key: remotepubkey}, Network::Testnet).script_pubkey();
let sighash = hash_to_message!(&bip143::SigHashCache::new(spend_tx).signature_hash(input_idx, &witness_script, descriptor.output.value, SigHashType::All)[..]);
let remotesig = secp_ctx.sign(&sighash, &self.payment_key);
let payment_script = bitcoin::Address::p2wpkh(&::bitcoin::PublicKey{compressed: true, key: remotepubkey}, Network::Bitcoin).unwrap().script_pubkey();

if payment_script != descriptor.output.script_pubkey { return Err(()); }

let mut witness = Vec::with_capacity(2);
witness.push(remotesig.serialize_der().to_vec());
Expand All @@ -552,8 +556,9 @@ impl InMemorySigner {
/// described by descriptor, returning the witness stack for the input.
///
/// Returns an Err if the input at input_idx does not exist, has a non-empty script_sig,
/// is not spending the outpoint described by `descriptor.outpoint`, or does not have a
/// sequence set to `descriptor.to_self_delay`.
/// is not spending the outpoint described by `descriptor.outpoint`, does not have a
/// sequence set to `descriptor.to_self_delay`, or if an output descriptor
/// script_pubkey does not match the one we can spend.
pub fn sign_dynamic_p2wsh_input<C: Signing>(&self, spend_tx: &Transaction, input_idx: usize, descriptor: &DelayedPaymentOutputDescriptor, secp_ctx: &Secp256k1<C>) -> Result<Vec<Vec<u8>>, ()> {
// TODO: We really should be taking the SigHashCache as a parameter here instead of
// spend_tx, but ideally the SigHashCache would expose the transaction's inputs read-only
Expand All @@ -570,6 +575,9 @@ impl InMemorySigner {
let witness_script = chan_utils::get_revokeable_redeemscript(&descriptor.revocation_pubkey, descriptor.to_self_delay, &delayed_payment_pubkey);
let sighash = hash_to_message!(&bip143::SigHashCache::new(spend_tx).signature_hash(input_idx, &witness_script, descriptor.output.value, SigHashType::All)[..]);
let local_delayedsig = secp_ctx.sign(&sighash, &delayed_payment_key);
let payment_script = bitcoin::Address::p2wsh(&witness_script, Network::Bitcoin).script_pubkey();

if descriptor.output.script_pubkey != payment_script { return Err(()); }

let mut witness = Vec::with_capacity(3);
witness.push(local_delayedsig.serialize_der().to_vec());
Expand Down Expand Up @@ -927,8 +935,9 @@ impl KeysManager {
/// output to the given change destination (if sufficient change value remains). The
/// transaction will have a feerate, at least, of the given value.
///
/// Returns `Err(())` if the output value is greater than the input value minus required fee or
/// if a descriptor was duplicated.
/// Returns `Err(())` if the output value is greater than the input value minus required fee,
/// if a descriptor was duplicated, or if an output descriptor script_pubkey
/// does not match the one we can spend.
///
/// We do not enforce that outputs meet the dust limit or that any output scripts are standard.
///
Expand Down Expand Up @@ -996,15 +1005,15 @@ impl KeysManager {
self.derive_channel_keys(descriptor.channel_value_satoshis, &descriptor.channel_keys_id),
descriptor.channel_keys_id));
}
spend_tx.input[input_idx].witness = keys_cache.as_ref().unwrap().0.sign_counterparty_payment_input(&spend_tx, input_idx, &descriptor, &secp_ctx).unwrap();
spend_tx.input[input_idx].witness = keys_cache.as_ref().unwrap().0.sign_counterparty_payment_input(&spend_tx, input_idx, &descriptor, &secp_ctx)?;
},
SpendableOutputDescriptor::DelayedPaymentOutput(descriptor) => {
if keys_cache.is_none() || keys_cache.as_ref().unwrap().1 != descriptor.channel_keys_id {
keys_cache = Some((
self.derive_channel_keys(descriptor.channel_value_satoshis, &descriptor.channel_keys_id),
descriptor.channel_keys_id));
}
spend_tx.input[input_idx].witness = keys_cache.as_ref().unwrap().0.sign_dynamic_p2wsh_input(&spend_tx, input_idx, &descriptor, &secp_ctx).unwrap();
spend_tx.input[input_idx].witness = keys_cache.as_ref().unwrap().0.sign_dynamic_p2wsh_input(&spend_tx, input_idx, &descriptor, &secp_ctx)?;
},
SpendableOutputDescriptor::StaticOutput { ref output, .. } => {
let derivation_idx = if output.script_pubkey == self.destination_script {
Expand All @@ -1029,6 +1038,10 @@ impl KeysManager {
assert_eq!(pubkey.key, self.shutdown_pubkey);
}
let witness_script = bitcoin::Address::p2pkh(&pubkey, Network::Testnet).script_pubkey();
let payment_script = bitcoin::Address::p2wpkh(&pubkey, Network::Testnet).expect("uncompressed key found").script_pubkey();

if payment_script != output.script_pubkey { return Err(()); };

let sighash = hash_to_message!(&bip143::SigHashCache::new(&spend_tx).signature_hash(input_idx, &witness_script, output.value, SigHashType::All)[..]);
let sig = secp_ctx.sign(&sighash, &secret.private_key.key);
spend_tx.input[input_idx].witness.push(sig.serialize_der().to_vec());
Expand Down
14 changes: 13 additions & 1 deletion lightning/src/ln/channelmanager.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6215,6 +6215,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> Writeable f
write_tlv_fields!(writer, {
(1, pending_outbound_payments_no_retry, required),
(3, pending_outbound_payments, required),
(5, self.our_network_pubkey, required)
});

Ok(())
Expand Down Expand Up @@ -6509,10 +6510,13 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
// pending_outbound_payments_no_retry is for compatibility with 0.0.101 clients.
let mut pending_outbound_payments_no_retry: Option<HashMap<PaymentId, HashSet<[u8; 32]>>> = None;
let mut pending_outbound_payments = None;
let mut received_network_pubkey: Option<PublicKey> = None;
read_tlv_fields!(reader, {
(1, pending_outbound_payments_no_retry, option),
(3, pending_outbound_payments, option),
(5, received_network_pubkey, option)
});

if pending_outbound_payments.is_none() && pending_outbound_payments_no_retry.is_none() {
pending_outbound_payments = Some(pending_outbound_payments_compat);
} else if pending_outbound_payments.is_none() {
Expand Down Expand Up @@ -6575,6 +6579,14 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
pending_events_read.append(&mut channel_closures);
}

let our_network_pubkey = PublicKey::from_secret_key(&secp_ctx, &args.keys_manager.get_node_secret());
if let Some(network_pubkey) = received_network_pubkey {
if network_pubkey != our_network_pubkey {
log_error!(args.logger, "Key that was generated does not match the existing key.");
return Err(DecodeError::InvalidValue);
}
}

let inbound_pmt_key_material = args.keys_manager.get_inbound_payment_key_material();
let expanded_inbound_key = inbound_payment::ExpandedKey::new(&inbound_pmt_key_material);
let channel_manager = ChannelManager {
Expand All @@ -6597,7 +6609,7 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
pending_outbound_payments: Mutex::new(pending_outbound_payments.unwrap()),

our_network_key: args.keys_manager.get_node_secret(),
our_network_pubkey: PublicKey::from_secret_key(&secp_ctx, &args.keys_manager.get_node_secret()),
our_network_pubkey,
secp_ctx,

last_node_announcement_serial: AtomicUsize::new(last_node_announcement_serial as usize),
Expand Down