diff --git a/api/client/contextdialer.go b/api/client/contextdialer.go index a2fc357549b0c..e0fb6d9a6aa0e 100644 --- a/api/client/contextdialer.go +++ b/api/client/contextdialer.go @@ -364,11 +364,8 @@ func newTLSRoutingWithConnUpgradeDialer(ssh ssh.ClientConfig, params connectPara }, ALPNConnUpgradeRequired: IsALPNConnUpgradeRequired(ctx, params.addr, insecure), GetClusterCAs: func(_ context.Context) (*x509.CertPool, error) { - tlsConfig, err := params.cfg.Credentials[0].TLSConfig() - if err != nil { - return nil, trace.Wrap(err) - } - return tlsConfig.RootCAs, nil + // Uses the Root CAs from the TLS Config of the Credentials. + return params.tlsConfig.RootCAs, nil }, }) if err != nil { diff --git a/api/client/credentials.go b/api/client/credentials.go index 87e698ee2802c..21bf758f7c886 100644 --- a/api/client/credentials.go +++ b/api/client/credentials.go @@ -452,6 +452,8 @@ func (d *DynamicIdentityFileCreds) Reload() error { // TLSConfig returns TLS configuration. Implementing the Credentials interface. func (d *DynamicIdentityFileCreds) TLSConfig() (*tls.Config, error) { + d.mu.RLock() + defer d.mu.RUnlock() // Build a "dynamic" tls.Config which can support a changing cert and root // CA pool. cfg := &tls.Config{ @@ -473,7 +475,10 @@ func (d *DynamicIdentityFileCreds) TLSConfig() (*tls.Config, error) { }, // VerifyConnection is used instead of the static RootCAs field. - RootCAs: nil, + // However, there's some client code which relies on the static RootCAs + // field. So we set it to a copy of the current root CAs pool to support + // those - e.g ALPNDialerConfig.GetClusterCAs + RootCAs: d.tlsRootCAs.Clone(), // InsecureSkipVerify is forced true to ensure that only our // VerifyConnection callback is used to verify the server's presented // certificate. @@ -488,7 +493,7 @@ func (d *DynamicIdentityFileCreds) TLSConfig() (*tls.Config, error) { opts := x509.VerifyOptions{ DNSName: state.ServerName, Intermediates: x509.NewCertPool(), - Roots: d.tlsRootCAs, + Roots: d.tlsRootCAs.Clone(), } for _, cert := range state.PeerCertificates[1:] { // Whilst we don't currently use intermediate certs at diff --git a/api/client/credentials_test.go b/api/client/credentials_test.go index 34ec62e8129b7..93649e2555956 100644 --- a/api/client/credentials_test.go +++ b/api/client/credentials_test.go @@ -416,6 +416,13 @@ func TestDynamicIdentityFileCreds(t *testing.T) { require.NoError(t, err) require.Equal(t, wantTLSCert, *gotTLSCert) + tlsCACertPEM, _ := pem.Decode(tlsCACert) + tlsCACertDER, err := x509.ParseCertificate(tlsCACertPEM.Bytes) + require.NoError(t, err) + wantCertPool := x509.NewCertPool() + wantCertPool.AddCert(tlsCACertDER) + require.True(t, wantCertPool.Equal(tlsConfig.RootCAs), "tlsconfig.RootCAs mismatch") + // Generate a new TLS certificate that contains the same private key as // the original. template := &x509.Certificate{