Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(wasmer/crypto): move sig verifier to crypto pkg #2057

Merged
merged 43 commits into from
Dec 6, 2021
Merged

feat(wasmer/crypto): move sig verifier to crypto pkg #2057

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
db85f05
chore: delete sig verifier from runtime pkg
1garo Nov 20, 2021
eda4236
feat: implement SignVerify for sr25519
1garo Nov 20, 2021
ed89c50
feat: implement SignVerify for ed25519
1garo Nov 20, 2021
6f41dfc
feat: implement SignVerify for srcp256k1
1garo Nov 20, 2021
73e8d87
fix: move sig_verifier to crypto pkg
1garo Nov 20, 2021
11b6016
chore: update usages for crypto pkg
1garo Nov 20, 2021
63a3442
fix: resolve lint problem
1garo Nov 20, 2021
969efc2
chore: add comment on exported type
1garo Nov 20, 2021
9146c37
remove spaces between imports
1garo Nov 23, 2021
a839018
rename function to improve readability
1garo Nov 23, 2021
43ad637
add err signature err msg and add return for docs purpose
1garo Nov 23, 2021
9531a54
improve naming and error handling
1garo Nov 23, 2021
7a9560e
implement tests for new function
1garo Nov 23, 2021
9c31504
gen by mockery
1garo Nov 23, 2021
fd188fc
create tests for sig verifier
1garo Nov 24, 2021
53ca92b
Merge branch 'development' into 1garo/move-sig-verifier-to-crypto
1garo Nov 24, 2021
610e4cd
rename type and refactor VerifyFunc call
1garo Nov 24, 2021
5c45a1f
remove ok bool as return value
1garo Nov 24, 2021
5c15e6f
update tests and add one for sig_verifier
1garo Nov 24, 2021
fbe70d8
add signature name when error occurs
1garo Nov 26, 2021
c164f4d
parallel tests and rename test case
1garo Nov 26, 2021
e1717bc
parallel tests and add log level
1garo Nov 26, 2021
f009530
gofmt
1garo Nov 26, 2021
b83cc03
Merge branch 'development' into 1garo/move-sig-verifier-to-crypto
1garo Nov 26, 2021
9534e7e
resolve make build erroring
1garo Nov 26, 2021
634ea82
rename type
1garo Nov 26, 2021
973ce6b
remove space between imports
1garo Nov 26, 2021
ddba19f
remove useless alias
1garo Nov 26, 2021
da14c99
check ok, if false errors
1garo Nov 26, 2021
664aa50
rename typ
1garo Nov 26, 2021
625ca02
solve tests failing
1garo Nov 26, 2021
975b406
use test cases to remove useless code
1garo Nov 26, 2021
75f6080
send empty bytes on error test case
1garo Nov 26, 2021
7a62d06
improve tests cases
1garo Nov 27, 2021
7da55ab
use constants instead of just []byte
1garo Nov 30, 2021
2fdabf8
switch validation order
1garo Nov 30, 2021
ae14785
use new err approach validation and add tests
1garo Nov 30, 2021
f223d65
make use of constants
1garo Nov 30, 2021
1ee5e82
fix error validation on ed25519
1garo Dec 1, 2021
4012a66
fix error validation on sr25519
1garo Dec 1, 2021
f7b81b1
put publicKey in var to avoid golangci-lint error
1garo Dec 1, 2021
52bc4c3
use a new err variable for each parallel test run
kishansagathiya Dec 6, 2021
698d36e
Merge branch 'development' into 1garo/move-sig-verifier-to-crypto
kishansagathiya Dec 6, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions lib/crypto/ed25519/ed25519.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,20 @@ type PublicKey ed25519.PublicKey
// PublicKeyBytes is an encoded ed25519 public key
type PublicKeyBytes [PublicKeyLength]byte

