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

Commit658db5a

Browse files
committed
fix(agent/agentssh): pin random seed for RSA key generation
Change-Id: I8c7e3070324e5d558374fd6891eea9d48660e1e9Signed-off-by: Thomas Kosiewski <tk@coder.com>
1 parent4449931 commit658db5a

File tree

6 files changed

+169
-17
lines changed

6 files changed

+169
-17
lines changed

‎agent/agent.go

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"encoding/json"
77
"errors"
88
"fmt"
9+
"hash/fnv"
910
"io"
1011
"net/http"
1112
"net/netip"
@@ -994,7 +995,6 @@ func (a *agent) createOrUpdateNetwork(manifestOK, networkOK *checkpoint) func(co
994995
iferr:=manifestOK.wait(ctx);err!=nil {
995996
returnxerrors.Errorf("no manifest: %w",err)
996997
}
997-
varerrerror
998998
deferfunc() {
999999
networkOK.complete(retErr)
10001000
}()
@@ -1003,9 +1003,20 @@ func (a *agent) createOrUpdateNetwork(manifestOK, networkOK *checkpoint) func(co
10031003
network:=a.network
10041004
a.closeMutex.Unlock()
10051005
ifnetwork==nil {
1006+
keySeed,err:=WorkspaceKeySeed(manifest.WorkspaceID,manifest.AgentName)
1007+
iferr!=nil {
1008+
returnxerrors.Errorf("generate seed from workspace id: %w",err)
1009+
}
10061010
// use the graceful context here, because creating the tailnet is not itself tied to the
10071011
// agent API.
1008-
network,err=a.createTailnet(a.gracefulCtx,manifest.AgentID,manifest.DERPMap,manifest.DERPForceWebSockets,manifest.DisableDirectConnections)
1012+
network,err=a.createTailnet(
1013+
a.gracefulCtx,
1014+
manifest.AgentID,
1015+
manifest.DERPMap,
1016+
manifest.DERPForceWebSockets,
1017+
manifest.DisableDirectConnections,
1018+
keySeed,
1019+
)
10091020
iferr!=nil {
10101021
returnxerrors.Errorf("create tailnet: %w",err)
10111022
}
@@ -1145,7 +1156,13 @@ func (a *agent) trackGoroutine(fn func()) error {
11451156
returnnil
11461157
}
11471158

1148-
func (a*agent)createTailnet(ctx context.Context,agentID uuid.UUID,derpMap*tailcfg.DERPMap,derpForceWebSockets,disableDirectConnectionsbool) (_*tailnet.Conn,errerror) {
1159+
func (a*agent)createTailnet(
1160+
ctx context.Context,
1161+
agentID uuid.UUID,
1162+
derpMap*tailcfg.DERPMap,
1163+
derpForceWebSockets,disableDirectConnectionsbool,
1164+
keySeedint64,
1165+
) (_*tailnet.Conn,errerror) {
11491166
// Inject `CODER_AGENT_HEADER` into the DERP header.
11501167
varheader http.Header
11511168
ifclient,ok:=a.client.(*agentsdk.Client);ok {
@@ -1172,6 +1189,10 @@ func (a *agent) createTailnet(ctx context.Context, agentID uuid.UUID, derpMap *t
11721189
}
11731190
}()
11741191

1192+
iferr:=a.sshServer.UpdateHostSigner(keySeed);err!=nil {
1193+
returnnil,xerrors.Errorf("update host signer: %w",err)
1194+
}
1195+
11751196
sshListener,err:=network.Listen("tcp",":"+strconv.Itoa(workspacesdk.AgentSSHPort))
11761197
iferr!=nil {
11771198
returnnil,xerrors.Errorf("listen on the ssh port: %w",err)
@@ -1849,3 +1870,20 @@ func PrometheusMetricsHandler(prometheusRegistry *prometheus.Registry, logger sl
18491870
}
18501871
})
18511872
}
1873+
1874+
// WorkspaceKeySeed converts a WorkspaceID UUID and agent name to an int64 hash.
1875+
// This uses the FNV-1a hash algorithm which provides decent distribution and collision
1876+
// resistance for string inputs.
1877+
funcWorkspaceKeySeed(workspaceID uuid.UUID,agentNamestring) (int64,error) {
1878+
h:=fnv.New64a()
1879+
_,err:=h.Write(workspaceID[:])
1880+
iferr!=nil {
1881+
return42,err
1882+
}
1883+
_,err=h.Write([]byte(agentName))
1884+
iferr!=nil {
1885+
return42,err
1886+
}
1887+
1888+
returnint64(h.Sum64()),nil
1889+
}

‎agent/agentssh/agentssh.go

Lines changed: 44 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,11 @@ package agentssh
33
import (
44
"bufio"
55
"context"
6-
"crypto/rand"
76
"crypto/rsa"
87
"errors"
98
"fmt"
109
"io"
10+
"math/rand"
1111
"net"
1212
"os"
1313
"os/exec"
@@ -128,17 +128,6 @@ type Server struct {
128128
}
129129

130130
funcNewServer(ctx context.Context,logger slog.Logger,prometheusRegistry*prometheus.Registry,fs afero.Fs,execer agentexec.Execer,config*Config) (*Server,error) {
131-
// Clients' should ignore the host key when connecting.
132-
// The agent needs to authenticate with coderd to SSH,
133-
// so SSH authentication doesn't improve security.
134-
randomHostKey,err:=rsa.GenerateKey(rand.Reader,2048)
135-
iferr!=nil {
136-
returnnil,err
137-
}
138-
randomSigner,err:=gossh.NewSignerFromKey(randomHostKey)
139-
iferr!=nil {
140-
returnnil,err
141-
}
142131
ifconfig==nil {
143132
config=&Config{}
144133
}
@@ -205,8 +194,10 @@ func NewServer(ctx context.Context, logger slog.Logger, prometheusRegistry *prom
205194
slog.F("local_addr",conn.LocalAddr()),
206195
slog.Error(err))
207196
},
208-
Handler:s.sessionHandler,
209-
HostSigners: []ssh.Signer{randomSigner},
197+
Handler:s.sessionHandler,
198+
// HostSigners are intentionally empty, as the host key will
199+
// be set before we start listening.
200+
HostSigners: []ssh.Signer{},
210201
LocalPortForwardingCallback:func(ctx ssh.Context,destinationHoststring,destinationPortuint32)bool {
211202
// Allow local port forwarding all!
212203
s.logger.Debug(ctx,"local port forward",
@@ -844,7 +835,13 @@ func (s *Server) CreateCommand(ctx context.Context, script string, env []string,
844835
returncmd,nil
845836
}
846837

838+
// Serve starts the server to handle incoming connections on the provided listener.
839+
// It returns an error if no host keys are set or if there is an issue accepting connections.
847840
func (s*Server)Serve(l net.Listener) (retErrerror) {
841+
iflen(s.srv.HostSigners)==0 {
842+
returnxerrors.New("no host keys set")
843+
}
844+
848845
s.logger.Info(context.Background(),"started serving listener",slog.F("listen_addr",l.Addr()))
849846
deferfunc() {
850847
s.logger.Info(context.Background(),"stopped serving listener",
@@ -1099,3 +1096,36 @@ func userHomeDir() (string, error) {
10991096
}
11001097
returnu.HomeDir,nil
11011098
}
1099+
1100+
// UpdateHostSigner updates the host signer with a new key generated from the provided seed.
1101+
// If an existing host key exists with the same algorithm, it is overwritten
1102+
func (s*Server)UpdateHostSigner(seedint64)error {
1103+
key,err:=CoderSigner(seed)
1104+
iferr!=nil {
1105+
returnerr
1106+
}
1107+
1108+
s.mu.Lock()
1109+
defers.mu.Unlock()
1110+
1111+
s.srv.AddHostKey(key)
1112+
1113+
returnnil
1114+
}
1115+
1116+
// CoderSigner generates a deterministic SSH signer based on the provided seed.
1117+
// It uses RSA with a key size of 2048 bits.
1118+
funcCoderSigner(seedint64) (gossh.Signer,error) {
1119+
// Clients should ignore the host key when connecting.
1120+
// The agent needs to authenticate with coderd to SSH,
1121+
// so SSH authentication doesn't improve security.
1122+
1123+
// nolint: gosec
1124+
deterministicRand:=rand.New(rand.NewSource(seed))
1125+
coderHostKey,err:=rsa.GenerateKey(deterministicRand,2048)
1126+
iferr!=nil {
1127+
returnnil,err
1128+
}
1129+
coderSigner,err:=gossh.NewSignerFromKey(coderHostKey)
1130+
returncoderSigner,err
1131+
}

‎agent/agentssh/agentssh_internal_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ func Test_sessionStart_orphan(t *testing.T) {
3939
s,err:=NewServer(ctx,logger,prometheus.NewRegistry(),afero.NewMemMapFs(),agentexec.DefaultExecer,nil)
4040
require.NoError(t,err)
4141
defers.Close()
42+
err=s.UpdateHostSigner(42)
43+
assert.NoError(t,err)
4244

4345
// Here we're going to call the handler directly with a faked SSH session
4446
// that just uses io.Pipes instead of a network socket. There is a large

‎agent/agentssh/agentssh_test.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,8 @@ func TestNewServer_ServeClient(t *testing.T) {
4141
s,err:=agentssh.NewServer(ctx,logger,prometheus.NewRegistry(),afero.NewMemMapFs(),agentexec.DefaultExecer,nil)
4242
require.NoError(t,err)
4343
defers.Close()
44+
err=s.UpdateHostSigner(42)
45+
assert.NoError(t,err)
4446

4547
ln,err:=net.Listen("tcp","127.0.0.1:0")
4648
require.NoError(t,err)
@@ -146,6 +148,8 @@ func TestNewServer_CloseActiveConnections(t *testing.T) {
146148
s,err:=agentssh.NewServer(ctx,logger,prometheus.NewRegistry(),afero.NewMemMapFs(),agentexec.DefaultExecer,nil)
147149
require.NoError(t,err)
148150
defers.Close()
151+
err=s.UpdateHostSigner(42)
152+
assert.NoError(t,err)
149153

150154
ln,err:=net.Listen("tcp","127.0.0.1:0")
151155
require.NoError(t,err)
@@ -197,6 +201,8 @@ func TestNewServer_Signal(t *testing.T) {
197201
s,err:=agentssh.NewServer(ctx,logger,prometheus.NewRegistry(),afero.NewMemMapFs(),agentexec.DefaultExecer,nil)
198202
require.NoError(t,err)
199203
defers.Close()
204+
err=s.UpdateHostSigner(42)
205+
assert.NoError(t,err)
200206

201207
ln,err:=net.Listen("tcp","127.0.0.1:0")
202208
require.NoError(t,err)
@@ -262,6 +268,8 @@ func TestNewServer_Signal(t *testing.T) {
262268
s,err:=agentssh.NewServer(ctx,logger,prometheus.NewRegistry(),afero.NewMemMapFs(),agentexec.DefaultExecer,nil)
263269
require.NoError(t,err)
264270
defers.Close()
271+
err=s.UpdateHostSigner(42)
272+
assert.NoError(t,err)
265273

266274
ln,err:=net.Listen("tcp","127.0.0.1:0")
267275
require.NoError(t,err)

‎agent/agentssh/x11_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ func TestServer_X11(t *testing.T) {
3838
s,err:=agentssh.NewServer(ctx,logger,prometheus.NewRegistry(),fs,agentexec.DefaultExecer,&agentssh.Config{})
3939
require.NoError(t,err)
4040
defers.Close()
41+
err=s.UpdateHostSigner(42)
42+
assert.NoError(t,err)
4143

4244
ln,err:=net.Listen("tcp","127.0.0.1:0")
4345
require.NoError(t,err)

‎cli/ssh_test.go

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -453,6 +453,78 @@ func TestSSH(t *testing.T) {
453453
<-cmdDone
454454
})
455455

456+
t.Run("DeterministicHostKey",func(t*testing.T) {
457+
t.Parallel()
458+
client,workspace,agentToken:=setupWorkspaceForAgent(t,func(agents []*proto.Agent) []*proto.Agent {
459+
fori,agent:=rangeagents {
460+
agent.Name=fmt.Sprintf("agent-%v",i)
461+
}
462+
463+
returnagents
464+
})
465+
_,_=tGoContext(t,func(ctx context.Context) {
466+
// Run this async so the SSH command has to wait for
467+
// the build and agent to connect!
468+
_=agenttest.New(t,client.URL,agentToken)
469+
<-ctx.Done()
470+
})
471+
472+
clientOutput,clientInput:=io.Pipe()
473+
serverOutput,serverInput:=io.Pipe()
474+
deferfunc() {
475+
for_,c:=range []io.Closer{clientOutput,clientInput,serverOutput,serverInput} {
476+
_=c.Close()
477+
}
478+
}()
479+
480+
ctx,cancel:=context.WithTimeout(context.Background(),testutil.WaitLong)
481+
defercancel()
482+
483+
inv,root:=clitest.New(t,"ssh","--stdio",workspace.Name)
484+
clitest.SetupConfig(t,client,root)
485+
inv.Stdin=clientOutput
486+
inv.Stdout=serverInput
487+
inv.Stderr=io.Discard
488+
489+
cmdDone:=tGo(t,func() {
490+
err:=inv.WithContext(ctx).Run()
491+
assert.NoError(t,err)
492+
})
493+
494+
keySeed,err:=agent.WorkspaceKeySeed(workspace.ID,"agent-0")
495+
assert.NoError(t,err)
496+
497+
signer,err:=agentssh.CoderSigner(keySeed)
498+
assert.NoError(t,err)
499+
500+
conn,channels,requests,err:=ssh.NewClientConn(&stdioConn{
501+
Reader:serverOutput,
502+
Writer:clientInput,
503+
},"",&ssh.ClientConfig{
504+
// #nosec
505+
HostKeyCallback:ssh.FixedHostKey(signer.PublicKey()),
506+
})
507+
require.NoError(t,err)
508+
deferconn.Close()
509+
510+
sshClient:=ssh.NewClient(conn,channels,requests)
511+
session,err:=sshClient.NewSession()
512+
require.NoError(t,err)
513+
defersession.Close()
514+
515+
command:="sh -c exit"
516+
ifruntime.GOOS=="windows" {
517+
command="cmd.exe /c exit"
518+
}
519+
err=session.Run(command)
520+
require.NoError(t,err)
521+
err=sshClient.Close()
522+
require.NoError(t,err)
523+
_=clientOutput.Close()
524+
525+
<-cmdDone
526+
})
527+
456528
t.Run("NetworkInfo",func(t*testing.T) {
457529
t.Parallel()
458530
client,workspace,agentToken:=setupWorkspaceForAgent(t)

0 commit comments

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