From 4d41bfe037bbd5be9aa0b8582e5b50bd99cf3d1e Mon Sep 17 00:00:00 2001 From: pazams Date: Tue, 9 Jul 2019 18:03:53 -0700 Subject: [PATCH 1/6] Refactor errors --- dappauth.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/dappauth.go b/dappauth.go index e8cff2d..fd9b0ce 100644 --- a/dappauth.go +++ b/dappauth.go @@ -89,5 +89,12 @@ func personalMessageHash(message string) []byte { } func mergeErrors(errEOA error, errCA error) error { - return fmt.Errorf("Authorisation check failed and errored in 2 alternative flows. 'External Owned Account' check errored with: '%v'. 'Contract Account' check errored with: '%v'", errEOA, errCA) + var msgEOA string + if errEOA == nil { + msgEOA = "returned false" + } else { + msgEOA = fmt.Sprintf("errored with: '%v'", errEOA) + } + + return fmt.Errorf("Authorisation check failed and errored in 2 alternative flows. 'External Owned Account' check %s. 'Contract Account' check errored with: '%v'", msgEOA, errCA) } From 3d835f047bf8709ecce5ab188aa89beeb7be05fb Mon Sep 17 00:00:00 2001 From: pazams Date: Fri, 12 Jul 2019 14:50:37 -0700 Subject: [PATCH 2/6] Tidy go mod --- go.sum | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/go.sum b/go.sum index 19836eb..777c085 100644 --- a/go.sum +++ b/go.sum @@ -1,4 +1,5 @@ github.com/aead/siphash v1.0.1/go.mod h1:Nywa3cDsYNNK3gaciGTWPwHt0wlpNV15vwmswBAUSII= +github.com/allegro/bigcache v1.2.0 h1:qDaE0QoF29wKBb3+pXFrJFy1ihe5OT9OiXhg1t85SxM= github.com/allegro/bigcache v1.2.0/go.mod h1:Cb/ax3seSYIx7SuZdm2G2xzfwmv3TPSk2ucNfQESPXM= github.com/aristanetworks/goarista v0.0.0-20190325233358-a123909ec740 h1:FD4/ikKOFxwP8muWDypbmBWc634+YcAs3eBrYAmRdZY= github.com/aristanetworks/goarista v0.0.0-20190325233358-a123909ec740/go.mod h1:D/tb0zPVXnP7fmsLZjtdUhSsumbK/ij54UXjjVgMGxQ= @@ -17,9 +18,11 @@ github.com/davecgh/go-spew v0.0.0-20171005155431-ecdeabc65495/go.mod h1:J7Y8YcW2 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/deckarep/golang-set v1.7.1 h1:SCQV0S6gTtp6itiFrTqI+pfmJ4LN85S1YzhDf9rTHJQ= github.com/deckarep/golang-set v1.7.1/go.mod h1:93vsz/8Wt4joVM7c2AVqh+YRMiUSc14yDtF28KmMOgQ= github.com/edsrzf/mmap-go v1.0.0 h1:CEBF7HpRnUCSJgGUb5h1Gm7e3VkmVDrR8lvWVLtrOFw= github.com/edsrzf/mmap-go v1.0.0/go.mod h1:YO35OhQPt3KJa3ryjFM5Bs14WD66h8eGKpfaBNrHW5M= +github.com/ethereum/go-ethereum v1.8.23 h1:xVKYpRpe3cbkaWN8gsRgStsyTvz3s82PcQsbEofjhEQ= github.com/ethereum/go-ethereum v1.8.23/go.mod h1:PwpWDrCLZrV+tfrhqqF6kPknbISMHaJv9Ln3kPCZLwY= github.com/fjl/memsize v0.0.0-20180929194037-2a09253e352a h1:1znxn4+q2MrEdTk1eCk6KIV3muTYVclBIB6CTVR/zBc= github.com/fjl/memsize v0.0.0-20180929194037-2a09253e352a/go.mod h1:VvhXpOYNQvB+uIk2RvXzuaQtkQJzzIx6lSBe1xv7hi0= @@ -31,7 +34,9 @@ github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5y github.com/golang/protobuf v1.3.1 h1:YF8+flBXS5eO826T4nzqPrxfhQThhXl0YzfuUPu4SBg= github.com/golang/protobuf v1.3.1/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/golang/snappy v0.0.0-20180518054509-2e65f85255db/go.mod h1:/XxbfmMg8lxefKM7IXC3fBNl/7bRcc72aCRzEWrmP2Q= +github.com/golang/snappy v0.0.1 h1:Qgr9rKW7uDUkrbSmQeiDsGa8SjGyCOGtuasMWwvp2P4= github.com/golang/snappy v0.0.1/go.mod h1:/XxbfmMg8lxefKM7IXC3fBNl/7bRcc72aCRzEWrmP2Q= +github.com/google/uuid v1.0.0 h1:b4Gk+7WdP/d3HZH8EJsZpvV7EtDOgaZLtnaNGIu1adA= github.com/google/uuid v1.0.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/hashicorp/golang-lru v0.5.1 h1:0hERBMJE1eitiLkihrMvRVBYAkpHzc/J3QdDN+dAcgU= github.com/hashicorp/golang-lru v0.5.1/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ4Ao+sR/qLZy8= @@ -62,15 +67,18 @@ github.com/onsi/ginkgo v1.7.0 h1:WSHQ+IS43OoUrWtD1/bbclrwK8TTH5hzp+umCiuxHgs= github.com/onsi/ginkgo v1.7.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE= github.com/onsi/gomega v1.4.3 h1:RE1xgDvH7imwFD45h+u2SgIfERHlS2yNG4DObb5BSKU= github.com/onsi/gomega v1.4.3/go.mod h1:ex+gbHU/CVuBBDIJjb2X0qEXbFg53c61hWP/1CpauHY= +github.com/pborman/uuid v1.2.0 h1:J7Q5mO4ysT1dv8hyrUGHb9+ooztCXu1D8MY8DZYsu3g= github.com/pborman/uuid v1.2.0/go.mod h1:X/NO0urCmaxf9VXbdlT7C2Yzkj2IKimNn4k+gtPdI/k= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/rjeczalik/notify v0.9.2 h1:MiTWrPj55mNDHEiIX5YUSKefw/+lCQVoAFmD6oQm5w8= github.com/rjeczalik/notify v0.9.2/go.mod h1:aErll2f0sUX9PXZnVNyeiObbmTlk5jnMoCa4QEjJeqM= github.com/rs/cors v1.6.0 h1:G9tHG9lebljV9mfp9SNPDL36nCDxmo3zTlAf1YgvzmI= github.com/rs/cors v1.6.0/go.mod h1:gFx+x8UowdsKA9AchylcLynDq+nNFfI8FkUZdN/jGCU= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/testify v1.3.0 h1:TivCn/peBQ7UY8ooIcPgZFpTNSz0Q2U6UrFlUfqbe0Q= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= +github.com/syndtr/goleveldb v1.0.0 h1:fBdIW9lB4Iz0n9khmH8w27SJ3QEJ7+IgjPEwGSZiFdE= github.com/syndtr/goleveldb v1.0.0/go.mod h1:ZVVdQEZoIme9iO1Ch2Jdy24qqXrMMOU6lpPAyBWyWuQ= golang.org/x/crypto v0.0.0-20170930174604-9419663f5a44/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= From 6334a4147f435cc769d32f4f4b0c838a4362188a Mon Sep 17 00:00:00 2001 From: pazams Date: Fri, 12 Jul 2019 15:12:39 -0700 Subject: [PATCH 3/6] Update travis go version --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 4fb0e9a..4506f8e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,7 +1,7 @@ language: go sudo: false go: - - 1.7 + - 1.12 branches: only: - master From 79ac634c152fd330a96ddb0d28f2a5eb28b8ec0b Mon Sep 17 00:00:00 2001 From: pazams Date: Fri, 12 Jul 2019 15:38:10 -0700 Subject: [PATCH 4/6] Fix tests coverage --- dappauth_test.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/dappauth_test.go b/dappauth_test.go index 38bbc88..7b8355c 100644 --- a/dappauth_test.go +++ b/dappauth_test.go @@ -154,6 +154,22 @@ func TestDappAuth(t *testing.T) { }) } + // this test adds to test coverage + t.Run("Invalid signature should fail", func(t *testing.T) { + authenticator := NewAuthenticator(nil, &mockContract{ + address: [20]byte{}, + authorizedKey: nil, + errorIsValidSignature: false, + }) + + invalidSigBytes := [65]byte{} + invalidSig := hex.EncodeToString(invalidSigBytes[:]) + isAuthorizedSigner, err := authenticator.IsAuthorizedSigner("foo", invalidSig, ethCrypto.PubkeyToAddress(keyA.PublicKey).Hex()) + + expectBool(err != nil, true, t) + expectBool(isAuthorizedSigner, false, t) + }) + } func generateSignature(isEOA bool, msg string, key *ecdsa.PrivateKey, address common.Address, t *testing.T) string { From 66480de365357a8ca9ecda6486ed31e2ddc67e66 Mon Sep 17 00:00:00 2001 From: pazams Date: Tue, 17 Sep 2019 00:55:03 -0700 Subject: [PATCH 5/6] Add failing test --- dappauth_test.go | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/dappauth_test.go b/dappauth_test.go index 7b8355c..74dc1d9 100644 --- a/dappauth_test.go +++ b/dappauth_test.go @@ -3,6 +3,7 @@ package dappauth import ( "crypto/ecdsa" "encoding/hex" + "fmt" "testing" "github.com/ethereum/go-ethereum/common" @@ -172,6 +173,25 @@ func TestDappAuth(t *testing.T) { } +// It should decode challenge as utf8 by default when computing EOA personal messages hash +func TestPersonalMessageDecodeUTF8(t *testing.T) { + ethMsgHash := personalMessageHash("foo") + eoaHash := hex.EncodeToString(ethMsgHash) + + expectString(fmt.Sprintf("0x%s", eoaHash), "0x76b2e96714d3b5e6eb1d1c509265430b907b44f72b2a22b06fcd4d96372b8565", t) +} + +// It should decode challenge as hex if hex is detected when computing EOA personal messages hash +// See https://github.com/MetaMask/eth-sig-util/issues/60 +func TestPersonalMessageDecodeHex(t *testing.T) { + ethMsgHash := personalMessageHash("0xffff") + eoaHash := hex.EncodeToString(ethMsgHash) + + // result if 0xffff is decoded as hex: 13a6aa3102b2d639f36804a2d7c31469618fd7a7907c658a7b2aa91a06e31e47 + // result if 0xffff is decoded as utf8: 247aefb5d2e5b17fca61f786c779f7388485460c13e51308f88b2ff84ffa6851 + expectString(fmt.Sprintf("0x%s", eoaHash), "0x13a6aa3102b2d639f36804a2d7c31469618fd7a7907c658a7b2aa91a06e31e47", t) +} + func generateSignature(isEOA bool, msg string, key *ecdsa.PrivateKey, address common.Address, t *testing.T) string { if isEOA { return signEOAPersonalMessage(msg, key, t) @@ -212,3 +232,9 @@ func expectBool(actual, expected bool, t *testing.T) { t.Errorf("expected %v to be %v", actual, expected) } } + +func expectString(actual, expected string, t *testing.T) { + if actual != expected { + t.Errorf("expected %s to be %s", actual, expected) + } +} From 1bcf4db5aa2a77334df3bfa8c62c683ff111d7ed Mon Sep 17 00:00:00 2001 From: pazams Date: Tue, 17 Sep 2019 01:17:46 -0700 Subject: [PATCH 6/6] Change personal sign hashing to follow established practice --- dappauth.go | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/dappauth.go b/dappauth.go index fd9b0ce..1b3e1b4 100644 --- a/dappauth.go +++ b/dappauth.go @@ -5,7 +5,9 @@ package dappauth import ( "bytes" "context" + "encoding/hex" "fmt" + "strings" "github.com/dapperlabs/dappauth/ERCs" "github.com/ethereum/go-ethereum/accounts/abi/bind" @@ -83,9 +85,16 @@ func (a *Authenticator) IsAuthorizedSigner(challenge, signature, addrHex string) return magicValue == _ERC1271MagicValue, nil } +// See https://github.com/MetaMask/eth-sig-util/issues/60 func personalMessageHash(message string) []byte { - msg := fmt.Sprintf("\x19Ethereum Signed Message:\n%d%s", len(message), message) - return ethCrypto.Keccak256([]byte(msg)) + b, err := hex.DecodeString(strings.TrimPrefix(message, "0x")) + // if hex decode was successful, then treat is as a hex string + if err == nil { + msgToHash := fmt.Sprintf("\x19Ethereum Signed Message:\n%d", len(b)) + return ethCrypto.Keccak256(append([]byte(msgToHash), b...)) + } + msgToHash := fmt.Sprintf("\x19Ethereum Signed Message:\n%d%s", len(message), message) + return ethCrypto.Keccak256([]byte(msgToHash)) } func mergeErrors(errEOA error, errCA error) error {