agate

commit a4bafa2c4eea2933321171256233d231a438387b
parent 92673c54fb9a77736975900b798b52554628149b
Author: Johann150 <johann.galle@protonmail.com>
Date:   Mon, 24 May 2021 12:23:21 +0200

carefully check unwrap and expect usage

closes #59
I did not find any other cases where the unwrap or expect usage does not depend
on an internal invariant.

Diffstat:
M
CHANGELOG.md
 | 
10
++++++++--
M
README.md
 | 
6
+++++-
M
src/main.rs
 | 
34
++++++++++++++++++++--------------

3 files changed, 33 insertions(+), 17 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,10 +6,16 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
-Thank you to @skittlesvampir for contributing to this release.
+Thank you to @06kellyjac, @cpnfeeny, @lifelike, @skittlesvampir and @steko for contributing to this release.
 
 ### Added
-* Dockerfile for compiling Agate from source (#52)
+* Dockerfile for compiling Agate from source (#52, #53, #56, #57)
+
+### Fixed
+* If the remote IP address can not be fetched, log an error instead of panicking.
+  The previous handling could be exploited as a DoS attack vector. (#59)
+* Two tests were running on the same port, causing them to fail nondeterministically. (#51)
+* Rephrased the changelog for 3.0.0 on continuing to use older certificates. (#55)
 
 ## [3.0.2] - 2021-04-08
 Thank you to @kvibber, @lifelike and @pasdechance for contributing to this release.
diff --git a/README.md b/README.md
@@ -174,10 +174,14 @@ All requests will be logged using this format:
 ```
 The "error:" part will only be logged if an error occurred. This should only be used for informative purposes as the status code should provide the information that an error occurred. If the error consisted in the connection not being established (e.g. because of TLS errors), the status code `00` will be used.
 
-By default, Agate will not log the remote IP addresses because that might be an issue because IPs are considered private data under the EU's GDPR. To enable logging of IP addresses, you can use the `--log-ip` option.
+By default, Agate will not log the remote IP addresses because that might be an issue because IPs are considered private data under the EU's GDPR. To enable logging of IP addresses, you can use the `--log-ip` option. Note that in this case some error conditions might still force Agate to log a dash instead of an IP address.
 
 There are some lines apart from these that might occur in logs depending on the selected log level. For example the initial "Listening on..." line or information about listing a particular directory.
 
+Agate uses some status codes that are not valid Gemini status codes when logging errors:
+* 00 - there was an error establishing the TLS connection
+* 01 - there was an error in fetching the peer's IP address
+
 ## Security considerations
 
 If you want to run agate on a multi-user system, you should be aware that all certificate and key data is loaded into memory and stored there until the server stops. Since the memory is also not explicitly overwritten or zeroed after use, the sensitive data might stay in memory after the server has terminated.
diff --git a/src/main.rs b/src/main.rs
@@ -237,6 +237,7 @@ fn args() -> Result<Args> {
     let certs = if reload_certs {
         certificates::CertStore::load_from(&certs_path)?
     } else {
+        // there must already have been certificates loaded
         certs.unwrap()
     };
 
@@ -296,20 +297,25 @@ impl RequestHandle {
     /// Creates a new request handle for the given stream. If establishing the TLS
     /// session fails, returns a corresponding log line.
     async fn new(stream: TcpStream, metadata: Arc<Mutex<FileOptions>>) -> Result<Self, String> {
-        let log_line = format!(
-            "{} {}",
-            stream.local_addr().unwrap(),
-            if ARGS.log_ips {
-                stream
-                    .peer_addr()
-                    .expect("could not get peer address")
-                    .ip()
-                    .to_string()
-            } else {
-                // Do not log IP address, but something else so columns still line up.
-                "-".into()
-            }
-        );
+        let local_addr = stream.local_addr().unwrap().to_string();
+
+        // try to get the remote IP address if desired
+        let peer_addr = if ARGS.log_ips {
+            stream
+                .peer_addr()
+                .or(Err(format!(
+                    // use nonexistent status code 01 if peer IP is unknown
+                    "{} - \"\" 01 \"IP error\" error:could not get peer address",
+                    local_addr,
+                )))?
+                .ip()
+                .to_string()
+        } else {
+            // Do not log IP address, but something else so columns still line up.
+            "-".into()
+        };
+
+        let log_line = format!("{} {}", local_addr, peer_addr,);
 
         match TLS.accept(stream).await {
             Ok(stream) => Ok(Self {