Per route authorization (#8901)

adleong · web-flow · commit fe2931831300 · 2022-07-20T12:56:06.000-07:00
This change updates the policy controller to admit `AuthorizationPolicy` resources that reference `HTTPRoute` parents. These policies configure proxies to augment server-level authorizations with per-route authorizations. Fixes #8890 Signed-off-by: Alex Leong <alex@buoyant.io>
diff --git 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},
@@ -11,6 +13,7 @@ use std::num::NonZeroU16;
 pub struct InboundHttpRoute {
     pub hostnames: Vec<HostMatch>,
     pub rules: Vec<InboundHttpRouteRule>,
+    pub authorizations: HashMap<AuthorizationRef, ClientAuthorization>,
 }
 
 #[derive(Clone, Debug, PartialEq, Eq)]
diff --git a/policy-controller/grpc/src/lib.rs b/policy-controller/grpc/src/lib.rs
@@ -177,7 +177,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(),
                 },
             )),
@@ -186,7 +186,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(),
                 },
             )),
@@ -195,7 +195,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(),
                 },
             )),
@@ -366,7 +366,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 {
@@ -391,11 +396,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
@@ -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<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> {
diff --git 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;
@@ -109,7 +110,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(),
+            },
         })
     }
 }
diff --git a/policy-controller/k8s/index/src/index.rs b/policy-controller/k8s/index/src/index.rs
@@ -1104,7 +1104,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),
@@ -1146,6 +1146,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!(
@@ -1176,11 +1182,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);
+
+            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()
     }
 
diff --git 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<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(),
+        },
+    }
+}
diff --git 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::<HttpRoute>() {
+        return Ok(());
+    }
+
     if tgt.targets_kind::<Namespace>() {
         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
@@ -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
