Close outbound credential and metadata bypasses
This commit is contained in:
parent
3088aa439e
commit
c4402985af
4 changed files with 194 additions and 23 deletions
|
|
@ -7,9 +7,6 @@ use ipnet::IpNet;
|
|||
|
||||
use crate::{CanonicalUrl, OutboundError, Resolver};
|
||||
|
||||
const METADATA_V4: &str = "169.254.169.254";
|
||||
const METADATA_V6: &str = "fd00:ec2::254";
|
||||
|
||||
#[derive(Clone, Debug)]
|
||||
pub struct OutboundPolicy {
|
||||
allowed_scheme_ports: HashSet<(String, u16)>,
|
||||
|
|
@ -75,7 +72,7 @@ impl OutboundPolicy {
|
|||
IpAddr::V6(address) => address.to_ipv4_mapped().map_or(address.into(), IpAddr::V4),
|
||||
address => address,
|
||||
};
|
||||
if effective.to_string() == METADATA_V4 || effective.to_string() == METADATA_V6 {
|
||||
if is_metadata_capable_space(effective) {
|
||||
return false;
|
||||
}
|
||||
if self
|
||||
|
|
@ -158,6 +155,18 @@ fn is_global_unicast(address: IpAddr) -> bool {
|
|||
}
|
||||
}
|
||||
|
||||
fn is_metadata_capable_space(address: IpAddr) -> bool {
|
||||
match address {
|
||||
IpAddr::V4(address) => {
|
||||
let [first, second, third, _] = address.octets();
|
||||
(first == 100 && (64..=127).contains(&second))
|
||||
|| (first == 169 && second == 254)
|
||||
|| (first == 192 && second == 0 && third == 0)
|
||||
}
|
||||
IpAddr::V6(address) => address.segments()[0] & 0xfe00 == 0xfc00,
|
||||
}
|
||||
}
|
||||
|
||||
fn is_global_ipv4(address: Ipv4Addr) -> bool {
|
||||
if address.is_unspecified()
|
||||
|| address.is_loopback()
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
use std::net::SocketAddr;
|
||||
use std::{fmt, net::SocketAddr};
|
||||
|
||||
use http::{HeaderMap, HeaderName, Method, StatusCode};
|
||||
use reqwest::{Certificate, redirect::Policy};
|
||||
|
|
@ -25,11 +25,11 @@ pub struct OutboundClientBuilder<R> {
|
|||
client: OutboundClient<R>,
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug)]
|
||||
pub struct OutboundRequest {
|
||||
method: Method,
|
||||
url: CanonicalUrl,
|
||||
headers: HeaderMap,
|
||||
credential_headers: HeaderMap,
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
|
|
@ -58,9 +58,11 @@ impl<R: Resolver> OutboundClient<R> {
|
|||
loop {
|
||||
let endpoint = self.policy.authorize(&request.url, &self.resolver).await?;
|
||||
let client = self.client_for(&request.url, &endpoint)?;
|
||||
let mut headers = request.headers.clone();
|
||||
headers.extend(request.credential_headers.clone());
|
||||
let response = client
|
||||
.request(request.method.clone(), request.url.as_url().clone())
|
||||
.headers(request.headers.clone())
|
||||
.headers(headers)
|
||||
.send()
|
||||
.await
|
||||
.map_err(|_| OutboundError::Transport)?;
|
||||
|
|
@ -84,7 +86,7 @@ impl<R: Resolver> OutboundClient<R> {
|
|||
return Err(OutboundError::HttpsDowngrade);
|
||||
}
|
||||
if request.url.origin()? != next.origin()? {
|
||||
strip_cross_origin_credentials(&mut request.headers);
|
||||
request.credential_headers.clear();
|
||||
}
|
||||
request.url = next;
|
||||
redirects += 1;
|
||||
|
|
@ -131,6 +133,7 @@ impl OutboundRequest {
|
|||
method: Method::GET,
|
||||
url: CanonicalUrl::parse(url)?,
|
||||
headers: HeaderMap::new(),
|
||||
credential_headers: HeaderMap::new(),
|
||||
})
|
||||
}
|
||||
|
||||
|
|
@ -139,19 +142,42 @@ impl OutboundRequest {
|
|||
method: Method::HEAD,
|
||||
url: CanonicalUrl::parse(url)?,
|
||||
headers: HeaderMap::new(),
|
||||
credential_headers: HeaderMap::new(),
|
||||
})
|
||||
}
|
||||
|
||||
pub fn with_headers(mut self, headers: HeaderMap) -> Result<Self, OutboundError> {
|
||||
if headers.contains_key(http::header::HOST)
|
||||
|| headers.contains_key(http::header::CONNECTION)
|
||||
|| headers.contains_key(http::header::PROXY_AUTHORIZATION)
|
||||
pub fn with_public_headers(mut self, headers: HeaderMap) -> Result<Self, OutboundError> {
|
||||
if headers
|
||||
.keys()
|
||||
.any(|name| !is_public_header(name) || self.credential_headers.contains_key(name))
|
||||
{
|
||||
return Err(OutboundError::InvalidHeaders);
|
||||
}
|
||||
self.headers = headers;
|
||||
Ok(self)
|
||||
}
|
||||
|
||||
pub fn with_credential_headers(mut self, headers: HeaderMap) -> Result<Self, OutboundError> {
|
||||
if headers
|
||||
.keys()
|
||||
.any(|name| is_connection_controlled_header(name) || self.headers.contains_key(name))
|
||||
{
|
||||
return Err(OutboundError::InvalidHeaders);
|
||||
}
|
||||
self.credential_headers = headers;
|
||||
Ok(self)
|
||||
}
|
||||
}
|
||||
|
||||
impl fmt::Debug for OutboundRequest {
|
||||
fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
|
||||
formatter
|
||||
.debug_struct("OutboundRequest")
|
||||
.field("method", &self.method)
|
||||
.field("header_count", &self.headers.len())
|
||||
.field("credential_header_count", &self.credential_headers.len())
|
||||
.finish_non_exhaustive()
|
||||
}
|
||||
}
|
||||
|
||||
impl OutboundResponse {
|
||||
|
|
@ -171,12 +197,37 @@ impl OutboundResponse {
|
|||
}
|
||||
}
|
||||
|
||||
fn strip_cross_origin_credentials(headers: &mut HeaderMap) {
|
||||
for name in [
|
||||
http::header::AUTHORIZATION,
|
||||
http::header::COOKIE,
|
||||
HeaderName::from_static("cookie2"),
|
||||
] {
|
||||
headers.remove(name);
|
||||
fn is_public_header(name: &HeaderName) -> bool {
|
||||
matches!(
|
||||
name.as_str(),
|
||||
"accept"
|
||||
| "accept-charset"
|
||||
| "accept-encoding"
|
||||
| "accept-language"
|
||||
| "cache-control"
|
||||
| "if-match"
|
||||
| "if-modified-since"
|
||||
| "if-none-match"
|
||||
| "if-range"
|
||||
| "if-unmodified-since"
|
||||
| "pragma"
|
||||
| "range"
|
||||
| "user-agent"
|
||||
)
|
||||
}
|
||||
|
||||
fn is_connection_controlled_header(name: &HeaderName) -> bool {
|
||||
matches!(
|
||||
name.as_str(),
|
||||
"connection"
|
||||
| "content-length"
|
||||
| "host"
|
||||
| "keep-alive"
|
||||
| "proxy-authenticate"
|
||||
| "proxy-authorization"
|
||||
| "te"
|
||||
| "trailer"
|
||||
| "transfer-encoding"
|
||||
| "upgrade"
|
||||
)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -7,7 +7,7 @@ use std::{
|
|||
|
||||
use ipnet::IpNet;
|
||||
use remora_outbound::{
|
||||
CanonicalUrl, OutboundError, OutboundPolicy, OutboundPolicyBuilder, Resolver,
|
||||
CanonicalUrl, OutboundError, OutboundPolicy, OutboundPolicyBuilder, Resolver, SystemResolver,
|
||||
};
|
||||
|
||||
#[derive(Clone, Default)]
|
||||
|
|
@ -163,7 +163,18 @@ async fn allowlists_never_override_metadata_denial() {
|
|||
authorize_literal(&policy, "10.1.2.3")
|
||||
.await
|
||||
.expect("allowlisted private address");
|
||||
for address in ["169.254.169.254", "fd00:ec2::254", "::ffff:169.254.169.254"] {
|
||||
for address in [
|
||||
"169.254.169.254",
|
||||
"169.254.170.2",
|
||||
"169.254.0.23",
|
||||
"::ffff:169.254.170.2",
|
||||
"100.100.100.200",
|
||||
"::ffff:100.100.100.200",
|
||||
"192.0.0.192",
|
||||
"fd00:ec2::254",
|
||||
"fd20:ce::254",
|
||||
"fd00:db8:deca:deed::fe",
|
||||
] {
|
||||
assert_eq!(
|
||||
authorize_literal(&policy, address).await,
|
||||
Err(OutboundError::AddressDenied)
|
||||
|
|
@ -171,6 +182,19 @@ async fn allowlists_never_override_metadata_denial() {
|
|||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn system_resolver_returns_complete_deduplicated_localhost_answers() {
|
||||
let answers = SystemResolver
|
||||
.resolve("localhost", 80)
|
||||
.await
|
||||
.expect("localhost resolution");
|
||||
assert!(!answers.is_empty());
|
||||
assert!(answers.iter().all(IpAddr::is_loopback));
|
||||
for (index, address) in answers.iter().enumerate() {
|
||||
assert!(!answers[..index].contains(address));
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn scheme_port_policy_is_exact() {
|
||||
let resolver = ScriptedResolver::with("example.test", vec![vec![ip("93.184.216.34")]]);
|
||||
|
|
|
|||
|
|
@ -32,6 +32,7 @@ struct ObservedRequest {
|
|||
host: Option<String>,
|
||||
authorization: Option<String>,
|
||||
cookie: Option<String>,
|
||||
api_key: Option<String>,
|
||||
}
|
||||
|
||||
impl ScriptedResolver {
|
||||
|
|
@ -134,6 +135,7 @@ async fn read_request(stream: &mut (impl AsyncRead + Unpin)) -> ObservedRequest
|
|||
host: headers.get("host").cloned(),
|
||||
authorization: headers.get("authorization").cloned(),
|
||||
cookie: headers.get("cookie").cloned(),
|
||||
api_key: headers.get("x-api-key").cloned(),
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -201,9 +203,10 @@ async fn redirects_reauthorize_and_strip_cross_origin_credentials() {
|
|||
HeaderValue::from_static("Bearer synthetic"),
|
||||
);
|
||||
headers.insert("cookie", HeaderValue::from_static("session=synthetic"));
|
||||
headers.insert("x-api-key", HeaderValue::from_static("synthetic-api-key"));
|
||||
let request = OutboundRequest::get(&format!("http://source.test:{port}/redirect"))
|
||||
.expect("request")
|
||||
.with_headers(headers)
|
||||
.with_credential_headers(headers)
|
||||
.expect("headers");
|
||||
let result = client.execute(request).await.expect("response");
|
||||
assert_eq!(result.final_url().host_str(), "other.test");
|
||||
|
|
@ -214,12 +217,40 @@ async fn redirects_reauthorize_and_strip_cross_origin_credentials() {
|
|||
Some("Bearer synthetic")
|
||||
);
|
||||
assert_eq!(observed[0].cookie.as_deref(), Some("session=synthetic"));
|
||||
assert_eq!(observed[0].api_key.as_deref(), Some("synthetic-api-key"));
|
||||
assert_eq!(observed[1].authorization, None);
|
||||
assert_eq!(observed[1].cookie, None);
|
||||
assert_eq!(observed[1].api_key, None);
|
||||
assert_eq!(resolver.calls("source.test"), 1);
|
||||
assert_eq!(resolver.calls("other.test"), 1);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn arbitrary_headers_must_be_classified_as_credentials() {
|
||||
let mut headers = HeaderMap::new();
|
||||
headers.insert("x-api-key", HeaderValue::from_static("synthetic-api-key"));
|
||||
assert_eq!(
|
||||
OutboundRequest::get("https://example.com/")
|
||||
.expect("request")
|
||||
.with_public_headers(headers)
|
||||
.expect_err("unclassified header"),
|
||||
OutboundError::InvalidHeaders
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn request_debug_redacts_url_and_credential_values() {
|
||||
let mut headers = HeaderMap::new();
|
||||
headers.insert("x-api-key", HeaderValue::from_static("synthetic-api-key"));
|
||||
let request = OutboundRequest::get("https://example.com/sensitive?token=synthetic")
|
||||
.expect("request")
|
||||
.with_credential_headers(headers)
|
||||
.expect("credential headers");
|
||||
let debug = format!("{request:?}");
|
||||
assert!(!debug.contains("sensitive"));
|
||||
assert!(!debug.contains("synthetic-api-key"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn denied_redirect_is_rejected_before_dial() {
|
||||
let (port, _, server) = serve_http(1, |request| {
|
||||
|
|
@ -375,6 +406,62 @@ async fn tls_verification_uses_canonical_hostname_not_pinned_address() {
|
|||
server.await.expect("server");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn ipv6_pinning_preserves_http_authority_and_tls_server_name() {
|
||||
let certified =
|
||||
rcgen::generate_simple_self_signed(vec!["source6.test".to_owned()]).expect("certificate");
|
||||
let certificate_der = certified.cert.der().clone();
|
||||
let key_der = certified.signing_key.serialize_der();
|
||||
let tls = rustls::ServerConfig::builder()
|
||||
.with_no_client_auth()
|
||||
.with_single_cert(
|
||||
vec![certificate_der.clone()],
|
||||
rustls::pki_types::PrivateKeyDer::Pkcs8(key_der.into()),
|
||||
)
|
||||
.expect("TLS config");
|
||||
let listener = match TcpListener::bind("[::1]:0").await {
|
||||
Ok(listener) => listener,
|
||||
Err(error)
|
||||
if error.kind() == io::ErrorKind::AddrNotAvailable
|
||||
&& std::env::var_os("REMORA_OUTBOUND_REQUIRE_IPV6").is_none() =>
|
||||
{
|
||||
return;
|
||||
}
|
||||
Err(error) => panic!("IPv6 bind: {error}"),
|
||||
};
|
||||
let port = listener.local_addr().expect("address").port();
|
||||
let server = tokio::spawn(async move {
|
||||
let (stream, _) = listener.accept().await.expect("accept");
|
||||
let mut stream = tokio_rustls::TlsAcceptor::from(Arc::new(tls))
|
||||
.accept(stream)
|
||||
.await
|
||||
.expect("TLS handshake");
|
||||
let request = read_request(&mut stream).await;
|
||||
assert_eq!(
|
||||
request.host.as_deref(),
|
||||
Some(format!("source6.test:{port}").as_str())
|
||||
);
|
||||
stream
|
||||
.write_all(response("200 OK", &[], "ok").as_bytes())
|
||||
.await
|
||||
.expect("response");
|
||||
});
|
||||
let policy = OutboundPolicyBuilder::new()
|
||||
.allow_scheme_port("https", port)
|
||||
.allow_non_global("::1/128".parse::<IpNet>().expect("network"))
|
||||
.build();
|
||||
let resolver = ScriptedResolver::new([("source6.test", vec![vec![ip("::1")]])]);
|
||||
let root = reqwest::Certificate::from_der(certificate_der.as_ref()).expect("root");
|
||||
let client = OutboundClient::builder(resolver, policy)
|
||||
.add_root_certificate(root)
|
||||
.build();
|
||||
client
|
||||
.execute(OutboundRequest::get(&format!("https://source6.test:{port}/")).expect("request"))
|
||||
.await
|
||||
.expect("IPv6 TLS response");
|
||||
server.await.expect("server");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn https_redirect_cannot_downgrade_to_http() {
|
||||
let certified =
|
||||
|
|
|
|||
Loading…
Reference in a new issue