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

ssh: fix relogin with jumphosts #6213

Merged
merged 4 commits into from
Mar 30, 2021
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 17 additions & 11 deletions lib/client/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -469,15 +469,6 @@ func RetryWithRelogin(ctx context.Context, tc *TeleportClient, fn func() error)
log.Warningf("Failed to save profile: %v", err)
return trace.Wrap(err)
}
// Override client's auth methods, current cluster and user name
authMethod, err := key.AsAuthMethod()
if err != nil {
return trace.Wrap(err)
}
// After successful login we have local agent updated with latest
// and greatest auth information, setup client to try only this new
// method fetched from key, to isolate the retry
tc.Config.AuthMethods = []ssh.AuthMethod{authMethod}
Comment on lines -472 to -480
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why did you remove this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This messes up the relogin flow.

tc.Config.AuthMethods is used as the first choice when connecting to a proxy over SSH.
If you were originally connecting to a leaf cluster (as in tsh ssh -J leaf.proxy.com somenode) and relogin kicked in, you're be logged in via the root cluster (which is correct).
But after you've re-logged through the root cluster tc.Config.AuthMethods would have an SSH cert for the root cluster, not for the leaf.
The cert caching and loading logic that @andrejtokarcik added correctly adds a leaf cert into the localAgent, but it gets overriden by tc.Config.AuthMethods.

return fn()
}

Expand Down Expand Up @@ -1066,7 +1057,11 @@ func (tc *TeleportClient) LoadKeyForClusterWithReissue(ctx context.Context, clus
return trace.Wrap(err)
}
// Reissuing also loads the new key.
return tc.ReissueUserCerts(ctx, ReissueParams{RouteToCluster: clusterName})
err = tc.ReissueUserCerts(ctx, ReissueParams{RouteToCluster: clusterName})
if err != nil {
return trace.Wrap(err)
}
return nil
}

// accessPoint returns access point based on the cache policy
Expand Down Expand Up @@ -2208,7 +2203,18 @@ func (tc *TeleportClient) ActivateKey(ctx context.Context, key *Key) error {
// Connect to the Auth Server of the root cluster and fetch the known hosts.
rootClusterName := key.TrustedCA[0].ClusterName
if err := tc.UpdateTrustedCA(ctx, rootClusterName); err != nil {
return trace.Wrap(err)
if len(tc.JumpHosts) == 0 {
return trace.Wrap(err)
}
errViaJumphost := err
// If JumpHosts was pointing at the leaf cluster (e.g. during 'tsh ssh
// -J leaf.example.com'), this could've caused the above error. Try to
// fetch CAs without JumpHosts to force it to use the root cluster.
if err := tc.WithoutJumpHosts(func(tc *TeleportClient) error {
return tc.UpdateTrustedCA(ctx, rootClusterName)
}); err != nil {
return trace.NewAggregate(errViaJumphost, err)
}
}

return nil
Expand Down
10 changes: 10 additions & 0 deletions lib/client/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,16 @@ func (k *Key) CheckCert() error {
return trace.Wrap(err)
}

// Check that the certificate was for the current public key. If not, the
// public/private key pair may have been rotated.
pub, _, _, _, err := ssh.ParseAuthorizedKey(k.Pub)
if err != nil {
return trace.Wrap(err)
}
if !sshutils.KeysEqual(cert.Key, pub) {
return trace.CompareFailed("public key in profile does not match the public key in SSH certificate")
}

// A valid principal is always passed in because the principals are not being
// checked here, but rather the validity period, signature, and algorithms.
certChecker := utils.CertChecker{
Expand Down