Use the standard GetConfigForClient in sshGRPCServer#37093
Use the standard GetConfigForClient in sshGRPCServer#37093espadolini merged 4 commits intomasterfrom
Conversation
espadolini
force-pushed
the
espadolini/sshgrpc-clustercas
branch
2 times, most recently
from
4f5f4bd to
509151b
Compare
espadolini
force-pushed
the
espadolini/sshgrpc-clustercas
branch
from
509151b to
4cfb69e
Compare
github-actions
Bot
added
size/sm
tsh
zmb3
left a comment
zmb3
left a comment
There was a problem hiding this comment.
I'm on the fence about the testing changes. They seem both complicated and brittle.
In this case I would be okay with merging without the tests since you've demonstrated already that the fix is good.
| ) | ||
|
|
||
| //go:linkname x509_systemRootsMu crypto/x509.systemRootsMu | ||
| //go:linkname x509_systemRoots crypto/x509.systemRoots |
There was a problem hiding this comment.
Can we at least do this in a _test.go file to be sure it doesn't ever affect a production build?
There was a problem hiding this comment.
The issue with that is that other packages' test files don't exist when building tests.
Should I move this back to tools/tsh/common/proxy_test.go for now?
There was a problem hiding this comment.
I figured out that with //go:debug x509usefallbackroots=1 (which is far more supported) we get to replace the system cert pool without using linkname hacks or changing all the test running environments to add the matching GODEBUG envvar (b986a46)
public-teleport-github-review-bot
Bot
removed request for
EdwardDowling and
Joerger
|
/excludeflake * |
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
|
@espadolini See the table below for backport results.
|
3 participants
The "SSH gRPC" service currently only trusts the host and user CAs for the local cluster; it should, however, since it's necessary for the tsh ProxyJump functionality (
tsh ssh -J addr-of-leaf-proxy user@host ...). This PR changes the TLS config for the service to use the sameGetConfigForClientused by most other servers, which will load user CAs on demand.The changes to the test make it fail without the fix (in the "ssh jump host access" subtest) and succeed with; however, this PR introduces some minor go:debug hackery (in test code) to extend the system cert pool.
Fixes #36964
changelog: Fixed incompatibility between leaf clusters and ProxyJump