From c14f2edff694937170612987f4ea6679cc570af8 Mon Sep 17 00:00:00 2001 From: mamoniot Date: Thu, 9 Mar 2023 11:52:52 -0500 Subject: [PATCH] cleared errors --- crypto/benches/benchmark_crypto.rs | 43 ------------ crypto/src/aes.rs | 104 +++++++++-------------------- crypto/src/secret.rs | 9 ++- zssp/src/zssp.rs | 49 ++++++-------- 4 files changed, 58 insertions(+), 147 deletions(-) delete mode 100644 crypto/benches/benchmark_crypto.rs diff --git a/crypto/benches/benchmark_crypto.rs b/crypto/benches/benchmark_crypto.rs deleted file mode 100644 index fd01855c9..000000000 --- a/crypto/benches/benchmark_crypto.rs +++ /dev/null @@ -1,43 +0,0 @@ -use criterion::{criterion_group, criterion_main, Criterion}; -use std::time::Duration; - -use zerotier_crypto::mimcvdf; -use zerotier_crypto::p384::*; -use zerotier_crypto::x25519::*; - -pub fn criterion_benchmark(c: &mut Criterion) { - let mut group = c.benchmark_group("cryptography"); - - let mut input = 1; - let mut proof = 0; - group.bench_function("mimcvdf::delay(1000)", |b| { - b.iter(|| { - input += 1; - proof = mimcvdf::delay(input, 1000); - }) - }); - group.bench_function("mimcvdf::verify(1000)", |b| { - b.iter(|| { - assert!(mimcvdf::verify(proof, input, 1000)); - }) - }); - - let p384_a = P384KeyPair::generate(); - let p384_b = P384KeyPair::generate(); - - let x25519_a = X25519KeyPair::generate(); - let x25519_b = X25519KeyPair::generate(); - let x25519_b_pub = x25519_b.public_bytes(); - - group.measurement_time(Duration::new(10, 0)); - - group.bench_function("ecdhp384", |b| { - b.iter(|| p384_a.agree(p384_b.public_key()).expect("ecdhp384 failed")) - }); - group.bench_function("ecdhx25519", |b| b.iter(|| x25519_a.agree(&x25519_b_pub))); - - group.finish(); -} - -criterion_group!(benches, criterion_benchmark); -criterion_main!(benches); diff --git a/crypto/src/aes.rs b/crypto/src/aes.rs index bc4f3d889..10b32840b 100644 --- a/crypto/src/aes.rs +++ b/crypto/src/aes.rs @@ -14,7 +14,7 @@ pub struct AesGcm (CipherCtx); impl AesGcm { /// Create an AesGcm context with the given key, key must be 16, 24 or 32 bytes long. - /// OpenSSL internally processes and caches this key, so it is recommended to reuse this context whenever encrypting under the same key. Call `set_iv` to change the IV for each reuse. + /// OpenSSL internally processes and caches this key, so it is recommended to reuse this context whenever encrypting under the same key. Call `reset_init_gcm` to change the IV for each reuse. pub fn new(key: &Secret) -> Self { let ctx = CipherCtx::new().unwrap(); unsafe { @@ -34,7 +34,7 @@ impl AesGcm { /// Set the IV of this AesGcm context. This call resets the IV but leaves the key and encryption algorithm alone. /// This method must be called before any other method on AesGcm. /// `iv` must be exactly 12 bytes in length, because that is what Aes supports. - pub fn set_iv(&self, iv: &[u8]) { + pub fn reset_init_gcm(&self, iv: &[u8]) { debug_assert_eq!(iv.len(), 12, "Aes IV must be 12 bytes long"); unsafe { self.0.cipher_init::(ptr::null(), ptr::null(), iv.as_ptr()).unwrap(); @@ -86,68 +86,18 @@ impl AesGcm { } } - - -/// An OpenSSL AES_CTR context. Automatically frees itself on drop. AES_CTR is similar to AES_GCM except it produces no authentication tag. -/// Whether `ENCRYPT` is true or false decides respectively whether this context encrypts or decrypts. -/// Even though OpenSSL lets you set this dynamically almost no operations work when you do this without resetting the context. -pub struct AesCtr (CipherCtx); - -impl AesCtr { - /// Create an AesCtr context with the given key, key must be 16, 24 or 32 bytes long. - /// OpenSSL internally processes and caches this key, so it is recommended to reuse this context whenever encrypting under the same key. Call `set_iv` to change the IV for each reuse. - pub fn new(key: &Secret) -> Self { - let ctx = CipherCtx::new().unwrap(); - unsafe { - let t = match KEY_SIZE { - 16 => ffi::EVP_aes_128_ctr(), - 24 => ffi::EVP_aes_192_ctr(), - 32 => ffi::EVP_aes_256_ctr(), - _ => panic!("Aes KEY_SIZE must be 16, 24 or 32") - }; - ctx.cipher_init::(t, key.as_ptr(), ptr::null()).unwrap(); - ffi::EVP_CIPHER_CTX_set_padding(ctx.as_ptr(), 0); - } - let ret = AesCtr(ctx); - ret - } - - /// Set the IV of this AesCtr context. This call resets the IV but leaves the key and encryption algorithm alone. - /// This method must be called before any other method on AesCtr. - /// `iv` must be exactly 12 bytes in length, because that is what Aes supports. - pub fn set_iv(&self, iv: &[u8]) { - debug_assert_eq!(iv.len(), 12, "Aes IV must be 12 bytes long"); - unsafe { - self.0.cipher_init::(ptr::null(), ptr::null(), iv.as_ptr()).unwrap(); - } - } - - /// Encrypt or decrypt. - #[inline(always)] - pub fn crypt(&self, input: &[u8], output: &mut [u8]) { - debug_assert!(output.len() >= input.len(), "output buffer must fit the size of the input buffer"); - unsafe { self.0.update::(input, output.as_mut_ptr()).unwrap() }; - } - - /// Encrypt or decrypt in place. - #[inline(always)] - pub fn crypt_in_place(&self, data: &mut [u8]) { - let ptr = data.as_mut_ptr(); - unsafe { self.0.update::(data, ptr).unwrap() } - } -} - const AES_BLOCK_SIZE: usize = 16; /// An OpenSSL AES_ECB context. Automatically frees itself on drop. /// AES_ECB is very insecure if used incorrectly so its public interface supports only exactly what ZeroTier uses it for. -pub struct AesEcb (CipherCtx); +pub struct Aes(CipherCtx, CipherCtx); -impl AesEcb { +impl Aes { /// Create an AesEcb context with the given key, key must be 16, 24 or 32 bytes long. /// OpenSSL internally processes and caches this key, so it is recommended to reuse this context whenever encrypting under the same key. pub fn new(key: &Secret) -> Self { - let ctx = CipherCtx::new().unwrap(); + let ctx0 = CipherCtx::new().unwrap(); + let ctx1 = CipherCtx::new().unwrap(); unsafe { let t = match KEY_SIZE { 16 => ffi::EVP_aes_128_ecb(), @@ -155,20 +105,28 @@ impl AesEcb { 32 => ffi::EVP_aes_256_ecb(), _ => panic!("Aes KEY_SIZE must be 16, 24 or 32") }; - ctx.cipher_init::(t, key.as_ptr(), ptr::null()).unwrap(); - ffi::EVP_CIPHER_CTX_set_padding(ctx.as_ptr(), 0); - debug_assert_eq!(ffi::EVP_CIPHER_CTX_get_block_size(ctx.as_ptr()) as usize, AES_BLOCK_SIZE, "Aes block size is incorrect, something is very wrong."); + ctx0.cipher_init::(t, key.as_ptr(), ptr::null()).unwrap(); + ffi::EVP_CIPHER_CTX_set_padding(ctx0.as_ptr(), 0); + ctx1.cipher_init::(t, key.as_ptr(), ptr::null()).unwrap(); + ffi::EVP_CIPHER_CTX_set_padding(ctx1.as_ptr(), 0); } - let ret = AesEcb(ctx); + let ret = Aes(ctx0, ctx1); ret } /// Do not ever encrypt the same plaintext twice. Make sure data is always different between calls. #[inline(always)] - pub fn crypt_in_place(&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."); let ptr = data.as_mut_ptr(); - unsafe { self.0.update::(data, ptr).unwrap() } + unsafe { self.0.update::(data, ptr).unwrap() } + } + /// Do not ever encrypt the same plaintext twice. Make sure data is always different between calls. + #[inline(always)] + 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."); + let ptr = data.as_mut_ptr(); + unsafe { self.1.update::(data, ptr).unwrap() } } } @@ -193,31 +151,31 @@ mod test { let mut cipher_out = [0u8; 127]; let mut plain_out = [0u8; 127]; - enc.set_iv(&iv0); + enc.reset_init_gcm(&iv0); enc.crypt(&plain, &mut cipher_out); tag_out = enc.finish_encrypt(); - dec.set_iv(&iv0); + dec.reset_init_gcm(&iv0); dec.crypt(&cipher_out, &mut plain_out); assert!(dec.finish_decrypt(&tag_out)); assert_eq!(plain, plain_out); - enc.set_iv(&iv1); + enc.reset_init_gcm(&iv1); enc.crypt(&plain, &mut cipher_out); tag_out = enc.finish_encrypt(); - dec.set_iv(&iv1); + dec.reset_init_gcm(&iv1); dec.crypt(&cipher_out, &mut plain_out); assert!(dec.finish_decrypt(&tag_out)); assert_eq!(plain, plain_out); - enc.set_iv(&iv0); + enc.reset_init_gcm(&iv0); enc.crypt(&plain, &mut cipher_out); tag_out = enc.finish_encrypt(); - dec.set_iv(&iv1); + dec.reset_init_gcm(&iv1); dec.crypt(&cipher_out, &mut plain_out); assert!(!dec.finish_decrypt(&tag_out)); } @@ -238,7 +196,7 @@ mod test { let benchmark_iterations: usize = 80000; let start = SystemTime::now(); for _ in 0..benchmark_iterations { - c.set_iv(&iv); + c.reset_init_gcm(&iv); c.crypt_in_place(&mut buf); } let duration = SystemTime::now().duration_since(start).unwrap(); @@ -251,7 +209,7 @@ mod test { let start = SystemTime::now(); for _ in 0..benchmark_iterations { - c.set_iv(&iv); + c.reset_init_gcm(&iv); c.crypt_in_place(&mut buf); } let duration = SystemTime::now().duration_since(start).unwrap(); @@ -266,7 +224,7 @@ mod test { // Even though we are just wrapping other implementations, it's still good to test thoroughly! for tv in NIST_AES_GCM_TEST_VECTORS.iter() { let gcm = AesGcm::new(unsafe { &Secret::<32>::from_bytes(tv.key) }); - gcm.set_iv(tv.nonce); + gcm.reset_init_gcm(tv.nonce); gcm.aad(tv.aad); let mut ciphertext = Vec::new(); ciphertext.resize(tv.plaintext.len(), 0); @@ -276,13 +234,13 @@ mod test { assert!(ciphertext.as_slice().eq(tv.ciphertext)); let gcm = AesGcm::new(unsafe { &Secret::<32>::from_bytes(tv.key) }); - gcm.set_iv(tv.nonce); + gcm.reset_init_gcm(tv.nonce); gcm.aad(tv.aad); let mut ct_copy = ciphertext.clone(); gcm.crypt_in_place(ct_copy.as_mut()); assert!(gcm.finish_decrypt(&tag)); - gcm.set_iv(tv.nonce); + gcm.reset_init_gcm(tv.nonce); gcm.aad(tv.aad); gcm.crypt_in_place(ciphertext.as_mut()); tag[0] ^= 1; diff --git a/crypto/src/secret.rs b/crypto/src/secret.rs index 79c3727b2..fb946ff38 100644 --- a/crypto/src/secret.rs +++ b/crypto/src/secret.rs @@ -35,9 +35,12 @@ impl Secret { /// Copy bytes into secret, then nuke the previous value, will panic if the slice does not match the size of this secret. #[inline(always)] pub fn from_bytes_then_nuke(b: &mut [u8]) -> Self { - let ret = Self (b.try_into().unwrap()); - unsafe { OPENSSL_cleanse(b.as_mut_ptr().cast(), L) }; - ret + let mut k = [0u8; L]; + k.copy_from_slice(b); + unsafe { + OPENSSL_cleanse(b.as_mut_ptr().cast(), L) + }; + Self (k) } #[inline(always)] pub unsafe fn from_bytes(b: &[u8]) -> Self { diff --git a/zssp/src/zssp.rs b/zssp/src/zssp.rs index 129d90c69..3a81b5d8f 100644 --- a/zssp/src/zssp.rs +++ b/zssp/src/zssp.rs @@ -134,8 +134,8 @@ struct SessionKey { ratchet_key: Secret, // Key used in derivation of the next session key receive_key: Secret, // Receive side AES-GCM key send_key: Secret, // Send side AES-GCM key - receive_cipher_pool: Mutex>>, // Pool of reusable sending ciphers - send_cipher_pool: Mutex>>, // Pool of reusable receiving ciphers + receive_cipher_pool: Mutex>>>, // Pool of reusable sending ciphers + send_cipher_pool: Mutex>>>, // Pool of reusable receiving ciphers rekey_at_time: i64, // Rekey at or after this time (ticks) created_at_counter: u64, // Counter at which session was created rekey_at_counter: u64, // Rekey at or after this counter @@ -313,7 +313,7 @@ impl Context { psk, send_counter: AtomicU64::new(3), // 1 and 2 are reserved for init and final ack receive_window: std::array::from_fn(|_| AtomicU64::new(0)), - header_protection_cipher: Aes::new(header_protection_key.as_bytes()), + header_protection_cipher: Aes::new(&header_protection_key), state: RwLock::new(State { remote_session_id: None, keys: [None, None], @@ -357,8 +357,7 @@ impl Context { // Encrypt and add authentication tag. let mut gcm = AesGcm::new( - kbkdf::(noise_es.as_bytes()).as_bytes(), - true, + &kbkdf::(noise_es.as_bytes()) ); gcm.reset_init_gcm(&create_message_nonce(PACKET_TYPE_ALICE_NOISE_XK_INIT, 1)); gcm.aad(&offer.noise_h); @@ -493,7 +492,7 @@ impl Context { } } else { if let Some(i) = self.sessions.read().unwrap().incoming.get(&local_session_id).cloned() { - Aes::new(i.header_protection_key.as_bytes()) + Aes::new(&i.header_protection_key) .decrypt_block_in_place(&mut incoming_packet[HEADER_PROTECT_ENCRYPT_START..HEADER_PROTECT_ENCRYPT_END]); incoming = Some(i); } else { @@ -731,8 +730,7 @@ impl Context { // Decrypt and authenticate init packet, also proving that caller knows our static identity. let mut gcm = AesGcm::new( - kbkdf::(noise_es.as_bytes()).as_bytes(), - false, + &kbkdf::(noise_es.as_bytes()) ); gcm.reset_init_gcm(&incoming_message_nonce); gcm.aad(&noise_h); @@ -782,8 +780,7 @@ impl Context { // Encrypt main section of reply and attach tag. let mut gcm = AesGcm::new( - kbkdf::(noise_es_ee.as_bytes()).as_bytes(), - true, + &kbkdf::(noise_es_ee.as_bytes()) ); gcm.reset_init_gcm(&create_message_nonce(PACKET_TYPE_BOB_NOISE_XK_ACK, 1)); gcm.aad(&noise_h_next); @@ -836,7 +833,7 @@ impl Context { Some(alice_session_id), 0, 1, - Some(&Aes::new(header_protection_key.as_bytes())), + Some(&Aes::new(&header_protection_key)), )?; return Ok(ReceiveResult::Ok(session)); @@ -885,8 +882,7 @@ impl Context { // Decrypt and authenticate Bob's reply. let mut gcm = AesGcm::new( - kbkdf::(noise_es_ee.as_bytes()).as_bytes(), - false, + &kbkdf::(noise_es_ee.as_bytes()) ); gcm.reset_init_gcm(&incoming_message_nonce); gcm.aad(&outgoing_offer.noise_h); @@ -931,12 +927,10 @@ impl Context { reply_len = append_to_slice(&mut reply_buffer, reply_len, alice_s_public_blob)?; let mut gcm = AesGcm::new( - kbkdf::(&hmac_sha512( + &kbkdf::(&hmac_sha512( noise_es_ee.as_bytes(), hk.as_bytes(), )) - .as_bytes(), - true, ); gcm.reset_init_gcm(&reply_message_nonce); gcm.aad(&noise_h_next); @@ -954,8 +948,7 @@ impl Context { reply_len = append_to_slice(&mut reply_buffer, reply_len, metadata)?; let mut gcm = AesGcm::new( - kbkdf::(noise_es_ee_se_hk_psk.as_bytes()).as_bytes(), - true, + &kbkdf::(noise_es_ee_se_hk_psk.as_bytes()) ); gcm.reset_init_gcm(&reply_message_nonce); gcm.aad(&noise_h_next); @@ -1082,7 +1075,7 @@ impl Context { psk, send_counter: AtomicU64::new(2), // 1 was already used during negotiation receive_window: std::array::from_fn(|_| AtomicU64::new(0)), - header_protection_cipher: Aes::new(incoming.header_protection_key.as_bytes()), + header_protection_cipher: Aes::new(&incoming.header_protection_key), state: RwLock::new(State { remote_session_id: Some(incoming.alice_session_id), keys: [ @@ -1587,14 +1580,14 @@ impl SessionKey { } } - fn get_send_cipher(&self, counter: u64) -> Result, Error> { + fn get_send_cipher(&self, counter: u64) -> Result>, Error> { if counter < self.expire_at_counter { Ok(self .send_cipher_pool .lock() .unwrap() .pop() - .unwrap_or_else(|| Box::new(AesGcm::new(self.send_key.as_bytes(), true)))) + .unwrap_or_else(|| Box::new(AesGcm::new(&self.send_key)))) } else { // Not only do we return an error, but we also destroy the key. let mut scp = self.send_cipher_pool.lock().unwrap(); @@ -1605,19 +1598,19 @@ impl SessionKey { } } - fn return_send_cipher(&self, c: Box) { + fn return_send_cipher(&self, c: Box>) { self.send_cipher_pool.lock().unwrap().push(c); } - fn get_receive_cipher(&self) -> Box { + fn get_receive_cipher(&self) -> Box> { self.receive_cipher_pool .lock() .unwrap() .pop() - .unwrap_or_else(|| Box::new(AesGcm::new(self.receive_key.as_bytes(), false))) + .unwrap_or_else(|| Box::new(AesGcm::new(&self.receive_key))) } - fn return_receive_cipher(&self, c: Box) { + fn return_receive_cipher(&self, c: Box>) { self.receive_cipher_pool.lock().unwrap().push(c); } } @@ -1640,7 +1633,7 @@ impl<'a> PktReader<'a> { fn read_decrypt_auth<'b>(&'b mut self, l: usize, k: Secret, gcm_aad: &[u8], nonce: &[u8]) -> Result<&'b [u8], Error> { let mut tmp = self.1 + l; if (tmp + AES_GCM_TAG_SIZE) <= self.0.len() { - let mut gcm = AesGcm::new(k.as_bytes(), false); + let mut gcm = AesGcm::new(&k); gcm.reset_init_gcm(nonce); gcm.aad(gcm_aad); gcm.crypt_in_place(&mut self.0[self.1..tmp]); @@ -1683,8 +1676,8 @@ fn mix_hash(h: &[u8; SHA384_HASH_SIZE], m: &[u8]) -> [u8; SHA384_HASH_SIZE] { fn kbkdf(key: &[u8]) -> Secret { //These are the values we have assigned to the 5 variables involved in https://csrc.nist.gov/publications/detail/sp/800-108/final: // K_in = key, i = 0x01, Label = 'Z'||'T'||LABEL, Context = 0x00, L = (OUTPUT_BYTES * 8) - Secret::::from_bytes( - &hmac_sha512( + Secret::::from_bytes_then_nuke( + &mut hmac_sha512( key, &[ 1,