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

fix: In dbKeybase.writeInfo, if replacing a name entry, remove the lookup by the old address #2685

Merged
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
58 changes: 38 additions & 20 deletions tm2/pkg/crypto/keys/keybase.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,33 +115,32 @@
pub := priv.PubKey()

// Note: Once Cosmos App v1.3.1 is compulsory, it could be possible to check that pubkey and addr match
return kb.writeLedgerKey(name, pub, *hdPath), nil
return kb.writeLedgerKey(name, pub, *hdPath)
}

// CreateOffline creates a new reference to an offline keypair. It returns the
// created key info.
func (kb dbKeybase) CreateOffline(name string, pub crypto.PubKey) (Info, error) {
return kb.writeOfflineKey(name, pub), nil
return kb.writeOfflineKey(name, pub)
}

// CreateMulti creates a new reference to a multisig (offline) keypair. It
// returns the created key info.
func (kb dbKeybase) CreateMulti(name string, pub crypto.PubKey) (Info, error) {
return kb.writeMultisigKey(name, pub), nil
return kb.writeMultisigKey(name, pub)

Check warning on line 130 in tm2/pkg/crypto/keys/keybase.go

View check run for this annotation

Codecov / codecov/patch

tm2/pkg/crypto/keys/keybase.go#L130

Added line #L130 was not covered by tests
}

func (kb *dbKeybase) persistDerivedKey(seed []byte, passwd, name, fullHdPath string) (info Info, err error) {
func (kb *dbKeybase) persistDerivedKey(seed []byte, passwd, name, fullHdPath string) (Info, error) {
// create master key and derive first key:
masterPriv, ch := hd.ComputeMastersFromSeed(seed)
derivedPriv, err := hd.DerivePrivateKeyForPath(masterPriv, ch, fullHdPath)
if err != nil {
return
return nil, err

Check warning on line 138 in tm2/pkg/crypto/keys/keybase.go

View check run for this annotation

Codecov / codecov/patch

tm2/pkg/crypto/keys/keybase.go#L138

Added line #L138 was not covered by tests
}

// use possibly blank password to encrypt the private
// key and store it. User must enforce good passwords.
info = kb.writeLocalKey(name, secp256k1.PrivKeySecp256k1(derivedPriv), passwd)
return
return kb.writeLocalKey(name, secp256k1.PrivKeySecp256k1(derivedPriv), passwd)
}

// List returns the keys from storage in alphabetical order.
Expand Down Expand Up @@ -475,41 +474,60 @@
kb.db.Close()
}

