Skip to content

Commit

Permalink
Deal with versions/suites being contradictory
Browse files Browse the repository at this point in the history
If we have TLS1.3 enabled but no TLS1.3 suites, we
shouldn't actually offer TLS1.3.
  • Loading branch information
ctz committed Sep 17, 2018
1 parent 4016cda commit c4323cf
Show file tree
Hide file tree
Showing 6 changed files with 28 additions and 8 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ Rustls is currently in development and hence unstable. [Here's what I'm working
- Introduce client-side support for 0-RTT data in TLS1.3.
- Fix a bug in rustls::Stream for non-blocking transports.
- Move TLS1.3 support from draft 28 to final RFC8446 version.
- Don't offer (eg) TLS1.3 if no TLS1.3 suites are configured.
* 0.13.1 (2018-08-17):
- Fix a bug in rustls::Stream for non-blocking transports
(backport).
Expand Down
8 changes: 4 additions & 4 deletions src/client/hs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,8 +247,8 @@ fn emit_client_hello_for_retry(sess: &mut ClientSessionImpl,
(SessionID::empty(), Vec::new(), ProtocolVersion::Unknown(0))
};

let support_tls12 = sess.config.versions.contains(&ProtocolVersion::TLSv1_2);
let support_tls13 = sess.config.versions.contains(&ProtocolVersion::TLSv1_3);
let support_tls12 = sess.config.supports_version(ProtocolVersion::TLSv1_2);
let support_tls13 = sess.config.supports_version(ProtocolVersion::TLSv1_3);

let mut supported_versions = Vec::new();
if support_tls13 {
Expand Down Expand Up @@ -642,7 +642,7 @@ impl State for ExpectServerHello {
trace!("We got ServerHello {:#?}", server_hello);

use ProtocolVersion::{TLSv1_2, TLSv1_3};
let tls13_supported = sess.config.versions.contains(&TLSv1_3);
let tls13_supported = sess.config.supports_version(TLSv1_3);

let server_version = if server_hello.legacy_version == TLSv1_2 {
server_hello.get_supported_versions()
Expand All @@ -655,7 +655,7 @@ impl State for ExpectServerHello {
TLSv1_3 if tls13_supported => {
sess.common.negotiated_version = Some(TLSv1_3);
}
TLSv1_2 if sess.config.versions.contains(&TLSv1_2) => {
TLSv1_2 if sess.config.supports_version(TLSv1_2) => {
if sess.early_data.is_enabled() && sess.common.early_traffic {
// The client must fail with a dedicated error code if the server
// responds with TLS 1.2 when offering 0-RTT.
Expand Down
8 changes: 8 additions & 0 deletions src/client/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,14 @@ impl ClientConfig {
}
}

#[doc(hidden)]
/// We support a given TLS version if it's quoted in the configured
/// versions *and* at least one ciphersuite for this version is
/// also configured.
pub fn supports_version(&self, v: ProtocolVersion) -> bool {
self.versions.contains(&v) && self.ciphersuites.iter().any(|cs| cs.usable_for_version(v))
}

#[doc(hidden)]
pub fn get_verifier(&self) -> &verify::ServerCertVerifier {
self.verifier.as_ref()
Expand Down
4 changes: 2 additions & 2 deletions src/server/hs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -957,8 +957,8 @@ impl State for ExpectClientHello {

fn handle(mut self: Box<Self>, sess: &mut ServerSessionImpl, m: Message) -> NextStateOrError {
let client_hello = extract_handshake!(m, HandshakePayload::ClientHello).unwrap();
let tls13_enabled = sess.config.versions.contains(&ProtocolVersion::TLSv1_3);
let tls12_enabled = sess.config.versions.contains(&ProtocolVersion::TLSv1_2);
let tls13_enabled = sess.config.supports_version(ProtocolVersion::TLSv1_3);
let tls12_enabled = sess.config.supports_version(ProtocolVersion::TLSv1_2);
trace!("we got a clienthello {:?}", client_hello);

if !client_hello.compression_methods.contains(&Compression::Null) {
Expand Down
8 changes: 8 additions & 0 deletions src/server/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,14 @@ impl ServerConfig {
}
}

#[doc(hidden)]
/// We support a given TLS version if it's quoted in the configured
/// versions *and* at least one ciphersuite for this version is
/// also configured.
pub fn supports_version(&self, v: ProtocolVersion) -> bool {
self.versions.contains(&v) && self.ciphersuites.iter().any(|cs| cs.usable_for_version(v))
}

#[doc(hidden)]
pub fn get_verifier(&self) -> &verify::ClientCertVerifier {
self.verifier.as_ref()
Expand Down
7 changes: 5 additions & 2 deletions tests/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1143,9 +1143,12 @@ fn stream_write_swallows_underlying_io_error_after_plaintext_processed() {
fn make_disjoint_suite_configs() -> (ClientConfig, ServerConfig) {
let kt = KeyType::RSA;
let mut server_config = make_server_config(kt);
server_config.ciphersuites = vec![];
server_config.ciphersuites = vec![find_suite(CipherSuite::TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256)];

(make_client_config(kt), server_config)
let mut client_config = make_client_config(kt);
client_config.ciphersuites = vec![find_suite(CipherSuite::TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384)];

(client_config, server_config)
}

#[test]
Expand Down

0 comments on commit c4323cf

Please sign in to comment.