Skip to content

Commit

Permalink
Moving from referene counting allows simpler move to native-tls / hyper.
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Adrian Cruceru committed Nov 30, 2020
1 parent fc8eeaf commit 0be7829
Show file tree
Hide file tree
Showing 32 changed files with 1,743 additions and 1,186 deletions.
451 changes: 225 additions & 226 deletions Cargo.lock

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions mbedtls-sys/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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_sys"

[lib]
name = "mbedtls_sys"
Expand Down
2 changes: 2 additions & 0 deletions mbedtls-sys/build/cmake.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,5 +49,7 @@ 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());
}
}
4 changes: 3 additions & 1 deletion mbedtls/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,10 @@ default-features = false
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"
Expand Down
5 changes: 5 additions & 0 deletions mbedtls/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ use std::env;

fn main() {
let mut b = cc::Build::new();
b.include(env::var_os("DEP_MBEDTLS_SYS_INCLUDE").expect("Links was not properly set in mbedtls-sys package, missing DEP_MBEDTLS_SYS_INCLUDE"));
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")
Expand All @@ -22,9 +24,12 @@ fn main() {
.define("_FORTIFY_SOURCE", Some("0"))
.flag("-ffreestanding");
}

b.compile("librust-mbedtls.a");

// Force correct link order for mbedtls_printf
println!("cargo:rustc-link-lib=static=mbedtls");
println!("cargo:rustc-link-lib=static=mbedx509");
println!("cargo:rustc-link-lib=static=mbedcrypto");

}
23 changes: 12 additions & 11 deletions mbedtls/examples/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,30 +14,31 @@ use std::net::TcpStream;
use mbedtls::rng::CtrDrbg;
use mbedtls::ssl::config::{Endpoint, Preset, Transport};
use mbedtls::ssl::{Config, Context};
use mbedtls::x509::Certificate;
use mbedtls::x509::{MbedtlsList, 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(MbedtlsList::<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(())
}

Expand Down
25 changes: 15 additions & 10 deletions mbedtls/examples/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@ use mbedtls::pk::Pk;
use mbedtls::rng::CtrDrbg;
use mbedtls::ssl::config::{Endpoint, Preset, Transport};
use mbedtls::ssl::{Config, Context};
use mbedtls::x509::Certificate;
use mbedtls::x509::{MbedtlsList, Certificate};
use mbedtls::Result as TlsResult;
use std::sync::Arc;

#[path = "../tests/support/mod.rs"]
mod support;
Expand All @@ -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(MbedtlsList::<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();
Expand Down
12 changes: 12 additions & 0 deletions mbedtls/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,3 +163,15 @@ 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); }
}


extern "C" {
pub fn forward_mbedtls_calloc(n: mbedtls_sys::types::size_t, size: mbedtls_sys::types::size_t) -> *mut mbedtls_sys::types::raw_types::c_void;
pub fn forward_mbedtls_free(n: *mut mbedtls_sys::types::raw_types::c_void);
}
6 changes: 6 additions & 0 deletions mbedtls/src/pk/dhparam.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand Down
110 changes: 102 additions & 8 deletions mbedtls/src/pk/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand All @@ -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 _
}
}

/// # 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.
///
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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 {
Expand All @@ -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)
Expand Down
10 changes: 5 additions & 5 deletions mbedtls/src/pk/rfc6979.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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 _
}
}
Loading

0 comments on commit 0be7829

Please sign in to comment.