From c5b13fb3f7cdc7488ec7b07378c45217a60b62b7 Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Thu, 7 Jun 2018 13:08:37 -1000 Subject: [PATCH] Proxy: Better encapsulate the details of TLS config watching. (#1082, #1083) * Fix non-Linux builds. The change to signal.rs is needed for Windows. The change to config.rs is needed for Windows and maybe other platforms. Signed-off-by: Brian Smith * Proxy: Better encapsulate the details of TLS config watching. Encapsulate more of the TLS configuration logic in the TLS submodule. This allows for easier refactoring. In particular, this will make adding the client TLS configuration easier. Signed-off-by: Brian Smith --- proxy/src/lib.rs | 9 +---- proxy/src/signal.rs | 2 +- proxy/src/transport/tls/config.rs | 63 ++++++++++++++++--------------- proxy/src/transport/tls/mod.rs | 2 +- 4 files changed, 36 insertions(+), 40 deletions(-) diff --git a/proxy/src/lib.rs b/proxy/src/lib.rs index b8370d3f2..c9f59f0e0 100644 --- a/proxy/src/lib.rs +++ b/proxy/src/lib.rs @@ -232,15 +232,8 @@ where let bind = Bind::new().with_sensors(sensors.clone()); - let tls_config_changes = config.tls_settings - // TODO: determine the correct interval for this. - .map(|settings| settings.stream_changes(Duration::from_secs(1))); - let (tls_server_config, tls_cfg_bg) = - tls_config_changes - .map(tls::ServerConfig::watch) - .unwrap_or_else(tls::ServerConfig::no_tls); - + tls::watch_for_config_changes(config.tls_settings.as_ref()); // Setup the public listener. This will listen on a publicly accessible // address and listen for inbound connections that should be forwarded diff --git a/proxy/src/signal.rs b/proxy/src/signal.rs index ca8f46005..7790c8768 100644 --- a/proxy/src/signal.rs +++ b/proxy/src/signal.rs @@ -74,7 +74,7 @@ mod imp { #[cfg(not(unix))] mod imp { - use super::{tokio_signal, Handle, ShutdownSignal}; + use super::{tokio_signal, ShutdownSignal}; use super::futures::{Future, Stream}; pub(super) fn shutdown() -> ShutdownSignal { diff --git a/proxy/src/transport/tls/config.rs b/proxy/src/transport/tls/config.rs index 81df95e6d..ed172de8c 100644 --- a/proxy/src/transport/tls/config.rs +++ b/proxy/src/transport/tls/config.rs @@ -1,8 +1,7 @@ use std::{ - collections::HashSet, fs::File, io::{self, Cursor, Read}, - path::{Path, PathBuf}, + path::PathBuf, sync::Arc, time::{Duration, Instant, SystemTime,}, }; @@ -33,7 +32,7 @@ pub type ServerConfigWatch = Watch>; /// The end-entity certificate and private key are in DER format because they /// are stored in the secret store where space utilization is a concern, and /// because PEM doesn't offer any advantages. -#[derive(Debug)] +#[derive(Clone, Debug)] pub struct CommonSettings { /// The trust anchors as concatenated PEM-encoded X.509 certificates. pub trust_anchors: PathBuf, @@ -58,9 +57,7 @@ pub struct ServerConfig(pub(super) Arc); #[derive(Debug)] pub enum Error { Io(PathBuf, io::Error), - FailedToParsePrivateKey, FailedToParseTrustAnchors(Option), - EmptyEndEntityCert, EndEntityCertIsNotValid(webpki::Error), InvalidPrivateKey, TimeConversionFailed, @@ -82,7 +79,7 @@ impl CommonSettings { /// The returned stream consists of each subsequent successfully loaded /// `CommonSettings` after each change. If the settings could not be /// reloaded (i.e., they were malformed), nothing is sent. - pub fn stream_changes(self, interval: Duration) + fn stream_changes(self, interval: Duration) -> impl Stream { // If we're on Linux, first atttempt to start an Inotify watch on the @@ -170,7 +167,9 @@ impl CommonSettings { fn stream_changes_inotify(&self) -> Result, Error> { + use std::{collections::HashSet, path::Path}; use inotify::{Inotify, WatchMask}; + // Use a broad watch mask so that we will pick up any events that might // indicate a change to the watched files. // @@ -236,7 +235,7 @@ impl CommonConfig { /// must be issued by the CA represented by a certificate in the /// trust anchors file. Since filesystem operations are not atomic, we /// need to check for this consistency. - pub fn load_from_disk(settings: &CommonSettings) -> Result { + fn load_from_disk(settings: &CommonSettings) -> Result { let trust_anchor_certs = load_file_contents(&settings.trust_anchors) .and_then(|file_contents| rustls::internal::pemfile::certs(&mut Cursor::new(file_contents)) @@ -270,36 +269,40 @@ impl CommonConfig { } +pub fn watch_for_config_changes(settings: Option<&CommonSettings>) + -> (ServerConfigWatch, Box + Send>) +{ + let settings = if let Some(settings) = settings { + settings.clone() + } else { + let (watch, _) = Watch::new(None); + let no_future = future::ok(()); + return (watch, Box::new(no_future)); + }; + + let changes = settings.stream_changes(Duration::from_secs(1)); + let (watch, store) = Watch::new(None); + let server_configs = changes.map(|ref config| Some(ServerConfig::from(config))); + let store = store + .sink_map_err(|_| warn!("all server config watches dropped")); + let f = server_configs.forward(store) + .map(|_| trace!("forwarding to server config watch finished.")); + + // This function and `ServerConfig::no_tls` return `Box>` + // rather than `impl Future<...>` so that they can have the _same_ return + // types (impl Traits are not the same type unless the original + // non-anonymized type was the same). + (watch, Box::new(f)) +} + impl ServerConfig { - pub fn from(common: &CommonConfig) -> Self { + fn from(common: &CommonConfig) -> Self { let mut config = rustls::ServerConfig::new(Arc::new(rustls::NoClientAuth)); set_common_settings(&mut config.versions); config.cert_resolver = common.cert_resolver.clone(); ServerConfig(Arc::new(config)) } - /// Watch a `Stream` of changes to a `CommonConfig`, such as those returned by - /// `CommonSettings::stream_changes`, and update a `futures_watch::Watch` cell - /// with a `ServerConfig` generated from each change. - pub fn watch(changes: C) - -> (ServerConfigWatch, Box + Send>) - where - C: Stream + Send + 'static, - { - let (watch, store) = Watch::new(None); - let server_configs = changes.map(|ref config| Self::from(config)); - let store = store - .sink_map_err(|_| warn!("all server config watches dropped")); - let f = server_configs.map(Some).forward(store) - .map(|_| trace!("forwarding to server config watch finished.")); - - // This function and `no_tls` return `Box>` rather than - // `impl Future<...>` so that they can have the _same_ return types - // (impl Traits are not the same type unless the original - // non-anonymized type was the same). - (watch, Box::new(f)) - } - pub fn no_tls() -> (ServerConfigWatch, Box + Send>) { diff --git a/proxy/src/transport/tls/mod.rs b/proxy/src/transport/tls/mod.rs index 363734399..94d5f0b80 100755 --- a/proxy/src/transport/tls/mod.rs +++ b/proxy/src/transport/tls/mod.rs @@ -12,7 +12,7 @@ mod dns_name; mod identity; pub use self::{ - config::{CommonSettings, CommonConfig, Error, ServerConfig, ServerConfigWatch}, + config::{CommonSettings, ServerConfig, ServerConfigWatch, watch_for_config_changes}, connection::Connection, dns_name::{DnsName, InvalidDnsName}, identity::Identity,