Merge #128

128: MbedTLS Reference counted instead of lifetimes r=jethrogb a=AdrianCX

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.


Edit:

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>


Fixes #1
Partial progress on #3
Fixes #4
Fixes #8
Partially addresses #9

Co-authored-by: Adrian Cruceru <[email protected]>
Co-authored-by: Jethro Beekman <[email protected]>

Author: 3 people

Cargo.lock

@@ -1,3 +1,4 @@
 status = [
   "continuous-integration/travis-ci/push",
 ]
+timeout_sec = 36000 # ten hours

bors.toml

@@ -19,6 +19,7 @@ if [ $TRAVIS_RUST_VERSION = "stable" ] || [ $TRAVIS_RUST_VERSION = "beta" ] || [
     cargo test --features pkcs12
     cargo test --features pkcs12_rc2
     cargo test --features force_aesni_support
+    cargo test --features default,pthread
 
 elif [ $TRAVIS_RUST_VERSION = $CORE_IO_NIGHTLY ]; then
     cargo +$CORE_IO_NIGHTLY test --no-default-features --features core_io,rdrand,time,custom_time,custom_gmtime_r

ct.sh

@@ -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/Cargo.toml

@@ -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-sys/build/cmake.rs

@@ -44,6 +44,7 @@ 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 +120,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 = ["std", "threading"]

mbedtls/Cargo.toml

@@ -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/mbedtls_malloc.c");
     b.file("src/rust_printf.c");
     if env::var_os("CARGO_FEATURE_STD").is_none()
         || ::std::env::var("TARGET")

mbedtls/build.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/client.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/examples/server.rs

@@ -0,0 +1,65 @@
+/* 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. */
+
+use core::fmt;
+use core::ops::{Deref, DerefMut};
+use core::ptr::NonNull;
+use core::ptr::drop_in_place;
+use core::mem::ManuallyDrop;
+
+use mbedtls_sys::types::raw_types::c_void;
+
+extern "C" {
+    pub(crate) 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> Box<T> {
+    pub(crate) fn into_raw(self) -> *mut T {
+        let v = ManuallyDrop::new(self);
+        v.inner.as_ptr()
+    }
+}
+
+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/alloc.rs

@@ -15,7 +15,7 @@ const ERROR: _MUST_USE_EITHER_STD_OR_CORE_IO_ = ();
 
 #[cfg(not(feature = "std"))]
 #[macro_use]
-extern crate alloc;
+extern crate alloc as rust_alloc;
 #[cfg(feature = "std")]
 extern crate core;
 #[cfg(not(feature = "std"))]
@@ -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;
@@ -112,11 +113,13 @@ mod mbedtls {
 #[cfg(not(feature = "std"))]
 mod alloc_prelude {
     #![allow(unused)]
-    pub(crate) use alloc::borrow::ToOwned;
-    pub(crate) use alloc::boxed::Box;
-    pub(crate) use alloc::string::String;
-    pub(crate) use alloc::string::ToString;
-    pub(crate) use alloc::vec::Vec;
+    pub(crate) use rust_alloc::borrow::ToOwned;
+    pub(crate) use rust_alloc::boxed::Box;
+    pub(crate) use rust_alloc::sync::Arc;
+    pub(crate) use rust_alloc::string::String;
+    pub(crate) use rust_alloc::string::ToString;
+    pub(crate) use rust_alloc::vec::Vec;
+    pub(crate) use rust_alloc::borrow::Cow;
 }
 
 #[cfg(all(feature="time", any(feature="custom_gmtime_r", feature="custom_time")))]
@@ -163,3 +166,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/lib.rs

@@ -0,0 +1,31 @@
+/* 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. */
+
+// Follow same pattern for config and alloc/free as everywhere in mbedtls
+#if !defined(MBEDTLS_CONFIG_FILE)
+#include "mbedtls/config.h"
+#else
+#include MBEDTLS_CONFIG_FILE
+#endif
+
+#if defined(MBEDTLS_PLATFORM_C)
+#include "mbedtls/platform.h"
+#else
+#include <stdlib.h>
+#define mbedtls_calloc    calloc
+#define mbedtls_free      free
+#endif
+
+extern void *forward_mbedtls_calloc( size_t n, size_t size ) {
+    return mbedtls_calloc(n, size);
+}
+
+extern void forward_mbedtls_free( void *ptr ) {
+    mbedtls_free(ptr);
+}
+

mbedtls/src/mbedtls_malloc.c

@@ -22,7 +22,7 @@ impl Dhm {
     /// Takes both DER and PEM forms of FFDH parameters in `DHParams` format.
     ///
     /// When calling on PEM-encoded data, `params` must be NULL-terminated
-    pub(crate) fn from_params(params: &[u8]) -> Result<Dhm> {
+    pub fn from_params(params: &[u8]) -> Result<Dhm> {
         let mut ret = Self::init();
         unsafe { dhm_parse_dhm(&mut ret.inner, params.as_ptr(), params.len()) }.into_result()?;
         Ok(ret)