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

Use internal server certificate for peering TLS #14796

Merged
merged 4 commits into from
Oct 7, 2022
Merged

Conversation

freddygv
Copy link
Contributor

@freddygv freddygv commented Sep 29, 2022

Description

#14485 and #14556 introduced an internally-managed server certificate
to use for peering-related purposes.

Now the peering token has been updated to match that behavior:

  • The server name matches the structure of the server cert
  • The CA PEMs correspond to the Connect CA

Testing & Reproduction steps

  • Unit and integration tests

PR Checklist

  • updated test coverage
  • external facing docs updated
  • not a security concern

@freddygv freddygv requested a review from a team September 29, 2022 14:03
@github-actions github-actions bot added theme/api Relating to the HTTP API interface theme/cli Flags and documentation for the CLI interface theme/connect Anything related to Consul Connect, Service Mesh, Side Car Proxies labels Sep 29, 2022
@freddygv
Copy link
Contributor Author

freddygv commented Sep 29, 2022

Hmm will look into the remaining test failures tomorrow done

@freddygv freddygv force-pushed the NET-643-update-mesh-gateway-envoy-config-for-inbound-peering-control-plane-traffic branch from 0d6d338 to b15d415 Compare October 3, 2022 18:42
Base automatically changed from NET-643-update-mesh-gateway-envoy-config-for-inbound-peering-control-plane-traffic to main October 3, 2022 18:54
return b.srv.tlsConfigurator.GRPCManualCAPems(), nil
// GetTLSMaterials returns the TLS materials for the dialer to dial the acceptor using TLS.
// It returns the server name to validate, and the CA certificate to validate with.
func (b *PeeringBackend) GetTLSMaterials(generatingToken bool) (string, []string, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm probably missing something, but am not seeing the code for falling back to the manually configured
certs. Where does that happen?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch! We decided to not have that fallback anymore. Since Connect is now required when generating tokens we're just always going to use the internally managed cert.

If someone wants to adjust details about how the CA is managed they can do that by updating the Connect CA config.

I updated the PR description to remove the fallback message.

Copy link
Contributor

Choose a reason for hiding this comment

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

Awesome, thank you

A previous commit introduced an internally-managed server certificate
to use for peering-related purposes.

Now the peering token has been updated to match that behavior:
- The server name matches the structure of the server cert
- The CA PEMs correspond to the Connect CA

Note that if Conect is disabled, and by extension the Connect CA, we
fall back to the previous behavior of returning the manually configured
certs and local server SNI.

Several tests were updated to use the gRPC TLS port since they enable
Connect by default. This means that the peering token will embed the
Connect CA, and the dialer will expect a TLS listener.
By requiring Connect and a gRPC TLS listener we can automatically
configure TLS for all peering control-plane traffic.
@freddygv freddygv merged commit 4abad02 into main Oct 7, 2022
@freddygv freddygv deleted the peering/use-connect-ca branch October 7, 2022 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme/api Relating to the HTTP API interface theme/cli Flags and documentation for the CLI interface theme/connect Anything related to Consul Connect, Service Mesh, Side Car Proxies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants