From 3cce10ac710e071ccc1638320f9ebd60df4f96f6 Mon Sep 17 00:00:00 2001 From: yacovm Date: Fri, 6 Dec 2019 23:40:16 +0200 Subject: [PATCH] [FAB-17220] Dynamically build TLS config in Raft client handshake When we expand the root TLS CA in the channel config, *after* Raft membership has expanded with an OSN that is issed a certificate by a new TLS CA, the TLS client handshake uses the old root CA pool and as a result the added orderer cannot be reached by the existing ones, because their dialers reject its certificate. This change set builds a dynamic transport credentials that re-computes the TLS config in every TLS client handshake. Expanded an integration test to ensure this works. Change-Id: I6578ba49f16e14b97eb4eef4feccdecbfe1b7015 Signed-off-by: yacovm --- core/comm/client.go | 10 +-- core/comm/client_test.go | 88 +++++++++++++++++++ core/comm/creds.go | 35 ++++++++ integration/nwo/configblock.go | 28 ++++++ integration/raft/config_test.go | 150 +++++++++++++++++++++++++++++++- orderer/common/cluster/util.go | 14 ++- 6 files changed, 317 insertions(+), 8 deletions(-) diff --git a/core/comm/client.go b/core/comm/client.go index 3533ca780f4..7817e172cb3 100644 --- a/core/comm/client.go +++ b/core/comm/client.go @@ -14,7 +14,6 @@ import ( "github.com/pkg/errors" "google.golang.org/grpc" - "google.golang.org/grpc/credentials" "google.golang.org/grpc/keepalive" ) @@ -188,12 +187,11 @@ func (client *GRPCClient) NewConnection(address string, tlsOptions ...TLSOption) // SetServerRootCAs / SetMaxRecvMsgSize / SetMaxSendMsgSize // to take effect on a per connection basis if client.tlsConfig != nil { - tlsConfigCopy := client.tlsConfig.Clone() - for _, tlsOption := range tlsOptions { - tlsOption(tlsConfigCopy) - } dialOpts = append(dialOpts, grpc.WithTransportCredentials( - credentials.NewTLS(tlsConfigCopy), + &DynamicClientCredentials{ + TLSConfig: client.tlsConfig, + TLSOptions: tlsOptions, + }, )) } else { dialOpts = append(dialOpts, grpc.WithInsecure()) diff --git a/core/comm/client_test.go b/core/comm/client_test.go index cb521685572..78eeed4d0a7 100644 --- a/core/comm/client_test.go +++ b/core/comm/client_test.go @@ -14,16 +14,21 @@ import ( "io/ioutil" "net" "path/filepath" + "sync" + "sync/atomic" "testing" "time" "github.com/golang/protobuf/proto" + "github.com/hyperledger/fabric/common/crypto/tlsgen" + "github.com/hyperledger/fabric/common/flogging" "github.com/hyperledger/fabric/core/comm" "github.com/hyperledger/fabric/core/comm/testpb" "github.com/pkg/errors" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "google.golang.org/grpc" + "google.golang.org/grpc/connectivity" "google.golang.org/grpc/credentials" ) @@ -579,3 +584,86 @@ func TestCertPoolOverride(t *testing.T) { RootCAs: &x509.CertPool{}, }, testConfig) } + +func TestDynamicClientTLSLoading(t *testing.T) { + t.Parallel() + ca1, err := tlsgen.NewCA() + assert.NoError(t, err) + + ca2, err := tlsgen.NewCA() + assert.NoError(t, err) + + clientKP, err := ca1.NewClientCertKeyPair() + assert.NoError(t, err) + + serverKP, err := ca2.NewServerCertKeyPair("127.0.0.1") + assert.NoError(t, err) + + client, err := comm.NewGRPCClient(comm.ClientConfig{ + AsyncConnect: true, + Timeout: time.Second * 1, + SecOpts: comm.SecureOptions{ + UseTLS: true, + ServerRootCAs: [][]byte{ca1.CertBytes()}, + Certificate: clientKP.Cert, + Key: clientKP.Key, + }, + }) + assert.NoError(t, err) + + server, err := comm.NewGRPCServer("127.0.0.1:0", comm.ServerConfig{ + Logger: flogging.MustGetLogger("test"), + SecOpts: comm.SecureOptions{ + UseTLS: true, + Key: serverKP.Key, + Certificate: serverKP.Cert, + }, + }) + assert.NoError(t, err) + + var wg sync.WaitGroup + wg.Add(1) + + go func() { + defer wg.Done() + server.Start() + }() + + var dynamicRootCerts atomic.Value + dynamicRootCerts.Store(ca1.CertBytes()) + + conn, err := client.NewConnection(server.Address(), func(tlsConfig *tls.Config) { + tlsConfig.RootCAs = x509.NewCertPool() + tlsConfig.RootCAs.AppendCertsFromPEM(dynamicRootCerts.Load().([]byte)) + }) + assert.NoError(t, err) + assert.NotNil(t, conn) + + waitForConnState := func(state connectivity.State, succeedOrFail string) { + deadline := time.Now().Add(time.Second * 30) + for conn.GetState() != state { + time.Sleep(time.Millisecond * 10) + if time.Now().After(deadline) { + t.Fatalf("Test timed out, waited for connection to %s", succeedOrFail) + } + } + } + + // Poll the connection state to wait for it to fail + waitForConnState(connectivity.TransientFailure, "fail") + + // Update the TLS root CAs with the good one + dynamicRootCerts.Store(ca2.CertBytes()) + + // Reset exponential back-off to make the test faster + conn.ResetConnectBackoff() + + // Poll the connection state to wait for it to succeed + waitForConnState(connectivity.Ready, "succeed") + + err = conn.Close() + assert.NoError(t, err) + + server.Stop() + wg.Wait() +} diff --git a/core/comm/creds.go b/core/comm/creds.go index 07387599881..8f9b2c120b0 100644 --- a/core/comm/creds.go +++ b/core/comm/creds.go @@ -18,6 +18,7 @@ import ( var ( ErrClientHandshakeNotImplemented = errors.New("core/comm: client handshakes are not implemented with serverCreds") + ErrServerHandshakeNotImplemented = errors.New("core/comm: server handshakes are not implemented with clientCreds") ErrOverrideHostnameNotSupported = errors.New("core/comm: OverrideServerName is not supported") // alpnProtoStr are the specified application level protocols for gRPC. @@ -85,3 +86,37 @@ func (sc *serverCreds) Clone() credentials.TransportCredentials { func (sc *serverCreds) OverrideServerName(string) error { return ErrOverrideHostnameNotSupported } + +type DynamicClientCredentials struct { + TLSConfig *tls.Config + TLSOptions []TLSOption +} + +func (dtc *DynamicClientCredentials) latestConfig() *tls.Config { + tlsConfigCopy := dtc.TLSConfig.Clone() + for _, tlsOption := range dtc.TLSOptions { + tlsOption(tlsConfigCopy) + } + return tlsConfigCopy +} + +func (dtc *DynamicClientCredentials) ClientHandshake(ctx context.Context, authority string, rawConn net.Conn) (net.Conn, credentials.AuthInfo, error) { + return credentials.NewTLS(dtc.latestConfig()).ClientHandshake(ctx, authority, rawConn) +} + +func (dtc *DynamicClientCredentials) ServerHandshake(rawConn net.Conn) (net.Conn, credentials.AuthInfo, error) { + return nil, nil, ErrServerHandshakeNotImplemented +} + +func (dtc *DynamicClientCredentials) Info() credentials.ProtocolInfo { + return credentials.NewTLS(dtc.latestConfig()).Info() +} + +func (dtc *DynamicClientCredentials) Clone() credentials.TransportCredentials { + return credentials.NewTLS(dtc.latestConfig()) +} + +func (dtc *DynamicClientCredentials) OverrideServerName(name string) error { + dtc.TLSConfig.ServerName = name + return nil +} diff --git a/integration/nwo/configblock.go b/integration/nwo/configblock.go index 8f4e4252f86..51f028ed05e 100644 --- a/integration/nwo/configblock.go +++ b/integration/nwo/configblock.go @@ -14,6 +14,7 @@ import ( "github.com/golang/protobuf/proto" "github.com/hyperledger/fabric-protos-go/common" + "github.com/hyperledger/fabric-protos-go/msp" protosorderer "github.com/hyperledger/fabric-protos-go/orderer" "github.com/hyperledger/fabric/integration/nwo/commands" "github.com/hyperledger/fabric/internal/configtxlator/update" @@ -290,6 +291,9 @@ func UnmarshalBlockFromFile(blockFile string) *common.Block { // ConsensusMetadataMutator receives ConsensusType.Metadata and mutates it. type ConsensusMetadataMutator func([]byte) []byte +// MSPMutator receives FabricMSPConfig and mutates it. +type MSPMutator func(config msp.FabricMSPConfig) msp.FabricMSPConfig + // UpdateConsensusMetadata executes a config update that updates the consensus // metadata according to the given ConsensusMetadataMutator. func UpdateConsensusMetadata(network *Network, peer *Peer, orderer *Orderer, channel string, mutateMetadata ConsensusMetadataMutator) { @@ -310,3 +314,27 @@ func UpdateConsensusMetadata(network *Network, peer *Peer, orderer *Orderer, cha UpdateOrdererConfig(network, orderer, channel, config, updatedConfig, peer, orderer) } + +func UpdateOrdererMSP(network *Network, peer *Peer, orderer *Orderer, channel, orgID string, mutateMSP MSPMutator) { + config := GetConfig(network, peer, orderer, channel) + updatedConfig := proto.Clone(config).(*common.Config) + + // Unpack the MSP config + rawMSPConfig := updatedConfig.ChannelGroup.Groups["Orderer"].Groups[orgID].Values["MSP"] + mspConfig := &msp.MSPConfig{} + err := proto.Unmarshal(rawMSPConfig.Value, mspConfig) + Expect(err).NotTo(HaveOccurred()) + + fabricConfig := &msp.FabricMSPConfig{} + err = proto.Unmarshal(mspConfig.Config, fabricConfig) + Expect(err).NotTo(HaveOccurred()) + + // Mutate it as we are asked + *fabricConfig = mutateMSP(*fabricConfig) + + // Wrap it back into the config + mspConfig.Config = protoutil.MarshalOrPanic(fabricConfig) + rawMSPConfig.Value = protoutil.MarshalOrPanic(mspConfig) + + UpdateOrdererConfig(network, orderer, channel, config, updatedConfig, peer, orderer) +} diff --git a/integration/raft/config_test.go b/integration/raft/config_test.go index d9ccf80bcf6..645e54ad346 100644 --- a/integration/raft/config_test.go +++ b/integration/raft/config_test.go @@ -8,6 +8,10 @@ package raft import ( "bytes" + "crypto/ecdsa" + "crypto/rand" + "crypto/x509" + "encoding/pem" "fmt" "io/ioutil" "os" @@ -236,7 +240,7 @@ var _ = Describe("EndToEnd reconfiguration and onboarding", func() { }) When("a single node cluster is expanded", func() { - It("is still possible to onboard the new cluster member", func() { + It("is still possible to onboard the new cluster member and then another one with a different TLS root CA", func() { launch := func(o *nwo.Orderer) { runner := network.OrdererRunner(o) ordererRunners = append(ordererRunners, runner) @@ -302,6 +306,109 @@ var _ = Describe("EndToEnd reconfiguration and onboarding", func() { launch(orderer2) By("Waiting for a leader to be re-elected") findLeader(ordererRunners) + + // In the next part of the test we're going to bring up a third node with a different TLS root CA + // and we're going to add the TLS root CA *after* we add it to the channel, to ensure + // that we can dynamically update TLS root CAs in Raft while membership stays the same. + + By("Creating configuration for a third orderer with a different TLS root CA") + orderer3 := &nwo.Orderer{ + Name: "orderer3", + Organization: "OrdererOrg", + } + ports = nwo.Ports{} + for _, portName := range nwo.OrdererPortNames() { + ports[portName] = network.ReservePort() + } + network.PortsByOrdererID[orderer3.ID()] = ports + network.Orderers = append(network.Orderers, orderer3) + network.GenerateOrdererConfig(orderer3) + + tmpDir, err := ioutil.TempDir("", "e2e-etcfraft_reconfig") + Expect(err).NotTo(HaveOccurred()) + defer os.RemoveAll(tmpDir) + + sess, err := network.Cryptogen(commands.Generate{ + Config: network.CryptoConfigPath(), + Output: tmpDir, + }) + Expect(err).NotTo(HaveOccurred()) + Eventually(sess, network.EventuallyTimeout).Should(gexec.Exit(0)) + + name := network.Orderers[0].Name + domain := network.Organization(network.Orderers[0].Organization).Domain + nameDomain := fmt.Sprintf("%s.%s", name, domain) + ordererTLSPath := filepath.Join(tmpDir, "ordererOrganizations", domain, "orderers", nameDomain, "tls") + + caKeyPath := filepath.Join(tmpDir, "ordererOrganizations", domain, "tlsca", "priv_sk") + caCertPath := filepath.Join(tmpDir, "ordererOrganizations", domain, "tlsca", fmt.Sprintf("tlsca.%s-cert.pem", domain)) + + caKey, err := ioutil.ReadFile(caKeyPath) + Expect(err).NotTo(HaveOccurred()) + + caCert, err := ioutil.ReadFile(caCertPath) + Expect(err).NotTo(HaveOccurred()) + + thirdOrdererCertificatePath := filepath.Join(ordererTLSPath, "server.crt") + thirdOrdererCertificate, err := ioutil.ReadFile(thirdOrdererCertificatePath) + Expect(err).NotTo(HaveOccurred()) + + By("Changing its subject name") + caCert, thirdOrdererCertificate = changeSubjectName(caCert, caKey, thirdOrdererCertificate, "tlsca2") + + By("Updating it on the file system") + err = ioutil.WriteFile(caCertPath, caCert, 0644) + Expect(err).NotTo(HaveOccurred()) + err = ioutil.WriteFile(thirdOrdererCertificatePath, thirdOrdererCertificate, 0644) + Expect(err).NotTo(HaveOccurred()) + + By("Overwriting the TLS directory of the new orderer") + for _, fileName := range []string{"server.crt", "server.key", "ca.crt"} { + dst := filepath.Join(network.OrdererLocalTLSDir(orderer3), fileName) + + data, err := ioutil.ReadFile(filepath.Join(ordererTLSPath, fileName)) + Expect(err).NotTo(HaveOccurred()) + + err = ioutil.WriteFile(dst, data, 0644) + Expect(err).NotTo(HaveOccurred()) + } + + By("Adding the third orderer to the channel") + addConsenter(network, peer, orderer, "systemchannel", etcdraft.Consenter{ + ServerTlsCert: thirdOrdererCertificate, + ClientTlsCert: thirdOrdererCertificate, + Host: "127.0.0.1", + Port: uint32(network.OrdererPort(orderer3, nwo.ClusterPort)), + }) + + By("Obtaining the last config block from the orderer once more to update the bootstrap file") + configBlock = nwo.GetConfigBlock(network, peer, orderer, "systemchannel") + err = ioutil.WriteFile(filepath.Join(testDir, "systemchannel_block.pb"), protoutil.MarshalOrPanic(configBlock), 0644) + Expect(err).NotTo(HaveOccurred()) + + By("Launching the third orderer") + launch(orderer3) + + By("Expanding the TLS root CA certificates") + nwo.UpdateOrdererMSP(network, peer, orderer, "systemchannel", "OrdererOrg", func(config msp.FabricMSPConfig) msp.FabricMSPConfig { + config.TlsRootCerts = append(config.TlsRootCerts, caCert) + return config + }) + + By("Waiting for orderer3 to see the leader") + leader := findLeader([]*ginkgomon.Runner{ordererRunners[2]}) + leaderIndex := leader - 1 + + fmt.Fprint(GinkgoWriter, "Killing the leader", leader) + ordererProcesses[leaderIndex].Signal(syscall.SIGTERM) + Eventually(ordererProcesses[leaderIndex].Wait(), network.EventuallyTimeout).Should(Receive()) + + By("Ensuring orderer3 detects leader loss") + leaderLoss := fmt.Sprintf("Raft leader changed: %d -> 0", leader) + Eventually(ordererRunners[2].Err(), network.EventuallyTimeout, time.Second).Should(gbytes.Say(leaderLoss)) + + By("Waiting for the leader to be re-elected") + findLeader([]*ginkgomon.Runner{ordererRunners[2]}) }) }) @@ -1619,3 +1726,44 @@ func updateEtcdRaftMetadata(network *nwo.Network, peer *nwo.Peer, orderer *nwo.O return newMetadata }) } + +func changeSubjectName(caCertPEM, caKeyPEM, leafPEM []byte, newSubjectName string) (newCA, newLeaf []byte) { + keyAsDER, _ := pem.Decode(caKeyPEM) + caKeyWithoutType, err := x509.ParsePKCS8PrivateKey(keyAsDER.Bytes) + Expect(err).NotTo(HaveOccurred()) + caKey := caKeyWithoutType.(*ecdsa.PrivateKey) + + caCertAsDER, _ := pem.Decode(caCertPEM) + caCert, err := x509.ParseCertificate(caCertAsDER.Bytes) + Expect(err).NotTo(HaveOccurred()) + + // Change its subject name + caCert.Subject.CommonName = newSubjectName + caCert.Issuer.CommonName = newSubjectName + caCert.RawTBSCertificate = nil + caCert.RawSubjectPublicKeyInfo = nil + caCert.Raw = nil + caCert.RawSubject = nil + caCert.RawIssuer = nil + + // The CA signs its own certificate + caCertBytes, err := x509.CreateCertificate(rand.Reader, caCert, caCert, caCert.PublicKey, caKey) + Expect(err).NotTo(HaveOccurred()) + + // Now it's the turn of the leaf certificate + leafAsDER, _ := pem.Decode(leafPEM) + leafCert, err := x509.ParseCertificate(leafAsDER.Bytes) + Expect(err).NotTo(HaveOccurred()) + + leafCert.Raw = nil + leafCert.RawIssuer = nil + leafCert.RawTBSCertificate = nil + + // The CA signs the leaf cert + leafCertBytes, err := x509.CreateCertificate(rand.Reader, leafCert, caCert, leafCert.PublicKey, caKey) + Expect(err).NotTo(HaveOccurred()) + + newCA = pem.EncodeToMemory(&pem.Block{Bytes: caCertBytes, Type: "CERTIFICATE"}) + newLeaf = pem.EncodeToMemory(&pem.Block{Bytes: leafCertBytes, Type: "CERTIFICATE"}) + return +} diff --git a/orderer/common/cluster/util.go b/orderer/common/cluster/util.go index d6004cf139b..227aefae755 100644 --- a/orderer/common/cluster/util.go +++ b/orderer/common/cluster/util.go @@ -8,6 +8,7 @@ package cluster import ( "bytes" + "crypto/tls" "crypto/x509" "encoding/hex" "encoding/json" @@ -132,7 +133,18 @@ func (dialer *PredicateDialer) Dial(address string, verifyFunc RemoteVerifier) ( if err != nil { return nil, errors.WithStack(err) } - return client.NewConnection(address) + return client.NewConnection(address, func(tlsConfig *tls.Config) { + // We need to dynamically overwrite the TLS root CAs, + // as they may be updated. + dialer.lock.RLock() + serverRootCAs := dialer.Config.Clone().SecOpts.ServerRootCAs + dialer.lock.RUnlock() + + tlsConfig.RootCAs = x509.NewCertPool() + for _, pem := range serverRootCAs { + tlsConfig.RootCAs.AppendCertsFromPEM(pem) + } + }) } // DERtoPEM returns a PEM representation of the DER