Skip to content

fix!: use names not IDs for agent SSH key seed #17258

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 4, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
fix: use names not IDs for agent SSH key seed
  • Loading branch information
spikecurtis committed Apr 4, 2025
commit c97ff5c859c494a91c279863487a775e15ddd377
29 changes: 24 additions & 5 deletions agent/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -1186,9 +1186,9 @@ func (a *agent) createOrUpdateNetwork(manifestOK, networkOK *checkpoint) func(co
network := a.network
a.closeMutex.Unlock()
if network == nil {
keySeed, err := WorkspaceKeySeed(manifest.WorkspaceID, manifest.AgentName)
keySeed, err := SSHKeySeed(manifest.OwnerName, manifest.WorkspaceName, manifest.AgentName)
if err != nil {
return xerrors.Errorf("generate seed from workspace id: %w", err)
return xerrors.Errorf("generate SSH key seed: %w", err)
}
// use the graceful context here, because creating the tailnet is not itself tied to the
// agent API.
Expand Down Expand Up @@ -2068,12 +2068,31 @@ func PrometheusMetricsHandler(prometheusRegistry *prometheus.Registry, logger sl
})
}

// WorkspaceKeySeed converts a WorkspaceID UUID and agent name to an int64 hash.
// SSHKeySeed converts an owner userName, workspaceName and agentName to an int64 hash.
// This uses the FNV-1a hash algorithm which provides decent distribution and collision
// resistance for string inputs.
func WorkspaceKeySeed(workspaceID uuid.UUID, agentName string) (int64, error) {
//
// Why owner username, workspace name, and agent name? These are the components that are used in hostnames for the
// workspace over SSH, and so we want the workspace to have a stable key with respect to these. We don't use the
// respective UUIDs. The workspace UUID would be different if you delete and recreate a workspace with the same name.
// The agent UUID is regenerated on each build. Since Coder's Tailnet networking is handling the authentication, we
// should not be showing users warnings about host SSH keys.
func SSHKeySeed(userName, workspaceName, agentName string) (int64, error) {
h := fnv.New64a()
_, err := h.Write(workspaceID[:])
_, err := h.Write([]byte(userName))
if err != nil {
return 42, err
}
// null separators between strings so that (dog, foodstuff) is distinct from (dogfood, stuff)
_, err = h.Write([]byte{0})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏻

if err != nil {
return 42, err
}
_, err = h.Write([]byte(workspaceName))
if err != nil {
return 42, err
}
_, err = h.Write([]byte{0})
if err != nil {
return 42, err
}
Expand Down
5 changes: 4 additions & 1 deletion cli/ssh_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -479,6 +479,9 @@ func TestSSH(t *testing.T) {
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
defer cancel()

user, err := client.User(ctx, codersdk.Me)
require.NoError(t, err)

inv, root := clitest.New(t, "ssh", "--stdio", workspace.Name)
clitest.SetupConfig(t, client, root)
inv.Stdin = clientOutput
Expand All @@ -490,7 +493,7 @@ func TestSSH(t *testing.T) {
assert.NoError(t, err)
})

keySeed, err := agent.WorkspaceKeySeed(workspace.ID, "dev")
keySeed, err := agent.SSHKeySeed(user.Username, workspace.Name, "dev")
assert.NoError(t, err)

signer, err := agentssh.CoderSigner(keySeed)
Expand Down
Loading