Skip to content

Commit 70484a4

Browse files
committed
[FAB-3715] Harden deliveryService TLS credentials
This change set makes the transport credentials to be obtained per channel and not globally, in order to prevent a spoofing attempt of a node that's not in the channel but has a TLS cert (with the Subject Alternative Name of the host of the real orderer) signed by a CA that its org is a member of some other channel that the peer is also a member of. Full details in the JIRA item. Change-Id: I79b64ff7729c92d0a4b9cf2e2d9c7b523218fcbc Signed-off-by: Yacov Manevich <[email protected]>
1 parent d25b994 commit 70484a4

File tree

10 files changed

+228
-38
lines changed

10 files changed

+228
-38
lines changed

core/comm/connection.go

+20-11
Original file line numberDiff line numberDiff line change
@@ -10,17 +10,17 @@ import (
1010
"crypto/tls"
1111
"crypto/x509"
1212
"encoding/pem"
13+
"fmt"
1314
"os"
1415
"sync"
1516
"time"
1617

17-
"google.golang.org/grpc"
18-
"google.golang.org/grpc/credentials"
19-
"google.golang.org/grpc/grpclog"
20-
2118
"github.com/hyperledger/fabric/common/flogging"
2219
"github.com/hyperledger/fabric/core/config"
2320
"github.com/spf13/viper"
21+
"google.golang.org/grpc"
22+
"google.golang.org/grpc/credentials"
23+
"google.golang.org/grpc/grpclog"
2424
)
2525

2626
const defaultTimeout = time.Second * 3
@@ -74,16 +74,25 @@ func (cas *CASupport) GetServerRootCAs() (appRootCAs, ordererRootCAs [][]byte) {
7474
return appRootCAs, ordererRootCAs
7575
}
7676

