Skip to content

Commit

Permalink
Use features to set default algorithm preferences
Browse files Browse the repository at this point in the history
Overwrite boringSSL's default key exchange preferences with safe
defaults using feature flags:

* "pq-kx-supported" enables support for PQ key exchange algorithms by
  default. Classical key exchange is still preferred, but will be
  upgraded to PQ if requested.

* "pq-kx-preferred"  enables preference for PQ key exchange by default,
  with fallback to classical key exchange if requested.

" "fips-required" disables non-FIPS-compliant algorithms by default.
  • Loading branch information
cjpatton committed Aug 11, 2023
1 parent 26acb64 commit 09d1eb6
Show file tree
Hide file tree
Showing 4 changed files with 97 additions and 12 deletions.
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ boring = { version = "3", path = "./boring" }
tokio-boring = { version = "3", path = "./tokio-boring" }

bindgen = { version = "0.66.1", default-features = false, features = ["runtime"] }
cfg-if = "1.0.0"
cmake = "0.1"
fs_extra = "1.3.0"
fslock = "0.2"
Expand Down
11 changes: 9 additions & 2 deletions boring-sys/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -589,8 +589,15 @@ fn main() {
println!("cargo:rerun-if-env-changed=BORING_SSL_PRECOMPILED_BCM_O");
println!("cargo:rerun-if-env-changed=BORINGSSL_BUILD_DIR");

#[cfg(all(feature = "fips", feature = "rpk"))]
compile_error!("`fips` and `rpk` features are mutually exclusive");
#[cfg(all(
feature = "fips",
any(
feature = "fips-link-precompiled",
feature = "rpk",
feature = "pq-experimental"
)
))]
compile_error!("The `fips` feature is incompatible with newer versions of boringSSL");

let bssl_dir = env::var("BORING_BSSL_PATH");

Expand Down
18 changes: 18 additions & 0 deletions boring/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,26 @@ 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 the safest option if you don't know if the peer supports PQ key
# exchange.
pq-kx-supported = ["pq-experimental"]

# Prefer PQ key exchange by default. The client will prefer PQ exchange, but
# fallback to classical key exchange if requested by the server. This is the
# best option if you know the peer supports PQ key exchange.
pq-kx-preferred = ["pq-kx-supported"]

# Disable support for non-FIPS-compliant algorithm by default.
#
# TODO(cjpatton) Disable non-compliant ciphersuites. Currently this feature
# only disables non-compliant key exchange algorithms.
fips-required = []

[dependencies]
bitflags = { workspace = true }
cfg-if = { workspace = true }
foreign-types = { workspace = true }
once_cell = { workspace = true }
libc = { workspace = true }
Expand Down
79 changes: 69 additions & 10 deletions boring/src/ssl/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -696,16 +696,20 @@ pub struct SslContextBuilder {
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 overwriting the default key exchange and
/// cipher preferences. The defaults are only overwritten if certain feature flags are set.
///
/// [`SSL_CTX_new`]: https://www.openssl.org/docs/manmaster/man3/SSL_CTX_new.html
pub fn new_rpk() -> Result<SslContextBuilder, ErrorStack> {
unsafe {
let mut ctx = unsafe {
init();
let ctx = cvt_p(ffi::SSL_CTX_new(SslMethod::tls_with_buffer().as_ptr()))?;

Ok(SslContextBuilder::from_ptr(ctx, true))
}
SslContextBuilder::from_ptr(ctx, true)
};

ctx.set_safe_defaults()?;
Ok(ctx)
}

/// Sets raw public key certificate in DER format.
Expand Down Expand Up @@ -739,24 +743,28 @@ impl SslContextBuilder {
impl SslContextBuilder {
/// Creates a new `SslContextBuilder`.
///
/// This corresponds to [`SSL_CTX_new`].
/// This corresponds to calling [`SSL_CTX_new`] then overwriting the default key exchange and
/// cipher preferences. The defaults are only overwritten if certain feature flags are set.
///
/// [`SSL_CTX_new`]: https://www.openssl.org/docs/manmaster/man3/SSL_CTX_new.html
pub fn new(method: SslMethod) -> Result<SslContextBuilder, ErrorStack> {
unsafe {
let mut ctx = unsafe {
init();
let ctx = cvt_p(ffi::SSL_CTX_new(method.as_ptr()))?;

#[cfg(feature = "rpk")]
{
Ok(SslContextBuilder::from_ptr(ctx, false))
SslContextBuilder::from_ptr(ctx, false)
}

#[cfg(not(feature = "rpk"))]
{
Ok(SslContextBuilder::from_ptr(ctx))
SslContextBuilder::from_ptr(ctx)
}
}
};

ctx.set_safe_defaults()?;
Ok(ctx)
}

/// Creates an `SslContextBuilder` from a pointer to a raw OpenSSL value.
Expand Down Expand Up @@ -1707,6 +1715,57 @@ impl SslContextBuilder {
}
}

fn set_safe_defaults(&mut self) -> Result<(), ErrorStack> {
// Override boringSSL's default key exchange and cipher preferences.
//
// For "fips-required" we could have considered the following:
//
// unsafe {
// let _ = cvt_0i(ffi::SSL_CTX_set_compliance_policy(self.as_ptr(), ffi::ssl_compliance_policy_t::ssl_compliance_policy_fips_202205))?;
// }
//
// However this is stricter than what our unit tests expect because it disables older
// version of TLS:
// ssl::test::test_connect_with_srtp_ctx
// ssl::test::test_connect_with_srtp_ssl
//
// It would also require disable PQ key exchange even though P256_KYBER768_DRAFT00 is
// FIPS-compliant.
//
// TODO(cjpatton) For "fips-required" we also need to disable ciphersuites (TLS 1.3, called
// the "cipher list" in 1.2 and below) with ChaCha20Poly1305.
cfg_if::cfg_if! {
if #[cfg(feature = "pq-kx-preferred")] {
self.set_curves(&[
#[cfg(not(feature = "fips-required"))]
SslCurve::X25519_KYBER512_DRAFT00,
SslCurve::P256_KYBER768_DRAFT00,
#[cfg(not(feature = "fips-required"))]
SslCurve::X25519,
SslCurve::SECP256R1,
SslCurve::SECP384R1,
])?;
} else if #[cfg(feature = "pq-kx-supported")] {
self.set_curves(&[
#[cfg(not(feature = "fips-required"))]
SslCurve::X25519,
SslCurve::SECP256R1,
SslCurve::SECP384R1,
#[cfg(not(feature = "fips-required"))]
SslCurve::X25519_KYBER512_DRAFT00,
SslCurve::P256_KYBER768_DRAFT00,
])?;
} else if #[cfg(feature = "fips-required")] {
self.set_curves(&[
SslCurve::SECP256R1,
SslCurve::SECP384R1,
])?;
}
}

Ok(())
}

/// Consumes the builder, returning a new `SslContext`.
pub fn build(self) -> SslContext {
self.ctx
Expand Down Expand Up @@ -1741,7 +1800,7 @@ impl ToOwned for SslContextRef {
}
}

// TODO: add useful info here
// TODOu: add useful info here
impl fmt::Debug for SslContext {
fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
write!(fmt, "SslContext")
Expand Down

0 comments on commit 09d1eb6

Please sign in to comment.