[CLI-1430] Weak encryption implementation (#1170)

lucy-fan · Brian Strauch · tadsul · web-flow · commit ceb2412c2170 · 2022-02-17T18:57:17.000-08:00
* Update to AES GCM 256 and crypto/rand for encryption * Change back to string iv * Add backwards compatability * Code review Co-authored-by: Brian Strauch <bstrauch@confluent.io> * Add iv length constant * Code review * Fix tests * Code review * Remove padding from GCM mode * Add support for feature based cipher mode (#1196) * Add support for feature based cipher mode * Apply suggestions from code review Co-authored-by: Brian Strauch <bstrauch@confluent.io> * Code review; add default to Cipher struct * Update Cipher constructor Co-authored-by: Brian Strauch <bstrauch@confluent.io> Co-authored-by: Brian Strauch <bstrauch@confluent.io> Co-authored-by: tadsul <43974298+tadsul@users.noreply.github.com>
diff --git a/internal/cmd/secret/command.go b/internal/cmd/secret/command.go
@@ -8,7 +8,7 @@ import (
 )
 
 type command struct {
-	*pcmd.CLICommand
+	*pcmd.AuthenticatedStateFlagCommand
 	flagResolver pcmd.FlagResolver
 	plugin       secret.PasswordProtection
 }
@@ -21,7 +21,7 @@ func New(prerunner pcmd.PreRunner, flagResolver pcmd.FlagResolver, plugin secret
 	}
 
 	c := &command{
-		CLICommand:   pcmd.NewAnonymousCLICommand(cmd, prerunner),
+		AuthenticatedStateFlagCommand:   pcmd.NewAuthenticatedWithMDSStateFlagCommand(cmd, prerunner),
 		flagResolver: flagResolver,
 		plugin:       plugin,
 	}
diff --git a/internal/cmd/secret/command_file.go b/internal/cmd/secret/command_file.go
@@ -1,9 +1,14 @@
 package secret
 
 import (
+	"context"
+	"github.com/confluentinc/cli/internal/pkg/secret"
 	"github.com/spf13/cobra"
+	"net/http"
+	"os"
 
 	pcmd "github.com/confluentinc/cli/internal/pkg/cmd"
+	mds "github.com/confluentinc/mds-sdk-go/mdsv1"
 	"github.com/confluentinc/cli/internal/pkg/errors"
 )
 
@@ -55,3 +60,21 @@ func (c *command) getConfigs(configSource string, inputType string, prompt strin
 	}
 	return newConfigs, nil
 }
+
+func (c *command) getCipherMode() string {
+	if os.Getenv("XX_SECRETS_GCM_MODE") != "" {
+		return secret.AES_GCM
+	}
+
+	ctx := context.WithValue(context.Background(), mds.ContextAccessToken, c.State.AuthToken)
+	featureInfo, response, err := c.MDSClient.MetadataServiceOperationsApi.Features(ctx)
+
+	if err != nil || response.StatusCode == http.StatusNotFound {
+		return secret.AES_CBC
+	}
+
+	if _, ok := featureInfo.Features[secret.MdsFeatureCipherFlag]; ok {
+		return secret.AES_GCM
+	}
+	return secret.AES_CBC
+}
diff --git a/internal/cmd/secret/command_file_add.go b/internal/cmd/secret/command_file_add.go
@@ -44,5 +44,7 @@ func (c *command) add(cmd *cobra.Command, _ []string) error {
 		return err
 	}
 
+	cipherMode := c.getCipherMode()
+	c.plugin.SetCipherMode(cipherMode)
 	return c.plugin.AddEncryptedPasswords(configPath, localSecretsPath, remoteSecretsPath, newConfigs)
 }
diff --git a/internal/cmd/secret/command_file_decrypt.go b/internal/cmd/secret/command_file_decrypt.go
@@ -48,5 +48,8 @@ func (c *command) decrypt(cmd *cobra.Command, _ []string) error {
 		return err
 	}
 
+	cipherMode := c.getCipherMode()
+	c.plugin.SetCipherMode(cipherMode)
+
 	return c.plugin.DecryptConfigFileSecrets(configPath, localSecretsPath, outputPath, configs)
 }
