diff --git a/policy-controller/core/src/http_route.rs b/policy-controller/core/src/http_route.rs index 3e2cc87fa290c..0209f23b35c12 100644 --- a/policy-controller/core/src/http_route.rs +++ b/policy-controller/core/src/http_route.rs @@ -1,3 +1,5 @@ +use crate::{AuthorizationRef, ClientAuthorization}; +use ahash::AHashMap as HashMap; use anyhow::Result; pub use http::{ header::{HeaderName, HeaderValue}, @@ -10,6 +12,7 @@ use regex::Regex; pub struct InboundHttpRoute { pub hostnames: Vec, pub rules: Vec, + pub authorizations: HashMap, } #[derive(Clone, Debug, PartialEq, Eq)] diff --git a/policy-controller/grpc/src/lib.rs b/policy-controller/grpc/src/lib.rs index b9e2bca1ec5e1..da3f7f2f7a1ba 100644 --- a/policy-controller/grpc/src/lib.rs +++ b/policy-controller/grpc/src/lib.rs @@ -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(), }, )), @@ -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(), }, )), @@ -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(), }, )), @@ -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 { @@ -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, } } diff --git a/policy-controller/k8s/index/src/authorization_policy.rs b/policy-controller/k8s/index/src/authorization_policy.rs index 10dcbe68f39a2..c2e97672d0caf 100644 --- a/policy-controller/k8s/index/src/authorization_policy.rs +++ b/policy-controller/k8s/index/src/authorization_policy.rs @@ -13,6 +13,7 @@ pub(crate) struct Spec { #[derive(Debug, PartialEq)] pub(crate) enum Target { + HttpRoute(String), Server(String), Namespace, } @@ -60,16 +61,17 @@ impl TryFrom for Spec { fn target(t: LocalTargetRef) -> Result { if t.targets_kind::() { - return Ok(Target::Server(t.name)); - } - if t.targets_kind::() { - return Ok(Target::Namespace); + Ok(Target::Server(t.name)) + } else if t.targets_kind::() { + Ok(Target::Namespace) + } else if t.targets_kind::() { + 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 { diff --git a/policy-controller/k8s/index/src/http_route.rs b/policy-controller/k8s/index/src/http_route.rs index 7ca8518c50164..46c231088bb88 100644 --- a/policy-controller/k8s/index/src/http_route.rs +++ b/policy-controller/k8s/index/src/http_route.rs @@ -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; @@ -108,7 +109,11 @@ impl TryFrom for InboundRouteBinding { Ok(InboundRouteBinding { parents, - route: http_route::InboundHttpRoute { hostnames, rules }, + route: http_route::InboundHttpRoute { + hostnames, + rules, + authorizations: HashMap::default(), + }, }) } } diff --git a/policy-controller/k8s/index/src/index.rs b/policy-controller/k8s/index/src/index.rs index 0b12a21ffa508..3941f71c7d43f 100644 --- a/policy-controller/k8s/index/src/index.rs +++ b/policy-controller/k8s/index/src/index.rs @@ -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), @@ -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!( @@ -1172,11 +1178,70 @@ impl PolicyIndex { authzs } - fn http_routes(&self, server_name: &str) -> HashMap { + fn route_client_authzs( + &self, + route_name: &str, + authentications: &AuthenticationNsIndex, + ) -> HashMap { + 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); + + 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 { 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() } diff --git a/policy-controller/k8s/index/src/tests/http_routes.rs b/policy-controller/k8s/index/src/tests/http_routes.rs index f28242a4caf04..b853315571cd9 100644 --- a/policy-controller/k8s/index/src/tests/http_routes.rs +++ b/policy-controller/k8s/index/src/tests/http_routes.rs @@ -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()); @@ -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", @@ -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")); @@ -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( @@ -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, +) -> 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(), + }, + } +} diff --git a/policy-controller/src/admission.rs b/policy-controller/src/admission.rs index 1587c9516c5db..07fd6a61b8107 100644 --- a/policy-controller/src/admission.rs +++ b/policy-controller/src/admission.rs @@ -201,6 +201,10 @@ fn validate_policy_target(ns: &str, tgt: &LocalTargetRef) -> Result<()> { return Ok(()); } + if tgt.targets_kind::() { + return Ok(()); + } + if tgt.targets_kind::() { if tgt.name != ns { bail!("cannot target another namespace: {}", tgt.name); diff --git a/policy-test/tests/admit_authorization_policy.rs b/policy-test/tests/admit_authorization_policy.rs index f238578896ee8..90e1aec02a44d 100644 --- a/policy-test/tests/admit_authorization_policy.rs +++ b/policy-test/tests/admit_authorization_policy.rs @@ -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 { diff --git a/policy-test/tests/api.rs b/policy-test/tests/api.rs index ab214e5aac4dd..16f50158026f3 100644 --- a/policy-test/tests/api.rs +++ b/policy-test/tests/api.rs @@ -334,7 +334,7 @@ async fn server_with_http_route() { // Create an http route that refers to the `linkerd-admin` server (by // name) and ensure that the update now reflects this route. - create( + let route = create( &client, k8s_gateway_api::HttpRoute { metadata: kube::api::ObjectMeta { @@ -387,8 +387,8 @@ async fn server_with_http_route() { } else { panic!("proxy protocol must be HTTP1") }; - let route = http1.routes.first().expect("must have route"); - let rule_match = route + let h1_route = http1.routes.first().expect("must have route"); + let rule_match = h1_route .rules .first() .expect("must have rule") @@ -396,10 +396,13 @@ async fn server_with_http_route() { .first() .expect("must have match"); // Route has no authorizations by default. - assert_eq!(route.authorizations, Vec::default()); + assert_eq!(h1_route.authorizations, Vec::default()); // Route has appropriate metadata. assert_eq!( - route.metadata.to_owned().expect("route must have metadata"), + h1_route + .metadata + .to_owned() + .expect("route must have metadata"), grpc::meta::Metadata { kind: Some(grpc::meta::metadata::Kind::Resource(grpc::meta::Resource { group: "gateway.networking.k8s.io".to_string(), @@ -419,26 +422,44 @@ async fn server_with_http_route() { grpc::http_route::path_match::Kind::Exact("/metrics".to_string()), ); - // Create a server authorization that refers to the `linkerd-admin` - // server (by name) and ensure that the authorization is copied onto - // the route. + // Create a network authentication and an authorization policy that + // refers to the `metrics-route` route (by name). + let all_nets = create( + &client, + k8s::policy::NetworkAuthentication { + metadata: kube::api::ObjectMeta { + namespace: Some(ns.clone()), + name: Some("all-admin".to_string()), + ..Default::default() + }, + spec: k8s::policy::NetworkAuthenticationSpec { + networks: vec![ + k8s::policy::network_authentication::Network { + cidr: Ipv4Net::default().into(), + except: None, + }, + k8s::policy::network_authentication::Network { + cidr: Ipv6Net::default().into(), + except: None, + }, + ], + }, + }, + ) + .await; create( &client, - k8s::policy::ServerAuthorization { + k8s::policy::AuthorizationPolicy { metadata: kube::api::ObjectMeta { namespace: Some(ns.clone()), name: Some("all-admin".to_string()), ..Default::default() }, - spec: k8s::policy::ServerAuthorizationSpec { - server: k8s::policy::server_authorization::Server { - name: Some("linkerd-admin".to_string()), - selector: None, - }, - client: k8s::policy::server_authorization::Client { - unauthenticated: true, - ..k8s::policy::server_authorization::Client::default() - }, + spec: k8s::policy::AuthorizationPolicySpec { + target_ref: k8s::policy::LocalTargetRef::from_resource(&route), + required_authentication_refs: vec![ + k8s::policy::NamespacedTargetRef::from_resource(&all_nets), + ], }, }, ) @@ -460,8 +481,8 @@ async fn server_with_http_route() { } else { panic!("proxy protocol must be HTTP1") }; - - assert_eq!(http1.routes.len(), 1, "must have routes"); + let h1_route = http1.routes.first().expect("must have route"); + assert_eq!(h1_route.authorizations.len(), 1, "must have authorizations"); // Delete the `HttpRoute` and ensure that the update reverts to the // default.