Take the master key from KEEPORG_MASTER_KEY_FILE

Why: the usual way to hand a secret to a container is to mount it as a file
and name the path in the environment. Compose, swarm and Kubernetes all do
it that way, and it keeps the key out of the process environment and off any
disk the container can write.

What: the master key is now looked for in KEEPORG_MASTER_KEY, then
KEEPORG_MASTER_KEY_FILE, then the configured master_key_file. A secret file
may hold base64, with or without padding or a trailing newline, or exactly
32 raw bytes, because secret stores produce both.

Only the configured path is ever created, which is what keeps a plain
package install working with no setup. The two environment variables name a
key something else owns, so a missing one is reported and vault creation
refused. Minting a replacement there would be worse than failing: it opens
none of the existing vaults, and the mistake would otherwise surface one
failed merge at a time.

The compose example now provisions the key as a real secret rather than an
environment variable, and the Dockerfile and README show the same. Neither
names a version any more, so they stop drifting as the server is bumped.

Proved: unit tests for every source and precedence, all four accepted
encodings, and that a missing secret writes nothing anywhere. In real
containers: a podman secret carrying base64 survives a stop and start with
"1 vault keys available for merging" and merges a stale push afterwards; a
secret holding 32 raw bytes creates a vault; and a secret that failed to
mount is named in the startup log, with vault creation answering with the
three places a key may come from.

internal/secret/keyring.go

