Fix TLS Routing jumphost flow #11282
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
github-actions
bot
added
the
tsh
r0mant
approved these changes
Mar 22, 2022
r0mant
reviewed
Mar 22, 2022
There was a problem hiding this comment.
@smallinsky Can you also make sure it works with ssh -J before merging?
jakule
approved these changes
Mar 22, 2022
| @@ -166,6 +174,57 @@ func testLeafClusterSSHAccess(t *testing.T, s *suite) { | |||
| require.NoError(t, err) | |||
| } | |||
|
|
|||
| func testJumpHostSSHAccess(t *testing.T, s *suite) { | |||
There was a problem hiding this comment.
I think converting this test cases to a test table would make this test easier to understand.
| // Check if JumpHost address is a proxy web address. | ||
| resp, err := webclient.Find(ctx, sshProxyAddr, tc.InsecureSkipVerify, nil) | ||
| // If JumpHost address is a proxy web port and proxy supports TLSRouting dial proxy with TLSWrapper. | ||
| if err == nil && resp.Proxy.TLSRoutingEnabled { |
There was a problem hiding this comment.
What if the error is returned here? Shouldn't be returned instead of "ignored"?
There was a problem hiding this comment.
This is on purpose if the call err != nil it means sshProxyAddr is not proxy web service flow should proceed with standard dialer.
|
Tested it manually both following cases works:
|
smallinsky
force-pushed
the
smallinsky/fix-tls-routing-jumphost
branch
from
ef0006a to
fa0e61a
Compare
smallinsky
added a commit
that referenced
this pull request
Mar 28, 2022
smallinsky
added a commit
that referenced
this pull request
Mar 28, 2022
This was referenced Mar 28, 2022
smallinsky
added a commit
that referenced
this pull request
Mar 28, 2022
smallinsky
added a commit
that referenced
this pull request
Mar 29, 2022
Closed
Closed
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.
Issue: #11271
What
TLS Routing broke JumpHost access in case where leaf cluster is used as JumpHost. The JumpHost address is not properly propagated to
makeProxySSHClientfunction call and alway root cluster is used.How
Fix JumpHost address propagation. Also added support for case where JumpHost is set to Teleport proxy web port and proxy support TLS Routing.