[FAB-10568] refactoring endpointConfig.TLSCACertPool

- updated TLSCACertPool() signature to return fab
CertPool implementation instead of returning x509 certpool
and error



Change-Id: I409f416c96bbe9c61b54459031db0e3bc0ba255b
Signed-off-by: Sudesh Shetty <sudesh.shetty@securekey.com>

pkg/common/providers/fab/provider.go

@@ -87,7 +87,7 @@ type EndpointConfig interface {
 	ChannelConfig(name string) (*ChannelEndpointConfig, bool)
 	ChannelPeers(name string) ([]ChannelPeer, bool)
 	ChannelOrderers(name string) ([]OrdererConfig, bool)
-	TLSCACertPool(certConfig ...*x509.Certificate) (*x509.CertPool, error)
+	TLSCACertPool() CertPool
 	EventServiceType() EventServiceType
 	TLSClientCerts() []tls.Certificate
 	CryptoConfigPath() string
@@ -156,3 +156,10 @@ type Providers interface {
 	InfraProvider() InfraProvider
 	EndpointConfig() EndpointConfig
 }
+
+// CertPool is a thread safe wrapper around the x509 standard library
+// cert pool implementation.
+type CertPool interface {
+	// Get returns the cert pool, optionally adding the provided certs
+	Get(certs ...*x509.Certificate) (*x509.CertPool, error)
+}

pkg/common/providers/test/mockfab/mockconfig.go

@@ -35,9 +35,8 @@ const ErrorMessage = "default error message"
 func DefaultMockConfig(mockCtrl *gomock.Controller) *MockEndpointConfig {
 	config := NewMockEndpointConfig(mockCtrl)
 
-	config.EXPECT().TLSCACertPool(GoodCert).Return(CertPool, nil).AnyTimes()
-	config.EXPECT().TLSCACertPool(BadCert).Return(CertPool, errors.New(ErrorMessage)).AnyTimes()
-	config.EXPECT().TLSCACertPool().Return(CertPool, nil).AnyTimes()
+	config.EXPECT().TLSCACertPool().Return(&MockCertPool{CertPool: CertPool}).AnyTimes()
+
 	config.EXPECT().Timeout(fab.EndorserConnection).Return(time.Second * 5).AnyTimes()
 	config.EXPECT().TLSClientCerts().Return([]tls.Certificate{TLSCert}).AnyTimes()
 
@@ -48,11 +47,20 @@ func DefaultMockConfig(mockCtrl *gomock.Controller) *MockEndpointConfig {
 func BadTLSClientMockConfig(mockCtrl *gomock.Controller) *MockEndpointConfig {
 	config := NewMockEndpointConfig(mockCtrl)
 
-	config.EXPECT().TLSCACertPool(GoodCert).Return(CertPool, nil).AnyTimes()
-	config.EXPECT().TLSCACertPool(BadCert).Return(CertPool, errors.New(ErrorMessage)).AnyTimes()
-	config.EXPECT().TLSCACertPool().Return(CertPool, nil).AnyTimes()
+	config.EXPECT().TLSCACertPool().Return(&MockCertPool{Err: errors.New(ErrorMessage)}).AnyTimes()
 	config.EXPECT().Timeout(fab.EndorserConnection).Return(time.Second * 5).AnyTimes()
 	config.EXPECT().TLSClientCerts().Return(nil).AnyTimes()
 
 	return config
 }
+
+//MockCertPool for unit tests to mock CertPool
+type MockCertPool struct {
+	CertPool *x509.CertPool
+	Err      error
+}
+
+//Get mock implementation of fab CertPool.Get()
+func (c *MockCertPool) Get(certs ...*x509.Certificate) (*x509.CertPool, error) {
+	return c.CertPool, c.Err
+}

pkg/core/config/comm/comm.go

@@ -18,7 +18,7 @@ import (
 // TLSConfig returns the appropriate config for TLS including the root CAs,
 // certs for mutual TLS, and server host override. Works with certs loaded either from a path or embedded pem.
 func TLSConfig(cert *x509.Certificate, serverName string, config fab.EndpointConfig) (*tls.Config, error) {
-	certPool, err := config.TLSCACertPool()
+	certPool, err := config.TLSCACertPool().Get()
 	if err != nil {
 		return nil, err
 	}
@@ -28,8 +28,7 @@ func TLSConfig(cert *x509.Certificate, serverName string, config fab.EndpointCon
 		return &tls.Config{}, nil
 	}
 
-	tlsCaCertPool, err := config.TLSCACertPool(cert)
-
+	tlsCaCertPool, err := config.TLSCACertPool().Get(cert)
 	if err != nil {
 		return nil, err
 	}

pkg/core/config/comm/comm_test.go

@@ -25,7 +25,7 @@ func TestTLSConfigErrorAddingCertificate(t *testing.T) {
 	mockCtrl := gomock.NewController(t)
 	defer mockCtrl.Finish()
 
-	config := mockfab.DefaultMockConfig(mockCtrl)
+	config := mockfab.BadTLSClientMockConfig(mockCtrl)
 
 	_, err := TLSConfig(mockfab.BadCert, "", config)
 	if err == nil {

pkg/core/config/comm/tls/certpool.go

@@ -11,17 +11,11 @@ import (
 	"sync"
 
 	"github.com/hyperledger/fabric-sdk-go/pkg/common/logging"
+	"github.com/hyperledger/fabric-sdk-go/pkg/common/providers/fab"
 )
 
 var logger = logging.NewLogger("fabsdk/core")
 
-// CertPool is a thread safe wrapper around the x509 standard library
-// cert pool implementation.
-type CertPool interface {
-	// Get returns the cert pool, optionally adding the provided certs
-	Get(certs ...*x509.Certificate) (*x509.CertPool, error)
-}
-
 // certPool is a thread safe wrapper around the x509 standard library
 // cert pool implementation.
 // It optionally allows loading the system trust store.
@@ -34,7 +28,7 @@ type certPool struct {
 }
 
 // NewCertPool new CertPool implementation
-func NewCertPool(useSystemCertPool bool) CertPool {
+func NewCertPool(useSystemCertPool bool) fab.CertPool {
 	return &certPool{
 		useSystemCertPool: useSystemCertPool,
 		certsByName:       make(map[string][]int),

pkg/fab/api.go

@@ -11,20 +11,6 @@ import (
 	"github.com/hyperledger/fabric-sdk-go/pkg/core/config/endpoint"
 )
 
-//endpointConfigEntity contains endpoint config elements needed by endpointconfig
-type endpointConfigEntity struct {
-	Client        ClientConfig
-	Channels      map[string]ChannelEndpointConfig
-	Organizations map[string]OrganizationConfig
-	Orderers      map[string]OrdererConfig
-	Peers         map[string]PeerConfig
-}
-
-//entityMatchers for endpoint configuration
-type entityMatchers struct {
-	matchers map[string][]MatchConfig
-}
-
 // ClientConfig provides the definition of the client configuration
 type ClientConfig struct {
 	Organization string

pkg/fab/channel/membership/membership.go

@@ -208,7 +208,7 @@ func addCertsToConfig(config fab.EndpointConfig, pemCertsList [][]byte) {
 			certs = append(certs, cert)
 		}
 	}
-	_, err := config.TLSCACertPool(certs...)
+	_, err := config.TLSCACertPool().Get(certs...)
 	if err != nil {
 		logger.Warnf("TLSCACertPool failed %s", err)
 	}

pkg/fab/endpointconfig.go

@@ -68,18 +68,6 @@ func ConfigFromBackend(coreBackend ...core.ConfigBackend) (fab.EndpointConfig, e
 		return nil, errors.WithMessage(err, "network configuration load failed")
 	}
 
-	config.tlsCertPool = commtls.NewCertPool(config.backend.GetBool("client.tlsCerts.systemCertPool"))
-
-	// preemptively add all TLS certs to cert pool as adding them at request time
-	// is expensive
-	certs, err := config.loadTLSCerts()
-	if err != nil {
-		logger.Infof("could not cache TLS certs: %s", err)
-	}
-	if _, err := config.TLSCACertPool(certs...); err != nil {
-		return nil, errors.WithMessage(err, "cert pool load failed")
-	}
-
 	//print deprecated warning
 	detectDeprecatedNetworkConfig(config)
 
@@ -90,7 +78,7 @@ func ConfigFromBackend(coreBackend ...core.ConfigBackend) (fab.EndpointConfig, e
 type EndpointConfig struct {
 	backend                  *lookup.ConfigLookup
 	networkConfig            *fab.NetworkConfig
-	tlsCertPool              commtls.CertPool
+	tlsCertPool              fab.CertPool
 	entityMatchers           *entityMatchers
 	peerConfigsByOrg         map[string][]fab.PeerConfig
 	networkPeers             []fab.NetworkPeer
@@ -103,6 +91,20 @@ type EndpointConfig struct {
 	channelMatchers          map[int]*regexp.Regexp
 }
 
+//endpointConfigEntity contains endpoint config elements needed by endpointconfig
+type endpointConfigEntity struct {
+	Client        ClientConfig
+	Channels      map[string]ChannelEndpointConfig
+	Organizations map[string]OrganizationConfig
+	Orderers      map[string]OrdererConfig
+	Peers         map[string]PeerConfig
+}
+
+//entityMatchers for endpoint configuration
+type entityMatchers struct {
+	matchers map[string][]MatchConfig
+}
+
 // Timeout reads timeouts for the given timeout type, if type is not found in the config
 // then default is set as per the const value above for the corresponding type
 func (c *EndpointConfig) Timeout(tType fab.TimeoutType) time.Duration {
@@ -276,8 +278,8 @@ func (c *EndpointConfig) ChannelOrderers(name string) ([]fab.OrdererConfig, bool
 
 // TLSCACertPool returns the configured cert pool. If a certConfig
 // is provided, the certificate is added to the pool
-func (c *EndpointConfig) TLSCACertPool(certs ...*x509.Certificate) (*x509.CertPool, error) {
-	return c.tlsCertPool.Get(certs...)
+func (c *EndpointConfig) TLSCACertPool() fab.CertPool {
+	return c.tlsCertPool
 }
 
 // EventServiceType returns the type of event service client to use
@@ -503,6 +505,12 @@ func (c *EndpointConfig) loadEndpointConfigEntities(configEntity *endpointConfig
 		return errors.WithMessage(err, "failed to load channel orderers")
 	}
 
+	//load tls cert pool
+	err = c.loadTLSCertPool()
+	if err != nil {
+		return errors.WithMessage(err, "failed to load TLS cert pool")
+	}
+
 	return nil
 }
 
@@ -845,6 +853,23 @@ func (c *EndpointConfig) loadChannelOrderers() error {
 	return nil
 }
 
+func (c *EndpointConfig) loadTLSCertPool() error {
+
+	c.tlsCertPool = commtls.NewCertPool(c.backend.GetBool("client.tlsCerts.systemCertPool"))
+
+	// preemptively add all TLS certs to cert pool as adding them at request time
+	// is expensive
+	certs, err := c.loadTLSCerts()
+	if err != nil {
+		logger.Infof("could not cache TLS certs: %s", err)
+	}
+
+	if _, err := c.tlsCertPool.Get(certs...); err != nil {
+		return errors.WithMessage(err, "cert pool load failed")
+	}
+	return nil
+}
+
 // loadTLSClientCerts loads the client's certs for mutual TLS
 // It checks the config for embedded pem files before looking for cert files
 func (c *EndpointConfig) loadTLSClientCerts(configEntity *endpointConfigEntity) error {

pkg/fab/endpointconfig_test.go

@@ -662,7 +662,7 @@ func TestSystemCertPoolDisabled(t *testing.T) {
 		t.Fatal("Failed to get endpoint config from backend")
 	}
 
-	_, err = endpointConfig.TLSCACertPool()
+	_, err = endpointConfig.TLSCACertPool().Get()
 	if err != nil {
 		t.Fatal("not supposed to get error")
 	}

pkg/fab/mocks/mockconfig.go

@@ -8,14 +8,16 @@ package mocks
 
 import (
 	"crypto/tls"
-	"crypto/x509"
 	"time"
 
 	"github.com/hyperledger/fabric-sdk-go/pkg/common/providers/core"
 	"github.com/hyperledger/fabric-sdk-go/pkg/core/config/endpoint"
 
+	"crypto/x509"
+
 	"github.com/hyperledger/fabric-sdk-go/pkg/common/providers/fab"
 	"github.com/hyperledger/fabric-sdk-go/pkg/common/providers/msp"
+	"github.com/hyperledger/fabric-sdk-go/pkg/common/providers/test/mockfab"
 	"github.com/pkg/errors"
 )
 
@@ -140,11 +142,11 @@ func (c *MockConfig) PeerConfig(nameOrURL string) (*fab.PeerConfig, bool) {
 }
 
 // TLSCACertPool ...
-func (c *MockConfig) TLSCACertPool(cert ...*x509.Certificate) (*x509.CertPool, error) {
+func (c *MockConfig) TLSCACertPool() fab.CertPool {
 	if c.errorCase {
-		return nil, errors.New("just to test error scenario")
+		return &mockfab.MockCertPool{Err: errors.New("just to test error scenario")}
 	}
-	return nil, nil
+	return &mockfab.MockCertPool{CertPool: x509.NewCertPool()}
 }
 
 // TcertBatchSize ...

pkg/fab/opts.go

@@ -8,7 +8,6 @@ package fab
 
 import (
 	"crypto/tls"
-	"crypto/x509"
 	"time"
 
 	"github.com/hyperledger/fabric-sdk-go/pkg/common/providers/fab"
@@ -90,7 +89,7 @@ type channelOrderers interface {
 
 // tlsCACertPool interface allows to uniquely override EndpointConfig interface's TLSCACertPool() function
 type tlsCACertPool interface {
-	TLSCACertPool(certConfig ...*x509.Certificate) (*x509.CertPool, error)
+	TLSCACertPool() fab.CertPool
 }
 
 // eventServiceType interface allows to uniquely override EndpointConfig interface's EventServiceType() function

pkg/fab/opts_test.go

@@ -8,7 +8,6 @@ package fab
 
 import (
 	"crypto/tls"
-	"crypto/x509"
 	"testing"
 	"time"
 
@@ -295,8 +294,8 @@ func (m *mockChannelOrderers) ChannelOrderers(name string) ([]fab.OrdererConfig,
 
 type mockTLSCACertPool struct{}
 
-func (m *mockTLSCACertPool) TLSCACertPool(certConfig ...*x509.Certificate) (*x509.CertPool, error) {
-	return nil, nil
+func (m *mockTLSCACertPool) TLSCACertPool() fab.CertPool {
+	return nil
 }
 
 type mockEventServiceType struct{}

pkg/fab/orderer/orderer_test.go

@@ -360,7 +360,7 @@ func TestBroadcastBadDial(t *testing.T) {
 	config := mockfab.NewMockEndpointConfig(mockCtrl)
 
 	config.EXPECT().Timeout(fab.OrdererConnection).Return(time.Second * 1)
-	config.EXPECT().TLSCACertPool(gomock.Any()).Return(x509.NewCertPool(), nil).AnyTimes()
+	config.EXPECT().TLSCACertPool().Return(&mockfab.MockCertPool{CertPool: x509.NewCertPool()}).AnyTimes()
 
 	orderer, err := New(config, WithURL("grpc://127.0.0.1:0"))
 	assert.Nil(t, err)

pkg/fab/peer/peer_test.go

@@ -15,7 +15,6 @@ import (
 	"github.com/golang/mock/gomock"
 	"github.com/hyperledger/fabric-sdk-go/pkg/common/providers/fab"
 	"github.com/hyperledger/fabric-sdk-go/pkg/common/providers/test/mockfab"
-	"github.com/pkg/errors"
 )
 
 const (
@@ -77,8 +76,7 @@ func TestNewPeerTLSFromCertBad(t *testing.T) {
 	mockCtrl := gomock.NewController(t)
 	defer mockCtrl.Finish()
 
-	config := mockfab.NewMockEndpointConfig(mockCtrl)
-	config.EXPECT().TLSCACertPool(gomock.Any()).Return(nil, errors.New("failed to get certpool")).AnyTimes()
+	config := mockfab.BadTLSClientMockConfig(mockCtrl)
 
 	url := "grpcs://0.0.0.0:1234"
 	_, err := New(config, WithURL(url))

pkg/fab/peer/peerendorser_test.go

@@ -114,7 +114,7 @@ func TestNewPeerEndorserTLSBadPool(t *testing.T) {
 	mockCtrl := gomock.NewController(t)
 	defer mockCtrl.Finish()
 
-	config := mockfab.DefaultMockConfig(mockCtrl)
+	config := mockfab.BadTLSClientMockConfig(mockCtrl)
 
 	url := "grpcs://0.0.0.0:1234"
 	_, err := newPeerEndorser(getPeerEndorserRequest(url, mockfab.BadCert, "", config, kap, false, false))