[FABG-961] Use system cert pool when connecting to fabric-ca (#69)

When client.tlsCerts.systemCertPool is set to true,
TLS connections to fabric-ca instances now use system
cert pool. There is no need to have tlsCACerts.path
defined in CAs' configurations in that case.

Signed-off-by: Aleksandar Likic <aleksandar.likic@securekey.com>

internal/github.com/hyperledger/fabric-ca/lib/tls/tls.go

@@ -45,9 +45,10 @@ var DefaultCipherSuites = []uint16{
 
 // ClientTLSConfig defines the key material for a TLS client
 type ClientTLSConfig struct {
-	Enabled   bool     `skip:"true"`
-	CertFiles [][]byte `help:"A list of comma-separated PEM-encoded trusted certificate bytes"`
-	Client    KeyCertFiles
+	Enabled     bool     `skip:"true"`
+	CertFiles   [][]byte `help:"A list of comma-separated PEM-encoded trusted certificate bytes"`
+	Client      KeyCertFiles
+	TlsCertPool *x509.CertPool
 }
 
 // KeyCertFiles defines the files need for client on TLS
@@ -79,15 +80,18 @@ func GetClientTLSConfig(cfg *ClientTLSConfig, csp core.CryptoSuite) (*tls.Config
 	} else {
 		log.Debug("Client TLS certificate and/or key file not provided")
 	}
-	rootCAPool := x509.NewCertPool()
-	if len(cfg.CertFiles) == 0 {
-		return nil, errors.New("No trusted root certificates for TLS were provided")
-	}
+	rootCAPool := cfg.TlsCertPool
+	if rootCAPool == nil {
+		rootCAPool = x509.NewCertPool()
+		if len(cfg.CertFiles) == 0 {
+			return nil, errors.New("No trusted root certificates for TLS were provided")
+		}
 
-	for _, cacert := range cfg.CertFiles {
-		ok := rootCAPool.AppendCertsFromPEM(cacert)
-		if !ok {
-			return nil, errors.New("Failed to process certificate")
+		for _, cacert := range cfg.CertFiles {
+			ok := rootCAPool.AppendCertsFromPEM(cacert)
+			if !ok {
+				return nil, errors.New("Failed to process certificate")
+			}
 		}
 	}
 

pkg/common/providers/fab/provider.go

@@ -9,12 +9,12 @@ package fab
 import (
 	reqContext "context"
 	"crypto/tls"
-	"crypto/x509"
 	"time"
 
 	"github.com/hyperledger/fabric-sdk-go/pkg/common/options"
 	"github.com/hyperledger/fabric-sdk-go/pkg/common/providers/core"
 	"github.com/hyperledger/fabric-sdk-go/pkg/common/providers/msp"
+	commtls "github.com/hyperledger/fabric-sdk-go/pkg/core/config/comm/tls"
 	"github.com/hyperledger/fabric-sdk-go/pkg/fabsdk/metrics"
 	"google.golang.org/grpc"
 )
@@ -101,7 +101,7 @@ type EndpointConfig interface {
 	ChannelConfig(name string) *ChannelEndpointConfig
 	ChannelPeers(name string) []ChannelPeer
 	ChannelOrderers(name string) []OrdererConfig
-	TLSCACertPool() CertPool
+	TLSCACertPool() commtls.CertPool
 	TLSClientCerts() []tls.Certificate
 	CryptoConfigPath() string
 }
@@ -157,16 +157,6 @@ type Providers interface {
 	MetricsProvider
 }
 
-// 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() (*x509.CertPool, error)
-	//Add allows adding certificates to CertPool
-	//Call Get() after Add() to get the updated certpool
-	Add(certs ...*x509.Certificate)
-}
-
 // MetricsProvider represents a provider of metrics.
 type MetricsProvider interface {
 	GetMetrics() *metrics.ClientMetrics

pkg/common/providers/msp/provider.go

@@ -8,6 +8,7 @@ package msp
 
 import (
 	"github.com/hyperledger/fabric-sdk-go/pkg/common/providers/core"
+	commtls "github.com/hyperledger/fabric-sdk-go/pkg/core/config/comm/tls"
 	logApi "github.com/hyperledger/fabric-sdk-go/pkg/core/logging/api"
 )
 
@@ -29,6 +30,7 @@ type IdentityConfig interface {
 	CAServerCerts(caID string) ([][]byte, bool)
 	CAClientKey(caID string) ([]byte, bool)
 	CAClientCert(caID string) ([]byte, bool)
+	TLSCACertPool() commtls.CertPool
 	CAKeyStorePath() string
 	CredentialStorePath() string
 }

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

@@ -12,11 +12,20 @@ import (
 	"sync/atomic"
 
 	"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() (*x509.CertPool, error)
+	//Add allows adding certificates to CertPool
+	//Call Get() after Add() to get the updated certpool
+	Add(certs ...*x509.Certificate)
+}
+
 // certPool is a thread safe wrapper around the x509 standard library
 // cert pool implementation.
 // It optionally allows loading the system trust store.
@@ -30,7 +39,7 @@ type certPool struct {
 }
 
 // NewCertPool new CertPool implementation
-func NewCertPool(useSystemCertPool bool) (fab.CertPool, error) {
+func NewCertPool(useSystemCertPool bool) (CertPool, error) {
 
 	c, err := loadSystemCertPool(useSystemCertPool)
 	if err != nil {

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

@@ -16,8 +16,6 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-
-	"github.com/hyperledger/fabric-sdk-go/pkg/common/providers/fab"
 )
 
 var goodCert = &x509.Certificate{
@@ -147,7 +145,7 @@ func TestTLSCAConfigWithMultipleCerts(t *testing.T) {
 
 }
 
-func verifyCertPoolInstance(t *testing.T, pool *x509.CertPool, fabPool fab.CertPool, numberOfCertsInPool, numberOfCerts, numberOfCertsByName, numberOfSubjects int, dirty int32) {
+func verifyCertPoolInstance(t *testing.T, pool *x509.CertPool, fabPool CertPool, numberOfCertsInPool, numberOfCerts, numberOfCertsByName, numberOfSubjects int, dirty int32) {
 	assert.NotNil(t, fabPool)
 	tlsCertPool := fabPool.(*certPool)
 	assert.Equal(t, dirty, tlsCertPool.dirty)

pkg/core/mocks/mockconfigbackend.go

@@ -6,6 +6,12 @@ SPDX-License-Identifier: Apache-2.0
 
 package mocks
 
+import (
+	"strings"
+
+	"github.com/hyperledger/fabric-sdk-go/pkg/core/config"
+)
+
 //MockConfigBackend mocks config backend for unit tests
 type MockConfigBackend struct {
 	//KeyValueMap map to override CustomBackend key-values.
@@ -17,3 +23,68 @@ func (b *MockConfigBackend) Lookup(key string) (interface{}, bool) {
 	v, ok := b.KeyValueMap[key]
 	return v, ok
 }
+
+// Set sets a backend value
+func (b *MockConfigBackend) Set(key string, value interface{}) bool {
+	if key != "" {
+		node := b.KeyValueMap
+		ss := strings.Split(key, ".")
+		for i, s := range ss {
+			if i == len(ss)-1 {
+				node[s] = value
+				return true
+			}
+			v, ok := node[s]
+			if !ok {
+				node[s] = map[string]interface{}{}
+				node = node[s].(map[string]interface{})
+			} else {
+				node, ok = v.(map[string]interface{})
+				if !ok {
+					return false
+				}
+			}
+
+		}
+	}
+	return false
+}
+
+// Get returns a value from the backend
+func (b *MockConfigBackend) Get(key string) (interface{}, bool) {
+	if key != "" {
+		node := b.KeyValueMap
+		ss := strings.Split(key, ".")
+		for i, s := range ss {
+			v, ok := node[s]
+			if i == len(ss)-1 {
+				return v, ok
+			}
+			if !ok {
+				return nil, ok
+			}
+			node = v.(map[string]interface{})
+		}
+	}
+	return nil, false
+}
+
+// BackendFromFile returns MockConfigBackend populated from file
+func BackendFromFile(configPath string) (*MockConfigBackend, error) {
+	b, err := config.FromFile(configPath)()
+	if err != nil {
+		return nil, err
+	}
+	configBackend := b[0]
+
+	backendMap := make(map[string]interface{})
+	backendMap["client"], _ = configBackend.Lookup("client")
+	backendMap["certificateAuthorities"], _ = configBackend.Lookup("certificateAuthorities")
+	backendMap["entityMatchers"], _ = configBackend.Lookup("entityMatchers")
+	backendMap["peers"], _ = configBackend.Lookup("peers")
+	backendMap["organizations"], _ = configBackend.Lookup("organizations")
+	backendMap["orderers"], _ = configBackend.Lookup("orderers")
+	backendMap["channels"], _ = configBackend.Lookup("channels")
+
+	return &MockConfigBackend{KeyValueMap: backendMap}, nil
+}

pkg/core/mocks/mockconfigbackend_test.go

@@ -0,0 +1,55 @@
+/*
+Copyright SecureKey Technologies Inc. All Rights Reserved.
+
+SPDX-License-Identifier: Apache-2.0
+*/
+
+package mocks
+
+import (
+	"path/filepath"
+	"reflect"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	"github.com/hyperledger/fabric-sdk-go/test/metadata"
+)
+
+func getConfigPath() string {
+	return filepath.Join(metadata.GetProjectPath(), "pkg", "core", "config", "testdata")
+}
+
+func TestMockConfigBackend(t *testing.T) {
+
+	configPath := filepath.Join(getConfigPath(), "config_test.yaml")
+	mockBackend, err := BackendFromFile(configPath)
+	if err != nil {
+		t.Fatalf("Unexpected error reading config: %s", err)
+	}
+
+	v, ok := mockBackend.Get("client")
+	assert.True(t, ok, "!ok")
+	assert.NotNil(t, v, "client not found")
+	assert.True(t, reflect.TypeOf(v) == reflect.TypeOf(map[string]interface{}{}), "wrong type")
+
+	v, ok = mockBackend.Get("client.tlscerts.systemcertpool")
+	assert.True(t, ok, "!ok")
+	assert.True(t, reflect.TypeOf(v) == reflect.TypeOf(true), "wrong type")
+	assert.False(t, v.(bool), "wrong value")
+
+	mockBackend.Set("client.tlscerts.systemcertpool", true)
+	v, ok = mockBackend.Get("client.tlscerts.systemcertpool")
+	assert.True(t, ok, "!ok")
+	assert.True(t, reflect.TypeOf(v) == reflect.TypeOf(true), "wrong type")
+	assert.True(t, v.(bool), "wrong value")
+
+	v, ok = mockBackend.Get("some.new.value")
+	assert.False(t, ok, "should not be ok")
+	mockBackend.Set("some.new.value", "Hello World")
+	v, ok = mockBackend.Get("some.new.value")
+	assert.True(t, ok, "!ok")
+	assert.True(t, reflect.TypeOf(v) == reflect.TypeOf(""), "wrong type")
+	assert.Equal(t, "Hello World", v)
+
+}

pkg/fab/endpointconfig.go

@@ -144,7 +144,7 @@ func ConfigFromBackend(coreBackend ...core.ConfigBackend) (fab.EndpointConfig, e
 type EndpointConfig struct {
 	backend                  *lookup.ConfigLookup
 	networkConfig            *fab.NetworkConfig
-	tlsCertPool              fab.CertPool
+	tlsCertPool              commtls.CertPool
 	entityMatchers           *entityMatchers
 	peerConfigsByOrg         map[string][]fab.PeerConfig
 	networkPeers             []fab.NetworkPeer
@@ -286,7 +286,7 @@ func (c *EndpointConfig) ChannelOrderers(name string) []fab.OrdererConfig {
 
 // TLSCACertPool returns the configured cert pool. If a certConfig
 // is provided, the certificate is added to the pool
-func (c *EndpointConfig) TLSCACertPool() fab.CertPool {
+func (c *EndpointConfig) TLSCACertPool() commtls.CertPool {
 	return c.tlsCertPool
 }
 

pkg/fab/mocks/mockconfig.go

@@ -20,6 +20,7 @@ import (
 	"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"
+	commtls "github.com/hyperledger/fabric-sdk-go/pkg/core/config/comm/tls"
 	"github.com/pkg/errors"
 )
 
@@ -32,7 +33,7 @@ type MockConfig struct {
 	customPeerCfg          *fab.PeerConfig
 	customOrdererCfg       *fab.OrdererConfig
 	customRandomOrdererCfg *fab.OrdererConfig
-	CustomTLSCACertPool    fab.CertPool
+	CustomTLSCACertPool    commtls.CertPool
 	chConfig               map[string]*fab.ChannelEndpointConfig
 }
 
@@ -149,7 +150,7 @@ func (c *MockConfig) PeerConfig(nameOrURL string) (*fab.PeerConfig, bool) {
 }
 
 // TLSCACertPool ...
-func (c *MockConfig) TLSCACertPool() fab.CertPool {
+func (c *MockConfig) TLSCACertPool() commtls.CertPool {
 	if c.errorCase {
 		return &mockfab.MockCertPool{Err: errors.New("just to test error scenario")}
 	} else if c.CustomTLSCACertPool != nil {

pkg/fab/opts.go

@@ -13,6 +13,7 @@ import (
 	"github.com/pkg/errors"
 
 	"github.com/hyperledger/fabric-sdk-go/pkg/common/providers/fab"
+	commtls "github.com/hyperledger/fabric-sdk-go/pkg/core/config/comm/tls"
 )
 
 // EndpointConfigOptions represents EndpointConfig interface with overridable interface functions
@@ -89,7 +90,7 @@ type channelOrderers interface {
 
 // tlsCACertPool interface allows to uniquely override EndpointConfig interface's TLSCACertPool() function
 type tlsCACertPool interface {
-	TLSCACertPool() fab.CertPool
+	TLSCACertPool() commtls.CertPool
 }
 
 // tlsClientCerts interface allows to uniquely override EndpointConfig interface's TLSClientCerts() function