Skip to content

Commit 0c58de6

Browse files
committed
[FAB-3701] Peer and orderer CAs should be separate
Prior to this change, there was a TODO in the code to separate the trusted roots that the peer should use when communicating with peers and orderers. Currently they are the same (both internal maps include all CAs for both orderer and peer/app orgs). With this change, it's no longer the case as the code now properly handles this. - renamed test to match what's actually being tested - revamped test scenarios to only test the functionality as well as increase coverage - regenerated crypto material required - slight modification to cert generator to add SKI to intermediate root certs as it is required - made sure to cleanup filesystem resources Change-Id: I20f35dcec9c787c9052fb5d8ef119cd3dbab2d6c Signed-off-by: Gari Singh <[email protected]>
1 parent c346b06 commit 0c58de6

27 files changed

+245
-207
lines changed

core/comm/connection.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ func (cas *CASupport) GetClientRootCAs() (appRootCAs, ordererRootCAs [][]byte) {
152152
appRootCAs = append(appRootCAs, appRootCA...)
153153
}
154154

155-
for _, ordererRootCA := range cas.AppRootCAsByChain {
155+
for _, ordererRootCA := range cas.OrdererRootCAsByChain {
156156
ordererRootCAs = append(ordererRootCAs, ordererRootCA...)
157157
}
158158

core/comm/testdata/certs/generate.go

+1
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,7 @@ func genIntermediateCertificateAuthorityECDSA(name string, signKey *ecdsa.Privat
231231
subject.CommonName = name
232232

233233
template.Subject = subject
234+
template.SubjectKeyId = []byte{1, 2, 3, 4}
234235

235236
x509Cert, err := genCertificateECDSA(name, &template, signCert, &key.PublicKey, signKey)
236237

core/peer/peer.go

+25-6
Original file line numberDiff line numberDiff line change
@@ -377,13 +377,20 @@ func buildTrustedRootsForChain(cm configtxapi.Manager) {
377377

378378
appRootCAs := [][]byte{}
379379
ordererRootCAs := [][]byte{}
380+
appOrgMSPs := make(map[string]struct{})
381+
382+
//loop through app orgs and build map of MSPIDs
383+
for _, appOrg := range cm.ApplicationConfig().Organizations() {
384+
appOrgMSPs[appOrg.MSPID()] = struct{}{}
385+
}
380386
cid := cm.ChainID()
387+
peerLogger.Debugf("updating root CAs for channel [%s]", cid)
381388
msps, err := cm.MSPManager().GetMSPs()
382389
if err != nil {
383-
peerLogger.Errorf("Error getting getting root CA for channel %s (%s)", cid, err)
390+
peerLogger.Errorf("Error getting root CAs for channel %s (%s)", cid, err)
384391
}
385392
if err == nil {
386-
for _, v := range msps {
393+
for k, v := range msps {
387394
// check to see if this is a FABRIC MSP
388395
if v.GetType() == msp.FABRIC {
389396
for _, root := range v.GetRootCerts() {
@@ -392,7 +399,14 @@ func buildTrustedRootsForChain(cm configtxapi.Manager) {
392399
id := &mspprotos.SerializedIdentity{}
393400
err = proto.Unmarshal(sid, id)
394401
if err == nil {
395-
appRootCAs = append(appRootCAs, id.IdBytes)
402+
// check to see of this is an app org MSP
403+
if _, ok := appOrgMSPs[k]; ok {
404+
peerLogger.Debugf("adding app root CAs for MSP [%s]", k)
405+
appRootCAs = append(appRootCAs, id.IdBytes)
406+
} else {
407+
peerLogger.Debugf("adding orderer root CAs for MSP [%s]", k)
408+
ordererRootCAs = append(ordererRootCAs, id.IdBytes)
409+
}
396410
}
397411
}
398412
}
@@ -402,14 +416,19 @@ func buildTrustedRootsForChain(cm configtxapi.Manager) {
402416
id := &mspprotos.SerializedIdentity{}
403417
err = proto.Unmarshal(sid, id)
404418
if err == nil {
405-
appRootCAs = append(appRootCAs, id.IdBytes)
419+
// check to see of this is an app org MSP
420+
if _, ok := appOrgMSPs[k]; ok {
421+
peerLogger.Debugf("adding app root CAs for MSP [%s]", k)
422+
appRootCAs = append(appRootCAs, id.IdBytes)
423+
} else {
424+
peerLogger.Debugf("adding orderer root CAs for MSP [%s]", k)
425+
ordererRootCAs = append(ordererRootCAs, id.IdBytes)
426+
}
406427
}
407428
}
408429
}
409430
}
410431
}
411-
// TODO: separate app and orderer CAs
412-
ordererRootCAs = appRootCAs
413432
rootCASupport.AppRootCAsByChain[cid] = appRootCAs
414433
rootCASupport.OrdererRootCAsByChain[cid] = ordererRootCAs
415434
}

core/peer/pkg_test.go

+112-64
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"errors"
2323
"fmt"
2424
"io/ioutil"
25+
"os"
2526
"path/filepath"
2627
"testing"
2728
"time"
@@ -95,10 +96,13 @@ func invokeEmptyCall(address string, dialOptions []grpc.DialOption) (*testpb.Emp
9596
}
9697

9798
// helper function to build an MSPConfig given root certs
98-
func createMSPConfig(rootCerts [][]byte, mspID string) (*mspproto.MSPConfig, error) {
99+
func createMSPConfig(rootCerts, intermediateCerts [][]byte,
100+
mspID string) (*mspproto.MSPConfig, error) {
101+
99102
fmspconf := &mspproto.FabricMSPConfig{
100-
RootCerts: rootCerts,
101-
Name: mspID}
103+
RootCerts: rootCerts,
104+
IntermediateCerts: intermediateCerts,
105+
Name: mspID}
102106

103107
fmpsjs, err := proto.Marshal(fmspconf)
104108
if err != nil {
@@ -114,130 +118,160 @@ func createConfigBlock(chainID string, appMSPConf, ordererMSPConf *mspproto.MSPC
114118
return block, err
115119
}
116120

117-
func TestCreatePeerServer(t *testing.T) {
121+
func TestUpdateRootsFromConfigBlock(t *testing.T) {
118122
// load test certs from testdata
119123
org1CA, err := ioutil.ReadFile(filepath.Join("testdata", "Org1-cert.pem"))
120-
org1Server1Key, err := ioutil.ReadFile(filepath.Join("testdata", "Org1-server1-key.pem"))
121-
org1Server1Cert, err := ioutil.ReadFile(filepath.Join("testdata", "Org1-server1-cert.pem"))
122-
org1Server2Key, err := ioutil.ReadFile(filepath.Join("testdata", "Org1-server2-key.pem"))
123-
org1Server2Cert, err := ioutil.ReadFile(filepath.Join("testdata", "Org1-server2-cert.pem"))
124+
org1Server1Key, err := ioutil.ReadFile(filepath.Join("testdata",
125+
"Org1-server1-key.pem"))
126+
org1Server1Cert, err := ioutil.ReadFile(filepath.Join("testdata",
127+
"Org1-server1-cert.pem"))
124128
org2CA, err := ioutil.ReadFile(filepath.Join("testdata", "Org2-cert.pem"))
125-
org2Server1Key, err := ioutil.ReadFile(filepath.Join("testdata", "Org2-server1-key.pem"))
126-
org2Server1Cert, err := ioutil.ReadFile(filepath.Join("testdata", "Org2-server1-cert.pem"))
127-
org3CA, err := ioutil.ReadFile(filepath.Join("testdata", "Org3-cert.pem"))
129+
org2Server1Key, err := ioutil.ReadFile(filepath.Join("testdata",
130+
"Org2-server1-key.pem"))
131+
org2Server1Cert, err := ioutil.ReadFile(filepath.Join("testdata",
132+
"Org2-server1-cert.pem"))
133+
org2IntermediateCA, err := ioutil.ReadFile(filepath.Join("testdata",
134+
"Org2-child1-cert.pem"))
135+
org2IntermediateServer1Key, err := ioutil.ReadFile(filepath.Join("testdata",
136+
"Org2-child1-server1-key.pem"))
137+
org2IntermediateServer1Cert, err := ioutil.ReadFile(filepath.Join("testdata",
138+
"Org2-child1-server1-cert.pem"))
139+
ordererOrgCA, err := ioutil.ReadFile(filepath.Join("testdata", "Org3-cert.pem"))
140+
ordererOrgServer1Key, err := ioutil.ReadFile(filepath.Join("testdata",
141+
"Org3-server1-key.pem"))
142+
ordererOrgServer1Cert, err := ioutil.ReadFile(filepath.Join("testdata",
143+
"Org3-server1-cert.pem"))
128144

129145
if err != nil {
130146
t.Fatalf("Failed to load test certificates: %v", err)
131147
}
132148

133149
// create test MSPConfigs
134-
org1MSPConf, err := createMSPConfig([][]byte{org1CA}, "Org1MSP")
135-
org2MSPConf, err := createMSPConfig([][]byte{org2CA}, "Org2MSP")
136-
org3MSPConf, err := createMSPConfig([][]byte{org3CA}, "Org3MSP")
150+
org1MSPConf, err := createMSPConfig([][]byte{org1CA}, [][]byte{}, "Org1MSP")
151+
org2MSPConf, err := createMSPConfig([][]byte{org2CA}, [][]byte{}, "Org2MSP")
152+
org2IntermediateMSPConf, err := createMSPConfig([][]byte{org2CA},
153+
[][]byte{org2IntermediateCA}, "Org2IntermediateMSP")
154+
ordererOrgMSPConf, err := createMSPConfig([][]byte{ordererOrgCA},
155+
[][]byte{}, "OrdererOrgMSP")
137156
if err != nil {
138157
t.Fatalf("Failed to create MSPConfigs (%s)", err)
139158
}
140159

141160
// create test channel create blocks
142-
channel1Block, err := createConfigBlock("channel1", org1MSPConf, org3MSPConf, "Org1MSP", "Org3MSP")
143-
channel2Block, err := createConfigBlock("channel2", org2MSPConf, org3MSPConf, "Org2MSP", "Org3MSP")
161+
channel1Block, err := createConfigBlock("channel1", org1MSPConf,
162+
ordererOrgMSPConf, "Org1MSP", "OrdererOrgMSP")
163+
channel2Block, err := createConfigBlock("channel2", org2MSPConf,
164+
ordererOrgMSPConf, "Org2MSP", "OrdererOrgMSP")
165+
channel3Block, err := createConfigBlock("channel3", org2IntermediateMSPConf,
166+
ordererOrgMSPConf, "Org2IntermediateMSP", "OrdererOrgMSP")
144167

145168
createChannel := func(cid string, block *cb.Block) {
146169
viper.Set("peer.tls.enabled", true)
147-
viper.Set("peer.tls.cert.file", filepath.Join("testdata", "Org1-server1-cert.pem"))
148-
viper.Set("peer.tls.key.file", filepath.Join("testdata", "Org1-server1-key.pem"))
149-
viper.Set("peer.tls.rootcert.file", filepath.Join("testdata", "Org1-cert.pem"))
170+
viper.Set("peer.tls.cert.file", filepath.Join("testdata",
171+
"Org1-server1-cert.pem"))
172+
viper.Set("peer.tls.key.file", filepath.Join("testdata",
173+
"Org1-server1-key.pem"))
174+
viper.Set("peer.tls.rootcert.file", filepath.Join("testdata",
175+
"Org1-cert.pem"))
176+
viper.Set("peer.fileSystemPath", "/var/hyperledger/test/")
177+
defer os.RemoveAll("/var/hyperledger/test/")
150178
err := peer.CreateChainFromBlock(block)
151179
if err != nil {
152180
t.Fatalf("Failed to create config block (%s)", err)
153181
}
154182
t.Logf("Channel %s MSPIDs: (%s)", cid, peer.GetMSPIDs(cid))
155-
appCAs, orgCAs := comm.GetCASupport().GetClientRootCAs()
156-
t.Logf("appCAs after update for channel %s: %d", cid, len(appCAs))
157-
t.Logf("orgCAs after update for channel %s: %d", cid, len(orgCAs))
158183
}
159184

160185
org1CertPool, err := createCertPool([][]byte{org1CA})
161-
org2CertPool, err := createCertPool([][]byte{org2CA})
162186

163187
if err != nil {
164188
t.Fatalf("Failed to load root certificates into pool: %v", err)
165189
}
166190

167-
org1Creds := credentials.NewClientTLSFromCert(org1CertPool, "")
168-
org2Creds := credentials.NewClientTLSFromCert(org2CertPool, "")
169-
170191
// use server cert as client cert
171-
org1ClientCert, err := tls.X509KeyPair(org1Server2Cert, org1Server2Key)
192+
org1ClientCert, err := tls.X509KeyPair(org1Server1Cert, org1Server1Key)
172193
if err != nil {
173194
t.Fatalf("Failed to load client certificate: %v", err)
174195
}
175-
org1Org1Creds := credentials.NewTLS(&tls.Config{
196+
org1Creds := credentials.NewTLS(&tls.Config{
176197
Certificates: []tls.Certificate{org1ClientCert},
177198
RootCAs: org1CertPool,
178199
})
179200
org2ClientCert, err := tls.X509KeyPair(org2Server1Cert, org2Server1Key)
180201
if err != nil {
181202
t.Fatalf("Failed to load client certificate: %v", err)
182203
}
183-
org1Org2Creds := credentials.NewTLS(&tls.Config{
204+
org2Creds := credentials.NewTLS(&tls.Config{
184205
Certificates: []tls.Certificate{org2ClientCert},
185206
RootCAs: org1CertPool,
186207
})
208+
org2IntermediateClientCert, err := tls.X509KeyPair(
209+
org2IntermediateServer1Cert, org2IntermediateServer1Key)
210+
if err != nil {
211+
t.Fatalf("Failed to load client certificate: %v", err)
212+
}
213+
org2IntermediateCreds := credentials.NewTLS(&tls.Config{
214+
Certificates: []tls.Certificate{org2IntermediateClientCert},
215+
RootCAs: org1CertPool,
216+
})
217+
ordererOrgClientCert, err := tls.X509KeyPair(ordererOrgServer1Cert,
218+
ordererOrgServer1Key)
219+
if err != nil {
220+
t.Fatalf("Failed to load client certificate: %v", err)
221+
}
222+
ordererOrgCreds := credentials.NewTLS(&tls.Config{
223+
Certificates: []tls.Certificate{ordererOrgClientCert},
224+
RootCAs: org1CertPool,
225+
})
187226

188227
// basic function tests
189228
var tests = []struct {
190229
name string
191230
listenAddress string
192231
secureConfig comm.SecureServerConfig
193-
expectError bool
194232
createChannel func()
195233
goodOptions []grpc.DialOption
196234
badOptions []grpc.DialOption
235+
numAppCAs int
236+
numOrdererCAs int
197237
}{
198238

199239
{
200-
name: "NoTLS",
201-
listenAddress: fmt.Sprintf("localhost:%d", 4050),
202-
secureConfig: comm.SecureServerConfig{
203-
UseTLS: false,
204-
},
205-
expectError: false,
206-
createChannel: func() {},
207-
goodOptions: []grpc.DialOption{grpc.WithInsecure()},
208-
badOptions: []grpc.DialOption{grpc.WithTransportCredentials(org1Creds)},
209-
},
210-
{
211-
name: "ServerTLSOrg1",
212-
listenAddress: fmt.Sprintf("localhost:%d", 4051),
240+
name: "MutualTLSOrg1Org1",
241+
listenAddress: fmt.Sprintf("localhost:%d", 4052),
213242
secureConfig: comm.SecureServerConfig{
214243
UseTLS: true,
215244
ServerCertificate: org1Server1Cert,
216245
ServerKey: org1Server1Key,
217246
ServerRootCAs: [][]byte{org1CA},
247+
RequireClientCert: true,
218248
},
219-
expectError: false,
220-
createChannel: func() {},
249+
createChannel: func() { createChannel("channel1", channel1Block) },
221250
goodOptions: []grpc.DialOption{grpc.WithTransportCredentials(org1Creds)},
222-
badOptions: []grpc.DialOption{grpc.WithTransportCredentials(org2Creds)},
251+
badOptions: []grpc.DialOption{grpc.WithTransportCredentials(ordererOrgCreds)},
252+
numAppCAs: 2, // each channel also has a DEFAULT MSP
253+
numOrdererCAs: 1,
223254
},
224255
{
225-
name: "MutualTLSOrg1Org1",
226-
listenAddress: fmt.Sprintf("localhost:%d", 4052),
256+
name: "MutualTLSOrg1Org2",
257+
listenAddress: fmt.Sprintf("localhost:%d", 4053),
227258
secureConfig: comm.SecureServerConfig{
228259
UseTLS: true,
229260
ServerCertificate: org1Server1Cert,
230261
ServerKey: org1Server1Key,
231262
ServerRootCAs: [][]byte{org1CA},
232263
RequireClientCert: true,
233264
},
234-
expectError: false,
235-
createChannel: func() { createChannel("channel1", channel1Block) },
236-
goodOptions: []grpc.DialOption{grpc.WithTransportCredentials(org1Org1Creds)},
237-
badOptions: []grpc.DialOption{grpc.WithTransportCredentials(org1Org2Creds)},
265+
createChannel: func() { createChannel("channel2", channel2Block) },
266+
goodOptions: []grpc.DialOption{
267+
grpc.WithTransportCredentials(org2Creds)},
268+
badOptions: []grpc.DialOption{
269+
grpc.WithTransportCredentials(ordererOrgCreds)},
270+
numAppCAs: 4,
271+
numOrdererCAs: 2,
238272
},
239273
{
240-
name: "MutualTLSOrg1Org2",
274+
name: "MutualTLSOrg1Org2Intermediate",
241275
listenAddress: fmt.Sprintf("localhost:%d", 4053),
242276
secureConfig: comm.SecureServerConfig{
243277
UseTLS: true,
@@ -246,10 +280,13 @@ func TestCreatePeerServer(t *testing.T) {
246280
ServerRootCAs: [][]byte{org1CA},
247281
RequireClientCert: true,
248282
},
249-
expectError: false,
250-
createChannel: func() { createChannel("channel2", channel2Block) },
251-
goodOptions: []grpc.DialOption{grpc.WithTransportCredentials(org1Org2Creds)},
252-
badOptions: []grpc.DialOption{grpc.WithTransportCredentials(org1Creds)},
283+
createChannel: func() { createChannel("channel3", channel3Block) },
284+
goodOptions: []grpc.DialOption{
285+
grpc.WithTransportCredentials(org2IntermediateCreds)},
286+
badOptions: []grpc.DialOption{
287+
grpc.WithTransportCredentials(ordererOrgCreds)},
288+
numAppCAs: 7,
289+
numOrdererCAs: 3,
253290
},
254291
}
255292

@@ -258,10 +295,8 @@ func TestCreatePeerServer(t *testing.T) {
258295
t.Run(test.name, func(t *testing.T) {
259296
t.Logf("Running test %s ...", test.name)
260297
_, err := peer.CreatePeerServer(test.listenAddress, test.secureConfig)
261-
// check to see whether to not we expect an error
262-
// we don't check the exact error because the comm package covers these cases
263-
if test.expectError {
264-
assert.Error(t, err, "CreatePeerServer should have returned an error")
298+
if err != nil {
299+
t.Fatalf("CreatePeerServer failed with error [%s]", err)
265300
} else {
266301
assert.NoError(t, err, "CreatePeerServer should not have returned an error")
267302
// get the server from peer
@@ -272,15 +307,28 @@ func TestCreatePeerServer(t *testing.T) {
272307
go server.Start()
273308
defer server.Stop()
274309

275-
// invoke the EmptyCall service with bad options
276-
_, err = invokeEmptyCall(test.listenAddress, test.badOptions)
277-
assert.Error(t, err, "Expected error using bad dial options")
310+
// invoke the EmptyCall service with good options but should fail
311+
// until channel is created and root CAs are updated
312+
_, err = invokeEmptyCall(test.listenAddress, test.goodOptions)
313+
assert.Error(t, err, "Expected error invoking the EmptyCall service ")
314+
278315
// creating channel should update the trusted client roots
279316
test.createChannel()
317+
318+
// make sure we have the expected number of CAs
319+
appCAs, ordererCAs := comm.GetCASupport().GetClientRootCAs()
320+
assert.Equal(t, test.numAppCAs, len(appCAs),
321+
"Did not find expected number of app CAs for channel")
322+
assert.Equal(t, test.numOrdererCAs, len(ordererCAs),
323+
"Did not find expected number of orderer CAs for channel")
324+
280325
// invoke the EmptyCall service with good options
281326
_, err = invokeEmptyCall(test.listenAddress, test.goodOptions)
282327
assert.NoError(t, err, "Failed to invoke the EmptyCall service")
283328

329+
// invoke the EmptyCall service with bad options
330+
_, err = invokeEmptyCall(test.listenAddress, test.badOptions)
331+
assert.Error(t, err, "Expected error using bad dial options")
284332
}
285333
})
286334
}

core/peer/testdata/Org1-cert.pem

+8-8
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
-----BEGIN CERTIFICATE-----
2-
MIIB8TCCAZegAwIBAgIQDpf6otmwkc2A6rw31znJvDAKBggqhkjOPQQDAjBYMQsw
2+
MIIB8TCCAZegAwIBAgIQU59imQ+xl+FmwuiFyUgFezAKBggqhkjOPQQDAjBYMQsw
33
CQYDVQQGEwJVUzETMBEGA1UECBMKQ2FsaWZvcm5pYTEWMBQGA1UEBxMNU2FuIEZy
4-
YW5jaXNjbzENMAsGA1UEChMET3JnMTENMAsGA1UEAxMET3JnMTAeFw0xNzAzMTAx
5-
MzM0MTNaFw0yNzAzMDgxMzM0MTNaMFgxCzAJBgNVBAYTAlVTMRMwEQYDVQQIEwpD
4+
YW5jaXNjbzENMAsGA1UEChMET3JnMTENMAsGA1UEAxMET3JnMTAeFw0xNzA1MDgw
5+
OTMwMzRaFw0yNzA1MDYwOTMwMzRaMFgxCzAJBgNVBAYTAlVTMRMwEQYDVQQIEwpD
66
YWxpZm9ybmlhMRYwFAYDVQQHEw1TYW4gRnJhbmNpc2NvMQ0wCwYDVQQKEwRPcmcx
7-
MQ0wCwYDVQQDEwRPcmcxMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAERtiI6lfR
8-
iYg+Qb/vzO2tGRyY4+V2sAmNEgtm2GvEx8OekOLKJBq0HANz9stONIoUZxPcCfcB
9-
U2DNiUPOrxjVWqNDMEEwDgYDVR0PAQH/BAQDAgGmMA8GA1UdJQQIMAYGBFUdJQAw
7+
MQ0wCwYDVQQDEwRPcmcxMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEFkpP6EqE
8+
87ghFi25UWLvgPatxDiYKYaVSPvpo/XDJ0+9uUmK/C2r5Bvvxx1t8eTROwN77tEK
9+
r+jbJIxX3ZYQMKNDMEEwDgYDVR0PAQH/BAQDAgGmMA8GA1UdJQQIMAYGBFUdJQAw
1010
DwYDVR0TAQH/BAUwAwEB/zANBgNVHQ4EBgQEAQIDBDAKBggqhkjOPQQDAgNIADBF
11-
AiEA2Aonayo68RgTKhtkR3vpP63e/0g1hyWyF2WKRcogj+gCIFetrCAGO7L6is7Q
12-
d0HEDbtymkO1LlIYoaTj1MO0vDDu
11+
AiEA1Xkrpq+wrmfVVuY12dJfMQlSx+v0Q3cYce9BE1i2mioCIAzqyduK/lHPI81b
12+
nWiU9JF9dRQ69dEV9dxd/gzamfFU
1313
-----END CERTIFICATE-----

0 commit comments

Comments
 (0)