From 4f89d44ab038b36ca26d6a92fea64f904799741e Mon Sep 17 00:00:00 2001 From: Christopher Patton Date: Tue, 8 Aug 2023 16:43:36 -0700 Subject: [PATCH] Use features to set default key exchange preference Overwrite boringSSL's default key exchange preferences with safe defaults. The defaults can be overwritten in the usual way (by calling `SslConnector::set_curves()`). They can also be controlled by feature flags: * "pq-supported" (introduced by this commit) enables support for PQ key exchange algorithms by default. Classical key exchange is still preferred, but will be upgraded to PQ if requested. * "pq-preferred" (introduced by this commit) enables preference for PQ key exchange by default, with fallback to classical key exchange if requested. " "fips(-link-precompiled)" disables support for X25519 by default. While at it, add `SslCurve::X25519_KYBER768_DRAFT00` to the default feature set. Previously this was gated to builds that don't have include the "fips" flag. There are two motivations for this change: 1. For consistency: no other X25519 hybrids (nor `SslCurve::X22519` itself) were gated in the same way. 2. The FIPS flags now control default preferences, but these can be overwritten by the user if necessary. --- boring/Cargo.toml | 18 ++++++++-- boring/src/ssl/mod.rs | 79 +++++++++++++++++++++++++++++++++++-------- 2 files changed, 80 insertions(+), 17 deletions(-) diff --git a/boring/Cargo.toml b/boring/Cargo.toml index 568a53ad8..a519f7967 100644 --- a/boring/Cargo.toml +++ b/boring/Cargo.toml @@ -16,10 +16,12 @@ features = ["rpk", "pq-experimental"] rustdoc-args = ["--cfg", "docsrs"] [features] -# Use a FIPS-validated version of boringssl. +# Use a FIPS-validated version of boringssl. Moreover, only support the NIST +# curves for the key exchange by default. fips = ["boring-sys/fips"] -# Link with precompiled FIPS-validated `bcm.o` module. +# Link with precompiled FIPS-validated `bcm.o` module. Moreover, only support +# the NIST curves for the key exchange by default. fips-link-precompiled = ["fips", "boring-sys/fips-link-precompiled"] # Enables Raw public key API (https://datatracker.ietf.org/doc/html/rfc7250) @@ -28,8 +30,20 @@ rpk = ["boring-sys/rpk"] # Enables experimental post-quantum crypto (https://blog.cloudflare.com/post-quantum-for-all/) pq-experimental = ["boring-sys/pq-experimental"] +# Support PQ key exchange by default. The client will prefer classical key +# exchange, but will upgrade to PQ key exchange if requested by the server. +# This is a safe option if you don't know if the peer supports PQ key exchange. +pq-supported = ["pq-experimental"] + +# Prefer PQ key exchange by default. The client will prefer PQ exchange, but +# will fallback to classical key exchange if requested by the server. This is +# the lowest-latency option if you know the peer supports PQ key exchange. +pq-preferred = ["pq-experimental", "pq-supported"] + + [dependencies] bitflags = { workspace = true } +cfg-if = "1.0.0" foreign-types = { workspace = true } once_cell = { workspace = true } libc = { workspace = true } diff --git a/boring/src/ssl/mod.rs b/boring/src/ssl/mod.rs index 67e16b181..9c623226d 100644 --- a/boring/src/ssl/mod.rs +++ b/boring/src/ssl/mod.rs @@ -630,7 +630,6 @@ impl SslCurve { pub const X25519: SslCurve = SslCurve(ffi::NID_X25519); - #[cfg(not(feature = "fips"))] pub const X25519_KYBER768_DRAFT00: SslCurve = SslCurve(ffi::NID_X25519Kyber768Draft00); #[cfg(feature = "pq-experimental")] @@ -641,6 +640,42 @@ impl SslCurve { #[cfg(feature = "pq-experimental")] pub const P256_KYBER768_DRAFT00: SslCurve = SslCurve(ffi::NID_P256Kyber768Draft00); + + cfg_if::cfg_if! { + if #[cfg(feature = "pq-preferred")] { + const DEFAULT_CURVES: &[SslCurve] = &[ + #[cfg(not(feature = "fips"))] + #[cfg(not(feature = "fips-link-precompiled"))] + SslCurve::X25519_KYBER512_DRAFT00, + SslCurve::P256_KYBER768_DRAFT00, + #[cfg(not(feature = "fips"))] + #[cfg(not(feature = "fips-link-precompiled"))] + SslCurve::X25519, + SslCurve::SECP256R1, + SslCurve::SECP384R1, + ]; + } else if #[cfg(feature = "pq-supported")] { + const DEFAULT_CURVES: &[SslCurve] = &[ + #[cfg(not(feature = "fips"))] + #[cfg(not(feature = "fips-link-precompiled"))] + SslCurve::X25519, + SslCurve::SECP256R1, + SslCurve::SECP384R1, + #[cfg(not(feature = "fips"))] + #[cfg(not(feature = "fips-link-precompiled"))] + SslCurve::X25519_KYBER512_DRAFT00, + SslCurve::P256_KYBER768_DRAFT00, + ]; + } else { + const DEFAULT_CURVES: &[SslCurve] = &[ + #[cfg(not(feature = "fips"))] + #[cfg(not(feature = "fips-link-precompiled"))] + SslCurve::X25519, + SslCurve::SECP256R1, + SslCurve::SECP384R1, + ]; + } + } } /// A standard implementation of protocol selection for Application Layer Protocol Negotiation @@ -692,17 +727,38 @@ pub struct SslContextBuilder { is_rpk: bool, } +unsafe fn new_ssl_ctx_with_default_curves( + method: SslMethod, +) -> Result<*mut ffi::SSL_CTX, ErrorStack> { + init(); + let ctx = cvt_p(ffi::SSL_CTX_new(method.as_ptr()))?; + ssl_ctx_set_curves(ctx, SslCurve::DEFAULT_CURVES)?; + Ok(ctx) +} + +unsafe fn ssl_ctx_set_curves( + ctx: *mut ffi::SSL_CTX, + curves: &[SslCurve], +) -> Result<(), ErrorStack> { + cvt_0i(ffi::SSL_CTX_set1_curves( + ctx, + curves.as_ptr() as *const _, + curves.len(), + )) + .map(|_| ()) +} + #[cfg(feature = "rpk")] impl SslContextBuilder { /// Creates a new `SslContextBuilder` to be used with Raw Public Key. /// - /// This corresponds to [`SSL_CTX_new`]. + /// This corresponds to calling [`SSL_CTX_new`] then setting the default key exchange + /// preferences. /// /// [`SSL_CTX_new`]: https://www.openssl.org/docs/manmaster/man3/SSL_CTX_new.html pub fn new_rpk() -> Result { unsafe { - init(); - let ctx = cvt_p(ffi::SSL_CTX_new(SslMethod::tls_with_buffer().as_ptr()))?; + let ctx = new_ssl_ctx_with_default_curves(SslMethod::tls_with_buffer())?; Ok(SslContextBuilder::from_ptr(ctx, true)) } @@ -739,13 +795,13 @@ impl SslContextBuilder { impl SslContextBuilder { /// Creates a new `SslContextBuilder`. /// - /// This corresponds to [`SSL_CTX_new`]. + /// This corresponds to calling [`SSL_CTX_new`] then setting the default key exchange + /// preferences. /// /// [`SSL_CTX_new`]: https://www.openssl.org/docs/manmaster/man3/SSL_CTX_new.html pub fn new(method: SslMethod) -> Result { unsafe { - init(); - let ctx = cvt_p(ffi::SSL_CTX_new(method.as_ptr()))?; + let ctx = new_ssl_ctx_with_default_curves(method)?; #[cfg(feature = "rpk")] { @@ -1697,14 +1753,7 @@ impl SslContextBuilder { /// /// [`SSL_CTX_set1_curves`]: https://commondatastorage.googleapis.com/chromium-boringssl-docs/ssl.h.html#SSL_CTX_set1_curves pub fn set_curves(&mut self, curves: &[SslCurve]) -> Result<(), ErrorStack> { - unsafe { - cvt_0i(ffi::SSL_CTX_set1_curves( - self.as_ptr(), - curves.as_ptr() as *const _, - curves.len(), - )) - .map(|_| ()) - } + unsafe { ssl_ctx_set_curves(self.as_ptr(), curves) } } /// Consumes the builder, returning a new `SslContext`.