tendermint · tarcieri · Mar 13, 2019 · Mar 13, 2019 · tarcieri · Mar 13, 2019
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -40,6 +40,7 @@ signatory = { version = "0.11.1", features = ["ed25519", "ecdsa"] }
 signatory-dalek = "0.11"
 signatory-secp256k1 = "0.11"
 signatory-ledger-tm = { version = "0.11", optional = true }
+subtle = "2"
 subtle-encoding = { version = "0.3", features = ["bech32-preview"] }
 tendermint = { version = "0.5.0-alpha1", path = "tendermint-rs" }
 tiny-bip39 = "0.6"

diff --git a/src/client.rs b/src/client.rs
@@ -5,7 +5,13 @@
 //! To dance around the fact the KMS isn't actually a service, we refer to it
 //! as a "Key Management System".
 
-use signatory::{ed25519, Decode, Encode};
+use crate::{
+    config::{ValidatorAddr, ValidatorConfig},
+    error::{KmsError, KmsErrorKind},
+    keyring::SecretKeyEncoding,
+    session::Session,
+};
+use signatory::{ed25519, Decode, Encode, PublicKeyed};
 use signatory_dalek::Ed25519Signer;
 use std::sync::atomic::{AtomicBool, Ordering};
 use std::sync::Arc;
@@ -15,14 +21,7 @@ use std::{
     thread::{self, JoinHandle},
     time::Duration,
 };
-use tendermint::{chain, public_keys::SecretConnectionKey};
-
-use crate::{
-    config::{ValidatorAddr, ValidatorConfig},
-    error::{KmsError, KmsErrorKind},
-    keyring::SecretKeyEncoding,
-    session::Session,
-};
+use tendermint::{chain, secret_connection};
 
 /// How long to wait after a crash before respawning (in seconds)
 pub const RESPAWN_DELAY: u64 = 1;
