From 67a22a2140ba51ee9cc2433d81f4e99fa02e4043 Mon Sep 17 00:00:00 2001 From: Manoranjith Date: Mon, 13 Dec 2021 20:49:40 +0530 Subject: [PATCH 01/12] :boom: [wallet] Change Sig to binary.(Un)marshaler - Previously Sig was defined as a variable length byte array and was encoded/decoded using the Encode/DecodeSig methods. - But, its length depends upon the wallet backend. So, redefine it as an interface that implements binary.(Un)marshaler interfaces. - Now, the wallet backend should implement the logic for converting a Sig to/from its binary representation. The perunio serializer can then directly Encode/Decode this binary representation as a byte array. Signed-off-by: Manoranjith --- wallet/account.go | 2 +- wallet/backend.go | 17 +++++++---------- wallet/sig.go | 34 ++++++++++++++++++++++++++-------- 3 files changed, 34 insertions(+), 19 deletions(-) diff --git a/wallet/account.go b/wallet/account.go index 2785bf55..58c0be75 100644 --- a/wallet/account.go +++ b/wallet/account.go @@ -21,5 +21,5 @@ type Account interface { // SignData requests a signature from this account. // It returns the signature or an error. - SignData(data []byte) ([]byte, error) + SignData(data []byte) (Sig, error) } diff --git a/wallet/backend.go b/wallet/backend.go index 43565fc7..70031ddf 100644 --- a/wallet/backend.go +++ b/wallet/backend.go @@ -14,10 +14,6 @@ package wallet -import ( - "io" -) - // backend is set to the global wallet backend. Must not be set directly but // through importing the needed backend. var backend Backend @@ -28,9 +24,9 @@ type Backend interface { // for unmarshalling an address from its binary representation. NewAddress() Address - // DecodeSig reads a signature from the provided stream. It is needed for - // decoding of wire messages. - DecodeSig(io.Reader) (Sig, error) + // NewSig returns a variable of type Sig, which can be used for + // unmarshalling a signature from its binary representation. + NewSig() Sig // VerifySignature verifies if this signature was signed by this address. // It should return an error iff the signature or message are malformed. @@ -53,9 +49,10 @@ func NewAddress() Address { return backend.NewAddress() } -// DecodeSig calls DecodeSig of the current backend. -func DecodeSig(r io.Reader) (Sig, error) { - return backend.DecodeSig(r) +// NewSig returns a variable of type Sig, which can be used for unmarshalling a +// signature from its binary representation. +func NewSig() Sig { + return backend.NewSig() } // VerifySignature calls VerifySignature of the current backend. diff --git a/wallet/sig.go b/wallet/sig.go index 98a0375e..44bc1b71 100644 --- a/wallet/sig.go +++ b/wallet/sig.go @@ -15,7 +15,7 @@ package wallet import ( - "bytes" + "encoding" "io" "math" @@ -24,8 +24,22 @@ import ( "perun.network/go-perun/wire/perunio" ) -// Sig is a single signature. -type Sig = []byte +// Sig represents a single signature generated over a given data using an +// account. It is usually a byte array and the length depends upon the specific +// signature scheme being used, which in turn depends upon the wallet backend. +// +// It is sent over wire as a binary data. +type Sig interface { + // BinaryMarshaler marshals the blockchain specific signature to binary + // format (a byte array). + encoding.BinaryMarshaler + // BinaryUnmarshaler unmarshals the blockchain specific signature from + // binary format (a byte array). + encoding.BinaryUnmarshaler + + // Clone returns a deep copy of the signature. + Clone() Sig +} // CloneSigs returns a deep copy of a slice of signatures. func CloneSigs(sigs []Sig) []Sig { @@ -35,7 +49,7 @@ func CloneSigs(sigs []Sig) []Sig { clonedSigs := make([]Sig, len(sigs)) for i, sig := range sigs { if sig != nil { - clonedSigs[i] = bytes.Repeat(sig, 1) + clonedSigs[i] = sig.Clone() } } return clonedSigs @@ -52,11 +66,13 @@ type SigDec struct { // Decode decodes a single signature. func (s SigDec) Decode(r io.Reader) (err error) { - *s.Sig, err = DecodeSig(r) + *s.Sig = NewSig() + err = perunio.Decode(r, *s.Sig) return err } -// EncodeSparseSigs encodes a collection of signatures in the form ( mask, sig, sig, sig, ...). +// EncodeSparseSigs encodes a collection of signatures in the form ( mask, sig, +// sig, sig, ...). func EncodeSparseSigs(w io.Writer, sigs []Sig) error { n := len(sigs) @@ -82,7 +98,8 @@ func EncodeSparseSigs(w io.Writer, sigs []Sig) error { return nil } -// DecodeSparseSigs decodes a collection of signatures in the form (mask, sig, sig, sig, ...). +// DecodeSparseSigs decodes a collection of signatures in the form (mask, sig, +// sig, sig, ...). func DecodeSparseSigs(r io.Reader, sigs *[]Sig) (err error) { masklen := int(math.Ceil(float64(len(*sigs)) / float64(bitsPerByte))) mask := make([]uint8, masklen) @@ -99,7 +116,8 @@ func DecodeSparseSigs(r io.Reader, sigs *[]Sig) (err error) { if ((mask[maskIdx] >> bitIdx) % binaryModulo) == 0 { (*sigs)[sigIdx] = nil } else { - (*sigs)[sigIdx], err = DecodeSig(r) + (*sigs)[sigIdx] = NewSig() + err = perunio.Decode(r, (*sigs)[sigIdx]) if err != nil { return errors.WithMessagef(err, "decoding signature %d", sigIdx) } From f475ad1b3ccabbe95a8b0e514e0e788effb94f53 Mon Sep 17 00:00:00 2001 From: Manoranjith Date: Mon, 13 Dec 2021 21:44:01 +0530 Subject: [PATCH 02/12] :recycle: [sim/wallet] Update sim wallet to implement Sig interface - Use a struct for storing the r and s components of the ECDSA signature. - This eliminates the need for serializing / deserializing the signature each time when it is generated / verified respectively. - Also, replace the test TestSignatureSerialize with GenericMarshalerTest Signed-off-by: Manoranjith --- backend/sim/channel/backend.go | 4 +- backend/sim/wallet/account.go | 37 +----------- backend/sim/wallet/backend.go | 27 ++++----- backend/sim/wallet/sig.go | 66 ++++++++++++++++++++++ backend/sim/wallet/wallet_internal_test.go | 33 +++-------- 5 files changed, 90 insertions(+), 77 deletions(-) create mode 100644 backend/sim/wallet/sig.go diff --git a/backend/sim/channel/backend.go b/backend/sim/channel/backend.go index 2ebfafa5..ddd3dcf7 100644 --- a/backend/sim/channel/backend.go +++ b/backend/sim/channel/backend.go @@ -54,7 +54,7 @@ func (*backend) CalcID(p *channel.Params) (id channel.ID) { } // Sign signs `state`. -func (b *backend) Sign(addr wallet.Account, state *channel.State) ([]byte, error) { +func (b *backend) Sign(addr wallet.Account, state *channel.State) (wallet.Sig, error) { log.WithFields(log.Fields{"channel": state.ID, "version": state.Version}).Tracef("Signing state") buff := new(bytes.Buffer) @@ -65,7 +65,7 @@ func (b *backend) Sign(addr wallet.Account, state *channel.State) ([]byte, error } // Verify verifies the signature for `state`. -func (b *backend) Verify(addr wallet.Address, state *channel.State, sig []byte) (bool, error) { +func (b *backend) Verify(addr wallet.Address, state *channel.State, sig wallet.Sig) (bool, error) { buff := new(bytes.Buffer) if err := state.Encode(buff); err != nil { return false, errors.WithMessage(err, "pack state") diff --git a/backend/sim/wallet/account.go b/backend/sim/wallet/account.go index 4dec6442..28d54628 100644 --- a/backend/sim/wallet/account.go +++ b/backend/sim/wallet/account.go @@ -18,7 +18,6 @@ import ( "crypto/ecdsa" "crypto/rand" "io" - "math/big" "github.com/pkg/errors" @@ -62,7 +61,7 @@ func (a *Account) Address() wallet.Address { // SignData is used to sign data with this account. If the account is locked, // returns an error instead of a signature. -func (a *Account) SignData(data []byte) ([]byte, error) { +func (a *Account) SignData(data []byte) (wallet.Sig, error) { if a.locked.IsSet() { return nil, errors.New("account locked") } @@ -70,37 +69,5 @@ func (a *Account) SignData(data []byte) ([]byte, error) { // escda.Sign needs a digest as input // ref https://golang.org/pkg/crypto/ecdsa/#Sign r, s, err := ecdsa.Sign(rand.Reader, a.privKey, digest(data)) - if err != nil { - return nil, errors.Wrap(err, "account could not sign data") - } - - return serializeSignature(r, s) -} - -// serializeSignature serializes a signature into a []byte or returns an error. -// The length of the []byte is dictated by the curves parameters and padded with 0 bytes if necessary. -func serializeSignature(r, s *big.Int) ([]byte, error) { - pointSize := curve.Params().BitSize / bitsPerByte - rBytes := append(make([]byte, pointSize-len(r.Bytes())), r.Bytes()...) - sBytes := append(make([]byte, pointSize-len(s.Bytes())), s.Bytes()...) - - return append(rBytes, sBytes...), nil -} - -// deserializeSignature deserializes a signature from a byteslice and returns `r` and `s` -// or an error. -func deserializeSignature(b []byte) (*big.Int, *big.Int, error) { - pointSize := curve.Params().BitSize / bitsPerByte - sigSize := pointsPerSig * pointSize - if len(b) != sigSize { - return nil, nil, errors.Errorf("expected %d bytes for a signature but got: %d", sigSize, len(b)) - } - - var r, s big.Int - rBytes := b[0:pointSize] - sBytes := b[pointSize:sigSize] - r.SetBytes(rBytes) - s.SetBytes(sBytes) - - return &r, &s, nil + return &Sig{r: r, s: s}, errors.Wrap(err, "account could not sign data") } diff --git a/backend/sim/wallet/backend.go b/backend/sim/wallet/backend.go index c26ce614..1938d024 100644 --- a/backend/sim/wallet/backend.go +++ b/backend/sim/wallet/backend.go @@ -18,13 +18,10 @@ import ( "crypto/ecdsa" "crypto/elliptic" "crypto/sha256" - "io" "github.com/pkg/errors" - "perun.network/go-perun/log" "perun.network/go-perun/wallet" - "perun.network/go-perun/wire/perunio" ) var curve = elliptic.P256() @@ -41,28 +38,28 @@ func (b *Backend) NewAddress() wallet.Address { return &addr } -// DecodeSig reads a []byte with length of a signature. -func (b *Backend) DecodeSig(r io.Reader) (wallet.Sig, error) { - buf := make(wallet.Sig, (curve.Params().BitSize/bitsPerByte)*pointsPerSig) - return buf, perunio.Decode(r, &buf) +// NewSig returns a variable of type Sig, which can be used for unmarshalling a +// signature from its binary representation. +func (*Backend) NewSig() wallet.Sig { + return &Sig{} } -// VerifySignature verifies if a signature was made by this account. +// VerifySignature verifies if the signature on the given message was made by +// this account. func (b *Backend) VerifySignature(msg []byte, sig wallet.Sig, a wallet.Address) (bool, error) { - addr, ok := a.(*Address) + ecdsaSig, ok := sig.(*Sig) if !ok { - log.Panic("Wrong address type passed to Backend.VerifySignature") + return false, errors.New("Wrong signature type passed to Backend.VerifySignature") } - pk := (*ecdsa.PublicKey)(addr) - r, s, err := deserializeSignature(sig) - if err != nil { - return false, errors.WithMessage(err, "could not deserialize signature") + addr, ok := a.(*Address) + if !ok { + return false, errors.New("Wrong address type passed to Backend.VerifySignature") } // escda.Verify needs a digest as input // ref https://golang.org/pkg/crypto/ecdsa/#Verify - return ecdsa.Verify(pk, digest(msg), r, s), nil + return ecdsa.Verify((*ecdsa.PublicKey)(addr), digest(msg), ecdsaSig.r, ecdsaSig.s), nil } func digest(msg []byte) []byte { diff --git a/backend/sim/wallet/sig.go b/backend/sim/wallet/sig.go new file mode 100644 index 00000000..11d61608 --- /dev/null +++ b/backend/sim/wallet/sig.go @@ -0,0 +1,66 @@ +// Copyright 2019 - See NOTICE file for copyright holders. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package wallet + +import ( + "fmt" + "math/big" + + "perun.network/go-perun/wallet" +) + +var ( + pointSize = curve.Params().BitSize / bitsPerByte + + // sigLen is the length of a signature in bytes. + sigLen = (curve.Params().BitSize / bitsPerByte) * pointsPerSig +) + +// Sig represents the signature generated by the sim wallet. +type Sig struct { + r, s *big.Int +} + +// MarshalBinary marshals the signature into its binary representation. Error +// will always be nil, it is for implementing BinaryMarshaler. +func (sig Sig) MarshalBinary() ([]byte, error) { + pointSize := curve.Params().BitSize / bitsPerByte + rBytes := append(make([]byte, pointSize-len(sig.r.Bytes())), sig.r.Bytes()...) + sBytes := append(make([]byte, pointSize-len(sig.s.Bytes())), sig.s.Bytes()...) + return append(rBytes, sBytes...), nil +} + +// UnmarshalBinary unmarshals the signature from its binary representation. +func (sig *Sig) UnmarshalBinary(data []byte) (err error) { + if len(data) != sigLen { + return fmt.Errorf("unexpected signature length %d, want %d", len(data), sigLen) //nolint: goerr113 + } + + rBytes := data[0:pointSize] + sBytes := data[pointSize:sigLen] + sig.r = new(big.Int).SetBytes(rBytes) + sig.s = new(big.Int).SetBytes(sBytes) + + return nil +} + +// Clone returns a deep copy of the signature. +func (sig Sig) Clone() wallet.Sig { + clone := Sig{ + r: new(big.Int).Set(sig.r), + s: new(big.Int).Set(sig.s), + } + return &clone +} diff --git a/backend/sim/wallet/wallet_internal_test.go b/backend/sim/wallet/wallet_internal_test.go index 48281baf..cfd188cb 100644 --- a/backend/sim/wallet/wallet_internal_test.go +++ b/backend/sim/wallet/wallet_internal_test.go @@ -24,36 +24,19 @@ import ( "perun.network/go-perun/wallet/test" "perun.network/go-perun/wire/perunio" + wiretest "perun.network/go-perun/wire/test" + pkgtest "polycry.pt/poly-go/test" ) -// TestSignatureSerialize tests serializeSignature and deserializeSignature since -// a signature is only a []byte, we cant use io.serializer here. -func TestSignatureSerialize(t *testing.T) { - a := assert.New(t) - // Constant seed for determinism +func Test_Sig_GenericMarshaler(t *testing.T) { rng := pkgtest.Prng(t) - - // More iterations are better for catching value dependent bugs for i := 0; i < 10; i++ { - rBytes := make([]byte, 32) - sBytes := make([]byte, 32) - - // These always return nil error - rng.Read(rBytes) - rng.Read(sBytes) - - r := new(big.Int).SetBytes(rBytes) - s := new(big.Int).SetBytes(sBytes) - - sig, err1 := serializeSignature(r, s) - a.Nil(err1, "Serialization should not fail") - a.Equal(curve.Params().BitSize/4, len(sig), "Signature has wrong size") - R, S, err2 := deserializeSignature(sig) - - a.Nil(err2, "Deserialization should not fail") - a.Equal(r, R, "Serialized and deserialized r values should be equal") - a.Equal(s, S, "Serialized and deserialized s values should be equal") + sig := Sig{ + r: big.NewInt(rng.Int63()), + s: big.NewInt(rng.Int63()), + } + wiretest.GenericMarshalerTest(t, &sig) } } From e93d17a66da7c8d51a221c6d1424aa83914cf0c3 Mon Sep 17 00:00:00 2001 From: Manoranjith Date: Tue, 14 Dec 2021 12:34:18 +0530 Subject: [PATCH 03/12] :recycle: [eth/wallet] Update eth wallet to implement Sig interface - Unexport the SigLen field, as it is not needed outside of the wallet package. - Remove the length check in signature tests. Because, VerifySignature function checks if the signature was generated by the corresponding wallet, before verifying it it. - In transactor tests, use the sigLen variable defined within the package. Signed-off-by: Manoranjith --- backend/ethereum/channel/test/transactor.go | 3 +- backend/ethereum/wallet/backend.go | 48 +++++++--------- backend/ethereum/wallet/hd/account.go | 5 +- backend/ethereum/wallet/keystore/account.go | 5 +- .../ethereum/wallet/keystore/wallet_test.go | 4 +- backend/ethereum/wallet/sig.go | 57 +++++++++++++++++++ backend/ethereum/wallet/simple/account.go | 5 +- backend/ethereum/wallet/simple/wallet_test.go | 1 - 8 files changed, 89 insertions(+), 39 deletions(-) create mode 100644 backend/ethereum/wallet/sig.go diff --git a/backend/ethereum/channel/test/transactor.go b/backend/ethereum/channel/test/transactor.go index 72089bb9..6a700614 100644 --- a/backend/ethereum/channel/test/transactor.go +++ b/backend/ethereum/channel/test/transactor.go @@ -27,7 +27,6 @@ import ( "github.com/stretchr/testify/require" "perun.network/go-perun/backend/ethereum/channel" - "perun.network/go-perun/backend/ethereum/wallet" ) // TransactorSetup holds the setup for running generic tests on a transactor implementation. @@ -90,7 +89,7 @@ func sigFromRSV(t *testing.T, r, s, _v *big.Int, chainID int64) []byte { sigVAdd = 35 sigVSubtract = 27 ) - sig := make([]byte, wallet.SigLen) + sig := make([]byte, sigLen) copy(sig[elemLen-len(r.Bytes()):elemLen], r.Bytes()) copy(sig[elemLen*2-len(s.Bytes()):elemLen*2], s.Bytes()) v := byte(_v.Uint64()) // Needed for chain ids > 110. diff --git a/backend/ethereum/wallet/backend.go b/backend/ethereum/wallet/backend.go index 10ec7272..825e6f3c 100644 --- a/backend/ethereum/wallet/backend.go +++ b/backend/ethereum/wallet/backend.go @@ -15,27 +15,15 @@ package wallet import ( - "io" - "github.com/ethereum/go-ethereum/crypto" "github.com/pkg/errors" "perun.network/go-perun/wallet" - "perun.network/go-perun/wire/perunio" ) // Backend implements the utility interface defined in the wallet package. type Backend struct{} -// SigLen length of a signature in byte. -// ref https://godoc.org/github.com/ethereum/go-ethereum/crypto/secp256k1#Sign -// ref https://github.com/ethereum/go-ethereum/blob/54b271a86dd748f3b0bcebeaf678dc34e0d6177a/crypto/signature_cgo.go#L66 -const SigLen = 65 - -// sigVSubtract value that is subtracted from the last byte of a signature if -// the last bytes exceeds it. -const sigVSubtract = 27 - // compile-time check that the ethereum backend implements the perun backend. var _ wallet.Backend = (*Backend)(nil) @@ -46,31 +34,35 @@ func (b *Backend) NewAddress() wallet.Address { return &addr } -// DecodeSig reads a []byte with length of an ethereum signature. -func (*Backend) DecodeSig(r io.Reader) (wallet.Sig, error) { - return DecodeSig(r) +// NewSig returns a variable of type Sig, which can be used for unmarshalling a +// signature from its binary representation. +func (*Backend) NewSig() wallet.Sig { + sig := Sig(make([]byte, sigLen)) + return &sig } -// VerifySignature verifies a signature. +// VerifySignature verifies if the signature on the given message was made by +// this account. func (*Backend) VerifySignature(msg []byte, sig wallet.Sig, a wallet.Address) (bool, error) { return VerifySignature(msg, sig, a) } -// DecodeSig reads a []byte with length of an ethereum signature. -func DecodeSig(r io.Reader) (wallet.Sig, error) { - buf := make(wallet.Sig, SigLen) - return buf, perunio.Decode(r, &buf) -} - -// VerifySignature verifies if a signature was made by this account. +// VerifySignature verifies if the signature on the given message was made by +// this account. func VerifySignature(msg []byte, sig wallet.Sig, a wallet.Address) (bool, error) { + ethSig, ok := sig.(*Sig) + if !ok { + return false, errors.New("signature was not of the expected type") + } + hash := PrefixedHash(msg) - sigCopy := make([]byte, SigLen) - copy(sigCopy, sig) - if len(sigCopy) == SigLen && (sigCopy[SigLen-1] >= sigVSubtract) { - sigCopy[SigLen-1] -= sigVSubtract + ethSigCopy := make([]byte, sigLen) + copy(ethSigCopy, *ethSig) + if ethSigCopy[sigLen-1] >= sigVSubtract { + ethSigCopy[sigLen-1] -= sigVSubtract } - pk, err := crypto.SigToPub(hash, sigCopy) + + pk, err := crypto.SigToPub(hash, ethSigCopy) if err != nil { return false, errors.WithStack(err) } diff --git a/backend/ethereum/wallet/hd/account.go b/backend/ethereum/wallet/hd/account.go index dd88e9d2..ad0b2fc6 100644 --- a/backend/ethereum/wallet/hd/account.go +++ b/backend/ethereum/wallet/hd/account.go @@ -35,14 +35,15 @@ func (a *Account) Address() wallet.Address { } // SignData is used to sign data with this account. -func (a *Account) SignData(data []byte) ([]byte, error) { +func (a *Account) SignData(data []byte) (wallet.Sig, error) { hash := crypto.Keccak256(data) sig, err := a.wallet.SignText(a.Account, hash) if err != nil { return nil, errors.Wrap(err, "SignText") } sig[64] += 27 - return sig, nil + ethSig := (ethwallet.Sig)(sig) + return ðSig, nil } // NewAccountFromEth creates a new perun account from a given ethereum account. diff --git a/backend/ethereum/wallet/keystore/account.go b/backend/ethereum/wallet/keystore/account.go index 6126c4d6..e4c8aca5 100644 --- a/backend/ethereum/wallet/keystore/account.go +++ b/backend/ethereum/wallet/keystore/account.go @@ -34,14 +34,15 @@ func (a *Account) Address() wallet.Address { } // SignData is used to sign data with this account. -func (a *Account) SignData(data []byte) ([]byte, error) { +func (a *Account) SignData(data []byte) (wallet.Sig, error) { hash := ethwallet.PrefixedHash(data) sig, err := a.wallet.Ks.SignHash(a.Account, hash) if err != nil { return nil, errors.Wrap(err, "SignHash") } sig[64] += 27 - return sig, nil + ethSig := (ethwallet.Sig)(sig) + return ðSig, nil } // NewAccountFromEth creates a new perun account from a given ethereum account. diff --git a/backend/ethereum/wallet/keystore/wallet_test.go b/backend/ethereum/wallet/keystore/wallet_test.go index 0f0c7558..d2638b5a 100644 --- a/backend/ethereum/wallet/keystore/wallet_test.go +++ b/backend/ethereum/wallet/keystore/wallet_test.go @@ -62,7 +62,6 @@ func TestSignatures(t *testing.T) { acc := ethwallettest.NewTmpWallet().NewAccount() sign, err := acc.SignData(dataToSign) assert.NoError(t, err, "Sign with new account should succeed") - assert.Equal(t, len(sign), ethwallet.SigLen, "Ethereum signature has wrong length") valid, err := new(ethwallet.Backend).VerifySignature(dataToSign, sign, acc.Address()) assert.True(t, valid, "Verification should succeed") assert.NoError(t, err, "Verification should succeed") @@ -113,7 +112,8 @@ func TestCurve_SigningAndVerifying(t *testing.T) { sig, err := hex.DecodeString("538da6430f7915832de165f89c69239020461b80861559a00d4f5a2a7705765219eb3969eb7095f8addb6bf9c9f96f6adf44cfd4a8136516f88b337a428bf1bb1b") require.NoError(t, err, "decode sig should not error") addr := ethwallet.Address(common.HexToAddress("f17f52151EbEF6C7334FAD080c5704D77216b732")) - b, err := ethwallet.VerifySignature(msg, sig, &addr) + ethSig := ethwallet.Sig(sig) + b, err := ethwallet.VerifySignature(msg, ðSig, &addr) assert.NoError(t, err, "VerifySignature should not error") assert.True(t, b, "VerifySignature") } diff --git a/backend/ethereum/wallet/sig.go b/backend/ethereum/wallet/sig.go new file mode 100644 index 00000000..10c6e731 --- /dev/null +++ b/backend/ethereum/wallet/sig.go @@ -0,0 +1,57 @@ +// Copyright 2019 - See NOTICE file for copyright holders. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package wallet + +import ( + "fmt" + + "perun.network/go-perun/wallet" +) + +const ( + // sigLen length of a signature in byte. + // ref https://godoc.org/github.com/ethereum/go-ethereum/crypto/secp256k1#Sign + // ref https://github.com/ethereum/go-ethereum/blob/54b271a86dd748f3b0bcebeaf678dc34e0d6177a/crypto/signature_cgo.go#L66 + sigLen = 65 + + // sigVSubtract value that is subtracted from the last byte of a signature if + // the last bytes exceeds it. + sigVSubtract = 27 +) + +// Sig represents a signature generated using an ethereum account. +type Sig []byte + +// MarshalBinary marshals the signature into its binary representation. Error +// will always be nil, it is for implementing BinaryMarshaler. +func (s Sig) MarshalBinary() ([]byte, error) { + return s[:], nil +} + +// UnmarshalBinary unmarshals the signature from its binary representation. +func (s *Sig) UnmarshalBinary(data []byte) error { + if len(data) != sigLen { + return fmt.Errorf("unexpected signature length %d, want %d", len(data), sigLen) //nolint: goerr113 + } + copy(*s, data) + return nil +} + +// Clone returns a deep copy of the signature. +func (s Sig) Clone() wallet.Sig { + clone := Sig(make([]byte, sigLen)) + copy(clone, s) + return &clone +} diff --git a/backend/ethereum/wallet/simple/account.go b/backend/ethereum/wallet/simple/account.go index afeda15a..84abd6c9 100644 --- a/backend/ethereum/wallet/simple/account.go +++ b/backend/ethereum/wallet/simple/account.go @@ -37,14 +37,15 @@ func (a *Account) Address() wallet.Address { } // SignData is used to sign data with this account. -func (a *Account) SignData(data []byte) ([]byte, error) { +func (a *Account) SignData(data []byte) (wallet.Sig, error) { hash := ethwallet.PrefixedHash(data) sig, err := a.SignHash(hash) if err != nil { return nil, errors.Wrap(err, "SignHash") } sig[64] += 27 - return sig, nil + ethSig := (ethwallet.Sig)(sig) + return ðSig, nil } // SignHash is used to sign an already prefixed hash with this account. diff --git a/backend/ethereum/wallet/simple/wallet_test.go b/backend/ethereum/wallet/simple/wallet_test.go index d41f083e..72d66a05 100644 --- a/backend/ethereum/wallet/simple/wallet_test.go +++ b/backend/ethereum/wallet/simple/wallet_test.go @@ -78,7 +78,6 @@ func TestSignatures(t *testing.T) { sig, err := acc.SignData(dataToSign) assert.NoError(t, err, "Sign with new account should succeed") assert.NotNil(t, sig) - assert.Equal(t, len(sig), ethwallet.SigLen, "Ethereum signature has wrong length") valid, err := new(ethwallet.Backend).VerifySignature(dataToSign, sig, acc.Address()) assert.True(t, valid, "Verification should succeed") assert.NoError(t, err, "Verification should succeed") From 817acbd5ea98348e923db3025dedffacb4a92d3c Mon Sep 17 00:00:00 2001 From: Manoranjith Date: Tue, 14 Dec 2021 12:51:15 +0530 Subject: [PATCH 04/12] :recycle: [eth/channel] Update eth/channel to use the Sig interface Signed-off-by: Manoranjith --- backend/ethereum/channel/adjudicator.go | 35 ++++++++++++++++++-- backend/ethereum/channel/adjudicator_test.go | 3 +- backend/ethereum/channel/subscription.go | 2 +- backend/ethereum/channel/withdraw.go | 7 +++- 4 files changed, 41 insertions(+), 6 deletions(-) diff --git a/backend/ethereum/channel/adjudicator.go b/backend/ethereum/channel/adjudicator.go index 2dfdd7f7..cde8604d 100644 --- a/backend/ethereum/channel/adjudicator.go +++ b/backend/ethereum/channel/adjudicator.go @@ -27,9 +27,11 @@ import ( "perun.network/go-perun/backend/ethereum/bindings" "perun.network/go-perun/backend/ethereum/bindings/adjudicator" cherrors "perun.network/go-perun/backend/ethereum/channel/errors" + ethwallet "perun.network/go-perun/backend/ethereum/wallet" "perun.network/go-perun/channel" "perun.network/go-perun/client" "perun.network/go-perun/log" + "perun.network/go-perun/wallet" psync "polycry.pt/poly-go/sync" ) @@ -81,7 +83,11 @@ func (a *Adjudicator) Progress(ctx context.Context, req channel.ProgressReq) err state adjudicator.ChannelState, _ [][]byte, ) (*types.Transaction, error) { - return a.contract.Progress(opts, params, state, ethNewState, ethActorIndex, req.Sig) + sigBytes, err := req.Sig.MarshalBinary() + if err != nil { + return nil, errors.WithMessage(err, "converting to byte array") + } + return a.contract.Progress(opts, params, state, ethNewState, ethActorIndex, sigBytes) } return a.call(ctx, req.AdjudicatorReq, progress, Progress) } @@ -105,12 +111,35 @@ func toEthSignedStates(subChannels []channel.SignedState) (ethSubChannels []adju ethSubChannels[i] = adjudicator.AdjudicatorSignedState{ Params: ToEthParams(x.Params), State: ToEthState(x.State), - Sigs: x.Sigs, + Sigs: sigsToByteArrays(x.Sigs), } } return } +func sigsToByteArrays(signs []wallet.Sig) [][]byte { + byteArrays := make([][]byte, len(signs)) + var err error + for i := range signs { + if signs[i] != nil { + byteArrays[i], err = signs[i].MarshalBinary() + if err != nil { + panic("Marshalling to Binary should not error: " + err.Error()) + } + } + } + return byteArrays +} + +func byteArraysToSigs(byteArrays [][]byte) []wallet.Sig { + sigs := make([]wallet.Sig, len(byteArrays)) + for i := range byteArrays { + sig := (ethwallet.Sig)(byteArrays[i]) + sigs[i] = &sig + } + return sigs +} + func (a *Adjudicator) callConclude(ctx context.Context, req channel.AdjudicatorReq, subStates channel.StateMap) error { ethSubStates := toEthSubStates(req.Tx.State, subStates) @@ -153,7 +182,7 @@ func (a *Adjudicator) call(ctx context.Context, req channel.AdjudicatorReq, fn a if err != nil { return nil, errors.WithMessage(err, "creating transactor") } - tx, err := fn(trans, ethParams, ethState, req.Tx.Sigs) + tx, err := fn(trans, ethParams, ethState, sigsToByteArrays(req.Tx.Sigs)) if err != nil { err = cherrors.CheckIsChainNotReachableError(err) return nil, errors.WithMessage(err, "calling adjudicator function") diff --git a/backend/ethereum/channel/adjudicator_test.go b/backend/ethereum/channel/adjudicator_test.go index da8dec7a..c4e6eaa3 100644 --- a/backend/ethereum/channel/adjudicator_test.go +++ b/backend/ethereum/channel/adjudicator_test.go @@ -30,6 +30,7 @@ import ( ethwallettest "perun.network/go-perun/backend/ethereum/wallet/test" "perun.network/go-perun/channel" channeltest "perun.network/go-perun/channel/test" + "perun.network/go-perun/wallet" pkgtest "polycry.pt/poly-go/test" ) @@ -43,7 +44,7 @@ func testSignState(t *testing.T, accounts []*keystore.Account, state *channel.St } func signState(accounts []*keystore.Account, state *channel.State) (channel.Transaction, error) { - sigs := make([][]byte, len(accounts)) + sigs := make([]wallet.Sig, len(accounts)) for i := range accounts { sig, err := channel.Sign(accounts[i], state) if err != nil { diff --git a/backend/ethereum/channel/subscription.go b/backend/ethereum/channel/subscription.go index 066592e8..a2f828f5 100644 --- a/backend/ethereum/channel/subscription.go +++ b/backend/ethereum/channel/subscription.go @@ -193,7 +193,7 @@ func (a *Adjudicator) convertEvent(ctx context.Context, e *adjudicator.Adjudicat return &channel.RegisteredEvent{ AdjudicatorEventBase: *base, State: &state, - Sigs: ch.Sigs, + Sigs: byteArraysToSigs(ch.Sigs), }, nil case phaseForceExec: diff --git a/backend/ethereum/channel/withdraw.go b/backend/ethereum/channel/withdraw.go index 59719909..291523e2 100644 --- a/backend/ethereum/channel/withdraw.go +++ b/backend/ethereum/channel/withdraw.go @@ -168,7 +168,12 @@ func (a *Adjudicator) newWithdrawalAuth(request channel.AdjudicatorReq, asset as } sig, err := request.Acc.SignData(enc) - return auth, sig, errors.WithMessage(err, "sign data") + if err != nil { + return assetholder.AssetHolderWithdrawalAuth{}, nil, errors.WithMessage(err, "sign data") + } + sigBytes, err := sig.MarshalBinary() + + return auth, sigBytes, errors.WithMessage(err, "marshalling signature into bytes") } func encodeAssetHolderWithdrawalAuth(auth assetholder.AssetHolderWithdrawalAuth) ([]byte, error) { From 0dfaf41990d1d081cb519fe37a3413635e3550b9 Mon Sep 17 00:00:00 2001 From: Manoranjith Date: Tue, 14 Dec 2021 12:52:23 +0530 Subject: [PATCH 05/12] :recycle: [client] Update client pkg to use the Sig interface - (En|De)code signatures directly using perunio.(En|De)code. - SignData method on dummy account returns a Sig instead of byte array. Signed-off-by: Manoranjith --- client/updatemsgs.go | 8 ++++---- client/virtual_channel.go | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/client/updatemsgs.go b/client/updatemsgs.go index 716435b2..71141888 100644 --- a/client/updatemsgs.go +++ b/client/updatemsgs.go @@ -142,8 +142,8 @@ func (c *msgChannelUpdate) Decode(r io.Reader) (err error) { if err := perunio.Decode(r, c.State, &c.ActorIdx); err != nil { return err } - c.Sig, err = wallet.DecodeSig(r) - return err + c.Sig = wallet.NewSig() + return perunio.Decode(r, c.Sig) } func (c msgChannelUpdateAcc) Encode(w io.Writer) error { @@ -154,8 +154,8 @@ func (c *msgChannelUpdateAcc) Decode(r io.Reader) (err error) { if err := perunio.Decode(r, &c.ChannelID, &c.Version); err != nil { return err } - c.Sig, err = wallet.DecodeSig(r) - return err + c.Sig = wallet.NewSig() + return perunio.Decode(r, c.Sig) } func (c msgChannelUpdateRej) Encode(w io.Writer) error { diff --git a/client/virtual_channel.go b/client/virtual_channel.go index 3ecf3bfc..9fbddc18 100644 --- a/client/virtual_channel.go +++ b/client/virtual_channel.go @@ -157,7 +157,7 @@ func (a *dummyAccount) Address() wallet.Address { return a.address } -func (a *dummyAccount) SignData([]byte) ([]byte, error) { +func (a *dummyAccount) SignData([]byte) (wallet.Sig, error) { panic("dummy") } From cae91e21a03faec9097aafffcf81507316c69e03 Mon Sep 17 00:00:00 2001 From: Manoranjith Date: Tue, 14 Dec 2021 15:28:11 +0530 Subject: [PATCH 06/12] :white_check_mark: [wallet/test] Update tests to work with Sig interface - After the wallet.Sig interface was introduced, each wallet backend defines its own concrete type for implementing the Sig interface. - These implementations also include an UnmarshalBinary function, which is used for unmarshalling signatures from their binary representation. - Because of this, when binary representations of signatures have incorrect length (when they are tampered), the unmarshalling of these signatures themselves fail. Which removes the need for checking the response of VerifySignature function for signatures of incorrect length. - Hence, the tests are updated accordingly. Signed-off-by: Manoranjith --- wallet/test/wallet.go | 48 +++++++++++++++++++++++++++---------------- 1 file changed, 30 insertions(+), 18 deletions(-) diff --git a/wallet/test/wallet.go b/wallet/test/wallet.go index b465aa91..71ff6652 100644 --- a/wallet/test/wallet.go +++ b/wallet/test/wallet.go @@ -60,27 +60,23 @@ func TestAccountWithWalletAndBackend(t *testing.T, s *Setup) { //nolint:revive / assert.False(t, valid, "Verification with wrong address should fail") assert.NoError(t, err, "Verification of valid signature should not produce error") - tampered := make([]byte, len(sig)) - copy(tampered, sig) // Invalidate the signature and check for error - tampered[0] = ^sig[0] + tamperedBytes := marshalAndTamper(t, sig, func(sigBytes *[]byte) { (*sigBytes)[0] = ^(*sigBytes)[0] }) + tampered := s.Backend.NewSig() + require.NoError(t, tampered.UnmarshalBinary(tamperedBytes)) valid, err = s.Backend.VerifySignature(s.DataToSign, tampered, acc.Address()) if valid && err == nil { t.Error("Verification of invalid signature should produce error or return false") } // Truncate the signature and check for error - tampered = sig[:len(sig)-1] - valid, err = s.Backend.VerifySignature(s.DataToSign, tampered, acc.Address()) - if valid && err != nil { - t.Error("Verification of invalid signature should produce error or return false") - } + tamperedBytes = marshalAndTamper(t, sig, func(sigBytes *[]byte) { *sigBytes = (*sigBytes)[:len(*sigBytes)-1] }) + tampered = s.Backend.NewSig() + require.Error(t, tampered.UnmarshalBinary(tamperedBytes), "unmarshalling should fail when length is incorrect") + // Expand the signature and check for error - // nolint:gocritic - tampered = append(sig, 0) - valid, err = s.Backend.VerifySignature(s.DataToSign, tampered, acc.Address()) - if valid && err != nil { - t.Error("Verification of invalid signature should produce error or return false") - } + tamperedBytes = marshalAndTamper(t, sig, func(sigBytes *[]byte) { *sigBytes = append(*sigBytes, 0) }) + tampered = s.Backend.NewSig() + require.Error(t, tampered.UnmarshalBinary(tamperedBytes), "unmarshalling should fail when length is incorrect") // Test DecodeSig sig, err = acc.SignData(s.DataToSign) @@ -89,7 +85,8 @@ func TestAccountWithWalletAndBackend(t *testing.T, s *Setup) { //nolint:revive / buff := new(bytes.Buffer) err = perunio.Encode(buff, sig) require.NoError(t, err, "encode sig") - sign2, err := s.Backend.DecodeSig(buff) + sign2 := s.Backend.NewSig() + err = perunio.Decode(buff, sign2) assert.NoError(t, err, "Decoded signature should work") assert.Equal(t, sig, sign2, "Decoded signature should be equal to the original") @@ -97,10 +94,20 @@ func TestAccountWithWalletAndBackend(t *testing.T, s *Setup) { //nolint:revive / err = perunio.Encode(buff, sig) require.NoError(t, err, "encode sig") shortBuff := bytes.NewBuffer(buff.Bytes()[:len(buff.Bytes())-1]) // remove one byte - _, err = s.Backend.DecodeSig(shortBuff) + sign3 := s.Backend.NewSig() + err = perunio.Decode(shortBuff, sign3) assert.Error(t, err, "DecodeSig on short stream should error") } +func marshalAndTamper(t *testing.T, sig wallet.Sig, tamperer func(sigBytes *[]byte)) []byte { + t.Helper() + + sigBytes, err := sig.MarshalBinary() + require.NoError(t, err) + tamperer(&sigBytes) + return sigBytes +} + // GenericSignatureSizeTest tests that the size of the signatures produced by // Account.Sign(…) does not vary between executions (tested with 2048 samples). func GenericSignatureSizeTest(t *testing.T, s *Setup) { @@ -111,15 +118,20 @@ func GenericSignatureSizeTest(t *testing.T, s *Setup) { sign, err := acc.SignData(s.DataToSign) require.NoError(t, err, "Sign with unlocked account should succeed") + signRaw, err := sign.MarshalBinary() + require.NoError(t, err) + // Test that signatures have constant length - l := len(sign) + l := len(signRaw) for i := 0; i < 8; i++ { t.Run("parallel signing", func(t *testing.T) { t.Parallel() for i := 0; i < 256; i++ { sign, err := acc.SignData(s.DataToSign) require.NoError(t, err, "Sign with unlocked account should succeed") - require.Equal(t, l, len(sign), "Signatures should have constant length: %d vs %d", l, len(sign)) + signRaw, err := sign.MarshalBinary() + require.NoError(t, err) + require.Equal(t, l, len(signRaw), "Signatures should have constant length: %d vs %d", l, len(signRaw)) } }) } From bf56d1c936905659c2194ade269f67542a4937fb Mon Sep 17 00:00:00 2001 From: Manoranjith Date: Tue, 14 Dec 2021 16:00:03 +0530 Subject: [PATCH 07/12] :recycle: [persistence/test] Use Sig.Clone for cloning a signature Signed-off-by: Manoranjith --- channel/persistence/test/persistrestorer.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/channel/persistence/test/persistrestorer.go b/channel/persistence/test/persistrestorer.go index 9160af5e..c9146670 100644 --- a/channel/persistence/test/persistrestorer.go +++ b/channel/persistence/test/persistrestorer.go @@ -17,7 +17,6 @@ package test // import "perun.network/go-perun/channel/persistence/test" import ( - "bytes" "context" "sync" "testing" @@ -108,7 +107,7 @@ func (pr *PersistRestorer) SigAdded(_ context.Context, s channel.Source, idx cha return errors.Errorf("no staging transaction set") } - ch.StagingTXV.Sigs[idx] = bytes.Repeat(s.StagingTX().Sigs[idx], 1) + ch.StagingTXV.Sigs[idx] = s.StagingTX().Sigs[idx].Clone() return nil } From be3a590522722fdf2272e550d69d3af4c414106f Mon Sep 17 00:00:00 2001 From: Manoranjith Date: Tue, 14 Dec 2021 16:00:39 +0530 Subject: [PATCH 08/12] :white_check_mark: [persistence/test] Use randomizer to generate signatures - Previously, signatures used in tests were directly generated as byte arrays. - After the changing type of wallet.Sig from byte array to interface, this does not work. - Hence, use the wallet randomizer to generate signatures needed for tests. Signed-off-by: Manoranjith --- .../persistence/test/channel_internal_test.go | 41 ++++++++++--------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/channel/persistence/test/channel_internal_test.go b/channel/persistence/test/channel_internal_test.go index 56a10e34..b0caf4e8 100644 --- a/channel/persistence/test/channel_internal_test.go +++ b/channel/persistence/test/channel_internal_test.go @@ -15,22 +15,33 @@ package test import ( - "math/rand" "testing" + "github.com/stretchr/testify/require" "perun.network/go-perun/wallet" - "polycry.pt/poly-go/test" + "perun.network/go-perun/wallet/test" + pkgtest "polycry.pt/poly-go/test" ) func TestRequireEqualSigsTX(t *testing.T) { - prng := test.Prng(t) + prng := pkgtest.Prng(t) + acc := test.NewRandomAccount(prng) + data1 := make([]byte, prng.Int63n(50)) + prng.Read(data1) + data2 := make([]byte, prng.Int63n(50)) + prng.Read(data2) + + sig1, err := acc.SignData(data1) + require.NoError(t, err) + sig2, err := acc.SignData(data2) + require.NoError(t, err) + equalSigsTableNegative := []struct { s1 []wallet.Sig s2 []wallet.Sig }{ - {initSigSlice(10, prng), make([]wallet.Sig, 10)}, - {make([]wallet.Sig, 10), initSigSlice(10, prng)}, - {[]wallet.Sig{{4, 3, 2, 1}}, []wallet.Sig{{1, 2, 3, 4}}}, + {[]wallet.Sig{sig1, sig2}, make([]wallet.Sig, 2)}, + {make([]wallet.Sig, 2), []wallet.Sig{sig1, sig2}}, {make([]wallet.Sig, 5), make([]wallet.Sig, 10)}, {make([]wallet.Sig, 10), make([]wallet.Sig, 5)}, } @@ -42,26 +53,16 @@ func TestRequireEqualSigsTX(t *testing.T) { {nil, make([]wallet.Sig, 10)}, {make([]wallet.Sig, 10), nil}, {make([]wallet.Sig, 10), make([]wallet.Sig, 10)}, - {[]wallet.Sig{{1, 2, 3, 4}}, []wallet.Sig{{1, 2, 3, 4}}}, + {[]wallet.Sig{sig1, sig2}, []wallet.Sig{sig1, sig2}}, + {[]wallet.Sig{sig2}, []wallet.Sig{sig2}}, } - tt := test.NewTester(t) + tt := pkgtest.NewTester(t) for _, _c := range equalSigsTableNegative { c := _c - tt.AssertFatal(func(t test.T) { requireEqualSigs(t, c.s1, c.s2) }) + tt.AssertFatal(func(t pkgtest.T) { requireEqualSigs(t, c.s1, c.s2) }) } for _, c := range equalSigsTablePositive { requireEqualSigs(t, c.s1, c.s2) } } - -func initSigSlice(length int, prng *rand.Rand) []wallet.Sig { - s := make([]wallet.Sig, length) - for i := range s { - s[i] = make(wallet.Sig, 32) - for j := 0; j < 32; j++ { - s[i][j] = byte(prng.Int()) - } - } - return s -} From 5eb91dfad02bb95b7013155f6599fe86ca127e69 Mon Sep 17 00:00:00 2001 From: Manoranjith Date: Tue, 14 Dec 2021 16:42:24 +0530 Subject: [PATCH 09/12] :boom: [wallet] Move VerifySignature from Backend to Sig - Previously, VerifySignature function is defined on the wallet backend. - This looked fine, because until recently, wallet.Sig was defined as a byte array. - But, now it has been redefined as an interface with each wallet implementation implementing the wallet.Sig interface. - Hence, moved the VerifySignature method from wallet.Backend to wallet.Sig interface. Signed-off-by: Manoranjith --- wallet/backend.go | 10 ---------- wallet/sig.go | 6 ++++++ 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/wallet/backend.go b/wallet/backend.go index 70031ddf..e6fc35a7 100644 --- a/wallet/backend.go +++ b/wallet/backend.go @@ -27,11 +27,6 @@ type Backend interface { // NewSig returns a variable of type Sig, which can be used for // unmarshalling a signature from its binary representation. NewSig() Sig - - // VerifySignature verifies if this signature was signed by this address. - // It should return an error iff the signature or message are malformed. - // If the signature does not match the address it should return false, nil - VerifySignature(msg []byte, sign Sig, a Address) (bool, error) } // SetBackend sets the global wallet backend. Must not be called directly but @@ -54,8 +49,3 @@ func NewAddress() Address { func NewSig() Sig { return backend.NewSig() } - -// VerifySignature calls VerifySignature of the current backend. -func VerifySignature(msg []byte, sign Sig, a Address) (bool, error) { - return backend.VerifySignature(msg, sign, a) -} diff --git a/wallet/sig.go b/wallet/sig.go index 44bc1b71..c8c67b87 100644 --- a/wallet/sig.go +++ b/wallet/sig.go @@ -39,6 +39,12 @@ type Sig interface { // Clone returns a deep copy of the signature. Clone() Sig + + // Verify verifies if the signature was generated by the given address on + // the given data. + // It should return an error iff the signature or message are malformed. + // If the signature does not match the address it should return false, nil + Verify(msg []byte, a Address) (bool, error) } // CloneSigs returns a deep copy of a slice of signatures. From db4d98d3c716c60783c17ec7a90a8be6a0cdc50c Mon Sep 17 00:00:00 2001 From: Manoranjith Date: Tue, 14 Dec 2021 16:45:34 +0530 Subject: [PATCH 10/12] :boom: [sim/wallet] Move VerifySignature from Backend to Sig Signed-off-by: Manoranjith --- backend/sim/channel/backend.go | 2 +- backend/sim/wallet/backend.go | 27 --------------------------- backend/sim/wallet/sig.go | 21 +++++++++++++++++++++ 3 files changed, 22 insertions(+), 28 deletions(-) diff --git a/backend/sim/channel/backend.go b/backend/sim/channel/backend.go index ddd3dcf7..c1465ab1 100644 --- a/backend/sim/channel/backend.go +++ b/backend/sim/channel/backend.go @@ -70,7 +70,7 @@ func (b *backend) Verify(addr wallet.Address, state *channel.State, sig wallet.S if err := state.Encode(buff); err != nil { return false, errors.WithMessage(err, "pack state") } - return wallet.VerifySignature(buff.Bytes(), sig, addr) + return sig.Verify(buff.Bytes(), addr) } // NewAsset returns a variable of type Asset, which can be used diff --git a/backend/sim/wallet/backend.go b/backend/sim/wallet/backend.go index 1938d024..30c864cc 100644 --- a/backend/sim/wallet/backend.go +++ b/backend/sim/wallet/backend.go @@ -15,11 +15,7 @@ package wallet import ( - "crypto/ecdsa" "crypto/elliptic" - "crypto/sha256" - - "github.com/pkg/errors" "perun.network/go-perun/wallet" ) @@ -43,26 +39,3 @@ func (b *Backend) NewAddress() wallet.Address { func (*Backend) NewSig() wallet.Sig { return &Sig{} } - -// VerifySignature verifies if the signature on the given message was made by -// this account. -func (b *Backend) VerifySignature(msg []byte, sig wallet.Sig, a wallet.Address) (bool, error) { - ecdsaSig, ok := sig.(*Sig) - if !ok { - return false, errors.New("Wrong signature type passed to Backend.VerifySignature") - } - - addr, ok := a.(*Address) - if !ok { - return false, errors.New("Wrong address type passed to Backend.VerifySignature") - } - - // escda.Verify needs a digest as input - // ref https://golang.org/pkg/crypto/ecdsa/#Verify - return ecdsa.Verify((*ecdsa.PublicKey)(addr), digest(msg), ecdsaSig.r, ecdsaSig.s), nil -} - -func digest(msg []byte) []byte { - digest := sha256.Sum256(msg) - return digest[:] -} diff --git a/backend/sim/wallet/sig.go b/backend/sim/wallet/sig.go index 11d61608..cead4288 100644 --- a/backend/sim/wallet/sig.go +++ b/backend/sim/wallet/sig.go @@ -15,9 +15,12 @@ package wallet import ( + "crypto/ecdsa" + "crypto/sha256" "fmt" "math/big" + "github.com/pkg/errors" "perun.network/go-perun/wallet" ) @@ -56,6 +59,24 @@ func (sig *Sig) UnmarshalBinary(data []byte) (err error) { return nil } +// Verify verifies if the signature on the given message was made by the given +// address. +func (sig Sig) Verify(msg []byte, addr wallet.Address) (bool, error) { + ethAddr, ok := addr.(*Address) + if !ok { + return false, errors.New("Wrong address type passed to Backend.VerifySignature") + } + + // escda.Verify needs a digest as input + // ref https://golang.org/pkg/crypto/ecdsa/#Verify + return ecdsa.Verify((*ecdsa.PublicKey)(ethAddr), digest(msg), sig.r, sig.s), nil +} + +func digest(msg []byte) []byte { + digest := sha256.Sum256(msg) + return digest[:] +} + // Clone returns a deep copy of the signature. func (sig Sig) Clone() wallet.Sig { clone := Sig{ From 1b08a6950ea93a242da767649129ca1a1b6728d4 Mon Sep 17 00:00:00 2001 From: Manoranjith Date: Tue, 14 Dec 2021 16:46:34 +0530 Subject: [PATCH 11/12] :boom: [eth/wallet] Move VerifySignature from Backend to Sig Signed-off-by: Manoranjith --- backend/ethereum/channel/backend.go | 2 +- backend/ethereum/wallet/backend.go | 30 ------------------- .../ethereum/wallet/keystore/wallet_test.go | 4 +-- backend/ethereum/wallet/sig.go | 20 +++++++++++++ backend/ethereum/wallet/simple/wallet_test.go | 2 +- 5 files changed, 24 insertions(+), 34 deletions(-) diff --git a/backend/ethereum/channel/backend.go b/backend/ethereum/channel/backend.go index a4ee4937..5d76f3a0 100644 --- a/backend/ethereum/channel/backend.go +++ b/backend/ethereum/channel/backend.go @@ -143,7 +143,7 @@ func Verify(addr wallet.Address, s *channel.State, sig wallet.Sig) (bool, error) if err != nil { return false, errors.WithMessage(err, "encoding state") } - return ethwallet.VerifySignature(enc, sig, addr) + return sig.Verify(enc, addr) } // NewAsset returns a variable of type Asset, which can be used diff --git a/backend/ethereum/wallet/backend.go b/backend/ethereum/wallet/backend.go index 825e6f3c..8bfde753 100644 --- a/backend/ethereum/wallet/backend.go +++ b/backend/ethereum/wallet/backend.go @@ -16,7 +16,6 @@ package wallet import ( "github.com/ethereum/go-ethereum/crypto" - "github.com/pkg/errors" "perun.network/go-perun/wallet" ) @@ -41,35 +40,6 @@ func (*Backend) NewSig() wallet.Sig { return &sig } -// VerifySignature verifies if the signature on the given message was made by -// this account. -func (*Backend) VerifySignature(msg []byte, sig wallet.Sig, a wallet.Address) (bool, error) { - return VerifySignature(msg, sig, a) -} - -// VerifySignature verifies if the signature on the given message was made by -// this account. -func VerifySignature(msg []byte, sig wallet.Sig, a wallet.Address) (bool, error) { - ethSig, ok := sig.(*Sig) - if !ok { - return false, errors.New("signature was not of the expected type") - } - - hash := PrefixedHash(msg) - ethSigCopy := make([]byte, sigLen) - copy(ethSigCopy, *ethSig) - if ethSigCopy[sigLen-1] >= sigVSubtract { - ethSigCopy[sigLen-1] -= sigVSubtract - } - - pk, err := crypto.SigToPub(hash, ethSigCopy) - if err != nil { - return false, errors.WithStack(err) - } - addr := crypto.PubkeyToAddress(*pk) - return a.Equal((*Address)(&addr)), nil -} - // PrefixedHash adds an ethereum specific prefix to the hash of given data, rehashes the results // and returns it. func PrefixedHash(data []byte) []byte { diff --git a/backend/ethereum/wallet/keystore/wallet_test.go b/backend/ethereum/wallet/keystore/wallet_test.go index d2638b5a..053fb531 100644 --- a/backend/ethereum/wallet/keystore/wallet_test.go +++ b/backend/ethereum/wallet/keystore/wallet_test.go @@ -62,7 +62,7 @@ func TestSignatures(t *testing.T) { acc := ethwallettest.NewTmpWallet().NewAccount() sign, err := acc.SignData(dataToSign) assert.NoError(t, err, "Sign with new account should succeed") - valid, err := new(ethwallet.Backend).VerifySignature(dataToSign, sign, acc.Address()) + valid, err := sign.Verify(dataToSign, acc.Address()) assert.True(t, valid, "Verification should succeed") assert.NoError(t, err, "Verification should succeed") } @@ -113,7 +113,7 @@ func TestCurve_SigningAndVerifying(t *testing.T) { require.NoError(t, err, "decode sig should not error") addr := ethwallet.Address(common.HexToAddress("f17f52151EbEF6C7334FAD080c5704D77216b732")) ethSig := ethwallet.Sig(sig) - b, err := ethwallet.VerifySignature(msg, ðSig, &addr) + b, err := ethSig.Verify(msg, &addr) assert.NoError(t, err, "VerifySignature should not error") assert.True(t, b, "VerifySignature") } diff --git a/backend/ethereum/wallet/sig.go b/backend/ethereum/wallet/sig.go index 10c6e731..4465eb1e 100644 --- a/backend/ethereum/wallet/sig.go +++ b/backend/ethereum/wallet/sig.go @@ -17,6 +17,8 @@ package wallet import ( "fmt" + "github.com/ethereum/go-ethereum/crypto" + "github.com/pkg/errors" "perun.network/go-perun/wallet" ) @@ -49,6 +51,24 @@ func (s *Sig) UnmarshalBinary(data []byte) error { return nil } +// Verify verifies if the signature on the given message was made by the given +// address. +func (s *Sig) Verify(msg []byte, addr wallet.Address) (bool, error) { + hash := PrefixedHash(msg) + sigCopy := make([]byte, sigLen) + copy(sigCopy, *s) + if sigCopy[sigLen-1] >= sigVSubtract { + sigCopy[sigLen-1] -= sigVSubtract + } + + pk, err := crypto.SigToPub(hash, sigCopy) + if err != nil { + return false, errors.WithStack(err) + } + addrInSig := crypto.PubkeyToAddress(*pk) + return addr.Equal((*Address)(&addrInSig)), nil +} + // Clone returns a deep copy of the signature. func (s Sig) Clone() wallet.Sig { clone := Sig(make([]byte, sigLen)) diff --git a/backend/ethereum/wallet/simple/wallet_test.go b/backend/ethereum/wallet/simple/wallet_test.go index 72d66a05..4a91d187 100644 --- a/backend/ethereum/wallet/simple/wallet_test.go +++ b/backend/ethereum/wallet/simple/wallet_test.go @@ -78,7 +78,7 @@ func TestSignatures(t *testing.T) { sig, err := acc.SignData(dataToSign) assert.NoError(t, err, "Sign with new account should succeed") assert.NotNil(t, sig) - valid, err := new(ethwallet.Backend).VerifySignature(dataToSign, sig, acc.Address()) + valid, err := sig.Verify(dataToSign, acc.Address()) assert.True(t, valid, "Verification should succeed") assert.NoError(t, err, "Verification should succeed") } From 310978a79a7c416248274961a6836dd9719ac044 Mon Sep 17 00:00:00 2001 From: Manoranjith Date: Tue, 14 Dec 2021 16:46:52 +0530 Subject: [PATCH 12/12] :refactor: [wallet/test] Replace Backend.VerifySignature w/ Sig.Verify Signed-off-by: Manoranjith --- wallet/test/wallet.go | 6 +++--- wallet/test/walletbench.go | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/wallet/test/wallet.go b/wallet/test/wallet.go index 71ff6652..ee07a669 100644 --- a/wallet/test/wallet.go +++ b/wallet/test/wallet.go @@ -49,14 +49,14 @@ func TestAccountWithWalletAndBackend(t *testing.T, s *Setup) { //nolint:revive / // Check unlocked account sig, err := acc.SignData(s.DataToSign) assert.NoError(t, err, "Sign with unlocked account should succeed") - valid, err := s.Backend.VerifySignature(s.DataToSign, sig, acc.Address()) + valid, err := sig.Verify(s.DataToSign, acc.Address()) assert.True(t, valid, "Verification should succeed") assert.NoError(t, err, "Verification should not produce error") addr := s.Backend.NewAddress() err = perunio.Decode(bytes.NewReader(s.AddressEncoded), addr) assert.NoError(t, err, "Byte deserialization of address should work") - valid, err = s.Backend.VerifySignature(s.DataToSign, sig, addr) + valid, err = sig.Verify(s.DataToSign, addr) assert.False(t, valid, "Verification with wrong address should fail") assert.NoError(t, err, "Verification of valid signature should not produce error") @@ -64,7 +64,7 @@ func TestAccountWithWalletAndBackend(t *testing.T, s *Setup) { //nolint:revive / tamperedBytes := marshalAndTamper(t, sig, func(sigBytes *[]byte) { (*sigBytes)[0] = ^(*sigBytes)[0] }) tampered := s.Backend.NewSig() require.NoError(t, tampered.UnmarshalBinary(tamperedBytes)) - valid, err = s.Backend.VerifySignature(s.DataToSign, tampered, acc.Address()) + valid, err = tampered.Verify(s.DataToSign, acc.Address()) if valid && err == nil { t.Error("Verification of invalid signature should produce error or return false") } diff --git a/wallet/test/walletbench.go b/wallet/test/walletbench.go index 602a4279..85d7e617 100644 --- a/wallet/test/walletbench.go +++ b/wallet/test/walletbench.go @@ -61,7 +61,7 @@ func benchBackendVerifySig(b *testing.B, s *Setup) { b.StartTimer() for n := 0; n < b.N; n++ { - ok, err := s.Backend.VerifySignature(s.DataToSign, signature, perunAcc.Address()) + ok, err := signature.Verify(s.DataToSign, perunAcc.Address()) if !ok { b.Fatal(err)