// SignVerify verify signature given pubkey and msg
1garo marked this conversation as resolved.
Show resolved Hide resolved
func SignVerify(pubkey, sig, msg []byte) (bool, error) {
1garo marked this conversation as resolved.
Show resolved Hide resolved
1garo marked this conversation as resolved.
Show resolved Hide resolved
pubKey, err := NewPublicKey(pubkey)
if err != nil {
return false, fmt.Errorf("failed to fetch ed25519 public key: %s", err)
}
ok, err := pubKey.Verify(msg, sig)
if err != nil || !ok {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to split those two conditions. If error is nil and ok is false (when the verification fails), we should wrap our own error defined in this package (or some shared package) and not wrap the nil error returned.

Alternatively we could modify Verify methods to return an error if the verification fails and not the boolean ok, but that might imply a lot of code changes, so not really a solution here.

Copy link
Contributor Author

@1garo 1garo Nov 30, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My suggestion would be:

ok, err := pubKey.Verify(message, signature)
if err != nil {
  return fmt.Errorf("sr25519: %w: %s", crypto.ErrSignatureVerificationFailed, err)
}

if !ok {
  return fmt.Errorf("sr25519: %w: for message 0x%x, signature 0x%x and public key 0x%x",
			crypto.ErrSignatureVerificationFailed, message, signature, publicKey)
}

since when err != nil, the cause of it will be explicit on err, e.g if one of those happen:

if k.key == nil {
  return false, errors.New("nil public key")
}

if len(sig) != SignatureLength {
  return false, errors.New("invalid signature length")
}

and in the case where err == nill && !ok, the problem was really the verification step, i.e:

t := sr25519.NewSigningContext(SigningContext, msg)
return k.key.Verify(s, t), nil

The same would apply for ed25518.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, although don't wrap both errors with crypto.ErrSignatureVerificationFailed that looks strange. The parent calling function should wrap the error in this case. Instead you can just have

ok, err := pubKey.Verify(message, signature)
if err != nil {
  return fmt.Errorf("sr25519: %w", err)
} else if !ok {
  return fmt.Errorf("sr25519: %w: for message 0x%x, signature 0x%x and public key 0x%x",
			crypto.ErrSignatureVerificationFailed, message, signature, publicKey)
}

?

Copy link
Contributor Author

@1garo 1garo Dec 1, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looking now it really seems strange that way, changed on 1ee5e82 4012a66

return false, fmt.Errorf("failed to verify ed25519 signature: %s", err)
1garo marked this conversation as resolved.
Show resolved Hide resolved
}
1garo marked this conversation as resolved.
Show resolved Hide resolved

return true, nil
}

