Refactor: use CertPool helper to limit the amount of encoding/ decodi…

…ng done.

Signed-off-by: Oleksandr Krutko <[email protected]>

pkg/bundle/bundle_test.go

@@ -41,14 +41,20 @@ import (
 
 	trustapi "github.com/cert-manager/trust-manager/pkg/apis/trust/v1alpha1"
 	"github.com/cert-manager/trust-manager/pkg/fspkg"
+	"github.com/cert-manager/trust-manager/pkg/util"
 	"github.com/cert-manager/trust-manager/test/dummy"
 	"github.com/cert-manager/trust-manager/test/gen"
 )
 
 func testEncodeJKS(t *testing.T, data string) []byte {
 	t.Helper()
 
-	encoded, err := jksEncoder{password: trustapi.DefaultJKSPassword}.encode(data)
+	certPool := util.NewCertPool()
+	if err := certPool.AddCertsFromPEM([]byte(data)); err != nil {
+		t.Fatal(err)
+	}
+
+	encoded, err := jksEncoder{password: trustapi.DefaultJKSPassword}.encode(certPool)
 	if err != nil {
 		t.Error(err)
 	}

pkg/bundle/source.go

@@ -21,9 +21,7 @@ import (
 	"context"
 	"crypto/sha256"
 	"encoding/hex"
-	"encoding/pem"
 	"fmt"
-	"slices"
 	"strings"
 
 	jks "github.com/pavlo-v-chernykh/keystore-go/v4"
@@ -54,7 +52,7 @@ type bundleData struct {
 // is each bundle is concatenated together with a new line character.
 func (b *bundle) buildSourceBundle(ctx context.Context, sources []trustapi.BundleSource, formats *trustapi.AdditionalFormats) (bundleData, error) {
 	var resolvedBundle bundleData
-	var bundles []string
+	certPool := util.NewCertPool(util.WithFilteredExpiredCerts(b.FilterExpiredCerts))
 
 	for _, source := range sources {
 		var (
@@ -89,27 +87,19 @@ func (b *bundle) buildSourceBundle(ctx context.Context, sources []trustapi.Bundl
 			return bundleData{}, fmt.Errorf("failed to retrieve bundle from source: %w", err)
 		}
 
-		opts := util.ValidateAndSanitizeOptions{FilterExpired: b.Options.FilterExpiredCerts}
-		sanitizedBundle, err := util.ValidateAndSanitizePEMBundleWithOptions([]byte(sourceData), opts)
+		err = certPool.AddCertsFromPEM([]byte(sourceData))
 		if err != nil {
 			return bundleData{}, fmt.Errorf("invalid PEM data in source: %w", err)
 		}
 
-		bundles = append(bundles, string(sanitizedBundle))
 	}
 
 	// NB: empty bundles are not valid so check and return an error if one somehow snuck through.
-
-	if len(bundles) == 0 {
+	if certPool.Size() == 0 {
 		return bundleData{}, fmt.Errorf("couldn't find any valid certificates in bundle")
 	}
 
-	deduplicatedBundles, err := deduplicateAndSortBundles(bundles)
-	if err != nil {
-		return bundleData{}, err
-	}
-
-	if err := resolvedBundle.populateData(deduplicatedBundles, formats); err != nil {
+	if err := resolvedBundle.populateData(certPool, formats); err != nil {
 		return bundleData{}, err
 	}
 
@@ -217,17 +207,12 @@ type jksEncoder struct {
 // encodeJKS creates a binary JKS file from the given PEM-encoded trust bundle and password.
 // Note that the password is not treated securely; JKS files generally seem to expect a password
 // to exist and so we have the option for one.
-func (e jksEncoder) encode(trustBundle string) ([]byte, error) {
-	cas, err := util.DecodeX509CertificateChainBytes([]byte(trustBundle))
-	if err != nil {
-		return nil, fmt.Errorf("failed to decode trust bundle: %w", err)
-	}
-
+func (e jksEncoder) encode(trustBundle *util.CertPool) ([]byte, error) {
 	// WithOrderedAliases ensures that trusted certs are added to the JKS file in order,
 	// which makes the files appear to be reliably deterministic.
 	ks := jks.New(jks.WithOrderedAliases())
 
-	for _, c := range cas {
+	for _, c := range trustBundle.Certificates() {
 		alias := certAlias(c.Raw, c.Subject.String())
 
 		// Note on CreationTime:
@@ -239,24 +224,21 @@ func (e jksEncoder) encode(trustBundle string) ([]byte, error) {
 		// - Using a fixed time (i.e. unix epoch)
 		// We use NotBefore here, arbitrarily.
 
-		err = ks.SetTrustedCertificateEntry(alias, jks.TrustedCertificateEntry{
+		if err := ks.SetTrustedCertificateEntry(alias, jks.TrustedCertificateEntry{
 			CreationTime: c.NotBefore,
 			Certificate: jks.Certificate{
 				Type:    "X509",
 				Content: c.Raw,
 			},
-		})
-
-		if err != nil {
+		}); err != nil {
 			// this error should never happen if we set jks.Certificate correctly
 			return nil, fmt.Errorf("failed to add cert with alias %q to trust store: %w", alias, err)
 		}
 	}
 
 	buf := &bytes.Buffer{}
 
-	err = ks.Store(buf, []byte(e.password))
-	if err != nil {
+	if err := ks.Store(buf, []byte(e.password)); err != nil {
 		return nil, fmt.Errorf("failed to create JKS file: %w", err)
 	}
 
@@ -285,14 +267,9 @@ type pkcs12Encoder struct {
 	password string
 }
 
-func (e pkcs12Encoder) encode(trustBundle string) ([]byte, error) {
-	cas, err := util.DecodeX509CertificateChainBytes([]byte(trustBundle))
-	if err != nil {
-		return nil, fmt.Errorf("failed to decode trust bundle: %w", err)
-	}
-
+func (e pkcs12Encoder) encode(trustBundle *util.CertPool) ([]byte, error) {
 	var entries []pkcs12.TrustStoreEntry
-	for _, c := range cas {
+	for _, c := range trustBundle.Certificates() {
 		entries = append(entries, pkcs12.TrustStoreEntry{
 			Cert:         c,
 			FriendlyName: certAlias(c.Raw, c.Subject.String()),
@@ -308,22 +285,22 @@ func (e pkcs12Encoder) encode(trustBundle string) ([]byte, error) {
 	return encoder.EncodeTrustStoreEntries(entries, e.password)
 }
 
-func (b *bundleData) populateData(bundles []string, formats *trustapi.AdditionalFormats) error {
-	b.data = strings.Join(bundles, "\n") + "\n"
+func (b *bundleData) populateData(pool *util.CertPool, formats *trustapi.AdditionalFormats) error {
+	b.data = pool.PEM()
 
 	if formats != nil {
 		b.binaryData = make(map[string][]byte)
 
 		if formats.JKS != nil {
-			encoded, err := jksEncoder{password: *formats.JKS.Password}.encode(b.data)
+			encoded, err := jksEncoder{password: *formats.JKS.Password}.encode(pool)
 			if err != nil {
 				return fmt.Errorf("failed to encode JKS: %w", err)
 			}
 			b.binaryData[formats.JKS.Key] = encoded
 		}
 
 		if formats.PKCS12 != nil {
-			encoded, err := pkcs12Encoder{password: *formats.PKCS12.Password}.encode(b.data)
+			encoded, err := pkcs12Encoder{password: *formats.PKCS12.Password}.encode(pool)
 			if err != nil {
 				return fmt.Errorf("failed to encode PKCS12: %w", err)
 			}
@@ -332,48 +309,3 @@ func (b *bundleData) populateData(bundles []string, formats *trustapi.Additional
 	}
 	return nil
 }
-
-// remove duplicate certificates from bundles and sort certificates by hash
-func deduplicateAndSortBundles(bundles []string) ([]string, error) {
-	var block *pem.Block
-
-	var certificatesHashes = make(map[[32]byte]string)
-
-	for _, cert := range bundles {
-		certBytes := []byte(cert)
-
-		for {
-			block, certBytes = pem.Decode(certBytes)
-			if block == nil {
-				break
-			}
-
-			if block.Type != "CERTIFICATE" {
-				return nil, fmt.Errorf("couldn't decode PEM block containing certificate")
-			}
-
-			// calculate hash sum of the given certificate
-			hash := sha256.Sum256(block.Bytes)
-			// check existence of the hash
-			if _, ok := certificatesHashes[hash]; !ok {
-				// neew to trim a newline which is added by Encoder
-				certificatesHashes[hash] = string(bytes.Trim(pem.EncodeToMemory(block), "\n"))
-			}
-		}
-	}
-
-	var orderedKeys [][32]byte
-	for key := range certificatesHashes {
-		orderedKeys = append(orderedKeys, key)
-	}
-	slices.SortFunc(orderedKeys, func(a, b [32]byte) int {
-		return bytes.Compare(a[:], b[:])
-	})
-
-	var sortedDeduplicatedCerts []string
-	for _, key := range orderedKeys {
-		sortedDeduplicatedCerts = append(sortedDeduplicatedCerts, certificatesHashes[key])
-	}
-
-	return sortedDeduplicatedCerts, nil
-}

pkg/bundle/source_test.go

@@ -22,6 +22,7 @@ import (
 	"crypto/x509"
 	"encoding/pem"
 	"errors"
+	"strings"
 	"testing"
 
 	jks "github.com/pavlo-v-chernykh/keystore-go/v4"
@@ -35,6 +36,7 @@ import (
 
 	trustapi "github.com/cert-manager/trust-manager/pkg/apis/trust/v1alpha1"
 	"github.com/cert-manager/trust-manager/pkg/fspkg"
+	"github.com/cert-manager/trust-manager/pkg/util"
 	"github.com/cert-manager/trust-manager/test/dummy"
 )
 
@@ -398,7 +400,12 @@ func Test_encodeJKSAliases(t *testing.T) {
 	// Using different dummy certs would allow this test to pass but wouldn't actually test anything useful!
 	bundle := dummy.JoinCerts(dummy.TestCertificate1, dummy.TestCertificate2)
 
-	jksFile, err := jksEncoder{password: trustapi.DefaultJKSPassword}.encode(bundle)
+	certPool := util.NewCertPool()
+	if err := certPool.AddCertsFromPEM([]byte(bundle)); err != nil {
+		t.Fatalf("failed to add certs to pool: %s", err)
+	}
+
+	jksFile, err := jksEncoder{password: trustapi.DefaultJKSPassword}.encode(certPool)
 	if err != nil {
 		t.Fatalf("didn't expect an error but got: %s", err)
 	}
@@ -449,6 +456,7 @@ func TestBundlesDeduplication(t *testing.T) {
 	tests := map[string]struct {
 		name       string
 		bundle     []string
+		expError   string
 		testBundle []string
 	}{
 		"single, different cert per source": {
@@ -464,6 +472,7 @@ func TestBundlesDeduplication(t *testing.T) {
 		"no certs in sources": {
 			bundle:     []string{},
 			testBundle: nil,
+			expError:   "no non-expired certificates found in input bundle",
 		},
 		"single cert in the first source, joined certs in the second source": {
 			bundle: []string{
@@ -512,9 +521,17 @@ func TestBundlesDeduplication(t *testing.T) {
 		t.Run(name, func(t *testing.T) {
 			t.Parallel()
 
-			resultBundle, err := deduplicateAndSortBundles(test.bundle)
+			certPool := util.NewCertPool()
+			err := certPool.AddCertsFromPEM([]byte(strings.Join(test.bundle, "\n")))
+			if test.expError != "" {
+				assert.NotNil(t, err)
+				assert.Equal(t, err.Error(), test.expError)
+				return
+			} else {
+				assert.Nil(t, err)
+			}
 
-			assert.Nil(t, err)
+			resultBundle := certPool.PEMSplit()
 
 			// check certificates bundle for duplicated certificates
 			assert.Equal(t, test.testBundle, resultBundle)

pkg/fspkg/package.go

@@ -64,7 +64,10 @@ func (p *Package) Clone() *Package {
 func (p *Package) Validate() error {
 	// Ignore the sanitized bundle here and preserve the bundle as-is.
 	// We'll sanitize later, when building a bundle on a reconcile.
-	_, err := util.ValidateAndSanitizePEMBundle([]byte(p.Bundle))
+
+	certPool := util.NewCertPool(util.WithFilteredExpiredCerts(false))
+
+	err := certPool.AddCertsFromPEM([]byte(p.Bundle))
 	if err != nil {
 		return fmt.Errorf("package bundle failed validation: %w", err)
 	}