Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate TLS certs during raft consenter addition #1223

Merged
merged 2 commits into from May 8, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 12 additions & 0 deletions common/crypto/tlsgen/ca.go
Expand Up @@ -29,6 +29,8 @@ type CA interface {
// CertBytes returns the certificate of the CA in PEM encoding
CertBytes() []byte

NewIntermediateCA() (CA, error)

// newCertKeyPair returns a certificate and private key pair and nil,
// or nil, error in case of failure
// The certificate is signed by the CA and is used for TLS client authentication
Expand All @@ -55,6 +57,16 @@ func NewCA() (CA, error) {
return c, nil
}

func (c *ca) NewIntermediateCA() (CA, error) {
intermediateCA := &ca{}
var err error
intermediateCA.caCert, err = newCertKeyPair(true, false, "", c.caCert.Signer, c.caCert.TLSCert)
if err != nil {
return nil, err
}
return intermediateCA, nil
}

// CertBytes returns the certificate of the CA in PEM encoding
func (c *ca) CertBytes() []byte {
return c.caCert.Cert
Expand Down
87 changes: 64 additions & 23 deletions integration/raft/config_test.go
Expand Up @@ -26,6 +26,7 @@ import (
"github.com/hyperledger/fabric-protos-go/msp"
protosorderer "github.com/hyperledger/fabric-protos-go/orderer"
"github.com/hyperledger/fabric-protos-go/orderer/etcdraft"
"github.com/hyperledger/fabric/common/crypto/tlsgen"
"github.com/hyperledger/fabric/integration/nwo"
"github.com/hyperledger/fabric/integration/nwo/commands"
"github.com/hyperledger/fabric/internal/configtxgen/encoder"
Expand Down Expand Up @@ -56,7 +57,7 @@ var _ = Describe("EndToEnd reconfiguration and onboarding", func() {
ordererProcesses = nil

var err error
testDir, err = ioutil.TempDir("", "e2e-etcfraft_reconfig")
testDir, err = ioutil.TempDir("", "e2e-etcdraft_reconfig")
Expect(err).NotTo(HaveOccurred())

client, err = docker.NewClientFromEnv()
Expand Down Expand Up @@ -303,9 +304,10 @@ var _ = Describe("EndToEnd reconfiguration and onboarding", func() {
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.
// In the next part of the test we're going to bring up a third node
// with a different TLS root CA. We're then going to remove the TLS
// root CA and restart the orderer, 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{
Expand Down Expand Up @@ -369,20 +371,12 @@ var _ = Describe("EndToEnd reconfiguration and onboarding", func() {
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")
By("Launching orderer3")
launch(orderer3)

By("Expanding the TLS root CA certificates")
Expand All @@ -391,20 +385,67 @@ var _ = Describe("EndToEnd reconfiguration and onboarding", func() {
return config
})

By("Adding orderer3 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("Waiting for orderer3 to see the leader")
leader := findLeader([]*ginkgomon.Runner{ordererRunners[2]})
leaderIndex := leader - 1
findLeader([]*ginkgomon.Runner{ordererRunners[2]})

fmt.Fprint(GinkgoWriter, "Killing the leader", leader)
ordererProcesses[leaderIndex].Signal(syscall.SIGTERM)
Eventually(ordererProcesses[leaderIndex].Wait(), network.EventuallyTimeout).Should(Receive())
By("Removing orderer3's TLS root CA certificate")
nwo.UpdateOrdererMSP(network, peer, orderer, "systemchannel", "OrdererOrg", func(config msp.FabricMSPConfig) msp.FabricMSPConfig {
config.TlsRootCerts = config.TlsRootCerts[:len(config.TlsRootCerts)-1]
return config
})

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("Killing orderer3")
o3Proc := ordererProcesses[2]
o3Proc.Signal(syscall.SIGKILL)
Eventually(o3Proc.Wait(), network.EventuallyTimeout).Should(Receive(MatchError("exit status 137")))

By("Restarting orderer3")
o3Runner := network.OrdererRunner(orderer3)
ordererRunners[2] = o3Runner
o3Proc = ifrit.Invoke(o3Runner)
Eventually(o3Proc.Ready(), network.EventuallyTimeout).Should(BeClosed())
ordererProcesses[2] = o3Proc

By("Ensuring TLS handshakes fail with the other orderers")
for i, oRunner := range ordererRunners {
if i < 2 {
Eventually(oRunner.Err(), network.EventuallyTimeout).Should(gbytes.Say("TLS handshake failed with error tls: client didn't provide a certificate"))
Copy link
Contributor

@yacovm yacovm May 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it because the rest of the OSNs don't want to connect to complete the handshake once they see our server certificate?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure on the exact mechanics here but I can research/investigate.

continue
}
Eventually(oRunner.Err(), network.EventuallyTimeout).Should(gbytes.Say("TLS handshake failed with error remote error: tls: bad certificate"))
Eventually(oRunner.Err(), network.EventuallyTimeout).Should(gbytes.Say("Suspecting our own eviction from the channel"))
}

By("Waiting for the leader to be re-elected")
findLeader([]*ginkgomon.Runner{ordererRunners[2]})
By("Attemping to add a consenter with invalid certs")
// create new certs that are not in the channel config
ca, err := tlsgen.NewCA()
Expect(err).NotTo(HaveOccurred())
client, err := ca.NewClientCertKeyPair()
Expect(err).NotTo(HaveOccurred())

current, updated := consenterAdder(
network,
peer,
orderer,
"systemchannel",
etcdraft.Consenter{
ServerTlsCert: client.Cert,
ClientTlsCert: client.Cert,
Host: "127.0.0.1",
Port: uint32(network.OrdererPort(orderer3, nwo.ListenPort)),
},
)
sess = nwo.UpdateOrdererConfigSession(network, orderer, network.SystemChannel.Name, current, updated, peer, orderer)
Eventually(sess, network.EventuallyTimeout).Should(gexec.Exit(1))
Expect(sess.Err).To(gbytes.Say(fmt.Sprintf("BAD_REQUEST -- error applying config update to existing channel 'systemchannel': consensus metadata update for channel config update is invalid: verifying tls client cert with serial number %d: x509: certificate signed by unknown authority", client.TLSCert.SerialNumber)))
})
})

Expand Down
4 changes: 2 additions & 2 deletions orderer/consensus/etcdraft/chain.go
Expand Up @@ -942,7 +942,7 @@ func (c *Chain) detectConfChange(block *common.Block) *MembershipChanges {
c.sizeLimit = configMetadata.Options.SnapshotIntervalSize
}

changes, err := ComputeMembershipChanges(c.opts.BlockMetadata, c.opts.Consenters, configMetadata.Consenters)
changes, err := ComputeMembershipChanges(c.opts.BlockMetadata, c.opts.Consenters, configMetadata.Consenters, c.support.SharedConfig())
if err != nil {
c.logger.Panicf("illegal configuration change detected: %s", err)
}
Expand Down Expand Up @@ -1306,7 +1306,7 @@ func (c *Chain) ValidateConsensusMetadata(oldMetadataBytes, newMetadataBytes []b
// create the dummy parameters for ComputeMembershipChanges
dummyOldBlockMetadata, _ := ReadBlockMetadata(nil, oldMetadata)
dummyOldConsentersMap := CreateConsentersMap(dummyOldBlockMetadata, oldMetadata)
changes, err := ComputeMembershipChanges(dummyOldBlockMetadata, dummyOldConsentersMap, newMetadata.Consenters)
changes, err := ComputeMembershipChanges(dummyOldBlockMetadata, dummyOldConsentersMap, newMetadata.Consenters, c.support.SharedConfig())
if err != nil {
return err
}
Expand Down