From 51179f5433fbc1617d59e25493a22072c0987726 Mon Sep 17 00:00:00 2001 From: Mathias Hall-Andersen Date: Sat, 31 Aug 2019 15:03:14 +0200 Subject: Better management of key material --- Cargo.lock | 1 + Cargo.toml | 1 + src/handshake/noise.rs | 393 +++++++++++++++++++++++++------------------------ src/handshake/peer.rs | 37 ++--- src/types/endpoint.rs | 7 +- src/types/keys.rs | 12 +- src/types/tun.rs | 6 +- 7 files changed, 234 insertions(+), 223 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6bb5f81..87c424c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1241,6 +1241,7 @@ dependencies = [ "blake2 0.8.1 (registry+https://github.com/rust-lang/crates.io-index)", "byteorder 1.3.2 (registry+https://github.com/rust-lang/crates.io-index)", "chacha20poly1305 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)", + "clear_on_drop 0.2.3 (registry+https://github.com/rust-lang/crates.io-index)", "crossbeam-deque 0.7.1 (registry+https://github.com/rust-lang/crates.io-index)", "digest 0.8.1 (registry+https://github.com/rust-lang/crates.io-index)", "futures 0.1.28 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/Cargo.toml b/Cargo.toml index 5a9464d..0689086 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -25,6 +25,7 @@ hjul = "0.1.2" ring = "0.16.7" chacha20poly1305 = "^0.1" aead = "^0.1.1" +clear_on_drop = "0.2.3" [dependencies.x25519-dalek] version = "^0.5" diff --git a/src/handshake/noise.rs b/src/handshake/noise.rs index 8efe4aa..db1a244 100644 --- a/src/handshake/noise.rs +++ b/src/handshake/noise.rs @@ -15,6 +15,8 @@ use rand::{CryptoRng, RngCore}; use generic_array::typenum::*; use generic_array::*; +use clear_on_drop::clear_stack_on_return; + use super::device::Device; use super::messages::{NoiseInitiation, NoiseResponse}; use super::messages::{TYPE_INITIATION, TYPE_RESPONSE}; @@ -39,6 +41,9 @@ const SIZE_HS: usize = 32; const SIZE_NONCE: usize = 8; const SIZE_TAG: usize = 16; +// number of pages to clear after sensitive call +const CLEAR_PAGES: usize = 1; + // C := Hash(Construction) const INITIAL_CK: [u8; SIZE_CK] = [ 0x60, 0xe2, 0x6d, 0xae, 0xf3, 0x27, 0xef, 0xc0, 0x2e, 0xc3, 0x35, 0xe2, 0xa0, 0x25, 0xd2, 0xd0, @@ -103,21 +108,21 @@ macro_rules! KDF3 { } macro_rules! SEAL { - ($key:expr, $ad:expr, $pt:expr, $ct:expr) => {{ - let ct = ChaCha20Poly1305::new(*GenericArray::from_slice($key)) + ($key:expr, $ad:expr, $pt:expr, $ct:expr) => { + ChaCha20Poly1305::new(*GenericArray::from_slice($key)) .encrypt(&ZERO_NONCE.into(), Payload { msg: $pt, aad: $ad }) - .unwrap(); - $ct.copy_from_slice(&ct); - }}; + .map(|ct| $ct.copy_from_slice(&ct)) + .unwrap() + }; } macro_rules! OPEN { - ($key:expr, $ad:expr, $pt:expr, $ct:expr) => {{ + ($key:expr, $ad:expr, $pt:expr, $ct:expr) => { ChaCha20Poly1305::new(*GenericArray::from_slice($key)) .decrypt(&ZERO_NONCE.into(), Payload { msg: $ct, aad: $ad }) .map_err(|_| HandshakeError::DecryptionFailure) .map(|pt| $pt.copy_from_slice(&pt)) - }}; + }; } #[cfg(test)] @@ -211,144 +216,151 @@ pub fn create_initiation( sender: u32, msg: &mut NoiseInitiation, ) -> Result<(), HandshakeError> { - // initialize state + clear_stack_on_return(CLEAR_PAGES, || { + // initialize state - let ck = INITIAL_CK; - let hs = INITIAL_HS; - let hs = HASH!(&hs, peer.pk.as_bytes()); + let ck = INITIAL_CK; + let hs = INITIAL_HS; + let hs = HASH!(&hs, peer.pk.as_bytes()); - msg.f_type.set(TYPE_INITIATION as u32); - msg.f_sender.set(sender); + msg.f_type.set(TYPE_INITIATION as u32); + msg.f_sender.set(sender); - // (E_priv, E_pub) := DH-Generate() + // (E_priv, E_pub) := DH-Generate() - let eph_sk = StaticSecret::new(rng); - let eph_pk = PublicKey::from(&eph_sk); + let eph_sk = StaticSecret::new(rng); + let eph_pk = PublicKey::from(&eph_sk); - // C := Kdf(C, E_pub) + // C := Kdf(C, E_pub) - let ck = KDF1!(&ck, eph_pk.as_bytes()); + let ck = KDF1!(&ck, eph_pk.as_bytes()); - // msg.ephemeral := E_pub + // msg.ephemeral := E_pub - msg.f_ephemeral = *eph_pk.as_bytes(); + msg.f_ephemeral = *eph_pk.as_bytes(); - // H := HASH(H, msg.ephemeral) + // H := HASH(H, msg.ephemeral) - let hs = HASH!(&hs, msg.f_ephemeral); + let hs = HASH!(&hs, msg.f_ephemeral); - // (C, k) := Kdf2(C, DH(E_priv, S_pub)) + // (C, k) := Kdf2(C, DH(E_priv, S_pub)) - let (ck, key) = KDF2!(&ck, eph_sk.diffie_hellman(&peer.pk).as_bytes()); + let (ck, key) = KDF2!(&ck, eph_sk.diffie_hellman(&peer.pk).as_bytes()); - // msg.static := Aead(k, 0, S_pub, H) + // msg.static := Aead(k, 0, S_pub, H) - SEAL!( - &key, - &hs, // ad - device.pk.as_bytes(), // pt - &mut msg.f_static // ct || tag - ); + SEAL!( + &key, + &hs, // ad + device.pk.as_bytes(), // pt + &mut msg.f_static // ct || tag + ); - // H := Hash(H || msg.static) + // H := Hash(H || msg.static) - let hs = HASH!(&hs, &msg.f_static[..]); + let hs = HASH!(&hs, &msg.f_static[..]); - // (C, k) := Kdf2(C, DH(S_priv, S_pub)) + // (C, k) := Kdf2(C, DH(S_priv, S_pub)) - let (ck, key) = KDF2!(&ck, peer.ss.as_bytes()); + let (ck, key) = KDF2!(&ck, peer.ss.as_bytes()); - // msg.timestamp := Aead(k, 0, Timestamp(), H) + // msg.timestamp := Aead(k, 0, Timestamp(), H) - SEAL!( - &key, - &hs, // ad - ×tamp::now(), // pt - &mut msg.f_timestamp // ct || tag - ); + SEAL!( + &key, + &hs, // ad + ×tamp::now(), // pt + &mut msg.f_timestamp // ct || tag + ); - // H := Hash(H || msg.timestamp) + // H := Hash(H || msg.timestamp) - let hs = HASH!(&hs, &msg.f_timestamp); + let hs = HASH!(&hs, &msg.f_timestamp); - // update state of peer + // update state of peer - peer.set_state(State::InitiationSent { - hs, - ck, - eph_sk, - sender, - }); + peer.set_state(State::InitiationSent { + hs, + ck, + eph_sk, + sender, + }); - Ok(()) + Ok(()) + }) } pub fn consume_initiation<'a, T: Copy>( device: &'a Device, msg: &NoiseInitiation, ) -> Result<(&'a Peer, TemporaryState), HandshakeError> { - // initialize state + clear_stack_on_return(CLEAR_PAGES, || { + // initialize state + + let ck = INITIAL_CK; + let hs = INITIAL_HS; + let hs = HASH!(&hs, device.pk.as_bytes()); - let ck = INITIAL_CK; - let hs = INITIAL_HS; - let hs = HASH!(&hs, device.pk.as_bytes()); + // C := Kdf(C, E_pub) - // C := Kdf(C, E_pub) + let ck = KDF1!(&ck, &msg.f_ephemeral); - let ck = KDF1!(&ck, &msg.f_ephemeral); + // H := HASH(H, msg.ephemeral) - // H := HASH(H, msg.ephemeral) + let hs = HASH!(&hs, &msg.f_ephemeral); - let hs = HASH!(&hs, &msg.f_ephemeral); + // (C, k) := Kdf2(C, DH(E_priv, S_pub)) - // (C, k) := Kdf2(C, DH(E_priv, S_pub)) + let eph_r_pk = PublicKey::from(msg.f_ephemeral); + let (ck, key) = KDF2!(&ck, device.sk.diffie_hellman(&eph_r_pk).as_bytes()); - let eph_r_pk = PublicKey::from(msg.f_ephemeral); - let (ck, key) = KDF2!(&ck, device.sk.diffie_hellman(&eph_r_pk).as_bytes()); + // msg.static := Aead(k, 0, S_pub, H) - // msg.static := Aead(k, 0, S_pub, H) + let mut pk = [0u8; 32]; - let mut pk = [0u8; 32]; + OPEN!( + &key, + &hs, // ad + &mut pk, // pt + &msg.f_static // ct || tag + )?; - OPEN!( - &key, - &hs, // ad - &mut pk, // pt - &msg.f_static // ct || tag - )?; + let peer = device.lookup_pk(&PublicKey::from(pk))?; - let peer = device.lookup_pk(&PublicKey::from(pk))?; + // H := Hash(H || msg.static) - // H := Hash(H || msg.static) + let hs = HASH!(&hs, &msg.f_static[..]); - let hs = HASH!(&hs, &msg.f_static[..]); + // (C, k) := Kdf2(C, DH(S_priv, S_pub)) - // (C, k) := Kdf2(C, DH(S_priv, S_pub)) + let (ck, key) = KDF2!(&ck, peer.ss.as_bytes()); - let (ck, key) = KDF2!(&ck, peer.ss.as_bytes()); + // msg.timestamp := Aead(k, 0, Timestamp(), H) - // msg.timestamp := Aead(k, 0, Timestamp(), H) + let mut ts = timestamp::ZERO; - let mut ts = timestamp::ZERO; + OPEN!( + &key, + &hs, // ad + &mut ts, // pt + &msg.f_timestamp // ct || tag + )?; - OPEN!( - &key, - &hs, // ad - &mut ts, // pt - &msg.f_timestamp // ct || tag - )?; + // check and update timestamp - // check and update timestamp + peer.check_replay_flood(device, &ts)?; - peer.check_replay_flood(device, &ts)?; + // H := Hash(H || msg.timestamp) - // H := Hash(H || msg.timestamp) + let hs = HASH!(&hs, &msg.f_timestamp); - let hs = HASH!(&hs, &msg.f_timestamp); + // clear initiation state + *peer.state.lock() = State::Reset; - // return state (to create response) + // return state (to create response) - Ok((peer, (msg.f_sender.get(), eph_r_pk, hs, ck))) + Ok((peer, (msg.f_sender.get(), eph_r_pk, hs, ck))) + }) } pub fn create_response( @@ -358,79 +370,77 @@ pub fn create_response( state: TemporaryState, // state from "consume_initiation" msg: &mut NoiseResponse, // resulting response ) -> Result { - // unpack state + clear_stack_on_return(CLEAR_PAGES, || { + // unpack state - let (receiver, eph_r_pk, hs, ck) = state; + let (receiver, eph_r_pk, hs, ck) = state; - msg.f_type.set(TYPE_RESPONSE as u32); - msg.f_sender.set(sender); - msg.f_receiver.set(receiver); + msg.f_type.set(TYPE_RESPONSE as u32); + msg.f_sender.set(sender); + msg.f_receiver.set(receiver); - // (E_priv, E_pub) := DH-Generate() + // (E_priv, E_pub) := DH-Generate() - let eph_sk = StaticSecret::new(rng); - let eph_pk = PublicKey::from(&eph_sk); + let eph_sk = StaticSecret::new(rng); + let eph_pk = PublicKey::from(&eph_sk); - // C := Kdf1(C, E_pub) + // C := Kdf1(C, E_pub) - let ck = KDF1!(&ck, eph_pk.as_bytes()); + let ck = KDF1!(&ck, eph_pk.as_bytes()); - // msg.ephemeral := E_pub + // msg.ephemeral := E_pub - msg.f_ephemeral = *eph_pk.as_bytes(); + msg.f_ephemeral = *eph_pk.as_bytes(); - // H := Hash(H || msg.ephemeral) + // H := Hash(H || msg.ephemeral) - let hs = HASH!(&hs, &msg.f_ephemeral); + let hs = HASH!(&hs, &msg.f_ephemeral); - // C := Kdf1(C, DH(E_priv, E_pub)) + // C := Kdf1(C, DH(E_priv, E_pub)) - let ck = KDF1!(&ck, eph_sk.diffie_hellman(&eph_r_pk).as_bytes()); + let ck = KDF1!(&ck, eph_sk.diffie_hellman(&eph_r_pk).as_bytes()); - // C := Kdf1(C, DH(E_priv, S_pub)) + // C := Kdf1(C, DH(E_priv, S_pub)) - let ck = KDF1!(&ck, eph_sk.diffie_hellman(&peer.pk).as_bytes()); + let ck = KDF1!(&ck, eph_sk.diffie_hellman(&peer.pk).as_bytes()); - // (C, tau, k) := Kdf3(C, Q) + // (C, tau, k) := Kdf3(C, Q) - let (ck, tau, key) = KDF3!(&ck, &peer.psk); + let (ck, tau, key) = KDF3!(&ck, &peer.psk); - // H := Hash(H || tau) + // H := Hash(H || tau) - let hs = HASH!(&hs, tau); + let hs = HASH!(&hs, tau); - // msg.empty := Aead(k, 0, [], H) + // msg.empty := Aead(k, 0, [], H) - SEAL!( - &key, - &hs, // ad - &[], // pt - &mut msg.f_empty // \epsilon || tag - ); - - /* not strictly needed - * // H := Hash(H || msg.empty) - * let hs = HASH!(&hs, &msg.f_empty_tag); - */ + SEAL!( + &key, + &hs, // ad + &[], // pt + &mut msg.f_empty // \epsilon || tag + ); - // derive key-pair - // (verbose code, due to GenericArray -> [u8; 32] conversion) + // Not strictly needed + // let hs = HASH!(&hs, &msg.f_empty_tag); - let (key_recv, key_send) = KDF2!(&ck, &[]); + // derive key-pair + let (key_recv, key_send) = KDF2!(&ck, &[]); - // return unconfirmed key-pair + // return unconfirmed key-pair - Ok(KeyPair { - birth: Instant::now(), - initiator: false, - send: Key { - id: sender, - key: key_send.into(), - }, - recv: Key { - id: receiver, - key: key_recv.into(), - }, + Ok(KeyPair { + birth: Instant::now(), + initiator: false, + send: Key { + id: sender, + key: key_send.into(), + }, + recv: Key { + id: receiver, + key: key_recv.into(), + }, + }) }) } @@ -438,73 +448,78 @@ pub fn consume_response( device: &Device, msg: &NoiseResponse, ) -> Result, HandshakeError> { - // retrieve peer and associated state + clear_stack_on_return(CLEAR_PAGES, || { + // retrieve peer and copy associated state + let peer = device.lookup_id(msg.f_receiver.get())?; + let (hs, ck, sender, eph_sk) = match *peer.state.lock() { + State::InitiationSent { + hs, + ck, + sender, + ref eph_sk, + } => Ok((hs, ck, sender, StaticSecret::from(eph_sk.to_bytes()))), + _ => Err(HandshakeError::InvalidState), + }?; - let peer = device.lookup_id(msg.f_receiver.get())?; - let (hs, ck, sender, eph_sk) = match peer.get_state() { - State::Reset => Err(HandshakeError::InvalidState), - State::InitiationSent { - hs, - ck, - sender, - eph_sk, - } => Ok((hs, ck, sender, eph_sk)), - }?; + // C := Kdf1(C, E_pub) - // C := Kdf1(C, E_pub) + let ck = KDF1!(&ck, &msg.f_ephemeral); - let ck = KDF1!(&ck, &msg.f_ephemeral); + // H := Hash(H || msg.ephemeral) - // H := Hash(H || msg.ephemeral) + let hs = HASH!(&hs, &msg.f_ephemeral); - let hs = HASH!(&hs, &msg.f_ephemeral); + // C := Kdf1(C, DH(E_priv, E_pub)) - // C := Kdf1(C, DH(E_priv, E_pub)) + let eph_r_pk = PublicKey::from(msg.f_ephemeral); + let ck = KDF1!(&ck, eph_sk.diffie_hellman(&eph_r_pk).as_bytes()); - let eph_r_pk = PublicKey::from(msg.f_ephemeral); - let ck = KDF1!(&ck, eph_sk.diffie_hellman(&eph_r_pk).as_bytes()); + // C := Kdf1(C, DH(E_priv, S_pub)) - // C := Kdf1(C, DH(E_priv, S_pub)) + let ck = KDF1!(&ck, device.sk.diffie_hellman(&eph_r_pk).as_bytes()); - let ck = KDF1!(&ck, device.sk.diffie_hellman(&eph_r_pk).as_bytes()); + // (C, tau, k) := Kdf3(C, Q) - // (C, tau, k) := Kdf3(C, Q) + let (ck, tau, key) = KDF3!(&ck, &peer.psk); - let (ck, tau, key) = KDF3!(&ck, &peer.psk); + // H := Hash(H || tau) - // H := Hash(H || tau) + let hs = HASH!(&hs, tau); - let hs = HASH!(&hs, tau); + // msg.empty := Aead(k, 0, [], H) - // msg.empty := Aead(k, 0, [], H) + OPEN!( + &key, + &hs, // ad + &mut [], // pt + &msg.f_empty // \epsilon || tag + )?; - OPEN!( - &key, - &hs, // ad - &mut [], // pt - &msg.f_empty // \epsilon || tag - )?; + // derive key-pair - // derive key-pair + let (key_send, key_recv) = KDF2!(&ck, &[]); - let (key_send, key_recv) = KDF2!(&ck, &[]); + // null the state - // return confirmed key-pair + *peer.state.lock() = State::Reset; - Ok(( - Some(peer.identifier), // proves overship of the public key (e.g. for updating the endpoint) - None, // no response message - Some(KeyPair { - birth: Instant::now(), - initiator: true, - send: Key { - id: sender, - key: key_send.into(), - }, - recv: Key { - id: msg.f_sender.get(), - key: key_recv.into(), - }, - }), - )) + // return confirmed key-pair + + Ok(( + Some(peer.identifier), // proves ownership of the public key (e.g. for updating the endpoint) + None, // no response message + Some(KeyPair { + birth: Instant::now(), + initiator: true, + send: Key { + id: sender, + key: key_send.into(), + }, + recv: Key { + id: msg.f_sender.get(), + key: key_recv.into(), + }, + }), + )) + }) } diff --git a/src/handshake/peer.rs b/src/handshake/peer.rs index 9629a7f..6f0f5af 100644 --- a/src/handshake/peer.rs +++ b/src/handshake/peer.rs @@ -9,6 +9,8 @@ use x25519_dalek::PublicKey; use x25519_dalek::SharedSecret; use x25519_dalek::StaticSecret; +use clear_on_drop::clear::Clear; + use super::device::Device; use super::macs; use super::timestamp; @@ -27,9 +29,9 @@ pub struct Peer { pub(crate) identifier: T, // mutable state - state: Mutex, - timestamp: Mutex>, - last_initiation_consumption: Mutex>, + pub(crate) state: Mutex, + pub(crate) timestamp: Mutex>, + pub(crate) last_initiation_consumption: Mutex>, // state related to DoS mitigation fields pub(crate) macs: Mutex, @@ -50,21 +52,15 @@ pub enum State { }, } -impl Clone for State { - fn clone(&self) -> State { +impl Drop for State { + fn drop(&mut self) { match self { - State::Reset => State::Reset, - State::InitiationSent { - sender, - eph_sk, - hs, - ck, - } => State::InitiationSent { - sender: *sender, - eph_sk: StaticSecret::from(eph_sk.to_bytes()), - hs: *hs, - ck: *ck, + State::InitiationSent{hs, ck, ..} => { + // eph_sk already cleared by dalek-x25519 + hs.clear(); + ck.clear(); }, + _ => () } } } @@ -90,13 +86,6 @@ where } } - /// Return the state of the peer - /// - /// # Arguments - pub fn get_state(&self) -> State { - self.state.lock().clone() - } - /// Set the state of the peer unconditionally /// /// # Arguments @@ -152,4 +141,4 @@ where *last_initiation_consumption = Some(Instant::now()); Ok(()) } -} +} \ No newline at end of file diff --git a/src/types/endpoint.rs b/src/types/endpoint.rs index aa4dfd7..6bc99b9 100644 --- a/src/types/endpoint.rs +++ b/src/types/endpoint.rs @@ -1,8 +1,5 @@ use std::net::SocketAddr; -/* The generic implementation (not supporting "sticky-sockets"), - * is to simply use SocketAddr directly as the endpoint. - */ -pub trait Endpoint: Into {} +pub trait Endpoint: Into + From {} -impl Endpoint for T where T: Into {} +impl Endpoint for T where T: Into + From {} diff --git a/src/types/keys.rs b/src/types/keys.rs index c39816c..d2c4139 100644 --- a/src/types/keys.rs +++ b/src/types/keys.rs @@ -1,15 +1,23 @@ +use clear_on_drop::clear::Clear; use std::time::Instant; /* This file holds types passed between components. * Whenever a type cannot be held local to a single module. */ -#[derive(Debug, Clone, Copy)] +#[derive(Debug, Clone)] pub struct Key { pub key: [u8; 32], pub id: u32, } +// zero key on drop +impl Drop for Key { + fn drop(&mut self) { + self.key.clear() + } +} + #[cfg(test)] impl PartialEq for Key { fn eq(&self, other: &Self) -> bool { @@ -17,7 +25,7 @@ impl PartialEq for Key { } } -#[derive(Debug, Clone, Copy)] +#[derive(Debug, Clone)] pub struct KeyPair { pub birth: Instant, // when was the key-pair created pub initiator: bool, // has the key-pair been confirmed? diff --git a/src/types/tun.rs b/src/types/tun.rs index 72caa71..b36089e 100644 --- a/src/types/tun.rs +++ b/src/types/tun.rs @@ -1,6 +1,6 @@ use std::error; -pub trait Tun: Send + Sync { +pub trait Tun: Send + Sync + 'static { type Error: error::Error; /// Returns the MTU of the device @@ -22,13 +22,13 @@ pub trait Tun: Send + Sync { /// /// # Arguments /// - /// - dst: Destination buffer (enough space for MTU bytes + header) + /// - buf: Destination buffer (enough space for MTU bytes + header) /// - offset: Offset for the beginning of the IP packet /// /// # Returns /// /// The size of the IP packet (ignoring the header) or an std::error::Error instance: - fn read(&self, dst: &mut [u8], offset: usize) -> Result; + fn read(&self, buf: &mut [u8], offset: usize) -> Result; /// Writes an IP packet to the tunnel device /// -- cgit v1.2.3-59-g8ed1b