Skip to content

Commit

Permalink
preparator: Skip unexpected TXs
Browse files Browse the repository at this point in the history
Notary requests can be sent by anybody who has GAS and knows how an Alphabet
node parses them. Add predefined allowed events on notary request parsing
level.

Closes #2315.

Signed-off-by: Pavel Karpy <[email protected]>
  • Loading branch information
carpawell committed Jul 28, 2023
1 parent ab860dc commit 4fd9775
Show file tree
Hide file tree
Showing 5 changed files with 73 additions and 7 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 3 additions & 1 deletion pkg/morph/event/listener.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()),
Expand Down Expand Up @@ -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")
}

Expand Down
33 changes: 32 additions & 1 deletion pkg/morph/event/notary_preparator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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{}),
}
}

Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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)

Expand Down
38 changes: 33 additions & 5 deletions pkg/morph/event/notary_preparator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package event

import (
"fmt"
"strconv"
"testing"

"github.com/nspcc-dev/neo-go/pkg/vm"
Expand All @@ -21,6 +22,8 @@ import (
"github.com/stretchr/testify/require"
)

const contractMethod = "test"

var (
alphaKeys keys.PublicKeys
wrongAlphaKeys keys.PublicKeys
Expand Down Expand Up @@ -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

Expand All @@ -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)

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions pkg/morph/event/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ type scriptHashWithType struct {

type notaryRequestTypes struct {
notaryRequestMempoolType
notaryScriptWithHash
}

type notaryScriptWithHash struct {
notaryRequestType
scriptHashValue
}
Expand Down

0 comments on commit 4fd9775

Please sign in to comment.