@@ -15,37 +15,73 @@ import (
 // a vault: it can decrypt a vault, so it holds that vault's key.
 //
 // The wrapped secrets live in the server's state document; the key that
-// unwraps them does not. It comes from the environment or from a file
-// outside the data directory, so a copy of the data directory alone yields
-// ciphertext. Whoever holds both holds the vaults.
+// unwraps them does not. It comes from the environment, from a secret file
+// an orchestrator provisioned, or from a file outside the data directory,
+// so a copy of the data directory alone yields ciphertext. Whoever holds
+// both holds the vaults.
 type Keyring struct {
 	aead cipher.AEAD
 }
 
-// MasterKeyEnv names the environment variable holding a base64 master key,
-// which takes precedence over the key file.
-const MasterKeyEnv = "KEEPORG_MASTER_KEY"
+// Environment variables that supply the master key. Both are read before
+// any configured path.
+const (
+	// MasterKeyEnv holds the base64 key itself.
+	MasterKeyEnv = "KEEPORG_MASTER_KEY"
+	// MasterKeyFileEnv holds the path to a file containing it, which is how
+	// a container runtime or an orchestrator hands over a provisioned
+	// secret without putting it in the process environment.
+	MasterKeyFileEnv = "KEEPORG_MASTER_KEY_FILE"
+)
 
-// LoadKeyring reads the master key from the environment or from path,
-// creating one at path if neither exists.
+// keySize is the master key length in bytes.
+const keySize = 32
+
+// LoadKeyring finds the master key, in this order:
+//
+//  1. $KEEPORG_MASTER_KEY, the base64 key itself.
+//  2. $KEEPORG_MASTER_KEY_FILE, a path to a provisioned secret. It must
+//     already exist: something else owns that file, so a missing one is a
+//     misconfiguration to report, not a reason to mint a new key that would
+//     fail to open any existing vault.
+//  3. path, from -master-key-file or master_key_file in the configuration.
+//     This one is created on first run if it is absent, which is what makes
+//     a plain package install work with no setup.
+//
+// A file may hold the key as base64 text, with or without a trailing
+// newline, or as exactly 32 raw bytes, because secret stores produce both.
 func LoadKeyring(path string) (*Keyring, error) {
 	if v := strings.TrimSpace(os.Getenv(MasterKeyEnv)); v != "" {
-		key, err := base64.StdEncoding.DecodeString(v)
+		key, err := decodeKey([]byte(v))
 		if err != nil {
-			return nil, fmt.Errorf("secret: %s is not valid base64: %w", MasterKeyEnv, err)
+			return nil, fmt.Errorf("secret: $%s: %w", MasterKeyEnv, err)
 		}
 		return NewKeyring(key)
 	}
+
+	if p := strings.TrimSpace(os.Getenv(MasterKeyFileEnv)); p != "" {
+		b, err := os.ReadFile(p)
+		if err != nil {
+			return nil, fmt.Errorf("secret: $%s points at %s: %w",
+				MasterKeyFileEnv, p, err)
+		}
+		key, err := decodeKey(b)
+		if err != nil {
+			return nil, fmt.Errorf("secret: $%s file %s: %w", MasterKeyFileEnv, p, err)
+		}
+		return NewKeyring(key)
+	}
+
 	if path == "" {
-		return nil, errors.New("secret: no master key: set " + MasterKeyEnv +
-			" or configure master_key_file")
+		return nil, errors.New("secret: no master key: set $" + MasterKeyEnv +
+			", $" + MasterKeyFileEnv + ", or configure master_key_file")
 	}
 	b, err := os.ReadFile(path)
 	switch {
 	case err == nil:
-		key, derr := base64.StdEncoding.DecodeString(strings.TrimSpace(string(b)))
+		key, derr := decodeKey(b)
 		if derr != nil {
-			return nil, fmt.Errorf("secret: master key file %s is not valid base64: %w", path, derr)
+			return nil, fmt.Errorf("secret: master key file %s: %w", path, derr)
 		}
 		return NewKeyring(key)
 	case !os.IsNotExist(err):
@@ -53,7 +89,7 @@ func LoadKeyring(path string) (*Keyring, error) {
 	}
 	// First run: mint one. It is written with owner-only permissions and is
 	// the one file an operator must back up separately from the data.
-	key := randBytes(32)
+	key := randBytes(keySize)
 	if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil {
 		return nil, err
 	}
@@ -64,10 +100,28 @@ func LoadKeyring(path string) (*Keyring, error) {
 	return NewKeyring(key)
 }
 
+// decodeKey accepts the shapes a provisioned secret arrives in: exactly 32
+// raw bytes, or base64 text with or without padding and trailing newline.
+func decodeKey(b []byte) ([]byte, error) {
+	if len(b) == keySize {
+		return b, nil
+	}
+	text := strings.TrimSpace(string(b))
+	if key, err := base64.StdEncoding.DecodeString(text); err == nil {
+		return key, nil
+	}
+	if key, err := base64.RawStdEncoding.DecodeString(text); err == nil {
+		return key, nil
+	}
+	return nil, fmt.Errorf("expected %d raw bytes or a base64 key, got %d bytes "+
+		"that are neither", keySize, len(b))
+}
+
 // NewKeyring builds a keyring from a 32-byte key.
 func NewKeyring(key []byte) (*Keyring, error) {
-	if len(key) != 32 {
-		return nil, fmt.Errorf("secret: master key must be 32 bytes, got %d", len(key))
+	if len(key) != keySize {
+		return nil, fmt.Errorf("secret: master key must be %d bytes, got %d",
+			keySize, len(key))
 	}
 	block, err := aes.NewCipher(key)
 	if err != nil {

internal/secret/keyring_test.go

@@ -4,6 +4,7 @@ import (
 	"encoding/base64"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 )
 
@@ -90,3 +91,140 @@ func TestLoadKeyringRejectsAShortKey(t *testing.T) {
 		t.Fatal("a short master key was accepted")
 	}
 }
+
+// wrapWith is a helper: build a keyring from a known key and wrap a secret
+// with it, so another keyring can be checked for being the same key.
+func wrapWith(t *testing.T, key []byte) string {
+	t.Helper()
+	k, err := NewKeyring(key)
+	if err != nil {
+		t.Fatal(err)
+	}
+	wrapped, err := k.Wrap("v", "secret")
+	if err != nil {
+		t.Fatal(err)
+	}
+	return wrapped
+}
+
+// sameKey reports whether k is the keyring that produced wrapped.
+func sameKey(t *testing.T, k *Keyring, wrapped string) bool {
+	t.Helper()
+	got, err := k.Unwrap("v", wrapped)
+	return err == nil && got == "secret"
+}
+
+func TestMasterKeyFileEnvReadsAProvisionedSecret(t *testing.T) {
+	key := randBytes(32)
+	wrapped := wrapWith(t, key)
+
+	for _, tc := range []struct {
+		name    string
+		content []byte
+	}{
+		{"base64", []byte(base64.StdEncoding.EncodeToString(key))},
+		{"base64 with a trailing newline", []byte(base64.StdEncoding.EncodeToString(key) + "\n")},
+		{"base64 without padding", []byte(base64.RawStdEncoding.EncodeToString(key))},
+		{"32 raw bytes", key},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			path := filepath.Join(t.TempDir(), "secret")
+			if err := os.WriteFile(path, tc.content, 0o400); err != nil {
+				t.Fatal(err)
+			}
+			t.Setenv(MasterKeyEnv, "")
+			t.Setenv(MasterKeyFileEnv, path)
+			k, err := LoadKeyring("")
+			if err != nil {
+				t.Fatal(err)
+			}
+			if !sameKey(t, k, wrapped) {
+				t.Fatal("the key read from the secret file is not the one written")
+			}
+		})
+	}
+}
+
+// A secret that was supposed to be mounted and is not must be reported. A
+// new key minted in its place would open none of the existing vaults.
+func TestMissingSecretFileIsReportedAndMintsNothing(t *testing.T) {
+	dir := t.TempDir()
+	missing := filepath.Join(dir, "not-mounted")
+	configured := filepath.Join(dir, "master.key")
+	t.Setenv(MasterKeyEnv, "")
+	t.Setenv(MasterKeyFileEnv, missing)
+
+	_, err := LoadKeyring(configured)
+	if err == nil {
+		t.Fatal("a missing secret file was accepted")
+	}
+	if !strings.Contains(err.Error(), MasterKeyFileEnv) {
+		t.Fatalf("the error does not name the variable: %v", err)
+	}
+	if _, err := os.Stat(configured); !os.IsNotExist(err) {
+		t.Fatal("a replacement key was minted at the configured path")
+	}
+	if _, err := os.Stat(missing); !os.IsNotExist(err) {
+		t.Fatal("a key was written where the secret should have been mounted")
+	}
+}
+
+func TestMasterKeyEnvWinsOverTheSecretFile(t *testing.T) {
+	envKey, fileKey := randBytes(32), randBytes(32)
+	wrappedByEnv := wrapWith(t, envKey)
+
+	path := filepath.Join(t.TempDir(), "secret")
+	if err := os.WriteFile(path, fileKey, 0o400); err != nil {
+		t.Fatal(err)
+	}
+	t.Setenv(MasterKeyEnv, base64.StdEncoding.EncodeToString(envKey))
+	t.Setenv(MasterKeyFileEnv, path)
+
+	k, err := LoadKeyring("")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !sameKey(t, k, wrappedByEnv) {
+		t.Fatal("the secret file took precedence over the environment key")
+	}
+}
+
+func TestSecretFileWinsOverTheConfiguredPath(t *testing.T) {
+	secretKey := randBytes(32)
+	wrapped := wrapWith(t, secretKey)
+	dir := t.TempDir()
+
+	secretPath := filepath.Join(dir, "provisioned")
+	if err := os.WriteFile(secretPath, secretKey, 0o400); err != nil {
+		t.Fatal(err)
+	}
+	configured := filepath.Join(dir, "master.key")
+	if err := os.WriteFile(configured,
+		[]byte(base64.StdEncoding.EncodeToString(randBytes(32))), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	t.Setenv(MasterKeyEnv, "")
+	t.Setenv(MasterKeyFileEnv, secretPath)
+
+	k, err := LoadKeyring(configured)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !sameKey(t, k, wrapped) {
+		t.Fatal("the configured path took precedence over the provisioned secret")
+	}
+}
+
+func TestSecretFileContentIsValidated(t *testing.T) {
+	for _, content := range []string{"", "not base64 at all !!", "c2hvcnQ="} {
+		path := filepath.Join(t.TempDir(), "secret")
+		if err := os.WriteFile(path, []byte(content), 0o400); err != nil {
+			t.Fatal(err)
+		}
+		t.Setenv(MasterKeyEnv, "")
+		t.Setenv(MasterKeyFileEnv, path)
+		if _, err := LoadKeyring(""); err == nil {
+			t.Fatalf("content %q was accepted as a master key", content)
+		}
+	}
+}

internal/server/server.go

@@ -132,9 +132,10 @@ func (s *Server) checkVaultKeys() {
 	case broken > 0:
 		s.log.Printf("WARNING: %d of %d stored vault keys do not open with this "+
 			"master key. Those vaults cannot be merged until the original key is "+
-			"restored (%s or %s). Nothing is lost: clients still open the vaults "+
-			"with their own passwords.",
-			broken, total, secret.MasterKeyEnv, s.cfg.MasterKeyFile)
+			"restored ($%s, $%s, or %s). Nothing is lost: clients still open the "+
+			"vaults with their own passwords.",
+			broken, total, secret.MasterKeyEnv, secret.MasterKeyFileEnv,
+			s.cfg.MasterKeyFile)
 	}
 }
 

internal/server/vault.go

@@ -18,8 +18,9 @@ import (
 const MinVaultPasswordLen = 12
 
 var errNoKeyring = errors.New(
-	"this server has no master key, so it cannot hold vault keys: set " +
-		secret.MasterKeyEnv + " or master_key_file and restart")
+	"this server has no master key, so it cannot hold vault keys: set $" +
+		secret.MasterKeyEnv + ", $" + secret.MasterKeyFileEnv +
+		", or master_key_file, and restart")
 
 // createVault makes a new, empty KDBX database, encrypts it with the given
 // vault password and stores it as version 1. When the strategy needs the