From 67a22a2140ba51ee9cc2433d81f4e99fa02e4043 Mon Sep 17 00:00:00 2001 From: Manoranjith Date: Mon, 13 Dec 2021 20:49:40 +0530 Subject: [PATCH 1/8] :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 2/8] :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 3/8] :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 4/8] :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 5/8] :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 6/8] :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 7/8] :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 8/8] :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 -}