Repository: moov-io/customers
Pull request #233 (2020-10-08 17:27:40)

Specify secret key per type
This change removes the environment variable that was previously used to specify a shared encryption key for local secrets. 

Per #198 , Documents and SSN storage can each specify their own local KMS key. See updates to Configuration.md for details.


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index a8e009e..0748e84 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -28,7 +28,7 @@ IMPROVEMENTS
 - cmd/server: read OFAC_ENDPOINT or WATCHMAN_ENDPOINT
 - customers: send back an array of search results, not null
 - database/mysql: fix migration for customer type
-- docs: reference ./cmd/genkey/ for SECRETS_LOCAL_BASE64_KEY
+- docs: reference ./cmd/genkey/ for [local secret keys](./docs/CONFIGURATION.md#local-filesystem-local)
 - fed: support debugging API calls
 - paygate: support debugging API calls
 - watchman: support debugging API calls
diff --git a/cmd/server/main.go b/cmd/server/main.go
index 583710d..2b7d898 100644
--- a/cmd/server/main.go
+++ b/cmd/server/main.go
@@ -137,7 +137,7 @@ func main() {
 
 	// Setup Customer SSN storage wrapper
 	ctx := context.Background()
-	keeper, err := secrets.OpenSecretKeeper(ctx, "customer-ssn", os.Getenv("SSN_SECRET_PROVIDER"))
+	keeper, err := secrets.OpenSecretKeeper(ctx, "customer-ssn", os.Getenv("SSN_SECRET_PROVIDER"), os.Getenv("SSN_SECRET_KEY"))
 	if err != nil {
 		panic(err)
 	}
@@ -165,8 +165,6 @@ func main() {
 		Repo: accountsRepo, WatchmanClient: watchmanClient,
 	}
 
-	bucket := storage.GetBucket(logger, util.Or(os.Getenv("DOCUMENTS_BUCKET_NAME"), "./storage"), util.Or(os.Getenv("DOCUMENTS_STORAGE_PROVIDER"), "file"), signer)
-
 	// Setup business HTTP routes
 	router := mux.NewRouter()
 	moovhttp.AddCORSHandler(router)
@@ -176,7 +174,12 @@ func main() {
 	customers.AddCustomerAddressRoutes(logger, router, customerRepo)
 	documents.AddDisclaimerRoutes(logger, router, disclaimerRepo)
 
-	docsKeeper, err := secrets.OpenSecretKeeper(context.Background(), "customer-documents", util.Or(os.Getenv("DOCUMENTS_SECRET_PROVIDER"), "local"))
+	docsStorageProvider := util.Or(os.Getenv("DOCUMENTS_STORAGE_PROVIDER"), "file")
+	docsBucketName := util.Or(os.Getenv("DOCUMENTS_BUCKET_NAME"), "./storage")
+	bucket := storage.GetBucket(logger, docsBucketName, docsStorageProvider, signer)
+
+	docsSecretProvider := util.Or(os.Getenv("DOCUMENTS_SECRET_PROVIDER"), "local")
+	docsKeeper, err := secrets.OpenSecretKeeper(context.Background(), "customer-documents", docsSecretProvider, os.Getenv("DOCUMENTS_SECRET_KEY"))
 	if err != nil {
 		panic(err)
 	}
diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md
index 28f2999..7c83346 100644
--- a/docs/CONFIGURATION.md
+++ b/docs/CONFIGURATION.md
@@ -1,5 +1,5 @@
 # Customers
-**[Purpose](README.md)** | **Configuration** | **[Running](RUNNING.md)** | **[Client](../pkg/client/README.md)**
+**[Purpose](README.md)** | **[Configuration](#configuration)** | **[Persistence](#persistent-storage)** | **[Running](RUNNING.md)** | **[Client](../pkg/client/README.md)**
 
 ---
 
@@ -123,17 +123,25 @@ For more information see the [Go Cloud Development Kit docs for fileblob](https:
 
 The following environment variables control which backend service is utilized for secret key management. These all follow a similar [key management](https://gocloud.dev/howto/secrets/) API provided by a library that Google [built and maintains](https://github.com/google/go-cloud).
 
-- `DOCUMENTS_SECRET_PROVIDER`: Determines which environment variables are used to initialize persistant document storage. Defaults to [local filesystem](#local-filesystem).
-- `SSN_SECRET_PROVIDER`: Determines which environment variables are used to initialize SSN storage persistence. Defaults to [local filesystem](#local-filesystem).
+- `DOCUMENTS_SECRET_PROVIDER`: Determines which environment variables are used to initialize persistant document storage. Defaults to `local` (see [local filesystem](#local-filesystem)).
+- `SSN_SECRET_PROVIDER`: Determines which environment variables are used to initialize SSN storage persistence. Defaults to `local` (see [local filesystem](#local-filesystem)).
+  - `SSN_SECRET_KEY`: Holds the documents encryption/decryption key **if** the documents secret provider is `local`.
 
 ##### Local Filesystem (`local`)
 
-- `SECRETS_LOCAL_BASE64_KEY`: A URI used to encrypt account numbers for storage in the database. This value needs to look like `base64key://value` where `value` is a base64 encoded 32 byte random key.
-   - Generate this key by running `./cmd/genkey/` and copying the `base64key://...` value
+The local secrets keeper (see [GoCloud Dev Kit - Secrets](https://gocloud.dev/howto/secrets/#local)) uses a 32 byte, base64 encoded encryption/decryption key. This value must be in the form `base64key://value` where `value` is the base64 encoded 32 byte random key.
+
+This repository provides a script for generating properly formatted local keys (see ./cmd/genkey). New keys can be generated by running `go run ./cmd/genkey`
+
+- `TRANSIT_LOCAL_BASE64_KEY`: The secret key to encrypt account numbers for storage in the database.
+- `DOCUMENTS_SECRET_KEY`: The encryption/decryption key used for document storage and retrieval **if** the documents secret provider is `local`.
+- `SSN_SECRET_KEY`: The encryption/decryption key used for customer SSN storage and retrieval **if** the SSN secret provider is `local`.
 
 ##### Google Cloud Storage (`gcp`)
 
-- `SECRETS_GCP_KEY_RESOURCE_ID`: A Google Cloud resource ID used to interact with their Key Management Service (KMS). This value has the form `projects/MYPROJECT/locations/MYLOCATION/keyRings/MYKEYRING/cryptoKeys/MYKEY` and [their documentation has more details](https://cloud.google.com/kms/docs/object-hierarchy#key).
+This secrets provider uses the [Google Cloud KMS](https://cloud.google.com/kms/docs/object-hierarchy#key). Secret Keys are identified by a GCP Resource ID in the form `projects/project-id/locations/location/keyRings/keyring/cryptoKeys/key` and [their documentation has more details]().
+
+- `SECRETS_GCP_KEY_RESOURCE_ID`: A Google Cloud resource ID used to interact with their Key Management Service (KMS). 
 
 ##### HashiCorp Vault Storage (`vault`)
 
diff --git a/pkg/secrets/secrets.go b/pkg/secrets/secrets.go
index bc51354..bed8a72 100644
--- a/pkg/secrets/secrets.go
+++ b/pkg/secrets/secrets.go
@@ -82,28 +82,13 @@ func (str *StringKeeper) DecryptString(in string) (string, error) {
 	return string(bs), nil
 }
 
-type SecretFunc func(path string) (*secrets.Keeper, error)
-
-var (
-	GetSecretKeeper SecretFunc = func(path string) (*secrets.Keeper, error) {
-		if path == "" {
-			return nil, errors.New("GetSecretKeeper: nil path")
-		}
-
-		ctx, cancelFn := context.WithTimeout(context.TODO(), 10*time.Second)
-		defer cancelFn()
-
-		return OpenSecretKeeper(ctx, path, os.Getenv("SSN_SECRET_PROVIDER"))
-	}
-)
-
 // OpenSecretKeeper returns a Go Cloud Development Kit (Go CDK) Keeper object which can be used
 // to encrypt and decrypt byte slices and stored in various services.
 // Checkout https://gocloud.dev/ref/secrets/ for more details.
-func OpenSecretKeeper(ctx context.Context, path, cloudProvider string) (*secrets.Keeper, error) {
+func OpenSecretKeeper(ctx context.Context, path, cloudProvider string, localBase64Key string) (*secrets.Keeper, error) {
 	switch strings.ToLower(cloudProvider) {
 	case "", "local":
-		return OpenLocal(os.Getenv("SECRETS_LOCAL_BASE64_KEY"))
+		return OpenLocal(localBase64Key)
 	case "gcp":
 		return openGCPKMS()
 	case "vault":
@@ -117,28 +102,27 @@ func OpenSecretKeeper(ctx context.Context, path, cloudProvider string) (*secrets
 // The scheme for the base64 key should look like: 'base64key://'
 // The URL hostname must be a base64-encoded key, of length 32 bytes when decoded.
 func OpenLocal(base64Key string) (*secrets.Keeper, error) {
-	var key [32]byte
-	if base64Key != "" {
-		if strings.HasPrefix(base64Key, localsecrets.Scheme) {
-			keeper, err := secrets.OpenKeeper(context.Background(), base64Key)
-			if err != nil {
-				return nil, fmt.Errorf("problem reading base64 url key: %v", err)
-			}
-			return keeper, nil
-		} else {
-			k, err := localsecrets.Base64Key(base64Key)
-			if err != nil {
-				return nil, fmt.Errorf("problem reading base64key: %v", err)
-			}
-			key = k
-		}
-	} else {
+	if base64Key == "" {
 		k, err := localsecrets.Base64Key(base64.StdEncoding.EncodeToString(bytes.Repeat([]byte("1"), 32)))
 		if err != nil {
 			return nil, err
 		}
-		key = k
+		return localsecrets.NewKeeper(k), nil
 	}
+
+	if strings.HasPrefix(base64Key, localsecrets.Scheme) {
+		keeper, err := secrets.OpenKeeper(context.Background(), base64Key)
+		if err != nil {
+			return nil, fmt.Errorf("problem reading base64 url key: %v", err)
+		}
+		return keeper, nil
+	}
+
+	key, err := localsecrets.Base64Key(base64Key)
+	if err != nil {
+		return nil, fmt.Errorf("problem reading base64key: %v", err)
+	}
+
 	return localsecrets.NewKeeper(key), nil
 }
 


Test patch:
diff --git a/pkg/secrets/secrets_test.go b/pkg/secrets/secrets_test.go
index 2ee34f3..9ce57c6 100644
--- a/pkg/secrets/secrets_test.go
+++ b/pkg/secrets/secrets_test.go
@@ -9,20 +9,22 @@ import (
 	"strings"
 	"testing"
 	"time"
+
+	"github.com/stretchr/testify/require"
 )
 
 func TestSecrets(t *testing.T) {
-	// We assume SSN_SECRET_PROVIDER is unset
-	keeper, err := GetSecretKeeper("foo")
+	ctx := context.Background()
+	keeper, err := OpenSecretKeeper(ctx, "foo", "", "")
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	encrypted, err := keeper.Encrypt(context.Background(), []byte("hello, world"))
+	encrypted, err := keeper.Encrypt(ctx, []byte("hello, world"))
 	if err != nil {
 		t.Fatal(err)
 	}
-	out, err := keeper.Decrypt(context.Background(), encrypted)
+	out, err := keeper.Decrypt(ctx, encrypted)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -132,6 +134,10 @@ func TestOpenSecretKeeper(t *testing.T) {
 	// Just call these and make sure they don't panic.
 	//
 	// The result depends on env variables, which in TravisCI is different than local.
-	OpenSecretKeeper(ctx, "", "gcp")
-	OpenSecretKeeper(ctx, "", "vault")
+	require.NotPanics(t, func() { OpenSecretKeeper(ctx, "", "gcp", "") })
+	require.NotPanics(t, func() { OpenSecretKeeper(ctx, "", "vault", "") })
+	require.NotPanics(t, func() { OpenSecretKeeper(ctx, "", "", "superSecretKey") })
+	require.NotPanics(t, func() { OpenSecretKeeper(ctx, "", "local", "superSecretKey") })
+	_, err := OpenSecretKeeper(ctx, "", "foo", "")
+	require.Error(t, err)
 }