Introduce per-route authorization policies by olix0r · Pull Request #1781 · linkerd/linkerd2-proxy

olix0r · 2022-06-24T08:06:59Z

This change adds per-route authorization policies to the inbound proxy.
This change does NOT introduce an API client to configure these
policies. That will be added in follow-up changes, as will richer
per-route configurations.

This change:

Modifies the inbound policy Protocol type so that each protocol
variant includes policy configuration. TCP protocols now include
authorization policies (moved from the Server type).
Implements an HTTP policy router that determines the route for each
request and applies its authorization policy.
Fails requests for which no route matches with a 404 Not Found
response. A new metric, inbound_http_route_not_found_total, is
introduced to count occurrences of this error.
Includes a default route for all inbound HTTP communication, using the
server's authorization policies.

Signed-off-by: Oliver Gould [email protected]

The inbound TLS detection stack may forward non-mesh-TLS connections to the application without recording TCP metrics. This change adds metrics for these connections. This change also includes other non-functional changes to setup for #1781. Signed-off-by: Oliver Gould <[email protected]>

Before making changes that support inbound HTTP route authorization policies, this change makes some cosmetic changes that will help minimize the changes needed for new funcationality. No funtional changes. Signed-off-by: Oliver Gould <[email protected]>

The inbound TLS detection stack may forward non-mesh-TLS connections to the application without recording TCP metrics. This change adds metrics for these connections. This change also includes other non-functional changes to setup for #1781. Signed-off-by: Oliver Gould <[email protected]>

Before making changes that support inbound HTTP route authorization policies, this change makes some cosmetic changes that will help minimize the changes needed for new funcationality. No funtional changes. Signed-off-by: Oliver Gould <[email protected]>

This change adds several route filters: * An HTTP header rewriting; * An HTTP redirection filter; * An HTTP error filter; and * A gRPC error filter. The header-rewriting and HTTP redirection filters are implemented as specified by the Kubernetes Gateway API HTTPRoute spec. These filters apply only on inbound policies are not yet configurable via the control plane. A followup change will be made to support configuring these filters. Depends on #1781. Signed-off-by: Oliver Gould <[email protected]>

This change adds per-route authorization policies to the inbound proxy. This change does NOT introduce an API client to configure these policies. That will be added in follow-up changes, as will richer per-route configurations. This change: * Modifies the inbound policy `Protocol` type so that each protocol variant includes policy configuration. TCP protocols now include authorization policies (moved from the `Server` type). * Implements an HTTP policy router that determines the route for each request and applies its authorization policy. * Fails requests for which no route matches with a `404 Not Found` response. A new metric, `inbound_http_route_not_found_total`, is introduced to count occurrences of this error. * Includes a default route for all inbound HTTP communication, using the server's authorization policies. Signed-off-by: Oliver Gould <[email protected]>

Signed-off-by: Oliver Gould <[email protected]>

hawkw

i read through this a couple times and left some comments, mostly on minor style nits. this seems good overall!

i am a bit confused about the linkerd-grpc-route crate added in this PR, which appears to be missing a lib.rs file? i'm surprised that compiles...it looks like it wasn't added to the list of crates in the workspace's Cargo.toml, which i think is the only reason we're not getting a build failure. was that crate intended to be added in this PR, or did it accidentally slip in from a different change or something?

linkerd/app/inbound/src/metrics/authz.rs

linkerd/app/inbound/src/policy/http/tests.rs

hawkw · 2022-06-27T19:50:43Z

linkerd/app/inbound/src/policy/http/tests.rs

+    assert!(svc
+        .call(
+            http::Request::builder()
+                .method(http::Method::POST)
+                .body(hyper::Body::default())
+                .unwrap(),
+        )
+        .await
+        .expect_err("fails")
+        .is::<HttpRouteUnauthorized>());
+
+    assert!(svc
+        .call(
+            http::Request::builder()
+                .method(http::Method::DELETE)
+                .body(hyper::Body::default())
+                .unwrap(),
+        )
+        .await
+        .expect_err("fails")
+        .is::<HttpRouteNotFound>());
+}


again, i might split the call out of the assertions, like

Suggested change

assert!(svc

.call(

http::Request::builder()

.method(http::Method::POST)

.body(hyper::Body::default())

.unwrap(),

)

.await

.expect_err("fails")

.is::<HttpRouteUnauthorized>());

assert!(svc

.call(

http::Request::builder()

.method(http::Method::DELETE)

.body(hyper::Body::default())

.unwrap(),

)

.await

.expect_err("fails")

.is::<HttpRouteNotFound>());

}

let unauthorized = svc

.call(

http::Request::builder()

.method(http::Method::POST)

.body(hyper::Body::default())

.unwrap(),

)

