Merge branch 'problem-report-work-without-dns'

6 files changed, 58 insertions, 39 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index a535fc7714..dc3abcec59 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -33,6 +33,7 @@ Line wrap the file at 100 chars.                                              Th
 ### Fixed
 - Pick new random relay for each reconnect attempt instead of just retrying with the same one.
 - Disable GPU acceleration on Linux to fix App on Ubuntu 14.04 and other older distributions.
+- Make the `problem-report` tool fall back to the bundled API IP if DNS resolution fails.
 
 #### macOS
 - Correctly backup and restore search domains and other DNS settings.
diff --git a/Cargo.lock b/Cargo.lock
index 2ce8627195..9e2b4b57af 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -983,6 +983,7 @@ version = "2018.4.0"
 dependencies = [
  "clap 2.32.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "dirs 1.0.2 (registry+https://github.com/rust-lang/crates.io-index)",
+ "env_logger 0.5.12 (registry+https://github.com/rust-lang/crates.io-index)",
  "error-chain 0.12.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "lazy_static 1.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "mullvad-paths 0.1.0",
diff --git a/mullvad-problem-report/Cargo.toml b/mullvad-problem-report/Cargo.toml
index b5ad1675df..307e0c79c9 100644
--- a/mullvad-problem-report/Cargo.toml
+++ b/mullvad-problem-report/Cargo.toml
@@ -19,6 +19,7 @@ path = "src/main.rs"
 [dependencies]
 clap = "2.25"
 dirs = "1.0"
+env_logger = "0.5"
 error-chain = "0.12"
 lazy_static = "1.0"
 regex = "1.0"
diff --git a/mullvad-problem-report/src/main.rs b/mullvad-problem-report/src/main.rs
index 1ffa870f4e..022d93bda1 100644
--- a/mullvad-problem-report/src/main.rs
+++ b/mullvad-problem-report/src/main.rs
@@ -11,6 +11,7 @@ extern crate clap;
 extern crate dirs;
 #[macro_use]
 extern crate error_chain;
+extern crate env_logger;
 #[macro_use]
 extern crate lazy_static;
 extern crate regex;
@@ -91,6 +92,7 @@ error_chain!{
 quick_main!(run);
 
 fn run() -> Result<()> {
+    env_logger::init();
     let app = clap::App::new("problem-report")
         .version(metadata::PRODUCT_VERSION)
         .author(crate_authors!())
@@ -159,11 +161,11 @@ fn run() -> Result<()> {
     if let Some(collect_matches) = matches.subcommand_matches("collect") {
         let redact_custom_strings = collect_matches
             .values_of_lossy("redact")
-            .unwrap_or(Vec::new());
+            .unwrap_or_else(Vec::new);
         let extra_logs = collect_matches
             .values_of_os("extra_logs")
             .map(|os_values| os_values.map(Path::new).collect())
-            .unwrap_or(Vec::new());
+            .unwrap_or_else(Vec::new);
         let output_path = Path::new(collect_matches.value_of_os("output").unwrap());
         collect_report(&extra_logs, output_path, redact_custom_strings)
     } else if let Some(send_matches) = matches.subcommand_matches("send") {
@@ -207,7 +209,7 @@ fn collect_report(
 
     problem_report.add_logs(extra_logs);
 
-    write_problem_report(&output_path, problem_report)
+    write_problem_report(&output_path, &problem_report)
         .chain_err(|| ErrorKind::WriteReportError(output_path.to_path_buf()))
 }
 
@@ -265,7 +267,7 @@ fn send_problem_report(user_email: &str, user_message: &str, report_path: &Path)
         .chain_err(|| ErrorKind::RpcError)
 }
 
-fn write_problem_report(path: &Path, problem_report: ProblemReport) -> io::Result<()> {
+fn write_problem_report(path: &Path, problem_report: &ProblemReport) -> io::Result<()> {
     let file = File::create(path)?;
     let mut permissions = file.metadata()?.permissions();
     permissions.set_readonly(true);
diff --git a/mullvad-rpc/src/cached_dns_resolver.rs b/mullvad-rpc/src/cached_dns_resolver.rs
index 67d4f6bcd6..ef3ed3a4f1 100644
--- a/mullvad-rpc/src/cached_dns_resolver.rs
+++ b/mullvad-rpc/src/cached_dns_resolver.rs
@@ -71,7 +71,6 @@ impl SystemDnsResolver {
 
 impl DnsResolver for SystemDnsResolver {
     fn resolve(&mut self, host: &str) -> Result<IpAddr> {
-        debug!("Resolving IP for {}", host);
         Self::resolve_in_background_thread(host)
             .recv_timeout(DNS_TIMEOUT)
             .chain_err(|| ErrorKind::DnsTimeout(host.to_owned()))
@@ -82,13 +81,13 @@ impl DnsResolver for SystemDnsResolver {
 pub struct CachedDnsResolver<R: DnsResolver = SystemDnsResolver> {
     hostname: String,
     dns_resolver: R,
-    cache_file: PathBuf,
+    cache_file: Option<PathBuf>,
     cached_address: IpAddr,
     last_updated: SystemTime,
 }
 
 impl CachedDnsResolver<SystemDnsResolver> {
-    pub fn new(hostname: String, cache_file: PathBuf, fallback_address: IpAddr) -> Self {
+    pub fn new(hostname: String, cache_file: Option<PathBuf>, fallback_address: IpAddr) -> Self {
         Self::with_dns_resolver(SystemDnsResolver, hostname, cache_file, fallback_address)
     }
 }
@@ -97,11 +96,13 @@ impl<R: DnsResolver> CachedDnsResolver<R> {
     pub fn with_dns_resolver(
         dns_resolver: R,
         hostname: String,
-        cache_file: PathBuf,
+        cache_file: Option<PathBuf>,
         fallback_address: IpAddr,
     ) -> Self {
-        let (cached_address, last_updated) =
-            Self::load_initial_cached_address(&cache_file, fallback_address);
+        let (cached_address, last_updated) = match &cache_file {
+            Some(cache_file) => Self::load_initial_cached_address(&cache_file, fallback_address),
+            None => (fallback_address, EXPIRED_CACHE_TIMESTAMP),
+        };
 
         CachedDnsResolver {
             hostname,
@@ -167,21 +168,28 @@ impl<R: DnsResolver> CachedDnsResolver<R> {
     }
 
     fn resolve_into_cache(&mut self) {
-        if let Ok(address) = self.dns_resolver.resolve(&self.hostname) {
-            if Self::is_bogus_address(address) {
-                warn!(
-                    "DNS lookup for {} returned bogus address {}, ignoring",
-                    self.hostname, address
-                );
-                return;
-            }
+        debug!("Resolving IP for {}", self.hostname);
+        match self.dns_resolver.resolve(&self.hostname) {
+            Ok(address) => {
+                if Self::is_bogus_address(address) {
+                    warn!(
+                        "DNS lookup for {} returned bogus address {}, ignoring",
+                        self.hostname, address
+                    );
+                    return;
+                }
 
-            debug!("Updating DNS cache for {} with {}", self.hostname, address);
-            self.cached_address = address;
-            self.last_updated = SystemTime::now();
+                debug!("Updating DNS cache for {} with {}", self.hostname, address);
+                self.cached_address = address;
+                self.last_updated = SystemTime::now();
 
-            if let Err(error) = self.update_cache_file() {
-                warn!("Failed to update cache file with new IP address: {}", error);
+                if let Err(error) = self.update_cache_file() {
+                    warn!("Failed to update cache file with new IP address: {}", error);
+                }
+            }
+            Err(e) => {
+                let chained_error = e.chain_err(|| format!("Unable to resolve {}", self.hostname));
+                warn!("{}", chained_error.display_chain());
             }
         }
     }
@@ -197,9 +205,12 @@ impl<R: DnsResolver> CachedDnsResolver<R> {
     }
 
     fn update_cache_file(&mut self) -> io::Result<()> {
-        let mut cache_file = File::create(&self.cache_file)?;
-
-        writeln!(cache_file, "{}", self.cached_address)
+        if let Some(cache_file_path) = &self.cache_file {
+            let mut cache_file = File::create(cache_file_path)?;
+            writeln!(cache_file, "{}", self.cached_address)
+        } else {
+            Ok(())
+        }
     }
 }
 
@@ -412,7 +423,12 @@ mod tests {
         let cache_file = cache_dir.join(::API_IP_CACHE_FILENAME);
         let fallback_address = fallback_address.unwrap_or(IpAddr::from([10, 0, 109, 91]));
 
-        CachedDnsResolver::with_dns_resolver(mock_resolver, hostname, cache_file, fallback_address)
+        CachedDnsResolver::with_dns_resolver(
+            mock_resolver,
+            hostname,
+            Some(cache_file),
+            fallback_address,
+        )
     }
 
     struct MockDnsResolver {
diff --git a/mullvad-rpc/src/lib.rs b/mullvad-rpc/src/lib.rs
index 6873fe926a..7d5728d159 100644
--- a/mullvad-rpc/src/lib.rs
+++ b/mullvad-rpc/src/lib.rs
@@ -65,7 +65,7 @@ lazy_static! {
 
 /// A type that helps with the creation of RPC connections.
 pub struct MullvadRpcFactory {
-    address_cache: Option<CachedDnsResolver>,
+    cached_dns_resolver: CachedDnsResolver,
     ca_path: PathBuf,
 }
 
@@ -73,7 +73,7 @@ impl MullvadRpcFactory {
     /// Create a new `MullvadRpcFactory`.
     pub fn new<P: Into<PathBuf>>(ca_path: P) -> Self {
         MullvadRpcFactory {
-            address_cache: None,
+            cached_dns_resolver: CachedDnsResolver::new(API_HOST.to_owned(), None, *API_IP),
             ca_path: ca_path.into(),
         }
     }
@@ -81,10 +81,11 @@ impl MullvadRpcFactory {
     /// Create a new `MullvadRpcFactory` using the specified cache directory.
     pub fn with_cache_dir<P: Into<PathBuf>>(cache_dir: &Path, ca_path: P) -> Self {
         let cache_file = cache_dir.join(API_IP_CACHE_FILENAME);
-        let cached_dns_resolver = CachedDnsResolver::new(API_HOST.to_owned(), cache_file, *API_IP);
+        let cached_dns_resolver =
+            CachedDnsResolver::new(API_HOST.to_owned(), Some(cache_file), *API_IP);
 
         MullvadRpcFactory {
-            address_cache: Some(cached_dns_resolver),
+            cached_dns_resolver,
             ca_path: ca_path.into(),
         }
     }
@@ -111,7 +112,9 @@ impl MullvadRpcFactory {
         let transport_builder = HttpTransportBuilder::with_client(client).timeout(RPC_TIMEOUT);
 
         let transport = create_transport(transport_builder)?;
-        let mut handle = transport.handle(&self.api_uri())?;
+        let api_uri = self.api_uri();
+        debug!("Using API URI {}", api_uri);
+        let mut handle = transport.handle(&api_uri)?;
 
         handle.set_header(Host::new(API_HOST, None));
 
@@ -119,13 +122,8 @@ impl MullvadRpcFactory {
     }
 
     fn api_uri(&mut self) -> String {
-        let address = if let Some(ref mut address_cache) = self.address_cache {
-            address_cache.resolve().to_string()
-        } else {
-            API_HOST.to_owned()
-        };
-
-        format!("https://{}/rpc/", address)
+        let ip = self.cached_dns_resolver.resolve().to_string();
+        format!("https://{}/rpc/", ip)
     }
 }