mbedTLS lifetimes to reference counting

Author: Adrian Cruceru

Cargo.lock

@@ -39,7 +39,7 @@ features = ["custom_printf", "trusted_cert_callback"]
 path = "../mbedtls-sys"
 
 [dev-dependencies]
-libc = "0.2.0"
+libc = "0.2.69"
 rand = "0.4.0"
 serde_cbor = "0.6"
 hex = "0.3"

mbedtls/Cargo.toml

@@ -16,28 +16,29 @@ use mbedtls::ssl::config::{Endpoint, Preset, Transport};
 use mbedtls::ssl::{Config, Context};
 use mbedtls::x509::Certificate;
 use mbedtls::Result as TlsResult;
+use std::sync::Arc;
 
 #[path = "../tests/support/mod.rs"]
 mod support;
 use support::entropy::entropy_new;
 use support::keys;
 
 fn result_main(addr: &str) -> TlsResult<()> {
-    let mut entropy = entropy_new();
-    let mut rng = CtrDrbg::new(&mut entropy, None)?;
-    let mut cert = Certificate::from_pem(keys::PEM_CERT)?;
+    let entropy = Arc::new(entropy_new());
+    let rng = Arc::new(CtrDrbg::new(entropy, None)?);
+    let cert = Arc::new(Certificate::from_pem(keys::PEM_CERT)?);
     let mut config = Config::new(Endpoint::Client, Transport::Stream, Preset::Default);
-    config.set_rng(Some(&mut rng));
-    config.set_ca_list(Some(&mut *cert), None);
-    let mut ctx = Context::new(&config)?;
+    config.set_rng(rng);
+    config.set_ca_list(cert, None);
+    let mut ctx = Context::new(Arc::new(config));
 
-    let mut conn = TcpStream::connect(addr).unwrap();
-    let mut session = ctx.establish(&mut conn, None)?;
+    let conn = TcpStream::connect(addr).unwrap();
+    ctx.establish(conn, None)?;
 
     let mut line = String::new();
     stdin().read_line(&mut line).unwrap();
-    session.write_all(line.as_bytes()).unwrap();
-    io::copy(&mut session, &mut stdout()).unwrap();
+    ctx.write_all(line.as_bytes()).unwrap();
+    io::copy(&mut ctx, &mut stdout()).unwrap();
     Ok(())
 }
 

mbedtls/examples/client.rs

@@ -17,6 +17,7 @@ use mbedtls::ssl::config::{Endpoint, Preset, Transport};
 use mbedtls::ssl::{Config, Context};
 use mbedtls::x509::Certificate;
 use mbedtls::Result as TlsResult;
+use std::sync::Arc;
 
 #[path = "../tests/support/mod.rs"]
 mod support;
@@ -29,21 +30,25 @@ fn listen<E, F: FnMut(TcpStream) -> Result<(), E>>(mut handle_client: F) -> Resu
         println!("Connection from {}", conn.peer_addr().unwrap());
         handle_client(conn)?;
     }
