Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clarify naming of variables in Socks5Local datastructures #5368

Merged
merged 2 commits into from
Nov 6, 2023
Merged

Clarify naming of variables in Socks5Local datastructures #5368

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7c34fa4
Prefix remote `ip` and `port` with `remote_` in Socks5Local contexts
MarkusPettersson98 Oct 26, 2023
d1ac9a5
Remove `from_args` constructors for `access_methods` datatypes
MarkusPettersson98 Oct 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion mullvad-api/src/https_client_with_sni.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,10 @@ impl TryFrom<ApiConnectionMode> for InnerConnectionMode {
ProxyConfig::Socks(config) => match config {
access_method::Socks5::Local(config) => {
InnerConnectionMode::Socks5(SocksConfig {
peer: SocketAddr::new(IpAddr::from(Ipv4Addr::LOCALHOST), config.port),
peer: SocketAddr::new(
IpAddr::from(Ipv4Addr::LOCALHOST),
config.local_port,
),
authentication: SocksAuth::None,
})
}
Expand Down
8 changes: 6 additions & 2 deletions mullvad-api/src/proxy.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ impl ProxyConfig {
match self {
ProxyConfig::Shadowsocks(ss) => ss.peer,
ProxyConfig::Socks(socks) => match socks {
access_method::Socks5::Local(s) => s.peer,
access_method::Socks5::Local(s) => s.remote_peer,
access_method::Socks5::Remote(s) => s.peer,
},
}
Expand All @@ -60,7 +60,11 @@ impl fmt::Display for ProxyConfig {
ProxyConfig::Shadowsocks(ss) => write!(f, "Shadowsocks {}/TCP", ss.peer),
ProxyConfig::Socks(socks) => match socks {
access_method::Socks5::Local(s) => {
write!(f, "Socks5 {}/TCP via localhost:{}", s.peer, s.port)
write!(
f,
"Socks5 {}/TCP via localhost:{}",
s.remote_peer, s.local_port
)
}
access_method::Socks5::Remote(s) => write!(f, "Socks5 {}/TCP", s.peer),
},
Expand Down
109 changes: 41 additions & 68 deletions mullvad-cli/src/cmds/api_access.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ impl ApiAccess {
let mut rpc = MullvadProxyClient::new().await?;
let name = cmd.name().to_string();
let enabled = cmd.enabled();
let access_method = AccessMethod::try_from(cmd)?;
let access_method = AccessMethod::from(cmd);
rpc.add_access_method(name, enabled, access_method).await?;
Ok(())
}
Expand All @@ -99,6 +99,9 @@ impl ApiAccess {

/// Edit the data of an API access method.
async fn edit(cmd: EditCustomCommands) -> Result<()> {
use mullvad_types::access_method::{
Shadowsocks, Socks5, Socks5Local, Socks5Remote, SocksAuth,
};
let mut rpc = MullvadProxyClient::new().await?;
let mut api_access_method = Self::get_access_method(&mut rpc, &cmd.item).await?;

Expand All @@ -107,38 +110,31 @@ impl ApiAccess {
None => return Err(anyhow!("Can not edit built-in access method")),
Some(x) => match x.clone() {
CustomAccessMethod::Shadowsocks(shadowsocks) => {
let ip = cmd.params.ip.unwrap_or(shadowsocks.peer.ip()).to_string();
let ip = cmd.params.ip.unwrap_or(shadowsocks.peer.ip());
let port = cmd.params.port.unwrap_or(shadowsocks.peer.port());
let password = cmd.params.password.unwrap_or(shadowsocks.password);
let cipher = cmd.params.cipher.unwrap_or(shadowsocks.cipher);
mullvad_types::access_method::Shadowsocks::from_args(ip, port, cipher, password)
.map(AccessMethod::from)
AccessMethod::from(Shadowsocks::new((ip, port), cipher, password))
}
CustomAccessMethod::Socks5(socks) => match socks {
mullvad_types::access_method::Socks5::Local(local) => {
let ip = cmd.params.ip.unwrap_or(local.peer.ip()).to_string();
let port = cmd.params.port.unwrap_or(local.peer.port());
let local_port = cmd.params.local_port.unwrap_or(local.port);
mullvad_types::access_method::Socks5Local::from_args(ip, port, local_port)
.map(AccessMethod::from)
Socks5::Local(local) => {
let remote_ip = cmd.params.ip.unwrap_or(local.remote_peer.ip());
let remote_port = cmd.params.port.unwrap_or(local.remote_peer.port());
let local_port = cmd.params.local_port.unwrap_or(local.local_port);
AccessMethod::from(Socks5Local::new((remote_ip, remote_port), local_port))
}
mullvad_types::access_method::Socks5::Remote(remote) => {
let ip = cmd.params.ip.unwrap_or(remote.peer.ip()).to_string();
Socks5::Remote(remote) => {
let ip = cmd.params.ip.unwrap_or(remote.peer.ip());
let port = cmd.params.port.unwrap_or(remote.peer.port());
match remote.authentication {
None => mullvad_types::access_method::Socks5Remote::from_args(ip, port),
Some(mullvad_types::access_method::SocksAuth {
username,
password,
}) => {
AccessMethod::from(match remote.authentication {
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);
mullvad_types::access_method::Socks5Remote::from_args_with_password(
ip, port, username, password,
)
let auth = SocksAuth { username, password };
Socks5Remote::new_with_authentication((ip, port), auth)
}
}
.map(AccessMethod::from)
})
}
},
},
Expand All @@ -147,9 +143,7 @@ impl ApiAccess {
if let Some(name) = cmd.params.name {
api_access_method.name = name;
};
if let Some(access_method) = access_method {
api_access_method.access_method = access_method;
}
api_access_method.access_method = access_method;

rpc.update_access_method(api_access_method).await?;

Expand Down Expand Up @@ -223,9 +217,8 @@ impl ApiAccess {
rpc.set_access_method(previous_access_method.get_id())
.await?;
return Err(anyhow!(
"Could not reach the Mullvad API using access method \"{}\". Rolling back to \"{}\"",
"Could not reach the Mullvad API using access method \"{}\"",
new_access_method.get_name(),
previous_access_method.get_name()
));
}
};
Expand Down Expand Up @@ -412,16 +405,12 @@ 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 anyhow::{anyhow, Error};
use mullvad_types::access_method as daemon_types;

use super::{AddCustomCommands, AddSocks5Commands, SocksAuthentication};
use mullvad_types::access_method as daemon_types;

impl TryFrom<AddCustomCommands> for daemon_types::AccessMethod {
type Error = Error;

fn try_from(value: AddCustomCommands) -> Result<Self, Self::Error> {
Ok(match value {
impl From<AddCustomCommands> for daemon_types::AccessMethod {
fn from(value: AddCustomCommands) -> Self {
match value {
AddCustomCommands::Socks5(socks) => match socks {
AddSocks5Commands::Local {
local_port,
Expand All @@ -431,44 +420,31 @@ mod conversions {
disabled: _,
} => {
println!("Adding SOCKS5-proxy: localhost:{local_port} => {remote_ip}:{remote_port}");
daemon_types::Socks5Local::from_args(
remote_ip.to_string(),
remote_port,
local_port,
)
.map(daemon_types::Socks5::Local)
.map(daemon_types::AccessMethod::from)
.ok_or(anyhow!("Could not create a local Socks5 access method"))?
daemon_types::Socks5Local::new((remote_ip, remote_port), local_port).into()
}
AddSocks5Commands::Remote {
remote_ip,
remote_port,
authentication,
name: _,
disabled: _,
} => {
} => daemon_types::AccessMethod::from(daemon_types::Socks5::Remote(
match authentication {
Some(SocksAuthentication { username, password }) => {
println!("Adding SOCKS5-proxy: {username}:{password}@{remote_ip}:{remote_port}");
daemon_types::Socks5Remote::from_args_with_password(
remote_ip.to_string(),
remote_port,
username,
password
let auth =
mullvad_types::access_method::SocksAuth { username, password };
daemon_types::Socks5Remote::new_with_authentication(
(remote_ip, remote_port),
auth,
)
}
None => {
println!("Adding SOCKS5-proxy: {remote_ip}:{remote_port}");
daemon_types::Socks5Remote::from_args(
remote_ip.to_string(),
remote_port,
)
daemon_types::Socks5Remote::new((remote_ip, remote_port))
}
}
.map(daemon_types::Socks5::Remote)
.map(daemon_types::AccessMethod::from)
.ok_or(anyhow!("Could not create a remote Socks5 access method"))?
}
},
)),
},
AddCustomCommands::Shadowsocks {
remote_ip,
Expand All @@ -481,16 +457,13 @@ mod conversions {
println!(
"Adding Shadowsocks-proxy: {password} @ {remote_ip}:{remote_port} using {cipher}"
);
daemon_types::Shadowsocks::from_args(
remote_ip.to_string(),
remote_port,
daemon_types::AccessMethod::from(daemon_types::Shadowsocks::new(
(remote_ip, remote_port),
cipher,
password,
)
.map(daemon_types::AccessMethod::from)
.ok_or(anyhow!("Could not create a Shadowsocks access method"))?
))
}
})
}
}
}
}
Expand Down Expand Up @@ -586,8 +559,8 @@ mod pp {
}
writeln!(f)?;
print_option!("Protocol", "Socks5 (local)");
print_option!("Peer", local.peer);
print_option!("Local port", local.port);
print_option!("Peer", local.remote_peer);
print_option!("Local port", local.local_port);
Ok(())
}
},
Expand Down
4 changes: 2 additions & 2 deletions mullvad-management-interface/proto/management_interface.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -333,8 +333,8 @@ message AccessMethod {
message Direct {}
message Bridges {}
message Socks5Local {
string ip = 1;
uint32 port = 2;
string remote_ip = 1;
uint32 remote_port = 2;
uint32 local_port = 3;
}
message SocksAuth {
Expand Down
69 changes: 41 additions & 28 deletions mullvad-management-interface/src/types/conversions/access_method.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ mod settings {
/// [`crate::types::proto::ApiAccessMethod`] type to the internal
/// [`mullvad_types::access_method::AccessMethodSetting`] data type.
mod data {
use std::net::Ipv4Addr;

use crate::types::{proto, FromProtobufTypeError};
use mullvad_types::access_method::{
AccessMethod, AccessMethodSetting, BuiltInAccessMethod, CustomAccessMethod, Id,
Expand Down Expand Up @@ -142,11 +144,15 @@ mod data {
type Error = FromProtobufTypeError;

fn try_from(value: proto::access_method::Socks5Local) -> Result<Self, Self::Error> {
Socks5Local::from_args(value.ip, value.port as u16, value.local_port as u16)
.ok_or(FromProtobufTypeError::InvalidArgument(
let remote_ip = value.remote_ip.parse::<Ipv4Addr>().map_err(|_| {
FromProtobufTypeError::InvalidArgument(
"Could not parse Socks5 (local) message from protobuf",
))
.map(AccessMethod::from)
)
})?;
Ok(AccessMethod::from(Socks5Local::new(
(remote_ip, value.remote_port as u16),
value.local_port as u16,
)))
}
}

Expand All @@ -159,31 +165,37 @@ mod data {
port,
authentication,
} = value;
let port = port as u16;
match authentication.map(SocksAuth::from) {
Some(SocksAuth { username, password }) => {
Socks5Remote::from_args_with_password(ip, port, username, password)
}
None => Socks5Remote::from_args(ip, port),
}
.ok_or({
let ip = ip.parse::<Ipv4Addr>().map_err(|_| {
FromProtobufTypeError::InvalidArgument(
"Could not parse Socks5 (remote) message from protobuf",
)
})
.map(AccessMethod::from)
})?;
let port = port as u16;

Ok(AccessMethod::from(
match authentication.map(SocksAuth::from) {
Some(auth) => Socks5Remote::new_with_authentication((ip, port), auth),
None => Socks5Remote::new((ip, port)),
},
))
}
}

impl TryFrom<proto::access_method::Shadowsocks> for AccessMethod {
type Error = FromProtobufTypeError;

fn try_from(value: proto::access_method::Shadowsocks) -> Result<Self, Self::Error> {
Shadowsocks::from_args(value.ip, value.port as u16, value.cipher, value.password)
.ok_or(FromProtobufTypeError::InvalidArgument(
"Could not parse Shadowsocks message from protobuf",
))
.map(AccessMethod::from)
let ip = value.ip.parse::<Ipv4Addr>().map_err(|_| {
FromProtobufTypeError::InvalidArgument(
"Could not parse Socks5 (remote) message from protobuf",
)
})?;

Ok(AccessMethod::from(Shadowsocks::new(
(ip, value.port as u16),
value.cipher,
value.password,
)))
}
}

Expand Down Expand Up @@ -216,15 +228,16 @@ mod data {
},
)
}
CustomAccessMethod::Socks5(Socks5::Local(Socks5Local { peer, port })) => {
proto::access_method::AccessMethod::Socks5local(
proto::access_method::Socks5Local {
ip: peer.ip().to_string(),
port: peer.port() as u32,
local_port: port as u32,
},
)
}
CustomAccessMethod::Socks5(Socks5::Local(Socks5Local {
remote_peer: peer,
local_port: port,
})) => proto::access_method::AccessMethod::Socks5local(
proto::access_method::Socks5Local {
remote_ip: peer.ip().to_string(),
remote_port: peer.port() as u32,
local_port: port as u32,
},
),
CustomAccessMethod::Socks5(Socks5::Remote(Socks5Remote {
peer,
authentication,
Expand Down
Loading
Loading