Repository: cloudflare/boringtun (language: rust)

Issue:
Improve error checking paths during startup
In `src/main.rs`, there are some code paths that ignore errors. Among the possibly problematic ones are:

* When run as root, any failure to change user/group IDs will not result in a fatal error. This might might increase the risk of running with unnecessary privileged. (btw, the option name is misspelled, #43 was incomplete.)
```rust
    if !matches.is_present("disable-drop-priviliges") {
        drop_privileges().ok();
    }
```
* When running backgrounded (without `--foreground`), any errors will result in the application being foregrounded anyway. This may be contrary to user expectations.
```rust
        match daemonize.start() {
            Ok(_) => println!("Success, daemonized"),
            Err(e) => eprintln!("Error, {}", e),
        }
```



Discussion / hints:

Fix patch:
diff --git a/src/main.rs b/src/main.rs
index 627e0ceb..eede8e37 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -13,6 +13,7 @@ use clap::{value_t, App, Arg};
 use daemonize::Daemonize;
 use std::fs::File;
 use std::os::unix::net::UnixDatagram;
+use std::process::exit;
 
 fn check_tun_name(_v: String) -> Result<(), String> {
     #[cfg(target_os = "macos")]
@@ -92,7 +93,7 @@ fn main() {
 
     // Create a socketpair to communicate between forked processes
     let (sock1, sock2) = UnixDatagram::pair().unwrap();
-    sock1.set_nonblocking(true).ok();
+    let _ = sock1.set_nonblocking(true);
 
     if background {
         let log = matches.value_of("log").unwrap();
@@ -109,20 +110,20 @@ fn main() {
             .stderr(stderr)
             .exit_action(move || {
                 let mut b = [0u8; 1];
-                sock2.recv(&mut b).ok();
-
-                println!(
-                    "{}",
-                    match b[0] {
-                        0 => "boringtun failed to start",
-                        _ => "boringtun started successfully",
-                    }
-                );
+                if sock2.recv(&mut b).is_ok() && b[0] == 1 {
+                    println!("BoringTun started successfully");
+                } else {
+                    eprintln!("BoringTun failed to start");
+                    exit(1);
+                };
             });
 
         match daemonize.start() {
             Ok(_) => println!("Success, daemonized"),
-            Err(e) => eprintln!("Error, {}", e),
+            Err(e) => {
+                eprintln!("Error, {}", e);
+                exit(1);
+            }
         }
     }
 
@@ -138,18 +139,22 @@ fn main() {
         Ok(d) => d,
         Err(e) => {
             // Notify parent that tunnel initialization failed
-            println!("{:?}", e);
-            sock1.send(&[0]).ok();
-            return;
+            eprintln!("Failed to initialize tunnel: {:?}", e);
+            sock1.send(&[0]).unwrap();
+            exit(1);
         }
     };
 
     if !matches.is_present("disable-drop-privileges") {
-        drop_privileges().ok();
+        if let Err(e) = drop_privileges() {
+            eprintln!("Failed to drop privileges: {:?}", e);
+            sock1.send(&[0]).unwrap();
+            exit(1);
+        }
     }
 
     // Notify parent that tunnel initialization succeeded
-    sock1.send(&[1]).ok();
+    sock1.send(&[1]).unwrap();
     drop(sock1);
 
     device_handle.wait();


Test patch: