Skip to content
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.

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

policy: Per route authorization #8901

Merged
merged 6 commits into from
Jul 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions policy-controller/core/src/http_route.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use crate::{AuthorizationRef, ClientAuthorization};
use ahash::AHashMap as HashMap;
use anyhow::Result;
pub use http::{
header::{HeaderName, HeaderValue},
Expand All @@ -10,6 +12,7 @@ use regex::Regex;
pub struct InboundHttpRoute {
pub hostnames: Vec<HostMatch>,
pub rules: Vec<InboundHttpRouteRule>,
pub authorizations: HashMap<AuthorizationRef, ClientAuthorization>,
}

#[derive(Clone, Debug, PartialEq, Eq)]
Expand Down
20 changes: 15 additions & 5 deletions policy-controller/grpc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ fn to_server(srv: &InboundServer, cluster_networks: &[IpNet]) -> proto::Server {
http_routes: srv
.http_routes
.iter()
.map(|(name, route)| to_http_route(name, route.clone()))
.map(|(name, route)| to_http_route(name, route.clone(), cluster_networks))
.collect(),
},
)),
Expand All @@ -192,7 +192,7 @@ fn to_server(srv: &InboundServer, cluster_networks: &[IpNet]) -> proto::Server {
routes: srv
.http_routes
.iter()
.map(|(name, route)| to_http_route(name, route.clone()))
.map(|(name, route)| to_http_route(name, route.clone(), cluster_networks))
.collect(),
},
)),
Expand All @@ -201,7 +201,7 @@ fn to_server(srv: &InboundServer, cluster_networks: &[IpNet]) -> proto::Server {
routes: srv
.http_routes
.iter()
.map(|(name, route)| to_http_route(name, route.clone()))
.map(|(name, route)| to_http_route(name, route.clone(), cluster_networks))
.collect(),
},
)),
Expand Down Expand Up @@ -372,7 +372,12 @@ fn to_authz(

fn to_http_route(
name: impl ToString,
InboundHttpRoute { hostnames, rules }: InboundHttpRoute,
InboundHttpRoute {
hostnames,
rules,
authorizations,
}: InboundHttpRoute,
cluster_networks: &[IpNet],
) -> proto::HttpRoute {
let metadata = Metadata {
kind: Some(metadata::Kind::Resource(api::meta::Resource {
Expand All @@ -397,11 +402,16 @@ fn to_http_route(
)
.collect();

let authorizations = authorizations
.iter()
.map(|(n, c)| to_authz(n, c, cluster_networks))
.collect();

proto::HttpRoute {
metadata: Some(metadata),
hosts,
rules,
authorizations: Vec::default(), // TODO populate per-route authorizations
authorizations,
}
}

Expand Down
20 changes: 11 additions & 9 deletions policy-controller/k8s/index/src/authorization_policy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ pub(crate) struct Spec {

#[derive(Debug, PartialEq)]
pub(crate) enum Target {
HttpRoute(String),
Server(String),
Namespace,
}
Expand Down Expand Up @@ -60,16 +61,17 @@ impl TryFrom<k8s::policy::AuthorizationPolicySpec> for Spec {

fn target(t: LocalTargetRef) -> Result<Target> {
if t.targets_kind::<k8s::policy::Server>() {
return Ok(Target::Server(t.name));
}
if t.targets_kind::<k8s::Namespace>() {
return Ok(Target::Namespace);
Ok(Target::Server(t.name))
} else if t.targets_kind::<k8s::Namespace>() {
Ok(Target::Namespace)
} else if t.targets_kind::<k8s_gateway_api::HttpRoute>() {
Ok(Target::HttpRoute(t.name))
} else {
anyhow::bail!(
"unsupported authorization target type: {}",
t.canonical_kind()
)
}

anyhow::bail!(
"unsupported authorization target type: {}",
t.canonical_kind()
);
}

fn authentication_ref(t: NamespacedTargetRef) -> Result<AuthenticationTarget> {
Expand Down
7 changes: 6 additions & 1 deletion policy-controller/k8s/index/src/http_route.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use ahash::AHashMap as HashMap;
use anyhow::{bail, Error, Result};
use k8s_gateway_api as api;
use linkerd_policy_controller_core::http_route;
Expand Down Expand Up @@ -108,7 +109,11 @@ impl TryFrom<api::HttpRoute> for InboundRouteBinding {

Ok(InboundRouteBinding {
parents,
route: http_route::InboundHttpRoute { hostnames, rules },
route: http_route::InboundHttpRoute {
hostnames,
rules,
authorizations: HashMap::default(),
},
})
}
}
Expand Down
71 changes: 68 additions & 3 deletions policy-controller/k8s/index/src/index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1100,7 +1100,7 @@ impl PolicyIndex {
) -> InboundServer {
tracing::trace!(%name, ?server, "Creating inbound server");
let authorizations = self.client_authzs(&name, server, authentications);
let routes = self.http_routes(&name);
let routes = self.http_routes(&name, authentications);

InboundServer {
reference: ServerRef::Server(name),
Expand Down Expand Up @@ -1142,6 +1142,12 @@ impl PolicyIndex {
}
}
authorization_policy::Target::Namespace => {}
authorization_policy::Target::HttpRoute(_) => {
// Policies which target HttpRoutes will be attached to
// the route authorizations and should not be included in
// the server authorizations.
continue;
}
}

tracing::trace!(
Expand Down Expand Up @@ -1172,11 +1178,70 @@ impl PolicyIndex {
authzs
}

fn http_routes(&self, server_name: &str) -> HashMap<String, InboundHttpRoute> {
fn route_client_authzs(
&self,
route_name: &str,
authentications: &AuthenticationNsIndex,
) -> HashMap<AuthorizationRef, ClientAuthorization> {
let mut authzs = HashMap::default();

for (name, spec) in &self.authorization_policies {
// Skip the policy if it doesn't apply to the route.
match &spec.target {
authorization_policy::Target::HttpRoute(n) if n == route_name => {}
_ => {
tracing::trace!(
ns = %self.namespace,
authorizationpolicy = %name,
route = %route_name,
target = ?spec.target,
"AuthorizationPolicy does not target HttpRoute",
);
continue;
}
}

tracing::trace!(
ns = %self.namespace,
authorizationpolicy = %name,
route = %route_name,
"AuthorizationPolicy targets HttpRoute",
);
tracing::trace!(authns = ?spec.authentications);
adleong marked this conversation as resolved.
Show resolved Hide resolved

let authz = match self.policy_client_authz(spec, authentications) {
Ok(authz) => authz,
Err(error) => {
tracing::info!(
route = %route_name,
authorizationpolicy = %name,
%error,
"Illegal AuthorizationPolicy; ignoring",
);
continue;
}
};

let reference = AuthorizationRef::AuthorizationPolicy(name.to_string());
authzs.insert(reference, authz);
}

authzs
}

fn http_routes(
&self,
server_name: &str,
authentications: &AuthenticationNsIndex,
) -> HashMap<String, InboundHttpRoute> {
self.http_routes
.iter()
.filter(|(_, route)| route.selects_server(server_name))
.map(|(name, route)| (name.clone(), route.route.clone()))
.map(|(name, route)| {
let mut route = route.route.clone();
route.authorizations = self.route_client_authzs(name, authentications);
(name.clone(), route)
})
.collect()
}

Expand Down
49 changes: 48 additions & 1 deletion policy-controller/k8s/index/src/tests/http_routes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use super::*;
fn route_attaches_to_server() {
let test = TestConfig::default();

// Create pod.
let mut pod = mk_pod("ns-0", "pod-0", Some(("container-0", None)));
pod.labels_mut()
.insert("app".to_string(), "app-0".to_string());
Expand All @@ -16,6 +17,7 @@ fn route_attaches_to_server() {
.expect("pod-0.ns-0 should exist");
assert_eq!(*rx.borrow_and_update(), test.default_server());

// Create server.
test.index.write().apply(mk_server(
"ns-0",
"srv-8080",
Expand All @@ -34,6 +36,8 @@ fn route_attaches_to_server() {
http_routes: HashMap::default(),
},
);

// Create route.
test.index
.write()
.apply(mk_http_route("ns-0", "route-foo", "srv-8080"));
Expand All @@ -42,7 +46,27 @@ fn route_attaches_to_server() {
rx.borrow().reference,
ServerRef::Server("srv-8080".to_string())
);
assert!(rx.borrow().http_routes.contains_key("route-foo"));
assert!(rx.borrow_and_update().http_routes.contains_key("route-foo"));

// Create authz policy.
test.index.write().apply(mk_authorization_policy(
"ns-0",
"authz-foo",
"route-foo",
vec![NamespacedTargetRef {
group: None,
kind: "ServiceAccount".to_string(),
namespace: Some("ns-0".to_string()),
name: "foo".to_string(),
}],
));

assert!(rx.has_changed().unwrap());
assert!(rx.borrow().http_routes["route-foo"]
.authorizations
.contains_key(&AuthorizationRef::AuthorizationPolicy(
"authz-foo".to_string()
)));
}

fn mk_http_route(
Expand Down Expand Up @@ -84,3 +108,26 @@ fn mk_http_route(
status: None,
}
}

fn mk_authorization_policy(
ns: impl ToString,
name: impl ToString,
route: impl ToString,
authns: impl IntoIterator<Item = NamespacedTargetRef>,
) -> k8s::policy::AuthorizationPolicy {
k8s::policy::AuthorizationPolicy {
metadata: k8s::ObjectMeta {
namespace: Some(ns.to_string()),
name: Some(name.to_string()),
..Default::default()
},
spec: k8s::policy::AuthorizationPolicySpec {
target_ref: LocalTargetRef {
group: Some("gateway.networking.k8s.io".to_string()),
kind: "HttpRoute".to_string(),
name: route.to_string(),
},
required_authentication_refs: authns.into_iter().collect(),
},
}
}
4 changes: 4 additions & 0 deletions policy-controller/src/admission.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,10 @@ fn validate_policy_target(ns: &str, tgt: &LocalTargetRef) -> Result<()> {
return Ok(());
}

if tgt.targets_kind::<HttpRoute>() {
return Ok(());
}

if tgt.targets_kind::<Namespace>() {
if tgt.name != ns {
bail!("cannot target another namespace: {}", tgt.name);
Expand Down
33 changes: 33 additions & 0 deletions policy-test/tests/admit_authorization_policy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,39 @@ async fn rejects_targets_other_namespace() {
.await;
}

#[tokio::test(flavor = "current_thread")]
async fn accepts_targets_route() {
admission::accepts(|ns| AuthorizationPolicy {
metadata: api::ObjectMeta {
namespace: Some(ns),
name: Some("test".to_string()),
..Default::default()
},
spec: AuthorizationPolicySpec {
target_ref: LocalTargetRef {
group: Some("gateway.networking.k8s.io".to_string()),
kind: "HttpRoute".to_string(),
name: "route-foo".to_string(),
},
required_authentication_refs: vec![
NamespacedTargetRef {
group: Some("policy.linkerd.io".to_string()),
kind: "MeshTLSAuthentication".to_string(),
name: "mtls-clients".to_string(),
namespace: None,
},
NamespacedTargetRef {
group: Some("policy.linkerd.io".to_string()),
kind: "NetworkAuthentication".to_string(),
name: "cluster-nets".to_string(),
namespace: Some("linkerd".to_string()),
},
],
},
})
.await;
}

#[tokio::test(flavor = "current_thread")]
async fn accepts_valid_with_only_meshtls() {
admission::accepts(|ns| AuthorizationPolicy {
Expand Down
Loading