From 4dfdbc6b05ea32dff55853c1cc35b2b220230118 Mon Sep 17 00:00:00 2001 From: Danilo Bargen Date: Mon, 7 May 2018 17:41:27 +0200 Subject: [PATCH] Implement support for server public permanent key Refs #12 --- src/crypto_types.rs | 102 ++++++++++++++++++++++- src/protocol/mod.rs | 77 +++++++++++++---- src/protocol/nonce.rs | 2 +- src/protocol/tests/signaling_messages.rs | 46 ++++++++++ 4 files changed, 207 insertions(+), 20 deletions(-) diff --git a/src/crypto_types.rs b/src/crypto_types.rs index c3e823a..d616ff5 100644 --- a/src/crypto_types.rs +++ b/src/crypto_types.rs @@ -4,6 +4,8 @@ use std::cmp; use std::fmt; +#[cfg(test)] +use std::io::Write; use data_encoding::{HEXLOWER, HEXLOWER_PERMISSIVE}; use rust_sodium::crypto::{box_, secretbox}; @@ -222,17 +224,50 @@ impl AuthToken { } +/// The number of bytes in the [`SignedKeys`](struct.SignedKeys.html) array. +const SIGNED_KEYS_BYTES: usize = 2 * box_::PUBLICKEYBYTES + box_::MACBYTES; + + /// A pair of not-yet-signed keys used in the [`ServerAuth`](../messages/struct.ServerAuth.html) /// message. #[derive(Debug, Clone, PartialEq, Eq)] pub struct UnsignedKeys { - server_session_key: PublicKey, - client_permanent_key: PublicKey, + pub server_public_session_key: PublicKey, + pub client_public_permanent_key: PublicKey, } +impl UnsignedKeys { + pub fn new(server_public_session_key: PublicKey, client_public_permanent_key: PublicKey) -> Self { + Self { server_public_session_key, client_public_permanent_key } + } + + /// Sign the server public session key and the client public permanent key. + /// + /// This is only used in testing. + #[cfg(test)] + pub(crate) fn sign( + self, + server_session_keypair: &KeyPair, + client_public_permanent_key: &PublicKey, + nonce: Nonce, + ) -> SignedKeys { + let mut bytes = [0u8; 64]; + (&mut bytes[0..32]).write_all(&self.server_public_session_key.0).unwrap(); + (&mut bytes[32..64]).write_all(&self.client_public_permanent_key.0).unwrap(); + let rust_sodium_nonce: box_::Nonce = nonce.into(); + let vec = box_::seal( + &bytes, + &rust_sodium_nonce, + client_public_permanent_key, + server_session_keypair.private_key(), + ); + assert_eq!(vec.len(), SIGNED_KEYS_BYTES); + let mut encrypted = [0u8; SIGNED_KEYS_BYTES]; + (&mut encrypted[..]).write_all(&vec).unwrap(); + SignedKeys(encrypted) + } +} -/// The number of bytes in the [`SignedKeys`](struct.SignedKeys.html) array. -const SIGNED_KEYS_BYTES: usize = 2 * box_::PUBLICKEYBYTES + box_::MACBYTES; /// Concatenated signed keys used in the [`ServerAuth`](../messages/struct.ServerAuth.html) /// message. @@ -242,6 +277,27 @@ impl SignedKeys { pub fn new(bytes: [u8; SIGNED_KEYS_BYTES]) -> Self { SignedKeys(bytes) } + + pub(crate) fn decrypt( + &self, + permanent_key: &KeyPair, + server_public_permanent_key: &PublicKey, + nonce: Nonce, + ) -> SignalingResult { + // Decrypt bytes + let rust_sodium_nonce: box_::Nonce = nonce.into(); + let decrypted = box_::open( + &self.0, + &rust_sodium_nonce, + server_public_permanent_key, + permanent_key.private_key(), + ).map_err(|_| SignalingError::Crypto("Could not decrypt signed keys".to_string()))?; + assert_eq!(decrypted.len(), 32 * 2); + Ok(UnsignedKeys::new( + PublicKey::from_slice(&decrypted[0..32]).unwrap(), + PublicKey::from_slice(&decrypted[32..64]).unwrap(), + )) + } } /// Implementation required because Clone cannot be derived for `[u8; 80]` on @@ -498,4 +554,42 @@ mod tests { assert_ne!((deref2.0).0, token_bytes); assert_eq!((deref2.0).0, zero_bytes); } + + #[test] + fn unsigned_keys_sign_decrypt() { + // Create keypairs + let kp_server = KeyPair::new(); + let kp_client = KeyPair::new(); + + // Create nonce + let nonce_hex = b"fe381c4bdb8bfc2a27d2c9a6485113e7638613ffb02b3747"; + let nonce_bytes = HEXLOWER.decode(nonce_hex).unwrap(); + let nonce = Nonce::from_bytes(&nonce_bytes).unwrap(); + + // Sign keys + let unsigned = UnsignedKeys::new( + kp_server.public_key().clone(), + kp_client.public_key().clone(), + ); + let signed = unsigned.clone().sign( + &kp_server, + kp_client.public_key(), + unsafe { nonce.clone() }, + ); + + // Decrypt directly with libsodium + let decrypted = box_::open( + &signed.0, + &{ unsafe { nonce.clone() } }.into(), + kp_server.public_key(), + kp_client.private_key(), + ).unwrap(); + assert_eq!(decrypted.len(), 2 * 32); + assert_eq!(&decrypted[0..32], &kp_server.public_key().0); + assert_eq!(&decrypted[32..64], &kp_client.public_key().0); + + // Decrypt through the `decrypt` method + let unsigned2 = signed.decrypt(&kp_client, kp_server.public_key(), nonce).unwrap(); + assert_eq!(unsigned, unsigned2); + } } diff --git a/src/protocol/mod.rs b/src/protocol/mod.rs index c194dd0..0835a76 100644 --- a/src/protocol/mod.rs +++ b/src/protocol/mod.rs @@ -15,6 +15,7 @@ use std::time::Duration; use boxes::{ByteBox, OpenBox}; use crypto::{KeyPair, AuthToken, PublicKey}; +use crypto_types::UnsignedKeys; use errors::{SignalingError, SignalingResult}; use rmpv::{Value}; @@ -271,8 +272,24 @@ pub(crate) trait Signaling { }; if bbox.nonce.source().is_server() { + // We need to clone the nonce here, in case we need it to verify + // the signed keys sent in the 'server-auth' message. + // Unfortunately at this point in time we don't know yet whether + // the message actually is a 'server-auth' message... + let nonce_unsafe_clone = unsafe { bbox.nonce.clone() }; + + // Decode the message from the server let obox: OpenBox = self.decode_server_message(bbox)?; - self.handle_server_message(obox) + + // Only keep the nonce clone if this is a 'server-auth' message + let nonce_clone_opt = if obox.message.get_type() == "server-auth" { + Some(nonce_unsafe_clone) + } else { + None + }; + + // Process the server message + self.handle_server_message(obox, nonce_clone_opt) } else { match self.common().signaling_state() { SignalingState::ServerHandshake => self.handle_handshake_peer_message(bbox), @@ -318,7 +335,7 @@ pub(crate) trait Signaling { // Peer handshake SignalingState::PeerHandshake if obox.nonce.source().is_server() => - self.handle_server_message(obox), + self.handle_server_message(obox, None), SignalingState::PeerHandshake => self.handle_peer_message(obox), @@ -519,14 +536,18 @@ pub(crate) trait Signaling { /// /// This method call may have some side effects, like updates in the peer /// context (cookie, CSN, etc). - fn handle_server_message(&mut self, obox: OpenBox) -> SignalingResult> { + /// + /// Note: The `nonce_clone` parameter is only set to a value if needed to + /// verify the signed keys inside the `server-auth` message. Otherwise it's + /// `None`. + fn handle_server_message(&mut self, obox: OpenBox, nonce_clone: Option) -> SignalingResult> { let old_state = self.server_handshake_state(); match (old_state, obox.message) { // Valid state transitions (ServerHandshakeState::New, Message::ServerHello(msg)) => self.handle_server_hello(msg), (ServerHandshakeState::ClientInfoSent, Message::ServerAuth(msg)) => - self.handle_server_auth(msg), + self.handle_server_auth(msg, nonce_clone), (ServerHandshakeState::Done, Message::NewInitiator(msg)) => self.handle_new_initiator(msg), (ServerHandshakeState::Done, Message::NewResponder(msg)) => @@ -633,7 +654,7 @@ pub(crate) trait Signaling { } /// Handle an incoming [`ServerAuth`](messages/struct.ServerAuth.html) message. - fn handle_server_auth(&mut self, msg: ServerAuth) -> SignalingResult> { + fn handle_server_auth(&mut self, msg: ServerAuth, nonce_clone: Option) -> SignalingResult> { debug!("--> Received server-auth from server"); // When the client receives a 'server-auth' message, it MUST @@ -654,16 +675,42 @@ pub(crate) trait Signaling { self.server().identity(), )?; - // If the client has knowledge of the server's public permanent - // key, it SHALL decrypt the signed_keys field by using the - // message's nonce, the client's private permanent key and the - // server's public permanent key. The decrypted message MUST - // match the concatenation of the server's public session key - // and the client's public permanent key (in that order). If - // the signed_keys is present but the client does not have - // knowledge of the server's permanent key, it SHALL log a - // warning. - // TODO (#12): Implement + if let Some(server_public_permanent_key) = self.server().permanent_key() { + // If the client has knowledge of the server's public permanent + // key, it SHALL decrypt the signed_keys field by using the + // message's nonce, the client's private permanent key and the + // server's public permanent key. + let nonce = nonce_clone.ok_or_else(|| SignalingError::Crash( + "This is a server-auth message, but no nonce clone was passed in".into() + ))?; + let signed_keys = msg.signed_keys.as_ref().ok_or_else(|| SignalingError::Protocol( + "Server's public permanent key is known, but server did not send signed keys".into() + ))?; + let decrypted = signed_keys.decrypt( + &self.common().permanent_keypair, + server_public_permanent_key, + nonce, + )?; + + // The decrypted message MUST match the concatenation of the + // server's public session key and the client's public permanent + // key (in that order). + let server_public_session_key = self.server().session_key() + .ok_or_else(|| SignalingError::Crash("Server session key not set".into()))?; + if &decrypted.server_public_session_key != server_public_session_key { + return Err(SignalingError::Protocol("Server public session key sent in `signed_keys` is not valid".into())); + } + if &decrypted.client_public_permanent_key != self.common().permanent_keypair.public_key() { + return Err(SignalingError::Protocol("Our public permanent key sent in `signed_keys` is not valid".into())); + } + } else { + // If the signed_keys is present but the client does not have + // knowledge of the server's permanent key, it SHALL log a + // warning. + if msg.signed_keys.is_some() { + warn!("Server sent signed keys, but we're not verifying them"); + } + } // Moreover, the client MUST do some checks depending on its role let actions = self.handle_server_auth_impl(&msg)?; diff --git a/src/protocol/nonce.rs b/src/protocol/nonce.rs index 1a4a8bb..195a979 100644 --- a/src/protocol/nonce.rs +++ b/src/protocol/nonce.rs @@ -105,7 +105,7 @@ impl Nonce { /// /// This is unsafe because a `Nonce` must never be reused for two messages. /// Only clone a `Nonce` if it's absolutely required and if you are sure - /// that it isn't reused. + /// that it isn't reused improperly. #[cfg_attr(feature="clippy", allow(should_implement_trait))] pub(crate) unsafe fn clone(&self) -> Nonce { Nonce { diff --git a/src/protocol/tests/signaling_messages.rs b/src/protocol/tests/signaling_messages.rs index bb40bfd..a1f350a 100644 --- a/src/protocol/tests/signaling_messages.rs +++ b/src/protocol/tests/signaling_messages.rs @@ -397,6 +397,52 @@ mod server_auth { HandleAction::Event(Event::ServerHandshakeDone(false)), ]); } + + #[test] + fn server_public_permanent_key_validate() { + // Create server public permanent key + let server_permanent_ks1 = KeyPair::new(); + let server_permanent_ks2 = KeyPair::new(); + + // Initialize signaling class + let mut ctx = TestContext::initiator( + ClientIdentity::Initiator, None, + SignalingState::ServerHandshake, ServerHandshakeState::ClientInfoSent, + ); + ctx.signaling.server_mut().permanent_key = Some(server_permanent_ks1.public_key().clone()); + + // Create nonce for ServerAuth message + let nonce = Nonce::new(ctx.server_cookie.clone(), Address(0), Address(1), CombinedSequenceSnapshot::random()); + + // Prepare signed keys + let unsigned_keys = UnsignedKeys::new( + ctx.signaling.server().session_key().unwrap().clone(), + ctx.our_ks.public_key().clone(), + ); + let signed_keys = unsigned_keys.sign(&server_permanent_ks1, ctx.our_ks.public_key(), unsafe { nonce.clone() }); + + // Prepare a ServerAuth message. + let msg = ServerAuth::for_initiator(ctx.our_cookie.clone(), Some(signed_keys), vec![]).into_message(); + let msg_bytes = msg.to_msgpack(); + let encrypted = ctx.our_ks.encrypt(&msg_bytes, unsafe { nonce.clone() }, ctx.server_ks.public_key()); + let bbox = ByteBox::new(encrypted, nonce); + + // Change server permanent key (to provoke a validation error) + ctx.signaling.server_mut().permanent_key = Some(server_permanent_ks2.public_key().clone()); + + // Handle message + let mut s = ctx.signaling; + assert_eq!(s.server().handshake_state(), ServerHandshakeState::ClientInfoSent); + assert_eq!( + s.handle_message(bbox), + Err(SignalingError::Crypto("Could not decrypt signed keys".into())) + ); + assert_eq!(s.server().handshake_state(), ServerHandshakeState::ClientInfoSent); + + // TODO: Add two additional tests: + // - Successful validation + // - Correct encryption but bad keys inside + } } mod client_auth {