.await

.expect_err("fails");

assert!(unauthorized.is::<HttpRouteUnauthorized>());

let not_found = svc

.call(

http::Request::builder()

.method(http::Method::DELETE)

.body(hyper::Body::default())

.unwrap(),

)

.await

.expect_err("fails");

assert!(not_found.is::<HttpRouteNotFound>());

}

but, it's not a huge deal.

linkerd/grpc-route/src/proto.rs

linkerd/app/inbound/src/policy/http.rs

hawkw · 2022-06-27T22:06:07Z

linkerd/app/inbound/src/policy/http/tests.rs

+macro_rules! new_svc {
+    ($proto:expr, $conn:expr) => {{
+        let (policy, tx) = AllowPolicy::for_test(
+            $conn.dst,
+            ServerPolicy {
+                protocol: $proto,
+                meta: Arc::new(Meta::Resource {
+                    group: "policy.linkerd.io".into(),
+                    kind: "server".into(),
+                    name: "testsrv".into(),
+                }),
+            },
+        );
+        let svc = HttpPolicyService {
+            target: (),
+            policy,
+            connection: $conn,
+            metrics: HttpAuthzMetrics::default(),
+            inner: |(permit, _): (HttpRoutePermit, ())| {
+                svc::mk(move |_: http::Request<hyper::Body>| {
+                    let permit = permit.clone();
+                    async move {
+                        let mut rsp = http::Response::builder()
+                            .body(hyper::Body::default())
+                            .unwrap();
+                        rsp.extensions_mut().insert(permit.clone());
+                        Ok::<_, Infallible>(rsp)
+                    }
+                })
+            },
+        };
+        (svc, tx)
+    }};
+
+    ($proto:expr) => {{
+        let conn = ConnectionMeta {
+            dst: OrigDstAddr(([192, 168, 3, 4], 8080).into()),
+            client: Remote(ClientAddr(([192, 168, 3, 3], 30120).into())),
+            tls: tls::ConditionalServerTls::Some(tls::ServerTls::Established {
+                client_id: Some("foo.bar.bah".parse().unwrap()),
+                negotiated_protocol: None,
+            }),
+        };
+        new_svc!($proto, conn)
+    }};
+}


nit, TIOLI: this kind of feels like it could just be a pair of functions rather than a macro, but i'm not sure if it's really a big deal either way...

Yeah, but I don't want to have to name the response type.

oh, missed that. yeah, let's keep the macro then!

linkerd/grpc-route/Cargo.toml

Signed-off-by: Oliver Gould <[email protected]>

hawkw

okay, this looks good to me whenever you feel like it's ready!

This release updates the proxy's service discovery module to avoid redundant load balancer updates that could cause unnecessary connection churn. This release also includes improvements to the proxy's retry handling of gRPC requests. The proxy would not retry requests when a response's status code was emitted in a TRAILERS frame. This has been fixed. This release also includes a number of internal changes that set up for per-route authorization. There should be no user-facing impact at this point except for the introduction of additional metrics labels. --- * build(deps): bump mio from 0.8.3 to 0.8.4 (linkerd/linkerd2-proxy#1760) * build(deps): bump quote from 1.0.18 to 1.0.19 (linkerd/linkerd2-proxy#1761) * build(deps): bump tower-service from 0.3.1 to 0.3.2 (linkerd/linkerd2-proxy#1762) * build(deps): bump proc-macro2 from 1.0.39 to 1.0.40 (linkerd/linkerd2-proxy#1763) * build(deps): bump syn from 1.0.96 to 1.0.98 (linkerd/linkerd2-proxy#1764) * build(deps): bump prettyplease from 0.1.12 to 0.1.14 (linkerd/linkerd2-proxy#1766) * build(deps): bump anyhow from 1.0.57 to 1.0.58 (linkerd/linkerd2-proxy#1767) * dev: Update build settings (linkerd/linkerd2-proxy#1765) * Dedupe discovery updates (linkerd/linkerd2-proxy#1759) * build(deps): bump quote from 1.0.19 to 1.0.20 (linkerd/linkerd2-proxy#1768) * deny: Remove tokio-util from exceptions (linkerd/linkerd2-proxy#1769) * dev: Update memory contraints (linkerd/linkerd2-proxy#1770) * Reorganize `server-policy` to set up for routes (linkerd/linkerd2-proxy#1771) * inbound: Rename policy-enforcement layers (linkerd/linkerd2-proxy#1772) * ci: Split fuzzer logic into a script (linkerd/linkerd2-proxy#1773) * build(deps): bump prettyplease from 0.1.14 to 0.1.15 (linkerd/linkerd2-proxy#1775) * build(deps): bump indexmap from 1.9.0 to 1.9.1 (linkerd/linkerd2-proxy#1776) * integration: Cleanup test server (linkerd/linkerd2-proxy#1777) * http-retry: Move the ReplayBody type into a module (linkerd/linkerd2-proxy#1778) * inbound: Add route authorization labels (linkerd/linkerd2-proxy#1774) * Rename HTTPRoutePermit to HttpRoutePermit (linkerd/linkerd2-proxy#1779) * retry gRPC requests are immediately terminated by trailers (linkerd/linkerd2-proxy#1706) * inbound: Record policy metrics for opaque-transport connections (linkerd/linkerd2-proxy#1780) * build(deps): bump tj-actions/changed-files from 23 to 23.1 (linkerd/linkerd2-proxy#1782) * build(deps): bump derive_arbitrary from 1.1.2 to 1.1.3 (linkerd/linkerd2-proxy#1783) * build(deps): bump arbitrary from 1.1.2 to 1.1.3 (linkerd/linkerd2-proxy#1784) * inbound: Record TCP metrics for forwarded TLS connections (linkerd/linkerd2-proxy#1785) * inbound: Cleanup in preparation for route policies #1781 (linkerd/linkerd2-proxy#1786) * Add HTTP route matchers to support the Gateway API (linkerd/linkerd2-proxy#1787) * build(deps): bump unicode-normalization from 0.1.19 to 0.1.20 (linkerd/linkerd2-proxy#1789) * build(deps): bump linked-hash-map from 0.5.4 to 0.5.6 (linkerd/linkerd2-proxy#1790) * build(deps): bump smallvec from 1.8.0 to 1.8.1 (linkerd/linkerd2-proxy#1791) * build(deps): bump jemalloc-sys from 0.5.0+5.3.0 to 0.5.1+5.3.0-patched (linkerd/linkerd2-proxy#1792) * Introduce per-route authorization policies (linkerd/linkerd2-proxy#1781) * inbound: Add a header-modification route filter (linkerd/linkerd2-proxy#1793) * docs: update justfile man page link (linkerd/linkerd2-proxy#1794) Signed-off-by: Oliver Gould <[email protected]>

