proxy: improve transparency of host headers and absolute-uris (#535)

In some cases, we would adjust an existing Host header, or add one. And in all cases when an HTTP/1 request was received with an absolute-form target, it was not passed on.

Now, the Host header is never changed. And if the Uri was in absolute-form, it is sent in the same format.

Closes #518
This commit is contained in:
Sean McArthur 2018-03-08 13:15:21 -08:00 committed by GitHub
parent 1b4a426d16
commit 83d6a1f579
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 68 additions and 70 deletions

6
Cargo.lock generated
View File

@ -111,7 +111,7 @@ dependencies = [
"h2 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)",
"http 0.1.5 (registry+https://github.com/rust-lang/crates.io-index)",
"httparse 1.2.4 (registry+https://github.com/rust-lang/crates.io-index)",
"hyper 0.11.21 (registry+https://github.com/rust-lang/crates.io-index)",
"hyper 0.11.22 (registry+https://github.com/rust-lang/crates.io-index)",
"indexmap 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)",
"ipnet 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)",
"libc 0.2.36 (registry+https://github.com/rust-lang/crates.io-index)",
@ -287,7 +287,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
[[package]]
name = "hyper"
version = "0.11.21"
version = "0.11.22"
source = "registry+https://github.com/rust-lang/crates.io-index"
dependencies = [
"base64 0.9.0 (registry+https://github.com/rust-lang/crates.io-index)",
@ -961,7 +961,7 @@ dependencies = [
"checksum heck 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)" = "ea04fa3ead4e05e51a7c806fc07271fdbde4e246a6c6d1efd52e72230b771b82"
"checksum http 0.1.5 (registry+https://github.com/rust-lang/crates.io-index)" = "75df369fd52c60635208a4d3e694777c099569b3dcf4844df8f652dc004644ab"
"checksum httparse 1.2.4 (registry+https://github.com/rust-lang/crates.io-index)" = "c2f407128745b78abc95c0ffbe4e5d37427fdc0d45470710cfef8c44522a2e37"
"checksum hyper 0.11.21 (registry+https://github.com/rust-lang/crates.io-index)" = "a3a77dea5dccbf32ba4e9ddd7d80a5a3bb3b9f1f3835e18daf5dbea6bee0efbf"
"checksum hyper 0.11.22 (registry+https://github.com/rust-lang/crates.io-index)" = "d595f999e90624f64d2c4bc74c72adb0f3e0f773dc5692ca91338363b3568fa0"
"checksum indexmap 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)" = "7164c96d6e18ccc3ce43f3dedac996c21a220670a106c275b96ad92110401362"
"checksum iovec 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)" = "dbe6e417e7d0975db6512b90796e8ce223145ac4e33c377e4a42882a0e88bb08"
"checksum ipnet 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "51268c3a27ad46afd1cca0bbf423a5be2e9fd3e6a7534736c195f0f834b763ef"

View File

@ -22,7 +22,7 @@ futures = "0.1"
h2 = "0.1"
http = "0.1"
httparse = "1.2"
hyper = { version = "0.11.21", default-features = false, features = ["compat"] }
hyper = { version = "0.11.22", default-features = false, features = ["compat"] }
ipnet = "1.0"
log = "0.4.1"
indexmap = "0.4.1"

View File

@ -6,8 +6,8 @@ use std::net::SocketAddr;
use std::sync::Arc;
use std::sync::atomic::AtomicUsize;
use futures::{future, Future, Poll};
use futures::future::{Either, Map};
use futures::{Future, Poll};
use futures::future::Map;
use http::{self, uri};
use tokio_core::reactor::Handle;
use tower;
@ -69,11 +69,11 @@ pub enum Host {
/// the authority given in the `Host:` header, or, failing that, from the
/// request's original destination according to `SO_ORIGINAL_DST`.
#[derive(Copy, Clone, Debug)]
pub struct ReconstructUri<S> {
pub struct NormalizeUri<S> {
inner: S
}
pub type Service<B> = Reconnect<ReconstructUri<NewHttp<B>>>;
pub type Service<B> = Reconnect<NormalizeUri<NewHttp<B>>>;
pub type NewHttp<B> = sensor::NewHttp<Client<B>, B, HttpBody>;
@ -204,7 +204,7 @@ where
// Rewrite the HTTP/1 URI, if the authorities in the Host header
// and request URI are not in agreement, or are not present.
let proxy = ReconstructUri::new(sensors);
let proxy = NormalizeUri::new(sensors);
// Automatically perform reconnects if the connection fails.
//
@ -241,16 +241,16 @@ where
}
// ===== impl ReconstructUri =====
// ===== impl NormalizeUri =====
impl<S> ReconstructUri<S> {
impl<S> NormalizeUri<S> {
fn new (inner: S) -> Self {
Self { inner }
}
}
impl<S, B> tower::NewService for ReconstructUri<S>
impl<S, B> tower::NewService for NormalizeUri<S>
where
S: tower::NewService<
Request=http::Request<B>,
@ -260,24 +260,24 @@ where
Request=http::Request<B>,
Response=HttpResponse,
>,
ReconstructUri<S::Service>: tower::Service,
NormalizeUri<S::Service>: tower::Service,
B: tower_h2::Body,
{
type Request = <Self::Service as tower::Service>::Request;
type Response = <Self::Service as tower::Service>::Response;
type Error = <Self::Service as tower::Service>::Error;
type Service = ReconstructUri<S::Service>;
type Service = NormalizeUri<S::Service>;
type InitError = S::InitError;
type Future = Map<
S::Future,
fn(S::Service) -> ReconstructUri<S::Service>
fn(S::Service) -> NormalizeUri<S::Service>
>;
fn new_service(&self) -> Self::Future {
self.inner.new_service().map(ReconstructUri::new)
self.inner.new_service().map(NormalizeUri::new)
}
}
impl<S, B> tower::Service for ReconstructUri<S>
impl<S, B> tower::Service for NormalizeUri<S>
where
S: tower::Service<
Request=http::Request<B>,
@ -288,29 +288,17 @@ where
type Request = S::Request;
type Response = HttpResponse;
type Error = S::Error;
type Future = Either<
S::Future,
future::FutureResult<Self::Response, Self::Error>,
>;
type Future = S::Future;
fn poll_ready(&mut self) -> Poll<(), S::Error> {
self.inner.poll_ready()
}
fn call(&mut self, mut request: S::Request) -> Self::Future {
if request.version() == http::Version::HTTP_2 {
// skip `reconstruct_uri` entirely if the request is HTTP/2.
return Either::A(self.inner.call(request));
if request.version() != http::Version::HTTP_2 {
h1::normalize_our_view_of_uri(&mut request);
}
if let Err(_) = h1::reconstruct_uri(&mut request) {
let res = http::Response::builder()
.status(http::StatusCode::BAD_REQUEST)
.body(Default::default())
.unwrap();
return Either::B(future::ok(res));
}
Either::A(self.inner.call(request))
self.inner.call(request)
}
}

View File

@ -9,6 +9,7 @@ use tower_h2;
use bind;
use super::glue::{BodyStream, HttpBody, HyperConnect};
use super::h1::UriIsAbsoluteForm;
/// A `NewService` that can speak either HTTP/1 or HTTP/2.
pub struct Client<C, B>
@ -83,6 +84,9 @@ where
let h1 = hyper::Client::configure()
.connector(HyperConnect::new(connect))
.body()
// hyper should never try to automatically set the Host
// header, instead always just passing whatever we received.
.set_host(false)
.build(&executor);
Client {
inner: ClientInner::Http1(h1),
@ -177,6 +181,12 @@ where
match self.inner {
ClientServiceInner::Http1(ref h1) => {
// This sentinel may be set in h1::normalize_our_view_of_uri
// when the original request-target was in absolute-form. In
// that case, for hyper 0.11.x, we need to call `req.set_proxy`.
let was_absolute_form = req.extensions()
.get::<UriIsAbsoluteForm>()
.is_some();
// As of hyper 0.11.x, a set body implies a body must be sent.
// If there is no content-length header, hyper adds a
// transfer-encoding: chunked header, since it has no other way
@ -206,6 +216,7 @@ where
if should_take_body {
req.body_mut().take();
}
req.set_proxy(was_absolute_form);
ClientServiceFuture::Http1(h1.request(req))
},
ClientServiceInner::Http2(ref mut h2) => {

View File

@ -4,31 +4,29 @@ use std::sync::Arc;
use bytes::BytesMut;
use http;
use http::header::{HeaderValue, HOST};
use http::header::HOST;
use http::uri::{Authority, Parts, Scheme, Uri};
use ctx::transport::{Server as ServerCtx};
pub fn reconstruct_uri<B>(req: &mut http::Request<B>) -> Result<(), ()> {
// RFC7230#section-5.4
// If an absolute-form uri is received, it must replace
// the host header
if let Some(auth) = req.uri().authority_part().cloned() {
if let Some(host) = req.headers().get(HOST) {
if auth.as_str().as_bytes() == host.as_bytes() {
// host and absolute-form agree, nothing more to do
return Ok(());
}
}
let host = HeaderValue::from_shared(auth.into_bytes())
.expect("a valid authority is valid header value");
req.headers_mut().insert(HOST, host);
return Ok(());
/// Sentinel extension to signal that we should tell hyper to serialize
/// the `Uri` in absolute-form.
pub struct UriIsAbsoluteForm;
/// Tries to make sure the `Uri` of the request is in a form needed by
/// hyper's Client.
///
/// Also sets the `UriIsAbsoluteForm` extension if received `Uri` was
/// already in absolute-form.
pub fn normalize_our_view_of_uri<B>(req: &mut http::Request<B>) {
if req.uri().authority_part().is_some() {
req.extensions_mut().insert(UriIsAbsoluteForm);
return;
}
// try to parse the Host header
if let Some(auth) = authority_from_host(&req) {
set_authority(req.uri_mut(), auth);
return Ok(());
return;
}
// last resort is to use the so_original_dst
@ -43,11 +41,7 @@ pub fn reconstruct_uri<B>(req: &mut http::Request<B>) -> Result<(), ()> {
let auth = Authority::from_shared(bytes)
.expect("socket address is valid authority");
set_authority(req.uri_mut(), auth);
return Ok(());
}
Err(())
}
/// Returns an Authority from a request's Host header.

View File

@ -117,6 +117,8 @@ fn http10_without_host() {
let srv = server::http1()
.route_fn("/", move |req| {
assert_eq!(req.version(), http::Version::HTTP_10);
assert!(!req.headers().contains_key("host"));
assert_eq!(req.uri().to_string(), "/");
Response::builder()
.version(http::Version::HTTP_10)
.body("".into())
@ -124,34 +126,37 @@ fn http10_without_host() {
})
.run();
let ctrl = controller::new()
.destination(&srv.addr.to_string(), srv.addr)
.run();
let proxy = proxy::new()
.controller(ctrl)
.outbound(srv)
.inbound(srv)
.run();
let client = client::http1(proxy.outbound, "foo.bar");
let res = client.request(client.request_builder("/")
.version(http::Version::HTTP_10)
.header("host", ""));
let client = client::tcp(proxy.inbound);
assert_eq!(res.status(), http::StatusCode::OK);
assert_eq!(res.version(), http::Version::HTTP_10);
let tcp_client = client.connect();
tcp_client.write("\
GET / HTTP/1.0\r\n\
\r\n\
");
let expected = "HTTP/1.0 200 OK\r\n";
assert_eq!(s(&tcp_client.read()[..expected.len()]), expected);
}
#[test]
fn http11_absolute_uri_differs_from_host() {
let _ = env_logger::try_init();
let host = "transparency.test.svc.cluster.local";
// We shouldn't touch the URI or the Host, just pass directly as we got.
let auth = "transparency.test.svc.cluster.local";
let host = "foo.bar";
let srv = server::http1()
.route_fn("/", move |req| {
assert_eq!(req.version(), http::Version::HTTP_11);
assert_eq!(req.headers().get("host").unwrap(), host);
Response::builder()
.body("".into())
.unwrap()
assert_eq!(req.headers()["host"], host);
assert_eq!(req.uri().to_string(), format!("http://{}/", auth));
Response::new("".into())
})
.run();
let ctrl = controller::new().run();
@ -159,11 +164,11 @@ fn http11_absolute_uri_differs_from_host() {
.controller(ctrl)
.inbound(srv)
.run();
let client = client::http1_absolute_uris(proxy.inbound, host);
let client = client::http1_absolute_uris(proxy.inbound, auth);
let res = client.request(client.request_builder("/")
.version(http::Version::HTTP_11)
.header("host", "foo.bar"));
.header("host", host));
assert_eq!(res.status(), http::StatusCode::OK);
assert_eq!(res.version(), http::Version::HTTP_11);