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 67d4cb5 commit 2300577
Show file tree
Hide file tree
Showing 3 changed files with 89 additions and 10 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
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
80 changes: 70 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,58 @@ impl SslContextBuilder {
}
}

fn set_safe_defaults(&mut self) -> Result<(), ErrorStack> {
// Override boringSSL's default key exchange and cipher preferences based on the set of
// features we're compiling this crate with.
//
// 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 imply disabling 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 +1801,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 2300577

Please sign in to comment.