Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
[package]
name = "crustls"
version = "0.1.0"
authors = ["Jacob Hoffman-Andrews <github@hoffman-andrews.com>"]
description = "C-to-rustls bindings"
edition = "2018"

[dependencies]
rustls = "0.19"
webpki = "0.21"
libc = "0.2"
env_logger = "0.8"
webpki-roots = "0.21"

[dev_dependencies]
cbindgen = "*"

[lib]
name = "crustls"
crate-type = ["staticlib"]
23 changes: 23 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
CFLAGS := -Werror -Wall -Wextra -Wpedantic
LDFLAGS := -Wl,--gc-sections -lpthread -ldl

all: target/crustls-demo
target/crustls-demo httpbin.org /headers

target:
mkdir -p $@

src/lib.h: src/lib.rs
cbindgen --lang C --output src/lib.h

target/crustls-demo: target/main.o target/debug/libcrustls.a
$(CC) -o $@ $^ $(LDFLAGS)

target/debug/libcrustls.a: src/lib.rs Cargo.toml
cargo build

target/main.o: src/main.c src/lib.h | target
$(CC) -o $@ -c $< $(CFLAGS)

clean:
rm -rf target
337 changes: 337 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,337 @@
#![crate_type = "staticlib"]

use libc::{c_char, c_int, c_void, size_t, ssize_t};
use std::io::{Cursor, Read, Write};
use std::slice;
use std::{ffi::CStr, sync::Arc};
use std::{io::ErrorKind::ConnectionAborted, mem};

use rustls::{ClientConfig, ClientSession, Session};

type CrustlsResult = c_int;

pub const CRUSTLS_OK: c_int = 0;
pub const CRUSTLS_ERROR: c_int = 1;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this be a Rust enum that has an appropriate Into implementation to convert into c_int?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I started on this and could not get cbindgen to generate the definitions right. I'm sure it's possible, but I'd like to punt to another PR.


/// Create a client_config. Caller owns the memory and must free it with
/// rustls_client_config_free.
#[no_mangle]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd strongly encourage you not to include global state, it makes things incredibly painful when multiple dependencies want to use this in single project.

pub extern "C" fn rustls_client_config_new() -> *const c_void {
let mut config = rustls::ClientConfig::new();
config
.root_store
.add_server_trust_anchors(&webpki_roots::TLS_SERVER_ROOTS);
env_logger::init();
Arc::into_raw(Arc::new(config)) as *const c_void

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't done and Rust/C interop so I'm not familiar with the idioms and best practices, but out of curiosity: it doesn't look like you're using the thread safety features of Arc here (since the Arc values never escape either this function or rustls_client_config_free, below). And even if they did, I would guess that none of the guarantees provided by Arc would hold while the pointer is loaned out to C-land. Is there a reason not to use an Rc instead, and maybe avoid taking a lock in the Arc?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, wait, I think I misunderstood how into_raw and from_raw work: the Arc and its reference count aren't thrown away, they are still in the buffer passed back to C-land. But suppose you have two threads, and one of them calls rustls_client_config_free at the same time that the other one calls rustls_client_session_new with the same pointer. The two functions will separately use Arc::from_raw to recreate an Arc. Will those two distinct Arcs give you the runtime mutual exclusivity you want? That is, is calling Arc::from_raw on the same raw buffer twice the same as doing std::clone::clone() on an Arc?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're correct that we're not getting any of the semantics of Arc while the pointer is owned by C-land. We don't get the reference counting, let alone the locking. The reason I'm using an Arc rather than an Rc or a Box is that rustls' ClientSession::new requires an Arc as part of its type signature. Also, we need C's copy of the pointer to remain valid long-term, so we can't do Arc::new(ptr) before each call to ClientSession::new, because that would move the value pointed to by ptr, invalidating C's copy.

Also a small note: When we do Arc::into_raw, we actually get a pointer to the inner value (i.e. ClientConfig). The strong and weak counters stay in memory right where they are, a few bytes lower than ClientConfig. When we do from_raw, it does a negative offset to reconstruct the Arc, including those strong and weak counters that were hiding out at a lower memory location.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, so presumably the same goes for the mutex protecting those counters, so two separate Arcs that were from_rawed into existence will be using the same lock and incrementing/decrementing the same reference count(s). Wow, that's pretty cool!

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, though to be pedantic, there's no lock, just atomic::AtomicUsize counts: https://doc.rust-lang.org/src/alloc/sync.rs.html#281-294

}

