Repository: TBD54566975/ssi-service (go)

Issue:
Remove password config for encryption key
The password is not necessary. The service can generate a key from a source of randomness on boot.

We would need to remove the use of argon2 and directly generate a 32 byte secret from a source of randomness. The rest would work the same.

Pull request:
remove password config
# Overview
Fixes #567. Removed the password from the keystore config. 

# Description
This is removing some functionality that is not needed.

# How Has This Been Tested?
Unit tests and integration tests all pass.



Patch:
diff --git a/README.md b/README.md
index 718ac9a..2da868a 100644
--- a/README.md
+++ b/README.md
@@ -49,7 +49,7 @@ service.
 
 SSI-service can store keys that are used to digitally sign credentials (and other data). All such keys are encrypted at
 the application before being stored using a MasterKey (a.k.a. a Key Encryption Key or KEK). The MasterKey can be
-generated automatically during boot time based on a password, or we can use the MasterKey housed in an external Key 
+generated automatically during boot time, or we can use the MasterKey housed in an external Key 
 Management System (KMS) like GCP KMS or AWS KMS.
 
 For production deployments, using external KMS is strongly recommended.
@@ -61,9 +61,8 @@ To use an external KMS:
 3. Set the `kms_credentials_path` field of the `[services.keystore]` section to point to your credentials file, according to [this section](https://github.com/google/tink/blob/9bc2667963e20eb42611b7581e570f0dddf65a2b/docs/KEY-MANAGEMENT.md#credentials).
 4. Win!
 
-To use a password based MasterKey (NOT RECOMMENDED FOR ANY PRODUCTION ENVIRONMENT):
+To use a randomly generated encryption key (NOT RECOMMENDED FOR ANY PRODUCTION ENVIRONMENT):
 1. Make sure that `master_key_uri` and `kms_credentials_path` of the `[services.keystore]` section are not set.
-2. Set the `password` field of the `[services.keystore]` section to your desired password.
 
 Note that at this time, we do not currently support rotating the master key.
 
diff --git a/config/config.go b/config/config.go
index 847976f..7d1433a 100644
--- a/config/config.go
+++ b/config/config.go
@@ -29,9 +29,8 @@ const (
 	EnvironmentTest Environment = "test"
 	EnvironmentProd Environment = "prod"
 
-	ConfigPath       EnvironmentVariable = "CONFIG_PATH"
-	KeystorePassword EnvironmentVariable = "KEYSTORE_PASSWORD"
-	DBPassword       EnvironmentVariable = "DB_PASSWORD"
+	ConfigPath EnvironmentVariable = "CONFIG_PATH"
+	DBPassword EnvironmentVariable = "DB_PASSWORD"
 )
 
 type (
@@ -94,13 +93,9 @@ type BaseServiceConfig struct {
 
 type KeyStoreServiceConfig struct {
 	*BaseServiceConfig
-	// Master key password. Used by a KDF whose key is used by a symmetric cypher for key encryption.
-	// The password is salted before usage.
-	// Note that this field is only used when MasterKeyURI is empty.
-	MasterKeyPassword string `toml:"password"`
 
 	// The URI for the master key. We use tink for envelope encryption as described in https://github.com/google/tink/blob/9bc2667963e20eb42611b7581e570f0dddf65a2b/docs/KEY-MANAGEMENT.md#key-management-with-tink
-	// When left empty, then MasterKeyPassword is used.
+	// When left empty, then a random key is generated and used.
 	MasterKeyURI string `toml:"master_key_uri"`
 
 	// Path for credentials. Required when using an external KMS. More info at https://github.com/google/tink/blob/9bc2667963e20eb42611b7581e570f0dddf65a2b/docs/KEY-MANAGEMENT.md#credentials
@@ -288,7 +283,6 @@ func getDefaultServicesConfig() ServicesConfig {
 		ServiceEndpoint: DefaultServiceEndpoint,
 		KeyStoreConfig: KeyStoreServiceConfig{
 			BaseServiceConfig: &BaseServiceConfig{Name: "keystore", ServiceEndpoint: DefaultServiceEndpoint + "/v1/keys"},
-			MasterKeyPassword: "default-password",
 		},
 		DIDConfig: DIDServiceConfig{
 			BaseServiceConfig:      &BaseServiceConfig{Name: "did", ServiceEndpoint: DefaultServiceEndpoint + "/v1/dids"},
@@ -396,11 +390,6 @@ func applyEnvVariables(config *SSIServiceConfig) error {
 		return errors.Wrap(err, "dotenv parsing")
 	}
 
-	keystorePassword, present := os.LookupEnv(KeystorePassword.String())
-	if present {
-		config.Services.KeyStoreConfig.MasterKeyPassword = keystorePassword
-	}
-
 	dbPassword, present := os.LookupEnv(DBPassword.String())
 	if present {
 		if len(config.Services.StorageOptions) != 0 {
diff --git a/config/dev.toml b/config/dev.toml
index 952a9c9..a3f4168 100644
--- a/config/dev.toml
+++ b/config/dev.toml
@@ -36,7 +36,6 @@ option = "bolt.db"
 # per-service configuration
 [services.keystore]
 name = "keystore"
-password = "default-password"
 
 [services.did]
 name = "did"
diff --git a/config/prod.toml b/config/prod.toml
index a962017..025a003 100644
--- a/config/prod.toml
+++ b/config/prod.toml
@@ -38,7 +38,6 @@ option = "password"
 # per-service configuration
 [services.keystore]
 name = "keystore"
-password = "default-password"
 # master_key_uri = "gcp-kms://projects/*/locations/*/keyRings/*/cryptoKeys/*"
 # kms_credentials_path = "credentials.json"
 
diff --git a/pkg/service/keystore/service.go b/pkg/service/keystore/service.go
index dacc5b2..2975a88 100644
--- a/pkg/service/keystore/service.go
+++ b/pkg/service/keystore/service.go
@@ -168,23 +168,15 @@ func (s Service) GetKeyDetails(ctx context.Context, request GetKeyDetailsRequest
 	}, nil
 }
 
-// GenerateServiceKey using argon2 for key derivation generate a service key and corresponding salt,
-// base58 encoding both values.
-func GenerateServiceKey(skPassword string) (key, salt string, err error) {
-	saltBytes, err := util.GenerateSalt(util.Argon2SaltSize)
+// GenerateServiceKey creates a random key that's 32 bytes encoded using base58.
+func GenerateServiceKey() (key string, err error) {
+	keyBytes, err := util.GenerateSalt(chacha20poly1305.KeySize)
 	if err != nil {
-		err = errors.Wrap(err, "generating salt for service key")
-		return "", "", sdkutil.LoggingError(err)
-	}
-
-	keyBytes, err := util.Argon2KeyGen(skPassword, saltBytes, chacha20poly1305.KeySize)
-	if err != nil {
-		err = errors.Wrap(err, "generating key for service key")
-		return "", "", sdkutil.LoggingError(err)
+		err = errors.Wrap(err, "generating bytes for service key")
+		return "", sdkutil.LoggingError(err)
 	}
 
 	key = base58.Encode(keyBytes)
-	salt = base58.Encode(saltBytes)
 	return
 }
 
diff --git a/pkg/service/keystore/storage.go b/pkg/service/keystore/storage.go
index 4c1eaba..f3b595d 100644
--- a/pkg/service/keystore/storage.go
+++ b/pkg/service/keystore/storage.go
@@ -123,14 +123,13 @@ func EnsureServiceKeyExists(config config.KeyStoreServiceConfig, provider storag
 		// Create the key only if it doesn't already exist.
 		gotKey, err := getServiceKey(ctx, provider)
 		if gotKey == nil && err.Error() == keyNotFoundErrMsg {
-			serviceKey, serviceKeySalt, err := GenerateServiceKey(config.MasterKeyPassword)
+			serviceKey, err := GenerateServiceKey()
 			if err != nil {
 				return nil, errors.Wrap(err, "generating service key")
 			}
 
 			key := ServiceKey{
-				Base58Key:  serviceKey,
-				Base58Salt: serviceKeySalt,
+				Base58Key: serviceKey,
 			}
 			if err := storeServiceKey(ctx, tx, key); err != nil {
 				return nil, err


Test patch:
diff --git a/config/test.toml b/config/test.toml
index 65f1785..141c6a4 100644
--- a/config/test.toml
+++ b/config/test.toml
@@ -41,7 +41,6 @@ option = "password"
 # per-service configuration
 [services.keystore]
 name = "keystore"
-password = "default-password"
 # master_key_uri = "gcp-kms://projects/*/locations/*/keyRings/*/cryptoKeys/*"
 # kms_credentials_path = "credentials.json"
 
diff --git a/pkg/server/router/keystore_test.go b/pkg/server/router/keystore_test.go
index 6ffe57b..51b5387 100644
--- a/pkg/server/router/keystore_test.go
+++ b/pkg/server/router/keystore_test.go
@@ -38,7 +38,6 @@ func TestKeyStoreRouter(t *testing.T) {
 
 				serviceConfig := config.KeyStoreServiceConfig{
 					BaseServiceConfig: &config.BaseServiceConfig{Name: "keystore"},
-					MasterKeyPassword: "test-password",
 				}
 				keyStoreService, err := keystore.NewKeyStoreService(serviceConfig, db)
 				assert.NoError(tt, err)
diff --git a/pkg/server/router/router_test.go b/pkg/server/router/router_test.go
index 8fd86ac..cb02e6e 100644
--- a/pkg/server/router/router_test.go
+++ b/pkg/server/router/router_test.go
@@ -21,7 +21,7 @@ func (s *testService) Status() framework.Status {
 func (s *testService) Config() config.ServicesConfig {
 	return config.ServicesConfig{
 		StorageProvider:  "bolt",
-		KeyStoreConfig:   config.KeyStoreServiceConfig{MasterKeyPassword: "test-password"},
+		KeyStoreConfig:   config.KeyStoreServiceConfig{},
 		DIDConfig:        config.DIDServiceConfig{Methods: []string{string(didsdk.KeyMethod)}},
 		SchemaConfig:     config.SchemaServiceConfig{},
 		CredentialConfig: config.CredentialServiceConfig{},
diff --git a/pkg/server/router/testutils_test.go b/pkg/server/router/testutils_test.go
index 4d422b5..f33ae87 100644
--- a/pkg/server/router/testutils_test.go
+++ b/pkg/server/router/testutils_test.go
@@ -24,7 +24,7 @@ func TestMain(t *testing.M) {
 }
 
 func testKeyStoreService(t *testing.T, db storage.ServiceStorage) *keystore.Service {
-	serviceConfig := config.KeyStoreServiceConfig{MasterKeyPassword: "test-password"}
+	serviceConfig := config.KeyStoreServiceConfig{}
 	// create a keystore service
 	keystoreService, err := keystore.NewKeyStoreService(serviceConfig, db)
 	require.NoError(t, err)
diff --git a/pkg/server/server_test.go b/pkg/server/server_test.go
index d6b810e..58b0e0b 100644
--- a/pkg/server/server_test.go
+++ b/pkg/server/server_test.go
@@ -226,7 +226,6 @@ func testKeyStore(t *testing.T, bolt storage.ServiceStorage) (*router.KeyStoreRo
 func testKeyStoreService(t *testing.T, db storage.ServiceStorage) (*keystore.Service, keystore.ServiceFactory) {
 	serviceConfig := config.KeyStoreServiceConfig{
 		BaseServiceConfig: &config.BaseServiceConfig{Name: "test-keystore"},
-		MasterKeyPassword: "test-password",
 	}
 
 	// create a keystore service
diff --git a/pkg/service/did/ion_test.go b/pkg/service/did/ion_test.go
index ff4e5df..6fdd038 100644
--- a/pkg/service/did/ion_test.go
+++ b/pkg/service/did/ion_test.go
@@ -286,7 +286,6 @@ func TestIONHandler(t *testing.T) {
 func testKeyStoreService(t *testing.T, db storage.ServiceStorage) *keystore.Service {
 	serviceConfig := config.KeyStoreServiceConfig{
 		BaseServiceConfig: &config.BaseServiceConfig{Name: "test-keystore"},
-		MasterKeyPassword: "test-password",
 	}
 
 	// create a keystore service
diff --git a/pkg/service/keystore/service_test.go b/pkg/service/keystore/service_test.go
index e202e15..4595aef 100644
--- a/pkg/service/keystore/service_test.go
+++ b/pkg/service/keystore/service_test.go
@@ -18,20 +18,13 @@ import (
 )
 
 func TestGenerateServiceKey(t *testing.T) {
-	emptySKPassword := ""
-	_, _, err := GenerateServiceKey(emptySKPassword)
-	assert.Error(t, err)
-
-	skPassword := "test-password"
-	key, salt, err := GenerateServiceKey(skPassword)
+	key, err := GenerateServiceKey()
 	assert.NoError(t, err)
 	assert.NotEmpty(t, key)
-	assert.NotEmpty(t, salt)
 }
 
 func TestEncryptDecryptAllKeyTypes(t *testing.T) {
-	skPassword := "test-password"
-	serviceKeyEncoded, _, err := GenerateServiceKey(skPassword)
+	serviceKeyEncoded, err := GenerateServiceKey()
 	assert.NoError(t, err)
 	serviceKey, err := base58.Decode(serviceKeyEncoded)
 	assert.NoError(t, err)
@@ -184,7 +177,6 @@ func createKeyStoreService(t *testing.T) (*Service, error) {
 			BaseServiceConfig: &config.BaseServiceConfig{
 				Name: "test-keyStore",
 			},
-			MasterKeyPassword: "test-password",
 		},
 		s)
 
diff --git a/pkg/testutil/well_known_generation_test.go b/pkg/testutil/well_known_generation_test.go
index 08e3131..b7f7f18 100644
--- a/pkg/testutil/well_known_generation_test.go
+++ b/pkg/testutil/well_known_generation_test.go
@@ -152,7 +152,7 @@ func createWellKnownDIDConfiguration(tt *testing.T, didResponse *did.CreateDIDRe
 }
 
 func testKeyStoreService(t *testing.T, db storage.ServiceStorage) *keystore.Service {
-	serviceConfig := config.KeyStoreServiceConfig{MasterKeyPassword: "test-password"}
+	serviceConfig := config.KeyStoreServiceConfig{}
 	// create a keystore service
 	keystoreService, err := keystore.NewKeyStoreService(serviceConfig, db)
 	require.NoError(t, err)