Skip to content

Commit

Permalink
Reenable PublicKey initialisation and allow public keys to be unset
Browse files Browse the repository at this point in the history
Signed-off-by: Silas Davis <[email protected]>
  • Loading branch information
Silas Davis committed Jul 27, 2018
1 parent 0d0ff89 commit d62c34f
Show file tree
Hide file tree
Showing 10 changed files with 96 additions and 19 deletions.
12 changes: 9 additions & 3 deletions crypto/crypto.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@ import (
type CurveType uint32

const (
CurveTypeSecp256k1 CurveType = iota
CurveTypeUnset CurveType = iota
CurveTypeEd25519
CurveTypeSecp256k1
)

func (k CurveType) String() string {
Expand All @@ -17,6 +18,8 @@ func (k CurveType) String() string {
return "secp256k1"
case CurveTypeEd25519:
return "ed25519"
case CurveTypeUnset:
return ""
default:
return "unknown"
}
Expand All @@ -27,6 +30,8 @@ func (k CurveType) ABCIType() string {
return "secp256k1"
case CurveTypeEd25519:
return "ed25519"
case CurveTypeUnset:
return ""
default:
return "unknown"
}
Expand All @@ -43,9 +48,10 @@ func CurveTypeFromString(s string) (CurveType, error) {
return CurveTypeSecp256k1, nil
case "ed25519":
return CurveTypeEd25519, nil
case "":
return CurveTypeUnset, nil
default:
var k CurveType
return k, ErrInvalidCurve(s)
return CurveTypeUnset, ErrInvalidCurve(s)
}
}

Expand Down
5 changes: 5 additions & 0 deletions crypto/private_key.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@ func PublicKeyFromBytes(bs []byte, curveType CurveType) (PublicKey, error) {
return PublicKey{}, fmt.Errorf("bytes passed have length %v but secp256k1 public keys have %v bytes",
len(bs), btcec.PubKeyBytesLenCompressed)
}
case CurveTypeUnset:
if len(bs) > 0 {
return PublicKey{}, fmt.Errorf("attempting to create an 'unset' PublicKey but passed non-empty key bytes: %X", bs)
}
return PublicKey{}, nil
default:
return PublicKey{}, ErrInvalidCurve(curveType)
}
Expand Down
20 changes: 14 additions & 6 deletions crypto/public_key.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,22 +73,30 @@ func (p PublicKey) IsValid() bool {
return publicKeyLength != 0 && publicKeyLength == len(p.Key)
}

