diff --git a/integration/helpers/discard.go b/integration/helpers/discard.go index d0baad31241..a525f21432b 100644 --- a/integration/helpers/discard.go +++ b/integration/helpers/discard.go @@ -43,7 +43,7 @@ func NewDiscardServer(hostSigner ssh.Signer, listener net.Listener) (*DiscardSer "integration-discard-server", utils.NetAddr{AddrNetwork: "tcp", Addr: listener.Addr().String()}, ds, - []ssh.Signer{hostSigner}, + sshutils.StaticHostSigners(hostSigner), sshutils.AuthMethods{ PublicKey: ds.userKeyAuth, }, diff --git a/integration/utmp_integration_test.go b/integration/utmp_integration_test.go index 1b91ea2b013..bed6f5c1d0e 100644 --- a/integration/utmp_integration_test.go +++ b/integration/utmp_integration_test.go @@ -314,7 +314,7 @@ func newSrvCtx(ctx context.Context, t *testing.T) *SrvCtx { ctx, utils.NetAddr{AddrNetwork: "tcp", Addr: "127.0.0.1:0"}, s.server.ClusterName(), - []ssh.Signer{s.signer}, + sshutils.StaticHostSigners(s.signer), s.nodeClient, nodeDir, "", diff --git a/lib/auth/init_test.go b/lib/auth/init_test.go index 84ffaad7a9f..7f92ffd555a 100644 --- a/lib/auth/init_test.go +++ b/lib/auth/init_test.go @@ -1731,7 +1731,7 @@ func TestIdentityChecker(t *testing.T) { "test", utils.NetAddr{AddrNetwork: "tcp", Addr: "localhost:0"}, handler, - []ssh.Signer{test.cert}, + sshutils.StaticHostSigners(test.cert), sshutils.AuthMethods{NoClient: true}, sshutils.SetInsecureSkipHostValidation(), ) diff --git a/lib/client/client_store_test.go b/lib/client/client_store_test.go index 81fc0c85fe3..3449ecc8bad 100644 --- a/lib/client/client_store_test.go +++ b/lib/client/client_store_test.go @@ -307,7 +307,7 @@ func TestProxySSHConfig(t *testing.T) { "test", utils.NetAddr{AddrNetwork: "tcp", Addr: "127.0.0.1:0"}, handler, - []ssh.Signer{hostSigner}, + sshutils.StaticHostSigners(hostSigner), sshutils.AuthMethods{ PublicKey: func(conn ssh.ConnMetadata, key ssh.PublicKey) (*ssh.Permissions, error) { certChecker := apisshutils.CertChecker{ diff --git a/lib/cloud/gcp/vm_test.go b/lib/cloud/gcp/vm_test.go index 09808a09e18..587291de1b6 100644 --- a/lib/cloud/gcp/vm_test.go +++ b/lib/cloud/gcp/vm_test.go @@ -53,7 +53,7 @@ func newMockInstance(t *testing.T, hostSigner ssh.Signer, listener net.Listener) "gcp-vm-server", utils.NetAddr{AddrNetwork: "tcp", Addr: listener.Addr().String()}, mock, - []ssh.Signer{hostSigner}, + sshutils.StaticHostSigners(hostSigner), sshutils.AuthMethods{ PublicKey: mock.userKeyAuth, }, diff --git a/lib/reversetunnel/agent_dialer_test.go b/lib/reversetunnel/agent_dialer_test.go index c9db778e736..7e517497f4a 100644 --- a/lib/reversetunnel/agent_dialer_test.go +++ b/lib/reversetunnel/agent_dialer_test.go @@ -74,7 +74,7 @@ func TestAgentCertChecker(t *testing.T) { "test", utils.NetAddr{AddrNetwork: "tcp", Addr: "localhost:0"}, handler, - []ssh.Signer{cert}, + sshutils.StaticHostSigners(cert), sshutils.AuthMethods{NoClient: true}, sshutils.SetInsecureSkipHostValidation(), ) diff --git a/lib/reversetunnel/srv.go b/lib/reversetunnel/srv.go index b178e5168dd..3d133c08f94 100644 --- a/lib/reversetunnel/srv.go +++ b/lib/reversetunnel/srv.go @@ -137,7 +137,7 @@ type Config struct { // Listener is a listener address for reverse tunnel server Listener net.Listener // HostSigners is a list of host signers - HostSigners []ssh.Signer + GetHostSigners sshutils.GetHostSignersFunc // HostKeyCallback // Limiter is optional request limiter Limiter *limiter.Limiter @@ -347,7 +347,7 @@ func NewServer(cfg Config) (reversetunnelclient.Server, error) { // this address is not used utils.NetAddr{Addr: "127.0.0.1:1", AddrNetwork: "tcp"}, srv, - cfg.HostSigners, + cfg.GetHostSigners, sshutils.AuthMethods{ PublicKey: srv.keyAuth, }, diff --git a/lib/service/service.go b/lib/service/service.go index 36916638fca..c49c7ab26cf 100644 --- a/lib/service/service.go +++ b/lib/service/service.go @@ -58,7 +58,6 @@ import ( "go.opentelemetry.io/otel/attribute" "golang.org/x/crypto/acme" "golang.org/x/crypto/acme/autocert" - "golang.org/x/crypto/ssh" "golang.org/x/sys/unix" "google.golang.org/grpc" "google.golang.org/grpc/credentials" @@ -2731,7 +2730,7 @@ func (process *TeleportProcess) initSSH() error { process.ExitContext(), cfg.SSH.Addr, cfg.Hostname, - []ssh.Signer{conn.ServerIdentity.KeySigner}, + sshutils.StaticHostSigners(conn.ServerIdentity.KeySigner), authClient, cfg.DataDir, cfg.AdvertiseIP, @@ -4080,7 +4079,7 @@ func (process *TeleportProcess) initProxyEndpoint(conn *Connector) error { ClusterName: clusterName, ClientTLS: clientTLSConfig, Listener: reverseTunnelLimiter.WrapListener(listeners.reverseTunnel), - HostSigners: []ssh.Signer{conn.ServerIdentity.KeySigner}, + GetHostSigners: sshutils.StaticHostSigners(conn.ServerIdentity.KeySigner), LocalAuthClient: conn.Client, LocalAccessPoint: accessPoint, NewCachingAccessPoint: process.newLocalCacheForRemoteProxy, @@ -4462,7 +4461,7 @@ func (process *TeleportProcess) initProxyEndpoint(conn *Connector) error { process.ExitContext(), cfg.SSH.Addr, cfg.Hostname, - []ssh.Signer{conn.ServerIdentity.KeySigner}, + sshutils.StaticHostSigners(conn.ServerIdentity.KeySigner), accessPoint, cfg.DataDir, "", diff --git a/lib/srv/regular/sshserver.go b/lib/srv/regular/sshserver.go index 183fd5dbd8f..df9a37e73a4 100644 --- a/lib/srv/regular/sshserver.go +++ b/lib/srv/regular/sshserver.go @@ -709,7 +709,7 @@ func New( ctx context.Context, addr utils.NetAddr, hostname string, - signers []ssh.Signer, + getHostSigners sshutils.GetHostSignersFunc, authService srv.AccessPoint, dataDir string, advertiseAddr string, @@ -830,7 +830,8 @@ func New( server, err := sshutils.NewServer( component, - addr, s, signers, + addr, s, + getHostSigners, sshutils.AuthMethods{PublicKey: s.authHandlers.UserKeyAuth}, sshutils.SetLimiter(s.limiter), sshutils.SetRequestHandler(s), diff --git a/lib/srv/regular/sshserver_test.go b/lib/srv/regular/sshserver_test.go index 026413b8fa3..e3c0e70b954 100644 --- a/lib/srv/regular/sshserver_test.go +++ b/lib/srv/regular/sshserver_test.go @@ -243,7 +243,7 @@ func newCustomFixture(t *testing.T, mutateCfg func(*auth.TestServerConfig), sshO ctx, utils.NetAddr{AddrNetwork: "tcp", Addr: "127.0.0.1:0"}, testServer.ClusterName(), - []ssh.Signer{signer}, + sshutils.StaticHostSigners(signer), nodeClient, nodeDir, "", @@ -1491,7 +1491,7 @@ func TestProxyRoundRobin(t *testing.T) { ClientTLS: proxyClient.TLSConfig(), ID: hostID, Listener: listener, - HostSigners: []ssh.Signer{f.signer}, + GetHostSigners: sshutils.StaticHostSigners(f.signer), LocalAuthClient: proxyClient, LocalAccessPoint: proxyClient, NewCachingAccessPoint: noCache, @@ -1533,7 +1533,7 @@ func TestProxyRoundRobin(t *testing.T) { ctx, utils.NetAddr{AddrNetwork: "tcp", Addr: "localhost:0"}, f.testSrv.ClusterName(), - []ssh.Signer{f.signer}, + sshutils.StaticHostSigners(f.signer), proxyClient, t.TempDir(), "", @@ -1629,7 +1629,7 @@ func TestProxyDirectAccess(t *testing.T) { ID: hostID, ClusterName: f.testSrv.ClusterName(), Listener: listener, - HostSigners: []ssh.Signer{f.signer}, + GetHostSigners: sshutils.StaticHostSigners(f.signer), LocalAuthClient: proxyClient, LocalAccessPoint: proxyClient, NewCachingAccessPoint: noCache, @@ -1672,7 +1672,7 @@ func TestProxyDirectAccess(t *testing.T) { ctx, utils.NetAddr{AddrNetwork: "tcp", Addr: "localhost:0"}, f.testSrv.ClusterName(), - []ssh.Signer{f.signer}, + sshutils.StaticHostSigners(f.signer), proxyClient, t.TempDir(), "", @@ -1910,7 +1910,7 @@ func TestLimiter(t *testing.T) { ctx, utils.NetAddr{AddrNetwork: "tcp", Addr: "127.0.0.1:0"}, f.testSrv.ClusterName(), - []ssh.Signer{f.signer}, + sshutils.StaticHostSigners(f.signer), nodeClient, nodeStateDir, "", @@ -2343,7 +2343,7 @@ func TestParseSubsystemRequest(t *testing.T) { ID: hostID, ClusterName: f.testSrv.ClusterName(), Listener: listener, - HostSigners: []ssh.Signer{f.signer}, + GetHostSigners: sshutils.StaticHostSigners(f.signer), LocalAuthClient: proxyClient, LocalAccessPoint: proxyClient, NewCachingAccessPoint: noCache, @@ -2388,7 +2388,7 @@ func TestParseSubsystemRequest(t *testing.T) { ctx, utils.NetAddr{AddrNetwork: "tcp", Addr: "localhost:0"}, f.testSrv.ClusterName(), - []ssh.Signer{f.signer}, + sshutils.StaticHostSigners(f.signer), proxyClient, t.TempDir(), "", @@ -2606,7 +2606,7 @@ func TestIgnorePuTTYSimpleChannel(t *testing.T) { ID: hostID, ClusterName: f.testSrv.ClusterName(), Listener: listener, - HostSigners: []ssh.Signer{f.signer}, + GetHostSigners: sshutils.StaticHostSigners(f.signer), LocalAuthClient: proxyClient, LocalAccessPoint: proxyClient, NewCachingAccessPoint: noCache, @@ -2648,7 +2648,7 @@ func TestIgnorePuTTYSimpleChannel(t *testing.T) { ctx, utils.NetAddr{AddrNetwork: "tcp", Addr: "localhost:0"}, f.testSrv.ClusterName(), - []ssh.Signer{f.signer}, + sshutils.StaticHostSigners(f.signer), proxyClient, t.TempDir(), "", @@ -2821,7 +2821,7 @@ func TestTargetMetadata(t *testing.T) { ctx, utils.NetAddr{AddrNetwork: "tcp", Addr: "127.0.0.1:0"}, testServer.ClusterName(), - []ssh.Signer{newSigner(t, ctx, testServer)}, + sshutils.StaticHostSigners(newSigner(t, ctx, testServer)), nodeClient, nodeDir, "", diff --git a/lib/sshutils/server.go b/lib/sshutils/server.go index 27a6b248e5e..7faedf42c0c 100644 --- a/lib/sshutils/server.go +++ b/lib/sshutils/server.go @@ -75,6 +75,7 @@ type Server struct { newChanHandler NewChanHandler reqHandler RequestHandler newConnHandler NewConnHandler + getHostSigners GetHostSignersFunc cfg ssh.ServerConfig limiter *limiter.Limiter @@ -193,7 +194,7 @@ func NewServer( component string, a utils.NetAddr, h NewChanHandler, - hostSigners []ssh.Signer, + getHostSigners GetHostSignersFunc, ah AuthMethods, opts ...ServerOption, ) (*Server, error) { @@ -209,6 +210,7 @@ func NewServer( }), addr: a, newChanHandler: h, + getHostSigners: getHostSigners, component: component, closeContext: closeContext, closeFunc: cancel, @@ -231,14 +233,11 @@ func NewServer( s.tracerProvider = tracing.DefaultProvider() } - err = s.checkArguments(a, h, hostSigners, ah) + err = s.checkArguments(a, h, getHostSigners, ah) if err != nil { return nil, err } - for _, signer := range hostSigners { - (&s.cfg).AddHostKey(signer) - } s.cfg.PublicKeyCallback = ah.PublicKey s.cfg.PasswordCallback = ah.Password s.cfg.NoClientAuth = ah.NoClient @@ -486,10 +485,21 @@ func (s *Server) HandleConnection(conn net.Conn) { defer s.ingressReporter.ConnectionClosed(s.ingressService, conn) } - cfg := &s.cfg + hostSigners := s.getHostSigners() + if err := s.validateHostSigners(hostSigners); err != nil { + s.log. + WithError(err). + WithField("remote_addr", conn.RemoteAddr()). + Error("Error during server setup for a new SSH connection (this is a bug).") + conn.Close() + return + } + + cfg := s.cfg + for _, signer := range hostSigners { + cfg.AddHostKey(signer) + } if v := serverVersionOverrideFromConn(conn); v != "" && v != cfg.ServerVersion { - cfg = new(ssh.ServerConfig) - *cfg = s.cfg cfg.ServerVersion = v } @@ -499,7 +509,7 @@ func (s *Server) HandleConnection(conn net.Conn) { // transmitted and received over the connection. conn = utils.NewTrackingConn(conn) - sconn, chans, reqs, err := ssh.NewServerConn(conn, cfg) + sconn, chans, reqs, err := ssh.NewServerConn(conn, &cfg) if err != nil { // Ignore EOF as these are triggered by loadbalancer health checks if !errors.Is(err, io.EOF) { @@ -704,7 +714,20 @@ type AuthMethods struct { NoClient bool } -func (s *Server) checkArguments(a utils.NetAddr, h NewChanHandler, hostSigners []ssh.Signer, ah AuthMethods) error { +// GetHostSignersFunc is an infallible function that returns host signers for +// use with a new SSH connection. It should not block, as it's called while the +// SSH client is already connected and waiting for the SSH handshake. +type GetHostSignersFunc = func() []ssh.Signer + +// StaticHostSigners returns a [GetHostSignersFunc] that always returns the +// given host signers. +func StaticHostSigners(hostSigners ...ssh.Signer) GetHostSignersFunc { + return func() []ssh.Signer { + return hostSigners + } +} + +func (s *Server) checkArguments(a utils.NetAddr, h NewChanHandler, getHostSigners GetHostSignersFunc, ah AuthMethods) error { // If the server is not in tunnel mode, an address must be specified. if s.listener != nil { if a.Addr == "" || a.AddrNetwork == "" { @@ -715,22 +738,29 @@ func (s *Server) checkArguments(a utils.NetAddr, h NewChanHandler, hostSigners [ if h == nil { return trace.BadParameter("missing NewChanHandler") } - if len(hostSigners) == 0 { - return trace.BadParameter("need at least one signer") + if getHostSigners == nil { + return trace.BadParameter("missing GetHostSignersFunc") } + if err := s.validateHostSigners(getHostSigners()); err != nil { + return trace.Wrap(err) + } + if ah.PublicKey == nil && ah.Password == nil && !ah.NoClient { + return trace.BadParameter("need at least one auth method") + } + return nil +} + +func (s *Server) validateHostSigners(hostSigners []ssh.Signer) error { for _, signer := range hostSigners { if signer == nil { return trace.BadParameter("host signer can not be nil") } - if !s.insecureSkipHostValidation { - err := validateHostSigner(s.fips, signer) - if err != nil { - return trace.Wrap(err) - } + if s.insecureSkipHostValidation { + continue + } + if err := validateHostSigner(s.fips, signer); err != nil { + return trace.Wrap(err) } - } - if ah.PublicKey == nil && ah.Password == nil && !ah.NoClient { - return trace.BadParameter("need at least one auth method") } return nil } diff --git a/lib/sshutils/server_test.go b/lib/sshutils/server_test.go index c6a6b920987..304f4bc4215 100644 --- a/lib/sshutils/server_test.go +++ b/lib/sshutils/server_test.go @@ -22,6 +22,7 @@ import ( "context" "fmt" "os" + "sync/atomic" "testing" "time" @@ -57,7 +58,7 @@ func TestStartStop(t *testing.T) { "test", utils.NetAddr{AddrNetwork: "tcp", Addr: "localhost:0"}, fn, - []ssh.Signer{signer}, + StaticHostSigners(signer), AuthMethods{Password: pass("abcdef123456")}, ) require.NoError(t, err) @@ -111,7 +112,7 @@ func TestShutdown(t *testing.T) { "test", utils.NetAddr{AddrNetwork: "tcp", Addr: "localhost:0"}, fn, - []ssh.Signer{signer}, + StaticHostSigners(signer), AuthMethods{Password: pass("abcdef123456")}, SetShutdownPollPeriod(10*time.Millisecond), ) @@ -163,7 +164,7 @@ func TestConfigureCiphers(t *testing.T) { "test", utils.NetAddr{AddrNetwork: "tcp", Addr: "localhost:0"}, fn, - []ssh.Signer{signer}, + StaticHostSigners(signer), AuthMethods{Password: pass("abcdef123456")}, SetCiphers([]string{"aes128-ctr"}), ) @@ -210,7 +211,7 @@ func TestHostSignerFIPS(t *testing.T) { assert.NoError(t, err) }) - var tests = []struct { + tests := []struct { inSigner ssh.Signer inFIPS bool assert require.ErrorAssertionFunc @@ -245,7 +246,7 @@ func TestHostSignerFIPS(t *testing.T) { "test", utils.NetAddr{AddrNetwork: "tcp", Addr: "localhost:0"}, fn, - []ssh.Signer{tt.inSigner}, + StaticHostSigners(tt.inSigner), AuthMethods{Password: pass("abcdef123456")}, SetCiphers([]string{"aes128-ctr"}), SetFIPS(tt.inFIPS), @@ -266,3 +267,49 @@ func pass(need string) PasswordFunc { return nil, fmt.Errorf("passwords don't match") } } + +func TestDynamicHostSigners(t *testing.T) { + t.Parallel() + + certFoo, signerFoo, err := cert.CreateCertificate("foo", ssh.HostCert) + require.NoError(t, err) + + certBar, signerBar, err := cert.CreateCertificate("bar", ssh.HostCert) + require.NoError(t, err) + + var activeSigner atomic.Pointer[ssh.Signer] + activeSigner.Store(&signerFoo) + + srv, err := NewServer( + "test", + utils.NetAddr{AddrNetwork: "tcp", Addr: "localhost:0"}, + NewChanHandlerFunc(func(_ context.Context, _ *ConnectionContext, nc ssh.NewChannel) { + err := nc.Reject(ssh.UnknownChannelType, ssh.UnknownChannelType.String()) + assert.NoError(t, err) + }), + func() []ssh.Signer { return []ssh.Signer{*activeSigner.Load()} }, + AuthMethods{NoClient: true}, + SetShutdownPollPeriod(10*time.Millisecond), + ) + require.NoError(t, err) + require.NoError(t, srv.Start()) + t.Cleanup(func() { _ = srv.Close() }) + + dial := func(pub ssh.PublicKey) error { + clt, err := ssh.Dial("tcp", srv.Addr(), &ssh.ClientConfig{ + HostKeyCallback: ssh.FixedHostKey(pub), + }) + if clt != nil { + defer clt.Close() + } + return err + } + + require.NoError(t, dial(certFoo)) + require.ErrorContains(t, dial(certBar), "ssh: host key mismatch") + + activeSigner.Store(&signerBar) + + require.NoError(t, dial(certBar)) + require.ErrorContains(t, dial(certFoo), "ssh: host key mismatch") +} diff --git a/lib/web/apiserver_test.go b/lib/web/apiserver_test.go index 2e56e18dd15..70d8f409aa6 100644 --- a/lib/web/apiserver_test.go +++ b/lib/web/apiserver_test.go @@ -330,7 +330,7 @@ func newWebSuiteWithConfig(t *testing.T, cfg webSuiteConfig) *WebSuite { ctx, utils.NetAddr{AddrNetwork: "tcp", Addr: "127.0.0.1:0"}, nodeID, - []ssh.Signer{signer}, + sshutils.StaticHostSigners(signer), nodeClient, nodeDataDir, "", @@ -394,7 +394,7 @@ func newWebSuiteWithConfig(t *testing.T, cfg webSuiteConfig) *WebSuite { Listener: revTunListener, ClientTLS: s.proxyClient.TLSConfig(), ClusterName: s.server.ClusterName(), - HostSigners: []ssh.Signer{signer}, + GetHostSigners: sshutils.StaticHostSigners(signer), LocalAuthClient: s.proxyClient, LocalAccessPoint: s.proxyClient, Emitter: s.proxyClient, @@ -434,7 +434,7 @@ func newWebSuiteWithConfig(t *testing.T, cfg webSuiteConfig) *WebSuite { ctx, utils.NetAddr{AddrNetwork: "tcp", Addr: "127.0.0.1:0"}, s.server.ClusterName(), - []ssh.Signer{signer}, + sshutils.StaticHostSigners(signer), s.proxyClient, t.TempDir(), "", @@ -610,7 +610,7 @@ func (s *WebSuite) addNode(t *testing.T, uuid string, hostname string, address s context.Background(), utils.NetAddr{AddrNetwork: "tcp", Addr: address}, hostname, - []ssh.Signer{signer}, + sshutils.StaticHostSigners(signer), nodeClient, t.TempDir(), "", @@ -7743,7 +7743,7 @@ func newWebPack(t *testing.T, numProxies int, opts ...proxyOption) *webPack { ctx, utils.NetAddr{AddrNetwork: "tcp", Addr: "127.0.0.1:0"}, nodeID, - hostSigners, + sshutils.StaticHostSigners(hostSigners...), nodeClient, nodeDataDir, "", @@ -7882,7 +7882,7 @@ func createProxy(ctx context.Context, t *testing.T, proxyID string, node *regula Listener: revTunListener, ClientTLS: authClient.TLSConfig(), ClusterName: authServer.ClusterName(), - HostSigners: hostSigners, + GetHostSigners: sshutils.StaticHostSigners(hostSigners...), LocalAuthClient: client, LocalAccessPoint: client, Emitter: client, @@ -8017,7 +8017,7 @@ func createProxy(ctx context.Context, t *testing.T, proxyID string, node *regula ctx, utils.NetAddr{AddrNetwork: proxyListener.Addr().Network(), Addr: mux.SSH().Addr().String()}, authServer.ClusterName(), - hostSigners, + sshutils.StaticHostSigners(hostSigners...), client, t.TempDir(), "",