Skip to content

Commit

Permalink
Do not allow verifier panic to take down the server
Browse files Browse the repository at this point in the history
If a verifier panics, do not crash. Instead report the error
and treat the verifier as if it failed.
  • Loading branch information
Jeff R. Allen committed Jun 20, 2018
1 parent 165068b commit b483914
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 1 deletion.
15 changes: 14 additions & 1 deletion skipchain/skipchain.go
Original file line number Diff line number Diff line change
Expand Up @@ -943,7 +943,20 @@ func (s *Service) bftForwardLinkLevel0(msg, data []byte) bool {
log.Lvlf2("Found no user verification for %x", ver)
return false
}
if !f(fl.To, fs.Newest) {
// Now we call the verification function. Wrap up f() inside of
// g(), so that we can recover panics from f().
g := func(to []byte, newest *SkipBlock) (out bool) {
defer func() {
if re := recover(); re != nil {
log.Error("verification function panic: " + re.(string))
out = false
}
}()
out = f(to, newest)
return
}

if !g(fl.To, fs.Newest) {
fname := runtime.FuncForPC(reflect.ValueOf(f).Pointer()).Name()
log.Lvlf2("verification function failed: %v %s", fname, ver)
return false
Expand Down
26 changes: 26 additions & 0 deletions skipchain/skipchain_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,32 @@ func TestService_ProtocolVerification(t *testing.T) {
}
}

func TestService_ProtocolVerificationPanic(t *testing.T) {
// Testing whether we sign correctly the SkipBlocks
local := onet.NewLocalTest(cothority.Suite)
defer waitPropagationFinished(t, local)
defer local.CloseAll()
_, el, s := local.MakeSRS(cothority.Suite, 3, skipchainSID)
s1 := s.(*Service)
verifyFunc := func(newID []byte, newSB *SkipBlock) bool {
panic("nope")
}
verifyID := VerifierID(uuid.NewV1())
for _, s := range local.Services {
s[skipchainSID].(*Service).registerVerification(verifyID, verifyFunc)
}

sbRoot, err := makeGenesisRosterArgs(s1, el, nil, []VerifierID{verifyID}, 1, 1)
require.NoError(t, err)
sbNext := sbRoot.Copy()
sbNext.BackLinkIDs = []SkipBlockID{sbRoot.Hash}
_, err = s1.StoreSkipBlock(&StoreSkipBlock{TargetSkipChainID: sbRoot.Hash, NewBlock: sbNext})
require.Error(t, err)

// We expect a failure here, because the verification function is panicing.
require.Contains(t, err.Error(), "couldn't sign forward-link")
}

func TestService_RegisterVerification(t *testing.T) {
// Testing whether we sign correctly the SkipBlocks
onet.RegisterNewService("ServiceVerify", newServiceVerify)
Expand Down

0 comments on commit b483914

Please sign in to comment.