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.

Sign up for GitHub

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

Jump to bottom

Remove the reuirement of SAN for root and intermediate certs #5228

Closed
Pothulapati opened this issue Nov 13, 2020 · 4 comments · Fixed by linkerd/website#860 or #5237
Closed

Remove the reuirement of SAN for root and intermediate certs #5228

Pothulapati opened this issue Nov 13, 2020 · 4 comments · Fixed by linkerd/website#860 or #5237
Assignees
@Pothulapati
Labels
area/cli priority/P0 Release Blocker
Milestone
stable-2.10

Comments

@Pothulapati
Copy link
Contributor

So, The step folks replied that it is not advised to use --san with root and intermediate certs. But our CLI uses the x509.verify fn which is used to validate leaf certs not root and intermediate and hence has a strict requirement on verifying hostnames through SAN only. The code path starts here

We should,

The SAN work we did with webhooks is still useful as they are used as leaf certs.
@frigus02 frigus02 mentioned this issue Nov 14, 2020
Closed
Pothulapati added a commit to linkerd/website that referenced this issue Nov 16, 2020
@Pothulapati
docs: remove --san with root and intermediate cert generation
cecb2cf 
Fixes linkerd/linkerd2#5228, #858

Signed-off-by: Tarun Pothulapati <tarunpothulapati@outlook.com>
@Pothulapati Pothulapati mentioned this issue Nov 16, 2020
Merged
Pothulapati added a commit that referenced this issue Nov 16, 2020
@Pothulapati
cli: don't check for SAN in root and intermediate Certs
2fd8345 
Fixes #5228

As discussed in the issue, It is not correct for root and intermediate
certs to have SAN. This PR updates the check to not verify the
intermediate issuer cert with the identity dns name (which checks with
SAN and not CN as the the `verify` func is used to verify leaf certs and
not root and intermediate certs). This PR also removes the extra san
field added in cert generation in CLI which was added previously.

Signed-off-by: Tarun Pothulapati <tarunpothulapati@outlook.com>
@Pothulapati Pothulapati mentioned this issue Nov 16, 2020
Merged
@olix0r olix0r assigned Pothulapati and unassigned Pothulapati Nov 16, 2020
olix0r pushed a commit that referenced this issue Nov 18, 2020
@Pothulapati
cli: Don't check for SAN in root and intermediate certs (#5237)
b389054 
As discussed in #5228, it is not correct for root and intermediate
certs to have SAN. This PR updates the check to not verify the
intermediate issuer cert with the identity dns name (which checks with
SAN and not CN as the the `verify` func is used to verify leaf certs and
not root and intermediate certs). This PR also avoids setting a SAN
field when generating certs in the `install` command.

Fixes #5228
@jiraguha
Copy link
Contributor

Thanks, @frigus02 and @Pothulapati ! I just spent hours trying to figure out what is:

x509: certificate relies on legacy Common Name field, use SANs or temporarily enable Common Name matching with GODEBUG=x509ignoreCN=0

When launching linkerd check... It was just linkerd brew-cli related.

I moved to the bash installation what is better now. I hope that no one else will fall in the same trap.
Maybe this question was asked but why both cli (brew and bash bin) do not have the same behavior?

@Pothulapati
Copy link
Contributor Author

@jiraguha Which version of Linkerd was this? This should not happen in the latest linkerd edge.

This happened because brew builds its own binaries with go 1.15 which had this breaking change!

@Pothulapati
Copy link
Contributor Author

Looks like Homebrew only builds stable binaries and hence the new edge is not present there. https://formulae.brew.sh/formula/linkerd#default
We are doing a stable release i.e 2.9.1 next week probably which should be propogated into homebrew!

@alpeb Do we also push the edge binaries through Homebrew?

@jiraguha
Copy link
Contributor

Which version of Linkerd was this?

on the lastest helm/stable 2.9.0

alpeb pushed a commit that referenced this issue Nov 30, 2020
@Pothulapati @alpeb
cli: Don't check for SAN in root and intermediate certs (#5237)
37c49a6 
As discussed in #5228, it is not correct for root and intermediate
certs to have SAN. This PR updates the check to not verify the
intermediate issuer cert with the identity dns name (which checks with
SAN and not CN as the the `verify` func is used to verify leaf certs and
not root and intermediate certs). This PR also avoids setting a SAN
field when generating certs in the `install` command.

