From 32675e969a13db970c0bc3e914f326072b6629ec Mon Sep 17 00:00:00 2001 From: James Fantin-Hardesty <24646452+jfantinhardesty@users.noreply.github.com> Date: Wed, 5 Mar 2025 15:38:59 -0700 Subject: [PATCH 1/3] Fix issue with secure config failed due to incorrectly decoding the base64 string, and allow for env variable when passing passphrase when generating config --- cmd/config-gen.go | 26 ++++++++++++++++++++-- cmd/secure_test.go | 55 ++++++++++++++++++++++++++++++++++++++++++++++ common/util.go | 9 ++++++-- 3 files changed, 86 insertions(+), 4 deletions(-) diff --git a/cmd/config-gen.go b/cmd/config-gen.go index a13133a7f..506642168 100644 --- a/cmd/config-gen.go +++ b/cmd/config-gen.go @@ -26,6 +26,8 @@ package cmd import ( + "encoding/base64" + "errors" "fmt" "os" "regexp" @@ -111,9 +113,11 @@ var generateConfig = &cobra.Command{ FlagErrorHandling: cobra.ExitOnError, RunE: func(cmd *cobra.Command, args []string) error { var templateConfig []byte - var err error - encryptedPassphrase = memguard.NewEnclave([]byte(opts.passphrase)) + err := validateGenConfigOptions() + if err != nil { + return fmt.Errorf("failed to validate options [%s]", err.Error()) + } templateConfig, err = os.ReadFile(opts.configFilePath) if err != nil { @@ -152,6 +156,24 @@ var generateConfig = &cobra.Command{ }, } +func validateGenConfigOptions() error { + if opts.passphrase == "" { + opts.passphrase = os.Getenv(SecureConfigEnvName) + if opts.passphrase == "" { + return errors.New("provide the passphrase as a cli parameter or configure the CLOUDFUSE_SECURE_CONFIG_PASSPHRASE environment variable") + } + } + + _, err := base64.StdEncoding.DecodeString(string(opts.passphrase)) + if err != nil { + return fmt.Errorf("passphrase is not valid base64 encoded [%s]", err.Error()) + } + + encryptedPassphrase = memguard.NewEnclave([]byte(opts.passphrase)) + + return nil +} + func init() { rootCmd.AddCommand(generateTestConfig) generateTestConfig.Flags().StringVar(&opts.configFilePath, "config-file", "", "Input config file.") diff --git a/cmd/secure_test.go b/cmd/secure_test.go index 40ab991d6..48515f591 100644 --- a/cmd/secure_test.go +++ b/cmd/secure_test.go @@ -130,6 +130,27 @@ func (suite *secureConfigTestSuite) TestSecureConfigEncrypt() { suite.assert.NoFileExists(confFile.Name()) } +func (suite *secureConfigTestSuite) TestSecureConfigEncrypt2() { + defer suite.cleanupTest() + confFile, _ := os.CreateTemp("", "conf*.yaml") + outFile, _ := os.CreateTemp("", "conf*.yaml") + passphrase := "hvHlJUKlmZql3gLAcP6Ho41Js5rm8zUAKnwGb1lIffg=" + + defer os.Remove(confFile.Name()) + defer os.Remove(outFile.Name()) + + _, err := confFile.WriteString(testPlainTextConfig) + suite.assert.NoError(err) + + confFile.Close() + + _, err = executeCommandSecure(rootCmd, "secure", "encrypt", fmt.Sprintf("--config-file=%s", confFile.Name()), fmt.Sprintf("--passphrase=%s", passphrase), fmt.Sprintf("--output-file=%s", outFile.Name())) + suite.assert.NoError(err) + + // Config file should be deleted + suite.assert.NoFileExists(confFile.Name()) +} + func (suite *secureConfigTestSuite) TestSecureConfigEncryptNoOutfile() { defer suite.cleanupTest() confFile, _ := os.CreateTemp("", "conf*.yaml") @@ -237,6 +258,40 @@ func (suite *secureConfigTestSuite) TestSecureConfigDecrypt() { os.Remove(confFile.Name() + "." + SecureConfigExtension) } +func (suite *secureConfigTestSuite) TestSecureConfigDecrypt2() { + defer suite.cleanupTest() + confFile, _ := os.CreateTemp("", "conf*.yaml") + outFile, _ := os.CreateTemp("", "conf*.yaml") + passphrase := "hvHlJUKlmZql3gLAcP6Ho41Js5rm8zUAKnwGb1lIffg=" + fmt.Println(passphrase) + + defer os.Remove(confFile.Name()) + defer os.Remove(outFile.Name()) + + _, err := confFile.WriteString(testPlainTextConfig) + suite.assert.NoError(err) + + confFile.Close() + outFile.Close() + + _, err = executeCommandSecure(rootCmd, "secure", "encrypt", fmt.Sprintf("--config-file=%s", confFile.Name()), fmt.Sprintf("--passphrase=%s", passphrase), fmt.Sprintf("--output-file=%s", outFile.Name())) + suite.assert.NoError(err) + + // Config file should be deleted + suite.assert.NoFileExists(confFile.Name()) + + _, err = executeCommandSecure(rootCmd, "secure", "decrypt", fmt.Sprintf("--config-file=%s", outFile.Name()), fmt.Sprintf("--passphrase=%s", passphrase), fmt.Sprintf("--output-file=./tmp.yaml")) + suite.assert.NoError(err) + + data, err := os.ReadFile("./tmp.yaml") + suite.assert.NoError(err) + + suite.assert.Equal(testPlainTextConfig, string(data)) + + os.Remove("./tmp.yaml") + os.Remove(confFile.Name() + "." + SecureConfigExtension) +} + func (suite *secureConfigTestSuite) TestSecureConfigDecryptNoOutputFile() { defer suite.cleanupTest() confFile, _ := os.CreateTemp("", "conf*.yaml") diff --git a/common/util.go b/common/util.go index 68c4af800..5203b2bbe 100644 --- a/common/util.go +++ b/common/util.go @@ -30,6 +30,7 @@ import ( "crypto/aes" "crypto/cipher" "crypto/rand" + "encoding/base64" "errors" "fmt" "io" @@ -245,7 +246,9 @@ func EncryptData(plainData []byte, key *memguard.Enclave) ([]byte, error) { } defer secretKey.Destroy() - block, err := aes.NewCipher(secretKey.Data()) + binaryKey, err := base64.StdEncoding.DecodeString(secretKey.String()) + + block, err := aes.NewCipher(binaryKey) if err != nil { return nil, err } @@ -276,7 +279,9 @@ func DecryptData(cipherData []byte, key *memguard.Enclave) ([]byte, error) { } defer secretKey.Destroy() - block, err := aes.NewCipher(secretKey.Data()) + binaryKey, err := base64.StdEncoding.DecodeString(secretKey.String()) + + block, err := aes.NewCipher(binaryKey) if err != nil { return nil, err } From 1fc6029aa80eb719b49a7f59fbaa7dcb4c64b1ba Mon Sep 17 00:00:00 2001 From: James Fantin-Hardesty <24646452+jfantinhardesty@users.noreply.github.com> Date: Thu, 6 Mar 2025 09:04:22 -0700 Subject: [PATCH 2/3] Fix failed tests --- common/util.go | 18 ++++++++++++++---- common/util_test.go | 21 ++++++++++++++++----- 2 files changed, 30 insertions(+), 9 deletions(-) diff --git a/common/util.go b/common/util.go index 5203b2bbe..b7d9d78ff 100644 --- a/common/util.go +++ b/common/util.go @@ -246,12 +246,17 @@ func EncryptData(plainData []byte, key *memguard.Enclave) ([]byte, error) { } defer secretKey.Destroy() - binaryKey, err := base64.StdEncoding.DecodeString(secretKey.String()) + decodedKey := make([]byte, 32) + _, err = base64.StdEncoding.Decode(decodedKey, secretKey.Bytes()) + if err != nil { + return nil, err + } - block, err := aes.NewCipher(binaryKey) + block, err := aes.NewCipher(decodedKey) if err != nil { return nil, err } + clear(decodedKey) gcm, err := cipher.NewGCM(block) if err != nil { @@ -279,12 +284,17 @@ func DecryptData(cipherData []byte, key *memguard.Enclave) ([]byte, error) { } defer secretKey.Destroy() - binaryKey, err := base64.StdEncoding.DecodeString(secretKey.String()) + decodedKey := make([]byte, 32) + _, err = base64.StdEncoding.Decode(decodedKey, secretKey.Bytes()) + if err != nil { + return nil, err + } - block, err := aes.NewCipher(binaryKey) + block, err := aes.NewCipher(decodedKey) if err != nil { return nil, err } + clear(decodedKey) gcm, err := cipher.NewGCM(block) if err != nil { diff --git a/common/util_test.go b/common/util_test.go index f3d3883f4..9ca1d3145 100644 --- a/common/util_test.go +++ b/common/util_test.go @@ -27,6 +27,7 @@ package common import ( "crypto/rand" + "encoding/base64" "fmt" "os" "path/filepath" @@ -80,9 +81,11 @@ func (suite *typesTestSuite) TestDirectoryDoesNotExist() { func (suite *typesTestSuite) TestEncryptBadKey() { // Generate a random key key := make([]byte, 20) + encodedKey := make([]byte, 28) rand.Read(key) + base64.StdEncoding.Encode(encodedKey, key) - encryptedPassphrase := memguard.NewEnclave(key) + encryptedPassphrase := memguard.NewEnclave(encodedKey) data := make([]byte, 1024) rand.Read(data) @@ -94,9 +97,11 @@ func (suite *typesTestSuite) TestEncryptBadKey() { func (suite *typesTestSuite) TestDecryptBadKey() { // Generate a random key key := make([]byte, 20) + encodedKey := make([]byte, 28) rand.Read(key) + base64.StdEncoding.Encode(encodedKey, key) - encryptedPassphrase := memguard.NewEnclave(key) + encryptedPassphrase := memguard.NewEnclave(encodedKey) data := make([]byte, 1024) rand.Read(data) @@ -108,9 +113,11 @@ func (suite *typesTestSuite) TestDecryptBadKey() { func (suite *typesTestSuite) TestEncryptDecrypt16() { // Generate a random key key := make([]byte, 16) + encodedKey := make([]byte, 24) rand.Read(key) + base64.StdEncoding.Encode(encodedKey, key) - encryptedPassphrase := memguard.NewEnclave(key) + encryptedPassphrase := memguard.NewEnclave(encodedKey) data := make([]byte, 1024) rand.Read(data) @@ -126,9 +133,11 @@ func (suite *typesTestSuite) TestEncryptDecrypt16() { func (suite *typesTestSuite) TestEncryptDecrypt24() { // Generate a random key key := make([]byte, 24) + encodedKey := make([]byte, 32) rand.Read(key) + base64.StdEncoding.Encode(encodedKey, key) - encryptedPassphrase := memguard.NewEnclave(key) + encryptedPassphrase := memguard.NewEnclave(encodedKey) data := make([]byte, 1024) rand.Read(data) @@ -144,9 +153,11 @@ func (suite *typesTestSuite) TestEncryptDecrypt24() { func (suite *typesTestSuite) TestEncryptDecrypt32() { // Generate a random key key := make([]byte, 32) + encodedKey := make([]byte, 44) rand.Read(key) + base64.StdEncoding.Encode(encodedKey, key) - encryptedPassphrase := memguard.NewEnclave(key) + encryptedPassphrase := memguard.NewEnclave(encodedKey) data := make([]byte, 1024) rand.Read(data) From e4b6ffdc9389d7b66b8491b73ed0b30e8e47ed62 Mon Sep 17 00:00:00 2001 From: James Fantin-Hardesty <24646452+jfantinhardesty@users.noreply.github.com> Date: Thu, 6 Mar 2025 10:45:31 -0700 Subject: [PATCH 3/3] Fix issues with base64 encode and decode --- common/util.go | 18 ++++++++++++++++-- common/util_test.go | 36 ++++++++++++++++++++++++++++++++++-- 2 files changed, 50 insertions(+), 4 deletions(-) diff --git a/common/util.go b/common/util.go index b7d9d78ff..86c6f9753 100644 --- a/common/util.go +++ b/common/util.go @@ -246,11 +246,18 @@ func EncryptData(plainData []byte, key *memguard.Enclave) ([]byte, error) { } defer secretKey.Destroy() - decodedKey := make([]byte, 32) + // A base64 encode of a key of length 32 will be at maximum a length of 44 bytes + if len(secretKey.Bytes()) > 44 { + return nil, errors.New("Provided decoded base64 key is longer than 32 bytes. Decoded key " + + "length shall be 16 (AES-128), 24 (AES-192), or 32 (AES-256) bytes in length.") + } + + decodedKey := make([]byte, 32) // Valid key can't be longer than 32 bytes _, err = base64.StdEncoding.Decode(decodedKey, secretKey.Bytes()) if err != nil { return nil, err } + decodedKey = bytes.Trim(decodedKey, "\x00") // trim any null bytes if key is 16, or 24 bytes block, err := aes.NewCipher(decodedKey) if err != nil { @@ -284,11 +291,18 @@ func DecryptData(cipherData []byte, key *memguard.Enclave) ([]byte, error) { } defer secretKey.Destroy() - decodedKey := make([]byte, 32) + // A base64 encode of a key of length 32 will be at maximum a length of 44 bytes + if len(secretKey.Bytes()) > 44 { + return nil, errors.New("Provided decoded base64 key is longer than 32 bytes. Decoded key " + + "length shall be 16 (AES-128), 24 (AES-192), or 32 (AES-256) bytes in length.") + } + + decodedKey := make([]byte, 32) // Valid key can't be longer than 32 bytes _, err = base64.StdEncoding.Decode(decodedKey, secretKey.Bytes()) if err != nil { return nil, err } + decodedKey = bytes.Trim(decodedKey, "\x00") // trim any null bytes if key is 16, or 24 bytes block, err := aes.NewCipher(decodedKey) if err != nil { diff --git a/common/util_test.go b/common/util_test.go index 9ca1d3145..b6e005cfa 100644 --- a/common/util_test.go +++ b/common/util_test.go @@ -78,7 +78,7 @@ func (suite *typesTestSuite) TestDirectoryDoesNotExist() { suite.assert.False(exists) } -func (suite *typesTestSuite) TestEncryptBadKey() { +func (suite *typesTestSuite) TestEncryptBadKeyTooSmall() { // Generate a random key key := make([]byte, 20) encodedKey := make([]byte, 28) @@ -94,7 +94,7 @@ func (suite *typesTestSuite) TestEncryptBadKey() { suite.assert.Error(err) } -func (suite *typesTestSuite) TestDecryptBadKey() { +func (suite *typesTestSuite) TestDecryptBadKeyTooSmall() { // Generate a random key key := make([]byte, 20) encodedKey := make([]byte, 28) @@ -110,6 +110,38 @@ func (suite *typesTestSuite) TestDecryptBadKey() { suite.assert.Error(err) } +func (suite *typesTestSuite) TestEncryptBadKeyTooLong() { + // Generate a random key + key := make([]byte, 36) + encodedKey := make([]byte, 48) + rand.Read(key) + base64.StdEncoding.Encode(encodedKey, key) + + encryptedPassphrase := memguard.NewEnclave(encodedKey) + + data := make([]byte, 1024) + rand.Read(data) + + _, err := EncryptData(data, encryptedPassphrase) + suite.assert.Error(err) +} + +func (suite *typesTestSuite) TestDecryptBadKeyTooLong() { + // Generate a random key + key := make([]byte, 36) + encodedKey := make([]byte, 48) + rand.Read(key) + base64.StdEncoding.Encode(encodedKey, key) + + encryptedPassphrase := memguard.NewEnclave(encodedKey) + + data := make([]byte, 1024) + rand.Read(data) + + _, err := DecryptData(data, encryptedPassphrase) + suite.assert.Error(err) +} + func (suite *typesTestSuite) TestEncryptDecrypt16() { // Generate a random key key := make([]byte, 16)