func (kb dbKeybase) writeLocalKey(name string, priv crypto.PrivKey, passphrase string) Info {
func (kb dbKeybase) writeLocalKey(name string, priv crypto.PrivKey, passphrase string) (Info, error) {
// encrypt private key using passphrase
privArmor := armor.EncryptArmorPrivKey(priv, passphrase)
// make Info
pub := priv.PubKey()
info := newLocalInfo(name, pub, privArmor)
kb.writeInfo(name, info)
return info
if err := kb.writeInfo(name, info); err != nil {
return nil, err

Check warning on line 484 in tm2/pkg/crypto/keys/keybase.go

View check run for this annotation

Codecov / codecov/patch

tm2/pkg/crypto/keys/keybase.go#L484

Added line #L484 was not covered by tests
}
return info, nil
}

func (kb dbKeybase) writeLedgerKey(name string, pub crypto.PubKey, path hd.BIP44Params) Info {
func (kb dbKeybase) writeLedgerKey(name string, pub crypto.PubKey, path hd.BIP44Params) (Info, error) {
info := newLedgerInfo(name, pub, path)
kb.writeInfo(name, info)
return info
if err := kb.writeInfo(name, info); err != nil {
return nil, err

Check warning on line 492 in tm2/pkg/crypto/keys/keybase.go

View check run for this annotation

Codecov / codecov/patch

tm2/pkg/crypto/keys/keybase.go#L492

Added line #L492 was not covered by tests
}
return info, nil
}

func (kb dbKeybase) writeOfflineKey(name string, pub crypto.PubKey) Info {
func (kb dbKeybase) writeOfflineKey(name string, pub crypto.PubKey) (Info, error) {
info := newOfflineInfo(name, pub)
kb.writeInfo(name, info)
return info
if err := kb.writeInfo(name, info); err != nil {
return nil, err

Check warning on line 500 in tm2/pkg/crypto/keys/keybase.go

View check run for this annotation

Codecov / codecov/patch

tm2/pkg/crypto/keys/keybase.go#L500

Added line #L500 was not covered by tests
}
return info, nil
}

func (kb dbKeybase) writeMultisigKey(name string, pub crypto.PubKey) Info {
func (kb dbKeybase) writeMultisigKey(name string, pub crypto.PubKey) (Info, error) {

Check warning on line 505 in tm2/pkg/crypto/keys/keybase.go

View check run for this annotation

Codecov / codecov/patch

tm2/pkg/crypto/keys/keybase.go#L505

Added line #L505 was not covered by tests
info := NewMultiInfo(name, pub)
kb.writeInfo(name, info)
return info
if err := kb.writeInfo(name, info); err != nil {
return nil, err

Check warning on line 508 in tm2/pkg/crypto/keys/keybase.go

View check run for this annotation

Codecov / codecov/patch

tm2/pkg/crypto/keys/keybase.go#L507-L508

Added lines #L507 - L508 were not covered by tests
}
return info, nil

Check warning on line 510 in tm2/pkg/crypto/keys/keybase.go

View check run for this annotation

Codecov / codecov/patch

tm2/pkg/crypto/keys/keybase.go#L510

Added line #L510 was not covered by tests
}

func (kb dbKeybase) writeInfo(name string, info Info) {
func (kb dbKeybase) writeInfo(name string, info Info) error {
// write the info by key
key := infoKey(name)
oldInfob := kb.db.Get(key)
if len(oldInfob) > 0 {
// Enforce 1-to-1 name to address. Remove the lookup by the old address
oldInfo, err := readInfo(oldInfob)
if err != nil {
return err

Check warning on line 521 in tm2/pkg/crypto/keys/keybase.go

View check run for this annotation

Codecov / codecov/patch

tm2/pkg/crypto/keys/keybase.go#L521

Added line #L521 was not covered by tests
}
kb.db.DeleteSync(addrKey(oldInfo.GetAddress()))
}

serializedInfo := writeInfo(info)
kb.db.SetSync(key, serializedInfo)
// store a pointer to the infokey by address for fast lookup
kb.db.SetSync(addrKey(info.GetAddress()), key)
return nil
}

func addrKey(address crypto.Address) []byte {
Expand Down
13 changes: 13 additions & 0 deletions tm2/pkg/crypto/keys/keybase_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,19 @@ func TestKeyManagement(t *testing.T) {
require.NoError(t, err)
require.Equal(t, 1, len(keyS))

// Lookup by original i2 address
infoByAddress, err := cstore.GetByAddress(i2.GetAddress())
require.NoError(t, err)
// GetByAddress should return Info with the corresponding public key
require.Equal(t, infoByAddress.GetPubKey(), i2.GetPubKey())
// Replace n2 with a new address
mn2New := `fancy assault crane note start invite ladder ordinary gold amateur check cousin text mercy speak chuckle wine raw chief isolate swallow cushion wrist piece`
_, err = cstore.CreateAccount(n2, mn2New, bip39Passphrase, p2, 0, 0)
require.NoError(t, err)
// Check that CreateAccount removes the entry for the original address (public key)
_, err = cstore.GetByAddress(i2.GetAddress())
require.NotNil(t, err)

// addr cache gets nuked - and test skip flag
err = cstore.Delete(n2, "", true)
require.NoError(t, err)
Expand Down
Loading