diff --git a/CHANGELOG.md b/CHANGELOG.md index 0bacb05a65..a13e18156a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ Changelog for NeoFS Node - `neo-go` RPC connection loss handling (#1337) - Concurrent morph cache misses (#1248) - Double voting for validators on IR startup (#2365) +- Skip unexpected notary events on notary request parsing step (#2315) ### Removed - Deprecated `morph.rpc_endpoint` SN and `morph.endpoint.client` IR config sections (#2400) diff --git a/pkg/morph/event/listener.go b/pkg/morph/event/listener.go index 1a9c078e1c..47d10c5e58 100644 --- a/pkg/morph/event/listener.go +++ b/pkg/morph/event/listener.go @@ -328,7 +328,7 @@ func (l *listener) parseAndHandleNotary(nr *result.NotaryRequestEvent) { notaryEvent, err := l.notaryEventsPreparator.Prepare(nr.NotaryRequest) if err != nil { switch { - case errors.Is(err, ErrTXAlreadyHandled): + case errors.Is(err, ErrTXAlreadyHandled) || errors.Is(err, ErrUnknownEvent): case errors.Is(err, ErrMainTXExpired): l.log.Warn("skip expired main TX notary event", zap.String("error", err.Error()), @@ -515,6 +515,8 @@ func (l *listener) SetNotaryParser(pi NotaryParserInfo) { l.notaryParsers[pi.notaryRequestTypes] = pi.parser() } + l.notaryEventsPreparator.allowNotaryEvent(pi.notaryScriptWithHash) + log.Info("registered new event parser") } diff --git a/pkg/morph/event/notary_preparator.go b/pkg/morph/event/notary_preparator.go index cb739201e6..3b17c34318 100644 --- a/pkg/morph/event/notary_preparator.go +++ b/pkg/morph/event/notary_preparator.go @@ -5,6 +5,7 @@ import ( "encoding/binary" "errors" "fmt" + "sync" lru "github.com/hashicorp/golang-lru/v2" "github.com/nspcc-dev/neo-go/pkg/core/interop/interopnames" @@ -43,6 +44,9 @@ var ( // ErrMainTXExpired is returned if received fallback TX is already valid. ErrMainTXExpired = errors.New("received main tx has expired") + + // ErrUnknownEvent is returned if main transaction is not expected. + ErrUnknownEvent = errors.New("received notary request is not allowed") ) // BlockCounter must return block count of the network @@ -76,6 +80,9 @@ type preparator struct { // just an optimization, already handled TX are not gonna // be signed once more anyway alreadyHandledTXs *lru.Cache[util.Uint256, struct{}] + + m *sync.RWMutex + allowedEvents map[notaryScriptWithHash]struct{} } // notaryPreparator inits and returns preparator. @@ -104,6 +111,8 @@ func notaryPreparator(prm PreparatorPrm) preparator { alphaKeys: prm.AlphaKeys, blockCounter: prm.BlockCounter, alreadyHandledTXs: cache, + m: &sync.RWMutex{}, + allowedEvents: make(map[notaryScriptWithHash]struct{}), } } @@ -114,6 +123,9 @@ func notaryPreparator(prm PreparatorPrm) preparator { // transaction is expected to be received one more time // from the Notary service but already signed. This happens // since every notary call is a new notary request in fact. +// +// Returns ErrUnknownEvent if main transactions is not an +// expected contract call. func (p preparator) Prepare(nr *payload.P2PNotaryRequest) (NotaryEvent, error) { if _, ok := p.alreadyHandledTXs.Get(nr.MainTransaction.Hash()); ok { // received already signed and sent TX @@ -208,6 +220,18 @@ func (p preparator) Prepare(nr *payload.P2PNotaryRequest) (NotaryEvent, error) { // retrieve contract's method contractMethod := string(ops[opsLen-3].param) + eventType := NotaryTypeFromString(contractMethod) + + p.m.RLock() + _, allowed := p.allowedEvents[notaryScriptWithHash{ + scriptHashValue: scriptHashValue{contractHash}, + notaryRequestType: notaryRequestType{eventType}, + }] + p.m.RUnlock() + + if !allowed { + return nil, ErrUnknownEvent + } // check if there is a call flag(must be in range [0:15)) callFlag := callflag.CallFlag(ops[opsLen-4].code - opcode.PUSH0) @@ -231,12 +255,19 @@ func (p preparator) Prepare(nr *payload.P2PNotaryRequest) (NotaryEvent, error) { return parsedNotaryEvent{ hash: contractHash, - notaryType: NotaryTypeFromString(contractMethod), + notaryType: eventType, params: args, raw: nr, }, nil } +func (p preparator) allowNotaryEvent(filter notaryScriptWithHash) { + p.m.Lock() + defer p.m.Unlock() + + p.allowedEvents[filter] = struct{}{} +} + func (p preparator) validateParameterOpcodes(ops []Op) error { l := len(ops) diff --git a/pkg/morph/event/notary_preparator_test.go b/pkg/morph/event/notary_preparator_test.go index 64245dcdc8..735784e905 100644 --- a/pkg/morph/event/notary_preparator_test.go +++ b/pkg/morph/event/notary_preparator_test.go @@ -2,6 +2,7 @@ package event import ( "fmt" + "strconv" "testing" "github.com/nspcc-dev/neo-go/pkg/vm" @@ -21,6 +22,8 @@ import ( "github.com/stretchr/testify/require" ) +const contractMethod = "test" + var ( alphaKeys keys.PublicKeys wrongAlphaKeys keys.PublicKeys @@ -63,12 +66,17 @@ func TestPrepare_IncorrectScript(t *testing.T) { }, ) + preparator.allowNotaryEvent(notaryScriptWithHash{ + notaryRequestType: notaryRequestType{contractMethod}, + scriptHashValue: scriptHashValue{scriptHash}, + }) + for _, dummyMultisig := range []bool{true, false} { // try both empty and dummy multisig/Notary invocation witness script t.Run(fmt.Sprintf("not contract call, compat: %t", dummyMultisig), func(t *testing.T) { bw := io.NewBufBinWriter() emit.Int(bw.BinWriter, 4) - emit.String(bw.BinWriter, "test") + emit.String(bw.BinWriter, contractMethod) emit.Bytes(bw.BinWriter, scriptHash.BytesBE()) emit.Syscall(bw.BinWriter, interopnames.SystemContractCallNative) // any != interopnames.SystemContractCall @@ -83,7 +91,7 @@ func TestPrepare_IncorrectScript(t *testing.T) { bw := io.NewBufBinWriter() emit.Int(bw.BinWriter, -1) - emit.String(bw.BinWriter, "test") + emit.String(bw.BinWriter, contractMethod) emit.Bytes(bw.BinWriter, scriptHash.BytesBE()) emit.Syscall(bw.BinWriter, interopnames.SystemContractCall) @@ -448,14 +456,20 @@ func TestPrepare_CorrectNR(t *testing.T) { for _, test := range tests { for i := 0; i < 1; i++ { // run tests against 3 and 4 witness NR - for _, dummyMultisig := range []bool{true, false} { // run tests against empty and dummy multisig/Notary witness + for j, dummyMultisig := range []bool{true, false} { // run tests against empty and dummy multisig/Notary witness + method := test.method + strconv.FormatInt(int64(j), 10) + preparator.allowNotaryEvent(notaryScriptWithHash{ + notaryRequestType: notaryRequestType{NotaryTypeFromString(method)}, + scriptHashValue: scriptHashValue{scriptHash}, + }) + additionalWitness := i == 0 - nr := correctNR(script(test.hash, test.method, test.args...), dummyMultisig, additionalWitness) + nr := correctNR(script(test.hash, method, test.args...), dummyMultisig, additionalWitness) event, err := preparator.Prepare(nr) require.NoError(t, err) - require.Equal(t, test.method, event.Type().String()) + require.Equal(t, method, event.Type().String()) require.Equal(t, test.hash.StringLE(), event.ScriptHash().StringLE()) // check args parsing @@ -488,6 +502,20 @@ func TestPrepare_CorrectNR(t *testing.T) { } } +func TestNotAllowedEvents(t *testing.T) { + preparator := notaryPreparator( + PreparatorPrm{ + alphaKeysSource(), + blockCounter{100, nil}, + }, + ) + + nr := correctNR(script(scriptHash, "test1", nil), false, false) + + _, err := preparator.Prepare(nr) + require.ErrorIs(t, err, ErrUnknownEvent) +} + func alphaKeysSource() client.AlphabetKeys { return func() (keys.PublicKeys, error) { return alphaKeys, nil diff --git a/pkg/morph/event/utils.go b/pkg/morph/event/utils.go index cd37a0f375..3a05334180 100644 --- a/pkg/morph/event/utils.go +++ b/pkg/morph/event/utils.go @@ -28,6 +28,10 @@ type scriptHashWithType struct { type notaryRequestTypes struct { notaryRequestMempoolType + notaryScriptWithHash +} + +type notaryScriptWithHash struct { notaryRequestType scriptHashValue }