Fixes #5228
alpeb pushed a commit to alpeb/linkerd2 that referenced this issue Dec 1, 2020
@Pothulapati @alpeb
cli: Don't check for SAN in root and intermediate certs (linkerd#5237)
da46aaa 
As discussed in linkerd#5228, it is not correct for root and intermediate
certs to have SAN. This PR updates the check to not verify the
intermediate issuer cert with the identity dns name (which checks with
SAN and not CN as the the `verify` func is used to verify leaf certs and
not root and intermediate certs). This PR also avoids setting a SAN
field when generating certs in the `install` command.

Fixes linkerd#5228
alpeb added a commit to alpeb/linkerd2 that referenced this issue Dec 1, 2020
@alpeb @olix0r
@kleimkuhler @Pothulapati
Alpeb/stable 2.9.1 (#25)
78f701c 
* proxy: v2.119.0 (linkerd#5200)

This release modifies the default idle timeout to 5s for outbound
clients and 20s for inbound clients. This prevents idle clients from
consuming memory at the cost of performing more discovery resolutions
for periodic but infrequent traffic. This is intended to reduce the
proxy's memory footprint, especially on Prometheus instances.

The proxy's *ring* and rustls dependencies have also been updated.

---

* Update *ring* and rustls dependencies (linkerd/linkerd2-proxy#735)
* http: Configure client connection pools (linkerd/linkerd2-proxy#734)

* Add endpoint to GetProfile response (linkerd#5227)

Context: linkerd#5209

This updates the destination service to set the `Endpoint` field in `GetProfile`
responses.

The `Endpoint` field is only set if the IP maps to a Pod--not a Service.

Additionally in this scenario, the default Service Profile is used as the base
profile so no other significant fields are set.

### Examples

```
# GetProfile for an IP that maps to a Service
❯ go run controller/script/destination-client/main.go -method getProfile -path 10.43.222.0:9090
INFO[0000] fully_qualified_name:"linkerd-prometheus.linkerd.svc.cluster.local"  retry_budget:{retry_ratio:0.2  min_retries_per_second:10  ttl:{seconds:10}}  dst_overrides:{authority:"linkerd-prometheus.linkerd.svc.cluster.local.:9090"  weight:10000}
```

Before:

```
# GetProfile for an IP that maps to a Pod
❯ go run controller/script/destination-client/main.go -method getProfile -path 10.42.0.20
INFO[0000] retry_budget:{retry_ratio:0.2 min_retries_per_second:10 ttl:{seconds:10}}
```

After:

```
# GetProfile for an IP that maps to a Pod
❯ go run controller/script/destination-client/main.go -method getProfile -path 10.42.0.20
INFO[0000] retry_budget:{retry_ratio:0.2  min_retries_per_second:10  ttl:{seconds:10}}  endpoint:{addr:{ip:{ipv4:170524692}}  weight:10000  metric_labels:{key:"control_plane_ns"  value:"linkerd"}  metric_labels:{key:"deployment"  value:"fast-1"}  metric_labels:{key:"pod"  value:"fast-1-5cc87f64bc-9hx7h"}  metric_labels:{key:"pod_template_hash"  value:"5cc87f64bc"}  metric_labels:{key:"serviceaccount"  value:"default"}  tls_identity:{dns_like_identity:{name:"default.default.serviceaccount.identity.linkerd.cluster.local"}}  protocol_hint:{h2:{}}}
```

Signed-off-by: Kevin Leimkuhler <kevin@kleimkuhler.com>

* cli: Fix custom namespace installation (linkerd#5241)

The `--linkerd-namespace` flag was not honored by the `install`
command. This change updates the install templating to use the
value of this flag.

* cli: Don't check for SAN in root and intermediate certs (linkerd#5237)

As discussed in linkerd#5228, it is not correct for root and intermediate
certs to have SAN. This PR updates the check to not verify the
intermediate issuer cert with the identity dns name (which checks with
SAN and not CN as the the `verify` func is used to verify leaf certs and
not root and intermediate certs). This PR also avoids setting a SAN
field when generating certs in the `install` command.

Fixes linkerd#5228

* proxy: v2.121.0 (linkerd#5253)

This release changes error handling to teardown the server-side
connection when an unexpected error is encountered.

Additionally, the outbound TCP routing stack can now skip redundant
service discovery lookups when profile responses include endpoint
information.

Finally, the cache implementation has been updated to reduce latency by
removing unnecessary buffers.

---

* h2: enable HTTP/2 keepalive PING frames (linkerd/linkerd2-proxy#737)
* actions: Add timeouts to GitHub actions (linkerd/linkerd2-proxy#738)
* outbound: Skip endpoint resolution on profile hint (linkerd/linkerd2-proxy#736)
* Add a FromStr for dns::Name (linkerd/linkerd2-proxy#746)
* outbound: Avoid redundant TCP endpoint resolution (linkerd/linkerd2-proxy#742)
* cache: Make the cache cloneable with RwLock (linkerd/linkerd2-proxy#743)
* http: Teardown serverside connections on error (linkerd/linkerd2-proxy#747)

* Check correct label value when setting protocl hint (linkerd#5267)

This fixes an issue where the protocol hint is always set on endpoint responses.
We now check the right value which determines if the pod has the required label.

A test for this has been added to linkerd#5266.

Signed-off-by: Kevin Leimkuhler <kevin@kleimkuhler.com>

* proxy: v2.122.0 (linkerd#5279)

This release addresses some issues reported around clients seeing
max-concurrency errors by increasing the default in-flight request limit
to 100K pending requests.

Additionally, the proxy now sets an appropriate content-type when
synthesizing gRPC error responses.

---

* style: fix some random clippy lints (linkerd/linkerd2-proxy#749)
* errors: Set `content-type` for synthesized grpc errors (linkerd/linkerd2-proxy#750)
* concurrency-limit: Drop permit on readiness (linkerd/linkerd2-proxy#751)
* Increase the default buffer capacity to 100K (linkerd/linkerd2-proxy#752)
* Change default max-in-flight and buffer-capacity (linkerd/linkerd2-proxy#753)

* notes for 2.9.1

Co-authored-by: Oliver Gould <ver@buoyant.io>
Co-authored-by: Kevin Leimkuhler <kevin@kleimkuhler.com>
Co-authored-by: Tarun Pothulapati <tarunpothulapati@outlook.com>
alpeb pushed a commit that referenced this issue Dec 1, 2020
@Pothulapati @alpeb
cli: Don't check for SAN in root and intermediate certs (#5237)
6bc9347 
As discussed in #5228, it is not correct for root and intermediate
certs to have SAN. This PR updates the check to not verify the
intermediate issuer cert with the identity dns name (which checks with
SAN and not CN as the the `verify` func is used to verify leaf certs and
not root and intermediate certs). This PR also avoids setting a SAN
field when generating certs in the `install` command.

Fixes #5228
GMarkfjard pushed a commit to GMarkfjard/linkerd2 that referenced this issue Dec 2, 2020
@Pothulapati @GMarkfjard
cli: Don't check for SAN in root and intermediate certs (linkerd#5237)
2acdf69 
As discussed in linkerd#5228, it is not correct for root and intermediate
certs to have SAN. This PR updates the check to not verify the
intermediate issuer cert with the identity dns name (which checks with
SAN and not CN as the the `verify` func is used to verify leaf certs and
not root and intermediate certs). This PR also avoids setting a SAN
field when generating certs in the `install` command.

Fixes linkerd#5228
@alpeb alpeb mentioned this issue Dec 2, 2020
Closed
9 tasks
alpeb added a commit that referenced this issue Dec 4, 2020
@alpeb @olix0r
@Pothulapati @kleimkuhler @hodbn @itaischwartz
Cherry-picks and release notes for stable-2.9.1 (#5302)
7ff1d01 
* proxy: v2.119.0 (#5200)

This release modifies the default idle timeout to 5s for outbound
clients and 20s for inbound clients. This prevents idle clients from
consuming memory at the cost of performing more discovery resolutions
for periodic but infrequent traffic. This is intended to reduce the
proxy's memory footprint, especially on Prometheus instances.

The proxy's *ring* and rustls dependencies have also been updated.

---

* Update *ring* and rustls dependencies (linkerd/linkerd2-proxy#735)
* http: Configure client connection pools (linkerd/linkerd2-proxy#734)

* cli: Remove get cmd and relevant tests (#5202)

Fixes #5190

`linkerd get` is not used currently and works only for pods. This can be
removed instead as per the issue. This branch removes the command and
also the associated unit and integration tests.

Signed-off-by: Tarun Pothulapati <tarunpothulapati@outlook.com>

* cli: remove logs subcommand and tests (#5203)

Fixes #5191

The logs command adds a external dependency that we forked to work but
does not fit within linkerd's core set of responsibilities. Hence, This
is being removed.

For capabilities like this, The Kubernetes plugin ecosystem has better
and well maintained tools that can be used.

Signed-off-by: Tarun Pothulapati <tarunpothulapati@outlook.com>

* Remove logs comparisons in integration tests (#5223)

The rare cases where these tests were useful don't make up for the burden of
maintaing them, having different k8s version change the messages and
having unexpected warnings come up that didn't affect the final
convergence of the system.

With this we also revert the indirection added back in #4538 that
fetched unmatched warnings after a test had failed.

* Add endpoint to GetProfile response (#5227)

Context: #5209

This updates the destination service to set the `Endpoint` field in `GetProfile`
responses.

The `Endpoint` field is only set if the IP maps to a Pod--not a Service.

Additionally in this scenario, the default Service Profile is used as the base
profile so no other significant fields are set.

### Examples

```
# GetProfile for an IP that maps to a Service
❯ go run controller/script/destination-client/main.go -method getProfile -path 10.43.222.0:9090
INFO[0000] fully_qualified_name:"linkerd-prometheus.linkerd.svc.cluster.local"  retry_budget:{retry_ratio:0.2  min_retries_per_second:10  ttl:{seconds:10}}  dst_overrides:{authority:"linkerd-prometheus.linkerd.svc.cluster.local.:9090"  weight:10000}
```

Before:

```
# GetProfile for an IP that maps to a Pod
❯ go run controller/script/destination-client/main.go -method getProfile -path 10.42.0.20
INFO[0000] retry_budget:{retry_ratio:0.2 min_retries_per_second:10 ttl:{seconds:10}}
```

After:

```
# GetProfile for an IP that maps to a Pod
❯ go run controller/script/destination-client/main.go -method getProfile -path 10.42.0.20
INFO[0000] retry_budget:{retry_ratio:0.2  min_retries_per_second:10  ttl:{seconds:10}}  endpoint:{addr:{ip:{ipv4:170524692}}  weight:10000  metric_labels:{key:"control_plane_ns"  value:"linkerd"}  metric_labels:{key:"deployment"  value:"fast-1"}  metric_labels:{key:"pod"  value:"fast-1-5cc87f64bc-9hx7h"}  metric_labels:{key:"pod_template_hash"  value:"5cc87f64bc"}  metric_labels:{key:"serviceaccount"  value:"default"}  tls_identity:{dns_like_identity:{name:"default.default.serviceaccount.identity.linkerd.cluster.local"}}  protocol_hint:{h2:{}}}
```

Signed-off-by: Kevin Leimkuhler <kevin@kleimkuhler.com>

* cli: Fix custom namespace installation (#5241)

The `--linkerd-namespace` flag was not honored by the `install`
command. This change updates the install templating to use the
value of this flag.

* cli: Don't check for SAN in root and intermediate certs (#5237)

As discussed in #5228, it is not correct for root and intermediate
certs to have SAN. This PR updates the check to not verify the
intermediate issuer cert with the identity dns name (which checks with
SAN and not CN as the the `verify` func is used to verify leaf certs and
not root and intermediate certs). This PR also avoids setting a SAN
field when generating certs in the `install` command.

Fixes #5228

* proxy: v2.121.0 (#5253)

This release changes error handling to teardown the server-side
connection when an unexpected error is encountered.

Additionally, the outbound TCP routing stack can now skip redundant
service discovery lookups when profile responses include endpoint
information.

Finally, the cache implementation has been updated to reduce latency by
removing unnecessary buffers.

---

* h2: enable HTTP/2 keepalive PING frames (linkerd/linkerd2-proxy#737)
* actions: Add timeouts to GitHub actions (linkerd/linkerd2-proxy#738)
* outbound: Skip endpoint resolution on profile hint (linkerd/linkerd2-proxy#736)
* Add a FromStr for dns::Name (linkerd/linkerd2-proxy#746)
* outbound: Avoid redundant TCP endpoint resolution (linkerd/linkerd2-proxy#742)
* cache: Make the cache cloneable with RwLock (linkerd/linkerd2-proxy#743)
* http: Teardown serverside connections on error (linkerd/linkerd2-proxy#747)

* Check correct label value when setting protocl hint (#5267)

This fixes an issue where the protocol hint is always set on endpoint responses.
We now check the right value which determines if the pod has the required label.

A test for this has been added to #5266.

Signed-off-by: Kevin Leimkuhler <kevin@kleimkuhler.com>

* Add safe accessor for Global in linkerd-config (#5269)

CLI crashes if linkerd-config contains unexpected values.

Add a safe accessor that initializes an empty Global on the first
access. Refactor all accesses to use the newly introduced accessor using
gopls.

Add test for linkerd-config data without Global.

Fixes #5215

Co-authored-by: Itai Schwartz <yitai27@gmail.com>
Signed-off-by: hodbn <hodbn@users.noreply.github.com>

* proxy: v2.122.0 (#5279)

This release addresses some issues reported around clients seeing
max-concurrency errors by increasing the default in-flight request limit
to 100K pending requests.

Additionally, the proxy now sets an appropriate content-type when
synthesizing gRPC error responses.

---

* style: fix some random clippy lints (linkerd/linkerd2-proxy#749)
* errors: Set `content-type` for synthesized grpc errors (linkerd/linkerd2-proxy#750)
* concurrency-limit: Drop permit on readiness (linkerd/linkerd2-proxy#751)
* Increase the default buffer capacity to 100K (linkerd/linkerd2-proxy#752)
* Change default max-in-flight and buffer-capacity (linkerd/linkerd2-proxy#753)

* proxy: v2.123.0 (#5301)

This release removes a potential panic: it was assumed that looking up a
socket's peer address was infallible, but in practice this call can
fail when a host is under high load. Now these failures only impact the
connection-level task and not the whole proxy proces.

Also, the `process_cpu_seconds_total` metric is now exposed as a float
so that its value may include fractional seconds with 10ms granularity.

---

* io: Make peer_addr fallible (linkerd/linkerd2-proxy#755)
* metrics: Expose process_cpu_seconds_total as a float (linkerd/linkerd2-proxy#754)

* Release notes for stable-2.9.1

## stable-2.9.1

This stable release contains a number of proxy enhancements: better support for
high-traffic workloads, improved performance by eliminating unnecessary endpoint
resolutions for TCP traffic and properly tearing down serverside connections
when errors occur, and reduced memory consumption on proxies which maintain many
idle connections (such as Prometheus' proxy).

On the CLI and control plane sides, it relaxes checks on root and intermediate
certificates (following X509 best practices), and fixes two issues: one that
prevented installation of the control plane into a custom namespace and one
which failed to update endpoint information when a headless service was
modified.

* Proxy:
  * Addressed some issues reported around clients seeing max-concurrency errors
    by increasing the default in-flight request limit to 100K pending requests
  * Reduced the default idle connection timeout to 5s for outbound clients and
    for inbound clients to reduce the proxy's memory footprint, especially on
      Prometheus instances
  * Fixed an issue where the proxy did not receive updated endpoint information
    when a headless service was modified
  * Added HTTP/2 keepalive PING frames
  * Removed logic to avoid redundant TCP endpoint resolution
  * Fixed an issue where serverside connections were not torn down when an error
    occurred

* CLI / Control Plane:
  * Fixed a CLI issue where the `linkerd-namespace` flag was not honored when
    passed to the `install` and `upgrade` commands
  * Updated `linkerd check` so that it doesn't attempt to validate the subject
    alternative name (SAN) on root and intermediate certificates. SANs for leaf
    certificates will continue to be validated
  * Fixed an issue in the destination service where endpoints always included a
    protocol hint, regardless of the controller label being present or not
  * Removed the `get` and `logs` command from the CLI
  * No longer panic in rare cases when `linkerd-config` doesn't have an entry
    for `Global` configs (thanks @hodbn!)

* proxy: v2.124.0 (#5323)

This release updates the proxy's `*ring*` dependency to pick up the
latest changes from BoringSSL.

Additionally, we've audited uses of non-cryptographic random number
generators in the proxy to ensure that each balancer/router intializes
its own RNG state.

---

* Audit uses of SmallRng (linkerd/linkerd2-proxy#757)
* Update *ring* to 0.6.19 (linkerd/linkerd2-proxy#758)
* metrics: Support the Summary metric type (linkerd/linkerd2-proxy#756)

Co-authored-by: Oliver Gould <ver@buoyant.io>
Co-authored-by: Tarun Pothulapati <tarunpothulapati@outlook.com>
Co-authored-by: Kevin Leimkuhler <kevin@kleimkuhler.com>
Co-authored-by: hodbn <hodbn@users.noreply.github.com>
Co-authored-by: Itai Schwartz <yitai27@gmail.com>
alpeb added a commit to linkerd/website that referenced this issue Dec 10, 2020
@Pothulapati @alpeb
docs: remove --san with root and intermediate cert generation (#860)
f743fc3 
Fixes linkerd/linkerd2#5228, #858

Signed-off-by: Tarun Pothulapati <tarunpothulapati@outlook.com>
Co-authored-by: Alejandro Pedraza <alejandro@buoyant.io>
@olix0r olix0r added this to the stable-2.10 milestone Jan 11, 2021
@jjcollinge jjcollinge mentioned this issue May 20, 2021
Closed
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@Pothulapati Pothulapati

Labels
area/cli priority/P0 Release Blocker
Projects
None yet
Milestone
stable-2.10
3 participants
@olix0r @Pothulapati @jiraguha