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

Commitad275dc

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

File tree

6 files changed

+226
-17
lines changed

6 files changed

+226
-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: 108 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,12 @@ package agentssh
33
import (
44
"bufio"
55
"context"
6-
"crypto/rand"
76
"crypto/rsa"
87
"errors"
98
"fmt"
109
"io"
10+
"math/big"
11+
"math/rand"
1112
"net"
1213
"os"
1314
"os/exec"
@@ -128,17 +129,6 @@ type Server struct {
128129
}
129130

130131
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-
}
142132
ifconfig==nil {
143133
config=&Config{}
144134
}
@@ -205,8 +195,10 @@ func NewServer(ctx context.Context, logger slog.Logger, prometheusRegistry *prom
205195
slog.F("local_addr",conn.LocalAddr()),
206196
slog.Error(err))
207197
},
208-
Handler:s.sessionHandler,
209-
HostSigners: []ssh.Signer{randomSigner},
198+
Handler:s.sessionHandler,
199+
// HostSigners are intentionally empty, as the host key will
200+
// be set before we start listening.
201+
HostSigners: []ssh.Signer{},
210202
LocalPortForwardingCallback:func(ctx ssh.Context,destinationHoststring,destinationPortuint32)bool {
211203
// Allow local port forwarding all!
212204
s.logger.Debug(ctx,"local port forward",
@@ -844,7 +836,13 @@ func (s *Server) CreateCommand(ctx context.Context, script string, env []string,
844836
returncmd,nil
845837
}
846838

839+
// Serve starts the server to handle incoming connections on the provided listener.
840+
// It returns an error if no host keys are set or if there is an issue accepting connections.
847841
func (s*Server)Serve(l net.Listener) (retErrerror) {
842+
iflen(s.srv.HostSigners)==0 {
843+
returnxerrors.New("no host keys set")
844+
}
845+
848846
s.logger.Info(context.Background(),"started serving listener",slog.F("listen_addr",l.Addr()))
849847
deferfunc() {
850848
s.logger.Info(context.Background(),"stopped serving listener",
@@ -1099,3 +1097,99 @@ func userHomeDir() (string, error) {
10991097
}
11001098
returnu.HomeDir,nil
11011099
}
1100+
1101+
// UpdateHostSigner updates the host signer with a new key generated from the provided seed.
1102+
// If an existing host key exists with the same algorithm, it is overwritten
1103+
func (s*Server)UpdateHostSigner(seedint64)error {
1104+
key,err:=CoderSigner(seed)
1105+
iferr!=nil {
1106+
returnerr
1107+
}
1108+
1109+
s.mu.Lock()
1110+
defers.mu.Unlock()
1111+
1112+
s.srv.AddHostKey(key)
1113+
1114+
returnnil
1115+
}
1116+
1117+
// CoderSigner generates a deterministic SSH signer based on the provided seed.
1118+
// It uses RSA with a key size of 2048 bits.
1119+
funcCoderSigner(seedint64) (gossh.Signer,error) {
1120+
// Clients should ignore the host key when connecting.
1121+
// The agent needs to authenticate with coderd to SSH,
1122+
// so SSH authentication doesn't improve security.
1123+
1124+
// Since the standard lib purposefully does not generate
1125+
// deterministic rsa keys, we need to do it ourselves.
1126+
coderHostKey:=func()*rsa.PrivateKey {
1127+
// Create deterministic random source
1128+
// nolint: gosec
1129+
deterministicRand:=rand.New(rand.NewSource(seed))
1130+
1131+
// Use fixed values for p and q based on the seed
1132+
p:=big.NewInt(0)
1133+
q:=big.NewInt(0)
1134+
e:=big.NewInt(65537)// Standard RSA public exponent
1135+
1136+
// Generate deterministic primes using the seeded random
1137+
// Each prime should be ~1024 bits to get a 2048-bit key
1138+
for {
1139+
p.SetBit(p,1024,1)// Ensure it's large enough
1140+
fori:=0;i<1024;i++ {
1141+
ifdeterministicRand.Int63()%2==1 {
1142+
p.SetBit(p,i,1)
1143+
}else {
1144+
p.SetBit(p,i,0)
1145+
}
1146+
}
1147+
ifp.ProbablyPrime(20) {
1148+
break
1149+
}
1150+
}
1151+
1152+
for {
1153+
q.SetBit(q,1024,1)// Ensure it's large enough
1154+
fori:=0;i<1024;i++ {
1155+
ifdeterministicRand.Int63()%2==1 {
1156+
q.SetBit(q,i,1)
1157+
}else {
1158+
q.SetBit(q,i,0)
1159+
}
1160+
}
1161+
ifq.ProbablyPrime(20)&&p.Cmp(q)!=0 {
1162+
break
1163+
}
1164+
}
1165+
1166+
// Calculate n = p * q
1167+
n:=new(big.Int).Mul(p,q)
1168+
1169+
// Calculate phi = (p-1) * (q-1)
1170+
p1:=new(big.Int).Sub(p,big.NewInt(1))
1171+
q1:=new(big.Int).Sub(q,big.NewInt(1))
1172+
phi:=new(big.Int).Mul(p1,q1)
1173+
1174+
// Calculate private exponent d
1175+
d:=new(big.Int).ModInverse(e,phi)
1176+
1177+
// Create the private key
1178+
privateKey:=&rsa.PrivateKey{
1179+
PublicKey: rsa.PublicKey{
1180+
N:n,
1181+
E:int(e.Int64()),
1182+
},
1183+
D:d,
1184+
Primes: []*big.Int{p,q},
1185+
}
1186+
1187+
// Compute precomputed values
1188+
privateKey.Precompute()
1189+
1190+
returnprivateKey
1191+
}()
1192+
1193+
coderSigner,err:=gossh.NewSignerFromKey(coderHostKey)
1194+
returncoderSigner,err
1195+
}

‎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: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -453,6 +453,71 @@ 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)
459+
_,_=tGoContext(t,func(ctx context.Context) {
460+
// Run this async so the SSH command has to wait for
461+
// the build and agent to connect!
462+
_=agenttest.New(t,client.URL,agentToken)
463+
<-ctx.Done()
464+
})
465+
466+
clientOutput,clientInput:=io.Pipe()
467+
serverOutput,serverInput:=io.Pipe()
468+
deferfunc() {
469+
for_,c:=range []io.Closer{clientOutput,clientInput,serverOutput,serverInput} {
470+
_=c.Close()
471+
}
472+
}()
473+
474+
ctx,cancel:=context.WithTimeout(context.Background(),testutil.WaitLong)
475+
defercancel()
476+
477+
inv,root:=clitest.New(t,"ssh","--stdio",workspace.Name)
478+
clitest.SetupConfig(t,client,root)
479+
inv.Stdin=clientOutput
480+
inv.Stdout=serverInput
481+
inv.Stderr=io.Discard
482+
483+
cmdDone:=tGo(t,func() {
484+
err:=inv.WithContext(ctx).Run()
485+
assert.NoError(t,err)
486+
})
487+
488+
keySeed,err:=agent.WorkspaceKeySeed(workspace.ID,"dev")
489+
assert.NoError(t,err)
490+
491+
signer,err:=agentssh.CoderSigner(keySeed)
492+
assert.NoError(t,err)
493+
494+
conn,channels,requests,err:=ssh.NewClientConn(&stdioConn{
495+
Reader:serverOutput,
496+
Writer:clientInput,
497+
},"",&ssh.ClientConfig{
498+
HostKeyCallback:ssh.FixedHostKey(signer.PublicKey()),
499+
})
500+
require.NoError(t,err)
501+
deferconn.Close()
502+
503+
sshClient:=ssh.NewClient(conn,channels,requests)
504+
session,err:=sshClient.NewSession()
505+
require.NoError(t,err)
506+
defersession.Close()
507+
508+
command:="sh -c exit"
509+
ifruntime.GOOS=="windows" {
510+
command="cmd.exe /c exit"
511+
}
512+
err=session.Run(command)
513+
require.NoError(t,err)
514+
err=sshClient.Close()
515+
require.NoError(t,err)
516+
_=clientOutput.Close()
517+
518+
<-cmdDone
519+
})
520+
456521
t.Run("NetworkInfo",func(t*testing.T) {
457522
t.Parallel()
458523
client,workspace,agentToken:=setupWorkspaceForAgent(t)

0 commit comments

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