diff options
| -rw-r--r-- | .github/workflows/clippy.yml | 2 | ||||
| -rw-r--r-- | talpid-core/src/dns/macos.rs | 28 | ||||
| -rw-r--r-- | talpid-core/src/firewall/macos.rs | 78 | ||||
| -rw-r--r-- | talpid-core/src/resolver.rs | 22 | ||||
| -rw-r--r-- | talpid-core/src/tunnel_state_machine/disconnected_state.rs | 12 | ||||
| -rw-r--r-- | talpid-routing/src/macos.rs | 2 | ||||
| -rw-r--r-- | talpid-types/src/tunnel.rs | 5 | ||||
| -rw-r--r-- | talpid-wireguard/src/lib.rs | 2 |
8 files changed, 54 insertions, 97 deletions
diff --git a/.github/workflows/clippy.yml b/.github/workflows/clippy.yml index 2c28af450b..8d55bd2c05 100644 --- a/.github/workflows/clippy.yml +++ b/.github/workflows/clippy.yml @@ -11,7 +11,7 @@ jobs: clippy_check: strategy: matrix: - os: [ubuntu-latest, windows-latest] + os: [ubuntu-latest, windows-latest, macos-latest] runs-on: ${{ matrix.os }} steps: - name: Checkout repository diff --git a/talpid-core/src/dns/macos.rs b/talpid-core/src/dns/macos.rs index a1ec334deb..e864d51591 100644 --- a/talpid-core/src/dns/macos.rs +++ b/talpid-core/src/dns/macos.rs @@ -92,7 +92,7 @@ impl State { let new_settings = DnsSettings::from_server_addresses(&servers, interface.to_string()); match &self.dns_settings { None => { - let backup = read_all_dns(&store); + let backup = read_all_dns(store); log::trace!("Backup of DNS settings: {:#?}", backup); for service_path in backup.keys() { new_settings.save(store, service_path.as_str())?; @@ -154,12 +154,9 @@ impl State { // If we changed a "state" entry, also set the corresponding "setup" entry. if let Some(setup_path_str) = state_to_setup_path(&path.to_string()) { let setup_path = CFString::new(&setup_path_str); - if !self.backup.contains_key(&setup_path_str) { - self.backup.insert( - setup_path_str, - DnsSettings::load(&store, setup_path.clone()).ok(), - ); - } + self.backup + .entry(setup_path_str) + .or_insert_with(|| DnsSettings::load(&store, setup_path.clone()).ok()); if let Err(e) = expected_settings.save(&store, setup_path.clone()) { log::error!("Failed changing DNS for {}: {}", setup_path, e); } @@ -395,13 +392,7 @@ fn parse_sc_config( ) -> Result<Option<(String, Vec<IpAddr>)>> { config .iter() - .filter_map(|(path, maybe_config)| { - if let Some(settings) = maybe_config { - Some((path, settings)) - } else { - None - } - }) + .filter_map(|(path, maybe_config)| maybe_config.as_ref().map(|settings| (path, settings))) .map(|(path, settings)| { let addresses = settings.interface_config(path.as_str())?; Ok((settings.name.clone(), addresses)) @@ -475,12 +466,9 @@ fn read_all_dns(store: &SCDynamicStore) -> HashMap<ServicePath, Option<DnsSettin if let Some(paths) = store.get_keys(SETUP_PATH_PATTERN) { for setup_path in paths.iter() { let setup_path_str = setup_path.to_string(); - if !backup.contains_key(&setup_path_str) { - backup.insert( - setup_path_str, - DnsSettings::load(store, setup_path.clone()).ok(), - ); - } + backup + .entry(setup_path_str) + .or_insert_with(|| DnsSettings::load(store, setup_path.clone()).ok()); } } backup diff --git a/talpid-core/src/firewall/macos.rs b/talpid-core/src/firewall/macos.rs index 6716e0542d..9146a27bfe 100644 --- a/talpid-core/src/firewall/macos.rs +++ b/talpid-core/src/firewall/macos.rs @@ -14,7 +14,7 @@ type Result<T> = std::result::Result<T, Error>; /// TODO(linus): This crate is not supposed to be Mullvad-aware. So at some point this should be /// replaced by allowing the anchor name to be configured from the public API of this crate. -const ANCHOR_NAME: &'static str = "mullvad"; +const ANCHOR_NAME: &str = "mullvad"; pub struct Firewall { pf: pfctl::PfCtl, @@ -85,7 +85,7 @@ impl Firewall { let mut anchor_change = pfctl::AnchorChange::new(); anchor_change.set_filter_rules(new_filter_rules); anchor_change.set_redirect_rules(self.get_dns_redirect_rules(&policy)?); - Ok(self.pf.set_rules(ANCHOR_NAME, anchor_change)?) + self.pf.set_rules(ANCHOR_NAME, anchor_change) } fn get_dns_redirect_rules( @@ -149,7 +149,7 @@ impl Firewall { let mut rules = vec![]; for server in dns_servers.iter() { - rules.append(&mut self.get_allow_dns_rules_when_connected(&tunnel, *server)?); + rules.append(&mut self.get_allow_dns_rules_when_connected(tunnel, *server)?); } rules.push(self.get_allow_relay_rule(*peer_endpoint)?); @@ -280,8 +280,7 @@ impl Firewall { fn get_allow_relay_rule(&self, relay_endpoint: net::Endpoint) -> Result<pfctl::FilterRule> { let pfctl_proto = as_pfctl_proto(relay_endpoint.protocol); - Ok(self - .create_rule_builder(FilterRuleAction::Pass) + self.create_rule_builder(FilterRuleAction::Pass) .direction(pfctl::Direction::Out) .to(relay_endpoint.address) .proto(pfctl_proto) @@ -289,7 +288,7 @@ impl Firewall { .tcp_flags(Self::get_tcp_flags()) .user(Uid::from(super::ROOT_UID)) .quick(true) - .build()?) + .build() } /// Produces a rule that allows traffic to flow to the API. Allows the app to reach the API in @@ -300,15 +299,14 @@ impl Firewall { ) -> Result<pfctl::FilterRule> { let pfctl_proto = as_pfctl_proto(allowed_endpoint.protocol); - Ok(self - .create_rule_builder(FilterRuleAction::Pass) + self.create_rule_builder(FilterRuleAction::Pass) .direction(pfctl::Direction::Out) .to(allowed_endpoint.address) .proto(pfctl_proto) .keep_state(pfctl::StatePolicy::Keep) .user(Uid::from(super::ROOT_UID)) .quick(true) - .build()?) + .build() } fn get_block_dns_rules(&self) -> Result<Vec<pfctl::FilterRule>> { @@ -508,40 +506,29 @@ impl Firewall { .af(pfctl::AddrFamily::Ipv6) .proto(pfctl::Proto::IcmpV6); - let mut rules = Vec::new(); - - // Outgoing router solicitation to `ff02::2` - rules.push( + Ok(vec![ + // Outgoing router solicitation to `ff02::2` ndp_rule_builder .clone() .direction(pfctl::Direction::Out) .icmp_type(pfctl::IcmpType::Icmp6(pfctl::Icmp6Type::RouterSol)) .to(*super::ROUTER_SOLICITATION_OUT_DST_ADDR) .build()?, - ); - - // Incoming router advertisement from `fe80::/10` - rules.push( + // Incoming router advertisement from `fe80::/10` ndp_rule_builder .clone() .direction(pfctl::Direction::In) .icmp_type(pfctl::IcmpType::Icmp6(pfctl::Icmp6Type::RouterAdv)) .from(pfctl::Ip::from(IpNetwork::V6(*super::IPV6_LINK_LOCAL))) .build()?, - ); - - // Incoming Redirect from `fe80::/10` - rules.push( + // Incoming Redirect from `fe80::/10` ndp_rule_builder .clone() .direction(pfctl::Direction::In) .icmp_type(pfctl::IcmpType::Icmp6(pfctl::Icmp6Type::Redir)) .from(pfctl::Ip::from(IpNetwork::V6(*super::IPV6_LINK_LOCAL))) .build()?, - ); - - // Outgoing neighbor solicitation to `ff02::1:ff00:0/104` and `fe80::/10` - rules.push( + // Outgoing neighbor solicitation to `ff02::1:ff00:0/104` and `fe80::/10` ndp_rule_builder .clone() .direction(pfctl::Direction::Out) @@ -550,46 +537,33 @@ impl Firewall { *super::SOLICITED_NODE_MULTICAST, ))) .build()?, - ); - rules.push( ndp_rule_builder .clone() .direction(pfctl::Direction::Out) .icmp_type(pfctl::IcmpType::Icmp6(pfctl::Icmp6Type::NeighbrSol)) .to(pfctl::Ip::from(IpNetwork::V6(*super::IPV6_LINK_LOCAL))) .build()?, - ); - - // Incoming neighbor solicitation from `fe80::/10` - rules.push( + // Incoming neighbor solicitation from `fe80::/10` ndp_rule_builder .clone() .direction(pfctl::Direction::In) .icmp_type(pfctl::IcmpType::Icmp6(pfctl::Icmp6Type::NeighbrSol)) .from(pfctl::Ip::from(IpNetwork::V6(*super::IPV6_LINK_LOCAL))) .build()?, - ); - - // Outgoing neighbor advertisement to fe80::/10` - rules.push( + // Outgoing neighbor advertisement to fe80::/10` ndp_rule_builder .clone() .direction(pfctl::Direction::Out) .icmp_type(pfctl::IcmpType::Icmp6(pfctl::Icmp6Type::NeighbrAdv)) .to(pfctl::Ip::from(IpNetwork::V6(*super::IPV6_LINK_LOCAL))) .build()?, - ); - - // Incoming neighbor advertisement from anywhere - rules.push( + // Incoming neighbor advertisement from anywhere ndp_rule_builder .clone() .direction(pfctl::Direction::In) .icmp_type(pfctl::IcmpType::Icmp6(pfctl::Icmp6Type::NeighbrAdv)) .build()?, - ); - - Ok(rules) + ]) } fn create_rule_builder(&self, action: FilterRuleAction) -> pfctl::FilterRuleBuilder { @@ -597,14 +571,12 @@ impl Firewall { builder.action(action); let rule_log = pfctl::RuleLog::IncludeMatchingState; let do_log = match action { - FilterRuleAction::Pass => match self.rule_logging { - RuleLogging::All | RuleLogging::Pass => true, - _ => false, - }, - FilterRuleAction::Drop(..) => match self.rule_logging { - RuleLogging::All | RuleLogging::Drop => true, - _ => false, - }, + FilterRuleAction::Pass => { + matches!(self.rule_logging, RuleLogging::All | RuleLogging::Pass) + } + FilterRuleAction::Drop(..) => { + matches!(self.rule_logging, RuleLogging::All | RuleLogging::Drop) + } }; if do_log { builder.log(rule_log); @@ -630,16 +602,16 @@ impl Firewall { if self.pf_was_enabled.is_none() { self.pf_was_enabled = Some(self.is_enabled()); } - Ok(self.pf.try_enable()?) + self.pf.try_enable() } fn is_enabled(&self) -> bool { let cmd = duct::cmd!("/sbin/pfctl", "-s", "info") .stderr_null() .stdout_capture(); - const EXPECTED_OUTPUT: &'static [u8] = b"Status: Enabled"; + const EXPECTED_OUTPUT: &[u8] = b"Status: Enabled"; match cmd.run() { - Ok(output) => output.stdout.as_slice().find(&EXPECTED_OUTPUT).is_some(), + Ok(output) => output.stdout.as_slice().find(EXPECTED_OUTPUT).is_some(), Err(err) => { log::error!( "Failed to execute pfctl, assuming pf is not enabled: {}", diff --git a/talpid-core/src/resolver.rs b/talpid-core/src/resolver.rs index 8e69157aca..d2c260ddff 100644 --- a/talpid-core/src/resolver.rs +++ b/talpid-core/src/resolver.rs @@ -179,6 +179,15 @@ impl FilteringResolver { } } +type LookupResponse<'a> = MessageResponse< + 'a, + 'a, + Box<dyn Iterator<Item = &'a Record> + Send + 'a>, + std::iter::Empty<&'a Record>, + std::iter::Empty<&'a Record>, + std::iter::Empty<&'a Record>, +>; + /// An implementation of [trust_dns_server::server::RequestHandler] that forwards queries to /// `FilteringResolver`. struct ResolverImpl { @@ -189,14 +198,7 @@ impl ResolverImpl { fn build_response<'a>( message: &'a MessageRequest, lookup: &'a mut Box<dyn LookupObject>, - ) -> MessageResponse< - 'a, - 'a, - Box<dyn Iterator<Item = &'a Record> + Send + 'a>, - std::iter::Empty<&'a Record>, - std::iter::Empty<&'a Record>, - std::iter::Empty<&'a Record>, - > { + ) -> LookupResponse<'a> { let mut response_header = Header::new(); response_header.set_id(message.id()); response_header.set_op_code(OpCode::Query); @@ -215,14 +217,14 @@ impl ResolverImpl { async fn lookup<R: ResponseHandler>(&self, message: &Request, mut response_handler: R) { if let Some(tx_ref) = self.tx.upgrade() { - let mut tx = (&*tx_ref).clone(); + let mut tx = (*tx_ref).clone(); let query = message.query(); let (lookup_tx, lookup_rx) = oneshot::channel(); let _ = tx.send((query.clone(), lookup_tx)).await; let mut lookup_result: Box<dyn LookupObject> = lookup_rx .await .unwrap_or_else(|_| Box::new(EmptyLookup) as Box<dyn LookupObject>); - let response = Self::build_response(&message, &mut lookup_result); + let response = Self::build_response(message, &mut lookup_result); if let Err(err) = response_handler.send_response(response).await { log::error!("Failed to send response: {}", err); diff --git a/talpid-core/src/tunnel_state_machine/disconnected_state.rs b/talpid-core/src/tunnel_state_machine/disconnected_state.rs index 8462f56e10..92a0586245 100644 --- a/talpid-core/src/tunnel_state_machine/disconnected_state.rs +++ b/talpid-core/src/tunnel_state_machine/disconnected_state.rs @@ -100,13 +100,11 @@ impl TunnelState for DisconnectedState { err.display_chain_with_msg("Failed to start filtering resolver:") ); } - } else { - if let Err(error) = shared_values.dns_monitor.reset() { - log::error!( - "{}", - error.display_chain_with_msg("Unable to disable filtering resolver") - ); - } + } else if let Err(error) = shared_values.dns_monitor.reset() { + log::error!( + "{}", + error.display_chain_with_msg("Unable to disable filtering resolver") + ); } #[cfg(windows)] diff --git a/talpid-routing/src/macos.rs b/talpid-routing/src/macos.rs index 568daf94f7..075fb35439 100644 --- a/talpid-routing/src/macos.rs +++ b/talpid-routing/src/macos.rs @@ -250,7 +250,7 @@ impl RouteManagerImpl { cmd.status().await.map_err(Error::FailedToAddRoute) } - async fn cleanup_routes(&self) -> () { + async fn cleanup_routes(&self) { let destinations_to_remove = self .applied_routes .iter() diff --git a/talpid-types/src/tunnel.rs b/talpid-types/src/tunnel.rs index 83f4e3fc0a..3d34f4a96a 100644 --- a/talpid-types/src/tunnel.rs +++ b/talpid-types/src/tunnel.rs @@ -109,10 +109,7 @@ pub enum ErrorStateCause { impl ErrorStateCause { #[cfg(target_os = "macos")] pub fn prevents_filtering_resolver(&self) -> bool { - match self { - Self::SetDnsError => true, - _ => false, - } + matches!(self, Self::SetDnsError) } } diff --git a/talpid-wireguard/src/lib.rs b/talpid-wireguard/src/lib.rs index f28879c512..cdaaf4cfc0 100644 --- a/talpid-wireguard/src/lib.rs +++ b/talpid-wireguard/src/lib.rs @@ -178,7 +178,7 @@ async fn maybe_create_obfuscator( ) -> Result<Option<ObfuscatorHandle>> { // There are one or two peers. // The first one is always the entry relay. - let mut first_peer = config.peers.get_mut(0).expect("missing peer"); + let first_peer = config.peers.get_mut(0).expect("missing peer"); if let Some(ref obfuscator_config) = config.obfuscator_config { match obfuscator_config { |