Skip to content

Codex/enable ssh refresh ca #520

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.

Sign up for GitHub

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

Draft
patelspratik wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Codex/enable ssh refresh ca #520

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5c4cae5
fix(ssh): fetch current CA when enabling SSH
patelspratik Oct 24, 2026
91622ff
fix(ssh): replace stale CA trust when enabling SSH
patelspratik Oct 24, 2026
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
46 changes: 23 additions & 23 deletions pkg/cmd/enablessh/enablessh.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,19 +121,15 @@ func enableSSH(
t.Vprintf(" Linux user: %s\n", linuxUsername)
t.Vprint("")

caPublicKey := reg.CertificateAuthority
if caPublicKey == "" {
node, err := fetchRegisteredNode(ctx, deps, s, reg)
if err != nil {
return fmt.Errorf("enable SSH failed: %w", err)
}
if node.GetLabels()[sshcert.LabelKeySSHProvider] != sshcert.SSHProviderCertAuth {
return legacyEnableSSH(ctx, t, deps, s, reg, node, linuxUsername, opts)
}
caPublicKey = node.GetCertificateAuthority()
node, err := fetchRegisteredNode(ctx, deps, s, reg)
if err != nil {
return fmt.Errorf("enable SSH failed: %w", err)
}
if node.GetLabels()[sshcert.LabelKeySSHProvider] != sshcert.SSHProviderCertAuth {
return legacyEnableSSH(ctx, t, deps, s, reg, node, linuxUsername, opts)
}

if err := installCertAuthority(linuxUser, caPublicKey, reg.ExternalNodeID, linuxUsername); err != nil {
if err := installCertAuthority(linuxUser, node.GetCertificateAuthority(), reg.ExternalNodeID, linuxUsername); err != nil {
return fmt.Errorf("enable SSH failed: %w", err)
}
t.Vprint(t.Green(" Certificate authority written to authorized_keys."))
Expand Down Expand Up @@ -321,12 +317,13 @@ func resolveLegacySSHPort(
}

func installCertAuthority(osUser *user.User, caPublicKey, nodeID, linuxUser string) error {
caPublicKey = strings.TrimSpace(caPublicKey)
if caPublicKey == "" {
return fmt.Errorf("certificate authority public key is required")
}

principal := fmt.Sprintf("brev:v1:vm:%s:login:%s", nodeID, linuxUser)
entry := fmt.Sprintf("cert-authority,principals=\"%s\" %s", principal, strings.TrimSpace(caPublicKey))
prefix := fmt.Sprintf("cert-authority,principals=%q ", sshcert.CertAuthorityPrincipal(nodeID, linuxUser))
entry := prefix + caPublicKey

sshDir := filepath.Join(osUser.HomeDir, ".ssh")
if err := os.MkdirAll(sshDir, 0o700); err != nil {
Expand All @@ -340,20 +337,23 @@ func installCertAuthority(osUser *user.User, caPublicKey, nodeID, linuxUser stri
return fmt.Errorf("reading authorized_keys: %w", err)
}

// skip if the entry already exists.
for line := range strings.SplitSeq(string(existing), "\n") {
if strings.TrimSpace(line) == entry {
return nil
var content strings.Builder
for line := range strings.SplitAfterSeq(string(existing), "\n") {
if strings.HasPrefix(strings.TrimSpace(line), prefix) {
continue
}
content.WriteString(line)
}

content := string(existing)
if content != "" && !strings.HasSuffix(content, "\n") {
content += "\n"
if content.Len() > 0 && !strings.HasSuffix(content.String(), "\n") {
content.WriteByte('\n')
}
content.WriteString(entry + "\n")
result := content.String()
if string(existing) == result {
return nil
}
content += entry + "\n"

if err := os.WriteFile(authKeysPath, []byte(content), 0o600); err != nil {
if err := os.WriteFile(authKeysPath, []byte(result), 0o600); err != nil {
return fmt.Errorf("writing authorized_keys: %w", err)
}

Expand Down
46 changes: 37 additions & 9 deletions pkg/cmd/enablessh/enablessh_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -589,7 +589,10 @@
if err := os.MkdirAll(sshDir, 0o700); err != nil {
t.Fatal(err)
}
original := "ssh-rsa EXISTING user@host\n"
original := "ssh-rsa EXISTING user@host\n" +
`cert-authority,principals="brev:v1:vm:other_node:login:ubuntu" ssh-ed25519 OTHER otherCA` + "\n" +
`cert-authority,principals="brev:v1:vm:unode_abc:login:ubuntu" ssh-ed25519 OLD oldCA` + "\n" +
`cert-authority,principals="brev:v1:vm:unode_abc:login:ubuntu" ` + caKey + "\n"
if err := os.WriteFile(filepath.Join(sshDir, "authorized_keys"), []byte(original), 0o600); err != nil {
t.Fatal(err)
}
Expand All @@ -605,6 +608,12 @@
if !strings.Contains(result, "cert-authority") {
t.Errorf("cert-authority line not written:\n%s", result)
}
if strings.Contains(result, "oldCA") || strings.Count(result, caKey) != 1 {
t.Errorf("expected only the current CA for this principal:\n%s", result)
}
if !strings.Contains(result, "otherCA") {
t.Errorf("another principal's CA was removed:\n%s", result)
}
})

t.Run("EmptyKeyErrors", func(t *testing.T) {
Expand Down Expand Up @@ -647,12 +656,15 @@
}

func Test_enableSSHNonInteractiveUsesProvidedInputs(t *testing.T) {
const caKey = "ssh-ed25519 AAAAC3Nz dummyCA"
caKey := "ssh-ed25519 AAAAC3Nz dummyCA"
svc := &fakeNodeService{
getNodeFn: func(_ *nodev1.GetNodeRequest) (*nodev1.GetNodeResponse, error) {
return &nodev1.GetNodeResponse{ExternalNode: &nodev1.ExternalNode{Ports: []*nodev1.Port{
{PortId: "port_ssh", PortNumber: 11640, ServerPort: 22},
}}}, nil
return &nodev1.GetNodeResponse{ExternalNode: &nodev1.ExternalNode{
Labels: map[string]string{"sshprovider": "certauth"},
CertificateAuthority: caKey,
Ports: []*nodev1.Port{
{PortId: "port_ssh", PortNumber: 11640, ServerPort: 22},
}}}, nil

Check failure on line 667 in pkg/cmd/enablessh/enablessh_test.go

View workflow job for this annotation

GitHub Actions / ci (ubuntu-22.04) 

File is not properly formatted (gofumpt)
},
}
deps := startFakeServer(t, svc)
Expand All @@ -676,9 +688,8 @@
return targetUser, nil
}
reg := &register.DeviceRegistration{
ExternalNodeID: "unode_abc",
OrgID: "org_1",
CertificateAuthority: caKey,
ExternalNodeID: "unode_abc",
OrgID: "org_1",
}

err := enableSSH(context.Background(), terminal.New(), deps, &mockEnableSSHStore{}, reg, enableSSHOpts{
Expand All @@ -695,9 +706,26 @@
t.Fatalf("OpenPort called %d times, want 0", svc.openCalls)
}
wantPrincipal := `principals="brev:v1:vm:unode_abc:login:ubuntu"`
if authorizedKeys := readAuthorizedKeys(t, targetUser); !strings.Contains(authorizedKeys, wantPrincipal) {
authorizedKeys := readAuthorizedKeys(t, targetUser)
if !strings.Contains(authorizedKeys, wantPrincipal) {
t.Fatalf("authorized_keys missing %s:\n%s", wantPrincipal, authorizedKeys)
}
if !strings.Contains(authorizedKeys, caKey) {
t.Fatalf("authorized_keys must contain the current CA:\n%s", authorizedKeys)
}

oldCA := caKey
caKey = "ssh-ed25519 NEW rotatedCA"
if err := enableSSH(context.Background(), terminal.New(), deps, &mockEnableSSHStore{}, reg, enableSSHOpts{
linuxUsername: "ubuntu",
sshPort: 22,
}); err != nil {
t.Fatalf("enableSSH after CA rotation: %v", err)
}
authorizedKeys = readAuthorizedKeys(t, targetUser)
if strings.Contains(authorizedKeys, oldCA) || strings.Count(authorizedKeys, caKey) != 1 {
t.Fatalf("enableSSH must replace the old CA with the current CA:\n%s", authorizedKeys)
}
}

func Test_enableSSH_LegacyNodeFallsBackToKeys(t *testing.T) {
Expand Down
17 changes: 8 additions & 9 deletions pkg/cmd/register/device_registration_store.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,14 @@ const (
// DeviceRegistration is the persistent identity file for a registered device.
// Fields align with the AddNodeResponse from dev-plane.
type DeviceRegistration struct {
ExternalNodeID string `json:"external_node_id"`
DisplayName string `json:"display_name"`
OrgID string `json:"org_id"`
OrgName string `json:"org_name"`
DeviceID string `json:"device_id"`
RegisteredAt string `json:"registered_at"`
HardwareProfile HardwareProfile `json:"hardware_profile"`
Status string `json:"status,omitempty"`
CertificateAuthority string `json:"certificate_authority,omitempty"`
ExternalNodeID string `json:"external_node_id"`
DisplayName string `json:"display_name"`
OrgID string `json:"org_id"`
OrgName string `json:"org_name"`
DeviceID string `json:"device_id"`
RegisteredAt string `json:"registered_at"`
HardwareProfile HardwareProfile `json:"hardware_profile"`
Status string `json:"status,omitempty"`
}

// RegistrationStore defines the contract for persisting device registration data.
Expand Down
17 changes: 8 additions & 9 deletions pkg/cmd/register/register.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -353,15 +353,14 @@ func runRegisterSteps(ctx context.Context, t *terminal.Terminal, s RegisterStore

node := addResp.Msg.GetExternalNode()
reg := &DeviceRegistration{
ExternalNodeID: node.GetExternalNodeId(),
DisplayName: name,
OrgID: org.ID,
OrgName: org.Name,
DeviceID: deviceID,
RegisteredAt: time.Now().UTC().Format(time.RFC3339),
HardwareProfile: *hwProfile,
Status: RegistrationStatusRegistered,
CertificateAuthority: node.GetCertificateAuthority(),
ExternalNodeID: node.GetExternalNodeId(),
DisplayName: name,
OrgID: org.ID,
OrgName: org.Name,
DeviceID: deviceID,
RegisteredAt: time.Now().UTC().Format(time.RFC3339),
HardwareProfile: *hwProfile,
Status: RegistrationStatusRegistered,
}

t.Vprint("")
Expand Down
Loading