Fix audit issue logging by default peer address

By default log format include remote address that is taken from headers.
This is very easy to replace making log untrusted.

Changing default log format value `%a` to peer address we are getting
this trusted data always. Also, remote address option is maintianed and
relegated to `%{r}a` value.

Related  kanidm/kanidm#191.

src/info.rs

@@ -30,7 +30,6 @@ impl ConnectionInfo {
         let mut host = None;
         let mut scheme = None;
         let mut remote = None;
-        let mut peer = None;
 
         // load forwarded header
         for hdr in req.headers.get_all(&header::FORWARDED) {
@@ -106,6 +105,9 @@ impl ConnectionInfo {
             }
         }
 
+        // get peeraddr from socketaddr
+        let peer = req.peer_addr.map(|addr| format!("{}", addr));
+
         // remote addr
         if remote.is_none() {
             if let Some(h) = req
@@ -116,10 +118,6 @@ impl ConnectionInfo {
                     remote = h.split(',').next().map(|v| v.trim());
                 }
             }
-            if remote.is_none() {
-                // get peeraddr from socketaddr
-                peer = req.peer_addr.map(|addr| format!("{}", addr));
-            }
         }
 
         ConnectionInfo {
@@ -155,6 +153,16 @@ impl ConnectionInfo {
         &self.host
     }
 
+    /// Peer address of the request.
+    ///
+    /// Get peer address from socket address
+    pub fn peer(&self) -> Option<&str> {
+        if let Some(ref peer) = self.peer {
+            Some(peer)
+        } else {
+            None
+        }
+    }
     /// Remote socket addr of client initiated HTTP request.
     ///
     /// The addr is resolved through the following headers, in this order:

src/middleware/logger.rs

@@ -55,7 +55,7 @@ use crate::HttpResponse;
 ///
 /// `%%`  The percent sign
 ///
-/// `%a`  Remote IP-address (IP-address of proxy if using reverse proxy)
+/// `%a`  Client IP-address
 ///
 /// `%t`  Time when the request was started to process (in rfc3339 format)
 ///
@@ -72,12 +72,18 @@ use crate::HttpResponse;
 ///
 /// `%U`  Request URL
 ///
+/// `%{r}a`  Remote IP-address (IP-address of proxy if using reverse proxy) **\***
+///
 /// `%{FOO}i`  request.headers['FOO']
 ///
 /// `%{FOO}o`  response.headers['FOO']
 ///
 /// `%{FOO}e`  os.environ['FOO']
 ///
+/// > **\* warning** It is critical to only use this value from intermediate
+/// > hosts(proxies, etc) which are trusted by this server, since it is trivial
+/// > for the remote client to simulate another client.
+///
 pub struct Logger(Rc<Inner>);
 
 struct Inner {
@@ -301,7 +307,7 @@ impl Format {
     /// Returns `None` if the format string syntax is incorrect.
     pub fn new(s: &str) -> Format {
         log::trace!("Access log format: {}", s);
-        let fmt = Regex::new(r"%(\{([A-Za-z0-9\-_]+)\}([ioe])|[atPrUsbTD]?)").unwrap();
+        let fmt = Regex::new(r"%(\{([A-Za-z0-9\-_]+)\}([aioe])|[atPrUsbTD]?)").unwrap();
 
         let mut idx = 0;
         let mut results = Vec::new();
@@ -315,6 +321,11 @@ impl Format {
 
             if let Some(key) = cap.get(2) {
                 results.push(match cap.get(3).unwrap().as_str() {
+                    "a" => if key.as_str() == "r" {
+                        FormatText::RemoteAddr
+                    } else {
+                        unreachable!()
+                    },
                     "i" => FormatText::RequestHeader(
                         HeaderName::try_from(key.as_str()).unwrap(),
                     ),
@@ -328,7 +339,7 @@ impl Format {
                 let m = cap.get(1).unwrap();
                 results.push(match m.as_str() {
                     "%" => FormatText::Percent,
-                    "a" => FormatText::RemoteAddr,
+                    "a" => FormatText::ClientIP,
                     "t" => FormatText::RequestTime,
                     "r" => FormatText::RequestLine,
                     "s" => FormatText::ResponseStatus,
@@ -361,6 +372,7 @@ pub enum FormatText {
     ResponseSize,
     Time,
     TimeMillis,
+    ClientIP,
     RemoteAddr,
     UrlPath,
     RequestHeader(HeaderName),
@@ -457,6 +469,14 @@ impl FormatText {
                 };
                 *self = FormatText::Str(s.to_string());
             }
+            FormatText::ClientIP => {
+                let s = if let Some(ref peer) = req.connection_info().peer() {
+                    FormatText::Str(peer.to_string())
+                } else {
+                    FormatText::Str("-".to_string())
+                };
+                *self = s;
+            }
             FormatText::RemoteAddr => {
                 let s = if let Some(remote) = req.connection_info().remote() {
                     FormatText::Str(remote.to_string())
@@ -549,6 +569,7 @@ mod tests {
             header::USER_AGENT,
             header::HeaderValue::from_static("ACTIX-WEB"),
         )
+        .peer_addr("127.0.0.1:8081".parse().unwrap())
         .to_srv_request();
 
         let now = OffsetDateTime::now_utc();
@@ -570,6 +591,7 @@ mod tests {
         };
         let s = format!("{}", FormatDisplay(&render));
         assert!(s.contains("GET / HTTP/1.1"));
+        assert!(s.contains("127.0.0.1"));
         assert!(s.contains("200 1024"));
         assert!(s.contains("ACTIX-WEB"));
     }
@@ -598,4 +620,36 @@ mod tests {
         let s = format!("{}", FormatDisplay(&render));
         assert!(s.contains(&format!("{}", now.format("%Y-%m-%dT%H:%M:%S"))));
     }
+
+    #[actix_rt::test]
+    async fn test_remote_addr_format() {
+        let mut format = Format::new("%{r}a");
+
+        let req = TestRequest::with_header(
+            header::FORWARDED,
+            header::HeaderValue::from_static("for=192.0.2.60;proto=http;by=203.0.113.43"),
+        )
+        .to_srv_request();
+
+        let now = OffsetDateTime::now_utc();
+        for unit in &mut format.0 {
+            unit.render_request(now, &req);
+        }
+
+        let resp = HttpResponse::build(StatusCode::OK).force_close().finish();
+        for unit in &mut format.0 {
+            unit.render_response(&resp);
+        }
+
+        let entry_time = OffsetDateTime::now_utc();
+        let render = |fmt: &mut Formatter<'_>| {
+            for unit in &format.0 {
+                unit.render(fmt, 1024, entry_time)?;
+            }
+            Ok(())
+        };
+        let s = format!("{}", FormatDisplay(&render));
+        println!("{}", s);
+        assert!(s.contains("192.0.2.60"));
+    }
 }