Skip to content

Commit

Permalink
feat: simplify the import / export API for the Keybase (gnolang#3285)
Browse files Browse the repository at this point in the history
## Description

This PR greatly simplifies the `Keybase` interface in regards to key
imports / exports, by removing redundant methods.

The goal of the PR is to put the application-level key armor management
outside the keybase (not including storage), towards the caller. In
fact, this is the reason why the `Keybase` has a chaotic API for imports
/ exports.

I've preserved existing `gnokey` functionality -- this is unchanged.
I will update `gnokey` imports / exports in a separate PR, to make the
flag usage make more sense.

**BREAKING CHANGE:**
- Removed 9 `Keybase` methods, and added 2 new ones (that replace them)

cc @jefft0 

<details><summary>Contributors' checklist...</summary>

- [x] Added new tests, or not needed, or not feasible
- [x] Provided an example (e.g. screenshot) to aid review or the PR is
self-explanatory
- [x] Updated the official documentation or not needed
- [ ] No breaking changes were made, or a `BREAKING CHANGE: xxx` message
was included in the description
- [ ] Added references to related issues and PRs
- [ ] Provided any useful hints for running manual tests
</details>
  • Loading branch information
zivkovicmilos authored and omarsy committed Dec 18, 2024
1 parent bdd1c2a commit 2aa86c9
Show file tree
Hide file tree
Showing 9 changed files with 247 additions and 354 deletions.
4 changes: 2 additions & 2 deletions contribs/gnodev/cmd/gnobro/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -429,14 +429,14 @@ func getSignerForAccount(io commands.IO, address string, kb keys.Keybase, cfg *b
}

// try empty password first
if _, err := kb.ExportPrivKeyUnsafe(address, ""); err != nil {
if _, err := kb.ExportPrivKey(address, ""); err != nil {
prompt := fmt.Sprintf("[%.10s] Enter password:", address)
signer.Password, err = io.GetPassword(prompt, true)
if err != nil {
return nil, fmt.Errorf("error while reading password: %w", err)
}

if _, err := kb.ExportPrivKeyUnsafe(address, signer.Password); err != nil {
if _, err := kb.ExportPrivKey(address, signer.Password); err != nil {
return nil, fmt.Errorf("invalid password: %w", err)
}
}
Expand Down
42 changes: 13 additions & 29 deletions tm2/pkg/crypto/keys/client/export.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (

"github.com/gnolang/gno/tm2/pkg/commands"
"github.com/gnolang/gno/tm2/pkg/crypto/keys"
"github.com/gnolang/gno/tm2/pkg/crypto/keys/armor"
)

type ExportCfg struct {
Expand Down Expand Up @@ -88,24 +89,19 @@ func execExport(cfg *ExportCfg, io commands.IO) error {
)
}

var (
armor string
exportErr error
)
var keyArmor string

if cfg.Unsafe {
// Generate the unencrypted armor
armor, exportErr = kb.ExportPrivKeyUnsafe(
cfg.NameOrBech32,
decryptPassword,
)
// Export the private key from the keybase
privateKey, err := kb.ExportPrivKey(cfg.NameOrBech32, decryptPassword)
if err != nil {
return fmt.Errorf("unable to export private key, %w", err)
}

privk, err := kb.ExportPrivateKeyObject(cfg.NameOrBech32, decryptPassword)
if err != nil {
panic(err)
}
if cfg.Unsafe {
io.Printf("privk:\n%x\n", privateKey.Bytes())

io.Printf("privk:\n%x\n", privk.Bytes())
// Generate the private key armor
keyArmor = armor.ArmorPrivateKey(privateKey)
} else {
// Get the armor encrypt password
encryptPassword, err := io.GetCheckPassword(
Expand All @@ -122,25 +118,13 @@ func execExport(cfg *ExportCfg, io commands.IO) error {
)
}

// Generate the encrypted armor
armor, exportErr = kb.ExportPrivKey(
cfg.NameOrBech32,
decryptPassword,
encryptPassword,
)
}

if exportErr != nil {
return fmt.Errorf(
"unable to export the private key, %w",
exportErr,
)
keyArmor = armor.EncryptArmorPrivKey(privateKey, encryptPassword)
}

// Write the armor to disk
if err := os.WriteFile(
cfg.OutputPath,
[]byte(armor),
[]byte(keyArmor),
0o644,
); err != nil {
return fmt.Errorf(
Expand Down
47 changes: 25 additions & 22 deletions tm2/pkg/crypto/keys/client/import.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ import (
"os"

"github.com/gnolang/gno/tm2/pkg/commands"
"github.com/gnolang/gno/tm2/pkg/crypto"
"github.com/gnolang/gno/tm2/pkg/crypto/keys"
"github.com/gnolang/gno/tm2/pkg/crypto/keys/armor"
)

type ImportCfg struct {
Expand Down Expand Up @@ -77,7 +79,7 @@ func execImport(cfg *ImportCfg, io commands.IO) error {
}

// Read the raw encrypted armor
armor, err := os.ReadFile(cfg.ArmorPath)
keyArmor, err := os.ReadFile(cfg.ArmorPath)
if err != nil {
return fmt.Errorf(
"unable to read armor from path %s, %w",
Expand Down Expand Up @@ -120,33 +122,34 @@ func execImport(cfg *ImportCfg, io commands.IO) error {
)
}

var privateKey crypto.PrivKey

if cfg.Unsafe {
// Import the unencrypted private key
if err := kb.ImportPrivKeyUnsafe(
cfg.KeyName,
string(armor),
encryptPassword,
); err != nil {
return fmt.Errorf(
"unable to import the unencrypted private key, %w",
err,
)
// Un-armor the private key
privateKey, err = armor.UnarmorPrivateKey(string(keyArmor))
if err != nil {
return fmt.Errorf("unable to unarmor private key, %w", err)
}
} else {
// Import the encrypted private key
if err := kb.ImportPrivKey(
cfg.KeyName,
string(armor),
decryptPassword,
encryptPassword,
); err != nil {
return fmt.Errorf(
"unable to import the encrypted private key, %w",
err,
)
// Decrypt the armor
privateKey, err = armor.UnarmorDecryptPrivKey(string(keyArmor), decryptPassword)
if err != nil {
return fmt.Errorf("unable to decrypt private key armor, %w", err)
}
}

// Import the private key
if err := kb.ImportPrivKey(
cfg.KeyName,
privateKey,
encryptPassword,
); err != nil {
return fmt.Errorf(
"unable to import the encrypted private key, %w",
err,
)
}

io.Printfln("Successfully imported private key %s", cfg.KeyName)

return nil
Expand Down
75 changes: 75 additions & 0 deletions tm2/pkg/crypto/keys/client/import_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,14 @@ package client
import (
"fmt"
"io"
"os"
"strings"
"testing"

"github.com/gnolang/gno/tm2/pkg/commands"
"github.com/gnolang/gno/tm2/pkg/testutils"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

type testImportKeyOpts struct {
Expand Down Expand Up @@ -154,6 +156,8 @@ func TestImport_ImportKey(t *testing.T) {
}

func TestImport_ImportKeyWithEmptyName(t *testing.T) {
t.Parallel()

// Generate a temporary key-base directory
_, kbHome := newTestKeybase(t)
err := importKey(
Expand All @@ -168,3 +172,74 @@ func TestImport_ImportKeyWithEmptyName(t *testing.T) {
assert.Error(t, err)
assert.EqualError(t, err, "name shouldn't be empty")
}

func TestImport_ImportKeyInvalidArmor(t *testing.T) {
t.Parallel()

_, kbHome := newTestKeybase(t)

armorFile, err := os.CreateTemp("", "armor.key")
require.NoError(t, err)

defer os.Remove(armorFile.Name())

// Write invalid armor
_, err = armorFile.Write([]byte("totally valid tendermint armor"))
require.NoError(t, err)

err = importKey(
testImportKeyOpts{
testCmdKeyOptsBase: testCmdKeyOptsBase{
kbHome: kbHome,
keyName: "key-name",
unsafe: true, // expect an unencrypted private key armor
},
armorPath: armorFile.Name(),
},
strings.NewReader(
fmt.Sprintf(
"%s\n%s\n",
"",
"",
),
),
)

assert.ErrorContains(t, err, "unable to unarmor private key")
}

func TestImport_ImportKeyInvalidPKArmor(t *testing.T) {
t.Parallel()

_, kbHome := newTestKeybase(t)

armorFile, err := os.CreateTemp("", "armor.key")
require.NoError(t, err)

defer os.Remove(armorFile.Name())

// Write invalid armor
_, err = armorFile.Write([]byte("totally valid tendermint armor"))
require.NoError(t, err)

err = importKey(
testImportKeyOpts{
testCmdKeyOptsBase: testCmdKeyOptsBase{
kbHome: kbHome,
keyName: "key-name",
unsafe: false, // expect an encrypted private key armor
},
armorPath: armorFile.Name(),
},
strings.NewReader(
fmt.Sprintf(
"%s\n%s\n%s\n",
"",
"",
"",
),
),
)

assert.ErrorContains(t, err, "unable to decrypt private key armor")
}
2 changes: 1 addition & 1 deletion tm2/pkg/crypto/keys/client/verify_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func Test_execVerify(t *testing.T) {
assert.NoError(t, err)

// sign test message.
priv, err := kb.ExportPrivateKeyObject(fakeKeyName1, encPassword)
priv, err := kb.ExportPrivKey(fakeKeyName1, encPassword)
assert.NoError(t, err)
testSig, err := priv.Sign([]byte(testMsg))
assert.NoError(t, err)
Expand Down
Loading

0 comments on commit 2aa86c9

Please sign in to comment.