From 2e336483d549384cea79a7971b10aa105281a146 Mon Sep 17 00:00:00 2001 From: Jonathan Chien Date: Tue, 23 May 2017 18:15:36 -0700 Subject: [PATCH 1/3] Enable specification of certain ciphers --- security-framework/src/cipher_suite.rs | 2 +- security-framework/src/secure_transport.rs | 66 ++++++++++++++++++++++ 2 files changed, 67 insertions(+), 1 deletion(-) diff --git a/security-framework/src/cipher_suite.rs b/security-framework/src/cipher_suite.rs index 05069f70..2e0d423e 100644 --- a/security-framework/src/cipher_suite.rs +++ b/security-framework/src/cipher_suite.rs @@ -7,7 +7,7 @@ macro_rules! make_suites { ($($suite:ident),+) => { /// Specifies cipher suites #[allow(non_camel_case_types, missing_docs)] - #[derive(Debug, Copy, Clone, PartialEq, Eq)] + #[derive(Debug, Copy, Clone, Hash, PartialEq, Eq)] pub enum CipherSuite { $($suite),+ } diff --git a/security-framework/src/secure_transport.rs b/security-framework/src/secure_transport.rs index ad89b227..68cdc0b2 100644 --- a/security-framework/src/secure_transport.rs +++ b/security-framework/src/secure_transport.rs @@ -1091,6 +1091,8 @@ pub struct ClientBuilder { protocol_min: Option, protocol_max: Option, trust_certs_only: bool, + whitelisted_ciphers: Vec, + blacklisted_ciphers: Vec, } impl Default for ClientBuilder { @@ -1109,6 +1111,8 @@ impl ClientBuilder { protocol_min: None, protocol_max: None, trust_certs_only: false, + whitelisted_ciphers: Vec::new(), + blacklisted_ciphers: Vec::new(), } } @@ -1126,6 +1130,18 @@ impl ClientBuilder { self } + /// Set a whitelist of enabled ciphers. Any ciphers not whitelisted will be disabled. + pub fn whitelist_ciphers(&mut self, whitelisted_ciphers: &[CipherSuite]) -> &mut Self { + self.whitelisted_ciphers = whitelisted_ciphers.to_owned(); + self + } + + /// Set a blacklist of disabled ciphers. Blacklisted ciphers will be disabled. + pub fn blacklist_ciphers(&mut self, blacklisted_ciphers: &[CipherSuite]) -> &mut Self { + self.blacklisted_ciphers = blacklisted_ciphers.to_owned(); + self + } + /// Use the specified identity as a SSL/TLS client certificate. pub fn identity(&mut self, identity: &SecIdentity, chain: &[SecCertificate]) -> &mut Self { self.identity = Some(identity.clone()); @@ -1214,6 +1230,7 @@ impl ClientBuilder { } try!(ctx.set_break_on_server_auth(true)); try!(self.configure_protocols(&mut ctx)); + try!(self.configure_ciphers(&mut ctx)); let certs = self.certs.clone(); @@ -1247,6 +1264,22 @@ impl ClientBuilder { fn configure_protocols(&self, _ctx: &mut SslContext) -> Result<()> { Ok(()) } + + fn configure_ciphers(&self, ctx: &mut SslContext) -> Result<()> { + let mut ciphers = if self.whitelisted_ciphers.is_empty() { + try!(ctx.enabled_ciphers()) + } + else { + self.whitelisted_ciphers.clone() + }; + + if !self.blacklisted_ciphers.is_empty() { + ciphers.retain(|cipher| !self.blacklisted_ciphers.contains(cipher)); + } + + try!(ctx.set_enabled_ciphers(&ciphers)); + Ok(()) + } } /// A builder type to simplify the creation of server-side `SslStream`s. @@ -1371,6 +1404,39 @@ mod test { assert_eq!(ciphers, p!(ctx.enabled_ciphers())); } + #[test] + fn test_builder_whitelist_ciphers() { + let stream = p!(TcpStream::connect("google.com:443")); + + let ctx = p!(SslContext::new(ProtocolSide::Client, ConnectionType::Stream)); + assert!(p!(ctx.enabled_ciphers()).len() > 1); + + let ciphers = p!(ctx.enabled_ciphers()); + let cipher = ciphers.first().unwrap(); + let stream = p!(ClientBuilder::new() + .whitelist_ciphers(&[*cipher]) + .handshake("google.com", stream)); + + assert_eq!(1, p!(stream.context().enabled_ciphers()).len()); + } + + #[test] + fn test_builder_blacklist_ciphers() { + let stream = p!(TcpStream::connect("google.com:443")); + + let ctx = p!(SslContext::new(ProtocolSide::Client, ConnectionType::Stream)); + let num = p!(ctx.enabled_ciphers()).len(); + assert!(num > 1); + + let ciphers = p!(ctx.enabled_ciphers()); + let cipher = ciphers.first().unwrap(); + let stream = p!(ClientBuilder::new() + .blacklist_ciphers(&[*cipher]) + .handshake("google.com", stream)); + + assert_eq!(num - 1, p!(stream.context().enabled_ciphers()).len()); + } + #[test] fn idle_context_peer_trust() { let ctx = p!(SslContext::new(ProtocolSide::Server, ConnectionType::Stream)); From 77b2fb7ee31fbc130f46bc3954ba66f9f5cffe3b Mon Sep 17 00:00:00 2001 From: Jonathan Chien Date: Wed, 24 May 2017 13:14:04 -0700 Subject: [PATCH 2/3] Split out ctx to avoid actual validation in tests --- security-framework/src/secure_transport.rs | 23 ++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/security-framework/src/secure_transport.rs b/security-framework/src/secure_transport.rs index 68cdc0b2..ebe94d39 100644 --- a/security-framework/src/secure_transport.rs +++ b/security-framework/src/secure_transport.rs @@ -1214,10 +1214,8 @@ impl ClientBuilder { self.handshake_inner(None, stream) } - fn handshake_inner(&self, - domain: Option<&str>, - stream: S) - -> result::Result, ClientHandshakeError> + + fn ctx_into_stream(&self, domain: Option<&str>, stream: S) -> Result> where S: Read + Write { let mut ctx = try!(SslContext::new(ProtocolSide::Client, ConnectionType::Stream)); @@ -1232,14 +1230,23 @@ impl ClientBuilder { try!(self.configure_protocols(&mut ctx)); try!(self.configure_ciphers(&mut ctx)); - let certs = self.certs.clone(); + ctx.into_stream(stream) + } + fn handshake_inner(&self, + domain: Option<&str>, + stream: S) + -> result::Result, ClientHandshakeError> + where S: Read + Write + { // the logic for trust validation is in MidHandshakeClientBuilder::connect, so run all // of the handshake logic through that. let stream = MidHandshakeSslStream { - stream: try!(ctx.into_stream(stream)), + stream: try!(self.ctx_into_stream(domain, stream)), error: Error::from(errSecSuccess), }; + + let certs = self.certs.clone(); let stream = MidHandshakeClientBuilder { stream: stream, domain: domain.map(|s| s.to_string()), @@ -1415,7 +1422,7 @@ mod test { let cipher = ciphers.first().unwrap(); let stream = p!(ClientBuilder::new() .whitelist_ciphers(&[*cipher]) - .handshake("google.com", stream)); + .ctx_into_stream(None, stream)); assert_eq!(1, p!(stream.context().enabled_ciphers()).len()); } @@ -1432,7 +1439,7 @@ mod test { let cipher = ciphers.first().unwrap(); let stream = p!(ClientBuilder::new() .blacklist_ciphers(&[*cipher]) - .handshake("google.com", stream)); + .ctx_into_stream(None, stream)); assert_eq!(num - 1, p!(stream.context().enabled_ciphers()).len()); } From 48a883ccaa128403cda658be2a0bd37e13712ccb Mon Sep 17 00:00:00 2001 From: Jonathan Chien Date: Wed, 24 May 2017 14:19:03 -0700 Subject: [PATCH 3/3] Disable test on iOS as it says 0 ciphers --- security-framework/src/secure_transport.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/security-framework/src/secure_transport.rs b/security-framework/src/secure_transport.rs index ebe94d39..896408b6 100644 --- a/security-framework/src/secure_transport.rs +++ b/security-framework/src/secure_transport.rs @@ -1428,6 +1428,7 @@ mod test { } #[test] + #[cfg_attr(target_os = "ios", ignore)] // FIXME same issue as cipher_configuration fn test_builder_blacklist_ciphers() { let stream = p!(TcpStream::connect("google.com:443"));