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

Feat/Migrated to bcrypt key derivation to argon2 and aead symethric encryption #15419

Closed
wants to merge 17 commits into from
Closed
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ Ref: https://keepachangelog.com/en/1.0.0/

### Improvements

* (crypto) [#3129](https://github.com/cosmos/cosmos-sdk/pull/3129) New armor and keyring key derivation uses aead and encryption uses chacha20poly
* (x/gov) [#15554](https://github.com/cosmos/cosmos-sdk/pull/15554) Add proposal result log in `active_proposal` event. When a proposal passes but fails to execute, the proposal result is logged in the `active_proposal` event.
* (mempool) [#15328](https://github.com/cosmos/cosmos-sdk/pull/15328) Improve the `PriorityNonceMempool`
* Support generic transaction prioritization, instead of `ctx.Priority()`
Expand Down
84 changes: 65 additions & 19 deletions crypto/armor.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@ import (
"io"

"github.com/cometbft/cometbft/crypto"
"golang.org/x/crypto/argon2"
"golang.org/x/crypto/openpgp/armor" //nolint:staticcheck

errorsmod "cosmossdk.io/errors"
chacha20poly1305 "golang.org/x/crypto/chacha20poly1305"

"github.com/cosmos/cosmos-sdk/codec/legacy"
"github.com/cosmos/cosmos-sdk/crypto/keys/bcrypt"
Expand All @@ -29,6 +31,19 @@ const (
headerType = "type"
)

var (
kdf = "kdf"
kdfBcrypt = "bcrypt"
kdfArgon2 = "argon2"
)

const (
Argon2Time = 1
Argon2Memory = 64 * 1024
Argon2Threads = 4
Argon2KeyLen = 32
)

// BcryptSecurityParameter is security parameter var, and it can be changed within the lcd test.
// Making the bcrypt security parameter a var shouldn't be a security issue:
// One can't verify an invalid key by maliciously changing the bcrypt
Expand Down Expand Up @@ -131,7 +146,7 @@ func unarmorBytes(armorStr, blockType string) (bz []byte, header map[string]stri
func EncryptArmorPrivKey(privKey cryptotypes.PrivKey, passphrase string, algo string) string {
saltBytes, encBytes := encryptPrivKey(privKey, passphrase)
header := map[string]string{
"kdf": "bcrypt",
kdf: kdfArgon2,
"salt": fmt.Sprintf("%X", saltBytes),
}

Expand All @@ -149,15 +164,20 @@ func EncryptArmorPrivKey(privKey cryptotypes.PrivKey, passphrase string, algo st
// encrypted priv key.
func encryptPrivKey(privKey cryptotypes.PrivKey, passphrase string) (saltBytes []byte, encBytes []byte) {
saltBytes = crypto.CRandBytes(16)
key, err := bcrypt.GenerateFromPassword(saltBytes, []byte(passphrase), BcryptSecurityParameter)

key := argon2.IDKey([]byte(passphrase), saltBytes, Argon2Time, Argon2Memory, Argon2Threads, Argon2KeyLen)
privKeyBytes := legacy.Cdc.MustMarshal(privKey)

aead, err := chacha20poly1305.New(key)
if err != nil {
panic(errorsmod.Wrap(err, "error generating bcrypt key from passphrase"))
panic(errorsmod.Wrap(err, "error generating cypher from key"))
}

key = crypto.Sha256(key) // get 32 bytes
privKeyBytes := legacy.Cdc.MustMarshal(privKey)
nonce := make([]byte, aead.NonceSize(), aead.NonceSize()+len(privKeyBytes)+aead.Overhead())

encBytes = aead.Seal(nonce, nonce, privKeyBytes, nil)

return saltBytes, xsalsa20symmetric.EncryptSymmetric(privKeyBytes, key)
return saltBytes, encBytes
}

// UnarmorDecryptPrivKey returns the privkey byte slice, a string of the algo type, and an error
Expand All @@ -171,8 +191,8 @@ func UnarmorDecryptPrivKey(armorStr string, passphrase string) (privKey cryptoty
return privKey, "", fmt.Errorf("unrecognized armor type: %v", blockType)
}

if header["kdf"] != "bcrypt" {
return privKey, "", fmt.Errorf("unrecognized KDF type: %v", header["kdf"])
if header[kdf] != kdfBcrypt && header[kdf] != kdfArgon2 {
return privKey, "", fmt.Errorf("unrecognized KDF type: %v", header[kdf])
}

if header["salt"] == "" {
Expand All @@ -184,7 +204,7 @@ func UnarmorDecryptPrivKey(armorStr string, passphrase string) (privKey cryptoty
return privKey, "", fmt.Errorf("error decoding salt: %v", err.Error())
}

privKey, err = decryptPrivKey(saltBytes, encBytes, passphrase)
privKey, err = decryptPrivKey(saltBytes, encBytes, passphrase, header[kdf])

if header[headerType] == "" {
header[headerType] = defaultAlgo
Expand All @@ -193,18 +213,44 @@ func UnarmorDecryptPrivKey(armorStr string, passphrase string) (privKey cryptoty
return privKey, header[headerType], err
}

func decryptPrivKey(saltBytes []byte, encBytes []byte, passphrase string) (privKey cryptotypes.PrivKey, err error) {
key, err := bcrypt.GenerateFromPassword(saltBytes, []byte(passphrase), BcryptSecurityParameter)
if err != nil {
return privKey, errorsmod.Wrap(err, "error generating bcrypt key from passphrase")
}
func decryptPrivKey(saltBytes []byte, encBytes []byte, passphrase string, kdf string) (privKey cryptotypes.PrivKey, err error) {
// Key derivation
var (
key []byte
privKeyBytes []byte
)

// Since the argon2 key derivation and chacha encryption was implemented together, it is not possible to have mixed kdf and encryption algorithms
switch kdf {
case kdfArgon2:
key = argon2.IDKey([]byte(passphrase), saltBytes, Argon2Time, Argon2Memory, Argon2Threads, Argon2KeyLen)

aead, err := chacha20poly1305.New(key)
if err != nil {
return privKey, errorsmod.Wrap(err, "Error generating aead cypher for key.")
} else if len(encBytes) < aead.NonceSize() {
return privKey, errorsmod.Wrap(nil, "Encrypted bytes length is smaller than aead nonce size.")
}

nonce, privKeyBytesEncrypted := encBytes[:aead.NonceSize()], encBytes[aead.NonceSize():] // Split nonce and ciphertext.
privKeyBytes, err = aead.Open(nil, nonce, privKeyBytesEncrypted, nil) // Decrypt the message and check it wasn't tampered with.
if err != nil {
return privKey, errorsmod.Wrap(err, "Error decrypting with aead.")
}
default:
key, err = bcrypt.GenerateFromPassword(saltBytes, []byte(passphrase), BcryptSecurityParameter)
if err != nil {
return privKey, errorsmod.Wrap(err, "Error generating bcrypt cypher for key.")
}
key = crypto.Sha256(key) // Get 32 bytes
privKeyBytes, err = xsalsa20symmetric.DecryptSymmetric(encBytes, key)

key = crypto.Sha256(key) // Get 32 bytes
if err == xsalsa20symmetric.ErrCiphertextDecrypt {
return privKey, sdkerrors.ErrWrongPassword
}
}

privKeyBytes, err := xsalsa20symmetric.DecryptSymmetric(encBytes, key)
if err != nil && err == xsalsa20symmetric.ErrCiphertextDecrypt {
return privKey, sdkerrors.ErrWrongPassword
} else if err != nil {
if err != nil {
return privKey, err
}

Expand Down
46 changes: 44 additions & 2 deletions crypto/armor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ import (
"testing"

cmtcrypto "github.com/cometbft/cometbft/crypto"
"github.com/cometbft/cometbft/crypto/xsalsa20symmetric"
"github.com/cosmos/cosmos-sdk/crypto/xsalsa20symmetric"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

Expand All @@ -21,7 +22,6 @@ import (
"github.com/cosmos/cosmos-sdk/crypto/keys/bcrypt"
"github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1"
cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
_ "github.com/cosmos/cosmos-sdk/runtime"
"github.com/cosmos/cosmos-sdk/testutil/configurator"
"github.com/cosmos/cosmos-sdk/types"
)
Expand Down Expand Up @@ -194,3 +194,45 @@ func TestArmor(t *testing.T) {
assert.Equal(t, blockType, blockType2)
assert.Equal(t, data, data2)
}

func TestBcryptLegacyEncryption(t *testing.T) {
privKey := secp256k1.GenPrivKey()
saltBytes := cmtcrypto.CRandBytes(16)
passphrase := "passphrase"
privKeyBytes := legacy.Cdc.MustMarshal(privKey)

// Bcrypt + Aead
headerBcrypt := map[string]string{
"kdf": "bcrypt",
"salt": fmt.Sprintf("%X", saltBytes),
}
keyBcrypt, _ := bcrypt.GenerateFromPassword(saltBytes, []byte(passphrase), 12) // Legacy key generation
keyBcrypt = cmtcrypto.Sha256(keyBcrypt)

// bcrypt + xsalsa20symmetric
encBytesBcryptXsalsa20symetric := xsalsa20symmetric.EncryptSymmetric(privKeyBytes, keyBcrypt)

type testCase struct {
description string
armor string
}

for _, scenario := range []testCase{
{
description: "Argon2 + Aead",
armor: crypto.EncryptArmorPrivKey(privKey, "passphrase", ""),
},
{
description: "Bcrypt + xsalsa20symmetric",
armor: crypto.EncodeArmor("TENDERMINT PRIVATE KEY", headerBcrypt, encBytesBcryptXsalsa20symetric),
},
} {
t.Run(scenario.description, func(t *testing.T) {
_, _, err := crypto.UnarmorDecryptPrivKey(scenario.armor, "wrongpassphrase")
require.Error(t, err)
decryptedPrivKey, _, err := crypto.UnarmorDecryptPrivKey(scenario.armor, "passphrase")
require.NoError(t, err)
require.True(t, privKey.Equals(decryptedPrivKey))
})
}
}
10 changes: 5 additions & 5 deletions crypto/keyring/keyring.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ import (

errorsmod "cosmossdk.io/errors"

cometbftCrypto "github.com/cometbft/cometbft/crypto"

"golang.org/x/crypto/argon2"
"golang.org/x/crypto/bcrypt"

"github.com/cosmos/cosmos-sdk/client/input"
Expand Down Expand Up @@ -751,11 +754,8 @@ func newRealPrompt(dir string, buf io.Reader) func(string) (string, error) {
continue
}

passwordHash, err := bcrypt.GenerateFromPassword([]byte(pass), 2)
if err != nil {
fmt.Fprintln(os.Stderr, err)
continue
}
saltBytes := cometbftCrypto.CRandBytes(16)
passwordHash := argon2.IDKey([]byte(pass), saltBytes, crypto.Argon2Time, crypto.Argon2Memory, crypto.Argon2Threads, crypto.Argon2KeyLen)

if err := os.WriteFile(keyhashFilePath, passwordHash, 0o600); err != nil {
return "", err
Expand Down
29 changes: 0 additions & 29 deletions crypto/keyring/keyring_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,13 @@ import (
"testing"

"github.com/99designs/keyring"
cmtcrypto "github.com/cometbft/cometbft/crypto"
"github.com/stretchr/testify/require"
"golang.org/x/crypto/bcrypt"

"github.com/cosmos/cosmos-sdk/codec"
codectypes "github.com/cosmos/cosmos-sdk/codec/types"
"github.com/cosmos/cosmos-sdk/crypto"
cryptocodec "github.com/cosmos/cosmos-sdk/crypto/codec"
"github.com/cosmos/cosmos-sdk/crypto/hd"
cosmosbcrypt "github.com/cosmos/cosmos-sdk/crypto/keys/bcrypt"
"github.com/cosmos/cosmos-sdk/crypto/keys/ed25519"
"github.com/cosmos/cosmos-sdk/crypto/keys/multisig"
"github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1"
Expand Down Expand Up @@ -1952,32 +1949,6 @@ func TestRenameKey(t *testing.T) {
}
}

// TestChangeBcrypt tests the compatibility from upstream Bcrypt and our own
func TestChangeBcrypt(t *testing.T) {
pw := []byte("somepasswword!")

saltBytes := cmtcrypto.CRandBytes(16)
cosmosHash, err := cosmosbcrypt.GenerateFromPassword(saltBytes, pw, 2)
require.NoError(t, err)

bcryptHash, err := bcrypt.GenerateFromPassword(pw, 2)
require.NoError(t, err)

// Check the new hash with the old bcrypt, vice-versa and with the same
// bcrypt version just because.
err = cosmosbcrypt.CompareHashAndPassword(bcryptHash, pw)
require.NoError(t, err)

err = cosmosbcrypt.CompareHashAndPassword(cosmosHash, pw)
require.NoError(t, err)

err = bcrypt.CompareHashAndPassword(cosmosHash, pw)
require.NoError(t, err)

err = bcrypt.CompareHashAndPassword(bcryptHash, pw)
require.NoError(t, err)
}

func requireEqualRenamedKey(t *testing.T, key *Record, mnemonic *Record, nameMatch bool) {
if nameMatch {
require.Equal(t, key.Name, mnemonic.Name)
Expand Down
18 changes: 4 additions & 14 deletions crypto/xsalsa20symmetric/symmetric_test.go
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
package xsalsa20symmetric

import (
"crypto/sha256"
"testing"

"github.com/cometbft/cometbft/crypto"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"golang.org/x/crypto/bcrypt"
"golang.org/x/crypto/argon2"
)

func TestSimple(t *testing.T) {
Expand All @@ -23,21 +22,12 @@ func TestSimple(t *testing.T) {
func TestSimpleWithKDF(t *testing.T) {
plaintext := []byte("sometext")
secretPass := []byte("somesecret")
secret, err := bcrypt.GenerateFromPassword(secretPass, 12)
if err != nil {
t.Error(err)
}
secret = sha256Sum(secret)
saltBytes := crypto.CRandBytes(16)
secret := argon2.IDKey(secretPass, saltBytes, 1, 64*1024, 4, 32)

ciphertext := EncryptSymmetric(plaintext, secret)
plaintext2, err := DecryptSymmetric(ciphertext, secret)

require.NoError(t, err, "%+v", err)
assert.Equal(t, plaintext, plaintext2)
}

func sha256Sum(bytes []byte) []byte {
hasher := sha256.New()
hasher.Write(bytes)
return hasher.Sum(nil)
}
5 changes: 5 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,11 @@ require (
nhooyr.io/websocket v1.8.6 // indirect
)

// Here are the short-lived replace from the Cosmos SDK
// Replace here are pending PRs, or version to be tagged
// TODO tag after https://github.com/cosmos/cosmos-sdk/pull/14207 merge
replace cosmossdk.io/store => ./store

// Below are the long-lived replace of the Cosmos SDK
replace (
// use cosmos fork of keyring
Expand Down
Loading