/// "Free" a client_config previously returned from rustls_client_config_new.
/// Since client_config is actually an atomically reference-counted pointer,
/// extant client_sessions may still hold an internal reference to the
/// Rust object. However, C code must consider this pointer unusable after
/// "free"ing it.
/// Calling with NULL is fine. Must not be called twice with the same value.
#[no_mangle]
pub extern "C" fn rustls_client_config_free(config: *const c_void) {
unsafe {
if let Some(c) = (config as *const ClientConfig).as_ref() {
// To free the client_config, we reconstruct the Arc. It should have a refcount of 1,
// representing the C code's copy. When it drops, that refcount will go down to 0
// and the inner ClientConfig will be dropped.
let arc: Arc<ClientConfig> = Arc::from_raw(c);
let strong_count = Arc::strong_count(&arc);
if strong_count < 1 {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, I'm pretty sure this is unreachable -- if it was 0 then the value would already have been freed.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, good point. My goal here was to catch a double free. And indeed, I can produce unexpected results by double freeing - but they are unsound. I.e. the Arc gets freed on the first call; on the second call, the Arc (including strong_count) is uninitialized. For instance in a test run it winds up as 94281166761504. I guess there really isn't anything I can do to guard against unsound usage on the C side and I should just delete this.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing out this unreachable code. Looks like I neglected to delete it in this PR. Followed up in #32.

eprintln!(
"rustls_client_config_free: invariant failed: arc.strong_count was < 1: {}. \
You must not free the same client_config multiple times.",
strong_count
);
}
} else {
eprintln!("rustls_client_config_free: config was NULL");
}
};
}

/// In rustls_client_config_new, we create an Arc, then call `into_raw` and return the resulting raw
/// pointer to C. C can then call rustls_client_session_new multiple times using that same raw
/// pointer. On each call, we need to reconstruct the Arc. But once we reconstruct the Arc, its
/// reference count will be decremented on drop. We need to reference count to stay at 1, because
/// the C code is holding a copy. This function turns the raw pointer back into an Arc, clones it
/// to increment the reference count (which will make it 2 in this particular case), and
/// mem::forgets the clone. The mem::forget prevents the reference count from being decremented when
/// we exit this function, so it will stay at 2 as long as we are in Rust code. Once the caller
/// drops its Arc, the reference count will go back down to 1, indicating the C code's copy.
///
/// Unsafety:
///
/// v must be a non-null pointer that resulted from previously calling `Arc::into_raw`.
unsafe fn arc_with_incref_from_raw<T>(v: *const T) -> Arc<T> {
let r = Arc::from_raw(v);
let val = Arc::clone(&r);
mem::forget(r);
val
}

