rebase

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

put some remarks from PR discussion

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

pkg/bundle/source.go

@@ -21,7 +21,6 @@ import (
 	"context"
 	"crypto/sha256"
 	"encoding/hex"
-	"encoding/pem"
 	"fmt"
 	"strings"
 
@@ -64,7 +63,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 (
@@ -99,27 +98,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 = util.ValidateAndSplitPEMBundle(certPool, []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 util.GetCertificatesQuantity(certPool) == 0 {
 		return bundleData{}, fmt.Errorf("couldn't find any valid certificates in bundle")
 	}
 
-	deduplicatedBundles, err := deduplicateBundles(bundles)
-	if err != nil {
-		return bundleData{}, err
-	}
-
-	if err := resolvedBundle.populateData(deduplicatedBundles, formats); err != nil {
+	if err := resolvedBundle.populateData(util.AsPEMBundleStrings(certPool), formats); err != nil {
 		return bundleData{}, err
 	}
 
@@ -342,39 +333,3 @@ func (b *bundleData) populateData(bundles []string, formats *trustapi.Additional
 	}
 	return nil
 }
-
-// remove duplicate certificates from bundles
-func deduplicateBundles(bundles []string) ([]string, error) {
-	var block *pem.Block
-
-	var certificatesHashes = make(map[[32]byte]struct{})
-	var dedupCerts []string
-
-	for _, cert := range bundles {
-		certBytes := []byte(cert)
-
-	LOOP:
-		for {
-			block, certBytes = pem.Decode(certBytes)
-			if block == nil {
-				break LOOP
-			}
-
-			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
-				dedupCerts = append(dedupCerts, string(bytes.Trim(pem.EncodeToMemory(block), "\n")))
-				certificatesHashes[hash] = struct{}{}
-			}
-		}
-
-	}
-
-	return dedupCerts, nil
-}

pkg/bundle/source_test.go

@@ -431,80 +431,3 @@ func Test_certAlias(t *testing.T) {
 		t.Fatalf("expected alias to be %q but got %q", expectedAlias, alias)
 	}
 }
-
-func TestBundlesDeduplication(t *testing.T) {
-	tests := map[string]struct {
-		name       string
-		bundle     []string
-		testBundle []string
-	}{
-		"single, different cert per source": {
-			bundle: []string{
-				dummy.TestCertificate1,
-				dummy.TestCertificate2,
-			},
-			testBundle: []string{
-				dummy.TestCertificate1,
-				dummy.TestCertificate2,
-			},
-		},
-		"no certs in sources": {
-			bundle:     []string{},
-			testBundle: []string{},
-		},
-		"single cert in the first source, joined certs in the second source": {
-			bundle: []string{
-				dummy.TestCertificate1,
-				dummy.JoinCerts(dummy.TestCertificate1, dummy.TestCertificate3),
-			},
-			testBundle: []string{
-				dummy.TestCertificate1,
-				dummy.TestCertificate3,
-			},
-		},
-		"joined certs in the first source, single cert in the second source": {
-			bundle: []string{
-				dummy.JoinCerts(dummy.TestCertificate1, dummy.TestCertificate3),
-				dummy.TestCertificate1,
-			},
-			testBundle: []string{
-				dummy.TestCertificate3,
-				dummy.TestCertificate1,
-			},
-		},
-		"joined, different certs in the first source; joined,different certs in the second source": {
-			bundle: []string{
-				dummy.JoinCerts(dummy.TestCertificate1, dummy.TestCertificate2),
-				dummy.JoinCerts(dummy.TestCertificate4, dummy.TestCertificate5),
-			},
-			testBundle: []string{
-				dummy.TestCertificate1,
-				dummy.TestCertificate2,
-				dummy.TestCertificate4,
-				dummy.TestCertificate5,
-			},
-		},
-		"all certs are joined ones and equal ones in all sources": {
-			bundle: []string{
-				dummy.JoinCerts(dummy.TestCertificate1, dummy.TestCertificate1),
-				dummy.JoinCerts(dummy.TestCertificate1, dummy.TestCertificate1),
-			},
-			testBundle: []string{
-				dummy.TestCertificate1,
-			},
-		},
-	}
-	for name, test := range tests {
-		test := test
-		t.Run(name, func(t *testing.T) {
-			t.Parallel()
-
-			resultBundle, err := deduplicateBundles(test.bundle)
-
-			assert.Nil(t, err)
-
-			// check certificates bundle for duplicated certificates
-			assert.ElementsMatch(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 := util.ValidateAndSplitPEMBundle(certPool, []byte(p.Bundle))
 	if err != nil {
 		return fmt.Errorf("package bundle failed validation: %w", err)
 	}

pkg/util/cert_pool.go

@@ -17,28 +17,44 @@ limitations under the License.
 package util
 
 import (
+	"crypto/sha256"
 	"crypto/x509"
 	"encoding/pem"
 	"fmt"
 	"time"
 )
 
 // CertPool is a set of certificates.
-type certPool struct {
-	certificates  []*x509.Certificate
-	filterExpired bool
+type CertPool struct {
+	certificatesHashes map[[32]byte]struct{}
+	certificates       []*x509.Certificate
+	filterExpired      bool
+}
+
+type Option func(*CertPool)
+
+func WithFilteredExpiredCerts(filterExpired bool) Option {
+	return func(cp *CertPool) {
+		cp.filterExpired = filterExpired
+	}
 }
 
 // newCertPool returns a new, empty CertPool.
-func newCertPool(filterExpired bool) *certPool {
-	return &certPool{
-		certificates:  make([]*x509.Certificate, 0),
-		filterExpired: filterExpired,
+func NewCertPool(options ...Option) *CertPool {
+	certPool := &CertPool{
+		certificates:       make([]*x509.Certificate, 0),
+		certificatesHashes: make(map[[32]byte]struct{}),
 	}
+
+	for _, option := range options {
+		option(certPool)
+	}
+
+	return certPool
 }
 
 // Append certificate to a pool
-func (cp *certPool) appendCertFromPEM(pemData []byte) error {
+func (cp *CertPool) appendCertFromPEM(pemData []byte) error {
 	if pemData == nil {
 		return fmt.Errorf("certificate data can't be nil")
 	}
@@ -75,14 +91,18 @@ func (cp *certPool) appendCertFromPEM(pemData []byte) error {
 			continue
 		}
 
+		if cp.isDuplicate(certificate) {
+			continue
+		}
+
 		cp.certificates = append(cp.certificates, certificate)
 	}
 
 	return nil
 }
 
 // Get PEM certificates from pool
-func (cp *certPool) getCertsPEM() [][]byte {
+func (cp *CertPool) getCertsPEM() [][]byte {
 	var certsData [][]byte = make([][]byte, len(cp.certificates))
 
 	for i, cert := range cp.certificates {
@@ -91,3 +111,25 @@ func (cp *certPool) getCertsPEM() [][]byte {
 
 	return certsData
 }
+
+// Get certificates quantity in the certificates pool
+func (cp *CertPool) size() int {
+	return len(cp.certificates)
+}
+
+// Check deplicates of certificate in the certificates pool
+func (cp *CertPool) isDuplicate(cert *x509.Certificate) bool {
+	hash := sha256.Sum256(cert.Raw)
+	// check existence of the hash
+	if _, ok := cp.certificatesHashes[hash]; !ok {
+		cp.certificatesHashes[hash] = struct{}{}
+		return false
+	}
+
+	return true
+}
+
+// Get the full list of x509 Certificates from the certificates pool
+func (cp *CertPool) getCertsList() []*x509.Certificate {
+	return cp.certificates
+}

pkg/util/cert_pool_test.go

@@ -23,7 +23,7 @@ import (
 )
 
 func TestNewCertPool(t *testing.T) {
-	certPool := newCertPool(false)
+	certPool := NewCertPool(WithFilteredExpiredCerts(false))
 
 	if certPool == nil {
 		t.Fatal("pool is nil")
@@ -76,7 +76,7 @@ func TestAppendCertFromPEM(t *testing.T) {
 
 	// populate certificates bundle
 	for _, crt := range certificateList {
-		certPool := newCertPool(false)
+		certPool := NewCertPool(WithFilteredExpiredCerts(false))
 
 		if err := certPool.appendCertFromPEM([]byte(crt.certificate)); err != nil {
 			t.Fatalf("error adding PEM certificate into pool %s", err)