Skip to content
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

Update rust-bitcoin and add secp256k1 context randomization #802

Merged
Merged
2 changes: 1 addition & 1 deletion .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ jobs:
sudo apt-get update
sudo apt-get -y install build-essential binutils-dev libunwind-dev
- name: Sanity check fuzz targets on Rust ${{ env.TOOLCHAIN }}
run: cd fuzz && cargo test --verbose --color always
run: cd fuzz && RUSTFLAGS="--cfg=fuzzing" cargo test --verbose --color always
- name: Run fuzzers
run: cd fuzz && ./ci-fuzz.sh

Expand Down
2 changes: 1 addition & 1 deletion background-processor/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ edition = "2018"
# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
bitcoin = "0.24"
bitcoin = "0.26"
lightning = { version = "0.0.12", path = "../lightning", features = ["allow_wallclock_use"] }
lightning-persister = { version = "0.0.1", path = "../lightning-persister" }

Expand Down
8 changes: 7 additions & 1 deletion fuzz/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,17 @@ stdin_fuzz = []
[dependencies]
afl = { version = "0.4", optional = true }
lightning = { path = "../lightning", features = ["fuzztarget"] }
bitcoin = { version = "0.24", features = ["fuzztarget"] }
bitcoin = { version = "0.26", features = ["fuzztarget", "secp-lowmemory"] }
hex = "0.3"
honggfuzz = { version = "0.5", optional = true }
libfuzzer-sys = { git = "https://github.com/rust-fuzz/libfuzzer-sys.git", optional = true }

[patch.crates-io]
# Rust-Secp256k1 PR 282. This patch should be dropped once that is merged.
secp256k1 = { git = 'https://github.com/TheBlueMatt/rust-secp256k1', rev = '32767e0e21e8861701ff7d5957613169d67ff1f8' }
# bitcoin_hashes PR 111 (without the top commit). This patch should be dropped once that is merged.
bitcoin_hashes = { git = 'https://github.com/TheBlueMatt/bitcoin_hashes', rev = 'c90d26339a3e34fd2f942aa80298f410cc41b743' }

[build-dependencies]
cc = "1.0"

Expand Down
86 changes: 43 additions & 43 deletions fuzz/src/full_stack.rs

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion lightning-block-sync/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ rest-client = [ "serde", "serde_json", "chunked_transfer" ]
rpc-client = [ "serde", "serde_json", "chunked_transfer" ]

[dependencies]
bitcoin = "0.24"
bitcoin = "0.26"
lightning = { version = "0.0.12", path = "../lightning" }
tokio = { version = "1.0", features = [ "io-util", "net" ], optional = true }
serde = { version = "1.0", features = ["derive"], optional = true }
Expand Down
2 changes: 1 addition & 1 deletion lightning-c-bindings/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ crate-type = ["staticlib"
,"cdylib"]

[dependencies]
bitcoin = "0.24"
bitcoin = "0.26"
lightning = { version = "0.0.12", path = "../lightning" }

