Validate SOCKS5 credentials

Validate SOCKS credentials by checking that both `username` and
`password` both have a length between 1 and 255 bytes.

Link to RFC detailing SOCKS5 username/password authentication:
https://datatracker.ietf.org/doc/html/rfc1929

3 files changed, 70 insertions, 85 deletions

diff --git a/mullvad-cli/src/cmds/api_access.rs b/mullvad-cli/src/cmds/api_access.rs
index 8883ea516b..d24d5a0674 100644
--- a/mullvad-cli/src/cmds/api_access.rs
+++ b/mullvad-cli/src/cmds/api_access.rs
@@ -86,7 +86,7 @@ impl ApiAccess {
         let mut rpc = MullvadProxyClient::new().await?;
         let name = cmd.name().to_string();
         let enabled = cmd.enabled();
-        let access_method = AccessMethod::from(cmd);
+        let access_method = AccessMethod::try_from(cmd)?;
         rpc.add_access_method(name, enabled, access_method).await?;
         Ok(())
     }
@@ -139,10 +139,16 @@ impl ApiAccess {
                     let port = cmd.params.port.unwrap_or(remote.endpoint.port());
                     AccessMethod::from(match remote.auth {
                         None => Socks5Remote::new((ip, port)),
-                        Some(SocksAuth { username, password }) => {
-                            let username = cmd.params.username.unwrap_or(username);
-                            let password = cmd.params.password.unwrap_or(password);
-                            let auth = SocksAuth { username, password };
+                        Some(credentials) => {
+                            let username = cmd
+                                .params
+                                .username
+                                .unwrap_or(credentials.username().to_string());
+                            let password = cmd
+                                .params
+                                .password
+                                .unwrap_or(credentials.password().to_string());
+                            let auth = SocksAuth::new(username, password)?;
                             Socks5Remote::new_with_authentication((ip, port), auth)
                         }
                     })
@@ -361,73 +367,45 @@ pub struct EditParams {
 /// Since these are not supposed to be used outside of the CLI,
 /// we define them in a hidden-away module.
 mod conversions {
-    use crate::cmds::proxies::SocksAuthentication;
-
     use super::{AddCustomCommands, AddSocks5Commands};
+    use crate::cmds::proxies::{Error, SocksAuthentication};
     use mullvad_types::access_method as daemon_types;
     use talpid_types::net::proxy as talpid_types;
 
-    impl From<AddCustomCommands> for daemon_types::AccessMethod {
-        fn from(value: AddCustomCommands) -> Self {
+    impl TryFrom<AddCustomCommands> for daemon_types::AccessMethod {
+        type Error = Error;
+        fn try_from(value: AddCustomCommands) -> Result<Self, Self::Error> {
             match value {
                 AddCustomCommands::Socks5(socks) => match socks {
-                    AddSocks5Commands::Local {
-                        name: _,
-                        disabled: _,
-                        add,
-                    } => {
-                        let (local_port, remote_ip, remote_port, transport_protocol) = (
+                    AddSocks5Commands::Local { add, .. } => Ok(daemon_types::AccessMethod::from(
+                        talpid_types::Socks5Local::new_with_transport_protocol(
+                            (add.remote_ip, add.remote_port),
                             add.local_port,
-                            add.remote_ip,
-                            add.remote_port,
                             add.transport_protocol,
-                        );
-                        println!("Adding SOCKS5-proxy: localhost:{local_port} => {remote_ip}:{remote_port}/{transport_protocol}");
-                        talpid_types::Socks5Local::new_with_transport_protocol(
-                            (remote_ip, remote_port),
-                            local_port,
-                            transport_protocol,
-                        )
-                        .into()
+                        ),
+                    )),
+                    AddSocks5Commands::Remote { add, .. } => {
+                        Ok(daemon_types::AccessMethod::from(match add.authentication {
+                            Some(SocksAuthentication { username, password }) => {
+                                let auth = talpid_types::SocksAuth::new(username, password)?;
+                                talpid_types::Socks5Remote::new_with_authentication(
+                                    (add.remote_ip, add.remote_port),
+                                    auth,
+                                )
+                            }
+                            None => {
+                                talpid_types::Socks5Remote::new((add.remote_ip, add.remote_port))
+                            }
+                        }))
                     }
-                    AddSocks5Commands::Remote {
-                        add,
-                        name: _,
-                        disabled: _,
-                    } => daemon_types::AccessMethod::from(match add.authentication {
-                        Some(SocksAuthentication { username, password }) => {
-                            println!(
-                                "Adding SOCKS5-proxy: {username}:{password}@{}:{}",
-                                add.remote_ip, add.remote_port
-                            );
-                            let auth = talpid_types::SocksAuth { username, password };
-                            talpid_types::Socks5Remote::new_with_authentication(
-                                (add.remote_ip, add.remote_port),
-                                auth,
-                            )
-                        }
-                        None => {
-                            println!("Adding SOCKS5-proxy: {}:{}", add.remote_ip, add.remote_port);
-                            talpid_types::Socks5Remote::new((add.remote_ip, add.remote_port))
-                        }
-                    }),
                 },
-                AddCustomCommands::Shadowsocks {
-                    add,
-                    name: _,
-                    disabled: _,
-                } => {
-                    let (password, cipher, remote_ip, remote_port) =
-                        (add.password, add.cipher, add.remote_ip, add.remote_port);
-                    println!(
-                "Adding Shadowsocks-proxy: {password} @ {remote_ip}:{remote_port} using {cipher}"
-                    );
-                    daemon_types::AccessMethod::from(talpid_types::Shadowsocks::new(
-                        (remote_ip, remote_port),
-                        cipher,
-                        password,
-                    ))
-                }
+                AddCustomCommands::Shadowsocks { add, .. } => Ok(daemon_types::AccessMethod::from(
+                    talpid_types::Shadowsocks::new(
+                        (add.remote_ip, add.remote_port),
+                        add.cipher,
+                        add.password,
+                    ),
+                )),
             }
         }
     }
diff --git a/mullvad-cli/src/cmds/bridge.rs b/mullvad-cli/src/cmds/bridge.rs
index 329921868b..861d73feb2 100644
--- a/mullvad-cli/src/cmds/bridge.rs
+++ b/mullvad-cli/src/cmds/bridge.rs
@@ -303,7 +303,7 @@ impl Bridge {
         match custom_bridge {
             CustomProxy::Shadowsocks(ss) => *ss = edit.merge_shadowsocks(ss),
             CustomProxy::Socks5Local(local) => *local = edit.merge_socks_local(local),
-            CustomProxy::Socks5Remote(remote) => *remote = edit.merge_socks_remote(remote),
+            CustomProxy::Socks5Remote(remote) => *remote = edit.merge_socks_remote(remote)?,
         };
 
         rpc.set_bridge_settings(settings.bridge_settings)
@@ -343,7 +343,7 @@ impl Bridge {
                 CustomProxy::Socks5Local(Socks5Local::from(add))
             }
             AddCustomCommands::Socks5(AddSocks5Commands::Remote { add }) => {
-                CustomProxy::Socks5Remote(Socks5Remote::from(add))
+                CustomProxy::Socks5Remote(Socks5Remote::try_from(add)?)
             }
             AddCustomCommands::Shadowsocks { add } => {
                 CustomProxy::Shadowsocks(Shadowsocks::from(add))
diff --git a/mullvad-cli/src/cmds/proxies.rs b/mullvad-cli/src/cmds/proxies.rs
index 6e32836a25..5c8a79ff6c 100644
--- a/mullvad-cli/src/cmds/proxies.rs
+++ b/mullvad-cli/src/cmds/proxies.rs
@@ -1,10 +1,15 @@
+use clap::Args;
 use std::net::{IpAddr, SocketAddr};
 use talpid_types::net::{
     proxy::{Shadowsocks, Socks5Local, Socks5Remote, SocksAuth, SHADOWSOCKS_CIPHERS},
     Endpoint, TransportProtocol,
 };
 
-use clap::Args;
+#[derive(err_derive::Error, Debug)]
+pub enum Error {
+    #[error(display = "{}", _0)]
+    InvalidAuth(#[error(source)] talpid_types::net::proxy::Error),
+}
 
 #[derive(Args, Debug, Clone)]
 pub struct Socks5LocalAdd {
@@ -48,15 +53,16 @@ pub struct Socks5RemoteAdd {
     pub authentication: Option<SocksAuthentication>,
 }
 
-impl From<Socks5RemoteAdd> for Socks5Remote {
-    fn from(add: Socks5RemoteAdd) -> Self {
-        Self {
+impl TryFrom<Socks5RemoteAdd> for Socks5Remote {
+    type Error = Error;
+    fn try_from(add: Socks5RemoteAdd) -> Result<Self, Self::Error> {
+        Ok(Self {
             endpoint: SocketAddr::new(add.remote_ip, add.remote_port),
-            auth: add.authentication.map(|auth| SocksAuth {
-                username: auth.username,
-                password: auth.password,
-            }),
-        }
+            auth: add
+                .authentication
+                .map(|auth| SocksAuth::new(auth.username, auth.password))
+                .transpose()?,
+        })
     }
 }
 
@@ -134,13 +140,13 @@ impl ProxyEditParams {
         )
     }
 
-    pub fn merge_socks_remote(self, remote: &Socks5Remote) -> Socks5Remote {
+    pub fn merge_socks_remote(self, remote: &Socks5Remote) -> Result<Socks5Remote, Error> {
         let ip = self.ip.unwrap_or(remote.endpoint.ip());
         let port = self.port.unwrap_or(remote.endpoint.port());
-        match &remote.auth {
+        let config = match &remote.auth {
             None => match (self.username, self.password) {
                 (Some(username), Some(password)) => {
-                    let auth = SocksAuth { username, password };
+                    let auth = SocksAuth::new(username, password)?;
                     Socks5Remote::new_with_authentication((ip, port), auth)
                 }
                 (None, None) => Socks5Remote::new((ip, port)),
@@ -149,13 +155,14 @@ impl ProxyEditParams {
                     Socks5Remote::new((ip, port))
                 }
             },
-            Some(SocksAuth { username, password }) => {
-                let username = self.username.unwrap_or(username.to_owned());
-                let password = self.password.unwrap_or(password.to_owned());
-                let auth = SocksAuth { username, password };
+            Some(credentials) => {
+                let username = self.username.unwrap_or(credentials.username().to_string());
+                let password = self.password.unwrap_or(credentials.password().to_string());
+                let auth = SocksAuth::new(username, password)?;
                 Socks5Remote::new_with_authentication((ip, port), auth)
             }
-        }
+        };
+        Ok(config)
     }
 
     pub fn merge_shadowsocks(self, shadowsocks: &Shadowsocks) -> Shadowsocks {
@@ -169,7 +176,7 @@ impl ProxyEditParams {
 
 pub mod pp {
     use crate::print_option;
-    use talpid_types::net::proxy::{CustomProxy, SocksAuth};
+    use talpid_types::net::proxy::CustomProxy;
 
     pub struct CustomProxyFormatter<'a> {
         pub custom_proxy: &'a CustomProxy,
@@ -188,9 +195,9 @@ pub mod pp {
                     print_option!("Protocol", "Socks5");
                     print_option!("Peer", remote.endpoint);
                     match &remote.auth {
-                        Some(SocksAuth { username, password }) => {
-                            print_option!("Username", username);
-                            print_option!("Password", password);
+                        Some(credentials) => {
+                            print_option!("Username", credentials.username());
+                            print_option!("Password", credentials.password());
                         }
                         None => (),
                     }