Skip to content

Commit

Permalink
Default to require_ems in FIPS mode
Browse files Browse the repository at this point in the history
Change default for `require_ems` based on `fips` crate feature,
generalising the existing tests for `require_ems` to verify this too.

Include `require_ems` in `fips()` determination.
  • Loading branch information
ctz committed Feb 5, 2024
1 parent bc71528 commit e8958e3
Show file tree
Hide file tree
Showing 7 changed files with 71 additions and 12 deletions.
2 changes: 1 addition & 1 deletion rustls/src/client/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ impl ConfigBuilder<ClientConfig, WantsClientCert> {
enable_secret_extraction: false,
enable_early_data: false,
#[cfg(feature = "tls12")]
require_ems: false,
require_ems: cfg!(feature = "fips"),
}
}
}
17 changes: 15 additions & 2 deletions rustls/src/client/client_conn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,8 @@ pub struct ClientConfig {
/// If set to `true`, requires the server to support the extended
/// master secret extraction method defined in [RFC 7627].
///
/// The default is `false`.
/// The default is `true` if the `fips` crate feature is enabled,
/// `false` otherwise.
///
/// It must be set to `true` to meet FIPS requirement mentioned in section
/// **D.Q Transition of the TLS 1.2 KDF to Support the Extended Master
Expand Down Expand Up @@ -279,8 +280,20 @@ impl ClientConfig {

/// Return true if connections made with this `ClientConfig` will
/// operate in FIPS mode.
///
/// This is different from [`CryptoProvider::fips()`]: [`CryptoProvider::fips()`]
/// is concerned only with cryptography, whereas this _also_ covers TLS-level
/// configuration that NIST recommends.
pub fn fips(&self) -> bool {
self.provider.fips()
#[cfg(feature = "tls12")]
{
self.provider.fips() && self.require_ems
}

#[cfg(not(feature = "tls12"))]
{
self.provider.fips()
}
}

/// We support a given TLS version if it's quoted in the configured
Expand Down
5 changes: 5 additions & 0 deletions rustls/src/crypto/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,11 @@ pub struct CryptoProvider {

impl CryptoProvider {
/// Returns `true` if this `CryptoProvider` is operating in FIPS mode.
///
/// This covers only the cryptographic parts of FIPS approval. There are
/// also TLS protocol-level recommendations made by NIST. You should
/// prefer to call [`ClientConfig::fips()`] or [`ServerConfig::fips()`]
/// which take these into account.
pub fn fips(&self) -> bool {
let Self {
cipher_suites,
Expand Down
3 changes: 2 additions & 1 deletion rustls/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,8 @@
//!
//! - `fips`: enable support for FIPS140-3-approved cryptography, via the aws-lc-rs crate.
//! This feature enables the `aws_lc_rs` feature, which makes the rustls crate depend
//! on [aws-lc-rs](https://github.com/aws/aws-lc-rs).
//! on [aws-lc-rs](https://github.com/aws/aws-lc-rs). It also changes the default
//! for [`ServerConfig::require_ems`] and [`ClientConfig::require_ems`].
//!
//! - `tls12` (enabled by default): enable support for TLS version 1.2. Note that, due to the
//! additive nature of Cargo features and because it is enabled by default, other crates
Expand Down
2 changes: 1 addition & 1 deletion rustls/src/server/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ impl ConfigBuilder<ServerConfig, WantsServerCert> {
send_half_rtt_data: false,
send_tls13_tickets: 4,
#[cfg(feature = "tls12")]
require_ems: false,
require_ems: cfg!(feature = "fips"),
}
}
}
17 changes: 15 additions & 2 deletions rustls/src/server/server_conn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,8 @@ pub struct ServerConfig {
/// If set to `true`, requires the client to support the extended
/// master secret extraction method defined in [RFC 7627].
///
/// The default is `false`.
/// The default is `true` if the "fips" crate feature is enabled,
/// `false` otherwise.
///
/// It must be set to `true` to meet FIPS requirement mentioned in section
/// **D.Q Transition of the TLS 1.2 KDF to Support the Extended Master
Expand Down Expand Up @@ -416,8 +417,20 @@ impl ServerConfig {

/// Return `true` if connections made with this `ServerConfig` will
/// operate in FIPS mode.
///
/// This is different from [`CryptoProvider::fips()`]: [`CryptoProvider::fips()`]
/// is concerned only with cryptography, whereas this _also_ covers TLS-level
/// configuration that NIST recommends.
pub fn fips(&self) -> bool {
self.provider.fips()
#[cfg(feature = "tls12")]
{
self.provider.fips() && self.require_ems
}

#[cfg(not(feature = "tls12"))]
{
self.provider.fips()
}
}

/// We support a given TLS version if it's quoted in the configured
Expand Down
37 changes: 32 additions & 5 deletions rustls/tests/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5063,14 +5063,19 @@ fn test_client_rejects_illegal_tls13_ccs() {

#[cfg(feature = "tls12")]
#[test]
fn test_client_rejects_no_extended_master_secret_extension_when_require_ems() {
fn test_client_rejects_no_extended_master_secret_extension_when_require_ems_or_fips() {
let key_type = KeyType::Rsa;
let mut client_config = make_client_config(key_type);
client_config.require_ems = true;
let server_config = finish_server_config(
if cfg!(feature = "fips") {
assert!(client_config.require_ems);
} else {
client_config.require_ems = true;
}
let mut server_config = finish_server_config(
key_type,
server_config_builder_with_versions(&[&rustls::version::TLS12]),
);
server_config.require_ems = false;
let (client, server) = make_pair_for_configs(client_config, server_config);
let (mut client, mut server) = (client.into(), server.into());
transfer_altered(&mut client, remove_ems_request, &mut server);
Expand All @@ -5086,14 +5091,18 @@ fn test_client_rejects_no_extended_master_secret_extension_when_require_ems() {

#[cfg(feature = "tls12")]
#[test]
fn test_server_rejects_no_extended_master_secret_extension_when_require_ems() {
fn test_server_rejects_no_extended_master_secret_extension_when_require_ems_or_fips() {
let key_type = KeyType::Rsa;
let client_config = make_client_config(key_type);
let mut server_config = finish_server_config(
key_type,
server_config_builder_with_versions(&[&rustls::version::TLS12]),
);
server_config.require_ems = true;
if cfg!(feature = "fips") {
assert!(server_config.require_ems);
} else {
server_config.require_ems = true;
}
let (client, server) = make_pair_for_configs(client_config, server_config);
let (mut client, mut server) = (client.into(), server.into());
transfer_altered(&mut client, remove_ems_request, &mut server);
Expand Down Expand Up @@ -5794,6 +5803,24 @@ fn test_server_fips_service_indicator() {
assert!(make_server_config(KeyType::Rsa).fips());
}

#[cfg(feature = "fips")]
#[test]
fn test_client_fips_service_indicator_includes_require_ems() {
let mut client_config = make_client_config(KeyType::Rsa);
assert!(client_config.fips());
client_config.require_ems = false;
assert!(!client_config.fips());
}

#[cfg(feature = "fips")]
#[test]
fn test_server_fips_service_indicator_includes_require_ems() {
let mut server_config = make_server_config(KeyType::Rsa);
assert!(server_config.fips());
server_config.require_ems = false;
assert!(!server_config.fips());
}

#[cfg(all(not(feature = "ring"), feature = "aws_lc_rs", not(feature = "fips")))]
#[test]
fn test_client_fips_service_indicator() {
Expand Down

0 comments on commit e8958e3

Please sign in to comment.