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: Add support for cluster-scoped default policies #1210

Merged
merged 6 commits into from
Aug 23, 2021

Conversation

olix0r
Copy link
Member

@olix0r olix0r commented Aug 20, 2021

The policy controller supports the default policies
cluster-authenticated and cluster-unauthenticated but the proxy
would error if configured with these policies. In order to use a single
policy annotation honored by both the policy controller and proxy, this
change adds support for a new configuration,
LINKERD2_POLICY_CLUSTER_NETWORKS, that allows the proxy to enforce
cluster-scoped defaults. When this configuration is not specified, the
cluster-scoped defaults are not supported.

This default will apply to all ports that are not documented in the
pod's spec (i.e. ports not in the LINKERD2_PROXY_INBOUND_PORTS
configuration).

This change also updates policy labeling to track server and authz labels
independently so that keys may overlap, as is required by linkerd/linkerd2#6722

The policy controller supports the default policies
`cluster-authenticated` and `cluster-unauthenticated` but the proxy
would error if configured with these policies. In order to use a single
policy annotation honored by both the policy controller and proxy, this
change adds support for a new configuration,
`LINKERD2_POLICY_CLUSTER_NETWORKS`, that allows the proxy to enforce
cluster-scoped defaults. When this configuration is not specified, the
cluster-scoped defaults are not supported.

This default will apply to all ports that are not documented in the
pod's spec (i.e. ports not in the `LINKERD2_PROXY_INBOUND_PORTS`
configuration).
Copy link
Contributor

@hawkw hawkw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm --- this change is pretty straightforward!

detect_timeout: Duration,
) -> Result<policy::DefaultPolicy, ParseError> {
match s {
"deny" => Ok(policy::DefaultPolicy::Deny),
"all-authenticated" => Ok(policy::defaults::all_authenticated(detect_timeout).into()),
"all-unauthenticated" => Ok(policy::defaults::all_unauthenticated(detect_timeout).into()),

// If cluster networks are configured, support cluster-scoped default policies.
name if cluster_nets.is_empty() => Err(ParseError::InvalidPortPolicy(name.to_string())),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, should there be a separate error case for this? this way, we will log an error that kind of implies "cluster-authenticated" isn't a valid default policy name, but it is a valid policy, it's just not valid in the case that there's no cluster networks.

possibly improving the error message here is not that important, though, since users aren't setting these env vars manually?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

possibly improving the error message here is not that important, though, since users aren't setting these env vars manually?

Correct. Seeing this is basically a programing error: the injector isn't doing the right thing. There's nothing a user could change in their config other than to change/unset the default policy on the pod.

linkerd/server-policy/src/lib.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@kleimkuhler kleimkuhler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aside from comment on some better error handling here looks good!

Co-authored-by: Eliza Weisman <eliza@buoyant.io>
@olix0r olix0r merged commit e9eaedc into main Aug 23, 2021
@olix0r olix0r deleted the ver/policy-cluster-nets branch August 23, 2021 21:51
olix0r added a commit to linkerd/linkerd2 that referenced this pull request Aug 26, 2021
This release improves policy handling for HTTP connections so that
requests are failed with a 403 Forbidden status (or a PERMISSION_DENIED
grpc-status, if appropriate).

Inbound metrics now include labels indicating the server and/or
authorization used to allow a connection or request to the proxy. Error
metrics now include an `unauthorized` error reason for traffic that is
denied by policy.

Finally, the outbound proxy no longer initializes mTLS or HTTP/2
upgrades when the target proxy is itself. This is done in preparation
for changes that will allow the proxy to stop forwarding connections on
`localhost` so that servers bound only on the loopback interface are not
exposed by Linkerd.

---

* build(deps): bump h2 from 0.3.3 to 0.3.4 (linkerd/linkerd2-proxy#1212)
* build(deps): bump libc from 0.2.99 to 0.2.100 (linkerd/linkerd2-proxy#1213)
* Use `ExtractParam` in transport metrics (linkerd/linkerd2-proxy#1211)
* policy: Add support for cluster-scoped default policies (linkerd/linkerd2-proxy#1210)
* Expose policy labels on inbound transport metrics (linkerd/linkerd2-proxy#1215)
* inbound: Expose permit labels on HTTP metrics (linkerd/linkerd2-proxy#1216)
* build(deps): bump tokio from 1.10.0 to 1.10.1 (linkerd/linkerd2-proxy#1218)
* build(deps): bump codecov/codecov-action from 2.0.2 to 2.0.3 (linkerd/linkerd2-proxy#1217)
* build(deps): bump hyper from 0.14.11 to 0.14.12 (linkerd/linkerd2-proxy#1221)
* build(deps): bump bytes from 1.0.1 to 1.1.0 (linkerd/linkerd2-proxy#1222)
* inbound: Return HTTP-level authorization errors (linkerd/linkerd2-proxy#1220)
* Skip TLS and H2 when target is inbound IP (linkerd/linkerd2-proxy#1219)
olix0r added a commit to linkerd/linkerd2 that referenced this pull request Aug 26, 2021
This release improves policy handling for HTTP connections so that
requests are failed with a 403 Forbidden status (or a PERMISSION_DENIED
grpc-status, if appropriate).

Inbound metrics now include labels indicating the server and/or
authorization used to allow a connection or request to the proxy. Error
metrics now include an `unauthorized` error reason for traffic that is
denied by policy.

Finally, the outbound proxy no longer initializes mTLS or HTTP/2
upgrades when the target proxy is itself. This is done in preparation
for changes that will allow the proxy to stop forwarding connections on
`localhost` so that servers bound only on the loopback interface are not
exposed by Linkerd.

---

* build(deps): bump h2 from 0.3.3 to 0.3.4 (linkerd/linkerd2-proxy#1212)
* build(deps): bump libc from 0.2.99 to 0.2.100 (linkerd/linkerd2-proxy#1213)
* Use `ExtractParam` in transport metrics (linkerd/linkerd2-proxy#1211)
* policy: Add support for cluster-scoped default policies (linkerd/linkerd2-proxy#1210)
* Expose policy labels on inbound transport metrics (linkerd/linkerd2-proxy#1215)
* inbound: Expose permit labels on HTTP metrics (linkerd/linkerd2-proxy#1216)
* build(deps): bump tokio from 1.10.0 to 1.10.1 (linkerd/linkerd2-proxy#1218)
* build(deps): bump codecov/codecov-action from 2.0.2 to 2.0.3 (linkerd/linkerd2-proxy#1217)
* build(deps): bump hyper from 0.14.11 to 0.14.12 (linkerd/linkerd2-proxy#1221)
* build(deps): bump bytes from 1.0.1 to 1.1.0 (linkerd/linkerd2-proxy#1222)
* inbound: Return HTTP-level authorization errors (linkerd/linkerd2-proxy#1220)
* Skip TLS and H2 when target is inbound IP (linkerd/linkerd2-proxy#1219)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants