Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor!: make functions infallible where possible #366

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 6 additions & 24 deletions boringtun/src/device/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -334,8 +334,7 @@ impl Device {
keepalive,
next_index,
None,
)
.unwrap();
);

let peer = Peer::new(tunn, next_index, endpoint, allowed_ips, preshared_key);

Expand Down Expand Up @@ -453,8 +452,6 @@ impl Device {
}

fn set_key(&mut self, private_key: x25519::StaticSecret) {
let mut bad_peers = vec![];

let public_key = x25519::PublicKey::from(&private_key);
let key_pair = Some((private_key.clone(), public_key));

Expand All @@ -467,30 +464,15 @@ impl Device {
let rate_limiter = Arc::new(RateLimiter::new(&public_key, HANDSHAKE_RATE_LIMIT));

for peer in self.peers.values_mut() {
let mut peer_mut = peer.lock();

if peer_mut
.tunnel
.set_static_private(
private_key.clone(),
public_key,
Some(Arc::clone(&rate_limiter)),
)
.is_err()
{
// In case we encounter an error, we will remove that peer
// An error will be a result of bad public key/secret key combination
bad_peers.push(Arc::clone(peer));
}
peer.lock().tunnel.set_static_private(
private_key.clone(),
public_key,
Some(Arc::clone(&rate_limiter)),
)
}

self.key_pair = key_pair;
self.rate_limiter = Some(rate_limiter);

// Remove all the bad peers
for _ in bad_peers {
unimplemented!();
}
}

#[cfg(any(target_os = "android", target_os = "fuchsia", target_os = "linux"))]
Expand Down
7 changes: 2 additions & 5 deletions boringtun/src/ffi/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -292,17 +292,14 @@ pub unsafe extern "C" fn new_tunnel(
Some(keep_alive)
};

let tunnel = match Tunn::new(
let tunnel = Box::new(Mutex::new(Tunn::new(
private_key,
public_key,
preshared_key,
keep_alive,
index,
None,
) {
Ok(t) => Box::new(Mutex::new(t)),
Err(_) => return ptr::null_mut(),
};
)));

PANIC_HOOK.call_once(|| {
// FFI won't properly unwind on panic, but it will if we cause a segmentation fault
Expand Down
19 changes: 9 additions & 10 deletions boringtun/src/noise/handshake.rs
Original file line number Diff line number Diff line change
Expand Up @@ -375,27 +375,27 @@ impl NoiseParams {
static_public: x25519::PublicKey,
peer_static_public: x25519::PublicKey,
preshared_key: Option<[u8; 32]>,
) -> Result<NoiseParams, WireGuardError> {
) -> NoiseParams {
let static_shared = static_private.diffie_hellman(&peer_static_public);

let initial_sending_mac_key = b2s_hash(LABEL_MAC1, peer_static_public.as_bytes());

Ok(NoiseParams {
NoiseParams {
static_public,
static_private,
peer_static_public,
static_shared,
sending_mac1_key: initial_sending_mac_key,
preshared_key,
})
}
}

/// Set a new private key
fn set_static_private(
&mut self,
static_private: x25519::StaticSecret,
static_public: x25519::PublicKey,
) -> Result<(), WireGuardError> {
) {
// Check that the public key indeed matches the private key
let check_key = x25519::PublicKey::from(&static_private);
assert_eq!(check_key.as_bytes(), static_public.as_bytes());
Expand All @@ -404,7 +404,6 @@ impl NoiseParams {
self.static_public = static_public;

self.static_shared = self.static_private.diffie_hellman(&self.peer_static_public);
Ok(())
}
}

Expand All @@ -415,15 +414,15 @@ impl Handshake {
peer_static_public: x25519::PublicKey,
global_idx: u32,
preshared_key: Option<[u8; 32]>,
) -> Result<Handshake, WireGuardError> {
) -> Handshake {
let params = NoiseParams::new(
static_private,
static_public,
peer_static_public,
preshared_key,
)?;
);

Ok(Handshake {
Handshake {
params,
next_index: global_idx,
previous: HandshakeState::None,
Expand All @@ -432,7 +431,7 @@ impl Handshake {
stamper: TimeStamper::new(),
cookies: Default::default(),
last_rtt: None,
})
}
}

pub(crate) fn is_in_progress(&self) -> bool {
Expand Down Expand Up @@ -475,7 +474,7 @@ impl Handshake {
&mut self,
private_key: x25519::StaticSecret,
public_key: x25519::PublicKey,
) -> Result<(), WireGuardError> {
) {
self.params.set_static_private(private_key, public_key)
}

Expand Down
21 changes: 8 additions & 13 deletions boringtun/src/noise/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -198,18 +198,17 @@ impl Tunn {
persistent_keepalive: Option<u16>,
index: u32,
rate_limiter: Option<Arc<RateLimiter>>,
) -> Result<Self, &'static str> {
) -> Self {
let static_public = x25519::PublicKey::from(&static_private);

let tunn = Tunn {
Tunn {
handshake: Handshake::new(
static_private,
static_public,
peer_static_public,
index << 8,
preshared_key,
)
.map_err(|_| "Invalid parameters")?,
),
sessions: Default::default(),
current: Default::default(),
tx_bytes: Default::default(),
Expand All @@ -221,9 +220,7 @@ impl Tunn {
rate_limiter: rate_limiter.unwrap_or_else(|| {
Arc::new(RateLimiter::new(&static_public, PEER_HANDSHAKE_RATE_LIMIT))
}),
};

Ok(tunn)
}
}

/// Update the private key and clear existing sessions
Expand All @@ -232,17 +229,16 @@ impl Tunn {
static_private: x25519::StaticSecret,
static_public: x25519::PublicKey,
rate_limiter: Option<Arc<RateLimiter>>,
) -> Result<(), WireGuardError> {
) {
self.timers.should_reset_rr = rate_limiter.is_none();
self.rate_limiter = rate_limiter.unwrap_or_else(|| {
Arc::new(RateLimiter::new(&static_public, PEER_HANDSHAKE_RATE_LIMIT))
});
self.handshake
.set_static_private(static_private, static_public)?;
.set_static_private(static_private, static_public);
for s in &mut self.sessions {
*s = None;
}
Ok(())
}

/// Encapsulate a single packet from the tunnel interface.
Expand Down Expand Up @@ -606,10 +602,9 @@ mod tests {
let their_public_key = x25519_dalek::PublicKey::from(&their_secret_key);
let their_idx = OsRng.next_u32();

let my_tun = Tunn::new(my_secret_key, their_public_key, None, None, my_idx, None).unwrap();
let my_tun = Tunn::new(my_secret_key, their_public_key, None, None, my_idx, None);

let their_tun =
Tunn::new(their_secret_key, my_public_key, None, None, their_idx, None).unwrap();
let their_tun = Tunn::new(their_secret_key, my_public_key, None, None, their_idx, None);

(my_tun, their_tun)
}
Expand Down
Loading