diff options
| -rw-r--r-- | mullvad-api/src/https_client.rs | 89 | ||||
| -rw-r--r-- | mullvad-api/src/lib.rs | 1 | ||||
| -rw-r--r-- | mullvad-api/src/proxy.rs | 71 | ||||
| -rw-r--r-- | mullvad-daemon/src/access_method.rs | 6 | ||||
| -rw-r--r-- | mullvad-daemon/src/api.rs | 18 | ||||
| -rw-r--r-- | mullvad-daemon/src/lib.rs | 28 | ||||
| -rw-r--r-- | talpid-types/src/net/proxy.rs | 3 |
7 files changed, 118 insertions, 98 deletions
diff --git a/mullvad-api/src/https_client.rs b/mullvad-api/src/https_client.rs index ff36e945ae..2b76006d7e 100644 --- a/mullvad-api/src/https_client.rs +++ b/mullvad-api/src/https_client.rs @@ -1,7 +1,7 @@ use crate::{ DnsResolver, abortable_stream::{AbortableStream, AbortableStreamHandle}, - proxy::{ApiConnection, ApiConnectionMode, ProxyConfig}, + proxy::{ApiConnection, ApiConnectionMode, ApiShadowsocksConfig, ProxyConfig}, tls_stream::TlsStream, }; use futures::{StreamExt, channel::mpsc, future, pin_mut}; @@ -17,7 +17,6 @@ use shadowsocks::{ ServerConfig, config::{ServerConfigError, ServerType}, context::{Context as SsContext, SharedContext}, - crypto::CipherKind, relay::tcprelay::ProxyClientStream, }; #[cfg(target_os = "android")] @@ -28,12 +27,12 @@ use std::{ io, net::{IpAddr, SocketAddr}, pin::Pin, - str::{self, FromStr}, + str, sync::{Arc, Mutex}, task::{Context, Poll}, time::Duration, }; -use talpid_types::{ErrorExt, net::proxy}; +use talpid_types::net::proxy; use tokio::{ io::{AsyncRead, AsyncWrite}, net::{TcpSocket, TcpStream}, @@ -109,7 +108,7 @@ impl InnerConnectionMode { } // Set up a Shadowsocks-connection. InnerConnectionMode::Shadowsocks(shadowsocks) => { - let first_hop = shadowsocks.params.peer; + let first_hop = shadowsocks.params.endpoint; let make_proxy_stream = |tcp_stream| async { Ok(ProxyClientStream::from_stream( shadowsocks.proxy_context, @@ -228,21 +227,14 @@ impl InnerConnectionMode { #[derive(Clone)] struct ShadowsocksConfig { proxy_context: SharedContext, - params: ParsedShadowsocksConfig, + params: ApiShadowsocksConfig, } -#[derive(Clone)] -struct ParsedShadowsocksConfig { - peer: SocketAddr, - password: String, - cipher: CipherKind, -} - -impl TryFrom<ParsedShadowsocksConfig> for ServerConfig { +impl TryFrom<ApiShadowsocksConfig> for ServerConfig { type Error = ServerConfigError; - fn try_from(config: ParsedShadowsocksConfig) -> Result<Self, Self::Error> { - ServerConfig::new(config.peer, config.password, config.cipher) + fn try_from(config: ApiShadowsocksConfig) -> Result<Self, Self::Error> { + ServerConfig::new(config.endpoint, config.password, config.cipher) } } @@ -252,34 +244,15 @@ struct SocksConfig { authentication: Option<proxy::SocksAuth>, } -#[derive(thiserror::Error, Debug)] -pub enum ProxyConfigError { - #[error("Unrecognized cipher selected: {0}")] - InvalidCipher(String), -} - -/// Validate an [`ApiConnectionMode`], returning an error if it contains -/// an unsupported cipher or other configuration issue. -pub fn validate_connection_mode(mode: &ApiConnectionMode) -> Result<(), ProxyConfigError> { - InnerConnectionMode::try_from(mode.clone()).map(|_| ()) -} - -impl TryFrom<ApiConnectionMode> for InnerConnectionMode { - type Error = ProxyConfigError; - - fn try_from(config: ApiConnectionMode) -> Result<Self, Self::Error> { +impl From<ApiConnectionMode> for InnerConnectionMode { + fn from(config: ApiConnectionMode) -> Self { use std::net::Ipv4Addr; - Ok(match config { + match config { ApiConnectionMode::Direct => InnerConnectionMode::Direct, ApiConnectionMode::Proxied(proxy_settings) => match proxy_settings { ProxyConfig::Shadowsocks(config) => { InnerConnectionMode::Shadowsocks(ShadowsocksConfig { - params: ParsedShadowsocksConfig { - peer: config.endpoint, - password: config.password, - cipher: CipherKind::from_str(&config.cipher) - .map_err(|_| ProxyConfigError::InvalidCipher(config.cipher))?, - }, + params: config, proxy_context: SsContext::new_shared(ServerType::Local), }) } @@ -295,7 +268,7 @@ impl TryFrom<ApiConnectionMode> for InnerConnectionMode { InnerConnectionMode::EncryptedDnsProxy(config) } }, - }) + } } } @@ -341,19 +314,7 @@ impl HttpsConnector { let mut inner = inner_copy.lock().unwrap(); if let HttpsConnectorRequest::SetConnectionMode(config) = request { - match InnerConnectionMode::try_from(config) { - Ok(config) => { - inner.proxy_config = config; - } - Err(error) => { - log::error!( - "{}", - error.display_chain_with_msg( - "Failed to parse new API proxy config" - ) - ); - } - } + inner.proxy_config = InnerConnectionMode::from(config); } std::mem::take(&mut inner.stream_handles) @@ -515,36 +476,30 @@ impl Service<Uri> for HttpsConnector { #[cfg(test)] mod tests { - use super::*; - use crate::proxy::ProxyConfig; + use crate::proxy::{ApiShadowsocksConfig, ProxyConfigError}; use std::net::{Ipv4Addr, SocketAddr, SocketAddrV4}; use talpid_types::net::proxy::Shadowsocks; - fn shadowsocks_mode(cipher: &str) -> ApiConnectionMode { - ApiConnectionMode::Proxied(ProxyConfig::Shadowsocks(Shadowsocks { + fn make_shadowsocks(cipher: &str) -> Shadowsocks { + Shadowsocks { endpoint: SocketAddr::V4(SocketAddrV4::new(Ipv4Addr::LOCALHOST, 1080)), password: "pass".to_owned(), cipher: cipher.to_owned(), - })) + } } #[test] - fn validate_valid_cipher() { - assert!(validate_connection_mode(&shadowsocks_mode("aes-256-gcm")).is_ok()); + fn parse_valid_cipher() { + assert!(ApiShadowsocksConfig::try_from(make_shadowsocks("aes-256-gcm")).is_ok()); } #[test] - fn validate_invalid_cipher() { - let err = validate_connection_mode(&shadowsocks_mode("xchacha20-ietf-poly1305")) + fn parse_invalid_cipher() { + let err = ApiShadowsocksConfig::try_from(make_shadowsocks("xchacha20-ietf-poly1305")) .unwrap_err(); assert!( matches!(err, ProxyConfigError::InvalidCipher(_)), "unexpected error variant: {err}" ); } - - #[test] - fn validate_direct_mode() { - assert!(validate_connection_mode(&ApiConnectionMode::Direct).is_ok()); - } } diff --git a/mullvad-api/src/lib.rs b/mullvad-api/src/lib.rs index e66ead63ab..c3feee218f 100644 --- a/mullvad-api/src/lib.rs +++ b/mullvad-api/src/lib.rs @@ -19,7 +19,6 @@ pub mod version; mod abortable_stream; pub mod access_mode; mod https_client; -pub use https_client::{ProxyConfigError, validate_connection_mode}; pub mod proxy; mod tls_stream; #[cfg(target_os = "android")] diff --git a/mullvad-api/src/proxy.rs b/mullvad-api/src/proxy.rs index e9b686cb00..b3a870c1c8 100644 --- a/mullvad-api/src/proxy.rs +++ b/mullvad-api/src/proxy.rs @@ -1,10 +1,12 @@ use hyper_util::client::legacy::connect::{Connected, Connection}; use serde::{Deserialize, Serialize}; +use shadowsocks::crypto::CipherKind; use std::{ fmt, io, net::SocketAddr, path::Path, pin::Pin, + str::FromStr, task::{self, Poll}, }; use talpid_types::{ @@ -70,9 +72,62 @@ impl fmt::Display for ApiConnectionMode { } } +#[derive(thiserror::Error, Debug)] +pub enum ProxyConfigError { + #[error("Unrecognized cipher selected: {0}")] + InvalidCipher(String), +} + +/// Shadowsocks proxy configuration with a parsed (validated) cipher. +#[derive(Clone, Debug, PartialEq)] +pub struct ApiShadowsocksConfig { + pub endpoint: SocketAddr, + pub password: String, + pub cipher: CipherKind, +} + +impl Serialize for ApiShadowsocksConfig { + fn serialize<S: serde::Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> { + // Delegate to the talpid-types Shadowsocks for on-disk format compatibility + let raw = proxy::Shadowsocks { + endpoint: self.endpoint, + password: self.password.clone(), + cipher: self.cipher.to_string(), + }; + raw.serialize(serializer) + } +} + +impl<'de> Deserialize<'de> for ApiShadowsocksConfig { + fn deserialize<D: serde::Deserializer<'de>>(deserializer: D) -> Result<Self, D::Error> { + let raw = proxy::Shadowsocks::deserialize(deserializer)?; + let cipher = CipherKind::from_str(&raw.cipher) + .map_err(|_| serde::de::Error::custom(format!("invalid cipher: {}", raw.cipher)))?; + Ok(ApiShadowsocksConfig { + endpoint: raw.endpoint, + password: raw.password, + cipher, + }) + } +} + +impl TryFrom<proxy::Shadowsocks> for ApiShadowsocksConfig { + type Error = ProxyConfigError; + + fn try_from(value: proxy::Shadowsocks) -> Result<Self, Self::Error> { + let cipher = CipherKind::from_str(&value.cipher) + .map_err(|_| ProxyConfigError::InvalidCipher(value.cipher))?; + Ok(ApiShadowsocksConfig { + endpoint: value.endpoint, + password: value.password, + cipher, + }) + } +} + #[derive(Clone, Debug, PartialEq, Deserialize, Serialize)] pub enum ProxyConfig { - Shadowsocks(proxy::Shadowsocks), + Shadowsocks(ApiShadowsocksConfig), Socks5Local(proxy::Socks5Local), Socks5Remote(proxy::Socks5Remote), EncryptedDnsProxy(mullvad_encrypted_dns_proxy::config::ProxyConfig), @@ -97,13 +152,17 @@ impl ProxyConfig { } } -impl From<proxy::CustomProxy> for ProxyConfig { - fn from(value: proxy::CustomProxy) -> Self { - match value { - proxy::CustomProxy::Shadowsocks(shadowsocks) => ProxyConfig::Shadowsocks(shadowsocks), +impl TryFrom<proxy::CustomProxy> for ProxyConfig { + type Error = ProxyConfigError; + + fn try_from(value: proxy::CustomProxy) -> Result<Self, Self::Error> { + Ok(match value { + proxy::CustomProxy::Shadowsocks(shadowsocks) => { + ProxyConfig::Shadowsocks(ApiShadowsocksConfig::try_from(shadowsocks)?) + } proxy::CustomProxy::Socks5Local(socks) => ProxyConfig::Socks5Local(socks), proxy::CustomProxy::Socks5Remote(socks) => ProxyConfig::Socks5Remote(socks), - } + }) } } diff --git a/mullvad-daemon/src/access_method.rs b/mullvad-daemon/src/access_method.rs index 21a1838efd..a93d7cc2db 100644 --- a/mullvad-daemon/src/access_method.rs +++ b/mullvad-daemon/src/access_method.rs @@ -30,10 +30,8 @@ pub enum Error { fn validate_access_method(access_method: &AccessMethod) -> Result<(), crate::Error> { if let AccessMethod::Custom(proxy) = access_method { - use mullvad_api::proxy::{ApiConnectionMode, ProxyConfig}; - let mode = ApiConnectionMode::Proxied(ProxyConfig::from(proxy.clone())); - mullvad_api::validate_connection_mode(&mode) - .map_err(|e| crate::Error::InvalidAccessMethod(e.to_string()))?; + use mullvad_api::proxy::ProxyConfig; + ProxyConfig::try_from(proxy.clone()).map_err(crate::Error::InvalidAccessMethod)?; } Ok(()) } diff --git a/mullvad-daemon/src/api.rs b/mullvad-daemon/src/api.rs index 4f37315196..6513c28c88 100644 --- a/mullvad-daemon/src/api.rs +++ b/mullvad-daemon/src/api.rs @@ -59,7 +59,13 @@ impl AccessMethodResolver for DaemonAccessMethodResolver { return None; }; let proxy = CustomProxy::Shadowsocks(bridge); - ApiConnectionMode::Proxied(ProxyConfig::from(proxy)) + match ProxyConfig::try_from(proxy) { + Ok(config) => ApiConnectionMode::Proxied(config), + Err(err) => { + log::error!("Bridge has invalid proxy config: {err}"); + return None; + } + } } AccessMethod::BuiltIn(BuiltInAccessMethod::EncryptedDnsProxy) => { if let Err(error) = self @@ -76,9 +82,13 @@ impl AccessMethodResolver for DaemonAccessMethodResolver { }; ApiConnectionMode::Proxied(ProxyConfig::from(edp)) } - AccessMethod::Custom(config) => { - ApiConnectionMode::Proxied(ProxyConfig::from(config.clone())) - } + AccessMethod::Custom(config) => match ProxyConfig::try_from(config.clone()) { + Ok(config) => ApiConnectionMode::Proxied(config), + Err(err) => { + log::error!("Custom access method has invalid config: {err}"); + return None; + } + }, } }; let endpoint = diff --git a/mullvad-daemon/src/lib.rs b/mullvad-daemon/src/lib.rs index e898c963a9..358693a622 100644 --- a/mullvad-daemon/src/lib.rs +++ b/mullvad-daemon/src/lib.rs @@ -222,8 +222,8 @@ pub enum Error { #[error("API connection mode error")] ApiConnectionModeError(#[source] mullvad_api::access_mode::Error), - #[error("Access method has an invalid configuration: {0}")] - InvalidAccessMethod(String), + #[error("Access method has an invalid configuration")] + InvalidAccessMethod(#[source] mullvad_api::proxy::ProxyConfigError), #[error("No custom bridge has been specified")] NoCustomProxySaved, @@ -3131,18 +3131,18 @@ impl Daemon { use mullvad_api::proxy::{ApiConnectionMode, ProxyConfig}; use talpid_types::net::AllowedEndpoint; - let connection_mode = ApiConnectionMode::Proxied(ProxyConfig::from(proxy.clone())); - - // Validate eagerly (e.g. unsupported Shadowsocks cipher) - if let Err(err) = mullvad_api::validate_connection_mode(&connection_mode) { - log::warn!("Rejecting access method test, invalid config: {err}"); - Self::oneshot_send( - tx, - Err(Error::InvalidAccessMethod(err.to_string())), - "on_test_proxy_as_access_method response", - ); - return; - } + let connection_mode = match ProxyConfig::try_from(proxy.clone()) { + Ok(config) => ApiConnectionMode::Proxied(config), + Err(err) => { + log::warn!("Rejecting access method test, invalid config: {err}"); + Self::oneshot_send( + tx, + Err(Error::InvalidAccessMethod(err)), + "on_test_proxy_as_access_method response", + ); + return; + } + }; let api_proxy = self.create_limited_api_proxy(connection_mode.clone()); let proxy_endpoint = AllowedEndpoint { diff --git a/talpid-types/src/net/proxy.rs b/talpid-types/src/net/proxy.rs index 59f1111040..c73db5a355 100644 --- a/talpid-types/src/net/proxy.rs +++ b/talpid-types/src/net/proxy.rs @@ -88,8 +88,7 @@ pub struct Shadowsocks { pub endpoint: SocketAddr, pub password: String, /// One of [`SHADOWSOCKS_CIPHERS`]. - /// Validated by the daemon at add/update time and before use. Must be a cipher name - /// recognised by the `shadowsocks` crate (see `shadowsocks::crypto::available_ciphers`). + /// Gets validated at a later stage. Is assumed to be valid. pub cipher: String, } |