From 378aec67eb0c53ae54b9c99f66188ca8f76a5d05 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=BDiga=20Kokelj?= Date: Fri, 24 Nov 2023 10:17:31 +0100 Subject: [PATCH] Use eth sign typed data v4 in gateway (#1643) --- design/ux/Obscuro_Gateway.md | 36 ++++- go/common/viewingkey/viewing_key.go | 133 ++++++++++++++++++ go/enclave/enclave.go | 24 ++-- go/enclave/events/subscription_manager.go | 6 +- go/enclave/vkhandler/vk_handler.go | 38 ++--- go/enclave/vkhandler/vk_handler_test.go | 42 ++++-- .../obscurogateway/obscurogateway_test.go | 1 + tools/walletextension/api/routes.go | 14 +- .../api/staticOG/javascript.js | 82 +++++++---- tools/walletextension/common/common.go | 29 ---- tools/walletextension/common/constants.go | 2 +- tools/walletextension/config/config.go | 1 + .../container/walletextension_container.go | 2 +- tools/walletextension/lib/client_lib.go | 19 ++- tools/walletextension/main/cli.go | 6 + .../storage/database/001_init.sql | 4 +- .../storage/database/sqlite.go | 4 +- tools/walletextension/test/apis.go | 3 +- tools/walletextension/wallet_extension.go | 74 ++-------- 19 files changed, 326 insertions(+), 194 deletions(-) diff --git a/design/ux/Obscuro_Gateway.md b/design/ux/Obscuro_Gateway.md index 6c58a31d28..6dad2356f3 100644 --- a/design/ux/Obscuro_Gateway.md +++ b/design/ux/Obscuro_Gateway.md @@ -112,7 +112,7 @@ group Second click end group Third click - Alice -> MM: Automatically open MM with request to \nsign over "Register $UserId for $Acct" + Alice -> MM: Automatically open MM with request to \nsign over EIP-712 formatted message note right This text will be sent as is accompanied by the signature and @@ -129,7 +129,33 @@ Alice -> OG: All further Ten interactions will be to\nhttps://gateway.ten.org/v1 The onboarding should be done in 3 clicks. 1. The user goes to a website (like "ten.org"), where she clicks "Join Ten". This will add a network to their wallet. 2. User connects the wallet to the page. -3. In the wallet popup, the user has to sign over a message: "Register $UserId for $ACCT" +3. In the wallet popup, the user has to sign over EIP-712 formatted message. + +Format of EIP-712 message used for signing viewing keys is: + +``` +types: { + EIP712Domain: [ + { name: "name", type: "string" }, + { name: "version", type: "string" }, + { name: "chainId", type: "uint256" }, + ], + Authentication: [ + { name: "Encryption Token", type: "address" }, + ], + }, + primaryType: "Authentication", + domain: { + name: "Ten", + version: "1.0", + chainId: obscuroChainIDDecimal, + }, + message: { + "Encryption Token": "0x"+userID + }, + }; + +``` ##### Click 1 1. Behind the scenes, a js functions calls "gateway.ten.org/v1/join" where it will generate a VK and send back the hash of the Public key. This is the "UserId" @@ -139,7 +165,7 @@ Notice that the UserId has to be included as a query parameter because it must b ##### Click 2 After these actions are complete, the same page will now ask the user to connect the wallet and switch to Ten. -Automatically the page will open metamask and ask the user to sign over a text "Register $UserId for $ACCT", where ACCT is the current account selected in metamask. +Automatically, the page will open metamask and ask the user to sign over an EIP-712 formatted message as described above. ##### Click 3 Once signed, this will be submitted in the background to: "https://gateway.ten.org/v1?u=$UserId&action=register" @@ -149,8 +175,6 @@ Note: Any further accounts will be registered similarly for the same UserId. Note: The user must guard the UserId. Anyone who can read it, will be able to read the data of this user. -The ultimate goal of this protocol is to submit the "Register $UserId for $ACCT" text to the gateway, which is required by an Ten node to authenticate viewing keys per address. - Note: Alternative UXes that achieve the same goal are ok. @@ -193,7 +217,7 @@ This endpoints responds a json of true or false if the address "a" is already re ### Authenticate address - POST "/authenticate?u=$UserId" JSON Fields: -- text +- address - signature This call will be made by a javascript function after it has collected the signed text containing the UserId and the Address from the wallet. diff --git a/go/common/viewingkey/viewing_key.go b/go/common/viewingkey/viewing_key.go index b5c740774d..7fb8d12b19 100644 --- a/go/common/viewingkey/viewing_key.go +++ b/go/common/viewingkey/viewing_key.go @@ -1,14 +1,19 @@ package viewingkey import ( + "bytes" "crypto/ecdsa" "encoding/hex" + "errors" "fmt" + "math/big" "strings" "github.com/ethereum/go-ethereum/accounts" + "github.com/ethereum/go-ethereum/common/math" "github.com/ethereum/go-ethereum/crypto" "github.com/ethereum/go-ethereum/crypto/ecies" + "github.com/ethereum/go-ethereum/signer/core/apitypes" "github.com/ten-protocol/go-ten/go/wallet" gethcommon "github.com/ethereum/go-ethereum/common" @@ -22,6 +27,18 @@ import ( // signed as-is. const SignedMsgPrefix = "vk" +const ( + EIP712Domain = "EIP712Domain" + EIP712Type = "Authentication" + EIP712DomainName = "name" + EIP712DomainVersion = "version" + EIP712DomainChainID = "chainId" + EIP712EncryptionToken = "Encryption Token" + EIP712DomainNameValue = "Ten" + EIP712DomainVersionValue = "1.0" + UserIDHexLength = 40 +) + // ViewingKey encapsulates the signed viewing key for an account for use in encrypted communication with an enclave type ViewingKey struct { Account *gethcommon.Address // Account address that this Viewing Key is bound to - Users Pubkey address @@ -109,3 +126,119 @@ func GenerateSignMessageOG(vkPubKey []byte, addr *gethcommon.Address) string { userID := crypto.Keccak256Hash(vkPubKey).Bytes() return fmt.Sprintf("Register %s for %s", hex.EncodeToString(userID), strings.ToLower(addr.Hex())) } + +// GenerateAuthenticationEIP712RawData generates raw data (bytes) +// for an EIP-712 message used to authenticate an address with user +func GenerateAuthenticationEIP712RawData(userID string, chainID int64) ([]byte, error) { + if len(userID) != UserIDHexLength { + return nil, fmt.Errorf("userID hex length must be %d, received %d", UserIDHexLength, len(userID)) + } + encryptionToken := "0x" + userID + + types := apitypes.Types{ + EIP712Domain: { + {Name: EIP712DomainName, Type: "string"}, + {Name: EIP712DomainVersion, Type: "string"}, + {Name: EIP712DomainChainID, Type: "uint256"}, + }, + EIP712Type: { + {Name: EIP712EncryptionToken, Type: "address"}, + }, + } + + domain := apitypes.TypedDataDomain{ + Name: EIP712DomainNameValue, + Version: EIP712DomainVersionValue, + ChainId: (*math.HexOrDecimal256)(big.NewInt(chainID)), + } + + message := map[string]interface{}{ + EIP712EncryptionToken: encryptionToken, + } + + typedData := apitypes.TypedData{ + Types: types, + PrimaryType: EIP712Type, + Domain: domain, + Message: message, + } + + // Now we need to create EIP-712 compliant hash. + // It involves hashing the message with its structure, hashing domain separator, + // and then encoding both hashes with specific EIP-712 bytes to construct the final message format. + + // Hash the EIP-712 message using its type and content + typedDataHash, err := typedData.HashStruct(typedData.PrimaryType, typedData.Message) + if err != nil { + return nil, err + } + // Create the domain separator hash for EIP-712 message context + domainSeparator, err := typedData.HashStruct(EIP712Domain, typedData.Domain.Map()) + if err != nil { + return nil, err + } + // Prefix domain and message hashes with EIP-712 version and encoding bytes + rawData := append([]byte("\x19\x01"), append(domainSeparator, typedDataHash...)...) + return rawData, nil +} + +// CalculateUserIDHex CalculateUserID calculates userID from a public key +// (we truncate it, because we want it to have length 20) and encode to hex strings +func CalculateUserIDHex(publicKeyBytes []byte) string { + return hex.EncodeToString(CalculateUserID(publicKeyBytes)) +} + +// CalculateUserID calculates userID from a public key (we truncate it, because we want it to have length 20) +func CalculateUserID(publicKeyBytes []byte) []byte { + return crypto.Keccak256Hash(publicKeyBytes).Bytes()[:20] +} + +func VerifySignatureEIP712(userID string, address *gethcommon.Address, signature []byte, chainID int64) (bool, error) { + // get raw data for structured message + rawData, err := GenerateAuthenticationEIP712RawData(userID, chainID) + if err != nil { + return false, err + } + + // create a hash of structured message (needed for signature verification) + hashBytes := crypto.Keccak256(rawData) + hash := gethcommon.BytesToHash(hashBytes) + + if len(signature) != 65 { + return false, fmt.Errorf("invalid signature length: %d", len(signature)) + } + + // We transform the V from 27/28 to 0/1. This same change is made in Geth internals, for legacy reasons to be able + // to recover the address: https://github.com/ethereum/go-ethereum/blob/55599ee95d4151a2502465e0afc7c47bd1acba77/internal/ethapi/api.go#L452-L459 + if signature[64] == 27 || signature[64] == 28 { + signature[64] -= 27 + } + + pubKeyBytes, err := crypto.Ecrecover(hash[:], signature) + if err != nil { + return false, fmt.Errorf("invalid signature: %w", err) + } + + pubKey, err := crypto.UnmarshalPubkey(pubKeyBytes) + if err != nil { + return false, fmt.Errorf("cannot unmarshal public key: %w", err) + } + + recoveredAddr := crypto.PubkeyToAddress(*pubKey) + + if !bytes.Equal(recoveredAddr.Bytes(), address.Bytes()) { + return false, errors.New("address from signature not the same as expected") + } + + r := new(big.Int).SetBytes(signature[:32]) + s := new(big.Int).SetBytes(signature[32:64]) + + // Verify the signature + isValid := ecdsa.Verify(pubKey, hashBytes, r, s) + + if !isValid { + return false, errors.New("signature is not valid") + } + + return true, nil +} diff --git a/go/enclave/enclave.go b/go/enclave/enclave.go index 2cc0fedab8..f0f7b37385 100644 --- a/go/enclave/enclave.go +++ b/go/enclave/enclave.go @@ -180,7 +180,7 @@ func NewEnclave( crossChainProcessors := crosschain.New(&config.MessageBusAddress, storage, big.NewInt(config.ObscuroChainID), logger) - subscriptionManager := events.NewSubscriptionManager(&rpcEncryptionManager, storage, logger) + subscriptionManager := events.NewSubscriptionManager(&rpcEncryptionManager, storage, config.ObscuroChainID, logger) gasOracle := gas.NewGasOracle() blockProcessor := components.NewBlockProcessor(storage, crossChainProcessors, gasOracle, logger) @@ -491,7 +491,7 @@ func (e *enclaveImpl) SubmitTx(tx common.EncryptedTx) (*responses.RawTx, common. } // extract, create and validate the VK encryption handler - vkHandler, err := createVKHandler(&viewingKeyAddress, paramList[0]) + vkHandler, err := createVKHandler(&viewingKeyAddress, paramList[0], e.config.ObscuroChainID) if err != nil { return responses.AsPlaintextError(fmt.Errorf("unable to create VK encryptor - %w", err)), nil } @@ -629,7 +629,7 @@ func (e *enclaveImpl) ObsCall(encryptedParams common.EncryptedParamsCall) (*resp } // extract, create and validate the VK encryption handler - vkHandler, err := createVKHandler(apiArgs.From, paramList[0]) + vkHandler, err := createVKHandler(apiArgs.From, paramList[0], e.config.ObscuroChainID) if err != nil { return responses.AsPlaintextError(fmt.Errorf("unable to create VK encryptor - %w", err)), nil } @@ -689,7 +689,7 @@ func (e *enclaveImpl) GetTransactionCount(encryptedParams common.EncryptedParams address := gethcommon.HexToAddress(addressStr) // extract, create and validate the VK encryption handler - vkHandler, err := createVKHandler(&address, paramList[0]) + vkHandler, err := createVKHandler(&address, paramList[0], e.config.ObscuroChainID) if err != nil { return responses.AsPlaintextError(fmt.Errorf("unable to create VK encryptor - %w", err)), nil } @@ -748,7 +748,7 @@ func (e *enclaveImpl) GetTransaction(encryptedParams common.EncryptedParamsGetTx } // extract, create and validate the VK encryption handler - vkHandler, err := createVKHandler(&viewingKeyAddress, paramList[0]) + vkHandler, err := createVKHandler(&viewingKeyAddress, paramList[0], e.config.ObscuroChainID) if err != nil { return responses.AsPlaintextError(fmt.Errorf("unable to create VK encryptor - %w", err)), nil } @@ -803,7 +803,7 @@ func (e *enclaveImpl) GetTransactionReceipt(encryptedParams common.EncryptedPara } // extract, create and validate the VK encryption handler - vkHandler, err := createVKHandler(&sender, paramList[0]) + vkHandler, err := createVKHandler(&sender, paramList[0], e.config.ObscuroChainID) if err != nil { e.logger.Trace("error getting the vk ", "txHash", txHash, log.ErrKey, err) return responses.AsPlaintextError(fmt.Errorf("unable to create VK encryptor - %w", err)), nil @@ -920,7 +920,7 @@ func (e *enclaveImpl) GetBalance(encryptedParams common.EncryptedParamsGetBalanc } // extract, create and validate the VK encryption handler - vkHandler, err := createVKHandler(encryptAddress, paramList[0]) + vkHandler, err := createVKHandler(encryptAddress, paramList[0], e.config.ObscuroChainID) if err != nil { return responses.AsPlaintextError(fmt.Errorf("unable to create VK encryptor - %w", err)), nil } @@ -1015,7 +1015,7 @@ func (e *enclaveImpl) EstimateGas(encryptedParams common.EncryptedParamsEstimate } // extract, create and validate the VK encryption handler - vkHandler, err := createVKHandler(callMsg.From, paramList[0]) + vkHandler, err := createVKHandler(callMsg.From, paramList[0], e.config.ObscuroChainID) if err != nil { return responses.AsPlaintextError(fmt.Errorf("unable to create VK encryptor - %w", err)), nil } @@ -1069,7 +1069,7 @@ func (e *enclaveImpl) GetLogs(encryptedParams common.EncryptedParamsGetLogs) (*r } // extract, create and validate the VK encryption handler - vkHandler, err := createVKHandler(forAddress, paramList[0]) + vkHandler, err := createVKHandler(forAddress, paramList[0], e.config.ObscuroChainID) if err != nil { return responses.AsPlaintextError(fmt.Errorf("unable to create VK encryptor - %w", err)), nil } @@ -1333,7 +1333,7 @@ func (e *enclaveImpl) GetCustomQuery(encryptedParams common.EncryptedParamsGetSt } // extract, create and validate the VK encryption handler - vkHandler, err := createVKHandler(&privateCustomQuery.Address, paramList[0]) + vkHandler, err := createVKHandler(&privateCustomQuery.Address, paramList[0], e.config.ObscuroChainID) if err != nil { return responses.AsPlaintextError(fmt.Errorf("unable to create VK encryptor - %w", err)), nil } @@ -1593,13 +1593,13 @@ func replayBatchesToValidState(storage storage.Storage, registry components.Batc return nil } -func createVKHandler(address *gethcommon.Address, vkIntf interface{}) (*vkhandler.VKHandler, error) { +func createVKHandler(address *gethcommon.Address, vkIntf interface{}, chainID int64) (*vkhandler.VKHandler, error) { vkPubKeyHexBytes, accountSignatureHexBytes, err := gethencoding.ExtractViewingKey(vkIntf) if err != nil { return nil, fmt.Errorf("unable to decode viewing key - %w", err) } - encryptor, err := vkhandler.New(address, vkPubKeyHexBytes, accountSignatureHexBytes) + encryptor, err := vkhandler.New(address, vkPubKeyHexBytes, accountSignatureHexBytes, chainID) if err != nil { return nil, fmt.Errorf("unable to create vk encryption for request - %w", err) } diff --git a/go/enclave/events/subscription_manager.go b/go/enclave/events/subscription_manager.go index 47625eb32d..170f4f980d 100644 --- a/go/enclave/events/subscription_manager.go +++ b/go/enclave/events/subscription_manager.go @@ -34,17 +34,19 @@ type SubscriptionManager struct { storage storage.Storage subscriptions map[gethrpc.ID]*common.LogSubscription + chainID int64 subscriptionMutex *sync.RWMutex // the mutex guards the subscriptions/lastHead pair logger gethlog.Logger } -func NewSubscriptionManager(rpcEncryptionManager *rpc.EncryptionManager, storage storage.Storage, logger gethlog.Logger) *SubscriptionManager { +func NewSubscriptionManager(rpcEncryptionManager *rpc.EncryptionManager, storage storage.Storage, chainID int64, logger gethlog.Logger) *SubscriptionManager { return &SubscriptionManager{ rpcEncryptionManager: rpcEncryptionManager, storage: storage, subscriptions: map[gethrpc.ID]*common.LogSubscription{}, + chainID: chainID, subscriptionMutex: &sync.RWMutex{}, logger: logger, } @@ -64,7 +66,7 @@ func (s *SubscriptionManager) AddSubscription(id gethrpc.ID, encryptedSubscripti } // create viewing key encryption handler for pushing future logs - encryptor, err := vkhandler.New(subscription.Account, subscription.PublicViewingKey, subscription.Signature) + encryptor, err := vkhandler.New(subscription.Account, subscription.PublicViewingKey, subscription.Signature, s.chainID) if err != nil { return fmt.Errorf("unable to create vk encryption for request - %w", err) } diff --git a/go/enclave/vkhandler/vk_handler.go b/go/enclave/vkhandler/vk_handler.go index bae8416548..9c4cd4e1a6 100644 --- a/go/enclave/vkhandler/vk_handler.go +++ b/go/enclave/vkhandler/vk_handler.go @@ -5,11 +5,11 @@ import ( "fmt" "github.com/ethereum/go-ethereum/accounts" + gethcommon "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/crypto" - "github.com/ethereum/go-ethereum/crypto/ecies" "github.com/ten-protocol/go-ten/go/common/viewingkey" - gethcommon "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/crypto/ecies" ) var ErrInvalidAddressSignature = fmt.Errorf("invalid viewing key signature for requested address") @@ -22,23 +22,24 @@ type VKHandler struct { publicViewingKey *ecies.PublicKey } -// New creates a new viewing key handler -// checks if the signature is valid -// as well if signature matches account address -// todo (@ziga) - function now accepts both old and new messages -func New(requestedAddr *gethcommon.Address, vkPubKeyBytes, accountSignatureHexBytes []byte) (*VKHandler, error) { - // Recalculate the message signed by MetaMask. - msgToSign := viewingkey.GenerateSignMessageOG(vkPubKeyBytes, requestedAddr) +// VKHandler is responsible for: +// - checking if received signature of a provided viewing key is signed by provided address +// - encrypting payloads with a viewing key (public key) that can only be decrypted by private key signed owned by an address signing it - // We recover the key based on the signed message and the signature. - recoveredAccountPublicKey, err := crypto.SigToPub(accounts.TextHash([]byte(msgToSign)), accountSignatureHexBytes) - if err != nil { - return nil, fmt.Errorf("viewing key but could not validate its signature - %w", err) - } - recoveredAccountAddress := crypto.PubkeyToAddress(*recoveredAccountPublicKey) +// New creates a new viewing key handler if signature is valid and was produced by given address +// It receives address, viewing key and a signature over viewing key. +// In order to check signature validity, we need to reproduce a message that was originally signed +func New(requestedAddr *gethcommon.Address, vkPubKeyBytes, accountSignatureHexBytes []byte, chainID int64) (*VKHandler, error) { + // get userID from viewingKey public key + userID := viewingkey.CalculateUserIDHex(vkPubKeyBytes) + + // check if the signature is valid + // TODO: @ziga - after removing old wallet extension signatures we can return if the signature is invalid + isValidSignature, _ := viewingkey.VerifySignatureEIP712(userID, requestedAddr, accountSignatureHexBytes, chainID) + // Old wallet extension message format // We recover the key based on the signed message and the signature (same as before, but with legacy message format "vk"+" - // todo (@ziga) remove this once old WE message format is deprecated + // todo (@ziga) remove support of old message format when removing old wallet extension endpoints (#2134) msgToSignLegacy := viewingkey.GenerateSignMessage(vkPubKeyBytes) recoveredAccountPublicKeyLegacy, err := crypto.SigToPub(accounts.TextHash([]byte(msgToSignLegacy)), accountSignatureHexBytes) if err != nil { @@ -47,9 +48,8 @@ func New(requestedAddr *gethcommon.Address, vkPubKeyBytes, accountSignatureHexBy recoveredAccountAddressLegacy := crypto.PubkeyToAddress(*recoveredAccountPublicKeyLegacy) // is the requested account address the same as the address recovered from the signature - // todo (@ziga) - we currently check also for legacy address and allow both (remove this after transition period) - if requestedAddr.Hash() != recoveredAccountAddress.Hash() && - requestedAddr.Hash() != recoveredAccountAddressLegacy.Hash() { + if requestedAddr.Hash() != recoveredAccountAddressLegacy.Hash() && + !isValidSignature { return nil, ErrInvalidAddressSignature } diff --git a/go/enclave/vkhandler/vk_handler_test.go b/go/enclave/vkhandler/vk_handler_test.go index bfd1bb9501..a86e6ee74d 100644 --- a/go/enclave/vkhandler/vk_handler_test.go +++ b/go/enclave/vkhandler/vk_handler_test.go @@ -10,6 +10,8 @@ import ( "github.com/ten-protocol/go-ten/go/common/viewingkey" ) +const chainID = 443 + func TestVKHandler(t *testing.T) { // generate user private Key userPrivKey, err := crypto.GenerateKey() @@ -24,19 +26,26 @@ func TestVKHandler(t *testing.T) { t.Fatalf(err.Error()) } vkPubKeyBytes := crypto.CompressPubkey(ecies.ImportECDSAPublic(&vkPrivKey.PublicKey).ExportECDSA()) + userID := viewingkey.CalculateUserIDHex(vkPubKeyBytes) + WEMessageFormatTestHash := accounts.TextHash([]byte(viewingkey.GenerateSignMessage(vkPubKeyBytes))) + EIP712MessageData, err := viewingkey.GenerateAuthenticationEIP712RawData(userID, chainID) + if err != nil { + t.Fatalf(err.Error()) + } + EIP712MessageFormatTestHash := crypto.Keccak256(EIP712MessageData) - tests := map[string]string{ - "WEMessageFormatTest": viewingkey.GenerateSignMessage(vkPubKeyBytes), - "OGMessageFormatTest": viewingkey.GenerateSignMessageOG(vkPubKeyBytes, &userAddr), + tests := map[string][]byte{ + "WEMessageFormatTest": WEMessageFormatTestHash, + "EIP712MessageFormatTest": EIP712MessageFormatTestHash, } - for testName, msgToSign := range tests { + for testName, msgHashToSign := range tests { t.Run(testName, func(t *testing.T) { - signature, err := crypto.Sign(accounts.TextHash([]byte(msgToSign)), userPrivKey) + signature, err := crypto.Sign(msgHashToSign, userPrivKey) assert.NoError(t, err) // Create a new vk Handler - _, err = New(&userAddr, vkPubKeyBytes, signature) + _, err = New(&userAddr, vkPubKeyBytes, signature, chainID) assert.NoError(t, err) }) } @@ -56,25 +65,32 @@ func TestSignAndCheckSignature(t *testing.T) { t.Fatalf(err.Error()) } vkPubKeyBytes := crypto.CompressPubkey(ecies.ImportECDSAPublic(&vkPrivKey.PublicKey).ExportECDSA()) + userID := viewingkey.CalculateUserIDHex(vkPubKeyBytes) + WEMessageFormatTestHash := accounts.TextHash([]byte(viewingkey.GenerateSignMessage(vkPubKeyBytes))) + EIP712MessageData, err := viewingkey.GenerateAuthenticationEIP712RawData(userID, chainID) + if err != nil { + t.Fatalf(err.Error()) + } + EIP712MessageFormatTestHash := crypto.Keccak256(EIP712MessageData) - tests := map[string]string{ - "WEMessageFormatTest": viewingkey.GenerateSignMessage(vkPubKeyBytes), - "OGMessageFormatTest": viewingkey.GenerateSignMessageOG(vkPubKeyBytes, &userAddr), + tests := map[string][]byte{ + "WEMessageFormatTest": WEMessageFormatTestHash, + "EIP712MessageFormatTest": EIP712MessageFormatTestHash, } - for testName, msgToSign := range tests { + for testName, msgHashToSign := range tests { t.Run(testName, func(t *testing.T) { // sign the message - signature, err := crypto.Sign(accounts.TextHash([]byte(msgToSign)), userPrivKey) + signature, err := crypto.Sign(msgHashToSign, userPrivKey) assert.NoError(t, err) // Recover the key based on the signed message and the signature. - recoveredAccountPublicKey, err := crypto.SigToPub(accounts.TextHash([]byte(msgToSign)), signature) + recoveredAccountPublicKey, err := crypto.SigToPub(msgHashToSign, signature) assert.NoError(t, err) recoveredAccountAddress := crypto.PubkeyToAddress(*recoveredAccountPublicKey) if recoveredAccountAddress.Hex() != userAddr.Hex() { - t.Errorf("unable to recover user address from signature") + t.Fatalf("Expected user address %s, got %s", userAddr.Hex(), recoveredAccountAddress.Hex()) } _, err = crypto.DecompressPubkey(vkPubKeyBytes) diff --git a/integration/obscurogateway/obscurogateway_test.go b/integration/obscurogateway/obscurogateway_test.go index 65f0391997..70091d1ac1 100644 --- a/integration/obscurogateway/obscurogateway_test.go +++ b/integration/obscurogateway/obscurogateway_test.go @@ -63,6 +63,7 @@ func TestObscuroGateway(t *testing.T) { LogPath: "sys_out", VerboseFlag: false, DBType: "sqlite", + TenChainID: 443, } obscuroGwContainer := container.NewWalletExtensionContainerFromConfig(obscuroGatewayConf, testlog.Logger()) diff --git a/tools/walletextension/api/routes.go b/tools/walletextension/api/routes.go index a9ebfd7973..a933874fcf 100644 --- a/tools/walletextension/api/routes.go +++ b/tools/walletextension/api/routes.go @@ -301,25 +301,25 @@ func authenticateRequestHandler(walletExt *walletextension.WalletExtension, conn return } - // get message from the request - message, ok := reqJSONMap[common.JSONKeyMessage] - if !ok || message == "" { - handleError(conn, walletExt.Logger(), fmt.Errorf("unable to read message field from the request")) + // get address from the request + address, ok := reqJSONMap[common.JSONKeyAddress] + if !ok || address == "" { + handleError(conn, walletExt.Logger(), fmt.Errorf("unable to read address field from the request")) return } // read userID from query params hexUserID, err := getUserID(conn, 2) if err != nil { - handleError(conn, walletExt.Logger(), fmt.Errorf("malformed query: 'u' required - representing userID - %w", err)) + handleError(conn, walletExt.Logger(), fmt.Errorf("malformed query: 'u' required - representing encryption token - %w", err)) return } // check signature and add address and signature for that user - err = walletExt.AddAddressToUser(hexUserID, message, signature) + err = walletExt.AddAddressToUser(hexUserID, address, signature) if err != nil { handleError(conn, walletExt.Logger(), fmt.Errorf("internal error")) - walletExt.Logger().Error("error adding address to user with message", "message", message, log.ErrKey, err) + walletExt.Logger().Error(fmt.Sprintf("error adding address: %s to user: %s with signature: %s", address, hexUserID, signature)) return } err = conn.WriteResponse([]byte(common.SuccessMsg)) diff --git a/tools/walletextension/api/staticOG/javascript.js b/tools/walletextension/api/staticOG/javascript.js index 538f85b9d5..69e869d078 100644 --- a/tools/walletextension/api/staticOG/javascript.js +++ b/tools/walletextension/api/staticOG/javascript.js @@ -19,6 +19,7 @@ const pathQuery = obscuroGatewayVersion + "/query/"; const pathRevoke = obscuroGatewayVersion + "/revoke/"; const pathVersion = "/version/"; const obscuroChainIDDecimal = 443; +const userIDHexLength = 40; const methodPost = "post"; const methodGet = "get"; const jsonHeaders = { @@ -30,7 +31,7 @@ const metamaskPersonalSign = "personal_sign"; const obscuroChainIDHex = "0x" + obscuroChainIDDecimal.toString(16); // Convert to hexadecimal and prefix with '0x' function isValidUserIDFormat(value) { - return typeof value === "string" && value.length === 64; + return typeof value === "string" && value.length === userIDHexLength; } let obscuroGatewayAddress = @@ -118,39 +119,58 @@ async function addNetworkToMetaMask(ethereum, userID, chainIDDecimal) { return true; } -async function authenticateAccountWithObscuroGateway( - ethereum, - account, - userID -) { - const isAuthenticated = await accountIsAuthenticated(account, userID); - if (isAuthenticated) { - return "Account is already authenticated"; - } +async function authenticateAccountWithObscuroGatewayEIP712(ethereum, account, userID) { + const isAuthenticated = await accountIsAuthenticated(account, userID) - const textToSign = "Register " + userID + " for " + account.toLowerCase(); - const signature = await ethereum - .request({ - method: metamaskPersonalSign, - params: [textToSign, account], - }) - .catch((_) => { - return -1; - }); - if (signature === -1) { - return "Signing failed"; + if (isAuthenticated) { + return "Account is already authenticated" } - const authenticateUserURL = pathAuthenticate + "?u=" + userID; - const authenticateFields = { signature: signature, message: textToSign }; - const authenticateResp = await fetch(authenticateUserURL, { - method: methodPost, - headers: jsonHeaders, - body: JSON.stringify(authenticateFields), + const typedData = { + types: { + EIP712Domain: [ + { name: "name", type: "string" }, + { name: "version", type: "string" }, + { name: "chainId", type: "uint256" }, + ], + Authentication: [ + { name: "Encryption Token", type: "address" }, + ], + }, + primaryType: "Authentication", + domain: { + name: "Ten", + version: "1.0", + chainId: obscuroChainIDDecimal, + }, + message: { + "Encryption Token": "0x"+userID + }, + }; + + const data = JSON.stringify(typedData); + const signature = await ethereum.request({ + method: "eth_signTypedData_v4", + params: [account, data], + }).catch(_ => { + console.log("signing failed!") + return -1; }); - return await authenticateResp.text(); + + + const authenticateUserURL = pathAuthenticate+"?u="+userID + const authenticateFields = {"signature": signature, "address": account } + const authenticateResp = await fetch( + authenticateUserURL, { + method: methodPost, + headers: jsonHeaders, + body: JSON.stringify(authenticateFields) + } + ); + return await authenticateResp.text() } + async function accountIsAuthenticated(account, userID) { const queryAccountUserID = pathQuery + "?u=" + userID + "&a=" + account; const isAuthenticatedResponse = await fetch(queryAccountUserID, { @@ -275,7 +295,7 @@ async function populateAccountsTable(document, tableBody, userID) { connectButton.style.cursor = "pointer"; connectButton.addEventListener("click", async (event) => { event.preventDefault(); - await authenticateAccountWithObscuroGateway(ethereum, account, userID); + await authenticateAccountWithObscuroGatewayEIP712(ethereum, account, userID); }); statusCell.appendChild(connectButton); } @@ -435,7 +455,7 @@ const initialize = async () => { beginBox.style.visibility = "hidden"; spinner.style.visibility = "visible"; for (const account of accounts) { - await authenticateAccountWithObscuroGateway(ethereum, account, userID); + await authenticateAccountWithObscuroGatewayEIP712(ethereum, account, userID); accountsTable.style.display = "block"; await populateAccountsTable(document, tableBody, userID); } @@ -445,7 +465,7 @@ const initialize = async () => { if (isValidUserIDFormat(await getUserID())) { userID = await getUserID(); for (const account of accounts) { - await authenticateAccountWithObscuroGateway( + await authenticateAccountWithObscuroGatewayEIP712( ethereum, account, userID diff --git a/tools/walletextension/common/common.go b/tools/walletextension/common/common.go index 4a3698d945..d2e974beec 100644 --- a/tools/walletextension/common/common.go +++ b/tools/walletextension/common/common.go @@ -1,12 +1,9 @@ package common import ( - "crypto/ecdsa" "encoding/hex" "encoding/json" - "errors" "fmt" - "regexp" "github.com/ethereum/go-ethereum/crypto" "github.com/ethereum/go-ethereum/crypto/ecies" @@ -17,8 +14,6 @@ import ( gethlog "github.com/ethereum/go-ethereum/log" ) -var authenticateMessageRegex = regexp.MustCompile(MessageFormatRegex) - // PrivateKeyToCompressedPubKey converts *ecies.PrivateKey to compressed PubKey ([]byte with length 33) func PrivateKeyToCompressedPubKey(prvKey *ecies.PrivateKey) []byte { ecdsaPublicKey := prvKey.PublicKey.ExportECDSA() @@ -37,30 +32,6 @@ func BytesToPrivateKey(keyBytes []byte) (*ecies.PrivateKey, error) { return eciesPrivateKey, nil } -// CalculateUserID calculates userID from a public key -func CalculateUserID(publicKeyBytes []byte) []byte { - return crypto.Keccak256Hash(publicKeyBytes).Bytes() -} - -// GetUserIDAndAddressFromMessage checks if message is in correct format and extracts userID and address from it -func GetUserIDAndAddressFromMessage(message string) (string, string, error) { - if authenticateMessageRegex.MatchString(message) { - params := authenticateMessageRegex.FindStringSubmatch(message) - return params[1], params[2], nil - } - return "", "", errors.New("invalid message format") -} - -// GetAddressAndPubKeyFromSignature gets an address that was used to sign given signature -func GetAddressAndPubKeyFromSignature(messageHash []byte, signature []byte) (gethcommon.Address, *ecdsa.PublicKey, error) { - pubKey, err := crypto.SigToPub(messageHash, signature) - if err != nil { - return gethcommon.Address{}, nil, err - } - - return crypto.PubkeyToAddress(*pubKey), pubKey, nil -} - // GetUserIDbyte converts userID from string to correct byte format func GetUserIDbyte(userID string) ([]byte, error) { return hex.DecodeString(userID) diff --git a/tools/walletextension/common/constants.go b/tools/walletextension/common/constants.go index f7ab956396..ee62e18742 100644 --- a/tools/walletextension/common/constants.go +++ b/tools/walletextension/common/constants.go @@ -40,7 +40,7 @@ const ( UserQueryParameter = "u" AddressQueryParameter = "a" MessageFormatRegex = `^Register\s(\w+)\sfor\s(\w+)$` - MessageUserIDLen = 64 + MessageUserIDLen = 40 SignatureLen = 65 EthereumAddressLen = 42 PersonalSignMessagePrefix = "\x19Ethereum Signed Message:\n%d%s" diff --git a/tools/walletextension/config/config.go b/tools/walletextension/config/config.go index 2ee16d3b65..0914a93fe4 100644 --- a/tools/walletextension/config/config.go +++ b/tools/walletextension/config/config.go @@ -12,4 +12,5 @@ type Config struct { VerboseFlag bool DBType string DBConnectionURL string + TenChainID int } diff --git a/tools/walletextension/container/walletextension_container.go b/tools/walletextension/container/walletextension_container.go index 3e2ebc94c4..65ee6f3144 100644 --- a/tools/walletextension/container/walletextension_container.go +++ b/tools/walletextension/container/walletextension_container.go @@ -70,7 +70,7 @@ func NewWalletExtensionContainerFromConfig(config config.Config, logger gethlog. } stopControl := stopcontrol.New() - walletExt := walletextension.New(hostRPCBindAddr, &userAccountManager, databaseStorage, stopControl, version, logger) + walletExt := walletextension.New(hostRPCBindAddr, &userAccountManager, databaseStorage, stopControl, version, logger, &config) httpRoutes := api.NewHTTPRoutes(walletExt) httpServer := api.NewHTTPServer(fmt.Sprintf("%s:%d", config.WalletExtensionHost, config.WalletExtensionPortHTTP), httpRoutes) diff --git a/tools/walletextension/lib/client_lib.go b/tools/walletextension/lib/client_lib.go index c0a3d5c390..be93d990ec 100644 --- a/tools/walletextension/lib/client_lib.go +++ b/tools/walletextension/lib/client_lib.go @@ -9,9 +9,11 @@ import ( "net/http" "strings" + "github.com/ten-protocol/go-ten/integration" + gethcommon "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/crypto" - "github.com/ten-protocol/go-ten/tools/walletextension/common" + "github.com/ten-protocol/go-ten/go/common/viewingkey" "github.com/valyala/fasthttp" ) @@ -44,17 +46,19 @@ func (o *OGLib) Join() error { func (o *OGLib) RegisterAccount(pk *ecdsa.PrivateKey, addr gethcommon.Address) error { // create the registration message - message := fmt.Sprintf("Register %s for %s", o.userID, strings.ToLower(addr.Hex())) - prefixedMessage := fmt.Sprintf(common.PersonalSignMessagePrefix, len(message), message) + rawMessage, err := viewingkey.GenerateAuthenticationEIP712RawData(string(o.userID), integration.ObscuroChainID) + if err != nil { + return err + } - messageHash := crypto.Keccak256([]byte(prefixedMessage)) + messageHash := crypto.Keccak256(rawMessage) sig, err := crypto.Sign(messageHash, pk) if err != nil { return fmt.Errorf("failed to sign message: %w", err) } sig[64] += 27 signature := "0x" + hex.EncodeToString(sig) - payload := fmt.Sprintf("{\"signature\": \"%s\", \"message\": \"%s\"}", signature, message) + payload := fmt.Sprintf("{\"signature\": \"%s\", \"address\": \"%s\"}", signature, addr.Hex()) // issue the registration message req, err := http.NewRequestWithContext( @@ -76,10 +80,13 @@ func (o *OGLib) RegisterAccount(pk *ecdsa.PrivateKey, addr gethcommon.Address) e } defer response.Body.Close() - _, err = io.ReadAll(response.Body) + r, err := io.ReadAll(response.Body) if err != nil { return fmt.Errorf("unable to read response - %w", err) } + if string(r) != "success" { + return fmt.Errorf("expected success, got %s", string(r)) + } return nil } diff --git a/tools/walletextension/main/cli.go b/tools/walletextension/main/cli.go index b79b7c2625..119a4b9289 100644 --- a/tools/walletextension/main/cli.go +++ b/tools/walletextension/main/cli.go @@ -51,6 +51,10 @@ const ( dbConnectionURLFlagName = "dbConnectionURL" dbConnectionURLFlagDefault = "" dbConnectionURLFlagUsage = "If dbType is set to mariaDB, this must be set. ex: obscurouser:password@tcp(127.0.0.1:3306)/ogdb" + + tenChainIDName = "tenChainID" + tenChainIDDefault = 443 + tenChainIDFlagUsage = "ChainID of Ten network that the gateway is communicating with" ) func parseCLIArgs() config.Config { @@ -65,6 +69,7 @@ func parseCLIArgs() config.Config { verboseFlag := flag.Bool(verboseFlagName, verboseFlagDefault, verboseFlagUsage) dbType := flag.String(dbTypeFlagName, dbTypeFlagDefault, dbTypeFlagUsage) dbConnectionURL := flag.String(dbConnectionURLFlagName, dbConnectionURLFlagDefault, dbConnectionURLFlagUsage) + tenChainID := flag.Int(tenChainIDName, tenChainIDDefault, tenChainIDFlagUsage) flag.Parse() return config.Config{ @@ -78,5 +83,6 @@ func parseCLIArgs() config.Config { VerboseFlag: *verboseFlag, DBType: *dbType, DBConnectionURL: *dbConnectionURL, + TenChainID: *tenChainID, } } diff --git a/tools/walletextension/storage/database/001_init.sql b/tools/walletextension/storage/database/001_init.sql index 20087a4c5a..9cf37e7a17 100644 --- a/tools/walletextension/storage/database/001_init.sql +++ b/tools/walletextension/storage/database/001_init.sql @@ -5,11 +5,11 @@ USE ogdb; GRANT SELECT, INSERT, UPDATE, DELETE ON ogdb.* TO 'obscurouser'; CREATE TABLE IF NOT EXISTS ogdb.users ( - user_id varbinary(32) PRIMARY KEY, + user_id varbinary(20) PRIMARY KEY, private_key varbinary(32) ); CREATE TABLE IF NOT EXISTS ogdb.accounts ( - user_id varbinary(32), + user_id varbinary(20), account_address varbinary(20), signature varbinary(65), FOREIGN KEY(user_id) REFERENCES users(user_id) ON DELETE CASCADE diff --git a/tools/walletextension/storage/database/sqlite.go b/tools/walletextension/storage/database/sqlite.go index 1b0b0c5a12..0140d99470 100644 --- a/tools/walletextension/storage/database/sqlite.go +++ b/tools/walletextension/storage/database/sqlite.go @@ -39,7 +39,7 @@ func NewSqliteDatabase(dbPath string) (*SqliteDatabase, error) { // create users table _, err = db.Exec(`CREATE TABLE IF NOT EXISTS users ( - user_id binary(32) PRIMARY KEY, + user_id binary(20) PRIMARY KEY, private_key binary(32) );`) @@ -49,7 +49,7 @@ func NewSqliteDatabase(dbPath string) (*SqliteDatabase, error) { // create accounts table _, err = db.Exec(`CREATE TABLE IF NOT EXISTS accounts ( - user_id binary(32), + user_id binary(20), account_address binary(20), signature binary(65), FOREIGN KEY(user_id) REFERENCES users(user_id) ON DELETE CASCADE diff --git a/tools/walletextension/test/apis.go b/tools/walletextension/test/apis.go index 81138b4dc6..746563a937 100644 --- a/tools/walletextension/test/apis.go +++ b/tools/walletextension/test/apis.go @@ -23,6 +23,7 @@ import ( const ( l2ChainIDHex = "0x309" + l2ChainIDDecimal = 443 enclavePrivateKeyHex = "81acce9620f0adf1728cb8df7f6b8b8df857955eb9e8b7aed6ef8390c09fc207" ) @@ -171,7 +172,7 @@ func (api *DummyAPI) reEncryptParams(encryptedParams []byte) (*responses.Enclave return responses.AsEmptyResponse(), fmt.Errorf("could not decrypt params with enclave private key. Cause: %w", err) } - encryptor, err := vkhandler.New(api.address, api.viewingKey, api.signature) + encryptor, err := vkhandler.New(api.address, api.viewingKey, api.signature, l2ChainIDDecimal) if err != nil { return nil, fmt.Errorf("unable to create vk encryption for request - %w", err) } diff --git a/tools/walletextension/wallet_extension.go b/tools/walletextension/wallet_extension.go index eb3b9aaf26..2920933d8d 100644 --- a/tools/walletextension/wallet_extension.go +++ b/tools/walletextension/wallet_extension.go @@ -2,11 +2,11 @@ package walletextension import ( "bytes" - "crypto/ecdsa" "encoding/hex" "errors" "fmt" - "math/big" + + "github.com/ten-protocol/go-ten/tools/walletextension/config" "github.com/ten-protocol/go-ten/go/common/log" @@ -35,6 +35,7 @@ type WalletExtension struct { logger gethlog.Logger stopControl *stopcontrol.StopControl version string + config *config.Config } func New( @@ -44,6 +45,7 @@ func New( stopControl *stopcontrol.StopControl, version string, logger gethlog.Logger, + config *config.Config, ) *WalletExtension { return &WalletExtension{ hostAddr: hostAddr, @@ -53,6 +55,7 @@ func New( logger: logger, stopControl: stopControl, version: version, + config: config, } } @@ -188,7 +191,7 @@ func (w *WalletExtension) GenerateAndStoreNewUser() (string, error) { } // create UserID and store it in the database with the private key - userID := common.CalculateUserID(common.PrivateKeyToCompressedPubKey(viewingPrivateKeyEcies)) + userID := viewingkey.CalculateUserID(common.PrivateKeyToCompressedPubKey(viewingPrivateKeyEcies)) err = w.storage.AddUser(userID, crypto.FromECDSA(viewingPrivateKeyEcies.ExportECDSA())) if err != nil { w.Logger().Error(fmt.Sprintf("failed to save user to the database: %s", err)) @@ -202,28 +205,13 @@ func (w *WalletExtension) GenerateAndStoreNewUser() (string, error) { return hexUserID, nil } -// AddAddressToUser checks if message is in correct format and if signature is valid. If all checks pass we save address and signature against userID -func (w *WalletExtension) AddAddressToUser(hexUserID string, message string, signature []byte) error { - // parse the message to get userID and account address - messageUserID, messageAddressHex, err := common.GetUserIDAndAddressFromMessage(message) - if err != nil { - w.Logger().Error(fmt.Errorf("submitted message (%s) is not in the correct format", message).Error()) - return err - } - - // check if userID corresponds to the one in the message and check if the length of hex encoded userID is correct - if hexUserID != messageUserID || len(messageUserID) != common.MessageUserIDLen { - w.Logger().Error(fmt.Errorf("submitted message (%s) is not in the correct format", message).Error()) - return errors.New("userID from message does not match userID from request") - } - - addressFromMessage := gethcommon.HexToAddress(messageAddressHex) - - // check if message was signed by the correct address and if signature is valid - valid, err := verifySignature(message, signature, addressFromMessage) +// AddAddressToUser checks if a message is in correct format and if signature is valid. If all checks pass we save address and signature against userID +func (w *WalletExtension) AddAddressToUser(hexUserID string, address string, signature []byte) error { + addressFromMessage := gethcommon.HexToAddress(address) + // check if a message was signed by the correct address and if the signature is valid + valid, err := viewingkey.VerifySignatureEIP712(hexUserID, &addressFromMessage, signature, int64(w.config.TenChainID)) if !valid && err != nil { - w.Logger().Error(fmt.Errorf("error: signature is not valid: %s", string(signature)).Error()) - return err + return fmt.Errorf("signature is not valid: %w", err) } // register the account for that viewing key @@ -328,44 +316,6 @@ func (w *WalletExtension) UserExists(hexUserID string) bool { return len(key) > 0 } -// verifySignature checks if a message was signed by the correct address and if signature is valid -func verifySignature(message string, signature []byte, address gethcommon.Address) (bool, error) { - // prefix the message like in the personal_sign method - prefixedMessage := fmt.Sprintf(common.PersonalSignMessagePrefix, len(message), message) - messageHash := crypto.Keccak256([]byte(prefixedMessage)) - - // check if the signature length is correct - if len(signature) != common.SignatureLen { - return false, errors.New("incorrect signature length") - } - - // We transform the V from 27/28 to 0/1. This same change is made in Geth internals, for legacy reasons to be able - // to recover the address: https://github.com/ethereum/go-ethereum/blob/55599ee95d4151a2502465e0afc7c47bd1acba77/internal/ethapi/api.go#L452-L459 - signature[64] -= 27 - - addressFromSignature, pubKeyFromSignature, err := common.GetAddressAndPubKeyFromSignature(messageHash, signature) - if err != nil { - return false, err - } - - if !bytes.Equal(addressFromSignature.Bytes(), address.Bytes()) { - return false, errors.New("address from signature not the same as expected") - } - - // Split signature into r, s - r := new(big.Int).SetBytes(signature[:32]) - s := new(big.Int).SetBytes(signature[32:64]) - - // Verify the signature - isValid := ecdsa.Verify(pubKeyFromSignature, messageHash, r, s) - - if !isValid { - return false, errors.New("signature is not valid") - } - - return true, nil -} - func adjustStateRoot(rpcResp interface{}, respMap map[string]interface{}) { if resultMap, ok := rpcResp.(map[string]interface{}); ok { if val, foundRoot := resultMap[common.JSONKeyRoot]; foundRoot {