Simplify crypto config structure

config/server-config.yaml.example

@@ -106,13 +106,12 @@ authz:
 #    name: healthcheck
 
 crypto:
-    key_store:
+    keystore:
         type: local
-        config:
+        local:
             key_dir: "./.ssh"
     default:
         key_id:    token_key_passphrase
         algorithm: aes-256-gcm
     fallback:
-        algorithms:
-            - aes-256-cfb
+        algorithm: aes-256-cfb

deployment/helm/templates/deployment.yaml

@@ -69,10 +69,8 @@ spec:
           - "--github-app-client-id-file=/secrets/github-app/github_app_client_id"
           - "--github-app-client-secret-file=/secrets/github-app/github_app_client_secret"
           env:
-          - name: "MINDER_KEY_PATH"
+          - name: "MINDER_CRYPTO_KEYSTORE_LOCAL_KEY_DIR"
             value: "/secrets/auth/"
-          - name: "MINDER_DEFAULT_TOKEN_FILENAME"
-            value: "token_key_passphrase"
           # TODO: remove this value once we migrate to the new structure
           - name: "MINDER_AUTH_TOKEN_KEY"
             value: "/secrets/auth/token_key_passphrase"
@@ -152,7 +150,7 @@ spec:
       - name: flags
         configMap:
           name: minder-flags
-          optional: true   # We expect the outside environment to create this ConfigMap
+          optional: true  # We expect the outside environment to create this ConfigMap
           items:
           - key: flags-config.yaml
             path: flags-config.yaml

docs/docs/run_minder_server/run_the_server.md

@@ -198,10 +198,16 @@ openssl rand -base64 32 > .ssh/token_key_passphrase
 If your key lives in a directory other than `.ssh`, you can specify the location of the key in the `server-config.yaml` file.
 
 ```yaml
-auth:
-   token_key: "./.ssh/token_key_passphrase"
+crypto:
+   keystore:
+      local:
+         key_dir: "./.ssh"
+   default:
+      key_id:    "token_key_passphrase"
 ```
 
+(Note that `key_id` is the filename, and `key_dir` is the path to the file)
+
 ## Configure the Repository Provider
 
 At this point, you should have the following:

internal/config/server/crypto.go

@@ -16,33 +16,37 @@ package server
 
 // CryptoConfig is the configuration for the crypto engine
 type CryptoConfig struct {
-	KeyStore KeyStoreConfig `mapstructure:"key_store"`
+	KeyStore KeyStoreConfig `mapstructure:"keystore"`
 	Default  DefaultCrypto  `mapstructure:"default"`
 	Fallback FallbackCrypto `mapstructure:"fallback"`
 }
 
 // KeyStoreConfig specifies the type of keystore to use and its configuration
+// If we support multiple types of keystore (e.g. local, aws secrets, vault, etc.)
+// it is intended that there will be one field for each type of keystore config,
+// and that the `Type` field specifies which one to use
 type KeyStoreConfig struct {
-	Type string `mapstructure:"type" default:"local"`
-	// is currently expected to match the structure of LocalFileKeyStoreConfig
-	Config map[string]any `mapstructure:"config"`
+	Type  string              `mapstructure:"type" default:"local"`
+	Local LocalKeyStoreConfig `mapstructure:"local"`
 }
 
 // DefaultCrypto defines the default crypto to be used for new data
 type DefaultCrypto struct {
+	// `token_key_passphrase` is the filename generated by `make bootstrap`
 	KeyID     string `mapstructure:"key_id"`
 	Algorithm string `mapstructure:"algorithm" default:"aes-256-gcm"`
 }
 
-// FallbackCrypto defines the optional list of keys and algorithms to fall
-// back to.
-// When rotating keys or algorithms, add the old ones here.
+// FallbackCrypto defines the optional key and algorithm which can be used for
+// decrypting old secrets.
+// This is used for rotating keys or algorithms.
 type FallbackCrypto struct {
-	KeyIDs     []string `mapstructure:"key_ids"`
-	Algorithms []string `mapstructure:"algorithms"`
+	KeyID     string `mapstructure:"key_id"`
+	Algorithm string `mapstructure:"algorithm" default:"aes-256-cfb"`
 }
 
