From 25aee784dfcaefa783cf3fd53e5312254454e66e Mon Sep 17 00:00:00 2001 From: Ed Zavada Date: Tue, 12 Nov 2024 04:22:16 -0500 Subject: [PATCH] refactor: separate validator from rest of server code (#806) Co-authored-by: Zulkhair Abdullah Daim <53172227+zulkhair@users.noreply.github.com> Co-authored-by: Ryan Martin <51780559+rmrt1n@users.noreply.github.com> Co-authored-by: Ryan Martin --- cardinal/option.go | 11 +- cardinal/server/handler/tx.go | 177 +++------ cardinal/server/option.go | 10 +- cardinal/server/server.go | 44 ++- cardinal/server/types/provider.go | 2 + cardinal/server/validator/validator.go | 175 +++++++++ cardinal/server/validator/validator_test.go | 381 ++++++++++++++++++++ cardinal/world_persona.go | 1 + sign/sign.go | 7 + sign/sign_test.go | 28 ++ 10 files changed, 678 insertions(+), 158 deletions(-) create mode 100644 cardinal/server/validator/validator.go create mode 100644 cardinal/server/validator/validator_test.go diff --git a/cardinal/option.go b/cardinal/option.go index 679f796e9..55a0daf3c 100644 --- a/cardinal/option.go +++ b/cardinal/option.go @@ -56,12 +56,21 @@ func WithDisableSignatureVerification() WorldOption { // This setting is ignored if the DisableSignatureVerification option is used // NOTE: this means that the real time clock for the sender and receiver // must be synchronized -func WithMessageExpiration(seconds int) WorldOption { +func WithMessageExpiration(seconds uint) WorldOption { return WorldOption{ serverOption: server.WithMessageExpiration(seconds), } } +// WithHashCacheSize how big the cache of hashes used for replay protection +// is allowed to be. Default is 1MB. +// This setting is ignored if the DisableSignatureVerification option is used +func WithHashCacheSize(sizeKB uint) WorldOption { + return WorldOption{ + serverOption: server.WithHashCacheSize(sizeKB), + } +} + // WithTickChannel sets the channel that will be used to decide when world.doTick is executed. If unset, a loop interval // of 1 second will be set. To set some other time, use: WithTickChannel(time.Tick()). Tests can pass // in a channel controlled by the test for fine-grained control over when ticks are executed. diff --git a/cardinal/server/handler/tx.go b/cardinal/server/handler/tx.go index 86c80bf70..694221bb2 100644 --- a/cardinal/server/handler/tx.go +++ b/cardinal/server/handler/tx.go @@ -1,49 +1,23 @@ package handler import ( - "errors" - "fmt" - "time" - - "github.com/coocood/freecache" - "github.com/ethereum/go-ethereum/common" "github.com/gofiber/fiber/v2" "github.com/gofiber/fiber/v2/log" "github.com/rotisserie/eris" personaMsg "pkg.world.dev/world-engine/cardinal/persona/msg" servertypes "pkg.world.dev/world-engine/cardinal/server/types" + "pkg.world.dev/world-engine/cardinal/server/validator" "pkg.world.dev/world-engine/cardinal/types" "pkg.world.dev/world-engine/sign" ) -const cacheRetentionExtraSeconds = 10 // this is how many seconds past normal expiration a hash is left in the cache. -// we want to ensure it's long enough that any message that's not expired but -// still has its hash in the cache for replay protection. Setting it too long -// would cause the cache to be bigger than necessary - -var ( - ErrNoPersonaTag = errors.New("persona tag is required") - ErrWrongNamespace = errors.New("incorrect namespace") - ErrSystemTransactionRequired = errors.New("system transaction required") - ErrSystemTransactionForbidden = errors.New("system transaction forbidden") -) - // PostTransactionResponse is the HTTP response for a successful transaction submission type PostTransactionResponse struct { TxHash string Tick uint64 } -type SignatureVerification struct { - IsDisabled bool - MessageExpirationSeconds int - HashCacheSizeKB int - Cache *freecache.Cache -} - -type Transaction = sign.Transaction - // PostTransaction godoc // // @Summary Submits a transaction @@ -52,14 +26,14 @@ type Transaction = sign.Transaction // @Produce application/json // @Param txGroup path string true "Message group" // @Param txName path string true "Name of a registered message" -// @Param txBody body Transaction true "Transaction details & message to be submitted" +// @Param txBody body sign.Transaction true "Transaction details & message to be submitted" // @Success 200 {object} PostTransactionResponse "Transaction hash and tick" // @Failure 400 {string} string "Invalid request parameter" // @Failure 403 {string} string "Forbidden" // @Failure 408 {string} string "Request Timeout - message expired" // @Router /tx/{txGroup}/{txName} [post] func PostTransaction( - world servertypes.ProviderWorld, msgs map[string]map[string]types.Message, verify SignatureVerification, + world servertypes.ProviderWorld, msgs map[string]map[string]types.Message, validator *validator.SignatureValidator, ) func(*fiber.Ctx) error { return func(ctx *fiber.Ctx) error { msgType, ok := msgs[ctx.Params("group")][ctx.Params("name")] @@ -69,15 +43,14 @@ func PostTransaction( } // extract the transaction from the fiber context - tx, fiberErr := extractTx(ctx, verify) - if fiberErr != nil { - return fiberErr + tx, err := extractTx(ctx, validator) + if err != nil { + return err } - // Validate the transaction - if err := validateTx(tx); err != nil { - log.Errorf("message %s has invalid transaction payload: %v", tx.Hash.String(), err) - return fiber.NewError(fiber.StatusBadRequest, "Bad Request - invalid payload") + // make sure the transaction hasn't expired + if err = validator.ValidateTransactionTTL(tx); err != nil { + return httpResultFromError(err, false) } // Decode the message from the transaction @@ -87,31 +60,19 @@ func PostTransaction( return fiber.NewError(fiber.StatusBadRequest, "Bad Request - failed to decode tx message") } - // check the signature - if !verify.IsDisabled { - var signerAddress string - if msgType.Name() == personaMsg.CreatePersonaMessageName { - // don't need to check the cast bc we already validated this above - createPersonaMsg, _ := msg.(personaMsg.CreatePersona) - signerAddress = createPersonaMsg.SignerAddress - } - - if err = lookupSignerAndValidateSignature(world, signerAddress, tx); err != nil { - log.Errorf("Signature validation failed for message %s: %v", tx.Hash.String(), err) - return fiber.NewError(fiber.StatusUnauthorized, "Unauthorized - invalid signature") + // there's a special case for the CreatePersona message + var signerAddress string + if msgType.Name() == personaMsg.CreatePersonaMessageName { + createPersonaMsg, ok := msg.(personaMsg.CreatePersona) + if !ok { + return fiber.NewError(fiber.StatusInternalServerError, "Internal Server Error - bad message type") } + signerAddress = createPersonaMsg.SignerAddress + } - // the message was valid, so add its hash to the cache - // we don't do this until we have verified the signature to prevent an attack where someone sends - // large numbers of hashes with unsigned/invalid messages and thus blocks legit messages from - // being handled - err = verify.Cache.Set(tx.Hash.Bytes(), nil, verify.MessageExpirationSeconds+cacheRetentionExtraSeconds) - if err != nil { - // if we couldn't store the hash in the cache, don't process the transaction, since that - // would open us up to replay attacks - log.Errorf("unexpected cache store error %v. message %s ignored", err, tx.Hash.String()) - return fiber.NewError(fiber.StatusInternalServerError, "Internal Server Error - cache store") - } + // Validate the transaction's signature + if err = validator.ValidateTransactionSignature(tx, signerAddress); err != nil { + return httpResultFromError(err, true) } // Add the transaction to the engine @@ -133,16 +94,16 @@ func PostTransaction( // @Accept application/json // @Produce application/json // @Param txName path string true "Name of a registered message" -// @Param txBody body Transaction true "Transaction details & message to be submitted" +// @Param txBody body sign.Transaction true "Transaction details & message to be submitted" // @Success 200 {object} PostTransactionResponse "Transaction hash and tick" // @Failure 400 {string} string "Invalid request parameter" // @Failure 403 {string} string "Forbidden" // @Failure 408 {string} string "Request Timeout - message expired" // @Router /tx/game/{txName} [post] func PostGameTransaction( - world servertypes.ProviderWorld, msgs map[string]map[string]types.Message, verify SignatureVerification, + world servertypes.ProviderWorld, msgs map[string]map[string]types.Message, validator *validator.SignatureValidator, ) func(*fiber.Ctx) error { - return PostTransaction(world, msgs, verify) + return PostTransaction(world, msgs, validator) } // NOTE: duplication for cleaner swagger docs @@ -152,7 +113,7 @@ func PostGameTransaction( // @Description Creates a persona // @Accept application/json // @Produce application/json -// @Param txBody body Transaction true "Transaction details & message to be submitted" +// @Param txBody body sign.Transaction true "Transaction details & message to be submitted" // @Success 200 {object} PostTransactionResponse "Transaction hash and tick" // @Failure 400 {string} string "Invalid request parameter" // @Failure 401 {string} string "Unauthorized - signature was invalid" @@ -161,31 +122,17 @@ func PostGameTransaction( // @Failure 500 {string} string "Internal Server Error - unexpected cache errors" // @Router /tx/persona/create-persona [post] func PostPersonaTransaction( - world servertypes.ProviderWorld, msgs map[string]map[string]types.Message, verify SignatureVerification, + world servertypes.ProviderWorld, msgs map[string]map[string]types.Message, validator *validator.SignatureValidator, ) func(*fiber.Ctx) error { - return PostTransaction(world, msgs, verify) -} - -func isHashInCache(hash common.Hash, cache *freecache.Cache) (bool, error) { - _, err := cache.Get(hash.Bytes()) - if err == nil { - // found it - return true, nil - } - if errors.Is(err, freecache.ErrNotFound) { - // ignore ErrNotFound, just return false - return false, nil - } - // return all other errors - return false, err + return PostTransaction(world, msgs, validator) } -func extractTx(ctx *fiber.Ctx, verify SignatureVerification) (*sign.Transaction, *fiber.Error) { +func extractTx(ctx *fiber.Ctx, validator *validator.SignatureValidator) (*sign.Transaction, error) { var tx *sign.Transaction var err error // Parse the request body into a sign.Transaction struct tx := new(Transaction) // this also calculates the hash - if !verify.IsDisabled { + if validator != nil && !validator.IsDisabled { // we are doing signature verification, so use sign's Unmarshal which does extra checks tx, err = sign.UnmarshalTransaction(ctx.Body()) } else { @@ -195,65 +142,31 @@ func extractTx(ctx *fiber.Ctx, verify SignatureVerification) (*sign.Transaction, } if err != nil { log.Errorf("body parse failed: %v", err) - return nil, fiber.NewError(fiber.StatusBadRequest, "Bad Request - unparseable body") - } - if !verify.IsDisabled { - txEarliestValidTimestamp := sign.TimestampAt( - time.Now().Add(-(time.Duration(verify.MessageExpirationSeconds) * time.Second))) - // before we even create the hash or validate the signature, check to see if the message has expired - if tx.Timestamp < txEarliestValidTimestamp { - log.Errorf("message older than %d seconds. Got timestamp: %d, current timestamp: %d ", - verify.MessageExpirationSeconds, tx.Timestamp, sign.TimestampNow()) - return nil, fiber.NewError(fiber.StatusRequestTimeout, "Request Timeout - signature too old") - } - // check for duplicate message via hash cache - if found, err := isHashInCache(tx.Hash, verify.Cache); err != nil { - log.Errorf("unexpected cache error %v. message %s ignored", err, tx.Hash.String()) - return nil, fiber.NewError(fiber.StatusInternalServerError, "Internal Server Error - cache failed") - } else if found { - // if found in the cache, the message hash has already been used, so reject it - log.Errorf("message %s already handled", tx.Hash.String()) - return nil, fiber.NewError(fiber.StatusForbidden, "Forbidden - duplicate message") - } - // at this point we know that the generated hash is not in the cache, so this message is not a replay + return nil, eris.Wrap(err, "Bad Request - unparseable body") } return tx, nil } -func lookupSignerAndValidateSignature(world servertypes.ProviderWorld, signerAddress string, tx *Transaction) error { - var err error - if signerAddress == "" { - signerAddress, err = world.GetSignerForPersonaTag(tx.PersonaTag, 0) - if err != nil { - return fmt.Errorf("could not get signer for persona %s: %w", tx.PersonaTag, err) - } +// turns the various errors into an appropriate HTTP result +func httpResultFromError(err error, isSignatureValidation bool) error { + log.Error(err) // log the private internal details + if eris.Is(err, validator.ErrDuplicateMessage) { + return fiber.NewError(fiber.StatusForbidden, "Forbidden - duplicate message") } - if err = validateSignature(tx, signerAddress, world.Namespace(), - tx.IsSystemTransaction()); err != nil { - return fmt.Errorf("could not validate signature for persona %s: %w", tx.PersonaTag, err) + if eris.Is(err, validator.ErrMessageExpired) { + return fiber.NewError(fiber.StatusRequestTimeout, "Request Timeout - message expired") } - return nil -} - -// validateTx validates the transaction payload -func validateTx(tx *Transaction) error { - // TODO(scott): we should use the validator package here - if tx.PersonaTag == "" { - return ErrNoPersonaTag + if eris.Is(err, validator.ErrBadTimestamp) { + return fiber.NewError(fiber.StatusBadRequest, "Bad Request - bad timestamp") } - return nil -} - -// validateSignature validates that the signature of transaction is valid -func validateSignature(tx *Transaction, signerAddr string, namespace string, systemTx bool) error { - if tx.Namespace != namespace { - return eris.Wrap(ErrWrongNamespace, fmt.Sprintf("expected %q got %q", namespace, tx.Namespace)) + if eris.Is(err, validator.ErrNoPersonaTag) { + return fiber.NewError(fiber.StatusBadRequest, "Bad Request - no persona tag") } - if systemTx && !tx.IsSystemTransaction() { - return eris.Wrap(ErrSystemTransactionRequired, "") + if eris.Is(err, validator.ErrInvalidSignature) { + return fiber.NewError(fiber.StatusUnauthorized, "Unauthorized - signature validation failed") } - if !systemTx && tx.IsSystemTransaction() { - return eris.Wrap(ErrSystemTransactionForbidden, "") + if isSignatureValidation { + return fiber.NewError(fiber.StatusInternalServerError, "Internal Server Error - signature validation failed") } - return eris.Wrap(tx.Verify(signerAddr), "") + return fiber.NewError(fiber.StatusInternalServerError, "Internal Server Error - ttl validation failed") } diff --git a/cardinal/server/option.go b/cardinal/server/option.go index a65d8c7bc..5677c41a5 100644 --- a/cardinal/server/option.go +++ b/cardinal/server/option.go @@ -19,7 +19,7 @@ func DisableSwagger() Option { // DisableSignatureVerification disables signature verification. func DisableSignatureVerification() Option { return func(s *Server) { - s.verify.IsDisabled = true + s.config.isSignatureValidationDisabled = true } } @@ -31,17 +31,17 @@ func DisableSignatureVerification() Option { // This setting is ignored if the DisableSignatureVerification option is used // NOTE: this means that the real time clock for the sender and receiver // must be synchronized -func WithMessageExpiration(seconds int) Option { +func WithMessageExpiration(seconds uint) Option { return func(s *Server) { - s.verify.MessageExpirationSeconds = seconds + s.config.messageExpirationSeconds = seconds } } // WithHashCacheSize how big the cache of hashes used for replay protection // is allowed to be. Default is 1MB. // This setting is ignored if the DisableSignatureVerification option is used -func WithHashCacheSize(sizeKB int) Option { +func WithHashCacheSize(sizeKB uint) Option { return func(s *Server) { - s.verify.HashCacheSizeKB = sizeKB + s.config.messageHashCacheSizeKB = sizeKB } } diff --git a/cardinal/server/server.go b/cardinal/server/server.go index d249fe1b0..ebe56b8d4 100644 --- a/cardinal/server/server.go +++ b/cardinal/server/server.go @@ -5,7 +5,6 @@ import ( "encoding/json" "time" - "github.com/coocood/freecache" "github.com/gofiber/contrib/socketio" "github.com/gofiber/fiber/v2" "github.com/gofiber/fiber/v2/middleware/cors" @@ -15,6 +14,7 @@ import ( "pkg.world.dev/world-engine/cardinal/server/handler" servertypes "pkg.world.dev/world-engine/cardinal/server/types" + "pkg.world.dev/world-engine/cardinal/server/validator" "pkg.world.dev/world-engine/cardinal/types" _ "pkg.world.dev/world-engine/cardinal/server/docs" // for swagger. @@ -24,18 +24,21 @@ const ( defaultPort = "4040" shutdownTimeout = 5 * time.Second defaultMessageExpiration = 10 // seconds - defaultHashCacheSizeKB = 1024 // default to 1MB hash Cache + defaultHashCacheSizeKB = 1024 // default to 1MB hash cache ) type config struct { - port string - isSwaggerDisabled bool + port string + isSwaggerDisabled bool + isSignatureValidationDisabled bool + messageExpirationSeconds uint + messageHashCacheSizeKB uint } type Server struct { - app *fiber.App - config config - verify handler.SignatureVerification + app *fiber.App + config config + validator *validator.SignatureValidator } // New returns an HTTP server with handlers for all QueryTypes and MessageTypes. @@ -53,24 +56,25 @@ func New( s := &Server{ app: app, config: config{ - port: defaultPort, - isSwaggerDisabled: false, - }, - verify: handler.SignatureVerification{ - IsDisabled: false, - MessageExpirationSeconds: defaultMessageExpiration, - HashCacheSizeKB: defaultHashCacheSizeKB, - Cache: nil, + port: defaultPort, + isSwaggerDisabled: false, + isSignatureValidationDisabled: false, + messageExpirationSeconds: defaultMessageExpiration, + messageHashCacheSizeKB: defaultHashCacheSizeKB, }, } for _, opt := range opts { opt(s) } - // Setup Cache for hashes to prevent replay attacks - if !s.verify.IsDisabled { - s.verify.Cache = freecache.NewCache(s.verify.HashCacheSizeKB) - } + // now that all the options are set, use them to create the Signature validator + s.validator = validator.NewSignatureValidator( + s.config.isSignatureValidationDisabled, + s.config.messageExpirationSeconds, + s.config.messageHashCacheSizeKB, + world.Namespace(), + world, // world is a provider of signature addresses + ) // Enable CORS app.Use(cors.New()) @@ -180,7 +184,7 @@ func (s *Server) setupRoutes( // Route: /tx/... tx := s.app.Group("/tx") - tx.Post("/:group/:name", handler.PostTransaction(world, msgIndex, s.verify)) + tx.Post("/:group/:name", handler.PostTransaction(world, msgIndex, s.validator)) // Route: /cql s.app.Post("/cql", handler.PostCQL(world)) diff --git a/cardinal/server/types/provider.go b/cardinal/server/types/provider.go index b76cd2edd..616875a4a 100644 --- a/cardinal/server/types/provider.go +++ b/cardinal/server/types/provider.go @@ -3,11 +3,13 @@ package types import ( "pkg.world.dev/world-engine/cardinal/gamestate" "pkg.world.dev/world-engine/cardinal/receipt" + "pkg.world.dev/world-engine/cardinal/server/validator" "pkg.world.dev/world-engine/cardinal/types" "pkg.world.dev/world-engine/sign" ) type ProviderWorld interface { + validator.SignerAddressProvider UseNonce(signerAddress string, nonce uint64) error GetSignerForPersonaTag(personaTag string, tick uint64) (addr string, err error) AddTransaction(id types.MessageID, v any, sig *sign.Transaction) (uint64, types.TxHash) diff --git a/cardinal/server/validator/validator.go b/cardinal/server/validator/validator.go new file mode 100644 index 000000000..c7b03dc0a --- /dev/null +++ b/cardinal/server/validator/validator.go @@ -0,0 +1,175 @@ +package validator + +import ( + "errors" + "fmt" + "time" + + "github.com/coocood/freecache" + "github.com/ethereum/go-ethereum/common" // for hash + "github.com/rotisserie/eris" + + "pkg.world.dev/world-engine/sign" +) + +// we define the particular interface we need here to avoid dragging in the whole world provider interface +// and making independent testing of validator require more complicated interfaces +type SignerAddressProvider interface { + // tick is used by world provider, but not by the validator package. we include it here + // to avoid creating an extra method for a very minor bit of abstraction + GetSignerForPersonaTag(personaTag string, tick uint64) (addr string, err error) +} + +const cacheRetentionExtraSeconds = 10 // this is how many seconds past normal expiration a hash is left in the cache. +// we want to ensure it's long enough that any message that's not expired but +// still has its hash in the cache for replay protection. Setting it too long +// would cause the cache to be bigger than necessary + +const ttlMaxFutureSeconds = 2 // this is how many seconds in the future a message is allowed to be timestamped +// this allows for some degree of clock drift. It's safe enough to accept a message that's stamped from the near +// future because we will still keep it in the hash cache and prevent it from being a replay attack vector. However, +// we don't want to take messages from an unlimited amount of time into the future since they could cause our hash +// cache to overflow + +const bytesPerKb = 1024 + +var ( + ErrNoPersonaTag = eris.New("persona tag is required") + ErrWrongNamespace = eris.New("incorrect namespace") + ErrMessageExpired = eris.New("signature too old") + ErrBadTimestamp = eris.New("invalid future timestamp") + ErrCacheReadFailed = eris.New("cache read failed") + ErrCacheWriteFailed = eris.New("cache store failed") + ErrDuplicateMessage = eris.New("duplicate message") + ErrInvalidSignature = eris.New("invalid signature") +) + +type SignatureValidator struct { + IsDisabled bool + MessageExpirationSeconds uint + HashCacheSizeKB uint + namespace string + cache *freecache.Cache + signerAddressProvider SignerAddressProvider +} + +func NewSignatureValidator(disabled bool, msgExpirationSec uint, hashCacheSizeKB uint, namespace string, + provider SignerAddressProvider, +) *SignatureValidator { + validator := SignatureValidator{ + IsDisabled: disabled, + MessageExpirationSeconds: msgExpirationSec, + HashCacheSizeKB: hashCacheSizeKB, + namespace: namespace, + cache: nil, + signerAddressProvider: provider, + } + if !disabled { + // freecache enforces its own minimum size of 512K + validator.cache = freecache.NewCache(int(validator.HashCacheSizeKB * bytesPerKb)) + } + return &validator +} + +// ValidateTransactionTTL checks that the timestamp on the message is valid, the message has not expired, +// and that the message is not previously handled as indicated by it being in the hash cache. +// returns an error (ErrMessageExpired, ErrBadTimestamp, ErrDuplicateMessage, or ErrCacheReadFailed) if +// there was a problem, and nil if everything was ok +// if signature validation is disabled, no checks are done and nil is always returned +func (validator *SignatureValidator) ValidateTransactionTTL(tx *sign.Transaction) error { + if !validator.IsDisabled { + now := time.Now() + txEarliestValidTimestamp := sign.TimestampAt( + now.Add(-(time.Duration(validator.MessageExpirationSeconds) * time.Second))) + txLatestValidTimestamp := sign.TimestampAt(now.Add(time.Duration(ttlMaxFutureSeconds) * time.Second)) + // before we even create the hash or validate the signature, check to see if the message has expired + if tx.Timestamp < txEarliestValidTimestamp { + return eris.Wrap(ErrMessageExpired, + fmt.Sprintf("message older than %d seconds. Got timestamp: %d, current timestamp: %d ", + validator.MessageExpirationSeconds, tx.Timestamp, sign.TimestampAt(now))) + } else if tx.Timestamp > txLatestValidTimestamp { + return eris.Wrap(ErrBadTimestamp, + fmt.Sprintf( + "message timestamp more than %d seconds in the future. Got timestamp: %d, current timestamp: %d ", + ttlMaxFutureSeconds, tx.Timestamp, sign.TimestampAt(now))) + } + // check for duplicate message via hash cache + if found, err := validator.isHashInCache(tx.Hash); err != nil { + return eris.Wrap(ErrCacheReadFailed, + fmt.Sprintf("unexpected cache error %v. message %s ignored", err, tx.Hash.String())) + } else if found { + // if found in the cache, the message hash has already been used, so reject it + return eris.Wrap(ErrDuplicateMessage, + fmt.Sprintf("message %s already handled", tx.Hash.String())) + } + } + return nil +} + +// ValidateTransactionSignature checks that the signature is valid, was signed by the persona (or signer passed in), +// has the correct namespace, and has not been altered. If all checks pass, it is added to the hash cache as a +// known message, and nil is returned. Other possible returns are ErrNoPersonaTag, ErrInvalidSignature, and +// ErrCacheWriteFailed. If signature validation is disabled, we only check for the presence of a persona tag. +func (validator *SignatureValidator) ValidateTransactionSignature(tx *sign.Transaction, signerAddress string, +) error { + // this is the only validation we do when signature validation is disabled + if tx.PersonaTag == "" { + return eris.Wrap(ErrNoPersonaTag, + fmt.Sprintf("missing persona tag for message %s", tx.Hash.String())) + } + if validator.IsDisabled { + return nil + } + + // if they didn't give us a signer address, we will have to look it up with the provider + var err error + if signerAddress == "" { + signerAddress, err = validator.signerAddressProvider.GetSignerForPersonaTag(tx.PersonaTag, 0) + if err != nil { + return eris.Wrap(ErrInvalidSignature, + fmt.Sprintf("could not get signer for persona %s: %v", tx.PersonaTag, err)) + } + } + + // check the signature against the address + if err = validator.validateSignature(tx, signerAddress); err != nil { + return eris.Wrap(ErrInvalidSignature, + fmt.Sprintf("signature validation failed for message %s: %v", tx.Hash.String(), err)) + } + + // the message was valid, so add its hash to the cache + // we don't do this until we have verified the signature to prevent an attack where someone sends + // large numbers of hashes with unsigned/invalid messages and thus blocks legit messages from + // being handled + err = validator.cache.Set(tx.Hash.Bytes(), nil, + int(validator.MessageExpirationSeconds+cacheRetentionExtraSeconds)) + if err != nil { + // if we couldn't store the hash in the cache, don't process the transaction, since that + // would open us up to replay attacks + return eris.Wrap(ErrCacheWriteFailed, + fmt.Sprintf("unexpected cache store error %v. message %s ignored", err, tx.Hash.String())) + } + return nil +} + +func (validator *SignatureValidator) isHashInCache(hash common.Hash) (bool, error) { + _, err := validator.cache.Get(hash.Bytes()) + if err == nil { + // found it + return true, nil + } + if errors.Is(err, freecache.ErrNotFound) { + // ignore ErrNotFound, just return false + return false, nil + } + // return all other errors + return false, err +} + +// validateSignature validates that the signature of transaction is valid +func (validator *SignatureValidator) validateSignature(tx *sign.Transaction, signerAddr string) error { + if tx.Namespace != validator.namespace { + return eris.Wrap(ErrWrongNamespace, fmt.Sprintf("expected %q got %q", validator.namespace, tx.Namespace)) + } + return tx.Verify(signerAddr) +} diff --git a/cardinal/server/validator/validator_test.go b/cardinal/server/validator/validator_test.go new file mode 100644 index 000000000..3ac52f1f1 --- /dev/null +++ b/cardinal/server/validator/validator_test.go @@ -0,0 +1,381 @@ +package validator + +import ( + "crypto/ecdsa" + "fmt" + "testing" + "time" + + "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/crypto" + "github.com/rotisserie/eris" + "github.com/stretchr/testify/suite" + + "pkg.world.dev/world-engine/cardinal/persona" + "pkg.world.dev/world-engine/sign" +) + +const goodRequestBody = `{"msg": "this is a request body"}` +const hackedRequestBody = `{"give": "much_gold", "to": "me"}` +const badRequestBody = `{{"junk"{{` +const emptyRequestBody = "" + +const goodPersona = "good_persona" +const badPersona = "bad_persona" + +const goodNamespace = "good_namespace" +const badNamespace = "bad_namespace" + +const badSignature = "bad_signature" +const badSignerAddress = "bad_signer_address" +const lookupSignerAddress = "" + +var emptyHash = common.Hash{} +var badHash = crypto.Keccak256Hash([]byte("complete_garbage")) + +var veryOldTimestamp = sign.TimestampAt(time.Now().Add(time.Hour * -1000)) +var futureTimestamp = sign.TimestampAt(time.Now().Add(time.Hour * 1000)) + +type ValidatorTestSuite struct { + suite.Suite + + privateKey *ecdsa.PrivateKey + signerAddr string + namespace string + provider SignerAddressProvider +} + +type ProviderFixture struct { + vts *ValidatorTestSuite +} + +func (pf *ProviderFixture) GetSignerForPersonaTag(personaTag string, _ uint64) (addr string, err error) { + if personaTag == badPersona { + // emulates what would happen if the personal lookup provider couldn't find a signer for the persona + return "", persona.ErrPersonaTagHasNoSigner + } + return pf.vts.signerAddr, nil +} + +func TestServerValidator(t *testing.T) { + suite.Run(t, new(ValidatorTestSuite)) +} + +// SetupTest runs before each test in the suite. +func (s *ValidatorTestSuite) SetupTest() { + var err error + s.privateKey, err = crypto.GenerateKey() + s.Require().NoError(err) + s.signerAddr = crypto.PubkeyToAddress(s.privateKey.PublicKey).Hex() + s.namespace = goodNamespace + s.provider = &ProviderFixture{ + vts: s, + } +} + +func (s *ValidatorTestSuite) createDisabledValidator() *SignatureValidator { + return NewSignatureValidator(true, 0, 0, s.namespace, s.provider) +} + +// create an enabled validator with a specific ttl +func (s *ValidatorTestSuite) createValidatorWithTTL(ttl uint) *SignatureValidator { //nolint: unparam // future use + return NewSignatureValidator(false, ttl, 200, s.namespace, s.provider) +} + +func (s *ValidatorTestSuite) simulateReceivedTransaction(personaTag, namespace string, + data any, //nolint: unparam // future use +) (*sign.Transaction, error) { + tx, err := sign.NewTransaction(s.privateKey, personaTag, namespace, data) + if err == nil { + // sign puts a hash value into the transaction, but a newly received transaction will not have a hash value + // because the Unmarshal function used to on the incoming message does not copy it + tx.Hash = emptyHash + } + return tx, err +} + +// TestCanValidateSignedTxWithVerificationDisabled tests that you can validate a full signed tx when +// sig verification is disabled (no actual validation is done) +func (s *ValidatorTestSuite) TestCanValidateSignedTxWithVerificationDisabled() { + validator := s.createDisabledValidator() + tx, err := s.simulateReceivedTransaction(goodPersona, goodNamespace, goodRequestBody) + s.Require().NoError(err) + err = validator.ValidateTransactionTTL(tx) + s.Require().NoError(err) + err = validator.ValidateTransactionSignature(tx, lookupSignerAddress) + s.Require().NoError(err) +} + +// TestCanValidateUnsignedTxWithVerificationDisabled tests that you can validate a tx with just a persona and body when +// sig verification is disabled. +func (s *ValidatorTestSuite) TestCanValidateUnsignedTxWithVerificationDisabled() { + validator := s.createDisabledValidator() + tx := &sign.Transaction{PersonaTag: goodPersona, Body: []byte(goodRequestBody)} + err := validator.ValidateTransactionTTL(tx) + s.Require().NoError(err) + err = validator.ValidateTransactionSignature(tx, lookupSignerAddress) + s.Require().NoError(err) +} + +// TestCanValidateBadSignatureTxWithVerificationDisabled tests that you can validate a tx with an invalid signature +// when sig verification is disabled. +func (s *ValidatorTestSuite) TestCanValidateBadSignatureTxWithVerificationDisabled() { + validator := s.createDisabledValidator() + tx := &sign.Transaction{PersonaTag: goodPersona, Signature: badSignature, Body: []byte(goodRequestBody)} + err := validator.ValidateTransactionTTL(tx) + s.Require().NoError(err) + err = validator.ValidateTransactionSignature(tx, badSignerAddress) + s.Require().NoError(err) +} + +// TestCanValidateBadNamespaceTxWithVerificationDisabled tests that you can validate a tx with the wrong namespace +// when sig verification is disabled. +func (s *ValidatorTestSuite) TestCanValidateBadNamespaceTxWithVerificationDisabled() { + validator := s.createDisabledValidator() + tx := &sign.Transaction{PersonaTag: goodPersona, Namespace: badNamespace, Body: []byte(goodRequestBody)} + err := validator.ValidateTransactionTTL(tx) + s.Require().NoError(err) + err = validator.ValidateTransactionSignature(tx, lookupSignerAddress) + s.Require().NoError(err) +} + +// TestCanValidateBadTimestampsTxWithVerificationDisabled tests that you can validate transactions with expired or +// future timestamps when sig verification is disabled. +func (s *ValidatorTestSuite) TestCanValidateBadTimestampsTxWithVerificationDisabled() { + validator := s.createDisabledValidator() + tx := &sign.Transaction{PersonaTag: goodPersona, Timestamp: veryOldTimestamp, Body: []byte(goodRequestBody)} + err := validator.ValidateTransactionTTL(tx) + s.Require().NoError(err) + err = validator.ValidateTransactionSignature(tx, badSignerAddress) + s.Require().NoError(err) + + tx = &sign.Transaction{PersonaTag: goodPersona, Timestamp: futureTimestamp, Body: []byte(goodRequestBody)} + err = validator.ValidateTransactionTTL(tx) + s.Require().NoError(err) + err = validator.ValidateTransactionSignature(tx, badSignerAddress) + s.Require().NoError(err) +} + +// TestIgnoresTxHashWithVerificationDisabled tests that you can validate a tx with a bogus hash value +// sig verification is disabled. +func (s *ValidatorTestSuite) TestIgnoresTxHashWithVerificationDisabled() { + validator := s.createDisabledValidator() + tx := &sign.Transaction{PersonaTag: goodPersona, Hash: emptyHash, Body: []byte(goodRequestBody)} + err := validator.ValidateTransactionTTL(tx) + s.Require().NoError(err) + err = validator.ValidateTransactionSignature(tx, badSignerAddress) + s.Require().NoError(err) + + tx = &sign.Transaction{PersonaTag: goodPersona, Hash: badHash, Body: []byte(goodRequestBody)} + err = validator.ValidateTransactionTTL(tx) + s.Require().NoError(err) + err = validator.ValidateTransactionSignature(tx, badSignerAddress) + s.Require().NoError(err) +} + +// TestValidationIgnoresBody tests that you can validate a tx without a valid body when +// sig verification is disabled. +func (s *ValidatorTestSuite) TestValidationIgnoresBodyWithVerificationDisabled() { + validator := s.createDisabledValidator() + tx := &sign.Transaction{PersonaTag: goodPersona, Body: []byte(badRequestBody)} + err := validator.ValidateTransactionTTL(tx) + s.Require().NoError(err) + err = validator.ValidateTransactionSignature(tx, badSignerAddress) + s.Require().NoError(err) + + tx = &sign.Transaction{PersonaTag: goodPersona, Body: []byte(emptyRequestBody)} + err = validator.ValidateTransactionTTL(tx) + s.Require().NoError(err) + err = validator.ValidateTransactionSignature(tx, badSignerAddress) + s.Require().NoError(err) + + tx = &sign.Transaction{PersonaTag: goodPersona, Body: nil} + err = validator.ValidateTransactionTTL(tx) + s.Require().NoError(err) + err = validator.ValidateTransactionSignature(tx, badSignerAddress) + s.Require().NoError(err) +} + +// TestCanValidateSignedTx tests that you can validate a full signed tx when +// sig verification is enabled. +func (s *ValidatorTestSuite) TestCanValidateSignedTx() { + validator := s.createValidatorWithTTL(10) + tx, err := s.simulateReceivedTransaction(goodPersona, goodNamespace, goodRequestBody) + s.Require().NoError(err) + err = validator.ValidateTransactionTTL(tx) + s.Require().NoError(err) + err = validator.ValidateTransactionSignature(tx, lookupSignerAddress) + s.Require().NoError(err) +} + +// TestRejectsMissingPersonaTx tests that transaction without a persona tag is always rejected, regardless +// of whether signature validation is enabled or not +func (s *ValidatorTestSuite) TestAlwaysRejectsMissingPersonaTx() { + validator := s.createValidatorWithTTL(10) + tx, e := s.simulateReceivedTransaction(goodPersona, goodNamespace, goodRequestBody) + s.Require().NoError(e) + + tx.PersonaTag = "" + err := validator.ValidateTransactionTTL(tx) + s.Require().NoError(err) + err = validator.ValidateTransactionSignature(tx, lookupSignerAddress) + s.Require().Error(err) + s.Require().True(eris.Is(err, ErrNoPersonaTag)) + + validator = s.createDisabledValidator() + tx = &sign.Transaction{PersonaTag: "", Body: []byte(goodRequestBody)} + + err = validator.ValidateTransactionTTL(tx) + s.Require().NoError(err) + err = validator.ValidateTransactionSignature(tx, lookupSignerAddress) + s.Require().Error(err) + s.Require().True(eris.Is(err, ErrNoPersonaTag)) +} + +// TestRejectsUnsignedTx tests that an unsigned transaction with a valid timestamp is rejected. +func (s *ValidatorTestSuite) TestRejectsUnsignedTx() { + validator := s.createValidatorWithTTL(10) + tx := &sign.Transaction{PersonaTag: goodPersona, Timestamp: sign.TimestampNow(), Body: []byte(goodRequestBody)} + err := validator.ValidateTransactionTTL(tx) + s.Require().NoError(err) + err = validator.ValidateTransactionSignature(tx, lookupSignerAddress) + s.Require().Error(err) + s.Require().True(eris.Is(err, ErrInvalidSignature)) +} + +// TestRejectsBadNamespaceTx tests that a signed transaction with the wrong namespace is rejected +func (s *ValidatorTestSuite) TestRejectsBadNamespaceTx() { + validator := s.createValidatorWithTTL(10) + tx, e := s.simulateReceivedTransaction(goodPersona, badNamespace, goodRequestBody) + s.Require().NoError(e) + err := validator.ValidateTransactionTTL(tx) + s.Require().NoError(err) + err = validator.ValidateTransactionSignature(tx, lookupSignerAddress) + s.Require().Error(err) + s.Require().True(eris.Is(err, ErrInvalidSignature)) +} + +// TestRejectsInvalidTimestampsTx tests that transactions with invalid timestamps or with a timestamp altered +// after signing are rejected +func (s *ValidatorTestSuite) TestRejectsInvalidTimestampsTx() { + ttl := uint(10) + validator := s.createValidatorWithTTL(ttl) + tx, e := s.simulateReceivedTransaction(goodPersona, goodNamespace, goodRequestBody) + s.Require().NoError(e) + s.Require().True(sign.IsZeroHash(tx.Hash)) + + saved := tx.Timestamp + + tx.Timestamp = veryOldTimestamp + err := validator.ValidateTransactionTTL(tx) + s.Require().Error(err) + s.Require().True(eris.Is(err, ErrMessageExpired)) + s.Require().Contains(err.Error(), fmt.Sprintf("message older than %d seconds", ttl)) + + tx.Timestamp = futureTimestamp + err = validator.ValidateTransactionTTL(tx) + s.Require().Error(err) + s.Require().True(eris.Is(err, ErrBadTimestamp)) + s.Require().Contains(err.Error(), fmt.Sprintf("message timestamp more than %d seconds in the future", + ttlMaxFutureSeconds)) + + tx.Timestamp = saved - 1 + err = validator.ValidateTransactionTTL(tx) + s.Require().NoError(err) + + // this step actually calculates the hash + err = validator.ValidateTransactionSignature(tx, lookupSignerAddress) + s.Require().Error(err) + s.Require().True(eris.Is(err, ErrInvalidSignature)) + s.Require().Contains(err.Error(), "signature validation failed for message") +} + +// TestRejectsAlteredHashTx tests that a transaction with a hashes that is altered after signing is rejected +func (s *ValidatorTestSuite) TestRejectsAlteredHashTx() { + validator := s.createValidatorWithTTL(10) + tx, e := s.simulateReceivedTransaction(goodPersona, goodNamespace, goodRequestBody) + s.Require().NoError(e) + s.Require().True(sign.IsZeroHash(tx.Hash)) + + tx.Hash = badHash // alter the hash + + err := validator.ValidateTransactionTTL(tx) + s.Require().NoError(err) + + // this step normally calculates the hash, but since it's altered it will use the altered one + err = validator.ValidateTransactionSignature(tx, lookupSignerAddress) + s.Require().Error(err) + s.Require().True(eris.Is(err, ErrInvalidSignature)) + s.Require().Contains(err.Error(), "signature validation failed for message") +} + +// TestRejectsAlteredSaltTx tests that a transaction with a salt value that is altered after signing is rejected +func (s *ValidatorTestSuite) TestRejectsAlteredSaltTx() { + validator := s.createValidatorWithTTL(10) + tx, e := s.simulateReceivedTransaction(goodPersona, goodNamespace, goodRequestBody) + s.Require().NoError(e) + s.Require().True(sign.IsZeroHash(tx.Hash)) + + tx.Salt++ // alter the salt value + + err := validator.ValidateTransactionTTL(tx) + s.Require().NoError(err) + + // this step normally calculates the hash, but since it's altered it will use the altered one + err = validator.ValidateTransactionSignature(tx, lookupSignerAddress) + s.Require().Error(err) + s.Require().True(eris.Is(err, ErrInvalidSignature)) + s.Require().Contains(err.Error(), "signature validation failed for message") +} + +// TestRejectsAlteredBodyTx tests that a transaction with a body that is altered after signing is rejected +func (s *ValidatorTestSuite) TestRejectsAlteredBodyTx() { + validator := s.createValidatorWithTTL(10) + tx, e := s.simulateReceivedTransaction(goodPersona, goodNamespace, goodRequestBody) + s.Require().NoError(e) + s.Require().True(sign.IsZeroHash(tx.Hash)) + + tx.Body = []byte(hackedRequestBody) // replace the body with another valid one + + err := validator.ValidateTransactionTTL(tx) + s.Require().NoError(err) + + // this step normally calculates the hash, but since it's altered it will use the altered one + err = validator.ValidateTransactionSignature(tx, lookupSignerAddress) + s.Require().Error(err) + s.Require().True(eris.Is(err, ErrInvalidSignature)) + s.Require().Contains(err.Error(), "signature validation failed for message") +} + +// TestRejectsInvalidPersonaTx tests that a transaction with an invalid signature is rejected. +func (s *ValidatorTestSuite) TestRejectsInvalidPersonaTx() { + validator := s.createValidatorWithTTL(10) + tx, e := s.simulateReceivedTransaction(badPersona, goodNamespace, goodRequestBody) + s.Require().NoError(e) + + err := validator.ValidateTransactionTTL(tx) + s.Require().NoError(err) + err = validator.ValidateTransactionSignature(tx, lookupSignerAddress) + s.Require().Error(err) + s.Require().True(eris.Is(err, ErrInvalidSignature)) + s.Require().Contains(err.Error(), "could not get signer for persona bad_persona") +} + +// TestRejectsDuplicateTx tests that a transaction that's previously been validated is rejected if you try to validate +// it again. This prevents replay attacks because each message sent must be uniquely signed. +func (s *ValidatorTestSuite) TestRejectsDuplicateTx() { + validator := s.createValidatorWithTTL(10) + tx, e := s.simulateReceivedTransaction(goodPersona, goodNamespace, goodRequestBody) + s.Require().NoError(e) + + err := validator.ValidateTransactionTTL(tx) + s.Require().NoError(err) + err = validator.ValidateTransactionSignature(tx, lookupSignerAddress) + s.Require().NoError(err) + + // try to validate same transaction again, detect an error + err = validator.ValidateTransactionTTL(tx) + s.Require().Error(err) + s.Require().True(eris.Is(err, ErrDuplicateMessage)) + s.Require().Contains(err.Error(), fmt.Sprintf("message %s already handled", tx.Hash)) +} diff --git a/cardinal/world_persona.go b/cardinal/world_persona.go index 477c53a57..f39a84ff6 100644 --- a/cardinal/world_persona.go +++ b/cardinal/world_persona.go @@ -14,6 +14,7 @@ import ( // GetSignerForPersonaTag returns the signer address that has been registered for the given persona tag after the // given tick. If the engine's tick is less than or equal to the given tick, ErrorCreatePersonaTXsNotProcessed is // returned. If the given personaTag has no signer address, ErrPersonaTagHasNoSigner is returned. +// implements the validator.SignerAddressProvider interface func (w *World) GetSignerForPersonaTag(personaTag string, tick uint64) (addr string, err error) { if tick >= w.CurrentTick() { return "", persona.ErrCreatePersonaTxsNotProcessed diff --git a/sign/sign.go b/sign/sign.go index 4eed887c3..72fcb71e7 100644 --- a/sign/sign.go +++ b/sign/sign.go @@ -47,14 +47,21 @@ type Transaction struct { Body json.RawMessage `json:"body" swaggertype:"object"` // json string } +// returns a sign compatible timestamp for the current wall time func TimestampNow() int64 { return time.Now().UnixMilli() // millisecond accuracy on timestamps, easily available on all platforms } +// returns a sign compatible timestamp for the time passed in func TimestampAt(t time.Time) int64 { return t.UnixMilli() } +// returns a GoLang time from a sign compatible timestamp +func Timestamp(t int64) time.Time { + return time.UnixMilli(t) +} + func UnmarshalTransaction(bz []byte) (*Transaction, error) { s := new(Transaction) dec := json.NewDecoder(bytes.NewBuffer(bz)) diff --git a/sign/sign_test.go b/sign/sign_test.go index 58504b9e2..f29ea19b5 100644 --- a/sign/sign_test.go +++ b/sign/sign_test.go @@ -102,6 +102,34 @@ func TestIsSignedSystemPayload(t *testing.T) { assert.Check(t, sp.IsSystemTransaction()) } +func TestTimestamps(t *testing.T) { + goodKey, err := crypto.GenerateKey() + assert.NilError(t, err) + body := `{"msg": "this is a request body"}` + personaTag := "my-tag" + namespace := "my-namespace" + + ts := TimestampNow() + tx, err := NewTransaction(goodKey, personaTag, namespace, body) + assert.NilError(t, err) + assert.Check(t, tx.Timestamp-ts <= 1) + + time.Sleep(100 * time.Millisecond) + tm := time.Now() + ts2 := TimestampNow() + ts3 := TimestampAt(tm) + assert.Check(t, ts3-ts2 <= 1) + tm2 := Timestamp(ts2) + tm3 := Timestamp(ts3) + assert.Check(t, tm.Sub(tm2) < 1*time.Millisecond) // check millisecond accuracy of timestamp + assert.Check(t, tm2.Sub(tm3) < 2*time.Millisecond) + + tx2, err := NewTransaction(goodKey, personaTag, namespace, body) + assert.NilError(t, err) + assert.Check(t, tx2.Timestamp-ts2 <= 1) + assert.Check(t, ts != ts2) +} + func TestFailsIfFieldsMissing(t *testing.T) { goodKey, err := crypto.GenerateKey() assert.NilError(t, err)