Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commite96652e

Browse files
authored
feat: block file transfers for security (#13501)
1 parent8326a3a commite96652e

File tree

5 files changed

+164
-0
lines changed

5 files changed

+164
-0
lines changed

‎agent/agent.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ type Options struct {
9191
ModifiedProcesseschan []*agentproc.Process
9292
// ProcessManagementTick is used for testing process priority management.
9393
ProcessManagementTick<-chan time.Time
94+
BlockFileTransferbool
9495
}
9596

9697
typeClientinterface {
@@ -184,6 +185,7 @@ func New(options Options) Agent {
184185
modifiedProcs:options.ModifiedProcesses,
185186
processManagementTick:options.ProcessManagementTick,
186187
logSender:agentsdk.NewLogSender(options.Logger),
188+
blockFileTransfer:options.BlockFileTransfer,
187189

188190
prometheusRegistry:prometheusRegistry,
189191
metrics:newAgentMetrics(prometheusRegistry),
@@ -239,6 +241,7 @@ type agent struct {
239241
sessionToken atomic.Pointer[string]
240242
sshServer*agentssh.Server
241243
sshMaxTimeout time.Duration
244+
blockFileTransferbool
242245

243246
lifecycleUpdatechanstruct{}
244247
lifecycleReportedchan codersdk.WorkspaceAgentLifecycle
@@ -277,6 +280,7 @@ func (a *agent) init() {
277280
AnnouncementBanners:func()*[]codersdk.BannerConfig {returna.announcementBanners.Load() },
278281
UpdateEnv:a.updateCommandEnv,
279282
WorkingDirectory:func()string {returna.manifest.Load().Directory },
283+
BlockFileTransfer:a.blockFileTransfer,
280284
})
281285
iferr!=nil {
282286
panic(err)

‎agent/agent_test.go‎

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -970,6 +970,99 @@ func TestAgent_SCP(t *testing.T) {
970970
require.NoError(t,err)
971971
}
972972

973+
funcTestAgent_FileTransferBlocked(t*testing.T) {
974+
t.Parallel()
975+
976+
assertFileTransferBlocked:=func(t*testing.T,errorMessagestring) {
977+
// NOTE: Checking content of the error message is flaky. Most likely there is a race condition, which results
978+
// in stopping the client in different phases, and returning different errors:
979+
// - client read the full error message: File transfer has been disabled.
980+
// - client's stream was terminated before reading the error message: EOF
981+
// - client just read the error code (Windows): Process exited with status 65
982+
isErr:=strings.Contains(errorMessage,agentssh.BlockedFileTransferErrorMessage)||
983+
strings.Contains(errorMessage,"EOF")||
984+
strings.Contains(errorMessage,"Process exited with status 65")
985+
require.True(t,isErr,fmt.Sprintf("Message: "+errorMessage))
986+
}
987+
988+
t.Run("SFTP",func(t*testing.T) {
989+
t.Parallel()
990+
991+
ctx,cancel:=context.WithTimeout(context.Background(),testutil.WaitLong)
992+
defercancel()
993+
994+
//nolint:dogsled
995+
conn,_,_,_,_:=setupAgent(t, agentsdk.Manifest{},0,func(_*agenttest.Client,o*agent.Options) {
996+
o.BlockFileTransfer=true
997+
})
998+
sshClient,err:=conn.SSHClient(ctx)
999+
require.NoError(t,err)
1000+
defersshClient.Close()
1001+
_,err=sftp.NewClient(sshClient)
1002+
require.Error(t,err)
1003+
assertFileTransferBlocked(t,err.Error())
1004+
})
1005+
1006+
t.Run("SCP with go-scp package",func(t*testing.T) {
1007+
t.Parallel()
1008+
1009+
ctx,cancel:=context.WithTimeout(context.Background(),testutil.WaitLong)
1010+
defercancel()
1011+
1012+
//nolint:dogsled
1013+
conn,_,_,_,_:=setupAgent(t, agentsdk.Manifest{},0,func(_*agenttest.Client,o*agent.Options) {
1014+
o.BlockFileTransfer=true
1015+
})
1016+
sshClient,err:=conn.SSHClient(ctx)
1017+
require.NoError(t,err)
1018+
defersshClient.Close()
1019+
scpClient,err:=scp.NewClientBySSH(sshClient)
1020+
require.NoError(t,err)
1021+
deferscpClient.Close()
1022+
tempFile:=filepath.Join(t.TempDir(),"scp")
1023+
err=scpClient.CopyFile(context.Background(),strings.NewReader("hello world"),tempFile,"0755")
1024+
require.Error(t,err)
1025+
assertFileTransferBlocked(t,err.Error())
1026+
})
1027+
1028+
t.Run("Forbidden commands",func(t*testing.T) {
1029+
t.Parallel()
1030+
1031+
for_,c:=rangeagentssh.BlockedFileTransferCommands {
1032+
t.Run(c,func(t*testing.T) {
1033+
t.Parallel()
1034+
1035+
ctx,cancel:=context.WithTimeout(context.Background(),testutil.WaitLong)
1036+
defercancel()
1037+
1038+
//nolint:dogsled
1039+
conn,_,_,_,_:=setupAgent(t, agentsdk.Manifest{},0,func(_*agenttest.Client,o*agent.Options) {
1040+
o.BlockFileTransfer=true
1041+
})
1042+
sshClient,err:=conn.SSHClient(ctx)
1043+
require.NoError(t,err)
1044+
defersshClient.Close()
1045+
1046+
session,err:=sshClient.NewSession()
1047+
require.NoError(t,err)
1048+
defersession.Close()
1049+
1050+
stdout,err:=session.StdoutPipe()
1051+
require.NoError(t,err)
1052+
1053+
//nolint:govet // we don't need `c := c` in Go 1.22
1054+
err=session.Start(c)
1055+
require.NoError(t,err)
1056+
defersession.Close()
1057+
1058+
msg,err:=io.ReadAll(stdout)
1059+
require.NoError(t,err)
1060+
assertFileTransferBlocked(t,string(msg))
1061+
})
1062+
}
1063+
})
1064+
}
1065+
9731066
funcTestAgent_EnvironmentVariables(t*testing.T) {
9741067
t.Parallel()
9751068
key:="EXAMPLE"

‎agent/agentssh/agentssh.go‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,16 @@ const (
5252
// MagicProcessCmdlineJetBrains is a string in a process's command line that
5353
// uniquely identifies it as JetBrains software.
5454
MagicProcessCmdlineJetBrains="idea.vendor.name=JetBrains"
55+
56+
// BlockedFileTransferErrorCode indicates that SSH server restricted the raw command from performing
57+
// the file transfer.
58+
BlockedFileTransferErrorCode=65// Error code: host not allowed to connect
59+
BlockedFileTransferErrorMessage="File transfer has been disabled."
5560
)
5661

62+
// BlockedFileTransferCommands contains a list of restricted file transfer commands.
63+
varBlockedFileTransferCommands= []string{"nc","rsync","scp","sftp"}
64+
5765
// Config sets configuration parameters for the agent SSH server.
5866
typeConfigstruct {
5967
// MaxTimeout sets the absolute connection timeout, none if empty. If set to
@@ -74,6 +82,8 @@ type Config struct {
7482
// X11SocketDir is the directory where X11 sockets are created. Default is
7583
// /tmp/.X11-unix.
7684
X11SocketDirstring
85+
// BlockFileTransfer restricts use of file transfer applications.
86+
BlockFileTransferbool
7787
}
7888

7989
typeServerstruct {
@@ -272,6 +282,18 @@ func (s *Server) sessionHandler(session ssh.Session) {
272282
extraEnv=append(extraEnv,fmt.Sprintf("DISPLAY=:%d.0",x11.ScreenNumber))
273283
}
274284

285+
ifs.fileTransferBlocked(session) {
286+
s.logger.Warn(ctx,"file transfer blocked",slog.F("session_subsystem",session.Subsystem()),slog.F("raw_command",session.RawCommand()))
287+
288+
ifsession.Subsystem()=="" {// sftp does not expect error, otherwise it fails with "package too long"
289+
// Response format: <status_code><message body>\n
290+
errorMessage:=fmt.Sprintf("\x02%s\n",BlockedFileTransferErrorMessage)
291+
_,_=session.Write([]byte(errorMessage))
292+
}
293+
_=session.Exit(BlockedFileTransferErrorCode)
294+
return
295+
}
296+
275297
switchss:=session.Subsystem();ss {
276298
case"":
277299
case"sftp":
@@ -322,6 +344,37 @@ func (s *Server) sessionHandler(session ssh.Session) {
322344
_=session.Exit(0)
323345
}
324346

347+
// fileTransferBlocked method checks if the file transfer commands should be blocked.
348+
//
349+
// Warning: consider this mechanism as "Do not trespass" sign, as a violator can still ssh to the host,
350+
// smuggle the `scp` binary, or just manually send files outside with `curl` or `ftp`.
351+
// If a user needs a more sophisticated and battle-proof solution, consider full endpoint security.
352+
func (s*Server)fileTransferBlocked(session ssh.Session)bool {
353+
if!s.config.BlockFileTransfer {
354+
returnfalse// file transfers are permitted
355+
}
356+
// File transfers are restricted.
357+
358+
ifsession.Subsystem()=="sftp" {
359+
returntrue
360+
}
361+
362+
cmd:=session.Command()
363+
iflen(cmd)==0 {
364+
returnfalse// no command?
365+
}
366+
367+
c:=cmd[0]
368+
c=filepath.Base(c)// in case the binary is absolute path, /usr/sbin/scp
369+
370+
for_,cmd:=rangeBlockedFileTransferCommands {
371+
ifcmd==c {
372+
returntrue
373+
}
374+
}
375+
returnfalse
376+
}
377+
325378
func (s*Server)sessionStart(logger slog.Logger,session ssh.Session,extraEnv []string) (retErrerror) {
326379
ctx:=session.Context()
327380
env:=append(session.Environ(),extraEnv...)

‎cli/agent.go‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import (
2727
"cdr.dev/slog/sloggers/slogstackdriver"
2828
"github.com/coder/coder/v2/agent"
2929
"github.com/coder/coder/v2/agent/agentproc"
30+
"github.com/coder/coder/v2/agent/agentssh"
3031
"github.com/coder/coder/v2/agent/reaper"
3132
"github.com/coder/coder/v2/buildinfo"
3233
"github.com/coder/coder/v2/codersdk"
@@ -48,6 +49,7 @@ func (r *RootCmd) workspaceAgent() *serpent.Command {
4849
slogHumanPathstring
4950
slogJSONPathstring
5051
slogStackdriverPathstring
52+
blockFileTransferbool
5153
)
5254
cmd:=&serpent.Command{
5355
Use:"agent",
@@ -314,6 +316,8 @@ func (r *RootCmd) workspaceAgent() *serpent.Command {
314316
// Intentionally set this to nil. It's mainly used
315317
// for testing.
316318
ModifiedProcesses:nil,
319+
320+
BlockFileTransfer:blockFileTransfer,
317321
})
318322

319323
promHandler:=agent.PrometheusMetricsHandler(prometheusRegistry,logger)
@@ -417,6 +421,13 @@ func (r *RootCmd) workspaceAgent() *serpent.Command {
417421
Default:"",
418422
Value:serpent.StringOf(&slogStackdriverPath),
419423
},
424+
{
425+
Flag:"block-file-transfer",
426+
Default:"false",
427+
Env:"CODER_AGENT_BLOCK_FILE_TRANSFER",
428+
Description:fmt.Sprintf("Block file transfer using known applications: %s.",strings.Join(agentssh.BlockedFileTransferCommands,",")),
429+
Value:serpent.BoolOf(&blockFileTransfer),
430+
},
420431
}
421432

422433
returncmd

‎cli/testdata/coder_agent_--help.golden‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@ OPTIONS:
1818
--auth string, $CODER_AGENT_AUTH (default: token)
1919
Specify the authentication type to use for the agent.
2020

21+
--block-file-transfer bool, $CODER_AGENT_BLOCK_FILE_TRANSFER (default: false)
22+
Block file transfer using known applications: nc,rsync,scp,sftp.
23+
2124
--debug-address string, $CODER_AGENT_DEBUG_ADDRESS (default: 127.0.0.1:2113)
2225
The bind address to serve a debug HTTP server.
2326

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp