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

Require identity configuration #1305

Merged
merged 3 commits into from
Oct 8, 2021
Merged

Require identity configuration #1305

merged 3 commits into from
Oct 8, 2021

Conversation

olix0r
Copy link
Member

@olix0r olix0r commented Oct 8, 2021

The proxy currently supports a mode where identity is disabled. This
proliferates complexity that isn't really needed: there doesn't appear
to be a real use case where disabling identity is necessary. And, if it
is really necessary, we should reintroduce it after decoupling TLS and
identity.

This change causes the proxy to error during startup if identity is
disabled by configuration.

Furthermore, the linkerd-proxy-identity crate now has a test-util
feature that makes it possible to build a LocalCrtKey identity from
credentials provided by the linkerd-identity/test-util feature. A
default set of credentials are used in inbound and outbound tests.

The proxy currently supports a mode where identity is disabled. This
proliferates complexity that isn't really needed: there doesn't appear
to be a real use case where disabling identity is necessary. And, if it
is really necessary, we should reintroduce it after decoupling TLS and
identity.

This change causes the proxy to error during startup if identity is
disabled by configuration.

Furthermore, the `linkerd-proxy-identity` crate now has a `test-util`
feature that makes it possible to build a `LocalCrtKey` identity from
credentials provided by the `linkerd-identity/test-util` feature. A
default set of credentials are used in inbound and outbound tests.
@olix0r olix0r requested a review from a team October 8, 2021 19:56
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.

overall, this is pretty mechanical, so 👍

I commented on a couple small things I noticed, but overall, this LGTM!

Comment on lines +1124 to +1127
error!(
"{} is no longer supported. Identity is must be enabled.",
ENV_IDENTITY_DISABLED
);
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 for adding an explicit error message here!

linkerd/app/src/identity.rs Outdated Show resolved Hide resolved
linkerd/app/src/identity.rs Outdated Show resolved Hide resolved
linkerd/proxy/identity/src/certify.rs Show resolved Hide resolved
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!

@olix0r olix0r merged commit 49227fe into main Oct 8, 2021
@olix0r olix0r deleted the ver/require-tls branch October 8, 2021 22:18
olix0r added a commit to linkerd/linkerd2 that referenced this pull request Oct 19, 2021
This release fixes a bug where the outbound proxy could loop infinitely
while handling errors on meshed HTTP/1 connections. This would typically
cause proxies to be fail health checks and be restarted.

Furthermore, the proxy now requires identity. Proxies will log an error
and fail to start if identity is disabled.

---

* dns: Avoid allocating in `Name::is_localhost` (linkerd/linkerd2-proxy#1303)
* metrics: Implement FmtMetrics for Option (linkerd/linkerd2-proxy#1302)
* tracing: simplify subscriber construction with `Box`ed layers (linkerd/linkerd2-proxy#1304)
* Require identity configuration (linkerd/linkerd2-proxy#1305)
* build(deps): bump thiserror from 1.0.29 to 1.0.30 (linkerd/linkerd2-proxy#1306)
* build(deps): bump tower from 0.4.8 to 0.4.9 (linkerd/linkerd2-proxy#1308)
* build(deps): bump trust-dns-resolver (linkerd/linkerd2-proxy#1311)
* build(deps): bump actions/checkout from 2.3.4 to 2.3.5 (linkerd/linkerd2-proxy#1313)
* dns-name: Remove `webpki` dependency (linkerd/linkerd2-proxy#1316)
* build(deps): bump libc from 0.2.103 to 0.2.104 (linkerd/linkerd2-proxy#1315)
* inbound: Add a box layer to reduce compile times (linkerd/linkerd2-proxy#1317)
* Split cryptographic dependencies into a dedicated crate (linkerd/linkerd2-proxy#1307)
* Fix an infinite loop when downgrading HTTP/2 errors (linkerd/linkerd2-proxy#1318)
olix0r added a commit to linkerd/linkerd2 that referenced this pull request Oct 20, 2021
This release fixes a bug where the outbound proxy could loop infinitely
while handling errors on meshed HTTP/1 connections. This would typically
cause proxies to be fail health checks and be restarted.

Furthermore, the proxy now requires identity. Proxies will log an error
and fail to start if identity is disabled.

---

* dns: Avoid allocating in `Name::is_localhost` (linkerd/linkerd2-proxy#1303)
* metrics: Implement FmtMetrics for Option (linkerd/linkerd2-proxy#1302)
* tracing: simplify subscriber construction with `Box`ed layers (linkerd/linkerd2-proxy#1304)
* Require identity configuration (linkerd/linkerd2-proxy#1305)
* build(deps): bump thiserror from 1.0.29 to 1.0.30 (linkerd/linkerd2-proxy#1306)
* build(deps): bump tower from 0.4.8 to 0.4.9 (linkerd/linkerd2-proxy#1308)
* build(deps): bump trust-dns-resolver (linkerd/linkerd2-proxy#1311)
* build(deps): bump actions/checkout from 2.3.4 to 2.3.5 (linkerd/linkerd2-proxy#1313)
* dns-name: Remove `webpki` dependency (linkerd/linkerd2-proxy#1316)
* build(deps): bump libc from 0.2.103 to 0.2.104 (linkerd/linkerd2-proxy#1315)
* inbound: Add a box layer to reduce compile times (linkerd/linkerd2-proxy#1317)
* Split cryptographic dependencies into a dedicated crate (linkerd/linkerd2-proxy#1307)
* Fix an infinite loop when downgrading HTTP/2 errors (linkerd/linkerd2-proxy#1318)
@taikulawo
Copy link

taikulawo commented Oct 20, 2022

Does this are some setup docs? I'm trying run linkderd2-proxy, but I don't know
the following

ENV_IDENTITY_TRUST_ANCHORS
ENV_IDENTITY_DIR
ENV_IDENTITY_IDENTITY_LOCAL_NAME
ENV_IDENTITY_TOKEN_FILE

should be. It cause program exit.

@taikulawo
Copy link

[     0.029147s] ERROR ThreadId(01) linkerd_app::env: LINKERD2_PROXY_IDENTITY_SVC_ADDR and LINKERD2_PROXY_IDENTITY_SVC_NAME must be set.
[     0.029205s] ERROR ThreadId(01) linkerd_app::env: LINKERD2_PROXY_IDENTITY_TRUST_ANCHORS must be set.
[     0.029222s] ERROR ThreadId(01) linkerd_app::env: LINKERD2_PROXY_IDENTITY_DIR must be set.
[     0.029239s] ERROR ThreadId(01) linkerd_app::env: LINKERD2_PROXY_IDENTITY_LOCAL_NAME must be set.
[     0.029256s] ERROR ThreadId(01) linkerd_app::env: LINKERD2_PROXY_IDENTITY_TOKEN_FILE must be set.
[     0.029342s]  INFO ThreadId(01) linkerd_app::env: `LINKERD2_PROXY_INBOUND_IPS` allowlist not configured, allowing all target addresses
[     0.029408s]  INFO ThreadId(01) linkerd_app::env: LINKERD2_PROXY_POLICY_CLUSTER_NETWORKS not set; cluster-scoped modes are unsupported
[     0.029432s]  WARN ThreadId(01) linkerd_app::env: LINKERD2_PROXY_INBOUND_DEFAULT_POLICY was not set; using `all-unauthenticated`

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