ssh: fix relogin with jumphosts #6213
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@andrejtokarcik after a bit of tweaking, this change makes the following work: I'm not confident that all of the changes are safe though, WDYT? |
awly
force-pushed
the
andrew/jumphost-relogin-fix
branch
from
8f45bc1 to
66f8061
Compare
added 2 commits
March 30, 2021 10:48
Several fixes to make `tsh ssh -J leaf.proxy.com` work if the root cert is missing/expired.
Correctly parse trusted CAs on GetKey. Move retry without jumphosts from relogin to UpdateClusterCAs.
awly
force-pushed
the
andrew/jumphost-relogin-fix
branch
from
66f8061 to
0d12ddd
Compare
andrejtokarcik
approved these changes
Mar 30, 2021
It doesn't seem to break anything.
awly
force-pushed
the
andrew/jumphost-relogin-fix
branch
from
1f632ef to
1097859
Compare
r0mant
approved these changes
Mar 30, 2021
Comment on lines
-472
to
-480
| // 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} |
There was a problem hiding this comment.
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.
awly
pushed a commit
that referenced
this pull request
Mar 30, 2021
* ssh: fix relogin with jumphosts Several fixes to make `tsh ssh -J leaf.proxy.com` work if the root cert is missing/expired. * Address review feedback Correctly parse trusted CAs on GetKey. Move retry without jumphosts from relogin to UpdateClusterCAs. * Remove TelpoertClient.AuthMethods override on relogin It doesn't seem to break anything.
awly
pushed a commit
that referenced
this pull request
Mar 30, 2021
* ssh: fix relogin with jumphosts Several fixes to make `tsh ssh -J leaf.proxy.com` work if the root cert is missing/expired. * Address review feedback Correctly parse trusted CAs on GetKey. Move retry without jumphosts from relogin to UpdateClusterCAs. * Remove TelpoertClient.AuthMethods override on relogin It doesn't seem to break anything.
pierrebeaucamp
pushed a commit
that referenced
this pull request
Mar 31, 2021
…e/dynamodb-gsi-autoscaling * 'master' of github.com:gravitational/teleport: (41 commits) Refactor ssh.ClientConfig used by tctl and API clients to use the first valid principal as User. Update Architecture Overview With Link To User Roles (#6224) Add `lint-api` target and fix lint errors (#6169) ssh: fix relogin with jumphosts (#6213) drone: use emptyDir for /var/lib/docker filesystem and prevent repetitive docker pulls (#6145) Remove ARM64 FIPS builds (#6236) tsh Profile SSH certs fix (#6214) mfa: fix gRPC unimplemented check in cert reissue Open Sources Access Controls Docs (#6188) (#6217) add PAM environment with interpolation support Cache per-cluster SSH certificates under ~/.tsh (#5938) add special resource type for access plugin data Enable DynamoDB autoscaling on global secondary indices (#6112) darwin fips builds (#5866) kube: add kubernetes_labels to role JSON schema mfa: send username instead of SSH login name in MFA cert request fix nil slice bug RFD 16: Add a section on `tctl rm` resetting resources back to defaults (#5673) Update application access docs (#6055) (#6137) Bump linux FIPS builds to use go1.16.2b7 release (#6143) ...
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Several fixes to make
tsh ssh -J leaf.proxy.comwork if the root certis missing/expired.