Repository: chef/automate
Pull request #4495 (2020-12-17 15:41:49)

Encrypt secrets that are put in the secrets store
This modifies the secrets store/secrets helper utilities to encrypt secrets that are stored on disk. Any existing secret data will not be encrypted unless it is modified. This is fine, as all existing data is generated and uses the secret helpers to share that data between services.

The reason we are doing this is be able to store customer provided secrets in non plaintext. We will take the secrets that are provided in the config and place them in the secret store encrypted.

This is not a user facing change.
This does not provide any sort of security, as the key is stored on next to the secrets on disk.

netlify[bot]: :construction_worker: Deploy preview for *chef-automate* processing.


:hammer: Explore the source changes: bc66322e23422417069e3c89e9ece25311d848eb

:mag: Inspect the deploy logs: [https://app.netlify.com/sites/chef-automate/deploys/5fdb7c3e678db20007b318aa](https://app.netlify.com/sites/chef-automate/deploys/5fdb7c3e678db20007b318aa?utm_source=github&utm_campaign=bot_dl)


Patch:
diff --git a/.expeditor/verify_private.pipeline.yml b/.expeditor/verify_private.pipeline.yml
index 62615dc84..2d1055348 100644
--- a/.expeditor/verify_private.pipeline.yml
+++ b/.expeditor/verify_private.pipeline.yml
@@ -621,15 +621,15 @@ steps:
         linux:
           privileged: true
 
-  - label: "ha chef server"
-    command:
-      - integration/run_test integration/tests/ha_chef_server.sh
-    timeout_in_minutes: 35
-    expeditor:
-      executor:
-        linux:
-          single-use: true
-          privileged: true
+  #- label: "ha chef server"
+    #command:
+      #- integration/run_test integration/tests/ha_chef_server.sh
+    #timeout_in_minutes: 35
+    #expeditor:
+      #executor:
+        #linux:
+          #single-use: true
+          #privileged: true
 
   - label: "workflow"
     command:
diff --git a/components/automate-cli/cmd/chef-automate/bootstrap.go b/components/automate-cli/cmd/chef-automate/bootstrap.go
index 5c562f92b..d4412f737 100644
--- a/components/automate-cli/cmd/chef-automate/bootstrap.go
+++ b/components/automate-cli/cmd/chef-automate/bootstrap.go
@@ -177,6 +177,10 @@ func runBootstrapBundleUnpack(cmd *cobra.Command, args []string) error {
 	secretStore := secrets.NewDiskStore(
 		secretStorePath, uid, gid)
 
+	if err := secretStore.Initialize(); err != nil {
+		return errors.Wrap(err, "could not initialize secret store")
+	}
+
 	b := bootstrapbundle.NewCreator(
 		secretStore,
 		bootstrapbundle.WithRootDir(bootstrapBundleCmdFlags.rootDir))
diff --git a/components/automate-deployment/habitat/plan.sh b/components/automate-deployment/habitat/plan.sh
index face88bc4..c151eb394 100644
--- a/components/automate-deployment/habitat/plan.sh
+++ b/components/automate-deployment/habitat/plan.sh
@@ -19,7 +19,7 @@ pkg_deps=(
   core/net-tools
   core/procps-ng
   core/util-linux
-  chef/automate-platform-tools
+  "${local_platform_tools_origin:-chef}/automate-platform-tools"
   core/bash
   core/cacerts # fetching manifest over HTTPS
   core/certstrap
diff --git a/lib/secrets/encrypt.go b/lib/secrets/encrypt.go
new file mode 100644
index 000000000..8f1199bb3
--- /dev/null
+++ b/lib/secrets/encrypt.go
@@ -0,0 +1,148 @@
+package secrets
+
+import (
+	"bytes"
+	"crypto/aes"
+	"crypto/cipher"
+	"crypto/rand"
+	"encoding/hex"
+	"io/ioutil"
+	"path/filepath"
+
+	"github.com/chef/automate/lib/io/fileutils"
+	"github.com/pkg/errors"
+)
+
+// SecretKeyJSON secret json structure thats required to write and read
+// to the encrypted secret file
+type SecretKeyJSON struct {
+	Algorithm  string               `json:"algorithm"`
+	IV         initializationVector `json:"iv"`
+	Ciphertext ciphertext           `json:"ciphertext"`
+}
+
+type secretStoreKey struct {
+	keyData rawBytes
+}
+
+func (d *diskStore) keyPath() string {
+	return filepath.Join(d.basePath, "key")
+}
+
+func (d *diskStore) keyExists() (bool, error) {
+	return fileutils.PathExists(d.keyPath())
+}
+
+func (d *diskStore) ensureKey() (secretStoreKey, error) {
+	exists, err := d.keyExists()
+	if err != nil {
+		return secretStoreKey{}, err
+	}
+
+	if exists {
+		// If the key is already created, we will not recreate it
+		return d.loadKey()
+	}
+
+	// Generate a 32 byte random key for AES
+	rb, err := randomBytes(32)
+	if err != nil {
+		return secretStoreKey{}, err
+	}
+
+	// Serialize the key. In our serialized form, it it represented
+	// with hex encoding
+	serializedKey, err := rb.MarshalText()
+	if err != nil {
+		return secretStoreKey{}, err
+	}
+
+	// creating path to store the secret key
+	s := bytes.NewReader(serializedKey)
+
+	// writing the key to a file
+	err = fileutils.AtomicWrite(d.keyPath(), s,
+		fileutils.WithAtomicWriteFileMode(0700),
+		fileutils.WithAtomicWriteChown(d.ownerUid, d.ownerGid),
+	)
+	if err != nil {
+		return secretStoreKey{}, errors.Wrap(err, "could not write secret to disk")
+	}
+
+	return secretStoreKey{
+		keyData: rb,
+	}, nil
+}
+
+func (d *diskStore) loadKey() (secretStoreKey, error) {
+	key, err := ioutil.ReadFile(d.keyPath())
+	if err != nil {
+		return secretStoreKey{}, err
+	}
+	rb := rawBytes{}
+	if err := rb.UnmarshalText(key); err != nil {
+		return secretStoreKey{}, err
+	}
+	return secretStoreKey{
+		keyData: rb,
+	}, nil
+}
+
+func (key secretStoreKey) encrypt(plaintext []byte) (initializationVector, ciphertext, error) {
+	block, err := aes.NewCipher(key.keyData)
+	if err != nil {
+		return nil, nil, errors.Wrap(err, "error creating cipher block during encryption")
+	}
+	iv, err := randomBytes(block.BlockSize())
+	if err != nil {
+		return nil, nil, errors.Wrap(err, "could not generate random iv for encryption")
+	}
+	stream := cipher.NewCTR(block, iv)
+	ct := make([]byte, len(plaintext))
+	stream.XORKeyStream(ct, plaintext)
+
+	return iv, ct, nil
+}
+
+func (key secretStoreKey) decrypt(iv initializationVector, ct ciphertext) ([]byte, error) {
+	block, err := aes.NewCipher(key.keyData)
+	if err != nil {
+		return nil, errors.Wrap(err, "Failed to covert secret key to block")
+	}
+	stream := cipher.NewCTR(block, iv)
+	plain := make([]byte, len(ct))
+	stream.XORKeyStream(plain, ct)
+
+	return plain, nil
+}
+
+type rawBytes []byte
+
+func randomBytes(numBytes int) (rawBytes, error) {
+	rb := make([]byte, numBytes)
+	if _, err := rand.Read(rb); err != nil {
+		return nil, errors.Wrap(err, "failed to read random data while generating random bytes")
+	}
+	return rb, nil
+}
+
+func (rb *rawBytes) UnmarshalText(text []byte) error {
+	b := make([]byte, hex.DecodedLen(len(text)))
+	_, err := hex.Decode(b, text)
+	if err != nil {
+		return err
+	}
+	*rb = b
+	return nil
+}
+
+func (rb rawBytes) MarshalText() ([]byte, error) {
+	if rb == nil {
+		return []byte(""), nil
+	}
+	strVal := hex.EncodeToString(rb)
+	return []byte(strVal), nil
+}
+
+type ciphertext = rawBytes
+type initializationVector = rawBytes
diff --git a/lib/secrets/secrets.go b/lib/secrets/secrets.go
index c0b13ee52..45c1fc4de 100644
--- a/lib/secrets/secrets.go
+++ b/lib/secrets/secrets.go
@@ -9,6 +9,7 @@ import (
 	"bytes"
 	"crypto/rand"
 	"encoding/hex"
+	"encoding/json"
 	"io/ioutil"
 	"os"
 	"path/filepath"
@@ -97,6 +98,9 @@ const (
 	// secrets store, read the secrets as root, and then drop privs when
 	// we exec the command.
 	DefaultDiskStoreDataOwner = "hab"
+
+	// SecretFileExtension is the extension name for the encrypted secrets file
+	SecretFileExtension = ".enc"
 )
 
 func NewDiskStoreReader(basePath string) SecretsReader {
@@ -147,46 +151,99 @@ func (d *diskStore) Initialize() error {
 		return errors.Wrap(err, "could now chown shared data directory for secrets store")
 	}
 
+	// creates encryption key if it does not exist
+	if _, err := d.ensureKey(); err != nil {
+		return errors.Wrap(err, "could not create encryption key")
+	}
+
 	return nil
 }
 
 func (d *diskStore) Exists(secret SecretName) (bool, error) {
+	// added logic to check if secret is encrypted or not
+	encryptedPath := filepath.Join(d.basePath, secret.Group, addSecretFileExtension(secret.Name))
+	encryptedExists, err := fileutils.PathExists(encryptedPath)
+	if err != nil {
+		return false, err
+	}
+	if encryptedExists {
+		return true, nil
+	}
 	path := filepath.Join(d.basePath, secret.Group, secret.Name)
 	return fileutils.PathExists(path)
 }
 
 func (d *diskStore) GetSecret(secret SecretName) ([]byte, error) {
-	path := filepath.Join(d.basePath, secret.Group, secret.Name)
-	ret, err := ioutil.ReadFile(path)
+	// checks if the encrypted secret exists
+	pathToEncryptedSecret := filepath.Join(d.basePath, secret.Group, addSecretFileExtension(secret.Name))
+	encryptedSecretExists, err := fileutils.PathExists(pathToEncryptedSecret)
 	if err != nil {
-		return nil, errors.Wrap(err, "could not read secrets data (secret may not have been generated yet)")
+		return nil, errors.Wrap(err, "unexpected error while checking if encrypted file exists (secret may not have been generated yet)")
+	}
+	if encryptedSecretExists {
+		sktJSON := SecretKeyJSON{}
+		data, err := ioutil.ReadFile(pathToEncryptedSecret)
+		if err != nil {
+			return nil, errors.Wrap(err, "unexpected error while reading encrypted secret data")
+		}
+
+		if err := json.Unmarshal(data, &sktJSON); err != nil {
+			return nil, errors.Wrap(err, "unexpected error while reading encrypted secret data")
+		}
+
+		secretKey, err := d.loadKey()
+		if err != nil {
+			return nil, errors.Wrap(err, "failed to load secret key")
+		}
+		return secretKey.decrypt(sktJSON.IV, sktJSON.Ciphertext)
 	}
 
+	// else return unencrypted secret
+	pathToUnencryptedSecret := filepath.Join(d.basePath, secret.Group, secret.Name)
+	ret, err := ioutil.ReadFile(pathToUnencryptedSecret)
+	if err != nil {
+		return nil, errors.Wrap(err, "could not read unencrypted secrets data (secret may not have been generated yet)")
+	}
 	return ret, err
 }
 
 func (d *diskStore) SetSecret(secret SecretName, data []byte) error {
-	parentDir := filepath.Join(d.basePath, secret.Group)
-	secretPath := filepath.Join(parentDir, secret.Name)
-	err := os.MkdirAll(parentDir, 0700)
+	secretKey, err := d.loadKey()
 	if err != nil {
+		return errors.Wrap(err, "failed to load secret key")
+	}
+
+	parentDir := filepath.Join(d.basePath, secret.Group)
+	secretPath := filepath.Join(parentDir, addSecretFileExtension(secret.Name))
+
+	if err := os.MkdirAll(parentDir, 0700); err != nil {
 		return errors.Wrap(err, "could not create directory for secrets data")
 	}
-	err = os.Chown(parentDir, d.ownerUid, d.ownerGid)
-	if err != nil {
+
+	if err := os.Chown(parentDir, d.ownerUid, d.ownerGid); err != nil {
 		return errors.Wrap(err, "could not chown directory for secrets data")
 	}
 
-	r := bytes.NewReader(data)
-	err = fileutils.AtomicWrite(secretPath, r, fileutils.WithAtomicWriteFileMode(0700))
+	iv, ct, err := secretKey.encrypt(data)
 	if err != nil {
-		return errors.Wrap(err, "could not write secrets data to disk")
+		return err
 	}
 
-	// TODO(ssd) 2018-08-20: Should this be an option we can pass to AtomicWrite?
-	err = os.Chown(secretPath, d.ownerUid, d.ownerGid)
-	if err != nil {
-		return errors.Wrap(err, "could not chown secrets data")
+	buf := new(bytes.Buffer)
+
+	if err := json.NewEncoder(buf).Encode(SecretKeyJSON{
+		Algorithm:  "AES256 CTR mode",
+		IV:         iv,
+		Ciphertext: ct,
+	}); err != nil {
+		return errors.Wrap(err, "failed to encode struct SecretKeyJSON data to json")
+	}
+
+	if err := fileutils.AtomicWrite(secretPath, bytes.NewReader(buf.Bytes()),
+		fileutils.WithAtomicWriteFileMode(0700),
+		fileutils.WithAtomicWriteChown(d.ownerUid, d.ownerGid),
+	); err != nil {
+		return errors.Wrap(err, "error writing the secret file")
 	}
 
 	return nil
@@ -229,3 +286,7 @@ func PrepareSSHPrivateKey(keyContent string) (string, error) {
 	}
 	return keyPath.Name(), nil
 }
+
+func addSecretFileExtension(name string) string {
+	return name + SecretFileExtension
+}


Test patch:
diff --git a/lib/secrets/encrypt_test.go b/lib/secrets/encrypt_test.go
new file mode 100644
index 000000000..e4a4f72b1
--- /dev/null
+++ b/lib/secrets/encrypt_test.go
@@ -0,0 +1,27 @@
+package secrets
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestSecretStoreKey(t *testing.T) {
+	keyData, err := randomBytes(32)
+	require.NoError(t, err)
+	k := secretStoreKey{
+		keyData: keyData,
+	}
+
+	plaintext, err := randomBytes(1477)
+	require.NoError(t, err)
+	iv, ct, err := k.encrypt(plaintext)
+	require.NoError(t, err)
+	require.Len(t, iv, 16)
+	require.Len(t, ct, 1477)
+	require.NotEqual(t, plaintext, ct)
+
+	pt, err := k.decrypt(iv, ct)
+	require.NoError(t, err)
+	require.Equal(t, []byte(plaintext), pt)
+}
diff --git a/lib/secrets/secrets_helpers_test.go b/lib/secrets/secrets_helpers_test.go
new file mode 100644
index 000000000..e9aa27fe5
--- /dev/null
+++ b/lib/secrets/secrets_helpers_test.go
@@ -0,0 +1,19 @@
+package secrets
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestMarshalers(t *testing.T) {
+	iv := initializationVector{0xff, 0xee, 0x0d, 0x00}
+	v, err := iv.MarshalText()
+	require.NoError(t, err)
+	require.Equal(t, []byte("ffee0d00"), v)
+
+	iv2 := initializationVector{}
+	err = iv2.UnmarshalText([]byte("ffee0d00"))
+	require.NoError(t, err)
+	require.Equal(t, iv, iv2)
+}
diff --git a/lib/secrets/secrets_test.go b/lib/secrets/secrets_test.go
index 7b4c0e5fe..4ff9701b9 100644
--- a/lib/secrets/secrets_test.go
+++ b/lib/secrets/secrets_test.go
@@ -1,12 +1,16 @@
 package secrets_test
 
 import (
+	"bytes"
 	"io/ioutil"
 	"os"
 	"path/filepath"
 	"syscall"
 	"testing"
 
+	"github.com/leanovate/gopter"
+	"github.com/leanovate/gopter/gen"
+	"github.com/leanovate/gopter/prop"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
@@ -61,7 +65,7 @@ func TestDiskSecretStore(t *testing.T) {
 		err := dataStore.SetSecret(testSecretName, testSecretContent)
 		require.NoError(t, err)
 
-		stat, err := os.Stat(filepath.Join(dataDir, "test", "test-secret-1"))
+		stat, err := os.Stat(filepath.Join(dataDir, "test", "test-secret-1"+secrets.SecretFileExtension))
 		require.NoError(t, err)
 		assert.Equal(t, os.FileMode(0700), stat.Mode())
 		assert.Equal(t, uint32(uid), stat.Sys().(*syscall.Stat_t).Uid)
@@ -94,4 +98,82 @@ func TestDiskSecretStore(t *testing.T) {
 		require.NoError(t, err)
 		assert.False(t, exists)
 	})
+
+	// checks if initialise method is setting the encryption key properly
+	t.Run("init creates the key file with mode 0700", func(t *testing.T) {
+		stat, err := os.Stat(filepath.Join(dataDir, "key"))
+		require.NoError(t, err)
+		assert.Equal(t, os.FileMode(0700), stat.Mode())
+		assert.Equal(t, uint32(uid), stat.Sys().(*syscall.Stat_t).Uid)
+	})
+
+	// checks if GetSecret method is able to read old unencrypted secrets
+	t.Run("Able to read preexisting unencrypted secrets", func(t *testing.T) {
+		testSecretName := secrets.SecretName{"test", "test-secret-3"}
+		testSecretContent := []byte("this-is-a-secret")
+
+		// creates group directory with owner permissions
+		parentDir := filepath.Join(dataDir, testSecretName.Group)
+		secretPath := filepath.Join(parentDir, testSecretName.Name)
+		err := os.MkdirAll(parentDir, 0700)
+		require.NoError(t, err)
+		err = os.Chown(parentDir, uid, gid)
+		require.NoError(t, err)
+
+		// creates secret file without extension and sets owner permissions
+		s := bytes.NewReader(testSecretContent)
+		err = fileutils.AtomicWrite(secretPath, s, fileutils.WithAtomicWriteFileMode(0700))
+		require.NoError(t, err)
+		err = os.Chown(parentDir, uid, gid)
+		require.NoError(t, err)
+
+		secretContent, err := dataStore.GetSecret(testSecretName)
+		require.NoError(t, err)
+		assert.Equal(t, testSecretContent, secretContent)
+	})
+
+	// checks if GetSecret method is able to properly get the secrets
+	// set using SetSecret method
+	// uses multiple randomly generated secret name, secret group
+	// genrates content of length range from 1 byte to 5 kb
+	t.Run("get and set can roundtrip a secret with random string combinations", func(t *testing.T) {
+		parameters := gopter.DefaultTestParameters()
+		parameters.MinSuccessfulTests = 500
+		parameters.MaxSize = 5 * 1024
+		properties := gopter.NewProperties(parameters)
+		properties.Property("generate multiple random test cases to check if get and set can roundtrip a secret", prop.ForAll(
+			func(group string, name string, plaintext []byte) bool {
+				testSecretName := secrets.SecretName{group, name}
+				if err != nil {
+					return reportErrorAndYieldFalse(t, err)
+				}
+				err = dataStore.SetSecret(testSecretName, plaintext)
+				if err != nil {
+					return reportErrorAndYieldFalse(t, err)
+				}
+				secretContent, err := dataStore.GetSecret(testSecretName)
+				if err != nil {
+					return reportErrorAndYieldFalse(t, err)
+				}
+				return bytes.Compare(plaintext, secretContent) == 0
+			},
+			stringGen(10).WithLabel("group"),
+			stringGen(10).WithLabel("name"),
+			gen.SliceOf(gen.UInt8Range(0, 255)).WithLabel("plaintext"),
+		))
+		properties.TestingRun(t)
+	})
+}
+
+// gopter helpers
+func stringGen(size int) gopter.Gen {
+	return gen.SliceOfN(size, gen.AlphaNumChar()).Map(func(r []rune) string {
+		return string(r)
+	}).WithShrinker(gen.StringShrinker)
+}
+
+func reportErrorAndYieldFalse(t *testing.T, err error) bool {
+	t.Helper()
+	t.Error(err.Error())
+	return false
 }