# We eventually want to join the root workspace, but for now, the bindings generation is
Expand Down
3 changes: 2 additions & 1 deletion lightning-c-bindings/src/c_types/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,8 @@ pub enum Secp256k1Error {
InvalidSecretKey,
InvalidRecoveryId,
InvalidTweak,
TweakCheckFailed,
NotEnoughMemory,
CallbackPanicked,
}
impl Secp256k1Error {
pub(crate) fn from_rust(err: SecpError) -> Self {
Expand All @@ -85,6 +85,7 @@ impl Secp256k1Error {
SecpError::InvalidSecretKey => Secp256k1Error::InvalidSecretKey,
SecpError::InvalidRecoveryId => Secp256k1Error::InvalidRecoveryId,
SecpError::InvalidTweak => Secp256k1Error::InvalidTweak,
SecpError::TweakCheckFailed => Secp256k1Error::TweakCheckFailed,
SecpError::NotEnoughMemory => Secp256k1Error::NotEnoughMemory,
}
}
Expand Down
2 changes: 1 addition & 1 deletion lightning-net-tokio/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ For Rust-Lightning clients which wish to make direct connections to Lightning P2
"""

[dependencies]
bitcoin = "0.24"
bitcoin = "0.26"
lightning = { version = "0.0.12", path = "../lightning" }
tokio = { version = "1.0", features = [ "io-util", "macros", "rt", "sync", "net", "time" ] }

Expand Down
4 changes: 2 additions & 2 deletions lightning-persister/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,15 @@ Utilities to manage channel data persistence and retrieval.
"""

[dependencies]
bitcoin = "0.24"
bitcoin = "0.26"
lightning = { version = "0.0.12", path = "../lightning" }
libc = "0.2"

[target.'cfg(windows)'.dependencies]
winapi = { version = "0.3", features = ["winbase"] }

[dev-dependencies.bitcoin]
version = "0.24"
version = "0.26"
features = ["bitcoinconsensus"]

[dev-dependencies]
Expand Down
4 changes: 2 additions & 2 deletions lightning/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,13 @@ unsafe_revoked_tx_signing = []
unstable = []

[dependencies]
bitcoin = "0.24"
bitcoin = "0.26"

hex = { version = "0.3", optional = true }
regex = { version = "0.1.80", optional = true }

[dev-dependencies.bitcoin]
version = "0.24"
version = "0.26"
features = ["bitcoinconsensus"]

[dev-dependencies]
Expand Down
14 changes: 8 additions & 6 deletions lightning/src/chain/channelmonitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -951,7 +951,7 @@ impl<Signer: Sign> Writeable for ChannelMonitor<Signer> {
}

impl<Signer: Sign> ChannelMonitor<Signer> {
pub(crate) fn new(keys: Signer, shutdown_pubkey: &PublicKey,
pub(crate) fn new(secp_ctx: Secp256k1<secp256k1::All>, keys: Signer, shutdown_pubkey: &PublicKey,
on_counterparty_tx_csv: u16, destination_script: &Script, funding_info: (OutPoint, Script),
channel_parameters: &ChannelTransactionParameters,
funding_redeemscript: Script, channel_value_satoshis: u64,
Expand All @@ -972,8 +972,6 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
let channel_keys_id = keys.channel_keys_id();
let holder_revocation_basepoint = keys.pubkeys().revocation_basepoint;

let secp_ctx = Secp256k1::new();

// block for Rust 1.34 compat
let (holder_commitment_tx, current_holder_commitment_number) = {
let trusted_tx = initial_holder_commitment_tx.trust();
Expand All @@ -994,7 +992,8 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
};

let onchain_tx_handler =
OnchainTxHandler::new(destination_script.clone(), keys, channel_parameters.clone(), initial_holder_commitment_tx);
OnchainTxHandler::new(destination_script.clone(), keys,
channel_parameters.clone(), initial_holder_commitment_tx, secp_ctx.clone());

let mut outputs_to_watch = HashMap::new();
outputs_to_watch.insert(funding_info.0.txid, vec![(funding_info.0.index as u32, funding_info.1.clone())]);
Expand Down Expand Up @@ -2558,6 +2557,9 @@ impl<'a, Signer: Sign, K: KeysInterface<Signer = Signer>> ReadableArgs<&'a K>
let lockdown_from_offchain = Readable::read(reader)?;
let holder_tx_signed = Readable::read(reader)?;

let mut secp_ctx = Secp256k1::new();
secp_ctx.seeded_randomize(&keys_manager.get_secure_random_bytes());

Ok((last_block_hash.clone(), ChannelMonitor {
latest_update_id,
commitment_transaction_number_obscure_factor,
Expand Down Expand Up @@ -2603,7 +2605,7 @@ impl<'a, Signer: Sign, K: KeysInterface<Signer = Signer>> ReadableArgs<&'a K>
holder_tx_signed,

last_block_hash,
secp_ctx: Secp256k1::new(),
secp_ctx,
}))
}
}
Expand Down Expand Up @@ -2718,7 +2720,7 @@ mod tests {
};
// Prune with one old state and a holder commitment tx holding a few overlaps with the
// old state.
let mut monitor = ChannelMonitor::new(keys,
let mut monitor = ChannelMonitor::new(Secp256k1::new(), keys,
&PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()), 0, &Script::new(),
(OutPoint { txid: Txid::from_slice(&[43; 32]).unwrap(), index: 0 }, Script::new()),
&channel_parameters,
Expand Down
27 changes: 17 additions & 10 deletions lightning/src/chain/keysinterface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -744,8 +744,10 @@ pub struct KeysManager {
shutdown_pubkey: PublicKey,
channel_master_key: ExtendedPrivKey,
channel_child_index: AtomicUsize,

rand_bytes_master_key: ExtendedPrivKey,
rand_bytes_child_index: AtomicUsize,
rand_bytes_unique_start: Sha256State,

seed: [u8; 32],
starting_time_secs: u64,
Expand Down Expand Up @@ -794,31 +796,36 @@ impl KeysManager {
let channel_master_key = master_key.ckd_priv(&secp_ctx, ChildNumber::from_hardened_idx(3).unwrap()).expect("Your RNG is busted");
let rand_bytes_master_key = master_key.ckd_priv(&secp_ctx, ChildNumber::from_hardened_idx(4).unwrap()).expect("Your RNG is busted");

KeysManager {
let mut rand_bytes_unique_start = Sha256::engine();
rand_bytes_unique_start.input(&byte_utils::be64_to_array(starting_time_secs));
rand_bytes_unique_start.input(&byte_utils::be32_to_array(starting_time_nanos));
rand_bytes_unique_start.input(seed);

let mut res = KeysManager {
secp_ctx,
node_secret,

destination_script,
shutdown_pubkey,

channel_master_key,
channel_child_index: AtomicUsize::new(0),

rand_bytes_master_key,
rand_bytes_child_index: AtomicUsize::new(0),
rand_bytes_unique_start,

seed: *seed,
starting_time_secs,
starting_time_nanos,
}
};
let secp_seed = res.get_secure_random_bytes();
res.secp_ctx.seeded_randomize(&secp_seed);
res
},
Err(_) => panic!("Your rng is busted"),
}
}
fn derive_unique_start(&self) -> Sha256State {
let mut unique_start = Sha256::engine();
unique_start.input(&byte_utils::be64_to_array(self.starting_time_secs));
unique_start.input(&byte_utils::be32_to_array(self.starting_time_nanos));
unique_start.input(&self.seed);
unique_start
}
/// Derive an old set of Sign for per-channel secrets based on a key derivation
/// parameters.
/// Key derivation parameters are accessible through a per-channel secrets
Expand Down Expand Up @@ -1017,7 +1024,7 @@ impl KeysInterface for KeysManager {
}

fn get_secure_random_bytes(&self) -> [u8; 32] {
let mut sha = self.derive_unique_start();
let mut sha = self.rand_bytes_unique_start.clone();

let child_ix = self.rand_bytes_child_index.fetch_add(1, Ordering::AcqRel);
let child_privkey = self.rand_bytes_master_key.ckd_priv(&self.secp_ctx, ChildNumber::from_hardened_idx(child_ix as u32).expect("key space exhausted")).expect("Your RNG is busted");
Expand Down
2 changes: 1 addition & 1 deletion lightning/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
//! instead of having a rather-separate lightning appendage to a wallet.

#![cfg_attr(not(any(feature = "fuzztarget", feature = "_test_utils")), deny(missing_docs))]
#![forbid(unsafe_code)]
#![cfg_attr(not(any(test, feature = "fuzztarget", feature = "_test_utils")), forbid(unsafe_code))]

// In general, rust is absolutely horrid at supporting users doing things like,
// for example, compiling Rust code for real environments. Disable useless lints
Expand Down
22 changes: 16 additions & 6 deletions lightning/src/ln/channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -521,13 +521,16 @@ impl<Signer: Sign> Channel<Signer> {

let feerate = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Normal);

let mut secp_ctx = Secp256k1::new();
secp_ctx.seeded_randomize(&keys_provider.get_secure_random_bytes());

Ok(Channel {
user_id,
config: config.channel_options.clone(),

channel_id: keys_provider.get_secure_random_bytes(),
channel_state: ChannelState::OurInitSent as u32,
secp_ctx: Secp256k1::new(),
secp_ctx,
channel_value_satoshis,

latest_monitor_update_id: 0,
Expand Down Expand Up @@ -755,13 +758,16 @@ impl<Signer: Sign> Channel<Signer> {
}
} else { None };

let mut secp_ctx = Secp256k1::new();
secp_ctx.seeded_randomize(&keys_provider.get_secure_random_bytes());

let chan = Channel {
user_id,
config: local_config,

channel_id: msg.temporary_channel_id,
channel_state: (ChannelState::OurInitSent as u32) | (ChannelState::TheirInitSent as u32),
secp_ctx: Secp256k1::new(),
secp_ctx,

latest_monitor_update_id: 0,

Expand Down Expand Up @@ -1564,7 +1570,7 @@ impl<Signer: Sign> Channel<Signer> {
let funding_redeemscript = self.get_funding_redeemscript();
let funding_txo_script = funding_redeemscript.to_v0_p2wsh();
let obscure_factor = get_commitment_transaction_number_obscure_factor(&self.get_holder_pubkeys().payment_point, &self.get_counterparty_pubkeys().payment_point, self.is_outbound());
let mut channel_monitor = ChannelMonitor::new(self.holder_signer.clone(),
let mut channel_monitor = ChannelMonitor::new(self.secp_ctx.clone(), self.holder_signer.clone(),
&self.shutdown_pubkey, self.get_holder_selected_contest_delay(),
&self.destination_script, (funding_txo, funding_txo_script.clone()),
&self.channel_transaction_parameters,
Expand Down Expand Up @@ -1634,7 +1640,7 @@ impl<Signer: Sign> Channel<Signer> {
let funding_txo = self.get_funding_txo().unwrap();
let funding_txo_script = funding_redeemscript.to_v0_p2wsh();
let obscure_factor = get_commitment_transaction_number_obscure_factor(&self.get_holder_pubkeys().payment_point, &self.get_counterparty_pubkeys().payment_point, self.is_outbound());
let mut channel_monitor = ChannelMonitor::new(self.holder_signer.clone(),
let mut channel_monitor = ChannelMonitor::new(self.secp_ctx.clone(), self.holder_signer.clone(),
&self.shutdown_pubkey, self.get_holder_selected_contest_delay(),
&self.destination_script, (funding_txo, funding_txo_script),
&self.channel_transaction_parameters,
Expand Down Expand Up @@ -4081,7 +4087,8 @@ impl<Signer: Sign> Channel<Signer> {
signature = res.0;
htlc_signatures = res.1;

log_trace!(logger, "Signed remote commitment tx {} with redeemscript {} -> {}",
log_trace!(logger, "Signed remote commitment tx {} (txid {}) with redeemscript {} -> {}",
encode::serialize_hex(&counterparty_commitment_tx.0.trust().built_transaction().transaction),
&counterparty_commitment_txid,
encode::serialize_hex(&self.get_funding_redeemscript()),
log_bytes!(signature.serialize_compact()[..]));
Expand Down Expand Up @@ -4608,13 +4615,16 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel<Signer>
let counterparty_shutdown_scriptpubkey = Readable::read(reader)?;
let commitment_secrets = Readable::read(reader)?;

let mut secp_ctx = Secp256k1::new();
secp_ctx.seeded_randomize(&keys_source.get_secure_random_bytes());

Ok(Channel {
user_id,

config,
channel_id,
channel_state,
secp_ctx: Secp256k1::new(),
secp_ctx,
channel_value_satoshis,

latest_monitor_update_id,
Expand Down
8 changes: 6 additions & 2 deletions lightning/src/ln/channelmanager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -766,7 +766,8 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
/// Users need to notify the new ChannelManager when a new block is connected or
/// disconnected using its `block_connected` and `block_disconnected` methods.
pub fn new(network: Network, fee_est: F, chain_monitor: M, tx_broadcaster: T, logger: L, keys_manager: K, config: UserConfig, current_blockchain_height: usize) -> Self {
let secp_ctx = Secp256k1::new();
let mut secp_ctx = Secp256k1::new();
secp_ctx.seeded_randomize(&keys_manager.get_secure_random_bytes());

ChannelManager {
default_configuration: config.clone(),
Expand Down Expand Up @@ -4129,6 +4130,9 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>

let last_node_announcement_serial: u32 = Readable::read(reader)?;

let mut secp_ctx = Secp256k1::new();
secp_ctx.seeded_randomize(&args.keys_manager.get_secure_random_bytes());

let channel_manager = ChannelManager {
genesis_hash,
fee_estimator: args.fee_estimator,
Expand All @@ -4137,7 +4141,7 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>

latest_block_height: AtomicUsize::new(latest_block_height as usize),
last_block_hash: Mutex::new(last_block_hash),
secp_ctx: Secp256k1::new(),
secp_ctx,

channel_state: Mutex::new(ChannelHolder {
by_id,
Expand Down
9 changes: 6 additions & 3 deletions lightning/src/ln/onchaintx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,9 @@ impl<'a, K: KeysInterface> ReadableArgs<&'a K> for OnchainTxHandler<K::Signer> {
}
let latest_height = Readable::read(reader)?;

let mut secp_ctx = Secp256k1::new();
secp_ctx.seeded_randomize(&keys_manager.get_secure_random_bytes());

Ok(OnchainTxHandler {
destination_script,
holder_commitment,
Expand All @@ -418,13 +421,13 @@ impl<'a, K: KeysInterface> ReadableArgs<&'a K> for OnchainTxHandler<K::Signer> {
pending_claim_requests,
onchain_events_waiting_threshold_conf,
latest_height,
secp_ctx: Secp256k1::new(),
secp_ctx,
})
}
}

impl<ChannelSigner: Sign> OnchainTxHandler<ChannelSigner> {
pub(crate) fn new(destination_script: Script, signer: ChannelSigner, channel_parameters: ChannelTransactionParameters, holder_commitment: HolderCommitmentTransaction) -> Self {
pub(crate) fn new(destination_script: Script, signer: ChannelSigner, channel_parameters: ChannelTransactionParameters, holder_commitment: HolderCommitmentTransaction, secp_ctx: Secp256k1<secp256k1::All>) -> Self {
OnchainTxHandler {
destination_script,
holder_commitment,
Expand All @@ -438,7 +441,7 @@ impl<ChannelSigner: Sign> OnchainTxHandler<ChannelSigner> {
onchain_events_waiting_threshold_conf: HashMap::new(),
latest_height: 0,

secp_ctx: Secp256k1::new(),
secp_ctx,
}
}

Expand Down
Loading