From 19d9b2a7741cab221cd0a1c6c3b169716f950d2f Mon Sep 17 00:00:00 2001 From: Paul Berg Date: Fri, 2 Nov 2018 12:03:14 +0100 Subject: [PATCH] Fixed bugs in 'data/typed' signs --- cmd/clef/audit.log | 2 + signer/core/api.go | 8 +- signer/core/auditlog.go | 20 ++-- signer/core/signed_data.go | 167 +++++++------------------------- signer/core/signed_data_test.go | 20 ++-- 5 files changed, 58 insertions(+), 159 deletions(-) diff --git a/cmd/clef/audit.log b/cmd/clef/audit.log index 0ce97ab5fe..28aa2f3596 100644 --- a/cmd/clef/audit.log +++ b/cmd/clef/audit.log @@ -402,3 +402,5 @@ t=2018-11-01T22:23:54+0100 lvl=info msg=Configured api=signer audit log=audit.lo t=2018-11-01T22:23:56+0100 lvl=info msg=SignData api=signer type=request metadata="{\"remote\":\"127.0.0.1:60509\",\"local\":\"localhost:8550\",\"scheme\":\"HTTP/1.1\",\"User-Agent\":\"PostmanRuntime/7.3.0\",\"Origin\":\"\"}" addr="0x66d76e6a80dc7d46d7ec1b79b15dfa34c3c6ef21 [chksum INVALID]" data="map[address:0xd090B1504cC9341BC18cE5d34D750545bb515FdC message:0xcafebabe]" content-type=text/validator t=2018-11-01T22:36:15+0100 lvl=info msg=Configured api=signer audit log=audit.log t=2018-11-01T22:38:04+0100 lvl=info msg=SignData api=signer type=request metadata="{\"remote\":\"127.0.0.1:60761\",\"local\":\"localhost:8550\",\"scheme\":\"HTTP/1.1\",\"User-Agent\":\"PostmanRuntime/7.3.0\",\"Origin\":\"\"}" addr="0x66d76e6a80dc7d46d7ec1b79b15dfa34c3c6ef21 [chksum INVALID]" data="map[address:0xE85E8fceAce32f641595bcAf5e16aba14667991D message:0xcafebabe]" content-type=text/validator +t=2018-11-02T11:46:08+0100 lvl=info msg=Configured api=signer audit log=audit.log +t=2018-11-02T11:46:14+0100 lvl=info msg=SignTypedData api=signer type=request metadata="{\"remote\":\"127.0.0.1:49691\",\"local\":\"localhost:8550\",\"scheme\":\"HTTP/1.1\",\"User-Agent\":\"PostmanRuntime/7.3.0\",\"Origin\":\"\"}" addr="0x66D76e6A80DC7D46d7EC1b79b15Dfa34c3C6eF21 [chksum ok]" data="{Types:map[EIP712Domain:[map[name:name type:string] map[name:version type:string] map[name:chainId type:uint256] map[name:verifyingContract type:address]] Person:[map[type:string name:name] map[name:wallet type:address]] Mail:[map[name:from type:Person] map[name:to type:Person] map[name:contents type:string]]] PrimaryType:Mail Domain:{Name:Ether Mail Version:1 ChainId:+1 VerifyingContract:0xCcCCccccCCCCcCCCCCCcCcCccCcCCCcCcccccccC Salt:} Message:map[from:map[name:Alice wallet:0x66d76e6a80dc7d46d7ec1b79b15dfa34c3c6ef21] to:map[name:Bob wallet:0xbBbBBBBbbBBBbbbBbbBbbbbBBbBbbbbBbBbbBBbB] contents:Hello, Bob!]}" diff --git a/signer/core/api.go b/signer/core/api.go index 4903937372..49532f6ac4 100644 --- a/signer/core/api.go +++ b/signer/core/api.go @@ -48,10 +48,10 @@ type ExternalAPI interface { SignTransaction(ctx context.Context, args SendTxArgs, methodSelector *string) (*ethapi.SignTransactionResult, error) // SignData - request to sign the given data (plus prefix) SignData(ctx context.Context, contentType string, addr common.MixedcaseAddress, data interface{}) (hexutil.Bytes, error) - // SignStructuredData - request to sign the given structured data (plus prefix) - //SignTypedData(ctx context.Context, addr common.MixedcaseAddress, data TypedData) (hexutil.Bytes, error) + // SignTypedData - request to sign the given structured data (plus prefix) + SignTypedData(ctx context.Context, addr common.MixedcaseAddress, data TypedData) (hexutil.Bytes, error) // EcRecover - recover public key from given message and signature - EcRecover(ctx context.Context, contentType string, data hexutil.Bytes, sig hexutil.Bytes) (common.Address, error) + EcRecover(ctx context.Context, data hexutil.Bytes, sig hexutil.Bytes) (common.Address, error) // Export - request to export an account Export(ctx context.Context, addr common.Address) (json.RawMessage, error) // Import - request to import an account @@ -175,7 +175,7 @@ type ( SignDataRequest struct { ContentType string `json:"content_type"` Address common.MixedcaseAddress `json:"address"` - Rawdata interface{} `json:"raw_data"` + Rawdata interface{} `json:"raw_data"` Message string `json:"message"` Hash hexutil.Bytes `json:"hash"` Meta Metadata `json:"meta"` diff --git a/signer/core/auditlog.go b/signer/core/auditlog.go index b9769b16e9..578e5ddcb5 100644 --- a/signer/core/auditlog.go +++ b/signer/core/auditlog.go @@ -70,18 +70,18 @@ func (l *AuditLogger) SignData(ctx context.Context, contentType string, addr com return b, e } -//func (l *AuditLogger) SignTypedData(ctx context.Context, addr common.MixedcaseAddress, data TypedData) (hexutil.Bytes, error) { -// l.log.Info("SignTypedData", "type", "request", "metadata", MetadataFromContext(ctx).String(), -// "addr", addr.String(), "data", data) -// b, e := l.api.SignTypedData(ctx, addr, data) -// l.log.Info("SignTypedData", "type", "response", "data", common.Bytes2Hex(b), "error", e) -// return b, e -//} +func (l *AuditLogger) SignTypedData(ctx context.Context, addr common.MixedcaseAddress, data TypedData) (hexutil.Bytes, error) { + l.log.Info("SignTypedData", "type", "request", "metadata", MetadataFromContext(ctx).String(), + "addr", addr.String(), "data", data) + b, e := l.api.SignTypedData(ctx, addr, data) + l.log.Info("SignTypedData", "type", "response", "data", common.Bytes2Hex(b), "error", e) + return b, e +} -func (l *AuditLogger) EcRecover(ctx context.Context, contentType string, data hexutil.Bytes, sig hexutil.Bytes) (common.Address, error) { +func (l *AuditLogger) EcRecover(ctx context.Context, data hexutil.Bytes, sig hexutil.Bytes) (common.Address, error) { l.log.Info("EcRecover", "type", "request", "metadata", MetadataFromContext(ctx).String(), - "data", common.Bytes2Hex(data), "sig", common.Bytes2Hex(sig), "content-type", contentType) - b, e := l.api.EcRecover(ctx, contentType, data, sig) + "data", common.Bytes2Hex(data), "sig", common.Bytes2Hex(sig)) + b, e := l.api.EcRecover(ctx, data, sig) l.log.Info("EcRecover", "type", "response", "address", b.String(), "error", e) return b, e } diff --git a/signer/core/signed_data.go b/signer/core/signed_data.go index f86845ed2f..b8dd636a87 100644 --- a/signer/core/signed_data.go +++ b/signer/core/signed_data.go @@ -19,6 +19,7 @@ package core import ( "bytes" "context" + "encoding/json" "errors" "fmt" "math/big" @@ -63,8 +64,8 @@ var ( ) type ValidatorData struct { - Address common.Address - Message hexutil.Bytes + Address common.Address + Message hexutil.Bytes } type TypedData struct { @@ -86,11 +87,11 @@ type EIP712TypePriority struct { type EIP712Data = map[string]interface{} type EIP712Domain struct { - Name string `json:"name"` - Version string `json:"version"` - ChainId *big.Int `json:"chainId"` - VerifyingContract string `json:"verifyingContract"` - Salt string `json:"salt"` + Name string `json:"name"` + Version string `json:"version"` + ChainId *big.Int `json:"chainId"` + VerifyingContract string `json:"verifyingContract"` + Salt string `json:"salt"` } const ( @@ -99,7 +100,7 @@ const ( TypeBytes = "bytes" TypeInt = "int" TypeString = "string" - TypeUint = "uint" + TypeUint = "uint" ) // Sign receives a request and produces a signature @@ -139,7 +140,7 @@ func (api *SignerAPI) Sign(ctx context.Context, addr common.MixedcaseAddress, re // // Different types of validation occur. func (api *SignerAPI) SignData(ctx context.Context, contentType string, addr common.MixedcaseAddress, data interface{}) (hexutil.Bytes, error) { - var req, err= api.determineSignatureFormat(contentType, addr, data) + var req, err = api.determineSignatureFormat(contentType, addr, data) if err != nil { return nil, err } @@ -173,19 +174,8 @@ func (api *SignerAPI) determineSignatureFormat(contentType string, addr common.M return nil, err } sighash, msg := SignTextValidator(validatorData) - fmt.Printf("sighash:%s", sighash) req = &SignDataRequest{Rawdata: validatorData, Message: msg, Hash: sighash, ContentType: mediaType} break - case DataTyped.Mime: - // Signs EIP-712 conformant typed data - // hash = keccak256("\x19${byteVersion}${domainSeparator}${hashStruct(message)}") - typedData, err := UnmarshalTypedData(data) - if err != nil { - return nil, err - } - sighash, msg := SignDataTyped(typedData) - req = &SignDataRequest{Rawdata: typedData, Message: msg, Hash: sighash, ContentType: mediaType} - break case ApplicationClique.Mime: // Clique is the Ethereum PoA standard cliqueData, err := hexutil.Decode(data.(string)) @@ -222,7 +212,6 @@ func (api *SignerAPI) determineSignatureFormat(contentType string, addr common.M // SignTextWithValidator signs the given message which can be further recovered // with the given validator. -// // hash = keccak256("\x19\x00"${address}${data}). func SignTextValidator(validatorData ValidatorData) (hexutil.Bytes, string) { msg := fmt.Sprintf("\x19\x00%s%s", string(validatorData.Address.Bytes()), string(validatorData.Message)) @@ -267,51 +256,33 @@ func SignCliqueHeader(header *types.Header) (hexutil.Bytes, error) { // SignTextPlain is a helper function that calculates a hash for the given message that can be // safely used to calculate a signature from. This gives context to the signed message and prevents // signing of transactions. -// // hash = keccak256("\x19$Ethereum Signed Message:\n"${message length}${message}). func SignTextPlain(data hexutil.Bytes) (hexutil.Bytes, string) { // The letter `E` is \x45 in hex, retrofitting // https://github.com/ethereum/go-ethereum/pull/2940/commits - msg := fmt.Sprintf("\x19Ethereum Signed Message:\n%d%s", len(data), data) + msg := fmt.Sprintf("\x19Ethereum Signed Message:\n%d%s", len(data), string(data)) return crypto.Keccak256([]byte(msg)), msg } -// SignDataTyped signs EIP-712 conformant typed data -// hash = keccak256("\x19${byteVersion}${domainSeparator}${hashStruct(message)}") -func SignDataTyped(typedData TypedData) (hexutil.Bytes, string) { - domainSeparator := typedData.HashStruct("EIP712Domain", typedData.Domain.Map()) - typedDataHash := typedData.HashStruct(typedData.PrimaryType, typedData.Message) - msg := fmt.Sprintf("\x19\x01%s%s", common.Bytes2Hex(domainSeparator), common.Bytes2Hex(typedDataHash)) - return crypto.Keccak256(common.Hex2Bytes(msg)), msg -} - // SignTypedData signs EIP-712 conformant typed data // hash = keccak256("\x19${byteVersion}${domainSeparator}${hashStruct(message)}") -//func (api *SignerAPI) SignTypedData(ctx context.Context, addr common.MixedcaseAddress, typedData TypedData) (hexutil.Bytes, error) { -// domainSeparator := typedData.HashStruct("EIP712Domain", typedData.Domain.Map()) -// typedDataHash := typedData.HashStruct(typedData.PrimaryType, typedData.Message) -// typedDataJson, err := json.Marshal(typedData.Map()) -// if err != nil { -// return nil, err -// } -// buffer := bytes.Buffer{} -// buffer.WriteString("\x19") -// buffer.WriteString("\x01") -// buffer.WriteString(common.Bytes2Hex(domainSeparator)) -// buffer.WriteString(common.Bytes2Hex(typedDataHash)) -// req := &SignDataRequest{ -// Rawdata: typedDataJson, -// Message: buffer.String(), -// Hash: crypto.Keccak256(buffer.Bytes()), -// ContentType: DataTyped.Mime, -// } -// signature, err := api.Sign(ctx, addr, req) -// if err != nil { -// api.UI.ShowError(err.Error()) -// return nil, err -// } -// return signature, nil -//} +func (api *SignerAPI) SignTypedData(ctx context.Context, addr common.MixedcaseAddress, typedData TypedData) (hexutil.Bytes, error) { + domainSeparator := typedData.HashStruct("EIP712Domain", typedData.Domain.Map()) + typedDataHash := typedData.HashStruct(typedData.PrimaryType, typedData.Message) + _, err := json.Marshal(typedData.Map()) + if err != nil { + return nil, err + } + msg := fmt.Sprintf("\x19\x01%s%s", string(domainSeparator), string(typedDataHash)) + sighash := crypto.Keccak256([]byte(msg)) + req := &SignDataRequest{Rawdata: typedData.Map(), Message: msg, Hash: sighash, ContentType: DataTyped.Mime} + signature, err := api.Sign(ctx, addr, req) + if err != nil { + api.UI.ShowError(err.Error()) + return nil, err + } + return signature, nil +} // HashStruct generates the following encoding for the given domain and message: // `encode(domainSeparator : 𝔹²⁵⁶, message : 𝕊) = "\x19\x01" ‖ domainSeparator ‖ hashStruct(message)` @@ -319,7 +290,7 @@ func (typedData *TypedData) HashStruct(primaryType string, data EIP712Data) hexu return crypto.Keccak256(typedData.EncodeData(primaryType, data)) } -// dependencies returns an array of custom types ordered by their hierarchical reference tree +// Dependencies returns an array of custom types ordered by their hierarchical reference tree func (typedData *TypedData) Dependencies(primaryType string, found []string) []string { includes := func(arr []string, str string) bool { for _, obj := range arr { @@ -347,7 +318,7 @@ func (typedData *TypedData) Dependencies(primaryType string, found []string) []s return found } -// encodeType generates the following encoding: +// EncodeType generates the following encoding: // `name ‖ "(" ‖ member₁ ‖ "," ‖ member₂ ‖ "," ‖ … ‖ memberₙ ")"` // // each member is written as `type ‖ " " ‖ name` encodings cascade down and are sorted by name @@ -496,7 +467,7 @@ func bytesValueOf(_interface interface{}) hexutil.Bytes { case reflect.TypeOf([]uint8{}): return _interface.([]uint8) case reflect.TypeOf(string("")): - return common.Hex2Bytes(_interface.(string)) + return hexutil.Bytes(_interface.(string)) default: break } @@ -507,7 +478,7 @@ func bytesValueOf(_interface interface{}) hexutil.Bytes { // EcRecover recovers the address associated with the given sig. // Only compatible with `text/plain` -func (api *SignerAPI) EcRecover(ctx context.Context, contentType string, data hexutil.Bytes, sig hexutil.Bytes) (common.Address, error) { +func (api *SignerAPI) EcRecover(ctx context.Context, data hexutil.Bytes, sig hexutil.Bytes) (common.Address, error) { // Returns the address for the Account that was used to create the signature. // // Note, this function is compatible with eth_sign and personal_sign. As such it recovers @@ -562,74 +533,6 @@ func UnmarshalValidatorData(data interface{}) (ValidatorData, error) { }, nil } -// UnmarshalTypedData converts the bytes input to typed data -func UnmarshalTypedData(data interface{}) (TypedData, error) { - raw := data.(map[string]interface{}) - - var _types, ok = raw["types"].(EIP712Types) - if !ok || _types == nil { - return TypedData{}, errors.New("types are undefined") - } - if err := _types.IsValid(); err != nil { - return TypedData{}, err - } - - if _types["EIP712Domain"] == nil { - return TypedData{}, errors.New("domain types are undefined") - } - - domain, err := UnmarshalDomain(data) - if err != nil { - return TypedData{}, err - } - - primaryType, ok := raw["primaryType"].(string) - if !ok || len(primaryType) == 0 { - return TypedData{}, errors.New("primary type is undefined") - } - - message, ok := raw["message"].(EIP712Data) - if !ok || message == nil { - return TypedData{}, errors.New("message is undefined") - } - return TypedData{ - Types: _types, - PrimaryType: primaryType, - Domain: domain, - Message: message, - }, nil -} - -// UnmarshalDomain converts the bytes input to a domain -func UnmarshalDomain(data interface{}) (EIP712Domain, error) { - raw := data.(map[string]interface{})["domain"].(map[string]interface{}) - - chainId := raw["chainId"].(*big.Int) - if chainId == big.NewInt(0) { - return EIP712Domain{}, errors.New("chainId must be specified according to EIP-155") - } - - name, nameOk := raw["name"].(string) - version, versionOk := raw["version"].(string) - verifyingContract, verifyingContractOk := raw["verifyingContract"].(string) - salt, saltOk := raw["salt"].(string) - if (!nameOk || len(name) == 0) && - (!versionOk || len(version) == 0) && - (!verifyingContractOk || len(verifyingContract) == 0) && - (!saltOk || len(salt) == 0) { - return EIP712Domain{}, errors.New("domain is undefined") - } - - return EIP712Domain{ - Name: name, - Version: version, - ChainId: chainId, - VerifyingContract: verifyingContract, - Salt: salt, - }, nil -} - - // Map is a helper function to generate a map version of the typed data func (typedData *TypedData) Map() map[string]interface{} { dataMap := map[string]interface{}{ @@ -686,7 +589,7 @@ func isStandardTypeStr(encType string) bool { } // Dynamic types - for _, standardType := range []string { + for _, standardType := range []string{ TypeBytes, TypeInt, TypeUint, @@ -712,7 +615,7 @@ func (domain *EIP712Domain) IsValid() error { } if len(domain.Name) == 0 && len(domain.Version) == 0 && len(domain.VerifyingContract) == 0 && len(domain.Salt) == 0 { - return errors.New("domain undefined") + return errors.New("domain is undefined") } return nil @@ -740,4 +643,4 @@ func (domain *EIP712Domain) Map() map[string]interface{} { dataMap["salt"] = domain.Salt } return dataMap -} \ No newline at end of file +} diff --git a/signer/core/signed_data_test.go b/signer/core/signed_data_test.go index e1483b7aff..abb5a7ac73 100644 --- a/signer/core/signed_data_test.go +++ b/signer/core/signed_data_test.go @@ -19,11 +19,12 @@ package core import ( "context" "fmt" + "math/big" + "testing" + "github.com/ethereum/go-ethereum/accounts/keystore" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/hexutil" - "math/big" - "testing" ) var typesStandard = EIP712Types{ @@ -94,19 +95,12 @@ var messageStandard = map[string]interface{}{ } var typedData = TypedData{ - Types: typesStandard, + Types: typesStandard, PrimaryType: primaryType, - Domain: domainStandard, - Message: messageStandard, + Domain: domainStandard, + Message: messageStandard, } -//var typedDataMap = map[string]interface{}{ -// "types": typesStandard, -// "primaryType": primaryType, -// "domain": domainStandard, -// "message": messageStandard, -//} - func TestSignData(t *testing.T) { api, control := setup(t) //Create two accounts @@ -149,7 +143,7 @@ func TestSignData(t *testing.T) { // data/typed control <- "Y" control <- "a_long_password" - signature, err = api.SignData(context.Background(), DataTyped.Mime, a, typedData.Map()) + signature, err = api.SignTypedData(context.Background(), a, typedData) if err != nil { t.Fatal(err) }