diff --git a/CHANGES.md b/CHANGES.md index 09b8f6a6..1a44caee 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -5,8 +5,14 @@ ### Changed * Resources and Scopes can now access non-overridden data types set on App (or containing scopes) when setting their own data. [#1486] + +* Fix audit issue logging by default peer address [#1485] + * Bump minimum supported Rust version to 1.40 +[#1485]: https://github.com/actix/actix-web/pull/1485 + + ## [3.0.0-alpha.2] - 2020-05-08 ### Changed @@ -21,6 +27,7 @@ [#1452]: https://github.com/actix/actix-web/pull/1452 [#1486]: https://github.com/actix/actix-web/pull/1486 + ## [3.0.0-alpha.1] - 2020-03-11 ### Added diff --git a/src/info.rs b/src/info.rs index c9a642b3..5b506d85 100644 --- a/src/info.rs +++ b/src/info.rs @@ -12,8 +12,8 @@ const X_FORWARDED_PROTO: &[u8] = b"x-forwarded-proto"; pub struct ConnectionInfo { scheme: String, host: String, - remote: Option, - peer: Option, + realip_remote_addr: Option, + remote_addr: Option, } impl ConnectionInfo { @@ -29,8 +29,7 @@ impl ConnectionInfo { fn new(req: &RequestHead, cfg: &AppConfig) -> ConnectionInfo { let mut host = None; let mut scheme = None; - let mut remote = None; - let mut peer = None; + let mut realip_remote_addr = None; // load forwarded header for hdr in req.headers.get_all(&header::FORWARDED) { @@ -42,8 +41,8 @@ impl ConnectionInfo { if let Some(val) = items.next() { match &name.to_lowercase() as &str { "for" => { - if remote.is_none() { - remote = Some(val.trim()); + if realip_remote_addr.is_none() { + realip_remote_addr = Some(val.trim()); } } "proto" => { @@ -106,27 +105,25 @@ impl ConnectionInfo { } } - // remote addr - if remote.is_none() { + // get remote_addraddr from socketaddr + let remote_addr = req.peer_addr.map(|addr| format!("{}", addr)); + + if realip_remote_addr.is_none() { if let Some(h) = req .headers .get(&HeaderName::from_lowercase(X_FORWARDED_FOR).unwrap()) { if let Ok(h) = h.to_str() { - remote = h.split(',').next().map(|v| v.trim()); + realip_remote_addr = h.split(',').next().map(|v| v.trim()); } } - if remote.is_none() { - // get peeraddr from socketaddr - peer = req.peer_addr.map(|addr| format!("{}", addr)); - } } ConnectionInfo { - peer, + remote_addr, scheme: scheme.unwrap_or("http").to_owned(), host: host.unwrap_or("localhost").to_owned(), - remote: remote.map(|s| s.to_owned()), + realip_remote_addr: realip_remote_addr.map(|s| s.to_owned()), } } @@ -155,13 +152,23 @@ impl ConnectionInfo { &self.host } - /// Remote socket addr of client initiated HTTP request. + /// remote_addr address of the request. + /// + /// Get remote_addr address from socket address + pub fn remote_addr(&self) -> Option<&str> { + if let Some(ref remote_addr) = self.remote_addr { + Some(remote_addr) + } else { + None + } + } + /// Real ip remote addr of client initiated HTTP request. /// /// The addr is resolved through the following headers, in this order: /// /// - Forwarded /// - X-Forwarded-For - /// - peer name of opened socket + /// - remote_addr name of opened socket /// /// # Security /// Do not use this function for security purposes, unless you can ensure the Forwarded and @@ -169,11 +176,11 @@ impl ConnectionInfo { /// address explicitly, use /// [`HttpRequest::peer_addr()`](../web/struct.HttpRequest.html#method.peer_addr) instead. #[inline] - pub fn remote(&self) -> Option<&str> { - if let Some(ref r) = self.remote { + pub fn realip_remote_addr(&self) -> Option<&str> { + if let Some(ref r) = self.realip_remote_addr { Some(r) - } else if let Some(ref peer) = self.peer { - Some(peer) + } else if let Some(ref remote_addr) = self.remote_addr { + Some(remote_addr) } else { None } @@ -202,7 +209,7 @@ mod tests { let info = req.connection_info(); assert_eq!(info.scheme(), "https"); assert_eq!(info.host(), "rust-lang.org"); - assert_eq!(info.remote(), Some("192.0.2.60")); + assert_eq!(info.realip_remote_addr(), Some("192.0.2.60")); let req = TestRequest::default() .header(header::HOST, "rust-lang.org") @@ -211,20 +218,20 @@ mod tests { let info = req.connection_info(); assert_eq!(info.scheme(), "http"); assert_eq!(info.host(), "rust-lang.org"); - assert_eq!(info.remote(), None); + assert_eq!(info.realip_remote_addr(), None); let req = TestRequest::default() .header(X_FORWARDED_FOR, "192.0.2.60") .to_http_request(); let info = req.connection_info(); - assert_eq!(info.remote(), Some("192.0.2.60")); + assert_eq!(info.realip_remote_addr(), Some("192.0.2.60")); let req = TestRequest::default() .header(X_FORWARDED_HOST, "192.0.2.60") .to_http_request(); let info = req.connection_info(); assert_eq!(info.host(), "192.0.2.60"); - assert_eq!(info.remote(), None); + assert_eq!(info.realip_remote_addr(), None); let req = TestRequest::default() .header(X_FORWARDED_PROTO, "https") diff --git a/src/middleware/logger.rs b/src/middleware/logger.rs index 7d1577c9..d6b931bb 100644 --- a/src/middleware/logger.rs +++ b/src/middleware/logger.rs @@ -72,12 +72,21 @@ use crate::HttpResponse; /// /// `%U` Request URL /// +/// `%{r}a` Real IP remote address **\*** +/// /// `%{FOO}i` request.headers['FOO'] /// /// `%{FOO}o` response.headers['FOO'] /// /// `%{FOO}e` os.environ['FOO'] /// +/// # Security +/// **\*** It is calculated using +/// [`ConnectionInfo::realip_remote_addr()`](../dev/struct.ConnectionInfo.html#method.realip_remote_addr) +/// +/// If you use this value ensure that all requests come from trusted hosts, since it is trivial +/// for the remote client to simulate been another client. +/// pub struct Logger(Rc); struct Inner { @@ -301,7 +310,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 +324,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::RealIPRemoteAddr + } else { + unreachable!() + }, "i" => FormatText::RequestHeader( HeaderName::try_from(key.as_str()).unwrap(), ), @@ -362,6 +376,7 @@ pub enum FormatText { Time, TimeMillis, RemoteAddr, + RealIPRemoteAddr, UrlPath, RequestHeader(HeaderName), ResponseHeader(HeaderName), @@ -458,7 +473,15 @@ impl FormatText { *self = FormatText::Str(s.to_string()); } FormatText::RemoteAddr => { - let s = if let Some(remote) = req.connection_info().remote() { + let s = if let Some(ref peer) = req.connection_info().remote_addr() { + FormatText::Str(peer.to_string()) + } else { + FormatText::Str("-".to_string()) + }; + *self = s; + } + FormatText::RealIPRemoteAddr => { + let s = if let Some(remote) = req.connection_info().realip_remote_addr() { FormatText::Str(remote.to_string()) } else { FormatText::Str("-".to_string()) @@ -549,6 +572,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 +594,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 +623,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")); + } }