SmartBFT-Go / fabric

Integration of the BFT consensus library into Fabric
Apache License 2.0
30 stars 11 forks source link

orderer crashes down after receiving fuzzed channel tx. #286

Open SecTechTool opened 1 year ago

SecTechTool commented 1 year ago

System information: version: fabric 2.3 OS: ubuntu 20.04

Experiments setup: Using first-network in https://github.com/SmartBFT-Go/fabric-samples. Repeatedly sending fuzzed channel tx with the same Channel name.

Expected behavior: Orderers run as usual.

Actual behaviour: orderers crash down, and can not be recovered.

Bug location: https://github.com/SmartBFT-Go/fabric/blob/release-2.3-bft/orderer/common/multichannel/registrar.go

// CreateChain makes the Registrar create a consensus.Chain with the given name.
func (r *Registrar) CreateChain(chainName string) {
    lf, err := r.ledgerFactory.GetOrCreate(chainName)
    if err != nil {
        logger.Panicf("Failed obtaining ledger factory for %s: %v", chainName, err)
    }
    chain := r.GetChain(chainName)
    if chain != nil {
        logger.Infof("A chain of type %T for channel %s already exists. "+
            "Halting it.", chain.Chain, chainName)
        chain.Halt()
    }
    r.newChain(configTx(lf))
}

In line 529, should not panic directly.

log info:

2022-11-06 03:12:09.321 UTC [orderer.commmon.multichannel] createNewChain -> PANI 028 Error creating chain support: error creating consenter for channel: fuzztest: failed to restore persisted raft data: failed to create or read WAL: failed to open WAL: fileutil: file already locked
panic: Error creating chain support: error creating consenter for channel: fuzztest:  failed to create or read WAL: failed to open WAL: fileutil: file already locked

goroutine 118 [running]:
go.uber.org/zap/zapcore.(*CheckedEntry).Write(0xc00002c000, 0x0, 0x0, 0x0)
/go/src/github.com/hyperledger/fabric/vendor/go.uber.org/zap/zapcore/entry.go:230 +0x545
yacovm commented 1 year ago

I remember that to create a channel you need to sign the transaction by enough administrators in the network. I think you can restrict it to a threshold of a majority, so I am not sure this is possible in a production network.

In any case, can you check the same problem on the official Fabric with Raft?

SecTechTool commented 1 year ago

Sorry for the delayed response.

In the oficial Fabric with Raft, there are a locking mechanism in function 'CreateChain' and a exsiting name check in function 'newChain' to prevent this problem. The detailed code is in : https://github.com/hyperledger/fabric/commit/9da0410f622e49d92750ace50246ac75c381ac0a#

func (r *Registrar) CreateChain(chainName string) {
    lf, err := r.ledgerFactory.GetOrCreate(chainName)
    if err != nil {
        logger.Panicf("Failed obtaining ledger factory for %s: %v", chainName, err)
    }
    chain := r.GetChain(chainName)
    if chain != nil {
        logger.Infof("A chain of type %T for channel %s already exists. "+
            "Halting it.", chain.Chain, chainName)
        r.lock.Lock()
        chain.Halt()
        delete(r.chains, chainName)
        r.lock.Unlock()
    }
    r.newChain(configTx(lf))
}
func (r *Registrar) newChain(configtx *cb.Envelope) {
    r.lock.Lock()
    defer r.lock.Unlock()

    channelName, err := channelNameFromConfigTx(configtx)
    if err != nil {
        logger.Warnf("Failed extracting channel name: %v", err)
        return
    }

    // fixes https://github.com/hyperledger/fabric/issues/2931
    if existingChain, exists := r.chains[channelName]; exists {
        if _, isRaftChain := existingChain.Chain.(*etcdraft.Chain); isRaftChain {
            logger.Infof("Channel %s already created, skipping its creation", channelName)
            return
        }
    }

    cs := r.createNewChain(configtx)
    cs.start()
    logger.Infof("Created and started new channel %s", cs.ChannelID())
}

Maybe similar mechnism should be added here?

yacovm commented 1 year ago

That's why I asked you to check in Fabric... I know this bug.

https://github.com/hyperledger/fabric/pull/2934

Do you want to push a fix?