@@ -69,8 +68,12 @@ fn client_loop(config: ValidatorConfig, should_term: &Arc<AtomicBool>) {
         }
 
         let session_result = match &addr {
-            ValidatorAddr::Tcp { host, port } => match &secret_key {
-                Some(path) => tcp_session(chain_id, host, *port, path, should_term),
+            ValidatorAddr::Tcp {
+                peer_id,
+                host,
+                port,
+            } => match &secret_key {
+                Some(path) => tcp_session(chain_id, *peer_id, host, *port, path, should_term),
                 None => {
                     error!(
                         "config error: missing field `secret_key` for validator {}",
@@ -104,6 +107,7 @@ fn client_loop(config: ValidatorConfig, should_term: &Arc<AtomicBool>) {
 /// Create a TCP connection to a validator (encrypted with SecretConnection)
 fn tcp_session(
     chain_id: chain::Id,
+    validator_peer_id: Option<secret_connection::PeerId>,
     host: &str,
     port: u16,
     secret_key_path: &Path,
@@ -112,12 +116,13 @@ fn tcp_session(
     let secret_key = load_secret_connection_key(secret_key_path)?;
 
     let node_public_key =
-        SecretConnectionKey::from(ed25519::public_key(&Ed25519Signer::from(&secret_key)).unwrap());
+        secret_connection::PublicKey::from(Ed25519Signer::from(&secret_key).public_key().unwrap());
 
     info!("KMS node ID: {}", &node_public_key);
 
     panic::catch_unwind(move || {
-        let mut session = Session::connect_tcp(chain_id, host, port, &secret_key)?;
+        let mut session =
+            Session::connect_tcp(chain_id, validator_peer_id, host, port, &secret_key)?;
 
         info!(
             "[{}@tcp://{}:{}] connected to validator successfully",

diff --git a/src/config/validator/addr.rs b/src/config/validator/addr.rs
@@ -1,18 +1,22 @@
 //! Validator addresses (`tcp://` or `unix://`)
 
+use crate::error::{KmsError, KmsErrorKind::*};
 use serde::{de::Error as DeError, Deserialize, Deserializer};
 use std::{
     fmt::{self, Display},
     path::PathBuf,
     str::{self, FromStr},
 };
-
-use crate::error::{KmsError, KmsErrorKind::*};
+use tendermint::secret_connection;
 
 #[derive(Clone, Debug)]
 pub enum ValidatorAddr {
     /// TCP connections (with SecretConnection transport encryption)
     Tcp {
+        /// Remote peer ID of the validator
+        // TODO(tarcieri): make this mandatory
+        peer_id: Option<secret_connection::PeerId>,
+
         /// Validator hostname or IP address
         host: String,
 
@@ -53,7 +57,14 @@ impl FromStr for ValidatorAddr {
     // TODO: less janky URL parser? (e.g. use `url` crate)
     fn from_str(addr: &str) -> Result<Self, KmsError> {
         if addr.starts_with("tcp://") {
-            let host_and_port: Vec<&str> = addr[6..].split(':').collect();
+            let authority_parts = addr[6..].split('@').collect::<Vec<_>>();
+            let (peer_id, authority) = match authority_parts.len() {
+                1 => (None, authority_parts[0]),
+                2 => (Some(authority_parts[0].parse()?), authority_parts[1]),
+                _ => fail!(ConfigError, "invalid tcp:// address: {}", addr),
+            };
+
+            let host_and_port: Vec<&str> = authority.split(':').collect();
 
             if host_and_port.len() != 2 {
                 fail!(ConfigError, "invalid tcp:// address: {}", addr);
@@ -64,7 +75,11 @@ impl FromStr for ValidatorAddr {
                 .parse()
                 .map_err(|_| err!(ConfigError, "invalid tcp:// address (bad port): {}", addr))?;
 
-            Ok(ValidatorAddr::Tcp { host, port })
+            Ok(ValidatorAddr::Tcp {
+                peer_id,
+                host,
+                port,
+            })
         } else if addr.starts_with("unix://") {
             let socket_path = PathBuf::from(&addr[7..]);
             Ok(ValidatorAddr::Unix { socket_path })

diff --git a/src/session.rs b/src/session.rs
@@ -1,6 +1,14 @@
 //! A session with a validator node
 
-use signatory::ed25519;
+use crate::{
+    chain,
+    error::{KmsError, KmsErrorKind::VerificationError},
+    keyring::KeyRing,
+    prost::Message,
+    rpc::{Request, Response, TendermintRequest},
+    unix_connection::UnixConnection,
+};
+use signatory::{ed25519, PublicKeyed};
 use signatory_dalek::Ed25519Signer;
 use std::{
     fmt::Debug,
@@ -14,19 +22,10 @@ use std::{
         Arc,
     },
 };
+use subtle::ConstantTimeEq;
 use tendermint::{
     amino_types::{PingRequest, PingResponse, PubKeyRequest},
-    public_keys::SecretConnectionKey,
-    SecretConnection,
-};
-
-use crate::{
-    chain,
-    error::KmsError,
-    keyring::KeyRing,
-    prost::Message,
-    rpc::{Request, Response, TendermintRequest},
-    unix_connection::UnixConnection,
+    secret_connection::{self, SecretConnection},
 };
 
 /// Encrypted session with a validator node
@@ -42,6 +41,7 @@ impl Session<SecretConnection<TcpStream>> {
     /// Create a new session with the validator at the given address/port
     pub fn connect_tcp(
         chain_id: chain::Id,
+        validator_peer_id: Option<secret_connection::PeerId>,
         host: &str,
         port: u16,
         secret_connection_key: &ed25519::Seed,
@@ -50,8 +50,29 @@ impl Session<SecretConnection<TcpStream>> {
 
         let socket = TcpStream::connect(format!("{}:{}", host, port))?;
         let signer = Ed25519Signer::from(secret_connection_key);
-        let public_key = SecretConnectionKey::from(ed25519::public_key(&signer)?);
+        let public_key = secret_connection::PublicKey::from(signer.public_key()?);
         let connection = SecretConnection::new(socket, &public_key, &signer)?;
+        let actual_peer_id = connection.remote_pubkey().peer_id();
+
+        // TODO(tarcieri): move this logic into `SecretConnection::new`?
+        if let Some(expected_peer_id) = validator_peer_id {
+            if expected_peer_id.ct_eq(&actual_peer_id).unwrap_u8() == 0 {
+                fail!(
+                    VerificationError,
+                    "{}:{}: validator peer ID mismatch! (expected {}, got {})",
+                    host,
+                    port,
+                    expected_peer_id,
+                    actual_peer_id
+                );
+            }
+        } else {
+            // TODO(tarcieri): make peer verification mandatory
+            warn!(
+                "[{}] {}:{}: unverified validator peer ID! ({})",
+                chain_id, host, port, actual_peer_id
+            );
+        }
 
         Ok(Self {
             chain_id,

diff --git a/tendermint-rs/Cargo.toml b/tendermint-rs/Cargo.toml
@@ -43,6 +43,7 @@ serde_derive = { version =  "1", optional = true }
 signatory = { version = "0.11.2", optional = true, features = ["ed25519", "ecdsa"] }
 signatory-dalek = { version = "0.11", optional = true }
 sha2 = { version = "0.8", optional = true, default-features = false }
+subtle = { version = "2", optional = true }
 subtle-encoding = { version = "0.3", features = ["bech32-preview"] }
 tai64 = { version = "1", optional = true, features = ["chrono"] }
 x25519-dalek = { version = "0.4.4", optional = true, default-features = false, features = ["u64_backend"] }
@@ -60,6 +61,7 @@ secret-connection = [
     "signatory",
     "signatory-dalek",
     "sha2",
+    "subtle",
     "x25519-dalek",
     "zeroize"
 ]

diff --git a/tendermint-rs/src/public_keys.rs b/tendermint-rs/src/public_keys.rs
@@ -1,13 +1,8 @@
 //! Public keys used in Tendermint networks
-// TODO:: account keys
 
 use crate::{amino_types::PubKeyResponse, error::Error};
-use sha2::{Digest, Sha256};
 use signatory::{ecdsa::curve::secp256k1, ed25519};
-use std::{
-    fmt::{self, Display},
-    ops::Deref,
-};
+use std::ops::Deref;
 use subtle_encoding::{bech32, hex};
 
 /// Public keys allowed in Tendermint protocols
@@ -112,70 +107,11 @@ impl Deref for TendermintKey {
     }
 }
 
-/// Secret Connection signing keys
-// TODO(tarcieri): unify with `TendermintKey`?
-#[derive(Copy, Clone, Debug, Hash, Eq, PartialEq)]
-pub enum SecretConnectionKey {
-    /// Ed25519 Secret Connection keys
-    Ed25519(ed25519::PublicKey),
-}
-
-impl SecretConnectionKey {
-    /// From raw Ed25519 public key bytes
-    pub fn from_raw_ed25519(bytes: &[u8]) -> Result<SecretConnectionKey, Error> {
-        Ok(SecretConnectionKey::Ed25519(
-            ed25519::PublicKey::from_bytes(bytes)?,
-        ))
-    }
-
-    /// Get Ed25519 public key
-    pub fn ed25519(self) -> Option<ed25519::PublicKey> {
-        match self {
-            SecretConnectionKey::Ed25519(pk) => Some(pk),
-        }
-    }
-}
-
-impl Display for SecretConnectionKey {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        match self {
-            SecretConnectionKey::Ed25519(ref pk) => {
-                for byte in &Sha256::digest(pk.as_bytes())[..20] {
-                    write!(f, "{:02X}", byte)?;
-                }
-            }
-        }
-        Ok(())
-    }
-}
-
-impl From<ed25519::PublicKey> for SecretConnectionKey {
-    fn from(pk: ed25519::PublicKey) -> SecretConnectionKey {
-        SecretConnectionKey::Ed25519(pk)
-    }
-}
-
 #[cfg(test)]
 mod tests {
-    use super::{PublicKey, SecretConnectionKey, TendermintKey};
+    use super::{PublicKey, TendermintKey};
     use subtle_encoding::hex;
 
-    const EXAMPLE_SECRET_CONN_KEY: &str =
-        "F7FEB0B5BA0760B2C58893E329475D1EA81781DD636E37144B6D599AD38AA825";
-
-    #[test]
-    fn test_address_serialization() {
-        let example_key = SecretConnectionKey::from_raw_ed25519(
-            &hex::decode_upper(EXAMPLE_SECRET_CONN_KEY).unwrap(),
-        )
-        .unwrap();
-
-        assert_eq!(
-            example_key.to_string(),
-            "117C95C4FD7E636C38D303493302D2C271A39669"
-        );
-    }
-
     const EXAMPLE_CONSENSUS_KEY: &str =
         "4A25C6640A1F72B9C975338294EF51B6D1C33158BB6ECBA69FBC3FB5A33C9DCE";
 

diff --git a/tendermint-rs/src/secret_connection/mod.rs b/tendermint-rs/src/secret_connection/mod.rs
@@ -2,9 +2,11 @@
 
 mod kdf;
 mod nonce;
+mod peer_id;
+mod public_key;
 
-pub use self::{kdf::Kdf, nonce::Nonce};
-use crate::{amino_types::AuthSigMessage, error::Error, public_keys::SecretConnectionKey};
+pub use self::{kdf::Kdf, nonce::Nonce, peer_id::PeerId, public_key::PublicKey};
+use crate::{amino_types::AuthSigMessage, error::Error};
 use byteorder::{ByteOrder, LE};
 use bytes::BufMut;
 use prost::{encoding::encode_varint, Message};
@@ -34,20 +36,20 @@ pub struct SecretConnection<IoHandler: Read + Write + Send + Sync> {
     send_nonce: Nonce,
     recv_secret: aead::OpeningKey,
     send_secret: aead::SealingKey,
-    remote_pubkey: SecretConnectionKey,
+    remote_pubkey: PublicKey,
     recv_buffer: Vec<u8>,
 }
 
 impl<IoHandler: Read + Write + Send + Sync> SecretConnection<IoHandler> {
     /// Returns authenticated remote pubkey
-    pub fn remote_pubkey(&self) -> SecretConnectionKey {
+    pub fn remote_pubkey(&self) -> PublicKey {
         self.remote_pubkey
     }
     #[allow(clippy::new_ret_no_self)]
     /// Performs handshake and returns a new authenticated SecretConnection.
     pub fn new(
         mut handler: IoHandler,
-        local_pubkey: &SecretConnectionKey,
+        local_pubkey: &PublicKey,
         local_privkey: &dyn Signer<ed25519::Signature>,
     ) -> Result<SecretConnection<IoHandler>, Error> {
         // Generate ephemeral keys for perfect forward secrecy.
@@ -82,7 +84,7 @@ impl<IoHandler: Read + Write + Send + Sync> SecretConnection<IoHandler> {
                 .map_err(|_| Error::Crypto)?,
             send_secret: aead::SealingKey::new(&aead::CHACHA20_POLY1305, &kdf.send_secret)
                 .map_err(|_| Error::Crypto)?,
-            remote_pubkey: SecretConnectionKey::from(ed25519::PublicKey::from_bytes(
+            remote_pubkey: PublicKey::from(ed25519::PublicKey::from_bytes(
                 remote_eph_pubkey.as_bytes(),
             )?),
         };
@@ -92,7 +94,7 @@ impl<IoHandler: Read + Write + Send + Sync> SecretConnection<IoHandler> {
 
         // Share (in secret) each other's pubkey & challenge signature
         let auth_sig_msg = match local_pubkey {
-            SecretConnectionKey::Ed25519(ref pk) => {
+            PublicKey::Ed25519(ref pk) => {
                 share_auth_signature(&mut sc, pk.as_bytes(), local_signature)?
             }
         };
@@ -105,7 +107,7 @@ impl<IoHandler: Read + Write + Send + Sync> SecretConnection<IoHandler> {
         ed25519::verify(&remote_verifier, &kdf.challenge, &remote_sig)?;
 
         // We've authorized.
-        sc.remote_pubkey = SecretConnectionKey::from(remote_pubkey);
+        sc.remote_pubkey = PublicKey::from(remote_pubkey);
 
         Ok(sc)
     }