From 3437a15a8b0a7a005a12c4316e19dd3493686395 Mon Sep 17 00:00:00 2001 From: Gus Luxton Date: Mon, 2 May 2022 17:38:18 -0300 Subject: [PATCH 1/2] ssh: Ignore PuTTY-specific channel requests When proxying connections, PuTTY automatically requests the 'simple@putty.projects.tartarus.org' channel type to indicate that this is a "simple" connection which will not be requesting other channel formats. A request to Teleport for this channel currently results in the connection being terminated. A better idea is just to ignore this request. --- lib/srv/regular/sshserver.go | 7 ++ lib/srv/regular/sshserver_test.go | 135 ++++++++++++++++++++++++++++++ lib/sshutils/req.go | 4 + 3 files changed, 146 insertions(+) diff --git a/lib/srv/regular/sshserver.go b/lib/srv/regular/sshserver.go index 84e6026cba384..9a341cca22d00 100644 --- a/lib/srv/regular/sshserver.go +++ b/lib/srv/regular/sshserver.go @@ -1428,6 +1428,13 @@ func (s *Server) dispatch(ch ssh.Channel, req *ssh.Request, ctx *srv.ServerConte log.Warn(err) } return nil + case sshutils.PuTTYSimpleRequest: + // PuTTY automatically requests a named 'simple@putty.projects.tartarus.org' channel any time it connects to a server + // as a proxy to indicate that it's in "simple" node and won't be requesting any other channels. + // As we don't support this request, we ignore it. + // https://the.earth.li/~sgtatham/putty/0.76/htmldoc/AppendixG.html#sshnames-channel + log.Debugf("%v: deliberately ignoring request for '%v' channel", s.Component(), sshutils.PuTTYSimpleRequest) + return nil default: return trace.BadParameter( "(%v) proxy doesn't support request type '%v'", s.Component(), req.Type) diff --git a/lib/srv/regular/sshserver_test.go b/lib/srv/regular/sshserver_test.go index e32e3f574cfd4..33c50fbce4cf7 100644 --- a/lib/srv/regular/sshserver_test.go +++ b/lib/srv/regular/sshserver_test.go @@ -1850,6 +1850,141 @@ func TestX11ProxySupport(t *testing.T) { require.Equal(t, string(msg), string(rsp)) } +// TestIgnorePuTTYSimpleChannel verifies that any request from the PuTTY SSH client for +// its "simple" mode is ignored and connections remain open. +func TestIgnorePuTTYSimpleChannel(t *testing.T) { + t.Parallel() + + f := newFixture(t) + ctx := context.Background() + + listener, _ := mustListen(t) + logger := logrus.WithField("test", "TestIgnorePuTTYSimpleChannel") + proxyClient, _ := newProxyClient(t, f.testSrv) + lockWatcher := newLockWatcher(ctx, t, proxyClient) + nodeWatcher := newNodeWatcher(ctx, t, proxyClient) + + reverseTunnelServer, err := reversetunnel.NewServer(reversetunnel.Config{ + ClientTLS: proxyClient.TLSConfig(), + ID: hostID, + ClusterName: f.testSrv.ClusterName(), + Listener: listener, + HostSigners: []ssh.Signer{f.signer}, + LocalAuthClient: proxyClient, + LocalAccessPoint: proxyClient, + NewCachingAccessPoint: noCache, + NewCachingAccessPointOldProxy: noCache, + DirectClusters: []reversetunnel.DirectCluster{{Name: f.testSrv.ClusterName(), Client: proxyClient}}, + DataDir: t.TempDir(), + Emitter: proxyClient, + Log: logger, + LockWatcher: lockWatcher, + NodeWatcher: nodeWatcher, + }) + require.NoError(t, err) + + require.NoError(t, reverseTunnelServer.Start()) + defer reverseTunnelServer.Close() + + nodeClient, _ := newNodeClient(t, f.testSrv) + + proxy, err := New( + utils.NetAddr{AddrNetwork: "tcp", Addr: "localhost:0"}, + f.testSrv.ClusterName(), + []ssh.Signer{f.signer}, + proxyClient, + t.TempDir(), + "", + utils.NetAddr{}, + proxyClient, + SetProxyMode(reverseTunnelServer, proxyClient), + SetSessionServer(proxyClient), + SetEmitter(nodeClient), + SetNamespace(apidefaults.Namespace), + SetPAMConfig(&pam.Config{Enabled: false}), + SetBPF(&bpf.NOP{}), + SetRestrictedSessionManager(&restricted.NOP{}), + SetClock(f.clock), + SetLockWatcher(lockWatcher), + SetNodeWatcher(nodeWatcher), + ) + require.NoError(t, err) + require.NoError(t, proxy.Start()) + defer proxy.Close() + + // set up SSH client using the user private key for signing + up, err := newUpack(f.testSrv, f.user, []string{f.user}, wildcardAllow) + require.NoError(t, err) + + sshConfig := &ssh.ClientConfig{ + User: f.user, + Auth: []ssh.AuthMethod{ssh.PublicKeys(up.certSigner)}, + HostKeyCallback: ssh.FixedHostKey(f.signer.PublicKey()), + } + + _, err = newUpack(f.testSrv, "user1", []string{f.user}, wildcardAllow) + require.NoError(t, err) + + // Connect SSH client to proxy + client, err := ssh.Dial("tcp", proxy.Addr(), sshConfig) + + require.NoError(t, err) + defer client.Close() + + se, err := client.NewSession() + require.NoError(t, err) + defer se.Close() + + writer, err := se.StdinPipe() + require.NoError(t, err) + + reader, err := se.StdoutPipe() + require.NoError(t, err) + + // Request the PuTTY-specific "simple@putty.projects.tartarus.org" channel type + // This request should be ignored, and the connection should remain open. + _, err = se.SendRequest(sshutils.PuTTYSimpleRequest, false, []byte{}) + require.NoError(t, err) + + // Request proxy subsystem routing TCP connection to the remote host + require.NoError(t, se.RequestSubsystem(fmt.Sprintf("proxy:%v", f.ssh.srvAddress))) + + local, err := utils.ParseAddr("tcp://" + proxy.Addr()) + require.NoError(t, err) + remote, err := utils.ParseAddr("tcp://" + f.ssh.srv.Addr()) + require.NoError(t, err) + + pipeNetConn := utils.NewPipeNetConn( + reader, + writer, + se, + local, + remote, + ) + + defer pipeNetConn.Close() + + // Open SSH connection via proxy subsystem's TCP tunnel + conn, chans, reqs, err := ssh.NewClientConn(pipeNetConn, + f.ssh.srv.Addr(), sshConfig) + require.NoError(t, err) + defer conn.Close() + + // Run commands over this connection like regular SSH + client2 := ssh.NewClient(conn, chans, reqs) + require.NoError(t, err) + defer client2.Close() + + se2, err := client2.NewSession() + require.NoError(t, err) + defer se2.Close() + + out, err := se2.Output("echo hello again") + require.NoError(t, err) + + require.Equal(t, "hello again\n", string(out)) +} + // upack holds all ssh signing artefacts needed for signing and checking user keys type upack struct { // key is a raw private user key diff --git a/lib/sshutils/req.go b/lib/sshutils/req.go index 5745033f0dea9..df4027adba857 100644 --- a/lib/sshutils/req.go +++ b/lib/sshutils/req.go @@ -166,6 +166,10 @@ const ( // X11ChannelRequest is the type of an X11 forwarding channel. X11ChannelRequest = "x11" + + // PuTTYSimpleRequest is a PuTTY-specific channel name which it automatically requests when it proxies + // connections. Teleport does not support this channel type, so deliberately ignores requests for it. + PuTTYSimpleRequest = "simple@putty.projects.tartarus.org" ) const ( From b6fdcb8c382089a9d4619a5ef572ae8336e43640 Mon Sep 17 00:00:00 2001 From: Gus Luxton Date: Mon, 16 May 2022 15:44:39 -0300 Subject: [PATCH 2/2] Update lib/srv/regular/sshserver.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Krzysztof Skrzętnicki --- lib/srv/regular/sshserver.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/srv/regular/sshserver.go b/lib/srv/regular/sshserver.go index 9a341cca22d00..6b3a5683d5e9d 100644 --- a/lib/srv/regular/sshserver.go +++ b/lib/srv/regular/sshserver.go @@ -1430,7 +1430,7 @@ func (s *Server) dispatch(ch ssh.Channel, req *ssh.Request, ctx *srv.ServerConte return nil case sshutils.PuTTYSimpleRequest: // PuTTY automatically requests a named 'simple@putty.projects.tartarus.org' channel any time it connects to a server - // as a proxy to indicate that it's in "simple" node and won't be requesting any other channels. + // as a proxy to indicate that it's in "simple" mode and won't be requesting any other channels. // As we don't support this request, we ignore it. // https://the.earth.li/~sgtatham/putty/0.76/htmldoc/AppendixG.html#sshnames-channel log.Debugf("%v: deliberately ignoring request for '%v' channel", s.Component(), sshutils.PuTTYSimpleRequest)