diff --git a/pkg/cmd/enablessh/enablessh.go b/pkg/cmd/enablessh/enablessh.go index 5ba47a6e..eade1a3d 100644 --- a/pkg/cmd/enablessh/enablessh.go +++ b/pkg/cmd/enablessh/enablessh.go @@ -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.")) @@ -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 { @@ -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) } diff --git a/pkg/cmd/enablessh/enablessh_test.go b/pkg/cmd/enablessh/enablessh_test.go index a4b6f7da..69cc6448 100644 --- a/pkg/cmd/enablessh/enablessh_test.go +++ b/pkg/cmd/enablessh/enablessh_test.go @@ -589,7 +589,10 @@ func Test_installCertAuthority(t *testing.T) { 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) } @@ -605,6 +608,12 @@ func Test_installCertAuthority(t *testing.T) { 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) { @@ -647,12 +656,15 @@ func Test_resolveLegacySSHPortNonInteractiveUsesProvidedPort(t *testing.T) { } 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 }, } deps := startFakeServer(t, svc) @@ -676,9 +688,8 @@ func Test_enableSSHNonInteractiveUsesProvidedInputs(t *testing.T) { return targetUser, nil } reg := ®ister.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{ @@ -695,9 +706,26 @@ func Test_enableSSHNonInteractiveUsesProvidedInputs(t *testing.T) { 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) { diff --git a/pkg/cmd/register/device_registration_store.go b/pkg/cmd/register/device_registration_store.go index 31f6ec75..eec64e0a 100644 --- a/pkg/cmd/register/device_registration_store.go +++ b/pkg/cmd/register/device_registration_store.go @@ -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. diff --git a/pkg/cmd/register/register.go b/pkg/cmd/register/register.go index 98c6e57c..7bfa0b99 100644 --- a/pkg/cmd/register/register.go +++ b/pkg/cmd/register/register.go @@ -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("")