/// Create a new rustls::ClientSession, and return it in the output parameter `out`.
/// If this returns an error code, the memory pointed to by `session_out` remains unchanged.
/// If this returns a non-error, the memory pointed to by `session_out` is modified to point
/// at a valid ClientSession. The caller now owns the ClientSession and must call
/// `rustls_client_session_free` when done with it.
#[no_mangle]
pub extern "C" fn rustls_client_session_new(
config: *const c_void,
hostname: *const c_char,
session_out: *mut *mut c_void,
) -> CrustlsResult {
let hostname: &CStr = unsafe {
if hostname.is_null() {
eprintln!("rustls_client_session_new: hostname was NULL");
return CRUSTLS_ERROR;
}
CStr::from_ptr(hostname)
};
let config: Arc<ClientConfig> = unsafe {
match (config as *const ClientConfig).as_ref() {
Some(c) => arc_with_incref_from_raw(c),
None => {
eprintln!("rustls_client_session_new: config was NULL");
return CRUSTLS_ERROR;
}
}
};
let hostname: &str = match hostname.to_str() {
Ok(s) => s,
Err(e) => {
eprintln!("converting hostname to Rust &str: {}", e);
return CRUSTLS_ERROR;
}
};
let name_ref = match webpki::DNSNameRef::try_from_ascii_str(hostname) {
Ok(nr) => nr,
Err(e) => {
eprintln!(
"turning hostname '{}' into webpki::DNSNameRef: {}",
hostname, e
);
return CRUSTLS_ERROR;
}
};
let client = ClientSession::new(&config, name_ref);

// We've succeeded. Put the client on the heap, and transfer ownership
// to the caller. After this point, we must return CRUSTLS_OK so the
// caller knows it is responsible for this memory.
let b = Box::new(client);
unsafe {
*session_out = Box::into_raw(b) as *mut c_void;
}

return CRUSTLS_OK;
}

#[no_mangle]
pub extern "C" fn rustls_client_session_wants_read(session: *const c_void) -> bool {
unsafe {
match (session as *const ClientSession).as_ref() {
Some(cs) => cs.wants_read(),
None => false,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In what case would we see None here? if session were NULL?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, as_ref turns a raw pointer into an Option<&T>, which is None if the pointer is NULL.

}
}
}

#[no_mangle]
pub extern "C" fn rustls_client_session_wants_write(session: *const c_void) -> bool {
unsafe {
match (session as *const ClientSession).as_ref() {
Some(cs) => cs.wants_write(),
None => false,
}
}
}

#[no_mangle]
pub extern "C" fn rustls_client_session_process_new_packets(session: *mut c_void) -> CrustlsResult {
let session: &mut ClientSession = unsafe {
match (session as *mut ClientSession).as_mut() {
Some(cs) => cs,
None => {
eprintln!("ClientSession::process_new_packets: session was NULL");
return CRUSTLS_ERROR;
}
}
};
let result: CrustlsResult = match session.process_new_packets() {
Ok(()) => CRUSTLS_OK,
Err(e) => {
eprintln!("ClientSession::process_new_packets: {}", e);
CRUSTLS_ERROR
}
};
result
}

/// Free a client_session previously returned from rustls_client_session_new.
/// Calling with NULL is fine. Must not be called twice with the same value.
#[no_mangle]
pub extern "C" fn rustls_client_session_free(session: *mut c_void) {
unsafe {
if let Some(c) = (session as *mut ClientSession).as_mut() {
// Convert the pointer to a Box and drop it.
Box::from_raw(c);
} else {
eprintln!("warning: rustls_client_config_free: config was NULL");
}
}
}

/// Write plaintext bytes into the ClientSession. This acts like
/// write(2). It returns the number of bytes written, or -1 on error.
#[no_mangle]
pub extern "C" fn rustls_client_session_write(
session: *const c_void,
buf: *const u8,
count: size_t,
) -> ssize_t {
let session: &mut ClientSession = unsafe {
match (session as *mut ClientSession).as_mut() {
Some(cs) => cs,
None => {
eprintln!("ClientSession::write: session was NULL");
return -1;
}
}
};
let write_buf: &[u8] = unsafe {
if buf.is_null() {
eprintln!("ClientSession::write: buf was NULL");
return -1;
}
slice::from_raw_parts(buf, count as usize)
};
let n_written: usize = match session.write(write_buf) {
Ok(n) => n,
Err(e) => {
eprintln!("ClientSession::write: {}", e);
return -1;
}
};
n_written as ssize_t
}

