Moving from referene counting allows simpler move to native-tls / hyper.

Arc Changes:
-    Each Config/Context/... will hold Arcs towards items it holds pointers to.
-    This forces objects to live as long as needed, once no longer used they get destroyed by reference counting.

This allows passing the objects to multiple threads without worrying about lifetime.
I've also added notes why classes are Sync where used. Let me know if I missed any classes.

Usage example of an intermediate mbed-hyper integration is at:
-    https://github.com/fortanix/rust-mbedtls/tree/acruceru/wip-mbed-hyper-v2/mbedtls-hyper/examples/integrations

There I added a crate to wrap hyper - similar to native-tls. (that will be moved to native-tls layer soon)
That crate can be considered an integration test that I will raise a separate PR for.

Changes after initial review:
-    Added forward_mbedtls_calloc / forward_mbedtls_free functions so we can pass certificates to and from mbedtls without allocator mismatches/corruptions.
-    Switched to MbedtlsList<Certificate> and Certificate. A MbedtlsBox is pending for this PR as well.
-    Fixed most comments.

Still pending:
-    Update define! macros
-    Add MbedtlsBox<Certificate>

mbedtls-sys/Cargo.toml

@@ -12,6 +12,7 @@ This version generates the correct bindings at compile time using bindgen."""
 readme = "../README.md"
 repository = "https://github.com/fortanix/rust-mbedtls"
 documentation = "https://docs.rs/mbedtls-sys-auto/"
+links = "mbedtls"
 
 [lib]
 name = "mbedtls_sys"

mbedtls-sys/build/cmake.rs

@@ -49,5 +49,8 @@ impl super::BuildConfig {
         println!("cargo:rustc-link-lib=mbedtls");
         println!("cargo:rustc-link-lib=mbedx509");
         println!("cargo:rustc-link-lib=mbedcrypto");
+
+        println!("cargo:include={}/{}", ::std::env::current_dir().unwrap().display(), self.mbedtls_src.join("include").display());
+        println!("cargo:config_h={}", self.config_h.to_str().expect("config.h UTF-8 error"));
     }
 }

mbedtls/Cargo.toml

@@ -38,12 +38,15 @@ default-features = false
 features = ["custom_printf", "trusted_cert_callback"]
 path = "../mbedtls-sys"
 
+
+
 [dev-dependencies]
 libc = "0.2.0"
 rand = "0.4.0"
 serde_cbor = "0.6"
 hex = "0.3"
 matches = "0.1.8"
+hyper = { version = "0.10.16", default-features = false }
 
 [build-dependencies]
 cc = "1.0"
@@ -119,3 +122,9 @@ required-features = ["std"]
 name = "ssl_conf_verify"
 path = "tests/ssl_conf_verify.rs"
 required-features = ["std"]
+
+
+[[test]]
+name = "hyper"
+path = "tests/hyper.rs"
+required-features = ["default", "pthread"]

mbedtls/build.rs

@@ -12,6 +12,12 @@ use std::env;
 
 fn main() {
     let mut b = cc::Build::new();
+    b.include(env::var_os("DEP_MBEDTLS_INCLUDE").expect("Links was not properly set in mbedtls-sys package, missing DEP_MBEDTLS_INCLUDE"));
+    let config_file = format!("\"{}\"", env::var_os("DEP_MBEDTLS_CONFIG_H").expect("Links was not properly set in mbedtls-sys package, missing DEP_MBEDTLS_CONFIG_H").to_str().unwrap());
+    b.define("MBEDTLS_CONFIG_FILE",
+             Some(config_file.as_str()));
+
+    b.file("src/rust_malloc.c");
     b.file("src/rust_printf.c");
     if env::var_os("CARGO_FEATURE_STD").is_none()
         || ::std::env::var("TARGET")

mbedtls/examples/client.rs

@@ -10,6 +10,7 @@ extern crate mbedtls;
 
 use std::io::{self, stdin, stdout, Write};
 use std::net::TcpStream;
+use std::sync::Arc;
 
 use mbedtls::rng::CtrDrbg;
 use mbedtls::ssl::config::{Endpoint, Preset, Transport};
@@ -23,21 +24,21 @@ 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::ROOT_CA_CERT)?;
+    let entropy = Arc::new(entropy_new());
+    let rng = Arc::new(CtrDrbg::new(entropy, None)?);
+    let cert = Arc::new(Certificate::from_pem_multiple(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/server.rs

@@ -10,6 +10,7 @@ extern crate mbedtls;
 
 use std::io::{BufRead, BufReader, Write};
 use std::net::{TcpListener, TcpStream};
+use std::sync::Arc;
 
 use mbedtls::pk::Pk;
 use mbedtls::rng::CtrDrbg;
@@ -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_multiple(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/src/alloc.rs

@@ -0,0 +1,60 @@
+/* Copyright (c) Fortanix, Inc.
+ *
+ * Licensed under the GNU General Public License, version 2 <LICENSE-GPL or
+ * https://www.gnu.org/licenses/gpl-2.0.html> or the Apache License, Version
+ * 2.0 <LICENSE-APACHE or http://www.apache.org/licenses/LICENSE-2.0>, at your
+ * option. This file may not be copied, modified, or distributed except
+ * according to those terms. */
+
+#[cfg(not(feature = "std"))]
+use crate::alloc_prelude::*;
+
+use core::fmt;
+use core::ops::{Deref, DerefMut};
+use core::ptr::NonNull;
+use core::ptr::drop_in_place;
+
+use mbedtls_sys::types::raw_types::c_void;
+
+extern "C" {
+    pub fn forward_mbedtls_free(n: *mut mbedtls_sys::types::raw_types::c_void);
+}
+
+#[repr(transparent)]
+pub struct Box<T> {
+    pub(crate) inner: NonNull<T>
+}
+
+impl<T> Deref for Box<T> {
+    type Target = T;
+    fn deref(&self) -> &T {
+        unsafe { self.inner.as_ref() }
+    }
+}
+
+impl<T> DerefMut for Box<T> {
+    fn deref_mut(&mut self) -> &mut T {
+        unsafe { self.inner.as_mut() }
+    }
+}
+
+impl<T: fmt::Debug> fmt::Debug for Box<T> {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        fmt::Debug::fmt(&**self, f)
+    }
+}
+
+impl<T> Drop for Box<T> {
+    fn drop(&mut self) {
+        unsafe {
+            drop_in_place(self.inner.as_ptr());
+            forward_mbedtls_free(self.inner.as_ptr() as *mut c_void)
+        }
+    }
+}
+
+#[repr(transparent)]
+pub struct List<T> {
+    pub(crate) inner: Option<Box<T>>
+}
+

mbedtls/src/lib.rs

@@ -59,6 +59,7 @@ pub mod rng;
 pub mod self_test;
 pub mod ssl;
 pub mod x509;
+pub mod alloc;
 
 #[cfg(feature = "pkcs12")]
 pub mod pkcs12;
@@ -163,3 +164,11 @@ 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 unsafe fn set_global_debug_threshold(threshold: i32) {
+    mbedtls_sys::debug_set_threshold(threshold);
+}
+
+

mbedtls/src/pk/mod.rs

@@ -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,91 @@ define!(
     impl<'a> UnsafeFrom<ptr> {}
 );
 
+// # Safety
+//
+// 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.
     ///
@@ -778,7 +864,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 {
@@ -803,8 +889,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/rfc6979.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 _
     }
 }

mbedtls/src/pkcs12/mod.rs

@@ -39,6 +39,7 @@ use crate::cipher::{Cipher, Decryption, Fresh, Traditional};
 use crate::hash::{pbkdf_pkcs12, Md, MdInfo, Type as MdType};
 use crate::pk::Pk;
 use crate::x509::Certificate;
+use crate::alloc::{Box as MbedtlsBox};
 use crate::Error as MbedtlsError;
 
 // Constants for various object identifiers used in PKCS12:
@@ -836,7 +837,7 @@ impl Pfx {
     /// of "friendly names" which are associated with said certificate.
     /// Some or all of the certificates stored in a Pfx may be encrypted in which case
     /// decrypt must be called to access them.
-    pub fn certificates<'a>(&'a self) -> impl Iterator<Item=(Result<Certificate, crate::Error>, Vec<String>)> + 'a {
+    pub fn certificates<'a>(&'a self) -> impl Iterator<Item=(Result<MbedtlsBox<Certificate>, crate::Error>, Vec<String>)> + 'a {
         self.authsafe_decrypted_contents()
             .filter_map(|sb| if let Pkcs12BagSet::Cert(CertBag(Some(cert))) = &sb.bag_value {
                 Some((Certificate::from_der(cert), sb.friendly_name()))