olix0r mentioned this pull request Jun 24, 2022

inbound: Record TCP metrics for forwarded TLS connections #1785

Merged

olix0r force-pushed the ver/inbound-routes branch from e589067 to 84709d5 Compare June 24, 2022 19:13

olix0r changed the base branch from main to ver/http-route June 24, 2022 19:14

olix0r force-pushed the ver/inbound-routes branch from 84709d5 to 1d0df3f Compare June 24, 2022 19:18

olix0r mentioned this pull request Jun 24, 2022

inbound: Add HTTP route filters #1788

Closed

Base automatically changed from ver/http-route to main June 25, 2022 01:06

olix0r force-pushed the ver/inbound-routes branch from 1d0df3f to 10cb66e Compare June 25, 2022 01:09

olix0r marked this pull request as ready for review June 25, 2022 01:09

olix0r requested a review from a team as a code owner June 25, 2022 01:09

olix0r added 2 commits June 25, 2022 22:09

Add inbound HTTP route authorization tests

0f9f9a0

Signed-off-by: Oliver Gould <[email protected]>

fmt

48d1547

Signed-off-by: Oliver Gould <[email protected]>

hawkw reviewed Jun 27, 2022

View reviewed changes

olix0r added 2 commits June 27, 2022 23:03

Remove unused crate

e842683

Signed-off-by: Oliver Gould <[email protected]>

Simplify tests

e74d0d4

Signed-off-by: Oliver Gould <[email protected]>

hawkw approved these changes Jun 27, 2022

View reviewed changes

olix0r merged commit 3b7769d into main Jun 27, 2022

olix0r deleted the ver/inbound-routes branch June 27, 2022 23:46

olix0r mentioned this pull request Jun 30, 2022

proxy: v2.182.0 linkerd/linkerd2#8781

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce per-route authorization policies#1781

Introduce per-route authorization policies#1781
olix0r merged 5 commits intomainfrom
ver/inbound-routes

olix0r commented Jun 24, 2022 •

edited

Loading

Uh oh!

hawkw left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hawkw Jun 27, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hawkw Jun 27, 2022

Uh oh!

olix0r Jun 27, 2022

Uh oh!

hawkw Jun 27, 2022

Uh oh!

Uh oh!

hawkw left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

olix0r commented Jun 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hawkw left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hawkw Jun 27, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hawkw Jun 27, 2022

Choose a reason for hiding this comment

Uh oh!

olix0r Jun 27, 2022

Choose a reason for hiding this comment

Uh oh!

hawkw Jun 27, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

hawkw left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

olix0r commented Jun 24, 2022 •

edited

Loading