diff --git a/rustls/src/server/hs.rs b/rustls/src/server/hs.rs index 3f24e1b4d8..20eb0dfcc6 100644 --- a/rustls/src/server/hs.rs +++ b/rustls/src/server/hs.rs @@ -465,7 +465,7 @@ impl ExpectClientHello { return Err(PeerIncompatible::NoKxGroupsInCommon); } - let suitable_suites = self + let mut suitable_suites_iter = self .config .provider .cipher_suites @@ -480,9 +480,7 @@ impl ExpectClientHello { // And key exchange groups && (!ecdhe_possible || suite.usable_for_kx_algorithm(KeyExchangeAlgorithm::ECDHE)) && (!ffdhe_possible || suite.usable_for_kx_algorithm(KeyExchangeAlgorithm::DHE)) - }) - .copied() - .collect::>(); + }); // RFC 7919 (https://datatracker.ietf.org/doc/html/rfc7919#section-4) requires us to send // the InsufficientSecurity alert in case we don't recognize client's FFDHE groups (i.e., @@ -491,9 +489,17 @@ impl ExpectClientHello { // and continue to send HandshakeFailure. let suite = if self.config.ignore_client_order { - suites::choose_ciphersuite_preferring_server(client_suites, &suitable_suites) + suitable_suites_iter.find(|suite| client_suites.contains(&suite.suite())) } else { - suites::choose_ciphersuite_preferring_client(client_suites, &suitable_suites) + let suitable_suites = suitable_suites_iter.collect::>(); + client_suites + .iter() + .find_map(|client_suite| { + suitable_suites + .iter() + .find(|x| *client_suite == x.suite()) + }) + .copied() } .ok_or(PeerIncompatible::NoCipherSuitesInCommon)?; @@ -510,18 +516,18 @@ impl ExpectClientHello { if selected_version == ProtocolVersion::TLSv1_3 { // This unwrap is structurally guaranteed by the early return for `!ffdhe_possible && !ecdhe_possible` - return Ok((suite, *maybe_skxg.unwrap())); + return Ok((*suite, *maybe_skxg.unwrap())); } // For TLS1.2, the server can unilaterally choose a DHE group if it has one and // there was no better option. match maybe_skxg { - Some(skxg) => Ok((suite, *skxg)), + Some(skxg) => Ok((*suite, *skxg)), None if suite.usable_for_kx_algorithm(KeyExchangeAlgorithm::DHE) => { // If kx for the selected cipher suite is DHE and no DHE groups are specified in the extension, // the server is free to choose DHE params, we choose the first DHE kx group of the provider. if let Some(server_selected_ffdhe_skxg) = first_supported_dhe_kxg { - Ok((suite, *server_selected_ffdhe_skxg)) + Ok((*suite, *server_selected_ffdhe_skxg)) } else { Err(PeerIncompatible::NoKxGroupsInCommon) } diff --git a/rustls/src/suites.rs b/rustls/src/suites.rs index 9a5d72a2a2..f66a834736 100644 --- a/rustls/src/suites.rs +++ b/rustls/src/suites.rs @@ -167,37 +167,6 @@ impl fmt::Debug for SupportedCipherSuite { } } -// These both O(N^2)! -pub(crate) fn choose_ciphersuite_preferring_client( - client_suites: &[CipherSuite], - server_suites: &[SupportedCipherSuite], -) -> Option { - for client_suite in client_suites { - if let Some(selected) = server_suites - .iter() - .find(|x| *client_suite == x.suite()) - { - return Some(*selected); - } - } - - None -} - -pub(crate) fn choose_ciphersuite_preferring_server( - client_suites: &[CipherSuite], - server_suites: &[SupportedCipherSuite], -) -> Option { - if let Some(selected) = server_suites - .iter() - .find(|x| client_suites.contains(&x.suite())) - { - return Some(*selected); - } - - None -} - /// Return true if `sigscheme` is usable by any of the given suites. pub(crate) fn compatible_sigscheme_for_suites( sigscheme: SignatureScheme, @@ -264,48 +233,8 @@ pub enum ConnectionTrafficSecrets { } test_for_each_provider! { - use super::*; - use crate::enums::CipherSuite; use provider::tls13::*; - #[test] - fn test_client_pref() { - let client = vec![ - CipherSuite::TLS13_AES_128_GCM_SHA256, - CipherSuite::TLS13_AES_256_GCM_SHA384, - ]; - let server = vec![TLS13_AES_256_GCM_SHA384, TLS13_AES_128_GCM_SHA256]; - let chosen = choose_ciphersuite_preferring_client(&client, &server); - assert!(chosen.is_some()); - assert_eq!(chosen.unwrap(), TLS13_AES_128_GCM_SHA256); - } - - #[test] - fn test_server_pref() { - let client = vec![ - CipherSuite::TLS13_AES_128_GCM_SHA256, - CipherSuite::TLS13_AES_256_GCM_SHA384, - ]; - let server = vec![TLS13_AES_256_GCM_SHA384, TLS13_AES_128_GCM_SHA256]; - let chosen = choose_ciphersuite_preferring_server(&client, &server); - assert!(chosen.is_some()); - assert_eq!(chosen.unwrap(), TLS13_AES_256_GCM_SHA384); - } - - #[test] - fn test_pref_fails() { - assert!(choose_ciphersuite_preferring_client( - &[CipherSuite::TLS_NULL_WITH_NULL_NULL], - provider::ALL_CIPHER_SUITES - ) - .is_none()); - assert!(choose_ciphersuite_preferring_server( - &[CipherSuite::TLS_NULL_WITH_NULL_NULL], - provider::ALL_CIPHER_SUITES - ) - .is_none()); - } - #[test] fn test_scs_is_debug() { println!("{:?}", provider::ALL_CIPHER_SUITES); diff --git a/rustls/tests/api.rs b/rustls/tests/api.rs index ac97602ee6..53fe411f6f 100644 --- a/rustls/tests/api.rs +++ b/rustls/tests/api.rs @@ -3011,6 +3011,45 @@ fn negotiated_ciphersuite_server() { } } +#[test] +fn negotiated_ciphersuite_server_ignoring_client_preference() { + for (version, kt, suite) in test_ciphersuites() { + let scs = find_suite(suite); + let scs_other = if scs.suite() == CipherSuite::TLS13_AES_256_GCM_SHA384 { + find_suite(CipherSuite::TLS13_AES_128_GCM_SHA256) + } else { + find_suite(CipherSuite::TLS13_AES_256_GCM_SHA384) + }; + let mut server_config = finish_server_config( + kt, + ServerConfig::builder_with_provider( + CryptoProvider { + cipher_suites: vec![scs, scs_other], + ..provider::default_provider() + } + .into(), + ) + .with_protocol_versions(&[version]) + .unwrap(), + ); + server_config.ignore_client_order = true; + + let client_config = finish_client_config( + kt, + ClientConfig::builder_with_provider( + CryptoProvider { + cipher_suites: vec![ scs_other, scs ], + ..provider::default_provider() + }.into(), + ) + .with_safe_default_protocol_versions() + .unwrap()); + + do_suite_test(client_config, server_config, scs, version.version); + } + +} + #[derive(Debug, PartialEq)] struct KeyLogItem { label: String,