From a6a2d2f444fc9c7c744c44a7ccf53aab6707c1e5 Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Mon, 18 Nov 2019 20:40:10 +0600 Subject: [PATCH] update ssl impls --- actix-http/Cargo.toml | 4 +- actix-http/src/client/connector.rs | 126 +++++++++++++----------- actix-http/src/cookie/secure/key.rs | 47 +++++---- actix-http/src/cookie/secure/private.rs | 38 ++++--- actix-http/src/cookie/secure/signed.rs | 9 +- actix-http/src/h1/payload.rs | 11 +-- actix-http/tests/test_server.rs | 35 ++++--- 7 files changed, 150 insertions(+), 120 deletions(-) diff --git a/actix-http/Cargo.toml b/actix-http/Cargo.toml index 1cc5e43a1..742938d5e 100644 --- a/actix-http/Cargo.toml +++ b/actix-http/Cargo.toml @@ -27,7 +27,7 @@ path = "src/lib.rs" default = [] # openssl -openssl = ["open-ssl", "actix-connect/openssl"] +openssl = ["open-ssl", "actix-connect/openssl", "tokio-openssl"] # rustls support rustls = ["rust-tls", "webpki-roots", "actix-connect/rustls"] @@ -100,6 +100,8 @@ flate2 = { version="1.0.7", optional = true, default-features = false } # optional deps failure = { version = "0.1.5", optional = true } open-ssl = { version="0.10", package="openssl", optional = true } +tokio-openssl = { version = "0.4.0-alpha.6", optional = true } + rust-tls = { version = "0.16.0", package="rustls", optional = true } webpki-roots = { version = "0.18", optional = true } diff --git a/actix-http/src/client/connector.rs b/actix-http/src/client/connector.rs index 7421cb02e..45625ca9f 100644 --- a/actix-http/src/client/connector.rs +++ b/actix-http/src/client/connector.rs @@ -20,22 +20,22 @@ use super::error::ConnectError; use super::pool::{ConnectionPool, Protocol}; use super::Connect; -#[cfg(feature = "ssl")] -use openssl::ssl::SslConnector as OpensslConnector; +#[cfg(feature = "openssl")] +use open_ssl::ssl::SslConnector as OpensslConnector; -#[cfg(feature = "rust-tls")] -use rustls::ClientConfig; -#[cfg(feature = "rust-tls")] +#[cfg(feature = "rustls")] +use rust_tls::ClientConfig; +#[cfg(feature = "rustls")] use std::sync::Arc; -#[cfg(any(feature = "ssl", feature = "rust-tls"))] +#[cfg(any(feature = "openssl", feature = "rustls"))] enum SslConnector { - #[cfg(feature = "ssl")] + #[cfg(feature = "openssl")] Openssl(OpensslConnector), - #[cfg(feature = "rust-tls")] + #[cfg(feature = "rustls")] Rustls(Arc), } -#[cfg(not(any(feature = "ssl", feature = "rust-tls")))] +#[cfg(not(any(feature = "openssl", feature = "rustls")))] type SslConnector = (); /// Manages http client network connectivity @@ -76,9 +76,9 @@ impl Connector<(), ()> { TcpStream, > { let ssl = { - #[cfg(feature = "ssl")] + #[cfg(feature = "openssl")] { - use openssl::ssl::SslMethod; + use open_ssl::ssl::SslMethod; let mut ssl = OpensslConnector::builder(SslMethod::tls()).unwrap(); let _ = ssl @@ -86,7 +86,7 @@ impl Connector<(), ()> { .map_err(|e| error!("Can not set alpn protocol: {:?}", e)); SslConnector::Openssl(ssl.build()) } - #[cfg(all(not(feature = "ssl"), feature = "rust-tls"))] + #[cfg(all(not(feature = "openssl"), feature = "rustls"))] { let protos = vec![b"h2".to_vec(), b"http/1.1".to_vec()]; let mut config = ClientConfig::new(); @@ -96,7 +96,7 @@ impl Connector<(), ()> { .add_server_trust_anchors(&webpki_roots::TLS_SERVER_ROOTS); SslConnector::Rustls(Arc::new(config)) } - #[cfg(not(any(feature = "ssl", feature = "rust-tls")))] + #[cfg(not(any(feature = "openssl", feature = "rustls")))] {} }; @@ -145,7 +145,8 @@ where Request = TcpConnect, Response = TcpConnection, Error = actix_connect::ConnectError, - > + 'static, + > + Clone + + 'static, { /// Connection timeout, i.e. max time to connect to remote host including dns name resolution. /// Set to 1 second by default. @@ -154,14 +155,14 @@ where self } - #[cfg(feature = "ssl")] + #[cfg(feature = "openssl")] /// Use custom `SslConnector` instance. pub fn ssl(mut self, connector: OpensslConnector) -> Self { self.ssl = SslConnector::Openssl(connector); self } - #[cfg(feature = "rust-tls")] + #[cfg(feature = "rustls")] pub fn rustls(mut self, connector: Arc) -> Self { self.ssl = SslConnector::Rustls(connector); self @@ -217,7 +218,7 @@ where self, ) -> impl Service + Clone { - #[cfg(not(any(feature = "ssl", feature = "rust-tls")))] + #[cfg(not(any(feature = "openssl", feature = "rustls")))] { let connector = TimeoutService::new( self.timeout, @@ -242,46 +243,49 @@ where ), } } - #[cfg(any(feature = "ssl", feature = "rust-tls"))] + #[cfg(any(feature = "openssl", feature = "rustls"))] { const H2: &[u8] = b"h2"; - #[cfg(feature = "ssl")] + #[cfg(feature = "openssl")] use actix_connect::ssl::OpensslConnector; - #[cfg(feature = "rust-tls")] + #[cfg(feature = "rustls")] use actix_connect::ssl::RustlsConnector; - use actix_service::boxed::service; - #[cfg(feature = "rust-tls")] - use rustls::Session; + use actix_service::{boxed::service, pipeline}; + #[cfg(feature = "rustls")] + use rust_tls::Session; let ssl_service = TimeoutService::new( self.timeout, - apply_fn(self.connector.clone(), |msg: Connect, srv| { - srv.call(TcpConnect::new(msg.uri).set_addr(msg.addr)) - }) - .map_err(ConnectError::from) + pipeline( + apply_fn( + UnpinWrapper(self.connector.clone()), + |msg: Connect, srv| { + srv.call(TcpConnect::new(msg.uri).set_addr(msg.addr)) + }, + ) + .map_err(ConnectError::from), + ) .and_then(match self.ssl { - #[cfg(feature = "ssl")] - SslConnector::Openssl(ssl) => service( - OpensslConnector::service(ssl) - .map_err(ConnectError::from) - .map(|stream| { - let sock = stream.into_parts().0; - let h2 = sock - .get_ref() - .ssl() - .selected_alpn_protocol() - .map(|protos| protos.windows(2).any(|w| w == H2)) - .unwrap_or(false); - if h2 { - (Box::new(sock) as Box, Protocol::Http2) - } else { - (Box::new(sock) as Box, Protocol::Http1) - } - }), - ), - #[cfg(feature = "rust-tls")] + #[cfg(feature = "openssl")] + SslConnector::Openssl(ssl) => OpensslConnector::service(ssl) + .map(|stream| { + let sock = stream.into_parts().0; + let h2 = sock + .ssl() + .selected_alpn_protocol() + .map(|protos| protos.windows(2).any(|w| w == H2)) + .unwrap_or(false); + if h2 { + (Box::new(sock) as Box, Protocol::Http2) + } else { + (Box::new(sock) as Box, Protocol::Http1) + } + }) + .map_err(ConnectError::from), + + #[cfg(feature = "rustls")] SslConnector::Rustls(ssl) => service( - RustlsConnector::service(ssl) + UnpinWrapper(RustlsConnector::service(ssl)) .map_err(ConnectError::from) .map(|stream| { let sock = stream.into_parts().0; @@ -292,9 +296,15 @@ where .map(|protos| protos.windows(2).any(|w| w == H2)) .unwrap_or(false); if h2 { - (Box::new(sock) as Box, Protocol::Http2) + ( + Box::new(sock) as Box, + Protocol::Http2, + ) } else { - (Box::new(sock) as Box, Protocol::Http1) + ( + Box::new(sock) as Box, + Protocol::Http1, + ) } }), ), @@ -307,7 +317,7 @@ where let tcp_service = TimeoutService::new( self.timeout, - apply_fn(self.connector.clone(), |msg: Connect, srv| { + apply_fn(UnpinWrapper(self.connector), |msg: Connect, srv| { srv.call(TcpConnect::new(msg.uri).set_addr(msg.addr)) }) .map_err(ConnectError::from) @@ -339,11 +349,11 @@ where } #[derive(Clone)] -struct UnpinWrapper(T); +struct UnpinWrapper(T); -impl Unpin for UnpinWrapper {} +impl Unpin for UnpinWrapper {} -impl Service for UnpinWrapper { +impl Service for UnpinWrapper { type Request = T::Request; type Response = T::Response; type Error = T::Error; @@ -374,7 +384,7 @@ impl Future for UnpinWrapperFut { } } -#[cfg(not(any(feature = "ssl", feature = "rust-tls")))] +#[cfg(not(any(feature = "openssl", feature = "rustls")))] mod connect_impl { use std::task::{Context, Poll}; @@ -439,7 +449,7 @@ mod connect_impl { } } -#[cfg(any(feature = "ssl", feature = "rust-tls"))] +#[cfg(any(feature = "openssl", feature = "rustls"))] mod connect_impl { use std::marker::PhantomData; @@ -510,11 +520,11 @@ mod connect_impl { fn call(&mut self, req: Connect) -> Self::Future { match req.uri.scheme_str() { - Some("https") | Some("wss") => Either::B(InnerConnectorResponseB { + Some("https") | Some("wss") => Either::Right(InnerConnectorResponseB { fut: self.ssl_pool.call(req), _t: PhantomData, }), - _ => Either::A(InnerConnectorResponseA { + _ => Either::Left(InnerConnectorResponseA { fut: self.tcp_pool.call(req), _t: PhantomData, }), diff --git a/actix-http/src/cookie/secure/key.rs b/actix-http/src/cookie/secure/key.rs index 39575c93f..95058ed81 100644 --- a/actix-http/src/cookie/secure/key.rs +++ b/actix-http/src/cookie/secure/key.rs @@ -1,13 +1,12 @@ -use ring::digest::{Algorithm, SHA256}; -use ring::hkdf::expand; -use ring::hmac::SigningKey; +use ring::hkdf::{Algorithm, KeyType, Prk, HKDF_SHA256}; +use ring::hmac; use ring::rand::{SecureRandom, SystemRandom}; use super::private::KEY_LEN as PRIVATE_KEY_LEN; use super::signed::KEY_LEN as SIGNED_KEY_LEN; -static HKDF_DIGEST: &Algorithm = &SHA256; -const KEYS_INFO: &str = "COOKIE;SIGNED:HMAC-SHA256;PRIVATE:AEAD-AES-256-GCM"; +static HKDF_DIGEST: Algorithm = HKDF_SHA256; +const KEYS_INFO: &[&[u8]] = &[b"COOKIE;SIGNED:HMAC-SHA256;PRIVATE:AEAD-AES-256-GCM"]; /// A cryptographic master key for use with `Signed` and/or `Private` jars. /// @@ -25,6 +24,13 @@ pub struct Key { encryption_key: [u8; PRIVATE_KEY_LEN], } +impl KeyType for &Key { + #[inline] + fn len(&self) -> usize { + SIGNED_KEY_LEN + PRIVATE_KEY_LEN + } +} + impl Key { /// Derives new signing/encryption keys from a master key. /// @@ -56,21 +62,26 @@ impl Key { ); } - // Expand the user's key into two. - let prk = SigningKey::new(HKDF_DIGEST, key); + // An empty `Key` structure; will be filled in with HKDF derived keys. + let mut output_key = Key { + signing_key: [0; SIGNED_KEY_LEN], + encryption_key: [0; PRIVATE_KEY_LEN], + }; + + // Expand the master key into two HKDF generated keys. let mut both_keys = [0; SIGNED_KEY_LEN + PRIVATE_KEY_LEN]; - expand(&prk, KEYS_INFO.as_bytes(), &mut both_keys); + let prk = Prk::new_less_safe(HKDF_DIGEST, key); + let okm = prk.expand(KEYS_INFO, &output_key).expect("okm expand"); + okm.fill(&mut both_keys).expect("fill keys"); - // Copy the keys into their respective arrays. - let mut signing_key = [0; SIGNED_KEY_LEN]; - let mut encryption_key = [0; PRIVATE_KEY_LEN]; - signing_key.copy_from_slice(&both_keys[..SIGNED_KEY_LEN]); - encryption_key.copy_from_slice(&both_keys[SIGNED_KEY_LEN..]); - - Key { - signing_key, - encryption_key, - } + // Copy the key parts into their respective fields. + output_key + .signing_key + .copy_from_slice(&both_keys[..SIGNED_KEY_LEN]); + output_key + .encryption_key + .copy_from_slice(&both_keys[SIGNED_KEY_LEN..]); + output_key } /// Generates signing/encryption keys from a secure, random source. Keys are diff --git a/actix-http/src/cookie/secure/private.rs b/actix-http/src/cookie/secure/private.rs index eb8e9beb1..6c16e94e8 100644 --- a/actix-http/src/cookie/secure/private.rs +++ b/actix-http/src/cookie/secure/private.rs @@ -1,8 +1,8 @@ use std::str; use log::warn; -use ring::aead::{open_in_place, seal_in_place, Aad, Algorithm, Nonce, AES_256_GCM}; -use ring::aead::{OpeningKey, SealingKey}; +use ring::aead::{Aad, Algorithm, Nonce, AES_256_GCM}; +use ring::aead::{LessSafeKey, UnboundKey}; use ring::rand::{SecureRandom, SystemRandom}; use super::Key; @@ -10,7 +10,7 @@ use crate::cookie::{Cookie, CookieJar}; // Keep these in sync, and keep the key len synced with the `private` docs as // well as the `KEYS_INFO` const in secure::Key. -static ALGO: &Algorithm = &AES_256_GCM; +static ALGO: &'static Algorithm = &AES_256_GCM; const NONCE_LEN: usize = 12; pub const KEY_LEN: usize = 32; @@ -53,11 +53,14 @@ impl<'a> PrivateJar<'a> { } let ad = Aad::from(name.as_bytes()); - let key = OpeningKey::new(ALGO, &self.key).expect("opening key"); - let (nonce, sealed) = data.split_at_mut(NONCE_LEN); + let key = LessSafeKey::new( + UnboundKey::new(&ALGO, &self.key).expect("matching key length"), + ); + let (nonce, mut sealed) = data.split_at_mut(NONCE_LEN); let nonce = Nonce::try_assume_unique_for_key(nonce).expect("invalid length of `nonce`"); - let unsealed = open_in_place(&key, nonce, ad, 0, sealed) + let unsealed = key + .open_in_place(nonce, ad, &mut sealed) .map_err(|_| "invalid key/nonce/value: bad seal")?; if let Ok(unsealed_utf8) = str::from_utf8(unsealed) { @@ -196,30 +199,33 @@ Please change it as soon as possible." fn encrypt_name_value(name: &[u8], value: &[u8], key: &[u8]) -> Vec { // Create the `SealingKey` structure. - let key = SealingKey::new(ALGO, key).expect("sealing key creation"); + let unbound = UnboundKey::new(&ALGO, key).expect("matching key length"); + let key = LessSafeKey::new(unbound); // Create a vec to hold the [nonce | cookie value | overhead]. - let overhead = ALGO.tag_len(); - let mut data = vec![0; NONCE_LEN + value.len() + overhead]; + let mut data = vec![0; NONCE_LEN + value.len() + ALGO.tag_len()]; // Randomly generate the nonce, then copy the cookie value as input. let (nonce, in_out) = data.split_at_mut(NONCE_LEN); + let (in_out, tag) = in_out.split_at_mut(value.len()); + in_out.copy_from_slice(value); + + // Randomly generate the nonce into the nonce piece. SystemRandom::new() .fill(nonce) .expect("couldn't random fill nonce"); - in_out[..value.len()].copy_from_slice(value); - let nonce = - Nonce::try_assume_unique_for_key(nonce).expect("invalid length of `nonce`"); + let nonce = Nonce::try_assume_unique_for_key(nonce).expect("invalid `nonce` length"); // Use cookie's name as associated data to prevent value swapping. let ad = Aad::from(name); + let ad_tag = key + .seal_in_place_separate_tag(nonce, ad, in_out) + .expect("in-place seal"); - // Perform the actual sealing operation and get the output length. - let output_len = - seal_in_place(&key, nonce, ad, in_out, overhead).expect("in-place seal"); + // Copy the tag into the tag piece. + tag.copy_from_slice(ad_tag.as_ref()); // Remove the overhead and return the sealed content. - data.truncate(NONCE_LEN + output_len); data } diff --git a/actix-http/src/cookie/secure/signed.rs b/actix-http/src/cookie/secure/signed.rs index 36a277cd5..3fcd2cd84 100644 --- a/actix-http/src/cookie/secure/signed.rs +++ b/actix-http/src/cookie/secure/signed.rs @@ -1,12 +1,11 @@ -use ring::digest::{Algorithm, SHA256}; -use ring::hmac::{sign, verify_with_own_key as verify, SigningKey}; +use ring::hmac::{self, sign, verify}; use super::Key; use crate::cookie::{Cookie, CookieJar}; // Keep these in sync, and keep the key len synced with the `signed` docs as // well as the `KEYS_INFO` const in secure::Key. -static HMAC_DIGEST: &Algorithm = &SHA256; +static HMAC_DIGEST: hmac::Algorithm = hmac::HMAC_SHA256; const BASE64_DIGEST_LEN: usize = 44; pub const KEY_LEN: usize = 32; @@ -21,7 +20,7 @@ pub const KEY_LEN: usize = 32; /// This type is only available when the `secure` feature is enabled. pub struct SignedJar<'a> { parent: &'a mut CookieJar, - key: SigningKey, + key: hmac::Key, } impl<'a> SignedJar<'a> { @@ -32,7 +31,7 @@ impl<'a> SignedJar<'a> { pub fn new(parent: &'a mut CookieJar, key: &Key) -> SignedJar<'a> { SignedJar { parent, - key: SigningKey::new(HMAC_DIGEST, key.signing()), + key: hmac::Key::new(HMAC_DIGEST, key.signing()), } } diff --git a/actix-http/src/h1/payload.rs b/actix-http/src/h1/payload.rs index 036138f98..20ff830e7 100644 --- a/actix-http/src/h1/payload.rs +++ b/actix-http/src/h1/payload.rs @@ -234,7 +234,7 @@ mod tests { fn test_unread_data() { Runtime::new() .unwrap() - .block_on(lazy(|| { + .block_on(async { let (_, mut payload) = Payload::create(false); payload.unread_data(Bytes::from("data")); @@ -242,13 +242,12 @@ mod tests { assert_eq!(payload.len(), 4); assert_eq!( - Async::Ready(Some(Bytes::from("data"))), - payload.poll().ok().unwrap() + Poll::Ready(Some(Bytes::from("data"))), + payload.next_item().await.ok().unwrap() ); - let res: Result<(), ()> = Ok(()); - result(res) - })) + result(()) + }) .unwrap(); } } diff --git a/actix-http/tests/test_server.rs b/actix-http/tests/test_server.rs index a31e4ac89..51ee9f2d6 100644 --- a/actix-http/tests/test_server.rs +++ b/actix-http/tests/test_server.rs @@ -4,10 +4,10 @@ use std::{net, thread}; use actix_http_test::TestServer; use actix_server_config::ServerConfig; -use actix_service::{new_service_cfg, service_fn, NewService}; +use actix_service::{factory_fn_cfg, pipeline, service_fn, ServiceFactory}; use bytes::Bytes; -use futures::future::{self, ok, Future}; -use futures::stream::{once, Stream}; +use futures::future::{self, err, ok, ready, Future, FutureExt}; +use futures::stream::{once, Stream, StreamExt}; use regex::Regex; use tokio_timer::sleep; @@ -58,9 +58,9 @@ fn test_expect_continue() { HttpService::build() .expect(service_fn(|req: Request| { if req.head().uri.query() == Some("yes=") { - Ok(req) + ok(req) } else { - Err(error::ErrorPreconditionFailed("error")) + err(error::ErrorPreconditionFailed("error")) } })) .finish(|_| future::ok::<_, ()>(Response::Ok().finish())) @@ -117,9 +117,12 @@ fn test_chunked_payload() { HttpService::build().h1(|mut request: Request| { request .take_payload() - .map_err(|e| panic!(format!("Error reading payload: {}", e))) - .fold(0usize, |acc, chunk| future::ok::<_, ()>(acc + chunk.len())) - .map(|req_size| Response::Ok().body(format!("size={}", req_size))) + .map(|res| match res { + Ok(pl) => pl, + Err(e) => panic!(format!("Error reading payload: {}", e)), + }) + .fold(0usize, |acc, chunk| ready(acc + chunk.len())) + .map(|req_size| ok(Response::Ok().body(format!("size={}", req_size)))) }) }); @@ -414,7 +417,7 @@ const STR: &str = "Hello World Hello World Hello World Hello World Hello World \ #[test] fn test_h1_body() { let mut srv = TestServer::new(|| { - HttpService::build().h1(|_| future::ok::<_, ()>(Response::Ok().body(STR))) + HttpService::build().h1(|_| ok::<_, ()>(Response::Ok().body(STR))) }); let response = srv.block_on(srv.get("/").send()).unwrap(); @@ -493,7 +496,7 @@ fn test_h1_head_binary2() { fn test_h1_body_length() { let mut srv = TestServer::new(|| { HttpService::build().h1(|_| { - let body = once(Ok(Bytes::from_static(STR.as_ref()))); + let body = once(ok(Bytes::from_static(STR.as_ref()))); ok::<_, ()>( Response::Ok().body(body::SizedStream::new(STR.len() as u64, body)), ) @@ -512,7 +515,7 @@ fn test_h1_body_length() { fn test_h1_body_chunked_explicit() { let mut srv = TestServer::new(|| { HttpService::build().h1(|_| { - let body = once::<_, Error>(Ok(Bytes::from_static(STR.as_ref()))); + let body = once(ok::<_, Error>(Bytes::from_static(STR.as_ref()))); ok::<_, ()>( Response::Ok() .header(header::TRANSFER_ENCODING, "chunked") @@ -544,7 +547,7 @@ fn test_h1_body_chunked_explicit() { fn test_h1_body_chunked_implicit() { let mut srv = TestServer::new(|| { HttpService::build().h1(|_| { - let body = once::<_, Error>(Ok(Bytes::from_static(STR.as_ref()))); + let body = once(ok::<_, Error>(Bytes::from_static(STR.as_ref()))); ok::<_, ()>(Response::Ok().streaming(body)) }) }); @@ -569,15 +572,15 @@ fn test_h1_body_chunked_implicit() { #[test] fn test_h1_response_http_error_handling() { let mut srv = TestServer::new(|| { - HttpService::build().h1(new_service_cfg(|_: &ServerConfig| { - Ok::<_, ()>(|_| { + HttpService::build().h1(factory_fn_cfg(|_: &ServerConfig| { + ok::<_, ()>(pipeline(|_| { let broken_header = Bytes::from_static(b"\0\0\0"); ok::<_, ()>( Response::Ok() .header(http::header::CONTENT_TYPE, broken_header) .body(STR), ) - }) + })) })) }); @@ -593,7 +596,7 @@ fn test_h1_response_http_error_handling() { fn test_h1_service_error() { let mut srv = TestServer::new(|| { HttpService::build() - .h1(|_| Err::(error::ErrorBadRequest("error"))) + .h1(|_| future::err::(error::ErrorBadRequest("error"))) }); let response = srv.block_on(srv.get("/").send()).unwrap();