From 0558e632527a03cfb57c499dcf124508fd95abf3 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Fri, 8 Jul 2022 13:34:20 -0700 Subject: [PATCH 1/7] wip Signed-off-by: Eliza Weisman --- linkerd/app/inbound/src/policy/http.rs | 17 ++++++++++ linkerd/http-route/src/http/filter.rs | 2 ++ .../src/http/filter/forwarded_for.rs | 34 +++++++++++++++++++ linkerd/server-policy/src/grpc.rs | 1 + linkerd/server-policy/src/http.rs | 1 + 5 files changed, 55 insertions(+) create mode 100644 linkerd/http-route/src/http/filter/forwarded_for.rs diff --git a/linkerd/app/inbound/src/policy/http.rs b/linkerd/app/inbound/src/policy/http.rs index 976efd1a13..750ff9e900 100644 --- a/linkerd/app/inbound/src/policy/http.rs +++ b/linkerd/app/inbound/src/policy/http.rs @@ -10,6 +10,7 @@ use linkerd_app_core::{ tls, transport::{ClientAddr, OrigDstAddr, Remote}, Error, Result, + proxy::http::ClientHandle, }; use linkerd_server_policy::{grpc, http, route::RouteMatch}; use std::{sync::Arc, task}; @@ -285,6 +286,14 @@ fn apply_http_filters( http::Filter::RequestHeaders(rh) => { rh.apply(req.headers_mut()); } + + http::Filter::ForwardedFor(ff) => { + if let Some(client) = req.extensions().get::() { + ff.apply(client.addr, req.headers_mut()); + } else { + debug_assert!(false, "missing `ClientHandle` extension!") + } + } } } @@ -303,6 +312,14 @@ fn apply_grpc_filters(route: &grpc::Policy, req: &mut ::http::Request) -> grpc::Filter::RequestHeaders(rh) => { rh.apply(req.headers_mut()); } + + grpc::Filter::ForwardedFor(ff) => { + if let Some(client) = req.extensions().get::() { + ff.apply(client.addr, req.headers_mut()); + } else { + debug_assert!(false, "missing `ClientHandle` extension!") + } + } } } diff --git a/linkerd/http-route/src/http/filter.rs b/linkerd/http-route/src/http/filter.rs index 4ba5771dcc..7df2e8c934 100644 --- a/linkerd/http-route/src/http/filter.rs +++ b/linkerd/http-route/src/http/filter.rs @@ -1,11 +1,13 @@ pub mod inject_failure; pub mod modify_header; pub mod redirect; +pub mod forwarded_for; pub use self::{ inject_failure::{Distribution, FailureResponse, InjectFailure}, modify_header::ModifyHeader, redirect::{InvalidRedirect, RedirectRequest, Redirection}, + forwarded_for::ForwardedFor, }; #[derive(Clone, Debug, Hash, PartialEq, Eq)] diff --git a/linkerd/http-route/src/http/filter/forwarded_for.rs b/linkerd/http-route/src/http/filter/forwarded_for.rs new file mode 100644 index 0000000000..c66b8772a3 --- /dev/null +++ b/linkerd/http-route/src/http/filter/forwarded_for.rs @@ -0,0 +1,34 @@ +use http::{ + header::{HeaderMap, HeaderName, HeaderValue}, +}; +use std::net::SocketAddr; + +#[derive(Clone, Debug, Default, Hash, PartialEq, Eq)] +pub struct ForwardedFor { + headers: Vec
, +} + +#[derive(Clone, Debug, Hash, PartialEq, Eq)] +pub struct Header { + pub name: HeaderName, + pub overwrite: bool, +} + +// === impl ForwardedFor === + +impl ForwardedFor { + pub fn apply(&self, client_addr: SocketAddr, headers: &mut HeaderMap) { + if self.headers.is_empty() { + return; + } + + let value = HeaderValue::try_from(client_addr.to_string()).expect("a SocketAddr should be a valid header value"); + for Header { name, overwrite } in &self.headers { + if *overwrite { + headers.insert(name.clone(), value.clone()); + } else { + headers.append(name.clone(), value.clone()); + } + } + } +} diff --git a/linkerd/server-policy/src/grpc.rs b/linkerd/server-policy/src/grpc.rs index 98ea167e39..5d58547890 100644 --- a/linkerd/server-policy/src/grpc.rs +++ b/linkerd/server-policy/src/grpc.rs @@ -9,6 +9,7 @@ pub type Rule = grpc::Rule; pub enum Filter { InjectFailure(filter::InjectFailure), RequestHeaders(http::filter::ModifyHeader), + ForwardedFor(http::filter::ForwardedFor) } #[inline] diff --git a/linkerd/server-policy/src/http.rs b/linkerd/server-policy/src/http.rs index 1a4543cade..e5252b7085 100644 --- a/linkerd/server-policy/src/http.rs +++ b/linkerd/server-policy/src/http.rs @@ -10,6 +10,7 @@ pub enum Filter { InjectFailure(filter::InjectFailure), Redirect(filter::RedirectRequest), RequestHeaders(filter::ModifyHeader), + ForwardedFor(filter::ForwardedFor), } #[inline] From adcb454fd688b6d7bd7a8e113eaf7f198c1ec364 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Fri, 8 Jul 2022 13:41:39 -0700 Subject: [PATCH 2/7] maybe this is nicer Signed-off-by: Eliza Weisman --- linkerd/app/inbound/src/policy/http.rs | 6 +-- linkerd/http-route/src/http/filter.rs | 4 +- .../src/http/filter/client_addr_headers.rs | 40 +++++++++++++++++++ .../src/http/filter/forwarded_for.rs | 34 ---------------- linkerd/server-policy/src/grpc.rs | 2 +- linkerd/server-policy/src/http.rs | 2 +- 6 files changed, 47 insertions(+), 41 deletions(-) create mode 100644 linkerd/http-route/src/http/filter/client_addr_headers.rs delete mode 100644 linkerd/http-route/src/http/filter/forwarded_for.rs diff --git a/linkerd/app/inbound/src/policy/http.rs b/linkerd/app/inbound/src/policy/http.rs index 750ff9e900..c17ea6fd31 100644 --- a/linkerd/app/inbound/src/policy/http.rs +++ b/linkerd/app/inbound/src/policy/http.rs @@ -6,11 +6,11 @@ use crate::{ use futures::{future, TryFutureExt}; use linkerd_app_core::{ metrics::{RouteAuthzLabels, RouteLabels}, + proxy::http::ClientHandle, svc::{self, ServiceExt}, tls, transport::{ClientAddr, OrigDstAddr, Remote}, Error, Result, - proxy::http::ClientHandle, }; use linkerd_server_policy::{grpc, http, route::RouteMatch}; use std::{sync::Arc, task}; @@ -287,7 +287,7 @@ fn apply_http_filters( rh.apply(req.headers_mut()); } - http::Filter::ForwardedFor(ff) => { + http::Filter::ClientAddrHeaders(ff) => { if let Some(client) = req.extensions().get::() { ff.apply(client.addr, req.headers_mut()); } else { @@ -313,7 +313,7 @@ fn apply_grpc_filters(route: &grpc::Policy, req: &mut ::http::Request) -> rh.apply(req.headers_mut()); } - grpc::Filter::ForwardedFor(ff) => { + grpc::Filter::ClientAddrHeaders(ff) => { if let Some(client) = req.extensions().get::() { ff.apply(client.addr, req.headers_mut()); } else { diff --git a/linkerd/http-route/src/http/filter.rs b/linkerd/http-route/src/http/filter.rs index 7df2e8c934..ec664895e3 100644 --- a/linkerd/http-route/src/http/filter.rs +++ b/linkerd/http-route/src/http/filter.rs @@ -1,13 +1,13 @@ +pub mod client_addr_headers; pub mod inject_failure; pub mod modify_header; pub mod redirect; -pub mod forwarded_for; pub use self::{ + client_addr_headers::ClientAddrHeaders, inject_failure::{Distribution, FailureResponse, InjectFailure}, modify_header::ModifyHeader, redirect::{InvalidRedirect, RedirectRequest, Redirection}, - forwarded_for::ForwardedFor, }; #[derive(Clone, Debug, Hash, PartialEq, Eq)] diff --git a/linkerd/http-route/src/http/filter/client_addr_headers.rs b/linkerd/http-route/src/http/filter/client_addr_headers.rs new file mode 100644 index 0000000000..adeba113f4 --- /dev/null +++ b/linkerd/http-route/src/http/filter/client_addr_headers.rs @@ -0,0 +1,40 @@ +use http::header::{HeaderMap, HeaderName, HeaderValue}; +use std::net::SocketAddr; + +/// Adds or sets HTTP headers containing the client's IP address. +/// +/// This is typically used to add headers such as +/// `Forwarded-For`, `X-Forwarded-For`, and friends. +#[derive(Clone, Debug, Default, Hash, PartialEq, Eq)] +pub struct ClientAddrHeaders { + headers: Vec<(HeaderName, Action)>, +} + +#[derive(Copy, Clone, Debug, Hash, PartialEq, Eq)] +pub enum Action { + Add, + Set, +} + +// === impl ForwardedFor === + +impl ClientAddrHeaders { + pub fn apply(&self, client_addr: SocketAddr, headers: &mut HeaderMap) { + if self.headers.is_empty() { + return; + } + + let value = HeaderValue::try_from(client_addr.to_string()) + .expect("a SocketAddr should be a valid header value"); + for (header, action) in &self.headers { + match action { + Action::Add => { + headers.append(header.clone(), value.clone()); + } + Action::Set => { + headers.insert(header.clone(), value.clone()); + } + } + } + } +} diff --git a/linkerd/http-route/src/http/filter/forwarded_for.rs b/linkerd/http-route/src/http/filter/forwarded_for.rs deleted file mode 100644 index c66b8772a3..0000000000 --- a/linkerd/http-route/src/http/filter/forwarded_for.rs +++ /dev/null @@ -1,34 +0,0 @@ -use http::{ - header::{HeaderMap, HeaderName, HeaderValue}, -}; -use std::net::SocketAddr; - -#[derive(Clone, Debug, Default, Hash, PartialEq, Eq)] -pub struct ForwardedFor { - headers: Vec
, -} - -#[derive(Clone, Debug, Hash, PartialEq, Eq)] -pub struct Header { - pub name: HeaderName, - pub overwrite: bool, -} - -// === impl ForwardedFor === - -impl ForwardedFor { - pub fn apply(&self, client_addr: SocketAddr, headers: &mut HeaderMap) { - if self.headers.is_empty() { - return; - } - - let value = HeaderValue::try_from(client_addr.to_string()).expect("a SocketAddr should be a valid header value"); - for Header { name, overwrite } in &self.headers { - if *overwrite { - headers.insert(name.clone(), value.clone()); - } else { - headers.append(name.clone(), value.clone()); - } - } - } -} diff --git a/linkerd/server-policy/src/grpc.rs b/linkerd/server-policy/src/grpc.rs index 5d58547890..70e5f4a978 100644 --- a/linkerd/server-policy/src/grpc.rs +++ b/linkerd/server-policy/src/grpc.rs @@ -9,7 +9,7 @@ pub type Rule = grpc::Rule; pub enum Filter { InjectFailure(filter::InjectFailure), RequestHeaders(http::filter::ModifyHeader), - ForwardedFor(http::filter::ForwardedFor) + ClientAddrHeaders(http::filter::ClientAddrHeaders), } #[inline] diff --git a/linkerd/server-policy/src/http.rs b/linkerd/server-policy/src/http.rs index e5252b7085..5b54077625 100644 --- a/linkerd/server-policy/src/http.rs +++ b/linkerd/server-policy/src/http.rs @@ -10,7 +10,7 @@ pub enum Filter { InjectFailure(filter::InjectFailure), Redirect(filter::RedirectRequest), RequestHeaders(filter::ModifyHeader), - ForwardedFor(filter::ForwardedFor), + ClientAddrHeaders(filter::ClientAddrHeaders), } #[inline] From c55b06d20d255ff436a2e25d5895fcf73aef4d4e Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Fri, 8 Jul 2022 13:46:00 -0700 Subject: [PATCH 3/7] make the filter responsible for getting exn Signed-off-by: Eliza Weisman --- Cargo.lock | 1 + linkerd/app/inbound/src/policy/http.rs | 17 ++++------------- linkerd/http-route/Cargo.toml | 1 + .../src/http/filter/client_addr_headers.rs | 19 ++++++++++++++----- 4 files changed, 20 insertions(+), 18 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4cff9f91a7..8944c70071 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1145,6 +1145,7 @@ name = "linkerd-http-route" version = "0.1.0" dependencies = [ "http", + "linkerd-proxy-http", "rand", "regex", "thiserror", diff --git a/linkerd/app/inbound/src/policy/http.rs b/linkerd/app/inbound/src/policy/http.rs index c17ea6fd31..26f4eef604 100644 --- a/linkerd/app/inbound/src/policy/http.rs +++ b/linkerd/app/inbound/src/policy/http.rs @@ -6,7 +6,6 @@ use crate::{ use futures::{future, TryFutureExt}; use linkerd_app_core::{ metrics::{RouteAuthzLabels, RouteLabels}, - proxy::http::ClientHandle, svc::{self, ServiceExt}, tls, transport::{ClientAddr, OrigDstAddr, Remote}, @@ -287,12 +286,8 @@ fn apply_http_filters( rh.apply(req.headers_mut()); } - http::Filter::ClientAddrHeaders(ff) => { - if let Some(client) = req.extensions().get::() { - ff.apply(client.addr, req.headers_mut()); - } else { - debug_assert!(false, "missing `ClientHandle` extension!") - } + http::Filter::ClientAddrHeaders(c) => { + c.apply(req); } } } @@ -313,12 +308,8 @@ fn apply_grpc_filters(route: &grpc::Policy, req: &mut ::http::Request) -> rh.apply(req.headers_mut()); } - grpc::Filter::ClientAddrHeaders(ff) => { - if let Some(client) = req.extensions().get::() { - ff.apply(client.addr, req.headers_mut()); - } else { - debug_assert!(false, "missing `ClientHandle` extension!") - } + grpc::Filter::ClientAddrHeaders(c) => { + c.apply(req); } } } diff --git a/linkerd/http-route/Cargo.toml b/linkerd/http-route/Cargo.toml index abfa77ddc1..b35dc42771 100644 --- a/linkerd/http-route/Cargo.toml +++ b/linkerd/http-route/Cargo.toml @@ -12,3 +12,4 @@ rand = "0.8" thiserror = "1" tracing = "0.1" url = "2" +linkerd-proxy-http = { path = "../proxy/http" } \ No newline at end of file diff --git a/linkerd/http-route/src/http/filter/client_addr_headers.rs b/linkerd/http-route/src/http/filter/client_addr_headers.rs index adeba113f4..abca177674 100644 --- a/linkerd/http-route/src/http/filter/client_addr_headers.rs +++ b/linkerd/http-route/src/http/filter/client_addr_headers.rs @@ -1,5 +1,7 @@ -use http::header::{HeaderMap, HeaderName, HeaderValue}; -use std::net::SocketAddr; +use http::{ + header::{HeaderName, HeaderValue}, + Request, +}; /// Adds or sets HTTP headers containing the client's IP address. /// @@ -19,13 +21,20 @@ pub enum Action { // === impl ForwardedFor === impl ClientAddrHeaders { - pub fn apply(&self, client_addr: SocketAddr, headers: &mut HeaderMap) { + pub fn apply(&self, req: &mut Request) { if self.headers.is_empty() { return; } + let value = match req.extensions().get::() { + Some(client) => HeaderValue::try_from(client.addr.to_string()) + .expect("a SocketAddr should be a valid header value"), + None => { + debug_assert!(false, "request missing `ClientHandle` extension"); + return; + } + }; - let value = HeaderValue::try_from(client_addr.to_string()) - .expect("a SocketAddr should be a valid header value"); + let headers = req.headers_mut(); for (header, action) in &self.headers { match action { Action::Add => { From 24b1a97a25983f60715631fe095af99efb4384ba Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Fri, 8 Jul 2022 13:52:54 -0700 Subject: [PATCH 4/7] don't include port Signed-off-by: Eliza Weisman --- linkerd/http-route/src/http/filter/client_addr_headers.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/linkerd/http-route/src/http/filter/client_addr_headers.rs b/linkerd/http-route/src/http/filter/client_addr_headers.rs index abca177674..aef6bf5004 100644 --- a/linkerd/http-route/src/http/filter/client_addr_headers.rs +++ b/linkerd/http-route/src/http/filter/client_addr_headers.rs @@ -26,8 +26,8 @@ impl ClientAddrHeaders { return; } let value = match req.extensions().get::() { - Some(client) => HeaderValue::try_from(client.addr.to_string()) - .expect("a SocketAddr should be a valid header value"), + Some(client) => HeaderValue::try_from(client.addr.ip().to_string()) + .expect("an IP address should format as a valid header value"), None => { debug_assert!(false, "request missing `ClientHandle` extension"); return; From 95c220f54587dcf5a123c22977170ec74d0e01da Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Fri, 8 Jul 2022 13:54:12 -0700 Subject: [PATCH 5/7] s/ClientAddrHeaders/ClientIpHeaders Signed-off-by: Eliza Weisman --- linkerd/app/inbound/src/policy/http.rs | 4 ++-- linkerd/http-route/src/http/filter.rs | 4 ++-- .../filter/{client_addr_headers.rs => client_ip_headers.rs} | 4 ++-- linkerd/server-policy/src/grpc.rs | 2 +- linkerd/server-policy/src/http.rs | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-) rename linkerd/http-route/src/http/filter/{client_addr_headers.rs => client_ip_headers.rs} (96%) diff --git a/linkerd/app/inbound/src/policy/http.rs b/linkerd/app/inbound/src/policy/http.rs index 26f4eef604..d1f5ed4f7c 100644 --- a/linkerd/app/inbound/src/policy/http.rs +++ b/linkerd/app/inbound/src/policy/http.rs @@ -286,7 +286,7 @@ fn apply_http_filters( rh.apply(req.headers_mut()); } - http::Filter::ClientAddrHeaders(c) => { + http::Filter::ClientIpHeaders(c) => { c.apply(req); } } @@ -308,7 +308,7 @@ fn apply_grpc_filters(route: &grpc::Policy, req: &mut ::http::Request) -> rh.apply(req.headers_mut()); } - grpc::Filter::ClientAddrHeaders(c) => { + grpc::Filter::ClientIpHeaders(c) => { c.apply(req); } } diff --git a/linkerd/http-route/src/http/filter.rs b/linkerd/http-route/src/http/filter.rs index ec664895e3..3cac51c83f 100644 --- a/linkerd/http-route/src/http/filter.rs +++ b/linkerd/http-route/src/http/filter.rs @@ -1,10 +1,10 @@ -pub mod client_addr_headers; +pub mod client_ip_headers; pub mod inject_failure; pub mod modify_header; pub mod redirect; pub use self::{ - client_addr_headers::ClientAddrHeaders, + client_ip_headers::ClientIpHeaders, inject_failure::{Distribution, FailureResponse, InjectFailure}, modify_header::ModifyHeader, redirect::{InvalidRedirect, RedirectRequest, Redirection}, diff --git a/linkerd/http-route/src/http/filter/client_addr_headers.rs b/linkerd/http-route/src/http/filter/client_ip_headers.rs similarity index 96% rename from linkerd/http-route/src/http/filter/client_addr_headers.rs rename to linkerd/http-route/src/http/filter/client_ip_headers.rs index aef6bf5004..ca29ec0fee 100644 --- a/linkerd/http-route/src/http/filter/client_addr_headers.rs +++ b/linkerd/http-route/src/http/filter/client_ip_headers.rs @@ -8,7 +8,7 @@ use http::{ /// This is typically used to add headers such as /// `Forwarded-For`, `X-Forwarded-For`, and friends. #[derive(Clone, Debug, Default, Hash, PartialEq, Eq)] -pub struct ClientAddrHeaders { +pub struct ClientIpHeaders { headers: Vec<(HeaderName, Action)>, } @@ -20,7 +20,7 @@ pub enum Action { // === impl ForwardedFor === -impl ClientAddrHeaders { +impl ClientIpHeaders { pub fn apply(&self, req: &mut Request) { if self.headers.is_empty() { return; diff --git a/linkerd/server-policy/src/grpc.rs b/linkerd/server-policy/src/grpc.rs index 70e5f4a978..1629b4e74f 100644 --- a/linkerd/server-policy/src/grpc.rs +++ b/linkerd/server-policy/src/grpc.rs @@ -9,7 +9,7 @@ pub type Rule = grpc::Rule; pub enum Filter { InjectFailure(filter::InjectFailure), RequestHeaders(http::filter::ModifyHeader), - ClientAddrHeaders(http::filter::ClientAddrHeaders), + ClientIpHeaders(http::filter::ClientIpHeaders), } #[inline] diff --git a/linkerd/server-policy/src/http.rs b/linkerd/server-policy/src/http.rs index 5b54077625..e2f1c5ef18 100644 --- a/linkerd/server-policy/src/http.rs +++ b/linkerd/server-policy/src/http.rs @@ -10,7 +10,7 @@ pub enum Filter { InjectFailure(filter::InjectFailure), Redirect(filter::RedirectRequest), RequestHeaders(filter::ModifyHeader), - ClientAddrHeaders(filter::ClientAddrHeaders), + ClientIpHeaders(filter::ClientIpHeaders), } #[inline] From 59c15e59268d58ffa8a5d57d873cb4c49152183e Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Fri, 8 Jul 2022 14:10:37 -0700 Subject: [PATCH 6/7] add test Signed-off-by: Eliza Weisman --- linkerd/app/inbound/src/policy/http/tests.rs | 83 ++++++++++++++++++-- 1 file changed, 75 insertions(+), 8 deletions(-) diff --git a/linkerd/app/inbound/src/policy/http/tests.rs b/linkerd/app/inbound/src/policy/http/tests.rs index e34b810738..9cd53d4be8 100644 --- a/linkerd/app/inbound/src/policy/http/tests.rs +++ b/linkerd/app/inbound/src/policy/http/tests.rs @@ -1,7 +1,7 @@ use super::*; use crate::policy::{Authentication, Authorization, Meta, Protocol, ServerPolicy}; use linkerd_app_core::{svc::Service, Infallible}; -use std::sync::Arc; +use std::{net, sync::Arc}; macro_rules! conn { ($client:expr, $dst:expr) => {{ @@ -15,10 +15,12 @@ macro_rules! conn { } }}; () => {{ - conn!([192, 168, 3, 3], [192, 168, 3, 4]) + conn!(CLIENT_IP, [192, 168, 3, 4]) }}; } +const CLIENT_IP: net::IpAddr = net::IpAddr::V4(net::Ipv4Addr::new(192, 168, 3, 3)); + macro_rules! new_svc { ($proto:expr, $conn:expr, $rsp:expr) => {{ let (policy, tx) = AllowPolicy::for_test( @@ -81,7 +83,7 @@ async fn http_route() { policy: Policy { authorizations: Arc::new([Authorization { authentication: Authentication::Unauthenticated, - networks: vec![std::net::IpAddr::from([192, 168, 3, 3]).into()], + networks: vec![CLIENT_IP.into()], meta: Arc::new(Meta::Resource { group: "policy.linkerd.io".into(), kind: "server".into(), @@ -162,7 +164,7 @@ async fn http_filter_header() { policy: Policy { authorizations: Arc::new([Authorization { authentication: Authentication::Unauthenticated, - networks: vec![std::net::IpAddr::from([192, 168, 3, 3]).into()], + networks: vec![CLIENT_IP.into()], meta: Arc::new(Meta::Resource { group: "policy.linkerd.io".into(), kind: "server".into(), @@ -206,6 +208,71 @@ async fn http_filter_header() { assert_eq!(permit.labels.route.route, rmeta); } +#[tokio::test(flavor = "current_thread")] +async fn http_filter_client_ip() { + use linkerd_server_policy::http::{filter, r#match::MatchRequest, Filter, Policy, Route, Rule}; + + let rmeta = Arc::new(Meta::Resource { + group: "gateway.networking.k8s.io".into(), + kind: "httproute".into(), + name: "testrt".into(), + }); + let proto = Protocol::Http1(Arc::new([Route { + hosts: vec![], + rules: vec![Rule { + matches: vec![MatchRequest { + method: Some(::http::Method::GET), + ..MatchRequest::default() + }], + policy: Policy { + authorizations: Arc::new([Authorization { + authentication: Authentication::Unauthenticated, + networks: vec![CLIENT_IP.into()], + meta: Arc::new(Meta::Resource { + group: "policy.linkerd.io".into(), + kind: "server".into(), + name: "testsaz".into(), + }), + }]), + filters: vec![Filter::ClientIpHeaders(filter::ClientIpHeaders { + headers: vec![( + "X-Forwarded-For".parse().unwrap(), + filter::client_ip_headers::Action::Add, + )], + })], + meta: rmeta.clone(), + }, + }], + }])); + let inner = |permit: HttpRoutePermit, req: ::http::Request| -> Result<_> { + assert_eq!(req.headers().len(), 1); + assert_eq!( + req.headers().get("X-Forwarded-For"), + Some(&CLIENT_IP.to_string().parse().unwrap()) + ); + let mut rsp = ::http::Response::builder() + .body(hyper::Body::default()) + .unwrap(); + rsp.extensions_mut().insert(permit); + Ok(rsp) + }; + let (mut svc, _tx) = new_svc!(proto, conn!(), inner); + + let rsp = svc + .call( + ::http::Request::builder() + .body(hyper::Body::default()) + .unwrap(), + ) + .await + .expect("serves"); + let permit = rsp + .extensions() + .get::() + .expect("permitted"); + assert_eq!(permit.labels.route.route, rmeta); +} + #[tokio::test(flavor = "current_thread")] async fn http_filter_inject_failure() { use linkerd_server_policy::http::{filter, r#match::MatchRequest, Filter, Policy, Route, Rule}; @@ -225,7 +292,7 @@ async fn http_filter_inject_failure() { policy: Policy { authorizations: Arc::new([Authorization { authentication: Authentication::Unauthenticated, - networks: vec![std::net::IpAddr::from([192, 168, 3, 3]).into()], + networks: vec![CLIENT_IP.into()], meta: Arc::new(Meta::Resource { group: "policy.linkerd.io".into(), kind: "server".into(), @@ -291,7 +358,7 @@ async fn grpc_route() { policy: Policy { authorizations: Arc::new([Authorization { authentication: Authentication::Unauthenticated, - networks: vec![std::net::IpAddr::from([192, 168, 3, 3]).into()], + networks: vec![CLIENT_IP.into()], meta: Arc::new(Meta::Resource { group: "policy.linkerd.io".into(), kind: "server".into(), @@ -389,7 +456,7 @@ async fn grpc_filter_header() { policy: Policy { authorizations: Arc::new([Authorization { authentication: Authentication::Unauthenticated, - networks: vec![std::net::IpAddr::from([192, 168, 3, 3]).into()], + networks: vec![CLIENT_IP.into()], meta: Arc::new(Meta::Resource { group: "policy.linkerd.io".into(), kind: "server".into(), @@ -462,7 +529,7 @@ async fn grpc_filter_inject_failure() { policy: Policy { authorizations: Arc::new([Authorization { authentication: Authentication::Unauthenticated, - networks: vec![std::net::IpAddr::from([192, 168, 3, 3]).into()], + networks: vec![CLIENT_IP.into()], meta: Arc::new(Meta::Resource { group: "policy.linkerd.io".into(), kind: "server".into(), From 5ff8953ddfd51f0fceb3111037b59072485c63af Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Fri, 8 Jul 2022 14:10:48 -0700 Subject: [PATCH 7/7] actually we can just use conn meta Signed-off-by: Eliza Weisman --- Cargo.lock | 1 - linkerd/app/inbound/src/policy/http.rs | 101 +++++++++--------- linkerd/http-route/Cargo.toml | 3 +- .../src/http/filter/client_ip_headers.rs | 21 ++-- 4 files changed, 60 insertions(+), 66 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8944c70071..4cff9f91a7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1145,7 +1145,6 @@ name = "linkerd-http-route" version = "0.1.0" dependencies = [ "http", - "linkerd-proxy-http", "rand", "regex", "thiserror", diff --git a/linkerd/app/inbound/src/policy/http.rs b/linkerd/app/inbound/src/policy/http.rs index d1f5ed4f7c..56960e4292 100644 --- a/linkerd/app/inbound/src/policy/http.rs +++ b/linkerd/app/inbound/src/policy/http.rs @@ -157,12 +157,12 @@ where None => err!(self.mk_route_not_found()), Some(Routes::Http(routes)) => { let (permit, mtch, route) = try_fut!(self.authorize(&routes, &req)); - try_fut!(apply_http_filters(mtch, route, &mut req)); + try_fut!(self.apply_http_filters(mtch, route, &mut req)); permit } Some(Routes::Grpc(routes)) => { let (permit, _, route) = try_fut!(self.authorize(&routes, &req)); - try_fut!(apply_grpc_filters(route, &mut req)); + try_fut!(self.apply_grpc_filters(route, &mut req)); permit } }; @@ -252,67 +252,72 @@ impl HttpPolicyService { .route_not_found(labels, self.connection.dst, self.connection.tls.clone()); HttpRouteNotFound(()).into() } -} -fn apply_http_filters( - r#match: http::RouteMatch, - route: &http::Policy, - req: &mut ::http::Request, -) -> Result<()> { - // TODO Do any metrics apply here? - for filter in &route.filters { - match filter { - http::Filter::InjectFailure(fail) => { - if let Some(http::filter::FailureResponse { status, message }) = fail.apply() { - return Err(HttpRouteInjectedFailure { status, message }.into()); + fn apply_http_filters( + &self, + r#match: http::RouteMatch, + route: &http::Policy, + req: &mut ::http::Request, + ) -> Result<()> { + // TODO Do any metrics apply here? + for filter in &route.filters { + match filter { + http::Filter::InjectFailure(fail) => { + if let Some(http::filter::FailureResponse { status, message }) = fail.apply() { + return Err(HttpRouteInjectedFailure { status, message }.into()); + } } - } - http::Filter::Redirect(redir) => match redir.apply(req.uri(), &r#match) { - Ok(Some(http::filter::Redirection { status, location })) => { - return Err(HttpRouteRedirect { status, location }.into()); - } + http::Filter::Redirect(redir) => match redir.apply(req.uri(), &r#match) { + Ok(Some(http::filter::Redirection { status, location })) => { + return Err(HttpRouteRedirect { status, location }.into()); + } - Err(invalid) => { - return Err(HttpRouteInvalidRedirect(invalid).into()); - } + Err(invalid) => { + return Err(HttpRouteInvalidRedirect(invalid).into()); + } - Ok(None) => { - tracing::debug!("Ignoring irrelevant redirect"); - } - }, + Ok(None) => { + tracing::debug!("Ignoring irrelevant redirect"); + } + }, - http::Filter::RequestHeaders(rh) => { - rh.apply(req.headers_mut()); - } + http::Filter::RequestHeaders(rh) => { + rh.apply(req.headers_mut()); + } - http::Filter::ClientIpHeaders(c) => { - c.apply(req); + http::Filter::ClientIpHeaders(c) => { + c.apply(self.connection.client.ip(), req.headers_mut()); + } } } - } - Ok(()) -} + Ok(()) + } -fn apply_grpc_filters(route: &grpc::Policy, req: &mut ::http::Request) -> Result<()> { - for filter in &route.filters { - match filter { - grpc::Filter::InjectFailure(fail) => { - if let Some(grpc::filter::FailureResponse { code, message }) = fail.apply() { - return Err(GrpcRouteInjectedFailure { code, message }.into()); + fn apply_grpc_filters( + &self, + route: &grpc::Policy, + req: &mut ::http::Request, + ) -> Result<()> { + for filter in &route.filters { + match filter { + grpc::Filter::InjectFailure(fail) => { + if let Some(grpc::filter::FailureResponse { code, message }) = fail.apply() { + return Err(GrpcRouteInjectedFailure { code, message }.into()); + } } - } - grpc::Filter::RequestHeaders(rh) => { - rh.apply(req.headers_mut()); - } + grpc::Filter::RequestHeaders(rh) => { + rh.apply(req.headers_mut()); + } - grpc::Filter::ClientIpHeaders(c) => { - c.apply(req); + grpc::Filter::ClientIpHeaders(c) => { + c.apply(self.connection.client.ip(), req.headers_mut()); + } } } - } - Ok(()) + Ok(()) + } } diff --git a/linkerd/http-route/Cargo.toml b/linkerd/http-route/Cargo.toml index b35dc42771..8fcbc2abc9 100644 --- a/linkerd/http-route/Cargo.toml +++ b/linkerd/http-route/Cargo.toml @@ -11,5 +11,4 @@ regex = "1" rand = "0.8" thiserror = "1" tracing = "0.1" -url = "2" -linkerd-proxy-http = { path = "../proxy/http" } \ No newline at end of file +url = "2" \ No newline at end of file diff --git a/linkerd/http-route/src/http/filter/client_ip_headers.rs b/linkerd/http-route/src/http/filter/client_ip_headers.rs index ca29ec0fee..3d68ba3d38 100644 --- a/linkerd/http-route/src/http/filter/client_ip_headers.rs +++ b/linkerd/http-route/src/http/filter/client_ip_headers.rs @@ -1,7 +1,5 @@ -use http::{ - header::{HeaderName, HeaderValue}, - Request, -}; +use http::header::{HeaderMap, HeaderName, HeaderValue}; +use std::net::IpAddr; /// Adds or sets HTTP headers containing the client's IP address. /// @@ -9,7 +7,7 @@ use http::{ /// `Forwarded-For`, `X-Forwarded-For`, and friends. #[derive(Clone, Debug, Default, Hash, PartialEq, Eq)] pub struct ClientIpHeaders { - headers: Vec<(HeaderName, Action)>, + pub headers: Vec<(HeaderName, Action)>, } #[derive(Copy, Clone, Debug, Hash, PartialEq, Eq)] @@ -21,20 +19,13 @@ pub enum Action { // === impl ForwardedFor === impl ClientIpHeaders { - pub fn apply(&self, req: &mut Request) { + pub fn apply(&self, client_addr: IpAddr, headers: &mut HeaderMap) { if self.headers.is_empty() { return; } - let value = match req.extensions().get::() { - Some(client) => HeaderValue::try_from(client.addr.ip().to_string()) - .expect("an IP address should format as a valid header value"), - None => { - debug_assert!(false, "request missing `ClientHandle` extension"); - return; - } - }; - let headers = req.headers_mut(); + let value = HeaderValue::try_from(client_addr.to_string()) + .expect("an IP address should format as a valid header value"); for (header, action) in &self.headers { match action { Action::Add => {