Refactor ca_chain package and improve test coverage

helpers/tls/ca_chain/builder_test.go

@@ -71,6 +71,43 @@ KZSdU7u7WMCjLYpyC5kbRmd/Qkdo/45wifomJNP3/16NSNZ0gatKVUJ6q6UjRsZl
 iG9V6qslKJvNR8A8A+RqvyfIJ0gjNzVLQHrZyTsEbC62w1IcxkBG7lR6W7ZCXal1
 RSKf+3OIln1a6DKx+zEzL20uwW5L/5l3FsLwwvOLybX4mAhiyxY=
 -----END CERTIFICATE-----`
+
+	// the same as testCert, but encoded with PKCS7
+	testCertPKCS7 = `-----BEGIN PKCS7-----
+MIIEQwYJKoZIhvcNAQcCoIIENDCCBDACAQExADALBgkqhkiG9w0BBwGgggQWMIIE
+EjCCAfoCFBhRTszftYHtN+HOfbU/q3zvYBYOMA0GCSqGSIb3DQEBCwUAMFUxCzAJ
+BgNVBAYTAlVTMQowCAYDVQQIDAEgMQowCAYDVQQKDAEgMQowCAYDVQQLDAEgMRAw
+DgYDVQQDDAdUZXN0IENBMRAwDgYJKoZIhvcNAQkBFgEgMCAXDTE5MTAxODA2MDA1
+MloYDzIxMTkwOTI0MDYwMDUyWjA0MQswCQYDVQQGEwJVUzEKMAgGA1UECAwBIDEK
+MAgGA1UECgwBIDENMAsGA1UEAwwEdGVzdDCCASIwDQYJKoZIhvcNAQEBBQADggEP
+ADCCAQoCggEBALc0+Xo61c0xCvebNg1OJl4iXC5blzGlbDfejWKn7266g+UUZ3xs
+cCDWMNruojd+7EbkQmAyUtdGifNw+xIHyNA/jiyIsB3KteN84X+toA4mjY1tSpql
+NMOUW0EZ9f0KZNn4GZnA/TyFWI3EC4gOcJyuuL7YfE7Qu1e3LeBwDcRYpJ3WZw1k
+3+aClC1N7iTPEP9scr64+KA0d5xIkrtl5t8qiSR8Tn+JLPygGre0G0hhIZeHpfPQ
+WX6iILbJMgPnbPmCivklkyUIE8WHh2qGbOGaO3LVKSS6/YfOshw4g/RQyusIIi65
+iXnFa/VvRY2dkn5w9EehZzbT8kQa7U39NwkCAwEAATANBgkqhkiG9w0BAQsFAAOC
+AgEAMAfp7FRBHm9t4byRfWrUYblI7eQOlcixXHSPc16VX93HTsNWwZV1EBiOGWcT
+Rcts5FQr9HWGHVukQ+4iXLWtb/Og+hHrjyLmOGvx7sgPeHuyWB89npSABdenrpMH
+PePMzsO/YTw1QuYJOijNYpLCL83YWk62DCSGwQ2HO1KKLDw3suBHudV80cHVnav7
+Q0VW+iA+3apdrgediCHCtc6PQDHPzdrXQSVA+OF2itX3Xhc6Mm3dn4D3HhqoWYJN
+eI0naNHTguoKFYdJHHjv07nX+1I+CAk6kjEv17VEKsU7SjhOizLYdtb9OrOSgnQ6
+KTkPfCeIlK2PNguwxgeLBNYQyTnUxr1QxgVkKFsBfwFV4hq9podEbjrgUSu1KZSd
+U7u7WMCjLYpyC5kbRmd/Qkdo/45wifomJNP3/16NSNZ0gatKVUJ6q6UjRsZl3va4
+QcB3QuNtGiQZqEuc/+KM21MSvC8cC/bIOaKZlWbKtEV+tsbuIIhng0opJrEw+5Zq
+VqrwIVjbsGaw/NPROth/XDJp5jzpwxnf5HDQhLV04sfdN9IRw005WC+l0f19iG9V
+6qslKJvNR8A8A+RqvyfIJ0gjNzVLQHrZyTsEbC62w1IcxkBG7lR6W7ZCXal1RSKf
++3OIln1a6DKx+zEzL20uwW5L/5l3FsLwwvOLybX4mAhiyxahADEA
+-----END PKCS7-----`
+
+	testCertPubKey = `-----BEGIN PUBLIC KEY-----
+MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAtzT5ejrVzTEK95s2DU4m
+XiJcLluXMaVsN96NYqfvbrqD5RRnfGxwINYw2u6iN37sRuRCYDJS10aJ83D7EgfI
+0D+OLIiwHcq143zhf62gDiaNjW1KmqU0w5RbQRn1/Qpk2fgZmcD9PIVYjcQLiA5w
+nK64vth8TtC7V7ct4HANxFikndZnDWTf5oKULU3uJM8Q/2xyvrj4oDR3nEiSu2Xm
+3yqJJHxOf4ks/KAat7QbSGEhl4el89BZfqIgtskyA+ds+YKK+SWTJQgTxYeHaoZs
+4Zo7ctUpJLr9h86yHDiD9FDK6wgiLrmJecVr9W9FjZ2SfnD0R6FnNtPyRBrtTf03
+CQIDAQAB
+-----END PUBLIC KEY-----`
 )
 
 func TestDefaultBuilder_BuildChainFromTLSConnectionState(t *testing.T) {

helpers/tls/ca_chain/decoder.go → helpers/tls/ca_chain/helpers.go

@@ -10,9 +10,13 @@ import (
 	"bytes"
 	"crypto/x509"
 	"encoding/pem"
+	"fmt"
+	"io/ioutil"
+	"net/http"
 	"strings"
 
 	"github.com/fullsailor/pkcs7"
+	"github.com/sirupsen/logrus"
 )
 
 const (
@@ -22,8 +26,8 @@ const (
 
 type ErrorInvalidCertificate struct {
 	inner            error
-	nilBlock         bool
 	nonCertBlockType bool
+	nilBlock         bool
 }
 
 func (e *ErrorInvalidCertificate) Error() string {
@@ -32,7 +36,7 @@ func (e *ErrorInvalidCertificate) Error() string {
 	if e.nilBlock {
 		msg = append(msg, "empty PEM block")
 	} else if e.nonCertBlockType {
-		msg = append(msg, "non certificate PEM block")
+		msg = append(msg, "non-certificate PEM block")
 	} else if e.inner != nil {
 		msg = append(msg, e.inner.Error())
 	}
@@ -40,7 +44,7 @@ func (e *ErrorInvalidCertificate) Error() string {
 	return strings.Join(msg, ": ")
 }
 
-func DecodeCertificate(data []byte) (*x509.Certificate, error) {
+func decodeCertificate(data []byte) (*x509.Certificate, error) {
 	if isPEM(data) {
 		block, _ := pem.Decode(data)
 		if block == nil {
@@ -69,3 +73,42 @@ func DecodeCertificate(data []byte) (*x509.Certificate, error) {
 func isPEM(data []byte) bool {
 	return bytes.HasPrefix(data, []byte(pemStart))
 }
+
+func isSelfSigned(cert *x509.Certificate) bool {
+	return cert.CheckSignatureFrom(cert) == nil
+}
+
+func prepareCertificateLogger(logger logrus.FieldLogger, cert *x509.Certificate) logrus.FieldLogger {
+	return preparePrefixedCertificateLogger(logger, cert, "")
+}
+
+func preparePrefixedCertificateLogger(logger logrus.FieldLogger, cert *x509.Certificate, prefix string) logrus.FieldLogger {
+	return logger.
+		WithFields(logrus.Fields{
+			fmt.Sprintf("%sSubject", prefix):       cert.Subject.CommonName,
+			fmt.Sprintf("%sIssuer", prefix):        cert.Issuer.CommonName,
+			fmt.Sprintf("%sSerial", prefix):        cert.SerialNumber.String(),
+			fmt.Sprintf("%sIssuerCertURL", prefix): cert.IssuingCertificateURL,
+		})
+}
+
+func fetchRemoteCertificate(url string) ([]byte, error) {
+	resp, err := http.Get(url)
+	if resp != nil {
+		defer resp.Body.Close()
+	}
+	if err != nil {
+		return nil, err
+	}
+
+	data, err := ioutil.ReadAll(resp.Body)
+	if err != nil {
+		return nil, err
+	}
+
+	return data, nil
+}
+
+func verifyCertificate(cert *x509.Certificate) ([][]*x509.Certificate, error) {
+	return cert.Verify(x509.VerifyOptions{})
+}

helpers/tls/ca_chain/helpers_test.go

+package ca_chain
+
+import (
+	"crypto/x509"
+	"encoding/pem"
+	"errors"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func loadCertificate(t *testing.T, dump string) *x509.Certificate {
+	block, _ := pem.Decode([]byte(dump))
+	cert, err := x509.ParseCertificate(block.Bytes)
+	require.NoError(t, err)
+
+	return cert
+}
+
+func TestErrorInvalidCertificate_Error(t *testing.T) {
+	testError := errors.New("test-error")
+
+	tests := map[string]struct {
+		err            *ErrorInvalidCertificate
+		expectedOutput string
+	}{
+		"no details provided": {
+			err:            new(ErrorInvalidCertificate),
+			expectedOutput: "invalid certificate",
+		},
+		"inner specified": {
+			err: &ErrorInvalidCertificate{
+				inner: testError,
+			},
+			expectedOutput: "invalid certificate: test-error",
+		},
+		"marked with nonCertBlockType": {
+			err: &ErrorInvalidCertificate{
+				inner:            testError,
+				nonCertBlockType: true,
+			},
+			expectedOutput: "invalid certificate: non-certificate PEM block",
+		},
+		"marked with nilBlock": {
+			err: &ErrorInvalidCertificate{
+				inner:            testError,
+				nonCertBlockType: true,
+				nilBlock:         true,
+			},
+			expectedOutput: "invalid certificate: empty PEM block",
+		},
+	}
+
+	for tn, tc := range tests {
+		t.Run(tn, func(t *testing.T) {
+			assert.EqualError(t, tc.err, tc.expectedOutput)
+		})
+	}
+}
+
+func TestDecodeCertificate(t *testing.T) {
+	block, _ := pem.Decode([]byte(testCert))
+	decodedPEMx509Data := block.Bytes
+
+	testX509Certificate, err := x509.ParseCertificate(decodedPEMx509Data)
+	require.NoError(t, err)
+
+	block, _ = pem.Decode([]byte(testCertPKCS7))
+	decodedPEMPKCS7Data := block.Bytes
+
+	tests := map[string]struct {
+		data                []byte
+		expectedError       string
+		expectedCertificate *x509.Certificate
+	}{
+		"invalid data": {
+			data:                []byte("test"),
+			expectedError:       "invalid certificate: ber2der: BER tag length is more than available data",
+			expectedCertificate: nil,
+		},
+		"invalid PEM type": {
+			data:                []byte(testCertPubKey),
+			expectedError:       "invalid certificate: non-certificate PEM block",
+			expectedCertificate: nil,
+		},
+		"raw PEM x509 data": {
+			data:                []byte(testCert),
+			expectedError:       "",
+			expectedCertificate: testX509Certificate,
+		},
+		"decoded PEM x509 data": {
+			data:                decodedPEMx509Data,
+			expectedError:       "",
+			expectedCertificate: testX509Certificate,
+		},
+		"decoded PEM pkcs7 data": {
+			data:                decodedPEMPKCS7Data,
+			expectedError:       "",
+			expectedCertificate: testX509Certificate,
+		},
+	}
+
+	for tn, tc := range tests {
+		t.Run(tn, func(t *testing.T) {
+			cert, err := decodeCertificate(tc.data)
+
+			if tc.expectedError != "" {
+				assert.EqualError(t, err, tc.expectedError)
+				return
+			}
+
+			assert.NoError(t, err)
+
+			if tc.expectedCertificate != nil {
+				assert.Equal(t, tc.expectedCertificate.SerialNumber, cert.SerialNumber)
+				return
+			}
+
+			assert.Nil(t, tc.expectedCertificate)
+		})
+	}
+}
+
+func TestIsPem(t *testing.T) {
+	assert.True(t, isPEM([]byte(testCert)))
+
+	block, _ := pem.Decode([]byte(testCert))
+	assert.False(t, isPEM(block.Bytes))
+}

helpers/tls/ca_chain/resolver.go

@@ -8,170 +8,17 @@ package ca_chain
 
 import (
 	"crypto/x509"
-	"fmt"
-	"io/ioutil"
-	"net/http"
 
 	"github.com/sirupsen/logrus"
 )
 
-const resolveChainLoopLimit = 15
-
 type resolver interface {
 	Resolve(certs []*x509.Certificate) ([]*x509.Certificate, error)
 }
 
 func newResolver(logger logrus.FieldLogger) resolver {
-	return &chainResolver{
-		logger: logger,
-	}
-}
-
-type chainResolver struct {
-	logger        logrus.FieldLogger
-	verifyOptions x509.VerifyOptions
-}
-
-func (d *chainResolver) Resolve(certs []*x509.Certificate) ([]*x509.Certificate, error) {
-	certs, err := d.resolveChain(certs)
-	if err != nil {
-		return nil, fmt.Errorf("error while resolving certificates chain: %v", err)
-	}
-
-	certs, err = d.lookForRootIfMissing(certs)
-	if err != nil {
-		return nil, fmt.Errorf("error while looking for a missing root certificate: %v", err)
-	}
-
-	return certs, err
-}
-
-func (d *chainResolver) resolveChain(certs []*x509.Certificate) ([]*x509.Certificate, error) {
-	if len(certs) < 1 {
-		return certs, nil
-	}
-
-	for i := 0; i < resolveChainLoopLimit; i++{
-		certificate := certs[len(certs)-1]
-		log := prepareCertificateLogger(d.logger, certificate)
-
-		if certificate.IssuingCertificateURL == nil {
-			log.Debug("Certificate doesn't provide parent URL: exiting the loop")
-			break
-		}
-
-		newCert, err := d.fetchIssuerCertificate(certificate)
-		if err != nil {
-			return nil, fmt.Errorf("error while fetching issuer certificate: %v", err)
-		}
-
-		certs = append(certs, newCert)
-
-		if isSelfSigned(newCert) {
-			log.Debug("Fetched issuer certificate is a ROOT certificate so exiting the loop")
-			break
-		}
-	}
-
-	return certs, nil
-}
-
-func prepareCertificateLogger(logger logrus.FieldLogger, cert *x509.Certificate) logrus.FieldLogger {
-	return logger.
-		WithFields(logrus.Fields{
-			"subject":       cert.Subject.CommonName,
-			"issuer":        cert.Issuer.CommonName,
-			"serial":        cert.SerialNumber.String(),
-			"issuerCertURL": cert.IssuingCertificateURL,
-		})
-}
-
-func (d *chainResolver) fetchIssuerCertificate(cert *x509.Certificate) (*x509.Certificate, error) {
-	log := prepareCertificateLogger(d.logger, cert).
-		WithField("method", "fetchIssuerCertificate")
-
-	parentURL := cert.IssuingCertificateURL[0]
-
-	resp, err := http.Get(parentURL)
-	if resp != nil {
-		defer resp.Body.Close()
-	}
-	if err != nil {
-		log.
-			WithError(err).
-			Warning("HTTP request error")
-		return nil, err
-	}
-
-	data, err := ioutil.ReadAll(resp.Body)
-	if err != nil {
-		log.
-			WithError(err).
-			Warning("Response body read error")
-		return nil, err
-	}
-
-	newCert, err := DecodeCertificate(data)
-	if err != nil {
-		log.
-			WithError(err).
-			Warning("Certificate decoding error")
-		return nil, err
-	}
-
-	log.
-		WithFields(logrus.Fields{
-			"newCert-subject":       newCert.Subject.CommonName,
-			"newCert-issuer":        newCert.Issuer.CommonName,
-			"newCert-serial":        newCert.SerialNumber.String(),
-			"newCert-issuerCertURL": newCert.IssuingCertificateURL,
-		}).
-		Debug("Appending the certificate to the chain")
-
-	return newCert, nil
-}
-
-func (d *chainResolver) lookForRootIfMissing(certs []*x509.Certificate) ([]*x509.Certificate, error) {
-	if len(certs) < 1 {
-		return certs, nil
-	}
-
-	lastCert := certs[len(certs)-1]
-
-	if isSelfSigned(lastCert) {
-		return certs, nil
-	}
-
-	prepareCertificateLogger(d.logger, lastCert).
-		Debug("Verifying last certificate to find the final root certificate")
-
-	verifyChains, err := lastCert.Verify(d.verifyOptions)
-	if err != nil {
-		if _, e := err.(x509.UnknownAuthorityError); e {
-			prepareCertificateLogger(d.logger, lastCert).
-				WithError(err).
-				Warning("Last certificate signed by unknown authority; will not update the chain")
-
-			return certs, nil
-		}
-
-		return nil, fmt.Errorf("error while verifying last certificate from the chain: %v", err)
-	}
-
-	for _, cert := range verifyChains[0] {
-		if lastCert.Equal(cert) {
-			continue
-		}
-
-		prepareCertificateLogger(d.logger, cert).
-			Debug("Adding cert from verify chain to the final chain")
-
-		certs = append(certs, cert)
-	}
-
-	return certs, nil
-}
-
-func isSelfSigned(cert *x509.Certificate) bool {
-	return cert.CheckSignatureFrom(cert) == nil
+	return newChainResolver(
+		newURLResolver(logger),
+		newVerifyResolver(logger),
+	)
 }

helpers/tls/ca_chain/resolver_chain.go

+package ca_chain
+
+import (
+	"crypto/x509"
+	"fmt"
+
+	"github.com/sirupsen/logrus"
+)
+
+type chainResolver struct {
+	logger logrus.FieldLogger
+
+	urlResolver    resolver
+	verifyResolver resolver
+}
+
+func newChainResolver(urlResolver resolver, verifyResolver resolver) resolver {
+	return &chainResolver{
+		urlResolver:    urlResolver,
+		verifyResolver: verifyResolver,
+	}
+}
+
+func (r *chainResolver) Resolve(certs []*x509.Certificate) ([]*x509.Certificate, error) {
+	certs, err := r.urlResolver.Resolve(certs)
+	if err != nil {
+		return nil, fmt.Errorf("error while resolving certificates chain with URL: %v", err)
+	}
+
+	certs, err = r.verifyResolver.Resolve(certs)
+	if err != nil {
+		return nil, fmt.Errorf("error while resolving certificates chain with verification: %v", err)
+	}
+
+	return certs, err
+}

helpers/tls/ca_chain/resolver_chain_test.go

+package ca_chain
+
+import (
+	"crypto/x509"
+	"errors"
+	"math/big"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+type resolverMockFactory func(t *testing.T) (resolver, func())
+
+func newResolverMock(inputCerts []*x509.Certificate, returnCerts []*x509.Certificate, returnErr error) resolverMockFactory {
+	return func(t *testing.T) (resolver, func()) {
+		mock := new(mockResolver)
+		cleanup := func() {
+			mock.AssertExpectations(t)
+		}
+
+		mock.
+			On("Resolve", inputCerts).
+			Return(returnCerts, returnErr).
+			Once()
+
+		return mock, cleanup
+	}
+}
+
+func TestChainResolver_Resolve(t *testing.T) {
+	testError := errors.New("test error")
+
+	certs := []*x509.Certificate{{SerialNumber: big.NewInt(1)}}
+	urlCerts := []*x509.Certificate{{SerialNumber: big.NewInt(2)}}
+	verifyCerts := []*x509.Certificate{{SerialNumber: big.NewInt(3)}}
+
+	noopMock := func(t *testing.T) (resolver, func()) { return nil, func() {} }
+
+	tests := map[string]struct {
+		urlResolver    resolverMockFactory
+		verifyResolver resolverMockFactory
+		expectedError  string
+		expectedCerts  []*x509.Certificate
+	}{
+		"error on urlResolver": {
+			urlResolver:    newResolverMock(certs, nil, testError),
+			verifyResolver: noopMock,
+			expectedError:  "error while resolving certificates chain with URL: test error",
+			expectedCerts:  nil,
+		},
+		"error on verifyResolver": {
+			urlResolver:    newResolverMock(certs, urlCerts, nil),
+			verifyResolver: newResolverMock(urlCerts, nil, testError),
+			expectedError:  "error while resolving certificates chain with verification: test error",
+			expectedCerts:  nil,
+		},
+		"certificates resolved properly": {
+			urlResolver:    newResolverMock(certs, urlCerts, nil),
+			verifyResolver: newResolverMock(urlCerts, verifyCerts, nil),
+			expectedError:  "",
+			expectedCerts:  verifyCerts,
+		},
+	}
+
+	for tn, tc := range tests {
+		t.Run(tn, func(t *testing.T) {
+			urlResolver, cleanupURLResolver := tc.urlResolver(t)
+			defer cleanupURLResolver()
+
+			verifyResolver, cleanupVerifyResolver := tc.verifyResolver(t)
+			defer cleanupVerifyResolver()
+
+			r := newChainResolver(urlResolver, verifyResolver)
+			newCerts, err := r.Resolve(certs)
+
+			if tc.expectedError != "" {
+				assert.EqualError(t, err, tc.expectedError)
+			} else {
+				assert.NoError(t, err)
+			}
+
+			assert.Equal(t, tc.expectedCerts, newCerts)
+		})
+	}
+}

helpers/tls/ca_chain/resolver_test.go

-package ca_chain
-
-import (
-	"bytes"
-	"crypto/x509"
-	"encoding/pem"
-	"net/http"
-	"net/http/httptest"
-	"testing"
-
-	"github.com/sirupsen/logrus"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-)
-
-const (
-	testCACertURI  = "/ca-cert"
-	invalidCertURI = "/invalid-cert"
-)
-
-func TestChainResolver_Resolve(t *testing.T) {
-	server := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
-		if r.RequestURI == testCACertURI {
-			_, err := rw.Write([]byte(testCACert))
-			require.NoError(t, err)
-
-			return
-		}
-
-		if r.RequestURI == invalidCertURI {
-			_, err := rw.Write([]byte("-----BEGIN CERTIFICATE-----"))
-			require.NoError(t, err)
-
-			return
-		}
-
-		rw.WriteHeader(http.StatusNotFound)
-	}))
-	defer server.Close()
-
-	caCertPool := func() *x509.CertPool {
-		block, _ := pem.Decode([]byte(testCACert))
-		testCACertificate, err := x509.ParseCertificate(block.Bytes)
-		require.NoError(t, err)
-
-		p := x509.NewCertPool()
-		p.AddCert(testCACertificate)
-
-		return p
-	}
-
-	tests := map[string]struct {
-		issuerURL           []string
-		certPool            func() *x509.CertPool
-		expectedError       string
-		expectedChainLength int
-		expectedOutput      []string
-	}{
-		"no issuer certificate URL": {
-			expectedError:       "",
-			expectedChainLength: 1,
-			expectedOutput: []string{
-				"Certificate doesn't provide parent URL",
-				"Verifying last certificate to find the final root certificate",
-				"Last certificate signed by unknown authority; will not update the chain",
-			},
-		},
-		"no issuer certificate URL but CA present in system cert pool": {
-			certPool:            caCertPool,
-			expectedError:       "",
-			expectedChainLength: 2,
-			expectedOutput: []string{
-				"Certificate doesn't provide parent URL",
-				"Verifying last certificate to find the final root certificate",
-				"Adding cert from verify chain to the final chain",
-			},
-		},
-		"invalid certificate as parent": {
-			issuerURL:           []string{server.URL + invalidCertURI},
-			expectedError:       "error while resolving certificates chain: error while fetching issuer certificate: invalid certificate: empty PEM block",
-			expectedChainLength: 0,
-			expectedOutput: []string{
-				"Certificate decoding error",
-			},
-		},
-		"issuer certificate as parent": {
-			issuerURL:           []string{server.URL + testCACertURI},
-			expectedError:       "",
-			expectedChainLength: 2,
-			expectedOutput: []string{
-				"Appending the certificate to the chain",
-				"Fetched issuer certificate is a ROOT certificate so exiting the loop",
-			},
-		},
-		"issuer certificate as parent and within system cert pool": {
-			issuerURL:           []string{server.URL + testCACertURI},
-			certPool:            caCertPool,
-			expectedError:       "",
-			expectedChainLength: 2,
-			expectedOutput: []string{
-				"Appending the certificate to the chain",
-				"Fetched issuer certificate is a ROOT certificate so exiting the loop",
-			},
-		},
-	}
-
-	for tn, tc := range tests {
-		t.Run(tn, func(t *testing.T) {
-			block, _ := pem.Decode([]byte(testCert))
-			testCertificate, err := x509.ParseCertificate(block.Bytes)
-			require.NoError(t, err)
-
-			testCertificate.IssuingCertificateURL = tc.issuerURL
-
-			out := new(bytes.Buffer)
-
-			logger := logrus.New()
-			logger.Level = logrus.DebugLevel
-			logger.Out = out
-
-			r := newResolver(logger).(*chainResolver)
-
-			if tc.certPool != nil {
-				r.verifyOptions = x509.VerifyOptions{
-					Roots: tc.certPool(),
-				}
-			}
-
-			certificates, err := r.Resolve([]*x509.Certificate{testCertificate})
-
-			if tc.expectedError != "" {
-				assert.EqualError(t, err, tc.expectedError)
-				return
-			}
-
-			assert.NoError(t, err)
-			assert.Len(t, certificates, tc.expectedChainLength)
-
-			output := out.String()
-			for _, outputPart := range tc.expectedOutput {
-				if outputPart == "" {
-					continue
-				}
-
-				assert.Contains(t, output, outputPart)
-			}
-		})
-	}
-}

helpers/tls/ca_chain/resolver_url.go

+package ca_chain
+
+import (
+	"crypto/x509"
+	"fmt"
+
+	"github.com/sirupsen/logrus"
+)
+
+const defaultURLResolverLoopLimit = 15
+
+type fetcher func(url string) ([]byte, error)
+type decoder func(data []byte) (*x509.Certificate, error)
+
+type urlResolver struct {
+	logger  logrus.FieldLogger
+	fetcher fetcher
+	decoder decoder
+
+	loopLimit int
+}
+
+func newURLResolver(logger logrus.FieldLogger) resolver {
+	return &urlResolver{
+		logger:    logger,
+		fetcher:   fetchRemoteCertificate,
+		decoder:   decodeCertificate,
+		loopLimit: defaultURLResolverLoopLimit,
+	}
+}
+
+func (r *urlResolver) Resolve(certs []*x509.Certificate) ([]*x509.Certificate, error) {
+	if len(certs) < 1 {
+		return nil, nil
+	}
+
+	loop := 0
+	for {
+		loop++
+		if loop >= r.loopLimit {
+			r.
+				logger.
+				Warning("urlResolver loop limit exceeded; exiting the loop")
+
+			break
+		}
+
+		certificate := certs[len(certs)-1]
+		log := prepareCertificateLogger(r.logger, certificate)
+
+		if certificate.IssuingCertificateURL == nil {
+			log.Debug("Certificate doesn't provide parent URL: exiting the loop")
+			break
+		}
+
+		newCert, err := r.fetchIssuerCertificate(certificate)
+		if err != nil {
+			return nil, fmt.Errorf("error while fetching issuer certificate: %v", err)
+		}
+
+		certs = append(certs, newCert)
+
+		if isSelfSigned(newCert) {
+			log.Debug("Fetched issuer certificate is a ROOT certificate so exiting the loop")
+			break
+		}
+	}
+
+	return certs, nil
+}
+
+func (r *urlResolver) fetchIssuerCertificate(cert *x509.Certificate) (*x509.Certificate, error) {
+	log := prepareCertificateLogger(r.logger, cert).
+		WithField("method", "fetchIssuerCertificate")
+
+	issuerURL := cert.IssuingCertificateURL[0]
+
+	data, err := r.fetcher(issuerURL)
+	if err != nil {
+		log.
+			WithError(err).
+			WithField("issuerURL", issuerURL).
+			Warning("Remote certificate fetching error")
+
+		return nil, fmt.Errorf("remote fetch failure: %v", err)
+	}
+
+	newCert, err := r.decoder(data)
+	if err != nil {
+		log.
+			WithError(err).
+			Warning("Certificate decoding error")
+
+		return nil, fmt.Errorf("decoding failure: %v", err)
+	}
+
+	preparePrefixedCertificateLogger(log, newCert, "newCert").
+		Debug("Appending the certificate to the chain")
+
+	return newCert, nil
+}

helpers/tls/ca_chain/resolver_url_test.go

+package ca_chain
+
+import (
+	"bytes"
+	"crypto/x509"
+	"errors"
+	"testing"
+
+	"github.com/sirupsen/logrus"
+	"github.com/stretchr/testify/assert"
+)
+
+type fetcherMockFactory func(t *testing.T) fetcher
+
+func newFetcherMock(url string, data []byte, err error) fetcherMockFactory {
+	return func(t *testing.T) fetcher {
+		return func(url string) ([]byte, error) {
+			assert.Equal(t, url, url)
+
+			return data, err
+		}
+	}
+}
+
+type decoderMockFactory func(t *testing.T) decoder
+
+func newDecoderMock(inputData []byte, cert *x509.Certificate, err error) decoderMockFactory {
+	return func(t *testing.T) decoder {
+		return func(data []byte) (*x509.Certificate, error) {
+			assert.Equal(t, inputData, data)
+
+			return cert, err
+		}
+	}
+}
+
+func TestUrlResolver_Resolve(t *testing.T) {
+	testError := errors.New("test-error")
+	url1 := "url1"
+
+	testCACertificate := loadCertificate(t, testCACert)
+	testCertificate := loadCertificate(t, testCert)
+	testCertificateWithURL := loadCertificate(t, testCert)
+	testCertificateWithURL.IssuingCertificateURL = []string{url1, "url2"}
+
+	tests := map[string]struct {
+		certs          []*x509.Certificate
+		mockLoopLimit  int
+		mockFetcher    fetcherMockFactory
+		mockDecoder    decoderMockFactory
+		expectedError  string
+		expectedCerts  []*x509.Certificate
+		expectedOutput []string
+	}{
+		"empty input chain": {
+			certs:          nil,
+			mockLoopLimit:  defaultURLResolverLoopLimit,
+			expectedError:  "",
+			expectedCerts:  nil,
+			expectedOutput: nil,
+		},
+		"last certificate without URL": {
+			certs:         []*x509.Certificate{testCertificate},
+			mockLoopLimit: defaultURLResolverLoopLimit,
+			expectedError: "",
+			expectedCerts: []*x509.Certificate{testCertificate},
+			expectedOutput: []string{
+				"Certificate doesn't provide parent URL: exiting the loop",
+			},
+		},
+		"last certificate with URL and fetcher error": {
+			certs:         []*x509.Certificate{testCertificateWithURL},
+			mockLoopLimit: defaultURLResolverLoopLimit,
+			mockFetcher:   newFetcherMock(url1, nil, testError),
+			expectedError: "error while fetching issuer certificate: remote fetch failure: test-error",
+			expectedCerts: nil,
+			expectedOutput: []string{
+				"Remote certificate fetching error",
+			},
+		},
+		"last certificate with URL and decoder error": {
+			certs:         []*x509.Certificate{testCertificateWithURL},
+			mockLoopLimit: defaultURLResolverLoopLimit,
+			mockFetcher:   newFetcherMock(url1, []byte("test"), nil),
+			mockDecoder:   newDecoderMock([]byte("test"), nil, testError),
+			expectedError: "error while fetching issuer certificate: decoding failure: test-error",
+			expectedCerts: nil,
+			expectedOutput: []string{
+				"Certificate decoding error",
+			},
+		},
+		"last certificate with URL with not self signed": {
+			certs:         []*x509.Certificate{testCertificateWithURL},
+			mockLoopLimit: defaultURLResolverLoopLimit,
+			mockFetcher:   newFetcherMock(url1, []byte("test"), nil),
+			mockDecoder:   newDecoderMock([]byte("test"), testCertificate, nil),
+			expectedError: "",
+			expectedCerts: []*x509.Certificate{testCertificateWithURL, testCertificate},
+			expectedOutput: []string{
+				"Appending the certificate to the chain",
+			},
+		},
+		"last certificate with URL with self signed": {
+			certs:         []*x509.Certificate{testCertificateWithURL},
+			mockLoopLimit: defaultURLResolverLoopLimit,
+			mockFetcher:   newFetcherMock(url1, []byte("test"), nil),
+			mockDecoder:   newDecoderMock([]byte("test"), testCACertificate, nil),
+			expectedError: "",
+			expectedCerts: []*x509.Certificate{testCertificateWithURL, testCACertificate},
+			expectedOutput: []string{
+				"Fetched issuer certificate is a ROOT certificate so exiting the loop",
+			},
+		},
+		"infinite loop": {
+			certs:         []*x509.Certificate{testCertificateWithURL},
+			mockLoopLimit: 3,
+			mockFetcher:   newFetcherMock(url1, []byte("test"), nil),
+			mockDecoder:   newDecoderMock([]byte("test"), testCertificateWithURL, nil),
+			expectedError: "",
+			expectedCerts: []*x509.Certificate{testCertificateWithURL, testCertificateWithURL, testCertificateWithURL},
+			expectedOutput: []string{
+				"urlResolver loop limit exceeded; exiting the loop",
+			},
+		},
+	}
+
+	for tn, tc := range tests {
+		t.Run(tn, func(t *testing.T) {
+			out := new(bytes.Buffer)
+
+			logger := logrus.New()
+			logger.SetLevel(logrus.DebugLevel)
+			logger.SetOutput(out)
+
+			r := newURLResolver(logger).(*urlResolver)
+			r.loopLimit = tc.mockLoopLimit
+
+			if tc.mockFetcher != nil {
+				r.fetcher = tc.mockFetcher(t)
+			}
+
+			if tc.mockDecoder != nil {
+				r.decoder = tc.mockDecoder(t)
+			}
+
+			newCerts, err := r.Resolve(tc.certs)
+
+			if tc.expectedError != "" {
+				assert.EqualError(t, err, tc.expectedError)
+			} else {
+				assert.NoError(t, err)
+			}
+
+			assert.Equal(t, tc.expectedCerts, newCerts)
+
+			output := out.String()
+			if len(tc.expectedOutput) > 0 {
+				for _, expectedLine := range tc.expectedOutput {
+					assert.Contains(t, output, expectedLine)
+				}
+			} else {
+				assert.Empty(t, output)
+			}
+		})
+	}
+}

helpers/tls/ca_chain/resolver_verify.go

+package ca_chain
+
+import (
+	"crypto/x509"
+	"fmt"
+
+	"github.com/sirupsen/logrus"
+)
+
+type verifier func(cert *x509.Certificate) ([][]*x509.Certificate, error)
+
+type verifyResolver struct {
+	logger   logrus.FieldLogger
+	verifier verifier
+}
+
+func newVerifyResolver(logger logrus.FieldLogger) resolver {
+	return &verifyResolver{
+		logger:   logger,
+		verifier: verifyCertificate,
+	}
+}
+
+func (r *verifyResolver) Resolve(certs []*x509.Certificate) ([]*x509.Certificate, error) {
+	if len(certs) < 1 {
+		return certs, nil
+	}
+
+	lastCert := certs[len(certs)-1]
+
+	if isSelfSigned(lastCert) {
+		return certs, nil
+	}
+
+	prepareCertificateLogger(r.logger, lastCert).
+		Debug("Verifying last certificate to find the final root certificate")
+
+	verifyChains, err := r.verifier(lastCert)
+	if err != nil {
+		_, ok := err.(x509.UnknownAuthorityError)
+		if ok {
+			prepareCertificateLogger(r.logger, lastCert).
+				WithError(err).
+				Warning("Last certificate signed by unknown authority; will not update the chain")
+
+			return certs, nil
+		}
+
+		return nil, fmt.Errorf("error while verifying last certificate from the chain: %v", err)
+	}
+
+	for _, cert := range verifyChains[0] {
+		if lastCert.Equal(cert) {
+			continue
+		}
+
+		prepareCertificateLogger(r.logger, cert).
+			Debug("Adding cert from verify chain to the final chain")
+
+		certs = append(certs, cert)
+	}
+
+	return certs, nil
+}

helpers/tls/ca_chain/resolver_verify_test.go

+package ca_chain
+
+import (
+	"bytes"
+	"crypto/x509"
+	"errors"
+	"testing"
+
+	"github.com/sirupsen/logrus"
+	"github.com/stretchr/testify/assert"
+)
+
+type verifierMockFactory func(t *testing.T) verifier
+
+func newVerifierMock(inputCert *x509.Certificate, chain [][]*x509.Certificate, err error) verifierMockFactory {
+	return func(t *testing.T) verifier {
+		return func(cert *x509.Certificate) ([][]*x509.Certificate, error) {
+			assert.Equal(t, inputCert, cert)
+
+			return chain, err
+		}
+	}
+}
+
+func TestVerifyResolver_Resolve(t *testing.T) {
+	testError := errors.New("test-error")
+	testUnknownAuthorityError := x509.UnknownAuthorityError{}
+
+	testCACertificate := loadCertificate(t, testCACert)
+	testCertificate := loadCertificate(t, testCert)
+
+	tests := map[string]struct {
+		certs          []*x509.Certificate
+		mockVerifier   verifierMockFactory
+		expectedError  string
+		expectedCerts  []*x509.Certificate
+		expectedOutput []string
+	}{
+		"empty input chain": {
+			certs:          nil,
+			expectedError:  "",
+			expectedCerts:  nil,
+			expectedOutput: nil,
+		},
+		"last certificate is self signed": {
+			certs:          []*x509.Certificate{testCACertificate},
+			expectedError:  "",
+			expectedCerts:  []*x509.Certificate{testCACertificate},
+			expectedOutput: nil,
+		},
+		"last certificate is not self signed, verifier fails with unknown authority": {
+			certs:         []*x509.Certificate{testCertificate},
+			mockVerifier:  newVerifierMock(testCertificate, [][]*x509.Certificate{{testCACertificate}}, testUnknownAuthorityError),
+			expectedError: "",
+			expectedCerts: []*x509.Certificate{testCertificate},
+			expectedOutput: []string{
+				"Verifying last certificate to find the final root certificate",
+				"Last certificate signed by unknown authority; will not update the chain",
+			},
+		},
+		"last certificate is not self signed, verifier fails with unexpected error": {
+			certs:         []*x509.Certificate{testCertificate},
+			mockVerifier:  newVerifierMock(testCertificate, [][]*x509.Certificate{{testCACertificate}}, testError),
+			expectedError: "error while verifying last certificate from the chain: test-error",
+			expectedCerts: nil,
+			expectedOutput: []string{
+				"Verifying last certificate to find the final root certificate",
+			},
+		},
+		"last certificate is not self signed, duplicate of input certificate in verify chain": {
+			certs:         []*x509.Certificate{testCertificate},
+			mockVerifier:  newVerifierMock(testCertificate, [][]*x509.Certificate{{testCertificate, testCertificate}, {testCertificate}}, nil),
+			expectedError: "",
+			expectedCerts: []*x509.Certificate{testCertificate},
+			expectedOutput: []string{
+				"Verifying last certificate to find the final root certificate",
+			},
+		},
+		"last certificate is not self signed, other certificates in verify chain": {
+			certs:         []*x509.Certificate{testCertificate},
+			mockVerifier:  newVerifierMock(testCertificate, [][]*x509.Certificate{{testCACertificate}, {testCertificate}}, nil),
+			expectedError: "",
+			expectedCerts: []*x509.Certificate{testCertificate, testCACertificate},
+			expectedOutput: []string{
+				"Verifying last certificate to find the final root certificate",
+				"Adding cert from verify chain to the final chain",
+			},
+		},
+	}
+
+	for tn, tc := range tests {
+		t.Run(tn, func(t *testing.T) {
+			out := new(bytes.Buffer)
+
+			logger := logrus.New()
+			logger.SetLevel(logrus.DebugLevel)
+			logger.SetOutput(out)
+
+			r := newVerifyResolver(logger).(*verifyResolver)
+
+			if tc.mockVerifier != nil {
+				r.verifier = tc.mockVerifier(t)
+			}
+
+			newCerts, err := r.Resolve(tc.certs)
+
+			if tc.expectedError != "" {
+				assert.EqualError(t, err, tc.expectedError)
+			} else {
+				assert.NoError(t, err)
+			}
+
+			assert.Equal(t, tc.expectedCerts, newCerts)
+
+			output := out.String()
+			if len(tc.expectedOutput) > 0 {
+				for _, expectedLine := range tc.expectedOutput {
+					assert.Contains(t, output, expectedLine)
+				}
+			} else {
+				assert.Empty(t, output)
+			}
+		})
+	}
+}