From 3b534b91d50e840f814cc4fab662890dab03c409 Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Thu, 29 Jun 2023 11:23:27 -0400 Subject: [PATCH 01/11] tests: common read_file and load_cert_and_key. Previously only the `tests/server.c` code needed to load a `rustls_certified_key` (for the server cert/keypair). In a subsequent commit the `tests/client.c` code will need to do the same for optionally providing a `rustls_certified_key` for client certificate authentication. In preparation, this commit lifts the `read_file` and `load_cert_and_key` helper functions from `tests/server.c` into `tests/common.c` (updating `tests/common.h` to match) where both client and server test programs can use the shared code. --- tests/common.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++++ tests/common.h | 6 ++++++ tests/server.c | 51 +------------------------------------------------- 3 files changed, 56 insertions(+), 50 deletions(-) diff --git a/tests/common.c b/tests/common.c index 9745129b..fb7c480b 100644 --- a/tests/common.c +++ b/tests/common.c @@ -353,3 +353,52 @@ log_cb(void *userdata, const struct rustls_log_params *params) fprintf(stderr, "%s[fd %d][%.*s]: %.*s\n", conn->program_name, conn->fd, (int)level_str.len, level_str.data, (int)params->message.len, params->message.data); } + +enum demo_result +read_file(const char *progname, const char *filename, char *buf, size_t buflen, size_t *n) +{ + FILE *f = fopen(filename, "r"); + if(f == NULL) { + fprintf(stderr, "%s: opening %s: %s\n", progname, filename, strerror(errno)); + return DEMO_ERROR; + } + *n = fread(buf, 1, buflen, f); + if(!feof(f)) { + fprintf(stderr, "%s: reading %s: %s\n", progname, filename, strerror(errno)); + fclose(f); + return DEMO_ERROR; + } + fclose(f); + return DEMO_OK; +} + +const struct rustls_certified_key * +load_cert_and_key(const char *progname, const char *certfile, const char *keyfile) +{ + char certbuf[10000]; + size_t certbuf_len; + char keybuf[10000]; + size_t keybuf_len; + + int result = read_file(progname, certfile, certbuf, sizeof(certbuf), &certbuf_len); + if(result != DEMO_OK) { + return NULL; + } + + result = read_file(progname, keyfile, keybuf, sizeof(keybuf), &keybuf_len); + if(result != DEMO_OK) { + return NULL; + } + + const struct rustls_certified_key *certified_key; + result = rustls_certified_key_build((uint8_t *)certbuf, + certbuf_len, + (uint8_t *)keybuf, + keybuf_len, + &certified_key); + if(result != RUSTLS_RESULT_OK) { + print_error(progname, "parsing certificate and key", result); + return NULL; + } + return certified_key; +} diff --git a/tests/common.h b/tests/common.h index 6efdffc3..37a4a567 100644 --- a/tests/common.h +++ b/tests/common.h @@ -125,4 +125,10 @@ get_first_header_value(const char *headers, size_t headers_len, void log_cb(void *userdata, const struct rustls_log_params *params); +enum demo_result +read_file(const char *progname, const char *filename, char *buf, size_t buflen, size_t *n); + +const struct rustls_certified_key * +load_cert_and_key(const char *progname, const char *certfile, const char *keyfile); + #endif /* COMMON_H */ diff --git a/tests/server.c b/tests/server.c index 6cd76bfe..2f2668d2 100644 --- a/tests/server.c +++ b/tests/server.c @@ -28,24 +28,6 @@ #include "rustls.h" #include "common.h" -enum demo_result -read_file(const char *filename, char *buf, size_t buflen, size_t *n) -{ - FILE *f = fopen(filename, "r"); - if(f == NULL) { - fprintf(stderr, "server: opening %s: %s\n", filename, strerror(errno)); - return DEMO_ERROR; - } - *n = fread(buf, 1, buflen, f); - if(!feof(f)) { - fprintf(stderr, "server: reading %s: %s\n", filename, strerror(errno)); - fclose(f); - return DEMO_ERROR; - } - fclose(f); - return DEMO_OK; -} - typedef enum exchange_state { READING_REQUEST, @@ -252,37 +234,6 @@ handle_conn(struct conndata *conn) free(conn); } -const struct rustls_certified_key * -load_cert_and_key(const char *certfile, const char *keyfile) -{ - char certbuf[10000]; - size_t certbuf_len; - char keybuf[10000]; - size_t keybuf_len; - - int result = read_file(certfile, certbuf, sizeof(certbuf), &certbuf_len); - if(result != DEMO_OK) { - return NULL; - } - - result = read_file(keyfile, keybuf, sizeof(keybuf), &keybuf_len); - if(result != DEMO_OK) { - return NULL; - } - - const struct rustls_certified_key *certified_key; - result = rustls_certified_key_build((uint8_t *)certbuf, - certbuf_len, - (uint8_t *)keybuf, - keybuf_len, - &certified_key); - if(result != RUSTLS_RESULT_OK) { - print_error("server", "parsing certificate and key", result); - return NULL; - } - return certified_key; -} - bool shutting_down = false; void handle_signal(int signo) { @@ -325,7 +276,7 @@ main(int argc, const char **argv) goto cleanup; } - certified_key = load_cert_and_key(argv[1], argv[2]); + certified_key = load_cert_and_key(argv[0], argv[1], argv[2]); if(certified_key == NULL) { goto cleanup; } From f7ca733570fc18ae44ea3c6344cc20cf921368bf Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Thu, 29 Jun 2023 11:26:32 -0400 Subject: [PATCH 02/11] tests/client: add client certificate support. This commit updates `tests/client.c` to allow setting two new env vars, `AUTH_CERT` and `AUTH_KEY`. If neither are set, the program works as it did before: no client certificate is sent for mTLS. If one but not both of these env vars are set, the program will error: they must both be provided. If both are set, the `AUTH_CERT` and `AUTH_KEY` files are loaded into a `rustls_certified_key` and the built `rustls_client_config` will be configured to offer client certificate authentication with the server using the cert/key pair. --- tests/client.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/client.c b/tests/client.c index 0f15cb24..36ba09c6 100644 --- a/tests/client.c +++ b/tests/client.c @@ -413,6 +413,7 @@ main(int argc, const char **argv) rustls_client_config_builder_new(); const struct rustls_client_config *client_config = NULL; struct rustls_slice_bytes alpn_http11; + const struct rustls_certified_key *certified_key = NULL; alpn_http11.data = (unsigned char*)"http/1.1"; alpn_http11.len = 8; @@ -438,6 +439,19 @@ main(int argc, const char **argv) goto cleanup; } + char* auth_cert = getenv("AUTH_CERT"); + char* auth_key = getenv("AUTH_KEY"); + if((auth_cert && !auth_key) || (!auth_cert && auth_key)) { + fprintf(stderr, "client: must set both AUTH_CERT and AUTH_KEY env vars, or neither\n"); + goto cleanup; + } else if (auth_cert && auth_key) { + certified_key = load_cert_and_key(argv[0], auth_cert, auth_key); + if(certified_key == NULL) { + goto cleanup; + } + rustls_client_config_builder_set_certified_key(config_builder, &certified_key, 1); + } + rustls_client_config_builder_set_alpn_protocols(config_builder, &alpn_http11, 1); client_config = rustls_client_config_builder_build(config_builder); @@ -454,6 +468,7 @@ main(int argc, const char **argv) ret = 0; cleanup: + rustls_certified_key_free(certified_key); rustls_client_config_free(client_config); #ifdef _WIN32 From 11a1ab7653c70e378b7e7e5a60de7bf37240ac1e Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Thu, 29 Jun 2023 11:31:01 -0400 Subject: [PATCH 03/11] tests/server: add optional required client cert auth. This commit updates the `tests/server.c` program so that if an `AUTH_CERT` env var is provided the server will be configured to require clients provide a client certificate issued that chains to the `AUTH_CERT` certificate authority. If no `AUTH_CERT` env var is set the server works as it did before, ignoring client certificate authentication. --- tests/server.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/server.c b/tests/server.c index 2f2668d2..019ba9a1 100644 --- a/tests/server.c +++ b/tests/server.c @@ -255,6 +255,8 @@ main(int argc, const char **argv) struct rustls_connection *rconn = NULL; const struct rustls_certified_key *certified_key = NULL; struct rustls_slice_bytes alpn_http11; + const struct rustls_client_cert_verifier *client_cert_verifier = NULL; + struct rustls_root_cert_store *client_cert_root_store = NULL; alpn_http11.data = (unsigned char*)"http/1.1"; alpn_http11.len = 8; @@ -285,6 +287,22 @@ main(int argc, const char **argv) config_builder, &certified_key, 1); rustls_server_config_builder_set_alpn_protocols(config_builder, &alpn_http11, 1); + char* auth_cert = getenv("AUTH_CERT"); + if(auth_cert) { + char certbuf[10000]; + size_t certbuf_len; + int result = read_file(argv[0], auth_cert, certbuf, sizeof(certbuf), &certbuf_len); + if(result != DEMO_OK) { + goto cleanup; + } + + client_cert_root_store = rustls_root_cert_store_new(); + rustls_root_cert_store_add_pem(client_cert_root_store, (uint8_t *)certbuf, certbuf_len, true); + + client_cert_verifier = rustls_client_cert_verifier_new(client_cert_root_store); + rustls_server_config_builder_set_client_verifier(config_builder, client_cert_verifier); + } + server_config = rustls_server_config_builder_build(config_builder); #ifdef _WIN32 @@ -360,6 +378,8 @@ main(int argc, const char **argv) cleanup: rustls_certified_key_free(certified_key); + rustls_root_cert_store_free(client_cert_root_store); + rustls_client_cert_verifier_free(client_cert_verifier); rustls_server_config_free(server_config); rustls_connection_free(rconn); if(sockfd>0) { From 6c0a9ce9c8fb69bf3ede5f75ddd9a7d8a8ca6455 Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Fri, 30 Jun 2023 13:11:48 -0400 Subject: [PATCH 04/11] tests: include mTLS tests. Tests that: * A client w/ AUTH_KEY + AUTH_CERT can connect to a server that doesn't require mTLS without error. * A client w/ AUTH_KEY + AUTH_CERT can connect to a server that requires mTLS without error. * A client w/o AUTH_KEY + AUTH_CERT errors when connecting to a server that requires mTLS. --- tests/client-server.py | 64 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 61 insertions(+), 3 deletions(-) diff --git a/tests/client-server.py b/tests/client-server.py index 8d3beb1c..4f0d10e4 100755 --- a/tests/client-server.py +++ b/tests/client-server.py @@ -35,12 +35,16 @@ def wait_tcp_port(host, port): print("Connected to {}:{}".format(host, port)) -def run_with_maybe_valgrind(args, env, valgrind): +def run_with_maybe_valgrind(args, env, valgrind, expect_error=False): if valgrind is not None: args = [valgrind] + args process_env = os.environ.copy() process_env.update(env) - subprocess.check_call(args, env=process_env, stdout=subprocess.DEVNULL) + try: + subprocess.check_call(args, env=process_env, stdout=subprocess.DEVNULL) + except subprocess.CalledProcessError as e: + if not expect_error: + raise e def run_client_tests(client, valgrind): @@ -81,6 +85,50 @@ def run_client_tests(client, valgrind): }, valgrind ) + run_with_maybe_valgrind( + [ + client, + HOST, + str(PORT), + "/" + ], + { + "CA_FILE": "testdata/minica.pem", + "AUTH_CERT": "testdata/localhost/cert.pem", + "AUTH_KEY": "testdata/localhost/key.pem", + }, + valgrind + ) + + +def run_mtls_client_tests(client, valgrind): + run_with_maybe_valgrind( + [ + client, + HOST, + str(PORT), + "/" + ], + { + "CA_FILE": "testdata/minica.pem", + }, + valgrind, + expect_error=True # Client connecting w/o AUTH_CERT/AUTH_KEY should err. + ) + run_with_maybe_valgrind( + [ + client, + HOST, + str(PORT), + "/" + ], + { + "CA_FILE": "testdata/minica.pem", + "AUTH_CERT": "testdata/localhost/cert.pem", + "AUTH_KEY": "testdata/localhost/key.pem", + }, + valgrind + ) def run_server(server, valgrind, env): @@ -116,17 +164,27 @@ def main(): .format(PORT)) sys.exit(1) + # Standard client/server tests. server_popen = run_server(server, valgrind, {}) wait_tcp_port(HOST, PORT) run_client_tests(client, valgrind) server_popen.kill() server_popen.wait() - run_server(server, valgrind, { + # Client/server tests w/ vectored I/O. + server_popen = run_server(server, valgrind, { "VECTORED_IO": "" }) wait_tcp_port(HOST, PORT) run_client_tests(client, valgrind) + server_popen.kill() + server_popen.wait() + + # Client/server tests w/ mandatory client authentication. + run_server(server, valgrind, { + "AUTH_CERT": "testdata/minica.pem", + }) + run_mtls_client_tests(client, valgrind) if __name__ == "__main__": From be8730c8036c2243b7f132baff8c4ccca04f40cd Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Wed, 28 Jun 2023 15:29:45 -0400 Subject: [PATCH 05/11] chore: ignore clion/jetbrains dir, venv dir. --- .gitignore | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.gitignore b/.gitignore index f9e44c1b..05161b08 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,5 @@ target Cargo.lock /build +.idea +.venv From ece658bfaad57bd5c08b00fbb3c6dd1108f479a6 Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Wed, 28 Jun 2023 15:27:58 -0400 Subject: [PATCH 06/11] deps: update pemfile, use unreleased Rustls, webpki. Updates `pemfile` from 0.2.1 to 1.0.3 to pick up support for reading DER encoded CRLs from .pem files. Updates webpki to use tip of main, picking up unreleased CRL support. Updates rustls to use a fork/branch that adds WIP CRL support. --- Cargo.toml | 8 ++++++-- build.rs | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 6202b60d..2b02fe44 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -23,13 +23,17 @@ read_buf = ["rustls/read_buf"] [dependencies] # Keep in sync with RUSTLS_CRATE_VERSION in build.rs -rustls = { version = "=0.21.0", features = [ "dangerous_configuration" ] } +rustls = { version = "=0.21.2", features = [ "dangerous_configuration" ] } webpki = "0.22" libc = "0.2" sct = "0.7" -rustls-pemfile = "0.2.1" +rustls-pemfile = "1.0.3" log = "0.4.17" [lib] name = "rustls_ffi" crate-type = ["lib", "staticlib"] + +[patch.crates-io] +rustls = { git = 'https://github.com/cpu/rustls', branch = 'cpu-crl-support' } +webpki = { package='rustls-webpki', git = 'https://github.com/rustls/webpki.git' } diff --git a/build.rs b/build.rs index 5e0b3b79..f038e130 100644 --- a/build.rs +++ b/build.rs @@ -3,7 +3,7 @@ use std::io::Write; use std::{env, fs, path::PathBuf}; // Keep in sync with Cargo.toml. -const RUSTLS_CRATE_VERSION: &str = "0.21.0"; +const RUSTLS_CRATE_VERSION: &str = "0.21.2"; fn main() { let out_dir = PathBuf::from(env::var_os("OUT_DIR").unwrap()); From 1c48d877735b31f981abc562056d9269ac5468d2 Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Fri, 30 Jun 2023 13:39:30 -0400 Subject: [PATCH 07/11] tidy: remove usage of removed upstream SCT features. The upstream Rustls project has dropped the minimal SCT support it offered. This commit tracks that change in rustls-ffi, removing the dep on the `sct` crate and removing related features. --- Cargo.toml | 1 - src/client.rs | 6 ++---- src/error.rs | 46 ++++++++-------------------------------------- src/rustls.h | 5 ----- 4 files changed, 10 insertions(+), 48 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 2b02fe44..31215cee 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -26,7 +26,6 @@ read_buf = ["rustls/read_buf"] rustls = { version = "=0.21.2", features = [ "dangerous_configuration" ] } webpki = "0.22" libc = "0.2" -sct = "0.7" rustls-pemfile = "1.0.3" log = "0.4.17" diff --git a/src/client.rs b/src/client.rs index 65ed94a8..098abc70 100644 --- a/src/client.rs +++ b/src/client.rs @@ -77,7 +77,6 @@ impl ServerCertVerifier for NoneVerifier { _end_entity: &Certificate, _intermediates: &[Certificate], _server_name: &rustls::ServerName, - _scts: &mut dyn Iterator, _ocsp_response: &[u8], _now: SystemTime, ) -> Result { @@ -232,7 +231,6 @@ impl rustls::client::ServerCertVerifier for Verifier { end_entity: &Certificate, intermediates: &[Certificate], server_name: &rustls::ServerName, - _scts: &mut dyn Iterator, ocsp_response: &[u8], _now: SystemTime, ) -> Result { @@ -327,7 +325,7 @@ impl rustls_client_config_builder { ffi_panic_boundary! { let builder = try_mut_from_ptr!(config_builder); let root_store: &RootCertStore = try_ref_from_ptr!(roots); - builder.verifier = Arc::new(rustls::client::WebPkiVerifier::new(root_store.clone(), None)); + builder.verifier = Arc::new(rustls::client::WebPkiVerifier::new(root_store.clone())); rustls_result::Ok } } @@ -371,7 +369,7 @@ impl rustls_client_config_builder { return rustls_result::CertificateParseError; } - config_builder.verifier = Arc::new(rustls::client::WebPkiVerifier::new(roots, None)); + config_builder.verifier = Arc::new(rustls::client::WebPkiVerifier::new(roots)); rustls_result::Ok } } diff --git a/src/error.rs b/src/error.rs index d238aa9b..edd4efff 100644 --- a/src/error.rs +++ b/src/error.rs @@ -159,14 +159,14 @@ u32_enum_builder! { AlertUnknownPSKIdentity => 7231, AlertCertificateRequired => 7232, AlertNoApplicationProtocol => 7233, - AlertUnknown => 7234, - - // https://docs.rs/sct/latest/sct/enum.Error.html - CertSCTMalformed => 7319, - CertSCTInvalidSignature => 7320, - CertSCTTimestampInFuture => 7321, - CertSCTUnsupportedVersion => 7322, - CertSCTUnknownLog => 7323 + AlertUnknown => 7234 + + // Reserved from previous use. + // CertSCTMalformed => 7319, + // CertSCTInvalidSignature => 7320, + // CertSCTTimestampInFuture => 7321, + // CertSCTUnsupportedVersion => 7322, + // CertSCTUnknownLog => 7323 } } @@ -214,11 +214,6 @@ impl rustls_result { | CertInvalidPurpose | CertApplicationVerificationFailure | CertOtherError - | CertSCTMalformed - | CertSCTInvalidSignature - | CertSCTTimestampInFuture - | CertSCTUnsupportedVersion - | CertSCTUnknownLog ) } } @@ -244,11 +239,6 @@ pub(crate) fn cert_result_to_error(result: rustls_result) -> rustls::Error { InvalidCertificate(CertificateError::ApplicationVerificationFailure) } CertOtherError => InvalidCertificate(CertificateError::Other(Arc::from(Box::from("")))), - CertSCTMalformed => InvalidSct(sct::Error::MalformedSct), - CertSCTInvalidSignature => InvalidSct(sct::Error::InvalidSignature), - CertSCTTimestampInFuture => InvalidSct(sct::Error::TimestampInFuture), - CertSCTUnsupportedVersion => InvalidSct(sct::Error::UnsupportedSctVersion), - CertSCTUnknownLog => InvalidSct(sct::Error::UnknownLog), _ => rustls::Error::General("".into()), } } @@ -279,17 +269,11 @@ fn test_rustls_result_is_cert_error() { for id in 7121..=7131 { assert!(rustls_result::rustls_result_is_cert_error(id)); } - - // Test SCTError range - for id in 7319..=7323 { - assert!(rustls_result::rustls_result_is_cert_error(id)); - } } pub(crate) fn map_error(input: rustls::Error) -> rustls_result { use rustls::AlertDescription as alert; use rustls_result::*; - use sct::Error as sct; match input { Error::InappropriateMessage { .. } => InappropriateMessage, @@ -387,13 +371,6 @@ pub(crate) fn map_error(input: rustls::Error) -> rustls_result { alert::Unknown(_) => AlertUnknown, _ => AlertUnknown, }, - Error::InvalidSct(e) => match e { - sct::MalformedSct => CertSCTMalformed, - sct::InvalidSignature => CertSCTInvalidSignature, - sct::TimestampInFuture => CertSCTTimestampInFuture, - sct::UnsupportedSctVersion => CertSCTUnsupportedVersion, - sct::UnknownLog => CertSCTUnknownLog, - }, _ => General, } } @@ -402,7 +379,6 @@ impl Display for rustls_result { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { use rustls::AlertDescription as alert; use rustls_result::*; - use sct::Error as sct; match self { // These variants are local to this glue layer. @@ -578,12 +554,6 @@ impl Display for rustls_result { AlertCertificateRequired => Error::AlertReceived(alert::CertificateRequired).fmt(f), AlertNoApplicationProtocol => Error::AlertReceived(alert::NoApplicationProtocol).fmt(f), AlertUnknown => Error::AlertReceived(alert::Unknown(0)).fmt(f), - - CertSCTMalformed => Error::InvalidSct(sct::MalformedSct).fmt(f), - CertSCTInvalidSignature => Error::InvalidSct(sct::InvalidSignature).fmt(f), - CertSCTTimestampInFuture => Error::InvalidSct(sct::TimestampInFuture).fmt(f), - CertSCTUnsupportedVersion => Error::InvalidSct(sct::UnsupportedSctVersion).fmt(f), - CertSCTUnknownLog => Error::InvalidSct(sct::UnknownLog).fmt(f), } } } diff --git a/src/rustls.h b/src/rustls.h index 5dc0c3eb..4387460c 100644 --- a/src/rustls.h +++ b/src/rustls.h @@ -104,11 +104,6 @@ enum rustls_result { RUSTLS_RESULT_ALERT_CERTIFICATE_REQUIRED = 7232, RUSTLS_RESULT_ALERT_NO_APPLICATION_PROTOCOL = 7233, RUSTLS_RESULT_ALERT_UNKNOWN = 7234, - RUSTLS_RESULT_CERT_SCT_MALFORMED = 7319, - RUSTLS_RESULT_CERT_SCT_INVALID_SIGNATURE = 7320, - RUSTLS_RESULT_CERT_SCT_TIMESTAMP_IN_FUTURE = 7321, - RUSTLS_RESULT_CERT_SCT_UNSUPPORTED_VERSION = 7322, - RUSTLS_RESULT_CERT_SCT_UNKNOWN_LOG = 7323, }; typedef uint32_t rustls_result; From 1d2f81153618e235a6f6c8b734d653c083978ceb Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Mon, 3 Jul 2023 12:37:43 -0400 Subject: [PATCH 08/11] cipher: switch client cert verifiers to mutable ptrs. In order to support adding CRLs to a constructed `rustls_client_cert_verifier` or `rustls_client_cert_verifier_optional` we need to change the constructor return type from `*const` to `*mut`. Corresponding destructors are updated as well. --- src/cipher.rs | 23 +++++++++++------------ src/rustls.h | 8 ++++---- 2 files changed, 15 insertions(+), 16 deletions(-) diff --git a/src/cipher.rs b/src/cipher.rs index 21409758..27b839c7 100644 --- a/src/cipher.rs +++ b/src/cipher.rs @@ -17,7 +17,7 @@ use crate::error::rustls_result; use crate::rslice::{rustls_slice_bytes, rustls_str}; use crate::{ ffi_panic_boundary, try_box_from_ptr, try_mut_from_ptr, try_ref_from_ptr, try_slice, - ArcCastPtr, BoxCastPtr, CastConstPtr, CastPtr, + ArcCastPtr, BoxCastPtr, CastPtr, }; use rustls_result::NullParameter; use std::ops::Deref; @@ -517,10 +517,11 @@ pub struct rustls_client_cert_verifier { _private: [u8; 0], } -impl CastConstPtr for rustls_client_cert_verifier { +impl CastPtr for rustls_client_cert_verifier { type RustType = AllowAnyAuthenticatedClient; } +impl BoxCastPtr for rustls_client_cert_verifier {} impl ArcCastPtr for rustls_client_cert_verifier {} impl rustls_client_cert_verifier { @@ -533,11 +534,11 @@ impl rustls_client_cert_verifier { #[no_mangle] pub extern "C" fn rustls_client_cert_verifier_new( store: *const rustls_root_cert_store, - ) -> *const rustls_client_cert_verifier { + ) -> *mut rustls_client_cert_verifier { ffi_panic_boundary! { let store: &RootCertStore = try_ref_from_ptr!(store); let client_cert_verifier = AllowAnyAuthenticatedClient::new(store.clone()); - return Arc::into_raw(client_cert_verifier.boxed()) as *const _; + BoxCastPtr::to_mut_ptr(client_cert_verifier) } } @@ -548,9 +549,7 @@ impl rustls_client_cert_verifier { /// 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_cert_verifier_free( - verifier: *const rustls_client_cert_verifier, - ) { + pub extern "C" fn rustls_client_cert_verifier_free(verifier: *mut rustls_client_cert_verifier) { ffi_panic_boundary! { rustls_client_cert_verifier::free(verifier); } @@ -568,10 +567,11 @@ pub struct rustls_client_cert_verifier_optional { _private: [u8; 0], } -impl CastConstPtr for rustls_client_cert_verifier_optional { +impl CastPtr for rustls_client_cert_verifier_optional { type RustType = AllowAnyAnonymousOrAuthenticatedClient; } +impl BoxCastPtr for rustls_client_cert_verifier_optional {} impl ArcCastPtr for rustls_client_cert_verifier_optional {} impl rustls_client_cert_verifier_optional { @@ -583,13 +583,12 @@ impl rustls_client_cert_verifier_optional { /// ownership of the pointed-to data. #[no_mangle] pub extern "C" fn rustls_client_cert_verifier_optional_new( - store: *const rustls_root_cert_store, + store: *mut rustls_root_cert_store, ) -> *const rustls_client_cert_verifier_optional { ffi_panic_boundary! { let store: &RootCertStore = try_ref_from_ptr!(store); let client_cert_verifier = AllowAnyAnonymousOrAuthenticatedClient::new(store.clone()); - return Arc::into_raw(client_cert_verifier.boxed()) - as *const _; + BoxCastPtr::to_mut_ptr(client_cert_verifier) } } @@ -601,7 +600,7 @@ impl rustls_client_cert_verifier_optional { /// Calling with NULL is fine. Must not be called twice with the same value. #[no_mangle] pub extern "C" fn rustls_client_cert_verifier_optional_free( - verifier: *const rustls_client_cert_verifier_optional, + verifier: *mut rustls_client_cert_verifier_optional, ) { ffi_panic_boundary! { rustls_client_cert_verifier_optional::free(verifier); diff --git a/src/rustls.h b/src/rustls.h index 4387460c..462ef637 100644 --- a/src/rustls.h +++ b/src/rustls.h @@ -947,7 +947,7 @@ void rustls_root_cert_store_free(struct rustls_root_cert_store *store); * This copies the contents of the rustls_root_cert_store. It does not take * ownership of the pointed-to memory. */ -const struct rustls_client_cert_verifier *rustls_client_cert_verifier_new(const struct rustls_root_cert_store *store); +struct rustls_client_cert_verifier *rustls_client_cert_verifier_new(const struct rustls_root_cert_store *store); /** * "Free" a verifier previously returned from @@ -957,7 +957,7 @@ const struct rustls_client_cert_verifier *rustls_client_cert_verifier_new(const * consider this pointer unusable after "free"ing it. * Calling with NULL is fine. Must not be called twice with the same value. */ -void rustls_client_cert_verifier_free(const struct rustls_client_cert_verifier *verifier); +void rustls_client_cert_verifier_free(struct rustls_client_cert_verifier *verifier); /** * Create a new rustls_client_cert_verifier_optional for the root store. The @@ -967,7 +967,7 @@ void rustls_client_cert_verifier_free(const struct rustls_client_cert_verifier * * This copies the contents of the rustls_root_cert_store. It does not take * ownership of the pointed-to data. */ -const struct rustls_client_cert_verifier_optional *rustls_client_cert_verifier_optional_new(const struct rustls_root_cert_store *store); +const struct rustls_client_cert_verifier_optional *rustls_client_cert_verifier_optional_new(struct rustls_root_cert_store *store); /** * "Free" a verifier previously returned from @@ -977,7 +977,7 @@ const struct rustls_client_cert_verifier_optional *rustls_client_cert_verifier_o * consider this pointer unusable after "free"ing it. * Calling with NULL is fine. Must not be called twice with the same value. */ -void rustls_client_cert_verifier_optional_free(const struct rustls_client_cert_verifier_optional *verifier); +void rustls_client_cert_verifier_optional_free(struct rustls_client_cert_verifier_optional *verifier); /** * Create a rustls_client_config_builder. Caller owns the memory and must From 15d632d5590f4207869c0b269b507f15ab611beb Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Mon, 3 Jul 2023 12:58:01 -0400 Subject: [PATCH 09/11] cipher: add client verifier CRL pem fns. This commit updates the `rustls_client_cert_verifier` and `rustls_client_cert_verifier_optional` API surface to include a function for loading CRLs from a PEM file. --- src/cipher.rs | 54 +++++++++++++++++++++++++++++++++++++++++++++++++-- src/error.rs | 1 + src/rustls.h | 16 +++++++++++++++ 3 files changed, 69 insertions(+), 2 deletions(-) diff --git a/src/cipher.rs b/src/cipher.rs index 27b839c7..8c4fbd1e 100644 --- a/src/cipher.rs +++ b/src/cipher.rs @@ -5,7 +5,9 @@ use std::ptr::null; use std::slice; use std::sync::Arc; -use rustls::server::{AllowAnyAnonymousOrAuthenticatedClient, AllowAnyAuthenticatedClient}; +use rustls::server::{ + AllowAnyAnonymousOrAuthenticatedClient, AllowAnyAuthenticatedClient, UnparsedCertRevocationList, +}; use rustls::sign::CertifiedKey; use rustls::{ Certificate, PrivateKey, RootCertStore, SupportedCipherSuite, ALL_CIPHER_SUITES, @@ -13,7 +15,7 @@ use rustls::{ }; use rustls_pemfile::{certs, pkcs8_private_keys, rsa_private_keys}; -use crate::error::rustls_result; +use crate::error::{map_error, rustls_result}; use crate::rslice::{rustls_slice_bytes, rustls_str}; use crate::{ ffi_panic_boundary, try_box_from_ptr, try_mut_from_ptr, try_ref_from_ptr, try_slice, @@ -542,6 +544,30 @@ impl rustls_client_cert_verifier { } } + /// Add one or more certificate revocation lists (CRLs) to the client certificate + /// verifier using PEM encoded data. + #[no_mangle] + pub extern "C" fn rustls_client_cert_verifier_add_crl_pem( + verifier: *mut rustls_client_cert_verifier, + pem: *const u8, + pem_len: size_t, + ) -> rustls_result { + ffi_panic_boundary! { + let verifier: &mut AllowAnyAuthenticatedClient = try_mut_from_ptr!(verifier); + let crl_pem: &[u8] = try_slice!(pem, pem_len); + let crls_der = match rustls_pemfile::crls(&mut Cursor::new(crl_pem)) { + Ok(vv) => vv, + Err(_) => return rustls_result::CertificateParseError, // TODO(XXX): Better err. + }; + for crl_der in crls_der { + if let Err(e) = verifier.add_crl(UnparsedCertRevocationList(crl_der)) { + return map_error(e); + } + } + rustls_result::Ok + } + } + /// "Free" a verifier previously returned from /// rustls_client_cert_verifier_new. Since rustls_client_cert_verifier is actually an /// atomically reference-counted pointer, extant server_configs may still @@ -592,6 +618,30 @@ impl rustls_client_cert_verifier_optional { } } + /// Add one or more certificate revocation lists (CRLs) to the client certificate + /// verifier using PEM encoded data. + #[no_mangle] + pub extern "C" fn rustls_client_cert_verifier_optional_add_crl_pem( + verifier: *mut rustls_client_cert_verifier_optional, + pem: *const u8, + pem_len: size_t, + ) -> rustls_result { + ffi_panic_boundary! { + let verifier: &mut AllowAnyAnonymousOrAuthenticatedClient = try_mut_from_ptr!(verifier); + let crl_pem: &[u8] = try_slice!(pem, pem_len); + let crls_der = match rustls_pemfile::crls(&mut Cursor::new(crl_pem)) { + Ok(vv) => vv, + Err(_) => return rustls_result::CertificateParseError, // TODO(XXX): Better err. + }; + for crl_der in crls_der { + if let Err(e) = verifier.add_crl(UnparsedCertRevocationList(crl_der)) { + return map_error(e); + } + } + rustls_result::Ok + } + } + /// "Free" a verifier previously returned from /// rustls_client_cert_verifier_optional_new. Since rustls_client_cert_verifier_optional /// is actually an atomically reference-counted pointer, extant server_configs may still diff --git a/src/error.rs b/src/error.rs index edd4efff..edd62a4a 100644 --- a/src/error.rs +++ b/src/error.rs @@ -271,6 +271,7 @@ fn test_rustls_result_is_cert_error() { } } +// TODO(@cpu): update for CRL errors. pub(crate) fn map_error(input: rustls::Error) -> rustls_result { use rustls::AlertDescription as alert; use rustls_result::*; diff --git a/src/rustls.h b/src/rustls.h index 462ef637..0c6b3703 100644 --- a/src/rustls.h +++ b/src/rustls.h @@ -949,6 +949,14 @@ void rustls_root_cert_store_free(struct rustls_root_cert_store *store); */ struct rustls_client_cert_verifier *rustls_client_cert_verifier_new(const struct rustls_root_cert_store *store); +/** + * Add one or more certificate revocation lists (CRLs) to the client certificate + * verifier using PEM encoded data. + */ +rustls_result rustls_client_cert_verifier_add_crl_pem(struct rustls_client_cert_verifier *verifier, + const uint8_t *pem, + size_t pem_len); + /** * "Free" a verifier previously returned from * rustls_client_cert_verifier_new. Since rustls_client_cert_verifier is actually an @@ -969,6 +977,14 @@ void rustls_client_cert_verifier_free(struct rustls_client_cert_verifier *verifi */ const struct rustls_client_cert_verifier_optional *rustls_client_cert_verifier_optional_new(struct rustls_root_cert_store *store); +/** + * Add one or more certificate revocation lists (CRLs) to the client certificate + * verifier using PEM encoded data. + */ +rustls_result rustls_client_cert_verifier_optional_add_crl_pem(struct rustls_client_cert_verifier_optional *verifier, + const uint8_t *pem, + size_t pem_len); + /** * "Free" a verifier previously returned from * rustls_client_cert_verifier_optional_new. Since rustls_client_cert_verifier_optional From ed3eb5dc9ff772a4ed94485c115925c8a89ed7e7 Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Mon, 3 Jul 2023 13:07:53 -0400 Subject: [PATCH 10/11] server: support reading CRL PEM for client auth. This commit updates the `tests/server.c` example program to support reading one or more CRLs from a single PEM encoded CRL file, provided via `AUTH_CRL`. This option is only processed when the server is performing mandatory client authentication (e.g. `AUTH_CERT` was provided). --- tests/server.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/tests/server.c b/tests/server.c index 019ba9a1..f9c3522c 100644 --- a/tests/server.c +++ b/tests/server.c @@ -255,7 +255,7 @@ main(int argc, const char **argv) struct rustls_connection *rconn = NULL; const struct rustls_certified_key *certified_key = NULL; struct rustls_slice_bytes alpn_http11; - const struct rustls_client_cert_verifier *client_cert_verifier = NULL; + struct rustls_client_cert_verifier *client_cert_verifier = NULL; struct rustls_root_cert_store *client_cert_root_store = NULL; alpn_http11.data = (unsigned char*)"http/1.1"; @@ -300,6 +300,22 @@ main(int argc, const char **argv) rustls_root_cert_store_add_pem(client_cert_root_store, (uint8_t *)certbuf, certbuf_len, true); client_cert_verifier = rustls_client_cert_verifier_new(client_cert_root_store); + + char* auth_crl = getenv("AUTH_CRL"); + char crlbuf[10000]; + size_t crlbuf_len; + if(auth_crl) { + result = read_file(argv[0], auth_crl, crlbuf, sizeof(crlbuf), &crlbuf_len); + if(result != DEMO_OK) { + goto cleanup; + } + + result = rustls_client_cert_verifier_add_crl_pem(client_cert_verifier, (uint8_t *)crlbuf, crlbuf_len); + if(result != RUSTLS_RESULT_OK) { + goto cleanup; + } + } + rustls_server_config_builder_set_client_verifier(config_builder, client_cert_verifier); } From 11b58b7b8d35b364f87c0b5583b9dc778712b98f Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Mon, 3 Jul 2023 13:19:45 -0400 Subject: [PATCH 11/11] tests: add CRL mTLS test. This commit adds a simple test CRL (`testdata/test.crl.pem`) that lists the `testdata/localhost/cert.pem` certificate as revoked, but _not_ the `testdata/example.com/cert.pem` certificate. The `client-server.py` integration test driver is then updated with a suite that will start the server binary in a mode that requires mTLS, and that loads the test crl. Two connection attempts are made with the client binary: one using the `example.com` client cert that isn't expected to error, and one using the `localhost` client cert that _is_ expected to error (since it's revoked). --- testdata/test.crl.pem | 12 ++++++++++++ tests/client-server.py | 43 +++++++++++++++++++++++++++++++++++++++++- 2 files changed, 54 insertions(+), 1 deletion(-) create mode 100644 testdata/test.crl.pem diff --git a/testdata/test.crl.pem b/testdata/test.crl.pem new file mode 100644 index 00000000..ca069cd5 --- /dev/null +++ b/testdata/test.crl.pem @@ -0,0 +1,12 @@ +-----BEGIN X509 CRL----- +MIIBxjCBrwIBATANBgkqhkiG9w0BAQsFADAgMR4wHAYDVQQDExVtaW5pY2Egcm9v +dCBjYSAxMGE3YTAXDTIzMDcwMzE3MTgxMloXDTIzMDcxMDE3MTgxMlowKTAnAghT +E+2CSHaYmBcNMjMwNzAzMTcxNzU5WjAMMAoGA1UdFQQDCgEBoDAwLjAfBgNVHSME +GDAWgBQ19H4hMuTID22xvBfISviOa+S+EDALBgNVHRQEBAICEAEwDQYJKoZIhvcN +AQELBQADggEBAF5fOpNZGLsHGAUasx5Il79My6EU66igE0YZWVzgX8EaCt1RMCFx +osumXkaPiohICSsczFlnJolpwacsHx/K/IMYvthna8lbAxhuWharRqoHUK+BdTDD +wtThMBC2dCNoLro/6cIpMov9OXjh8291ogIy0qIiSm20JiaWTB+0V7A6gA7riTXC +yzJTyGECLS9XP6rt+SYmcDn0D1jxfsIli0kYBJdKb3O0xF05oBaWadSLuXbcA41+ +Kcw07HACaUrR6BCrR3CjnnlTl6Pr25cQi3zPya7lNDQWqhLNx0sU2jviVZQe1nIA +Ie8Ha2syCv0aa33s0dUY6hOKDbLTGpI8f/E= +-----END X509 CRL----- diff --git a/tests/client-server.py b/tests/client-server.py index 4f0d10e4..c7f579cf 100755 --- a/tests/client-server.py +++ b/tests/client-server.py @@ -131,6 +131,38 @@ def run_mtls_client_tests(client, valgrind): ) +def run_mtls_client_crl_tests(client, valgrind): + run_with_maybe_valgrind( + [ + client, + HOST, + str(PORT), + "/" + ], + { + "CA_FILE": "testdata/minica.pem", + "AUTH_CERT": "testdata/example.com/cert.pem", + "AUTH_KEY": "testdata/example.com/key.pem", + }, + valgrind + ) + run_with_maybe_valgrind( + [ + client, + HOST, + str(PORT), + "/" + ], + { + "CA_FILE": "testdata/minica.pem", + "AUTH_CERT": "testdata/localhost/cert.pem", + "AUTH_KEY": "testdata/localhost/key.pem", + }, + valgrind, + expect_error=True # Client connecting w/ revoked cert should err. + ) + + def run_server(server, valgrind, env): args = [ server, @@ -181,10 +213,19 @@ def main(): server_popen.wait() # Client/server tests w/ mandatory client authentication. - run_server(server, valgrind, { + server_popen = run_server(server, valgrind, { "AUTH_CERT": "testdata/minica.pem", }) run_mtls_client_tests(client, valgrind) + server_popen.kill() + server_popen.wait() + + # Client/server tests w/ mandatory client authentication & CRL. + run_server(server, valgrind, { + "AUTH_CERT": "testdata/minica.pem", + "AUTH_CRL": "testdata/test.crl.pem", + }) + run_mtls_client_crl_tests(client, valgrind) if __name__ == "__main__":