77-
// GetDeliverServiceCredentials returns GRPC transport credentials for use by GRPC
77+
// GetDeliverServiceCredentials returns GRPC transport credentials for given channel to be used by GRPC
7878
// clients which communicate with ordering service endpoints.
79-
func (cas *CASupport) GetDeliverServiceCredentials() credentials.TransportCredentials {
79+
// If the channel isn't found, error is returned.
80+
func (cas *CASupport) GetDeliverServiceCredentials(channelID string) (credentials.TransportCredentials, error) {
81+
cas.RLock()
82+
defer cas.RUnlock()
83+
8084
var creds credentials.TransportCredentials
8185
var tlsConfig = &tls.Config{}
8286
var certPool = x509.NewCertPool()
83-
// loop through the orderer CAs
84-
_, roots := cas.GetServerRootCAs()
85-
for _, root := range roots {
86-
block, _ := pem.Decode(root)
87+
88+
rootCACerts, exists := cas.OrdererRootCAsByChain[channelID]
89+
if !exists {
90+
commLogger.Errorf("Attempted to obtain root CA certs of a non existent channel: %s", channelID)
91+
return nil, fmt.Errorf("didn't find any root CA certs for channel %s", channelID)
92+
}
93+
94+
for _, cert := range rootCACerts {
95+
block, _ := pem.Decode(cert)
8796
if block != nil {
8897
cert, err := x509.ParseCertificate(block.Bytes)
8998
if err == nil {
@@ -97,7 +106,7 @@ func (cas *CASupport) GetDeliverServiceCredentials() credentials.TransportCreden
97106
}
98107
tlsConfig.RootCAs = certPool
99108
creds = credentials.NewTLS(tlsConfig)
100-
return creds
109+
return creds, nil
101110
}
102111

103112
// GetPeerCredentials returns GRPC transport credentials for use by GRPC

core/comm/connection_test.go

+113-6
Original file line numberDiff line numberDiff line change
@@ -17,17 +17,20 @@ limitations under the License.
1717
package comm
1818

1919
import (
20+
"crypto/tls"
2021
"fmt"
2122
"io/ioutil"
23+
"net"
2224
"path/filepath"
25+
"sync/atomic"
2326
"testing"
27+
"time"
2428

25-
"github.com/spf13/viper"
26-
27-
"crypto/tls"
28-
29+
testpb "github.com/hyperledger/fabric/core/comm/testdata/grpc"
2930
"github.com/hyperledger/fabric/core/testutil"
31+
"github.com/spf13/viper"
3032
"github.com/stretchr/testify/assert"
33+
"golang.org/x/net/context"
3134
"google.golang.org/grpc"
3235
)
3336

@@ -141,7 +144,7 @@ func TestCASupport(t *testing.T) {
141144
casClone := GetCASupport()
142145
assert.Exactly(t, casClone, cas, "Expected GetCASupport to be a singleton")
143146

144-
creds := cas.GetDeliverServiceCredentials()
147+
creds, _ := cas.GetDeliverServiceCredentials("channel1")
145148
assert.Equal(t, "1.2", creds.Info().SecurityVersion,
146149
"Expected Security version to be 1.2")
147150
creds = cas.GetPeerCredentials(tls.Certificate{})
@@ -151,11 +154,115 @@ func TestCASupport(t *testing.T) {
151154
// append some bad certs and make sure things still work
152155
cas.ServerRootCAs = append(cas.ServerRootCAs, []byte("badcert"))
153156
cas.ServerRootCAs = append(cas.ServerRootCAs, []byte(badPEM))
154-
creds = cas.GetDeliverServiceCredentials()
157+
creds, _ = cas.GetDeliverServiceCredentials("channel1")
155158
assert.Equal(t, "1.2", creds.Info().SecurityVersion,
156159
"Expected Security version to be 1.2")
157160
creds = cas.GetPeerCredentials(tls.Certificate{})
158161
assert.Equal(t, "1.2", creds.Info().SecurityVersion,
159162
"Expected Security version to be 1.2")
160163

161164
}
165+
166+
type srv struct {
167+
port int
168+
GRPCServer
169+
caCert []byte
170+
serviced uint32
171+
}
172+
173+
func (s *srv) assertServiced(t *testing.T) {
174+
assert.Equal(t, uint32(1), atomic.LoadUint32(&s.serviced))
175+
atomic.StoreUint32(&s.serviced, 0)
176+
}
177+
178+
func (s *srv) EmptyCall(context.Context, *testpb.Empty) (*testpb.Empty, error) {
179+
atomic.StoreUint32(&s.serviced, 1)
180+
return &testpb.Empty{}, nil
181+
}
182+
183+
func newServer(org string, port int) *srv {
184+
certs := map[string][]byte{
185+
"ca.crt": nil,
186+
"server.crt": nil,
187+
"server.key": nil,
188+
}
189+
for suffix := range certs {
190+
fName := filepath.Join("testdata", "impersonation", org, suffix)
191+
cert, err := ioutil.ReadFile(fName)
192+
if err != nil {
193+
panic(fmt.Errorf("Failed reading %s: %v", fName, err))
194+
}
195+
certs[suffix] = cert
196+
}
197+
l, err := net.Listen("tcp", fmt.Sprintf(":%d", port))
198+
if err != nil {
199+
panic(fmt.Errorf("Failed listening on port %d: %v", port, err))
200+
}
201+
gSrv, err := NewGRPCServerFromListener(l, SecureServerConfig{
202+
ServerCertificate: certs["server.crt"],
203+
ServerKey: certs["server.key"],
204+
UseTLS: true,
205+
})
206+
if err != nil {
207+
panic(fmt.Errorf("Failed starting gRPC server: %v", err))
208+
}
209+
s := &srv{
210+
port: port,
211+
caCert: certs["ca.crt"],
212+
GRPCServer: gSrv,
213+
}
214+
testpb.RegisterTestServiceServer(gSrv.Server(), s)
215+
go s.Start()
216+
return s
217+
}
218+
219+
func TestImpersonation(t *testing.T) {
220+
// Scenario: We have 2 organizations: orgA, orgB
221+
// and each of them are in their respected channels- A, B.
222+
// The test would obtain credentials.TransportCredentials by calling GetDeliverServiceCredentials.
223+
// Each organization would have its own gRPC server (srvA and srvB) with a TLS certificate
224+
// signed by its root CA and with a SAN entry of 'localhost'.
225+
// We test the following assertions:
226+
// 1) Invocation with GetDeliverServiceCredentials("A") to srvA succeeds
227+
// 2) Invocation with GetDeliverServiceCredentials("B") to srvB succeeds
228+
// 3) Invocation with GetDeliverServiceCredentials("A") to srvB fails
229+
// 4) Invocation with GetDeliverServiceCredentials("B") to srvA fails
230+
231+
osA := newServer("orgA", 7070)
232+
defer osA.Stop()
233+
osB := newServer("orgB", 7080)
234+
defer osB.Stop()
235+
time.Sleep(time.Second)
236+
237+
cas := GetCASupport()
238+
_, err := GetCASupport().GetDeliverServiceCredentials("C")
239+
assert.Error(t, err)
240+
241+
cas.OrdererRootCAsByChain["A"] = [][]byte{osA.caCert}
242+
cas.OrdererRootCAsByChain["B"] = [][]byte{osB.caCert}
243+
244+
testInvoke(t, "A", osA, true)
245+
testInvoke(t, "B", osB, true)
246+
testInvoke(t, "A", osB, false)
247+
testInvoke(t, "B", osA, false)
248+
249+
}
250+
251+
func testInvoke(t *testing.T, channelID string, s *srv, shouldSucceed bool) {
252+
creds, err := GetCASupport().GetDeliverServiceCredentials(channelID)
253+
assert.NoError(t, err)
254+
endpoint := fmt.Sprintf("localhost:%d", s.port)
255+
conn, err := grpc.Dial(endpoint, grpc.WithTimeout(time.Second*3), grpc.WithTransportCredentials(creds), grpc.WithBlock())
256+
if shouldSucceed {
257+
assert.NoError(t, err)
258+
defer conn.Close()
259+
} else {
260+
assert.Error(t, err)
261+
assert.Contains(t, err.Error(), "certificate signed by unknown authority")
262+
return
263+
}
264+
client := testpb.NewTestServiceClient(conn)
265+
_, err = client.EmptyCall(context.Background(), &testpb.Empty{})
266+
assert.NoError(t, err)
267+
s.assertServiced(t)
268+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
-----BEGIN CERTIFICATE-----
2+
MIICHDCCAcOgAwIBAgIQHaelrvVzGi4qoc91lIEEzjAKBggqhkjOPQQDAjBbMQsw
3+
CQYDVQQGEwJVUzETMBEGA1UECBMKQ2FsaWZvcm5pYTEWMBQGA1UEBxMNU2FuIEZy
4+
YW5jaXNjbzENMAsGA1UEChMEb3JnQTEQMA4GA1UEAxMHY2Eub3JnQTAeFw0xNzA2
5+
MDgxMjAzMzFaFw0yNzA2MDYxMjAzMzFaMFsxCzAJBgNVBAYTAlVTMRMwEQYDVQQI
6+
EwpDYWxpZm9ybmlhMRYwFAYDVQQHEw1TYW4gRnJhbmNpc2NvMQ0wCwYDVQQKEwRv
7+
cmdBMRAwDgYDVQQDEwdjYS5vcmdBMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAE
8+
vHfWYfXiQwG58vR+iIlAj0TJV09fEXUElfnOY0T1uqhBftrQCEonzT8j5kxjuksl
9+
cYCB+nJOZTOeNZyLWygslaNpMGcwDgYDVR0PAQH/BAQDAgGmMBkGA1UdJQQSMBAG
10+
BFUdJQAGCCsGAQUFBwMBMA8GA1UdEwEB/wQFMAMBAf8wKQYDVR0OBCIEIFz7UdB7
11+
Lxhy5u8mM20w0z1oimfCdf4ju3PSF1z27ePRMAoGCCqGSM49BAMCA0cAMEQCIEtY
12+
oJWa2n3oPJbofWEHZBy8weAyS8idNpdZ215U/Db/AiAIkxL9MUk7M476ZxUctXj+
13+
Ll7oLHhcRudsmIeUJu7xww==
14+
-----END CERTIFICATE-----
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
-----BEGIN CERTIFICATE-----
2+
MIICNjCCAd2gAwIBAgIRAIVdca7gRBEyyp/DyCmPRV4wCgYIKoZIzj0EAwIwWzEL
3+
MAkGA1UEBhMCVVMxEzARBgNVBAgTCkNhbGlmb3JuaWExFjAUBgNVBAcTDVNhbiBG
4+
cmFuY2lzY28xDTALBgNVBAoTBG9yZ0ExEDAOBgNVBAMTB2NhLm9yZ0EwHhcNMTcw
5+
NjA4MTIwMzMxWhcNMjcwNjA2MTIwMzMxWjBTMQswCQYDVQQGEwJVUzETMBEGA1UE
6+
CBMKQ2FsaWZvcm5pYTEWMBQGA1UEBxMNU2FuIEZyYW5jaXNjbzEXMBUGA1UEAxMO
7+
bG9jYWxob3N0Lm9yZ0EwWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAAQlAdoVacaS
8+
Eo8MrjemN+oN7WMPNCTDoobJw+/KiLR6ItoaDrMhzvoIaP4DJP8xvn9YIloUODaK
9+
pX/CZXjtxs8Xo4GJMIGGMA4GA1UdDwEB/wQEAwIFoDATBgNVHSUEDDAKBggrBgEF
10+
BQcDATAMBgNVHRMBAf8EAjAAMCsGA1UdIwQkMCKAIFz7UdB7Lxhy5u8mM20w0z1o
11+
imfCdf4ju3PSF1z27ePRMCQGA1UdEQQdMBuCDmxvY2FsaG9zdC5vcmdBgglsb2Nh
12+
bGhvc3QwCgYIKoZIzj0EAwIDRwAwRAIgJFESyVamhihN9adp8ekl78/80DmfSd2N
13+
ZVAC2g4Gp+0CIFquN7/Z+75yMW4jYrXFLlR6iM1Iy4oRR1S9kQ1e0h18
14+
-----END CERTIFICATE-----
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
-----BEGIN PRIVATE KEY-----
2+
MIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQgiiG7wI1OuYPHb9Ft
3+
6o/uVjSH6CM2Yq63yvXE+bg9JJyhRANCAAQlAdoVacaSEo8MrjemN+oN7WMPNCTD
4+
oobJw+/KiLR6ItoaDrMhzvoIaP4DJP8xvn9YIloUODaKpX/CZXjtxs8X
5+
-----END PRIVATE KEY-----
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
-----BEGIN CERTIFICATE-----
2+
MIICHTCCAcSgAwIBAgIRAKZxKXGx9VIoK2vAWHF2u1MwCgYIKoZIzj0EAwIwWzEL
3+
MAkGA1UEBhMCVVMxEzARBgNVBAgTCkNhbGlmb3JuaWExFjAUBgNVBAcTDVNhbiBG
4+
cmFuY2lzY28xDTALBgNVBAoTBG9yZ0IxEDAOBgNVBAMTB2NhLm9yZ0IwHhcNMTcw
5+
NjA4MTIwMzMxWhcNMjcwNjA2MTIwMzMxWjBbMQswCQYDVQQGEwJVUzETMBEGA1UE
6+
CBMKQ2FsaWZvcm5pYTEWMBQGA1UEBxMNU2FuIEZyYW5jaXNjbzENMAsGA1UEChME
7+
b3JnQjEQMA4GA1UEAxMHY2Eub3JnQjBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IA
8+
BKP7/XEYcHqW2915PucqcZtiT86bkPqU52W7t2Xe8hELEsad8XMjZcQ0Kwcs++O/
9+
BVrMv5R8UoS3H5hx+LLVu8+jaTBnMA4GA1UdDwEB/wQEAwIBpjAZBgNVHSUEEjAQ
10+
BgRVHSUABggrBgEFBQcDATAPBgNVHRMBAf8EBTADAQH/MCkGA1UdDgQiBCAZB75b
11+
7YJN/QFjWiX7B4mcwI80mwkaR6vB5y0Ne6iszDAKBggqhkjOPQQDAgNHADBEAiBn
12+
lF1PNfg6X9IKj39y/TmPIHVBbmClchTEeNE2D6SdDQIgb9iqNW+/UekuSTn/XVtT
13+
jtpdqcbKE8pDNp3Gi9Fv1as=
14+
-----END CERTIFICATE-----
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
-----BEGIN CERTIFICATE-----
2+
MIICNTCCAdygAwIBAgIQEytB52SKNRuBcfMledS0LzAKBggqhkjOPQQDAjBbMQsw
3+
CQYDVQQGEwJVUzETMBEGA1UECBMKQ2FsaWZvcm5pYTEWMBQGA1UEBxMNU2FuIEZy
4+
YW5jaXNjbzENMAsGA1UEChMEb3JnQjEQMA4GA1UEAxMHY2Eub3JnQjAeFw0xNzA2
5+
MDgxMjAzMzFaFw0yNzA2MDYxMjAzMzFaMFMxCzAJBgNVBAYTAlVTMRMwEQYDVQQI
6+
EwpDYWxpZm9ybmlhMRYwFAYDVQQHEw1TYW4gRnJhbmNpc2NvMRcwFQYDVQQDEw5s
7+
b2NhbGhvc3Qub3JnQjBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABNeIm7orPT/U
8+
NzM43TaEg3PfyY6pNCyD1bswq8+ykVayDuBz71TidBJL/HHn02RLhSHJ+oHBRV0w
9+
xel1QTQGjYujgYkwgYYwDgYDVR0PAQH/BAQDAgWgMBMGA1UdJQQMMAoGCCsGAQUF
10+
BwMBMAwGA1UdEwEB/wQCMAAwKwYDVR0jBCQwIoAgGQe+W+2CTf0BY1ol+weJnMCP
11+
NJsJGkerwectDXuorMwwJAYDVR0RBB0wG4IObG9jYWxob3N0Lm9yZ0KCCWxvY2Fs
12+
aG9zdDAKBggqhkjOPQQDAgNHADBEAiBDQsH0omgTJlznoFdN9U7Cn11mdRKgNReA
13+
/iB8p63rmgIgf8e7ErCPgoz+zbUPSkOFtZPW+0hgdPRXhczhZwIgS8A=
14+
-----END CERTIFICATE-----
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
-----BEGIN PRIVATE KEY-----
2+
MIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQgpEGx/HVbldUaoaiP
3+
9n3DFtxUr1RSvCdQFHFVIRya2uShRANCAATXiJu6Kz0/1DczON02hINz38mOqTQs
4+
g9W7MKvPspFWsg7gc+9U4nQSS/xx59NkS4UhyfqBwUVdMMXpdUE0Bo2L
5+
-----END PRIVATE KEY-----

core/deliverservice/deliveryclient.go

+23-17
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,8 @@ type deliverServiceImpl struct {
6363
// how it verifies messages received from it,
6464
// and how it disseminates the messages to other peers
6565
type Config struct {
66-
// ConnFactory creates a connection to an endpoint
67-
ConnFactory func(endpoint string) (*grpc.ClientConn, error)
66+
// ConnFactory returns a function that creates a connection to an endpoint
67+
ConnFactory func(channelID string) func(endpoint string) (*grpc.ClientConn, error)
6868
// ABCFactory creates an AtomicBroadcastClient out of a connection
6969
ABCFactory func(*grpc.ClientConn) orderer.AtomicBroadcastClient
7070
// CryptoSvc performs cryptographic actions like message verification and signing
@@ -183,27 +183,33 @@ func (d *deliverServiceImpl) newClient(chainID string, ledgerInfoProvider blocks
183183
}
184184
return time.Duration(math.Pow(2, float64(attemptNum))) * time.Millisecond * 500, true
185185
}
186-
connProd := comm.NewConnectionProducer(d.conf.ConnFactory, d.conf.Endpoints)
186+
connProd := comm.NewConnectionProducer(d.conf.ConnFactory(chainID), d.conf.Endpoints)
187187
bClient := NewBroadcastClient(connProd, d.conf.ABCFactory, broadcastSetup, backoffPolicy)
188188
requester.client = bClient
189189
return bClient
190190
}
191191

192-
func DefaultConnectionFactory(endpoint string) (*grpc.ClientConn, error) {
193-
dialOpts := []grpc.DialOption{grpc.WithTimeout(connTimeout), grpc.WithBlock()}
194-
// set max send/recv msg sizes
195-
dialOpts = append(dialOpts, grpc.WithDefaultCallOptions(grpc.MaxCallRecvMsgSize(comm.MaxRecvMsgSize()),
196-
grpc.MaxCallSendMsgSize(comm.MaxSendMsgSize())))
197-
// set the keepalive options
198-
dialOpts = append(dialOpts, comm.ClientKeepaliveOptions()...)
199-
200-
if comm.TLSEnabled() {
201-
dialOpts = append(dialOpts, grpc.WithTransportCredentials(comm.GetCASupport().GetDeliverServiceCredentials()))
202-
} else {
203-
dialOpts = append(dialOpts, grpc.WithInsecure())
192+
func DefaultConnectionFactory(channelID string) func(endpoint string) (*grpc.ClientConn, error) {
193+
return func(endpoint string) (*grpc.ClientConn, error) {
194+
dialOpts := []grpc.DialOption{grpc.WithTimeout(connTimeout), grpc.WithBlock()}
195+
// set max send/recv msg sizes
196+
dialOpts = append(dialOpts, grpc.WithDefaultCallOptions(grpc.MaxCallRecvMsgSize(comm.MaxRecvMsgSize()),
197+
grpc.MaxCallSendMsgSize(comm.MaxSendMsgSize())))
198+
// set the keepalive options
199+
dialOpts = append(dialOpts, comm.ClientKeepaliveOptions()...)
200+
201+
if comm.TLSEnabled() {
202+
creds, err := comm.GetCASupport().GetDeliverServiceCredentials(channelID)
203+
if err != nil {
204+
return nil, fmt.Errorf("Failed obtaining credentials for channel %s: %v", channelID, err)
205+
}
206+
dialOpts = append(dialOpts, grpc.WithTransportCredentials(creds))
207+
} else {
208+
dialOpts = append(dialOpts, grpc.WithInsecure())
209+
}
210+
grpc.EnableTracing = true
211+
return grpc.Dial(endpoint, dialOpts...)
204212
}
205-
grpc.EnableTracing = true
206-
return grpc.Dial(endpoint, dialOpts...)
207213
}
208214

209215
func DefaultABCFactory(conn *grpc.ClientConn) orderer.AtomicBroadcastClient {

core/deliverservice/deliveryclient_test.go

+6-4
Original file line numberDiff line numberDiff line change
@@ -95,10 +95,12 @@ func TestNewDeliverService(t *testing.T) {
9595
return &mocks.MockAtomicBroadcastClient{blocksDeliverer}
9696
}
9797

98-
connFactory := func(endpoint string) (*grpc.ClientConn, error) {
99-
lock.Lock()
100-
defer lock.Unlock()
101-
return newConnection(), nil
98+
connFactory := func(_ string) func(string) (*grpc.ClientConn, error) {
99+
return func(endpoint string) (*grpc.ClientConn, error) {
100+
lock.Lock()
101+
defer lock.Unlock()
102+
return newConnection(), nil
103+
}
102104
}
103105
service, err := NewDeliverService(&Config{
104106
Endpoints: []string{"a"},

0 commit comments

Comments
 (0)