Skip to content

bug: Handle private networks as trusted proxies #53

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Nov 7, 2018
Merged

bug: Handle private networks as trusted proxies #53

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3f3627d
bug: Handle private networks as trusted proxies
jrconlin Nov 6, 2018
ecab860
f r's
jrconlin Nov 7, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 22 additions & 5 deletions channelserver/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,22 +165,39 @@ fn main() {
};
let msettings = settings.clone();
let mut trusted_list: Vec<ipnet::IpNet> = Vec::new();
// Add the known private networks to the trusted proxy list
trusted_list.push("10.0.0.0/8".parse().unwrap());
trusted_list.push("172.16.0.0/12".parse().unwrap());
trusted_list.push("192.168.0.0/16".parse().unwrap());

// Add the list of trusted proxies.
if settings.trusted_proxy_list.len() > 0 {
for mut proxy in settings.trusted_proxy_list.split(",") {
proxy = proxy.trim();
if proxy.len() > 0 {
let addr: ipnet::IpNet = proxy.parse().unwrap();
trusted_list.push(addr);
// ipnet::IpNet only wants CIDRs. Normally that's not a problem, but the
// user may specify a single address. In that case, force the single
// into a CIDR by giving it a single address scope.
let mut fixed = proxy.to_owned();
if !proxy.contains("/") {
fixed = format!("{}/32", proxy);
debug!(log.log, "Fixing single address {}", fixed);
}
match fixed.parse::<ipnet::IpNet>() {
Ok(addr) => trusted_list.push(addr),
Err(err) => {
error!(logger.log, r#"Ignoring unparsable IP address "{}"#, proxy);
}
};
}
}
}
debug!(logger.log, "Trusted Proxies: {:?}", trusted_list);
// check that the maxmind db is where it should be.
if !Path::new(&settings.mmdb_loc).exists() {
slog_error!(
error!(
logger.log,
"Cannot find geoip database: {}",
settings.mmdb_loc
"Cannot find geoip database: {}", settings.mmdb_loc
);
return;
};
Expand Down
61 changes: 30 additions & 31 deletions channelserver/src/meta.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,24 +127,16 @@ fn get_ua(
None
}

fn is_trusted_proxy(proxy_list: &[IpNet], host: &str) -> Result<bool, HandlerError> {
// Return if an address is NOT part of the allow list
let test_addr: IpAddr = match host.parse() {
Ok(addr) => addr,
Err(e) => return Err(HandlerErrorKind::BadRemoteAddrError(format!("{:?}", e)).into()),
};
for proxy_range in proxy_list {
if proxy_range.contains(&test_addr) {
return Ok(true);
}
}
Ok(false)
fn is_trusted_proxy(proxy_list: &[IpNet], host: &IpAddr) -> bool {
// Return if an address is part of the allow list
proxy_list.iter().any(|range| range.contains(host))
}

fn get_remote(
peer: &Option<SocketAddr>,
headers: &http::HeaderMap,
proxy_list: &[IpNet],
log: &logging::MozLogger,
) -> Result<String, HandlerError> {
// Actix determines the connection_info.remote() from the first entry in the
// Forwarded then X-Fowarded-For, Forwarded-For, then peer name. The problem is that any
Expand All @@ -156,31 +148,36 @@ fn get_remote(
if peer.is_none() {
return Err(HandlerErrorKind::BadRemoteAddrError("Peer is unspecified".to_owned()).into());
}
let peer_ip = peer.unwrap().ip().to_string();
let peer_ip = peer.unwrap().ip();
// if the peer is not a known proxy, ignore the X-Forwarded-For headers
if !is_trusted_proxy(proxy_list, &peer_ip)? {
return Ok(peer_ip);
if !is_trusted_proxy(proxy_list, &peer_ip) {
return Ok(peer_ip.to_string());
}

// The peer is a known proxy, so take rightmost X-Forwarded-For that is not a trusted proxy.
match headers.get(HeaderName::from_lowercase("x-forwarded-for".as_bytes()).unwrap()) {
Some(header) => {
match header.to_str() {
Ok(hstr) => {
info!(log.log, "Remote IP List: {:?}", hstr);
// successive proxies are appeneded to this header.
let mut host_list: Vec<&str> = hstr.split(',').collect();
host_list.reverse();
for host_str in host_list {
let host = host_str.trim().to_owned();
if !is_trusted_proxy(proxy_list, &host)? {
return Ok(host.to_owned());
match host_str.trim().parse::<IpAddr>() {
Ok(addr) => {
if !addr.is_loopback() &&
!is_trusted_proxy(proxy_list, &addr) {
return Ok(addr.to_string());
}
},
Err(err) => {
return Err(HandlerErrorKind::BadRemoteAddrError("Bad IP Specified".to_owned()).into());
}
}
}
Err(HandlerErrorKind::BadRemoteAddrError(format!(
"Could not find remote IP in X-Forwarded-For"
))
.into())
}
};
Err(HandlerErrorKind::BadRemoteAddrError("Only proxies specified".to_owned()).into())
},
Err(err) => Err(HandlerErrorKind::BadRemoteAddrError(format!(
"Unknown address in X-Forwarded-For: {:?}",
err
Expand Down Expand Up @@ -302,6 +299,7 @@ impl From<HttpRequest<WsChannelSessionState>> for SenderData {
&req.peer_addr(),
&req.headers(),
&req.state().trusted_proxy_list,
&log,
) {
Ok(addr) => Some(addr),
Err(err) => {
Expand Down Expand Up @@ -485,30 +483,31 @@ mod test {

let true_remote: SocketAddr = "1.2.3.4:0".parse().unwrap();
let proxy_server: SocketAddr = "192.168.0.4:0".parse().unwrap();
let log = logging::MozLogger::new_human();

bad_headers.insert(
http::header::HeaderName::from_lowercase("x-forwarded-for".as_bytes()).unwrap(),
"".parse().unwrap(),
);

// Proxy only, no XFF header
let remote = get_remote(&Some(proxy_server), &empty_headers, &proxy_list);
let remote = get_remote(&Some(proxy_server), &empty_headers, &proxy_list, &log);
assert!(remote.is_err());

// Proxy only, bad XFF header
let remote = get_remote(&Some(proxy_server), &bad_headers, &proxy_list);
let remote = get_remote(&Some(proxy_server), &bad_headers, &proxy_list, &log);
assert!(remote.is_err());

// Proxy only, crap XFF header
bad_headers.insert(
http::header::HeaderName::from_lowercase("x-forwarded-for".as_bytes()).unwrap(),
"invalid".parse().unwrap(),
);
let remote = get_remote(&Some(proxy_server), &bad_headers, &proxy_list);
let remote = get_remote(&Some(proxy_server), &bad_headers, &proxy_list, &log);
assert!(remote.is_err());

// Peer only, no header
let remote = get_remote(&Some(true_remote), &empty_headers, &proxy_list);
let remote = get_remote(&Some(true_remote), &empty_headers, &proxy_list, &log);
assert_eq!(remote.unwrap(), "1.2.3.4".to_owned());

headers.insert(
Expand All @@ -517,7 +516,7 @@ mod test {
);

// Peer proxy, fetch from XFF header
let remote = get_remote(&Some(proxy_server), &headers, &proxy_list);
let remote = get_remote(&Some(proxy_server), &headers, &proxy_list, &log);
assert_eq!(remote.unwrap(), "1.2.3.4".to_owned());

// Peer proxy, ensure right most XFF client fetched
Expand All @@ -526,7 +525,7 @@ mod test {
"1.2.3.4, 2.3.4.5".parse().unwrap(),
);

let remote = get_remote(&Some(proxy_server), &headers, &proxy_list);
let remote = get_remote(&Some(proxy_server), &headers, &proxy_list, &log);
assert_eq!(remote.unwrap(), "2.3.4.5".to_owned());

// Peer proxy, ensure right most non-proxy XFF client fetched
Expand All @@ -535,7 +534,7 @@ mod test {
"1.2.3.4, 2.3.4.5, 192.168.0.10".parse().unwrap(),
);

let remote = get_remote(&Some(proxy_server), &headers, &proxy_list);
let remote = get_remote(&Some(proxy_server), &headers, &proxy_list, &log);
assert_eq!(remote.unwrap(), "2.3.4.5".to_owned());
}
}