From cb9f762675a49f4cf64eeb959e6163d0dadd873c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=BCrgen=20Eckel?= <eckelj@users.noreply.github.com>
Date: Fri, 15 Sep 2023 10:10:04 +0200
Subject: [PATCH] Eckelj/fix store resolve issues (#79)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* added upper and lower case TA resolution testing

* added more detailed error reporting to the ValidateSignature method.
* extended test cases to test and verify these errs and their differences

* fixed CID attestation issue. CIDs are send in web compatible encoding that is not hex encoded and can be utilized without any further decoding on the server side.

* added checks and error handling for the Ta store object storage/loading

Signed-off-by: Jürgen Eckel <juergen@riddleandcode.com>
---
 tests/e2e/asset/rest.go                       | 24 ++++++++----
 tests/e2e/asset/suite.go                      | 26 +++++++++----
 testutil/sample/sample.go                     |  4 +-
 util/validate_signature.go                    | 32 ++++++++++++----
 x/asset/keeper/msg_server_notarize_asset.go   |  4 +-
 x/asset/keeper/msg_server_test.go             | 15 +++++++-
 x/machine/keeper/msg_server_attest_machine.go | 12 +++---
 .../msg_server_register_trust_anchor.go       |  5 +--
 x/machine/keeper/trust_anchor.go              | 21 +++++++---
 x/machine/keeper/trust_anchor_test.go         | 38 ++++++++++++++++---
 10 files changed, 134 insertions(+), 47 deletions(-)

diff --git a/tests/e2e/asset/rest.go b/tests/e2e/asset/rest.go
index 77757f2..a937ab0 100644
--- a/tests/e2e/asset/rest.go
+++ b/tests/e2e/asset/rest.go
@@ -28,7 +28,7 @@ func (s *E2ETestSuite) TestNotarizeAssetREST() {
 	privKey, _ := xskKey.ECPrivKey()
 	byte_key := privKey.Serialize()
 	sk := hex.EncodeToString(byte_key)
-	cidHash, signature := sample.Asset(sk)
+	cid, signatureHex := sample.Asset(sk)
 
 	testCases := []struct {
 		name   string
@@ -39,18 +39,28 @@ func (s *E2ETestSuite) TestNotarizeAssetREST() {
 			"machine not found",
 			assettypes.MsgNotarizeAsset{
 				Creator:   addr.String(),
-				Hash:      cidHash,
-				Signature: signature,
+				Hash:      cid,
+				Signature: signatureHex,
 				PubKey:    "human pubkey",
 			},
 			"machine not found",
 		},
+		{
+			"invalid signature hex string",
+			assettypes.MsgNotarizeAsset{
+				Creator:   addr.String(),
+				Hash:      cid,
+				Signature: "invalid signature",
+				PubKey:    xPubKey,
+			},
+			"invalid signature hex string",
+		},
 		{
 			"invalid signature",
 			assettypes.MsgNotarizeAsset{
 				Creator:   addr.String(),
-				Hash:      cidHash,
-				Signature: "invalid signature",
+				Hash:      cid,
+				Signature: hex.EncodeToString([]byte("invalid signature")),
 				PubKey:    xPubKey,
 			},
 			"invalid signature",
@@ -59,8 +69,8 @@ func (s *E2ETestSuite) TestNotarizeAssetREST() {
 			"valid notarization",
 			assettypes.MsgNotarizeAsset{
 				Creator:   addr.String(),
-				Hash:      cidHash,
-				Signature: signature,
+				Hash:      cid,
+				Signature: signatureHex,
 				PubKey:    xPubKey,
 			},
 			"planetmintgo.asset.MsgNotarizeAsset",
diff --git a/tests/e2e/asset/suite.go b/tests/e2e/asset/suite.go
index b426d68..01ec9b3 100644
--- a/tests/e2e/asset/suite.go
+++ b/tests/e2e/asset/suite.go
@@ -131,7 +131,7 @@ func (s *E2ETestSuite) TestNotarizeAsset() {
 	privKey, _ := xskKey.ECPrivKey()
 	byte_key := privKey.Serialize()
 	sk := hex.EncodeToString(byte_key)
-	cidHash, signature := sample.Asset(sk)
+	cid, signatureHex := sample.Asset(sk)
 
 	testCases := []struct {
 		name   string
@@ -141,8 +141,8 @@ func (s *E2ETestSuite) TestNotarizeAsset() {
 		{
 			"machine not found",
 			[]string{
-				cidHash,
-				signature,
+				cid,
+				signatureHex,
 				"pubkey",
 				fmt.Sprintf("--%s=%s", flags.FlagFrom, sample.Name),
 				fmt.Sprintf("--%s=%s", flags.FlagFees, sample.Fees),
@@ -150,11 +150,23 @@ func (s *E2ETestSuite) TestNotarizeAsset() {
 			},
 			"machine not found",
 		},
+		{
+			"invalid signature hex string",
+			[]string{
+				cid,
+				"signature",
+				xPubKey,
+				fmt.Sprintf("--%s=%s", flags.FlagFrom, sample.Name),
+				fmt.Sprintf("--%s=%s", flags.FlagFees, sample.Fees),
+				"--yes",
+			},
+			"invalid signature hex string",
+		},
 		{
 			"invalid signature",
 			[]string{
-				"cid",
-				"signature",
+				cid,
+				hex.EncodeToString([]byte("signature")),
 				xPubKey,
 				fmt.Sprintf("--%s=%s", flags.FlagFrom, sample.Name),
 				fmt.Sprintf("--%s=%s", flags.FlagFees, sample.Fees),
@@ -165,8 +177,8 @@ func (s *E2ETestSuite) TestNotarizeAsset() {
 		{
 			"valid notarization",
 			[]string{
-				cidHash,
-				signature,
+				cid,
+				signatureHex,
 				xPubKey,
 				fmt.Sprintf("--%s=%s", flags.FlagFrom, sample.Name),
 				fmt.Sprintf("--%s=%s", flags.FlagFees, sample.Fees),
diff --git a/testutil/sample/sample.go b/testutil/sample/sample.go
index 74f5f3e..f889fd1 100644
--- a/testutil/sample/sample.go
+++ b/testutil/sample/sample.go
@@ -110,9 +110,7 @@ func Asset(sk string) (string, string) {
 	skBytes, _ := hex.DecodeString(sk)
 	privKey := &secp256k1.PrivKey{Key: skBytes}
 
-	cid_bytes, _ := hex.DecodeString(cid)
-	sign, _ := privKey.Sign(cid_bytes)
-
+	sign, _ := privKey.Sign([]byte(cid))
 	signatureHex := hex.EncodeToString(sign)
 
 	return cid, signatureHex
diff --git a/util/validate_signature.go b/util/validate_signature.go
index fbe8b12..91f0ad8 100644
--- a/util/validate_signature.go
+++ b/util/validate_signature.go
@@ -2,24 +2,42 @@ package util
 
 import (
 	"encoding/hex"
+	"errors"
 
 	"github.com/btcsuite/btcd/btcutil/hdkeychain"
 	"github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1"
 )
 
-func ValidateSignature(message string, signature string, publicKey string) bool {
+func ValidateSignature(message string, signature string, publicKey string) (bool, error) {
 	// Convert the message, signature, and public key from hex to bytes
-	messageBytes, _ := hex.DecodeString(message)
-	signatureBytes, _ := hex.DecodeString(signature)
-	publicKeyBytes, _ := hex.DecodeString(publicKey)
+	messageBytes, err := hex.DecodeString(message)
+	if err != nil {
+		return false, errors.New("invalid message hex string")
+	}
+	return ValidateSignatureByteMsg(messageBytes, signature, publicKey)
+}
+
+func ValidateSignatureByteMsg(message []byte, signature string, publicKey string) (bool, error) {
+	// Convert  signature, and public key from hex to bytes
+	signatureBytes, err := hex.DecodeString(signature)
+	if err != nil {
+		return false, errors.New("invalid signature hex string")
+	}
+	publicKeyBytes, err := hex.DecodeString(publicKey)
+	if err != nil {
+		return false, errors.New("invalid public key hex string")
+	}
 
 	// Create a secp256k1 public key object
 	pubKey := &secp256k1.PubKey{Key: publicKeyBytes}
 
 	// Verify the signature
-	isValid := pubKey.VerifySignature(messageBytes, signatureBytes)
-
-	return isValid
+	isValid := pubKey.VerifySignature(message, signatureBytes)
+	if !isValid {
+		return false, errors.New("invalid signature")
+	} else {
+		return isValid, nil
+	}
 }
 
 func GetHexPubKey(ext_pub_key string) (string, error) {
diff --git a/x/asset/keeper/msg_server_notarize_asset.go b/x/asset/keeper/msg_server_notarize_asset.go
index e106f94..abb5434 100644
--- a/x/asset/keeper/msg_server_notarize_asset.go
+++ b/x/asset/keeper/msg_server_notarize_asset.go
@@ -22,9 +22,9 @@ func (k msgServer) NotarizeAsset(goCtx context.Context, msg *types.MsgNotarizeAs
 	if err != nil {
 		return nil, errors.New("could not convert xpub key to hex pub key")
 	}
-	valid := util.ValidateSignature(msg.Hash, msg.Signature, hex_pub_key)
+	valid, err := util.ValidateSignatureByteMsg([]byte(msg.Hash), msg.Signature, hex_pub_key)
 	if !valid {
-		return nil, errors.New("invalid signature")
+		return nil, err
 	}
 
 	var asset = types.Asset{
diff --git a/x/asset/keeper/msg_server_test.go b/x/asset/keeper/msg_server_test.go
index 7fc97b3..1a6c45e 100644
--- a/x/asset/keeper/msg_server_test.go
+++ b/x/asset/keeper/msg_server_test.go
@@ -52,9 +52,20 @@ func TestMsgServerNotarizeAssetMachineNotFound(t *testing.T) {
 	assert.EqualError(t, err, "machine not found")
 }
 
-func TestMsgServerNotarizeAssetInvalidAsset(t *testing.T) {
+func TestMsgServerNotarizeAssetInvalidAssetSignatureType(t *testing.T) {
 	_, pk := sample.ExtendedKeyPair(config.PlmntNetParams)
-	msg := types.NewMsgNotarizeAsset(pk, "cid", "sign", pk)
+	hex_string := hex.EncodeToString([]byte("cid"))
+	msg := types.NewMsgNotarizeAsset(pk, hex_string, "sign", pk)
+	msgServer, ctx := setupMsgServer(t)
+	_, err := msgServer.NotarizeAsset(ctx, msg)
+	assert.EqualError(t, err, "invalid signature hex string")
+}
+
+func TestMsgServerNotarizeAssetInvalidAssetSignature(t *testing.T) {
+	_, pk := sample.ExtendedKeyPair(config.PlmntNetParams)
+	hex_string_cid := hex.EncodeToString([]byte("cid"))
+	hex_string_sid := hex.EncodeToString([]byte("sign"))
+	msg := types.NewMsgNotarizeAsset(pk, hex_string_cid, hex_string_sid, pk)
 	msgServer, ctx := setupMsgServer(t)
 	_, err := msgServer.NotarizeAsset(ctx, msg)
 	assert.EqualError(t, err, "invalid signature")
diff --git a/x/machine/keeper/msg_server_attest_machine.go b/x/machine/keeper/msg_server_attest_machine.go
index 61bb9a9..ca94772 100644
--- a/x/machine/keeper/msg_server_attest_machine.go
+++ b/x/machine/keeper/msg_server_attest_machine.go
@@ -33,9 +33,9 @@ func (k msgServer) AttestMachine(goCtx context.Context, msg *types.MsgAttestMach
 		return nil, errors.New("trust anchor has already been used for attestation")
 	}
 
-	isValidMachineId := util.ValidateSignature(msg.Machine.MachineId, msg.Machine.MachineIdSignature, msg.Machine.MachineId)
+	isValidMachineId, err := util.ValidateSignature(msg.Machine.MachineId, msg.Machine.MachineIdSignature, msg.Machine.MachineId)
 	if !isValidMachineId {
-		return nil, errors.New("invalid machine id")
+		return nil, err
 	}
 
 	isValidIssuerPlanetmint := validateExtendedPublicKey(msg.Machine.IssuerPlanetmint, config.PlmntNetParams)
@@ -59,9 +59,11 @@ func (k msgServer) AttestMachine(goCtx context.Context, msg *types.MsgAttestMach
 
 	k.StoreMachine(ctx, *msg.Machine)
 	k.StoreMachineIndex(ctx, *msg.Machine)
-	k.StoreTrustAnchor(ctx, ta, true)
-
-	return &types.MsgAttestMachineResponse{}, nil
+	err = k.StoreTrustAnchor(ctx, ta, true)
+	if err != nil {
+		return nil, err
+	}
+	return &types.MsgAttestMachineResponse{}, err
 }
 
 func validateExtendedPublicKey(issuer string, cfg chaincfg.Params) bool {
diff --git a/x/machine/keeper/msg_server_register_trust_anchor.go b/x/machine/keeper/msg_server_register_trust_anchor.go
index 99a329f..eda6209 100644
--- a/x/machine/keeper/msg_server_register_trust_anchor.go
+++ b/x/machine/keeper/msg_server_register_trust_anchor.go
@@ -23,9 +23,8 @@ func (k msgServer) RegisterTrustAnchor(goCtx context.Context, msg *types.MsgRegi
 		return nil, errors.New("trust anchor is already registered")
 	}
 
-	k.StoreTrustAnchor(ctx, *msg.TrustAnchor, false)
-
-	return &types.MsgRegisterTrustAnchorResponse{}, nil
+	err := k.StoreTrustAnchor(ctx, *msg.TrustAnchor, false)
+	return &types.MsgRegisterTrustAnchorResponse{}, err
 }
 
 func validatePublicKey(pubkey string) bool {
diff --git a/x/machine/keeper/trust_anchor.go b/x/machine/keeper/trust_anchor.go
index 782b89d..cfe9c43 100644
--- a/x/machine/keeper/trust_anchor.go
+++ b/x/machine/keeper/trust_anchor.go
@@ -1,13 +1,15 @@
 package keeper
 
 import (
+	"encoding/hex"
+	"errors"
 	"planetmint-go/x/machine/types"
 
 	"github.com/cosmos/cosmos-sdk/store/prefix"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 )
 
-func (k Keeper) StoreTrustAnchor(ctx sdk.Context, ta types.TrustAnchor, activated bool) {
+func (k Keeper) StoreTrustAnchor(ctx sdk.Context, ta types.TrustAnchor, activated bool) error {
 	store := prefix.NewStore(ctx.KVStore(k.taStoreKey), types.KeyPrefix(types.TrustAnchorKey))
 	// if activated is set to true then store 1 else 0
 	var appendValue []byte
@@ -16,12 +18,21 @@ func (k Keeper) StoreTrustAnchor(ctx sdk.Context, ta types.TrustAnchor, activate
 	} else {
 		appendValue = []byte{0}
 	}
-	store.Set(GetTrustAnchorBytes(ta.Pubkey), appendValue)
+	pubKey_bytes, err := getTrustAnchorBytes(ta.Pubkey)
+	if err != nil {
+		return errors.New("the given public key could not be decoded (malformed string)")
+	}
+	store.Set(pubKey_bytes, appendValue)
+	return nil
 }
 
 func (k Keeper) GetTrustAnchor(ctx sdk.Context, pubKey string) (val types.TrustAnchor, activated bool, found bool) {
 	store := prefix.NewStore(ctx.KVStore(k.taStoreKey), types.KeyPrefix(types.TrustAnchorKey))
-	trustAnchorActivated := store.Get(GetTrustAnchorBytes(pubKey))
+	pubKey_bytes, err := getTrustAnchorBytes(pubKey)
+	if err != nil {
+		return val, false, false
+	}
+	trustAnchorActivated := store.Get(pubKey_bytes)
 
 	if trustAnchorActivated == nil {
 		return val, false, false
@@ -36,6 +47,6 @@ func (k Keeper) GetTrustAnchor(ctx sdk.Context, pubKey string) (val types.TrustA
 	}
 }
 
-func GetTrustAnchorBytes(pubKey string) []byte {
-	return []byte(pubKey)
+func getTrustAnchorBytes(pubKey string) ([]byte, error) {
+	return hex.DecodeString(pubKey)
 }
diff --git a/x/machine/keeper/trust_anchor_test.go b/x/machine/keeper/trust_anchor_test.go
index 1e7b3ac..78c0903 100644
--- a/x/machine/keeper/trust_anchor_test.go
+++ b/x/machine/keeper/trust_anchor_test.go
@@ -1,7 +1,9 @@
 package keeper_test
 
 import (
+	"encoding/hex"
 	"fmt"
+	"strings"
 	"testing"
 
 	keepertest "planetmint-go/testutil/keeper"
@@ -13,24 +15,30 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-func createNTrustAnchor(keeper *keeper.Keeper, ctx sdk.Context, n int) []types.TrustAnchor {
+func createNTrustAnchor(t *testing.T, keeper *keeper.Keeper, ctx sdk.Context, n int) []types.TrustAnchor {
 	items := make([]types.TrustAnchor, n)
 	for i := range items {
-		items[i].Pubkey = fmt.Sprintf("pubkey%v", i)
+		pk := fmt.Sprintf("pubkey%v", i)
+		if i%2 == 1 {
+			pk = strings.ToUpper(pk)
+		}
+
+		items[i].Pubkey = hex.EncodeToString([]byte(pk))
 		var activated bool
 		if i%2 == 1 {
 			activated = true
 		} else {
 			activated = false
 		}
-		keeper.StoreTrustAnchor(ctx, items[i], activated)
+		err := keeper.StoreTrustAnchor(ctx, items[i], activated)
+		assert.False(t, (err != nil))
 	}
 	return items
 }
 
 func TestGetTrustAnchor(t *testing.T) {
 	keeper, ctx := keepertest.MachineKeeper(t)
-	items := createNTrustAnchor(keeper, ctx, 10)
+	items := createNTrustAnchor(t, keeper, ctx, 10)
 	for i, item := range items {
 		ta, activated, found := keeper.GetTrustAnchor(ctx, item.Pubkey)
 		assert.True(t, found)
@@ -45,11 +53,29 @@ func TestGetTrustAnchor(t *testing.T) {
 
 func TestUpdateTrustAnchor(t *testing.T) {
 	keeper, ctx := keepertest.MachineKeeper(t)
-	items := createNTrustAnchor(keeper, ctx, 10)
+	items := createNTrustAnchor(t, keeper, ctx, 10)
 	for _, item := range items {
 		ta, activated, _ := keeper.GetTrustAnchor(ctx, item.Pubkey)
 		if !activated {
-			keeper.StoreTrustAnchor(ctx, ta, true)
+			err := keeper.StoreTrustAnchor(ctx, ta, true)
+			assert.False(t, (err != nil))
+		}
+	}
+
+	for _, item := range items {
+		_, activated, _ := keeper.GetTrustAnchor(ctx, item.Pubkey)
+		assert.True(t, activated)
+	}
+}
+
+func TestUpdateTrustAnchorInvalidPubKey(t *testing.T) {
+	keeper, ctx := keepertest.MachineKeeper(t)
+	items := createNTrustAnchor(t, keeper, ctx, 10)
+	for _, item := range items {
+		ta, activated, _ := keeper.GetTrustAnchor(ctx, item.Pubkey)
+		if !activated {
+			err := keeper.StoreTrustAnchor(ctx, ta, true)
+			assert.False(t, (err != nil))
 		}
 	}