From f17212babe667dc0d60464a80d4cbe76f2a36290 Mon Sep 17 00:00:00 2001 From: Nic Klaassen Date: Wed, 20 Nov 2024 18:29:41 -0800 Subject: [PATCH] improve error message when v17 tctl reads v16 tsh profile --- api/utils/keypaths/keypaths.go | 20 ++++++++--------- lib/client/client_store.go | 38 +++++++++++++++++++++----------- lib/client/keystore.go | 40 ++++++++++++++++++++++++++++++++++ lib/client/profile.go | 3 +++ tool/tctl/common/tctl.go | 8 +++++++ 5 files changed, 86 insertions(+), 23 deletions(-) diff --git a/api/utils/keypaths/keypaths.go b/api/utils/keypaths/keypaths.go index 65d855ce2f3ec..5e2d62c4c4ebb 100644 --- a/api/utils/keypaths/keypaths.go +++ b/api/utils/keypaths/keypaths.go @@ -32,8 +32,8 @@ const ( sshDirSuffix = "-ssh" // fileNameKnownHosts is a file where known hosts are stored. fileNameKnownHosts = "known_hosts" - // fileExtTLSCertLegacy is the legacy suffix/extension of a file where a TLS cert is stored. - fileExtTLSCertLegacy = "-x509.pem" + // FileExtTLSCertLegacy is the legacy suffix/extension of a file where a TLS cert is stored. + FileExtTLSCertLegacy = "-x509.pem" // FileExtTLSCert is the suffix/extension of a file where a TLS cert is stored. FileExtTLSCert = ".crt" // FileExtKubeCred is the suffix/extension of a file where a kubernetes @@ -188,6 +188,14 @@ func TLSCertPath(baseDir, proxy, username string) string { return filepath.Join(ProxyKeyDir(baseDir, proxy), username+FileExtTLSCert) } +// TLSCertPathLegacy returns the legacy path used in Teleport 16.x and older to the +// users's TLS certificate for the given proxy. +// +// /keys//-x509.pem +func TLSCertPathLegacy(baseDir, proxy, username string) string { + return filepath.Join(ProxyKeyDir(baseDir, proxy), username+FileExtTLSCertLegacy) +} + // PublicKeyPath returns the path to the users's public key // for the given proxy. // @@ -378,14 +386,6 @@ func IdentitySSHCertPath(path string) string { return path + fileExtSSHCert } -// TrimCertPathSuffix returns the given path with any cert suffix/extension trimmed off. -func TrimCertPathSuffix(path string) string { - trimmedPath := strings.TrimSuffix(path, fileExtTLSCertLegacy) - trimmedPath = strings.TrimSuffix(trimmedPath, FileExtTLSCert) - trimmedPath = strings.TrimSuffix(trimmedPath, fileExtSSHCert) - return trimmedPath -} - // TrimKeyPathSuffix returns the given path with any key suffix/extension trimmed off. func TrimKeyPathSuffix(path string) string { return strings.TrimSuffix(path, fileExtTLSKey) diff --git a/lib/client/client_store.go b/lib/client/client_store.go index df4fcb05de968..f00992b513cef 100644 --- a/lib/client/client_store.go +++ b/lib/client/client_store.go @@ -20,6 +20,7 @@ package client import ( "errors" + "fmt" "net/url" "time" @@ -87,21 +88,31 @@ func (s *Store) SetCustomHardwareKeyPrompt(prompt keys.HardwareKeyPrompt) { s.KeyStore.SetCustomHardwareKeyPrompt(prompt) } -var ( - // ErrNoCredentials is returned by the client store when a specific key is not found. - // This error can be used to determine whether a client should retrieve new credentials, - // like how it is used with lib/client.RetryWithRelogin. - ErrNoCredentials = &trace.NotFoundError{Message: "no credentials"} +// ErrNoProfile is returned by the client store when a specific profile is not found. +var ErrNoProfile = &trace.NotFoundError{Message: "no profile"} - // ErrNoProfile is returned by the client store when a specific profile is not found. - // This error can be used to determine whether a client should retrieve new credentials, - // like how it is used with lib/client.RetryWithRelogin. - ErrNoProfile = &trace.NotFoundError{Message: "no profile"} -) +// noCredentialsError is returned by the client store when a specific key is not found. +// It unwraps to the original error to allow checks for underlying error types. +// Use [IsNoCredentialsError] instead of checking for this type directly. +type noCredentialsError struct { + wrappedError error +} + +func newNoCredentialsError(wrappedError error) *noCredentialsError { + return &noCredentialsError{wrappedError} +} + +func (e *noCredentialsError) Error() string { + return fmt.Sprintf("no credentials: %v", e.wrappedError) +} + +func (e *noCredentialsError) Unwrap() error { + return e.wrappedError +} // IsNoCredentialsError returns whether the given error implies that the user should retrieve new credentials. func IsNoCredentialsError(err error) bool { - return errors.Is(err, ErrNoCredentials) || errors.Is(err, ErrNoProfile) + return errors.As(err, new(*noCredentialsError)) || errors.Is(err, ErrNoProfile) } // GetKeyRing gets the requested key ring with trusted the requested @@ -111,7 +122,7 @@ func IsNoCredentialsError(err error) bool { func (s *Store) GetKeyRing(idx KeyRingIndex, opts ...CertOption) (*KeyRing, error) { keyRing, err := s.KeyStore.GetKeyRing(idx, opts...) if trace.IsNotFound(err) { - return nil, trace.Wrap(ErrNoCredentials, err.Error()) + return nil, newNoCredentialsError(err) } else if err != nil { return nil, trace.Wrap(err) } @@ -199,8 +210,9 @@ func (s *Store) ReadProfileStatus(profileName string) (*ProfileStatus, error) { Username: profile.Username, Cluster: profile.SiteName, KubeEnabled: profile.KubeProxyAddr != "", - // Set ValidUntil to now to show that the keys are not available. + // Set ValidUntil to now and GetKeyRingError to show that the keys are not available. ValidUntil: time.Now(), + GetKeyRingError: err, SAMLSingleLogoutEnabled: profile.SAMLSingleLogoutEnabled, SSOHost: profile.SSOHost, }, nil diff --git a/lib/client/keystore.go b/lib/client/keystore.go index 250689ff97c28..2cba0d5810825 100644 --- a/lib/client/keystore.go +++ b/lib/client/keystore.go @@ -22,6 +22,7 @@ import ( "bytes" "context" "errors" + "fmt" iofs "io/fs" "os" "path/filepath" @@ -130,6 +131,12 @@ func (fs *FSKeyStore) tlsCertPath(idx KeyRingIndex) string { return keypaths.TLSCertPath(fs.KeyDir, idx.ProxyHost, idx.Username) } +// tlsCertPathLegacy returns the legacy TLS certificate path used in Teleport v16 and +// older given KeyRingIndex. +func (fs *FSKeyStore) tlsCertPathLegacy(idx KeyRingIndex) string { + return keypaths.TLSCertPathLegacy(fs.KeyDir, idx.ProxyHost, idx.Username) +} + // sshDir returns the SSH certificate path for the given KeyRingIndex. func (fs *FSKeyStore) sshDir(proxy, user string) string { return keypaths.SSHDir(fs.KeyDir, proxy, user) @@ -497,6 +504,33 @@ func (fs *FSKeyStore) DeleteKeys() error { return nil } +// LegacyCertPathError will be returned when [(*FSKeyStore).GetKeyRing] does not +// find a user TLS certificate at the expected path used in v17+ but does find +// one at the legacy path used in Teleport v16-. +type LegacyCertPathError struct { + wrappedError error + expectedPath, foundPath string +} + +func newLegacyCertPathError(wrappedError error, expectedPath, foundPath string) *LegacyCertPathError { + return &LegacyCertPathError{ + wrappedError: wrappedError, + expectedPath: expectedPath, + foundPath: foundPath, + } +} + +// Error implements the error interface. +func (e *LegacyCertPathError) Error() string { + return fmt.Sprintf( + "user TLS certificate was found at unsupported legacy path (expected path: %s, found path: %s)", + e.expectedPath, e.foundPath) +} + +func (e *LegacyCertPathError) Unwrap() error { + return e.wrappedError +} + // GetKeyRing returns the user's key including the specified certs. // If the key is not found, returns trace.NotFound error. func (fs *FSKeyStore) GetKeyRing(idx KeyRingIndex, opts ...CertOption) (*KeyRing, error) { @@ -512,6 +546,12 @@ func (fs *FSKeyStore) GetKeyRing(idx KeyRingIndex, opts ...CertOption) (*KeyRing tlsCred, err := readTLSCredential(fs.userTLSKeyPath(idx), fs.tlsCertPath(idx), fs.CustomHardwareKeyPrompt) if err != nil { + if trace.IsNotFound(err) { + if _, statErr := os.Stat(fs.tlsCertPathLegacy(idx)); statErr == nil { + return nil, newLegacyCertPathError(err, fs.tlsCertPath(idx), fs.tlsCertPathLegacy(idx)) + } + return nil, err + } return nil, trace.Wrap(err) } diff --git a/lib/client/profile.go b/lib/client/profile.go index f037d2e67fd23..5b06fe0e48dcc 100644 --- a/lib/client/profile.go +++ b/lib/client/profile.go @@ -203,6 +203,9 @@ type ProfileStatus struct { // ValidUntil is the time at which this SSH certificate will expire. ValidUntil time.Time + // GetKeyRingError is any error encountered while loading the KeyRing. + GetKeyRingError error + // Extensions is a list of enabled SSH features for the certificate. Extensions []string diff --git a/tool/tctl/common/tctl.go b/tool/tctl/common/tctl.go index 14f954ce91f4a..f9f7c189e4636 100644 --- a/tool/tctl/common/tctl.go +++ b/tool/tctl/common/tctl.go @@ -457,6 +457,14 @@ func LoadConfigFromProfile(ccf *GlobalCLIFlags, cfg *servicecfg.Config) (*authcl return nil, trace.Wrap(err) } if profile.IsExpired(time.Now()) { + if profile.GetKeyRingError != nil { + if errors.As(profile.GetKeyRingError, new(*client.LegacyCertPathError)) { + // Intentionally avoid wrapping the error because the caller + // ignores NotFound errors. + return nil, trace.Errorf("it appears tsh v16 or older was used to log in, make sure to use tsh and tctl on the same major version\n\t%v", profile.GetKeyRingError) + } + return nil, trace.Wrap(profile.GetKeyRingError) + } return nil, trace.BadParameter("your credentials have expired, please login using `tsh login`") }