From 296505163261e214840830dc119faa629df543dc Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Mon, 7 Dec 2020 17:43:41 -0800 Subject: [PATCH 01/22] initial commit --- Cargo.toml | 11 ++++++++++ Makefile | 23 ++++++++++++++++++++ src/lib.rs | 13 +++++++++++ src/main.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 110 insertions(+) create mode 100644 Cargo.toml create mode 100644 Makefile create mode 100644 src/lib.rs create mode 100644 src/main.c diff --git a/Cargo.toml b/Cargo.toml new file mode 100644 index 00000000..afe658d8 --- /dev/null +++ b/Cargo.toml @@ -0,0 +1,11 @@ +[package] +name = "c-to-rust" +version = "0.1.0" +authors = ["Alex Crichton "] + +[dependencies] +rustls = "0.19" + +[lib] +name = "double_input" +crate-type = ["staticlib"] diff --git a/Makefile b/Makefile new file mode 100644 index 00000000..d46e3d59 --- /dev/null +++ b/Makefile @@ -0,0 +1,23 @@ +ifeq ($(shell uname),Darwin) + LDFLAGS := -Wl,-dead_strip +else + LDFLAGS := -Wl,--gc-sections -lpthread -ldl +endif + +all: target/double + target/double + +target: + mkdir -p $@ + +target/double: target/main.o target/debug/libdouble_input.a + $(CC) -o $@ $^ $(LDFLAGS) + +target/debug/libdouble_input.a: src/lib.rs Cargo.toml + cargo build + +target/main.o: src/main.c | target + $(CC) -o $@ -c $< + +clean: + rm -rf target diff --git a/src/lib.rs b/src/lib.rs new file mode 100644 index 00000000..12df9a91 --- /dev/null +++ b/src/lib.rs @@ -0,0 +1,13 @@ +#![crate_type = "staticlib"] + +extern crate rustls; +use rustls::ALL_CIPHERSUITES; + +#[no_mangle] +pub extern "C" fn print_ciphersuites() { + println!("Supported ciphersuites in rustls:"); + for cs in ALL_CIPHERSUITES.iter() { + println!(" {:?}", cs.suite); + } + () +} diff --git a/src/main.c b/src/main.c new file mode 100644 index 00000000..bcc4493b --- /dev/null +++ b/src/main.c @@ -0,0 +1,63 @@ +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +int +main(void) +{ + int sockfd = 0, n = 0, m = 0, result = 0; + char buf[1024]; + struct sockaddr_in serv_addr; + + memset(buf, '0', sizeof(buf)); + sockfd = socket(AF_INET, SOCK_STREAM, 0); + if(sockfd < 0) { + perror("Could not create socket"); + return 1; + } + + serv_addr.sin_family = AF_INET; + serv_addr.sin_port = htons(4444); + serv_addr.sin_addr.s_addr = inet_addr("127.0.0.1"); + + result = connect(sockfd, (struct sockaddr *)&serv_addr, sizeof(serv_addr)); + if(result < 0) { + perror("connecting"); + return 1; + } + + while(1) { + n = read(sockfd, buf, sizeof(buf) - 1); + if(n == 0) { + // EOF + break; + } + else if(n < 0) { + perror("reading bytes"); + return 1; + } + buf[n] = 0; + + while(n > 0) { + m = write(STDOUT_FILENO, buf, n); + if(m < 0) { + perror("writing to stdout"); + return 1; + } + if(m == 0) { + fprintf(stderr, "early EOF when writing to stdout\n"); + return 1; + } + n -= m; + } + } + + return 0; +} From 95a2c5ed7b233a8010b1d8385ea8024d85ee7408 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Mon, 7 Dec 2020 18:33:05 -0800 Subject: [PATCH 02/22] Round-trip ClientSession. --- Cargo.toml | 2 ++ src/lib.rs | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++++- src/main.c | 15 +++++++++++ 3 files changed, 92 insertions(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index afe658d8..2f4aa394 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -5,6 +5,8 @@ authors = ["Alex Crichton "] [dependencies] rustls = "0.19" +webpki = "0.21" +libc = "0.2.81" [lib] name = "double_input" diff --git a/src/lib.rs b/src/lib.rs index 12df9a91..54932dcd 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,7 +1,81 @@ #![crate_type = "staticlib"] +extern crate libc; extern crate rustls; -use rustls::ALL_CIPHERSUITES; +extern crate webpki; + +use libc::{c_char, c_int}; +use std::ffi::{CStr, CString}; +use std::sync::Arc; + +use rustls::{ClientSession, ALL_CIPHERSUITES}; + +static mut RUSTLS_CONFIG: Option> = None; + +#[no_mangle] +pub extern "C" fn init_rustls() { + unsafe { + RUSTLS_CONFIG = Some(Arc::new(rustls::ClientConfig::new())); + } +} + +const CRUSTLS_OK: c_int = 0; +const CRUSTLS_ERROR: c_int = 1; + +// Create a new rustls::ClientSession, and return it in the output parameter `out`. +// If this returns an error code, `out` remains unchanged. +// If this returns a non-error, `out` is modified to point at a valid ClientSession. +// The caller now owns the ClientSession and must call `drop_client_session` when +// done with it. +#[no_mangle] +pub extern "C" fn new_client_session( + hostname: *const c_char, + out: *mut *const ClientSession, +) -> c_int { + unsafe { + if RUSTLS_CONFIG.is_none() { + eprintln!("RUSTLS_CONFIG not initialized"); + return CRUSTLS_ERROR; + } + } + let hostname: &CStr = unsafe { CStr::from_ptr(hostname) }; + 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 config = unsafe { RUSTLS_CONFIG.clone().unwrap() }; + 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 { + *out = Box::into_raw(b); + } + + return CRUSTLS_OK; +} + +#[no_mangle] +pub extern "C" fn drop_client_session(ptr: *mut ClientSession) { + // Convert the pointer to a Box and drop it. + unsafe { Box::from_raw(ptr) }; + () +} #[no_mangle] pub extern "C" fn print_ciphersuites() { diff --git a/src/main.c b/src/main.c index bcc4493b..51cd5997 100644 --- a/src/main.c +++ b/src/main.c @@ -9,12 +9,25 @@ #include #include +extern int new_client_session(const char *hostname, void **client_session); +extern void drop_client_session(void *client_session); + int main(void) { int sockfd = 0, n = 0, m = 0, result = 0; char buf[1024]; struct sockaddr_in serv_addr; + void *client_session = NULL; + + init_rustls(); + printf("gonna make a client session. current value %p\n", client_session); + result = new_client_session("localhost", &client_session); + if(result != 0) { + return 1; + } + printf("successfully made a client session. current value %p\n", + client_session); memset(buf, '0', sizeof(buf)); sockfd = socket(AF_INET, SOCK_STREAM, 0); @@ -59,5 +72,7 @@ main(void) } } + printf("gonna drop it! %p\n", client_session); + drop_client_session(client_session); return 0; } From 0cef369e6fe32242e42a97bc373b57259bce2b3d Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Mon, 7 Dec 2020 18:43:40 -0800 Subject: [PATCH 03/22] Fix pointer types --- src/lib.rs | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 54932dcd..42a04d2e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -4,7 +4,7 @@ extern crate libc; extern crate rustls; extern crate webpki; -use libc::{c_char, c_int}; +use libc::{c_char, c_int, c_void}; use std::ffi::{CStr, CString}; use std::sync::Arc; @@ -28,10 +28,7 @@ const CRUSTLS_ERROR: c_int = 1; // The caller now owns the ClientSession and must call `drop_client_session` when // done with it. #[no_mangle] -pub extern "C" fn new_client_session( - hostname: *const c_char, - out: *mut *const ClientSession, -) -> c_int { +pub extern "C" fn new_client_session(hostname: *const c_char, out: *mut *const c_void) -> c_int { unsafe { if RUSTLS_CONFIG.is_none() { eprintln!("RUSTLS_CONFIG not initialized"); @@ -64,16 +61,16 @@ pub extern "C" fn new_client_session( // caller knows it is responsible for this memory. let b = Box::new(client); unsafe { - *out = Box::into_raw(b); + *out = Box::into_raw(b) as *const c_void; } return CRUSTLS_OK; } #[no_mangle] -pub extern "C" fn drop_client_session(ptr: *mut ClientSession) { +pub extern "C" fn drop_client_session(ptr: *const c_void) { // Convert the pointer to a Box and drop it. - unsafe { Box::from_raw(ptr) }; + unsafe { Box::from_raw(ptr as *mut ClientSession) }; () } From 31b010d93821dac5e61c1b0cd0e4ccd80b3b8ac9 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Mon, 7 Dec 2020 18:44:28 -0800 Subject: [PATCH 04/22] Add cbindgen and fix up crate name. --- Cargo.toml | 9 ++++++--- Makefile | 11 +++++++---- src/lib.rs | 2 +- src/main.c | 3 +-- 4 files changed, 15 insertions(+), 10 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 2f4aa394..e4c49726 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,13 +1,16 @@ [package] -name = "c-to-rust" +name = "crustls" version = "0.1.0" -authors = ["Alex Crichton "] +authors = ["Jacob Hoffman-Andrews "] [dependencies] rustls = "0.19" webpki = "0.21" libc = "0.2.81" +[dev_devpendencies] +cbindgen = "*" + [lib] -name = "double_input" +name = "crustls" crate-type = ["staticlib"] diff --git a/Makefile b/Makefile index d46e3d59..1ea58710 100644 --- a/Makefile +++ b/Makefile @@ -4,16 +4,19 @@ else LDFLAGS := -Wl,--gc-sections -lpthread -ldl endif -all: target/double - target/double +all: target/crustls-demo + target/crustls-demo target: mkdir -p $@ -target/double: target/main.o target/debug/libdouble_input.a +src/lib.h: + cbindgen --lang C --output src/lib.h + +target/crustls-demo: target/main.o target/debug/libcrustls.a $(CC) -o $@ $^ $(LDFLAGS) -target/debug/libdouble_input.a: src/lib.rs Cargo.toml +target/debug/libcrustls.a: src/lib.rs Cargo.toml cargo build target/main.o: src/main.c | target diff --git a/src/lib.rs b/src/lib.rs index 42a04d2e..7ac7100d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -5,7 +5,7 @@ extern crate rustls; extern crate webpki; use libc::{c_char, c_int, c_void}; -use std::ffi::{CStr, CString}; +use std::ffi::CStr; use std::sync::Arc; use rustls::{ClientSession, ALL_CIPHERSUITES}; diff --git a/src/main.c b/src/main.c index 51cd5997..4beb4c6b 100644 --- a/src/main.c +++ b/src/main.c @@ -9,8 +9,7 @@ #include #include -extern int new_client_session(const char *hostname, void **client_session); -extern void drop_client_session(void *client_session); +#include "lib.h" int main(void) From 3bb0dea5e91635d610a6d6b8068913b9965292a8 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Tue, 8 Dec 2020 20:18:30 -0800 Subject: [PATCH 05/22] Implement more methods. --- Cargo.toml | 3 +- Makefile | 8 +-- src/lib.rs | 174 ++++++++++++++++++++++++++++++++++++++++++++++++----- src/main.c | 125 +++++++++++++++++++++++++++++--------- 4 files changed, 261 insertions(+), 49 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index e4c49726..07e927c0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -7,8 +7,9 @@ authors = ["Jacob Hoffman-Andrews "] rustls = "0.19" webpki = "0.21" libc = "0.2.81" +env_logger = "0.8.2" -[dev_devpendencies] +[dev_dependencies] cbindgen = "*" [lib] diff --git a/Makefile b/Makefile index 1ea58710..ae0e4f4c 100644 --- a/Makefile +++ b/Makefile @@ -10,17 +10,17 @@ all: target/crustls-demo target: mkdir -p $@ -src/lib.h: +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) + $(CC) -Werror -o $@ $^ $(LDFLAGS) target/debug/libcrustls.a: src/lib.rs Cargo.toml cargo build -target/main.o: src/main.c | target - $(CC) -o $@ -c $< +target/main.o: src/main.c src/lib.h | target + $(CC) -Werror -o $@ -c $< clean: rm -rf target diff --git a/src/lib.rs b/src/lib.rs index 7ac7100d..1d790e7e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,34 +1,45 @@ #![crate_type = "staticlib"] +extern crate env_logger; extern crate libc; extern crate rustls; extern crate webpki; -use libc::{c_char, c_int, c_void}; -use std::ffi::CStr; -use std::sync::Arc; +use libc::{c_char, c_int, c_void, size_t, ssize_t}; +use std::{ + ffi::CStr, + io::{Cursor, Read}, + slice, +}; +use std::{io::Write, sync::Arc}; -use rustls::{ClientSession, ALL_CIPHERSUITES}; +use rustls::{ClientSession, Session, ALL_CIPHERSUITES}; static mut RUSTLS_CONFIG: Option> = None; #[no_mangle] -pub extern "C" fn init_rustls() { +pub extern "C" fn rustls_init() { unsafe { RUSTLS_CONFIG = Some(Arc::new(rustls::ClientConfig::new())); } + env_logger::init(); } -const CRUSTLS_OK: c_int = 0; -const CRUSTLS_ERROR: c_int = 1; +type CrustlsResult = c_int; + +pub const CRUSTLS_OK: c_int = 0; +pub const CRUSTLS_ERROR: c_int = 1; // Create a new rustls::ClientSession, and return it in the output parameter `out`. -// If this returns an error code, `out` remains unchanged. -// If this returns a non-error, `out` is modified to point at a valid ClientSession. -// The caller now owns the ClientSession and must call `drop_client_session` when -// done with it. +// 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_drop` when done with it. #[no_mangle] -pub extern "C" fn new_client_session(hostname: *const c_char, out: *mut *const c_void) -> c_int { +pub extern "C" fn rustls_client_session_new( + hostname: *const c_char, + session_out: *mut *const c_void, +) -> CrustlsResult { unsafe { if RUSTLS_CONFIG.is_none() { eprintln!("RUSTLS_CONFIG not initialized"); @@ -61,19 +72,152 @@ pub extern "C" fn new_client_session(hostname: *const c_char, out: *mut *const c // caller knows it is responsible for this memory. let b = Box::new(client); unsafe { - *out = Box::into_raw(b) as *const c_void; + *session_out = Box::into_raw(b) as *const c_void; } return CRUSTLS_OK; } #[no_mangle] -pub extern "C" fn drop_client_session(ptr: *const c_void) { +pub extern "C" fn rustls_client_session_wants_read(session: *const c_void) -> bool { + unsafe { + (session as *const ClientSession) + .as_ref() + .map(|cs| cs.wants_read()) + .unwrap_or_default() + } +} + +#[no_mangle] +pub extern "C" fn rustls_client_session_wants_write(session: *const c_void) -> bool { + unsafe { + (session as *const ClientSession) + .as_ref() + .map(|cs| cs.wants_write()) + .unwrap_or_default() + } +} + +#[no_mangle] +pub extern "C" fn rustls_client_session_process_new_packets( + session: *const c_void, +) -> CrustlsResult { + let mut session: Box = unsafe { Box::from_raw(session as *mut ClientSession) }; + let result: CrustlsResult = match session.process_new_packets() { + Ok(()) => CRUSTLS_OK, + Err(e) => { + eprintln!("ClientSession::process_new_packets: {}", e); + CRUSTLS_ERROR + } + }; + Box::leak(session); + result +} + +#[no_mangle] +pub extern "C" fn rustls_client_session_drop(session: *const c_void) { // Convert the pointer to a Box and drop it. - unsafe { Box::from_raw(ptr as *mut ClientSession) }; + unsafe { Box::from_raw(session as *mut ClientSession) }; () } +// 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 mut session: Box = unsafe { Box::from_raw(session as *mut ClientSession) }; + let write_buf: &[u8] = unsafe { + assert!(!buf.is_null()); + 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; + } + }; + Box::leak(session); + n_written as ssize_t +} + +// Read plaintext bytes from 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( + session: *const c_void, + buf: *mut u8, + count: size_t, +) -> ssize_t { + let mut session: Box = unsafe { Box::from_raw(session as *mut ClientSession) }; + let read_buf: &mut [u8] = unsafe { + assert!(!buf.is_null()); + slice::from_raw_parts_mut(buf, count as usize) + }; + let n_read: usize = match session.read(read_buf) { + Ok(n) => n, + Err(e) => { + eprintln!("ClientSession::read: {}", e); + return -1; + } + }; + Box::leak(session); + 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 mut session: Box = unsafe { Box::from_raw(session as *mut ClientSession) }; + let input_buf: &[u8] = unsafe { + assert!(!buf.is_null()); + 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) { + Ok(n) => n, + Err(e) => { + eprintln!("ClientSession::read_tls: {}", e); + return -1; + } + }; + Box::leak(session); + 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 mut session: Box = unsafe { Box::from_raw(session as *mut ClientSession) }; + let mut output_buf: &mut [u8] = unsafe { + assert!(!buf.is_null()); + 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; + } + }; + Box::leak(session); + n_written as ssize_t +} + #[no_mangle] pub extern "C" fn print_ciphersuites() { println!("Supported ciphersuites in rustls:"); diff --git a/src/main.c b/src/main.c index 4beb4c6b..b3e854cc 100644 --- a/src/main.c +++ b/src/main.c @@ -11,22 +11,37 @@ #include "lib.h" +void +write_all(int fd, const char *buf, int n) +{ + int m = 0; + while(n > 0) { + m = write(fd, buf, n); + if(m < 0) { + perror("writing to stdout"); + exit(1); + } + if(m == 0) { + fprintf(stderr, "early EOF when writing to stdout\n"); + exit(1); + } + n -= m; + } +} + int main(void) { int sockfd = 0, n = 0, m = 0, result = 0; - char buf[1024]; + char buf[2048]; struct sockaddr_in serv_addr; - void *client_session = NULL; + const void *client_session = NULL; - init_rustls(); - printf("gonna make a client session. current value %p\n", client_session); - result = new_client_session("localhost", &client_session); - if(result != 0) { + rustls_init(); + result = rustls_client_session_new("localhost", &client_session); + if(result != CRUSTLS_OK) { return 1; } - printf("successfully made a client session. current value %p\n", - client_session); memset(buf, '0', sizeof(buf)); sockfd = socket(AF_INET, SOCK_STREAM, 0); @@ -36,8 +51,9 @@ main(void) } serv_addr.sin_family = AF_INET; - serv_addr.sin_port = htons(4444); - serv_addr.sin_addr.s_addr = inet_addr("127.0.0.1"); + serv_addr.sin_port = htons(443); + // serv_addr.sin_addr.s_addr = inet_addr("127.0.0.1"); + serv_addr.sin_addr.s_addr = inet_addr("93.184.216.34"); result = connect(sockfd, (struct sockaddr *)&serv_addr, sizeof(serv_addr)); if(result < 0) { @@ -45,33 +61,84 @@ main(void) return 1; } + const char *request = "GET / HTTP/1.1\r\n\r\n"; + n = rustls_client_session_write(client_session, request, strlen(request)); + if(n < 0) { + fprintf(stderr, "error writing plaintext bytes to ClientSession\n"); + } + while(1) { - n = read(sockfd, buf, sizeof(buf) - 1); - if(n == 0) { - // EOF - break; - } - else if(n < 0) { - perror("reading bytes"); - return 1; - } - buf[n] = 0; + if(rustls_client_session_wants_read(client_session)) { + fprintf(stderr, + "ClientSession wants us to read_tls. First we need to pull some " + "bytes from the socket\n"); + + memset(buf, 0, sizeof(buf)); + n = read(sockfd, buf, sizeof(buf)); + if(n == 0) { + // EOF + fprintf(stderr, "EOF reading from socket\n"); + break; + } + else if(n < 0) { + perror("reading from socket"); + return 1; + } + fprintf(stderr, "read %d bytes from socket\n", n); - while(n > 0) { - m = write(STDOUT_FILENO, buf, n); - if(m < 0) { - perror("writing to stdout"); + // Now pull those bytes from the buffer into ClientSession. + // Note that we pass buf, n; not buf, sizeof(buf). We don't + // want to pull in unitialized memory that we didn't just + // read from the socket. + n = rustls_client_session_read_tls(client_session, buf, n); + if(n == 0) { + fprintf(stderr, "EOF from ClientSession::read_tls\n"); + // TODO: What to do here? + break; + } + else if(n < 0) { + fprintf(stderr, "Error in ClientSession::read_tls\n"); return 1; } - if(m == 0) { - fprintf(stderr, "early EOF when writing to stdout\n"); + + result = rustls_client_session_process_new_packets(client_session); + if(result != CRUSTLS_OK) { + fprintf(stderr, "Error in process_new_packets"); + return 1; + } + + memset(buf, 0, sizeof(buf)); + n = rustls_client_session_read(client_session, buf, sizeof(buf)); + if(n == 0) { + fprintf(stderr, "EOF from ClientSession::read\n"); + // TODO: What to do? + break; + } + else if(n < 0) { + fprintf(stderr, "Error in ClientSession::read\n"); return 1; } - n -= m; + + write_all(STDOUT_FILENO, buf, n); + } + if(rustls_client_session_wants_write(client_session)) { + fprintf(stderr, "ClientSession wants us to write_tls.\n"); + memset(buf, 0, sizeof(buf)); + n = rustls_client_session_write_tls(client_session, buf, sizeof(buf)); + if(n == 0) { + fprintf(stderr, "EOF from ClientSession::write_tls\n"); + // TODO: What to do? + break; + } + else if(n < 0) { + fprintf(stderr, "Error in ClientSession::write_tls\n"); + return 1; + } + + write_all(sockfd, buf, n); } } - printf("gonna drop it! %p\n", client_session); - drop_client_session(client_session); + rustls_client_session_drop(client_session); return 0; } From fc0d1b14f0d2bce670519e4ac53545ca0f472b4f Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Tue, 8 Dec 2020 21:40:13 -0800 Subject: [PATCH 06/22] It works!! --- Cargo.toml | 1 + Makefile | 2 +- src/lib.rs | 6 ++- src/main.c | 113 +++++++++++++++++++++++++++++++++++++++++------------ 4 files changed, 96 insertions(+), 26 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 07e927c0..4120fb9c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -8,6 +8,7 @@ rustls = "0.19" webpki = "0.21" libc = "0.2.81" env_logger = "0.8.2" +webpki-roots = "0.21.0" [dev_dependencies] cbindgen = "*" diff --git a/Makefile b/Makefile index ae0e4f4c..5360961c 100644 --- a/Makefile +++ b/Makefile @@ -5,7 +5,7 @@ else endif all: target/crustls-demo - target/crustls-demo + target/crustls-demo httpbin.org /headers target: mkdir -p $@ diff --git a/src/lib.rs b/src/lib.rs index 1d790e7e..960edbaa 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -19,8 +19,12 @@ static mut RUSTLS_CONFIG: Option> = None; #[no_mangle] pub extern "C" fn rustls_init() { + let mut config = rustls::ClientConfig::new(); + config + .root_store + .add_server_trust_anchors(&webpki_roots::TLS_SERVER_ROOTS); unsafe { - RUSTLS_CONFIG = Some(Arc::new(rustls::ClientConfig::new())); + RUSTLS_CONFIG = Some(Arc::new(config)); } env_logger::init(); } diff --git a/src/main.c b/src/main.c index b3e854cc..4b0fe49b 100644 --- a/src/main.c +++ b/src/main.c @@ -1,3 +1,4 @@ +#include #include #include #include @@ -29,46 +30,111 @@ write_all(int fd, const char *buf, int n) } } +// Connect to the given hostname on port 443 and return the file descriptor of +// the socket. On error, print the error and return -1. Caller is responsible +// for closing socket. int -main(void) +make_conn(const char *hostname) { - int sockfd = 0, n = 0, m = 0, result = 0; + struct addrinfo *getaddrinfo_output, *rp; + int getaddrinfo_result = + getaddrinfo(hostname, "443", NULL, &getaddrinfo_output); + if(getaddrinfo_result != 0) { + fprintf(stderr, "getaddrinfo: %s\n", gai_strerror(getaddrinfo_result)); + return -1; + } + + int sockfd = socket(getaddrinfo_output->ai_family, + getaddrinfo_output->ai_socktype, + getaddrinfo_output->ai_protocol); + if(sockfd < 0) { + perror("making socket"); + return -1; + } + + int connect_result = connect( + sockfd, getaddrinfo_output->ai_addr, getaddrinfo_output->ai_addrlen); + if(connect_result < 0) { + perror("connecting"); + return -1; + } + freeaddrinfo(getaddrinfo_output); + return sockfd; +} + +int +main(int argc, const char **argv) +{ + int n = 0, m = 0, result = 0; char buf[2048]; struct sockaddr_in serv_addr; const void *client_session = NULL; + if(argc <= 2) { + fprintf(stderr, + "usage: %s hostname path\n\n" + "Connect to a host via HTTPS on port 443, make a request for the\n" + "given path, and emit response to stdout.\n", + argv[0]); + } + const char *hostname = argv[1]; + const char *path = argv[2]; + rustls_init(); - result = rustls_client_session_new("localhost", &client_session); + + int sockfd = make_conn(hostname); + if(sockfd < 0) { + // No perror because make_conn printed error already. + return 1; + } + + result = rustls_client_session_new(hostname, &client_session); if(result != CRUSTLS_OK) { return 1; } memset(buf, '0', sizeof(buf)); - sockfd = socket(AF_INET, SOCK_STREAM, 0); - if(sockfd < 0) { - perror("Could not create socket"); - return 1; + snprintf(buf, + sizeof(buf), + "GET %s HTTP/1.1\r\n" + "Host: %s\r\n" + "User-Agent: crustls-demo\r\n" + "Accept: carcinization/inevitable, text/html\r\n" + "Connection: close\r\n" + "\r\n", + path, + hostname); + n = rustls_client_session_write(client_session, buf, strlen(buf)); + if(n < 0) { + fprintf(stderr, "error writing plaintext bytes to ClientSession\n"); } - serv_addr.sin_family = AF_INET; - serv_addr.sin_port = htons(443); - // serv_addr.sin_addr.s_addr = inet_addr("127.0.0.1"); - serv_addr.sin_addr.s_addr = inet_addr("93.184.216.34"); +#define MAX_EVENTS 1 + struct epoll_event ev, events[MAX_EVENTS]; + int conn_sock, nfds, epollfd; - result = connect(sockfd, (struct sockaddr *)&serv_addr, sizeof(serv_addr)); - if(result < 0) { - perror("connecting"); + epollfd = epoll_create1(0); + if(epollfd == -1) { + perror("epoll_create1"); return 1; } - const char *request = "GET / HTTP/1.1\r\n\r\n"; - n = rustls_client_session_write(client_session, request, strlen(request)); - if(n < 0) { - fprintf(stderr, "error writing plaintext bytes to ClientSession\n"); + ev.events = EPOLLIN | EPOLLOUT; + ev.data.fd = sockfd; + if(epoll_ctl(epollfd, EPOLL_CTL_ADD, sockfd, &ev) == -1) { + perror("epoll_ctl: listen_sock"); + return 1; } - while(1) { - if(rustls_client_session_wants_read(client_session)) { + for(;;) { + nfds = epoll_wait(epollfd, events, MAX_EVENTS, -1); + if(nfds == -1) { + perror("epoll_wait"); + exit(EXIT_FAILURE); + } + + if(rustls_client_session_wants_read(client_session) && + (events[0].events & EPOLLIN) > 0) { fprintf(stderr, "ClientSession wants us to read_tls. First we need to pull some " "bytes from the socket\n"); @@ -110,9 +176,7 @@ main(void) memset(buf, 0, sizeof(buf)); n = rustls_client_session_read(client_session, buf, sizeof(buf)); if(n == 0) { - fprintf(stderr, "EOF from ClientSession::read\n"); - // TODO: What to do? - break; + fprintf(stderr, "EOF from ClientSession::read (this is expected)\n"); } else if(n < 0) { fprintf(stderr, "Error in ClientSession::read\n"); @@ -121,7 +185,8 @@ main(void) write_all(STDOUT_FILENO, buf, n); } - if(rustls_client_session_wants_write(client_session)) { + if(rustls_client_session_wants_write(client_session) && + (events[0].events & EPOLLOUT) > 0) { fprintf(stderr, "ClientSession wants us to write_tls.\n"); memset(buf, 0, sizeof(buf)); n = rustls_client_session_write_tls(client_session, buf, sizeof(buf)); From 6b96a4a67ab6704db433170d8bce47bcccf5fe9b Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Wed, 9 Dec 2020 10:27:46 -0800 Subject: [PATCH 07/22] Cleanups from self-review. --- Cargo.toml | 2 + src/lib.rs | 148 +++++++++++++++++++++++++++++++++++------------------ src/main.c | 81 ++++++++++++++++++----------- 3 files changed, 150 insertions(+), 81 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 4120fb9c..9f5b55cc 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -2,6 +2,8 @@ name = "crustls" version = "0.1.0" authors = ["Jacob Hoffman-Andrews "] +description = "C-to-rustls bindings" +edition = "2018" [dependencies] rustls = "0.19" diff --git a/src/lib.rs b/src/lib.rs index 960edbaa..dc87f5d7 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,20 +1,18 @@ #![crate_type = "staticlib"] -extern crate env_logger; -extern crate libc; -extern crate rustls; -extern crate webpki; - use libc::{c_char, c_int, c_void, size_t, ssize_t}; -use std::{ - ffi::CStr, - io::{Cursor, Read}, - slice, -}; -use std::{io::Write, sync::Arc}; +use std::ffi::CStr; +use std::io::{Cursor, Read, Write}; +use std::slice; +use std::sync::Arc; use rustls::{ClientSession, Session, ALL_CIPHERSUITES}; +type CrustlsResult = c_int; + +pub const CRUSTLS_OK: c_int = 0; +pub const CRUSTLS_ERROR: c_int = 1; + static mut RUSTLS_CONFIG: Option> = None; #[no_mangle] @@ -29,28 +27,32 @@ pub extern "C" fn rustls_init() { env_logger::init(); } -type CrustlsResult = c_int; - -pub const CRUSTLS_OK: c_int = 0; -pub const CRUSTLS_ERROR: c_int = 1; - // 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_drop` when done with it. +// `rustls_client_session_free` when done with it. #[no_mangle] pub extern "C" fn rustls_client_session_new( hostname: *const c_char, - session_out: *mut *const c_void, + session_out: *mut *mut c_void, ) -> CrustlsResult { - unsafe { - if RUSTLS_CONFIG.is_none() { - eprintln!("RUSTLS_CONFIG not initialized"); + let config = unsafe { + match &RUSTLS_CONFIG { + Some(c) => c.clone(), + None => { + eprintln!("RUSTLS_CONFIG not initialized"); + return CRUSTLS_ERROR; + } + } + }; + let hostname: &CStr = unsafe { + if hostname.is_null() { + eprintln!("rustls_client_session_new: hostname was NULL"); return CRUSTLS_ERROR; } - } - let hostname: &CStr = unsafe { CStr::from_ptr(hostname) }; + CStr::from_ptr(hostname) + }; let hostname: &str = match hostname.to_str() { Ok(s) => s, Err(e) => { @@ -68,7 +70,6 @@ pub extern "C" fn rustls_client_session_new( return CRUSTLS_ERROR; } }; - let config = unsafe { RUSTLS_CONFIG.clone().unwrap() }; let client = ClientSession::new(&config, name_ref); // We've succeeded. Put the client on the heap, and transfer ownership @@ -76,7 +77,7 @@ pub extern "C" fn rustls_client_session_new( // caller knows it is responsible for this memory. let b = Box::new(client); unsafe { - *session_out = Box::into_raw(b) as *const c_void; + *session_out = Box::into_raw(b) as *mut c_void; } return CRUSTLS_OK; @@ -85,28 +86,34 @@ pub extern "C" fn rustls_client_session_new( #[no_mangle] pub extern "C" fn rustls_client_session_wants_read(session: *const c_void) -> bool { unsafe { - (session as *const ClientSession) - .as_ref() - .map(|cs| cs.wants_read()) - .unwrap_or_default() + match (session as *const ClientSession).as_ref() { + Some(cs) => cs.wants_read(), + None => false, + } } } #[no_mangle] pub extern "C" fn rustls_client_session_wants_write(session: *const c_void) -> bool { unsafe { - (session as *const ClientSession) - .as_ref() - .map(|cs| cs.wants_write()) - .unwrap_or_default() + 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: *const c_void, -) -> CrustlsResult { - let mut session: Box = unsafe { Box::from_raw(session as *mut ClientSession) }; +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) => { @@ -114,12 +121,11 @@ pub extern "C" fn rustls_client_session_process_new_packets( CRUSTLS_ERROR } }; - Box::leak(session); result } #[no_mangle] -pub extern "C" fn rustls_client_session_drop(session: *const c_void) { +pub extern "C" fn rustls_client_session_free(session: *const c_void) { // Convert the pointer to a Box and drop it. unsafe { Box::from_raw(session as *mut ClientSession) }; () @@ -133,9 +139,20 @@ pub extern "C" fn rustls_client_session_write( buf: *const u8, count: size_t, ) -> ssize_t { - let mut session: Box = unsafe { Box::from_raw(session as *mut ClientSession) }; + 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 -1; + } + } + }; let write_buf: &[u8] = unsafe { - assert!(!buf.is_null()); + 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) { @@ -145,7 +162,6 @@ pub extern "C" fn rustls_client_session_write( return -1; } }; - Box::leak(session); n_written as ssize_t } @@ -157,9 +173,20 @@ pub extern "C" fn rustls_client_session_read( buf: *mut u8, count: size_t, ) -> ssize_t { - let mut session: Box = unsafe { Box::from_raw(session as *mut ClientSession) }; + 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 -1; + } + } + }; let read_buf: &mut [u8] = unsafe { - assert!(!buf.is_null()); + if buf.is_null() { + eprintln!("ClientSession::read: buf was NULL"); + return -1; + } slice::from_raw_parts_mut(buf, count as usize) }; let n_read: usize = match session.read(read_buf) { @@ -169,7 +196,6 @@ pub extern "C" fn rustls_client_session_read( return -1; } }; - Box::leak(session); n_read as ssize_t } @@ -181,9 +207,20 @@ pub extern "C" fn rustls_client_session_read_tls( buf: *const u8, count: size_t, ) -> ssize_t { - let mut session: Box = unsafe { Box::from_raw(session as *mut ClientSession) }; + 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 -1; + } + } + }; let input_buf: &[u8] = unsafe { - assert!(!buf.is_null()); + 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); @@ -194,7 +231,6 @@ pub extern "C" fn rustls_client_session_read_tls( return -1; } }; - Box::leak(session); n_read as ssize_t } @@ -206,9 +242,20 @@ pub extern "C" fn rustls_client_session_write_tls( buf: *mut u8, count: size_t, ) -> ssize_t { - let mut session: Box = unsafe { Box::from_raw(session as *mut ClientSession) }; + 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 -1; + } + } + }; let mut output_buf: &mut [u8] = unsafe { - assert!(!buf.is_null()); + 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) { @@ -218,7 +265,6 @@ pub extern "C" fn rustls_client_session_write_tls( return -1; } }; - Box::leak(session); n_written as ssize_t } diff --git a/src/main.c b/src/main.c index 4b0fe49b..a50070b8 100644 --- a/src/main.c +++ b/src/main.c @@ -12,6 +12,11 @@ #include "lib.h" +/* + * Write n bytes from buf to the provided fd, retrying short writes until + * we finish or hit an error. Assumes fd is blocking and therefore doesn't + * handle EAGAIN. + */ void write_all(int fd, const char *buf, int n) { @@ -30,9 +35,11 @@ write_all(int fd, const char *buf, int n) } } -// Connect to the given hostname on port 443 and return the file descriptor of -// the socket. On error, print the error and return -1. Caller is responsible -// for closing socket. +/* + * Connect to the given hostname on port 443 and return the file descriptor of + * the socket. On error, print the error and return -1. Caller is responsible + * for closing socket. + */ int make_conn(const char *hostname) { @@ -62,36 +69,17 @@ make_conn(const char *hostname) return sockfd; } +/* + * Given an established TCP connection, and a rustls client_session, send an + * HTTP request and read the response. On success, return 0. On error, print + * the message and return 1. + */ int -main(int argc, const char **argv) +send_request_and_read_response(int sockfd, void *client_session, + const char *hostname, const char *path) { int n = 0, m = 0, result = 0; char buf[2048]; - struct sockaddr_in serv_addr; - const void *client_session = NULL; - - if(argc <= 2) { - fprintf(stderr, - "usage: %s hostname path\n\n" - "Connect to a host via HTTPS on port 443, make a request for the\n" - "given path, and emit response to stdout.\n", - argv[0]); - } - const char *hostname = argv[1]; - const char *path = argv[2]; - - rustls_init(); - - int sockfd = make_conn(hostname); - if(sockfd < 0) { - // No perror because make_conn printed error already. - return 1; - } - - result = rustls_client_session_new(hostname, &client_session); - if(result != CRUSTLS_OK) { - return 1; - } memset(buf, '0', sizeof(buf)); snprintf(buf, @@ -204,6 +192,39 @@ main(int argc, const char **argv) } } - rustls_client_session_drop(client_session); return 0; } + +int +main(int argc, const char **argv) +{ + if(argc <= 2) { + fprintf(stderr, + "usage: %s hostname path\n\n" + "Connect to a host via HTTPS on port 443, make a request for the\n" + "given path, and emit response to stdout.\n", + argv[0]); + return 1; + } + const char *hostname = argv[1]; + const char *path = argv[2]; + + rustls_init(); + + int sockfd = make_conn(hostname); + if(sockfd < 0) { + // No perror because make_conn printed error already. + return 1; + } + + void *client_session = NULL; + int result = rustls_client_session_new(hostname, &client_session); + if(result != CRUSTLS_OK) { + return 1; + } + + int return_code = + send_request_and_read_response(sockfd, client_session, hostname, path); + rustls_client_session_free(client_session); + return return_code; +} From bd9a7695db31adce6f9f2dd60ba568060eec3058 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Wed, 9 Dec 2020 11:07:29 -0800 Subject: [PATCH 08/22] Handle CloseNotify. --- src/lib.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/lib.rs b/src/lib.rs index dc87f5d7..5896a603 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -2,6 +2,7 @@ use libc::{c_char, c_int, c_void, size_t, ssize_t}; use std::ffi::CStr; +use std::io::ErrorKind::ConnectionAborted; use std::io::{Cursor, Read, Write}; use std::slice; use std::sync::Arc; @@ -191,6 +192,16 @@ pub extern "C" fn rustls_client_session_read( }; 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_tls: CloseNotify (this is expected): {}", + e + ); + return 0; + } Err(e) => { eprintln!("ClientSession::read: {}", e); return -1; From d139d88748a9234d53407bcf47c0bfd493424a84 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Wed, 9 Dec 2020 11:15:50 -0800 Subject: [PATCH 09/22] Read all available bytes from ClientSession. --- src/main.c | 39 ++++++++++++++++++++++++--------------- 1 file changed, 24 insertions(+), 15 deletions(-) diff --git a/src/main.c b/src/main.c index a50070b8..d272be7d 100644 --- a/src/main.c +++ b/src/main.c @@ -140,10 +140,12 @@ send_request_and_read_response(int sockfd, void *client_session, } fprintf(stderr, "read %d bytes from socket\n", n); - // Now pull those bytes from the buffer into ClientSession. - // Note that we pass buf, n; not buf, sizeof(buf). We don't - // want to pull in unitialized memory that we didn't just - // read from the socket. + /* + * Now pull those bytes from the buffer into ClientSession. + * Note that we pass buf, n; not buf, sizeof(buf). We don't + * want to pull in unitialized memory that we didn't just + * read from the socket. + */ n = rustls_client_session_read_tls(client_session, buf, n); if(n == 0) { fprintf(stderr, "EOF from ClientSession::read_tls\n"); @@ -155,23 +157,30 @@ send_request_and_read_response(int sockfd, void *client_session, return 1; } - result = rustls_client_session_process_new_packets(client_session); + int result = rustls_client_session_process_new_packets(client_session); if(result != CRUSTLS_OK) { fprintf(stderr, "Error in process_new_packets"); return 1; } - memset(buf, 0, sizeof(buf)); - n = rustls_client_session_read(client_session, buf, sizeof(buf)); - if(n == 0) { - fprintf(stderr, "EOF from ClientSession::read (this is expected)\n"); - } - else if(n < 0) { - fprintf(stderr, "Error in ClientSession::read\n"); - return 1; + /* Read all available bytes from the client_session until EOF. + * Note that EOF here indicates "no more bytes until + * process_new_packets", not "stream is closed". + */ + for(;;) { + memset(buf, 0, sizeof(buf)); + n = rustls_client_session_read(client_session, buf, sizeof(buf)); + if(n == 0) { + fprintf(stderr, "EOF from ClientSession::read (this is expected)\n"); + break; + } + else if(n < 0) { + fprintf(stderr, "Error in ClientSession::read\n"); + return 1; + } + + write_all(STDOUT_FILENO, buf, n); } - - write_all(STDOUT_FILENO, buf, n); } if(rustls_client_session_wants_write(client_session) && (events[0].events & EPOLLOUT) > 0) { From dfb06686a722d1bf4e60c03c1029b47dc319c577 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Wed, 9 Dec 2020 11:16:19 -0800 Subject: [PATCH 10/22] Tidy up variables, and return error on EOF for write_tls --- src/main.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/main.c b/src/main.c index d272be7d..4a7a1e99 100644 --- a/src/main.c +++ b/src/main.c @@ -78,7 +78,6 @@ int send_request_and_read_response(int sockfd, void *client_session, const char *hostname, const char *path) { - int n = 0, m = 0, result = 0; char buf[2048]; memset(buf, '0', sizeof(buf)); @@ -92,7 +91,7 @@ send_request_and_read_response(int sockfd, void *client_session, "\r\n", path, hostname); - n = rustls_client_session_write(client_session, buf, strlen(buf)); + int n = rustls_client_session_write(client_session, buf, strlen(buf)); if(n < 0) { fprintf(stderr, "error writing plaintext bytes to ClientSession\n"); } @@ -130,7 +129,6 @@ send_request_and_read_response(int sockfd, void *client_session, memset(buf, 0, sizeof(buf)); n = read(sockfd, buf, sizeof(buf)); if(n == 0) { - // EOF fprintf(stderr, "EOF reading from socket\n"); break; } @@ -189,8 +187,7 @@ send_request_and_read_response(int sockfd, void *client_session, n = rustls_client_session_write_tls(client_session, buf, sizeof(buf)); if(n == 0) { fprintf(stderr, "EOF from ClientSession::write_tls\n"); - // TODO: What to do? - break; + return 1; } else if(n < 0) { fprintf(stderr, "Error in ClientSession::write_tls\n"); From 58e96b9d81238a380f1bce4d730c62f9badeb0e4 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Wed, 9 Dec 2020 11:24:30 -0800 Subject: [PATCH 11/22] Make dep versions wider. --- Cargo.toml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 9f5b55cc..25b13b9a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -8,9 +8,9 @@ edition = "2018" [dependencies] rustls = "0.19" webpki = "0.21" -libc = "0.2.81" -env_logger = "0.8.2" -webpki-roots = "0.21.0" +libc = "0.2" +env_logger = "0.8" +webpki-roots = "0.21" [dev_dependencies] cbindgen = "*" From fef350bfd6e3906858a1bcc5e333431248f6e299 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Wed, 9 Dec 2020 12:58:27 -0800 Subject: [PATCH 12/22] Clean up some error messages. Also remove print_ciphersuites. --- src/lib.rs | 24 ++++++------------------ 1 file changed, 6 insertions(+), 18 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 5896a603..dee50386 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -7,7 +7,7 @@ use std::io::{Cursor, Read, Write}; use std::slice; use std::sync::Arc; -use rustls::{ClientSession, Session, ALL_CIPHERSUITES}; +use rustls::{ClientSession, Session}; type CrustlsResult = c_int; @@ -144,7 +144,7 @@ pub extern "C" fn rustls_client_session_write( match (session as *mut ClientSession).as_mut() { Some(cs) => cs, None => { - eprintln!("ClientSession::process_new_packets: session was NULL"); + eprintln!("ClientSession::write: session was NULL"); return -1; } } @@ -178,7 +178,7 @@ pub extern "C" fn rustls_client_session_read( match (session as *mut ClientSession).as_mut() { Some(cs) => cs, None => { - eprintln!("ClientSession::process_new_packets: session was NULL"); + eprintln!("ClientSession::read: session was NULL"); return -1; } } @@ -196,10 +196,7 @@ pub extern "C" fn rustls_client_session_read( // 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_tls: CloseNotify (this is expected): {}", - e - ); + eprintln!("ClientSession::read: CloseNotify (this is expected): {}", e); return 0; } Err(e) => { @@ -222,7 +219,7 @@ pub extern "C" fn rustls_client_session_read_tls( match (session as *mut ClientSession).as_mut() { Some(cs) => cs, None => { - eprintln!("ClientSession::process_new_packets: session was NULL"); + eprintln!("ClientSession::read_tls: session was NULL"); return -1; } } @@ -257,7 +254,7 @@ pub extern "C" fn rustls_client_session_write_tls( match (session as *mut ClientSession).as_mut() { Some(cs) => cs, None => { - eprintln!("ClientSession::process_new_packets: session was NULL"); + eprintln!("ClientSession::write_tls: session was NULL"); return -1; } } @@ -278,12 +275,3 @@ pub extern "C" fn rustls_client_session_write_tls( }; n_written as ssize_t } - -#[no_mangle] -pub extern "C" fn print_ciphersuites() { - println!("Supported ciphersuites in rustls:"); - for cs in ALL_CIPHERSUITES.iter() { - println!(" {:?}", cs.suite); - } - () -} From 65dc8713d0b742077468c1c8e1beb62cb0a776c0 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Wed, 9 Dec 2020 13:04:32 -0800 Subject: [PATCH 13/22] Zeroize memory before passing to read. --- src/lib.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/lib.rs b/src/lib.rs index dee50386..b3a8dc15 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -190,6 +190,12 @@ pub extern "C" fn rustls_client_session_read( } 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 From 40b0480ccbbcbad2887709c7f15da110ba0a60ea Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Wed, 9 Dec 2020 20:10:30 -0800 Subject: [PATCH 14/22] Switch from global init to returning ClientConfig. --- src/lib.rs | 22 +++++++++------------- src/main.c | 5 +++-- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index b3a8dc15..9ffe8a01 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,31 +1,26 @@ #![crate_type = "staticlib"] use libc::{c_char, c_int, c_void, size_t, ssize_t}; -use std::ffi::CStr; use std::io::ErrorKind::ConnectionAborted; use std::io::{Cursor, Read, Write}; use std::slice; -use std::sync::Arc; +use std::{ffi::CStr, sync::Arc}; -use rustls::{ClientSession, Session}; +use rustls::{ClientConfig, ClientSession, Session}; type CrustlsResult = c_int; pub const CRUSTLS_OK: c_int = 0; pub const CRUSTLS_ERROR: c_int = 1; -static mut RUSTLS_CONFIG: Option> = None; - #[no_mangle] -pub extern "C" fn rustls_init() { +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); - unsafe { - RUSTLS_CONFIG = Some(Arc::new(config)); - } env_logger::init(); + Arc::into_raw(Arc::new(config)) as *const c_void } // Create a new rustls::ClientSession, and return it in the output parameter `out`. @@ -35,14 +30,15 @@ pub extern "C" fn rustls_init() { // `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 config = unsafe { - match &RUSTLS_CONFIG { - Some(c) => c.clone(), + let config: Arc = unsafe { + match (config as *const ClientConfig).as_ref() { + Some(c) => Arc::from_raw(c), None => { - eprintln!("RUSTLS_CONFIG not initialized"); + eprintln!("rustls_client_session_new: config was NULL"); return CRUSTLS_ERROR; } } diff --git a/src/main.c b/src/main.c index 4a7a1e99..adb5666a 100644 --- a/src/main.c +++ b/src/main.c @@ -215,7 +215,7 @@ main(int argc, const char **argv) const char *hostname = argv[1]; const char *path = argv[2]; - rustls_init(); + const void *client_config = rustls_client_config_new(); int sockfd = make_conn(hostname); if(sockfd < 0) { @@ -224,7 +224,8 @@ main(int argc, const char **argv) } void *client_session = NULL; - int result = rustls_client_session_new(hostname, &client_session); + int result = + rustls_client_session_new(client_config, hostname, &client_session); if(result != CRUSTLS_OK) { return 1; } From 479ee3998ae7c5cd3182d6c327a1ddecfbe7d005 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Wed, 9 Dec 2020 20:40:50 -0800 Subject: [PATCH 15/22] Add some Arc and mem::forget --- src/lib.rs | 19 +++++++++++-------- src/main.c | 18 ++++++++++++++++++ 2 files changed, 29 insertions(+), 8 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 9ffe8a01..550eadd1 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,10 +1,10 @@ #![crate_type = "staticlib"] use libc::{c_char, c_int, c_void, size_t, ssize_t}; -use std::io::ErrorKind::ConnectionAborted; 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}; @@ -34,6 +34,13 @@ pub extern "C" fn rustls_client_session_new( 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 = unsafe { match (config as *const ClientConfig).as_ref() { Some(c) => Arc::from_raw(c), @@ -43,17 +50,11 @@ pub extern "C" fn rustls_client_session_new( } } }; - let hostname: &CStr = unsafe { - if hostname.is_null() { - eprintln!("rustls_client_session_new: hostname was NULL"); - return CRUSTLS_ERROR; - } - CStr::from_ptr(hostname) - }; let hostname: &str = match hostname.to_str() { Ok(s) => s, Err(e) => { eprintln!("converting hostname to Rust &str: {}", e); + mem::forget(config); return CRUSTLS_ERROR; } }; @@ -64,6 +65,7 @@ pub extern "C" fn rustls_client_session_new( "turning hostname '{}' into webpki::DNSNameRef: {}", hostname, e ); + mem::forget(config); return CRUSTLS_ERROR; } }; @@ -77,6 +79,7 @@ pub extern "C" fn rustls_client_session_new( *session_out = Box::into_raw(b) as *mut c_void; } + mem::forget(config); return CRUSTLS_OK; } diff --git a/src/main.c b/src/main.c index adb5666a..14e2da42 100644 --- a/src/main.c +++ b/src/main.c @@ -233,5 +233,23 @@ main(int argc, const char **argv) int return_code = send_request_and_read_response(sockfd, client_session, hostname, path); rustls_client_session_free(client_session); + + int sockfd2 = make_conn(hostname); + if(sockfd2 < 0) { + // No perror because make_conn printed error already. + return 1; + } + + void *client_session2 = NULL; + int result2 = + rustls_client_session_new(client_config, hostname, &client_session2); + if(result2 != CRUSTLS_OK) { + return 1; + } + + int return_code2 = + send_request_and_read_response(sockfd2, client_session, hostname, path); + rustls_client_session_free(client_session); + return return_code; } From 398317bad51738566d27e9f5895e6e6097989b56 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Wed, 9 Dec 2020 21:05:03 -0800 Subject: [PATCH 16/22] Add arc_with_incref_from_raw --- src/lib.rs | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 550eadd1..ab510b09 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -23,6 +23,26 @@ pub extern "C" fn rustls_client_config_new() -> *const c_void { Arc::into_raw(Arc::new(config)) as *const c_void } +// 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(v: *const T) -> Arc { + 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 @@ -43,7 +63,7 @@ pub extern "C" fn rustls_client_session_new( }; let config: Arc = unsafe { match (config as *const ClientConfig).as_ref() { - Some(c) => Arc::from_raw(c), + Some(c) => arc_with_incref_from_raw(c), None => { eprintln!("rustls_client_session_new: config was NULL"); return CRUSTLS_ERROR; @@ -54,7 +74,6 @@ pub extern "C" fn rustls_client_session_new( Ok(s) => s, Err(e) => { eprintln!("converting hostname to Rust &str: {}", e); - mem::forget(config); return CRUSTLS_ERROR; } }; @@ -65,7 +84,6 @@ pub extern "C" fn rustls_client_session_new( "turning hostname '{}' into webpki::DNSNameRef: {}", hostname, e ); - mem::forget(config); return CRUSTLS_ERROR; } }; @@ -79,7 +97,6 @@ pub extern "C" fn rustls_client_session_new( *session_out = Box::into_raw(b) as *mut c_void; } - mem::forget(config); return CRUSTLS_OK; } From cd3aeeac53046bedddf6d88b61bb4cb9f97b7044 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Wed, 9 Dec 2020 21:15:50 -0800 Subject: [PATCH 17/22] Add rustls_client_config_free --- src/lib.rs | 21 +++++++++++++++++++++ src/main.c | 2 ++ 2 files changed, 23 insertions(+) diff --git a/src/lib.rs b/src/lib.rs index ab510b09..5aadf73b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -23,6 +23,27 @@ pub extern "C" fn rustls_client_config_new() -> *const c_void { Arc::into_raw(Arc::new(config)) as *const c_void } +#[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 = Arc::from_raw(c); + let strong_count = Arc::strong_count(&arc); + if strong_count != 1 { + eprintln!( + "rustls_client_config_free: invariant failed: arc.strong_count was not 1: {}. probably this function was called twice or more for the same pointer", + 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 diff --git a/src/main.c b/src/main.c index 14e2da42..6ddfea1a 100644 --- a/src/main.c +++ b/src/main.c @@ -251,5 +251,7 @@ main(int argc, const char **argv) send_request_and_read_response(sockfd2, client_session, hostname, path); rustls_client_session_free(client_session); + rustls_client_config_free(client_config); + return return_code; } From 03b3c670e406b5d06ea268140a72f4d831b0b76b Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Thu, 10 Dec 2020 18:23:10 -0800 Subject: [PATCH 18/22] Detect more warnings. --- Makefile | 6 ++++-- src/main.c | 40 +++++++++++++++++++++++++--------------- 2 files changed, 29 insertions(+), 17 deletions(-) diff --git a/Makefile b/Makefile index 5360961c..e3cd2926 100644 --- a/Makefile +++ b/Makefile @@ -4,6 +4,8 @@ else LDFLAGS := -Wl,--gc-sections -lpthread -ldl endif +CFLAGS := -Werror -Wall -Wextra -Wpedantic + all: target/crustls-demo target/crustls-demo httpbin.org /headers @@ -14,13 +16,13 @@ src/lib.h: src/lib.rs cbindgen --lang C --output src/lib.h target/crustls-demo: target/main.o target/debug/libcrustls.a - $(CC) -Werror -o $@ $^ $(LDFLAGS) + $(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) -Werror -o $@ -c $< + $(CC) -o $@ -c $< $(CFLAGS) clean: rm -rf target diff --git a/src/main.c b/src/main.c index 6ddfea1a..a36e0d2a 100644 --- a/src/main.c +++ b/src/main.c @@ -43,7 +43,7 @@ write_all(int fd, const char *buf, int n) int make_conn(const char *hostname) { - struct addrinfo *getaddrinfo_output, *rp; + struct addrinfo *getaddrinfo_output; int getaddrinfo_result = getaddrinfo(hostname, "443", NULL, &getaddrinfo_output); if(getaddrinfo_result != 0) { @@ -91,16 +91,16 @@ send_request_and_read_response(int sockfd, void *client_session, "\r\n", path, hostname); - int n = rustls_client_session_write(client_session, buf, strlen(buf)); + int n = + rustls_client_session_write(client_session, (uint8_t *)buf, strlen(buf)); if(n < 0) { fprintf(stderr, "error writing plaintext bytes to ClientSession\n"); } #define MAX_EVENTS 1 struct epoll_event ev, events[MAX_EVENTS]; - int conn_sock, nfds, epollfd; - - epollfd = epoll_create1(0); + int nfds = 0; + int epollfd = epoll_create1(0); if(epollfd == -1) { perror("epoll_create1"); return 1; @@ -144,7 +144,7 @@ send_request_and_read_response(int sockfd, void *client_session, * want to pull in unitialized memory that we didn't just * read from the socket. */ - n = rustls_client_session_read_tls(client_session, buf, n); + n = rustls_client_session_read_tls(client_session, (uint8_t *)buf, n); if(n == 0) { fprintf(stderr, "EOF from ClientSession::read_tls\n"); // TODO: What to do here? @@ -167,7 +167,8 @@ send_request_and_read_response(int sockfd, void *client_session, */ for(;;) { memset(buf, 0, sizeof(buf)); - n = rustls_client_session_read(client_session, buf, sizeof(buf)); + n = rustls_client_session_read( + client_session, (uint8_t *)buf, sizeof(buf)); if(n == 0) { fprintf(stderr, "EOF from ClientSession::read (this is expected)\n"); break; @@ -184,7 +185,8 @@ send_request_and_read_response(int sockfd, void *client_session, (events[0].events & EPOLLOUT) > 0) { fprintf(stderr, "ClientSession wants us to write_tls.\n"); memset(buf, 0, sizeof(buf)); - n = rustls_client_session_write_tls(client_session, buf, sizeof(buf)); + n = rustls_client_session_write_tls( + client_session, (uint8_t *)buf, sizeof(buf)); if(n == 0) { fprintf(stderr, "EOF from ClientSession::write_tls\n"); return 1; @@ -230,10 +232,11 @@ main(int argc, const char **argv) return 1; } - int return_code = - send_request_and_read_response(sockfd, client_session, hostname, path); - rustls_client_session_free(client_session); - + int ret = 1; + ret = send_request_and_read_response(sockfd, client_session, hostname, path); + if(ret != CRUSTLS_OK) { + goto cleanup; + } int sockfd2 = make_conn(hostname); if(sockfd2 < 0) { // No perror because make_conn printed error already. @@ -247,11 +250,18 @@ main(int argc, const char **argv) return 1; } - int return_code2 = + ret = send_request_and_read_response(sockfd2, client_session, hostname, path); - rustls_client_session_free(client_session); + if(ret != CRUSTLS_OK) { + goto cleanup; + } + + // Success! + return 0; +cleanup: rustls_client_config_free(client_config); - return return_code; + rustls_client_session_free(client_session); + return ret; } From ed480911a85b91053fa2b253c4987677d046c7ed Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Fri, 11 Dec 2020 12:55:19 -0800 Subject: [PATCH 19/22] Review feedback. --- src/lib.rs | 76 ++++++++++++++++------------ src/main.c | 144 ++++++++++++++++++++++++++++++++--------------------- 2 files changed, 131 insertions(+), 89 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 5aadf73b..b5824354 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -13,6 +13,8 @@ type CrustlsResult = c_int; pub const CRUSTLS_OK: c_int = 0; pub const CRUSTLS_ERROR: c_int = 1; +/// Create a client_config. Caller owns the memory and must free it with +/// rustls_client_config_free. #[no_mangle] pub extern "C" fn rustls_client_config_new() -> *const c_void { let mut config = rustls::ClientConfig::new(); @@ -23,6 +25,8 @@ pub extern "C" fn rustls_client_config_new() -> *const c_void { Arc::into_raw(Arc::new(config)) as *const c_void } +/// Free a client_config previously returned from rustls_client_config_new. +/// 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 { @@ -34,7 +38,9 @@ pub extern "C" fn rustls_client_config_free(config: *const c_void) { let strong_count = Arc::strong_count(&arc); if strong_count != 1 { eprintln!( - "rustls_client_config_free: invariant failed: arc.strong_count was not 1: {}. probably this function was called twice or more for the same pointer", + "rustls_client_config_free: invariant failed: arc.strong_count was not 1: {}. \ + You must free all client_sessions that depend on this client_config first. \ + Also you must not free client_config multiple times", strong_count ); } @@ -44,19 +50,19 @@ pub extern "C" fn rustls_client_config_free(config: *const c_void) { }; } -// 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`. +/// 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(v: *const T) -> Arc { let r = Arc::from_raw(v); let val = Arc::clone(&r); @@ -64,11 +70,11 @@ unsafe fn arc_with_incref_from_raw(v: *const T) -> Arc { 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. +/// 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, @@ -162,15 +168,22 @@ pub extern "C" fn rustls_client_session_process_new_packets(session: *mut c_void 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: *const c_void) { - // Convert the pointer to a Box and drop it. - unsafe { Box::from_raw(session as *mut ClientSession) }; - () +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. +/// 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, @@ -203,8 +216,9 @@ pub extern "C" fn rustls_client_session_write( n_written as ssize_t } -// Read plaintext bytes from the ClientSession. This acts like -// read(2). It returns the number of bytes read, or -1 on error. +/// 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, @@ -250,8 +264,8 @@ pub extern "C" fn rustls_client_session_read( 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. +/// 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, @@ -285,8 +299,8 @@ pub extern "C" fn rustls_client_session_read_tls( 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. +/// 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, diff --git a/src/main.c b/src/main.c index a36e0d2a..098f49a1 100644 --- a/src/main.c +++ b/src/main.c @@ -10,14 +10,15 @@ #include #include +/* lib.h is autogenerated in the Makefile using cbindgen. */ #include "lib.h" /* * Write n bytes from buf to the provided fd, retrying short writes until * we finish or hit an error. Assumes fd is blocking and therefore doesn't - * handle EAGAIN. + * handle EAGAIN. Returns 0 for success or 1 for error. */ -void +int write_all(int fd, const char *buf, int n) { int m = 0; @@ -25,19 +26,20 @@ write_all(int fd, const char *buf, int n) m = write(fd, buf, n); if(m < 0) { perror("writing to stdout"); - exit(1); + return 1; } if(m == 0) { fprintf(stderr, "early EOF when writing to stdout\n"); - exit(1); + return 1; } n -= m; } + return 0; } /* * Connect to the given hostname on port 443 and return the file descriptor of - * the socket. On error, print the error and return -1. Caller is responsible + * the socket. On error, print the error and return 1. Caller is responsible * for closing socket. */ int @@ -48,7 +50,7 @@ make_conn(const char *hostname) getaddrinfo(hostname, "443", NULL, &getaddrinfo_output); if(getaddrinfo_result != 0) { fprintf(stderr, "getaddrinfo: %s\n", gai_strerror(getaddrinfo_result)); - return -1; + goto cleanup; } int sockfd = socket(getaddrinfo_output->ai_family, @@ -56,17 +58,26 @@ make_conn(const char *hostname) getaddrinfo_output->ai_protocol); if(sockfd < 0) { perror("making socket"); - return -1; + goto cleanup; } int connect_result = connect( sockfd, getaddrinfo_output->ai_addr, getaddrinfo_output->ai_addrlen); if(connect_result < 0) { perror("connecting"); - return -1; + goto cleanup; } - freeaddrinfo(getaddrinfo_output); + return sockfd; + +cleanup: + if(getaddrinfo_output != NULL) { + freeaddrinfo(getaddrinfo_output); + } + if(sockfd > 0) { + close(sockfd); + } + return -1; } /* @@ -78,9 +89,11 @@ int send_request_and_read_response(int sockfd, void *client_session, const char *hostname, const char *path) { + int ret = 1; + int result = 1; char buf[2048]; - memset(buf, '0', sizeof(buf)); + bzero(buf, sizeof(buf)); snprintf(buf, sizeof(buf), "GET %s HTTP/1.1\r\n" @@ -95,6 +108,7 @@ send_request_and_read_response(int sockfd, void *client_session, rustls_client_session_write(client_session, (uint8_t *)buf, strlen(buf)); if(n < 0) { fprintf(stderr, "error writing plaintext bytes to ClientSession\n"); + goto cleanup; } #define MAX_EVENTS 1 @@ -103,21 +117,21 @@ send_request_and_read_response(int sockfd, void *client_session, int epollfd = epoll_create1(0); if(epollfd == -1) { perror("epoll_create1"); - return 1; + goto cleanup; } ev.events = EPOLLIN | EPOLLOUT; ev.data.fd = sockfd; if(epoll_ctl(epollfd, EPOLL_CTL_ADD, sockfd, &ev) == -1) { perror("epoll_ctl: listen_sock"); - return 1; + goto cleanup; } for(;;) { nfds = epoll_wait(epollfd, events, MAX_EVENTS, -1); if(nfds == -1) { perror("epoll_wait"); - exit(EXIT_FAILURE); + goto cleanup; } if(rustls_client_session_wants_read(client_session) && @@ -126,7 +140,7 @@ send_request_and_read_response(int sockfd, void *client_session, "ClientSession wants us to read_tls. First we need to pull some " "bytes from the socket\n"); - memset(buf, 0, sizeof(buf)); + bzero(buf, sizeof(buf)); n = read(sockfd, buf, sizeof(buf)); if(n == 0) { fprintf(stderr, "EOF reading from socket\n"); @@ -134,7 +148,7 @@ send_request_and_read_response(int sockfd, void *client_session, } else if(n < 0) { perror("reading from socket"); - return 1; + goto cleanup; } fprintf(stderr, "read %d bytes from socket\n", n); @@ -152,13 +166,13 @@ send_request_and_read_response(int sockfd, void *client_session, } else if(n < 0) { fprintf(stderr, "Error in ClientSession::read_tls\n"); - return 1; + goto cleanup; } - int result = rustls_client_session_process_new_packets(client_session); + result = rustls_client_session_process_new_packets(client_session); if(result != CRUSTLS_OK) { fprintf(stderr, "Error in process_new_packets"); - return 1; + goto cleanup; } /* Read all available bytes from the client_session until EOF. @@ -166,7 +180,7 @@ send_request_and_read_response(int sockfd, void *client_session, * process_new_packets", not "stream is closed". */ for(;;) { - memset(buf, 0, sizeof(buf)); + bzero(buf, sizeof(buf)); n = rustls_client_session_read( client_session, (uint8_t *)buf, sizeof(buf)); if(n == 0) { @@ -175,93 +189,107 @@ send_request_and_read_response(int sockfd, void *client_session, } else if(n < 0) { fprintf(stderr, "Error in ClientSession::read\n"); - return 1; + goto cleanup; } - write_all(STDOUT_FILENO, buf, n); + result = write_all(STDOUT_FILENO, buf, n); + if(result != 0) { + goto cleanup; + } } } if(rustls_client_session_wants_write(client_session) && (events[0].events & EPOLLOUT) > 0) { fprintf(stderr, "ClientSession wants us to write_tls.\n"); - memset(buf, 0, sizeof(buf)); + bzero(buf, sizeof(buf)); n = rustls_client_session_write_tls( client_session, (uint8_t *)buf, sizeof(buf)); if(n == 0) { fprintf(stderr, "EOF from ClientSession::write_tls\n"); - return 1; + goto cleanup; } else if(n < 0) { fprintf(stderr, "Error in ClientSession::write_tls\n"); - return 1; + goto cleanup; } - write_all(sockfd, buf, n); + result = write_all(sockfd, buf, n); + if(result != 0) { + goto cleanup; + } } } - return 0; + ret = 0; +cleanup: + if(epollfd > 0) { + close(epollfd); + } + return ret; } int -main(int argc, const char **argv) +do_request(const void *client_config, const char *hostname, const char *path) { - if(argc <= 2) { - fprintf(stderr, - "usage: %s hostname path\n\n" - "Connect to a host via HTTPS on port 443, make a request for the\n" - "given path, and emit response to stdout.\n", - argv[0]); - return 1; - } - const char *hostname = argv[1]; - const char *path = argv[2]; - - const void *client_config = rustls_client_config_new(); - + int ret = 1; int sockfd = make_conn(hostname); if(sockfd < 0) { // No perror because make_conn printed error already. - return 1; + goto cleanup; } void *client_session = NULL; int result = rustls_client_session_new(client_config, hostname, &client_session); if(result != CRUSTLS_OK) { - return 1; + goto cleanup; } - int ret = 1; ret = send_request_and_read_response(sockfd, client_session, hostname, path); if(ret != CRUSTLS_OK) { goto cleanup; } - int sockfd2 = make_conn(hostname); - if(sockfd2 < 0) { - // No perror because make_conn printed error already. - return 1; + + ret = 0; + +cleanup: + rustls_client_session_free(client_session); + if(sockfd > 0) { + close(sockfd); } + return ret; +} - void *client_session2 = NULL; - int result2 = - rustls_client_session_new(client_config, hostname, &client_session2); - if(result2 != CRUSTLS_OK) { +int +main(int argc, const char **argv) +{ + int ret = 1; + int result = 1; + if(argc <= 2) { + fprintf(stderr, + "usage: %s hostname path\n\n" + "Connect to a host via HTTPS on port 443, make a request for the\n" + "given path, and emit response to stdout.\n", + argv[0]); return 1; } + const char *hostname = argv[1]; + const char *path = argv[2]; - ret = - send_request_and_read_response(sockfd2, client_session, hostname, path); - if(ret != CRUSTLS_OK) { - goto cleanup; + const void *client_config = rustls_client_config_new(); + + int i; + for(i = 0; i < 3; i++) { + result = do_request(client_config, hostname, path); + if(result != 0) { + goto cleanup; + } } // Success! - return 0; + ret = 0; cleanup: rustls_client_config_free(client_config); - - rustls_client_session_free(client_session); return ret; } From 5dddca34e10a4af2269ef9e19dcbcf28a2007575 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Fri, 11 Dec 2020 13:12:02 -0800 Subject: [PATCH 20/22] Fix up "free" comment. --- src/lib.rs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index b5824354..a23c70e1 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -25,7 +25,11 @@ pub extern "C" fn rustls_client_config_new() -> *const c_void { Arc::into_raw(Arc::new(config)) as *const c_void } -/// Free a client_config previously returned from rustls_client_config_new. +/// "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) { @@ -36,11 +40,10 @@ pub extern "C" fn rustls_client_config_free(config: *const c_void) { // and the inner ClientConfig will be dropped. let arc: Arc = Arc::from_raw(c); let strong_count = Arc::strong_count(&arc); - if strong_count != 1 { + if strong_count < 1 { eprintln!( - "rustls_client_config_free: invariant failed: arc.strong_count was not 1: {}. \ - You must free all client_sessions that depend on this client_config first. \ - Also you must not free client_config multiple times", + "rustls_client_config_free: invariant failed: arc.strong_count was < 1: {}. \ + You must not free the same client_config multiple times.", strong_count ); } From 053ecd2bbad29e73acc7910972d76fdafd22f519 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Fri, 11 Dec 2020 14:40:23 -0800 Subject: [PATCH 21/22] Remove Darwin from Makefile. --- Makefile | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/Makefile b/Makefile index e3cd2926..56967714 100644 --- a/Makefile +++ b/Makefile @@ -1,10 +1,5 @@ -ifeq ($(shell uname),Darwin) - LDFLAGS := -Wl,-dead_strip -else - LDFLAGS := -Wl,--gc-sections -lpthread -ldl -endif - CFLAGS := -Werror -Wall -Wextra -Wpedantic +LDFLAGS := -Wl,--gc-sections -lpthread -ldl all: target/crustls-demo target/crustls-demo httpbin.org /headers From 56c7d73a1defc39e02e103d407198225afff4cf4 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Fri, 11 Dec 2020 15:09:16 -0800 Subject: [PATCH 22/22] Initialize getaddrinfo_output = NULL --- src/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main.c b/src/main.c index 098f49a1..47a6c472 100644 --- a/src/main.c +++ b/src/main.c @@ -45,7 +45,7 @@ write_all(int fd, const char *buf, int n) int make_conn(const char *hostname) { - struct addrinfo *getaddrinfo_output; + struct addrinfo *getaddrinfo_output = NULL; int getaddrinfo_result = getaddrinfo(hostname, "443", NULL, &getaddrinfo_output); if(getaddrinfo_result != 0) {