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 keys migrate command #8703

Merged
merged 3 commits into from
Feb 25, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
40 changes: 10 additions & 30 deletions client/keys/migrate.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import (
// is not needed for importing into the Keyring keystore.
const migratePassphrase = "NOOP_PASSPHRASE"

const flagOldHome = "old-home"

// MigrateCommand migrates key information from legacy keybase to OS secret store.
func MigrateCommand() *cobra.Command {
cmd := &cobra.Command{
Expand All @@ -35,16 +37,18 @@ It is recommended to run in 'dry-run' mode first to verify all key migration mat
RunE: runMigrateCmd,
}

cmd.Flags().String(flagOldHome, "", "The root directory of the old keyring")
cmd.Flags().Bool(flags.FlagDryRun, false, "Run migration without actually persisting any changes to the new Keybase")
return cmd
}

func runMigrateCmd(cmd *cobra.Command, args []string) error {
oldRootDir, _ := cmd.Flags().GetString(flagOldHome)
rootDir, _ := cmd.Flags().GetString(flags.FlagHome)

// instantiate legacy keybase
var legacyKb keyring.LegacyKeybase
legacyKb, err := NewLegacyKeyBaseFromDir(rootDir)
legacyKb, err := NewLegacyKeyBaseFromDir(oldRootDir)
if err != nil {
return err
}
Expand Down Expand Up @@ -91,11 +95,11 @@ func runMigrateCmd(cmd *cobra.Command, args []string) error {
return nil
}

for _, key := range oldKeys {
keyName := key.GetName()
keyType := key.GetType()
for _, oldInfo := range oldKeys {
keyName := oldInfo.GetName()
keyType := oldInfo.GetType()

cmd.PrintErrf("Migrating key: '%s (%s)' ...\n", key.GetName(), keyType)
cmd.PrintErrf("Migrating key: '%s (%s)' ...\n", keyName, keyType)

// allow user to skip migrating specific keys
ok, err := input.GetConfirmation("Skip key migration?", buf, cmd.ErrOrStderr())
Expand All @@ -106,35 +110,11 @@ func runMigrateCmd(cmd *cobra.Command, args []string) error {
continue
}

if keyType != keyring.TypeLocal {
pubkeyArmor, err := legacyKb.ExportPubKey(keyName)
if err != nil {
return err
}

if err := migrator.ImportPubKey(keyName, pubkeyArmor, keyType); err != nil {
return err
}

continue
}

password, err := input.GetPassword("Enter passphrase to decrypt key:", buf)
err = migrator.MigrateInfo(oldInfo)
if err != nil {
return err
}

// NOTE: A passphrase is not actually needed here as when the key information
// is imported into the Keyring-based Keybase it only needs the password
// (see: writeLocalKey).
armoredPriv, err := legacyKb.ExportPrivKey(keyName, password, migratePassphrase)
if err != nil {
return err
}

if err := migrator.ImportPrivKey(keyName, armoredPriv, migratePassphrase); err != nil {
return err
}
}
cmd.Print("Migration Complete")

Expand Down
7 changes: 0 additions & 7 deletions codec/legacy/codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package legacy
import (
"github.com/cosmos/cosmos-sdk/codec"
cryptocodec "github.com/cosmos/cosmos-sdk/crypto/codec"
"github.com/cosmos/cosmos-sdk/crypto/keys/multisig"
cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
)

Expand All @@ -30,9 +29,3 @@ func PubKeyFromBytes(pubKeyBytes []byte) (pubKey cryptotypes.PubKey, err error)
err = Cdc.UnmarshalBinaryBare(pubKeyBytes, &pubKey)
return
}

// AminoPubKeyFromBytes unmarshals public key bytes and returns a PubKey
func AminoPubKeyFromBytes(pubKeyBytes []byte) (pubKey multisig.LegacyAminoPubKey, err error) {
err = Cdc.UnmarshalBinaryBare(pubKeyBytes, &pubKey)
return
}
19 changes: 16 additions & 3 deletions crypto/keyring/keyring.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,10 @@ type Importer interface {

// ImportPubKey imports ASCII armored public keys.
ImportPubKey(uid string, armor string, keyType KeyType) error
Copy link
Contributor Author

@amaury1093 amaury1093 Feb 25, 2021

Choose a reason for hiding this comment

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

ImportPubKeyis only used in tests now. I propose to remove it in a subsequent PR (to not introduce breaking change). For reasons, see #8703 (comment).


// MigrateInfo takes a keyring.Info (in practise, from an old keyring), and
// writes it to the current keyring.
MigrateInfo(oldInfo Info) error
}

// Exporter is implemented by key stores that support export of public and private keys.
Expand Down Expand Up @@ -305,20 +309,20 @@ func (ks keystore) ImportPubKey(uid string, armor string, keyType KeyType) error
return err
}

pubKey, err := legacy.AminoPubKeyFromBytes(pubBytes)
pubKey, err := legacy.PubKeyFromBytes(pubBytes)
if err != nil {
return err
}

if keyType == TypeMulti {
_, err = ks.writeMultisigKey(uid, &pubKey)
_, err = ks.writeMultisigKey(uid, pubKey)
if err != nil {
return err
}
}

if keyType == TypeOffline || keyType == TypeLedger {
_, err = ks.writeOfflineKey(uid, &pubKey, hd.PubKeyType(algo))
_, err = ks.writeOfflineKey(uid, pubKey, hd.PubKeyType(algo))
if err != nil {
return err
}
Expand All @@ -327,6 +331,15 @@ func (ks keystore) ImportPubKey(uid string, armor string, keyType KeyType) error
return nil
}

// MigrateInfo implements Importer.MigrateInfo.
func (ks keystore) MigrateInfo(oldInfo Info) error {
Copy link
Contributor Author

@amaury1093 amaury1093 Feb 25, 2021

Choose a reason for hiding this comment

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

This is only for Type{Multi,Ledger,Offline}.

Before

The old migration logic was inside ImportPubKey: we read data from the old Info, decode it (depending on the type), extract pubkey, and create and write a new Info in the new keyring.

In the process, we completely lose the Info: we were migrating old TypeMulti and old TypeLedger into new TypeOffline. #8639 (comment)

After

We don't decode Info anymore (to extract pubkey etc). Since the infos didn't change, we just copy/paste the old Info into the new keyring. It's imo much simpler.

if _, err := ks.Key(oldInfo.GetName()); err == nil {
return fmt.Errorf("cannot overwrite key: %s", oldInfo.GetName())
}

return ks.writeInfo(oldInfo)
}

func (ks keystore) Sign(uid string, msg []byte) ([]byte, types.PubKey, error) {
info, err := ks.Key(uid)
if err != nil {
Expand Down