Merge pull request #1486 from openziti/fix.1225.dupe.certs.in.ca.bundle

fixes edge#1225 broken CA de-dupe logic, adds tests

controller/config/config.go

@@ -375,7 +375,7 @@ func LoadFromMap(configMap map[interface{}]interface{}) (*Config, error) {
 }
 
 // CalculateCaPems takes the supplied caPems buffer as a set of PEM Certificates separated by new lines. Duplicate
-// certificates are removed and the result is returned as a bytes.Buffer of PEM Certificates separated by new lines.
+// certificates are removed, and the result is returned as a bytes.Buffer of PEM Certificates separated by new lines.
 func CalculateCaPems(caPems *bytes.Buffer) *bytes.Buffer {
 	caPemMap := map[string][]byte{}
 
@@ -391,11 +391,14 @@ func CalculateCaPems(caPems *bytes.Buffer) *bytes.Buffer {
 			fingerprint := toHex(hash[:])
 			newPem := pem.EncodeToMemory(block)
 			caPemMap[fingerprint] = newPem
-			_, _ = newCaPems.WriteString("\n")
-			_, _ = newCaPems.Write(newPem)
 		}
 	}
 
+	for _, caPem := range caPemMap {
+		_, _ = newCaPems.WriteString("\n")
+		_, _ = newCaPems.Write(caPem)
+	}
+
 	return &newCaPems
 }
 

controller/config/config_test.go

@@ -1,8 +1,19 @@
 package config
 
 import (
+	"bytes"
+	"crypto"
+	"crypto/ecdsa"
+	"crypto/elliptic"
+	"crypto/rand"
+	"crypto/x509"
+	"crypto/x509/pkix"
+	"github.com/google/uuid"
+	nfpem "github.com/openziti/foundation/v2/pem"
 	"github.com/stretchr/testify/require"
+	"math/big"
 	"testing"
+	"time"
 )
 
 func Test_validateHostPortString(t *testing.T) {
@@ -109,7 +120,7 @@ func Test_validateHostPortString(t *testing.T) {
 
 		req.Error(err)
 	})
-	
+
 	t.Run("host with non-integer port should fail", func(t *testing.T) {
 		req := require.New(t)
 
@@ -182,3 +193,156 @@ func Test_validateHostPortString(t *testing.T) {
 		req.Error(err)
 	})
 }
+
+func Test_CalculateCaPems(t *testing.T) {
+	ca1, _ := newSelfSignedCert(uuid.NewString())
+	ca2, _ := newSelfSignedCert(uuid.NewString())
+	ca3, _ := newSelfSignedCert(uuid.NewString())
+
+	ca1Pem := nfpem.EncodeToBytes(ca1)
+	ca2Pem := nfpem.EncodeToBytes(ca2)
+	ca3Pem := nfpem.EncodeToBytes(ca3)
+
+	inCas := []*x509.Certificate{
+		ca1,
+		ca2,
+		ca3,
+	}
+
+	t.Run("three unique CAs in, three out", func(t *testing.T) {
+		req := require.New(t)
+
+		buf := bytes.NewBuffer([]byte{})
+
+		buf.Write(ca1Pem)
+		buf.Write(ca2Pem)
+		buf.Write(ca3Pem)
+
+		outBuf := CalculateCaPems(buf)
+
+		outCerts := nfpem.PemBytesToCertificates(outBuf.Bytes())
+
+		req.Len(outCerts, 3)
+
+		for _, inCert := range inCas {
+			found := false
+			for _, outCert := range outCerts {
+				if bytes.Equal(inCert.Raw, outCert.Raw) {
+					req.Falsef(found, "certificate %s was found multiple times, expected once instance in output", inCert.Subject.String())
+
+					found = true
+				}
+			}
+			req.Truef(found, "certificate %s was provided as input but not found as output", inCert.Subject.String())
+		}
+	})
+
+	t.Run("0 unique CAs in, 0 out", func(t *testing.T) {
+		req := require.New(t)
+
+		buf := bytes.NewBuffer([]byte{})
+
+		outBuf := CalculateCaPems(buf)
+
+		outCerts := nfpem.PemBytesToCertificates(outBuf.Bytes())
+
+		req.Len(outCerts, 0)
+	})
+
+	t.Run("1 unique CAs in, 1 out", func(t *testing.T) {
+		req := require.New(t)
+
+		buf := bytes.NewBuffer([]byte{})
+
+		buf.Write(ca1Pem)
+
+		outBuf := CalculateCaPems(buf)
+
+		outCerts := nfpem.PemBytesToCertificates(outBuf.Bytes())
+
+		req.Len(outCerts, 1)
+
+		req.True(bytes.Equal(outCerts[0].Raw, ca1.Raw), "the in ca did not match the out ca")
+	})
+
+	t.Run("2 duplicate CAs in, 1 out", func(t *testing.T) {
+		req := require.New(t)
+
+		buf := bytes.NewBuffer([]byte{})
+
+		buf.Write(ca1Pem)
+		buf.Write(ca1Pem)
+
+		outBuf := CalculateCaPems(buf)
+
+		outCerts := nfpem.PemBytesToCertificates(outBuf.Bytes())
+
+		req.Len(outCerts, 1)
+
+		req.True(bytes.Equal(outCerts[0].Raw, ca1.Raw), "the in ca did not match the out ca")
+	})
+
+	t.Run("2 sets of 2 duplicate CAs in and 1 unique, 3 unique out", func(t *testing.T) {
+		req := require.New(t)
+
+		buf := bytes.NewBuffer([]byte{})
+
+		buf.Write(ca1Pem) //uniques
+		buf.Write(ca2Pem)
+		buf.Write(ca3Pem)
+
+		buf.Write(ca1Pem) //dupe 1
+		buf.Write(ca2Pem) //dupe 2
+
+		outBuf := CalculateCaPems(buf)
+
+		outCerts := nfpem.PemBytesToCertificates(outBuf.Bytes())
+
+		req.Len(outCerts, 3)
+
+		for _, inCert := range inCas {
+			found := false
+			for _, outCert := range outCerts {
+				if bytes.Equal(inCert.Raw, outCert.Raw) {
+					req.Falsef(found, "certificate %s was found multiple times, expected once instance in output", inCert.Subject.String())
+
+					found = true
+				}
+			}
+			req.Truef(found, "certificate %s was provided as input but not found as output", inCert.Subject.String())
+		}
+	})
+
+}
+
+func newSelfSignedCert(commonName string) (*x509.Certificate, crypto.PrivateKey) {
+	priv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+	if err != nil {
+		panic(err)
+	}
+	template := x509.Certificate{
+		SerialNumber: big.NewInt(1),
+		Subject: pkix.Name{
+			CommonName:   commonName,
+			Organization: []string{"API Test Co"},
+		},
+		NotBefore: time.Now(),
+		NotAfter:  time.Now().Add(time.Hour * 24 * 180),
+
+		KeyUsage:              x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature,
+		ExtKeyUsage:           []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth},
+		BasicConstraintsValid: true,
+	}
+
+	der, err := x509.CreateCertificate(rand.Reader, &template, &template, &priv.PublicKey, priv)
+	if err != nil {
+		panic(err)
+	}
+	cert, err := x509.ParseCertificate(der)
+
+	if err != nil {
+		panic(err)
+	}
+
+	return cert, priv
+}