// String returns the PublicKeyBytes formatted as a hex string
func (b PublicKeyBytes) String() string {
pk := [PublicKeyLength]byte(b)
Expand Down
11 changes: 11 additions & 0 deletions lib/crypto/secp256k1/secp256k1.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"crypto/ecdsa"
"encoding/hex"
"errors"
"fmt"

"github.com/ChainSafe/gossamer/lib/common"
"github.com/ChainSafe/gossamer/lib/crypto"
Expand Down Expand Up @@ -36,6 +37,16 @@ type PublicKey struct {
key ecdsa.PublicKey
}

// SignVerify verify signature given pubkey and msg
1garo marked this conversation as resolved.
Show resolved Hide resolved
func SignVerify(pubkey, sig, msg []byte) (bool, error) {
1garo marked this conversation as resolved.
Show resolved Hide resolved
1garo marked this conversation as resolved.
Show resolved Hide resolved
ok := secp256k1.VerifySignature(pubkey, msg, sig)
if !ok {
return false, fmt.Errorf("failed to verify secp256k1 signature")
1garo marked this conversation as resolved.
Show resolved Hide resolved
}

return true, nil
}

// RecoverPublicKey returns the 64-byte uncompressed public key that created the given signature.
func RecoverPublicKey(msg, sig []byte) ([]byte, error) {
// update recovery bit
Expand Down
55 changes: 12 additions & 43 deletions lib/runtime/sig_verifier.go → lib/crypto/sig_verifier.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,26 +1,24 @@
// Copyright 2021 ChainSafe Systems (ON)
// SPDX-License-Identifier: LGPL-3.0-only

package runtime
package crypto

import (
"fmt"
"sync"
"time"

"github.com/ChainSafe/gossamer/internal/log"
"github.com/ChainSafe/gossamer/lib/crypto"
"github.com/ChainSafe/gossamer/lib/crypto/ed25519"
"github.com/ChainSafe/gossamer/lib/crypto/sr25519"
"github.com/ethereum/go-ethereum/crypto/secp256k1"
)

// SigVerifyFunc verify an signature given a pubkey and msg
1garo marked this conversation as resolved.
Show resolved Hide resolved
type SigVerifyFunc func(pubkey, sig, msg []byte) (bool, error)
1garo marked this conversation as resolved.
Show resolved Hide resolved

// Signature ...
type Signature struct {
1garo marked this conversation as resolved.
Show resolved Hide resolved
PubKey []byte
Sign []byte
Msg []byte
KeyTypeID crypto.KeyType
PubKey []byte
Sign []byte
Msg []byte
VerifyFunc SigVerifyFunc
}

// SignatureVerifier ...
Expand Down Expand Up @@ -66,12 +64,12 @@ func (sv *SignatureVerifier) Start() {
case <-sv.closeCh:
return
default:
sign := sv.Remove()
if sign == nil {
sig := sv.Remove()
1garo marked this conversation as resolved.
Show resolved Hide resolved
if sig == nil {
continue
}
err := sign.verify()
if err != nil {
ok, err := sig.VerifyFunc(sig.PubKey, sig.Sign, sig.Msg)
if err != nil || !ok {
1garo marked this conversation as resolved.
Show resolved Hide resolved
sv.logger.Errorf("[ext_crypto_start_batch_verify_version_1]: %s", err)
sv.Invalid()
return
Expand Down Expand Up @@ -153,32 +151,3 @@ func (sv *SignatureVerifier) Finish() bool {
sv.Reset()
return !isInvalid
}

func (sig *Signature) verify() error {
switch sig.KeyTypeID {
case crypto.Ed25519Type:
pubKey, err := ed25519.NewPublicKey(sig.PubKey)
if err != nil {
return fmt.Errorf("failed to fetch ed25519 public key: %s", err)
}
ok, err := pubKey.Verify(sig.Msg, sig.Sign)
if err != nil || !ok {
return fmt.Errorf("failed to verify ed25519 signature: %s", err)
}
case crypto.Sr25519Type:
pubKey, err := sr25519.NewPublicKey(sig.PubKey)
if err != nil {
return fmt.Errorf("failed to fetch sr25519 public key: %s", err)
}
ok, err := pubKey.Verify(sig.Msg, sig.Sign)
if err != nil || !ok {
return fmt.Errorf("failed to verify sr25519 signature: %s", err)
}
case crypto.Secp256k1Type:
ok := secp256k1.VerifySignature(sig.PubKey, sig.Msg, sig.Sign)
if !ok {
return fmt.Errorf("failed to verify secp256k1 signature")
}
}
return nil
}
15 changes: 15 additions & 0 deletions lib/crypto/sr25519/sr25519.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package sr25519
import (
"encoding/hex"
"errors"
"fmt"

"github.com/ChainSafe/gossamer/lib/common"
"github.com/ChainSafe/gossamer/lib/crypto"
Expand Down Expand Up @@ -43,6 +44,20 @@ type PrivateKey struct {
key *sr25519.SecretKey
}

// SignVerify verify signature given pubkey and msg
1garo marked this conversation as resolved.
Show resolved Hide resolved
func SignVerify(pubkey, sig, msg []byte) (bool, error) {
1garo marked this conversation as resolved.
Show resolved Hide resolved
pubKey, err := NewPublicKey(pubkey)
if err != nil {
return false, fmt.Errorf("failed to fetch sr25519 public key: %s", err)
1garo marked this conversation as resolved.
Show resolved Hide resolved
}
ok, err := pubKey.Verify(msg, sig)
if err != nil || !ok {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here, we need these two split

return false, fmt.Errorf("failed to verify sr25519 signature: %s", err)
1garo marked this conversation as resolved.
Show resolved Hide resolved
1garo marked this conversation as resolved.
Show resolved Hide resolved
}

return true, nil
}

// NewKeypair returns a sr25519 Keypair given a schnorrkel secret key
func NewKeypair(priv *sr25519.SecretKey) (*Keypair, error) {
pub, err := priv.Public()
Expand Down
3 changes: 2 additions & 1 deletion lib/runtime/life/instance.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (

"github.com/ChainSafe/gossamer/internal/log"
"github.com/ChainSafe/gossamer/lib/common"
"github.com/ChainSafe/gossamer/lib/crypto"
"github.com/ChainSafe/gossamer/lib/keystore"
"github.com/ChainSafe/gossamer/lib/runtime"

Expand Down Expand Up @@ -112,7 +113,7 @@ func NewInstance(code []byte, cfg *Config) (*Instance, error) {
NodeStorage: cfg.NodeStorage,
Network: cfg.Network,
Transaction: cfg.Transaction,
SigVerifier: runtime.NewSignatureVerifier(logger),
SigVerifier: crypto.NewSignatureVerifier(logger),
}

logger.Debugf("creating new runtime instance with context: %v", runtimeCtx)
Expand Down
20 changes: 10 additions & 10 deletions lib/runtime/life/resolver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -933,11 +933,11 @@ func ext_crypto_ed25519_verify_version_1(vm *exec.VirtualMachine) int64 {
}

if sigVerifier.IsStarted() {
signature := runtime.Signature{
PubKey: pubKey.Encode(),
Sign: signature,
Msg: message,
KeyTypeID: crypto.Ed25519Type,
signature := crypto.Signature{
PubKey: pubKey.Encode(),
Sign: signature,
Msg: message,
VerifyFunc: ed25519.SignVerify,
}
sigVerifier.Add(&signature)
return 1
Expand Down Expand Up @@ -1120,11 +1120,11 @@ func ext_crypto_sr25519_verify_version_1(vm *exec.VirtualMachine) int64 {
pub.Hex(), message, signature)

if sigVerifier.IsStarted() {
signature := runtime.Signature{
PubKey: pub.Encode(),
Sign: signature,
Msg: message,
KeyTypeID: crypto.Sr25519Type,
signature := crypto.Signature{
PubKey: pub.Encode(),
Sign: signature,
Msg: message,
VerifyFunc: sr25519.SignVerify,
}
sigVerifier.Add(&signature)
return 1
Expand Down
17 changes: 9 additions & 8 deletions lib/runtime/test_helpers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,10 @@ import (
"testing"
"time"

"github.com/ChainSafe/gossamer/lib/crypto"

1garo marked this conversation as resolved.
Show resolved Hide resolved
"github.com/ChainSafe/chaindb"
"github.com/ChainSafe/gossamer/lib/common"
"github.com/ChainSafe/gossamer/lib/crypto"
"github.com/ChainSafe/gossamer/lib/crypto/ed25519"
"github.com/ChainSafe/gossamer/lib/utils"
ma "github.com/multiformats/go-multiaddr"
Expand Down Expand Up @@ -115,9 +116,9 @@ func (*TestRuntimeNetwork) NetworkState() common.NetworkState {
}
}

func generateEd25519Signatures(t *testing.T, n int) []*Signature {
func generateEd25519Signatures(t *testing.T, n int) []*crypto.Signature {
t.Helper()
signs := make([]*Signature, n)
signs := make([]*crypto.Signature, n)
for i := 0; i < n; i++ {
msg := []byte("Hello")
key, err := ed25519.GenerateKeypair()
Expand All @@ -126,11 +127,11 @@ func generateEd25519Signatures(t *testing.T, n int) []*Signature {
sign, err := key.Private().Sign(msg)
require.NoError(t, err)

signs[i] = &Signature{
PubKey: key.Public().Encode(),
Sign: sign,
Msg: msg,
KeyTypeID: crypto.Ed25519Type,
signs[i] = &crypto.Signature{
PubKey: key.Public().Encode(),
Sign: sign,
Msg: msg,
VerifyFunc: ed25519.SignVerify,
}
}
return signs
Expand Down
3 changes: 2 additions & 1 deletion lib/runtime/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package runtime
import (
"github.com/ChainSafe/gossamer/internal/log"
"github.com/ChainSafe/gossamer/lib/common"
"github.com/ChainSafe/gossamer/lib/crypto"
"github.com/ChainSafe/gossamer/lib/keystore"
"github.com/ChainSafe/gossamer/lib/runtime/offchain"
)
Expand Down Expand Up @@ -67,7 +68,7 @@ type Context struct {
NodeStorage NodeStorage
Network BasicNetwork
Transaction TransactionState
SigVerifier *SignatureVerifier
SigVerifier *crypto.SignatureVerifier
OffchainHTTPSet *offchain.HTTPSet
}

Expand Down
40 changes: 20 additions & 20 deletions lib/runtime/types_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import (

func TestBackgroundSignVerification(t *testing.T) {
signs := generateEd25519Signatures(t, 2)
signVerify := NewSignatureVerifier(log.New(log.SetWriter(io.Discard)))
signVerify := crypto.NewSignatureVerifier(log.New(log.SetWriter(io.Discard)))

signVerify.Start()

Expand All @@ -34,7 +34,7 @@ func TestBackgroundSignVerification(t *testing.T) {

func TestBackgroundSignVerificationMultipleStart(t *testing.T) {
signs := generateEd25519Signatures(t, 2)
signVerify := NewSignatureVerifier(log.New(log.SetWriter(io.Discard)))
signVerify := crypto.NewSignatureVerifier(log.New(log.SetWriter(io.Discard)))

for ii := 0; ii < 5; ii++ {
require.False(t, signVerify.IsStarted())
Expand All @@ -60,16 +60,16 @@ func TestInvalidSignatureBatch(t *testing.T) {
sigData, err := common.HexToBytes("0x90f27b8b488db00b00606796d2987f6a5f59ae62ea05effe84fef5b8b0e549984a691139ad57a3f0b906637673aa2f63d1f55cb1a69199d4009eea23ceaddc9301")
require.Nil(t, err)

signature := &Signature{
PubKey: key.Public().Encode(),
Sign: sigData,
Msg: msg,
KeyTypeID: crypto.Ed25519Type,
signature := &crypto.Signature{
PubKey: key.Public().Encode(),
Sign: sigData,
Msg: msg,
VerifyFunc: ed25519.SignVerify,
}

signs = append(signs, signature)

signVerify := NewSignatureVerifier(log.New(log.SetWriter(io.Discard)))
signVerify := crypto.NewSignatureVerifier(log.New(log.SetWriter(io.Discard)))
signVerify.Start()

for _, sig := range signs {
Expand All @@ -80,7 +80,7 @@ func TestInvalidSignatureBatch(t *testing.T) {

func TestValidSignatureBatch(t *testing.T) {
signs := generateEd25519Signatures(t, 2)
signVerify := NewSignatureVerifier(log.New(log.SetWriter(io.Discard)))
signVerify := crypto.NewSignatureVerifier(log.New(log.SetWriter(io.Discard)))

signVerify.Start()

Expand All @@ -101,11 +101,11 @@ func TestAllCryptoTypeSignature(t *testing.T) {
srSig, err := srKey.Private().Sign(srMsg)
require.NoError(t, err)

srSignature := &Signature{
PubKey: srKey.Public().Encode(),
Sign: srSig,
Msg: srMsg,
KeyTypeID: crypto.Sr25519Type,
srSignature := &crypto.Signature{
PubKey: srKey.Public().Encode(),
Sign: srSig,
Msg: srMsg,
VerifyFunc: sr25519.SignVerify,
}

blakeHash, err := common.Blake2bHash([]byte("secp256k1"))
Expand All @@ -118,14 +118,14 @@ func TestAllCryptoTypeSignature(t *testing.T) {
require.NoError(t, err)

secpSigData = secpSigData[:len(secpSigData)-1] // remove recovery id
secpSignature := &Signature{
PubKey: kp.Public().Encode(),
Sign: secpSigData,
Msg: blakeHash.ToBytes(),
KeyTypeID: crypto.Secp256k1Type,
secpSignature := &crypto.Signature{
PubKey: kp.Public().Encode(),
Sign: secpSigData,
Msg: blakeHash.ToBytes(),
VerifyFunc: secp256k1.SignVerify,
}

signVerify := NewSignatureVerifier(log.New(log.SetWriter(io.Discard)))
signVerify := crypto.NewSignatureVerifier(log.New(log.SetWriter(io.Discard)))

signVerify.Start()

Expand Down
Loading