Private PKI Support

agent/agent.go

@@ -58,6 +58,16 @@ func PollAndSync(forceSync bool) {
 		return
 	}
 
+	// Private CA roots install before certificates deploy so that update
+	// commands (and the services they reload) can already trust chains
+	// issued by those CAs on the very first synchronization.
+	caStatuses := SynchronizePrivateCAs()
+	if len(caStatuses) > 0 {
+		if err := api.UpdatePrivateCaStatus(caStatuses); err != nil {
+			reportAgentError(fmt.Errorf("update private ca status: %w", err), "", "")
+		}
+	}
+
 	statuses := SynchronizeCertificates(configChanges, forceSync)
 	if len(statuses) > 0 {
 		if err := api.UpdateConfigStatus(statuses); err != nil {
@@ -132,6 +142,10 @@ func PollForConfiguration(forceSync bool) (configChanges map[string]ConfigChange
 		removeKeystoreConfig()
 	}
 
+	// Like the keystore config, private CA updates apply even when the agent
+	// is locked. The poll response list is authoritative on a full response.
+	applyPrivateCAUpdates(response.PrivateCAs)
+
 	if response.LockRequested && !isLocked {
 		if err := config.CreateLockFile(config.CurrentPath); err != nil {
 			return nil, err
@@ -149,7 +163,7 @@ func PollForConfiguration(forceSync bool) (configChanges map[string]ConfigChange
 		configChanges = applyLockedConfigUpdates(response.UpdatedCertificateConfigurations)
 	} else {
 		configChanges = detectChangedConfigs(config.CurrentConfig.CertificateConfigurations, response.UpdatedCertificateConfigurations)
-		preserveRetryableStatuses(config.CurrentConfig.CertificateConfigurations, response.UpdatedCertificateConfigurations)
+		preserveUnresolvedStatuses(config.CurrentConfig.CertificateConfigurations, response.UpdatedCertificateConfigurations)
 		config.CurrentConfig.CertificateConfigurations = response.UpdatedCertificateConfigurations
 	}
 
@@ -203,7 +217,7 @@ func applyLockedConfigUpdates(updated []config.CertificateConfiguration) map[str
 	return changedIDs
 }
 
-func preserveRetryableStatuses(previousConfigurations, incomingConfigurations []config.CertificateConfiguration) {
+func preserveUnresolvedStatuses(previousConfigurations, incomingConfigurations []config.CertificateConfiguration) {
 	previousByID := make(map[string]config.CertificateConfiguration, len(previousConfigurations))
 	for _, cfg := range previousConfigurations {
 		if cfg.Id != "" {
@@ -216,7 +230,7 @@ func preserveRetryableStatuses(previousConfigurations, incomingConfigurations []
 		if !ok {
 			continue
 		}
-		if isRetryableStatus(prev.LastStatus) {
+		if isUnresolvedStatus(prev.LastStatus) {
 			incomingConfigurations[i].LastStatus = prev.LastStatus
 		}
 	}

agent/agent_test.go

@@ -1,6 +1,16 @@
 package agent
 
 import (
+	"crypto/ecdsa"
+	"crypto/elliptic"
+	"crypto/rand"
+	"crypto/sha1"
+	"crypto/x509"
+	"crypto/x509/pkix"
+	"encoding/hex"
+	"encoding/pem"
+	"math/big"
+	"os"
 	"path/filepath"
 	"slices"
 	"testing"
@@ -251,12 +261,12 @@ func TestDetectChangedConfigs_EmptyIncomingListReturnsEmptyMap(t *testing.T) {
 	}
 }
 
-func TestPreserveRetryableStatuses_PreservesUpdateCommandFailure(t *testing.T) {
+func TestPreserveUnresolvedStatuses_PreservesUpdateCommandFailure(t *testing.T) {
 	incoming := []config.CertificateConfiguration{
 		{Id: "a", LastStatus: statusSynced},
 	}
 
-	preserveRetryableStatuses(
+	preserveUnresolvedStatuses(
 		[]config.CertificateConfiguration{
 			{Id: "a", LastStatus: statusErrorUpdateCmd},
 		},
@@ -268,12 +278,12 @@ func TestPreserveRetryableStatuses_PreservesUpdateCommandFailure(t *testing.T) {
 	}
 }
 
-func TestPreserveRetryableStatuses_DoesNotPreserveSynced(t *testing.T) {
+func TestPreserveUnresolvedStatuses_DoesNotPreserveSynced(t *testing.T) {
 	incoming := []config.CertificateConfiguration{
 		{Id: "a", LastStatus: ""},
 	}
 
-	preserveRetryableStatuses(
+	preserveUnresolvedStatuses(
 		[]config.CertificateConfiguration{
 			{Id: "a", LastStatus: statusSynced},
 		},
@@ -285,30 +295,43 @@ func TestPreserveRetryableStatuses_DoesNotPreserveSynced(t *testing.T) {
 	}
 }
 
-func TestIsRetryableStatus(t *testing.T) {
-	retryableStatuses := []string{
+func TestIsUnresolvedStatus(t *testing.T) {
+	unresolved := []string{
 		statusErrorUpdateCmd,
 		statusWaitingWindow,
-		statusPendingSync,
 		statusErrorGetCert,
 		statusErrorWriteCert,
 		statusErrorGeneral,
 	}
 
-	for _, status := range retryableStatuses {
-		if !isRetryableStatus(status) {
-			t.Fatalf("isRetryableStatus(%q) = false, want true", status)
+	for _, status := range unresolved {
+		if !isUnresolvedStatus(status) {
+			t.Fatalf("isUnresolvedStatus(%q) = false, want true", status)
 		}
 	}
 
 	for _, status := range []string{"", statusSynced} {
-		if isRetryableStatus(status) {
-			t.Fatalf("isRetryableStatus(%q) = true, want false", status)
+		if isUnresolvedStatus(status) {
+			t.Fatalf("isUnresolvedStatus(%q) = true, want false", status)
 		}
 	}
 }
 
-func TestSynchronizeCertificates_ProcessesUpdateCommandFailureDuringNormalPoll(t *testing.T) {
+func TestIsPendingWorkStatus(t *testing.T) {
+	if !isPendingWorkStatus(statusWaitingWindow) {
+		t.Fatalf("isPendingWorkStatus(%q) = false, want true", statusWaitingWindow)
+	}
+
+	// Error statuses must NOT re-enter synchronization on an unchanged
+	// config — a failure sticks until the config is saved again on the app.
+	for _, status := range []string{"", statusSynced, statusErrorUpdateCmd, statusErrorGetCert, statusErrorWriteCert, statusErrorGeneral} {
+		if isPendingWorkStatus(status) {
+			t.Fatalf("isPendingWorkStatus(%q) = true, want false", status)
+		}
+	}
+}
+
+func TestSynchronizeCertificates_DoesNotRetryFailureDuringNormalPoll(t *testing.T) {
 	previousConfig := config.CurrentConfig
 	previousPath := config.CurrentPath
 	t.Cleanup(func() {
@@ -329,15 +352,15 @@ func TestSynchronizeCertificates_ProcessesUpdateCommandFailureDuringNormalPoll(t
 
 	statuses := SynchronizeCertificates(nil, false)
 
-	if len(statuses) != 1 {
-		t.Fatalf("len(statuses) = %d, want 1", len(statuses))
+	if len(statuses) != 0 {
+		t.Fatalf("len(statuses) = %d, want 0 (failed config must not retry without a change)", len(statuses))
 	}
-	if statuses[0].Status != statusErrorGeneral {
-		t.Fatalf("Status = %q, want %q", statuses[0].Status, statusErrorGeneral)
+	if got := config.CurrentConfig.CertificateConfigurations[0].LastStatus; got != statusErrorUpdateCmd {
+		t.Fatalf("LastStatus = %q, want %q preserved", got, statusErrorUpdateCmd)
 	}
 }
 
-func TestSynchronizeCertificates_ReportsRepeatedErrorStatus(t *testing.T) {
+func TestSynchronizeCertificates_ReportsRepeatedErrorStatusOnRetry(t *testing.T) {
 	previousConfig := config.CurrentConfig
 	previousPath := config.CurrentPath
 	t.Cleanup(func() {
@@ -350,16 +373,17 @@ func TestSynchronizeCertificates_ReportsRepeatedErrorStatus(t *testing.T) {
 		CertificateConfigurations: []config.CertificateConfiguration{
 			{
 				// Missing destination paths fail with ERROR_GENERAL, the same
-				// status the config is already in — the retry must still be
-				// reported so the backend sees the fresh message.
+				// status the config is already in — the retry (triggered by a
+				// config change) must still be reported so the backend sees
+				// the fresh message.
 				Id:            "a",
 				CertificateId: "cert-a",
 				LastStatus:    statusErrorGeneral,
 			},
 		},
 	}
 
-	statuses := SynchronizeCertificates(nil, false)
+	statuses := SynchronizeCertificates(map[string]ConfigChange{"a": {Changed: true}}, false)
 
 	if len(statuses) != 1 {
 		t.Fatalf("len(statuses) = %d, want 1", len(statuses))
@@ -369,14 +393,80 @@ func TestSynchronizeCertificates_ReportsRepeatedErrorStatus(t *testing.T) {
 	}
 }
 
+// writeSelfSignedCertPEM writes a freshly generated self-signed certificate to
+// path and returns its SHA1 fingerprint in hex.
+func writeSelfSignedCertPEM(t *testing.T, path string) string {
+	t.Helper()
+
+	key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+	if err != nil {
+		t.Fatalf("generate key: %v", err)
+	}
+	template := x509.Certificate{
+		SerialNumber: big.NewInt(1),
+		Subject:      pkix.Name{CommonName: "certkit-test"},
+		NotBefore:    time.Now().Add(-time.Hour),
+		NotAfter:     time.Now().Add(time.Hour),
+	}
+	der, err := x509.CreateCertificate(rand.Reader, &template, &template, &key.PublicKey, key)
+	if err != nil {
+		t.Fatalf("create certificate: %v", err)
+	}
+	pemBytes := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der})
+	if err := os.WriteFile(path, pemBytes, 0o600); err != nil {
+		t.Fatalf("write cert: %v", err)
+	}
+	sum := sha1.Sum(der)
+	return hex.EncodeToString(sum[:])
+}
+
+func TestSynchronizeCertificates_ForceSyncPreservesFailureWhenNothingToDo(t *testing.T) {
+	previousConfig := config.CurrentConfig
+	previousPath := config.CurrentPath
+	t.Cleanup(func() {
+		config.CurrentConfig = previousConfig
+		config.CurrentPath = previousPath
+	})
+
+	// The on-disk certificate matches the expected SHA1, so a force sync (as
+	// happens on every agent restart) has nothing to do. The standing update
+	// command failure must persist — no erroneous SYNCED may be pushed.
+	certPath := filepath.Join(t.TempDir(), "cert.pem")
+	certSha1 := writeSelfSignedCertPEM(t, certPath)
+
+	config.CurrentPath = filepath.Join(t.TempDir(), "config.json")
+	config.CurrentConfig = config.Config{
+		CertificateConfigurations: []config.CertificateConfiguration{
+			{
+				Id:                    "a",
+				CertificateId:         "cert-a",
+				PemDestination:        certPath,
+				AllInOne:              true,
+				LatestCertificateSha1: certSha1,
+				UpdateCmd:             "echo hi",
+				LastStatus:            statusErrorUpdateCmd,
+			},
+		},
+	}
+
+	statuses := SynchronizeCertificates(nil, true)
+
+	if len(statuses) != 0 {
+		t.Fatalf("len(statuses) = %d, want 0 (got %+v)", len(statuses), statuses)
+	}
+	if got := config.CurrentConfig.CertificateConfigurations[0].LastStatus; got != statusErrorUpdateCmd {
+		t.Fatalf("LastStatus = %q, want %q preserved", got, statusErrorUpdateCmd)
+	}
+}
+
 func TestIsErrorStatus(t *testing.T) {
 	for _, status := range []string{statusErrorUpdateCmd, statusErrorGetCert, statusErrorWriteCert, statusErrorGeneral} {
 		if !isErrorStatus(status) {
 			t.Fatalf("isErrorStatus(%q) = false, want true", status)
 		}
 	}
 
-	for _, status := range []string{"", statusSynced, statusPendingSync, statusWaitingWindow} {
+	for _, status := range []string{"", statusSynced, statusWaitingWindow} {
 		if isErrorStatus(status) {
 			t.Fatalf("isErrorStatus(%q) = true, want false", status)
 		}

agent/synchronize.go

@@ -20,7 +20,6 @@ import (
 
 const (
 	statusSynced         = "SYNCED"
-	statusPendingSync    = "PENDING_SYNC"
 	statusWaitingWindow  = "WAITING_FOR_WINDOW"
 	statusErrorUpdateCmd = "ERROR_UPDATE_CMD"
 	statusErrorGetCert   = "ERROR_GET_CERTS"
@@ -36,7 +35,7 @@ func SynchronizeCertificates(configChanges map[string]ConfigChange, forceSync bo
 		cfg := &config.CurrentConfig.CertificateConfigurations[i]
 		change := configChanges[cfg.Id]
 
-		if !change.Changed && !forceSync && !isRetryableStatus(cfg.LastStatus) {
+		if !change.Changed && !forceSync && !isPendingWorkStatus(cfg.LastStatus) {
 			continue
 		}
 		if change.Changed {
@@ -66,18 +65,22 @@ func SynchronizeCertificates(configChanges map[string]ConfigChange, forceSync bo
 	return statuses
 }
 
-func isRetryableStatus(status string) bool {
-	switch status {
-	case statusErrorUpdateCmd,
-		statusWaitingWindow,
-		statusPendingSync,
-		statusErrorGetCert,
-		statusErrorWriteCert,
-		statusErrorGeneral:
-		return true
-	default:
-		return false
-	}
+// isUnresolvedStatus reports whether a locally tracked status must survive
+// the poll response replacing the configuration list. Error statuses persist
+// so a failure is never forgotten (or masked by a later no-op SYNCED);
+// WAITING_FOR_WINDOW persists because the work it describes hasn't happened
+// yet.
+func isUnresolvedStatus(status string) bool {
+	return isErrorStatus(status) || status == statusWaitingWindow
+}
+
+// isPendingWorkStatus reports whether a status re-enters synchronization even
+// though the configuration hasn't changed. Error statuses are deliberately
+// excluded: a failure sticks until the configuration is saved again on the
+// server or the certificate rotates, rather than re-running a failing update
+// command on every poll cycle.
+func isPendingWorkStatus(status string) bool {
+	return status == statusWaitingWindow
 }
 
 func isErrorStatus(status string) bool {
@@ -132,11 +135,7 @@ func synchronizeCertificate(cfg config.CertificateConfiguration, change ConfigCh
 		return synchronizeWindowsCertStoreCertificate(cfg, change)
 	}
 
-	retryUpdateOnly := cfg.LastStatus == statusErrorUpdateCmd || cfg.LastStatus == statusWaitingWindow
-	retryFull := cfg.LastStatus == statusPendingSync ||
-		cfg.LastStatus == statusErrorGetCert ||
-		cfg.LastStatus == statusErrorWriteCert ||
-		cfg.LastStatus == statusErrorGeneral
+	resumeApply := cfg.LastStatus == statusWaitingWindow
 
 	isPfx := cfg.IsPfx
 	isJks := cfg.IsJKS()
@@ -159,8 +158,8 @@ func synchronizeCertificate(cfg config.CertificateConfiguration, change ConfigCh
 		return status
 	}
 
-	shouldFetch := needsFetch || change.FormatChanged || retryFull
-	needsApply := needsFetch || change.Changed || retryUpdateOnly || retryFull
+	shouldFetch := needsFetch || change.FormatChanged
+	needsApply := needsFetch || change.Changed || resumeApply
 
 	if shouldFetch {
 		if isJks {
@@ -246,9 +245,7 @@ func synchronizeCertificate(cfg config.CertificateConfiguration, change ConfigCh
 			if !needsFetch && change.Changed {
 				log.Print("Running update cmd due to configuration change...")
 			}
-			if (retryUpdateOnly || retryFull) && cfg.LastStatus != statusWaitingWindow {
-				log.Print("Retrying update command due to previous failure...")
-			} else if (retryUpdateOnly || retryFull) && cfg.LastStatus == statusWaitingWindow {
+			if resumeApply {
 				log.Print("Running update command inside deploy window...")
 			}
 			if commandOutput, err := runUpdateCommand(cfg, getUpdateVariables(cfg.Id)); err != nil {
@@ -260,6 +257,13 @@ func synchronizeCertificate(cfg config.CertificateConfiguration, change ConfigCh
 			}
 		}
 	} else {
+		// Nothing was done, so a standing failure must not be overwritten
+		// with SYNCED — it sticks until the configuration is saved again on
+		// the server or the certificate rotates.
+		if isErrorStatus(cfg.LastStatus) {
+			log.Printf("Config %s unchanged since last failure (%s); leaving status as-is (config=%s).", cfg.Id, cfg.LastStatus, cfg.Id)
+			return api.AgentConfigStatusUpdate{}
+		}
 		log.Printf("Synchronization checks complete.  No action taken, everything up to date (config=%s).", cfg.Id)
 	}