From 4f8c22bd67961d4b2ece9968aa0aed90de073d97 Mon Sep 17 00:00:00 2001 From: Noah Date: Fri, 14 Jul 2023 18:11:18 +0100 Subject: [PATCH 01/12] Skeleton of hot-swappable tls credentials for api client --- api/client/dynamic_credentials.go | 129 ++++++++++++++++++++++++++++++ 1 file changed, 129 insertions(+) create mode 100644 api/client/dynamic_credentials.go diff --git a/api/client/dynamic_credentials.go b/api/client/dynamic_credentials.go new file mode 100644 index 0000000000000..dd2e6d12459a0 --- /dev/null +++ b/api/client/dynamic_credentials.go @@ -0,0 +1,129 @@ +package client + +import ( + "crypto/tls" + "crypto/x509" + "github.com/gravitational/teleport/api/identityfile" + "github.com/gravitational/teleport/api/utils" + "github.com/gravitational/teleport/api/utils/keys" + "github.com/gravitational/trace" + "golang.org/x/crypto/ssh" + "golang.org/x/net/http2" + "sync" +) + +type WatchedIdentityFileCreds struct { + mu sync.RWMutex + tlsCert *tls.Certificate + pool *x509.CertPool + + path string + clusterName string +} + +func LoadAndWatchIdentityFile(path string, clusterName string) (*WatchedIdentityFileCreds, error) { + d := &WatchedIdentityFileCreds{ + path: path, + clusterName: clusterName, + } + + err := d.Reload() + if err != nil { + return nil, trace.Wrap(err) + } + + return d, nil +} + +func (d *WatchedIdentityFileCreds) Reload() error { + id, err := identityfile.ReadFile(d.path) + if err != nil { + return trace.Wrap(err) + } + + // This section is essentially id.TLSConfig() + cert, err := keys.X509KeyPair(id.Certs.TLS, id.PrivateKey) + if err != nil { + return trace.Wrap(err) + } + pool := x509.NewCertPool() + for _, caCerts := range id.CACerts.TLS { + if !pool.AppendCertsFromPEM(caCerts) { + return trace.BadParameter("invalid CA cert PEM") + } + } + + d.mu.Lock() + d.pool = pool + d.tlsCert = &cert + d.mu.Unlock() + return nil +} + +// Dialer is used to dial a connection to an Auth server. +func (d *WatchedIdentityFileCreds) Dialer( + _ Config, +) (ContextDialer, error) { + // Returning a dialer isn't necessary for this credential. + return nil, trace.NotImplemented("no dialer") +} + +// TLSConfig returns TLS configuration. +func (d *WatchedIdentityFileCreds) TLSConfig() (*tls.Config, error) { + cfg := &tls.Config{ + // GetClientCertificate is used instead of the static Certificates + // field. + Certificates: nil, + // Encoded cluster name required to ensure requests are routed to the + // correct cloud tenants. + // TODO: Make this optional - yet encouraged. + ServerName: utils.EncodeClusterName(d.clusterName), + GetClientCertificate: func( + info *tls.CertificateRequestInfo, + ) (*tls.Certificate, error) { + // GetClientCertificate callback is used to allow us to dynamically + // change the certificate when reloaded. + d.mu.RLock() + defer d.mu.RUnlock() + return d.tlsCert, nil + }, + // InsecureSkipVerify is forced true to ensure that only our + // VerifyConnection callback is used to verify the server's presented + // certificate. + InsecureSkipVerify: true, + VerifyConnection: func(state tls.ConnectionState) error { + // This VerifyConnection callback is based on the standard library + // implementation of verifyServerCertificate in the tls package. + // We provide our own implementation so we can dynamically handle + // a changing CA Roots pool. + d.mu.RLock() + defer d.mu.RUnlock() + + opts := x509.VerifyOptions{ + DNSName: state.ServerName, + Intermediates: x509.NewCertPool(), + Roots: d.pool, + } + for _, cert := range state.PeerCertificates[1:] { + // Whilst we don't currently use intermediate certs at + // Teleport, including this here means that we are + // future-proofed in case we do. + opts.Intermediates.AddCert(cert) + } + _, err := state.PeerCertificates[0].Verify(opts) + return err + }, + NextProtos: []string{http2.NextProtoTLS}, + } + + return cfg, nil +} + +// SSHClientConfig returns SSH configuration. +func (d *WatchedIdentityFileCreds) SSHClientConfig() (*ssh.ClientConfig, error) { + // For now, SSH Client Config is disabled until I can wrap my head around + // the changes needed to make an SSH config dynamic. + // This means the auth server must be available directly or using + // the ALPN/SNI. + return nil, trace.NotImplemented("no ssh config") +} From 8b6eaf313c454466cd1520bc0f9a1b7dfa326493 Mon Sep 17 00:00:00 2001 From: Noah Date: Fri, 14 Jul 2023 19:14:00 +0100 Subject: [PATCH 02/12] Start work on SSH support --- api/client/dynamic_credentials.go | 65 ++++++++++++++++++++++++++----- 1 file changed, 56 insertions(+), 9 deletions(-) diff --git a/api/client/dynamic_credentials.go b/api/client/dynamic_credentials.go index dd2e6d12459a0..b294ec809b134 100644 --- a/api/client/dynamic_credentials.go +++ b/api/client/dynamic_credentials.go @@ -1,11 +1,14 @@ package client import ( + "crypto" "crypto/tls" "crypto/x509" + "github.com/gravitational/teleport/api/constants" "github.com/gravitational/teleport/api/identityfile" "github.com/gravitational/teleport/api/utils" "github.com/gravitational/teleport/api/utils/keys" + "github.com/gravitational/teleport/api/utils/sshutils" "github.com/gravitational/trace" "golang.org/x/crypto/ssh" "golang.org/x/net/http2" @@ -16,6 +19,8 @@ type WatchedIdentityFileCreds struct { mu sync.RWMutex tlsCert *tls.Certificate pool *x509.CertPool + sshCert *ssh.Certificate + sshKey crypto.Signer path string clusterName string @@ -53,9 +58,27 @@ func (d *WatchedIdentityFileCreds) Reload() error { } } + // This sections is essentially id.SSHClientConfig() + sshCert, err := sshutils.ParseCertificate(id.Certs.SSH) + if err != nil { + return trace.Wrap(err) + } + sshPrivateKey, err := keys.ParsePrivateKey(id.PrivateKey) + if err != nil { + return trace.Wrap(err) + } + _, err := sshutils.ProxyClientSSHConfig( + sshCert, sshPrivateKey, id.CACerts.SSH..., + ) + if err != nil { + return trace.Wrap(err) + } + d.mu.Lock() d.pool = pool d.tlsCert = &cert + d.sshCert = sshCert + d.sshKey = sshPrivateKey d.mu.Unlock() return nil } @@ -74,10 +97,7 @@ func (d *WatchedIdentityFileCreds) TLSConfig() (*tls.Config, error) { // GetClientCertificate is used instead of the static Certificates // field. Certificates: nil, - // Encoded cluster name required to ensure requests are routed to the - // correct cloud tenants. - // TODO: Make this optional - yet encouraged. - ServerName: utils.EncodeClusterName(d.clusterName), + GetClientCertificate: func( info *tls.CertificateRequestInfo, ) (*tls.Certificate, error) { @@ -116,14 +136,41 @@ func (d *WatchedIdentityFileCreds) TLSConfig() (*tls.Config, error) { NextProtos: []string{http2.NextProtoTLS}, } + // If the cluster name has been provided, we should use that for SNI. + // This enables alpn routed connections on Teleport Cloud which relies on + // a distinct SNI to route traffic to the correct Teleport Tenant. + // TODO: Could we determine this from the cert 🤔 and not need to ask + // the user explicitly for this. + if d.clusterName != "" { + cfg.ServerName = utils.EncodeClusterName(d.clusterName) + } else { + // Otherwise fall back to `teleport.cluster.local` which should work + // in most general teleport installs. + cfg.ServerName = constants.APIDomain + } + return cfg, nil } // SSHClientConfig returns SSH configuration. func (d *WatchedIdentityFileCreds) SSHClientConfig() (*ssh.ClientConfig, error) { - // For now, SSH Client Config is disabled until I can wrap my head around - // the changes needed to make an SSH config dynamic. - // This means the auth server must be available directly or using - // the ALPN/SNI. - return nil, trace.NotImplemented("no ssh config") + // Build a "dynamic" ssh config. Based roughly on + // `sshutils.ProxyClientSSHConfig` with modifications to make it work with + // dynamically changing credentials. + cfg := &ssh.ClientConfig{ + Auth: []ssh.AuthMethod{ + ssh.PublicKeysCallback(func() (signers []ssh.Signer, err error) { + d.mu.RLock() + defer d.mu.Unlock() + + sshSigner, err := sshutils.SSHSigner(d.sshCert, d.sshKey) + if err != nil { + return nil, trace.Wrap(err) + } + return []ssh.Signer{sshSigner}, nil + }), + }, + } + + return cfg, nil } From b461243a868e856e4413a956806f2050a4e2b180 Mon Sep 17 00:00:00 2001 From: Noah Date: Tue, 15 Aug 2023 17:55:54 +0100 Subject: [PATCH 03/12] Add note to what needs doing --- api/client/dynamic_credentials.go | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/api/client/dynamic_credentials.go b/api/client/dynamic_credentials.go index b294ec809b134..888f90f8e163c 100644 --- a/api/client/dynamic_credentials.go +++ b/api/client/dynamic_credentials.go @@ -1,3 +1,19 @@ +/* +Copyright 2023 Gravitational, Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + package client import ( @@ -67,7 +83,7 @@ func (d *WatchedIdentityFileCreds) Reload() error { if err != nil { return trace.Wrap(err) } - _, err := sshutils.ProxyClientSSHConfig( + cc, err := sshutils.ProxyClientSSHConfig( sshCert, sshPrivateKey, id.CACerts.SSH..., ) if err != nil { @@ -157,6 +173,7 @@ func (d *WatchedIdentityFileCreds) SSHClientConfig() (*ssh.ClientConfig, error) // Build a "dynamic" ssh config. Based roughly on // `sshutils.ProxyClientSSHConfig` with modifications to make it work with // dynamically changing credentials. + // TODO: Handle changing host certs cfg := &ssh.ClientConfig{ Auth: []ssh.AuthMethod{ ssh.PublicKeysCallback(func() (signers []ssh.Signer, err error) { From 94f1ef19023a9bc481bda14b5dd91490fff9aee5 Mon Sep 17 00:00:00 2001 From: Noah Date: Wed, 16 Aug 2023 11:54:17 +0100 Subject: [PATCH 04/12] Support rotating ssh host cert --- api/client/dynamic_credentials.go | 46 ++++++++++++++++++++++++------- 1 file changed, 36 insertions(+), 10 deletions(-) diff --git a/api/client/dynamic_credentials.go b/api/client/dynamic_credentials.go index 888f90f8e163c..d485dc7d4433a 100644 --- a/api/client/dynamic_credentials.go +++ b/api/client/dynamic_credentials.go @@ -21,6 +21,7 @@ import ( "crypto/tls" "crypto/x509" "github.com/gravitational/teleport/api/constants" + "github.com/gravitational/teleport/api/defaults" "github.com/gravitational/teleport/api/identityfile" "github.com/gravitational/teleport/api/utils" "github.com/gravitational/teleport/api/utils/keys" @@ -28,15 +29,17 @@ import ( "github.com/gravitational/trace" "golang.org/x/crypto/ssh" "golang.org/x/net/http2" + "net" "sync" ) type WatchedIdentityFileCreds struct { - mu sync.RWMutex - tlsCert *tls.Certificate - pool *x509.CertPool - sshCert *ssh.Certificate - sshKey crypto.Signer + mu sync.RWMutex + tlsCert *tls.Certificate + pool *x509.CertPool + sshCert *ssh.Certificate + sshKey crypto.Signer + sshKnownHosts []ssh.PublicKey path string clusterName string @@ -83,9 +86,7 @@ func (d *WatchedIdentityFileCreds) Reload() error { if err != nil { return trace.Wrap(err) } - cc, err := sshutils.ProxyClientSSHConfig( - sshCert, sshPrivateKey, id.CACerts.SSH..., - ) + knownHosts, err := sshutils.ParseKnownHosts(id.CACerts.SSH) if err != nil { return trace.Wrap(err) } @@ -95,6 +96,7 @@ func (d *WatchedIdentityFileCreds) Reload() error { d.tlsCert = &cert d.sshCert = sshCert d.sshKey = sshPrivateKey + d.sshKnownHosts = knownHosts d.mu.Unlock() return nil } @@ -109,6 +111,9 @@ func (d *WatchedIdentityFileCreds) Dialer( // TLSConfig returns TLS configuration. func (d *WatchedIdentityFileCreds) TLSConfig() (*tls.Config, error) { + d.mu.RLock() + defer d.mu.RUnlock() + cfg := &tls.Config{ // GetClientCertificate is used instead of the static Certificates // field. @@ -129,7 +134,7 @@ func (d *WatchedIdentityFileCreds) TLSConfig() (*tls.Config, error) { InsecureSkipVerify: true, VerifyConnection: func(state tls.ConnectionState) error { // This VerifyConnection callback is based on the standard library - // implementation of verifyServerCertificate in the tls package. + // implementation of verifyServerCertificate in the `tls` package. // We provide our own implementation so we can dynamically handle // a changing CA Roots pool. d.mu.RLock() @@ -170,6 +175,8 @@ func (d *WatchedIdentityFileCreds) TLSConfig() (*tls.Config, error) { // SSHClientConfig returns SSH configuration. func (d *WatchedIdentityFileCreds) SSHClientConfig() (*ssh.ClientConfig, error) { + d.mu.RLock() + defer d.mu.RUnlock() // Build a "dynamic" ssh config. Based roughly on // `sshutils.ProxyClientSSHConfig` with modifications to make it work with // dynamically changing credentials. @@ -178,7 +185,7 @@ func (d *WatchedIdentityFileCreds) SSHClientConfig() (*ssh.ClientConfig, error) Auth: []ssh.AuthMethod{ ssh.PublicKeysCallback(func() (signers []ssh.Signer, err error) { d.mu.RLock() - defer d.mu.Unlock() + defer d.mu.RUnlock() sshSigner, err := sshutils.SSHSigner(d.sshCert, d.sshKey) if err != nil { @@ -187,6 +194,25 @@ func (d *WatchedIdentityFileCreds) SSHClientConfig() (*ssh.ClientConfig, error) return []ssh.Signer{sshSigner}, nil }), }, + HostKeyCallback: func(hostname string, remote net.Addr, key ssh.PublicKey) error { + d.mu.RLock() + defer d.mu.RUnlock() + hostKeyCallback, err := sshutils.HostKeyCallback( + d.sshKnownHosts, + false, + ) + if err != nil { + return trace.Wrap(err) + } + return hostKeyCallback(hostname, remote, key) + }, + Timeout: defaults.DefaultIOTimeout, + } + + // We can only really set the user once + cfg.User = d.sshCert.KeyId + if len(d.sshCert.ValidPrincipals) > 0 { + cfg.User = d.sshCert.ValidPrincipals[0] } return cfg, nil From a412d9df63163c35b15e7638ba2f73086e520057 Mon Sep 17 00:00:00 2001 From: Noah Date: Wed, 16 Aug 2023 15:20:04 +0100 Subject: [PATCH 05/12] Tidy u implementation --- api/client/dynamic_credentials.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/api/client/dynamic_credentials.go b/api/client/dynamic_credentials.go index d485dc7d4433a..6d0fd1c5bbfa3 100644 --- a/api/client/dynamic_credentials.go +++ b/api/client/dynamic_credentials.go @@ -34,6 +34,8 @@ import ( ) type WatchedIdentityFileCreds struct { + // mu protects the fields that may change if the underlying identity file + // is reloaded. mu sync.RWMutex tlsCert *tls.Certificate pool *x509.CertPool @@ -139,7 +141,6 @@ func (d *WatchedIdentityFileCreds) TLSConfig() (*tls.Config, error) { // a changing CA Roots pool. d.mu.RLock() defer d.mu.RUnlock() - opts := x509.VerifyOptions{ DNSName: state.ServerName, Intermediates: x509.NewCertPool(), @@ -180,13 +181,11 @@ func (d *WatchedIdentityFileCreds) SSHClientConfig() (*ssh.ClientConfig, error) // Build a "dynamic" ssh config. Based roughly on // `sshutils.ProxyClientSSHConfig` with modifications to make it work with // dynamically changing credentials. - // TODO: Handle changing host certs cfg := &ssh.ClientConfig{ Auth: []ssh.AuthMethod{ ssh.PublicKeysCallback(func() (signers []ssh.Signer, err error) { d.mu.RLock() defer d.mu.RUnlock() - sshSigner, err := sshutils.SSHSigner(d.sshCert, d.sshKey) if err != nil { return nil, trace.Wrap(err) @@ -209,7 +208,7 @@ func (d *WatchedIdentityFileCreds) SSHClientConfig() (*ssh.ClientConfig, error) Timeout: defaults.DefaultIOTimeout, } - // We can only really set the user once + // TODO: Evaluate if this needs to be made "dynamic" cfg.User = d.sshCert.KeyId if len(d.sshCert.ValidPrincipals) > 0 { cfg.User = d.sshCert.ValidPrincipals[0] From 772250f1b161746a5b30915939abc743dc9f405d Mon Sep 17 00:00:00 2001 From: Noah Date: Tue, 22 Aug 2023 13:52:43 +0100 Subject: [PATCH 06/12] Further tidy dynamic credential implementation --- api/client/dynamic_credentials.go | 96 +++++++++++++++---------------- 1 file changed, 48 insertions(+), 48 deletions(-) diff --git a/api/client/dynamic_credentials.go b/api/client/dynamic_credentials.go index 6d0fd1c5bbfa3..f0464756e28cb 100644 --- a/api/client/dynamic_credentials.go +++ b/api/client/dynamic_credentials.go @@ -23,7 +23,6 @@ import ( "github.com/gravitational/teleport/api/constants" "github.com/gravitational/teleport/api/defaults" "github.com/gravitational/teleport/api/identityfile" - "github.com/gravitational/teleport/api/utils" "github.com/gravitational/teleport/api/utils/keys" "github.com/gravitational/teleport/api/utils/sshutils" "github.com/gravitational/trace" @@ -33,36 +32,39 @@ import ( "sync" ) -type WatchedIdentityFileCreds struct { +// DynamicIdentityFileCreds allows a changing identity file to be used as the +// source of authentication for Client. It does not automatically watch the +// identity file or reload on an interval, this is left as an exercise for the +// consumer. +type DynamicIdentityFileCreds struct { // mu protects the fields that may change if the underlying identity file // is reloaded. mu sync.RWMutex tlsCert *tls.Certificate - pool *x509.CertPool + tlsRootCAs *x509.CertPool sshCert *ssh.Certificate sshKey crypto.Signer sshKnownHosts []ssh.PublicKey + sshUser string - path string - clusterName string + // Path is the path to the identity file to load and reload. + Path string } -func LoadAndWatchIdentityFile(path string, clusterName string) (*WatchedIdentityFileCreds, error) { - d := &WatchedIdentityFileCreds{ - path: path, - clusterName: clusterName, +func NewDynamicIdentityFileCreds(path string) (*DynamicIdentityFileCreds, error) { + d := &DynamicIdentityFileCreds{ + Path: path, } - - err := d.Reload() - if err != nil { + if err := d.Reload(); err != nil { return nil, trace.Wrap(err) } - return d, nil } -func (d *WatchedIdentityFileCreds) Reload() error { - id, err := identityfile.ReadFile(d.path) +// Reload causes the identity file to be re-read from the disk. It will return +// an error if loading the credentials fails. +func (d *DynamicIdentityFileCreds) Reload() error { + id, err := identityfile.ReadFile(d.Path) if err != nil { return trace.Wrap(err) } @@ -92,37 +94,48 @@ func (d *WatchedIdentityFileCreds) Reload() error { if err != nil { return trace.Wrap(err) } + // The KeyId is not always a valid principal, so we use the first valid + // principal instead. + sshUser := d.sshCert.KeyId + if len(d.sshCert.ValidPrincipals) > 0 { + sshUser = d.sshCert.ValidPrincipals[0] + } d.mu.Lock() - d.pool = pool + defer d.mu.Unlock() + d.tlsRootCAs = pool d.tlsCert = &cert d.sshCert = sshCert d.sshKey = sshPrivateKey d.sshKnownHosts = knownHosts - d.mu.Unlock() + d.sshUser = sshUser return nil } -// Dialer is used to dial a connection to an Auth server. -func (d *WatchedIdentityFileCreds) Dialer( +// Dialer returns a dialer for the client to use. This is not used, but is +// needed to implement the Credentials interface. +func (d *DynamicIdentityFileCreds) Dialer( _ Config, ) (ContextDialer, error) { // Returning a dialer isn't necessary for this credential. return nil, trace.NotImplemented("no dialer") } -// TLSConfig returns TLS configuration. -func (d *WatchedIdentityFileCreds) TLSConfig() (*tls.Config, 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{ + NextProtos: []string{http2.NextProtoTLS}, + // GetClientCertificate is used instead of the static Certificates // field. Certificates: nil, - GetClientCertificate: func( - info *tls.CertificateRequestInfo, + _ *tls.CertificateRequestInfo, ) (*tls.Certificate, error) { // GetClientCertificate callback is used to allow us to dynamically // change the certificate when reloaded. @@ -130,6 +143,9 @@ func (d *WatchedIdentityFileCreds) TLSConfig() (*tls.Config, error) { defer d.mu.RUnlock() return d.tlsCert, nil }, + + // VerifyConnection is used instead of the static RootCAs field. + RootCAs: nil, // InsecureSkipVerify is forced true to ensure that only our // VerifyConnection callback is used to verify the server's presented // certificate. @@ -144,7 +160,7 @@ func (d *WatchedIdentityFileCreds) TLSConfig() (*tls.Config, error) { opts := x509.VerifyOptions{ DNSName: state.ServerName, Intermediates: x509.NewCertPool(), - Roots: d.pool, + Roots: d.tlsRootCAs, } for _, cert := range state.PeerCertificates[1:] { // Whilst we don't currently use intermediate certs at @@ -155,32 +171,22 @@ func (d *WatchedIdentityFileCreds) TLSConfig() (*tls.Config, error) { _, err := state.PeerCertificates[0].Verify(opts) return err }, - NextProtos: []string{http2.NextProtoTLS}, - } - - // If the cluster name has been provided, we should use that for SNI. - // This enables alpn routed connections on Teleport Cloud which relies on - // a distinct SNI to route traffic to the correct Teleport Tenant. - // TODO: Could we determine this from the cert 🤔 and not need to ask - // the user explicitly for this. - if d.clusterName != "" { - cfg.ServerName = utils.EncodeClusterName(d.clusterName) - } else { - // Otherwise fall back to `teleport.cluster.local` which should work - // in most general teleport installs. - cfg.ServerName = constants.APIDomain + // Set the default name that's included in all Teleport issued + // certificates. + ServerName: constants.APIDomain, } return cfg, nil } -// SSHClientConfig returns SSH configuration. -func (d *WatchedIdentityFileCreds) SSHClientConfig() (*ssh.ClientConfig, error) { +// SSHClientConfig returns SSH configuration, implementing the Credentials +// interface. +func (d *DynamicIdentityFileCreds) SSHClientConfig() (*ssh.ClientConfig, error) { d.mu.RLock() defer d.mu.RUnlock() // Build a "dynamic" ssh config. Based roughly on // `sshutils.ProxyClientSSHConfig` with modifications to make it work with - // dynamically changing credentials. + // dynamically changing credentials and CAs. cfg := &ssh.ClientConfig{ Auth: []ssh.AuthMethod{ ssh.PublicKeysCallback(func() (signers []ssh.Signer, err error) { @@ -206,13 +212,7 @@ func (d *WatchedIdentityFileCreds) SSHClientConfig() (*ssh.ClientConfig, error) return hostKeyCallback(hostname, remote, key) }, Timeout: defaults.DefaultIOTimeout, + User: d.sshUser, } - - // TODO: Evaluate if this needs to be made "dynamic" - cfg.User = d.sshCert.KeyId - if len(d.sshCert.ValidPrincipals) > 0 { - cfg.User = d.sshCert.ValidPrincipals[0] - } - return cfg, nil } From 8a8779660e636ff8b1cbb315606bcb1121e7df94 Mon Sep 17 00:00:00 2001 From: Noah Date: Tue, 22 Aug 2023 14:17:55 +0100 Subject: [PATCH 07/12] Add godoc to new function --- api/client/dynamic_credentials.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/api/client/dynamic_credentials.go b/api/client/dynamic_credentials.go index f0464756e28cb..1ff98b1dafea9 100644 --- a/api/client/dynamic_credentials.go +++ b/api/client/dynamic_credentials.go @@ -51,6 +51,8 @@ type DynamicIdentityFileCreds struct { Path string } +// NewDynamicIdentityFileCreds returns a DynamicIdentityFileCreds which has +// been initially loaded and is ready for use. func NewDynamicIdentityFileCreds(path string) (*DynamicIdentityFileCreds, error) { d := &DynamicIdentityFileCreds{ Path: path, From 5d077e9ee5b1704aae6955af4421fcdb4ef5fa6e Mon Sep 17 00:00:00 2001 From: Noah Date: Tue, 22 Aug 2023 19:33:46 +0100 Subject: [PATCH 08/12] Move code into credentials.go --- api/client/credentials.go | 193 ++++++++++++++++++++++++++ api/client/dynamic_credentials.go | 220 ------------------------------ 2 files changed, 193 insertions(+), 220 deletions(-) delete mode 100644 api/client/dynamic_credentials.go diff --git a/api/client/credentials.go b/api/client/credentials.go index e5ed9545e1c48..3ba28aaaa3ecc 100644 --- a/api/client/credentials.go +++ b/api/client/credentials.go @@ -17,18 +17,24 @@ limitations under the License. package client import ( + "crypto" "crypto/tls" "crypto/x509" + "net" "os" + "sync" "github.com/gravitational/trace" "golang.org/x/crypto/ssh" "golang.org/x/net/http2" "github.com/gravitational/teleport/api/constants" + "github.com/gravitational/teleport/api/defaults" "github.com/gravitational/teleport/api/identityfile" "github.com/gravitational/teleport/api/profile" "github.com/gravitational/teleport/api/utils" + "github.com/gravitational/teleport/api/utils/keys" + "github.com/gravitational/teleport/api/utils/sshutils" ) // Credentials are used to authenticate the API auth client. Some Credentials @@ -394,3 +400,190 @@ func configureTLS(c *tls.Config) *tls.Config { return tlsConfig } + +// DynamicIdentityFileCreds allows a changing identity file to be used as the +// source of authentication for Client. It does not automatically watch the +// identity file or reload on an interval, this is left as an exercise for the +// consumer. +type DynamicIdentityFileCreds struct { + // mu protects the fields that may change if the underlying identity file + // is reloaded. + mu sync.RWMutex + tlsCert *tls.Certificate + tlsRootCAs *x509.CertPool + sshCert *ssh.Certificate + sshKey crypto.Signer + sshKnownHosts []ssh.PublicKey + sshUser string + + // Path is the path to the identity file to load and reload. + Path string +} + +// NewDynamicIdentityFileCreds returns a DynamicIdentityFileCreds which has +// been initially loaded and is ready for use. +func NewDynamicIdentityFileCreds(path string) (*DynamicIdentityFileCreds, error) { + d := &DynamicIdentityFileCreds{ + Path: path, + } + if err := d.Reload(); err != nil { + return nil, trace.Wrap(err) + } + return d, nil +} + +// Reload causes the identity file to be re-read from the disk. It will return +// an error if loading the credentials fails. +func (d *DynamicIdentityFileCreds) Reload() error { + id, err := identityfile.ReadFile(d.Path) + if err != nil { + return trace.Wrap(err) + } + + // This section is essentially id.TLSConfig() + cert, err := keys.X509KeyPair(id.Certs.TLS, id.PrivateKey) + if err != nil { + return trace.Wrap(err) + } + pool := x509.NewCertPool() + for _, caCerts := range id.CACerts.TLS { + if !pool.AppendCertsFromPEM(caCerts) { + return trace.BadParameter("invalid CA cert PEM") + } + } + + // This sections is essentially id.SSHClientConfig() + sshCert, err := sshutils.ParseCertificate(id.Certs.SSH) + if err != nil { + return trace.Wrap(err) + } + sshPrivateKey, err := keys.ParsePrivateKey(id.PrivateKey) + if err != nil { + return trace.Wrap(err) + } + knownHosts, err := sshutils.ParseKnownHosts(id.CACerts.SSH) + if err != nil { + return trace.Wrap(err) + } + // The KeyId is not always a valid principal, so we use the first valid + // principal instead. + sshUser := d.sshCert.KeyId + if len(d.sshCert.ValidPrincipals) > 0 { + sshUser = d.sshCert.ValidPrincipals[0] + } + + d.mu.Lock() + defer d.mu.Unlock() + d.tlsRootCAs = pool + d.tlsCert = &cert + d.sshCert = sshCert + d.sshKey = sshPrivateKey + d.sshKnownHosts = knownHosts + d.sshUser = sshUser + return nil +} + +// Dialer returns a dialer for the client to use. This is not used, but is +// needed to implement the Credentials interface. +func (d *DynamicIdentityFileCreds) Dialer( + _ Config, +) (ContextDialer, error) { + // Returning a dialer isn't necessary for this credential. + return nil, trace.NotImplemented("no dialer") +} + +// 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{ + NextProtos: []string{http2.NextProtoTLS}, + + // GetClientCertificate is used instead of the static Certificates + // field. + Certificates: nil, + GetClientCertificate: func( + _ *tls.CertificateRequestInfo, + ) (*tls.Certificate, error) { + // GetClientCertificate callback is used to allow us to dynamically + // change the certificate when reloaded. + d.mu.RLock() + defer d.mu.RUnlock() + return d.tlsCert, nil + }, + + // VerifyConnection is used instead of the static RootCAs field. + RootCAs: nil, + // InsecureSkipVerify is forced true to ensure that only our + // VerifyConnection callback is used to verify the server's presented + // certificate. + InsecureSkipVerify: true, + VerifyConnection: func(state tls.ConnectionState) error { + // This VerifyConnection callback is based on the standard library + // implementation of verifyServerCertificate in the `tls` package. + // We provide our own implementation so we can dynamically handle + // a changing CA Roots pool. + d.mu.RLock() + defer d.mu.RUnlock() + opts := x509.VerifyOptions{ + DNSName: state.ServerName, + Intermediates: x509.NewCertPool(), + Roots: d.tlsRootCAs, + } + for _, cert := range state.PeerCertificates[1:] { + // Whilst we don't currently use intermediate certs at + // Teleport, including this here means that we are + // future-proofed in case we do. + opts.Intermediates.AddCert(cert) + } + _, err := state.PeerCertificates[0].Verify(opts) + return err + }, + // Set the default name that's included in all Teleport issued + // certificates. + ServerName: constants.APIDomain, + } + + return cfg, nil +} + +// SSHClientConfig returns SSH configuration, implementing the Credentials +// interface. +func (d *DynamicIdentityFileCreds) SSHClientConfig() (*ssh.ClientConfig, error) { + d.mu.RLock() + defer d.mu.RUnlock() + // Build a "dynamic" ssh config. Based roughly on + // `sshutils.ProxyClientSSHConfig` with modifications to make it work with + // dynamically changing credentials and CAs. + cfg := &ssh.ClientConfig{ + Auth: []ssh.AuthMethod{ + ssh.PublicKeysCallback(func() (signers []ssh.Signer, err error) { + d.mu.RLock() + defer d.mu.RUnlock() + sshSigner, err := sshutils.SSHSigner(d.sshCert, d.sshKey) + if err != nil { + return nil, trace.Wrap(err) + } + return []ssh.Signer{sshSigner}, nil + }), + }, + HostKeyCallback: func(hostname string, remote net.Addr, key ssh.PublicKey) error { + d.mu.RLock() + defer d.mu.RUnlock() + hostKeyCallback, err := sshutils.HostKeyCallback( + d.sshKnownHosts, + false, + ) + if err != nil { + return trace.Wrap(err) + } + return hostKeyCallback(hostname, remote, key) + }, + Timeout: defaults.DefaultIOTimeout, + User: d.sshUser, + } + return cfg, nil +} diff --git a/api/client/dynamic_credentials.go b/api/client/dynamic_credentials.go deleted file mode 100644 index 1ff98b1dafea9..0000000000000 --- a/api/client/dynamic_credentials.go +++ /dev/null @@ -1,220 +0,0 @@ -/* -Copyright 2023 Gravitational, Inc. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package client - -import ( - "crypto" - "crypto/tls" - "crypto/x509" - "github.com/gravitational/teleport/api/constants" - "github.com/gravitational/teleport/api/defaults" - "github.com/gravitational/teleport/api/identityfile" - "github.com/gravitational/teleport/api/utils/keys" - "github.com/gravitational/teleport/api/utils/sshutils" - "github.com/gravitational/trace" - "golang.org/x/crypto/ssh" - "golang.org/x/net/http2" - "net" - "sync" -) - -// DynamicIdentityFileCreds allows a changing identity file to be used as the -// source of authentication for Client. It does not automatically watch the -// identity file or reload on an interval, this is left as an exercise for the -// consumer. -type DynamicIdentityFileCreds struct { - // mu protects the fields that may change if the underlying identity file - // is reloaded. - mu sync.RWMutex - tlsCert *tls.Certificate - tlsRootCAs *x509.CertPool - sshCert *ssh.Certificate - sshKey crypto.Signer - sshKnownHosts []ssh.PublicKey - sshUser string - - // Path is the path to the identity file to load and reload. - Path string -} - -// NewDynamicIdentityFileCreds returns a DynamicIdentityFileCreds which has -// been initially loaded and is ready for use. -func NewDynamicIdentityFileCreds(path string) (*DynamicIdentityFileCreds, error) { - d := &DynamicIdentityFileCreds{ - Path: path, - } - if err := d.Reload(); err != nil { - return nil, trace.Wrap(err) - } - return d, nil -} - -// Reload causes the identity file to be re-read from the disk. It will return -// an error if loading the credentials fails. -func (d *DynamicIdentityFileCreds) Reload() error { - id, err := identityfile.ReadFile(d.Path) - if err != nil { - return trace.Wrap(err) - } - - // This section is essentially id.TLSConfig() - cert, err := keys.X509KeyPair(id.Certs.TLS, id.PrivateKey) - if err != nil { - return trace.Wrap(err) - } - pool := x509.NewCertPool() - for _, caCerts := range id.CACerts.TLS { - if !pool.AppendCertsFromPEM(caCerts) { - return trace.BadParameter("invalid CA cert PEM") - } - } - - // This sections is essentially id.SSHClientConfig() - sshCert, err := sshutils.ParseCertificate(id.Certs.SSH) - if err != nil { - return trace.Wrap(err) - } - sshPrivateKey, err := keys.ParsePrivateKey(id.PrivateKey) - if err != nil { - return trace.Wrap(err) - } - knownHosts, err := sshutils.ParseKnownHosts(id.CACerts.SSH) - if err != nil { - return trace.Wrap(err) - } - // The KeyId is not always a valid principal, so we use the first valid - // principal instead. - sshUser := d.sshCert.KeyId - if len(d.sshCert.ValidPrincipals) > 0 { - sshUser = d.sshCert.ValidPrincipals[0] - } - - d.mu.Lock() - defer d.mu.Unlock() - d.tlsRootCAs = pool - d.tlsCert = &cert - d.sshCert = sshCert - d.sshKey = sshPrivateKey - d.sshKnownHosts = knownHosts - d.sshUser = sshUser - return nil -} - -// Dialer returns a dialer for the client to use. This is not used, but is -// needed to implement the Credentials interface. -func (d *DynamicIdentityFileCreds) Dialer( - _ Config, -) (ContextDialer, error) { - // Returning a dialer isn't necessary for this credential. - return nil, trace.NotImplemented("no dialer") -} - -// 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{ - NextProtos: []string{http2.NextProtoTLS}, - - // GetClientCertificate is used instead of the static Certificates - // field. - Certificates: nil, - GetClientCertificate: func( - _ *tls.CertificateRequestInfo, - ) (*tls.Certificate, error) { - // GetClientCertificate callback is used to allow us to dynamically - // change the certificate when reloaded. - d.mu.RLock() - defer d.mu.RUnlock() - return d.tlsCert, nil - }, - - // VerifyConnection is used instead of the static RootCAs field. - RootCAs: nil, - // InsecureSkipVerify is forced true to ensure that only our - // VerifyConnection callback is used to verify the server's presented - // certificate. - InsecureSkipVerify: true, - VerifyConnection: func(state tls.ConnectionState) error { - // This VerifyConnection callback is based on the standard library - // implementation of verifyServerCertificate in the `tls` package. - // We provide our own implementation so we can dynamically handle - // a changing CA Roots pool. - d.mu.RLock() - defer d.mu.RUnlock() - opts := x509.VerifyOptions{ - DNSName: state.ServerName, - Intermediates: x509.NewCertPool(), - Roots: d.tlsRootCAs, - } - for _, cert := range state.PeerCertificates[1:] { - // Whilst we don't currently use intermediate certs at - // Teleport, including this here means that we are - // future-proofed in case we do. - opts.Intermediates.AddCert(cert) - } - _, err := state.PeerCertificates[0].Verify(opts) - return err - }, - // Set the default name that's included in all Teleport issued - // certificates. - ServerName: constants.APIDomain, - } - - return cfg, nil -} - -// SSHClientConfig returns SSH configuration, implementing the Credentials -// interface. -func (d *DynamicIdentityFileCreds) SSHClientConfig() (*ssh.ClientConfig, error) { - d.mu.RLock() - defer d.mu.RUnlock() - // Build a "dynamic" ssh config. Based roughly on - // `sshutils.ProxyClientSSHConfig` with modifications to make it work with - // dynamically changing credentials and CAs. - cfg := &ssh.ClientConfig{ - Auth: []ssh.AuthMethod{ - ssh.PublicKeysCallback(func() (signers []ssh.Signer, err error) { - d.mu.RLock() - defer d.mu.RUnlock() - sshSigner, err := sshutils.SSHSigner(d.sshCert, d.sshKey) - if err != nil { - return nil, trace.Wrap(err) - } - return []ssh.Signer{sshSigner}, nil - }), - }, - HostKeyCallback: func(hostname string, remote net.Addr, key ssh.PublicKey) error { - d.mu.RLock() - defer d.mu.RUnlock() - hostKeyCallback, err := sshutils.HostKeyCallback( - d.sshKnownHosts, - false, - ) - if err != nil { - return trace.Wrap(err) - } - return hostKeyCallback(hostname, remote, key) - }, - Timeout: defaults.DefaultIOTimeout, - User: d.sshUser, - } - return cfg, nil -} From e47a1acc4c9df116a02af302e42ed161c5bb1444 Mon Sep 17 00:00:00 2001 From: Noah Date: Wed, 23 Aug 2023 11:50:07 +0100 Subject: [PATCH 09/12] Add some more explanatory docs --- api/client/credentials.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/api/client/credentials.go b/api/client/credentials.go index 3ba28aaaa3ecc..21e2ec01e5a80 100644 --- a/api/client/credentials.go +++ b/api/client/credentials.go @@ -500,6 +500,8 @@ func (d *DynamicIdentityFileCreds) TLSConfig() (*tls.Config, error) { // Build a "dynamic" tls.Config which can support a changing cert and root // CA pool. cfg := &tls.Config{ + // Set the default NextProto of "h2". Based on the value in + // configureTLS() NextProtos: []string{http2.NextProtoTLS}, // GetClientCertificate is used instead of the static Certificates @@ -542,8 +544,9 @@ func (d *DynamicIdentityFileCreds) TLSConfig() (*tls.Config, error) { _, err := state.PeerCertificates[0].Verify(opts) return err }, - // Set the default name that's included in all Teleport issued - // certificates. + // Set ServerName for SNI & Certificate Validation to the sentinel + // teleport.cluster.local which is included on all Teleport Auth Server + // certificates. Based on the value in configureTLS() ServerName: constants.APIDomain, } From 075d342a02cce8112912bbe7f0e1931beb050f5a Mon Sep 17 00:00:00 2001 From: Noah Date: Wed, 23 Aug 2023 13:47:14 +0100 Subject: [PATCH 10/12] Fix panics intrdouced by refactor --- api/client/credentials.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/api/client/credentials.go b/api/client/credentials.go index 21e2ec01e5a80..f96cbafdf598c 100644 --- a/api/client/credentials.go +++ b/api/client/credentials.go @@ -467,9 +467,9 @@ func (d *DynamicIdentityFileCreds) Reload() error { } // The KeyId is not always a valid principal, so we use the first valid // principal instead. - sshUser := d.sshCert.KeyId - if len(d.sshCert.ValidPrincipals) > 0 { - sshUser = d.sshCert.ValidPrincipals[0] + sshUser := sshCert.KeyId + if len(sshCert.ValidPrincipals) > 0 { + sshUser = sshCert.ValidPrincipals[0] } d.mu.Lock() From 6b6594504522f1f54894fe60de4a7651ed71b227 Mon Sep 17 00:00:00 2001 From: Noah Date: Wed, 23 Aug 2023 16:38:03 +0100 Subject: [PATCH 11/12] Add tests & make fix-imports --- api/client/credentials_test.go | 85 ++++++++++++++++++++++++++++++++++ 1 file changed, 85 insertions(+) diff --git a/api/client/credentials_test.go b/api/client/credentials_test.go index 5cf6d63ae1b82..051a9d96b73e4 100644 --- a/api/client/credentials_test.go +++ b/api/client/credentials_test.go @@ -17,9 +17,15 @@ limitations under the License. package client import ( + "crypto/rand" + "crypto/rsa" "crypto/tls" "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" + "math/big" "os" + "path" "path/filepath" "testing" @@ -28,6 +34,7 @@ import ( "github.com/stretchr/testify/require" "golang.org/x/crypto/ssh" + "github.com/gravitational/teleport/api/constants" "github.com/gravitational/teleport/api/identityfile" "github.com/gravitational/teleport/api/profile" "github.com/gravitational/teleport/api/utils/keys" @@ -382,3 +389,81 @@ AIBV1ZA8WqvC+xZrPwmtmN87BHwGjqpE52kbUfcD94k8IqqhPR9oN9uOlcoBzZiS k53lH1qmEOm9+vrhNwNzpHk4AqDkP+0YDG++B4n0BtJJpw== Private-MAC: 8951bbe929e0714a61df01bc8fbc5223e3688f174aee29339931984fb9224c7d`) ) + +func TestDynamicIdentityFileCreds(t *testing.T) { + dir := t.TempDir() + identityPath := path.Join(dir, "identity") + + idFile := &identityfile.IdentityFile{ + PrivateKey: keyPEM, + Certs: identityfile.Certs{ + TLS: tlsCert, + SSH: sshCert, + }, + CACerts: identityfile.CACerts{ + TLS: [][]byte{tlsCACert}, + SSH: [][]byte{sshCACert}, + }, + } + require.NoError(t, identityfile.Write(idFile, identityPath)) + + cred, err := NewDynamicIdentityFileCreds(identityPath) + require.NoError(t, err) + + // Check the initial TLS certificate/key has been loaded. + tlsConfig, err := cred.TLSConfig() + require.NoError(t, err) + gotTLSCert, err := tlsConfig.GetClientCertificate(&tls.CertificateRequestInfo{ + // We always return the same cert so this can be empty + }) + require.NoError(t, err) + wantTLSCert, err := tls.X509KeyPair(tlsCert, keyPEM) + require.NoError(t, err) + require.Equal(t, wantTLSCert, *gotTLSCert) + + // Generate a new TLS certificate that contains the same private key as + // the original. + template := &x509.Certificate{ + SerialNumber: big.NewInt(0), + Subject: pkix.Name{ + CommonName: "example", + }, + KeyUsage: x509.KeyUsageDigitalSignature, + BasicConstraintsValid: true, + DNSNames: []string{constants.APIDomain}, + } + secondTLSCert, err := x509.CreateCertificate( + rand.Reader, template, template, &wantTLSCert.PrivateKey.(*rsa.PrivateKey).PublicKey, wantTLSCert.PrivateKey, + ) + require.NoError(t, err) + secondTLSCertPem := pem.EncodeToMemory(&pem.Block{ + Type: "CERTIFICATE", + Bytes: secondTLSCert, + }) + + // Write the new TLS certificate as part of the identity file and reload. + secondIDFile := &identityfile.IdentityFile{ + PrivateKey: keyPEM, + Certs: identityfile.Certs{ + TLS: secondTLSCertPem, + SSH: sshCert, + }, + CACerts: identityfile.CACerts{ + TLS: [][]byte{tlsCACert}, + SSH: [][]byte{sshCACert}, + }, + } + require.NoError(t, identityfile.Write(secondIDFile, identityPath)) + require.NoError(t, cred.Reload()) + + // Test that calling GetClientCertificate on the original tls.Config now + // returns the new certificate we wrote and reloaded. + gotTLSCert, err = tlsConfig.GetClientCertificate(&tls.CertificateRequestInfo{ + // We always return the same cert so this can be empty + }) + require.NoError(t, err) + wantTLSCert, err = tls.X509KeyPair(secondTLSCertPem, keyPEM) + require.NoError(t, err) + require.Equal(t, wantTLSCert, *gotTLSCert) + +} From f70693e9ab0f0707936f1e1b73b6aa50cf088cd9 Mon Sep 17 00:00:00 2001 From: Noah Date: Thu, 24 Aug 2023 13:33:00 +0100 Subject: [PATCH 12/12] Add helper for extracting principal --- api/client/credentials.go | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/api/client/credentials.go b/api/client/credentials.go index f96cbafdf598c..03e9c5e6e8275 100644 --- a/api/client/credentials.go +++ b/api/client/credentials.go @@ -414,7 +414,6 @@ type DynamicIdentityFileCreds struct { sshCert *ssh.Certificate sshKey crypto.Signer sshKnownHosts []ssh.PublicKey - sshUser string // Path is the path to the identity file to load and reload. Path string @@ -465,12 +464,6 @@ func (d *DynamicIdentityFileCreds) Reload() error { if err != nil { return trace.Wrap(err) } - // The KeyId is not always a valid principal, so we use the first valid - // principal instead. - sshUser := sshCert.KeyId - if len(sshCert.ValidPrincipals) > 0 { - sshUser = sshCert.ValidPrincipals[0] - } d.mu.Lock() defer d.mu.Unlock() @@ -479,7 +472,6 @@ func (d *DynamicIdentityFileCreds) Reload() error { d.sshCert = sshCert d.sshKey = sshPrivateKey d.sshKnownHosts = knownHosts - d.sshUser = sshUser return nil } @@ -494,9 +486,6 @@ func (d *DynamicIdentityFileCreds) Dialer( // 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{ @@ -556,6 +545,7 @@ func (d *DynamicIdentityFileCreds) TLSConfig() (*tls.Config, error) { // SSHClientConfig returns SSH configuration, implementing the Credentials // interface. func (d *DynamicIdentityFileCreds) SSHClientConfig() (*ssh.ClientConfig, error) { + // This lock is necessary for `d.sshCert` used in `cfg.User`. d.mu.RLock() defer d.mu.RUnlock() // Build a "dynamic" ssh config. Based roughly on @@ -586,7 +576,16 @@ func (d *DynamicIdentityFileCreds) SSHClientConfig() (*ssh.ClientConfig, error) return hostKeyCallback(hostname, remote, key) }, Timeout: defaults.DefaultIOTimeout, - User: d.sshUser, + User: userFromSSHCert(d.sshCert), } return cfg, nil } + +func userFromSSHCert(c *ssh.Certificate) string { + // The KeyId is not always a valid principal, so we prefer the first valid + // principal. + if len(c.ValidPrincipals) > 0 { + return c.ValidPrincipals[0] + } + return c.KeyId +}