comm: move Certificate to SecureOptions#ClientCert

Signed-off-by: Matthew Sykes <[email protected]>

core/deliverservice/deliveryclient.go

@@ -140,7 +140,11 @@ func (d *deliverServiceImpl) StartDeliverForChannel(chainID string, ledgerInfo b
 	}
 
 	if d.conf.DeliverServiceConfig.SecOpts.RequireClientCert {
-		dc.TLSCertHash = util.ComputeSHA256(d.conf.DeliverGRPCClient.Certificate().Certificate[0])
+		cert, err := d.conf.DeliverServiceConfig.SecOpts.ClientCertificate()
+		if err != nil {
+			return fmt.Errorf("failed to access client TLS configuration: %w", err)
+		}
+		dc.TLSCertHash = util.ComputeSHA256(cert.Certificate[0])
 	}
 
 	d.blockProviders[chainID] = dc

internal/peer/common/common.go

@@ -83,8 +83,33 @@ var (
 
 type CommonClient struct {
 	*comm.GRPCClient
-	Address string
-	sn      string
+	clientConfig comm.ClientConfig
+	address      string
+	sn           string
+}
+
+func newCommonClient(address, override string, clientConfig comm.ClientConfig) (*CommonClient, error) {
+	gClient, err := comm.NewGRPCClient(clientConfig)
+	if err != nil {
+		return nil, err
+	}
+	return &CommonClient{
+		GRPCClient:   gClient,
+		clientConfig: clientConfig,
+		address:      address,
+		sn:           override,
+	}, nil
+}
+
+func (cc *CommonClient) Certificate() tls.Certificate {
+	if !cc.clientConfig.SecOpts.RequireClientCert {
+		return tls.Certificate{}
+	}
+	cert, err := cc.clientConfig.SecOpts.ClientCertificate()
+	if err != nil {
+		panic(err)
+	}
+	return cert
 }
 
 func init() {

internal/peer/common/ordererclient.go

@@ -17,7 +17,7 @@ import (
 // OrdererClient represents a client for communicating with an ordering
 // service
 type OrdererClient struct {
-	CommonClient
+	*CommonClient
 }
 
 // NewOrdererClientFromEnv creates an instance of an OrdererClient from the
@@ -27,35 +27,28 @@ func NewOrdererClientFromEnv() (*OrdererClient, error) {
 	if err != nil {
 		return nil, errors.WithMessage(err, "failed to load config for OrdererClient")
 	}
-	gClient, err := comm.NewGRPCClient(clientConfig)
+	cc, err := newCommonClient(address, override, clientConfig)
 	if err != nil {
 		return nil, errors.WithMessage(err, "failed to create OrdererClient from config")
 	}
-	oClient := &OrdererClient{
-		CommonClient: CommonClient{
-			GRPCClient: gClient,
-			Address:    address,
-			sn:         override,
-		},
-	}
-	return oClient, nil
+	return &OrdererClient{CommonClient: cc}, nil
 }
 
 // Broadcast returns a broadcast client for the AtomicBroadcast service
 func (oc *OrdererClient) Broadcast() (ab.AtomicBroadcast_BroadcastClient, error) {
-	conn, err := oc.CommonClient.NewConnection(oc.Address, comm.ServerNameOverride(oc.sn))
+	conn, err := oc.CommonClient.NewConnection(oc.address, comm.ServerNameOverride(oc.sn))
 	if err != nil {
-		return nil, errors.WithMessagef(err, "orderer client failed to connect to %s", oc.Address)
+		return nil, errors.WithMessagef(err, "orderer client failed to connect to %s", oc.address)
 	}
 	// TODO: check to see if we should actually handle error before returning
 	return ab.NewAtomicBroadcastClient(conn).Broadcast(context.TODO())
 }
 
 // Deliver returns a deliver client for the AtomicBroadcast service
 func (oc *OrdererClient) Deliver() (ab.AtomicBroadcast_DeliverClient, error) {
-	conn, err := oc.CommonClient.NewConnection(oc.Address, comm.ServerNameOverride(oc.sn))
+	conn, err := oc.CommonClient.NewConnection(oc.address, comm.ServerNameOverride(oc.sn))
 	if err != nil {
-		return nil, errors.WithMessagef(err, "orderer client failed to connect to %s", oc.Address)
+		return nil, errors.WithMessagef(err, "orderer client failed to connect to %s", oc.address)
 	}
 	// TODO: check to see if we should actually handle error before returning
 	return ab.NewAtomicBroadcastClient(conn).Deliver(context.TODO())

internal/peer/common/peerclient.go

@@ -20,7 +20,7 @@ import (
 
 // PeerClient represents a client for communicating with a peer
 type PeerClient struct {
-	CommonClient
+	*CommonClient
 }
 
 // NewPeerClientFromEnv creates an instance of a PeerClient from the global
@@ -78,44 +78,37 @@ func NewPeerClientForAddress(address, tlsRootCertFile string) (*PeerClient, erro
 func newPeerClientForClientConfig(address, override string, clientConfig comm.ClientConfig) (*PeerClient, error) {
 	// set the default keepalive options to match the server
 	clientConfig.KaOpts = comm.DefaultKeepaliveOptions
-	gClient, err := comm.NewGRPCClient(clientConfig)
+	cc, err := newCommonClient(address, override, clientConfig)
 	if err != nil {
 		return nil, errors.WithMessage(err, "failed to create PeerClient from config")
 	}
-	pClient := &PeerClient{
-		CommonClient: CommonClient{
-			GRPCClient: gClient,
-			Address:    address,
-			sn:         override,
-		},
-	}
-	return pClient, nil
+	return &PeerClient{CommonClient: cc}, nil
 }
 
 // Endorser returns a client for the Endorser service
 func (pc *PeerClient) Endorser() (pb.EndorserClient, error) {
-	conn, err := pc.CommonClient.NewConnection(pc.Address, comm.ServerNameOverride(pc.sn))
+	conn, err := pc.CommonClient.NewConnection(pc.address, comm.ServerNameOverride(pc.sn))
 	if err != nil {
-		return nil, errors.WithMessagef(err, "endorser client failed to connect to %s", pc.Address)
+		return nil, errors.WithMessagef(err, "endorser client failed to connect to %s", pc.address)
 	}
 	return pb.NewEndorserClient(conn), nil
 }
 
 // Deliver returns a client for the Deliver service
 func (pc *PeerClient) Deliver() (pb.Deliver_DeliverClient, error) {
-	conn, err := pc.CommonClient.NewConnection(pc.Address, comm.ServerNameOverride(pc.sn))
+	conn, err := pc.CommonClient.NewConnection(pc.address, comm.ServerNameOverride(pc.sn))
 	if err != nil {
-		return nil, errors.WithMessagef(err, "deliver client failed to connect to %s", pc.Address)
+		return nil, errors.WithMessagef(err, "deliver client failed to connect to %s", pc.address)
 	}
 	return pb.NewDeliverClient(conn).Deliver(context.TODO())
 }
 
 // PeerDeliver returns a client for the Deliver service for peer-specific use
 // cases (i.e. DeliverFiltered)
 func (pc *PeerClient) PeerDeliver() (pb.DeliverClient, error) {
-	conn, err := pc.CommonClient.NewConnection(pc.Address, comm.ServerNameOverride(pc.sn))
+	conn, err := pc.CommonClient.NewConnection(pc.address, comm.ServerNameOverride(pc.sn))
 	if err != nil {
-		return nil, errors.WithMessagef(err, "deliver client failed to connect to %s", pc.Address)
+		return nil, errors.WithMessagef(err, "deliver client failed to connect to %s", pc.address)
 	}
 	return pb.NewDeliverClient(conn), nil
 }
@@ -181,9 +174,9 @@ func GetPeerDeliverClient(address, tlsRootCertFile string) (pb.DeliverClient, er
 
 // SnapshotClient returns a client for the snapshot service
 func (pc *PeerClient) SnapshotClient() (pb.SnapshotClient, error) {
-	conn, err := pc.CommonClient.NewConnection(pc.Address, comm.ServerNameOverride(pc.sn))
+	conn, err := pc.CommonClient.NewConnection(pc.address, comm.ServerNameOverride(pc.sn))
 	if err != nil {
-		return nil, errors.WithMessagef(err, "snapshot client failed to connect to %s", pc.Address)
+		return nil, errors.WithMessagef(err, "snapshot client failed to connect to %s", pc.address)
 	}
 	return pb.NewSnapshotClient(conn), nil
 }

internal/pkg/comm/client.go

@@ -41,16 +41,6 @@ func NewGRPCClient(config ClientConfig) (*GRPCClient, error) {
 	}, nil
 }
 
-// Certificate returns the tls.Certificate used to make TLS connections
-// when client certificates are required by the server
-func (client *GRPCClient) Certificate() tls.Certificate {
-	cert := tls.Certificate{}
-	if client.tlsConfig != nil && len(client.tlsConfig.Certificates) > 0 {
-		cert = client.tlsConfig.Certificates[0]
-	}
-	return cert
-}
-
 // TLSEnabled is a flag indicating whether to use TLS for client
 // connections
 func (client *GRPCClient) TLSEnabled() bool {

internal/pkg/comm/client_test.go

@@ -47,7 +47,6 @@ func TestNewGRPCClient_GoodConfig(t *testing.T) {
 	config := comm.ClientConfig{}
 	client, err := comm.NewGRPCClient(config)
 	require.NoError(t, err)
-	require.Equal(t, tls.Certificate{}, client.Certificate())
 	require.False(t, client.TLSEnabled())
 
 	secOpts := comm.SecureOptions{
@@ -56,7 +55,6 @@ func TestNewGRPCClient_GoodConfig(t *testing.T) {
 	config.SecOpts = secOpts
 	client, err = comm.NewGRPCClient(config)
 	require.NoError(t, err)
-	require.Equal(t, tls.Certificate{}, client.Certificate())
 	require.False(t, client.TLSEnabled())
 
 	secOpts = comm.SecureOptions{
@@ -76,11 +74,13 @@ func TestNewGRPCClient_GoodConfig(t *testing.T) {
 		ServerRootCAs:     [][]byte{testCerts.caPEM},
 		RequireClientCert: true,
 	}
+	clientCert, err := secOpts.ClientCertificate()
+	require.NoError(t, err)
+	require.Equal(t, testCerts.clientCert, clientCert)
 	config.SecOpts = secOpts
 	client, err = comm.NewGRPCClient(config)
 	require.NoError(t, err)
 	require.True(t, client.TLSEnabled())
-	require.Equal(t, testCerts.clientCert, client.Certificate())
 }
 
 func TestNewGRPCClient_BadConfig(t *testing.T) {
@@ -105,7 +105,7 @@ func TestNewGRPCClient_BadConfig(t *testing.T) {
 		RequireClientCert: true,
 	}
 	_, err = comm.NewGRPCClient(config)
-	require.Equal(t, missing, err.Error())
+	require.ErrorContains(t, err, missing)
 
 	// missing cert
 	config.SecOpts = comm.SecureOptions{
@@ -114,7 +114,7 @@ func TestNewGRPCClient_BadConfig(t *testing.T) {
 		RequireClientCert: true,
 	}
 	_, err = comm.NewGRPCClient(config)
-	require.Equal(t, missing, err.Error())
+	require.ErrorContains(t, err, missing)
 
 	// bad key
 	failed := "failed to load client certificate"

internal/pkg/comm/config.go

@@ -173,11 +173,7 @@ func (so SecureOptions) TLSConfig() (*tls.Config, error) {
 	}
 
 	if so.RequireClientCert {
-		// make sure we have both Key and Certificate
-		if so.Key == nil || so.Certificate == nil {
-			return nil, errors.New("both Key and Certificate are required when using mutual TLS")
-		}
-		cert, err := tls.X509KeyPair(so.Certificate, so.Key)
+		cert, err := so.ClientCertificate()
 		if err != nil {
 			return nil, errors.WithMessage(err, "failed to load client certificate")
 		}
@@ -193,6 +189,15 @@ func (so SecureOptions) TLSConfig() (*tls.Config, error) {
 	return tlsConfig, nil
 }
 
+// ClientCertificate returns the client certificate that will be used
+// for mutual TLS.
+func (so SecureOptions) ClientCertificate() (tls.Certificate, error) {
+	if so.Key == nil || so.Certificate == nil {
+		return tls.Certificate{}, errors.New("both Key and Certificate are required when using mutual TLS")
+	}
+	return tls.X509KeyPair(so.Certificate, so.Key)
+}
+
 // KeepaliveOptions is used to set the gRPC keepalive settings for both
 // clients and servers
 type KeepaliveOptions struct {