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.

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

crypto/keys: remove DecodeSignature from baseKeybase #5844

Merged
merged 1 commit into from
Mar 20, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ are now replaced by human-readable expressions. This change can potentially brea
that parse log messages.
* (client) [\#5799](https://github.com/cosmos/cosmos-sdk/pull/5799) The `tx encode/decode` commands, due to change on encoding break compatibility with
older clients.
* (x/auth) [\#5844](https://github.com/cosmos/cosmos-sdk/pull/5844) `tx sign` command now returns an error when signing is attempted with offline/multisig keys.

### API Breaking Changes

Expand Down Expand Up @@ -87,6 +88,8 @@ resulted in a panic when the tx execution mode was `CheckTx`.
* (modules) [\#5569](https://github.com/cosmos/cosmos-sdk/issues/5569) `InitGenesis`, for the relevant modules, now ensures module accounts exist.
* (crypto/keys/mintkey) [\#5823](https://github.com/cosmos/cosmos-sdk/pull/5823) fix errors handling in UnarmorPubKeyBytes (underlying armoring function's
return error was not being checked).
* (crypto/keys) [\#5844](https://github.com/cosmos/cosmos-sdk/pull/5844) Keybase/Keyring `Sign()` methods no longer decode amino signatures
when method receivers are offline/multisig keys.

### State Machine Breaking

Expand Down
2 changes: 1 addition & 1 deletion crypto/keys/keybase.go
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ func (kb dbKeybase) Sign(name, passphrase string, msg []byte) (sig []byte, pub t
return SignWithLedger(info, msg)

case offlineInfo, multiInfo:
return kb.base.DecodeSignature(info, msg)
return nil, info.GetPubKey(), errors.New("cannot sign with offline keys")
}

sig, err = priv.Sign(msg)
Expand Down
31 changes: 0 additions & 31 deletions crypto/keys/keybase_base.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,6 @@
package keys

import (
"bufio"
"fmt"
"os"

"github.com/cosmos/go-bip39"
"github.com/pkg/errors"
tmcrypto "github.com/tendermint/tendermint/crypto"
Expand Down Expand Up @@ -104,33 +100,6 @@ func SecpPrivKeyGen(bz []byte) tmcrypto.PrivKey {
return secp256k1.PrivKeySecp256k1(bzArr)
}

// DecodeSignature decodes a an length-prefixed binary signature from standard input
// and return it as a byte slice.
func (kb baseKeybase) DecodeSignature(info Info, msg []byte) (sig []byte, pub tmcrypto.PubKey, err error) {
_, err = fmt.Fprintf(os.Stderr, "Message to sign:\n\n%s\n", msg)
if err != nil {
return nil, nil, err
}

buf := bufio.NewReader(os.Stdin)
_, err = fmt.Fprintf(os.Stderr, "\nEnter Amino-encoded signature:\n")
if err != nil {
return nil, nil, err
}

// will block until user inputs the signature
signed, err := buf.ReadString('\n')
if err != nil {
return nil, nil, err
}

if err := CryptoCdc.UnmarshalBinaryLengthPrefixed([]byte(signed), sig); err != nil {
return nil, nil, errors.Wrap(err, "failed to decode signature")
}

return sig, info.GetPubKey(), nil
}

// CreateAccount creates an account Info object.
func (kb baseKeybase) CreateAccount(
keyWriter keyWriter, name, mnemonic, bip39Passphrase, encryptPasswd, hdPath string, algo SigningAlgo,
Expand Down
5 changes: 2 additions & 3 deletions crypto/keys/keybase_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,7 @@
package keys

import (
"errors"
"fmt"
"io"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -280,7 +278,8 @@ func TestSignVerify(t *testing.T) {

// Now try to sign data with a secret-less key
_, _, err = cstore.Sign(n3, p3, d3)
require.True(t, errors.Is(io.EOF, err))
require.Error(t, err)
require.Equal(t, "cannot sign with offline keys", err.Error())
}

func assertPassword(t *testing.T, cstore Keybase, name, pass, badpass string) {
Expand Down
2 changes: 1 addition & 1 deletion crypto/keys/keyring.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ func (kb keyringKeybase) Sign(name, passphrase string, msg []byte) (sig []byte,
return SignWithLedger(info, msg)

case offlineInfo, multiInfo:
return kb.base.DecodeSignature(info, msg)
return nil, info.GetPubKey(), errors.New("cannot sign with offline keys")
}

sig, err = priv.Sign(msg)
Expand Down
3 changes: 2 additions & 1 deletion crypto/keys/keyring_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,8 @@ func TestLazySignVerifyKeyRing(t *testing.T) {

// Now try to sign data with a secret-less key
_, _, err = kb.Sign(n3, p3, d3)
require.NotNil(t, err)
require.Error(t, err)
require.Equal(t, "cannot sign with offline keys", err.Error())
}

func TestLazyExportImportKeyRing(t *testing.T) {
Expand Down