/// Read plaintext bytes from the ClientSession. This acts like
/// read(2), writing the plaintext bytes into `buf`. It returns
/// the number of bytes read, or -1 on error.
#[no_mangle]
pub extern "C" fn rustls_client_session_read(
session: *const c_void,
buf: *mut u8,
count: size_t,
) -> ssize_t {
let session: &mut ClientSession = unsafe {
match (session as *mut ClientSession).as_mut() {
Some(cs) => cs,
None => {
eprintln!("ClientSession::read: session was NULL");
return -1;
}
}
};
let read_buf: &mut [u8] = unsafe {
if buf.is_null() {
eprintln!("ClientSession::read: buf was NULL");
return -1;
}
slice::from_raw_parts_mut(buf, count as usize)
};
// Since it's *possible* for a Read impl to consume the possibly-uninitialized memory from buf,
// zero it out just in case. TODO: use Initializer once it's stabilized.
// https://doc.rust-lang.org/nightly/std/io/trait.Read.html#method.initializer
for c in read_buf.iter_mut() {
*c = 0;
}
let n_read: usize = match session.read(read_buf) {
Ok(n) => n,
// The CloseNotify TLS alert is benign, but rustls returns it as an Error. See comment on
// https://docs.rs/rustls/0.19.0/rustls/struct.ClientSession.html#impl-Read.
// Log it and return EOF.
Err(e) if e.kind() == ConnectionAborted && e.to_string().contains("CloseNotify") => {
eprintln!("ClientSession::read: CloseNotify (this is expected): {}", e);
return 0;
}
Err(e) => {
eprintln!("ClientSession::read: {}", e);
return -1;
}
};
n_read as ssize_t
}

/// Read TLS bytes taken from a socket into the ClientSession. This acts like
/// read(2). It returns the number of bytes read, or -1 on error.
#[no_mangle]
pub extern "C" fn rustls_client_session_read_tls(
session: *const c_void,
buf: *const u8,
count: size_t,
) -> ssize_t {
let session: &mut ClientSession = unsafe {
match (session as *mut ClientSession).as_mut() {
Some(cs) => cs,
None => {
eprintln!("ClientSession::read_tls: session was NULL");
return -1;
}
}
};
let input_buf: &[u8] = unsafe {
if buf.is_null() {
eprintln!("ClientSession::read_tls: buf was NULL");
return -1;
}
slice::from_raw_parts(buf, count as usize)
};
let mut cursor = Cursor::new(input_buf);
let n_read: usize = match session.read_tls(&mut cursor) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not clear to me that this is sound -- the caller is quite likely to provide a buf which contains uninitialized memory, I believe turning those until a slice is technically not sound.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any tips on where I can read about the soundness of turning uninitialized memory into a slice? Do I just need to initialize it? Before or after from_raw_parts?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have a great citation (boo me!), it's only a soundness issue if read_tls ever reads from &mut cursor, as long as it writes to it only, then it's find.

This is, classically, a big issue in the design of io::Read, which led to this whole thing rust-lang/rust#42002

Ok(n) => n,
Err(e) => {
eprintln!("ClientSession::read_tls: {}", e);
return -1;
}
};
n_read as ssize_t
}

/// Write TLS bytes from the ClientSession into a buffer. Those bytes should then be written to
/// a socket. This acts like write(2). It returns the number of bytes read, or -1 on error.
#[no_mangle]
pub extern "C" fn rustls_client_session_write_tls(
session: *const c_void,
buf: *mut u8,
count: size_t,
) -> ssize_t {
let session: &mut ClientSession = unsafe {
match (session as *mut ClientSession).as_mut() {
Some(cs) => cs,
None => {
eprintln!("ClientSession::write_tls: session was NULL");
return -1;
}
}
};
let mut output_buf: &mut [u8] = unsafe {
if buf.is_null() {
eprintln!("ClientSession::write_tls: buf was NULL");
return -1;
}
slice::from_raw_parts_mut(buf, count as usize)
};
let n_written: usize = match session.write_tls(&mut output_buf) {
Ok(n) => n,
Err(e) => {
eprintln!("ClientSession::write_tls: {}", e);
return -1;
}
};
n_written as ssize_t
}
Loading