diff --git a/internal/cmd/secret/command_file_encrypt.go b/internal/cmd/secret/command_file_encrypt.go
@@ -38,5 +38,8 @@ func (c *command) encrypt(cmd *cobra.Command, _ []string) error {
 		return err
 	}
 
+	cipherMode := c.getCipherMode()
+	c.plugin.SetCipherMode(cipherMode)
+
 	return c.plugin.EncryptConfigFileSecrets(configPath, localSecretsPath, remoteSecretsPath, configs)
 }
diff --git a/internal/cmd/secret/command_file_remove.go b/internal/cmd/secret/command_file_remove.go
@@ -47,6 +47,9 @@ func (c *command) remove(cmd *cobra.Command, _ []string) error {
 		return err
 	}
 
+	cipherMode := c.getCipherMode()
+	c.plugin.SetCipherMode(cipherMode)
+
 	if err := c.plugin.RemoveEncryptedPasswords(configPath, localSecretsPath, removeConfigs); err != nil {
 		return err
 	}
diff --git a/internal/cmd/secret/command_file_rotate.go b/internal/cmd/secret/command_file_rotate.go
@@ -41,6 +41,9 @@ func (c *command) rotate(cmd *cobra.Command, _ []string) error {
 		return err
 	}
 
+	cipherMode := c.getCipherMode()
+	c.plugin.SetCipherMode(cipherMode)
+
 	if rotateMEK {
 		oldPassphraseSource, err := cmd.Flags().GetString("passphrase")
 		if err != nil {
diff --git a/internal/cmd/secret/command_file_update.go b/internal/cmd/secret/command_file_update.go
@@ -46,6 +46,9 @@ func (c *command) update(cmd *cobra.Command, _ []string) error {
 		return err
 	}
 
+	cipherMode := c.getCipherMode()
+	c.plugin.SetCipherMode(cipherMode)
+
 	if err := c.plugin.UpdateEncryptedPasswords(configPath, localSecretsPath, remoteSecretsPath, newConfigs); err != nil {
 		return err
 	}
diff --git a/internal/cmd/secret/command_master_key_generate.go b/internal/cmd/secret/command_master_key_generate.go
@@ -51,6 +51,9 @@ func (c *command) generate(cmd *cobra.Command, _ []string) error {
 		return err
 	}
 
+	cipherMode := c.getCipherMode()
+	c.plugin.SetCipherMode(cipherMode)
+
 	masterKey, err := c.plugin.CreateMasterKey(passphrase, localSecretsPath)
 	if err != nil {
 		return err
diff --git a/internal/pkg/errors/error_message.go b/internal/pkg/errors/error_message.go
@@ -303,6 +303,7 @@ const (
 	InvalidJSONFileFormatErrorMsg      = "invalid json file format"
 	InvalidFilePathErrorMsg            = "invalid file path \"%s\""
 	UnsupportedFileFormatErrorMsg      = "unsupported file format for file \"%s\""
+	InvalidAlgorithmErrorMsg           = "invalid algorithm \"%s\""
 
 	// sso package
 	StartHTTPServerErrorMsg            = "unable to start HTTP server"
diff --git a/internal/pkg/secret/cipher.go b/internal/pkg/secret/cipher.go
@@ -9,12 +9,17 @@ type Cipher struct {
 	EncryptedDataKey string
 }
 
-func NewDefaultCipher() *Cipher {
-	return &Cipher{
-		Iterations:       MetadataKeyDefaultIterations,
-		KeyLength:        MetadataKeyDefaultLengthBytes,
-		SaltMEK:          "",
-		SaltDEK:          "",
-		EncryptionAlgo:   MetadataEncAlgorithm,
-		EncryptedDataKey: ""}
+func NewCipher(cipherMode string) *Cipher {
+	
+	cipher := &Cipher{
+		Iterations:     MetadataKeyDefaultIterations,
+		KeyLength:      MetadataKeyDefaultLengthBytes,
+		EncryptionAlgo: AES_CBC,
+	}
+	
+	if cipherMode != "" {
+		cipher.EncryptionAlgo = cipherMode
+	}
+	
+	return cipher
 }
diff --git a/internal/pkg/secret/encryption_engine.go b/internal/pkg/secret/encryption_engine.go
@@ -4,41 +4,37 @@ import (
 	"bytes"
 	"crypto/aes"
 	"crypto/cipher"
+	"crypto/rand"
 	"crypto/sha512"
 	"encoding/base64"
-	"math/rand"
-
-	"golang.org/x/crypto/pbkdf2"
-
 	"github.com/confluentinc/cli/internal/pkg/errors"
 	"github.com/confluentinc/cli/internal/pkg/log"
+	"golang.org/x/crypto/pbkdf2"
 )
 
 // Encryption Engine performs Encryption, Decryption and Hash operations.
 type EncryptionEngine interface {
-	Encrypt(plainText string, key []byte) (string, string, error)
+	Encrypt(plainText string, key []byte, algo string) (string, string, error)
 	Decrypt(cipher string, iv string, algo string, key []byte) (string, error)
 	GenerateRandomDataKey(keyLength int) ([]byte, string, error)
 	GenerateMasterKey(masterKeyPassphrase string, salt string) (string, string, error)
-	WrapDataKey(dataKey []byte, masterKey string) (string, string, error)
+	WrapDataKey(dataKey []byte, masterKey string, algo string) (string, string, error)
 	UnwrapDataKey(dataKey string, iv string, algo string, masterKey string) ([]byte, error)
 }
 
 // EncryptEngineImpl is the EncryptionEngine implementation
 type EncryptEngineImpl struct {
-	Cipher     *Cipher
-	Logger     *log.Logger
-	RandSource rand.Source
+	Cipher *Cipher
+	Logger *log.Logger
 }
 
-func NewEncryptionEngine(suite *Cipher, randSource rand.Source) *EncryptEngineImpl {
-	return &EncryptEngineImpl{Cipher: suite, RandSource: randSource}
+func NewEncryptionEngine(suite *Cipher) *EncryptEngineImpl {
+	return &EncryptEngineImpl{Cipher: suite}
 }
 
 func (c *EncryptEngineImpl) generateRandomString(keyLength int) (string, error) {
 	randomBytes := make([]byte, keyLength)
-	r := rand.New(c.RandSource)
-	_, err := r.Read(randomBytes)
+	_, err := rand.Read(randomBytes)
 	if err != nil {
 		return "", err
 	}
@@ -85,30 +81,30 @@ func (c *EncryptEngineImpl) GenerateMasterKey(masterKeyPassphrase string, salt s
 	return encodedKey, salt, nil
 }
 
-func (c *EncryptEngineImpl) WrapDataKey(dataKey []byte, masterKey string) (string, string, error) {
+func (c *EncryptEngineImpl) WrapDataKey(dataKey []byte, masterKey string, algo string) (string, string, error) {
 	dataKeyStr := base64.StdEncoding.EncodeToString(dataKey)
 	masterKeyByte, err := base64.StdEncoding.DecodeString(masterKey)
 	if err != nil {
 		return "", "", err
 	}
-	return c.Encrypt(dataKeyStr, masterKeyByte)
+	return c.Encrypt(dataKeyStr, masterKeyByte, algo)
 }
 
-func (c *EncryptEngineImpl) UnwrapDataKey(dataKey string, iv string, _ string, masterKey string) ([]byte, error) {
+func (c *EncryptEngineImpl) UnwrapDataKey(dataKey string, iv string, algo string, masterKey string) ([]byte, error) {
 	masterKeyByte, err := base64.StdEncoding.DecodeString(masterKey)
 	if err != nil {
 		return []byte{}, err
 	}
 
-	dataKeyEnc, err := c.Decrypt(dataKey, iv, c.Cipher.EncryptionAlgo, masterKeyByte)
+	dataKeyEnc, err := c.Decrypt(dataKey, iv, algo, masterKeyByte)
 	if err != nil {
 		return []byte{}, err
 	}
 
 	return base64.StdEncoding.DecodeString(dataKeyEnc)
 }
 
-func (c *EncryptEngineImpl) Encrypt(plainText string, key []byte) (data string, ivStr string, err error) {
+func (c *EncryptEngineImpl) Encrypt(plainText string, key []byte, algo string) (data string, ivStr string, err error) {
 	defer func() {
 		if r := recover(); r != nil {
 			switch x := r.(type) {
@@ -122,6 +118,36 @@ func (c *EncryptEngineImpl) Encrypt(plainText string, key []byte) (data string,
 		}
 	}()
 
+	if algo == AES_CBC { // Backwards compatability
+		return c.encryptCBCMode(plainText, key)
+	} else {
+		return c.encryptGCMMode(plainText, key)
+	}
+}
+
+func (c *EncryptEngineImpl) Decrypt(cipher string, iv string, algo string, key []byte) (string, error) {
+	cipherBytes, err := base64.StdEncoding.DecodeString(cipher)
+	if err != nil {
+		return "", err
+	}
+	ivBytes, err := base64.StdEncoding.DecodeString(iv)
+	if err != nil {
+		return "", err
+	}
+	plainText, err := c.decrypt(cipherBytes, key, ivBytes, algo)
+	if err != nil {
+		return "", err
+	}
+
+	return string(plainText), nil
+}
+
+func (c *EncryptEngineImpl) generateEncryptionKey(keyPhrase string, salt string) ([]byte, error) {
+	key := pbkdf2.Key([]byte(keyPhrase), []byte(salt), c.Cipher.Iterations, c.Cipher.KeyLength, sha512.New)
+	return key, nil
+}
+
+func (c *EncryptEngineImpl) encryptCBCMode(plainText string, key []byte) (data string, ivStr string, err error) {
 	block, err := aes.NewCipher(key)
 	if err != nil {
 		return "", "", err
@@ -138,36 +164,42 @@ func (c *EncryptEngineImpl) Encrypt(plainText string, key []byte) (data string,
 	}
 	ecb := cipher.NewCBCEncrypter(block, ivBytes)
 	content := []byte(plainText)
-	content = c.pKCS5Padding(content, block.BlockSize())
+	content = c.pkcs5Padding(content, block.BlockSize())
 	crypted := make([]byte, len(content))
 	ecb.CryptBlocks(crypted, content)
 	result := base64.StdEncoding.EncodeToString(crypted)
 	return result, ivStr, nil
 }
 
-func (c *EncryptEngineImpl) Decrypt(cipher string, iv string, _ string, key []byte) (string, error) {
-	cipherBytes, err := base64.StdEncoding.DecodeString(cipher)
+func (c *EncryptEngineImpl) encryptGCMMode(plainText string, key []byte) (string, string, error) {
+	block, err := aes.NewCipher(key)
 	if err != nil {
-		return "", err
+		return "", "", err
 	}
-	ivBytes, err := base64.StdEncoding.DecodeString(iv)
+
+	ivStr, err := c.generateRandomString(MetadataIVLength)
 	if err != nil {
-		return "", err
+		return "", "", err
 	}
-	plainText, err := c.decrypt(cipherBytes, key, ivBytes)
+
+	ivBytes, err := base64.StdEncoding.DecodeString(ivStr)
 	if err != nil {
-		return "", err
+		return "", "", err
 	}
 
-	return string(plainText), nil
-}
+	aesGcm, err := cipher.NewGCM(block)
+	if err != nil {
+		return "", "", err
+	}
 
-func (c *EncryptEngineImpl) generateEncryptionKey(keyPhrase string, salt string) ([]byte, error) {
-	key := pbkdf2.Key([]byte(keyPhrase), []byte(salt), c.Cipher.Iterations, c.Cipher.KeyLength, sha512.New)
-	return key, nil
+	content := []byte(plainText)
+
+	ciphertext := aesGcm.Seal(nil, ivBytes, content, nil)
+	result := base64.StdEncoding.EncodeToString(ciphertext)
+	return result, ivStr, nil
 }
 
-func (c *EncryptEngineImpl) decrypt(crypt []byte, key []byte, iv []byte) (plain []byte, err error) {
+func (c *EncryptEngineImpl) decrypt(crypt []byte, key []byte, iv []byte, algo string) (plain []byte, err error) {
 	defer func() {
 		if r := recover(); r != nil {
 			switch x := r.(type) {
@@ -186,25 +218,41 @@ func (c *EncryptEngineImpl) decrypt(crypt []byte, key []byte, iv []byte) (plain
 		return []byte{}, err
 	}
 
-	ecb := cipher.NewCBCDecrypter(block, iv)
-	decrypted := make([]byte, len(crypt))
-	ecb.CryptBlocks(decrypted, crypt)
+	var decrypted []byte
 
-	return c.pKCS5Trimming(decrypted)
-}
+	if algo == AES_CBC { // Backwards compatability
+		ecb := cipher.NewCBCDecrypter(block, iv)
+		decrypted = make([]byte, len(crypt))
+		ecb.CryptBlocks(decrypted, crypt)
+		return c.pkcs5Trimming(decrypted)
+	} else if algo == AES_GCM {
+		aesGcm, err := cipher.NewGCM(block)
+		if err != nil {
+			return []byte{}, err
+		}
 
-func (c *EncryptEngineImpl) pKCS5Padding(ciphertext []byte, blockSize int) []byte {
-	length := len(ciphertext) % blockSize
-	padding := blockSize - length
-	padtext := bytes.Repeat([]byte{byte(padding)}, padding)
-	return append(ciphertext, padtext...)
+		decrypted, err = aesGcm.Open(nil, iv, crypt, nil)
+		if err != nil {
+			return []byte{}, err
+		}
+		return decrypted, nil
+	} else {
+		return []byte{}, errors.Errorf(errors.InvalidAlgorithmErrorMsg, algo)
+	}
 }
 
-func (c *EncryptEngineImpl) pKCS5Trimming(encrypt []byte) ([]byte, error) {
+func (c *EncryptEngineImpl) pkcs5Trimming(encrypt []byte) ([]byte, error) {
 	padding := encrypt[len(encrypt)-1]
 	length := len(encrypt) - int(padding)
 	if length < 0 || length > len(encrypt) {
 		return nil, errors.New(errors.DataCorruptedErrorMsg)
 	}
 	return encrypt[:len(encrypt)-int(padding)], nil
 }
+
+func (c *EncryptEngineImpl) pkcs5Padding(ciphertext []byte, blockSize int) []byte {
+	length := len(ciphertext) % blockSize
+	padding := blockSize - length
+	padtext := bytes.Repeat([]byte{byte(padding)}, padding)
+	return append(ciphertext, padtext...)
+}
diff --git a/internal/pkg/secret/password_protection_constants.go b/internal/pkg/secret/password_protection_constants.go
diff --git a/internal/pkg/secret/password_protection_plugin.go b/internal/pkg/secret/password_protection_plugin.go
diff --git a/internal/pkg/secret/password_protection_test.go b/internal/pkg/secret/password_protection_test.go

Original file line number	Diff line number	Diff line change
`@@ -8,7 +8,7 @@ import (`
`8`	`8`	`)`
`9`	`9`
`10`	`10`	`type command struct {`
`11`		`- *pcmd.CLICommand`
	`11`	`+ *pcmd.AuthenticatedStateFlagCommand`
`12`	`12`	`flagResolver pcmd.FlagResolver`
`13`	`13`	`plugin secret.PasswordProtection`
`14`	`14`	`}`
`@@ -21,7 +21,7 @@ func New(prerunner pcmd.PreRunner, flagResolver pcmd.FlagResolver, plugin secret`
`21`	`21`	`}`
`22`	`22`
`23`	`23`	`c := &command{`
`24`		`- CLICommand: pcmd.NewAnonymousCLICommand(cmd, prerunner),`
	`24`	`+ AuthenticatedStateFlagCommand: pcmd.NewAuthenticatedWithMDSStateFlagCommand(cmd, prerunner),`
`25`	`25`	`flagResolver: flagResolver,`
`26`	`26`	`plugin: plugin,`
`27`	`27`	`}`
Original file line number	Diff line number	Diff line change
`@@ -44,5 +44,7 @@ func (c command) add(cmd cobra.Command, _ []string) error {`
`44`	`44`	`return err`
`45`	`45`	`}`
`46`	`46`
	`47`	`+ cipherMode := c.getCipherMode()`
	`48`	`+ c.plugin.SetCipherMode(cipherMode)`
`47`	`49`	`return c.plugin.AddEncryptedPasswords(configPath, localSecretsPath, remoteSecretsPath, newConfigs)`
`48`	`50`	`}`
Original file line number	Diff line number	Diff line change
`@@ -48,5 +48,8 @@ func (c command) decrypt(cmd cobra.Command, _ []string) error {`
`48`	`48`	`return err`
`49`	`49`	`}`
`50`	`50`
	`51`	`+ cipherMode := c.getCipherMode()`
	`52`	`+ c.plugin.SetCipherMode(cipherMode)`
	`53`	`+`
`51`	`54`	`return c.plugin.DecryptConfigFileSecrets(configPath, localSecretsPath, outputPath, configs)`
`52`	`55`	`}`
Original file line number	Diff line number	Diff line change
`@@ -38,5 +38,8 @@ func (c command) encrypt(cmd cobra.Command, _ []string) error {`
`38`	`38`	`return err`
`39`	`39`	`}`
`40`	`40`
	`41`	`+ cipherMode := c.getCipherMode()`
	`42`	`+ c.plugin.SetCipherMode(cipherMode)`
	`43`	`+`
`41`	`44`	`return c.plugin.EncryptConfigFileSecrets(configPath, localSecretsPath, remoteSecretsPath, configs)`
`42`	`45`	`}`
Original file line number	Diff line number	Diff line change
`@@ -47,6 +47,9 @@ func (c command) remove(cmd cobra.Command, _ []string) error {`
`47`	`47`	`return err`
`48`	`48`	`}`
`49`	`49`
	`50`	`+ cipherMode := c.getCipherMode()`
	`51`	`+ c.plugin.SetCipherMode(cipherMode)`
	`52`	`+`
`50`	`53`	`if err := c.plugin.RemoveEncryptedPasswords(configPath, localSecretsPath, removeConfigs); err != nil {`
`51`	`54`	`return err`
`52`	`55`	`}`
Original file line number	Diff line number	Diff line change
`@@ -41,6 +41,9 @@ func (c command) rotate(cmd cobra.Command, _ []string) error {`
`41`	`41`	`return err`
`42`	`42`	`}`
`43`	`43`
	`44`	`+ cipherMode := c.getCipherMode()`
	`45`	`+ c.plugin.SetCipherMode(cipherMode)`
	`46`	`+`
`44`	`47`	`if rotateMEK {`
`45`	`48`	`oldPassphraseSource, err := cmd.Flags().GetString("passphrase")`
`46`	`49`	`if err != nil {`
Original file line number	Diff line number	Diff line change
`@@ -46,6 +46,9 @@ func (c command) update(cmd cobra.Command, _ []string) error {`
`46`	`46`	`return err`
`47`	`47`	`}`
`48`	`48`
	`49`	`+ cipherMode := c.getCipherMode()`
	`50`	`+ c.plugin.SetCipherMode(cipherMode)`
	`51`	`+`
`49`	`52`	`if err := c.plugin.UpdateEncryptedPasswords(configPath, localSecretsPath, remoteSecretsPath, newConfigs); err != nil {`
`50`	`53`	`return err`
`51`	`54`	`}`
Original file line number	Diff line number	Diff line change
`@@ -51,6 +51,9 @@ func (c command) generate(cmd cobra.Command, _ []string) error {`
`51`	`51`	`return err`
`52`	`52`	`}`
`53`	`53`
	`54`	`+ cipherMode := c.getCipherMode()`
	`55`	`+ c.plugin.SetCipherMode(cipherMode)`
	`56`	`+`
`54`	`57`	`masterKey, err := c.plugin.CreateMasterKey(passphrase, localSecretsPath)`
`55`	`58`	`if err != nil {`
`56`	`59`	`return err`