func (p PublicKey) Verify(msg []byte, signature Signature) bool {
func (p PublicKey) Verify(msg []byte, signature Signature) error {
switch p.CurveType {
case CurveTypeUnset:
return fmt.Errorf("public key is unset")
case CurveTypeEd25519:
return ed25519.Verify(p.Key, msg, signature)
if ed25519.Verify(p.Key, msg, signature) {
return nil
}
return fmt.Errorf("'%X' is not a valid ed25519 signature for message: %X", signature, msg)
case CurveTypeSecp256k1:
pub, err := btcec.ParsePubKey(p.Key, btcec.S256())
if err != nil {
return false
return fmt.Errorf("could not parse secp256k1 public key: %v", err)
}
sig, err := btcec.ParseDERSignature(signature, btcec.S256())
if err != nil {
return false
return fmt.Errorf("could not parse DER signature for secp256k1 key: %v", err)
}
if sig.Verify(msg, pub) {
return nil
}
return sig.Verify(msg, pub)
return fmt.Errorf("'%X' is not a valid secp256k1 signature for message: %X", signature, msg)
default:
panic(fmt.Sprintf("invalid curve type"))
return fmt.Errorf("invalid curve type")
}
}

Expand Down
2 changes: 1 addition & 1 deletion execution/exec/tx_execution.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ func (txe *TxExecution) Call(call *CallEvent, exception *errors.Exception) {

func (txe *TxExecution) GovernAccount(governAccount *GovernAccountEvent, exception *errors.Exception) {
txe.Append(&Event{
Header: txe.Header(TypeCall, EventStringGovernAccount(governAccount.AccountUpdate.Address), exception),
Header: txe.Header(TypeGovernAccount, EventStringGovernAccount(governAccount.AccountUpdate.Address), exception),
GovernAccount: governAccount,
})
}
Expand Down
15 changes: 15 additions & 0 deletions execution/execution.go
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,21 @@ func (exe *executor) Execute(txEnv *txs.Envelope) (txe *exec.TxExecution, err er
return nil, err
}

// Initialise public keys for accounts we have seen
for _, sig := range txEnv.Signatories {
// pointer dereferences are safe since txEnv.Validate() is run by txEnv.Verify() above which checks they are
// non-nil
acc, err := state.GetMutableAccount(exe.stateCache, *sig.Address)
if err != nil {
return nil, fmt.Errorf("error getting account on which to set public key: %v", *sig.Address)
}
acc.SetPublicKey(*sig.PublicKey)
err = exe.stateCache.UpdateAccount(acc)
if err != nil {
return nil, fmt.Errorf("error updating account after setting public key: %v", err)
}
}

if txExecutor, ok := exe.contexts[txEnv.Tx.Type()]; ok {
// Establish new TxExecution
txe := exe.blockExecution.Tx(txEnv)
Expand Down
2 changes: 2 additions & 0 deletions integration/governance/governance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,8 @@ func TestAlterAmount(t *testing.T) {
ca, err := qcli.GetAccount(context.Background(), &rpcquery.GetAccountParam{Address: acc.Address()})
require.NoError(t, err)
assert.Equal(t, amount, ca.Balance)
// Check we haven't altered permissions
assert.Equal(t, genesisDoc.Accounts[5].Permissions, ca.Permissions)
}

func TestAlterPermissions(t *testing.T) {
Expand Down
20 changes: 20 additions & 0 deletions integration/rpctransact/transact_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (

"time"

"github.com/hyperledger/burrow/crypto"
"github.com/hyperledger/burrow/execution/exec"
"github.com/hyperledger/burrow/integration/rpctest"
"github.com/hyperledger/burrow/rpc/rpcevents"
Expand All @@ -38,6 +39,25 @@ import (
var inputAccount = rpctest.PrivateAccounts[0]
var inputAddress = inputAccount.Address()

func TestInputAccountPublicKeySet(t *testing.T) {
input := rpctest.PrivateAccounts[9]
tcli := rpctest.NewTransactClient(t, testConfig.RPC.GRPC.ListenAddress)
qcli := rpctest.NewQueryClient(t, testConfig.RPC.GRPC.ListenAddress)
acc, err := qcli.GetAccount(context.Background(), &rpcquery.GetAccountParam{Address: input.Address()})
require.NoError(t, err)

// Account PublicKey should be initially unset
assert.Equal(t, crypto.PublicKey{}, acc.PublicKey)

// Sign with this account - should set public key
rpctest.CreateContract(t, tcli, input.Address())
acc, err = qcli.GetAccount(context.Background(), &rpcquery.GetAccountParam{Address: input.Address()})

// Check public key set
require.NoError(t, err)
assert.Equal(t, input.PublicKey(), acc.PublicKey)
}

func TestBroadcastTxLocallySigned(t *testing.T) {
qcli := rpctest.NewQueryClient(t, testConfig.RPC.GRPC.ListenAddress)
acc, err := qcli.GetAccount(context.Background(), &rpcquery.GetAccountParam{
Expand Down
6 changes: 3 additions & 3 deletions keys/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,9 +144,9 @@ func (k *KeyStore) Verify(ctx context.Context, in *VerifyRequest) (*VerifyRespon
if err != nil {
return nil, err
}
match := pubkey.Verify(in.GetMessage(), sig)
if !match {
return nil, fmt.Errorf("Signature does not match")
err = pubkey.Verify(in.GetMessage(), sig)
if err != nil {
return nil, err
}

return &VerifyResponse{}, nil
Expand Down
12 changes: 8 additions & 4 deletions permission/account_permissions.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,14 @@ func (ap *AccountPermissions) RmRole(role string) bool {
func (ap *AccountPermissions) Clone() AccountPermissions {
// clone base permissions
basePermissionsClone := ap.Base
// clone roles []string
rolesClone := make([]string, len(ap.Roles))
// strings are immutable so copy suffices
copy(rolesClone, ap.Roles)
var rolesClone []string
// It helps if we normalise empty roles to []string(nil) rather than []string{}
if len(ap.Roles) > 0 {
// clone roles []string
rolesClone = make([]string, len(ap.Roles))
// strings are immutable so copy suffices
copy(rolesClone, ap.Roles)
}

return AccountPermissions{
Base: basePermissionsClone,
Expand Down
21 changes: 19 additions & 2 deletions txs/envelope.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,22 @@ func (txEnv *Envelope) Validate() error {
if len(txEnv.Signatories) == 0 {
return fmt.Errorf("transaction envelope contains no (successfully unmarshalled) signatories")
}
for i, sig := range txEnv.Signatories {
err := sig.Validate()
if err != nil {
return fmt.Errorf("Signatory %v is invalid: %v", i, err)
}
}
return nil
}

func (sig *Signatory) Validate() error {
if sig.Address == nil {
return fmt.Errorf("has nil Address: %v", sig)
}
if sig.PublicKey == nil {
return fmt.Errorf("has nil PublicKey: %v", sig)
}
return nil
}

Expand Down Expand Up @@ -102,8 +118,9 @@ func (txEnv *Envelope) Verify(getter state.AccountGetter, chainID string) error
return fmt.Errorf("signatory %v has address %v but input %v has address %v",
i, *s.Address, i, inputs[i].Address)
}
if !s.PublicKey.Verify(signBytes, s.Signature) {
return fmt.Errorf("invalid signature in signatory %v ", *s.Address)
err = s.PublicKey.Verify(signBytes, s.Signature)
if err != nil {
return fmt.Errorf("invalid signature in signatory %v: %v", *s.Address, err)
}
}
return nil
Expand Down

0 comments on commit d62c34f

Please sign in to comment.