Skip to content

Generate local signatures with additional randomness #2205

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.

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 4 commits into from
Apr 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 4 additions & 3 deletions ci/ci-tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,14 @@ for DIR in lightning lightning-invoice lightning-rapid-gossip-sync; do
cargo test --verbose --color always --no-default-features --features no-std
# check if there is a conflict between no-std and the default std feature
cargo test --verbose --color always --features no-std
# check that things still pass without grind_signatures
# note that outbound_commitment_test only runs in this mode, because of hardcoded signature values
cargo test --verbose --color always --no-default-features --features std
# check if there is a conflict between no-std and the c_bindings cfg
RUSTFLAGS="--cfg=c_bindings" cargo test --verbose --color always --no-default-features --features=no-std
popd
done
# Note that outbound_commitment_test only runs in this mode because of hardcoded signature values
pushd lightning
cargo test --verbose --color always --no-default-features --features=std,_test_vectors
popd
# This one only works for lightning-invoice
pushd lightning-invoice
# check that compile with no-std and serde works in lightning-invoice
Expand Down
3 changes: 2 additions & 1 deletion fuzz/src/chanmon_consistency.rs
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,7 @@ impl SignerProvider for KeyProvider {
[id, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 9, self.node_secret[31]],
channel_value_satoshis,
channel_keys_id,
channel_keys_id,
);
let revoked_commitment = self.make_enforcement_state_cell(keys.commitment_seed);
EnforcingSigner::new_with_revoked(keys, revoked_commitment, false)
Expand All @@ -248,7 +249,7 @@ impl SignerProvider for KeyProvider {
fn read_chan_signer(&self, buffer: &[u8]) -> Result<Self::Signer, DecodeError> {
let mut reader = std::io::Cursor::new(buffer);

let inner: InMemorySigner = Readable::read(&mut reader)?;
let inner: InMemorySigner = ReadableArgs::read(&mut reader, self)?;
let state = self.make_enforcement_state_cell(inner.commitment_seed);

Ok(EnforcingSigner {
Expand Down
6 changes: 4 additions & 2 deletions fuzz/src/full_stack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ use lightning::util::config::UserConfig;
use lightning::util::errors::APIError;
use lightning::util::enforcing_trait_impls::{EnforcingSigner, EnforcementState};
use lightning::util::logger::Logger;
use lightning::util::ser::{Readable, Writeable};
use lightning::util::ser::{Readable, ReadableArgs, Writeable};

use crate::utils::test_logger;
use crate::utils::test_persister::TestPersister;
Expand Down Expand Up @@ -347,6 +347,7 @@ impl SignerProvider for KeyProvider {
[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 6, ctr],
channel_value_satoshis,
channel_keys_id,
channel_keys_id,
)
} else {
InMemorySigner::new(
Expand All @@ -359,12 +360,13 @@ impl SignerProvider for KeyProvider {
[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 12, ctr],
channel_value_satoshis,
channel_keys_id,
channel_keys_id,
)
}, state, false)
}

fn read_chan_signer(&self, mut data: &[u8]) -> Result<EnforcingSigner, DecodeError> {
let inner: InMemorySigner = Readable::read(&mut data)?;
let inner: InMemorySigner = ReadableArgs::read(&mut data, self)?;
let state = Arc::new(Mutex::new(EnforcementState::new()));

Ok(EnforcingSigner::new_with_revoked(
Expand Down
2 changes: 2 additions & 0 deletions lightning/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ max_level_trace = []
# This is unsafe to use in production because it may result in the counterparty publishing taking our funds.
unsafe_revoked_tx_signing = []
_bench_unstable = []
# Override signing to not include randomness when generating signatures for test vectors.
_test_vectors = []

no-std = ["hashbrown", "bitcoin/no-std", "core2/alloc"]
std = ["bitcoin/std"]
Expand Down
1 change: 1 addition & 0 deletions lightning/src/chain/channelmonitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4235,6 +4235,7 @@ mod tests {
[41; 32],
0,
[0; 32],
[0; 32],
);

let counterparty_pubkeys = ChannelPublicKeys {
Expand Down
82 changes: 61 additions & 21 deletions lightning/src/chain/keysinterface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ use bitcoin::secp256k1::ecdsa::RecoverableSignature;
use bitcoin::{PackedLockTime, secp256k1, Sequence, Witness};

use crate::util::transaction_utils;
use crate::util::crypto::{hkdf_extract_expand_twice, sign};
use crate::util::ser::{Writeable, Writer, Readable};
use crate::util::crypto::{hkdf_extract_expand_twice, sign, sign_with_aux_rand};
use crate::util::ser::{Writeable, Writer, Readable, ReadableArgs};
use crate::chain::transaction::OutPoint;
#[cfg(anchors)]
use crate::events::bump_transaction::HTLCDescriptor;
Expand All @@ -45,6 +45,7 @@ use crate::ln::script::ShutdownScript;

use crate::prelude::*;
use core::convert::TryInto;
use core::ops::Deref;
use core::sync::atomic::{AtomicUsize, Ordering};
use crate::io::{self, Error};
use crate::ln::msgs::{DecodeError, MAX_VALUE_MSAT};
Expand Down Expand Up @@ -553,7 +554,6 @@ pub trait SignerProvider {
fn get_shutdown_scriptpubkey(&self) -> ShutdownScript;
}

#[derive(Clone)]
/// A simple implementation of [`WriteableEcdsaChannelSigner`] that just keeps the private keys in memory.
///
/// This implementation performs no policy checks and is insufficient by itself as
Expand All @@ -580,6 +580,30 @@ pub struct InMemorySigner {
channel_value_satoshis: u64,
/// Key derivation parameters.
channel_keys_id: [u8; 32],
/// Seed from which all randomness produced is derived from.
rand_bytes_unique_start: [u8; 32],
/// Tracks the number of times we've produced randomness to ensure we don't return the same
/// bytes twice.
rand_bytes_index: AtomicCounter,
}

impl Clone for InMemorySigner {
fn clone(&self) -> Self {
Self {
funding_key: self.funding_key.clone(),
revocation_base_key: self.revocation_base_key.clone(),
payment_key: self.payment_key.clone(),
delayed_payment_base_key: self.delayed_payment_base_key.clone(),
htlc_base_key: self.htlc_base_key.clone(),
commitment_seed: self.commitment_seed.clone(),
holder_channel_pubkeys: self.holder_channel_pubkeys.clone(),
channel_parameters: self.channel_parameters.clone(),
channel_value_satoshis: self.channel_value_satoshis,
channel_keys_id: self.channel_keys_id,
rand_bytes_unique_start: self.get_secure_random_bytes(),
rand_bytes_index: AtomicCounter::new(),
}
}
}

impl InMemorySigner {
Expand All @@ -594,6 +618,7 @@ impl InMemorySigner {
commitment_seed: [u8; 32],
channel_value_satoshis: u64,
channel_keys_id: [u8; 32],
rand_bytes_unique_start: [u8; 32],
) -> InMemorySigner {
let holder_channel_pubkeys =
InMemorySigner::make_holder_keys(secp_ctx, &funding_key, &revocation_base_key,
Expand All @@ -610,6 +635,8 @@ impl InMemorySigner {
holder_channel_pubkeys,
channel_parameters: None,
channel_keys_id,
rand_bytes_unique_start,
rand_bytes_index: AtomicCounter::new(),
}
}

Expand Down Expand Up @@ -686,7 +713,7 @@ impl InMemorySigner {
let remotepubkey = self.pubkeys().payment_point;
let witness_script = bitcoin::Address::p2pkh(&::bitcoin::PublicKey{compressed: true, inner: remotepubkey}, Network::Testnet).script_pubkey();
let sighash = hash_to_message!(&sighash::SighashCache::new(spend_tx).segwit_signature_hash(input_idx, &witness_script, descriptor.output.value, EcdsaSighashType::All).unwrap()[..]);
let remotesig = sign(secp_ctx, &sighash, &self.payment_key);
let remotesig = sign_with_aux_rand(secp_ctx, &sighash, &self.payment_key, &self);
let payment_script = bitcoin::Address::p2wpkh(&::bitcoin::PublicKey{compressed: true, inner: remotepubkey}, Network::Bitcoin).unwrap().script_pubkey();

if payment_script != descriptor.output.script_pubkey { return Err(()); }
Expand Down Expand Up @@ -722,7 +749,7 @@ impl InMemorySigner {
let delayed_payment_pubkey = PublicKey::from_secret_key(&secp_ctx, &delayed_payment_key);
let witness_script = chan_utils::get_revokeable_redeemscript(&descriptor.revocation_pubkey, descriptor.to_self_delay, &delayed_payment_pubkey);
let sighash = hash_to_message!(&sighash::SighashCache::new(spend_tx).segwit_signature_hash(input_idx, &witness_script, descriptor.output.value, EcdsaSighashType::All).unwrap()[..]);
let local_delayedsig = sign(secp_ctx, &sighash, &delayed_payment_key);
let local_delayedsig = sign_with_aux_rand(secp_ctx, &sighash, &delayed_payment_key, &self);
let payment_script = bitcoin::Address::p2wsh(&witness_script, Network::Bitcoin).script_pubkey();

if descriptor.output.script_pubkey != payment_script { return Err(()); }
Expand All @@ -736,6 +763,15 @@ impl InMemorySigner {
}
}

impl EntropySource for InMemorySigner {
fn get_secure_random_bytes(&self) -> [u8; 32] {
let index = self.rand_bytes_index.get_increment();
let mut nonce = [0u8; 16];
nonce[..8].copy_from_slice(&index.to_be_bytes());
ChaCha20::get_single_block(&self.rand_bytes_unique_start, &nonce)
}
}

impl ChannelSigner for InMemorySigner {
fn get_per_commitment_point(&self, idx: u64, secp_ctx: &Secp256k1<secp256k1::All>) -> PublicKey {
let commitment_secret = SecretKey::from_slice(&chan_utils::build_commitment_secret(&self.commitment_seed, idx)).unwrap();
Expand Down Expand Up @@ -774,7 +810,7 @@ impl EcdsaChannelSigner for InMemorySigner {
let channel_funding_redeemscript = make_funding_redeemscript(&funding_pubkey, &self.counterparty_pubkeys().funding_pubkey);

let built_tx = trusted_tx.built_transaction();
let commitment_sig = built_tx.sign(&self.funding_key, &channel_funding_redeemscript, self.channel_value_satoshis, secp_ctx);
let commitment_sig = built_tx.sign_counterparty_commitment(&self.funding_key, &channel_funding_redeemscript, self.channel_value_satoshis, secp_ctx);
let commitment_txid = built_tx.txid;

let mut htlc_sigs = Vec::with_capacity(commitment_tx.htlcs().len());
Expand All @@ -799,9 +835,9 @@ impl EcdsaChannelSigner for InMemorySigner {
let funding_pubkey = PublicKey::from_secret_key(secp_ctx, &self.funding_key);
let funding_redeemscript = make_funding_redeemscript(&funding_pubkey, &self.counterparty_pubkeys().funding_pubkey);
let trusted_tx = commitment_tx.trust();
let sig = trusted_tx.built_transaction().sign(&self.funding_key, &funding_redeemscript, self.channel_value_satoshis, secp_ctx);
let sig = trusted_tx.built_transaction().sign_holder_commitment(&self.funding_key, &funding_redeemscript, self.channel_value_satoshis, &self, secp_ctx);
let channel_parameters = self.get_channel_parameters();
let htlc_sigs = trusted_tx.get_htlc_sigs(&self.htlc_base_key, &channel_parameters.as_holder_broadcastable(), secp_ctx)?;
let htlc_sigs = trusted_tx.get_htlc_sigs(&self.htlc_base_key, &channel_parameters.as_holder_broadcastable(), &self, secp_ctx)?;
Ok((sig, htlc_sigs))
}

Expand All @@ -810,9 +846,9 @@ impl EcdsaChannelSigner for InMemorySigner {
let funding_pubkey = PublicKey::from_secret_key(secp_ctx, &self.funding_key);
let funding_redeemscript = make_funding_redeemscript(&funding_pubkey, &self.counterparty_pubkeys().funding_pubkey);
let trusted_tx = commitment_tx.trust();
let sig = trusted_tx.built_transaction().sign(&self.funding_key, &funding_redeemscript, self.channel_value_satoshis, secp_ctx);
let sig = trusted_tx.built_transaction().sign_holder_commitment(&self.funding_key, &funding_redeemscript, self.channel_value_satoshis, &self, secp_ctx);
let channel_parameters = self.get_channel_parameters();
let htlc_sigs = trusted_tx.get_htlc_sigs(&self.htlc_base_key, &channel_parameters.as_holder_broadcastable(), secp_ctx)?;
let htlc_sigs = trusted_tx.get_htlc_sigs(&self.htlc_base_key, &channel_parameters.as_holder_broadcastable(), &self, secp_ctx)?;
Ok((sig, htlc_sigs))
}

Expand All @@ -826,7 +862,7 @@ impl EcdsaChannelSigner for InMemorySigner {
};
let mut sighash_parts = sighash::SighashCache::new(justice_tx);
let sighash = hash_to_message!(&sighash_parts.segwit_signature_hash(input, &witness_script, amount, EcdsaSighashType::All).unwrap()[..]);
return Ok(sign(secp_ctx, &sighash, &revocation_key))
return Ok(sign_with_aux_rand(secp_ctx, &sighash, &revocation_key, &self))
}

fn sign_justice_revoked_htlc(&self, justice_tx: &Transaction, input: usize, amount: u64, per_commitment_key: &SecretKey, htlc: &HTLCOutputInCommitment, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<Signature, ()> {
Expand All @@ -840,7 +876,7 @@ impl EcdsaChannelSigner for InMemorySigner {
};
let mut sighash_parts = sighash::SighashCache::new(justice_tx);
let sighash = hash_to_message!(&sighash_parts.segwit_signature_hash(input, &witness_script, amount, EcdsaSighashType::All).unwrap()[..]);
return Ok(sign(secp_ctx, &sighash, &revocation_key))
return Ok(sign_with_aux_rand(secp_ctx, &sighash, &revocation_key, &self))
}

#[cfg(anchors)]
Expand All @@ -858,7 +894,7 @@ impl EcdsaChannelSigner for InMemorySigner {
let our_htlc_private_key = chan_utils::derive_private_key(
&secp_ctx, &per_commitment_point, &self.htlc_base_key
);
Ok(sign(&secp_ctx, &hash_to_message!(sighash), &our_htlc_private_key))
Ok(sign_with_aux_rand(&secp_ctx, &hash_to_message!(sighash), &our_htlc_private_key, &self))
}

fn sign_counterparty_htlc_transaction(&self, htlc_tx: &Transaction, input: usize, amount: u64, per_commitment_point: &PublicKey, htlc: &HTLCOutputInCommitment, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<Signature, ()> {
Expand All @@ -869,7 +905,7 @@ impl EcdsaChannelSigner for InMemorySigner {
let witness_script = chan_utils::get_htlc_redeemscript_with_explicit_keys(&htlc, self.opt_anchors(), &counterparty_htlcpubkey, &htlcpubkey, &revocation_pubkey);
let mut sighash_parts = sighash::SighashCache::new(htlc_tx);
let sighash = hash_to_message!(&sighash_parts.segwit_signature_hash(input, &witness_script, amount, EcdsaSighashType::All).unwrap()[..]);
Ok(sign(secp_ctx, &sighash, &htlc_key))
Ok(sign_with_aux_rand(secp_ctx, &sighash, &htlc_key, &self))
}

fn sign_closing_transaction(&self, closing_tx: &ClosingTransaction, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<Signature, ()> {
Expand All @@ -885,14 +921,14 @@ impl EcdsaChannelSigner for InMemorySigner {
let sighash = sighash::SighashCache::new(&*anchor_tx).segwit_signature_hash(
input, &witness_script, ANCHOR_OUTPUT_VALUE_SATOSHI, EcdsaSighashType::All,
).unwrap();
Ok(sign(secp_ctx, &hash_to_message!(&sighash[..]), &self.funding_key))
Ok(sign_with_aux_rand(secp_ctx, &hash_to_message!(&sighash[..]), &self.funding_key, &self))
}

fn sign_channel_announcement_with_funding_key(
&self, msg: &UnsignedChannelAnnouncement, secp_ctx: &Secp256k1<secp256k1::All>
) -> Result<Signature, ()> {
let msghash = hash_to_message!(&Sha256dHash::hash(&msg.encode()[..])[..]);
Ok(sign(secp_ctx, &msghash, &self.funding_key))
Ok(secp_ctx.sign_ecdsa(&msghash, &self.funding_key))
}
}

Expand Down Expand Up @@ -922,8 +958,8 @@ impl Writeable for InMemorySigner {
}
}

impl Readable for InMemorySigner {
fn read<R: io::Read>(reader: &mut R) -> Result<Self, DecodeError> {
impl<ES: Deref> ReadableArgs<ES> for InMemorySigner where ES::Target: EntropySource {
fn read<R: io::Read>(reader: &mut R, entropy_source: ES) -> Result<Self, DecodeError> {
let _ver = read_ver_prefix!(reader, SERIALIZATION_VERSION);

let funding_key = Readable::read(reader)?;
Expand Down Expand Up @@ -953,6 +989,8 @@ impl Readable for InMemorySigner {
holder_channel_pubkeys,
channel_parameters: counterparty_channel_data,
channel_keys_id: keys_id,
rand_bytes_unique_start: entropy_source.get_secure_random_bytes(),
rand_bytes_index: AtomicCounter::new(),
})
}
}
Expand Down Expand Up @@ -1107,6 +1145,7 @@ impl KeysManager {
let payment_key = key_step!(b"payment key", revocation_base_key);
let delayed_payment_base_key = key_step!(b"delayed payment base key", payment_key);
let htlc_base_key = key_step!(b"HTLC base key", delayed_payment_base_key);
let prng_seed = self.get_secure_random_bytes();

InMemorySigner::new(
&self.secp_ctx,
Expand All @@ -1118,6 +1157,7 @@ impl KeysManager {
commitment_seed,
channel_value_satoshis,
params.clone(),
prng_seed,
)
}

Expand Down Expand Up @@ -1233,7 +1273,7 @@ impl KeysManager {
if payment_script != output.script_pubkey { return Err(()); };

let sighash = hash_to_message!(&sighash::SighashCache::new(&spend_tx).segwit_signature_hash(input_idx, &witness_script, output.value, EcdsaSighashType::All).unwrap()[..]);
let sig = sign(secp_ctx, &sighash, &secret.private_key);
let sig = sign_with_aux_rand(secp_ctx, &sighash, &secret.private_key, &self);
let mut sig_ser = sig.serialize_der().to_vec();
sig_ser.push(EcdsaSighashType::All as u8);
spend_tx.input[input_idx].witness.push(sig_ser);
Expand Down Expand Up @@ -1295,7 +1335,7 @@ impl NodeSigner for KeysManager {

fn sign_gossip_message(&self, msg: UnsignedGossipMessage) -> Result<Signature, ()> {
let msg_hash = hash_to_message!(&Sha256dHash::hash(&msg.encode()[..])[..]);
Ok(sign(&self.secp_ctx, &msg_hash, &self.node_secret))
Ok(self.secp_ctx.sign_ecdsa(&msg_hash, &self.node_secret))
}
}

Expand Down Expand Up @@ -1323,7 +1363,7 @@ impl SignerProvider for KeysManager {
}

fn read_chan_signer(&self, reader: &[u8]) -> Result<Self::Signer, DecodeError> {
InMemorySigner::read(&mut io::Cursor::new(reader))
InMemorySigner::read(&mut io::Cursor::new(reader), self)
}

fn get_destination_script(&self) -> Script {
Expand Down
Loading