fix zombie subprocesses by waiting on them

Also bump to 0.2.1.

Cargo.lock

@@ -13,9 +13,9 @@ dependencies = [
 
 [[package]]
 name = "ansi_term"
-version = "0.11.0"
+version = "0.12.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "ee49baf6cb617b853aa8d93bf420db2383fab46d314482ca2803b40d5fde979b"
+checksum = "d52a9bb7ec0cf484c551830a7ce27bd20d67eac647e1befb56b0be4ee39a55d2"
 dependencies = [
  "winapi",
 ]
@@ -70,9 +70,9 @@ dependencies = [
 
 [[package]]
 name = "clap"
-version = "2.33.3"
+version = "2.34.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "37e58ac78573c40708d45522f0d80fa2f01cc4f9b4e2bf749807255454312002"
+checksum = "a0610544180c38b88101fecf2dd634b174a62eef6946f84dfc6a7127512b381c"
 dependencies = [
  "ansi_term",
  "atty",
@@ -149,9 +149,9 @@ dependencies = [
 
 [[package]]
 name = "libc"
-version = "0.2.108"
+version = "0.2.112"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "8521a1b57e76b1ec69af7599e75e38e7b7fad6610f037db8c79b127201b5d119"
+checksum = "1b03d17f364a3a042d5e5d46b053bbbf82c92c9430c592dd4c064dc6ee997125"
 
 [[package]]
 name = "log"
@@ -195,13 +195,13 @@ dependencies = [
 
 [[package]]
 name = "once_cell"
-version = "1.8.0"
+version = "1.9.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "692fcb63b64b1758029e0a96ee63e049ce8c5948587f2f7208df04625e5f6b56"
+checksum = "da32515d9f6e6e489d7bc9d84c71b060db7247dc035bbe44eac88cf87486d8d5"
 
 [[package]]
 name = "opal"
-version = "0.1.0"
+version = "0.2.0"
 dependencies = [
  "chrono",
  "clap",
@@ -228,9 +228,9 @@ dependencies = [
 
 [[package]]
 name = "openssl-sys"
-version = "0.9.71"
+version = "0.9.72"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "7df13d165e607909b363a4757a6f133f8a818a74e9d3a98d09c6128e15fa4c73"
+checksum = "7e46109c383602735fa0a2e48dd2b7c892b048e1bf69e5c3b1d804b7d9c203cb"
 dependencies = [
  "autocfg",
  "cc",
@@ -247,9 +247,9 @@ checksum = "d4fd5641d01c8f18a23da7b6fe29298ff4b55afcccdf78973b24cf3175fee32e"
 
 [[package]]
 name = "pkg-config"
-version = "0.3.22"
+version = "0.3.24"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "12295df4f294471248581bc09bef3c38a5e46f1e36d6a37353621a0c6c357e1f"
+checksum = "58893f751c9b0412871a09abd62ecd2a00298c6c83befa223ef98c52aef40cbe"
 
 [[package]]
 name = "regex"

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "opal"
-version = "0.1.0"
+version = "0.2.1"
 edition = "2021"
 license = "GPL-3.0-or-later"
 

README.md

@@ -8,7 +8,7 @@ project's goals are:
 
 - Focus on a small set of features (around CGI) but do them correctly.
 - Be nice with old/stupid hardware (TLS 1.2 is OK, be efficient, etc).
-- Don't add features unless someone actively wants them in.
+- Don't add features (see the roadmap at the end of this file).
 - Try to keep resources (binary size, memory, etc) under tight control.
 
 Opal uses the `openssl` Rust bindings, which work with OpenSSL and LibreSSL, so
@@ -21,19 +21,89 @@ Opal is licensed as GPLv3.
 
 
 
+Installation
+------------
+
+### Pre-compiled releases
+
+Binary releases for 64-bit Linux systems are available on [my Gitea][gitea-rel]
+and on [Github][gh-rel].
+
+[gitea-rel]: https://git.dece.space/Dece/Opal/releases
+[gh-rel]: https://github.com/Dece/Opal/releases
+
+### Compiling from sources
+
+Compiling Opal requires Cargo installed with the stable Rust toolchain.
+
+Opal tries to use as few dependencies as possible. Here is the current list
+along with the motivation:
+
+- `chrono`: helps to convert ASN.1 times to RFC 3339… yeah sorry about that;
+- `clap`: sturdy CLI library;
+- `env_logger`: simple logging frontend for log;
+- `log`: logging framework;
+- `openssl`: SSL support, relies on your system's TLS libraries;
+- `percent-encoding`: handle percent-encoded strings, not provided by `url`;
+- `url`: URL parsing, best not to reinvent the wheel here;
+
+
+
 Usage
 -----
 
 Use `opal -h` to get a list of options. There is no config file, every setting
 can be configured from the command line.
 
-- `-a, --address <address>`: specify the address to listen to.
+- `-a, --address <address>`: specify the address(es) to listen to.
 - `-c, --cert <cert>`: server certificate path.
 - `-k, --key <key>`: server private key path.
 - `-r, --root-path <root_path>`: path to CGI scripts root.
 - `-e, --env <key=value>`: additional environment variables for CGI scripts;
     this option can be used multiple times.
 
+You can specify multiple addresses to listen to by using several `-a` options.
+Note that if you just want to listen to both IPv4 and IPv6 on any interface,
+listening only on `[::]:1965` should suffice for systems with dual-stack
+enabled (default on many Linux systems, maybe not BSD).
+
+### Systemd
+
+I personally run Opal as a Systemd service. Here is an example unit file:
+
+``` ini
+[Unit]
+Description=Opal Gemini server
+
+[Service]
+WorkingDirectory=/home/gemini/opal
+User=gemini
+Group=gemini
+ExecStart=/usr/local/bin/opal -a "[::]:1966" -c certs/cert.pem -k certs/key.pem -r cgi -e STORAGE_ROOT=storage
+Restart=always
+RestartSec=1
+SyslogIdentifier=opal
+# Security options:
+NoNewPrivileges=yes
+ProtectSystem=full
+ProtectHome=tmpfs
+BindReadOnlyPaths="/home/gemini/opal"
+BindPaths="/home/gemini/opal/storage"
+
+[Install]
+WantedBy=multi-user.target
+```
+
+- Opal has been installed in `/usr/local/bin`
+- The directory `/home/gemini/opal` contains the directories `certs`, `cgi` and
+    `storage`, for certificates, the CGI scripts and a storage path.
+- The `/home` directory is not readable, except for `/home/gemini/opal` which is
+    read-only, except for the `storage` directory which is writeable.
+
+This is just an example, please do not mindlessly copy and paste it without
+understanding what the options stand for. It is also possible to use a chrooted
+environment or the Systemd equivalent option RootDirectory. Your choice!
+
 
 
 CGI support
@@ -77,9 +147,9 @@ Opal does not provide `CONTENT_LENGTH`, `CONTENT_TYPE`, `REMOTE_IDENT` because
 they do not make much sense in Gemini. `PATH_TRANSLATED` is also not implemented
 by pure laziness.
 
-The `TLS_CLIENT_HASH` is a string that starts with "SHA256:" followed by the
-SHA256 digest of the DER representation of the client certificate, as an
-uppercase hex-string.
+The `TLS_CLIENT_HASH` is a string that starts with the 7 bytes `SHA256:`
+followed by the SHA256 digest of the DER representation of the client
+certificate, as an uppercase hex-string.
 
 It can be a bit confusing which variable represent what data, especially those
 related to the URL and the path. Take the following request as example:
@@ -101,14 +171,12 @@ QUERY_STRING=search=%C3%A9l%C3%A9ment
 Roadmap
 -------
 
-Things to consider:
+Things that might end up in Opal one day:
 
-- Multiple listening addresses, at least so we can easily listen on both IPv4
-    and IPv6.
 - Support SCGI; a bit more complex but should save resources on smol hardware.
 
 Things that probably won't be considered:
 
-- Serve static files; so many other servers to that correctly already!
+- Serve static files; so many other servers do that correctly already!
 - Any kind of security mechanism that is not properly motivated.
-- FastCGI; come on…
+- FastCGI; un-smol…

src/main.rs

@@ -1,6 +1,7 @@
 use std::collections::HashMap;
 use std::fmt::Write;
 use std::fs;
+use std::io::Read;
 use std::net;
 use std::path;
 use std::process;
@@ -40,7 +41,8 @@ fn run() -> Result<(), i32> {
                 .short("a")
                 .long("address")
                 .help("Address to listen to")
-                .takes_value(true),
+                .takes_value(true)
+                .multiple(true),
         )
         .arg(
             clap::Arg::with_name("cert")
@@ -107,26 +109,40 @@ fn run() -> Result<(), i32> {
         matches.value_of("cert").unwrap(),
         matches.value_of("key").unwrap(),
     )
-    .map_err(|err| run_failure("Could not create TLS acceptor", &err))?;
+    .map_err(|err| run_failure("Can't create TLS acceptor", &err))?;
 
-    let address = matches.value_of("address").unwrap();
-    let listener = net::TcpListener::bind(address)
-        .map_err(|err| run_failure("Could not create TCP listener", &err))?;
-
-    for stream in listener.incoming() {
-        match stream {
-            Ok(stream) => {
-                let acceptor = acceptor.clone();
-                let cgi_config = cgi_config.clone();
-                thread::spawn(move || match acceptor.accept(stream) {
-                    Ok(mut tls_stream) => handle_client(&mut tls_stream, &cgi_config),
-                    Err(err) => error!("Can't initiate TLS stream: {}", err),
-                });
-            }
+    let mut threads = vec![];
+    for address in matches.values_of("address").unwrap() {
+        let listener = match net::TcpListener::bind(address) {
+            Ok(l) => l,
             Err(err) => {
-                error!("Can't accept connection: {}", err);
+                error!("Can't create TCP listener: {}", &err);
+                continue
             }
-        }
+        };
+        info!("Listening on {}", address);
+        let acceptor = acceptor.clone();
+        let cgi_config = cgi_config.clone();
+        threads.push(thread::spawn(move || {
+            for stream in listener.incoming() {
+                match stream {
+                    Ok(stream) => {
+                        let acceptor = acceptor.clone();
+                        let cgi_config = cgi_config.clone();
+                        thread::spawn(move || match acceptor.accept(stream) {
+                            Ok(mut tls_stream) => handle_client(&mut tls_stream, &cgi_config),
+                            Err(err) => error!("Can't initiate TLS stream: {}", err),
+                        });
+                    }
+                    Err(err) => {
+                        error!("Can't accept connection: {}", err);
+                    }
+                }
+            }
+        }));
+    }
+    for t in threads.into_iter() {
+        t.join().unwrap();
     }
     Ok(())
 }
@@ -193,10 +209,42 @@ fn handle_client(tls_stream: &mut ssl::SslStream<net::TcpStream>, cgi_config: &C
     }
 
     // Get appropriate response from either Opal or the CGI process.
-    let response: Vec<u8> = match get_response(&request[..read_bytes], cgi_config, &tls_stream) {
-        Ok((url, data)) => {
-            info!("\"{}\" → reply {} bytes", url, data.len());
-            data
+    match get_response(&request[..read_bytes], cgi_config, &tls_stream) {
+        Ok((url, mut child)) => {
+            let mut buffer = vec![0u8; 4096];
+            let mut stdout = child.stdout.take().expect("child process stdout not available");
+            let mut num_sent = 0;
+            loop {
+                match stdout.read(&mut buffer) {
+                    Ok(n) if n == 0 => break,
+                    Ok(num_read) => match tls_stream.ssl_write(&buffer[..num_read]) {
+                        Ok(n) => num_sent += n,
+                        Err(err) => error!("Can't write response: {}", err),
+                    },
+                    Err(err) => {
+                        error!("Can't read child process stdout: {}", err);
+                        break;
+                    }
+                }
+            }
+            info!("\"{}\" → replied {} bytes", url, num_sent);
+            child.wait().expect("child process can't be waited for");
+            let mut stderr = child.stderr.expect("child process' stderr not available");
+            let mut errors = vec![];
+            match stderr.read_to_end(&mut errors) {
+                Ok(n) if n == 0 => {}
+                Ok(_) => {
+                    warn!("Child process stderr:");
+                    if let Ok(errors_utf8) = std::str::from_utf8(errors.as_slice()) {
+                        for line in errors_utf8.lines() {
+                            warn!("  {}", line);
+                        }
+                    } else {
+                        error!("Can't decode process standard error.")
+                    }
+                }
+                Err(err) => error!("Can't read child process stderr: {}", err),
+            }
         }
         Err((url, code, meta)) => {
             info!(
@@ -205,19 +253,17 @@ fn handle_client(tls_stream: &mut ssl::SslStream<net::TcpStream>, cgi_config: &C
                 code,
                 meta
             );
-            format!("{} {}\r\n", code, meta).as_bytes().to_vec()
+            let error_response = format!("{} {}\r\n", code, meta).as_bytes().to_vec();
+            if let Err(err) = tls_stream.ssl_write(&error_response) {
+                error!("Can't write error response: {}", err);
+            }
         }
     };
 
-    // Whether the request succeeded or not, send the response.
-    if let Err(err) = tls_stream.ssl_write(&response) {
-        error!("Error while writing TLS data: {}", err);
-    }
-
     // Properly close the connection with a close notify.
     match tls_stream.shutdown() {
         Ok(shutdown) => debug!("Connection shutdown (state: {:?})", shutdown),
-        Err(err) => error!("Could not properly shutdown: {}", err),
+        Err(err) => error!("Can't properly shutdown: {}", err),
     }
 }
 
@@ -232,7 +278,7 @@ fn get_response(
     request: &[u8],
     cgi_config: &CgiConfig,
     tls: &ssl::SslStream<net::TcpStream>,
-) -> Result<(String, Vec<u8>), (Option<String>, u8, &'static str)> {
+) -> Result<(String, process::Child), (Option<String>, u8, &'static str)> {
     // Convert the URL to UTF-8.
     let url_str = std::str::from_utf8(&request[..request.len() - 2])
         .map_err(|_| (None, 59, "URL is not valid UTF-8"))?;
@@ -357,28 +403,19 @@ fn get_response(
         .collect::<HashMap<String, String>>();
 
     // Run the subprocess!
-    let output = process::Command::new(script_path)
+    let child = process::Command::new(script_path)
         .env_clear()
         .envs(&envs)
         .envs(&cgi_config.envs)
-        .output()
+        .stdout(process::Stdio::piped())
+        .stderr(process::Stdio::piped())
+        .spawn()
         .map_err(|err| {
             error!("Can't execute script: {}", err);
             cgi_error.to_owned()
         })?;
 
-    if output.stderr.len() > 0 {
-        warn!("Process standard error:");
-        if let Ok(stderr) = std::str::from_utf8(output.stderr.as_slice()) {
-            for line in stderr.lines() {
-                warn!("  {}", line);
-            }
-        } else {
-            error!("Can't decode process standard error.")
-        }
-    }
-
-    Ok((url_str.to_string(), output.stdout))
+    Ok((url_str.to_string(), child))
 }
 
 /// Return a validated script path from the requested URL along with CGI PATH_INFO.