aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMathias Hall-Andersen <mathias@hall-andersen.dk>2019-08-31 15:46:18 +0200
committerMathias Hall-Andersen <mathias@hall-andersen.dk>2019-08-31 15:46:18 +0200
commit7e5852ec263cb8ba8feb84f549be54e9fdcd57e7 (patch)
treeca7068193b5688172a04965cc687ba8a058f62ea
parentBetter management of key material (diff)
downloadwireguard-rs-7e5852ec263cb8ba8feb84f549be54e9fdcd57e7.tar.xz
wireguard-rs-7e5852ec263cb8ba8feb84f549be54e9fdcd57e7.zip
Fix race condition on response processing
-rw-r--r--src/handshake/noise.rs80
1 files changed, 52 insertions, 28 deletions
diff --git a/src/handshake/noise.rs b/src/handshake/noise.rs
index db1a244..5673938 100644
--- a/src/handshake/noise.rs
+++ b/src/handshake/noise.rs
@@ -1,3 +1,5 @@
+#![allow(dead_code)]
+
// DH
use x25519_dalek::PublicKey;
use x25519_dalek::StaticSecret;
@@ -16,6 +18,7 @@ use generic_array::typenum::*;
use generic_array::*;
use clear_on_drop::clear_stack_on_return;
+use subtle::ConstantTimeEq;
use super::device::Device;
use super::messages::{NoiseInitiation, NoiseResponse};
@@ -279,12 +282,12 @@ pub fn create_initiation<T: Copy, R: RngCore + CryptoRng>(
// update state of peer
- peer.set_state(State::InitiationSent {
+ *peer.state.lock() = State::InitiationSent {
hs,
ck,
eph_sk,
sender,
- });
+ };
Ok(())
})
@@ -295,7 +298,7 @@ pub fn consume_initiation<'a, T: Copy>(
msg: &NoiseInitiation,
) -> Result<(&'a Peer<T>, TemporaryState), HandshakeError> {
clear_stack_on_return(CLEAR_PAGES, || {
- // initialize state
+ // initialize new state
let ck = INITIAL_CK;
let hs = INITIAL_HS;
@@ -327,6 +330,10 @@ pub fn consume_initiation<'a, T: Copy>(
let peer = device.lookup_pk(&PublicKey::from(pk))?;
+ // reset initiation state
+
+ *peer.state.lock() = State::Reset;
+
// H := Hash(H || msg.static)
let hs = HASH!(&hs, &msg.f_static[..]);
@@ -354,9 +361,6 @@ pub fn consume_initiation<'a, T: Copy>(
let hs = HASH!(&hs, &msg.f_timestamp);
- // clear initiation state
- *peer.state.lock() = State::Reset;
-
// return state (to create response)
Ok((peer, (msg.f_sender.get(), eph_r_pk, hs, ck)))
@@ -425,6 +429,7 @@ pub fn create_response<T: Copy, R: RngCore + CryptoRng>(
// let hs = HASH!(&hs, &msg.f_empty_tag);
// derive key-pair
+
let (key_recv, key_send) = KDF2!(&ck, &[]);
// return unconfirmed key-pair
@@ -444,13 +449,18 @@ pub fn create_response<T: Copy, R: RngCore + CryptoRng>(
})
}
+/* The state lock is released while processing the message to
+ * allow concurrent processing of potential responses to the initiation,
+ * in order to better mitigate DoS from malformed response messages.
+ */
pub fn consume_response<T: Copy>(
device: &Device<T>,
msg: &NoiseResponse,
) -> Result<Output<T>, HandshakeError> {
clear_stack_on_return(CLEAR_PAGES, || {
- // retrieve peer and copy associated state
+ // retrieve peer and copy initiation state
let peer = device.lookup_id(msg.f_receiver.get())?;
+
let (hs, ck, sender, eph_sk) = match *peer.state.lock() {
State::InitiationSent {
hs,
@@ -497,29 +507,43 @@ pub fn consume_response<T: Copy>(
// derive key-pair
+ let birth = Instant::now();
let (key_send, key_recv) = KDF2!(&ck, &[]);
- // null the state
-
- *peer.state.lock() = State::Reset;
+ // check for new initiation sent while lock released
- // 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(),
- },
- }),
- ))
+ let mut state = peer.state.lock();
+ let update = match *state {
+ State::InitiationSent {
+ eph_sk: ref old, ..
+ } => old.to_bytes().ct_eq(&eph_sk.to_bytes()).into(),
+ _ => false,
+ };
+
+ if update {
+ // null the initiation state
+ // (to avoid replay of this response message)
+ *state = State::Reset;
+
+ // return confirmed key-pair
+ Ok((
+ Some(peer.identifier),
+ None,
+ Some(KeyPair {
+ birth,
+ initiator: true,
+ send: Key {
+ id: sender,
+ key: key_send.into(),
+ },
+ recv: Key {
+ id: msg.f_sender.get(),
+ key: key_recv.into(),
+ },
+ }),
+ ))
+ } else {
+ Err(HandshakeError::InvalidState)
+ }
})
}