Reject synthetic signature sent without transaction [#3417]

Closes #3417. Rejects authority signatures and other synthetic messages when sent without a transaction. Otherwise, healing can cause a synthetic message to fail by sending it without the transaction.

Changelog: fix

internal/core/execute/v2/block/exec_process.go

@@ -46,7 +46,7 @@ func (b *Block) Process(envelope *messaging.Envelope) ([]*protocol.TransactionSt
 	}
 
 	// Make sure every transaction is signed
-	err = checkForUnsignedTransactions(messages)
+	err = b.Executor.checkForUnsignedTransactions(messages)
 	if err != nil {
 		return nil, errors.UnknownError.Wrap(err)
 	}
@@ -229,26 +229,49 @@ func (d *bundle) process() ([]*protocol.TransactionStatus, error) {
 
 // checkForUnsignedTransactions returns an error if the message bundle includes
 // any unsigned transactions.
-func checkForUnsignedTransactions(messages []messaging.Message) error {
-	unsigned := set[[32]byte]{}
+func (x *Executor) checkForUnsignedTransactions(messages []messaging.Message) error {
+	// Record signatures and transactions
+	haveSigFor := map[[32]byte]bool{}
+	haveTxn := map[[32]byte]bool{}
 	for _, msg := range messages {
-		if msg, ok := msg.(*messaging.TransactionMessage); ok {
-			unsigned[msg.ID().Hash()] = struct{}{}
+		if x.globals.Active.ExecutorVersion.V2BaikonurEnabled() {
+			if _, ok := msg.(*messaging.BlockAnchor); ok {
+				continue
+			}
+		}
+
+		if msg, ok := messaging.UnwrapAs[messaging.MessageForTransaction](msg); ok {
+			// User signatures can be sent without a transaction, but authority
+			// signatures and other message-for-transaction types cannot
+			sig, ok := msg.(*messaging.SignatureMessage)
+			isUser := ok && sig.Signature.Type() != protocol.SignatureTypeAuthority
+			haveSigFor[msg.GetTxID().Hash()] = isUser
+		}
+
+		if txn, ok := msg.(*messaging.TransactionMessage); ok {
+			isRemote := txn.GetTransaction().Body.Type() == protocol.TransactionTypeRemote
+			haveTxn[txn.ID().Hash()] = isRemote
 		}
 	}
-	for _, msg := range messages {
-	again:
-		switch m := msg.(type) {
-		case messaging.MessageForTransaction:
-			delete(unsigned, m.GetTxID().Hash())
-		case interface{ Unwrap() messaging.Message }:
-			msg = m.Unwrap()
-			goto again
+
+	// If a transaction does not have a signature, reject the bundle
+	for txn := range haveTxn {
+		if _, has := haveSigFor[txn]; !has {
+			return errors.BadRequest.With("message bundle includes an unsigned transaction")
 		}
 	}
-	if len(unsigned) > 0 {
-		return errors.BadRequest.With("message bundle includes an unsigned transaction")
+
+	// If an authority signature or other synthetic message-for-transaction is
+	// sent without its transaction, reject the bundle
+	if x.globals.Active.ExecutorVersion.V2BaikonurEnabled() {
+		for txn, isUserSig := range haveSigFor {
+			isRemote, has := haveTxn[txn]
+			if !isUserSig && (!has || isRemote) {
+				return errors.BadRequest.With("message bundle is missing a transaction")
+			}
+		}
 	}
+
 	return nil
 }
 

internal/core/execute/v2/block/exec_validate.go

@@ -28,7 +28,7 @@ func (x *Executor) Validate(envelope *messaging.Envelope, _ bool) ([]*protocol.T
 	}
 
 	// Make sure every transaction is signed
-	err = checkForUnsignedTransactions(messages)
+	err = x.checkForUnsignedTransactions(messages)
 	if err != nil {
 		return nil, errors.UnknownError.Wrap(err)
 	}

internal/core/execute/v2/block/utils.go

@@ -17,21 +17,6 @@ import (
 	"gitlab.com/accumulatenetwork/accumulate/protocol"
 )
 
-// none is an empty struct.
-type none = struct{}
-
-// set is an unordered set of T implemented as a map of T to [none].
-type set[T comparable] map[T]none
-
-// Add adds a value to the set.
-func (s set[T]) Add(v T) { s[v] = none{} }
-
-// Remove removes a value from the set.
-func (s set[T]) Remove(v T) { delete(s, v) }
-
-// Hash checks if the set has the given value.
-func (s set[T]) Has(v T) bool { _, ok := s[v]; return ok }
-
 // orderedMap is an ordered map from K to V implemented with a builtin map,
 // slice of keys, and comparison function.
 type orderedMap[K comparable, V any] struct {

test/e2e/regression2_test.go

@@ -8,6 +8,7 @@ package e2e
 
 import (
 	"bytes"
+	"context"
 	"crypto/sha256"
 	"fmt"
 	"math/big"
@@ -1137,3 +1138,85 @@ func TestChainUpdateAnchor(t *testing.T) {
 	// Verify the anchor matches the receipt
 	require.Equal(t, r1.Receipt.LocalBlock, anchor.MinorBlockIndex)
 }
+
+func TestAuthoritySignatureWithoutTransaction(t *testing.T) {
+	cases := []struct {
+		Version ExecutorVersion
+		Fails   bool
+	}{
+		{Version: ExecutorVersionV2, Fails: false},
+		{Version: ExecutorVersionLatest, Fails: true},
+	}
+
+	for _, c := range cases {
+		t.Run(c.Version.String(), func(t *testing.T) {
+			alice := url.MustParse("alice")
+			aliceKey := acctesting.GenerateKey(alice)
+
+			// Capture the authority signature
+			var captured *messaging.Envelope
+			capFn := func(_ context.Context, _ *url.URL, env *messaging.Envelope) (send bool, err error) {
+				for _, m := range env.Messages {
+					syn, ok := m.(interface{ Data() *messaging.SynthFields })
+					if !ok {
+						continue
+					}
+					seq, ok := syn.Data().Message.(*messaging.SequencedMessage)
+					if !ok {
+						continue
+					}
+					sig, ok := seq.Message.(*messaging.SignatureMessage)
+					if !ok || sig.Signature.Type() != SignatureTypeAuthority {
+						continue
+					}
+					require.Nil(t, captured)
+					captured = env
+					return false, nil
+				}
+				return true, nil
+			}
+
+			// Initialize
+			sim := NewSim(t,
+				simulator.SimpleNetwork(t.Name(), 1, 1),
+				simulator.GenesisWithVersion(GenesisTime, c.Version),
+				simulator.CaptureDispatchedMessages(capFn),
+			)
+
+			MakeIdentity(t, sim.DatabaseFor(alice), alice, aliceKey[32:])
+			CreditCredits(t, sim.DatabaseFor(alice), alice.JoinPath("book", "1"), 1e9)
+
+			// Execute
+			sim.BuildAndSubmitTxnSuccessfully(
+				build.Transaction().For("bob.acme", "tokens").
+					BurnTokens(1, 0).
+					SignWith(alice, "book", "1").Version(1).Timestamp(1).PrivateKey(aliceKey))
+
+			sim.StepUntil(
+				True(func(h *Harness) bool { return captured != nil }))
+
+			// Remove the transaction
+			msgs, err := captured.Normalize()
+			require.NoError(t, err)
+			captured = new(messaging.Envelope)
+			var removed int
+			for _, msg := range msgs {
+				if msg.Type() == messaging.MessageTypeTransaction {
+					removed++
+				} else {
+					captured.Messages = append(captured.Messages, msg)
+				}
+			}
+			require.NotZero(t, removed)
+
+			// Resubmit
+			_, err = sim.SubmitTo("BVN0", captured)
+			if c.Fails {
+				require.Error(t, err)
+				require.Contains(t, err.Error(), "message bundle is missing a transaction")
+			} else {
+				require.NoError(t, err)
+			}
+		})
+	}
+}

test/simulator/dispatcher.go

@@ -33,20 +33,30 @@ func (fakeDispatcher) Send(context.Context) <-chan error {
 	return ch
 }
 
+type dispatchInterceptor = func(ctx context.Context, u *url.URL, env *messaging.Envelope) (send bool, err error)
+
 // dispatcher implements [block.Dispatcher] for the simulator.
 //
 // dispatcher maintains a separate bundle (slice) of messages for each call to
 // Submit to make it easier to write tests that drop certain messages.
 type dispatcher struct {
-	client    *services.Network
-	router    routing.Router
-	envelopes map[string][]*messaging.Envelope
+	client      *services.Network
+	router      routing.Router
+	envelopes   map[string][]*messaging.Envelope
+	interceptor dispatchInterceptor
 }
 
 var _ execute.Dispatcher = (*dispatcher)(nil)
 
 // Submit routes the envelope and adds it to the queue for a partition.
 func (d *dispatcher) Submit(ctx context.Context, u *url.URL, env *messaging.Envelope) error {
+	if d.interceptor != nil {
+		keep, err := d.interceptor(ctx, u, env)
+		if !keep || err != nil {
+			return err
+		}
+	}
+
 	partition, err := d.router.RouteAccount(u)
 	if err != nil {
 		return err

test/simulator/factory.go

@@ -44,11 +44,12 @@ type simFactory struct {
 	recordings RecordingFunc
 	abci       abciFunc
 
-	dropDispatchedMessages bool
-	skipProposalCheck      bool
-	ignoreDeliverResults   bool
-	ignoreCommitResults    bool
-	deterministic          bool
+	dropDispatchedMessages      bool
+	skipProposalCheck           bool
+	ignoreDeliverResults        bool
+	ignoreCommitResults         bool
+	deterministic               bool
+	interceptDispatchedMessages dispatchInterceptor
 
 	// State
 	logger           log.Logger
@@ -298,12 +299,14 @@ func (f *simFactory) getDispatcherFunc() func() execute.Dispatcher {
 	// Avoid capture
 	services := f.getServices()
 	router := f.getRouter()
+	interceptor := f.interceptDispatchedMessages
 
 	f.dispatcherFunc = func() execute.Dispatcher {
 		d := new(dispatcher)
 		d.client = services
 		d.router = router
 		d.envelopes = map[string][]*messaging.Envelope{}
+		d.interceptor = interceptor
 		return d
 	}
 	return f.dispatcherFunc

test/simulator/options.go

@@ -52,6 +52,15 @@ func DropDispatchedMessages(opts *simFactory) error {
 	return nil
 }
 
+// CaptureDispatchedMessages allows the caller to capture internally dispatched
+// messages.
+func CaptureDispatchedMessages(fn dispatchInterceptor) Option {
+	return func(opts *simFactory) error {
+		opts.interceptDispatchedMessages = fn
+		return nil
+	}
+}
+
 // SkipProposalCheck skips checking if each non-leader node agrees with the
 // leader's proposed block.
 func SkipProposalCheck(opts *simFactory) error {