+
     Ok(())
 }
 
 fn result_main() -> TlsResult<()> {
-    let mut entropy = entropy_new();
-    let mut rng = CtrDrbg::new(&mut entropy, None)?;
-    let mut cert = Certificate::from_pem(keys::PEM_CERT)?;
-    let mut key = Pk::from_private_key(keys::PEM_KEY, None)?;
+    let entropy = entropy_new();
+    let rng = Arc::new(CtrDrbg::new(Arc::new(entropy), None)?);
+    let cert = Arc::new(Certificate::from_pem(keys::PEM_CERT)?);
+    let key = Arc::new(Pk::from_private_key(keys::PEM_KEY, None)?);
     let mut config = Config::new(Endpoint::Server, Transport::Stream, Preset::Default);
-    config.set_rng(Some(&mut rng));
-    config.push_cert(&mut *cert, &mut key)?;
-    let mut ctx = Context::new(&config)?;
+    config.set_rng(rng);
+    config.push_cert(cert, key)?;
+
+    let rc_config = Arc::new(config);
 
-    listen(|mut conn| {
-        let mut session = BufReader::new(ctx.establish(&mut conn, None)?);
+    listen(move |conn| {
+        let mut ctx = Context::new(rc_config.clone());
+        ctx.establish(conn, None)?;
+        let mut session = BufReader::new(ctx);
         let mut line = Vec::new();
         session.read_until(b'\n', &mut line).unwrap();
         session.get_mut().write_all(&line).unwrap();

mbedtls/examples/server.rs

@@ -7,6 +7,7 @@
  * according to those terms. */
 
 #![deny(warnings)]
+#![allow(dead_code)]
 #![allow(unused_doc_comments)]
 #![cfg_attr(not(feature = "std"), no_std)]
 
@@ -163,3 +164,9 @@ pub unsafe extern "C" fn mbedtls_time(tp: *mut time_t) -> time_t {
     }
     timestamp
 }
+
+// Debug not available in SGX
+#[cfg(not(target_env = "sgx"))]
+pub fn set_global_debug_threshold(threshold: i32) {
+    unsafe { mbedtls_sys::debug_set_threshold(threshold); }
+}

mbedtls/src/lib.rs

@@ -18,6 +18,12 @@ define!(
     impl<'a> Into<ptr> {}
 );
 
+impl Into<*mut mbedtls_sys::dhm_context> for &Dhm {
+    fn into(self) -> *mut mbedtls_sys::dhm_context {
+        &self.inner as *const _ as *mut _
+    }
+}
+
 impl Dhm {
     /// Takes both DER and PEM forms of FFDH parameters in `DHParams` format.
     ///

mbedtls/src/pk/dhparam.rs

@@ -22,7 +22,7 @@ use crate::hash::Type as MdType;
 use byteorder::{BigEndian, ByteOrder};
 
 pub(crate) mod dhparam;
-mod ec;
+pub mod ec;
 mod rfc6979;
 
 use self::rfc6979::Rfc6979Rng;
@@ -132,6 +132,7 @@ const CUSTOM_PK_INFO: pk_info_t = {
     }
 };
 
+// If this changes then certificate.rs unsafe code in public_key needs to also change.
 define!(
     #[c_ty(pk_context)]
     #[repr(C)]
@@ -142,6 +143,97 @@ define!(
     impl<'a> UnsafeFrom<ptr> {}
 );
 
+impl Into<*mut mbedtls_sys::pk_context> for &Pk {
+    fn into(self) -> *mut mbedtls_sys::pk_context {
+        &self.inner as *const _ as *mut _
+    }
+}
+
+
+//
+// Thread safety analysis for Pk.
+//
+// A. Usage example of Pk.
+//
+// 1.1. Common use case is to to pass it as parameter to the SSL Config class.
+// 1.2. SSL Config class is then used by multiple Context classes (one for each connection)
+// 1.3. Context classes, handled by different threads will do calls towards Pk.
+//
+// Since this is a common use case for MbedTLS it should be thread safe if threading is enabled.
+//
+// B. Verifying thread safety.
+//
+// 1. Calls towards the specific Pk implementation are done via function pointers.
+// 
+// - Example call towards Pk:
+//    ../../../mbedtls-sys/vendor/library/ssl_srv.c:3707 - mbedtls_pk_decrypt( private_key, p, len, ...
+// - This calls a generic function pointer via:
+//    ../../../mbedtls-sys/vendor/crypto/library/pk.c:475 - return( ctx->pk_info->decrypt_func( ctx->pk_ctx, input, ilen,
+//
+// 2. Pk implementation types.
+//
+// - The function pointers are defined via function:
+//      ../../../mbedtls-sys/vendor/crypto/library/pk.c:115 - mbedtls_pk_info_from_type
+// - They are as follows: mbedtls_rsa_info / mbedtls_eckey_info / mbedtls_ecdsa_info
+// - These are defined in: 
+//       ../../../mbedtls-sys/vendor/crypto/library/pk_wrap.c:196
+//
+// C. Checking types one by one.
+//
+// 1. RSA: mbedtls_rsa_info at ../../../mbedtls-sys/vendor/crypto/library/pk_wrap.c:196
+// This uses internal locks in: ../../../mbedtls-sys/vendor/crypto/library/rsa.c:718
+//
+// 2. ECKEY: mbedtls_eckey_info at ../../../mbedtls-sys/vendor/crypto/library/pk_wrap.c:418
+// This does not use internal locks but avoids interior mutability.
+//
+// Function checks one by one:
+// - Only const access to context: eckey_check_pair, eckey_get_bitlen, eckey_can_do, eckey_check_pair
+//
+// - Const acccess / copies context to a stack based variable
+//   eckey_verify_wrap, eckey_sign_wrap: ../../../mbedtls-sys/vendor/crypto/library/pk_wrap.c:251
+//       creates a stack ecdsa variable and uses ctx to initialize it.
+//       ctx is passed as 'key', a const pointer to mbedtls_ecdsa_from_keypair( &ecdsa, ctx )
+//           ../../../mbedtls-sys/vendor/crypto/library/ecdsa.c:819
+//           int mbedtls_ecdsa_from_keypair( mbedtls_ecdsa_context *ctx, const mbedtls_ecp_keypair *key )
+//           key does not mutate.
+//
+// - Ignored due to not defined: eckey_verify_rs_wrap, eckey_sign_rs_wrap
+//   (Undefined - MBEDTLS_ECP_RESTARTABLE - ../../../mbedtls-sys/build/config.rs:173)
+//
+// - Only used when creating/freeing - which is safe by design - eckey_alloc_wrap / eckey_free_wrap
+//
+// 3. ECDSA: mbedtls_ecdsa_info at ../../../mbedtls-sys/vendor/crypto/library/pk_wrap.c:729
+// This does not use internal locks but avoids interior mutability.
+//
+// - Const access / copies context to stack based variables:
+//   ecdsa_verify_wrap: ../../../mbedtls-sys/vendor/crypto/library/pk_wrap.c:544
+//       This copies the public key on the stack - in buf[] and copies the group id and nbits.
+//       That is done via: mbedtls_pk_write_pubkey( &p, buf, &key ) where key.pk_ctx = ctx;
+//       And the key is a const parameter to mbedtls_pk_write_pubkey - ../../../mbedtls-sys/vendor/crypto/library/pkwrite.c:158
+//
+// - Const access with additional notes due to call stacks involved.
+//
+//   ecdsa_sign_wrap: ../../../mbedtls-sys/vendor/crypto/library/pk_wrap.c:657
+//       mbedtls_ecdsa_write_signature ../../../mbedtls-sys/vendor/crypto/library/ecdsa.c:688
+//           mbedtls_ecdsa_write_signature_restartable ../../../mbedtls-sys/vendor/crypto/library/ecdsa.c:640
+//               MBEDTLS_ECDSA_DETERMINISTIC is not defined.
+//               MBEDTLS_ECDSA_SIGN_ALT is not defined.
+//               Passes grp to: ecdsa_sign_restartable: ../../../mbedtls-sys/vendor/crypto/library/ecdsa.c:253
+//                    Const access to group - reads parameters, passed as const to mbedtls_ecp_gen_privkey,
+//                    mbedtls_ecp_mul_restartable: ../../../mbedtls-sys/vendor/crypto/library/ecp.c:2351
+//                        MBEDTLS_ECP_INTERNAL_ALT is not defined. (otherwise it might not be safe depending on ecp_init/ecp_free) ../../../mbedtls-sys/build/config.rs:131
+//                        Passes as const to: mbedtls_ecp_check_privkey / mbedtls_ecp_check_pubkey / mbedtls_ecp_get_type( grp
+//        
+// - Ignored due to not defined: ecdsa_verify_rs_wrap, ecdsa_sign_rs_wrap, ecdsa_rs_alloc, ecdsa_rs_free
+//   (Undefined - MBEDTLS_ECP_RESTARTABLE - ../../../mbedtls-sys/build/config.rs:173)
+//
+// - Only const access to context: eckey_check_pair
+//
+// - Only used when creating/freeing - which is safe by design: ecdsa_alloc_wrap, ecdsa_free_wrap
+//
+#[cfg(feature = "threading")]
+unsafe impl Sync for Pk {}
+
 impl Pk {
     /// Takes both DER and PEM forms of PKCS#1 or PKCS#8 encoded keys.
     ///
@@ -323,11 +415,13 @@ impl Pk {
             .is_ok()
     }
 
-    getter!(
-        /// Key length in bits
-        len() -> usize = fn pk_get_bitlen
-    );
-    getter!(pk_type() -> Type = fn pk_get_type);
+    pub fn len(&self) -> usize {
+        unsafe { pk_get_bitlen(&self.inner) }
+    }
+
+    pub fn pk_type(&self) -> Type {
+        unsafe { pk_get_type(&self.inner).into() }
+    }
 
     pub fn curve(&self) -> Result<EcGroupId> {
         match self.pk_type() {
@@ -692,7 +786,7 @@ impl Pk {
         sig: &mut [u8],
         rng: &mut F,
     ) -> Result<usize> {
-        use crate::rng::RngCallback;
+        use crate::rng::RngCallbackMut;
 
         if self.pk_type() == Type::Ecdsa || self.pk_type() == Type::Eckey {
             if sig.len() < ECDSA_MAX_LEN {
@@ -717,8 +811,8 @@ impl Pk {
                     hash.len(),
                     sig.as_mut_ptr(),
                     &mut ret,
-                    Some(Rfc6979Rng::call),
-                    rng.data_ptr(),
+                    Some(Rfc6979Rng::call_mut),
+                    rng.data_ptr_mut(),
                 ).into_result()?;
             };
             Ok(ret)

mbedtls/src/pk/mod.rs

@@ -12,7 +12,7 @@ use crate::alloc_prelude::*;
 use mbedtls_sys::types::raw_types::{c_int, c_uchar, c_void};
 use mbedtls_sys::types::size_t;
 
-use crate::rng::{HmacDrbg, Random, RngCallback};
+use crate::rng::{HmacDrbg, Random, RngCallbackMut};
 
 use crate::error::Result;
 use crate::bignum::Mpi;
@@ -67,7 +67,7 @@ fn generate_rfc6979_nonce(md: &MdInfo, x: &Mpi, q: &Mpi, digest_bytes: &[u8]) ->
 pub(crate) struct Rfc6979Rng {
     pub k: Vec<u8>,
     pub k_read: usize,
-    pub rng: HmacDrbg<'static>,
+    pub rng: HmacDrbg,
 }
 
 /// An RNG which first outputs the k for RFC 6797 followed by random data
@@ -110,8 +110,8 @@ impl Rfc6979Rng {
     }
 }
 
-impl RngCallback for Rfc6979Rng {
-    unsafe extern "C" fn call(
+impl RngCallbackMut for Rfc6979Rng {
+    unsafe extern "C" fn call_mut(
         user_data: *mut c_void,
         data_ptr: *mut c_uchar,
         len: size_t,
@@ -126,7 +126,7 @@ impl RngCallback for Rfc6979Rng {
         }
     }
 
-    fn data_ptr(&mut self) -> *mut c_void {
+    fn data_ptr_mut(&mut self) -> *mut c_void {
         self as *const _ as *mut _
     }
 }