-// LocalFileKeyStoreConfig contains configuration for the local file keystore
-type LocalFileKeyStoreConfig struct {
-	KeyDir string `mapstructure:"key_dir"`
+// LocalKeyStoreConfig contains configuration for the local file keystore
+type LocalKeyStoreConfig struct {
+	// `./.ssh/` is the directory generated by `make bootstrap`
+	KeyDir string `mapstructure:"key_dir" default:"./.ssh/"`
 }

internal/crypto/engine.go

@@ -65,7 +65,7 @@ type engine struct {
 func NewEngineFromConfig(config *serverconfig.Config) (Engine, error) {
 	// Use fallback if the new config structure is missing
 	var cryptoCfg serverconfig.CryptoConfig
-	if config.Crypto.Default.KeyID != "" && config.Crypto.Default.Algorithm != "" {
+	if config.Crypto.Default.KeyID != "" {
 		cryptoCfg = config.Crypto
 	} else if config.Auth.TokenKey != "" {
 		fallbackConfig, err := convertToCryptoConfig(&config.Auth)
@@ -88,9 +88,12 @@ func NewEngineFromConfig(config *serverconfig.Config) (Engine, error) {
 	}
 
 	// Instantiate all the algorithms we need
-	algos := append([]string{cryptoCfg.Default.Algorithm}, cryptoCfg.Fallback.Algorithms...)
-	supportedAlgorithms := make(algorithmsByName, len(algos))
-	for _, algoName := range algos {
+	algoNames := []string{cryptoCfg.Default.Algorithm}
+	if cryptoCfg.Fallback.Algorithm != "" {
+		algoNames = append(algoNames, cryptoCfg.Fallback.Algorithm)
+	}
+	supportedAlgorithms := make(algorithmsByName, len(algoNames))
+	for _, algoName := range algoNames {
 		algoType, err := algorithms.TypeFromString(algoName)
 		if err != nil {
 			return nil, err
@@ -229,8 +232,10 @@ func convertToCryptoConfig(a *serverconfig.AuthConfig) (serverconfig.CryptoConfi
 
 	return serverconfig.CryptoConfig{
 		KeyStore: serverconfig.KeyStoreConfig{
-			Type:   keystores.LocalKeyStore,
-			Config: map[string]any{"key_dir": dir},
+			Type: keystores.LocalKeyStore,
+			Local: serverconfig.LocalKeyStoreConfig{
+				KeyDir: dir,
+			},
 		},
 		Default: serverconfig.DefaultCrypto{
 			KeyID:     name,

internal/crypto/engine_test.go

@@ -36,8 +36,8 @@ func TestNewFromCryptoConfig(t *testing.T) {
 		Crypto: server.CryptoConfig{
 			KeyStore: server.KeyStoreConfig{
 				Type: "local",
-				Config: map[string]any{
-					"key_dir": "./testdata",
+				Local: server.LocalKeyStoreConfig{
+					KeyDir: "./testdata",
 				},
 			},
 			Default: server.DefaultCrypto{
@@ -77,8 +77,8 @@ func TestNewRejectsBadAlgo(t *testing.T) {
 		Crypto: server.CryptoConfig{
 			KeyStore: server.KeyStoreConfig{
 				Type: "local",
-				Config: map[string]any{
-					"key_dir": "./testdata",
+				Local: server.LocalKeyStoreConfig{
+					KeyDir: "./testdata",
 				},
 			},
 			Default: server.DefaultCrypto{
@@ -98,16 +98,16 @@ func TestNewRejectsBadFallbackAlgo(t *testing.T) {
 		Crypto: server.CryptoConfig{
 			KeyStore: server.KeyStoreConfig{
 				Type: "local",
-				Config: map[string]any{
-					"key_dir": "./testdata",
+				Local: server.LocalKeyStoreConfig{
+					KeyDir: "./testdata",
 				},
 			},
 			Default: server.DefaultCrypto{
 				KeyID:     "test_encryption_key",
 				Algorithm: string(algorithms.Aes256Cfb),
 			},
 			Fallback: server.FallbackCrypto{
-				Algorithms: []string{"what even is this?"},
+				Algorithm: "what even is this?",
 			},
 		},
 	}
@@ -146,16 +146,17 @@ func TestFallbackDecrypt(t *testing.T) {
 		Crypto: server.CryptoConfig{
 			KeyStore: server.KeyStoreConfig{
 				Type: "local",
-				Config: map[string]any{
-					"key_dir": "./testdata",
+				Local: server.LocalKeyStoreConfig{
+					KeyDir: "./testdata",
 				},
 			},
 			Default: server.DefaultCrypto{
 				KeyID:     "test_encryption_key",
 				Algorithm: string(algorithms.Aes256Gcm),
 			},
 			Fallback: server.FallbackCrypto{
-				Algorithms: []string{string(algorithms.Aes256Cfb)},
+				Algorithm: string(algorithms.Aes256Cfb),
+				KeyID:     "test_encryption_key2",
 			},
 		},
 	}

internal/crypto/keystores/keystore.go

@@ -21,8 +21,6 @@ import (
 	"os"
 	"path/filepath"
 
-	"github.com/go-viper/mapstructure/v2"
-
 	serverconfig "github.com/stacklok/minder/internal/config/server"
 )
 
@@ -45,29 +43,24 @@ type keysByID map[string][]byte
 // Since our only implementation is based on reading from the local disk, do
 // all key loading during construction of the struct.
 func NewKeyStoreFromConfig(config serverconfig.CryptoConfig) (KeyStore, error) {
-	// Check if config is nil first - otherwise it will deserialize to an
-	// empty struct.
-	if config.KeyStore.Config == nil {
-		return nil, errors.New("keystore config is missing")
-	}
-
 	// TODO: support other methods in future
 	if config.KeyStore.Type != LocalKeyStore {
 		return nil, fmt.Errorf("unexpected keystore type: %s", config.KeyStore.Type)
 	}
 
-	var keystoreCfg serverconfig.LocalFileKeyStoreConfig
-	err := mapstructure.Decode(config.KeyStore.Config, &keystoreCfg)
-	if err != nil {
-		return nil, fmt.Errorf("unable to read keystore config: %w", err)
+	if config.KeyStore.Local.KeyDir == "" {
+		return nil, errors.New("key directory not defined in keystore config")
 	}
 
 	// Join the default key to the fallback keys to assemble the full
 	// set of keys to load.
-	keyIDs := append([]string{config.Default.KeyID}, config.Fallback.KeyIDs...)
+	keyIDs := []string{config.Default.KeyID}
+	if config.Fallback.KeyID != "" {
+		keyIDs = append(keyIDs, config.Fallback.KeyID)
+	}
 	keys := make(keysByID, len(keyIDs))
 	for _, keyID := range keyIDs {
-		key, err := readKey(keystoreCfg.KeyDir, keyID)
+		key, err := readKey(config.KeyStore.Local.KeyDir, keyID)
 		if err != nil {
 			return nil, fmt.Errorf("unable to read key %s: %w", keyID, err)
 		}

internal/crypto/keystores/keystore_test.go

@@ -31,21 +31,19 @@ func TestNewKeyStoreFromConfig(t *testing.T) {
 		ExpectedError string
 	}{
 		{
-			Name: "NewKeyStoreFromConfig rejects missing keystore config",
+			Name: "NewKeyStoreFromConfig rejects empty keystore config",
 			Config: server.CryptoConfig{
 				KeyStore: server.KeyStoreConfig{
-					Type:   keystores.LocalKeyStore,
-					Config: nil,
+					Type: keystores.LocalKeyStore,
 				},
 			},
-			ExpectedError: "keystore config is missing",
+			ExpectedError: "key directory not defined in keystore config",
 		},
 		{
 			Name: "NewKeyStoreFromConfig rejects invalid keystore type",
 			Config: server.CryptoConfig{
 				KeyStore: server.KeyStoreConfig{
-					Type:   "derp",
-					Config: map[string]any{},
+					Type: "derp",
 				},
 			},
 			ExpectedError: "unexpected keystore type",
@@ -55,8 +53,8 @@ func TestNewKeyStoreFromConfig(t *testing.T) {
 			Config: server.CryptoConfig{
 				KeyStore: server.KeyStoreConfig{
 					Type: keystores.LocalKeyStore,
-					Config: map[string]any{
-						"key_dir": "../testdata",
+					Local: server.LocalKeyStoreConfig{
+						KeyDir: "../testdata",
 					},
 				},
 				Default: server.DefaultCrypto{
@@ -70,8 +68,8 @@ func TestNewKeyStoreFromConfig(t *testing.T) {
 			Config: server.CryptoConfig{
 				KeyStore: server.KeyStoreConfig{
 					Type: keystores.LocalKeyStore,
-					Config: map[string]any{
-						"key_dir": "../testdata",
+					Local: server.LocalKeyStoreConfig{
+						KeyDir: "../testdata",
 					},
 				},
 				Default: server.DefaultCrypto{

internal/crypto/testdata/test_encryption_key2

@@ -0,0 +1 @@
+AHB2hS8LR+PFzlwJyX0bSXIWWXF7ycxlHQz8twlbqYU=