Skip to content

Commit

Permalink
mbedTLS lifetimes to reference counting
Browse files Browse the repository at this point in the history
  • Loading branch information
Adrian Cruceru committed Nov 23, 2020
1 parent fc8eeaf commit 2ce3aa1
Show file tree
Hide file tree
Showing 28 changed files with 1,689 additions and 1,251 deletions.
451 changes: 225 additions & 226 deletions Cargo.lock

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion mbedtls/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
21 changes: 11 additions & 10 deletions mbedtls/examples/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
}

Expand Down
23 changes: 14 additions & 9 deletions mbedtls/examples/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
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(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
7 changes: 7 additions & 0 deletions mbedtls/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
* according to those terms. */

#![deny(warnings)]
#![allow(dead_code)]
#![allow(unused_doc_comments)]
#![cfg_attr(not(feature = "std"), no_std)]

Expand Down Expand Up @@ -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); }
}
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
112 changes: 103 additions & 9 deletions mbedtls/src/pk/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down 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 _
}
}


//
// 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 2ce3aa1

Please sign in to comment.