From f98bfda6f90692d6055253805b10aa950634aaf2 Mon Sep 17 00:00:00 2001 From: sakridge Date: Fri, 8 May 2020 10:00:23 -0700 Subject: [PATCH] Security changes (#9923) * Move test-only functions to test modules * Remove sigverify disable * Remove chacha CTR code --- chacha-sys/cpu-crypt/chacha.h | 4 -- chacha-sys/cpu-crypt/chacha20_core.c | 53 -------------------------- core/src/cluster_info_vote_listener.rs | 18 +++------ core/src/storage_stage.rs | 32 ++++++---------- core/src/tpu.rs | 9 +---- core/src/tvu.rs | 21 +++------- core/src/validator.rs | 8 ---- core/tests/storage_stage.rs | 16 ++++++-- validator/src/main.rs | 7 ---- 9 files changed, 36 insertions(+), 132 deletions(-) diff --git a/chacha-sys/cpu-crypt/chacha.h b/chacha-sys/cpu-crypt/chacha.h index ca3fc31a6..3a9c7b98f 100644 --- a/chacha-sys/cpu-crypt/chacha.h +++ b/chacha-sys/cpu-crypt/chacha.h @@ -19,10 +19,6 @@ void chacha20_encrypt(const u32 input[16], unsigned char output[64], int num_rounds); -void chacha20_encrypt_ctr(const uint8_t *in, uint8_t *out, size_t in_len, - const uint8_t key[CHACHA_KEY_SIZE], const uint8_t nonce[CHACHA_NONCE_SIZE], - uint32_t counter); - void chacha20_cbc128_encrypt(const unsigned char* in, unsigned char* out, uint32_t len, const uint8_t* key, unsigned char* ivec); diff --git a/chacha-sys/cpu-crypt/chacha20_core.c b/chacha-sys/cpu-crypt/chacha20_core.c index 7360ff050..1c914c735 100644 --- a/chacha-sys/cpu-crypt/chacha20_core.c +++ b/chacha-sys/cpu-crypt/chacha20_core.c @@ -22,10 +22,6 @@ x[a] = PLUS(x[a],x[b]); x[d] = ROTATE(XOR(x[d],x[a]), 8); \ x[c] = PLUS(x[c],x[d]); x[b] = ROTATE(XOR(x[b],x[c]), 7); -// sigma contains the ChaCha constants, which happen to be an ASCII string. -static const uint8_t sigma[16] = { 'e', 'x', 'p', 'a', 'n', 'd', ' ', '3', - '2', '-', 'b', 'y', 't', 'e', ' ', 'k' }; - void chacha20_encrypt(const u32 input[16], unsigned char output[64], int num_rounds) @@ -51,52 +47,3 @@ void chacha20_encrypt(const u32 input[16], } } -void chacha20_encrypt_ctr(const uint8_t *in, uint8_t *out, size_t in_len, - const uint8_t key[CHACHA_KEY_SIZE], - const uint8_t nonce[CHACHA_NONCE_SIZE], - uint32_t counter) -{ - uint32_t input[16]; - uint8_t buf[64]; - size_t todo, i; - - input[0] = U8TO32_LITTLE(sigma + 0); - input[1] = U8TO32_LITTLE(sigma + 4); - input[2] = U8TO32_LITTLE(sigma + 8); - input[3] = U8TO32_LITTLE(sigma + 12); - - input[4] = U8TO32_LITTLE(key + 0); - input[5] = U8TO32_LITTLE(key + 4); - input[6] = U8TO32_LITTLE(key + 8); - input[7] = U8TO32_LITTLE(key + 12); - - input[8] = U8TO32_LITTLE(key + 16); - input[9] = U8TO32_LITTLE(key + 20); - input[10] = U8TO32_LITTLE(key + 24); - input[11] = U8TO32_LITTLE(key + 28); - - input[12] = counter; - input[13] = U8TO32_LITTLE(nonce + 0); - input[14] = U8TO32_LITTLE(nonce + 4); - input[15] = U8TO32_LITTLE(nonce + 8); - - while (in_len > 0) { - todo = sizeof(buf); - if (in_len < todo) { - todo = in_len; - } - - chacha20_encrypt(input, buf, 20); - for (i = 0; i < todo; i++) { - out[i] = in[i] ^ buf[i]; - } - - out += todo; - in += todo; - in_len -= todo; - - input[12]++; - } -} - - diff --git a/core/src/cluster_info_vote_listener.rs b/core/src/cluster_info_vote_listener.rs index e35ce8778..93c2dded0 100644 --- a/core/src/cluster_info_vote_listener.rs +++ b/core/src/cluster_info_vote_listener.rs @@ -198,7 +198,6 @@ impl ClusterInfoVoteListener { pub fn new( exit: &Arc, cluster_info: Arc, - sigverify_disabled: bool, sender: CrossbeamSender>, poh_recorder: &Arc>, vote_tracker: Arc, @@ -214,7 +213,6 @@ impl ClusterInfoVoteListener { let _ = Self::recv_loop( exit_, &cluster_info, - sigverify_disabled, verified_vote_packets_sender, verified_vote_transactions_sender, ); @@ -263,7 +261,6 @@ impl ClusterInfoVoteListener { fn recv_loop( exit: Arc, cluster_info: &ClusterInfo, - sigverify_disabled: bool, verified_vote_packets_sender: VerifiedVotePacketsSender, verified_vote_transactions_sender: VerifiedVoteTransactionsSender, ) -> Result<()> { @@ -277,7 +274,7 @@ impl ClusterInfoVoteListener { last_ts = new_ts; if !votes.is_empty() { - let (vote_txs, packets) = Self::verify_votes(votes, labels, sigverify_disabled); + let (vote_txs, packets) = Self::verify_votes(votes, labels); verified_vote_transactions_sender.send(vote_txs)?; verified_vote_packets_sender.send(packets)?; } @@ -289,14 +286,9 @@ impl ClusterInfoVoteListener { fn verify_votes( votes: Vec, labels: Vec, - sigverify_disabled: bool, ) -> (Vec, Vec<(CrdsValueLabel, Packets)>) { let msgs = packet::to_packets_chunked(&votes, 1); - let r = if sigverify_disabled { - sigverify::ed25519_verify_disabled(&msgs) - } else { - sigverify::ed25519_verify_cpu(&msgs) - }; + let r = sigverify::ed25519_verify_cpu(&msgs); assert_eq!( r.iter() @@ -986,7 +978,7 @@ mod tests { solana_logger::setup(); let votes = vec![]; let labels = vec![]; - let (vote_txs, packets) = ClusterInfoVoteListener::verify_votes(votes, labels, false); + let (vote_txs, packets) = ClusterInfoVoteListener::verify_votes(votes, labels); assert!(vote_txs.is_empty()); assert!(packets.is_empty()); } @@ -1017,7 +1009,7 @@ mod tests { let vote_tx = test_vote_tx(); let votes = vec![vote_tx.clone()]; let labels = vec![CrdsValueLabel::Vote(0, Pubkey::new_rand())]; - let (vote_txs, packets) = ClusterInfoVoteListener::verify_votes(votes, labels, false); + let (vote_txs, packets) = ClusterInfoVoteListener::verify_votes(votes, labels); assert_eq!(vote_txs.len(), 1); verify_packets_len(&packets, 1); } @@ -1033,7 +1025,7 @@ mod tests { .into_iter() .map(|_| label.clone()) .collect(); - let (vote_txs, packets) = ClusterInfoVoteListener::verify_votes(votes, labels, false); + let (vote_txs, packets) = ClusterInfoVoteListener::verify_votes(votes, labels); assert_eq!(vote_txs.len(), 2); verify_packets_len(&packets, 2); } diff --git a/core/src/storage_stage.rs b/core/src/storage_stage.rs index f02dea7f3..a637797f4 100644 --- a/core/src/storage_stage.rs +++ b/core/src/storage_stage.rs @@ -51,7 +51,7 @@ type ArchiverMap = Vec>>; #[derive(Default)] pub struct StorageStateInner { - storage_results: StorageResults, + pub storage_results: StorageResults, pub storage_keys: StorageKeys, archiver_map: ArchiverMap, storage_blockhash: Hash, @@ -86,16 +86,6 @@ const KEY_SIZE: usize = 64; type InstructionSender = Sender; -fn get_identity_index_from_signature(key: &Signature) -> usize { - let rkey = key.as_ref(); - let mut res: usize = (rkey[0] as usize) - | ((rkey[1] as usize) << 8) - | ((rkey[2] as usize) << 16) - | ((rkey[3] as usize) << 24); - res &= NUM_IDENTITIES - 1; - res -} - impl StorageState { pub fn new(hash: &Hash, slots_per_turn: u64, slots_per_segment: u64) -> Self { let storage_keys = vec![0u8; KEY_SIZE * NUM_IDENTITIES]; @@ -117,16 +107,6 @@ impl StorageState { } } - pub fn get_mining_key(&self, key: &Signature) -> Vec { - let idx = get_identity_index_from_signature(key); - self.state.read().unwrap().storage_keys[idx..idx + KEY_SIZE].to_vec() - } - - pub fn get_mining_result(&self, key: &Signature) -> Hash { - let idx = get_identity_index_from_signature(key); - self.state.read().unwrap().storage_results[idx] - } - pub fn get_storage_blockhash(&self) -> Hash { self.state.read().unwrap().storage_blockhash } @@ -658,6 +638,16 @@ pub fn test_cluster_info(id: &Pubkey) -> Arc { Arc::new(cluster_info) } +pub fn get_identity_index_from_signature(key: &Signature) -> usize { + let rkey = key.as_ref(); + let mut res: usize = (rkey[0] as usize) + | ((rkey[1] as usize) << 8) + | ((rkey[2] as usize) << 16) + | ((rkey[3] as usize) << 24); + res &= NUM_IDENTITIES - 1; + res +} + #[cfg(test)] mod tests { use super::*; diff --git a/core/src/tpu.rs b/core/src/tpu.rs index 2d484e054..35699d963 100644 --- a/core/src/tpu.rs +++ b/core/src/tpu.rs @@ -9,7 +9,7 @@ use crate::{ fetch_stage::FetchStage, poh_recorder::{PohRecorder, WorkingBankEntry}, sigverify::TransactionSigVerifier, - sigverify_stage::{DisabledSigVerifier, SigVerifyStage}, + sigverify_stage::SigVerifyStage, }; use crossbeam_channel::unbounded; use solana_ledger::{ @@ -43,7 +43,6 @@ impl Tpu { transactions_sockets: Vec, tpu_forwards_sockets: Vec, broadcast_sockets: Vec, - sigverify_disabled: bool, transaction_status_sender: Option, blockstore: &Arc, broadcast_type: &BroadcastStageType, @@ -62,19 +61,15 @@ impl Tpu { ); let (verified_sender, verified_receiver) = unbounded(); - let sigverify_stage = if !sigverify_disabled { + let sigverify_stage = { let verifier = TransactionSigVerifier::default(); SigVerifyStage::new(packet_receiver, verified_sender, verifier) - } else { - let verifier = DisabledSigVerifier::default(); - SigVerifyStage::new(packet_receiver, verified_sender, verifier) }; let (verified_vote_sender, verified_vote_receiver) = unbounded(); let cluster_info_vote_listener = ClusterInfoVoteListener::new( &exit, cluster_info.clone(), - sigverify_disabled, verified_vote_sender, &poh_recorder, vote_tracker, diff --git a/core/src/tvu.rs b/core/src/tvu.rs index 1d9b7d4a9..4e92639da 100644 --- a/core/src/tvu.rs +++ b/core/src/tvu.rs @@ -17,7 +17,7 @@ use crate::{ rpc_subscriptions::RpcSubscriptions, shred_fetch_stage::ShredFetchStage, sigverify_shreds::ShredSigVerifier, - sigverify_stage::{DisabledSigVerifier, SigVerifyStage}, + sigverify_stage::SigVerifyStage, storage_stage::{StorageStage, StorageState}, }; use crossbeam_channel::unbounded; @@ -64,7 +64,6 @@ pub struct Sockets { #[derive(Default)] pub struct TvuConfig { pub max_ledger_shreds: Option, - pub sigverify_disabled: bool, pub shred_version: u16, pub halt_on_trusted_validators_accounts_hash_mismatch: bool, pub trusted_validators: Option>, @@ -128,19 +127,11 @@ impl Tvu { ); let (verified_sender, verified_receiver) = unbounded(); - let sigverify_stage = if !tvu_config.sigverify_disabled { - SigVerifyStage::new( - fetch_receiver, - verified_sender, - ShredSigVerifier::new(bank_forks.clone(), leader_schedule_cache.clone()), - ) - } else { - SigVerifyStage::new( - fetch_receiver, - verified_sender, - DisabledSigVerifier::default(), - ) - }; + let sigverify_stage = SigVerifyStage::new( + fetch_receiver, + verified_sender, + ShredSigVerifier::new(bank_forks.clone(), leader_schedule_cache.clone()), + ); let cluster_slots = Arc::new(ClusterSlots::default()); let (duplicate_slots_reset_sender, duplicate_slots_reset_receiver) = unbounded(); diff --git a/core/src/validator.rs b/core/src/validator.rs index 4f55b6bb8..8776a06c5 100644 --- a/core/src/validator.rs +++ b/core/src/validator.rs @@ -59,7 +59,6 @@ use std::{ #[derive(Clone, Debug)] pub struct ValidatorConfig { - pub dev_sigverify_disabled: bool, pub dev_halt_at_slot: Option, pub expected_genesis_hash: Option, pub expected_shred_version: Option, @@ -87,7 +86,6 @@ pub struct ValidatorConfig { impl Default for ValidatorConfig { fn default() -> Self { Self { - dev_sigverify_disabled: false, dev_halt_at_slot: None, expected_genesis_hash: None, expected_shred_version: None, @@ -441,7 +439,6 @@ impl Validator { retransmit_slots_sender, TvuConfig { max_ledger_shreds: config.max_ledger_shreds, - sigverify_disabled: config.dev_sigverify_disabled, halt_on_trusted_validators_accounts_hash_mismatch: config .halt_on_trusted_validators_accounts_hash_mismatch, shred_version: node.info.shred_version, @@ -450,10 +447,6 @@ impl Validator { }, ); - if config.dev_sigverify_disabled { - warn!("signature verification disabled"); - } - let tpu = Tpu::new( &cluster_info, &poh_recorder, @@ -462,7 +455,6 @@ impl Validator { node.sockets.tpu, node.sockets.tpu_forwards, node.sockets.broadcast, - config.dev_sigverify_disabled, transaction_status_sender, &blockstore, &config.broadcast_stage_type, diff --git a/core/tests/storage_stage.rs b/core/tests/storage_stage.rs index 995c84b61..4594aa76a 100644 --- a/core/tests/storage_stage.rs +++ b/core/tests/storage_stage.rs @@ -5,7 +5,10 @@ mod tests { use log::*; use solana_core::{ commitment::BlockCommitmentCache, - storage_stage::{test_cluster_info, StorageStage, StorageState, SLOTS_PER_TURN_TEST}, + storage_stage::{ + get_identity_index_from_signature, test_cluster_info, StorageStage, StorageState, + SLOTS_PER_TURN_TEST, + }, }; use solana_ledger::{ bank_forks::BankForks, @@ -19,7 +22,7 @@ mod tests { hash::Hash, message::Message, pubkey::Pubkey, - signature::{Keypair, Signer}, + signature::{Keypair, Signature, Signer}, transaction::Transaction, }; use solana_storage_program::storage_instruction::{self, StorageAccountType}; @@ -34,6 +37,11 @@ mod tests { time::Duration, }; + fn get_mining_result(storage_state: &StorageState, key: &Signature) -> Hash { + let idx = get_identity_index_from_signature(key); + storage_state.state.read().unwrap().storage_results[idx] + } + #[test] fn test_storage_stage_process_account_proofs() { solana_logger::setup(); @@ -205,7 +213,7 @@ mod tests { let hash = Hash::default(); let signature = keypair.sign_message(&hash.as_ref()); - let mut result = storage_state.get_mining_result(&signature); + let mut result = get_mining_result(&storage_state, &signature); assert_eq!(result, Hash::default()); @@ -228,7 +236,7 @@ mod tests { if solana_perf::perf_libs::api().is_some() { for _ in 0..5 { - result = storage_state.get_mining_result(&signature); + result = get_mining_result(&storage_state, &signature); if result != Hash::default() { info!("found result = {:?} sleeping..", result); break; diff --git a/validator/src/main.rs b/validator/src/main.rs index c9e547678..edf69de7a 100644 --- a/validator/src/main.rs +++ b/validator/src/main.rs @@ -572,12 +572,6 @@ pub fn main() { .requires("entrypoint") .help("Skip the RPC vote account sanity check") ) - .arg( - Arg::with_name("dev_no_sigverify") - .long("dev-no-sigverify") - .takes_value(false) - .help("Run without signature verification"), - ) .arg( Arg::with_name("dev_halt_at_slot") .long("dev-halt-at-slot") @@ -867,7 +861,6 @@ pub fn main() { }; let mut validator_config = ValidatorConfig { - dev_sigverify_disabled: matches.is_present("dev_no_sigverify"), dev_halt_at_slot: value_t!(matches, "dev_halt_at_slot", Slot).ok(), expected_genesis_hash: matches .value_of("expected_genesis_hash")