refactor: Migrated bcrypt key derivation to argon2 and aead symmetric encryption

crypto/armor.go

@@ -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"
@@ -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
@@ -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),
 	}
 
@@ -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"))
 	}
 
-	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
@@ -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"] == "" {
@@ -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
@@ -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 from 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 key from passphrase.")
+		}
+		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
 	}
 

crypto/armor_test.go

@@ -8,7 +8,9 @@ import (
 	"testing"
 
 	cmtcrypto "github.com/cometbft/cometbft/crypto"
+	"github.com/cometbft/cometbft/crypto/armor"
 	"github.com/cometbft/cometbft/crypto/xsalsa20symmetric"
+
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
@@ -194,3 +196,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 Ley gemeratopm
+	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:       armor.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))
+		})
+	}
+}

crypto/keyring/keyring.go

@@ -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"
@@ -54,6 +57,13 @@ var (
 	maxPassphraseEntryAttempts         = 3
 )
 
+const (
+	argon2Time    = 1
+	argon2Memory  = 64 * 1024
+	argon2Threads = 4
+	argon2KeyLen  = 32
+)
+
 // Keyring exposes operations over a backend supported by github.com/99designs/keyring.
 type Keyring interface {
 	// Get the backend type used in the keyring config: "file", "os", "kwallet", "pass", "test", "memory".
@@ -751,11 +761,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, argon2Time, argon2Memory, argon2Threads, argon2KeyLen)
 
 			if err := os.WriteFile(keyhashFilePath, passwordHash, 0o600); err != nil {
 				return "", err

crypto/keyring/keyring_test.go

@@ -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"
@@ -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)

crypto/xsalsa20symmetric/symmetric_test.go

@@ -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) {
@@ -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)
-}

go.mod

@@ -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