From 11f508f19c9a05d94acc92c276a8574a5cbe9c75 Mon Sep 17 00:00:00 2001 From: ehila Date: Wed, 9 Sep 2026 02:09:48 -0400 Subject: [PATCH 1/2] feat: Add PKI certificate inventory Add a deterministic role-based inventory for certificate chains and use it for existing regeneration and rotation inspection paths. Co-Authored-By: GPT-5 Signed-off-by: ehila --- pkg/cmd/init.go | 13 +-- .../cryptomaterial/certchains/certchains.go | 12 +- .../cryptomaterial/certchains/inventory.go | 85 ++++++++++++++ .../certchains/inventory_test.go | 104 ++++++++++++++++++ 4 files changed, 201 insertions(+), 13 deletions(-) create mode 100644 pkg/util/cryptomaterial/certchains/inventory.go create mode 100644 pkg/util/cryptomaterial/certchains/inventory_test.go diff --git a/pkg/cmd/init.go b/pkg/cmd/init.go index 8c8e9ec6ff..05d8e2a067 100644 --- a/pkg/cmd/init.go +++ b/pkg/cmd/init.go @@ -16,7 +16,6 @@ limitations under the License. package cmd import ( - "crypto/x509" "fmt" "net" "net/url" @@ -590,7 +589,9 @@ func initKubeconfigs( // bundle that need to be regenerated func certsToRegenerate(cs *certchains.CertificateChains) ([][]string, error) { regenCerts := [][]string{} - err := cs.WalkChains(nil, func(certPath []string, c x509.Certificate) error { + for _, entry := range cs.Inventory() { + certPath := entry.Path + c := entry.Certificate if now := time.Now(); now.Before(c.NotBefore) || now.After(c.NotAfter) { regenCerts = append(regenCerts, certPath) } @@ -605,18 +606,16 @@ func certsToRegenerate(cs *certchains.CertificateChains) ([][]string, error) { if timeLeft < until { regenCerts = append(regenCerts, certPath) } - return nil + continue } // long lived certs if timeLeft < 18*month { regenCerts = append(regenCerts, certPath) } + } - return nil - }) - - return regenCerts, err + return regenCerts, nil } func cleanupStaleKubeconfigs(cfg *config.Config, path string) error { diff --git a/pkg/util/cryptomaterial/certchains/certchains.go b/pkg/util/cryptomaterial/certchains/certchains.go index 912c251e05..ff79de2d9a 100644 --- a/pkg/util/cryptomaterial/certchains/certchains.go +++ b/pkg/util/cryptomaterial/certchains/certchains.go @@ -110,7 +110,9 @@ func WhenToRotateAtEarliest(cs *CertificateChains) ([]string, time.Time, error) rotationDate time.Time ) - err := cs.WalkChains(nil, func(currentPath []string, c x509.Certificate) error { + for _, entry := range cs.Inventory() { + currentPath := entry.Path + c := entry.Certificate const month = 30 * time.Hour * 24 rotateAt := c.NotAfter.Add(-4 * month) @@ -122,16 +124,14 @@ func WhenToRotateAtEarliest(cs *CertificateChains) ([]string, time.Time, error) if rotationDate.IsZero() { rotationDate = rotateAt certPath = currentPath - return nil + continue } if rotateAt.Before(rotationDate) { rotationDate = rotateAt certPath = currentPath } + } - return nil - }) - - return certPath, rotationDate, err + return certPath, rotationDate, nil } diff --git a/pkg/util/cryptomaterial/certchains/inventory.go b/pkg/util/cryptomaterial/certchains/inventory.go new file mode 100644 index 0000000000..57a2d56929 --- /dev/null +++ b/pkg/util/cryptomaterial/certchains/inventory.go @@ -0,0 +1,85 @@ +package certchains + +import "crypto/x509" + +// CertificateRole identifies how a certificate is used by MicroShift. +type CertificateRole string + +const ( + CertificateRoleUnknown CertificateRole = "unknown" + CertificateRoleCA CertificateRole = "ca" + CertificateRoleClient CertificateRole = "client" + CertificateRoleServing CertificateRole = "serving" + CertificateRolePeer CertificateRole = "peer" +) + +// CertificateInventoryEntry describes one certificate managed by the chain. +// Path can be passed to CertificateChains methods such as Regenerate. +type CertificateInventoryEntry struct { + Path []string + Role CertificateRole + Certificate x509.Certificate +} + +// CertificateInventory is a snapshot of certificates managed by a chain. +type CertificateInventory []CertificateInventoryEntry + +// ByRole returns the inventory entries matching role. +func (i CertificateInventory) ByRole(role CertificateRole) CertificateInventory { + entries := make(CertificateInventory, 0) + for _, entry := range i { + if entry.Role == role { + entries = append(entries, entry) + } + } + return entries +} + +// Inventory returns a deterministic snapshot of all certificates managed by +// the chain. Signers are listed before their sub-CAs and leaf certificates. +func (cs *CertificateChains) Inventory() CertificateInventory { + entries := make(CertificateInventory, 0) + for _, signerName := range cs.GetSignerNames() { + signer := cs.GetSigner(signerName) + entries = append(entries, signer.inventory([]string{signerName})...) + } + return entries +} + +func (s *CertificateSigner) inventory(path []string) CertificateInventory { + entries := CertificateInventory{{ + Path: append([]string(nil), path...), + Role: CertificateRoleCA, + Certificate: *s.signerConfig.Config.Certs[0], + }} + + for _, subCAName := range s.GetSubCANames() { + subCAPath := append(append([]string(nil), path...), subCAName) + entries = append(entries, s.GetSubCA(subCAName).inventory(subCAPath)...) + } + + for _, certName := range s.GetCertNames() { + cert := s.signedCertificates[certName] + certPath := append(append([]string(nil), path...), certName) + entries = append(entries, CertificateInventoryEntry{ + Path: certPath, + Role: certificateRole(cert.CSRInfo), + Certificate: *cert.tlsConfig.Certs[0], + }) + } + + return entries +} + +func certificateRole(info CSRInfo) CertificateRole { + switch info.(type) { + case *ClientCertificateSigningRequestInfo: + return CertificateRoleClient + case *ServingCertificateSigningRequestInfo: + return CertificateRoleServing + case *PeerCertificateSigningRequestInfo: + return CertificateRolePeer + default: + return CertificateRoleUnknown + } +} diff --git a/pkg/util/cryptomaterial/certchains/inventory_test.go b/pkg/util/cryptomaterial/certchains/inventory_test.go new file mode 100644 index 0000000000..839b9186db --- /dev/null +++ b/pkg/util/cryptomaterial/certchains/inventory_test.go @@ -0,0 +1,104 @@ +package certchains + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestCertificateChains_Inventory(t *testing.T) { + chains := testChains(t, t.TempDir()) + + type expectedEntry struct { + path string + role CertificateRole + } + expected := []expectedEntry{ + {"test-signer1", CertificateRoleCA}, + {"test-signer1/test-signer1-subca", CertificateRoleCA}, + {"test-signer1/test-signer1-subca/test-signer1-subca-too", CertificateRoleCA}, + {"test-signer1/test-signer1-subca/test-signer1-subca-too/test-signer1-subca-too-too", CertificateRoleCA}, + {"test-signer1/test-signer1-subca/test-signer1-subca-too/test-signer1-subca-too-too/subca-too-too-test-client2", CertificateRoleClient}, + {"test-signer1/test-signer1-subca/test-signer1-subca-too/test-signer1-subca-too-too2", CertificateRoleCA}, + {"test-signer1/test-signer1-subca/test-signer1-subca-too/subca-too-test-client1", CertificateRoleClient}, + {"test-signer1/test-signer1-subca/test-signer1-subca-server1", CertificateRoleServing}, + {"test-signer1/test-client1", CertificateRoleClient}, + {"test-signer1/test-client2", CertificateRoleClient}, + {"test-signer1/test-signer1-server1", CertificateRoleServing}, + {"test-signer2", CertificateRoleCA}, + {"test-signer2/test-signer2-server1", CertificateRoleServing}, + {"test-signer3", CertificateRoleCA}, + {"test-signer3/test-signer3-subca1", CertificateRoleCA}, + {"test-signer3/test-signer3-subca1/test-client1", CertificateRoleClient}, + {"test-signer3/test-peer1", CertificateRolePeer}, + {"test-signer3/test-signer3-server1", CertificateRoleServing}, + } + + inventory := chains.Inventory() + require.Len(t, inventory, len(expected)) + for index, want := range expected { + entry := inventory[index] + require.Equal(t, want.path, strings.Join(entry.Path, "/")) + require.Equal(t, want.role, entry.Role) + require.False(t, entry.Certificate.NotAfter.IsZero()) + + if entry.Role == CertificateRoleCA { + require.NotNil(t, chains.GetSigner(entry.Path...)) + continue + } + _, _, err := chains.GetCertKey(entry.Path...) + require.NoError(t, err) + } +} + +func TestCertificateInventory_ByRole(t *testing.T) { + inventory := testChains(t, t.TempDir()).Inventory() + + tests := []struct { + name string + role CertificateRole + paths []string + }{ + { + name: "CAs", + role: CertificateRoleCA, + paths: []string{ + "test-signer1", + "test-signer1/test-signer1-subca", + "test-signer1/test-signer1-subca/test-signer1-subca-too", + "test-signer1/test-signer1-subca/test-signer1-subca-too/test-signer1-subca-too-too", + "test-signer1/test-signer1-subca/test-signer1-subca-too/test-signer1-subca-too-too2", + "test-signer2", + "test-signer3", + "test-signer3/test-signer3-subca1", + }, + }, + { + name: "serving certificates", + role: CertificateRoleServing, + paths: []string{ + "test-signer1/test-signer1-subca/test-signer1-subca-server1", + "test-signer1/test-signer1-server1", + "test-signer2/test-signer2-server1", + "test-signer3/test-signer3-server1", + }, + }, + { + name: "unknown role", + role: CertificateRoleUnknown, + paths: []string{}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + entries := inventory.ByRole(tt.role) + paths := make([]string, 0, len(entries)) + for _, entry := range entries { + paths = append(paths, strings.Join(entry.Path, "/")) + } + require.Equal(t, tt.paths, paths) + }) + } +} From 8e185c627b2bdf845b5ff6530756728e86e6d405 Mon Sep 17 00:00:00 2001 From: ehila Date: Wed, 9 Sep 2026 08:32:19 -0400 Subject: [PATCH 2/2] upkeep: resolve golint errors Signed-off-by: ehila --- pkg/cmd/init.go | 12 ++++-------- pkg/cmd/init_test.go | 13 ++++--------- pkg/util/cryptomaterial/certchains/inventory.go | 10 ++++++---- 3 files changed, 14 insertions(+), 21 deletions(-) diff --git a/pkg/cmd/init.go b/pkg/cmd/init.go index 05d8e2a067..e692dd1cbe 100644 --- a/pkg/cmd/init.go +++ b/pkg/cmd/init.go @@ -45,18 +45,14 @@ func initCerts(cfg *config.Config) (*certchains.CertificateChains, error) { // we cannot just remove the certs dir and regenerate all the certificates // because there are some long-lived certs and CAs that shouldn't be swapped // - for example system:admin client certs, KAS serving CAs - regenCerts, err := certsToRegenerate(certChains) - if err != nil { - return nil, err - } - + regenCerts := certsToRegenerate(certChains) for _, c := range regenCerts { if err := certChains.Regenerate(c...); err != nil { return nil, err } } - return certChains, err + return certChains, nil } func certSetup(cfg *config.Config) (*certchains.CertificateChains, error) { @@ -587,7 +583,7 @@ func initKubeconfigs( // certsToRegenerate returns paths to certificates in the given certificate chains // bundle that need to be regenerated -func certsToRegenerate(cs *certchains.CertificateChains) ([][]string, error) { +func certsToRegenerate(cs *certchains.CertificateChains) [][]string { regenCerts := [][]string{} for _, entry := range cs.Inventory() { certPath := entry.Path @@ -615,7 +611,7 @@ func certsToRegenerate(cs *certchains.CertificateChains) ([][]string, error) { } } - return regenCerts, nil + return regenCerts } func cleanupStaleKubeconfigs(cfg *config.Config, path string) error { diff --git a/pkg/cmd/init_test.go b/pkg/cmd/init_test.go index 7934daf18e..47bead9615 100644 --- a/pkg/cmd/init_test.go +++ b/pkg/cmd/init_test.go @@ -31,10 +31,9 @@ import ( func Test_certsToRegenerate(t *testing.T) { tests := []struct { - name string - chains *certchains.CertificateChains - want [][]string - wantErr bool + name string + chains *certchains.CertificateChains + want [][]string }{ { name: "empty chains", @@ -118,11 +117,7 @@ func Test_certsToRegenerate(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := certsToRegenerate(tt.chains) - if (err != nil) != tt.wantErr { - t.Errorf("certsToRegenerate() error = %v, wantErr %v", err, tt.wantErr) - return - } + got := certsToRegenerate(tt.chains) if !reflect.DeepEqual(got, tt.want) { t.Errorf("certsToRegenerate() = %v, want %v", got, tt.want) } diff --git a/pkg/util/cryptomaterial/certchains/inventory.go b/pkg/util/cryptomaterial/certchains/inventory.go index 57a2d56929..722807f73d 100644 --- a/pkg/util/cryptomaterial/certchains/inventory.go +++ b/pkg/util/cryptomaterial/certchains/inventory.go @@ -38,8 +38,9 @@ func (i CertificateInventory) ByRole(role CertificateRole) CertificateInventory // Inventory returns a deterministic snapshot of all certificates managed by // the chain. Signers are listed before their sub-CAs and leaf certificates. func (cs *CertificateChains) Inventory() CertificateInventory { - entries := make(CertificateInventory, 0) - for _, signerName := range cs.GetSignerNames() { + signerNames := cs.GetSignerNames() + entries := make(CertificateInventory, 0, len(signerNames)) + for _, signerName := range signerNames { signer := cs.GetSigner(signerName) entries = append(entries, signer.inventory([]string{signerName})...) } @@ -47,11 +48,12 @@ func (cs *CertificateChains) Inventory() CertificateInventory { } func (s *CertificateSigner) inventory(path []string) CertificateInventory { - entries := CertificateInventory{{ + entries := make(CertificateInventory, 0, 1+len(s.subCAs)+len(s.signedCertificates)) + entries = append(entries, CertificateInventoryEntry{ Path: append([]string(nil), path...), Role: CertificateRoleCA, Certificate: *s.signerConfig.Config.Certs[0], - }} + }) for _, subCAName := range s.GetSubCANames() { subCAPath := append(append([]string(nil), path...), subCAName)