Skip to content

Commit 9d77cc6

Browse files
committed
Improve web ui host dialing for file transfers
Converts web file transfers to use the same in process dialing that web sessions use. This eliminates extra latency by avoiding an SSH dial from the proxy to the proxy as a result of no longer using client.TeleportClient to establish the connection to the target host. Closes #24419.
1 parent c8d8c21 commit 9d77cc6

File tree

1 file changed

+77
-5
lines changed

1 file changed

+77
-5
lines changed

lib/web/files.go

Lines changed: 77 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,15 +28,21 @@ import (
2828
"github.com/julienschmidt/httprouter"
2929
"golang.org/x/crypto/ssh"
3030

31+
"github.com/gravitational/teleport"
3132
"github.com/gravitational/teleport/api/client/proto"
3233
"github.com/gravitational/teleport/api/defaults"
34+
apidefaults "github.com/gravitational/teleport/api/defaults"
3335
"github.com/gravitational/teleport/api/utils/keys"
3436
"github.com/gravitational/teleport/api/utils/sshutils"
37+
"github.com/gravitational/teleport/lib/agentless"
3538
"github.com/gravitational/teleport/lib/auth/authclient"
3639
"github.com/gravitational/teleport/lib/client"
40+
"github.com/gravitational/teleport/lib/modules"
3741
"github.com/gravitational/teleport/lib/multiplexer"
3842
"github.com/gravitational/teleport/lib/reversetunnelclient"
3943
"github.com/gravitational/teleport/lib/sshutils/sftp"
44+
"github.com/gravitational/teleport/lib/teleagent"
45+
"github.com/gravitational/teleport/lib/utils"
4046
)
4147

4248
// fileTransferRequest describes HTTP file transfer request
@@ -144,8 +150,7 @@ func (h *Handler) transferFile(w http.ResponseWriter, r *http.Request, p httprou
144150
}
145151

146152
if req.mfaResponse != "" {
147-
err = ft.issueSingleUseCert(mfaResponse, r, tc)
148-
if err != nil {
153+
if err = ft.issueSingleUseCert(mfaResponse, r, tc); err != nil {
149154
return nil, trace.Wrap(err)
150155
}
151156
}
@@ -155,17 +160,84 @@ func (h *Handler) transferFile(w http.ResponseWriter, r *http.Request, p httprou
155160
ctx = context.WithValue(ctx, sftp.ModeratedSessionID, req.moderatedSessionID)
156161
}
157162

158-
cl, err := tc.ConnectToCluster(ctx)
163+
accessPoint, err := site.CachingAccessPoint()
159164
if err != nil {
165+
h.logger.DebugContext(r.Context(), "Unable to get auth access point", "error", err)
160166
return nil, trace.Wrap(err)
161167
}
162-
defer cl.Close()
163168

164-
err = tc.TransferFiles(ctx, cl, req.login, req.serverID+":0", cfg)
169+
accessChecker, err := sctx.GetUserAccessChecker()
165170
if err != nil {
171+
return nil, trace.Wrap(err)
172+
}
173+
174+
getAgent := func() (teleagent.Agent, error) {
175+
return teleagent.NopCloser(tc.LocalAgent()), nil
176+
}
177+
cert, err := sctx.GetSSHCertificate()
178+
if err != nil {
179+
return nil, trace.Wrap(err)
180+
}
181+
signer := agentless.SignerFromSSHCertificate(cert, h.auth.accessPoint, tc.SiteName, tc.Username)
182+
183+
conn, err := h.cfg.Router.DialHost(
184+
ctx,
185+
&utils.NetAddr{Addr: r.RemoteAddr},
186+
&h.cfg.ProxyWebAddr,
187+
req.serverID,
188+
"0",
189+
tc.SiteName,
190+
accessChecker,
191+
getAgent,
192+
signer,
193+
)
194+
if err != nil {
195+
if errors.Is(err, teleport.ErrNodeIsAmbiguous) {
196+
const message = "error: ambiguous host could match multiple nodes\n\nHint: try addressing the node by unique id (ex: user@node-id)\n"
197+
return nil, trace.NotFound(message)
198+
}
199+
200+
return nil, trace.Wrap(err)
201+
}
202+
203+
dialTimeout := apidefaults.DefaultIOTimeout
204+
if netConfig, err := accessPoint.GetClusterNetworkingConfig(ctx); err != nil {
205+
h.logger.DebugContext(r.Context(), "Unable to fetch cluster networking config", "error", err)
206+
} else {
207+
dialTimeout = netConfig.GetSSHDialTimeout()
208+
}
209+
210+
sshConfig := &ssh.ClientConfig{
211+
User: tc.HostLogin,
212+
Auth: tc.AuthMethods,
213+
HostKeyCallback: tc.HostKeyCallback,
214+
Timeout: dialTimeout,
215+
}
216+
217+
nodeClient, err := client.NewNodeClient(
218+
ctx,
219+
sshConfig,
220+
conn,
221+
req.serverID+":0",
222+
req.serverID,
223+
tc,
224+
modules.GetModules().IsBoringBinary(),
225+
)
226+
if err != nil {
227+
// The close error is ignored instead of using [trace.NewAggregate] because
228+
// aggregate errors do not allow error inspection with things like [trace.IsAccessDenied].
229+
_ = conn.Close()
230+
231+
return nil, trace.Wrap(err)
232+
}
233+
234+
defer nodeClient.Close()
235+
236+
if err := nodeClient.TransferFiles(ctx, cfg); err != nil {
166237
if errors.As(err, new(*sftp.NonRecursiveDirectoryTransferError)) {
167238
return nil, trace.Errorf("transferring directories through the Web UI is not supported at the moment, please use tsh scp -r")
168239
}
240+
169241
return nil, trace.Wrap(err)
170242
}
171243

0 commit comments

Comments
 (0)