refactor: slightly improve security for secret keys

common/config-parser/src/types/config.rs

@@ -4,7 +4,10 @@ use clap::builder::{StringValueParser, TypedValueParser, ValueParserFactory};
 use serde::Deserialize;
 use tentacle_multiaddr::MultiAddr;
 
-use protocol::types::{Hex, H160};
+use protocol::{
+    codec::deserialize_256bits_key,
+    types::{Key256Bits, H160},
+};
 
 use crate::parse_file;
 
@@ -20,7 +23,8 @@ pub const DEFAULT_CACHE_SIZE: usize = 100;
 #[derive(Clone, Debug, Deserialize)]
 pub struct Config {
     // crypto
-    pub privkey:   Privkey,
+    #[serde(deserialize_with = "deserialize_256bits_key")]
+    pub privkey:   Key256Bits,
     // db config
     pub data_path: PathBuf,
 
@@ -258,76 +262,3 @@ fn default_max_gas_cap() -> u64 {
 fn default_log_filter_max_block_range() -> u64 {
     10_000
 }
-
-#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)]
-pub struct Privkey(Hex);
-
-impl Privkey {
-    pub fn as_string_trim0x(&self) -> String {
-        self.0.as_string_trim0x()
-    }
-
-    pub fn inner(self) -> Hex {
-        self.0
-    }
-}
-
-impl serde::Serialize for Privkey {
-    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
-    where
-        S: serde::ser::Serializer,
-    {
-        self.0.serialize(serializer)
-    }
-}
-
-impl<'de> serde::Deserialize<'de> for Privkey {
-    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
-    where
-        D: serde::de::Deserializer<'de>,
-    {
-        struct PVisitor;
-
-        impl<'de> serde::de::Visitor<'de> for PVisitor {
-            type Value = Privkey;
-
-            fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result {
-                formatter.write_str("Expect a hex string")
-            }
-
-            fn visit_string<E>(self, v: String) -> Result<Self::Value, E>
-            where
-                E: serde::de::Error,
-            {
-                match Hex::from_string(v.clone()) {
-                    Ok(v) => Ok(Privkey(v)),
-                    Err(_) => {
-                        let key = std::env::var(v)
-                            .map_err(|e| serde::de::Error::custom(e.to_string()))?;
-                        Hex::from_string(key)
-                            .map(Privkey)
-                            .map_err(|e| serde::de::Error::custom(e.to_string()))
-                    }
-                }
-            }
-
-            fn visit_str<E>(self, v: &str) -> Result<Self::Value, E>
-            where
-                E: serde::de::Error,
-            {
-                match Hex::from_string(v.to_string()) {
-                    Ok(v) => Ok(Privkey(v)),
-                    Err(_) => {
-                        let key = std::env::var(v)
-                            .map_err(|e| serde::de::Error::custom(e.to_string()))?;
-                        Hex::from_string(key)
-                            .map(Privkey)
-                            .map_err(|e| serde::de::Error::custom(e.to_string()))
-                    }
-                }
-            }
-        }
-
-        deserializer.deserialize_string(PVisitor)
-    }
-}

core/consensus/src/consensus.rs

@@ -13,6 +13,7 @@ use protocol::{
 
 use common_apm::tracing::{AxonTracer, Tag};
 use common_apm_derive::trace_span;
+use common_crypto::PublicKey as _;
 
 use crate::wal::{ConsensusWal, SignedTxsWAL};
 use crate::{
@@ -124,7 +125,12 @@ impl<Adapter: ConsensusAdapter + 'static> OverlordConsensus<Adapter> {
             .await
             .unwrap();
 
-        let overlord = Overlord::new(node_info.self_pub_key, Arc::clone(&engine), crypto, engine);
+        let overlord = Overlord::new(
+            node_info.self_pub_key.to_bytes(),
+            Arc::clone(&engine),
+            crypto,
+            engine,
+        );
         let overlord_handler = overlord.get_handler();
 
         if status.last_number == 0 {

core/network/examples/simple.rs

@@ -2,9 +2,7 @@
 
 use core_network::{NetworkConfig, NetworkService, NetworkServiceHandle};
 use protocol::{
-    async_trait,
-    codec::hex_encode,
-    tokio,
+    async_trait, tokio,
     traits::{Context, Gossip, MessageHandler, Priority, Rpc, TrustFeedback},
     types::Bytes,
     ProtocolError,
@@ -71,17 +69,17 @@ impl MessageHandler for Checkout {
 async fn main() {
     env_logger::init();
 
-    let bt_seckey_bytes = "8".repeat(32);
-    let bt_seckey = hex_encode(&bt_seckey_bytes);
-    let bt_keypair = SecioKeyPair::secp256k1_raw_key(bt_seckey_bytes).expect("keypair");
+    let bt_seckey = [8u8; 32];
+    let bt_keypair = SecioKeyPair::secp256k1_raw_key(bt_seckey).expect("keypair");
     let peer_id = bt_keypair.peer_id().to_base58();
 
     if std::env::args().nth(1) == Some("server".to_string()) {
         log::info!("Starting server");
 
         let bt_conf = NetworkConfig::new()
             .listen_addr("/ip4/127.0.0.1/tcp/1337".parse().unwrap())
-            .secio_keypair(&bt_seckey);
+            .secio_keypair(&bt_seckey)
+            .unwrap();
         let mut bootstrap = NetworkService::new(bt_conf, bt_keypair);
         let handle = bootstrap.handle();
 

core/network/src/config.rs

@@ -10,7 +10,7 @@ use tentacle::{
 };
 
 use common_config_parser::types::Config;
-use protocol::{codec::hex_decode, ProtocolResult};
+use protocol::{ProtocolError, ProtocolErrorKind, ProtocolResult};
 
 use crate::error::NetworkError;
 
@@ -114,7 +114,7 @@ impl NetworkConfig {
                     .collect(),
             )
             .listen_addr(config.network.listening_address.clone())
-            .secio_keypair(&config.privkey.as_string_trim0x())
+            .secio_keypair(config.privkey.as_ref())?
             .chain_id(chain_id)
             .max_connections(config.network.max_connected_peers)
     }
@@ -163,14 +163,11 @@ impl NetworkConfig {
         self
     }
 
-    pub fn secio_keypair(mut self, sk_hex: &str) -> Self {
-        let skp = hex_decode(sk_hex)
-            .map(SecioKeyPair::secp256k1_raw_key)
-            .unwrap()
-            .unwrap();
+    pub fn secio_keypair(mut self, sk_hex: &[u8]) -> ProtocolResult<Self> {
+        let skp = SecioKeyPair::secp256k1_raw_key(sk_hex)
+            .map_err(|err| ProtocolError::new(ProtocolErrorKind::Network, Box::new(err)))?;
         self.secio_keypair = skp;
-
-        self
+        Ok(self)
     }
 
     pub fn ping_interval(mut self, interval: Option<u64>) -> Self {

core/run/src/lib.rs

@@ -17,10 +17,7 @@ use common_apm::metrics::mempool::{MEMPOOL_CO_QUEUE_LEN, MEMPOOL_LEN_GAUGE};
 use common_apm::{server::run_prometheus_server, tracing::global_tracer_register};
 use common_config_parser::types::spec::{ChainSpec, InitialAccount};
 use common_config_parser::types::{Config, ConfigJaeger, ConfigPrometheus};
-use common_crypto::{
-    BlsPrivateKey, BlsPublicKey, PublicKey, Secp256k1, Secp256k1PrivateKey, Secp256k1PublicKey,
-    ToPublicKey, UncompressedPublicKey,
-};
+use common_crypto::{BlsPrivateKey, BlsPublicKey, Secp256k1, Secp256k1PrivateKey, ToPublicKey};
 
 use protocol::codec::{hex_decode, ProtocolCodec};
 #[cfg(unix)]
@@ -33,8 +30,8 @@ use protocol::traits::{
     Rpc, Storage, SynchronizationAdapter,
 };
 use protocol::types::{
-    Account, Address, Block, ExecResp, MerkleRoot, Metadata, Proposal, RichBlock,
-    SignedTransaction, Validator, ValidatorExtend, H256, NIL_DATA, RLP_NULL,
+    Account, Block, ExecResp, MerkleRoot, Metadata, Proposal, RichBlock, SignedTransaction,
+    Validator, ValidatorExtend, H256, NIL_DATA, RLP_NULL,
 };
 use protocol::{
     async_trait, lazy::CHAIN_ID, trie::DB as TrieDB, Display, From, ProtocolError,
@@ -515,28 +512,9 @@ impl Axon {
         system_contract::init(inner_db, &mut backend)
     }
 
-    fn get_my_pubkey(&self, hex_privkey: Vec<u8>) -> Secp256k1PublicKey {
-        let my_privkey = Secp256k1PrivateKey::try_from(hex_privkey.as_ref())
-            .map_err(MainError::Crypto)
-            .unwrap();
-
-        my_privkey.pub_key()
-    }
-
-    fn get_my_address(&self, hex_privkey: Vec<u8>) -> Address {
-        let my_pubkey = self.get_my_pubkey(hex_privkey);
-
-        Address::from_pubkey_bytes(my_pubkey.to_uncompressed_bytes()).unwrap()
-    }
-
-    fn get_my_hex_privkey(&self) -> Vec<u8> {
-        hex_decode(&self.config.privkey.as_string_trim0x()).unwrap()
-    }
-
     fn init_crypto(&self, validators: &[ValidatorExtend]) -> Arc<OverlordCrypto> {
         // self private key
-        let hex_privkey = self.get_my_hex_privkey();
-        let bls_priv_key = BlsPrivateKey::try_from(hex_privkey.as_ref())
+        let bls_priv_key = BlsPrivateKey::try_from(self.config.privkey.as_ref())
             .map_err(MainError::Crypto)
             .unwrap();
 
@@ -605,12 +583,10 @@ impl Axon {
             .to_string();
         let consensus_wal = Arc::new(ConsensusWal::new(consensus_wal_path));
 
-        let hex_privkey = self.get_my_hex_privkey();
-        let node_info = NodeInfo {
-            chain_id:     self.genesis.block.header.chain_id,
-            self_address: self.get_my_address(hex_privkey.clone()),
-            self_pub_key: self.get_my_pubkey(hex_privkey).to_bytes(),
-        };
+        let my_privkey = Secp256k1PrivateKey::try_from(self.config.privkey.as_ref())
+            .map_err(MainError::Crypto)
+            .unwrap();
+        let node_info = NodeInfo::new(self.genesis.block.header.chain_id, my_privkey.pub_key());
 
         Arc::new(
             OverlordConsensus::new(

devtools/genesis-generator/src/main.rs

@@ -121,7 +121,7 @@ fn get_metadata(config_path: String) -> (Metadata, Metadata) {
         .map(|file_name| {
             let config: Config = parse_file(file_name, false).unwrap();
             let priv_key = config.privkey;
-            get_ve(priv_key.inner(), propose_weight, vote_weight)
+            get_ve(Hex::encode(priv_key.as_ref()), propose_weight, vote_weight)
         })
         .collect::<Vec<_>>();
 

protocol/Cargo.toml

@@ -32,6 +32,7 @@ serde = { version = "1.0", features = ["derive"] }
 thiserror = "1.0"
 tokio = { version = "1.32", features = ["full"] }
 trie = { package = "cita_trie", git = "https://github.com/KaoImin/cita-trie.git", rev = "eea569c", version = "4.1" }
+zeroize = "1.6.0"
 
 common-crypto = { path = "../common/crypto" }
 common-hasher = { path = "../common/hasher" }

protocol/src/codec/mod.rs

@@ -10,7 +10,7 @@ use ethers_core::utils::parse_checksummed;
 use rlp::{Decodable, DecoderError, Encodable, Rlp, RlpStream};
 use serde::{Deserialize as _, Deserializer, Serializer};
 
-use crate::types::{Address, Bytes, DBBytes, Hex, TypesError, H160, U256};
+use crate::types::{Address, Bytes, DBBytes, Hex, Key256Bits, TypesError, H160, U256};
 use crate::ProtocolResult;
 
 static CHARS: &[u8] = b"0123456789abcdef";
@@ -115,6 +115,35 @@ where
     }
 }
 
+pub fn deserialize_256bits_key<'de, D>(deserializer: D) -> Result<Key256Bits, D::Error>
+where
+    D: Deserializer<'de>,
+{
+    const LEN: usize = 66;
+    let s = String::deserialize(deserializer)?;
+    if s.starts_with("0x") || s.starts_with("0X") {
+        if s.len() == LEN {
+            let slice = &s.as_bytes()[2..];
+            let mut v = [0u8; 32];
+            faster_hex::hex_decode(slice, &mut v)
+                .map(|_| Key256Bits::from(v))
+                .map_err(|err| {
+                    let msg = format!("failed to parse the 256 bits key since {err}.");
+                    serde::de::Error::custom(msg)
+                })
+        } else {
+            let msg = format!(
+                "failed to parse the 256 bits key since its length is {} but expect {LEN}.",
+                s.len()
+            );
+            Err(serde::de::Error::custom(msg))
+        }
+    } else {
+        let msg = "failed to parse the 256 bits key since it's not 0x-prefixed.";
+        Err(serde::de::Error::custom(msg))
+    }
+}
+
 pub fn deserialize_address<'de, D>(deserializer: D) -> Result<H160, D::Error>
 where
     D: Deserializer<'de>,

protocol/src/traits/consensus.rs

@@ -1,5 +1,7 @@
 use std::collections::HashMap;
 
+use ophelia_secp256k1::Secp256k1PublicKey;
+
 use crate::types::{
     Address, Block, BlockNumber, Bytes, ExecResp, Hash, Header, Hex, MerkleRoot, Metadata,
     PackedTxHashes, Proof, Proposal, Receipt, SignedTransaction, Validator, U256,
@@ -15,10 +17,21 @@ pub enum MessageTarget {
 #[derive(Debug, Clone)]
 pub struct NodeInfo {
     pub chain_id:     u64,
-    pub self_pub_key: Bytes,
+    pub self_pub_key: Secp256k1PublicKey,
     pub self_address: Address,
 }
 
+impl NodeInfo {
+    pub fn new(chain_id: u64, pubkey: Secp256k1PublicKey) -> Self {
+        let address = Address::from_pubkey(&pubkey);
+        Self {
+            chain_id,
+            self_pub_key: pubkey,
+            self_address: address,
+        }
+    }
+}
+
 #[async_trait]
 pub trait Consensus: Send + Sync {
     /// Network set a received signed proposal to consensus.

protocol/src/types/primitive.rs

@@ -5,6 +5,7 @@ pub use ethereum_types::{
     BigEndianHash, Bloom, Public, Secret, Signature, H128, H160, H256, H512, H520, H64, U128, U256,
     U512, U64,
 };
+use zeroize::Zeroizing;
 
 use ophelia::{PublicKey, UncompressedPublicKey};
 use ophelia_secp256k1::Secp256k1PublicKey;
@@ -174,6 +175,8 @@ impl<'de> Deserialize<'de> for Hex {
     }
 }
 
+pub type Key256Bits = Zeroizing<[u8; 32]>;
+
 #[derive(Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
 pub struct Address(pub H160);
 
@@ -250,6 +253,11 @@ impl Address {
         Ok(Self::from_hash(hash))
     }
 
+    pub fn from_pubkey(pubkey: &Secp256k1PublicKey) -> Self {
+        let hash = Hasher::digest(&(pubkey.to_uncompressed_bytes())[1..]);
+        Self::from_hash(hash)
+    }
+
     pub fn from_hash(hash: Hash) -> Self {
         Self(H160::from_slice(&hash.as_bytes()[12..]))
     }