fixed threadsafety issue

This commit is contained in:
mamoniot 2023-03-09 14:29:40 -05:00
commit b369a1c375
No known key found for this signature in database
GPG key ID: ADCCDBBE0E3D3B3B
4 changed files with 26 additions and 31 deletions

View file

@ -3,6 +3,7 @@
// MacOS implementation of AES primitives since CommonCrypto seems to be faster than OpenSSL, especially on ARM64. // MacOS implementation of AES primitives since CommonCrypto seems to be faster than OpenSSL, especially on ARM64.
use std::os::raw::{c_int, c_void}; use std::os::raw::{c_int, c_void};
use std::ptr::{null, null_mut}; use std::ptr::{null, null_mut};
use std::sync::Mutex;
use crate::secret::Secret; use crate::secret::Secret;
use crate::secure_eq; use crate::secure_eq;
@ -173,14 +174,14 @@ impl AesGcm<false> {
pub struct Aes(*mut c_void, *mut c_void); pub struct Aes(Mutex<*mut c_void>, Mutex<*mut c_void>);
impl Drop for Aes { impl Drop for Aes {
#[inline(always)] #[inline(always)]
fn drop(&mut self) { fn drop(&mut self) {
unsafe { unsafe {
CCCryptorRelease(self.0); CCCryptorRelease(*self.0.lock().unwrap());
CCCryptorRelease(self.1); CCCryptorRelease(*self.1.lock().unwrap());
} }
} }
} }
@ -189,7 +190,7 @@ impl Aes {
pub fn new<const KEY_SIZE: usize>(k: &Secret<KEY_SIZE>) -> Self { pub fn new<const KEY_SIZE: usize>(k: &Secret<KEY_SIZE>) -> Self {
unsafe { unsafe {
debug_assert!(KEY_SIZE == 32 || KEY_SIZE == 24 || KEY_SIZE == 16, "AES supports 128, 192, or 256 bits keys"); debug_assert!(KEY_SIZE == 32 || KEY_SIZE == 24 || KEY_SIZE == 16, "AES supports 128, 192, or 256 bits keys");
let mut aes: Self = std::mem::zeroed(); let aes: Self = std::mem::zeroed();
assert_eq!( assert_eq!(
CCCryptorCreateWithMode( CCCryptorCreateWithMode(
kCCEncrypt, kCCEncrypt,
@ -203,7 +204,7 @@ impl Aes {
0, 0,
0, 0,
kCCOptionECBMode, kCCOptionECBMode,
&mut aes.0 &mut *aes.0.lock().unwrap()
), ),
0 0
); );
@ -220,7 +221,7 @@ impl Aes {
0, 0,
0, 0,
kCCOptionECBMode, kCCOptionECBMode,
&mut aes.1 &mut *aes.1.lock().unwrap()
), ),
0 0
); );
@ -229,20 +230,20 @@ impl Aes {
} }
#[inline(always)] #[inline(always)]
pub fn encrypt_block_in_place(&mut self, data: &mut [u8]) { pub fn encrypt_block_in_place(&self, data: &mut [u8]) {
assert_eq!(data.len(), 16); assert_eq!(data.len(), 16);
unsafe { unsafe {
let mut data_out_written = 0; let mut data_out_written = 0;
CCCryptorUpdate(self.0, data.as_ptr().cast(), 16, data.as_mut_ptr().cast(), 16, &mut data_out_written); CCCryptorUpdate(*self.0.lock().unwrap(), data.as_ptr().cast(), 16, data.as_mut_ptr().cast(), 16, &mut data_out_written);
} }
} }
#[inline(always)] #[inline(always)]
pub fn decrypt_block_in_place(&mut self, data: &mut [u8]) { pub fn decrypt_block_in_place(&self, data: &mut [u8]) {
assert_eq!(data.len(), 16); assert_eq!(data.len(), 16);
unsafe { unsafe {
let mut data_out_written = 0; let mut data_out_written = 0;
CCCryptorUpdate(self.1, data.as_ptr().cast(), 16, data.as_mut_ptr().cast(), 16, &mut data_out_written); CCCryptorUpdate(*self.1.lock().unwrap(), data.as_ptr().cast(), 16, data.as_mut_ptr().cast(), 16, &mut data_out_written);
} }
} }
} }

View file

@ -116,14 +116,14 @@ impl Aes {
/// Do not ever encrypt the same plaintext twice. Make sure data is always different between calls. /// Do not ever encrypt the same plaintext twice. Make sure data is always different between calls.
#[inline(always)] #[inline(always)]
pub fn encrypt_block_in_place(&mut self, data: &mut [u8]) { pub fn encrypt_block_in_place(&self, data: &mut [u8]) {
debug_assert_eq!(data.len(), AES_BLOCK_SIZE, "AesEcb should not be used to encrypt more than one block at a time unless you really know what you are doing."); debug_assert_eq!(data.len(), AES_BLOCK_SIZE, "AesEcb should not be used to encrypt more than one block at a time unless you really know what you are doing.");
let ptr = data.as_mut_ptr(); let ptr = data.as_mut_ptr();
unsafe { self.0.update::<true>(data, ptr).unwrap() } unsafe { self.0.update::<true>(data, ptr).unwrap() }
} }
/// Do not ever encrypt the same plaintext twice. Make sure data is always different between calls. /// Do not ever encrypt the same plaintext twice. Make sure data is always different between calls.
#[inline(always)] #[inline(always)]
pub fn decrypt_block_in_place(&mut self, data: &mut [u8]) { pub fn decrypt_block_in_place(&self, data: &mut [u8]) {
debug_assert_eq!(data.len(), AES_BLOCK_SIZE, "AesEcb should not be used to encrypt more than one block at a time unless you really know what you are doing."); debug_assert_eq!(data.len(), AES_BLOCK_SIZE, "AesEcb should not be used to encrypt more than one block at a time unless you really know what you are doing.");
let ptr = data.as_mut_ptr(); let ptr = data.as_mut_ptr();
unsafe { self.1.update::<false>(data, ptr).unwrap() } unsafe { self.1.update::<false>(data, ptr).unwrap() }

View file

@ -15,7 +15,6 @@ pub mod salsa;
pub mod typestate; pub mod typestate;
pub mod x25519; pub mod x25519;
/// NOTE: we assume that each aes library is threadsafe
pub mod aes_fruity; pub mod aes_fruity;
pub mod aes_openssl; pub mod aes_openssl;
#[cfg(target_os = "macos")] #[cfg(target_os = "macos")]

View file

@ -83,7 +83,7 @@ pub struct Session<Application: ApplicationLayer> {
psk: Secret<BASE_KEY_SIZE>, psk: Secret<BASE_KEY_SIZE>,
send_counter: AtomicU64, send_counter: AtomicU64,
receive_window: [AtomicU64; COUNTER_WINDOW_MAX_OOO], receive_window: [AtomicU64; COUNTER_WINDOW_MAX_OOO],
header_protection_cipher: Mutex<Aes>, header_protection_cipher: Aes,
state: RwLock<State>, state: RwLock<State>,
defrag: [Mutex<Fragged<Application::IncomingPacketBuffer, MAX_FRAGMENTS>>; COUNTER_WINDOW_MAX_OOO], defrag: [Mutex<Fragged<Application::IncomingPacketBuffer, MAX_FRAGMENTS>>; COUNTER_WINDOW_MAX_OOO],
} }
@ -216,7 +216,7 @@ impl<Application: ApplicationLayer> Context<Application> {
state.remote_session_id, state.remote_session_id,
0, 0,
2, 2,
Some(&mut *session.get_header_cipher()), Some(&session.header_protection_cipher),
); );
} }
false false
@ -314,7 +314,7 @@ impl<Application: ApplicationLayer> Context<Application> {
psk, psk,
send_counter: AtomicU64::new(3), // 1 and 2 are reserved for init and final ack send_counter: AtomicU64::new(3), // 1 and 2 are reserved for init and final ack
receive_window: std::array::from_fn(|_| AtomicU64::new(0)), receive_window: std::array::from_fn(|_| AtomicU64::new(0)),
header_protection_cipher: Mutex::new(Aes::new(&header_protection_key)), header_protection_cipher: Aes::new(&header_protection_key),
state: RwLock::new(State { state: RwLock::new(State {
remote_session_id: None, remote_session_id: None,
keys: [None, None], keys: [None, None],
@ -443,7 +443,7 @@ impl<Application: ApplicationLayer> Context<Application> {
debug_assert!(!self.sessions.read().unwrap().incoming.contains_key(&local_session_id)); debug_assert!(!self.sessions.read().unwrap().incoming.contains_key(&local_session_id));
session session
.get_header_cipher() .header_protection_cipher
.decrypt_block_in_place(&mut incoming_packet[HEADER_PROTECT_ENCRYPT_START..HEADER_PROTECT_ENCRYPT_END]); .decrypt_block_in_place(&mut incoming_packet[HEADER_PROTECT_ENCRYPT_START..HEADER_PROTECT_ENCRYPT_END]);
let (key_index, packet_type, fragment_count, fragment_no, incoming_counter) = parse_packet_header(&incoming_packet); let (key_index, packet_type, fragment_count, fragment_no, incoming_counter) = parse_packet_header(&incoming_packet);
@ -834,7 +834,7 @@ impl<Application: ApplicationLayer> Context<Application> {
Some(alice_session_id), Some(alice_session_id),
0, 0,
1, 1,
Some(&mut Aes::new(&header_protection_key)), Some(&Aes::new(&header_protection_key)),
)?; )?;
return Ok(ReceiveResult::Ok(session)); return Ok(ReceiveResult::Ok(session));
@ -979,7 +979,7 @@ impl<Application: ApplicationLayer> Context<Application> {
Some(bob_session_id), Some(bob_session_id),
0, 0,
2, 2,
Some(&mut *session.get_header_cipher()), Some(&session.header_protection_cipher),
)?; )?;
return Ok(ReceiveResult::Ok(Some(session))); return Ok(ReceiveResult::Ok(Some(session)));
@ -1076,7 +1076,7 @@ impl<Application: ApplicationLayer> Context<Application> {
psk, psk,
send_counter: AtomicU64::new(2), // 1 was already used during negotiation send_counter: AtomicU64::new(2), // 1 was already used during negotiation
receive_window: std::array::from_fn(|_| AtomicU64::new(0)), receive_window: std::array::from_fn(|_| AtomicU64::new(0)),
header_protection_cipher: Mutex::new(Aes::new(&incoming.header_protection_key)), header_protection_cipher: Aes::new(&incoming.header_protection_key),
state: RwLock::new(State { state: RwLock::new(State {
remote_session_id: Some(incoming.alice_session_id), remote_session_id: Some(incoming.alice_session_id),
keys: [ keys: [
@ -1165,7 +1165,7 @@ impl<Application: ApplicationLayer> Context<Application> {
drop(c); drop(c);
session session
.get_header_cipher() .header_protection_cipher
.encrypt_block_in_place(&mut reply_buf[HEADER_PROTECT_ENCRYPT_START..HEADER_PROTECT_ENCRYPT_END]); .encrypt_block_in_place(&mut reply_buf[HEADER_PROTECT_ENCRYPT_START..HEADER_PROTECT_ENCRYPT_END]);
send(Some(&session), &mut reply_buf); send(Some(&session), &mut reply_buf);
@ -1314,7 +1314,7 @@ impl<Application: ApplicationLayer> Session<Application> {
fragment_size = tagged_fragment_size; fragment_size = tagged_fragment_size;
} }
self.get_header_cipher() self.header_protection_cipher
.encrypt_block_in_place(&mut mtu_sized_buffer[HEADER_PROTECT_ENCRYPT_START..HEADER_PROTECT_ENCRYPT_END]); .encrypt_block_in_place(&mut mtu_sized_buffer[HEADER_PROTECT_ENCRYPT_START..HEADER_PROTECT_ENCRYPT_END]);
send(&mut mtu_sized_buffer[..fragment_size]); send(&mut mtu_sized_buffer[..fragment_size]);
} }
@ -1340,7 +1340,7 @@ impl<Application: ApplicationLayer> Session<Application> {
nop[HEADER_SIZE..].copy_from_slice(&c.finish_encrypt()); nop[HEADER_SIZE..].copy_from_slice(&c.finish_encrypt());
drop(c); drop(c);
set_packet_header(&mut nop, 1, 0, PACKET_TYPE_NOP, u64::from(remote_session_id), state.current_key, counter); set_packet_header(&mut nop, 1, 0, PACKET_TYPE_NOP, u64::from(remote_session_id), state.current_key, counter);
self.get_header_cipher() self.header_protection_cipher
.encrypt_block_in_place(&mut nop[HEADER_PROTECT_ENCRYPT_START..HEADER_PROTECT_ENCRYPT_END]); .encrypt_block_in_place(&mut nop[HEADER_PROTECT_ENCRYPT_START..HEADER_PROTECT_ENCRYPT_END]);
send(&mut nop); send(&mut nop);
} }
@ -1402,7 +1402,7 @@ impl<Application: ApplicationLayer> Session<Application> {
//drop(gcm); //drop(gcm);
//drop(state); //drop(state);
self.get_header_cipher() self.header_protection_cipher
.encrypt_block_in_place(&mut rekey_buf[HEADER_PROTECT_ENCRYPT_START..HEADER_PROTECT_ENCRYPT_END]); .encrypt_block_in_place(&mut rekey_buf[HEADER_PROTECT_ENCRYPT_START..HEADER_PROTECT_ENCRYPT_END]);
send(&mut rekey_buf); send(&mut rekey_buf);
@ -1433,11 +1433,6 @@ impl<Application: ApplicationLayer> Session<Application> {
let prev_counter = self.receive_window[(counter as usize) % COUNTER_WINDOW_MAX_OOO].fetch_max(counter, Ordering::AcqRel); let prev_counter = self.receive_window[(counter as usize) % COUNTER_WINDOW_MAX_OOO].fetch_max(counter, Ordering::AcqRel);
prev_counter < counter && counter.wrapping_sub(prev_counter) < COUNTER_WINDOW_MAX_SKIP_AHEAD prev_counter < counter && counter.wrapping_sub(prev_counter) < COUNTER_WINDOW_MAX_SKIP_AHEAD
} }
#[inline(always)]
fn get_header_cipher<'a>(&'a self) -> MutexGuard<'a, Aes>{
self.header_protection_cipher.lock().unwrap()
}
} }
#[inline(always)] #[inline(always)]
@ -1500,7 +1495,7 @@ fn send_with_fragmentation<SendFunction: FnMut(&mut [u8])>(
remote_session_id: Option<SessionId>, remote_session_id: Option<SessionId>,
key_index: usize, key_index: usize,
counter: u64, counter: u64,
mut header_protect_cipher: Option<&mut Aes>, header_protect_cipher: Option<&Aes>,
) -> Result<(), Error> { ) -> Result<(), Error> {
let packet_len = packet.len(); let packet_len = packet.len();
let recipient_session_id = remote_session_id.map_or(SessionId::NONE, |s| u64::from(s)); let recipient_session_id = remote_session_id.map_or(SessionId::NONE, |s| u64::from(s));
@ -1518,7 +1513,7 @@ fn send_with_fragmentation<SendFunction: FnMut(&mut [u8])>(
key_index, key_index,
counter, counter,
); );
if let Some(hcc) = &mut header_protect_cipher { if let Some(hcc) = header_protect_cipher {
hcc.encrypt_block_in_place(&mut fragment[HEADER_PROTECT_ENCRYPT_START..HEADER_PROTECT_ENCRYPT_END]); hcc.encrypt_block_in_place(&mut fragment[HEADER_PROTECT_ENCRYPT_START..HEADER_PROTECT_ENCRYPT_END]);
} }
send(fragment); send(fragment);