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

feat(kuma-cp): separate CA for Envoy Admin communication #4676

Merged

Conversation

jakubdyszkiewicz
Copy link
Contributor

Summary

This PR introduces a separate CA for Envoy Admin communication (CP making requests to the DP).

So far we protected the Envoy Admin server by:

  • Mesh mTLS if mTLS is enabled and proxy type is Dataplane. Verified by the CP.
  • Self-signed in-memory cert for mTLS off and ZoneIngress / Zone Egress (not mesh scoped). Not verified by the CP (not stable between instances of CP).

For the client cert that CP presented, we used DP server cert.

This was implemented that way so we can avoid introducing yet another CA/certs that we need to explain to the user.

It worked for most cases, but for some edge cases when a specific DP server cert was provided it didn't.
DP server cert has x509 extension of ExtKeyUsageServerAuth so using it as a client cert is technically wrong even though it worked for the majority of cases.

Now, we autogenerate separate CA and store it in GlobalSecret from which we generate:

  • server certs for each DP with SAN of IP from Networking#address
  • client certs for CP with SAN of kuma-cp

CA can be replaced, but after replacing, we need to restart all instances of the CP.
To implement it without restart, we would need to do a lot of I/O and cert generation.
Since CP -> DP Envoy Admin communication is not critical, there can easily be downtime while replacing the CA.

Security threats

Obtaining server cert
A malicious actor can present a fake config dump, stats, or clusters. Not much damage.
They cannot impersonate service-to-service communication.

Obtaining client cert == obtaining CA
A malicious actor can terminate all Envoys.

Multizone

We don't sync it because there is no need for it.

Issues resolved

Fix #4582

Documentation

Should we document this or keep it internal?

Testing

  • Unit tests
  • E2E tests
  • Manual testing on Universal
  • Manual testing on Kubernetes

Backwards compatibility

We don't need to build backward compatibility because there can be downtime when we swap tls mechanism.

- [ ] Update UPGRADE.md with any steps users will need to take when upgrading.
- [ ] Add backport-to-stable label if the code follows our backporting policy

Signed-off-by: Jakub Dyszkiewicz <jakub.dyszkiewicz@gmail.com>
@jakubdyszkiewicz jakubdyszkiewicz requested a review from a team as a code owner July 26, 2022 09:46
Signed-off-by: Jakub Dyszkiewicz <jakub.dyszkiewicz@gmail.com>
Signed-off-by: Jakub Dyszkiewicz <jakub.dyszkiewicz@gmail.com>
@codecov-commenter
Copy link

codecov-commenter commented Jul 27, 2022

Codecov Report

Merging #4676 (579a8eb) into master (f604312) will decrease coverage by 0.01%.
The diff coverage is 22.09%.

@@            Coverage Diff             @@
##           master    #4676      +/-   ##
==========================================
- Coverage   46.47%   46.45%   -0.02%     
==========================================
  Files         684      686       +2     
  Lines       46502    46577      +75     
==========================================
+ Hits        21610    21636      +26     
- Misses      22974    23019      +45     
- Partials     1918     1922       +4     
Impacted Files Coverage Δ
pkg/core/bootstrap/bootstrap.go 0.00% <0.00%> (ø)
pkg/core/xds/types.go 41.93% <ø> (ø)
pkg/defaults/components.go 53.33% <0.00%> (-3.81%) ⬇️
pkg/envoy/admin/client.go 0.00% <0.00%> (ø)
pkg/envoy/admin/tls/pki.go 0.00% <0.00%> (ø)
...kg/xds/envoy/listeners/filter_chain_configurers.go 14.19% <0.00%> (+0.09%) ⬆️
pkg/xds/sync/components.go 31.14% <0.00%> (-0.52%) ⬇️
pkg/xds/sync/dataplane_watchdog.go 0.00% <0.00%> (ø)
pkg/xds/sync/dataplane_watchdog_factory.go 0.00% <0.00%> (ø)
pkg/defaults/envoy_admin_ca.go 50.00% <50.00%> (ø)
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f604312...579a8eb. Read the comment docs.

@jakubdyszkiewicz jakubdyszkiewicz merged commit 0cb0e06 into kumahq:master Jul 29, 2022
@jakubdyszkiewicz jakubdyszkiewicz deleted the feat/admin-client-cert branch July 29, 2022 11:07
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.

Use mTLS certs as a client cert when executing Envoy Admin actions from CP
4 participants