From 2b95cc30df67c1a99f683e63f0e887681beb5dac Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Tue, 1 Oct 2024 13:16:19 -0400 Subject: [PATCH 1/7] connection: tidy up extra bindings/match statements In a few places we create named bindings without needing them. In a couple other places we're doing work with explicit `match`'s that could be done more naturally with `map()`/`and_then()`/`unwrap_or()`. --- src/connection.rs | 51 +++++++++++++++++++---------------------------- 1 file changed, 21 insertions(+), 30 deletions(-) diff --git a/src/connection.rs b/src/connection.rs index c9d418f5..2bdb76d1 100644 --- a/src/connection.rs +++ b/src/connection.rs @@ -104,8 +104,7 @@ impl rustls_connection { conn: *mut rustls_connection, userdata: *mut c_void, ) { - let conn = try_mut_from_ptr!(conn); - conn.userdata = userdata; + try_mut_from_ptr!(conn).userdata = userdata; } /// Set the logging callback for this connection. The log callback will be invoked @@ -261,8 +260,7 @@ impl rustls_connection { #[no_mangle] pub extern "C" fn rustls_connection_wants_read(conn: *const rustls_connection) -> bool { ffi_panic_boundary! { - let conn = try_ref_from_ptr!(conn); - conn.wants_read() + try_ref_from_ptr!(conn).wants_read() } } @@ -270,8 +268,7 @@ impl rustls_connection { #[no_mangle] pub extern "C" fn rustls_connection_wants_write(conn: *const rustls_connection) -> bool { ffi_panic_boundary! { - let conn = try_ref_from_ptr!(conn); - conn.wants_write() + try_ref_from_ptr!(conn).wants_write() } } @@ -286,8 +283,7 @@ impl rustls_connection { #[no_mangle] pub extern "C" fn rustls_connection_is_handshaking(conn: *const rustls_connection) -> bool { ffi_panic_boundary! { - let conn = try_ref_from_ptr!(conn); - conn.is_handshaking() + try_ref_from_ptr!(conn).is_handshaking() } } @@ -299,8 +295,7 @@ impl rustls_connection { #[no_mangle] pub extern "C" fn rustls_connection_set_buffer_limit(conn: *mut rustls_connection, n: usize) { ffi_panic_boundary! { - let conn = try_mut_from_ptr!(conn); - conn.set_buffer_limit(Some(n)); + try_mut_from_ptr!(conn).set_buffer_limit(Some(n)); } } @@ -309,8 +304,7 @@ impl rustls_connection { #[no_mangle] pub extern "C" fn rustls_connection_send_close_notify(conn: *mut rustls_connection) { ffi_panic_boundary! { - let conn = try_mut_from_ptr!(conn); - conn.send_close_notify(); + try_mut_from_ptr!(conn).send_close_notify(); } } @@ -329,8 +323,10 @@ impl rustls_connection { i: size_t, ) -> *const rustls_certificate<'a> { ffi_panic_boundary! { - let conn = try_ref_from_ptr!(conn); - match conn.peer_certificates().and_then(|c| c.get(i)) { + match try_ref_from_ptr!(conn) + .peer_certificates() + .and_then(|c| c.get(i)) + { Some(cert) => cert as *const CertificateDer as *const _, None => null(), } @@ -382,11 +378,10 @@ impl rustls_connection { conn: *const rustls_connection, ) -> u16 { ffi_panic_boundary! { - let conn = try_ref_from_ptr!(conn); - match conn.protocol_version() { - Some(p) => u16::from(p), - _ => 0, - } + try_ref_from_ptr!(conn) + .protocol_version() + .map(u16::from) + .unwrap_or_default() } } @@ -400,10 +395,10 @@ impl rustls_connection { conn: *const rustls_connection, ) -> u16 { ffi_panic_boundary! { - match try_ref_from_ptr!(conn).negotiated_cipher_suite() { - Some(cs) => u16::from(cs.suite()), - None => u16::from(TLS_NULL_WITH_NULL_NULL), - } + try_ref_from_ptr!(conn) + .negotiated_cipher_suite() + .map(|cs| u16::from(cs.suite())) + .unwrap_or(u16::from(TLS_NULL_WITH_NULL_NULL)) } } @@ -420,14 +415,10 @@ impl rustls_connection { conn: *const rustls_connection, ) -> rustls_str<'static> { ffi_panic_boundary! { - let cs_name = try_ref_from_ptr!(conn) + try_ref_from_ptr!(conn) .negotiated_cipher_suite() - .and_then(|cs| cs.suite().as_str()) - .and_then(|name| rustls_str::try_from(name).ok()); - match cs_name { - Some(cs) => cs, - None => rustls_str::from_str_unchecked(""), - } + .and_then(|cs| rustls_str::try_from(cs.suite().as_str().unwrap_or_default()).ok()) + .unwrap_or(rustls_str::from_str_unchecked("")) } } From d54e630d48a7340da602fd253592fb56c439e0ba Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Tue, 1 Oct 2024 13:19:22 -0400 Subject: [PATCH 2/7] connection: allow getting negotiated KEX group This commit adds `rustls_connection_get_negotiated_key_exchange_group()` and `rustls_connection_get_negotiated_key_exchange_group_name()` functions. These operate similar to the existing `rustls_connection_get_negotiated_ciphersuite()` and `rustls_connection_get_negotiated_ciphersuite_name()` functions, except returning details of the negotiated key exchange group (when available) as opposed to the ciphersuite. --- src/connection.rs | 35 +++++++++++++++++++++++++++++++++++ src/rustls.h | 19 +++++++++++++++++++ tests/client.c | 11 +++++++++-- tests/server.c | 11 +++++++++-- 4 files changed, 72 insertions(+), 4 deletions(-) diff --git a/src/connection.rs b/src/connection.rs index 2bdb76d1..4cbbd15c 100644 --- a/src/connection.rs +++ b/src/connection.rs @@ -422,6 +422,41 @@ impl rustls_connection { } } + /// Retrieves the [IANA registered supported group identifier][IANA] agreed with the peer. + /// + /// This returns Reserved (0x0000) until the key exchange group is agreed. + /// + /// [IANA]: + #[no_mangle] + pub extern "C" fn rustls_connection_get_negotiated_key_exchange_group( + conn: *const rustls_connection, + ) -> u16 { + ffi_panic_boundary! { + try_ref_from_ptr!(conn) + .negotiated_key_exchange_group() + .map(|kxg| u16::from(kxg.name())) + .unwrap_or_default() + } + } + + /// Retrieves the key exchange group name agreed with the peer. + /// + /// This returns "" until the key exchange group is agreed. + /// + /// The lifetime of the `rustls_str` is the lifetime of the program, it does not + /// need to be freed. + #[no_mangle] + pub extern "C" fn rustls_connection_get_negotiated_key_exchange_group_name( + conn: *const rustls_connection, + ) -> rustls_str<'static> { + ffi_panic_boundary! { + try_ref_from_ptr!(conn) + .negotiated_key_exchange_group() + .and_then(|kxg| rustls_str::try_from(kxg.name().as_str().unwrap_or_default()).ok()) + .unwrap_or(rustls_str::from_str_unchecked("")) + } + } + /// Write up to `count` plaintext bytes from `buf` into the `rustls_connection`. /// This will increase the number of output bytes available to /// `rustls_connection_write_tls`. diff --git a/src/rustls.h b/src/rustls.h index e4f9be1d..33e67fdd 100644 --- a/src/rustls.h +++ b/src/rustls.h @@ -1898,6 +1898,25 @@ uint16_t rustls_connection_get_negotiated_ciphersuite(const struct rustls_connec */ struct rustls_str rustls_connection_get_negotiated_ciphersuite_name(const struct rustls_connection *conn); +/** + * Retrieves the [IANA registered supported group identifier][IANA] agreed with the peer. + * + * This returns Reserved (0x0000) until the key exchange group is agreed. + * + * [IANA]: + */ +uint16_t rustls_connection_get_negotiated_key_exchange_group(const struct rustls_connection *conn); + +/** + * Retrieves the key exchange group name agreed with the peer. + * + * This returns "" until the key exchange group is agreed. + * + * The lifetime of the `rustls_str` is the lifetime of the program, it does not + * need to be freed. + */ +struct rustls_str rustls_connection_get_negotiated_key_exchange_group_name(const struct rustls_connection *conn); + /** * Write up to `count` plaintext bytes from `buf` into the `rustls_connection`. * This will increase the number of output bytes available to diff --git a/tests/client.c b/tests/client.c index a147023f..1a1c4ef7 100644 --- a/tests/client.c +++ b/tests/client.c @@ -170,8 +170,8 @@ send_request_and_read_response(struct conndata *conn, unsigned long content_length = 0; size_t headers_len = 0; struct rustls_str version; - int ciphersuite_id; - struct rustls_str ciphersuite_name; + int ciphersuite_id, kex_id; + struct rustls_str ciphersuite_name, kex_name; version = rustls_version(); memset(buf, '\0', sizeof(buf)); @@ -298,6 +298,13 @@ send_request_and_read_response(struct conndata *conn, LOG_SIMPLE("send_request_and_read_response: loop fell through"); drain_plaintext: + kex_id = rustls_connection_get_negotiated_key_exchange_group(rconn); + kex_name = rustls_connection_get_negotiated_key_exchange_group_name(rconn); + LOG("negotiated key exchange: %.*s (%#x)", + (int)kex_name.len, + kex_name.data, + kex_id); + result = copy_plaintext_to_buffer(conn); if(result != DEMO_OK && result != DEMO_EOF) { goto cleanup; diff --git a/tests/server.c b/tests/server.c index 3c6846fc..fa6a8f27 100644 --- a/tests/server.c +++ b/tests/server.c @@ -127,8 +127,8 @@ handle_conn(struct conndata *conn) int sockfd = conn->fd; struct timeval tv; enum exchange_state state = READING_REQUEST; - int ciphersuite_id; - struct rustls_str ciphersuite_name; + int ciphersuite_id, kex_id; + struct rustls_str ciphersuite_name, kex_name; LOG("acccepted conn on fd %d", conn->fd); @@ -198,6 +198,13 @@ handle_conn(struct conndata *conn) (int)ciphersuite_name.len, ciphersuite_name.data, ciphersuite_id); + kex_id = rustls_connection_get_negotiated_key_exchange_group(rconn); + kex_name = + rustls_connection_get_negotiated_key_exchange_group_name(rconn); + LOG("negotiated key exchange: %.*s (%#x)", + (int)kex_name.len, + kex_name.data, + kex_id); rustls_connection_get_alpn_protocol( rconn, &negotiated_alpn, &negotiated_alpn_len); From dbb9522a8e4b80361a0f0c0e8bcb3f5a48a11011 Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Tue, 1 Oct 2024 13:32:37 -0400 Subject: [PATCH 3/7] connection: add API for queued traffic key refresh Adds a `rustls_connection_refresh_traffic_keys()` fn for queuing a traffic key refresh, e.g. because you know the connection is about to be idle for a long time and you wish to roll keys ahead of this. There's not a great place to use this from `client.c` or `server.c` so for now I've added this API without integration test coverage. --- src/connection.rs | 19 +++++++++++++++++++ src/rustls.h | 11 +++++++++++ 2 files changed, 30 insertions(+) diff --git a/src/connection.rs b/src/connection.rs index 4cbbd15c..1e4036e0 100644 --- a/src/connection.rs +++ b/src/connection.rs @@ -308,6 +308,25 @@ impl rustls_connection { } } + /// Queues a TLS1.3 key_update message to refresh a connection’s keys. + /// + /// Rustls internally manages key updates as required and so this function should + /// seldom be used. See the Rustls documentation for important caveats and suggestions + /// on occasions that merit its use. + /// + /// + #[no_mangle] + pub extern "C" fn rustls_connection_refresh_traffic_keys( + conn: *mut rustls_connection, + ) -> rustls_result { + ffi_panic_boundary! { + match try_mut_from_ptr!(conn).refresh_traffic_keys() { + Ok(_) => rustls_result::Ok, + Err(e) => map_error(e), + } + } + } + /// Return the i-th certificate provided by the peer. /// Index 0 is the end entity certificate. Higher indexes are certificates /// in the chain. Requesting an index higher than what is available returns diff --git a/src/rustls.h b/src/rustls.h index 33e67fdd..45ae69f1 100644 --- a/src/rustls.h +++ b/src/rustls.h @@ -1837,6 +1837,17 @@ void rustls_connection_set_buffer_limit(struct rustls_connection *conn, size_t n */ void rustls_connection_send_close_notify(struct rustls_connection *conn); +/** + * Queues a TLS1.3 key_update message to refresh a connection’s keys. + * + * Rustls internally manages key updates as required and so this function should + * seldom be used. See the Rustls documentation for important caveats and suggestions + * on occasions that merit its use. + * + * + */ +rustls_result rustls_connection_refresh_traffic_keys(struct rustls_connection *conn); + /** * Return the i-th certificate provided by the peer. * Index 0 is the end entity certificate. Higher indexes are certificates From 2aef9dd5cdc0d9b82d9e379abba9e8aee1bbb2c6 Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Tue, 1 Oct 2024 13:44:36 -0400 Subject: [PATCH 4/7] cipher: add rustls_certified_keys_match() This commit exposes the upstream `rustls::CertifiedKey::keys_match()` fn, and uses it in the test `common.c` helper for loading a `rustls_certified_key`. This lets us bail early for mismatched certs/keys, and offers downstream projects the chance to do similar. --- src/cipher.rs | 19 +++++++++++++++++++ src/rustls.h | 11 +++++++++++ tests/common.c | 10 ++++++++++ 3 files changed, 40 insertions(+) diff --git a/src/cipher.rs b/src/cipher.rs index 9aef182a..f9ba7167 100644 --- a/src/cipher.rs +++ b/src/cipher.rs @@ -300,6 +300,25 @@ impl rustls_certified_key { } } + /// Verify the consistency of this `rustls_certified_key`'s public and private keys. + /// + /// This is done by performing a comparison of subject public key information (SPKI) bytes + /// between the certificate and private key. + /// + /// If the private key matches the certificate this function returns `RUSTLS_RESULT_OK`, + /// otherwise an error `rustls_result` is returned. + #[no_mangle] + pub extern "C" fn rustls_certified_key_keys_match( + key: *const rustls_certified_key, + ) -> rustls_result { + ffi_panic_boundary! { + match try_ref_from_ptr!(key).keys_match() { + Ok(_) => rustls_result::Ok, + Err(e) => map_error(e), + } + } + } + /// "Free" a certified_key previously returned from `rustls_certified_key_build`. /// /// Since certified_key is actually an atomically reference-counted pointer, diff --git a/src/rustls.h b/src/rustls.h index 45ae69f1..748d3efe 100644 --- a/src/rustls.h +++ b/src/rustls.h @@ -1147,6 +1147,17 @@ rustls_result rustls_certified_key_clone_with_ocsp(const struct rustls_certified const struct rustls_slice_bytes *ocsp_response, const struct rustls_certified_key **cloned_key_out); +/** + * Verify the consistency of this `rustls_certified_key`'s public and private keys. + * + * This is done by performing a comparison of subject public key information (SPKI) bytes + * between the certificate and private key. + * + * If the private key matches the certificate this function returns `RUSTLS_RESULT_OK`, + * otherwise an error `rustls_result` is returned. + */ +rustls_result rustls_certified_key_keys_match(const struct rustls_certified_key *key); + /** * "Free" a certified_key previously returned from `rustls_certified_key_build`. * diff --git a/tests/common.c b/tests/common.c index b03df1a2..dbebd51c 100644 --- a/tests/common.c +++ b/tests/common.c @@ -383,6 +383,16 @@ load_cert_and_key(const char *certfile, const char *keyfile) print_error("parsing certificate and key", result); return NULL; } + + if(rustls_certified_key_keys_match(certified_key) != RUSTLS_RESULT_OK) { + fprintf(stderr, + "private key %s does not match certificate %s public key\n", + keyfile, + certfile); + rustls_certified_key_free(certified_key); + return NULL; + } + return certified_key; } From b9641543dc9022b428fbaa554f0df2d10fe694e9 Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Tue, 1 Oct 2024 14:03:16 -0400 Subject: [PATCH 5/7] add compression crate feature, default to disabled This commit adds a new crate feature `cert_compression` that when enabled will activate the `rustls/brotli` and `rustls/zlib` features. This in turn will update client and server connections to attempt to use RFC 8879[0] certificate compression. No support is provided for implementing custom compression algorithms. I suspect the need for this is quite niche. Similarly there's no API surface for enabling the crate feature but disabling compression support per-connection. Let's wait for someone with a use-case to come along before making things more complicated. Both Makefiles and the CMake build are updated to _disable_ the feature by default, toggleable with CERT_COMPRESSION=true build variable. This is disabled by default because the zlib-rs crate requires a MSRV of 1.73+. [0]: https://www.rfc-editor.org/rfc/rfc8879 --- .github/workflows/test.yaml | 38 +++++++++++++++++++++++++++++++ CMakeLists.txt | 6 +++++ Cargo.lock | 45 +++++++++++++++++++++++++++++++++++++ Cargo.toml | 1 + Makefile | 6 +++++ Makefile.pkg-config | 5 +++++ 6 files changed, 101 insertions(+) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 46edb178..0038a11d 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -82,6 +82,23 @@ jobs: run: sudo apt-get update && sudo apt-get install -y valgrind - run: VALGRIND=valgrind make PROFILE=release test integration + cert-compression: + name: Certificate Compression + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + persist-credentials: false + - uses: actions/checkout@v4 + with: + persist-credentials: false + - name: Install nightly rust toolchain + uses: dtolnay/rust-toolchain@nightly + - name: Unit tests + run: make PROFILE=debug CERT_COMPRESSION=true test + - name: Integration tests + run: make PROFILE=debug CERT_COMPRESSION=true integration + test-windows-cmake-debug: name: Windows CMake, Debug configuration runs-on: windows-latest @@ -130,6 +147,27 @@ jobs: CLIENT_BINARY: D:\a\rustls-ffi\rustls-ffi\build\tests\Release\client.exe SERVER_BINARY: D:\a\rustls-ffi\rustls-ffi\build\tests\Release\server.exe + test-windows-cmake-compression: + name: Windows CMake, Cert. Compression + runs-on: windows-latest + steps: + - uses: actions/checkout@v4 + with: + persist-credentials: false + - name: Install nightly rust toolchain + uses: dtolnay/rust-toolchain@nightly + - name: Install NASM for aws-lc-rs + uses: ilammy/setup-nasm@v1 + - name: Configure CMake enabling cert compression + run: cmake -DCERT_COMPRESSION="true" -S . -B build + - name: Build, release configuration, compression + run: cmake --build build --config Release + - name: Integration test, release configuration, compression + run: cargo test --features=cert_compression --locked --test client_server client_server_integration -- --ignored --exact + env: + CLIENT_BINARY: D:\a\rustls-ffi\rustls-ffi\build\tests\Release\client.exe + SERVER_BINARY: D:\a\rustls-ffi\rustls-ffi\build\tests\Release\server.exe + ensure-header-updated: runs-on: ubuntu-latest steps: diff --git a/CMakeLists.txt b/CMakeLists.txt index eb884b6d..f460752e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -8,6 +8,8 @@ if (NOT (CRYPTO_PROVIDER STREQUAL "aws-lc-rs" OR CRYPTO_PROVIDER STREQUAL "ring" message(FATAL_ERROR "Invalid crypto provider specified: ${CRYPTO_PROVIDER}. Must be 'aws-lc-rs' or 'ring'.") endif () +set(CERT_COMPRESSION "false" CACHE STRING "Whether to enable brotli and zlib certificate compression support") + set(CARGO_FEATURES --no-default-features) if (CRYPTO_PROVIDER STREQUAL "aws-lc-rs") list(APPEND CARGO_FEATURES --features=aws-lc-rs) @@ -15,6 +17,10 @@ elseif (CRYPTO_PROVIDER STREQUAL "ring") list(APPEND CARGO_FEATURES --features=ring) endif () +if (CERT_COMPRESSION STREQUAL "true") + list(APPEND CARGO_FEATURES --features=cert_compression) +endif () + add_subdirectory(tests) include(ExternalProject) diff --git a/Cargo.lock b/Cargo.lock index 1f554084..41e9793f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -11,6 +11,21 @@ dependencies = [ "memchr", ] +[[package]] +name = "alloc-no-stdlib" +version = "2.0.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cc7bb162ec39d46ab1ca8c77bf72e890535becd1751bb45f64c597edb4c8c6b3" + +[[package]] +name = "alloc-stdlib" +version = "0.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "94fb8275041c72129eb51b7d0322c29b8387a0386127718b096429201a5d6ece" +dependencies = [ + "alloc-no-stdlib", +] + [[package]] name = "autocfg" version = "1.2.0" @@ -79,6 +94,27 @@ version = "2.6.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b048fb63fd8b5923fc5aa7b340d8e156aec7ec02f0c78fa8a6ddc2613f6f71de" +[[package]] +name = "brotli" +version = "6.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "74f7971dbd9326d58187408ab83117d8ac1bb9c17b085fdacd1cf2f598719b6b" +dependencies = [ + "alloc-no-stdlib", + "alloc-stdlib", + "brotli-decompressor", +] + +[[package]] +name = "brotli-decompressor" +version = "4.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9a45bd2e4095a8b518033b128020dd4a55aab1c0a381ba4404a472630f4bc362" +dependencies = [ + "alloc-no-stdlib", + "alloc-stdlib", +] + [[package]] name = "bytes" version = "1.6.0" @@ -492,6 +528,8 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f2dabaac7466917e566adb06783a81ca48944c6898a1b08b9374106dd671f4c8" dependencies = [ "aws-lc-rs", + "brotli", + "brotli-decompressor", "once_cell", "ring", "rustls-pki-types", @@ -499,6 +537,7 @@ dependencies = [ "rustversion", "subtle", "zeroize", + "zlib-rs", ] [[package]] @@ -960,3 +999,9 @@ name = "zeroize" version = "1.7.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "525b4ec142c6b68a2d10f01f7bbf6755599ca3f81ea53b8431b7dd348f5fdb2d" + +[[package]] +name = "zlib-rs" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4bf919c619da9eaede02291295e9c5ae230fc7b5f2a5f4257ff859b075111faf" diff --git a/Cargo.toml b/Cargo.toml index 6d574aab..f328bc46 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -23,6 +23,7 @@ read_buf = ["rustls/read_buf"] capi = [] ring = ["rustls/ring", "webpki/ring"] aws-lc-rs = ["rustls/aws-lc-rs", "webpki/aws_lc_rs"] +cert_compression = ["rustls/brotli", "rustls/zlib"] [dependencies] # Keep in sync with RUSTLS_CRATE_VERSION in build.rs diff --git a/Makefile b/Makefile index 0199d87c..4d8d6660 100644 --- a/Makefile +++ b/Makefile @@ -10,6 +10,7 @@ CARGOFLAGS += --locked CFLAGS := -Werror -Wall -Wextra -Wpedantic -g -I src/ PROFILE := release CRYPTO_PROVIDER := aws-lc-rs +COMPRESSION := false DESTDIR=/usr/local ifeq ($(PROFILE), debug) @@ -35,6 +36,11 @@ else ifeq ($(CRYPTO_PROVIDER), ring) CARGOFLAGS += --no-default-features --features ring endif +ifeq ($(COMPRESSION), true) + CARGOFLAGS += --features cert_compression + LDFLAGS += -lm +endif + all: target/client target/server test: all diff --git a/Makefile.pkg-config b/Makefile.pkg-config index fe25c925..8baf7298 100644 --- a/Makefile.pkg-config +++ b/Makefile.pkg-config @@ -14,6 +14,7 @@ CARGOFLAGS += --locked CFLAGS := -Werror -Wall -Wextra -Wpedantic -g -I src/ PROFILE := release CRYPTO_PROVIDER := aws-lc-rs +CERT_COMPRESSION := false PREFIX=/usr/local ifeq ($(PROFILE), debug) @@ -34,6 +35,10 @@ else ifeq ($(CRYPTO_PROVIDER), ring) CARGOFLAGS += --no-default-features --features ring endif +ifeq ($(CERT_COMPRESSION), true) + CARGOFLAGS += --features cert_compression +endif + all: target/client target/server integration: all From 5963c8252c8eaee06276a9f66e035b5b592ada6d Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Tue, 1 Oct 2024 14:13:13 -0400 Subject: [PATCH 6/7] cipher: toggle server verifier CRL expiry policy This commit adds a new `rustls_web_pki_server_cert_verifier_enforce_revocation_expiry()` fn that can update the CRL expiration policy of a `rustls_web_pki_server_cert_verifier_builder` instance to enforce that the CRL's nextUpdate is not in the past. This augments the existing controls for revocation checking depth, and unknown status error handling. By default we match the upstream default behaviour and ignore CRL expiration. --- src/cipher.rs | 30 ++++++++++++++++++++++++++++-- src/rustls.h | 11 ++++++++++- 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/src/cipher.rs b/src/cipher.rs index f9ba7167..99009d82 100644 --- a/src/cipher.rs +++ b/src/cipher.rs @@ -16,7 +16,7 @@ use rustls::server::WebPkiClientVerifier; use rustls::sign::CertifiedKey; use rustls::{DistinguishedName, RootCertStore, SupportedCipherSuite}; use rustls_pemfile::{certs, crls}; -use webpki::{RevocationCheckDepth, UnknownStatusPolicy}; +use webpki::{ExpirationPolicy, RevocationCheckDepth, UnknownStatusPolicy}; use crate::crypto_provider::{rustls_crypto_provider, rustls_signing_key}; use crate::enums::rustls_tls_version; @@ -890,6 +890,7 @@ pub(crate) struct ServerCertVerifierBuilder { crls: Vec>, revocation_depth: RevocationCheckDepth, revocation_policy: UnknownStatusPolicy, + revocation_expiration_policy: ExpirationPolicy, } impl ServerCertVerifierBuilder { @@ -926,6 +927,7 @@ impl ServerCertVerifierBuilder { crls: Vec::default(), revocation_depth: RevocationCheckDepth::Chain, revocation_policy: UnknownStatusPolicy::Deny, + revocation_expiration_policy: ExpirationPolicy::Ignore, })) } } @@ -946,7 +948,8 @@ impl ServerCertVerifierBuilder { /// `rustls_web_pki_server_cert_verifier_only_check_end_entity_revocation` is used. Unknown /// revocation status for certificates considered for revocation status will be treated as /// an error unless `rustls_web_pki_server_cert_verifier_allow_unknown_revocation_status` is - /// used. + /// used. Expired CRLs will not be treated as an error unless + /// `rustls_web_pki_server_cert_verifier_enforce_revocation_expiry` is used. /// /// This copies the contents of the `rustls_root_cert_store`. It does not take /// ownership of the pointed-to data. @@ -964,6 +967,7 @@ impl ServerCertVerifierBuilder { crls: Vec::default(), revocation_depth: RevocationCheckDepth::Chain, revocation_policy: UnknownStatusPolicy::Deny, + revocation_expiration_policy: ExpirationPolicy::Ignore, })) } } @@ -1046,6 +1050,24 @@ impl ServerCertVerifierBuilder { } } + /// When CRLs are provided with `rustls_web_pki_server_cert_verifier_builder_add_crl`, and the + /// CRL nextUpdate field is in the past, treat it as an error condition. + /// + /// Overrides the default behavior where CRL expiration is ignored. + #[no_mangle] + pub extern "C" fn rustls_web_pki_server_cert_verifier_enforce_revocation_expiry( + builder: *mut rustls_web_pki_server_cert_verifier_builder, + ) -> rustls_result { + let server_verifier_builder = try_mut_from_ptr!(builder); + let server_verifier_builder = match server_verifier_builder { + None => return AlreadyUsed, + Some(v) => v, + }; + + server_verifier_builder.revocation_expiration_policy = ExpirationPolicy::Enforce; + rustls_result::Ok + } + /// Create a new server certificate verifier from the builder. /// /// The builder is consumed and cannot be used again, but must still be freed. @@ -1082,6 +1104,10 @@ impl ServerCertVerifierBuilder { UnknownStatusPolicy::Allow => builder = builder.allow_unknown_revocation_status(), UnknownStatusPolicy::Deny => {} } + match server_verifier_builder.revocation_expiration_policy { + ExpirationPolicy::Enforce => builder = builder.enforce_revocation_expiration(), + ExpirationPolicy::Ignore => {} + } let verifier = match builder.build() { Ok(v) => v, diff --git a/src/rustls.h b/src/rustls.h index 748d3efe..e14fa100 100644 --- a/src/rustls.h +++ b/src/rustls.h @@ -1424,7 +1424,8 @@ struct rustls_web_pki_server_cert_verifier_builder *rustls_web_pki_server_cert_v * `rustls_web_pki_server_cert_verifier_only_check_end_entity_revocation` is used. Unknown * revocation status for certificates considered for revocation status will be treated as * an error unless `rustls_web_pki_server_cert_verifier_allow_unknown_revocation_status` is - * used. + * used. Expired CRLs will not be treated as an error unless + * `rustls_web_pki_server_cert_verifier_enforce_revocation_expiry` is used. * * This copies the contents of the `rustls_root_cert_store`. It does not take * ownership of the pointed-to data. @@ -1462,6 +1463,14 @@ rustls_result rustls_web_pki_server_cert_verifier_only_check_end_entity_revocati */ rustls_result rustls_web_pki_server_cert_verifier_allow_unknown_revocation_status(struct rustls_web_pki_server_cert_verifier_builder *builder); +/** + * When CRLs are provided with `rustls_web_pki_server_cert_verifier_builder_add_crl`, and the + * CRL nextUpdate field is in the past, treat it as an error condition. + * + * Overrides the default behavior where CRL expiration is ignored. + */ +rustls_result rustls_web_pki_server_cert_verifier_enforce_revocation_expiry(struct rustls_web_pki_server_cert_verifier_builder *builder); + /** * Create a new server certificate verifier from the builder. * From 4a5f903fdf09435a1dae5163c228a44184e9493e Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Tue, 1 Oct 2024 15:03:10 -0400 Subject: [PATCH 7/7] connection: expose handshake kind This commit adds a `rustls_connection_handshake_kind()` fn for getting the handshake kind of a `rustls_connection`. The kind of connection is returned as a `rustls_handshake_kind` enum, which can be translated to a `rustls_str` using `rustls_handshake_kind_str()`. The `rustls_handshake_kind` enum has variants for full, full with hello retry request, and resumed handshake types matching the upstream `rustls::HandshakeKind` enum. --- src/connection.rs | 13 ++++++++++ src/enums.rs | 62 ++++++++++++++++++++++++++++++++++++++++++++++- src/panic.rs | 4 ++- src/rustls.h | 46 +++++++++++++++++++++++++++++++++++ tests/client.c | 7 +++++- tests/server.c | 6 ++++- 6 files changed, 134 insertions(+), 4 deletions(-) diff --git a/src/connection.rs b/src/connection.rs index 1e4036e0..f1dc4fb4 100644 --- a/src/connection.rs +++ b/src/connection.rs @@ -20,6 +20,7 @@ use crate::{ try_slice, try_slice_mut, userdata_push, }; +use crate::enums::rustls_handshake_kind; use rustls_result::NullParameter; pub(crate) struct Connection { @@ -287,6 +288,18 @@ impl rustls_connection { } } + #[no_mangle] + pub extern "C" fn rustls_connection_handshake_kind( + conn: *const rustls_connection, + ) -> rustls_handshake_kind { + ffi_panic_boundary! { + try_ref_from_ptr!(conn) + .handshake_kind() + .map(Into::into) + .unwrap_or(rustls_handshake_kind::Unknown) + } + } + /// Sets a limit on the internal buffers used to buffer unsent plaintext (prior /// to completing the TLS handshake) and unsent TLS records. By default, there /// is no limit. The limit can be set at any time, even if the current buffer diff --git a/src/enums.rs b/src/enums.rs index 6ab4b132..d9906fcb 100644 --- a/src/enums.rs +++ b/src/enums.rs @@ -1,4 +1,6 @@ -use rustls::{ProtocolVersion, SupportedProtocolVersion}; +use crate::ffi_panic_boundary; +use crate::rslice::rustls_str; +use rustls::{HandshakeKind, ProtocolVersion, SupportedProtocolVersion}; #[derive(Debug, Default)] #[repr(C)] @@ -52,6 +54,64 @@ pub static RUSTLS_DEFAULT_VERSIONS: [u16; 2] = [ #[no_mangle] pub static RUSTLS_DEFAULT_VERSIONS_LEN: usize = RUSTLS_DEFAULT_VERSIONS.len(); +#[derive(Debug, Default)] +#[repr(C)] +/// Describes which sort of handshake happened. +pub enum rustls_handshake_kind { + /// The type of handshake could not be determined. + /// + /// This variant should not be used. + #[default] + Unknown = 0x0, + + /// A full TLS handshake. + /// + /// This is the typical TLS connection initiation process when resumption is + /// not yet unavailable, and the initial client hello was accepted by the server. + Full = 0x1, + + /// A full TLS handshake, with an extra round-trip for a hello retry request. + /// + /// The server can respond with a hello retry request (HRR) if the initial client + /// hello is unacceptable for several reasons, the most likely if no supported key + /// shares were offered by the client. + FullWithHelloRetryRequest = 0x2, + + /// A resumed TLS handshake. + /// + /// Resumed handshakes involve fewer round trips and less cryptography than + /// full ones, but can only happen when the peers have previously done a full + /// handshake together, and then remember data about it. + Resumed = 0x3, +} + +/// Convert a `rustls_handshake_kind` to a string with a friendly description of the kind +/// of handshake. +/// +/// The returned `rustls_str` has a static lifetime equal to that of the program and does +/// not need to be manually freed. +#[no_mangle] +pub extern "C" fn rustls_handshake_kind_str(kind: rustls_handshake_kind) -> rustls_str<'static> { + ffi_panic_boundary! { + rustls_str::from_str_unchecked(match kind { + rustls_handshake_kind::Unknown => "unknown", + rustls_handshake_kind::Full => "full", + rustls_handshake_kind::FullWithHelloRetryRequest => "full with hello retry request", + rustls_handshake_kind::Resumed => "resumed", + }) + } +} + +impl From for rustls_handshake_kind { + fn from(kind: HandshakeKind) -> Self { + match kind { + HandshakeKind::Full => Self::Full, + HandshakeKind::FullWithHelloRetryRequest => Self::FullWithHelloRetryRequest, + HandshakeKind::Resumed => Self::Resumed, + } + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/src/panic.rs b/src/panic.rs index 4b8489c9..0ab7d89c 100644 --- a/src/panic.rs +++ b/src/panic.rs @@ -1,6 +1,6 @@ use libc::EINVAL; -use crate::enums::rustls_tls_version; +use crate::enums::{rustls_handshake_kind, rustls_tls_version}; use crate::error::{rustls_io_result, rustls_result}; use crate::rslice::{rustls_slice_bytes, rustls_str}; @@ -38,6 +38,8 @@ impl Defaultable for () {} impl Defaultable for rustls_tls_version {} +impl Defaultable for rustls_handshake_kind {} + impl Defaultable for Option {} impl<'a> Defaultable for rustls_slice_bytes<'a> {} diff --git a/src/rustls.h b/src/rustls.h index e14fa100..c92acb84 100644 --- a/src/rustls.h +++ b/src/rustls.h @@ -7,6 +7,41 @@ #include #include +/** + * Describes which sort of handshake happened. + */ +typedef enum rustls_handshake_kind { + /** + * The type of handshake could not be determined. + * + * This variant should not be used. + */ + RUSTLS_HANDSHAKE_KIND_UNKNOWN = 0, + /** + * A full TLS handshake. + * + * This is the typical TLS connection initiation process when resumption is + * not yet unavailable, and the initial client hello was accepted by the server. + */ + RUSTLS_HANDSHAKE_KIND_FULL = 1, + /** + * A full TLS handshake, with an extra round-trip for a hello retry request. + * + * The server can respond with a hello retry request (HRR) if the initial client + * hello is unacceptable for several reasons, the most likely if no supported key + * shares were offered by the client. + */ + RUSTLS_HANDSHAKE_KIND_FULL_WITH_HELLO_RETRY_REQUEST = 2, + /** + * A resumed TLS handshake. + * + * Resumed handshakes involve fewer round trips and less cryptography than + * full ones, but can only happen when the peers have previously done a full + * handshake together, and then remember data about it. + */ + RUSTLS_HANDSHAKE_KIND_RESUMED = 3, +} rustls_handshake_kind; + enum rustls_result { RUSTLS_RESULT_OK = 7000, RUSTLS_RESULT_IO = 7001, @@ -1842,6 +1877,8 @@ bool rustls_connection_wants_write(const struct rustls_connection *conn); */ bool rustls_connection_is_handshaking(const struct rustls_connection *conn); +enum rustls_handshake_kind rustls_connection_handshake_kind(const struct rustls_connection *conn); + /** * Sets a limit on the internal buffers used to buffer unsent plaintext (prior * to completing the TLS handshake) and unsent TLS records. By default, there @@ -2235,6 +2272,15 @@ rustls_result rustls_default_crypto_provider_random(uint8_t *buff, size_t len); */ void rustls_signing_key_free(struct rustls_signing_key *signing_key); +/** + * Convert a `rustls_handshake_kind` to a string with a friendly description of the kind + * of handshake. + * + * The returned `rustls_str` has a static lifetime equal to that of the program and does + * not need to be manually freed. + */ +struct rustls_str rustls_handshake_kind_str(enum rustls_handshake_kind kind); + /** * After a rustls function returns an error, you may call * this to get a pointer to a buffer containing a detailed error diff --git a/tests/client.c b/tests/client.c index 1a1c4ef7..deb427d7 100644 --- a/tests/client.c +++ b/tests/client.c @@ -170,8 +170,9 @@ send_request_and_read_response(struct conndata *conn, unsigned long content_length = 0; size_t headers_len = 0; struct rustls_str version; + rustls_handshake_kind hs_kind; int ciphersuite_id, kex_id; - struct rustls_str ciphersuite_name, kex_name; + struct rustls_str ciphersuite_name, kex_name, hs_kind_name; version = rustls_version(); memset(buf, '\0', sizeof(buf)); @@ -298,6 +299,10 @@ send_request_and_read_response(struct conndata *conn, LOG_SIMPLE("send_request_and_read_response: loop fell through"); drain_plaintext: + hs_kind = rustls_connection_handshake_kind(rconn); + hs_kind_name = rustls_handshake_kind_str(hs_kind); + LOG("handshake kind: %.*s", (int)hs_kind_name.len, hs_kind_name.data); + kex_id = rustls_connection_get_negotiated_key_exchange_group(rconn); kex_name = rustls_connection_get_negotiated_key_exchange_group_name(rconn); LOG("negotiated key exchange: %.*s (%#x)", diff --git a/tests/server.c b/tests/server.c index fa6a8f27..316bee77 100644 --- a/tests/server.c +++ b/tests/server.c @@ -127,8 +127,9 @@ handle_conn(struct conndata *conn) int sockfd = conn->fd; struct timeval tv; enum exchange_state state = READING_REQUEST; + rustls_handshake_kind hs_kind; int ciphersuite_id, kex_id; - struct rustls_str ciphersuite_name, kex_name; + struct rustls_str ciphersuite_name, kex_name, hs_kind_name; LOG("acccepted conn on fd %d", conn->fd); @@ -191,6 +192,9 @@ handle_conn(struct conndata *conn) if(state == READING_REQUEST && body_beginning(&conn->data) != NULL) { state = SENT_RESPONSE; LOG_SIMPLE("writing response"); + hs_kind = rustls_connection_handshake_kind(rconn); + hs_kind_name = rustls_handshake_kind_str(hs_kind); + LOG("handshake kind: %.*s", (int)hs_kind_name.len, hs_kind_name.data); ciphersuite_id = rustls_connection_get_negotiated_ciphersuite(rconn); ciphersuite_name = rustls_connection_get_negotiated_ciphersuite_name(rconn);